diff --git a/src/cli/cli.c b/src/cli/cli.c index 7902f34d0..d07d2ec5e 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 6ecb85ad4..bd4c7c7da 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1573,6 +1573,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; @@ -1586,6 +1588,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)); @@ -2641,10 +2646,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), @@ -12020,6 +12028,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 7637e67ac..8abfd6325 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -14205,6 +14205,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]; @@ -17050,6 +17077,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 22af972af..21346aac2 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -8478,6 +8478,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 @@ -21188,6 +21371,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_index_repository_reuses_existing_project_for_root_issue2134);