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 <ATHundt@gmail.com>
This commit is contained in:
Andrew Hundt
2026-06-30 07:40:19 -04:00
parent 26f574c215
commit 4fb9c8c6e6
2 changed files with 56 additions and 12 deletions
+15 -12
View File
@@ -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 —
+41
View File
@@ -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);