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([]) == []