diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 75ff8e396c..1477698f09 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 = cbm_realloc(CBM_MEM_CLASS_OTHER, *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)) { + 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) { + safe_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); + safe_free(derived); + + if (ok && !derived_exists && owner_count == 1) { + cbm_free(CBM_MEM_CLASS_OTHER, owners); + *owner_out = owner; + return true; + } + safe_free(owner); + if (ok && (derived_exists || owner_count == 0)) { + cbm_free(CBM_MEM_CLASS_OTHER, 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); + } + cbm_free(CBM_MEM_CLASS_OTHER, 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 @@ -11151,6 +11271,26 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { 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; + 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); + safe_free(owner_error); + return result; + } + if (owner) { + cbm_log_info("index.root_owner_reused", "root", repo_path, "project", owner); + safe_free(name_override); + name_override = owner; + } + } + 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))) { @@ -11161,7 +11301,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 +11330,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..9754a60573 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -8382,6 +8382,112 @@ 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_4K]; /* cbm_canonical_path needs >= 4096 bytes */ + 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"); + 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 +20703,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);