From abf00764a7b8904976b0f3f828657e7adff196f1 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 29 Sep 2026 01:47:24 +0200 Subject: [PATCH] fix(mcp): resolve a directory project arg to its owning project (#1690) An agent in a checkout whose directory name differs from the repository name (git clone .../repo_name.git some_name) could not find the indexed project. The installed skill only said "list_projects - check if project is indexed", so Codex and Copilot guessed the project from the repo name, got "project not found" and stopped using the graph. Passing the working directory did not help whenever it was not exactly the indexed root: a path was only ever turned into the path-DERIVED name, which misses a checkout indexed under an explicit name (the stored identity, see #1660) and every directory below an indexed root. Fix: - mcp: a path-shaped project keeps the derived name when an index exists under it (unchanged fast path); otherwise it resolves to the indexed project whose stored root_path owns the directory. The deepest owning root wins; two projects owning that root stay unresolved, so the not-found error lists every project instead of guessing. Reuses the list_projects identity reader and the canonical-path containment helpers. - mcp: the project-not-found hint says how to pick the project (the name whose root_path contains the working directory, or that absolute directory; never a repo or folder name), so an agent that guessed can recover. - cli: the SKILL.md every skill client installs (Codex, Copilot and the rest), the shared agent instructions and the Aider CLI instructions say the same thing. Proof (dev binary, synthetic HOME; repo_name cloned as some_name and indexed by path; a second checkout other_dir indexed with name=my-alias): before: project= -> project not found project=/pkg -> project not found after: project= -> my-alias (index_status agrees) project=/pkg -> the some_name project unchanged: the checkout root path and the explicit name resolve; the unindexed sibling some_name_x stays not found; a guessed "repo_name" stays not found and now carries the how-to-pick hint. Tests: - mcp tool_project_path_resolves_owning_project_issue1690: an explicitly named root and its subdirectory resolve; normal naming and similar names (some_name2, some_name_x) are not confused; two owners of one root are never guessed; a guessed repo name gets the hint. RED 3x before, green after, RED again when the production change is reverted (the hint check also goes RED on its own revert). - cli cli_guidance_resolves_project_by_root_path_issue1690: pins the instruction in the skill, agent-instructions and Aider variants. Fixes #1690 Signed-off-by: Martin Vogel --- src/cli/cli.c | 11 ++- src/mcp/mcp.c | 74 ++++++++++++++++++- tests/test_cli.c | 28 ++++++++ tests/test_mcp.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 293 insertions(+), 4 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index d7b0cc740..ed65013d1 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -1504,7 +1504,11 @@ static const char skill_content[] = "| Text search | `search_code` or Grep |\n" "\n" "## Exploration Workflow\n" - "1. `list_projects` — check if project is indexed\n" + /* #1690: a guessed repo or folder name misses the project whenever the + * checkout directory differs from it; the root_path is what identifies it. */ + "1. `list_projects` — pick the project whose `root_path` contains your working directory; " + "pass its `name` (or the absolute directory itself) as `project`, never a repo or folder " + "name\n" "2. `get_graph_schema` — understand node/edge types\n" "3. `search_graph(label=\"Function\", name_pattern=\".*Pattern.*\")` — find code\n" "4. `get_code_snippet(qualified_name=\"project.path.FuncName\")` — read source\n" @@ -3274,6 +3278,9 @@ static const char agent_instructions_content[] = "### Session resets and subagents\n" "- At session start or after compaction, confirm the nearest graph project and generation with " "`list_projects` or `index_status`, then choose Scout, Verify, or Auditor.\n" + "- The graph project is the `list_projects` entry whose `root_path` contains your working " + "directory; pass its `name` (or the absolute directory itself) as `project`, never a repo or " + "folder name.\n" "- Before spawning a subagent, query the graph and coverage in the parent. Pass the tier, " "project, generation/freshness, bounded scope, queries and pagination state, qualified " "symbols, " @@ -3679,6 +3686,8 @@ static const char aider_instructions_content[] = "First use in a repo: codebase-memory-mcp cli index_repository '{\"repo_path\":\"\"}'\n" "List indexed projects (for ): codebase-memory-mcp cli list_projects '{}'\n" + " is the entry whose root_path contains your working directory; pass it (or the " + "absolute directory itself), never a repo or folder name.\n" "\n" "## When to fall back to grep/glob\n" "- Searching for string literals, error messages, config values\n" diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 2ed29799b..250a464de 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1572,6 +1572,8 @@ static bool bare_project_encoding_usable(const char *encoded) { return strcmp(encoded, "root") != 0 && cbm_validate_project_name(encoded); } +static char *resolve_path_owner(const char *path, char *derived); /* #1690, defined below */ + static char *normalize_project_arg(char *project) { if (!project) { return project; @@ -1585,6 +1587,9 @@ static char *normalize_project_arg(char *project) { } else { project = canonicalize_repo_path_if_exists(project); normalized = cbm_project_name_from_path(project); + if (normalized) { + normalized = resolve_path_owner(project, normalized); + } } /* Keep exactly one of the two strings and release the other. */ bool use_normalized = normalized && (!bare || bare_project_encoding_usable(normalized)); @@ -2640,10 +2645,13 @@ static char *build_project_list_error(const char *reason) { enum { ERR_BUF_SZ = 5120 }; char buf[ERR_BUF_SZ]; if (count > 0) { + /* #1690: say how to pick the project, so an agent that guessed a repo + * or folder name can recover instead of dropping the graph. */ snprintf(buf, sizeof(buf), - "{\"error\":\"%s\",\"hint\":\"Use list_projects to see all indexed projects, " - "then pass it as the \\\"project\\\" " - "argument.\",\"available_projects\":[%s],\"count\":%d}", + "{\"error\":\"%s\",\"hint\":\"Use list_projects and pass the name whose " + "root_path contains your working directory (or that absolute directory) as " + "the \\\"project\\\" argument; never a repo or folder " + "name.\",\"available_projects\":[%s],\"count\":%d}", reason, projects, count); } else { snprintf(buf, sizeof(buf), @@ -11844,6 +11852,66 @@ bool cbm_path_within_root(const char *root_path, const char *abs_path) { return false; } +/* #1690: a path-shaped `project` names a directory; the installed guidance tells + * agents to pass their working directory. Turning it into the path-DERIVED name + * alone missed a checkout indexed under an explicit name and every directory + * below an indexed root, so the agent got "project not found" and dropped the + * graph. Keep the derived name when an index exists under it (the unchanged + * fast path); otherwise adopt the indexed project whose stored root_path owns + * the directory. The deepest owning root wins. Two projects owning that root + * are ambiguous and keep the derived name, so the not-found error lists every + * project instead of a guess. Takes ownership of `derived`; returns a heap name. */ +static char *resolve_path_owner(const char *path, char *derived) { + char db_path[CBM_SZ_1K]; + project_db_path(derived, db_path, sizeof(db_path)); + if (db_path[0] && cbm_file_exists(db_path)) { + return derived; + } + char real_path[CBM_SZ_4K]; + char dir_path[CBM_SZ_1K]; + cache_dir(dir_path, sizeof(dir_path)); + cbm_dir_t *d = + resolve_canonical_path(path, real_path, sizeof(real_path)) ? cbm_opendir(dir_path) : NULL; + if (!d) { + return derived; + } + char *owner = NULL; + size_t owner_depth = 0; + bool ambiguous = false; + cbm_dirent_t *entry; + while ((entry = cbm_readdir(d)) != NULL) { + mcp_project_record_t record = {0}; + if (!is_project_db_file(entry->name, strlen(entry->name)) || + read_project_record_identity(dir_path, entry->name, 0, &record) != PROJECT_RECORD_OK) { + continue; + } + char real_root[CBM_SZ_4K]; + if (record.root_path[0] && + resolve_canonical_path(record.root_path, real_root, sizeof(real_root)) && + canonical_path_has_root(real_root, real_path)) { + size_t depth = strlen(real_root); + if (depth > owner_depth) { + safe_free(owner); + owner = record.name; + record.name = NULL; + owner_depth = depth; + ambiguous = false; + } else if (depth == owner_depth) { + ambiguous = true; + } + } + project_record_clear(&record); + } + cbm_closedir(d); + if (!owner || ambiguous) { + safe_free(owner); + return derived; + } + cbm_log_info("mcp.project_path_resolved", "path", real_path, "resolved", owner); + safe_free(derived); + return owner; +} + static char *resolve_snippet_source(const char *root_path, const char *file_path, int start, int end, char **out_abs_path) { *out_abs_path = NULL; diff --git a/tests/test_cli.c b/tests/test_cli.c index 267b403dd..27f7c0fc8 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -14154,6 +14154,33 @@ TEST(cli_agent_instructions_content) { PASS(); } +/* #1690: an agent in a checkout whose directory name differs from the repo + * name guessed the project from the repo name, got "project not found" and + * dropped the graph. Every installed guidance variant must say how to pick the + * project: the list_projects entry whose root_path contains the working + * directory, or that absolute directory itself, never a repo or folder name. */ +static int i1690_names_project_by_root(const char *text) { + return text && strstr(text, "root_path") != NULL && + strstr(text, "contains your working directory") != NULL && + strstr(text, "absolute directory") != NULL && + strstr(text, "never a repo or folder name") != NULL; +} + +TEST(cli_guidance_resolves_project_by_root_path_issue1690) { + const cbm_skill_t *skills = cbm_get_skills(); + ASSERT_NOT_NULL(skills); + for (int i = 0; i < CBM_SKILL_COUNT; i++) { + if (!i1690_names_project_by_root(skills[i].content)) { + fprintf(stderr, " [1690] skill %s does not say how to pick the project\n", + skills[i].name); + } + ASSERT(i1690_names_project_by_root(skills[i].content)); + } + ASSERT(i1690_names_project_by_root(cbm_get_agent_instructions())); + ASSERT(i1690_names_project_by_root(cbm_get_aider_instructions())); + PASS(); +} + TEST(cli_qwen_windows_hook_command_uses_powershell_schema) { char command[1024]; char shell[32]; @@ -16964,6 +16991,7 @@ SUITE(cli) { RUN_TEST(cli_upsert_instructions_no_duplicate); RUN_TEST(cli_remove_instructions); RUN_TEST(cli_agent_instructions_content); + RUN_TEST(cli_guidance_resolves_project_by_root_path_issue1690); RUN_TEST(cli_qwen_windows_hook_command_uses_powershell_schema); RUN_TEST(cli_windows_optional_hooks_require_a_documented_shell); RUN_TEST(cli_installed_skill_limits_match_server_contract); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 7a2219c5a..b4ed740ed 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -8417,6 +8417,189 @@ TEST(tool_search_graph_accepts_project_name_alias_issue640) { PASS(); } +/* #1690: the installed skill tells an agent to pass its absolute working + * directory (or the list_projects name whose root_path contains it) as + * `project`, never a guessed repo or folder name. A path-shaped project used to + * be turned into the path-DERIVED name only, so it missed a checkout indexed + * under an explicit name and every directory below an indexed root: the agent + * got "project not found" and dropped the graph. It must resolve to the indexed + * project whose stored root_path owns the path, whatever that project is + * called; the deepest owning root wins, and two owners of one root are never + * guessed between. */ +static char *i1690_search(cbm_mcp_server_t *srv, const char *project) { + char args[CBM_SZ_4K]; + snprintf(args, sizeof(args), "{\"project\":\"%s\",\"name_pattern\":\".*_fn_1690\"}", project); + return cbm_mcp_handle_tool(srv, "search_graph", args); +} + +static char *i1690_index(cbm_mcp_server_t *srv, const char *repo, const char *name) { + char args[CBM_SZ_4K]; + if (name) { + snprintf(args, sizeof(args), "{\"repo_path\":\"%s\",\"name\":\"%s\"}", repo, name); + } else { + snprintf(args, sizeof(args), "{\"repo_path\":\"%s\"}", repo); + } + return cbm_mcp_handle_tool(srv, "index_repository", args); +} + +TEST(tool_project_path_resolves_owning_project_issue1690) { + char tmp[CBM_SZ_256]; + char cache[CBM_SZ_256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm-i1690r-XXXXXX"); + snprintf(cache, sizeof(cache), "/tmp/cbm-i1690c-XXXXXX"); + if (!cbm_mkdtemp(tmp) || !cbm_mkdtemp(cache)) { + FAIL("mkdtemp failed"); + } + /* Forward slashes keep the JSON arguments valid on every platform. */ + char base[CBM_SZ_4K]; /* cbm_canonical_path needs >= 4096 bytes */ + if (!cbm_canonical_path(tmp, base, sizeof(base))) { + FAIL("cbm_canonical_path failed"); + } + cbm_normalize_path_sep(base); + 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); + + /* some_name: the checkout of "repo_name", indexed under that explicit name. + * some_name2: a similarly named checkout indexed by path (derived name). + * some_name_x: shares the "some_name" prefix and is NOT indexed. + * twin_root: one root indexed under two explicit names. */ + char named[CBM_SZ_4K]; + char named_sub[CBM_SZ_4K]; + char similar[CBM_SZ_4K]; + char similar_sub[CBM_SZ_4K]; + char unindexed[CBM_SZ_4K]; + char twin[CBM_SZ_4K]; + snprintf(named, sizeof(named), "%s/some_name", base); + snprintf(named_sub, sizeof(named_sub), "%s/some_name/pkg", base); + snprintf(similar, sizeof(similar), "%s/some_name2", base); + snprintf(similar_sub, sizeof(similar_sub), "%s/some_name2/pkg", base); + snprintf(unindexed, sizeof(unindexed), "%s/some_name_x", base); + snprintf(twin, sizeof(twin), "%s/twin_root", base); + ASSERT_EQ(th_write_file(TH_PATH(named, "app.py"), "def named_fn_1690(x):\n return x\n"), 0); + ASSERT_EQ(th_write_file(TH_PATH(named_sub, "entry.py"), "def sub_fn_1690():\n return 1\n"), + 0); + ASSERT_EQ(th_write_file(TH_PATH(similar, "app.py"), "def similar_fn_1690(x):\n return x\n"), + 0); + ASSERT_EQ( + th_write_file(TH_PATH(similar_sub, "entry.py"), "def deep_fn_1690():\n return 2\n"), 0); + ASSERT_EQ(th_write_file(TH_PATH(unindexed, "app.py"), "def stray_fn_1690():\n return 3\n"), + 0); + ASSERT_EQ(th_write_file(TH_PATH(twin, "app.py"), "def twin_fn_1690():\n return 4\n"), 0); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + char *r = i1690_index(srv, named, "repo_name1690"); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "repo_name1690")); + free(r); + r = i1690_index(srv, similar, NULL); + ASSERT_NOT_NULL(r); + ASSERT_NULL(strstr(r, "\"isError\":true")); + free(r); + r = i1690_index(srv, twin, "twin1690a"); + ASSERT_NOT_NULL(r); + free(r); + r = i1690_index(srv, twin, "twin1690b"); + ASSERT_NOT_NULL(r); + free(r); + + /* 1. The explicitly named checkout's root path resolves to that project + * (RED before the fix: the derived name has no index). */ + r = i1690_search(srv, named); + ASSERT_NOT_NULL(r); + if (strstr(r, "project not found")) { + fprintf(stderr, " [1690] FAIL named root path did not resolve: %.300s\n", r); + } + ASSERT_NULL(strstr(r, "project not found")); + ASSERT_NOT_NULL(strstr(r, "named_fn_1690")); + ASSERT_NULL(strstr(r, "similar_fn_1690")); + free(r); + + /* 2. A working directory below that root resolves to the same project. */ + r = i1690_search(srv, named_sub); + ASSERT_NOT_NULL(r); + if (strstr(r, "project not found")) { + fprintf(stderr, " [1690] FAIL subdirectory path did not resolve: %.300s\n", r); + } + ASSERT_NULL(strstr(r, "project not found")); + ASSERT_NOT_NULL(strstr(r, "sub_fn_1690")); + free(r); + + /* 3. index_status names the resolved identity, not the derived name. */ + char status_args[CBM_SZ_4K]; + snprintf(status_args, sizeof(status_args), "{\"project\":\"%s\",\"format\":\"json\"}", + named_sub); + r = cbm_mcp_handle_tool(srv, "index_status", status_args); + ASSERT_NOT_NULL(r); + char *status = extract_text_content(r); + ASSERT_NOT_NULL(status); + ASSERT_NOT_NULL(strstr(status, "\"project\":\"repo_name1690\"")); + free(status); + free(r); + + /* 4. Control, normal naming: a path-derived project resolves as before, + * and its subdirectory resolves to it, never to the similar name. */ + r = i1690_search(srv, similar); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "similar_fn_1690")); + ASSERT_NULL(strstr(r, "named_fn_1690")); + free(r); + r = i1690_search(srv, similar_sub); + ASSERT_NOT_NULL(r); + ASSERT_NULL(strstr(r, "project not found")); + ASSERT_NOT_NULL(strstr(r, "deep_fn_1690")); + ASSERT_NULL(strstr(r, "named_fn_1690")); + free(r); + + /* 5. Control, similar names: a sibling that only shares the "some_name" + * prefix is owned by no project and must not borrow one. */ + r = i1690_search(srv, unindexed); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "project not found")); + free(r); + + /* 6. Two projects owning one root are ambiguous: never guess. */ + r = i1690_search(srv, twin); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "project not found")); + free(r); + + /* 7. Control: the exact explicit name keeps working. */ + r = i1690_search(srv, "repo_name1690"); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "named_fn_1690")); + free(r); + + /* 8. A guessed repo name stays unresolved (nothing is guessed), but the + * error says how to pick the project so the agent can recover. */ + r = i1690_search(srv, "repo_name"); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(strstr(r, "project not found")); + ASSERT_NOT_NULL(strstr(r, "root_path contains your working directory")); + 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(tmp); + th_rmtree(cache); + PASS(); +} + /* #1025: agents pass the repo FOLDER name ("codebase-memory-mcp"), but * indexed project names derive from the full path * (E:\project\graph\x -> "E-project-graph-x"), so exact lookup fails with @@ -21020,6 +21203,7 @@ SUITE(mcp) { RUN_TEST(tool_get_architecture_rejects_unknown_aspect_pr560); RUN_TEST(tool_get_architecture_accepts_project_name_alias_issue640); RUN_TEST(tool_search_graph_accepts_project_name_alias_issue640); + RUN_TEST(tool_project_path_resolves_owning_project_issue1690); RUN_TEST(tool_project_arg_resolves_unique_tail_issue1025); RUN_TEST(tool_project_arg_resolves_non_ascii_folder_issue1827); RUN_TEST(tool_get_architecture_path_scoping);