From 7658954140b98c1c3e56e9429c3613dccc3afe04 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 26 Sep 2026 02:26:33 +0200 Subject: [PATCH 1/2] fix(artifact): write and detect artifacts through wide paths on Windows (#1171) With persistence=true, indexing a repository under a CJK path on Windows made the worker exit nonzero and no .codebase-memory/graph.db.zst was written, while an ASCII junction to the same directory worked. Root cause: write_file_atomic() -- used for artifact.json, graph.db.zst and the import temp db -- opened its temp file with the raw CRT fopen() and published it with MoveFileExA(). Both go through the ANSI code page, so a repo path containing characters outside it cannot be opened. The same file had two more ANSI calls on the same flow: cbm_artifact_exists() used stat() on the repo path, and the import published the cache db with CRT rename(), which is ANSI and also fails when the destination exists. Fix: use the repo's UTF-8 helpers -- cbm_fopen() for the temp file, cbm_rename_replace() (MoveFileExW, write-through, replace-existing) for both publishes, and a cbm_fopen() probe for the non-empty existence check. rename_temp now reports errno on every platform (cbm_rename_replace translates the Win32 error). Test: artifact::artifact_roundtrip_non_ascii_paths exports from a CJK repo directory, checks cbm_artifact_exists(), and imports twice into a CJK cache directory (the second import replaces the existing db). Signed-off-by: Martin Vogel --- src/pipeline/artifact.c | 32 ++++++++++++++++--------------- tests/test_artifact.c | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 2dd368c28c..9594c8a09b 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -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; @@ -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; } @@ -1188,7 +1183,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; @@ -1218,8 +1213,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; } diff --git a/tests/test_artifact.c b/tests/test_artifact.c index a7d96cdd95..6206463e36 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -334,6 +334,47 @@ TEST(artifact_gitattributes_created) { PASS(); } +/* #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 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 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); @@ -1102,6 +1143,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); From 402dd7c59b7287416361edac782fd642ac027cc6 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 1 Oct 2026 19:59:02 +0200 Subject: [PATCH 2/2] fix(artifact): create .gitattributes through the UTF-8 path layer Follow-up to the #1171 fix. Exporting an artifact from a repository under a CJK path on Windows wrote graph.db.zst and artifact.json, but the .codebase-memory/.gitattributes file carrying the merge=ours protection was silently missing. Root cause: ensure_gitattributes() created the file with the raw POSIX open(O_WRONLY | O_CREAT | O_EXCL). On Windows that is the ANSI CRT, so a repo path with characters outside the active code page fails with ENOENT. The failure was only logged, and the warning itself was malformed: it passed printf-style arguments to the key/value logger, which printed the literal "msg=artifact.gitattributes.open_path=%s_err=%s =". Fix: create the file with cbm_fopen(path, "wbx") (_wfopen on Windows). The "x" mode keeps the O_CREAT | O_EXCL create-only-if-absent semantics, so an existing (possibly user-edited) .gitattributes is never rewritten. Binary mode writes the same LF bytes on every platform. The warning now uses the logger's key/value form (path=..., err=...). Test: artifact::artifact_roundtrip_non_ascii_paths now reads .gitattributes back from the CJK repo through cbm_fopen and asserts the merge=ours line. It also replaces the file with user content, exports again, and asserts the content is unchanged. On the Windows VM the assertion failed 3 of 3 without the fix and passes 3 of 3 with it. The no-overwrite check fails if the mode loses its "x". Refs #1171 Signed-off-by: Martin Vogel --- src/pipeline/artifact.c | 43 +++++++++++++++++++++-------------------- tests/test_artifact.c | 39 +++++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 9594c8a09b..b1a1ac6c70 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -772,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 */ diff --git a/tests/test_artifact.c b/tests/test_artifact.c index 6206463e36..8bbec658d5 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -334,12 +334,30 @@ 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 bytes are escaped UTF-8 for a CJK repo - * directory name and a CJK cache directory name (kept ASCII in source). */ + * 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) { @@ -356,6 +374,23 @@ TEST(artifact_roundtrip_non_ascii_paths) { 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));