From ec99ec27f7f8c7d930ca457ee28e581be51f667e Mon Sep 17 00:00:00 2001 From: Andrew Hundt Date: Mon, 23 Mar 2026 02:58:20 -0400 Subject: [PATCH] mcp: fix get_code returning ambiguous with 1 match + cold-start project detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs fixed in handle_get_code_snippet: Bug A (root cause): Tiers 1-3 all use WHERE project = ?1 AND ... in SQL. When project param is NULL, SQLite binds NULL and the comparison is always false — all exact/suffix/name lookups silently return 0 rows. Bug B: Tier 4 fuzzy search found 1 result but snippet_suggestions() always sets status=ambiguous regardless of count. Bug C: Dedup block with cand_count==1 after dedup fell through to ambiguous instead of resolving. Fixes: 1. extract_project_from_qn(): scans each dot-prefix of the QN and tests for a matching ~/.cache/codebase-memory-mcp/{prefix}.db file (O(n), ~5-15 access() calls). Returns longest matching prefix — the QN is self-describing so this works even on cold start (no prior search call). Single malloc+memcpy pattern: best_end offset avoids repeated strdup. 2. handle_get_code_snippet: when project param is NULL, calls extract_project_from_qn(qn) and opens the correct DB via resolve_store. Falls back to srv->current_project if no DB found. Assigns result into project (was NULL) so all existing free(project) exit paths own the memory. 3. Tier 4 fuzzy: fuzzy_count==1 now resolves directly (build_snippet_response) instead of calling snippet_suggestions. 4. Dedup block: cand_count==1 after dedup now resolves directly. Tests added (3 new, total 2190): - get_code_no_project_uses_open_store_tier1: after search_graph opens a store, get_code without project resolves via Tier 1 exact QN + eff_project - get_code_single_fuzzy_result_resolves_not_ambiguous: wrong-prefix QN forces Tier 4 fuzzy; single result must not return status=ambiguous - get_code_cold_start_parses_project_from_qn: fresh server, no prior call, extract_project_from_qn finds the DB and resolves the symbol Signed-off-by: Andrew Hundt --- src/mcp/mcp.c | 106 ++++++++++++++++++++++++-- tests/test_tool_consolidation.c | 127 ++++++++++++++++++++++++++++++++ 2 files changed, 227 insertions(+), 6 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 37ebfdee6..383373ce3 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -816,6 +816,54 @@ static const char *project_db_path(const char *project, char *buf, size_t bufsz) return buf; } +/* ── QN project extraction ─────────────────────────────────────── */ + +/* Try to identify the project prefix of a qualified name by scanning each + * dot-separated prefix and checking if a matching DB file exists. + * Returns a heap-allocated project name (caller must free), or NULL if no + * matching DB is found. Cost: one access() call per dot in the QN (~5-10). */ +static char *extract_project_from_qn(const char *qn) { + if (!qn) return NULL; + const char *home = getenv("HOME"); + if (!home) return NULL; + + /* Scan each dot-separated prefix of the QN and test if a matching DB file + * exists. Walk left-to-right so the last hit is the longest (most + * specific) match. Record only the winning offset to do a single strdup + * at the end — avoids repeated alloc/free on multi-dot project names. */ + size_t qn_len = strlen(qn); + char *candidate = malloc(qn_len + 1); + if (!candidate) return NULL; + memcpy(candidate, qn, qn_len + 1); + + size_t best_end = 0; /* length of the longest matching prefix found */ + char db_path[1024]; + const char *home_val = home; + + for (size_t i = 0; i < qn_len; i++) { + if (candidate[i] == '.') { + candidate[i] = '\0'; + snprintf(db_path, sizeof(db_path), + "%s/.cache/codebase-memory-mcp/%s.db", home_val, candidate); + if (access(db_path, F_OK) == 0) { + best_end = i; /* length of this prefix */ + } + candidate[i] = '.'; + } + } + + char *result = NULL; + if (best_end > 0) { + result = malloc(best_end + 1); + if (result) { + memcpy(result, qn, best_end); + result[best_end] = '\0'; + } + } + free(candidate); + return result; /* NULL if no matching DB found; caller frees */ +} + /* ── Store resolution ──────────────────────────────────────────── */ /* Open the right project's .db file for query tools. @@ -2513,6 +2561,24 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) { char *qn = cbm_mcp_get_string_arg(args, "qualified_name"); char *project = cbm_mcp_get_string_arg(args, "project"); cbm_store_t *store = resolve_store(srv, project); + /* When no project param given, try to parse the project prefix from the + * qualified name by checking for a matching .db file. This is Option C: + * the QN is self-describing, so we can always open the right store even on + * a cold start (no prior search_code_graph call). + * Falls back to the currently-open store's project as a secondary option. */ + const char *eff_project = project; + if (!eff_project && qn) { + /* Option C: QN is self-describing — try to find the project prefix by + * checking for a matching .db file. assign into project so the + * existing free(project) calls at every exit path own the memory. */ + project = extract_project_from_qn(qn); + if (project) { + eff_project = project; + store = resolve_store(srv, project); /* open the correct DB */ + } else if (srv->current_project && srv->current_project[0]) { + eff_project = srv->current_project; /* fallback: last-used project */ + } + } bool auto_resolve = cbm_mcp_get_bool_arg(args, "auto_resolve"); bool include_neighbors = cbm_mcp_get_bool_arg(args, "include_neighbors"); int cfg_max_lines = cbm_config_get_int(srv->config, CBM_CONFIG_SNIPPET_MAX_LINES, @@ -2539,7 +2605,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) { /* Tier 1: Exact QN match */ cbm_node_t node = {0}; - int rc = cbm_store_find_node_by_qn(store, project, qn, &node); + int rc = cbm_store_find_node_by_qn(store, eff_project, qn, &node); if (rc == CBM_STORE_OK) { char *result = build_snippet_response(srv, &node, NULL /*exact*/, include_neighbors, NULL, 0, @@ -2554,7 +2620,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) { /* Tier 2: QN suffix match */ cbm_node_t *suffix_nodes = NULL; int suffix_count = 0; - cbm_store_find_nodes_by_qn_suffix(store, project, qn, &suffix_nodes, &suffix_count); + cbm_store_find_nodes_by_qn_suffix(store, eff_project, qn, &suffix_nodes, &suffix_count); if (suffix_count == 1) { copy_node(&suffix_nodes[0], &node); cbm_store_free_nodes(suffix_nodes, suffix_count); @@ -2570,7 +2636,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) { /* Tier 3: Short name match */ cbm_node_t *name_nodes = NULL; int name_count = 0; - cbm_store_find_nodes_by_name(store, project, qn, &name_nodes, &name_count); + cbm_store_find_nodes_by_name(store, eff_project, qn, &name_nodes, &name_count); if (name_count == 1) { copy_node(&name_nodes[0], &node); cbm_store_free_nodes(name_nodes, name_count); @@ -2610,8 +2676,22 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) { cbm_store_free_nodes(suffix_nodes, suffix_count); cbm_store_free_nodes(name_nodes, name_count); - /* Auto-resolve: pick best candidate by degree */ - if (auto_resolve && cand_count >= 2 && cand_count <= 2) { + /* Single candidate after dedup — resolve immediately, not ambiguous */ + if (cand_count == 1) { + copy_node(&candidates[0], &node); + free_node_contents(&candidates[0]); + free(candidates); + char *result = build_snippet_response(srv, &node, "name", include_neighbors, NULL, 0, + max_lines, snippet_mode); + free_node_contents(&node); + free(qn); + free(project); + free(snippet_mode); + return result; + } + + /* Auto-resolve: pick best candidate by degree when 2+ candidates */ + if (auto_resolve && cand_count >= 2) { /* Find best: highest total degree, prefer non-test files */ int best_idx = 0; int best_deg = -1; @@ -2687,7 +2767,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) { /* Use search with name pattern for fuzzy matching */ cbm_search_params_t params = {0}; - params.project = project; + params.project = eff_project; params.name_pattern = search_name; params.limit = 5; params.min_degree = -1; @@ -2705,6 +2785,20 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) { int fuzzy_count = search_out.count; cbm_store_search_free(&search_out); + /* Single fuzzy result — resolve immediately rather than reporting ambiguous */ + if (fuzzy_count == 1) { + copy_node(&fuzzy[0], &node); + free_node_contents(&fuzzy[0]); + free(fuzzy); + char *result = build_snippet_response(srv, &node, "fuzzy", include_neighbors, NULL, 0, + max_lines, snippet_mode); + free_node_contents(&node); + free(qn); + free(project); + free(snippet_mode); + return result; + } + char *result = snippet_suggestions(qn, fuzzy, fuzzy_count); for (int i = 0; i < fuzzy_count; i++) { free_node_contents(&fuzzy[i]); diff --git a/tests/test_tool_consolidation.c b/tests/test_tool_consolidation.c index 4b5ddbde4..55fe784d9 100644 --- a/tests/test_tool_consolidation.c +++ b/tests/test_tool_consolidation.c @@ -1188,6 +1188,129 @@ TEST(prefix_collision_session_is_substring_of_project) { PASS(); } +/* ── 16. get_code NULL-project regression tests ─────────── */ + +/* Bug: Tier 1-3 use WHERE project = ?1, so they return nothing when project + * is NULL (SQL NULL comparison is always false). Fix: eff_project falls back + * to srv->current_project when the caller omits the project param. + * + * Test: after search_graph opens a store, get_code with no project param + * should resolve via Tier 1 exact QN match. */ +TEST(get_code_no_project_uses_open_store_tier1) { + /* Create a file DB with one node */ + char db_path[1024]; + snprintf(db_path, sizeof(db_path), "%s/.cache/codebase-memory-mcp/_tc_gc_proj_.db", + getenv("HOME")); + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + cbm_store_upsert_project(s, "_tc_gc_proj_", "/tmp"); + cbm_node_t n = {.project = "_tc_gc_proj_", .label = "Function", + .name = "tc_resolve_fn", + .qualified_name = "_tc_gc_proj_.src.tc_resolve_fn", + .file_path = "src/tc_resolve_fn.c"}; + cbm_store_upsert_node(s, &n); + cbm_store_close(s); + + /* Create server; call search_graph to open the store (sets current_project) */ + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + char *sr = cbm_mcp_handle_tool(srv, "search_graph", + "{\"project\":\"_tc_gc_proj_\",\"name_pattern\":\"tc_resolve_fn\",\"limit\":1}"); + ASSERT_NOT_NULL(sr); + free(sr); + + /* get_code with no project param — eff_project must fall back to current_project */ + char *gr = cbm_mcp_handle_tool(srv, "get_code", + "{\"qualified_name\":\"_tc_gc_proj_.src.tc_resolve_fn\"}"); + ASSERT_NOT_NULL(gr); + /* Must NOT be ambiguous — Tier 1 exact QN should resolve via eff_project */ + ASSERT_NULL(strstr(gr, "\"ambiguous\"")); + /* Must contain the function name in the response */ + ASSERT_NOT_NULL(strstr(gr, "tc_resolve_fn")); + free(gr); + + cbm_mcp_server_free(srv); + (void)unlink(db_path); + PASS(); +} + +/* Bug: Tier 4 fuzzy search finding exactly 1 result returned status=ambiguous. + * Fix: when fuzzy_count == 1, resolve immediately instead of calling + * snippet_suggestions which always sets status=ambiguous. */ +TEST(get_code_single_fuzzy_result_resolves_not_ambiguous) { + /* Create a file DB with one node */ + char db_path[1024]; + snprintf(db_path, sizeof(db_path), "%s/.cache/codebase-memory-mcp/_tc_gc_fuzzy_.db", + getenv("HOME")); + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + cbm_store_upsert_project(s, "_tc_gc_fuzzy_", "/tmp"); + cbm_node_t n = {.project = "_tc_gc_fuzzy_", .label = "Function", + .name = "tc_unique_fuzzy_fn", + .qualified_name = "_tc_gc_fuzzy_.src.tc_unique_fuzzy_fn", + .file_path = "src/tc_unique_fuzzy_fn.c"}; + cbm_store_upsert_node(s, &n); + cbm_store_close(s); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + /* Open the store via search_graph so current_project is set */ + char *sr = cbm_mcp_handle_tool(srv, "search_graph", + "{\"project\":\"_tc_gc_fuzzy_\",\"name_pattern\":\"tc_unique_fuzzy_fn\",\"limit\":1}"); + ASSERT_NOT_NULL(sr); + free(sr); + + /* QN with a wrong prefix — Tiers 1-3 will miss, Tier 4 fuzzy finds 1 by name */ + char *gr = cbm_mcp_handle_tool(srv, "get_code", + "{\"qualified_name\":\"wrong.prefix.tc_unique_fuzzy_fn\"}"); + ASSERT_NOT_NULL(gr); + /* Must NOT be ambiguous — single fuzzy result should auto-resolve */ + ASSERT_NULL(strstr(gr, "\"ambiguous\"")); + /* Must contain the function name */ + ASSERT_NOT_NULL(strstr(gr, "tc_unique_fuzzy_fn")); + free(gr); + + cbm_mcp_server_free(srv); + (void)unlink(db_path); + PASS(); +} + +/* Option C: cold-start test — no prior search_code_graph call. + * extract_project_from_qn() must find the DB by scanning dot-prefixes of the + * QN, so get_code works even when srv->current_project is unset. */ +TEST(get_code_cold_start_parses_project_from_qn) { + char db_path[1024]; + snprintf(db_path, sizeof(db_path), "%s/.cache/codebase-memory-mcp/_tc_gc_cold_.db", + getenv("HOME")); + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + cbm_store_upsert_project(s, "_tc_gc_cold_", "/tmp"); + cbm_node_t n = {.project = "_tc_gc_cold_", .label = "Function", + .name = "tc_cold_fn", + .qualified_name = "_tc_gc_cold_.src.tc_cold_fn", + .file_path = "src/tc_cold_fn.c"}; + cbm_store_upsert_node(s, &n); + cbm_store_close(s); + + /* Fresh server — no prior tool calls, srv->current_project is unset */ + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + + /* get_code with no project — must parse "_tc_gc_cold_" from the QN */ + char *gr = cbm_mcp_handle_tool(srv, "get_code", + "{\"qualified_name\":\"_tc_gc_cold_.src.tc_cold_fn\"}"); + ASSERT_NOT_NULL(gr); + /* Cold-start Option C: must resolve, not return ambiguous or not-found */ + ASSERT_NULL(strstr(gr, "\"ambiguous\"")); + ASSERT_NULL(strstr(gr, "\"error\"")); + ASSERT_NOT_NULL(strstr(gr, "tc_cold_fn")); + free(gr); + + cbm_mcp_server_free(srv); + (void)unlink(db_path); + PASS(); +} + /* ── Suite registration ──────────────────────────────────── */ SUITE(tool_consolidation) { @@ -1268,4 +1391,8 @@ SUITE(tool_consolidation) { RUN_TEST(prefix_collision_longer_name_with_dot_not_dep); RUN_TEST(prefix_collision_completely_different_project); RUN_TEST(prefix_collision_session_is_substring_of_project); + /* get_code NULL-project regression */ + RUN_TEST(get_code_no_project_uses_open_store_tier1); + RUN_TEST(get_code_single_fuzzy_result_resolves_not_ambiguous); + RUN_TEST(get_code_cold_start_parses_project_from_qn); }