mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-24 06:24:18 +02:00
Fix circular reference crash in OpenAPI schemas (#3245)
* Fix circular reference crash in OpenAPI schemas (#3242, #1206, #1016) * chore: Update SDK documentation --------- Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
This commit is contained in:
Jeremiah Lowin
GitHub
parent
5e2d698356
commit
1caf40eee8
4 changed files with 311 additions and 5 deletions
|
|
@ -7,7 +7,7 @@ sidebarTitle: json_schema
|
|||
|
||||
## Functions
|
||||
|
||||
### `dereference_refs` <sup><a href="https://github.com/PrefectHQ/fastmcp/blob/main/src/fastmcp/utilities/json_schema.py#L9" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
### `dereference_refs` <sup><a href="https://github.com/PrefectHQ/fastmcp/blob/main/src/fastmcp/utilities/json_schema.py#L56" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
|
||||
```python
|
||||
dereference_refs(schema: dict[str, Any]) -> dict[str, Any]
|
||||
|
|
@ -35,7 +35,7 @@ $defs for nested references.
|
|||
- when no longer needed
|
||||
|
||||
|
||||
### `resolve_root_ref` <sup><a href="https://github.com/PrefectHQ/fastmcp/blob/main/src/fastmcp/utilities/json_schema.py#L131" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
### `resolve_root_ref` <sup><a href="https://github.com/PrefectHQ/fastmcp/blob/main/src/fastmcp/utilities/json_schema.py#L185" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
|
||||
```python
|
||||
resolve_root_ref(schema: dict[str, Any]) -> dict[str, Any]
|
||||
|
|
@ -57,7 +57,7 @@ the referenced definition while preserving $defs for nested references.
|
|||
- if no resolution is needed
|
||||
|
||||
|
||||
### `compress_schema` <sup><a href="https://github.com/PrefectHQ/fastmcp/blob/main/src/fastmcp/utilities/json_schema.py#L364" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
### `compress_schema` <sup><a href="https://github.com/PrefectHQ/fastmcp/blob/main/src/fastmcp/utilities/json_schema.py#L418" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
|
||||
```python
|
||||
compress_schema(schema: dict[str, Any], prune_params: list[str] | None = None, prune_additional_properties: bool = False, prune_titles: bool = False, dereference: bool = False) -> dict[str, Any]
|
||||
|
|
|
|||
|
|
@ -6,6 +6,53 @@ from typing import Any
|
|||
from jsonref import JsonRefError, replace_refs
|
||||
|
||||
|
||||
def _defs_have_cycles(defs: dict[str, Any]) -> bool:
|
||||
"""Check whether any definitions in ``$defs`` form a reference cycle.
|
||||
|
||||
A cycle means a definition directly or transitively references itself
|
||||
(e.g. Node → children → Node, or A → B → A). ``jsonref.replace_refs``
|
||||
silently produces Python-level object cycles for these, which Pydantic's
|
||||
serializer rejects.
|
||||
"""
|
||||
if not defs:
|
||||
return False
|
||||
|
||||
# Build adjacency: def_name -> set of def_names it references.
|
||||
edges: dict[str, set[str]] = defaultdict(set)
|
||||
|
||||
def _collect_refs(obj: Any, source: str) -> None:
|
||||
if isinstance(obj, dict):
|
||||
ref = obj.get("$ref")
|
||||
if isinstance(ref, str) and ref.startswith("#/$defs/"):
|
||||
edges[source].add(ref.split("/")[-1])
|
||||
for v in obj.values():
|
||||
_collect_refs(v, source)
|
||||
elif isinstance(obj, list):
|
||||
for item in obj:
|
||||
_collect_refs(item, source)
|
||||
|
||||
for name, definition in defs.items():
|
||||
_collect_refs(definition, name)
|
||||
|
||||
# DFS cycle detection.
|
||||
UNVISITED, IN_STACK, DONE = 0, 1, 2
|
||||
state: dict[str, int] = defaultdict(int)
|
||||
|
||||
def _has_cycle(node: str) -> bool:
|
||||
state[node] = IN_STACK
|
||||
for neighbor in edges.get(node, ()):
|
||||
if neighbor not in defs:
|
||||
continue
|
||||
if state[neighbor] == IN_STACK:
|
||||
return True
|
||||
if state[neighbor] == UNVISITED and _has_cycle(neighbor):
|
||||
return True
|
||||
state[node] = DONE
|
||||
return False
|
||||
|
||||
return any(state[name] == UNVISITED and _has_cycle(name) for name in defs)
|
||||
|
||||
|
||||
def dereference_refs(schema: dict[str, Any]) -> dict[str, Any]:
|
||||
"""Resolve all $ref references in a JSON schema by inlining definitions.
|
||||
|
||||
|
|
@ -35,6 +82,13 @@ def dereference_refs(schema: dict[str, Any]) -> dict[str, Any]:
|
|||
>>> resolved = dereference_refs(schema)
|
||||
>>> # Result: {"properties": {"cat": {"enum": ["a", "b"], "type": "string", "default": "a"}}}
|
||||
"""
|
||||
# Circular $defs can't be fully inlined — jsonref.replace_refs produces
|
||||
# Python dicts with object-identity cycles that Pydantic's model_dump
|
||||
# rejects with "Circular reference detected (id repeated)".
|
||||
# Detect cycles up front and fall back to root-only resolution.
|
||||
if _defs_have_cycles(schema.get("$defs", {})):
|
||||
return resolve_root_ref(schema)
|
||||
|
||||
try:
|
||||
# Use jsonref to resolve all $ref references
|
||||
# proxies=False returns plain dicts (not proxy objects)
|
||||
|
|
|
|||
|
|
@ -116,8 +116,8 @@ def _replace_ref_with_defs(
|
|||
elif item_schema := schema.get("items"):
|
||||
schema["items"] = _replace_ref_with_defs(item_schema)
|
||||
for section in ["anyOf", "allOf", "oneOf"]:
|
||||
for i, item in enumerate(schema.get(section, [])):
|
||||
schema[section][i] = _replace_ref_with_defs(item)
|
||||
if section in schema:
|
||||
schema[section] = [_replace_ref_with_defs(item) for item in schema[section]]
|
||||
if additionalProperties := schema.get("additionalProperties"):
|
||||
if not isinstance(additionalProperties, bool):
|
||||
schema["additionalProperties"] = _replace_ref_with_defs(
|
||||
|
|
|
|||
|
|
@ -1,5 +1,8 @@
|
|||
"""Comprehensive tests for transitive and nested reference handling (Issue #1372)."""
|
||||
|
||||
import httpx
|
||||
|
||||
from fastmcp import FastMCP
|
||||
from fastmcp.utilities.openapi.models import (
|
||||
HTTPRoute,
|
||||
ParameterInfo,
|
||||
|
|
@ -9,6 +12,7 @@ from fastmcp.utilities.openapi.models import (
|
|||
from fastmcp.utilities.openapi.parser import parse_openapi_to_http_routes
|
||||
from fastmcp.utilities.openapi.schemas import (
|
||||
_combine_schemas_and_map_params,
|
||||
_replace_ref_with_defs,
|
||||
extract_output_schema_from_responses,
|
||||
)
|
||||
|
||||
|
|
@ -840,3 +844,251 @@ class TestTransitiveAndNestedReferences:
|
|||
assert "Person" in route.response_schemas
|
||||
assert "Name" in route.response_schemas
|
||||
assert "Job" in route.response_schemas
|
||||
|
||||
|
||||
class TestCircularReferencesSerialization:
|
||||
"""Tests for circular/self-referential schemas surviving MCP serialization.
|
||||
|
||||
Issues: #1016, #1206, #3242
|
||||
|
||||
The crash occurs when Pydantic's model_dump() encounters the same Python
|
||||
dict object at multiple positions in the serialization tree. This happens
|
||||
because _replace_ref_with_defs mutates shared list objects (anyOf/allOf/oneOf)
|
||||
in place via shallow copy, causing different tools to share internal dict
|
||||
references.
|
||||
"""
|
||||
|
||||
def test_replace_ref_with_defs_does_not_mutate_input(self):
|
||||
"""_replace_ref_with_defs must not mutate its input dict's lists."""
|
||||
schema = {
|
||||
"oneOf": [
|
||||
{"$ref": "#/components/schemas/Cat"},
|
||||
{"$ref": "#/components/schemas/Dog"},
|
||||
]
|
||||
}
|
||||
original_list = schema["oneOf"]
|
||||
original_items = list(original_list) # snapshot
|
||||
|
||||
_replace_ref_with_defs(schema)
|
||||
|
||||
# The original list object must not have been mutated
|
||||
assert original_list is schema["oneOf"]
|
||||
assert original_list == original_items
|
||||
|
||||
def test_replace_ref_with_defs_produces_independent_results(self):
|
||||
"""Calling _replace_ref_with_defs twice on the same input must produce
|
||||
independent dict trees with no shared mutable objects."""
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"pet": {
|
||||
"oneOf": [
|
||||
{"$ref": "#/components/schemas/Cat"},
|
||||
{"$ref": "#/components/schemas/Dog"},
|
||||
]
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
result1 = _replace_ref_with_defs(schema)
|
||||
result2 = _replace_ref_with_defs(schema)
|
||||
|
||||
# The oneOf lists should be different objects
|
||||
list1 = result1["properties"]["pet"]["oneOf"]
|
||||
list2 = result2["properties"]["pet"]["oneOf"]
|
||||
assert list1 is not list2
|
||||
|
||||
# Items within the lists should also be independent
|
||||
assert list1[0] is not list2[0]
|
||||
|
||||
def test_circular_output_schema_serialization(self):
|
||||
"""Output schemas with self-referential types must survive model_dump().
|
||||
|
||||
This is the exact crash from issue #3242: Pydantic raises
|
||||
ValueError('Circular reference detected (id repeated)') when
|
||||
serializing MCP Tool objects whose schemas share Python dict references.
|
||||
"""
|
||||
responses = {
|
||||
"200": ResponseInfo(
|
||||
description="A tree node",
|
||||
content_schema={
|
||||
"application/json": {"$ref": "#/components/schemas/Node"}
|
||||
},
|
||||
)
|
||||
}
|
||||
schema_definitions = {
|
||||
"Node": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": {"type": "string"},
|
||||
"children": {
|
||||
"type": "array",
|
||||
"items": {"$ref": "#/components/schemas/Node"},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
output_schema = extract_output_schema_from_responses(
|
||||
responses, schema_definitions=schema_definitions, openapi_version="3.0.0"
|
||||
)
|
||||
assert output_schema is not None
|
||||
|
||||
# Build an MCP Tool with this schema and try to serialize it —
|
||||
# this is the exact path that crashes in the reported issue.
|
||||
from mcp.types import Tool as MCPTool
|
||||
|
||||
tool = MCPTool(
|
||||
name="get_node",
|
||||
description="Get a node",
|
||||
inputSchema={"type": "object", "properties": {}},
|
||||
outputSchema=output_schema,
|
||||
)
|
||||
# This must not raise ValueError: Circular reference detected
|
||||
tool.model_dump(by_alias=True, mode="json", exclude_none=True)
|
||||
|
||||
def test_mutual_circular_references_serialization(self):
|
||||
"""Mutually circular schemas (A→B→A) must survive serialization."""
|
||||
responses = {
|
||||
"200": ResponseInfo(
|
||||
description="A pull request",
|
||||
content_schema={
|
||||
"application/json": {"$ref": "#/components/schemas/PullRequest"}
|
||||
},
|
||||
)
|
||||
}
|
||||
schema_definitions = {
|
||||
"PullRequest": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"title": {"type": "string"},
|
||||
"author": {"$ref": "#/components/schemas/User"},
|
||||
},
|
||||
},
|
||||
"User": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"pull_requests": {
|
||||
"type": "array",
|
||||
"items": {"$ref": "#/components/schemas/PullRequest"},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
output_schema = extract_output_schema_from_responses(
|
||||
responses, schema_definitions=schema_definitions, openapi_version="3.0.0"
|
||||
)
|
||||
assert output_schema is not None
|
||||
|
||||
from mcp.types import Tool as MCPTool
|
||||
|
||||
tool = MCPTool(
|
||||
name="get_pr",
|
||||
description="Get a pull request",
|
||||
inputSchema={"type": "object", "properties": {}},
|
||||
outputSchema=output_schema,
|
||||
)
|
||||
tool.model_dump(by_alias=True, mode="json", exclude_none=True)
|
||||
|
||||
async def test_multiple_tools_sharing_circular_schemas(self):
|
||||
"""Multiple tools from the same spec must not share Python dict objects
|
||||
in their schemas, which would cause Pydantic to raise circular reference
|
||||
errors when serializing the list_tools response."""
|
||||
spec = {
|
||||
"openapi": "3.0.0",
|
||||
"info": {"title": "Test API", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/nodes": {
|
||||
"get": {
|
||||
"operationId": "list_nodes",
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "List of nodes",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"$ref": "#/components/schemas/Node"
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
},
|
||||
"post": {
|
||||
"operationId": "create_node",
|
||||
"requestBody": {
|
||||
"required": True,
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {"$ref": "#/components/schemas/Node"}
|
||||
}
|
||||
},
|
||||
},
|
||||
"responses": {
|
||||
"201": {
|
||||
"description": "Created node",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {"$ref": "#/components/schemas/Node"}
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
},
|
||||
},
|
||||
"/nodes/{id}": {
|
||||
"get": {
|
||||
"operationId": "get_node",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": True,
|
||||
"schema": {"type": "string"},
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "A node",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {"$ref": "#/components/schemas/Node"}
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
"components": {
|
||||
"schemas": {
|
||||
"Node": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": {"type": "string"},
|
||||
"children": {
|
||||
"type": "array",
|
||||
"items": {"$ref": "#/components/schemas/Node"},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
server = FastMCP.from_openapi(spec, httpx.AsyncClient())
|
||||
tools = await server.list_tools()
|
||||
assert len(tools) >= 3
|
||||
|
||||
# Simulate what the MCP SDK does: serialize all tools together.
|
||||
# This is the exact crash path — model_dump on a list of tools
|
||||
# whose schemas share Python dict objects.
|
||||
|
||||
mcp_tools = [tool.to_mcp_tool(name=tool.name) for tool in tools]
|
||||
for mcp_tool in mcp_tools:
|
||||
mcp_tool.model_dump(by_alias=True, mode="json", exclude_none=True)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue