mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-22 05:24:18 +02:00
Fix compress_schema to preserve additionalProperties: false for MCP compatibility (#3102)
Changes: - Changed default of prune_additional_properties from True to False in compress_schema - Added test demonstrating MCP client compatibility requirement - Updated existing tests to explicitly enable pruning when needed - Added additionalProperties: false to manually constructed schemas in tool_transform - Updated inline snapshots to reflect new behavior Fixes #3008 Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Jeremiah Lowin <jlowin@users.noreply.github.com>
This commit is contained in:
parent
b8d789c1b4
commit
6a358902f2
5 changed files with 72 additions and 7 deletions
|
|
@ -685,6 +685,7 @@ class TransformedTool(Tool):
|
|||
"type": "object",
|
||||
"properties": new_props,
|
||||
"required": list(new_required),
|
||||
"additionalProperties": False,
|
||||
}
|
||||
|
||||
if parent_defs:
|
||||
|
|
@ -868,6 +869,7 @@ class TransformedTool(Tool):
|
|||
"type": "object",
|
||||
"properties": merged_props,
|
||||
"required": list(final_required),
|
||||
"additionalProperties": False,
|
||||
}
|
||||
|
||||
if merged_defs:
|
||||
|
|
|
|||
|
|
@ -364,7 +364,7 @@ def _single_pass_optimize(
|
|||
def compress_schema(
|
||||
schema: dict[str, Any],
|
||||
prune_params: list[str] | None = None,
|
||||
prune_additional_properties: bool = True,
|
||||
prune_additional_properties: bool = False,
|
||||
prune_titles: bool = False,
|
||||
) -> dict[str, Any]:
|
||||
"""
|
||||
|
|
@ -378,7 +378,9 @@ def compress_schema(
|
|||
Args:
|
||||
schema: The schema to compress
|
||||
prune_params: List of parameter names to remove from properties
|
||||
prune_additional_properties: Whether to remove additionalProperties: false
|
||||
prune_additional_properties: Whether to remove additionalProperties: false.
|
||||
Defaults to False to maintain MCP client compatibility, as some clients
|
||||
(e.g., Claude) require additionalProperties: false for strict validation.
|
||||
prune_titles: Whether to remove title fields from the schema
|
||||
"""
|
||||
# Dereference $ref - this inlines all definitions and removes $defs
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ class TestToolFromFunction:
|
|||
"description": "Add two numbers.",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {
|
||||
"a": {"type": "integer"},
|
||||
"b": {"type": "integer"},
|
||||
|
|
@ -83,6 +84,7 @@ class TestToolFromFunction:
|
|||
"description": "Fetch data from URL.",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {"url": {"type": "string"}},
|
||||
"required": ["url"],
|
||||
"type": "object",
|
||||
|
|
@ -117,6 +119,7 @@ class TestToolFromFunction:
|
|||
"description": "Adds two numbers.",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {
|
||||
"x": {"type": "integer"},
|
||||
"y": {"type": "integer"},
|
||||
|
|
@ -153,6 +156,7 @@ class TestToolFromFunction:
|
|||
"description": "Adds two numbers.",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {
|
||||
"x": {"type": "integer"},
|
||||
"y": {"type": "integer"},
|
||||
|
|
@ -192,6 +196,7 @@ class TestToolFromFunction:
|
|||
"description": "Create a new user.",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {
|
||||
"user": {
|
||||
"properties": {
|
||||
|
|
@ -270,6 +275,7 @@ class TestToolFromFunction:
|
|||
"name": "my_tool",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {"x": {"title": "X"}},
|
||||
"required": ["x"],
|
||||
"type": "object",
|
||||
|
|
@ -302,6 +308,7 @@ class TestToolFromFunction:
|
|||
"description": "Add two numbers.",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {
|
||||
"_a": {"type": "integer"},
|
||||
"_b": {"type": "integer"},
|
||||
|
|
@ -353,6 +360,7 @@ class TestToolFromFunction:
|
|||
"description": "Add two numbers.",
|
||||
"tags": set(),
|
||||
"parameters": {
|
||||
"additionalProperties": False,
|
||||
"properties": {
|
||||
"x": {"type": "integer"},
|
||||
"y": {"type": "integer"},
|
||||
|
|
|
|||
|
|
@ -383,6 +383,7 @@ class TestInputSchema:
|
|||
"field2": {"type": "boolean"},
|
||||
},
|
||||
"required": [],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -424,6 +425,7 @@ class TestInputSchema:
|
|||
}
|
||||
},
|
||||
"required": ["used_param"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -464,6 +466,7 @@ class TestInputSchema:
|
|||
}
|
||||
},
|
||||
"required": ["renamed_input"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -508,6 +511,7 @@ class TestInputSchema:
|
|||
},
|
||||
},
|
||||
"required": IsList("param_b", "param_a", check_order=False),
|
||||
"additionalProperties": False,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -530,5 +534,6 @@ class TestInputSchema:
|
|||
}
|
||||
},
|
||||
"required": ["param_a"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
)
|
||||
|
|
|
|||
|
|
@ -228,13 +228,14 @@ class TestCompressSchema:
|
|||
assert result["required"] == ["bar"]
|
||||
|
||||
def test_pruning_additional_properties(self):
|
||||
"""Test pruning additionalProperties when False."""
|
||||
"""Test pruning additionalProperties when explicitly enabled."""
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {"foo": {"type": "string"}},
|
||||
"additionalProperties": False,
|
||||
}
|
||||
result = compress_schema(schema)
|
||||
# Must explicitly enable pruning now (default changed for MCP compatibility)
|
||||
result = compress_schema(schema, prune_additional_properties=True)
|
||||
assert "additionalProperties" not in result
|
||||
|
||||
def test_disable_pruning_additional_properties(self):
|
||||
|
|
@ -263,7 +264,9 @@ class TestCompressSchema:
|
|||
"unused_def": {"type": "number"},
|
||||
},
|
||||
}
|
||||
result = compress_schema(schema, prune_params=["remove"])
|
||||
result = compress_schema(
|
||||
schema, prune_params=["remove"], prune_additional_properties=True
|
||||
)
|
||||
# Check that parameter was removed
|
||||
assert "remove" not in result["properties"]
|
||||
# Check that required list was updated
|
||||
|
|
@ -296,7 +299,7 @@ class TestCompressSchema:
|
|||
assert "title" not in result["properties"]["bar"]["properties"]["nested"]
|
||||
|
||||
def test_prune_nested_additional_properties(self):
|
||||
"""Test pruning additionalProperties: false at all levels."""
|
||||
"""Test pruning additionalProperties: false at all levels when explicitly enabled."""
|
||||
schema = {
|
||||
"type": "object",
|
||||
"additionalProperties": False,
|
||||
|
|
@ -313,7 +316,7 @@ class TestCompressSchema:
|
|||
},
|
||||
},
|
||||
}
|
||||
result = compress_schema(schema)
|
||||
result = compress_schema(schema, prune_additional_properties=True)
|
||||
assert "additionalProperties" not in result
|
||||
assert "additionalProperties" not in result["properties"]["foo"]
|
||||
assert (
|
||||
|
|
@ -393,6 +396,51 @@ class TestCompressSchema:
|
|||
)
|
||||
assert "title" not in compressed["properties"]["normal_field"]
|
||||
|
||||
def test_mcp_client_compatibility_requires_additional_properties(self):
|
||||
"""Test that compress_schema preserves additionalProperties: false for MCP clients.
|
||||
|
||||
MCP clients like Claude require strict JSON schemas with additionalProperties: false.
|
||||
When tools use Pydantic models with extra="forbid", this constraint must be preserved.
|
||||
|
||||
Without this, MCP clients return:
|
||||
"Invalid schema for function 'X': In context=('properties', 'Y'),
|
||||
'additionalProperties' is required to be supplied and to be false"
|
||||
|
||||
See: https://github.com/jlowin/fastmcp/issues/3008
|
||||
"""
|
||||
# Schema representing a Pydantic model with extra="forbid"
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"graph_table": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"columns": {"type": "array", "items": {"type": "string"}},
|
||||
},
|
||||
"required": ["name"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
},
|
||||
"required": ["graph_table"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
|
||||
# By default, compress_schema should NOT strip additionalProperties: false
|
||||
# This is the new expected behavior for MCP compatibility
|
||||
result = compress_schema(schema)
|
||||
|
||||
# Root level should preserve additionalProperties: false
|
||||
assert result.get("additionalProperties") is False, (
|
||||
"Root additionalProperties: false was removed, breaking MCP compatibility"
|
||||
)
|
||||
|
||||
# Nested object should also preserve additionalProperties: false
|
||||
graph_table = result["properties"]["graph_table"]
|
||||
assert graph_table.get("additionalProperties") is False, (
|
||||
"Nested additionalProperties: false was removed, breaking MCP compatibility"
|
||||
)
|
||||
|
||||
|
||||
class TestResolveRootRef:
|
||||
"""Tests for the resolve_root_ref function.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue