From 4b2b3b90adc44694d70624c9d0372752ffd0e2bf Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 25 Sep 2026 19:14:13 +0200 Subject: [PATCH 1/4] fix(mcp): reuse the project that owns a root on unnamed re-index (#2134) index_repository without `name` derived the project key from the path alone. When the root had first been indexed under an explicit name, a later unnamed re-index forked a second index (same root_path, path-derived name) and left the original untouched, so the stale index kept being served while the call reported success. Resolve the key from the indexes on disk before any work starts: - the path-derived project exists -> keep it (unchanged behavior); - exactly one other project owns the canonical root -> adopt its name and update it in place, so existing indexes are never orphaned; - several projects own the root -> fail loudly, name them and ask for `name` instead of guessing or forking again; - none -> first index under the derived name, as before. The resolved name is written into the args handed to the daemon job coordinator and the supervised worker, so the project lease key and the index actually written stay the same project. Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 147 +++++++++++++++++++++++++++++++++++++++++++++-- tests/test_mcp.c | 105 +++++++++++++++++++++++++++++++++ 2 files changed, 248 insertions(+), 4 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 75ff8e396c..17db27b116 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11015,8 +11015,12 @@ static bool resolve_session_repo_path(cbm_mcp_server_t *srv, char **repo_path) { } /* Preserve every index option while replacing all caller-supplied repo_path - * keys with the one canonical path that was actually authorized. */ + * keys with the one canonical path that was actually authorized. A non-empty + * project_name likewise replaces any caller "name" (#2134: the project key the + * parent resolved for this root travels to the daemon job / worker verbatim, + * so lease key and written index cannot diverge). */ static char *index_args_with_repo_path(const char *args, const char *canonical_repo_path, + const char *project_name, const cbm_index_resource_policy_t *policy) { if (!args || !canonical_repo_path || !policy) { return NULL; @@ -11041,7 +11045,12 @@ static char *index_args_with_repo_path(const char *args, const char *canonical_r while (yyjson_mut_obj_get(copy_root, "_cbm_index_policy")) { (void)yyjson_mut_obj_remove_key(copy_root, "_cbm_index_policy"); } - if (!yyjson_mut_obj_add_strcpy(copy, copy_root, "repo_path", canonical_repo_path) || + bool has_name = project_name && project_name[0]; + while (has_name && yyjson_mut_obj_get(copy_root, "name")) { + (void)yyjson_mut_obj_remove_key(copy_root, "name"); + } + if ((has_name && !yyjson_mut_obj_add_strcpy(copy, copy_root, "name", project_name)) || + !yyjson_mut_obj_add_strcpy(copy, copy_root, "repo_path", canonical_repo_path) || !cbm_mcp_index_policy_add_to_args(copy, copy_root, policy)) { yyjson_mut_doc_free(copy); return NULL; @@ -11094,6 +11103,117 @@ static bool project_db_is_servable(const char *project, const char *db_path) { return servable; } +/* #2134: an index_repository call without `name` derived the project key from + * the path alone, so re-indexing a root that was first indexed under an + * explicit name silently forked a second index (same root_path, path-derived + * name) and left the original stale while it kept being served. Resolve the + * key from the indexes already on disk instead: + * - the path-derived project exists -> keep it (unchanged path); + * - exactly one other project owns this root -> adopt its name (update it); + * - several other projects own this root -> ambiguous: fail loudly, + * naming them, never guess; + * - none -> first index, derived name. + * Roots compare as stored: both sides are the canonical repo path. Returns + * false only for the ambiguous / allocation-failure case, with *error_out set + * to a heap message; *owner_out is a heap name or NULL. */ +static bool index_root_owner_append(char **list, size_t *len, size_t *cap, const char *name) { + size_t need = *len + strlen(name) + 3; + if (need > *cap) { + size_t grown_cap = need * 2; + char *grown = realloc(*list, grown_cap); + if (!grown) { + return false; + } + *list = grown; + *cap = grown_cap; + } + *len += (size_t)snprintf(*list + *len, *cap - *len, "%s%s", *len ? ", " : "", name); + return true; +} + +static bool index_root_owner_resolve(const char *repo_path, char **owner_out, char **error_out) { + *owner_out = NULL; + *error_out = NULL; + char *derived = cbm_project_name_from_path(repo_path); + if (!derived) { + return true; /* the caller reports the derivation failure itself */ + } + char derived_db[CBM_SZ_1K]; + project_db_path(derived, derived_db, sizeof(derived_db)); + if (derived_db[0] && cbm_file_exists(derived_db)) { + free(derived); + return true; /* fast path: the path-derived project is already on disk */ + } + char dir_path[CBM_SZ_1K]; + cache_dir(dir_path, sizeof(dir_path)); + cbm_dir_t *d = cbm_opendir(dir_path); + if (!d) { + free(derived); + return true; + } + char *owner = NULL; + char *owners = NULL; + size_t owners_len = 0; + size_t owners_cap = 0; + int owner_count = 0; + bool derived_exists = false; + bool ok = true; + cbm_dirent_t *entry; + while (ok && !derived_exists && (entry = cbm_readdir(d)) != NULL) { + size_t len = strlen(entry->name); + if (!is_project_db_file(entry->name, len)) { + continue; + } + mcp_project_record_t record = {0}; + project_record_status_t status = + read_project_record_identity(dir_path, entry->name, 0, &record); + if (status == PROJECT_RECORD_OOM) { + ok = false; + break; + } + if (status != PROJECT_RECORD_OK) { + continue; + } + if (strcmp(record.name, derived) == 0) { + derived_exists = true; + } else if (strcmp(record.root_path, repo_path) == 0) { + owner_count++; + ok = index_root_owner_append(&owners, &owners_len, &owners_cap, record.name); + if (owner_count == 1 && ok) { + owner = heap_strdup(record.name); + ok = owner != NULL; + } + } + project_record_clear(&record); + } + cbm_closedir(d); + free(derived); + + if (ok && !derived_exists && owner_count == 1) { + free(owners); + *owner_out = owner; + return true; + } + free(owner); + if (ok && (derived_exists || owner_count == 0)) { + free(owners); + return true; + } + char msg[CBM_SZ_4K]; + if (ok) { + snprintf(msg, sizeof(msg), + "several indexed projects share root_path %s: %s. Pass name= to " + "choose which one to re-index (or delete_project the stale ones).", + repo_path, owners); + } else { + snprintf(msg, sizeof(msg), "out of memory while resolving the project for root_path %s", + repo_path); + } + free(owners); + *error_out = heap_strdup(msg); + return false; +} + /* The three heap strings handle_index_repository owns from * cbm_mcp_get_string_arg / resolved_repo_path_from_project_arg. One release * point keeps the dozen early-return paths in step; free(NULL) is a no-op, so @@ -11145,6 +11265,23 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { return cbm_mcp_text_result(boundary_err, true); } + if ((!name_override || !name_override[0]) && repo_path && repo_path[0]) { + char *owner = NULL; + char *owner_error = NULL; + if (!index_root_owner_resolve(repo_path, &owner, &owner_error)) { + index_args_free(repo_path, mode_str, name_override); + char *result = cbm_mcp_text_result( + owner_error ? owner_error : "could not resolve index project name", true); + free(owner_error); + return result; + } + if (owner) { + cbm_log_info("index.root_owner_reused", "root", repo_path, "project", owner); + free(name_override); + name_override = owner; + } + } + if (mode_str && strcmp(mode_str, "cross-repo-intelligence") == 0) { char *result = handle_cross_repo_mode(srv, repo_path, name_override, args); index_args_free(repo_path, mode_str, name_override); @@ -11161,7 +11298,8 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { /* A daemon session delegates the one physical write to its shared job * registry only after path canonicalization and workspace authorization. */ if (srv->index_executor) { - char *worker_args = index_args_with_repo_path(args, repo_path, &resource_policy); + char *worker_args = + index_args_with_repo_path(args, repo_path, name_override, &resource_policy); char *coordinated = worker_args ? srv->index_executor(srv->index_executor_context, repo_path, worker_args) : NULL; @@ -11189,7 +11327,8 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { * installs the same guard before running the in-process pipeline. A marked * host fails closed if preparation or worker startup cannot complete. */ if (cbm_index_supervisor_should_wrap()) { - char *worker_args = index_args_with_repo_path(args, repo_path, &resource_policy); + char *worker_args = + index_args_with_repo_path(args, repo_path, name_override, &resource_policy); if (!worker_args) { free(mutation_project); index_args_free(repo_path, mode_str, name_override); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index ffa9ea3a3b..66329e2c24 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -8382,6 +8382,110 @@ TEST(tool_project_arg_resolves_unique_tail_issue1025) { PASS(); } +/* #2134: re-indexing a root WITHOUT `name` must update the project that + * already owns that root_path, not fork a second index under the + * path-derived name (both then list the same root_path and the stale one + * keeps being read as fresh). Several owners of one root are ambiguous: the + * call must fail loudly and name them instead of guessing or forking. */ +static int i2134_count_occurrences(const char *haystack, const char *needle) { + int count = 0; + for (const char *p = haystack ? strstr(haystack, needle) : NULL; p; p = strstr(p + 1, needle)) { + count++; + } + return count; +} + +TEST(tool_index_repository_reuses_existing_project_for_root_issue2134) { + char repo[CBM_SZ_256]; + char cache[CBM_SZ_256]; + snprintf(repo, sizeof(repo), "/tmp/cbm-i2134r-XXXXXX"); + snprintf(cache, sizeof(cache), "/tmp/cbm-i2134c-XXXXXX"); + if (!cbm_mkdtemp(repo) || !cbm_mkdtemp(cache)) { + FAIL("mkdtemp failed"); + } + char canonical_repo[CBM_SZ_1K]; + if (!realpath(repo, canonical_repo)) { + FAIL("realpath failed"); + } + const char *saved_cache = getenv("CBM_CACHE_DIR"); + char *saved_cache_copy = saved_cache ? cbm_strdup(saved_cache) : NULL; + const char *saved_sup = getenv("CBM_INDEX_SUPERVISOR"); + char *saved_sup_copy = saved_sup ? cbm_strdup(saved_sup) : NULL; + cbm_setenv("CBM_CACHE_DIR", cache, 1); + cbm_setenv("CBM_INDEX_SUPERVISOR", "0", 1); + i1025_write_repo(repo, "root_owner_2134"); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + + /* 1. Index once under an explicit name. */ + char args[CBM_SZ_1K]; + snprintf(args, sizeof(args), "{\"repo_path\":\"%s\",\"name\":\"named-2134\"}", repo); + char *r = cbm_mcp_handle_tool(srv, "index_repository", args); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "named-2134")); + free(r); + + /* 2. Re-index the same root without a name: must update named-2134. */ + snprintf(args, sizeof(args), "{\"repo_path\":\"%s\"}", repo); + r = cbm_mcp_handle_tool(srv, "index_repository", args); + ASSERT_NOT_NULL(r); + if (!strstr(r, "named-2134")) { + fprintf(stderr, " [2134] reindex without name forked: %.300s\n", r); + } + ASSERT_NOT_NULL(strstr(r, "named-2134")); + free(r); + + /* Every listed project carries its root_path once in structuredContent, + * so the canonical root occurring once there means exactly one owner. */ + r = cbm_mcp_handle_tool(srv, "list_projects", "{\"format\":\"json\"}"); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "\"structuredContent\"")); + int owners = i2134_count_occurrences(strstr(r, "\"structuredContent\""), canonical_repo); + if (owners != 1) { + fprintf(stderr, " [2134] %d projects share root %s: %.400s\n", owners, canonical_repo, r); + } + ASSERT_EQ(owners, 1); + free(r); + + /* 3. An explicit second name is the caller's choice; afterwards the root + * has two owners, so an unnamed re-index must refuse and list both. */ + snprintf(args, sizeof(args), "{\"repo_path\":\"%s\",\"name\":\"other-2134\"}", repo); + r = cbm_mcp_handle_tool(srv, "index_repository", args); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "other-2134")); + free(r); + snprintf(args, sizeof(args), "{\"repo_path\":\"%s\"}", repo); + r = cbm_mcp_handle_tool(srv, "index_repository", args); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "named-2134")); + ASSERT_NOT_NULL(strstr(r, "other-2134")); + ASSERT_NOT_NULL(strstr(r, "\"isError\":true")); + free(r); + r = cbm_mcp_handle_tool(srv, "list_projects", "{\"format\":\"json\"}"); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "\"structuredContent\"")); + ASSERT_EQ(i2134_count_occurrences(strstr(r, "\"structuredContent\""), canonical_repo), 2); + free(r); + + cbm_mcp_server_free(srv); + if (saved_cache_copy) { + cbm_setenv("CBM_CACHE_DIR", saved_cache_copy, 1); + free(saved_cache_copy); + } else { + cbm_unsetenv("CBM_CACHE_DIR"); + } + if (saved_sup_copy) { + cbm_setenv("CBM_INDEX_SUPERVISOR", saved_sup_copy, 1); + free(saved_sup_copy); + } else { + cbm_unsetenv("CBM_INDEX_SUPERVISOR"); + } + th_rmtree(repo); + th_rmtree(cache); + PASS(); +} + /* Regression for #604: path scopes architecture totals and content. */ TEST(tool_get_architecture_path_scoping) { cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); @@ -20597,6 +20701,7 @@ SUITE(mcp) { RUN_TEST(tool_get_architecture_accepts_project_name_alias_issue640); RUN_TEST(tool_search_graph_accepts_project_name_alias_issue640); RUN_TEST(tool_project_arg_resolves_unique_tail_issue1025); + RUN_TEST(tool_index_repository_reuses_existing_project_for_root_issue2134); RUN_TEST(tool_get_architecture_path_scoping); RUN_TEST(tool_query_graph_missing_query); From bac3fc80b532eb294fd6eda6a2cc93d656df9369 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 26 Sep 2026 01:07:11 +0200 Subject: [PATCH 2/4] chore(lint): route the #2134 root-owner scratch through the memory core The memory-core ratchet flagged src/mcp/mcp.c growing by 10 raw allocator sites (784 -> 794) from index_root_owner_resolve and its caller. The owner list buffer, which this code allocates and frees itself, now uses cbm_realloc/cbm_free (class other). Strings handed over by other APIs (cbm_project_name_from_path, heap_strdup, the index argument strings) are released through safe_free, as elsewhere in the file, so no memory the core never counted reaches cbm_free. Behaviour is unchanged; mcp.c is back at its baseline of 784. Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 17db27b116..29c08f3e2d 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11120,7 +11120,7 @@ static bool index_root_owner_append(char **list, size_t *len, size_t *cap, const size_t need = *len + strlen(name) + 3; if (need > *cap) { size_t grown_cap = need * 2; - char *grown = realloc(*list, grown_cap); + char *grown = cbm_realloc(CBM_MEM_CLASS_OTHER, *list, grown_cap); if (!grown) { return false; } @@ -11141,14 +11141,14 @@ static bool index_root_owner_resolve(const char *repo_path, char **owner_out, ch char derived_db[CBM_SZ_1K]; project_db_path(derived, derived_db, sizeof(derived_db)); if (derived_db[0] && cbm_file_exists(derived_db)) { - free(derived); + safe_free(derived); return true; /* fast path: the path-derived project is already on disk */ } char dir_path[CBM_SZ_1K]; cache_dir(dir_path, sizeof(dir_path)); cbm_dir_t *d = cbm_opendir(dir_path); if (!d) { - free(derived); + safe_free(derived); return true; } char *owner = NULL; @@ -11187,16 +11187,16 @@ static bool index_root_owner_resolve(const char *repo_path, char **owner_out, ch project_record_clear(&record); } cbm_closedir(d); - free(derived); + safe_free(derived); if (ok && !derived_exists && owner_count == 1) { - free(owners); + cbm_free(CBM_MEM_CLASS_OTHER, owners); *owner_out = owner; return true; } - free(owner); + safe_free(owner); if (ok && (derived_exists || owner_count == 0)) { - free(owners); + cbm_free(CBM_MEM_CLASS_OTHER, owners); return true; } char msg[CBM_SZ_4K]; @@ -11209,7 +11209,7 @@ static bool index_root_owner_resolve(const char *repo_path, char **owner_out, ch snprintf(msg, sizeof(msg), "out of memory while resolving the project for root_path %s", repo_path); } - free(owners); + cbm_free(CBM_MEM_CLASS_OTHER, owners); *error_out = heap_strdup(msg); return false; } @@ -11272,12 +11272,12 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { index_args_free(repo_path, mode_str, name_override); char *result = cbm_mcp_text_result( owner_error ? owner_error : "could not resolve index project name", true); - free(owner_error); + safe_free(owner_error); return result; } if (owner) { cbm_log_info("index.root_owner_reused", "root", repo_path, "project", owner); - free(name_override); + safe_free(name_override); name_override = owner; } } From 7f649b29dc778b0f54c28dfcc2f25b76f3d1947a Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 28 Sep 2026 00:42:57 +0200 Subject: [PATCH 3/4] fix(mcp): keep #2134 root-owner reuse out of cross-repo mode; portable test path Linux, MSan, TSan and diag CI failed in mcp_mutation_guard: tool_cross_repo_missing_inputs_fail_without_creating_ghost_databases ASSERT(source_failed). handle_index_repository resolved the root owner before dispatching cross-repo-intelligence mode. That test registers its named target with the source's own root, so the unindexed source adopted existing-cross-target as its project (log: index.root_owner_reused project=existing-cross-target), and the missing source was no longer reported. Cross-repo mode reads an existing source index and never writes one, so the #2134 reuse now runs only on the re-index path, after that dispatch. macOS CI stayed green because cbm_tmpdir() is /tmp, a symlink to /private/tmp there, so the canonical repo_path never equalled the fixture's stored root; with the fixture path canonicalized, the failure reproduced 3/3 on macOS and passes 3/3 with this change. The #2134 test itself called realpath() into a 1 KiB buffer. glibc's FORTIFY check aborts on that ("buffer overflow detected", a SIGABRT of the mcp suite on the ubuntu 2/3 shards), and MinGW has no realpath at all (Windows compile error). It now uses cbm_canonical_path with a 4 KiB buffer, the same canonicalizer production uses. Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 15 +++++++++------ tests/test_mcp.c | 6 +++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 29c08f3e2d..1477698f09 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11265,6 +11265,15 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { return cbm_mcp_text_result(boundary_err, true); } + if (mode_str && strcmp(mode_str, "cross-repo-intelligence") == 0) { + char *result = handle_cross_repo_mode(srv, repo_path, name_override, args); + index_args_free(repo_path, mode_str, name_override); + return result; + } + + /* #2134 applies to re-indexing only. Cross-repo mode reads an existing + * source index and never writes one; adopting a root owner there could + * turn one of its own named targets into the source. */ if ((!name_override || !name_override[0]) && repo_path && repo_path[0]) { char *owner = NULL; char *owner_error = NULL; @@ -11282,12 +11291,6 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { } } - if (mode_str && strcmp(mode_str, "cross-repo-intelligence") == 0) { - char *result = handle_cross_repo_mode(srv, repo_path, name_override, args); - index_args_free(repo_path, mode_str, name_override); - return result; - } - cbm_index_resource_policy_t resource_policy; char policy_error[CBM_SZ_256] = {0}; if (!load_index_policy(srv, args, &resource_policy, policy_error, sizeof(policy_error))) { diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 66329e2c24..770f901d52 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -8403,9 +8403,9 @@ TEST(tool_index_repository_reuses_existing_project_for_root_issue2134) { if (!cbm_mkdtemp(repo) || !cbm_mkdtemp(cache)) { FAIL("mkdtemp failed"); } - char canonical_repo[CBM_SZ_1K]; - if (!realpath(repo, canonical_repo)) { - FAIL("realpath failed"); + char canonical_repo[CBM_SZ_4K]; /* cbm_canonical_path needs >= 4096 bytes */ + if (!cbm_canonical_path(repo, canonical_repo, sizeof(canonical_repo))) { + FAIL("cbm_canonical_path failed"); } const char *saved_cache = getenv("CBM_CACHE_DIR"); char *saved_cache_copy = saved_cache ? cbm_strdup(saved_cache) : NULL; From b205f0b51dacd89da57f6fcbeb46f55617606091 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 28 Sep 2026 19:37:07 +0200 Subject: [PATCH 4/4] test(mcp): compare the #2134 root with forward slashes on Windows tool_index_repository_reuses_existing_project_for_root_issue2134 counts how often the canonical repo root occurs in list_projects. On Windows, cbm_canonical_path returns "C:\Users\...", while the stored root_path is "C:/Users/...", so the count was 0 and the test failed on the CLANG64 leg (test_mcp.c:8448: owners == 0, expected 1). Normalize the expected root with cbm_normalize_path_sep, a no-op on POSIX, before counting. Signed-off-by: Martin Vogel --- tests/test_mcp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 770f901d52..9754a60573 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -8407,6 +8407,8 @@ TEST(tool_index_repository_reuses_existing_project_for_root_issue2134) { if (!cbm_canonical_path(repo, canonical_repo, sizeof(canonical_repo))) { FAIL("cbm_canonical_path failed"); } + /* Stored root_path values use forward slashes on every platform. */ + cbm_normalize_path_sep(canonical_repo); const char *saved_cache = getenv("CBM_CACHE_DIR"); char *saved_cache_copy = saved_cache ? cbm_strdup(saved_cache) : NULL; const char *saved_sup = getenv("CBM_INDEX_SUPERVISOR");