From 71ba4dbebf68462812c97640ac2071b83592af33 Mon Sep 17 00:00:00 2001 From: Chenglun Hu Date: Fri, 5 Jun 2026 09:29:33 +0800 Subject: [PATCH] fix(mcp): preserve full JSON Schema in build_input_schema (items/enum/format/required/etc) Fixes #29715. build_input_schema only copied `type` and `description` from each parameter/body schema, dropping every other JSON Schema keyword. For array-of-string query parameters (common OpenAPI pattern) the resulting MCP tool inputSchema lost `items`, so downstream consumers (CrewAI etc.) converted it to Pydantic and produced `items: {}` which OpenAI rejects as an invalid tool schema. Spread the full resolved schema into properties[param_name], then merge the OpenAPI parameter-level description back in (it wins over a schema-level one when both are present, matching the OpenAPI 3 spec). Same treatment for requestBody so `required`, `additionalProperties`, `$defs`, and nested `items`/`enum` survive. 6 new tests: array-items roundtrip, enum/format/default, nested object properties+required, parameter-vs-schema description precedence, schema-level description fallback, requestBody required + additionalProperties. 63 total tests in this file pass. Negative-test verified: reverting the helper makes the new array-items test fail (`assert {"type": ..."} == {}` because items is missing). --- .../mcp_server/openapi_to_mcp_generator.py | 39 ++-- .../test_openapi_to_mcp_generator.py | 202 +++++++++++++++--- 2 files changed, 200 insertions(+), 41 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py b/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py index 083a98cdd36..91dd3e4f77d 100644 --- a/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py +++ b/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py @@ -315,13 +315,23 @@ def build_input_schema(operation: _OpenAPIOperation) -> dict[str, object]: if "name" not in param: continue param_name = param["name"] - param_schema = param.get("schema", {}) - param_type = param_schema.get("type", "string") - - properties[param_name] = { - "type": param_type, - "description": param.get("description", ""), - } + raw_schema = param.get("schema") + # Preserve the full resolved JSON Schema for the parameter + # (#29715). Previously only `type` was copied, dropping `items`, + # `enum`, `format`, `default`, `properties`, `required`, etc. + # Downstream consumers (CrewAI etc.) then converted the + # truncated schema to Pydantic and produced `items: {}` for + # arrays, which OpenAI rejects as an invalid tool schema. + schema_copy = ( + dict(raw_schema) if isinstance(raw_schema, dict) else {} + ) # mutable-ok: copy the full param schema so downstream edits don't mutate the source operation + schema_copy.setdefault("type", "string") + # OpenAPI puts the human description on the parameter object, + # not on the schema; let it win when present, else fall back to + # any schema-level description, else empty string. + description = param.get("description") or schema_copy.get("description") or "" + schema_copy["description"] = description + properties[param_name] = schema_copy if param.get("required", False): required.append(param_name) @@ -334,11 +344,16 @@ def build_input_schema(operation: _OpenAPIOperation) -> dict[str, object]: # Try to get JSON schema if "application/json" in content: schema: Final[_OpenAPIJSONSchema] = content["application/json"].get("schema", {}) - properties["body"] = { - "type": "object", - "description": request_body.get("description", "Request body"), - "properties": schema.get("properties", {}), - } + # Preserve the full body schema (#29715) instead of only + # `properties` — required, additionalProperties, $defs, + # nested items/enum, etc. are all load-bearing for downstream + # tool-call validation. + body_copy: Final = ( + dict(schema) if schema else {} + ) # mutable-ok: copy the full body schema so downstream edits don't mutate the source operation + body_copy.setdefault("type", "object") + body_copy["description"] = request_body.get("description", "Request body") + properties["body"] = body_copy if request_body.get("required", False): required.append("body") diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py index e59616e53c1..de2c43c3f8b 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py @@ -85,9 +85,7 @@ class TestCreateToolFunction: # Verify URL was constructed correctly call_args = async_client.get.call_args - assert "repository-id" in str(call_args[0][0]) or "test-repo" in str( - call_args[0][0] - ) + assert "repository-id" in str(call_args[0][0]) or "test-repo" in str(call_args[0][0]) @pytest.mark.asyncio async def test_leading_digit_parameter(self): @@ -420,6 +418,167 @@ class TestBuildInputSchema: # Required should include original names assert "repository-id" in schema["required"] + def test_preserves_array_items_for_repeatable_query_params(self): + """#29715: array-of-string query parameters must carry their + `items` keyword through to the MCP schema. Without it, downstream + consumers (CrewAI etc.) emit `items: {}` and OpenAI rejects the + tool schema.""" + operation = { + "parameters": [ + { + "name": "domain", + "in": "query", + "required": True, + "description": "Repeatable query parameter", + "schema": { + "type": "array", + "items": {"type": "string"}, + }, + } + ] + } + + schema = build_input_schema(operation) + + domain = schema["properties"]["domain"] + assert domain["type"] == "array" + assert domain["items"] == {"type": "string"} + assert domain["description"] == "Repeatable query parameter" + + def test_preserves_enum_format_default_on_parameters(self): + """#29715: enum/format/default and similar JSON Schema keywords + must round-trip into the MCP schema.""" + operation = { + "parameters": [ + { + "name": "status", + "in": "query", + "schema": { + "type": "string", + "enum": ["open", "closed"], + "default": "open", + }, + }, + { + "name": "created_at", + "in": "query", + "schema": { + "type": "string", + "format": "date-time", + }, + }, + ] + } + + schema = build_input_schema(operation) + + status = schema["properties"]["status"] + assert status["enum"] == ["open", "closed"] + assert status["default"] == "open" + created = schema["properties"]["created_at"] + assert created["format"] == "date-time" + + def test_preserves_nested_object_properties_and_required(self): + """#29715: an inline object parameter must keep its `properties` + and `required` so downstream validators see the full shape.""" + operation = { + "parameters": [ + { + "name": "filter", + "in": "query", + "schema": { + "type": "object", + "properties": { + "min": {"type": "integer"}, + "max": {"type": "integer"}, + }, + "required": ["min"], + }, + } + ] + } + + schema = build_input_schema(operation) + + f = schema["properties"]["filter"] + assert f["properties"]["min"] == {"type": "integer"} + assert f["properties"]["max"] == {"type": "integer"} + assert f["required"] == ["min"] + + def test_parameter_description_overrides_schema_description(self): + """OpenAPI 3 places the description on the parameter object; let it + win over a schema-level description when both are present.""" + operation = { + "parameters": [ + { + "name": "q", + "in": "query", + "description": "from parameter", + "schema": { + "type": "string", + "description": "from schema", + }, + } + ] + } + + schema = build_input_schema(operation) + assert schema["properties"]["q"]["description"] == "from parameter" + + def test_parameter_falls_back_to_schema_description(self): + """If only the schema has a description, surface that instead of an + empty string.""" + operation = { + "parameters": [ + { + "name": "q", + "in": "query", + "schema": { + "type": "string", + "description": "from schema only", + }, + } + ] + } + + schema = build_input_schema(operation) + assert schema["properties"]["q"]["description"] == "from schema only" + + def test_preserves_request_body_required_and_additional_props(self): + """#29715 (requestBody half): the body schema must keep `required`, + `additionalProperties`, and other top-level JSON Schema keywords — + not just `properties`.""" + operation = { + "requestBody": { + "required": True, + "description": "Create user", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "email": {"type": "string"}, + "tags": { + "type": "array", + "items": {"type": "string"}, + }, + }, + "required": ["email"], + "additionalProperties": False, + } + } + }, + } + } + + schema = build_input_schema(operation) + body = schema["properties"]["body"] + assert body["properties"]["email"] == {"type": "string"} + assert body["properties"]["tags"]["items"] == {"type": "string"} + assert body["required"] == ["email"] + assert body["additionalProperties"] is False + assert body["description"] == "Create user" + class TestExtractParameters: """Test parameter extraction from OpenAPI operations.""" @@ -432,9 +591,7 @@ class TestExtractParameters: {"name": "filter", "in": "query"}, {"name": "data", "in": "body"}, ], - "requestBody": { - "content": {"application/json": {"schema": {"type": "object"}}} - }, + "requestBody": {"content": {"application/json": {"schema": {"type": "object"}}}}, } path_params, query_params, body_params = extract_parameters(operation) @@ -636,7 +793,6 @@ class TestGetBaseUrl: base_url = get_base_url(spec, spec_path) assert base_url == "https://production.example.com" - def test_fallback_with_nested_path(self): """Test fallback with deeply nested spec path.""" spec = {"openapi": "3.0.0", "paths": {}} @@ -864,9 +1020,7 @@ class TestResolveOperationParams: ], } path_item = {"parameters": path_level_params, "get": operation} - result = resolve_operation_params( - operation, path_item, {"parameters": component_params} - ) + result = resolve_operation_params(operation, path_item, {"parameters": component_params}) names = [p["name"] for p in result["parameters"]] assert "owner" in names assert "repo" in names @@ -969,22 +1123,16 @@ class TestRegisterToolsFromOpenAPI: } } - openapi_to_mcp_generator.register_tools_from_openapi( - spec, base_url="https://api.example.com" - ) + openapi_to_mcp_generator.register_tools_from_openapi(spec, base_url="https://api.example.com") assert registered, "expected at least one registered tool" anthropic_re = re.compile(r"^[a-zA-Z0-9_-]{1,128}$") for name in registered: - assert anthropic_re.match( - name - ), f"tool name {name!r} violates ^[a-zA-Z0-9_-]+$" + assert anthropic_re.match(name), f"tool name {name!r} violates ^[a-zA-Z0-9_-]+$" assert "actions_download-job-logs-for-workflow-run" in registered assert "pulls_list-files" in registered - def test_missing_operation_id_uses_sanitized_method_path_fallback( - self, monkeypatch - ): + def test_missing_operation_id_uses_sanitized_method_path_fallback(self, monkeypatch): import re from litellm.proxy._experimental.mcp_server import openapi_to_mcp_generator @@ -1007,15 +1155,11 @@ class TestRegisterToolsFromOpenAPI: } } } - openapi_to_mcp_generator.register_tools_from_openapi( - spec, base_url="https://api.example.com" - ) + openapi_to_mcp_generator.register_tools_from_openapi(spec, base_url="https://api.example.com") assert registered for name in registered: - assert re.match( - r"^[a-zA-Z0-9_-]+$", name - ), f"fallback tool name {name!r} not sanitized" + assert re.match(r"^[a-zA-Z0-9_-]+$", name), f"fallback tool name {name!r} not sanitized" class TestRequestExtraHeaders: @@ -1171,9 +1315,7 @@ class TestRequestExtraHeaders: async_client = _create_mock_client("get", "secure-data") mock_client.return_value = async_client - extra_token = _request_extra_headers.set( - {"Authorization": "Bearer extra", "X-TOKEN": "token-value"} - ) + extra_token = _request_extra_headers.set({"Authorization": "Bearer extra", "X-TOKEN": "token-value"}) auth_token = _request_auth_header.set("Bearer byok-credential") try: result = await func() @@ -1301,7 +1443,9 @@ class TestUpstreamStatusIsClassified: @pytest.mark.asyncio async def test_401_raises_the_reauth_signal_carrying_the_challenge(self): - tool, client = self._tool(401, text='{"error":"invalid_token"}', headers={"www-authenticate": 'Bearer realm="x"'}) + tool, client = self._tool( + 401, text='{"error":"invalid_token"}', headers={"www-authenticate": 'Bearer realm="x"'} + ) with patch(GET_ASYNC_CLIENT_TARGET, return_value=client): with pytest.raises(MCPUpstreamAuthError) as exc: await tool()