From 2af8b8ff3712c71620f32b1fa57e92289e6ca202 Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Thu, 30 Apr 2026 23:10:25 -0700
Subject: [PATCH] fix(moonshot): also strip nullable/enum after anyOf collapse
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The anyOf collapse in _repair_schema returned early, skipping the
nullable-strip and enum-cleanup steps. When a schema had anyOf
[{enum: [..., null, '']}, {type: null}] alongside a parent-level
'nullable: true', collapsing to the single non-null branch produced a
merged node that still had both 'nullable' and the bad enum values —
Moonshot would still 400 on it.

Fix: fall through to Rules 1/3 when the collapse produces a single
merged node; only return early for the multi-branch case (pure
anyOf preservation) or when there was no null branch to remove.

Adds a test that locks in the combined-case expectation.
---
 agent/moonshot_schema.py            | 10 ++++++++--
 tests/agent/test_moonshot_schema.py | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/agent/moonshot_schema.py b/agent/moonshot_schema.py
index 391087a5..aeefd4a0 100644
--- a/agent/moonshot_schema.py
+++ b/agent/moonshot_schema.py
@@ -90,14 +90,20 @@ def _repair_schema(node: Any, is_schema: bool = True) -> Any:
                     if isinstance(b, dict) and b.get("type") != "null"]
         if non_null and len(non_null) < len(repaired["anyOf"]):
             # Drop the anyOf wrapper — keep only the non-null branch.
-            # If there's a single non-null branch, promote it.
+            # If there's a single non-null branch, promote it and fall
+            # through to Rules 1/3 so nullable/enum cleanup still applies
+            # to the merged node.
             if len(non_null) == 1:
                 merge = {k: v for k, v in repaired.items() if k != "anyOf"}
                 merge.update(non_null[0])
                 repaired = merge
             else:
                 repaired["anyOf"] = non_null
-        return repaired
+                return repaired
+        else:
+            # Nothing to collapse — parent type stripped, children already
+            # repaired by the recursive walk above.
+            return repaired
 
     # Moonshot also rejects non-standard keywords like ``nullable`` on
     # parameter schemas — strip it.
diff --git a/tests/agent/test_moonshot_schema.py b/tests/agent/test_moonshot_schema.py
index 6e8fdc81..2ce2daa0 100644
--- a/tests/agent/test_moonshot_schema.py
+++ b/tests/agent/test_moonshot_schema.py
@@ -372,3 +372,28 @@ class TestEnumNullStripping:
         out = sanitize_moonshot_tool_parameters(params)
         # object-typed enum should pass through unchanged
         assert "enum" in out["properties"]["config"]
+
+    def test_anyof_collapse_still_runs_nullable_and_enum_cleanup(self):
+        """After anyOf collapses to a single non-null branch, the merged
+        node must still have ``nullable`` stripped and null/empty-string
+        values removed from enum — not skipped by the early anyOf return.
+        """
+        params = {
+            "type": "object",
+            "properties": {
+                "db_type": {
+                    "anyOf": [
+                        {"enum": ["mysql", "postgresql", "", None]},
+                        {"type": "null"},
+                    ],
+                    "nullable": True,
+                },
+            },
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        db_type = out["properties"]["db_type"]
+        assert "anyOf" not in db_type
+        assert "nullable" not in db_type, "nullable must be stripped after anyOf collapse"
+        assert db_type["type"] == "string"
+        assert db_type["enum"] == ["mysql", "postgresql"], \
+            "null/empty enum values must be stripped after anyOf collapse"