From acc1b6e76a1c089074674e2f78223cb577cd03aa Mon Sep 17 00:00:00 2001 From: PRATHAMESH75 Date: Sat, 18 Jul 2026 15:15:02 +0530 Subject: [PATCH] fix(agent): inject empty required array on Moonshot object schemas Moonshot/Kimi's tool-parameter validator rejects object schemas that omit the required key with HTTP 400 ("required must be an array"), even though standard JSON Schema allows omitting it. Any Hermes tool with zero required parameters (browser_back, delegate_task, project_list, several MCP list_* tools, etc.) tripped this when routed to a Moonshot endpoint. Add a Rule 4 to the sanitizer: every object schema gets a required array, defaulting to []. Existing lists are preserved but pruned to names that actually appear in properties (dangling entries are also rejected upstream). Applied recursively to nested object schemas and to the coerced/empty top-level fallback. Fixes #66835 --- agent/moonshot_schema.py | 31 +++++++++++- tests/agent/test_moonshot_schema.py | 73 +++++++++++++++++++++++++++-- 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/agent/moonshot_schema.py b/agent/moonshot_schema.py index a7629c3f5dcb..3f1708ceb0ae 100644 --- a/agent/moonshot_schema.py +++ b/agent/moonshot_schema.py @@ -15,6 +15,9 @@ and MoonshotAI/kimi-cli#1595: 2. When ``anyOf`` is used, ``type`` must be on the ``anyOf`` children, not the parent. Presence of both causes "type should be defined in anyOf items instead of the parent schema". +3. Every object schema must carry a ``required`` array, even an empty one. + Standard JSON Schema allows omitting it; Moonshot 400s with + "required must be an array". The ``#/definitions/...`` → ``#/$defs/...`` rewrite for draft-07 refs is handled separately in ``tools/mcp_tool._normalize_mcp_input_schema`` so it @@ -130,9 +133,32 @@ def _repair_schema(node: Any, is_schema: bool = True) -> Any: else: repaired.pop("enum") + # Rule 4: object schemas must carry a `required` array, even when empty. + if repaired.get("type") == "object": + repaired = _ensure_required_array(repaired) + return repaired +def _ensure_required_array(node: Dict[str, Any]) -> Dict[str, Any]: + """Guarantee an object schema carries a ``required`` array (Moonshot rule). + + Standard JSON Schema lets you omit ``required`` when nothing is required; + Moonshot 400s on that ("required must be an array"). Ensure the key is a + list. When ``properties`` is known, prune ``required`` entries that don't + name a real property — defensive against dangling names, which Moonshot + also rejects. Mutates and returns ``node``. + """ + props = node.get("properties") + req = node.get("required") + if isinstance(req, list): + if isinstance(props, dict): + node["required"] = [r for r in req if r in props] + else: + node["required"] = [] + return node + + def _fill_missing_type(node: Dict[str, Any]) -> Dict[str, Any]: """Infer a reasonable ``type`` if this schema node has none.""" node_type = node.get("type") @@ -174,17 +200,18 @@ def sanitize_moonshot_tool_parameters(parameters: Any) -> Dict[str, Any]: applied. Input is not mutated. """ if not isinstance(parameters, dict): - return {"type": "object", "properties": {}} + return {"type": "object", "properties": {}, "required": []} repaired = _repair_schema(copy.deepcopy(parameters), is_schema=True) if not isinstance(repaired, dict): - return {"type": "object", "properties": {}} + return {"type": "object", "properties": {}, "required": []} # Top-level must be an object schema if repaired.get("type") != "object": repaired["type"] = "object" if "properties" not in repaired: repaired["properties"] = {} + _ensure_required_array(repaired) return repaired diff --git a/tests/agent/test_moonshot_schema.py b/tests/agent/test_moonshot_schema.py index 339e6ab529ee..6dc7944292bc 100644 --- a/tests/agent/test_moonshot_schema.py +++ b/tests/agent/test_moonshot_schema.py @@ -188,9 +188,10 @@ class TestTopLevelGuarantees: """The returned top-level schema is always a well-formed object.""" def test_non_dict_input_returns_empty_object(self): - assert sanitize_moonshot_tool_parameters(None) == {"type": "object", "properties": {}} - assert sanitize_moonshot_tool_parameters("garbage") == {"type": "object", "properties": {}} - assert sanitize_moonshot_tool_parameters([]) == {"type": "object", "properties": {}} + empty = {"type": "object", "properties": {}, "required": []} + assert sanitize_moonshot_tool_parameters(None) == empty + assert sanitize_moonshot_tool_parameters("garbage") == empty + assert sanitize_moonshot_tool_parameters([]) == empty def test_non_object_top_level_coerced(self): params = {"type": "string"} @@ -212,6 +213,66 @@ class TestTopLevelGuarantees: assert "type" not in params["properties"]["q"] +class TestRequiredArray: + """Rule 4: every object schema must carry a ``required`` array (#66835).""" + + def test_empty_object_gets_empty_required(self): + out = sanitize_moonshot_tool_parameters({"type": "object", "properties": {}}) + assert out["required"] == [] + + def test_object_with_only_optional_props_gets_empty_required(self): + params = { + "type": "object", + "properties": {"q": {"type": "string"}}, + } + out = sanitize_moonshot_tool_parameters(params) + assert out["required"] == [] + + def test_existing_required_preserved(self): + params = { + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"], + } + out = sanitize_moonshot_tool_parameters(params) + assert out["required"] == ["q"] + + def test_dangling_required_pruned(self): + params = { + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q", "ghost"], + } + out = sanitize_moonshot_tool_parameters(params) + assert out["required"] == ["q"] + + def test_non_list_required_replaced(self): + params = { + "type": "object", + "properties": {}, + "required": "q", # invalid: string, not array + } + out = sanitize_moonshot_tool_parameters(params) + assert out["required"] == [] + + def test_nested_object_property_gets_required(self): + params = { + "type": "object", + "properties": { + "filter": {"type": "object", "properties": {}}, + }, + } + out = sanitize_moonshot_tool_parameters(params) + assert out["properties"]["filter"]["required"] == [] + assert out["required"] == [] + + def test_coerced_top_level_gets_required(self): + # A non-object top level is forced to object and must gain required. + out = sanitize_moonshot_tool_parameters({"type": "string"}) + assert out["type"] == "object" + assert out["required"] == [] + + class TestToolListSanitizer: """sanitize_moonshot_tools() walks an OpenAI-format tool list.""" @@ -239,8 +300,10 @@ class TestToolListSanitizer: ] out = sanitize_moonshot_tools(tools) assert out[0]["function"]["parameters"]["properties"]["q"]["type"] == "string" - # Second tool already clean — should be structurally equivalent - assert out[1]["function"]["parameters"] == {"type": "object", "properties": {}} + # Second tool: empty object gains the required-array Moonshot demands + assert out[1]["function"]["parameters"] == { + "type": "object", "properties": {}, "required": [] + } def test_empty_list_is_passthrough(self): assert sanitize_moonshot_tools([]) == []