diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 2d6e93d79..f0784bc98 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1063,6 +1063,9 @@ static const tool_def_t TOOLS[] = { "\"persistence\":{\"type\":\"boolean\",\"default\":false,\"description\":" "\"Write compressed artifact to .codebase-memory/graph.db.zst for team sharing. " "Teammates can bootstrap from the artifact instead of full re-indexing.\"}" + ",\"format\":{\"type\":\"string\",\"enum\":[\"toon\",\"json\"],\"default\":\"toon\"," + "\"description\":\"Compact TOON by default; json returns legacy objects. Omit to use " + "default_response_format.\"}" "},\"required\":[\"repo_path\"]}"}, {"search_graph", "Search graph", @@ -1315,7 +1318,9 @@ static const tool_def_t TOOLS[] = { "overlay compaction state. Git metadata labels HEAD as committed-only and reports whether " "the current working tree is dirty.", "{\"type\":\"object\",\"properties\":{\"project\":{\"type\":\"string\",\"description\":" - "\"Indexed project name to inspect.\"}},\"required\":[" + "\"Indexed project name to inspect.\"},\"verbose\":{\"type\":\"boolean\",\"default\":false," + "\"description\":\"Include repository path and Git identity/freshness " + "metadata.\"}},\"required\":[" "\"project\"]}"}, {"check_index_coverage", "Check index coverage", @@ -1434,6 +1439,7 @@ static const int STREAMLINED_TOOL_COUNT = sizeof(STREAMLINED_TOOLS) / sizeof(STR static const char MCP_TOOL_OUTPUT_SCHEMA[] = "{\"type\":\"object\",\"additionalProperties\":true}"; +static const char MCP_HIDDEN_TOOL_INPUT_SCHEMA[] = "{\"type\":\"object\",\"properties\":{}}"; typedef struct { const char *name; @@ -1492,6 +1498,30 @@ static void mcp_add_json_schema(yyjson_mut_doc *doc, yyjson_mut_val *obj, const yyjson_doc_free(schema_doc); } +/* MCP inputs are closed globally: an undeclared key is almost always a typo + * that would otherwise leave a real option at its default and return a + * plausible but wrong result. Keep the property/required definitions in the + * canonical registry and apply this policy during serialization rather than + * duplicating it in every schema literal. Runtime dispatch enforces the same + * policy for raw-JSON CLI and clients that do not validate tools/list. */ +static void mcp_add_tool_input_schema(yyjson_mut_doc *doc, yyjson_mut_val *obj, + const char *schema_json) { + if (!schema_json) { + return; + } + yyjson_doc *schema_doc = yyjson_read(schema_json, strlen(schema_json), 0); + if (!schema_doc) { + return; + } + yyjson_mut_val *schema = yyjson_val_mut_copy(doc, yyjson_doc_get_root(schema_doc)); + if (schema && yyjson_mut_is_obj(schema)) { + (void)yyjson_mut_obj_remove_key(schema, "additionalProperties"); + yyjson_mut_obj_add_bool(doc, schema, "additionalProperties", false); + yyjson_mut_obj_add_val(doc, obj, "inputSchema", schema); + } + yyjson_doc_free(schema_doc); +} + /* Canonical tool serialization for classic, streamlined, and paged lists. */ static void emit_tool(yyjson_mut_doc *doc, yyjson_mut_val *tools, const tool_def_t *tool_def, const char *description_override) { @@ -1501,7 +1531,7 @@ static void emit_tool(yyjson_mut_doc *doc, yyjson_mut_val *tools, const tool_def tool_def->title ? tool_def->title : tool_def->name); yyjson_mut_obj_add_str(doc, tool, "description", description_override ? description_override : tool_def->description); - mcp_add_json_schema(doc, tool, "inputSchema", tool_def->input_schema); + mcp_add_tool_input_schema(doc, tool, tool_def->input_schema); mcp_add_json_schema(doc, tool, "outputSchema", MCP_TOOL_OUTPUT_SCHEMA); const tool_annotation_def_t *def = mcp_tool_annotations(tool_def->name); yyjson_mut_val *annotations = yyjson_mut_obj(doc); @@ -1526,7 +1556,9 @@ static bool is_streamlined_default_tool(const char *name) { strcmp(name, "trace_path") == 0); } -/* Return the same schema advertised by tools/list. Static lifetime; do not free. */ +/* Return the canonical property and required definitions used by tools/list, + * CLI flag typing, and runtime key validation. tools/list additionally applies + * the global closed-input policy. Static lifetime; do not free. */ const char *cbm_mcp_tool_input_schema(const char *tool_name) { if (!tool_name) { return NULL; @@ -1536,6 +1568,9 @@ const char *cbm_mcp_tool_input_schema(const char *tool_name) { if (strcmp(tool_name, "trace_call_path") == 0) { tool_name = "trace_path"; } + if (strcmp(tool_name, "_hidden_tools") == 0) { + return MCP_HIDDEN_TOOL_INPUT_SCHEMA; + } for (int i = 0; i < TOOL_COUNT; i++) { if (strcmp(TOOLS[i].name, tool_name) == 0) { return TOOLS[i].input_schema; @@ -1550,8 +1585,7 @@ const char *cbm_mcp_tool_input_schema(const char *tool_name) { } static bool mcp_tool_name_is_known(const char *tool_name) { - return cbm_mcp_tool_input_schema(tool_name) != NULL || - (tool_name && strcmp(tool_name, "_hidden_tools") == 0); + return cbm_mcp_tool_input_schema(tool_name) != NULL; } static bool mcp_tool_allowed(cbm_mcp_tool_profile_t profile, const char *name) { @@ -3001,6 +3035,7 @@ static char *cbm_mcp_tools_list_range(cbm_mcp_server_t *srv, int offset, int lim yyjson_mut_obj_add_str(doc, hint_schema, "type", "object"); yyjson_mut_val *hint_props = yyjson_mut_obj(doc); yyjson_mut_obj_add_val(doc, hint_schema, "properties", hint_props); + yyjson_mut_obj_add_bool(doc, hint_schema, "additionalProperties", false); yyjson_mut_obj_add_val(doc, hint_tool, "inputSchema", hint_schema); mcp_add_json_schema(doc, hint_tool, "outputSchema", MCP_TOOL_OUTPUT_SCHEMA); yyjson_mut_arr_add_val(tools, hint_tool); @@ -14951,6 +14986,129 @@ static char *build_hidden_tools_payload(cbm_mcp_server_t *srv) { return out ? out : heap_strdup("{\"error\":\"failed to serialize hidden tools payload\"}"); } +/* Compatibility spellings accepted by older raw-JSON callers but deliberately + * omitted from tools/list and CLI help. Keep this list narrow and semantic: + * each entry must map unambiguously to a current canonical input. + * + * History: 3d133a3c added search_in to the former combined search tool; + * 39539eff split graph and source search into search_graph and search_code. + * Existing callers/tests still pass search_in="source", which is an exact + * no-op spelling of search_code's contract. 32a3820c accepted `cypher` before + * the query_graph schema standardized on `query`; those strings are exact + * aliases. Other values/keys are ambiguous and must fail with the canonical + * spelling or tool. Remove an exception only in an announced compatibility + * break after usage evidence shows it is unused. */ +static bool mcp_legacy_argument_is_valid(const char *tool_name, yyjson_val *properties, + yyjson_val *args, const char *key, + bool *out_invalid_search_in) { + if (out_invalid_search_in) { + *out_invalid_search_in = false; + } + if (!tool_name || !properties || !args || !key) { + return false; + } + yyjson_val *value = yyjson_obj_get(args, key); + if (yyjson_obj_get(properties, "project") && yyjson_is_str(value) && + (strcmp(key, "project_name") == 0 || strcmp(key, "project_id") == 0 || + strcmp(key, "projectName") == 0)) { + return true; + } + if (strcmp(tool_name, "search_code") == 0 && strcmp(key, "search_in") == 0) { + bool is_source = yyjson_is_str(value) && strcmp(yyjson_get_str(value), "source") == 0; + if (!is_source && out_invalid_search_in) { + *out_invalid_search_in = true; + } + return is_source; + } + if (strcmp(tool_name, "query_graph") == 0 && strcmp(key, "cypher") == 0) { + return yyjson_is_str(value); + } + return false; +} + +static char *mcp_validate_tool_argument_keys(const char *tool_name, const char *args_json) { + const char *schema_json = cbm_mcp_tool_input_schema(tool_name); + if (!schema_json) { + return NULL; /* unknown-tool handling remains a protocol-level authority */ + } + + const char *effective_args = args_json ? args_json : "{}"; + yyjson_doc *args_doc = yyjson_read(effective_args, strlen(effective_args), 0); + yyjson_val *args = args_doc ? yyjson_doc_get_root(args_doc) : NULL; + if (!args || !yyjson_is_obj(args)) { + yyjson_doc_free(args_doc); + return cbm_mcp_text_result( + "tool arguments must be a JSON object; use tools/list or 'cli --help' " + "for the supported arguments", + true); + } + + yyjson_doc *schema_doc = yyjson_read(schema_json, strlen(schema_json), 0); + yyjson_val *schema = schema_doc ? yyjson_doc_get_root(schema_doc) : NULL; + yyjson_val *properties = schema ? yyjson_obj_get(schema, "properties") : NULL; + if (!properties || !yyjson_is_obj(properties)) { + yyjson_doc_free(schema_doc); + yyjson_doc_free(args_doc); + return cbm_mcp_text_result("tool input schema is unavailable; retry after reinstalling", + true); + } + + const char *unknown = NULL; + bool invalid_search_in = false; + yyjson_obj_iter arg_iter; + yyjson_obj_iter_init(args, &arg_iter); + yyjson_val *arg_key; + while ((arg_key = yyjson_obj_iter_next(&arg_iter)) != NULL) { + const char *key = yyjson_get_str(arg_key); + bool accepted = key && yyjson_obj_get(properties, key); + if (!accepted) { + accepted = + mcp_legacy_argument_is_valid(tool_name, properties, args, key, &invalid_search_in); + } + if (!accepted) { + unknown = key ? key : ""; + break; + } + } + + char *error_result = NULL; + if (invalid_search_in) { + error_result = cbm_mcp_text_result( + "search_code supports source text search directly: omit search_in or use " + "search_in='source' for legacy callers; use search_graph for graph search", + true); + } else if (unknown) { + char supported[CBM_SZ_1K] = ""; + size_t used = 0; + yyjson_obj_iter prop_iter; + yyjson_obj_iter_init(properties, &prop_iter); + yyjson_val *prop_key; + while ((prop_key = yyjson_obj_iter_next(&prop_iter)) != NULL) { + const char *name = yyjson_get_str(prop_key); + if (!name) { + continue; + } + int written = snprintf(supported + used, sizeof(supported) - used, "%s%s", + used ? ", " : "", name); + if (written < 0 || (size_t)written >= sizeof(supported) - used) { + break; + } + used += (size_t)written; + } + char message[CBM_SZ_2K]; + snprintf(message, sizeof(message), + "unknown argument '%.*s' for tool '%.*s'; supported arguments: %s. " + "Use tools/list or 'cli %.*s --help' for types and descriptions.", + CBM_SZ_256, unknown, CBM_SZ_256, tool_name, supported[0] ? supported : "(none)", + CBM_SZ_256, tool_name); + error_result = cbm_mcp_text_result(message, true); + } + + yyjson_doc_free(schema_doc); + yyjson_doc_free(args_doc); + return error_result; +} + char *cbm_mcp_handle_tool(cbm_mcp_server_t *srv, const char *tool_name, const char *args_json) { if (!tool_name) { return cbm_mcp_text_result( @@ -14965,6 +15123,10 @@ char *cbm_mcp_handle_tool(cbm_mcp_server_t *srv, const char *tool_name, const ch tool_name, mcp_tool_profile_name(srv->tool_profile)); return cbm_mcp_text_result(message, true); } + char *argument_error = mcp_validate_tool_argument_keys(tool_name, args_json); + if (argument_error) { + return argument_error; + } /* Streamlined alias: get_code → get_code_snippet handler. * (The 3 search tools dispatch by their real names below.) */ diff --git a/tests/test_mcp.c b/tests/test_mcp.c index eb23ebc35..0863f9df2 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -558,6 +558,58 @@ TEST(mcp_tools_list_latest_metadata) { PASS(); } +TEST(mcp_tool_input_schemas_are_closed_in_classic_and_streamlined_modes) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + char *revealed = cbm_mcp_handle_tool(srv, "_hidden_tools", "{}"); + ASSERT_NOT_NULL(revealed); + ASSERT_NULL(strstr(revealed, "\"isError\":true")); + free(revealed); + + char *snapshots[] = {mcp_tools_list_classic_snapshot(), cbm_mcp_tools_list(NULL), + cbm_mcp_tools_list(srv)}; + for (size_t si = 0; si < sizeof(snapshots) / sizeof(snapshots[0]); si++) { + ASSERT_NOT_NULL(snapshots[si]); + yyjson_doc *doc = yyjson_read(snapshots[si], strlen(snapshots[si]), 0); + ASSERT_NOT_NULL(doc); + yyjson_val *tools = yyjson_obj_get(yyjson_doc_get_root(doc), "tools"); + ASSERT_TRUE(yyjson_is_arr(tools)); + yyjson_arr_iter iter; + yyjson_arr_iter_init(tools, &iter); + yyjson_val *tool; + while ((tool = yyjson_arr_iter_next(&iter)) != NULL) { + yyjson_val *schema = yyjson_obj_get(tool, "inputSchema"); + ASSERT_TRUE(yyjson_is_obj(schema)); + yyjson_val *closed = yyjson_obj_get(schema, "additionalProperties"); + ASSERT_TRUE(yyjson_is_bool(closed)); + ASSERT_FALSE(yyjson_get_bool(closed)); + } + yyjson_doc_free(doc); + free(snapshots[si]); + } + cbm_mcp_server_free(srv); + PASS(); +} + +TEST(mcp_canonical_input_schemas_cover_implemented_format_and_verbose_options) { + struct { + const char *tool; + const char *property; + } cases[] = {{"index_repository", "format"}, {"index_status", "verbose"}}; + + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + const char *schema_json = cbm_mcp_tool_input_schema(cases[i].tool); + ASSERT_NOT_NULL(schema_json); + yyjson_doc *doc = yyjson_read(schema_json, strlen(schema_json), 0); + ASSERT_NOT_NULL(doc); + yyjson_val *properties = yyjson_obj_get(yyjson_doc_get_root(doc), "properties"); + ASSERT_TRUE(yyjson_is_obj(properties)); + ASSERT_NOT_NULL(yyjson_obj_get(properties, cases[i].property)); + yyjson_doc_free(doc); + } + PASS(); +} + TEST(mcp_tools_have_behavior_annotations) { struct { const char *name; @@ -1791,6 +1843,73 @@ TEST(tool_unknown_tool) { PASS(); } +TEST(tool_unknown_argument_is_actionable_execution_error) { + cbm_mcp_server_t *srv = setup_mcp_with_data(); + + char *direct = cbm_mcp_handle_tool( + srv, "search_code", + "{\"pattern\":\"HandleOrder\",\"repo_path\":\"/tmp/not-a-project-argument\"}"); + ASSERT_NOT_NULL(direct); + ASSERT_NOT_NULL(strstr(direct, "\"isError\":true")); + ASSERT_NOT_NULL(strstr(direct, "repo_path")); + ASSERT_NOT_NULL(strstr(direct, "project")); + ASSERT_NOT_NULL(strstr(direct, "supported")); + free(direct); + + char *framed = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":1201,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\",\"arguments\":{" + "\"pattern\":\"HandleOrder\",\"repo_path\":\"/tmp/not-a-project-argument\"}}}"); + ASSERT_NOT_NULL(framed); + /* MCP input validation is a Tool Execution Error so a model receives the + * actionable correction; malformed tools/call envelopes remain protocol errors. */ + ASSERT_NOT_NULL(strstr(framed, "\"result\"")); + ASSERT_NOT_NULL(strstr(framed, "\"isError\":true")); + ASSERT_NULL(strstr(framed, "\"code\":-32602")); + ASSERT_NOT_NULL(strstr(framed, "repo_path")); + ASSERT_NOT_NULL(strstr(framed, "project")); + free(framed); + + cbm_mcp_server_free(srv); + PASS(); +} + +TEST(tool_search_code_legacy_search_in_is_bounded_and_actionable) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + + char *response = + cbm_mcp_handle_tool(srv, "search_code", "{\"pattern\":\"needle\",\"search_in\":\"graph\"}"); + ASSERT_NOT_NULL(response); + ASSERT_NOT_NULL(strstr(response, "\"isError\":true")); + ASSERT_NOT_NULL(strstr(response, "search_graph")); + ASSERT_NOT_NULL(strstr(response, "omit search_in")); + free(response); + + cbm_mcp_server_free(srv); + PASS(); +} + +TEST(tool_query_graph_legacy_cypher_alias_remains_bounded) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + + char *response = cbm_mcp_handle_tool( + srv, "query_graph", "{\"cypher\":\"MATCH (n) RETURN n LIMIT 1\"}"); + ASSERT_NOT_NULL(response); + ASSERT_NULL(strstr(response, "unknown argument 'cypher'")); + free(response); + + response = cbm_mcp_handle_tool(srv, "query_graph", "{\"label\":\"Function\"}"); + ASSERT_NOT_NULL(response); + ASSERT_NOT_NULL(strstr(response, "unknown argument 'label'")); + ASSERT_NOT_NULL(strstr(response, "query")); + free(response); + + cbm_mcp_server_free(srv); + PASS(); +} + TEST(tool_search_graph_basic) { cbm_mcp_server_t *srv = setup_mcp_with_data(); @@ -12299,6 +12418,8 @@ SUITE(mcp) { RUN_TEST(mcp_tools_list); RUN_TEST(mcp_tools_list_classic_mode); RUN_TEST(mcp_tools_list_latest_metadata); + RUN_TEST(mcp_tool_input_schemas_are_closed_in_classic_and_streamlined_modes); + RUN_TEST(mcp_canonical_input_schemas_cover_implemented_format_and_verbose_options); RUN_TEST(mcp_tools_have_behavior_annotations); RUN_TEST(mcp_index_repository_declares_name_override_issue571); RUN_TEST(mcp_tools_array_schemas_have_items); @@ -12367,6 +12488,9 @@ SUITE(mcp) { RUN_TEST(tool_get_graph_schema_uses_ready_overlay_schema); RUN_TEST(first_response_context_uses_ready_overlay_schema); RUN_TEST(tool_unknown_tool); + RUN_TEST(tool_unknown_argument_is_actionable_execution_error); + RUN_TEST(tool_search_code_legacy_search_in_is_bounded_and_actionable); + RUN_TEST(tool_query_graph_legacy_cypher_alias_remains_bounded); RUN_TEST(tool_search_graph_basic); RUN_TEST(tool_trace_totals_respect_test_filter); RUN_TEST(tool_get_architecture_cycles_detects_scc);