mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-22 13:34:17 +02:00
Fix OpenAPI 3.0 nullable fields in tool input schemas (#3768)
* Fix OpenAPI 3.0 nullable fields leaking into tool input schemas * fix: convert nullable fields in input schemas and fix recursion * Fix unused loop variable in OpenAPI converter * Refactor OpenAPI nullable conversion and add tests * Clean up and add integration tests for nullable input schemas --------- Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com>
This commit is contained in:
Kakarlamudi Venkata Dhanush
GitHub
parent
e5b96343d1
commit
042db1d0e3
3 changed files with 195 additions and 20 deletions
|
|
@ -26,6 +26,8 @@ OPENAPI_SPECIFIC_FIELDS = {
|
|||
# Fields that should be recursively processed
|
||||
RECURSIVE_FIELDS = {
|
||||
"properties": dict,
|
||||
"$defs": dict,
|
||||
"$definitions": dict,
|
||||
"items": dict,
|
||||
"additionalProperties": dict,
|
||||
"allOf": list,
|
||||
|
|
@ -108,19 +110,19 @@ def convert_openapi_schema_to_json_schema(
|
|||
for field_name, field_type in RECURSIVE_FIELDS.items():
|
||||
if field_name in result:
|
||||
if field_type is dict and isinstance(result[field_name], dict):
|
||||
if field_name == "properties":
|
||||
# Handle properties specially - each property is a schema
|
||||
if field_name in ("properties", "$defs", "$definitions"):
|
||||
# Handle maps of schemas (properties, $defs, $definitions)
|
||||
result[field_name] = {
|
||||
prop_name: convert_openapi_schema_to_json_schema(
|
||||
prop_schema,
|
||||
name: convert_openapi_schema_to_json_schema(
|
||||
sub_schema,
|
||||
openapi_version,
|
||||
remove_read_only,
|
||||
remove_write_only,
|
||||
convert_one_of_to_any_of,
|
||||
)
|
||||
if isinstance(prop_schema, dict)
|
||||
else prop_schema
|
||||
for prop_name, prop_schema in result[field_name].items()
|
||||
if isinstance(sub_schema, dict)
|
||||
else sub_schema
|
||||
for name, sub_schema in result[field_name].items()
|
||||
}
|
||||
else:
|
||||
result[field_name] = convert_openapi_schema_to_json_schema(
|
||||
|
|
@ -214,20 +216,20 @@ def _needs_recursive_processing(
|
|||
for field_name, field_type in RECURSIVE_FIELDS.items():
|
||||
if field_name in schema:
|
||||
if field_type is dict and isinstance(schema[field_name], dict):
|
||||
if field_name == "properties":
|
||||
# Check if any property needs conversion
|
||||
for prop_schema in schema[field_name].values():
|
||||
if isinstance(prop_schema, dict):
|
||||
if field_name in ("properties", "$defs", "$definitions"):
|
||||
# Check if any schema in the map needs conversion
|
||||
for sub_schema in schema[field_name].values():
|
||||
if isinstance(sub_schema, dict):
|
||||
nested_needs_conversion = (
|
||||
any(
|
||||
field in prop_schema
|
||||
field in sub_schema
|
||||
for field in OPENAPI_SPECIFIC_FIELDS
|
||||
)
|
||||
or (remove_read_only and prop_schema.get("readOnly"))
|
||||
or (remove_write_only and prop_schema.get("writeOnly"))
|
||||
or (convert_one_of_to_any_of and "oneOf" in prop_schema)
|
||||
or (remove_read_only and sub_schema.get("readOnly"))
|
||||
or (remove_write_only and sub_schema.get("writeOnly"))
|
||||
or (convert_one_of_to_any_of and "oneOf" in sub_schema)
|
||||
or _needs_recursive_processing(
|
||||
prop_schema,
|
||||
sub_schema,
|
||||
openapi_version,
|
||||
remove_read_only,
|
||||
remove_write_only,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ from typing import Any
|
|||
|
||||
from fastmcp.utilities.logging import get_logger
|
||||
|
||||
from .json_schema_converter import convert_openapi_schema_to_json_schema
|
||||
from .models import HTTPRoute, JsonSchema, ResponseInfo
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
|
@ -451,6 +452,9 @@ def _combine_schemas_and_map_params(
|
|||
# From parser - already converted and pruned
|
||||
result["$defs"] = schema_defs
|
||||
|
||||
if route.openapi_version and route.openapi_version.startswith("3"):
|
||||
result = convert_openapi_schema_to_json_schema(result, route.openapi_version)
|
||||
|
||||
return result, parameter_map
|
||||
|
||||
|
||||
|
|
@ -555,8 +559,6 @@ def extract_output_schema_from_responses(
|
|||
if openapi_version and openapi_version.startswith("3"):
|
||||
# Convert OpenAPI 3.x schema to JSON Schema format for proper handling
|
||||
# of constructs like oneOf, anyOf, and nullable fields
|
||||
from .json_schema_converter import convert_openapi_schema_to_json_schema
|
||||
|
||||
output_schema = convert_openapi_schema_to_json_schema(
|
||||
output_schema, openapi_version
|
||||
)
|
||||
|
|
@ -584,8 +586,6 @@ def extract_output_schema_from_responses(
|
|||
|
||||
# Convert OpenAPI schema definitions to JSON Schema format if needed
|
||||
if openapi_version and openapi_version.startswith("3"):
|
||||
from .json_schema_converter import convert_openapi_schema_to_json_schema
|
||||
|
||||
for def_name in list(processed_defs.keys()):
|
||||
processed_defs[def_name] = convert_openapi_schema_to_json_schema(
|
||||
processed_defs[def_name], openapi_version
|
||||
|
|
|
|||
|
|
@ -1,8 +1,12 @@
|
|||
"""Tests for nullable field handling in OpenAPI schemas."""
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from jsonschema import ValidationError, validate
|
||||
|
||||
from fastmcp import FastMCP
|
||||
from fastmcp.client import Client
|
||||
from fastmcp.server.providers.openapi import OpenAPIProvider
|
||||
from fastmcp.utilities.openapi.json_schema_converter import (
|
||||
convert_openapi_schema_to_json_schema,
|
||||
)
|
||||
|
|
@ -350,6 +354,70 @@ class TestHandleNullableFields:
|
|||
result = convert_openapi_schema_to_json_schema(input_schema, "3.0.0")
|
||||
assert result == expected
|
||||
|
||||
def test_nullable_in_definitions(self):
|
||||
"""Test nullable field inside $defs."""
|
||||
input_schema = {
|
||||
"type": "object",
|
||||
"properties": {"user": {"$ref": "#/$defs/User"}},
|
||||
"$defs": {
|
||||
"User": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"bio": {"type": "string", "nullable": True},
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
expected = {
|
||||
"type": "object",
|
||||
"properties": {"user": {"$ref": "#/$defs/User"}},
|
||||
"$defs": {
|
||||
"User": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"bio": {"type": ["string", "null"]},
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
result = convert_openapi_schema_to_json_schema(input_schema, "3.0.0")
|
||||
assert result == expected
|
||||
|
||||
def test_nullable_in_nested_properties(self):
|
||||
"""Test nullable field in deeply nested properties."""
|
||||
input_schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"a": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"b": {
|
||||
"type": "object",
|
||||
"properties": {"c": {"type": "string", "nullable": True}},
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
expected = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"a": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"b": {
|
||||
"type": "object",
|
||||
"properties": {"c": {"type": ["string", "null"]}},
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
result = convert_openapi_schema_to_json_schema(input_schema, "3.0.0")
|
||||
assert result == expected
|
||||
|
||||
|
||||
class TestNullableFieldValidation:
|
||||
"""Test that converted schemas validate correctly with jsonschema."""
|
||||
|
|
@ -383,3 +451,108 @@ class TestNullableFieldValidation:
|
|||
# Invalid values should fail
|
||||
with pytest.raises(ValidationError):
|
||||
validate(instance="INVALID", schema=json_schema)
|
||||
|
||||
|
||||
class TestNullableInputSchemaIntegration:
|
||||
"""Test that nullable fields are converted in tool input schemas end-to-end.
|
||||
|
||||
These tests exercise the full pipeline: OpenAPI spec -> OpenAPIProvider ->
|
||||
tool.inputSchema, verifying that `nullable: true` doesn't leak through.
|
||||
"""
|
||||
|
||||
async def test_nullable_query_param_converted_in_tool_input_schema(self):
|
||||
"""Nullable query parameter should produce type union in tool input schema."""
|
||||
spec = {
|
||||
"openapi": "3.0.0",
|
||||
"info": {"title": "Test", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/search": {
|
||||
"get": {
|
||||
"operationId": "search",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "query",
|
||||
"in": "query",
|
||||
"required": True,
|
||||
"schema": {"type": "string"},
|
||||
},
|
||||
{
|
||||
"name": "category",
|
||||
"in": "query",
|
||||
"schema": {"type": "string", "nullable": True},
|
||||
},
|
||||
],
|
||||
"responses": {"200": {"description": "OK"}},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
async with httpx.AsyncClient(base_url="https://api.example.com") as client:
|
||||
provider = OpenAPIProvider(openapi_spec=spec, client=client)
|
||||
mcp = FastMCP("test")
|
||||
mcp.add_provider(provider)
|
||||
|
||||
async with Client(mcp) as mcp_client:
|
||||
tools = await mcp_client.list_tools()
|
||||
assert len(tools) == 1
|
||||
schema = tools[0].inputSchema
|
||||
category_prop = schema["properties"]["category"]
|
||||
assert "nullable" not in category_prop
|
||||
assert category_prop["type"] == ["string", "null"]
|
||||
|
||||
async def test_nullable_in_request_body_defs_converted(self):
|
||||
"""Nullable field inside $defs referenced by request body should be converted."""
|
||||
spec = {
|
||||
"openapi": "3.0.0",
|
||||
"info": {"title": "Test", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/users": {
|
||||
"post": {
|
||||
"operationId": "create_user",
|
||||
"requestBody": {
|
||||
"required": True,
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/CreateUser"
|
||||
}
|
||||
}
|
||||
},
|
||||
},
|
||||
"responses": {"201": {"description": "Created"}},
|
||||
}
|
||||
}
|
||||
},
|
||||
"components": {
|
||||
"schemas": {
|
||||
"CreateUser": {
|
||||
"type": "object",
|
||||
"required": ["name"],
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"bio": {"type": "string", "nullable": True},
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
async with httpx.AsyncClient(base_url="https://api.example.com") as client:
|
||||
provider = OpenAPIProvider(openapi_spec=spec, client=client)
|
||||
mcp = FastMCP("test")
|
||||
mcp.add_provider(provider)
|
||||
|
||||
async with Client(mcp) as mcp_client:
|
||||
tools = await mcp_client.list_tools()
|
||||
assert len(tools) == 1
|
||||
schema = tools[0].inputSchema
|
||||
|
||||
# Find the bio property — it may be inline or in $defs
|
||||
if "$defs" in schema:
|
||||
# Resolve through $defs
|
||||
user_schema = next(iter(schema["$defs"].values()))
|
||||
bio_prop = user_schema["properties"]["bio"]
|
||||
else:
|
||||
bio_prop = schema["properties"]["bio"]
|
||||
|
||||
assert "nullable" not in bio_prop
|
||||
assert bio_prop["type"] == ["string", "null"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue