diff --git a/src/fastmcp/experimental/server/openapi/server.py b/src/fastmcp/experimental/server/openapi/server.py index e5cae8fcc..101427375 100644 --- a/src/fastmcp/experimental/server/openapi/server.py +++ b/src/fastmcp/experimental/server/openapi/server.py @@ -11,7 +11,7 @@ from jsonschema_path import SchemaPath from fastmcp.experimental.utilities.openapi import ( HTTPRoute, extract_output_schema_from_responses, - format_description_with_responses, + format_simple_description, parse_openapi_to_http_routes, ) from fastmcp.experimental.utilities.openapi.director import RequestDirector @@ -267,7 +267,9 @@ class FastMCPOpenAPI(FastMCP): # Extract output schema from OpenAPI responses output_schema = extract_output_schema_from_responses( - route.responses, route.schema_definitions, route.openapi_version + route.responses, + route.response_schemas, + route.openapi_version, ) # Get a unique tool name @@ -279,10 +281,9 @@ class FastMCPOpenAPI(FastMCP): or f"Executes {route.method} {route.path}" ) - # Format enhanced description with parameters and request body - enhanced_description = format_description_with_responses( + # Use simplified description formatter for tools + enhanced_description = format_simple_description( base_description=base_description, - responses=route.responses, parameters=route.parameters, request_body=route.request_body, ) @@ -331,10 +332,9 @@ class FastMCPOpenAPI(FastMCP): route.description or route.summary or f"Represents {route.path}" ) - # Format enhanced description with parameters and request body - enhanced_description = format_description_with_responses( + # Use simplified description for resources + enhanced_description = format_simple_description( base_description=base_description, - responses=route.responses, parameters=route.parameters, request_body=route.request_body, ) @@ -388,10 +388,9 @@ class FastMCPOpenAPI(FastMCP): route.description or route.summary or f"Template for {route.path}" ) - # Format enhanced description with parameters and request body - enhanced_description = format_description_with_responses( + # Use simplified description for resource templates + enhanced_description = format_simple_description( base_description=base_description, - responses=route.responses, parameters=route.parameters, request_body=route.request_body, ) diff --git a/src/fastmcp/experimental/utilities/openapi/__init__.py b/src/fastmcp/experimental/utilities/openapi/__init__.py index 4231539fc..92a76ec61 100644 --- a/src/fastmcp/experimental/utilities/openapi/__init__.py +++ b/src/fastmcp/experimental/utilities/openapi/__init__.py @@ -20,6 +20,7 @@ from .formatters import ( format_deep_object_parameter, format_description_with_responses, format_json_for_description, + format_simple_description, generate_example_from_schema, ) @@ -54,6 +55,7 @@ __all__ = [ "format_deep_object_parameter", "format_description_with_responses", "format_json_for_description", + "format_simple_description", "generate_example_from_schema", # Schemas "_combine_schemas", diff --git a/src/fastmcp/experimental/utilities/openapi/formatters.py b/src/fastmcp/experimental/utilities/openapi/formatters.py index be6294c71..0b3e3f899 100644 --- a/src/fastmcp/experimental/utilities/openapi/formatters.py +++ b/src/fastmcp/experimental/utilities/openapi/formatters.py @@ -189,6 +189,39 @@ def format_json_for_description(data: Any, indent: int = 2) -> str: return f"```\nCould not serialize to JSON: {data}\n```" +def format_simple_description( + base_description: str, + parameters: list[ParameterInfo] | None = None, + request_body: RequestBodyInfo | None = None, +) -> str: + """ + Formats a simple description for MCP objects (tools, resources, prompts). + Excludes response details, examples, and verbose status codes. + + Args: + base_description (str): The initial description to be formatted. + parameters (list[ParameterInfo] | None, optional): A list of parameter information. + request_body (RequestBodyInfo | None, optional): Information about the request body. + + Returns: + str: The formatted description string with minimal details. + """ + desc_parts = [base_description] + + # Only add critical parameter information if they have descriptions + if parameters: + path_params = [p for p in parameters if p.location == "path" and p.description] + if path_params: + desc_parts.append("\n\n**Path Parameters:**") + for param in path_params: + desc_parts.append(f"\n- **{param.name}**: {param.description}") + + # Skip query parameters, request body details, and all response information + # These are already captured in the inputSchema + + return "\n".join(desc_parts) + + def format_description_with_responses( base_description: str, responses: dict[ @@ -351,5 +384,6 @@ __all__ = [ "format_deep_object_parameter", "format_description_with_responses", "format_json_for_description", + "format_simple_description", "generate_example_from_schema", ] diff --git a/src/fastmcp/experimental/utilities/openapi/models.py b/src/fastmcp/experimental/utilities/openapi/models.py index 6116dc8c8..c1d13b2c0 100644 --- a/src/fastmcp/experimental/utilities/openapi/models.py +++ b/src/fastmcp/experimental/utilities/openapi/models.py @@ -58,9 +58,12 @@ class HTTPRoute(FastMCPBaseModel): responses: dict[str, ResponseInfo] = Field( default_factory=dict ) # Key: status code str - schema_definitions: dict[str, JsonSchema] = Field( + request_schemas: dict[str, JsonSchema] = Field( default_factory=dict - ) # Store component schemas + ) # Store schemas needed for input (parameters/request body) + response_schemas: dict[str, JsonSchema] = Field( + default_factory=dict + ) # Store schemas needed for output (responses) extensions: dict[str, Any] = Field(default_factory=dict) openapi_version: str | None = None diff --git a/src/fastmcp/experimental/utilities/openapi/parser.py b/src/fastmcp/experimental/utilities/openapi/parser.py index 53d26a7a4..285b62309 100644 --- a/src/fastmcp/experimental/utilities/openapi/parser.py +++ b/src/fastmcp/experimental/utilities/openapi/parser.py @@ -402,77 +402,116 @@ class OpenAPIParser( ) return None + def _is_success_status_code(self, status_code: str) -> bool: + """Check if a status code represents a successful response (2xx).""" + try: + code_int = int(status_code) + return 200 <= code_int < 300 + except (ValueError, TypeError): + # Handle special cases like 'default' or other non-numeric codes + return status_code.lower() in ["default", "2xx"] + + def _get_primary_success_response( + self, operation_responses: dict[str, Any] + ) -> tuple[str, Any] | None: + """Get the primary success response for an MCP tool. We only need one success response.""" + if not operation_responses: + return None + + # Priority order: 200, 201, 202, 204, 207, then any other 2xx + priority_codes = ["200", "201", "202", "204", "207"] + + # First check priority codes + for code in priority_codes: + if code in operation_responses: + return (code, operation_responses[code]) + + # Then check any other 2xx codes + for status_code, resp_or_ref in operation_responses.items(): + if self._is_success_status_code(status_code): + return (status_code, resp_or_ref) + + # If no success codes found, return None (tool will have no output schema) + return None + def _extract_responses( self, operation_responses: dict[str, Any] | None ) -> dict[str, ResponseInfo]: - """Extract and resolve response information.""" + """Extract and resolve response information. Only includes the primary success response for MCP tools.""" extracted_responses: dict[str, ResponseInfo] = {} if not operation_responses: return extracted_responses - for status_code, resp_or_ref in operation_responses.items(): - try: - response = self._resolve_ref(resp_or_ref) + # For MCP tools, we only need the primary success response + primary_response = self._get_primary_success_response(operation_responses) + if not primary_response: + logger.debug("No success responses found, tool will have no output schema") + return extracted_responses - if not isinstance(response, self.response_cls): - logger.warning( - f"Expected Response after resolving for status code {status_code}, " - f"got {type(response)}. Skipping." - ) - continue + status_code, resp_or_ref = primary_response + logger.debug(f"Using primary success response: {status_code}") - # Create response info - resp_info = ResponseInfo(description=response.description) + try: + response = self._resolve_ref(resp_or_ref) - # Extract content schemas - if hasattr(response, "content") and response.content: - for media_type_str, media_type_obj in response.content.items(): - if ( - media_type_obj - and hasattr(media_type_obj, "media_type_schema") - and media_type_obj.media_type_schema - ): - try: - schema_dict = self._extract_schema_as_dict( - media_type_obj.media_type_schema - ) - resp_info.content_schema[media_type_str] = schema_dict - except ValueError as e: - # Re-raise ValueError for external reference errors - if ( - "External or non-local reference not supported" - in str(e) - ): - raise - logger.error( - f"Failed to extract schema for media type '{media_type_str}' " - f"in response {status_code}: {e}" - ) - except Exception as e: - logger.error( - f"Failed to extract schema for media type '{media_type_str}' " - f"in response {status_code}: {e}" - ) - - extracted_responses[str(status_code)] = resp_info - except ValueError as e: - # Re-raise ValueError for external reference errors - if "External or non-local reference not supported" in str(e): - raise - ref_name = getattr(resp_or_ref, "ref", "unknown") - logger.error( - f"Failed to extract response for status code {status_code} " - f"from reference '{ref_name}': {e}", - exc_info=False, - ) - except Exception as e: - ref_name = getattr(resp_or_ref, "ref", "unknown") - logger.error( - f"Failed to extract response for status code {status_code} " - f"from reference '{ref_name}': {e}", - exc_info=False, + if not isinstance(response, self.response_cls): + logger.warning( + f"Expected Response after resolving for status code {status_code}, " + f"got {type(response)}. Returning empty responses." ) + return extracted_responses + + # Create response info + resp_info = ResponseInfo(description=response.description) + + # Extract content schemas + if hasattr(response, "content") and response.content: + for media_type_str, media_type_obj in response.content.items(): + if ( + media_type_obj + and hasattr(media_type_obj, "media_type_schema") + and media_type_obj.media_type_schema + ): + try: + schema_dict = self._extract_schema_as_dict( + media_type_obj.media_type_schema + ) + resp_info.content_schema[media_type_str] = schema_dict + except ValueError as e: + # Re-raise ValueError for external reference errors + if "External or non-local reference not supported" in str( + e + ): + raise + logger.error( + f"Failed to extract schema for media type '{media_type_str}' " + f"in response {status_code}: {e}" + ) + except Exception as e: + logger.error( + f"Failed to extract schema for media type '{media_type_str}' " + f"in response {status_code}: {e}" + ) + + extracted_responses[str(status_code)] = resp_info + except ValueError as e: + # Re-raise ValueError for external reference errors + if "External or non-local reference not supported" in str(e): + raise + ref_name = getattr(resp_or_ref, "ref", "unknown") + logger.error( + f"Failed to extract response for status code {status_code} " + f"from reference '{ref_name}': {e}", + exc_info=False, + ) + except Exception as e: + ref_name = getattr(resp_or_ref, "ref", "unknown") + logger.error( + f"Failed to extract response for status code {status_code} " + f"from reference '{ref_name}': {e}", + exc_info=False, + ) return extracted_responses @@ -525,24 +564,22 @@ class OpenAPIParser( find_refs(schema) return collected - def _extract_route_schema_dependencies( + def _extract_input_schema_dependencies( self, parameters: list[ParameterInfo], request_body: RequestBodyInfo | None, - responses: dict[str, ResponseInfo], all_schemas: dict[str, Any], ) -> dict[str, Any]: """ - Extract only the schema definitions needed for a specific route. + Extract only the schema definitions needed for input (parameters and request body). Args: parameters: Route parameters request_body: Route request body - responses: Route responses all_schemas: All available schema definitions Returns: - Dictionary containing only the schemas needed for this route + Dictionary containing only the schemas needed for input """ needed_schemas = set() @@ -558,6 +595,28 @@ class OpenAPIParser( deps = self._extract_schema_dependencies(content_schema, all_schemas) needed_schemas.update(deps) + # Return only the needed input schemas + return { + name: all_schemas[name] for name in needed_schemas if name in all_schemas + } + + def _extract_output_schema_dependencies( + self, + responses: dict[str, ResponseInfo], + all_schemas: dict[str, Any], + ) -> dict[str, Any]: + """ + Extract only the schema definitions needed for outputs (responses). + + Args: + responses: Route responses + all_schemas: All available schema definitions + + Returns: + Dictionary containing only the schemas needed for outputs + """ + needed_schemas = set() + # Check responses for schema references for response in responses.values(): if response.content_schema: @@ -567,7 +626,7 @@ class OpenAPIParser( ) needed_schemas.update(deps) - # Return only the needed schemas + # Return only the needed output schemas return { name: all_schemas[name] for name in needed_schemas if name in all_schemas } @@ -661,10 +720,13 @@ class OpenAPIParser( if k.startswith("x-") } - # Extract only the schemas needed for this route - route_schemas = self._extract_route_schema_dependencies( + # Extract schemas separately for input and output + input_schemas = self._extract_input_schema_dependencies( parameters, request_body_info, + schema_definitions, + ) + output_schemas = self._extract_output_schema_dependencies( responses, schema_definitions, ) @@ -680,7 +742,8 @@ class OpenAPIParser( parameters=parameters, request_body=request_body_info, responses=responses, - schema_definitions=route_schemas, # Use pre-pruned schemas + request_schemas=input_schemas, + response_schemas=output_schemas, extensions=extensions, openapi_version=self.openapi_version, ) diff --git a/src/fastmcp/experimental/utilities/openapi/schemas.py b/src/fastmcp/experimental/utilities/openapi/schemas.py index 98713ca37..234283fcb 100644 --- a/src/fastmcp/experimental/utilities/openapi/schemas.py +++ b/src/fastmcp/experimental/utilities/openapi/schemas.py @@ -364,10 +364,11 @@ def _combine_schemas_and_map_params( "required": required, } # Add schema definitions if available - if route.schema_definitions: + schema_defs = route.request_schemas + if schema_defs: if convert_refs: # Need to convert refs and prune - all_defs = route.schema_definitions.copy() + all_defs = schema_defs.copy() # Convert each schema definition recursively for name, schema in all_defs.items(): if isinstance(schema, dict): @@ -409,7 +410,7 @@ def _combine_schemas_and_map_params( } else: # From parser - already converted and pruned - result["$defs"] = route.schema_definitions + result["$defs"] = schema_defs return result, parameter_map diff --git a/tests/experimental/openapi_parser/server/openapi/test_end_to_end_compatibility.py b/tests/experimental/openapi_parser/server/openapi/test_end_to_end_compatibility.py index f2f8c3ac3..a28669670 100644 --- a/tests/experimental/openapi_parser/server/openapi/test_end_to_end_compatibility.py +++ b/tests/experimental/openapi_parser/server/openapi/test_end_to_end_compatibility.py @@ -116,8 +116,10 @@ class TestEndToEndCompatibility: assert legacy_tool.name == new_tool.name assert legacy_tool.name == "get_user" - # Descriptions should be identical - assert legacy_tool.description == new_tool.description + # Descriptions may differ (new server has simplified descriptions) + # Just check that both have descriptions + assert legacy_tool.description + assert new_tool.description # Input schemas should be identical legacy_schema = legacy_tool.inputSchema diff --git a/tests/experimental/openapi_parser/utilities/openapi/test_models.py b/tests/experimental/openapi_parser/utilities/openapi/test_models.py index 44943ceae..9b9c3d2e8 100644 --- a/tests/experimental/openapi_parser/utilities/openapi/test_models.py +++ b/tests/experimental/openapi_parser/utilities/openapi/test_models.py @@ -248,7 +248,7 @@ class TestHTTPRoute: parameters=parameters, request_body=request_body, responses=responses, - schema_definitions={"User": {"type": "object"}}, + request_schemas={"User": {"type": "object"}}, extensions={"x-custom": "value"}, ) @@ -261,7 +261,7 @@ class TestHTTPRoute: assert len(route.parameters) == 1 assert route.request_body is not None assert "200" in route.responses - assert "User" in route.schema_definitions + assert "User" in route.request_schemas assert route.extensions["x-custom"] == "value" def test_route_pre_calculated_fields(self): @@ -303,14 +303,15 @@ class TestHTTPRoute: tags=[], parameters=[], responses={}, - schema_definitions={}, + request_schemas={}, extensions={}, ) assert route.tags == [] assert route.parameters == [] assert route.responses == {} - assert route.schema_definitions == {} + assert route.request_schemas == {} + assert route.response_schemas == {} assert route.extensions == {} def test_route_defaults(self): @@ -327,7 +328,8 @@ class TestHTTPRoute: assert route.parameters == [] assert route.request_body is None assert route.responses == {} - assert route.schema_definitions == {} + assert route.request_schemas == {} + assert route.response_schemas == {} assert route.extensions == {} assert route.flat_param_schema == {} assert route.parameter_map == {} diff --git a/tests/experimental/openapi_parser/utilities/openapi/test_parser.py b/tests/experimental/openapi_parser/utilities/openapi/test_parser.py index afd31edda..a4a66f8a6 100644 --- a/tests/experimental/openapi_parser/utilities/openapi/test_parser.py +++ b/tests/experimental/openapi_parser/utilities/openapi/test_parser.py @@ -253,12 +253,12 @@ class TestOpenAPIParser: routes = parse_openapi_to_http_routes(spec) route = routes[0] - # SchemaA is expanded inline, so it's NOT in schema_definitions - assert "SchemaA" not in route.schema_definitions + # SchemaA is expanded inline, so it's NOT in request_schemas + assert "SchemaA" not in route.request_schemas # But SchemaB and SchemaC MUST be there (transitive dependencies) - assert "SchemaB" in route.schema_definitions - assert "SchemaC" in route.schema_definitions + assert "SchemaB" in route.request_schemas + assert "SchemaC" in route.request_schemas # Same in the flat parameter schema assert "SchemaB" in route.flat_param_schema["$defs"] @@ -316,11 +316,11 @@ class TestOpenAPIParser: route = routes[0] # Profile is expanded inline, NOT in schema_defs - assert "Profile" not in route.schema_definitions + assert "Profile" not in route.request_schemas # Bug fix: countryCode and AccountInfo MUST be in schema_defs - assert "countryCode" in route.schema_definitions # Was missing in #1372 - assert "AccountInfo" in route.schema_definitions # Was missing in #1372 + assert "countryCode" in route.request_schemas # Was missing in #1372 + assert "AccountInfo" in route.request_schemas # Was missing in #1372 # Same in flat parameter schema assert "countryCode" in route.flat_param_schema["$defs"] diff --git a/tests/experimental/openapi_parser/utilities/openapi/test_transitive_references.py b/tests/experimental/openapi_parser/utilities/openapi/test_transitive_references.py index 2eab6e2be..da1000da8 100644 --- a/tests/experimental/openapi_parser/utilities/openapi/test_transitive_references.py +++ b/tests/experimental/openapi_parser/utilities/openapi/test_transitive_references.py @@ -6,6 +6,7 @@ from fastmcp.experimental.utilities.openapi.models import ( RequestBodyInfo, ResponseInfo, ) +from fastmcp.experimental.utilities.openapi.parser import parse_openapi_to_http_routes from fastmcp.experimental.utilities.openapi.schemas import ( _combine_schemas_and_map_params, extract_output_schema_from_responses, @@ -35,7 +36,7 @@ class TestTransitiveAndNestedReferences: } }, ), - schema_definitions={ + request_schemas={ "User": { "type": "object", "properties": {"profile": {"$ref": "#/components/schemas/Profile"}}, @@ -183,7 +184,7 @@ class TestTransitiveAndNestedReferences: "application/json": {"$ref": "#/components/schemas/Profile"} }, ), - schema_definitions={ + request_schemas={ "Profile": { "type": "object", "properties": { @@ -234,7 +235,7 @@ class TestTransitiveAndNestedReferences: "application/json": {"$ref": "#/components/schemas/User"} }, ), - schema_definitions={ + request_schemas={ "User": { "type": "object", "properties": { @@ -297,7 +298,7 @@ class TestTransitiveAndNestedReferences: } }, ), - schema_definitions={ + request_schemas={ "User": { "type": "object", "properties": {"profile": {"$ref": "#/components/schemas/Profile"}}, @@ -352,7 +353,7 @@ class TestTransitiveAndNestedReferences: } }, ), - schema_definitions={ + request_schemas={ "TypeA": { "type": "object", "properties": {"nested": {"$ref": "#/components/schemas/Nested"}}, @@ -403,7 +404,7 @@ class TestTransitiveAndNestedReferences: "application/json": {"$ref": "#/components/schemas/Level1"} }, ), - schema_definitions={ + request_schemas={ "Level1": { "type": "object", "properties": {"level2": {"$ref": "#/components/schemas/Level2"}}, @@ -470,7 +471,7 @@ class TestTransitiveAndNestedReferences: "application/json": {"$ref": "#/components/schemas/Node"} }, ), - schema_definitions={ + request_schemas={ "Node": { "type": "object", "properties": { @@ -511,7 +512,7 @@ class TestTransitiveAndNestedReferences: } }, ), - schema_definitions={ + request_schemas={ "Left": { "type": "object", "properties": {"shared": {"$ref": "#/components/schemas/Shared"}}, @@ -558,7 +559,7 @@ class TestTransitiveAndNestedReferences: "application/json": {"$ref": "#/components/schemas/Content"} }, ), - schema_definitions={ + request_schemas={ "Content": { "type": "object", "properties": { @@ -612,7 +613,7 @@ class TestTransitiveAndNestedReferences: } }, ), - schema_definitions={ + request_schemas={ "SimpleString": {"type": "string"}, "EmptyObject": {"type": "object"}, "UnreferencedSchema": {"type": "number"}, @@ -645,7 +646,7 @@ class TestTransitiveAndNestedReferences: "application/json": {"$ref": "#/components/schemas/DirectBody"} }, ), - schema_definitions={ + request_schemas={ "DirectBody": { "type": "object", "properties": { @@ -675,3 +676,113 @@ class TestTransitiveAndNestedReferences: combined_schema["$defs"]["DirectBody"]["properties"]["nested"]["$ref"] == "#/$defs/NestedBody" ) + + def test_separate_input_output_schemas(self): + """Test that input and output schemas contain different schema + definitions and don't overlap in the ultimate schema definitions.""" + # OpenAPI spec with transitive dependencies to force schema inclusion + openapi_spec = { + "openapi": "3.0.1", + "info": {"title": "Test API", "version": "1.0.0"}, + "paths": { + "/test": { + "post": { + "summary": "Test endpoint", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/InputContainer" + } + } + } + }, + "responses": { + "200": { + "description": "Success", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/OutputContainer" + } + } + }, + } + }, + } + } + }, + "components": { + "schemas": { + "InputContainer": { + "type": "object", + "properties": { + "data": {"$ref": "#/components/schemas/InputData"} + }, + }, + "InputData": { + "type": "object", + "properties": {"input_field": {"type": "string"}}, + }, + "OutputContainer": { + "type": "object", + "properties": { + "result": {"$ref": "#/components/schemas/OutputData"} + }, + }, + "OutputData": { + "type": "object", + "properties": {"output_field": {"type": "string"}}, + }, + "UnusedSchema": { + "type": "object", + "properties": {"unused_field": {"type": "string"}}, + }, + } + }, + } + + routes = parse_openapi_to_http_routes(openapi_spec) + assert len(routes) == 1 + + route = routes[0] + + # Check that schemas are properly separated + input_schema_names = set(route.request_schemas.keys()) + output_schema_names = set(route.response_schemas.keys()) + + # Input should contain transitive dependencies from InputContainer + assert "InputData" in input_schema_names, ( + f"Expected InputData in request schemas: {input_schema_names}" + ) + assert "OutputContainer" not in input_schema_names, ( + "OutputContainer should not be in request schemas" + ) + assert "OutputData" not in input_schema_names, ( + "OutputData should not be in request schemas" + ) + + # Output should contain transitive dependencies from OutputContainer + assert "OutputData" in output_schema_names, ( + f"Expected OutputData in response schemas: {output_schema_names}" + ) + assert "InputContainer" not in output_schema_names, ( + "InputContainer should not be in response schemas" + ) + assert "InputData" not in output_schema_names, ( + "InputData should not be in response schemas" + ) + + # Neither should contain unused schema + assert "UnusedSchema" not in input_schema_names, ( + "UnusedSchema should not be in request schemas" + ) + assert "UnusedSchema" not in output_schema_names, ( + "UnusedSchema should not be in response schemas" + ) + + # Verify no overlap + overlap = input_schema_names & output_schema_names + assert len(overlap) == 0, ( + f"Found overlapping schemas between input and output: {overlap}" + )