From 4fb9c8c6e603362ee3ee4268435875baba9f1bef Mon Sep 17 00:00:00 2001 From: Andrew Hundt Date: Tue, 30 Jun 2026 07:40:19 -0400 Subject: [PATCH] fix(mcp): normalize positive result limits search_code and trace_path accepted nonpositive per-call limits even though their config ranges are positive. search_code(limit=0) could emit no structured results, and trace_path(max_results=0) bound LIMIT 0 into traversal. Add a shared MCP helper for positive integer args and use it for search_graph, search_code, and trace_path so invalid config or request values consistently fall back to the documented defaults. Add focused MCP regressions for search_code(limit=0) and trace_path(max_results=0). Validation: make -f Makefile.cbm build/c/test-runner; CBM_ONLY_SUITE=mcp ./build/c/test-runner; CBM_ONLY_SUITE=tool_consolidation ./build/c/test-runner; bash scripts/check-source-safety.sh; git diff --check. Signed-off-by: Andrew Hundt --- src/mcp/mcp.c | 27 +++++++++++++++------------ tests/test_mcp.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 791fbff9f..a8acc133d 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -962,6 +962,13 @@ static int cbm_mcp_config_int_clamped(cbm_mcp_server_t *srv, const char *key, in return value; } +static int cbm_mcp_get_positive_int_arg(const char *args_json, const char *key, + int default_val, int fallback_val) { + int effective_default = default_val > 0 ? default_val : fallback_val; + int value = cbm_mcp_get_int_arg(args_json, key, effective_default); + return value > 0 ? value : effective_default; +} + static int cbm_mcp_store_idle_timeout_s(cbm_mcp_server_t *srv) { return cbm_mcp_config_int_clamped(srv, CBM_CONFIG_STORE_IDLE_TIMEOUT_S, CBM_MCP_DEFAULT_STORE_IDLE_TIMEOUT_S, 1, @@ -2912,15 +2919,10 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) { * through to the regex path. */ int cfg_search_limit = cbm_config_get_int(srv->config, CBM_CONFIG_SEARCH_LIMIT, CBM_MCP_DEFAULT_SEARCH_LIMIT); - if (cfg_search_limit <= 0) { - cfg_search_limit = CBM_MCP_DEFAULT_SEARCH_LIMIT; - } char *query = cbm_mcp_get_string_arg(args, "query"); if (query && query[0]) { - int q_limit = cbm_mcp_get_int_arg(args, "limit", cfg_search_limit); - if (q_limit <= 0) { - q_limit = cfg_search_limit; - } + int q_limit = cbm_mcp_get_positive_int_arg(args, "limit", cfg_search_limit, + CBM_MCP_DEFAULT_SEARCH_LIMIT); int q_offset = cbm_mcp_get_int_arg(args, "offset", 0); char *q_file_pattern = cbm_mcp_get_string_arg(args, "file_pattern"); char *bm25_json = bm25_search(store, project, query, q_file_pattern, q_limit, q_offset); @@ -3047,9 +3049,8 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) { free(file_pattern); free(relationship); free(sort_by); free(pe.value); return cbm_mcp_text_result(errbuf, true); } - int limit = cbm_mcp_get_int_arg(args, "limit", cfg_search_limit); - /* F4: treat limit<=0 as default */ - if (limit <= 0) limit = cfg_search_limit; + int limit = cbm_mcp_get_positive_int_arg(args, "limit", cfg_search_limit, + CBM_MCP_DEFAULT_SEARCH_LIMIT); int offset = cbm_mcp_get_int_arg(args, "offset", 0); bool cfg_compact = cbm_config_get_bool(srv->config, "compact", true); bool compact = cbm_mcp_get_bool_arg_default(args, "compact", cfg_compact); @@ -4172,7 +4173,8 @@ static char *handle_trace_path(cbm_mcp_server_t *srv, const char *args) { if (depth < 1) depth = 1; int cfg_trace_max = cbm_config_get_int(srv->config, CBM_CONFIG_TRACE_MAX_RESULTS, CBM_DEFAULT_TRACE_MAX_RESULTS); - int max_results = cbm_mcp_get_int_arg(args, "max_results", cfg_trace_max); + int max_results = cbm_mcp_get_positive_int_arg(args, "max_results", cfg_trace_max, + CBM_DEFAULT_TRACE_MAX_RESULTS); bool cfg_compact_t = cbm_config_get_bool(srv->config, "compact", true); bool compact = cbm_mcp_get_bool_arg_default(args, "compact", cfg_compact_t); bool include_tests = cbm_mcp_get_bool_arg(args, "include_tests"); @@ -6024,7 +6026,8 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { int context_lines = cbm_mcp_get_int_arg(args, "context", 0); int cfg_search_limit_sc = cbm_config_get_int(srv->config, CBM_CONFIG_SEARCH_LIMIT, CBM_MCP_DEFAULT_SEARCH_LIMIT); - int limit = cbm_mcp_get_int_arg(args, "limit", cfg_search_limit_sc); + int limit = cbm_mcp_get_positive_int_arg(args, "limit", cfg_search_limit_sc, + CBM_MCP_DEFAULT_SEARCH_LIMIT); bool use_regex = cbm_mcp_get_bool_arg(args, "regex"); uint64_t search_t0 = cbm_now_ms(); /* In literal (non-regex) mode a '|' is matched as a byte, not alternation — diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 58d6631e6..0367f2682 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -1040,6 +1040,18 @@ TEST(tool_trace_path_prefers_definition) { ASSERT_NOT_NULL(strstr(inner, "callee")); free(inner); free(resp); + + resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":63,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"trace_path\",\"arguments\":{\"function_name\":\"dup\"," + "\"project\":\"pref-proj\",\"direction\":\"outbound\",\"max_results\":0}}}"); + ASSERT_NOT_NULL(resp); + inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "callee")); + free(inner); + free(resp); + cbm_mcp_server_free(srv); PASS(); } @@ -1335,6 +1347,34 @@ TEST(search_code_multi_word) { PASS(); } +TEST(search_code_limit_zero_uses_config_default) { + char tmp[512]; + cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp)); + ASSERT_NOT_NULL(srv); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":190,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\"," + "\"arguments\":{\"pattern\":\"HandleRequest\"," + "\"project\":\"test-project\",\"limit\":0}}}"); + 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"); + ASSERT_NOT_NULL(results); + ASSERT_GT(yyjson_arr_size(results), 0); + + yyjson_doc_free(doc); + free(inner); + free(resp); + cleanup_snippet_dir(tmp); + cbm_mcp_server_free(srv); + PASS(); +} + /* issue #283: search_code with regex=true and a syntactically invalid pattern * must return an explicit error, not an empty result indistinguishable from a * legitimate no-match. */ @@ -2963,6 +3003,7 @@ SUITE(mcp) { RUN_TEST(tool_search_code_missing_pattern); RUN_TEST(tool_search_code_no_project); RUN_TEST(search_code_multi_word); + RUN_TEST(search_code_limit_zero_uses_config_default); RUN_TEST(search_code_invalid_regex_errors_issue283); RUN_TEST(search_code_literal_pipe_warns_issue282); RUN_TEST(search_code_ampersand_accepted_issue272);