diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index e9f1353dc..035fa000f 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -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); diff --git a/src/store/store.h b/src/store/store.h index 5ff8a0c7a..adc6fe7e6 100644 --- a/src/store/store.h +++ b/src/store/store.h @@ -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 */ diff --git a/tests/test_input_validation.c b/tests/test_input_validation.c index 6ade3f945..b9c94bc7f 100644 --- a/tests/test_input_validation.c +++ b/tests/test_input_validation.c @@ -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); }