mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-09 09:11:00 +08:00
fix(mcp): skip unfiltered rows for semantic-only JSON
search_graph previously ran an unfiltered graph search before a JSON semantic-only lookup, so an empty vector result returned unrelated nodes and paid an unnecessary graph-result-page scan. Detect semantic-only requests, skip cbm_store_search and cbm_store_search_overlay_view, and return explicit empty semantic_results with recovery guidance. Define query_graph as an early compositional tool for effective, computationally efficient custom Cypher. Mark examples, labels, properties, and LIMIT as optional guidance; retain server caps and dependency-ranking instructions; make truncation hints offer narrowing or cap changes without prescribing LIMIT. Install portable sandbox recovery guidance that distinguishes MCP approval from shell sandbox authorization. Verification: ASan/UBSan MCP 228 passed; tool_consolidation 103 passed; token_reduction 50 passed; CLI 150 passed; source-safety passed. The repository-wide clang-format target remains red on pre-existing files; changed-line formatting issues were corrected. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This commit is contained in:
+16
-5
@@ -554,12 +554,16 @@ static const char skill_content[] =
|
||||
"MATCH (f:Function) WHERE f.name =~ '.*Handler.*' RETURN f.name, f.file_path\n"
|
||||
"MATCH (a)-[r:CALLS]->(b) WHERE a.name = 'main' RETURN b.name\n"
|
||||
"```\n"
|
||||
"Examples are non-exhaustive guidance. Write problem-specific Cypher for effective, "
|
||||
"computationally efficient results; explicit labels/properties and exploratory LIMIT are "
|
||||
"optional optimizations.\n"
|
||||
"\n"
|
||||
"## Gotchas\n"
|
||||
"1. `search_graph(relationship=\"HTTP_CALLS\")` filters nodes by degree — "
|
||||
"use `query_graph` with Cypher to see actual edges.\n"
|
||||
"2. `query_graph` output is capped by query_max_output_bytes; add LIMIT or set "
|
||||
"max_output_bytes=0.\n"
|
||||
"2. `query_graph` output is capped by query_max_output_bytes. Use LIMIT when it helps query "
|
||||
"efficiency during exploration; omit it when the full result is necessary, and set "
|
||||
"max_output_bytes=0 only when uncapped output is appropriate.\n"
|
||||
"3. `trace_path` works best with exact names — use `search_graph(name_pattern=...)` first.\n"
|
||||
"4. `direction=\"outbound\"` returns callees only; use `direction=\"both\"` for callers too.\n"
|
||||
"5. Results default to search_limit (50 unless configured); check `has_more` and use "
|
||||
@@ -576,11 +580,17 @@ static const char codex_instructions_content[] =
|
||||
"auto_index_limit\n"
|
||||
"- `trace_path` — trace who calls a function or what it calls\n"
|
||||
"- `get_code` — read function source code by qualified_name\n"
|
||||
"- `query_graph` — run Cypher queries for complex patterns\n"
|
||||
"- `query_graph` — write problem-specific Cypher for effective, computationally efficient "
|
||||
"structural answers; examples and LIMIT are optional guidance\n"
|
||||
"- `get_architecture` — high-level summary after `_hidden_tools` reveal or "
|
||||
"`CBM_TOOL_MODE=classic`\n"
|
||||
"\n"
|
||||
"Prefer graph tools over grep for structural code discovery.\n";
|
||||
"Prefer graph tools over grep for structural code discovery.\n"
|
||||
"\n"
|
||||
"If a sandbox blocks an MCP or CLI operation because it crosses a shell or filesystem "
|
||||
"boundary, "
|
||||
"retry that operation with escalation instead of treating the block as a project failure. "
|
||||
"MCP approval and shell sandbox authorization are separate.\n";
|
||||
|
||||
/* Old skill names — cleaned up during install to remove stale directories. */
|
||||
static const char *old_skill_names[] = {
|
||||
@@ -1392,7 +1402,8 @@ static const char agent_instructions_content[] =
|
||||
"1. `search_graph` — find functions, classes, routes, variables by pattern\n"
|
||||
"2. `trace_path` — trace who calls a function or what it calls\n"
|
||||
"3. `get_code` — read specific function/class source code by qualified_name\n"
|
||||
"4. `query_graph` — run Cypher queries for complex patterns\n"
|
||||
"4. `query_graph` — write problem-specific Cypher for effective, computationally efficient "
|
||||
"structural answers; examples and LIMIT are optional guidance\n"
|
||||
"5. `get_architecture` — high-level summary after `_hidden_tools` reveal or "
|
||||
"`CBM_TOOL_MODE=classic`\n"
|
||||
"\n"
|
||||
|
||||
+59
-19
@@ -1117,21 +1117,26 @@ static const tool_def_t TOOLS[] = {
|
||||
"signature. Internal fp/sp/bt indexing fields are never returned.\"}}}"},
|
||||
|
||||
{"query_graph", "Query graph",
|
||||
"Execute a Cypher query against the knowledge graph for complex multi-hop patterns, "
|
||||
"aggregations, and cross-service analysis. Row scan and output bytes are capped by default "
|
||||
"(config keys query_max_rows and query_max_output_bytes). Set max_output_bytes=0 for "
|
||||
"unlimited output bytes or add LIMIT. "
|
||||
"Dependency sub-project symbols (proj.dep.*) are tagged source:dependency; to rank your own "
|
||||
"project's symbols above them, ORDER BY CASE WHEN n.project LIKE '%.dep.%' THEN 1 ELSE 0 END.",
|
||||
"Core compositional tool; use it early for bespoke structural questions. Create new, "
|
||||
"effective, computationally efficient custom Cypher for multi-hop paths, aggregates/hotspots, "
|
||||
"arbitrary predicates, cross-service links, or graph=\"missed\". Non-exhaustive examples: "
|
||||
"MATCH (f:Function)-[:CALLS]->(g) RETURN f.name,g.name LIMIT 20; "
|
||||
"MATCH (f:File) RETURN f.file_path,count(*). Any valid query shape is allowed; explicit "
|
||||
"labels/properties and LIMIT are optional efficiency aids. Server caps query_max_rows and "
|
||||
"query_max_output_bytes; raise only when needed. Dependency symbols use proj.dep.* and "
|
||||
"source:dependency; rank primary symbols first with "
|
||||
"ORDER BY CASE WHEN n.project LIKE '%.dep.%' THEN 1 ELSE 0 END.",
|
||||
"{\"type\":\"object\",\"properties\":{\"query\":{\"type\":\"string\",\"description\":\"Cypher "
|
||||
"query\"},\"project\":{\"type\":\"string\",\"description\":\"Indexed project name. Omit to "
|
||||
"use the MCP server project derived from server CWD.\"},\"max_rows\":{\"type\":\"integer\","
|
||||
"\"description\":\"Scan-level row limit. Omit to use query_max_rows config. Set 0 to use "
|
||||
"the implementation ceiling. Note: limits nodes scanned, not rows returned. For output size, "
|
||||
"use max_output_bytes or add LIMIT to your Cypher query.\"},\"max_output_bytes\":{\"type\":"
|
||||
"set max_output_bytes; an optional Cypher LIMIT can reduce computation and output.\"},"
|
||||
"\"max_output_bytes\":{\"type\":"
|
||||
"\"integer\",\"description\":\"Max response size in bytes (configurable via "
|
||||
"query_max_output_bytes config key). Set to 0 for unlimited. When exceeded, returns "
|
||||
"truncated=true with total_bytes and hint to add LIMIT.\"},\"graph\":{\"type\":\"string\","
|
||||
"truncated=true with total_bytes and optional ways to narrow the query or raise the cap.\"},"
|
||||
"\"graph\":{\"type\":\"string\","
|
||||
"\"enum\":[\"code\",\"missed\"],\"default\":\"code\",\"description\":\"Query the code "
|
||||
"graph or the best-effort graph of files not fully indexed.\"},\"format\":{\"type\":\"string\","
|
||||
"\"enum\":[\"toon\",\"json\"],\"default\":\"toon\"}},\"required\":[\"query\"]}"},
|
||||
@@ -1398,6 +1403,12 @@ static void emit_tool(yyjson_mut_doc *doc, yyjson_mut_val *tools, const tool_def
|
||||
}
|
||||
|
||||
static bool is_streamlined_default_tool(const char *name) {
|
||||
/* query_graph is intentionally a core tool, not merely an advanced escape
|
||||
* hatch. It composes labels, properties, relationships, predicates and
|
||||
* aggregations into new solutions without requiring a dedicated MCP
|
||||
* tool/schema for every structural code question. Its description asks
|
||||
* callers to create effective, computationally efficient custom Cypher;
|
||||
* examples and optimization guidance are explicitly non-binding. */
|
||||
return name && (strcmp(name, "search_graph") == 0 ||
|
||||
strcmp(name, "query_graph") == 0 ||
|
||||
strcmp(name, "search_code") == 0 ||
|
||||
@@ -5216,13 +5227,21 @@ static char *semantic_query_type_error_response(void) {
|
||||
}
|
||||
|
||||
static bool run_semantic_query(yyjson_mut_doc *doc, yyjson_mut_val *root, const char *args,
|
||||
cbm_store_t *store, const char *project, int limit) {
|
||||
cbm_store_t *store, const char *project, int limit,
|
||||
bool *out_present, int *out_count) {
|
||||
cbm_vector_result_t *vresults = NULL;
|
||||
int vcount = 0;
|
||||
bool present = false;
|
||||
bool type_error =
|
||||
run_semantic_query_core(args, store, project, limit, &vresults, &vcount, NULL);
|
||||
if (project && cbm_store_derived_view_is_stale(
|
||||
store, project, CBM_STORE_DERIVED_VIEW_SEMANTIC_EDGES)) {
|
||||
run_semantic_query_core(args, store, project, limit, &vresults, &vcount, &present);
|
||||
if (out_present) {
|
||||
*out_present = present;
|
||||
}
|
||||
if (out_count) {
|
||||
*out_count = vcount;
|
||||
}
|
||||
if (present && project &&
|
||||
cbm_store_derived_view_is_stale(store, project, CBM_STORE_DERIVED_VIEW_SEMANTIC_EDGES)) {
|
||||
add_stale_derived_view_warning(
|
||||
doc, root, CBM_STORE_DERIVED_VIEW_SEMANTIC_EDGES,
|
||||
"semantic_edges derived view is stale; semantic_results may be stale.");
|
||||
@@ -5384,7 +5403,7 @@ static char *append_semantic_query_to_json(const char *base_json, const char *ar
|
||||
}
|
||||
yyjson_mut_doc_set_root(mdoc, root);
|
||||
|
||||
bool sq_type_error = run_semantic_query(mdoc, root, args, store, project, limit);
|
||||
bool sq_type_error = run_semantic_query(mdoc, root, args, store, project, limit, NULL, NULL);
|
||||
if (sq_type_error) {
|
||||
if (type_error) {
|
||||
*type_error = true;
|
||||
@@ -5831,6 +5850,15 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) {
|
||||
true);
|
||||
}
|
||||
|
||||
/* A semantic-only request must not silently prepend an unrelated unfiltered
|
||||
* graph search. Besides misleading callers, that legacy JSON behavior added
|
||||
* an unnecessary O(graph-result-page) scan to the requested vector lookup.
|
||||
* This mirrors the compact TOON path above. */
|
||||
bool has_graph_filters = label || name_pattern || qn_pattern || unified_pattern ||
|
||||
file_pattern || relationship || exclude_entry_points ||
|
||||
min_degree != CBM_NOT_FOUND || max_degree != CBM_NOT_FOUND;
|
||||
bool semantic_only = cbm_mcp_has_arg(args, "semantic_query") && !has_graph_filters;
|
||||
|
||||
cbm_search_output_t out = {0};
|
||||
cbm_store_overlay_node_view_summary_t overlay_summary = {0};
|
||||
bool overlay_ready_for_nodes =
|
||||
@@ -5844,10 +5872,12 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) {
|
||||
(sort_by && (strcmp(sort_by, "degree") == 0 || strcmp(sort_by, "calls") == 0 ||
|
||||
strcmp(sort_by, "linkrank") == 0));
|
||||
bool overlay_search_used = overlay_ready_for_nodes;
|
||||
if (overlay_search_used) {
|
||||
cbm_store_search_overlay_view(store, ¶ms, &out);
|
||||
} else {
|
||||
cbm_store_search(store, ¶ms, &out);
|
||||
if (!semantic_only) {
|
||||
if (overlay_search_used) {
|
||||
cbm_store_search_overlay_view(store, ¶ms, &out);
|
||||
} else {
|
||||
cbm_store_search(store, ¶ms, &out);
|
||||
}
|
||||
}
|
||||
|
||||
yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL);
|
||||
@@ -5986,7 +6016,10 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) {
|
||||
|
||||
/* Semantic (vector) search: append semantic_results if semantic_query
|
||||
* array was provided. Returns true on type error (non-array value). */
|
||||
bool sq_type_error = run_semantic_query(doc, root, args, store, project, limit);
|
||||
bool sq_present = false;
|
||||
int semantic_count = 0;
|
||||
bool sq_type_error =
|
||||
run_semantic_query(doc, root, args, store, project, limit, &sq_present, &semantic_count);
|
||||
if (sq_type_error) {
|
||||
for (int pi = 0; pi < props_doc_count; pi++) {
|
||||
yyjson_doc_free(props_docs[pi]);
|
||||
@@ -6000,6 +6033,12 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) {
|
||||
free_string_array(exclude);
|
||||
return semantic_query_type_error_response();
|
||||
}
|
||||
if (semantic_only && sq_present && semantic_count == 0) {
|
||||
yyjson_mut_obj_add_val(doc, root, "semantic_results", yyjson_mut_arr(doc));
|
||||
yyjson_mut_obj_add_str(doc, root, "hint",
|
||||
"No semantic matches. semantic_query needs a moderate/full index; "
|
||||
"try broader or fewer keywords.");
|
||||
}
|
||||
|
||||
char *json = yy_doc_to_str(doc);
|
||||
/* Property docs are zero-copy referenced by the mut doc — they must
|
||||
@@ -6226,7 +6265,8 @@ static char *handle_query_graph(cbm_mcp_server_t *srv, const char *args) {
|
||||
snprintf(trunc_json, sizeof(trunc_json),
|
||||
"{\"truncated\":true,\"total_bytes\":%lu,"
|
||||
"\"rows_returned\":%d,"
|
||||
"\"hint\":\"Add LIMIT to your Cypher query\"}",
|
||||
"\"hint\":\"Narrow returned fields, add LIMIT when appropriate, or raise "
|
||||
"max_output_bytes\"}",
|
||||
(unsigned long)json_len, total_rows);
|
||||
char *result_text = cbm_mcp_text_result(trunc_json, false);
|
||||
free(json);
|
||||
|
||||
@@ -730,6 +730,10 @@ TEST(cli_codex_instructions) {
|
||||
ASSERT_NOT_NULL(instr);
|
||||
ASSERT(strstr(instr, "Codebase Knowledge Graph") != NULL);
|
||||
ASSERT(strstr(instr, "trace_path") != NULL);
|
||||
ASSERT(strstr(instr, "effective, computationally efficient") != NULL);
|
||||
ASSERT(strstr(instr, "examples and LIMIT are optional guidance") != NULL);
|
||||
ASSERT(strstr(instr, "retry that operation with escalation") != NULL);
|
||||
ASSERT(strstr(instr, "MCP approval and shell sandbox authorization are separate") != NULL);
|
||||
PASS();
|
||||
}
|
||||
|
||||
|
||||
@@ -2445,6 +2445,56 @@ TEST(tool_search_graph_semantic_query_warns_on_stale_semantic_view) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(tool_search_graph_semantic_only_json_does_not_return_unfiltered_nodes) {
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
ASSERT_NOT_NULL(srv);
|
||||
cbm_store_t *st = cbm_mcp_server_store(srv);
|
||||
ASSERT_NOT_NULL(st);
|
||||
|
||||
const char *proj = "semantic-only-json";
|
||||
ASSERT_EQ(cbm_store_upsert_project(st, proj, "/tmp/semantic-only-json"), CBM_STORE_OK);
|
||||
cbm_mcp_server_set_project(srv, proj);
|
||||
|
||||
/* A graph node without a semantic vector proves the handler does not
|
||||
* silently substitute an unrelated unfiltered graph search when the
|
||||
* semantic-only request has no matches. */
|
||||
cbm_node_t unrelated = {.project = proj,
|
||||
.label = "Function",
|
||||
.name = "unrelated_ranked_function",
|
||||
.qualified_name = "semantic.only.unrelated_ranked_function",
|
||||
.file_path = "src/unrelated.c"};
|
||||
ASSERT_GT(cbm_store_upsert_node(st, &unrelated), 0);
|
||||
|
||||
char *resp = cbm_mcp_server_handle(
|
||||
srv, "{\"jsonrpc\":\"2.0\",\"id\":481,\"method\":\"tools/call\","
|
||||
"\"params\":{\"name\":\"search_graph\","
|
||||
"\"arguments\":{\"project\":\"semantic-only-json\","
|
||||
"\"semantic_query\":[\"transport\",\"lifecycle\"],\"format\":\"json\"}}}");
|
||||
ASSERT_NOT_NULL(resp);
|
||||
char *inner = extract_text_content(resp);
|
||||
ASSERT_NOT_NULL(inner);
|
||||
|
||||
yyjson_doc *doc = yyjson_read(inner, strlen(inner), 0);
|
||||
ASSERT_NOT_NULL(doc);
|
||||
yyjson_val *root = yyjson_doc_get_root(doc);
|
||||
yyjson_val *results = yyjson_obj_get(root, "results");
|
||||
yyjson_val *semantic_results = yyjson_obj_get(root, "semantic_results");
|
||||
ASSERT_NOT_NULL(results);
|
||||
ASSERT_TRUE(yyjson_is_arr(results));
|
||||
ASSERT_EQ(yyjson_arr_size(results), 0);
|
||||
ASSERT_NOT_NULL(semantic_results);
|
||||
ASSERT_TRUE(yyjson_is_arr(semantic_results));
|
||||
ASSERT_EQ(yyjson_arr_size(semantic_results), 0);
|
||||
ASSERT_NOT_NULL(yyjson_obj_get(root, "hint"));
|
||||
ASSERT_NULL(strstr(inner, "unrelated_ranked_function"));
|
||||
|
||||
yyjson_doc_free(doc);
|
||||
free(inner);
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* MCP discovery probes must return valid lists, not -32601 Method-not-found:
|
||||
* clients like Cline call these on connect and
|
||||
* resources/list + prompts/list + resources/templates/list on connect and
|
||||
@@ -10077,6 +10127,7 @@ SUITE(mcp) {
|
||||
RUN_TEST(tool_search_graph_query_uses_search_limit_config);
|
||||
RUN_TEST(tool_search_graph_query_rejects_bad_semantic_query);
|
||||
RUN_TEST(tool_search_graph_semantic_query_warns_on_stale_semantic_view);
|
||||
RUN_TEST(tool_search_graph_semantic_only_json_does_not_return_unfiltered_nodes);
|
||||
RUN_TEST(tool_search_graph_blocks_internal_fields_and_compacts_json_properties);
|
||||
RUN_TEST(tool_output_byte_budgets);
|
||||
RUN_TEST(mcp_discovery_methods_return_supported_lists);
|
||||
|
||||
@@ -771,6 +771,8 @@ TEST(query_graph_max_output_bytes_truncates) {
|
||||
|
||||
/* Response should indicate truncation */
|
||||
ASSERT_NOT_NULL(strstr(resp, "\"truncated\":true"));
|
||||
ASSERT_NOT_NULL(strstr(resp, "Narrow returned fields, add LIMIT when appropriate, or raise "
|
||||
"max_output_bytes"));
|
||||
/* Response body should be near the byte limit */
|
||||
ASSERT_TRUE(strlen(resp) <= 2048); /* some slack for metadata */
|
||||
|
||||
|
||||
@@ -544,6 +544,36 @@ TEST(default_tool_autoindex_description_is_precise) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(query_graph_description_explains_compositional_value) {
|
||||
char *streamlined = cbm_mcp_tools_list(NULL);
|
||||
ASSERT_NOT_NULL(streamlined);
|
||||
ASSERT_NOT_NULL(strstr(streamlined, "Core compositional tool; use it early"));
|
||||
ASSERT_NOT_NULL(
|
||||
strstr(streamlined, "Create new, effective, computationally efficient custom Cypher"));
|
||||
ASSERT_NOT_NULL(strstr(streamlined, "Non-exhaustive examples"));
|
||||
ASSERT_NOT_NULL(strstr(streamlined, "Any valid query shape is allowed"));
|
||||
ASSERT_NOT_NULL(strstr(streamlined, "multi-hop paths"));
|
||||
ASSERT_NOT_NULL(strstr(streamlined, "aggregates/hotspots"));
|
||||
ASSERT_NOT_NULL(strstr(streamlined, "LIMIT are optional efficiency aids"));
|
||||
free(streamlined);
|
||||
|
||||
/* query_graph is serialized from the same canonical definition in both
|
||||
* modes; this guards the user-facing value contract as well as schema parity. */
|
||||
char *saved_mode = save_tool_mode();
|
||||
cbm_setenv("CBM_TOOL_MODE", "classic", 1);
|
||||
char *classic = cbm_mcp_tools_list(NULL);
|
||||
restore_tool_mode(saved_mode);
|
||||
ASSERT_NOT_NULL(classic);
|
||||
ASSERT_NOT_NULL(strstr(classic, "Core compositional tool; use it early"));
|
||||
ASSERT_NOT_NULL(
|
||||
strstr(classic, "Create new, effective, computationally efficient custom Cypher"));
|
||||
ASSERT_NOT_NULL(strstr(classic, "Non-exhaustive examples"));
|
||||
ASSERT_NOT_NULL(strstr(classic, "Any valid query shape is allowed"));
|
||||
free(classic);
|
||||
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(revealed_trace_path_parameter_contract) {
|
||||
char *saved_mode = save_tool_mode();
|
||||
cbm_unsetenv("CBM_TOOL_MODE");
|
||||
@@ -2918,6 +2948,7 @@ SUITE(tool_consolidation) {
|
||||
RUN_TEST(streamlined_reveal_covers_classic_capabilities);
|
||||
RUN_TEST(streamlined_core_parameter_contract);
|
||||
RUN_TEST(default_tool_autoindex_description_is_precise);
|
||||
RUN_TEST(query_graph_description_explains_compositional_value);
|
||||
RUN_TEST(revealed_trace_path_parameter_contract);
|
||||
RUN_TEST(revealed_advanced_tool_schema_matches_handlers);
|
||||
/* Dispatch */
|
||||
|
||||
Reference in New Issue
Block a user