[Fix] Gemini tasks fail when MCP tools use nullable arrays - #1302
Open
mrubens wants to merge 10 commits into
Open
[Fix] Gemini tasks fail when MCP tools use nullable arrays#1302mrubens wants to merge 10 commits into
mrubens wants to merge 10 commits into
Conversation
Google AI Studio strictly validates function declarations: every ARRAY schema must carry items, and the AI SDK's JSON-Schema-to-Gemini conversion splits type: [T, "null"] unions into anyOf branches while leaving items/properties at the outer level. An upstream MCP tool schema that is valid JSON Schema therefore turns into an invalid request, failing every Gemini task turn for any workspace with that integration connected. Normalize the offending shapes at the MCP proxy boundary for all integrations: rewrite array/object type unions into explicit anyOf branches carrying their own type's keywords, fill missing array items with a permissive string schema, and rebuild every tools/list reply (not just restricted ones) so the normalization always applies.
Contributor
|
No new code issues found. See task
Reviewed b242080 |
- Never forward an unfiltered tools/list to a connection with tool restrictions or schema stripping: uncorrelatable SSE replies get a bounded response-frame-matched read, and unfilterable replies return a JSON-RPC error instead of the raw upstream stream - Tolerate upstreams that echo the JSON-RPC id as a different type - Only inject items into array schemas that declare none; declared items of any form (tuple, boolean, empty object) pass through - Whitelist per-type keywords when splitting type unions into anyOf branches; annotations stay at the top level only - Stop recursing into default/const/enum/examples data values - Log when a composite union with a sibling combinator is left un-normalized so strict-provider rejections stay diagnosable - Cancel the unread upstream tee branch on every tools/list rebuild
Splitting a composite type union that carries a non-type-specific sibling constraint ($ref, $dynamicRef, not, if/then/else) would drop the constraint and widen the accepted arguments. Treat those siblings like combinators and leave the schema untouched (with the decline log). dependentRequired/dependentSchemas are object-only keywords and now travel with the object branch.
additionalItems (draft-04 tuple companion) and unevaluatedItems join the array-only whitelist, unevaluatedProperties joins the object-only whitelist, so a nullable tuple union no longer loses its extra-item bound when split into anyOf branches.
unevaluatedItems is not equivalent to additionalItems: injecting a permissive items next to unevaluatedItems: false voids the constraint (every element becomes evaluated), widening an empty-only array to arbitrary string arrays. Treat items, prefixItems, additionalItems, unevaluatedItems, and contains as declared item constraints and only inject when none are present.
Invert the split decision from blacklisting known-dangerous siblings to allowlisting exactly-understood keywords: a composite type union is split only when every keyword it carries has a known branch placement. Combinators, references, conditionals, vendor extensions, and future-dialect keywords all decline (with a log naming them) instead of risking a semantic rewrite. Declining costs at most the pre-fix status quo for one tool; a wrong rewrite would corrupt the contract for every provider. Annotation keywords (description, title, default, examples, deprecated, readOnly, writeOnly) stay on the anyOf wrapper. R_DISABLE_MCP_SCHEMA_NORMALIZATION=true disables the transform without a rollback, since it runs on every tools/list for every workspace.
dependencies and dependentRequired are keyed by property names with array-of-property-name values; recursing into them as schemas rewrote an entry named 'type' into a type declaration. A dedicated traversal context leaves array-valued entries as data and normalizes only object-valued legacy draft-04 dependency schemas. dependencies also joins the object-only whitelist so it travels with the object branch.
Values under unrecognized keywords (vendor extensions, future dialects) are opaque data; recursing into them rewrote schema lookalikes nested inside even when the outer schema was declined. Recursion is now allowlist-driven like the split decision: only SCHEMA_VALUE_KEYWORDS positions, schema maps, and dependency maps are traversed. This also subsumes the previous default/const/enum/examples skip list.
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.
What changed
tools/listresults so Gemini receives valid array and object union declarations.tools/listresponses incrementally and return as soon as the matching JSON-RPC response arrives, rather than waiting for a long-lived stream to close.tools/listresponses.Why this change was made
Valid upstream MCP schemas can become invalid Gemini function declarations when composite nullable unions are converted downstream. Applying the normalization to every
tools/listresponse fixes that provider failure, while incremental SSE parsing prevents the proxy from hanging on Streamable HTTP servers that keep their response channel open.Impact
Gemini-backed tasks can load tools with nullable arrays and objects without poisoning the full tool list, and MCP integrations using long-lived SSE responses continue returning promptly. All 208 MCP handler tests, API type checking, changed-file lint and formatting, and the repository pre-push checks pass. Package-wide API lint still reports an unrelated pre-existing unused-disable warning in
apps/api/src/handlers/github/__tests__/notifyPullRequestTerminalStatus.test.ts.