Fix: resolve root-level $ref in outputSchema for MCP spec compliance#2720
Conversation
MCP specification requires outputSchema to have "type": "object" at root. Pydantic generates schemas with $ref at root for self-referential models, which violates this requirement and causes MCP clients to reject tools. Added resolve_root_ref() to inline referenced definitions at root level while preserving $defs for nested references. Closes PrefectHQ#2455 🤖 Generated with Claude Code Signed-off-by: majiayu000 <1835304752@qq.com>
marvin-context-protocol
Bot
added
bug
WalkthroughThis PR implements a utility function Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/tools/test_tool.pyis excluded by none and included by nonetests/utilities/test_json_schema.pyis excluded by none and included by none
📒 Files selected for processing (2)
src/fastmcp/tools/tool.pysrc/fastmcp/utilities/json_schema.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/tools/tool.pysrc/fastmcp/utilities/json_schema.py
🧬 Code graph analysis (1)
src/fastmcp/tools/tool.py (1)
src/fastmcp/utilities/json_schema.py (2)
compress_schema(240-270)resolve_root_ref(7-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (3)
src/fastmcp/utilities/json_schema.py (1)
4-5: LGTM! Type import added for annotations.The
Anytype import properly supports the type annotations in the newresolve_root_reffunction.src/fastmcp/tools/tool.py (2)
36-36: LGTM! Import addition for root reference resolution.The import of
resolve_root_refis correctly added alongside the existingcompress_schemaimport.
639-641: LGTM! Root reference resolution correctly positioned.The call to
resolve_root_refis placed exactly where specified in the PR objectives—aftercompress_schemaand before returning the parsed function. This ensures that:
- Self-referential Pydantic models have their root-level
$refinlined- The resulting
output_schemaconforms to MCP spec withtype: "object"at the root- Nested
$defsare preserved for internal referencesThe placement ensures downstream validation in
FunctionTool.from_function(lines 430-434) receives a fully resolved schema.
| def resolve_root_ref(schema: dict[str, Any]) -> dict[str, Any]: | ||
| """Resolve $ref at root level to meet MCP spec requirements. | ||
|
|
||
| MCP specification requires outputSchema to have "type": "object" at the root level. | ||
| When Pydantic generates schemas for self-referential models, it uses $ref at the | ||
| root level pointing to $defs. This function resolves such references by inlining | ||
| the referenced definition while preserving $defs for nested references. | ||
|
|
||
| Args: | ||
| schema: JSON schema dict that may have $ref at root level | ||
|
|
||
| Returns: | ||
| A new schema dict with root-level $ref resolved, or the original schema | ||
| if no resolution is needed | ||
|
|
||
| Example: | ||
| >>> schema = { | ||
| ... "$defs": {"Node": {"type": "object", "properties": {...}}}, | ||
| ... "$ref": "#/$defs/Node" | ||
| ... } | ||
| >>> resolved = resolve_root_ref(schema) | ||
| >>> # Result: {"type": "object", "properties": {...}, "$defs": {...}} | ||
| """ | ||
| # Only resolve if we have $ref at root level with $defs but no explicit type | ||
| if "$ref" in schema and "$defs" in schema and "type" not in schema: | ||
| ref = schema["$ref"] | ||
| # Only handle local $defs references | ||
| if isinstance(ref, str) and ref.startswith("#/$defs/"): | ||
| def_name = ref.split("/")[-1] | ||
| defs = schema["$defs"] | ||
| if def_name in defs: | ||
| # Create a new schema by copying the referenced definition | ||
| resolved = dict(defs[def_name]) | ||
| # Preserve $defs for nested references (other fields may still use them) | ||
| resolved["$defs"] = defs | ||
| return resolved | ||
| return schema |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Find _is_object_schema function and its implementation
rg -nP -A10 'def _is_object_schema' --type=py
# Step 2: Find where output schemas are validated
rg -nP -C5 'Output schemas must represent object types' --type=py
# Step 3: Find external $ref patterns in tests
rg -nP '\$ref.*http' --type=pyRepository: jlowin/fastmcp
Length of output: 1658
🏁 Script executed:
#!/bin/bash
# Step 4: Find FunctionTool.from_function to understand where validation happens
rg -nP -B5 -A15 'class FunctionTool' --type=py | head -100
# Step 5: Find all usages of resolve_root_ref
rg -nP 'resolve_root_ref' --type=py
# Step 6: Check for validation of non-local references
rg -nP 'startswith.*http|external.*ref' --type=pyRepository: jlowin/fastmcp
Length of output: 3241
🏁 Script executed:
#!/bin/bash
# Step 7: Get the full context of resolve_root_ref function usage
cat -n src/fastmcp/utilities/json_schema.py | head -60Repository: jlowin/fastmcp
Length of output: 2834
🏁 Script executed:
#!/bin/bash
# Step 8: Look for validation tests related to JSON schema
fd -e py -path '*/test*' | xargs grep -l 'resolve_root_ref\|_is_object_schema' 2>/dev/null || echo "No test files found with these functions"Repository: jlowin/fastmcp
Length of output: 284
🏁 Script executed:
#!/bin/bash
# Get the full _is_object_schema function
rg -nP -A20 'def _is_object_schema' src/fastmcp/tools/tool.pyRepository: jlowin/fastmcp
Length of output: 769
🏁 Script executed:
#!/bin/bash
# Get context around line 641 where resolve_root_ref is called
sed -n '630,650p' src/fastmcp/tools/tool.pyRepository: jlowin/fastmcp
Length of output: 1043
🏁 Script executed:
#!/bin/bash
# Understand the full validation context in from_function
sed -n '420,445p' src/fastmcp/tools/tool.pyRepository: jlowin/fastmcp
Length of output: 1224
🏁 Script executed:
#!/bin/bash
# Check if there are any tests or other validation for non-local $ref
rg -nP 'http.*\$ref|external.*ref|non.*local' --type=py src/Repository: jlowin/fastmcp
Length of output: 1988
🏁 Script executed:
#!/bin/bash
# Look for any other schema validation beyond _is_object_schema
rg -nP 'validate.*schema|schema.*validat' --type=py src/fastmcp/tools/tool.py | head -20Repository: jlowin/fastmcp
Length of output: 40
Non-local $ref references bypass validation, violating MCP spec requirements.
The implementation correctly handles local #/$defs/ references. However, _is_object_schema (line 509 in tool.py) returns True for ANY schema with both "$ref" and "$defs" present, regardless of whether the reference is local or external. When resolve_root_ref returns an unresolved schema (lines 34-43), the validation check passes even though the schema still has an unresolved $ref at root instead of the required "type": "object".
Schemas with non-local $ref (external URLs or invalid pointers) should either:
- Be explicitly rejected in
resolve_root_refwith a clear error message, or - Have
_is_object_schemavalidate that any$refwas actually resolved before accepting it
Without this safeguard, downstream code may receive schemas that violate the MCP spec requirement for "type": "object" at the root level.
|
The Windows test failure appears to be a flaky test issue unrelated to this PR's changes. The failing tests ( Could you please re-run the CI or confirm if this is a known flaky test? |
|
Happy to help! |
2 participants
Summary
Test plan
Closes #2455
🤖 Generated with Claude Code