mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 18:58:00 +00:00
Merge pull request #2446 from DeusData/fix/issue-1690-skill-project-name
fix(mcp): resolve a directory project arg to its owning project (#1690)
This commit is contained in:
4 files changed
+293
-4
No files matched your search
+10
-1
@@ -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\":\"<abs "
|
||||
"path>\"}'\n"
|
||||
"List indexed projects (for <name>): codebase-memory-mcp cli list_projects '{}'\n"
|
||||
"<name> 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"
|
||||
|
||||
+71
-3
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user