fix(openapi): preserve anyOf for non-null union body params instead of injecting an arbitrary type - #345
Open
K4bain wants to merge 3 commits into
Open
fix(openapi): preserve anyOf for non-null union body params instead of injecting an arbitrary type#345K4bain wants to merge 3 commits into
K4bain wants to merge 3 commits into
Conversation
…f injecting an arbitrary type
## Problem
For a request body field typed as a union of two real types (e.g. `dict[str, list[str]] | list[str]`), the projected MCP tool schema gets a single arbitrary `"type"` injected alongside `anyOf`. The MCP server's jsonschema validator then enforces that `"type"` strictly and rejects every call using the variant that lost the pick, with a misleading error like:
```
Input validation error: {'Skills': [...]} is not of type 'array'
```
The injected type is also non-deterministic (`next(iter(set))`), so which variant breaks can differ per run. Fixes tadata-org#307.
## Root cause
In `convert_openapi_to_mcp_tools`, both the query-param and body-param loops do:
```python
if "type" not in properties[param_name]:
properties[param_name]["type"] = get_single_param_type_from_schema(param_schema)
```
For `T | U` (both real types) Pydantic emits `{"anyOf": [...]}` with no top-level `"type"`, so the branch fires and one arbitrary variant type wins.
## Fix
Only fall back to `get_single_param_type_from_schema` when the union has at most one non-null variant. `T | None` (nullable) keeps the existing collapse behavior — its existing tests still pass unchanged — while `T | U` keeps `anyOf` verbatim, which the JSON Schema validator at the MCP layer handles correctly.
## Verification
- New test `test_union_type_schema_not_collapsed` asserts `anyOf` is preserved with no injected `"type"` and both `object`/`array` variants present.
- Repro from the issue validated with `jsonschema`: both the dict variant and the list variant pass validation (previously one of them always failed).
- Full suite: 70 passed (the 20 `test_sse_real_transport` errors are the pre-existing Windows `os.fork` limitation tracked in tadata-org#332).
This was referenced Sep 3, 2026
Hand-written OpenAPI specs can express unions with oneOf instead of anyOf. The injection guard only looked at anyOf, so a oneOf-only body schema still received a bogus "type": "string" alongside the union — same rejection risk as tadata-org#307. Treat anyOf/oneOf uniformly.
Author
|
Follow-up pushed: the guard now also covers oneOf-only unions (hand-written OpenAPI specs), which previously still received a bogus single injected type alongside the union — same rejection risk as the anyOf case. New test test_oneof_body_schema_not_collapsed covers it. 79/79 tests green. |
The path-param loop injected a default "type" with no union awareness, so a path param typed int | str received both anyOf and a bogus "type": "string" - the same corruption tadata-org#307 describes, through the one loop the previous commits missed. Apply the same guard used in the query and body loops.
Author
|
Second follow-up: the path-param loop had the same injection bug (path param typed int | str got both anyOf and a bogus injected type — verified live before the fix). Same guard applied there; test_union_path_param_not_collapsed covers it. All three param loops (path/query/body) now treat unions uniformly. 80/80 tests green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(openapi): preserve anyOf for non-null union body params instead of injecting an arbitrary type
Problem
For a request body field typed as a union of two real types (e.g.
dict[str, list[str]] | list[str]), the projected MCP tool schema gets a single arbitrary"type"injected alongsideanyOf. The MCP server's jsonschema validator then enforces that"type"strictly and rejects every call using the variant that lost the pick, with a misleading error like:The injected type is also non-deterministic (
next(iter(set))), so which variant breaks can differ per run. Fixes #307.Root cause
In
convert_openapi_to_mcp_tools, both the query-param and body-param loops do:For
T | U(both real types) Pydantic emits{"anyOf": [...]}with no top-level"type", so the branch fires and one arbitrary variant type wins.Fix
Only fall back to
get_single_param_type_from_schemawhen the union has at most one non-null variant.T | None(nullable) keeps the existing collapse behavior — its existing tests still pass unchanged — whileT | UkeepsanyOfverbatim, which the JSON Schema validator at the MCP layer handles correctly.Verification
test_union_type_schema_not_collapsedassertsanyOfis preserved with no injected"type"and bothobject/arrayvariants present.jsonschema: both the dict variant and the list variant pass validation (previously one of them always failed).test_sse_real_transporterrors are the pre-existing Windowsos.forklimitation tracked in test: skip real-transport tests on Windows (they require os.fork) #332).