fix(moonshot): strip $ref siblings and collapse tuple items in tool schemas#18130
Closed
teknium1 wants to merge 1 commit into
Closed
fix(moonshot): strip $ref siblings and collapse tuple items in tool schemas#18130teknium1 wants to merge 1 commit into
teknium1 wants to merge 1 commit into
Conversation
…chemas Port from anomalyco/opencode#24730: Moonshot's JSON Schema validator rejects two shapes that the rest of the JSON Schema ecosystem accepts: 1. $ref nodes with sibling keywords. Moonshot expands the reference before validation and then rejects the node if keys like `description`, `type`, or `default` appear alongside $ref. MCP-sourced tool schemas commonly put a `description` on $ref-typed properties so the model sees the field hint — which worked on every provider except Moonshot. 2. Tuple-style `items` arrays (positional element schemas). Moonshot's engine requires ONE schema applied to every array element. Common in tool schemas generated from Go/Protobuf that model fixed-length arrays as `[{type:number}, {type:number}]`. Repairs applied in `agent/moonshot_schema.py`: - Rule 3: when a node has `$ref`, return `{"$ref": <value>}` only (strip every sibling). The referenced definition still carries its own description on the target node, which Moonshot accepts. - Rule 4: when `items` is a list, collapse to the first element schema (falling back to `{}` which is then filled by the generic missing-type rule). Preserves `minItems` / `maxItems` / other siblings. Tests: 10 new cases across TestRefSiblingStripping + TestTupleItems, plus the existing TestMissingTypeFilled::test_ref_node_is_not_given_synthetic_type still passes (it asserted plain $ref passes through; now it passes through as exactly `{"$ref": "..."}` which is strictly compatible). All 35 tests in test_moonshot_schema.py pass.
4 tasks
Contributor
Author
|
Closing in favor of the merged salvage PR. Conflict resolution: main added an enum-null cleanup (its own Rule 3) between this PR's branch and current main; renumbered so enum=Rule 3, $ref-sibling=Rule 4, tuple-items=Rule 5. One test expectation updated to match main's anyOf-with-null collapse. 42 moonshot-schema tests pass, 98 adjacent moonshot/kimi tests pass, E2E with all 5 rules on one schema confirms correct interaction. |
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
2 participants
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.
Summary
Extends Hermes's Moonshot/Kimi tool-schema sanitizer with two repairs that MCP-sourced tool schemas routinely trip over, ported from anomalyco/opencode#24730.
Before: a subset of MCP tool schemas (any containing
$refwith adescriptionsibling, or any with tuple-styleitems) caused Moonshot/Kimi to return HTTP 400tools.function.parameters is not a valid moonshot flavored json schemaeven though every other provider accepted them.After: those two shapes are normalized in-place before the request goes out.
Root cause
Moonshot's schema validator expands
$refbefore validation, then rejects the node when any sibling keyword survives alongside$ref(includingdescription, which MCP tools use heavily to expose the field hint to the model). Separately, its engine refuses tuple-styleitemsarrays and requires one schema applied to every array element.Hermes's existing sanitizer already handled Rules 1–2 (missing
type,typeatanyOfparent). The$refpath previouslyreturn repairedverbatim — passing any siblings straight through. Tupleitemswasn't touched at all.Changes
agent/moonshot_schema.py$refnodes collapse to{"$ref": "…"}only. Rule 4:itemslists collapse to the first element schema (then run through the normal repair).tests/agent/test_moonshot_schema.pyTestRefSiblingStrippingandTestTupleItems. The pre-existingtest_ref_node_is_not_given_synthetic_typestill passes — it asserted a plain$refpasses through, and it still does (now exactly as{"$ref": "…"}).Validation
scripts/run_tests.sh tests/agent/test_moonshot_schema.py→ 35/35 passed.$ref+description, tuple items, anyOf parent type, missing type, nested$defs): all four rules fire together, output is Moonshot-compatible, target definitions' own descriptions are preserved.Source
Ported from anomalyco/opencode#24730 ("fix: sanitize tools for moonshot"), adapted from the TypeScript
sanitizeMoonshothelper to fit Hermes's recursive_repair_schemawalker. The opencode version also runs onkimimodels routed through aggregators (OpenRouter); Hermes's existingis_moonshot_model()already covers that case.