From 798cc60f4c2c29c8edecc5584edba52085157294 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Mon, 14 Sep 2026 11:56:34 +0800 Subject: [PATCH] fix(mcp): repair schema-map keywords per-entry in _repair_object_shape _repair_object_shape recursed over every dict value as a schema node, including the properties map itself. When one of the map's keys was literally named properties/required, the missing-type heuristic fired on the map and injected a bogus "type": "object" string as a parameter, 400ing the whole tool array on strict providers. Recurse into map values only for the mapping-valued keywords (properties, patternProperties, $defs, definitions, dependentSchemas), matching the in-tree precedent in _rewrite_local_refs. Fixes #110530 --- tests/tools/test_mcp_tool.py | 59 ++++++++++++++++++++++++++++++++++++ tools/mcp_tool_schema.py | 15 ++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index f9a333af52..9968026be9 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -518,6 +518,65 @@ class TestSchemaConversion: assert "definitions" not in schema["parameters"] + def test_properties_map_entry_named_properties_is_not_injected_with_type(self): + """A ``properties`` map must be repaired per-entry, never as a schema node. + + Regression: ``_repair_object_shape`` recursed over every value as a + schema node, including the ``properties`` map itself. When one of its + KEYS was literally named ``properties``/``required``, the + missing-``type`` heuristic fired on the map and injected + ``"type": "object"`` — a bare string, not a schema — as a *parameter*. + Strict providers then 400 the whole tool array with + ``"object" is not of types "boolean", "object"``. Real-world repro: a + Tencent Docs MCP server whose ``smartsheet_add_table`` tool has a + parameter named ``properties`` (#110530). + """ + from tools.mcp_tool_schema import _normalize_mcp_input_schema + + normalized = _normalize_mcp_input_schema({ + "type": "object", + "properties": { + "file_id": {"type": "string"}, + "properties": { + "type": "object", + "properties": {"title": {"type": "string"}}, + }, + }, + }) + + props = normalized["properties"] + # No bogus "type" parameter was injected into the properties map itself. + assert set(props) == {"file_id", "properties"} + # The legitimately-named `properties` parameter keeps its schema shape. + assert props["properties"]["type"] == "object" + assert props["properties"]["properties"] == {"title": {"type": "string"}} + + # Same signature one level deeper (smartsheet add_view: items.properties map). + nested = _normalize_mcp_input_schema({ + "type": "object", + "properties": { + "condition_items": { + "type": "object", + "properties": { + "properties": {"type": "string"}, + "value": {"type": "string"}, + }, + }, + }, + }) + inner = nested["properties"]["condition_items"]["properties"] + assert set(inner) == {"properties", "value"} + + # ``$defs`` is a schema map too: an entry literally named ``properties`` must not + # gain a bogus ``type`` sibling inside the ``$defs`` map. + defs_case = _normalize_mcp_input_schema({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "$defs": {"properties": {"type": "string"}}, + }) + assert set(defs_case["$defs"]) == {"properties"} + + def test_optional_nullable_field_is_collapsed_to_non_null_schema(self): """Anthropic rejects MCP/Pydantic anyOf-null optional parameter schemas.""" from tools.mcp_tool_schema import _normalize_mcp_input_schema diff --git a/tools/mcp_tool_schema.py b/tools/mcp_tool_schema.py index e0967b0126..8248047360 100644 --- a/tools/mcp_tool_schema.py +++ b/tools/mcp_tool_schema.py @@ -66,6 +66,11 @@ def _rewrite_local_refs(node): return normalized +# Mapping-valued JSON Schema keywords: the value maps entry NAMES to schemas and is never a +# schema node itself. Repair must recurse into the map's values only (#110530). +_SCHEMA_MAP_KEYS = ("properties", "patternProperties", "$defs", "definitions", "dependentSchemas") + + def _repair_object_shape(node): """Recursively fill a missing object ``type``, ensure ``properties`` (so ``required`` can't dangle) and prune ``required`` to names present in ``properties`` (Gemini 400s @@ -74,7 +79,15 @@ def _repair_object_shape(node): return [_repair_object_shape(item) for item in node] if not isinstance(node, dict): return node - repaired = {k: _repair_object_shape(v) for k, v in node.items()} + repaired = {} + for key, value in node.items(): + if key in _SCHEMA_MAP_KEYS and isinstance(value, dict): + # A schema map (entry name -> schema), never a schema node itself: recursing over + # the whole map would inject a bogus ``"type": "object"`` *entry* when one of its + # keys is literally named ``properties``/``required`` (#110530). + repaired[key] = {name: _repair_object_shape(schema) for name, schema in value.items()} + else: + repaired[key] = _repair_object_shape(value) if not repaired.get("type") and ("properties" in repaired or "required" in repaired): repaired["type"] = "object" if repaired.get("type") == "object":