Skip to content

Commit 264f2f1

Browse files
teknium1hermes-agent
authored andcommitted
fix(moonshot): strip $ref siblings and collapse tuple items in tool schemas (NousResearch#27104)
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.
1 parent 9b70ad2 commit 264f2f1

2 files changed

Lines changed: 194 additions & 0 deletions

File tree

agent/moonshot_schema.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@
1515
2. When ``anyOf`` is used, ``type`` must be on the ``anyOf`` children, not
1616
the parent. Presence of both causes "type should be defined in anyOf
1717
items instead of the parent schema".
18+
3. ``enum`` arrays on scalar-typed nodes may not contain ``null`` or empty
19+
strings. Strip those entries (drop the enum entirely if it becomes empty).
20+
4. ``$ref`` nodes may not carry sibling keywords. Moonshot expands the
21+
reference before validation and then rejects the node if sibling keys
22+
like ``description`` remain on the same node as ``$ref``. Strip every
23+
sibling from ``$ref`` nodes so only ``{"$ref": "..."}`` survives.
24+
(Ported from anomalyco/opencode#24730.)
25+
5. ``items`` may not be a tuple-style array (``items: [schemaA, schemaB]``
26+
for positional element schemas). Moonshot's schema engine requires a
27+
single object schema applied to every array element. Collapse tuple
28+
``items`` to the first element schema (or ``{}`` if the tuple is empty).
29+
(Ported from anomalyco/opencode#24730.)
1830
1931
The ``#/definitions/...`` → ``#/$defs/...`` rewrite for draft-07 refs is
2032
handled separately in ``tools/mcp_tool._normalize_mcp_input_schema`` so it
@@ -66,6 +78,16 @@ def _repair_schema(node: Any, is_schema: bool = True) -> Any:
6678
}
6779
elif key in _SCHEMA_LIST_KEYS and isinstance(value, list):
6880
repaired[key] = [_repair_schema(v, is_schema=True) for v in value]
81+
elif key == "items" and isinstance(value, list):
82+
# Rule 5: tuple-style ``items`` arrays (positional element
83+
# schemas) are not accepted by Moonshot. Collapse to the
84+
# first element schema if present, else to ``{}``. This
85+
# matches opencode's behaviour for moonshotai / kimi models.
86+
first = value[0] if value else {}
87+
if isinstance(first, dict):
88+
repaired[key] = _repair_schema(first, is_schema=True)
89+
else:
90+
repaired[key] = first
6991
elif key in _SCHEMA_NODE_KEYS:
7092
# items / not / additionalProperties: single nested schema.
7193
# additionalProperties can also be a bool — leave those alone.
@@ -130,6 +152,15 @@ def _repair_schema(node: Any, is_schema: bool = True) -> Any:
130152
else:
131153
repaired.pop("enum")
132154

155+
# Rule 4: $ref nodes must not have sibling keywords. Moonshot expands
156+
# the reference before validation and then rejects the node if siblings
157+
# like ``description`` / ``type`` / ``default`` appear alongside $ref.
158+
# The referenced definition still carries its own description on the
159+
# target node, which Moonshot accepts.
160+
# (Ported from anomalyco/opencode#24730.)
161+
if "$ref" in repaired:
162+
return {"$ref": repaired["$ref"]}
163+
133164
return repaired
134165

135166

tests/agent/test_moonshot_schema.py

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
1. Properties without ``type`` — Moonshot requires ``type`` on every node.
77
2. ``type`` at the parent of ``anyOf`` — Moonshot requires it only inside
88
``anyOf`` children.
9+
3. ``$ref`` with sibling keywords — Moonshot expands the ref first and then
10+
rejects ``description``/``type`` siblings on the same node.
11+
(Ported from anomalyco/opencode#24730.)
12+
4. Tuple-style ``items`` arrays — Moonshot requires a single item schema,
13+
not positional ones. (Ported from anomalyco/opencode#24730.)
914
1015
These tests cover the repairs applied by ``agent/moonshot_schema.py``.
1116
"""
@@ -180,6 +185,164 @@ def test_anyof_enum_with_null_collapsed(self):
180185
assert db_type["enum"] == ["mysql", "postgresql"] # "" stripped by enum cleanup
181186

182187

188+
class TestRefSiblingStripping:
189+
"""Rule 4: ``$ref`` nodes may not carry sibling keywords on Moonshot.
190+
191+
Ported from anomalyco/opencode#24730. The real-world failure was MCP tools
192+
whose generated schemas put a ``description`` on a ``$ref`` property so the
193+
model would see the field's human-readable hint. The reference stays — the
194+
referenced definition still owns the description (on the target node itself)
195+
and still serves the model's context.
196+
"""
197+
198+
def test_description_sibling_stripped_from_ref(self):
199+
params = {
200+
"type": "object",
201+
"properties": {
202+
"variantOptions": {
203+
"$ref": "#/$defs/VariantOptions",
204+
"description": "Required. The variant options for generation.",
205+
},
206+
},
207+
"$defs": {
208+
"VariantOptions": {
209+
"type": "object",
210+
"properties": {},
211+
"description": "Configuration options.",
212+
},
213+
},
214+
}
215+
out = sanitize_moonshot_tool_parameters(params)
216+
# Sibling stripped.
217+
assert out["properties"]["variantOptions"] == {"$ref": "#/$defs/VariantOptions"}
218+
# The target definition's own description is preserved — we only strip
219+
# siblings ON the $ref node, not on the thing it points at.
220+
assert out["$defs"]["VariantOptions"]["description"] == "Configuration options."
221+
222+
def test_multiple_siblings_all_stripped(self):
223+
params = {
224+
"type": "object",
225+
"properties": {
226+
"p": {
227+
"$ref": "#/$defs/T",
228+
"type": "object",
229+
"description": "x",
230+
"default": {},
231+
"title": "P",
232+
},
233+
},
234+
"$defs": {"T": {"type": "object"}},
235+
}
236+
out = sanitize_moonshot_tool_parameters(params)
237+
assert out["properties"]["p"] == {"$ref": "#/$defs/T"}
238+
239+
def test_ref_without_siblings_unchanged(self):
240+
params = {
241+
"type": "object",
242+
"properties": {"p": {"$ref": "#/$defs/T"}},
243+
"$defs": {"T": {"type": "object"}},
244+
}
245+
out = sanitize_moonshot_tool_parameters(params)
246+
assert out["properties"]["p"] == {"$ref": "#/$defs/T"}
247+
248+
def test_ref_inside_anyof_children(self):
249+
params = {
250+
"type": "object",
251+
"properties": {
252+
"v": {
253+
"anyOf": [
254+
{"$ref": "#/$defs/A", "description": "variant A"},
255+
{"type": "null"},
256+
],
257+
},
258+
},
259+
"$defs": {"A": {"type": "object"}},
260+
}
261+
out = sanitize_moonshot_tool_parameters(params)
262+
# Main's existing Rule 2 collapses anyOf-with-null down to the
263+
# single non-null branch (Moonshot rejects null branches in anyOf
264+
# outright). That branch was originally `{"$ref": ..., "description": ...}`;
265+
# Rule 4 then strips the sibling, leaving exactly `{"$ref": "..."}`.
266+
# The test name still applies — Rule 4 ran on the $ref branch — it
267+
# just happens after the anyOf collapse on this input.
268+
assert out["properties"]["v"] == {"$ref": "#/$defs/A"}
269+
270+
271+
class TestTupleItems:
272+
"""Rule 5: tuple-style ``items`` arrays collapse to a single schema.
273+
274+
Ported from anomalyco/opencode#24730. Moonshot's schema engine requires
275+
``items`` to be ONE schema object applied to every array element; tuple-
276+
style positional item schemas are rejected. We collapse to the first
277+
element's schema (which is the "closest" interpretation of positional →
278+
single) and drop the rest.
279+
"""
280+
281+
def test_tuple_items_collapsed_to_first(self):
282+
params = {
283+
"type": "object",
284+
"properties": {
285+
"renderedSize": {
286+
"type": "array",
287+
"items": [{"type": "number"}, {"type": "number"}],
288+
"minItems": 2,
289+
"maxItems": 2,
290+
},
291+
},
292+
}
293+
out = sanitize_moonshot_tool_parameters(params)
294+
assert out["properties"]["renderedSize"]["items"] == {"type": "number"}
295+
# Sibling constraints are preserved — only the tuple shape is repaired.
296+
assert out["properties"]["renderedSize"]["minItems"] == 2
297+
298+
def test_empty_tuple_items_becomes_empty_schema(self):
299+
# Empty tuple collapses to ``{}``; the generic repair then fills a
300+
# synthetic ``type`` because Moonshot requires ``type`` on every
301+
# schema node. Either ``{}`` or ``{"type": "string"}`` is a valid
302+
# final shape for Moonshot — both accept any string element — but we
303+
# always go through ``_fill_missing_type`` so the result is fully
304+
# well-formed without needing the consumer to patch it later.
305+
params = {
306+
"type": "object",
307+
"properties": {
308+
"things": {"type": "array", "items": []},
309+
},
310+
}
311+
out = sanitize_moonshot_tool_parameters(params)
312+
items = out["properties"]["things"]["items"]
313+
# Must be a dict and must carry a ``type`` (the whole point of Rule 1).
314+
assert isinstance(items, dict)
315+
assert items.get("type")
316+
317+
def test_tuple_items_first_element_is_repaired(self):
318+
# The first element itself has a missing type — it should be filled.
319+
params = {
320+
"type": "object",
321+
"properties": {
322+
"pair": {
323+
"type": "array",
324+
"items": [{"description": "first"}, {"description": "second"}],
325+
},
326+
},
327+
}
328+
out = sanitize_moonshot_tool_parameters(params)
329+
# Repaired to a single schema with a synthetic type.
330+
assert out["properties"]["pair"]["items"] == {
331+
"description": "first",
332+
"type": "string",
333+
}
334+
335+
def test_single_schema_items_unchanged(self):
336+
params = {
337+
"type": "object",
338+
"properties": {
339+
"tags": {"type": "array", "items": {"type": "string"}},
340+
},
341+
}
342+
out = sanitize_moonshot_tool_parameters(params)
343+
assert out["properties"]["tags"]["items"] == {"type": "string"}
344+
345+
183346
class TestTopLevelGuarantees:
184347
"""The returned top-level schema is always a well-formed object."""
185348

0 commit comments

Comments
 (0)