fix: remove top-level anyOf/oneOf/allOf from tool input schemas for Anthropic API compatibility#558
fix: remove top-level anyOf/oneOf/allOf from tool input schemas for Anthropic API compatibility#558GenNetEng wants to merge 3 commits into
Conversation
…c API compatibility
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesAnthropic-compatible schema flattening
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@utils/schema.ts`:
- Around line 112-128: Flattening in the schema normalization logic drops
variant-level required fields because the merge in the combiner loop only copies
variant.properties from fixedSchema[combiner]. Update the flattening path in
utils/schema.ts to also collect and merge each variant’s required array into
fixedSchema.required (deduplicated), alongside the existing property merge,
before deleting the combiner. Keep the change scoped to the object-variant
handling in the combiner loop so inputSchema preserves required args after
flattening.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 366f6d39-7ae7-4ead-8b5f-02bbc0beabed
📒 Files selected for processing (1)
utils/schema.ts
…e intersection of variant required arrays so only fields required in ALL variants remain globally required after flattening. This correctly handles mutually exclusive unions (e.g., id OR full_path) without erroneously requiring both.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@utils/schema.ts`:
- Around line 127-139: The required-field merge in the allOf handling inside
schema flattening is using an intersection, which is semantically wrong for
allOf. Update the logic around the variants/requiredSets/sharedRequired block in
utils/schema.ts so that fixedSchema.required is built from the union of required
fields across all variants, not the intersection, while still preserving any
existing required entries already on fixedSchema.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 64ca3712-92ef-4491-b0aa-126c73018610
📒 Files selected for processing (1)
utils/schema.ts
…anyOf/oneOf allOf means all schemas apply simultaneously so all their requirements apply (union). anyOf/oneOf means one schema applies so only shared requirements are universal (intersection).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
utils/schema.ts (1)
121-124: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDropping duplicate property schemas weakens flattened validation semantics.
When the same property exists in multiple variants, the current “first wins” merge discards later constraints. That is incorrect for flattened combiners:
allOfshould preserve all constraints, andanyOf/oneOfshould preserve alternates. Otherwise the resulting tool schema can misvalidate inputs.Suggested fix
for (const variant of variants) { for (const [key, value] of Object.entries(variant.properties)) { - if (!fixedSchema.properties[key]) { - fixedSchema.properties[key] = value; - } + const existingProp = fixedSchema.properties[key]; + if (!existingProp) { + fixedSchema.properties[key] = value; + } else { + const nested = combiner; + const current = fixedSchema.properties[key]; + if (current?.[nested] && Array.isArray(current[nested])) { + current[nested].push(value); + } else { + fixedSchema.properties[key] = { [nested]: [current, value] }; + } + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/schema.ts` around lines 121 - 124, The merge logic in the schema flattening path is dropping duplicate properties by keeping only the first variant’s definition, which loses constraints for later variants. Update the property merge in the schema combiner/flattening code (the loop over variant.properties in the schema builder) so duplicate keys are not skipped: preserve all constraints for allOf, and for anyOf/oneOf keep alternate schemas rather than discarding later ones. Make sure the final fixedSchema.properties construction reflects the full combinator semantics instead of a “first wins” merge.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@utils/schema.ts`:
- Around line 121-124: The merge logic in the schema flattening path is dropping
duplicate properties by keeping only the first variant’s definition, which loses
constraints for later variants. Update the property merge in the schema
combiner/flattening code (the loop over variant.properties in the schema
builder) so duplicate keys are not skipped: preserve all constraints for allOf,
and for anyOf/oneOf keep alternate schemas rather than discarding later ones.
Make sure the final fixedSchema.properties construction reflects the full
combinator semantics instead of a “first wins” merge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1936d42e-3527-47fd-b243-a0dcedf92ce3
📒 Files selected for processing (1)
utils/schema.ts
|
I have also validated in kiro :) |
|
👋 Any chance this could get merged soon? This is currently blocking all users of Anthropic-based MCP clients (including opencode with GitHub Copilot provider) from using the gitlab-mcp server at all on versions >= 2.1.25. Multiple users hitting this in the wild, thanks for the fix! |
|
@Duo-Huang / other opencode users a workaround until this is merged is to disable this specific tool in opencode.json: "tools": {
"gitlab_get_ci_catalog_resource": false
} |
3 participants
Summary
Fixes the
toJSONSchema()utility to properly flatten and remove top-levelanyOf/oneOf/allOffrom tool input schemas. The Anthropic API rejects tool definitions that use these JSON Schema constructs at the top level ofinput_schema, breaking all Anthropic-based MCP clients (Kiro IDE, Claude Code, Claude Desktop, etc.) starting from v2.1.25.Problem
The existing flattening logic in
utils/schema.tsmerged anyOf variant properties into a single object but never deleted theanyOfkey itself, leaving it in the final output. This caused the Anthropic API to reject the tool list with:Affected tool:
get_ci_catalog_resource(usesz.union()for mutually exclusiveid/full_pathparameters).Fix
Replaced the incomplete flattening block with a generic handler that loops over all three combiners (
anyOf,oneOf,allOf), merges variant properties into a flat object schema, and deletes the combiner key. This runs in the sharedtoJSONSchema()utility so any future union schemas are handled automatically.Validation
type: "object"output with no top-level combinersFixes #552
Fixes #550
test-output.md