fix(google): sanitize unsupported JSON Schema keywords for Gemini API#13666
fix(google): sanitize unsupported JSON Schema keywords for Gemini API#13666kui123456789 wants to merge 2 commits intoanomalyco:devfrom
Conversation
The sanitizeGemini function was incomplete, causing INVALID_ARGUMENT (400) errors when MCP tools exposed schemas with unsupported JSON Schema constructs. Changes: - Strip additionalProperties, , , , , definitions, const, default, title, pattern, patternProperties, propertyNames, uniqueItems, minItems, maxItems, minLength, maxLength, minimum, maximum, exclusiveMinimum, exclusiveMaximum, multipleOf, if/then/else, not, allOf, anyOf, oneOf from tool schemas before sending to Gemini - Convert anyOf/oneOf with const values to string enum - Infer type=object when properties/required exist but type is missing Fixes anomalyco#12908 anomalyco#13148 anomalyco#9233 anomalyco#12295
|
The following comment was made by an LLM, it may be inaccurate: Found several related PRs that may be duplicates or closely related: Potential Duplicates/Related PRs:
Why these are related: PR #13666 consolidates multiple related Gemini schema sanitization fixes into a single comprehensive solution. PRs #12911 and #13150 appear to be partial solutions addressing overlapping issues. |
There was a problem hiding this comment.
Pull request overview
This PR enhances the sanitizeGemini function in the provider transformation layer to fix multiple issues where Google Gemini API rejects MCP tool schemas containing unsupported JSON Schema keywords. The changes address four related GitHub issues where Gemini returns INVALID_ARGUMENT (400) errors when encountering schemas with additionalProperties, anyOf/oneOf patterns, missing type fields, and $ref references.
Changes:
- Adds conversion logic for
anyOf/oneOfwith const values to Gemini-compatible enum format - Strips 28 unsupported JSON Schema keywords (including
additionalProperties,$ref,$schema, validation constraints, and composition keywords) - Infers
type: "object"whenpropertiesorrequiredexist buttypeis missing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If anyOf/oneOf contains type variants, pick the first valid one | ||
| const typeVariants = variants.filter((v: any) => v && typeof v === "object" && v.type) | ||
| if (typeVariants.length > 0) { | ||
| const merged: any = { ...obj, ...typeVariants[0] } | ||
| delete merged.anyOf | ||
| delete merged.oneOf | ||
| return sanitizeGemini(merged) | ||
| } |
There was a problem hiding this comment.
The fallback logic for handling anyOf/oneOf with type variants may produce unexpected results. When multiple type variants exist, selecting only the first variant discards the other variants without considering whether they represent compatible types or important constraints. For example, if anyOf contains [{"type": "string", "minLength": 5}, {"type": "string", "maxLength": 10}], only the minLength constraint would be preserved. Consider whether this behavior is acceptable for Gemini API compatibility, or if a more sophisticated merge strategy is needed.
| key === "else" || | ||
| key === "not" || | ||
| key === "title" || | ||
| key === "allOf" || |
There was a problem hiding this comment.
The code strips allOf, anyOf, and oneOf keywords from the schema but does not handle allOf which could be used to merge schemas. Unlike anyOf/oneOf which represent alternatives, allOf represents a combination of all schemas and could potentially be merged into a single schema object. Consider adding logic to handle allOf by merging its array elements into the parent schema, similar to how anyOf/oneOf with const values are handled.
| if (obj.anyOf || obj.oneOf) { | ||
| const variants = obj.anyOf || obj.oneOf | ||
| if (Array.isArray(variants)) { | ||
| const constValues = variants | ||
| .filter((v: any) => v && typeof v === "object" && "const" in v) | ||
| .map((v: any) => String(v.const)) | ||
| if (constValues.length === variants.length && constValues.length > 0) { | ||
| const merged: any = { ...obj, type: "string", enum: constValues } | ||
| delete merged.anyOf | ||
| delete merged.oneOf | ||
| delete merged.const | ||
| return sanitizeGemini(merged) | ||
| } | ||
| // If anyOf/oneOf contains type variants, pick the first valid one | ||
| const typeVariants = variants.filter((v: any) => v && typeof v === "object" && v.type) | ||
| if (typeVariants.length > 0) { | ||
| const merged: any = { ...obj, ...typeVariants[0] } | ||
| delete merged.anyOf | ||
| delete merged.oneOf | ||
| return sanitizeGemini(merged) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
When anyOf/oneOf contains a mix of const values and other schema types (e.g., [{"const": "foo"}, {"type": "string"}]), the const values are extracted but the logic checks if constValues.length === variants.length. This means mixed arrays are not converted to enum, and the variants are later stripped entirely on line 939-940. This could result in loss of schema information. Consider logging a warning when schema information is being discarded, or implementing a more graceful degradation strategy.
| const constValues = variants | ||
| .filter((v: any) => v && typeof v === "object" && "const" in v) | ||
| .map((v: any) => String(v.const)) | ||
| if (constValues.length === variants.length && constValues.length > 0) { | ||
| const merged: any = { ...obj, type: "string", enum: constValues } | ||
| delete merged.anyOf | ||
| delete merged.oneOf | ||
| delete merged.const | ||
| return sanitizeGemini(merged) |
There was a problem hiding this comment.
When converting anyOf/oneOf with const values to enum, the description field from individual const objects is lost. For example, {"anyOf": [{"const": "append", "description": "Add to end"}, {"const": "prepend", "description": "Add to start"}]} becomes {"type": "string", "enum": ["append", "prepend"]} without preserving the descriptions for each enum value. Consider preserving the parent description if it exists, or documenting that per-value descriptions cannot be represented in Gemini's enum format.
| key === "$id" || | ||
| key === "$defs" || | ||
| key === "definitions" || | ||
| key === "default" || |
There was a problem hiding this comment.
The code strips the "default" keyword which may result in loss of default values that could be useful for API consumers. According to issue #12295, Gemini allows "default" alongside $ref, suggesting it may be supported in some contexts. Consider verifying through testing whether "default" needs to be stripped in all contexts or only in specific ones, and document the reasoning for stripping it if it's necessary for API compatibility.
| @@ -896,6 +955,11 @@ export namespace ProviderTransform { | |||
| } | |||
| } | |||
|
|
|||
| // Infer type="object" when properties exist but type is missing | |||
| if (!result.type && (result.properties || result.required)) { | |||
| result.type = "object" | |||
| } | |||
There was a problem hiding this comment.
The new functionality for handling anyOf/oneOf with const values, stripping unsupported keywords, and inferring type: "object" lacks test coverage. While the test file packages/opencode/test/provider/transform.test.ts contains tests for other Gemini schema transformations, there are no tests verifying:
- Conversion of anyOf/oneOf with const values to enum
- Stripping of the 28 newly-added unsupported keywords (additionalProperties, $ref, $schema, etc.)
- Type inference when properties/required exist without type
Consider adding comprehensive tests for these new behaviors to prevent regressions.
| // Skip keywords unsupported by Gemini | ||
| if ( | ||
| key === "additionalProperties" || | ||
| key === "$ref" || |
There was a problem hiding this comment.
The code strips $ref without resolving it to the referenced schema. Issue #12295 mentions that Gemini API rejects schemas containing $ref references and expects them to be expanded. Simply removing $ref will result in an incomplete schema (e.g., {"$ref": "#/$defs/MyType", "description": "..."} becomes just {"description": "..."}). To properly fix this issue, $ref should be resolved by looking up the reference in $defs/definitions and merging the referenced schema into the current location before stripping occurs.
Address code review feedback on sanitizeGemini: - Resolve local $ref references (#/$defs, #/definitions) instead of stripping - Merge allOf sub-schemas (properties, required, type) instead of deleting - Merge object-type anyOf/oneOf variants instead of picking only first - Add comprehensive unit tests for all sanitization paths Co-authored-by: Copilot <copilot@github.com>
1 participant
Summary
The
sanitizeGeminifunction inpackages/opencode/src/provider/transform.tswas incomplete, causingINVALID_ARGUMENT(400) errors when MCP tools exposed schemas with unsupported JSON Schema constructs (e.g.additionalProperties,anyOf/oneOf,$ref, missingtype).Problem
When using Google Gemini models with MCP tools, the API frequently returns:
Root causes:
convertMcpTool()addsadditionalProperties: falseto every MCP tool schema, but Gemini API doesn't support this keywordsanitizeGemini()doesn't remove unsupported JSON Schema keywords likeadditionalProperties,$ref,$schema,minItems,maxItems,pattern,const,default,title, etc.anyOf/oneOfwithconstvalues are not converted to Gemini-compatibleenumtypeis undefined, the cleanup logic forproperties/requiredis short-circuited (becauseresult.type && result.type !== "object"evaluates tofalse, skipping all cleanup)Changes
additionalProperties,$ref,$schema,$id,$defs,definitions,const,default,title,pattern,patternProperties,propertyNames,uniqueItems,minItems,maxItems,minLength,maxLength,minimum,maximum,exclusiveMinimum,exclusiveMaximum,multipleOf,if,then,else,not,allOf,anyOf,oneOfanyOf/oneOfwithconstvalues to stringenum(Gemini-compatible)type: "object"whenpropertiesorrequiredexist buttypeis missingRelated Issues
Fixes #12908 (anyOf/const not handled)
Fixes #13148 (type=undefined causes missed cleanup)
Fixes #9233 (empty params missing type:object)
Fixes #12295 ($ref not resolved)
Testing
Tested with MCP tools that have:
{}) → correctly inferred astype: "object"additionalProperties: false→ correctly strippedanyOfwithconstvalues → correctly converted toenumtype→ unchanged behavior