trending/codebase-memory-mcp
1
0
Fork 0
mirror of https://github.com/DeusData/codebase-memory-mcp.git synced 2026-11-03 08:03:38 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(mcp): 6 bugs found in post-implementation review

Browse Source 
store.h:108 — comment typo: "AND" → "OR" for pattern OR-search field

mcp.c:1754 — indentation bug in mode-error free() block: free(file_pattern)
  had 1-space indent instead of 8 and a spurious blank line

mcp.c:2422 — OOM leak in QN lookup: when cbm_store_find_node_by_qn succeeds
  but calloc fails, qn_tmp heap fields (name/project/label/etc.) leaked.
  Fix: free_node_contents(&qn_tmp) in else branch. Added forward declaration
  of free_node_contents before handle_trace_call_path so it resolves.

mcp.c:2464 — "function not found" hint was wrong when only qualified_name
  passed. Old hint said "use name_pattern" but user passed a QN. New hint
  says "use pattern= to find the correct qualified_name".

mcp.c:472 — search_in schema description now documents the response format
  difference: graph mode returns {total,results:[{qualified_name,...}]},
  source mode returns {matches:[{file,line,content}],count}.

tests: 4 new TDD tests covering:
  - pattern glob wildcard auto-convert (*foo* → .*foo.*)
  - pattern invalid regex returns error with hint
  - trace: qualified_name takes priority over function_name when both given
  - trace: QN-not-found returns specific hint mentioning qualified_name
Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This commit is contained in:
Andrew Hundt
2026-04-06 02:04:27 -04:00
parent 6da71c7973
commit a86d3d5659
3 changed files with 119 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mcp/mcp.c
+24 -9
View File
@@ -470,7 +470,9 @@ static const tool_def_t STREAMLINED_TOOLS[] = {
"\"exclude\":{\"type\":\"array\",\"items\":{\"type\":\"string\"},"
"\"description\":\"Glob patterns for file paths to exclude (e.g. [\\\"tests/**\\\",\\\"scripts/**\\\"])\"},"
"\"search_in\":{\"type\":\"string\",\"enum\":[\"graph\",\"source\"],\"default\":\"graph\","
"\"description\":\"'graph' (default): search indexed symbols. 'source': grep raw source files.\"},"
"\"description\":\"'graph' (default): search indexed symbols — returns {total,results:[{qualified_name,label,...}]}. "
"'source': grep raw source files — returns {matches:[{file,line,content}],count}. "
"Use 'source' for string literals, error messages, and text not in the symbol graph.\"},"
"\"pattern\":{\"type\":\"string\",\"description\":\"OR-search: matches symbol name OR qualified name. "
"Also used as the grep pattern when search_in='source'. Glob wildcards auto-convert to regex.\"},"
"\"case_sensitive\":{\"type\":\"boolean\",\"default\":false,"
@@ -1752,9 +1754,7 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) {
"{\"error\":\"invalid mode '%s'\","
"\"hint\":\"Valid values: full, summary\"}", search_mode);
free(label); free(name_pattern); free(qn_pattern); free(unified_pattern);
free(file_pattern);
free(relationship); free(sort_by); free(search_mode); free(pe.value);
free(file_pattern); free(relationship); free(sort_by); free(search_mode); free(pe.value);
return cbm_mcp_text_result(errbuf, true);
}
bool case_sensitive = cbm_mcp_get_bool_arg(args, "case_sensitive");
@@ -2361,6 +2361,9 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) {
return result;
}
/* Forward declaration: defined after handle_trace_call_path */
static void free_node_contents(cbm_node_t *n);
static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) {
char *func_name = cbm_mcp_get_string_arg(args, "function_name");
char *qn_input = cbm_mcp_get_string_arg(args, "qualified_name"); /* cross-tool chaining */
@@ -2420,7 +2423,11 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) {
cbm_node_t qn_tmp = {0};
if (cbm_store_find_node_by_qn(store, project, qn_input, &qn_tmp) == 0 && qn_tmp.id > 0) {
qn_node = calloc(1, sizeof(cbm_node_t));
if (qn_node) *qn_node = qn_tmp;
if (qn_node) {
*qn_node = qn_tmp; /* shallow copy; ownership of heap fields transferred */
} else {
free_node_contents(&qn_tmp); /* OOM: free fields to avoid leak */
}
}
}
@@ -2459,10 +2466,18 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) {
}
if (node_count == 0) {
char errbuf[512];
snprintf(errbuf, sizeof(errbuf),
"{\"error\":\"function not found: '%s'\","
"\"hint\":\"Use search_code_graph with name_pattern to find similar symbols.\"}",
func_name ? func_name : (qn_input ? qn_input : ""));
if (qn_input && !func_name) {
snprintf(errbuf, sizeof(errbuf),
"{\"error\":\"function not found for qualified_name: '%s'\","
"\"hint\":\"Use search_code_graph with pattern= to find the correct qualified_name, "
"then pass it here.\"}",
qn_input);
} else {
snprintf(errbuf, sizeof(errbuf),
"{\"error\":\"function not found: '%s'\","
"\"hint\":\"Use search_code_graph with name_pattern to find similar symbols.\"}",
func_name ? func_name : "");
}
free(func_name);
free(qn_input);
free(project);
src/store/store.h
+1 -1
View File
@@ -105,7 +105,7 @@ typedef struct {
const char *label; /* NULL = any label */
const char *name_pattern; /* regex on name, NULL = any */
const char *qn_pattern; /* regex on qualified_name, NULL = any */
const char *pattern; /* OR-search: matches name AND qualified_name, NULL = any */
const char *pattern; /* OR-search: matches name OR qualified_name, NULL = any */
const char *file_pattern; /* glob on file_path, NULL = any */
const char *relationship; /* edge type filter, NULL = any */
const char *direction; /* "inbound" / "outbound" / "any", NULL = any */
tests/test_input_validation.c
+94
View File
@@ -699,6 +699,96 @@ TEST(trace_accepts_qualified_name_param) {
PASS();
}
/* ══════════════════════════════════════════════════════════════════
* pattern= glob wildcard auto-converts to regex (*foo* → .*foo.*)
* ══════════════════════════════════════════════════════════════════ */
TEST(pattern_glob_wildcards_auto_convert) {
char tmp[256];
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
ASSERT_NOT_NULL(srv);
/* "*foo*" is not valid regex but valid glob — should auto-convert and find "foo" node */
char *raw = cbm_mcp_handle_tool(srv, "search_code_graph",
"{\"pattern\":\"*foo*\",\"limit\":5}");
char *resp = extract_text(raw); free(raw);
ASSERT_NOT_NULL(resp);
ASSERT_NULL(strstr(resp, "\"error\""));
ASSERT_NOT_NULL(strstr(resp, "\"results\""));
free(resp);
cbm_mcp_server_free(srv);
cleanup_validation_dir(tmp);
PASS();
}
/* ══════════════════════════════════════════════════════════════════
* pattern= invalid regex that can't be salvaged returns error
* ══════════════════════════════════════════════════════════════════ */
TEST(pattern_invalid_regex_returns_error) {
char tmp[256];
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
ASSERT_NOT_NULL(srv);
/* "[invalid" is not valid regex and not a glob — should return error */
char *raw = cbm_mcp_handle_tool(srv, "search_code_graph",
"{\"pattern\":\"[invalid\",\"limit\":5}");
char *resp = extract_text(raw); free(raw);
ASSERT_NOT_NULL(resp);
ASSERT_NOT_NULL(strstr(resp, "\"error\""));
ASSERT_NOT_NULL(strstr(resp, "invalid regex"));
free(resp);
cbm_mcp_server_free(srv);
cleanup_validation_dir(tmp);
PASS();
}
/* ══════════════════════════════════════════════════════════════════
* trace: when both function_name AND qualified_name given, QN takes
* priority (QN-first lookup runs before name-based lookup)
* ══════════════════════════════════════════════════════════════════ */
TEST(trace_qn_takes_priority_over_function_name) {
char tmp[256];
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
ASSERT_NOT_NULL(srv);
/* Pass a valid QN and a non-existent function_name.
* QN lookup should find "foo" and succeed; function_name is ignored. */
char *raw = cbm_mcp_handle_tool(srv, "trace_call_path",
"{\"qualified_name\":\"validation-test.test.foo\","
"\"function_name\":\"does_not_exist_anywhere\","
"\"project\":\"validation-test\"}");
char *resp = extract_text(raw); free(raw);
ASSERT_NOT_NULL(resp);
/* QN lookup finds "foo" → no error, trace succeeds */
ASSERT_NULL(strstr(resp, "\"error\""));
free(resp);
cbm_mcp_server_free(srv);
cleanup_validation_dir(tmp);
PASS();
}
/* ══════════════════════════════════════════════════════════════════
* trace: qualified_name not found returns actionable error hint
* ══════════════════════════════════════════════════════════════════ */
TEST(trace_qn_not_found_returns_specific_hint) {
char tmp[256];
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
ASSERT_NOT_NULL(srv);
/* Pass a QN that doesn't exist — should get specific hint about using pattern= */
char *raw = cbm_mcp_handle_tool(srv, "trace_call_path",
"{\"qualified_name\":\"no-such.project.func\","
"\"project\":\"validation-test\"}");
char *resp = extract_text(raw); free(raw);
ASSERT_NOT_NULL(resp);
ASSERT_NOT_NULL(strstr(resp, "\"error\""));
/* Should mention qualified_name in error, not generic function_name hint */
ASSERT_NOT_NULL(strstr(resp, "qualified_name"));
free(resp);
cbm_mcp_server_free(srv);
cleanup_validation_dir(tmp);
PASS();
}
/* ══════════════════════════════════════════════════════════════════
* Regression: classic tool names still work
* ══════════════════════════════════════════════════════════════════ */
@@ -752,5 +842,9 @@ void suite_input_validation(void) {
RUN_TEST(config_compact_default_false);
RUN_TEST(config_default_sort_by_calls);
RUN_TEST(trace_accepts_qualified_name_param);
RUN_TEST(pattern_glob_wildcards_auto_convert);
RUN_TEST(pattern_invalid_regex_returns_error);
RUN_TEST(trace_qn_takes_priority_over_function_name);
RUN_TEST(trace_qn_not_found_returns_specific_hint);
RUN_TEST(regression_trace_call_path_tool_name_still_works);
}