Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 39 additions & 36 deletions src/pipeline/artifact.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,10 @@ static int write_file_atomic(const char *path, const char *data, size_t len,
return CBM_NOT_FOUND;
}

FILE *fp = fopen(tmp, "wb");
/* cbm_fopen / cbm_rename_replace: wide paths on Windows. The ANSI CRT fopen
* and MoveFileExA mangled a repo path outside the active code page (a CJK
* checkout), so every persistent export failed there (#1171). */
FILE *fp = cbm_fopen(tmp, "wb");
if (!fp) {
file_error_set(out_err, "open_temp", errno);
return CBM_NOT_FOUND;
Expand All @@ -207,22 +210,14 @@ static int write_file_atomic(const char *path, const char *data, size_t len,
return CBM_NOT_FOUND;
}

#ifdef _WIN32
/* MoveFileEx replace approach suggested by @Ayush7Ranjan in #492. */
if (!MoveFileExA(tmp, path, MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH)) {
DWORD saved_error = GetLastError();
cbm_unlink(tmp);
file_error_set(out_err, "rename_temp", (int)saved_error);
return CBM_NOT_FOUND;
}
#else
if (rename(tmp, path) != 0) {
/* Replace-existing rename (MoveFileExW write-through on Windows, the
* approach suggested by @Ayush7Ranjan in #492); errno carries the cause. */
if (cbm_rename_replace(tmp, path) != 0) {
int saved_errno = errno;
cbm_unlink(tmp);
file_error_set(out_err, "rename_temp", saved_errno);
return CBM_NOT_FOUND;
}
#endif
return 0;
}

Expand Down Expand Up @@ -777,30 +772,31 @@ static void ensure_gitattributes(const char *repo_path) {
char ga_path[CBM_SZ_4K];
artifact_path(ga_path, sizeof(ga_path), repo_path, ".gitattributes");

/* Atomic create-only-if-absent: O_EXCL closes the TOCTOU window
* between checking existence and writing. If the file exists, open
* fails with EEXIST and we leave it untouched. */
int fd = open(ga_path, O_WRONLY | O_CREAT | O_EXCL, 0644);
if (fd < 0) {
if (errno != EEXIST) {
cbm_log_warn("artifact.gitattributes.open path=%s err=%s", ga_path, strerror(errno));
/* Atomic create-only-if-absent: the "x" mode (O_CREAT|O_EXCL) closes the
* TOCTOU window between checking existence and writing. If the file
* exists, the open fails with EEXIST and we leave it untouched.
* cbm_fopen, not open(): the ANSI CRT open() cannot create the file under
* a repo path outside the active code page on Windows, which silently
* dropped the merge=ours protection there (#1171). */
errno = 0;
FILE *fp = cbm_fopen(ga_path, "wbx");
if (!fp) {
int open_errno = errno;
if (open_errno != EEXIST) {
cbm_log_warn("artifact.gitattributes.open", "path", ga_path, "err",
strerror(open_errno));
}
/* fall through to merge driver setup either way */
} else {
FILE *fp = fdopen(fd, "w");
if (fp) {
/* Order matters: attributes apply left to right and the `binary`
* macro expands to `-diff -merge -text`, so a trailing `binary`
* unsets `merge=ours` and the conflict prevention this file
* exists for never engages. The macro must come first. */
(void)fputs("# Auto-generated by codebase-memory-mcp\n"
"# Prevent merge conflicts on compressed artifact\n" CBM_ARTIFACT_FILENAME
" binary merge=ours\n",
fp);
(void)fclose(fp);
} else {
(void)close(fd);
}
/* Order matters: attributes apply left to right and the `binary`
* macro expands to `-diff -merge -text`, so a trailing `binary`
* unsets `merge=ours` and the conflict prevention this file
* exists for never engages. The macro must come first. */
(void)fputs("# Auto-generated by codebase-memory-mcp\n"
"# Prevent merge conflicts on compressed artifact\n" CBM_ARTIFACT_FILENAME
" binary merge=ours\n",
fp);
(void)fclose(fp);
}

/* Best-effort: configure merge driver */
Expand Down Expand Up @@ -1188,7 +1184,7 @@ int cbm_artifact_import(const char *repo_path, const char *cache_db_path) {
* stale WAL next to the cache path would be replayed on top of the
* imported file at the next open (#897). */
cbm_remove_db_sidecars(cache_db_path);
if (rename(tmp_path, cache_db_path) != 0) {
if (cbm_rename_replace(tmp_path, cache_db_path) != 0) {
cbm_log_error("artifact.import", "err", "rename_to_cache");
cbm_unlink(tmp_path);
return CBM_NOT_FOUND;
Expand Down Expand Up @@ -1218,8 +1214,15 @@ bool cbm_artifact_exists(const char *repo_path) {
char zst_path[CBM_SZ_4K];
artifact_path(zst_path, sizeof(zst_path), repo_path, CBM_ARTIFACT_FILENAME);

struct stat st;
if (stat(zst_path, &st) != 0 || st.st_size == 0) {
/* Present and non-empty. cbm_fopen, not stat(): the ANSI CRT stat misses a
* repo path outside the active Windows code page (#1171). */
FILE *fp = cbm_fopen(zst_path, "rb");
if (!fp) {
return false;
}
int first = fgetc(fp);
(void)fclose(fp);
if (first == EOF) {
return false;
}

Expand Down
77 changes: 77 additions & 0 deletions tests/test_artifact.c
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,82 @@ TEST(artifact_gitattributes_created) {
PASS();
}

/* Read a small file through the UTF-8 path layer (wide on Windows), so a
* non-ASCII path is never misread by the test's own ANSI CRT call. Returns the
* byte count, or 0 when the file cannot be opened or is empty. */
static size_t read_small_file_utf8(const char *path, char *buf, size_t cap) {
buf[0] = '\0';
FILE *fp = cbm_fopen(path, "rb");
if (!fp) {
return 0;
}
size_t rd = fread(buf, 1, cap - 1, fp);
(void)fclose(fp);
buf[rd] = '\0';
return rd;
}

/* #1171: a repository (and a cache directory) under a non-ASCII path must
* export, report the artifact present, and import — also onto an existing
* cache db. On Windows the atomic writer used the ANSI CRT fopen + MoveFileExA
* and the existence check the ANSI stat, so CJK characters outside the active
* code page made every step fail. The same export also writes the
* .gitattributes merge=ours protection, which ensure_gitattributes created
* with the ANSI CRT open(): on Windows it was silently missing. The bytes are
* escaped UTF-8 for a CJK repo directory name and a CJK cache directory name
* (kept ASCII in source). */
#define ART_CJK_REPO "\xe6\x8c\x81\xe4\xb9\x85\xe5\x8c\x96\xe5\xa4\x8d\xe7\x8e\xb0"
#define ART_CJK_CACHE "\xe5\xaf\xbc\xe5\x85\xa5"
TEST(artifact_roundtrip_non_ascii_paths) {
setup_artifact_test();
create_test_db(g_db);

char repo[1024];
snprintf(repo, sizeof(repo), "%s/cbm-" ART_CJK_REPO, g_tmpdir);
ASSERT_TRUE(cbm_mkdir_p(repo, 0755));

ASSERT_EQ(cbm_artifact_export(g_db, repo, "test-proj", CBM_ARTIFACT_FAST), 0);
char zst[1024];
snprintf(zst, sizeof(zst), "%s/.codebase-memory/graph.db.zst", repo);
ASSERT_TRUE(cbm_file_exists(zst));
ASSERT_TRUE(cbm_artifact_exists(repo));

char ga[1024];
snprintf(ga, sizeof(ga), "%s/.codebase-memory/.gitattributes", repo);
char ga_content[512];
ASSERT_TRUE(read_small_file_utf8(ga, ga_content, sizeof(ga_content)) > 0);
ASSERT_NOT_NULL(strstr(ga_content, CBM_ARTIFACT_FILENAME " binary merge=ours"));

/* Create-only-if-absent: a re-export never rewrites an existing
* .gitattributes the user may have edited. */
static const char user_ga[] = "# user-owned\n";
FILE *uga = cbm_fopen(ga, "wb");
ASSERT_NOT_NULL(uga);
ASSERT_TRUE(fputs(user_ga, uga) >= 0);
ASSERT_EQ(fclose(uga), 0);
ASSERT_EQ(cbm_artifact_export(g_db, repo, "test-proj", CBM_ARTIFACT_FAST), 0);
ASSERT_EQ(read_small_file_utf8(ga, ga_content, sizeof(ga_content)), strlen(user_ga));
ASSERT_STR_EQ(ga_content, user_ga);

char cache_dir[1024];
snprintf(cache_dir, sizeof(cache_dir), "%s/" ART_CJK_CACHE, g_tmpdir);
ASSERT_TRUE(cbm_mkdir_p(cache_dir, 0755));
char import_db[1024];
snprintf(import_db, sizeof(import_db), "%s/imported.db", cache_dir);
ASSERT_EQ(cbm_artifact_import(repo, import_db), 0);
/* Re-import replaces the existing cache db (the second-session path). */
ASSERT_EQ(cbm_artifact_import(repo, import_db), 0);

cbm_store_t *s = cbm_store_open_path(import_db);
ASSERT_NOT_NULL(s);
ASSERT_EQ(cbm_store_count_nodes(s, "test-proj"), 2);
ASSERT_EQ(cbm_store_count_edges(s, "test-proj"), 1);
cbm_store_close(s);

cleanup_dir(g_tmpdir);
PASS();
}

TEST(artifact_export_rename_failure_logs_specific_error) {
setup_artifact_test();
create_test_db(g_db);
Expand Down Expand Up @@ -1102,6 +1178,7 @@ SUITE(artifact) {
RUN_TEST(artifact_schema_version_mismatch);
RUN_TEST(artifact_import_missing);
RUN_TEST(artifact_gitattributes_created);
RUN_TEST(artifact_roundtrip_non_ascii_paths);
RUN_TEST(artifact_export_rename_failure_logs_specific_error);
RUN_TEST(pipeline_persistence_export_failure_returns_error);
RUN_TEST(artifact_import_rejects_size_mismatch);
Expand Down
Loading