fix: repair anyOf tool schemas rejected by the Kimi wire - #14
Merged
Conversation
Kimi rejects a tool whose parameters declare `anyOf` next to `type`, and also rejects a validation keyword appearing on both a node and its `anyOf` branches. TaskStop hits both: it adds a hand-written root `anyOf` to an object schema, so every request carrying it failed with a 400 before the model ever ran. Repair the shape where all tools converge instead of at the one call site, so MCP- and plugin-contributed schemas are covered too: - Nested nodes distribute their own constraints into each branch, which accepts exactly the same instances as before. - The root instead drops its `anyOf`. A tool's parameters must be an object, and the wire demands `type: "object"` there, so the branch form is unsatisfiable at that position; branch properties are merged up first so a tool cannot lose its arguments. Verified by replaying every builtin and connected MCP tool schema through the provider's own validator.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe schema normalizer folds root-level ChangesPythinker schema normalization
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/kosong/test/providers/pythinker-schema.test.ts (1)
573-580: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse neutral test identifiers.
Replace
TaskStop,task_id, andshell_idwith neutral test identifiers. This test checks schema normalization and needs only two distinct generic field names.As per coding guidelines, use neutral placeholders such as
example.com,example.test, andYOUR_API_KEYinstead of real internal identifiers in public text and test data.Also applies to: 598-603
🤖 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 `@packages/kosong/test/providers/pythinker-schema.test.ts` around lines 573 - 580, Update the test case around the root anyOf normalization scenario to use neutral identifiers: replace the TaskStop reference and the task_id and shell_id property names with two distinct generic field names, keeping the schema structure and assertions unchanged.Source: Coding guidelines
🤖 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 `@packages/kosong/src/providers/pythinker-schema.ts`:
- Around line 354-358: Update the anyOf normalization logic around the
inherited-key loop in packages/kosong/src/providers/pythinker-schema.ts:354-358
so parent and branch constraints are combined rather than allowing an existing
branch key to replace the parent restriction; preserve conjunctive required
constraints across every branch. In
packages/kosong/test/providers/pythinker-schema.test.ts:615-638, replace the
current string-versus-integer items expectation with tests proving parent
constraints remain enforced in each normalized anyOf branch.
- Around line 163-169: The root anyOf merge in the branch-property loop must
preserve conflicting schemas instead of retaining only the first property
encountered. Update the merge logic around merged and cloneJsonValue so
duplicate names combine both alternatives without depending on iteration order,
preserving valid string-or-integer contracts for convertTool; add a regression
test covering conflicting root property names.
In `@packages/kosong/test/providers/pythinker-schema.test.ts`:
- Around line 654-671: Update the test case around normalizePythinkerToolSchema
to place the cyclic $defs and $ref directly on the object containing anyOf,
rather than on the root. Assert that both keys remain on the anyOf parent and
are absent from each branch, using non-vacuous checks that fail when the
expected retained values are missing.
---
Nitpick comments:
In `@packages/kosong/test/providers/pythinker-schema.test.ts`:
- Around line 573-580: Update the test case around the root anyOf normalization
scenario to use neutral identifiers: replace the TaskStop reference and the
task_id and shell_id property names with two distinct generic field names,
keeping the schema structure and assertions unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38cfebe6-f821-4dc5-88b9-fc32980738d3
📒 Files selected for processing (3)
.changeset/kimi-anyof-tool-schema.mdpackages/kosong/src/providers/pythinker-schema.tspackages/kosong/test/providers/pythinker-schema.test.ts
Closed
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
1 participant
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.
Related Issue
No existing issue — the problem is described below. This is a reproducible bug fix with a focused diff.
Problem
Every request to a Kimi / Moonshot model fails before the model runs:
The wire validator refuses a tool schema that uses
anyOfas a refinement of thenode it sits on. Two rules are involved:
typemay not appear next toanyOfon the same node.properties,items,additionalProperties, …) mayappear on both a node and one of its
anyOfbranches.Both are legal standard JSON Schema, so schemas that work on every other provider
are rejected here.
TaskStoptrips them: it adds a hand-writtenanyOf: [{ required: ['task_id'] }, { required: ['shell_id'] }]on top of anobject schema that already declares
typeandproperties. Because tooldefinitions are sent with every request, a single offending tool takes down the
whole session.
normalizePythinkerToolSchemaalready existed for exactly this class ofprovider-compatibility repair, but it only visited nested property schemas and
deliberately skipped the root, so it never saw the offending node.
What changed
The repair lives in the provider's schema normalizer rather than in
TaskStop, soMCP- and plugin-contributed schemas are covered by the same pass — this shape is
common enough that several other clients have hit it independently.
Nested nodes — distribute. A node's own constraints are pushed down into each
anyOfbranch, and branches that already declare a keyword keep their own(narrower) value. This is lossless:
P ∧ (B₁ ∨ B₂)and(P ∧ B₁) ∨ (P ∧ B₂)accept exactly the same instances. Sibling combinators (
allOf/oneOf/not/if) are left in place — the validator does not read them, so moving them wouldonly churn the schema.
Root — fold away. The root cannot use the branch form, because a tool's
parameters must be a plain object: the wire separately requires
parameters.type == "object", which cannot coexist with a rootanyOf. The twoconstraints are jointly unsatisfiable, so the root's
anyOfis dropped. That onlyever widens what is accepted, and a root
anyOfis in practice a "one of thesefields is required" hint that the tool re-checks at run time (
TaskStopalreadyreturns
Missing required parameter: task_id). Branch properties are merged intothe root first, so a schema that kept its arguments inside the branches does not
silently lose them.
Verification
Rather than asserting against an assumed reading of the spec, every schema was
replayed through the provider's own validator:
connected MCP servers through the normalizer and into the provider's validator:
106 accepted, 0 rejected. Before the change,
TaskStopwas the single failure.kosongsuite passes (1185 tests);typecheckclean.actually fail without it.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update — no user-facing behavior or CLI surface changes.Summary by CodeRabbit
Bug Fixes
anyOf,oneOf, andallOfcombinations.Tests