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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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":
|
||||
|
||||
Reference in New Issue
Block a user