From dc0915ce354d6186a9108c9a29cee044b10d251d Mon Sep 17 00:00:00 2001 From: chelsealong Date: Thu, 10 Sep 2026 06:33:08 +0000 Subject: [PATCH] fix(moonshot): stop stamping a synthetic type on if/then/else nodes _repair_schema didn't recurse into if/then/else, so a bare conditional wrapper (e.g. an allOf branch expressing "when goal is set, mode must be one of ...") fell through _fill_missing_type's default and was stamped with type: "string". That corrupts the schema: the wrapper's actual instance is an object, so Moonshot (and any strict validator) now rejects every real argument against it. if/then/else are added to the recursed node keys, and _fill_missing_type leaves a bare conditional node untyped instead of defaulting to "string" since it constrains the parent instance rather than describing its own shape. --- agent/moonshot_schema.py | 10 ++++++++-- tests/agent/test_moonshot_schema.py | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/agent/moonshot_schema.py b/agent/moonshot_schema.py index 079280cfbb..99a8f2333e 100644 --- a/agent/moonshot_schema.py +++ b/agent/moonshot_schema.py @@ -19,7 +19,9 @@ _SCHEMA_MAP_KEYS = frozenset({"properties", "patternProperties", "$defs", "defin # Values are lists of schemas. _SCHEMA_LIST_KEYS = frozenset({"anyOf", "oneOf", "allOf", "prefixItems"}) # Values are a single nested schema (additionalProperties may also be a bool). -_SCHEMA_NODE_KEYS = frozenset({"items", "contains", "not", "additionalProperties", "propertyNames"}) +_SCHEMA_NODE_KEYS = frozenset( + {"items", "contains", "not", "additionalProperties", "propertyNames", "if", "then", "else"} +) _SCALAR_TYPES = frozenset({"string", "integer", "number", "boolean"}) # bool before int: bool is an int subclass. @@ -103,7 +105,9 @@ def _fill_missing_type(node: Dict[str, Any]) -> Dict[str, Any]: A type list collapses to its first concrete member; otherwise ``properties``/``required``/``additionalProperties`` → object, ``items``/``prefixItems`` → array, ``enum`` → type of its first value, - else ``string`` (safest scalar). + else ``string`` (safest scalar). A bare ``if``/``then``/``else`` node + constrains whatever instance it is attached to rather than describing + its own type, so it is left untyped instead of defaulting to ``string``. """ node_type = node.get("type") if isinstance(node_type, list): @@ -119,6 +123,8 @@ def _fill_missing_type(node: Dict[str, Any]) -> Dict[str, Any]: elif isinstance(node.get("enum"), list) and node["enum"]: sample = node["enum"][0] inferred = next((t for cls, t in _ENUM_SAMPLE_TYPES if isinstance(sample, cls)), "string") + elif "if" in node or "then" in node or "else" in node: + return node else: inferred = "string" return {**node, "type": inferred} diff --git a/tests/agent/test_moonshot_schema.py b/tests/agent/test_moonshot_schema.py index a1264980bd..5fc50dd905 100644 --- a/tests/agent/test_moonshot_schema.py +++ b/tests/agent/test_moonshot_schema.py @@ -92,6 +92,30 @@ class TestMissingTypeFilled: assert out["properties"]["payload"]["$ref"] == "#/$defs/Payload" +class TestConditionalSchemaNotGivenSyntheticType: + """A bare if/then/else conditional constrains the enclosing instance; it + does not describe its own type, so it must not be defaulted to "string".""" + + def test_if_then_node_is_not_given_synthetic_type(self): + params = { + "oneOf": [ + {"required": ["prompt"], "not": {"required": ["goal"]}}, + {"required": ["goal"], "not": {"required": ["prompt"]}}, + ], + "allOf": [ + { + "if": {"required": ["goal"]}, + "then": {"properties": {"mode": {"enum": ["build", "edit"]}}}, + } + ], + } + out = sanitize_moonshot_tool_parameters(params) + conditional = out["allOf"][0] + assert "type" not in conditional + # Nested schemas under then/if still get repaired. + assert conditional["then"]["properties"]["mode"]["type"] == "string" + + class TestAnyOfParentType: """Rule 2: type must not appear at the anyOf parent level.