mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-10 09:13:53 +08:00
fix(mcp): resolve index_repository's repo_path from a known project name
handle_index_repository requires repo_path and errors with "repo_path is required" if it is absent. list_projects only ever advertises the project NAME, never repo_path, and every read tool accepts that name back through get_project_arg's project/project_name/ project_id/projectName aliases -- but index_repository did not, so re-indexing an already-indexed project by the name list_projects gave you had no resolution path and failed immediately. Add resolved_repo_path_from_project_arg(): a query-only lookup of the project's own stored root_path, tried when repo_path is absent before giving up. It never touches srv->store or srv->current_project, so it cannot disturb whatever project the server already has cached, and an unresolvable name still falls through to the original, correct "repo_path is required" error. Verified in a clean ubuntu:24.04 container (TEST_SUITES=mcp make -f Makefile.cbm test-focused): the new tool_index_repository_resolves_root_path_from_project_name_issue1211 fails on unpatched main (strstr(resp, "repo_path is required") is not NULL) and passes with the fix; the companion tool_index_repository_unknown_project_name_still_requires_repo_path guards the fallback stays exact for a name that was never indexed. Full mcp suite: 179 passed on both sides, with one pre-existing, unrelated failure (tool_detect_changes_contained_commands_clean_up_error_and_success, test_mcp.c:5814) reproducing identically on unmodified origin/main. scripts/lint.sh --ci and scripts/security-audit.sh both clean. Fixes #1211 Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
This commit is contained in:
@@ -7566,12 +7566,47 @@ static char *index_args_with_repo_path(const char *args, const char *canonical_r
|
||||
return rewritten;
|
||||
}
|
||||
|
||||
/* #1211: index_repository requires repo_path, but list_projects only ever
|
||||
* advertises the project NAME, and every read tool accepts that name back via
|
||||
* get_project_arg's "project"/"project_name"/"project_id"/"projectName"
|
||||
* aliases (mcp.c:1385). Re-indexing an already-indexed project by that same
|
||||
* name had no resolution path and fell straight to "repo_path is required".
|
||||
* Look up the project's own stored root_path (list_projects proves it's on
|
||||
* file) before giving up. Query-only open, always closed here: this never
|
||||
* creates a store or touches srv->store/srv->current_project, so it cannot
|
||||
* disturb whatever project the server has cached. */
|
||||
static char *resolved_repo_path_from_project_arg(const char *args) {
|
||||
char *project = get_project_arg(args);
|
||||
if (!project) {
|
||||
return NULL;
|
||||
}
|
||||
char db_path[CBM_SZ_1K];
|
||||
project_db_path(project, db_path, sizeof(db_path));
|
||||
cbm_store_t *store = db_path[0] ? cbm_store_open_path_query(db_path) : NULL;
|
||||
char *root_path = NULL;
|
||||
if (store) {
|
||||
cbm_project_t proj = {0};
|
||||
if (cbm_store_get_project(store, project, &proj) == CBM_STORE_OK) {
|
||||
root_path = proj.root_path ? heap_strdup(proj.root_path) : NULL;
|
||||
cbm_project_free_fields(&proj);
|
||||
}
|
||||
cbm_store_close(store);
|
||||
}
|
||||
free(project);
|
||||
return root_path;
|
||||
}
|
||||
|
||||
static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) {
|
||||
char *repo_path = cbm_mcp_get_string_arg(args, "repo_path");
|
||||
char *mode_str = cbm_mcp_get_string_arg(args, "mode");
|
||||
char *name_override = cbm_mcp_get_string_arg(args, "name");
|
||||
cbm_normalize_path_sep(repo_path);
|
||||
|
||||
if (!repo_path) {
|
||||
repo_path = resolved_repo_path_from_project_arg(args);
|
||||
cbm_normalize_path_sep(repo_path);
|
||||
}
|
||||
|
||||
if (!repo_path) {
|
||||
free(mode_str);
|
||||
free(name_override);
|
||||
|
||||
@@ -5349,6 +5349,97 @@ TEST(tool_index_repository_reports_store_backed_adr) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* #1211: list_projects only ever advertises the project NAME, never the
|
||||
* repo_path, but re-indexing by that same name (the natural next call) used
|
||||
* to fall straight to "repo_path is required" because nothing resolved the
|
||||
* name back to its stored root_path. Index once by repo_path, then re-index
|
||||
* by project name alone and confirm it actually indexes instead of erroring. */
|
||||
TEST(tool_index_repository_resolves_root_path_from_project_name_issue1211) {
|
||||
char tmp_dir[256];
|
||||
snprintf(tmp_dir, sizeof(tmp_dir), "/tmp/cbm-index-byname-test-XXXXXX");
|
||||
if (!cbm_mkdtemp(tmp_dir)) {
|
||||
PASS();
|
||||
}
|
||||
char cache[256];
|
||||
snprintf(cache, sizeof(cache), "/tmp/cbm-index-byname-cache-XXXXXX");
|
||||
if (!cbm_mkdtemp(cache)) {
|
||||
cbm_rmdir(tmp_dir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
const char *saved = getenv("CBM_CACHE_DIR");
|
||||
char *saved_copy = saved ? strdup(saved) : NULL;
|
||||
cbm_setenv("CBM_CACHE_DIR", cache, 1);
|
||||
|
||||
char src_path[512];
|
||||
snprintf(src_path, sizeof(src_path), "%s/main.py", tmp_dir);
|
||||
FILE *fp = fopen(src_path, "w");
|
||||
ASSERT_NOT_NULL(fp);
|
||||
fputs("def main():\n return 'ok'\n", fp);
|
||||
fclose(fp);
|
||||
|
||||
char *project = cbm_project_name_from_path(tmp_dir);
|
||||
ASSERT_NOT_NULL(project);
|
||||
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
ASSERT_NOT_NULL(srv);
|
||||
|
||||
char index_args[1024];
|
||||
snprintf(index_args, sizeof(index_args), "{\"repo_path\":\"%s\",\"mode\":\"fast\"}", tmp_dir);
|
||||
char *resp = cbm_mcp_handle_tool(srv, "index_repository", index_args);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT(response_contains_json_fragment(resp, "\"status\":\"indexed\""));
|
||||
free(resp);
|
||||
|
||||
char by_name_args[512];
|
||||
snprintf(by_name_args, sizeof(by_name_args), "{\"project\":\"%s\",\"mode\":\"fast\"}", project);
|
||||
resp = cbm_mcp_handle_tool(srv, "index_repository", by_name_args);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "repo_path is required"));
|
||||
ASSERT(response_contains_json_fragment(resp, "\"status\":\"indexed\""));
|
||||
free(resp);
|
||||
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_project_db(cache, project);
|
||||
restore_cache_dir(saved_copy);
|
||||
free(saved_copy);
|
||||
free(project);
|
||||
remove(src_path);
|
||||
cbm_rmdir(cache);
|
||||
cbm_rmdir(tmp_dir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Same gap, opposite outcome: a project name that was never indexed has no
|
||||
* stored root_path to resolve, so it must still fail with the same clear
|
||||
* "repo_path is required" error rather than a resolver crash or silent
|
||||
* no-op. Guards the fallback path the fix above added. */
|
||||
TEST(tool_index_repository_unknown_project_name_still_requires_repo_path) {
|
||||
char cache[256];
|
||||
snprintf(cache, sizeof(cache), "/tmp/cbm-index-byname-unknown-cache-XXXXXX");
|
||||
if (!cbm_mkdtemp(cache)) {
|
||||
PASS();
|
||||
}
|
||||
const char *saved = getenv("CBM_CACHE_DIR");
|
||||
char *saved_copy = saved ? strdup(saved) : NULL;
|
||||
cbm_setenv("CBM_CACHE_DIR", cache, 1);
|
||||
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
ASSERT_NOT_NULL(srv);
|
||||
|
||||
char *resp = cbm_mcp_handle_tool(srv, "index_repository",
|
||||
"{\"project\":\"never-indexed-project\"}");
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NOT_NULL(strstr(resp, "repo_path is required"));
|
||||
free(resp);
|
||||
|
||||
cbm_mcp_server_free(srv);
|
||||
restore_cache_dir(saved_copy);
|
||||
free(saved_copy);
|
||||
cbm_rmdir(cache);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(tool_index_repository_dot_uses_absolute_project_key_and_preserves_adr) {
|
||||
char tmp_dir[256];
|
||||
snprintf(tmp_dir, sizeof(tmp_dir), "/tmp/cbm-index-dot-adr-test-XXXXXX");
|
||||
@@ -9424,6 +9515,8 @@ SUITE(mcp) {
|
||||
RUN_TEST(tool_manage_adr_get_with_existing_adr);
|
||||
RUN_TEST(tool_manage_adr_unified_backend_issue256);
|
||||
RUN_TEST(tool_index_repository_reports_store_backed_adr);
|
||||
RUN_TEST(tool_index_repository_resolves_root_path_from_project_name_issue1211);
|
||||
RUN_TEST(tool_index_repository_unknown_project_name_still_requires_repo_path);
|
||||
RUN_TEST(tool_index_repository_dot_uses_absolute_project_key_and_preserves_adr);
|
||||
RUN_TEST(index_repository_relative_path_uses_explicit_session_root);
|
||||
RUN_TEST(index_repository_supervisor_uses_canonical_session_path);
|
||||
|
||||
Reference in New Issue
Block a user