Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/mcp/server/mcpserver/utilities/func_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,25 @@
raise ValueError(f"JSON schema warning: {kind} - {detail}")


_LOCAL_DEFS_PREFIX = "#/$defs/"


def _inline_root_ref(schema: dict[str, Any]) -> dict[str, Any]:
"""Give a schema whose root is a bare `$ref` into `$defs` an inline root.

pydantic emits a self-referential model as `{"$defs": {...}, "$ref": "#/$defs/Model"}`, with no
`type` at the root; `Tool.outputSchema` needs an object root (required on the wire through
2025-11-25). The referenced definition is copied onto the root and `$defs` is kept, since nested
references still point into it. Root siblings of the `$ref` win over the definition's keys.
"""
ref = schema.get("$ref")
if not isinstance(ref, str) or not ref.startswith(_LOCAL_DEFS_PREFIX):
return schema
definition = cast(dict[str, Any], schema["$defs"][ref.removeprefix(_LOCAL_DEFS_PREFIX)])

Check warning on line 91 in src/mcp/server/mcpserver/utilities/func_metadata.py

View check run for this annotation

Claude / Claude Code Review

_inline_root_ref assumes a root `$ref` starting with `#/$defs/` always has a matching local definition; when it doesn't, the unguarded `schema["$defs"][ref.removeprefix(...)]` raises KeyError inside FuncMetadata.model_post_init, and KeyError is not in fun

_inline_root_ref assumes a root `$ref` starting with `#/$defs/` always has a matching local definition; when it doesn't, the unguarded `schema["$defs"][ref.removeprefix(...)]` raises KeyError inside FuncMetadata.model_post_init, and KeyError is not in func_metadata's except tuple (lines 451-459; pydantic wraps only ValueError/AssertionError from model_post_init into ValidationError), so tool registration crashes with a bare KeyError instead of falling back to unstructured output or raising Inval

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 _inline_root_ref assumes a root $ref starting with #/$defs/ always has a matching local definition; when it doesn't, the unguarded schema["$defs"][ref.removeprefix(...)] raises KeyError inside FuncMetadata.model_post_init, and KeyError is not in func_metadata's except tuple (lines 451-459; pydantic wraps only ValueError/AssertionError from model_post_init into ValidationError), so tool registration crashes with a bare KeyError instead of falling back to unstructured output or raising InvalidSignature. Before this diff such schemas registered and published as-is, so this is a registration-time regression for schema-override edge cases. Fix: guard with e.g. definition = schema.get("$defs", {}).get(name) and return the schema unchanged when the definition is absent (covers both a…

Extended reasoning...

A tool returns a BaseModel whose schema is customized to carry a root $ref without local defs — e.g. class Payload(BaseModel): model_config = ConfigDict(json_schema_extra={"$ref": "#/$defs/Payload"}) (mirroring an externally-managed schema), or a model whose __get_pydantic_json_schema__ returns {"$ref": "#/$defs/External"}. TypeAdapter(...).json_schema() then produces a root containing that $ref but no matching $defs entry. On @ mcp.tool() decoration, FuncMetadata construction calls _inline_root_ref, schema["$defs"] raises KeyError, which propagates raw out of model_post_init past the except tuple at func_metadata lines 451-459, crashing server startup with an unexplained KeyError: '$defs'. On the pre-diff code the same tool registered successfully and published its schema unchanged.

Verification: nit — src/mcp/server/mcpserver/utilities/func_metadata.py:91 definition = cast(dict[str, Any], schema["$defs"][ref.removeprefix(_LOCAL_DEFS_PREFIX)]) is guarded only by the prefix check on lines 89-90 (if not isinstance(ref, str) or not ref.startswith(_LOCAL_DEFS_PREFIX)); nothing verifies "$defs" exists or contains the referenced key, so a root "$ref": "#/$defs/X" without a matchi

siblings = {key: value for key, value in schema.items() if key != "$ref"}
return {**definition, **siblings}


class ArgModelBase(BaseModel):
"""A model representing the arguments to a function."""

Expand Down Expand Up @@ -108,7 +127,8 @@
def model_post_init(self, context: Any, /) -> None:
if self.output_model is not None and self.output_schema is None:
# StrictJsonSchema raises instead of warning, so an unserializable return type fails construction.
self.output_schema = self._output_adapter(self.output_model).json_schema(schema_generator=StrictJsonSchema)
schema = self._output_adapter(self.output_model).json_schema(schema_generator=StrictJsonSchema)
self.output_schema = _inline_root_ref(schema)

def _output_adapter(self, output_model: type[Any]) -> TypeAdapter[Any]:
"""The validator/serializer for `output_model`, built once and rebuilt only if the field is reassigned."""
Expand Down
28 changes: 28 additions & 0 deletions tests/server/mcpserver/test_func_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,34 @@ def func_nested() -> PersonWithAddress: # pragma: no cover
}


def test_structured_output_self_referential_model_gets_an_object_root():
"""pydantic publishes a recursive model as a bare root `$ref`; the definition is inlined onto the
root and `$defs` is kept for the nested reference."""

class Node(BaseModel):
name: str
children: list["Node"] = []

def tree() -> Node:
return Node(name="root", children=[Node(name="leaf")])

node_definition: dict[str, Any] = {
"properties": {
"name": {"title": "Name", "type": "string"},
"children": {"default": [], "items": {"$ref": "#/$defs/Node"}, "title": "Children", "type": "array"},
},
"required": ["name"],
"title": "Node",
"type": "object",
}
meta = func_metadata(tree)
assert meta.output_schema == {**node_definition, "$defs": {"Node": node_definition}}

result = meta.convert_result(tree())
assert isinstance(result, CallToolResult)
assert result.structured_content == {"name": "root", "children": [{"name": "leaf", "children": []}]}


def test_structured_output_unserializable_type_error():
"""Test error when structured_output=True is used with unserializable types"""

Expand Down
23 changes: 23 additions & 0 deletions tests/server/mcpserver/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2167,6 +2167,29 @@ async def briefing(ctx: Context) -> list[UserMessage] | InputRequiredResult:
assert exc.value.error.message == "Handler returned an invalid result"


async def test_recursive_tool_return_type_lists_and_calls_on_legacy_session():
"""A 2025-11-25 session requires `type: object` at the outputSchema root; a self-referential
return type must not fail the whole listing, and its result validates client-side via `$defs`."""

class Node(BaseModel):
name: str
children: list["Node"] = []

mcp = MCPServer()

@mcp.tool()
def tree() -> Node:
return Node(name="root", children=[Node(name="leaf")])

async with Client(mcp, mode="legacy") as client:
[tool] = (await client.list_tools()).tools
assert tool.output_schema is not None
assert tool.output_schema["type"] == "object"
assert tool.output_schema["properties"]["children"]["items"] == {"$ref": "#/$defs/Node"}
result = await client.call_tool("tree", {})
assert result.structured_content == {"name": "root", "children": [{"name": "leaf", "children": []}]}


async def test_resource_template_input_required_result_on_legacy_session_is_a_serialization_error():
"""Pins the shared era gate for resources/read: a pre-2026 session has no
input_required vocabulary, so the runner rejects the frame with -32603."""
Expand Down
Loading