From 00d9905781873d5ff67c2b675d5ef88e1ac2ad6c Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Wed, 26 Aug 2026 12:50:20 -0400
Subject: [PATCH] =?UTF-8?q?fix:=20address=20Copilot=20review=20round=2032?=
 =?UTF-8?q?=20=E2=80=94=20normalize=20before=20the=20type=20vote,=20enum-s?=
 =?UTF-8?q?pelled=20null?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- coercionProperties collapses each declaration before comparing types. A
  nullable schema written as an `anyOf` states no top-level `type`, so
  `number | null` and `boolean | null` both read as "no type" and counted
  as agreeing — and `value=true` then came back as `NaN` through the first
  branch's number.
- admitsNull reads an `enum` offering null, under the same sibling conditions
  a `const: null` answers to. Without it `{ enum: [null] }` reported that it
  rejects null, and a required read-only field spelled that way stayed
  permanently invalid.

Signed-off-by: cliffhall <cliff@futurescale.com>
---
 clients/web/src/test/core/jsonUtils.test.ts   | 31 +++++++++++++++++++
 .../web/src/test/core/nullableUnion.test.ts   |  7 +++++
 core/json/jsonUtils.ts                        |  7 ++++-
 core/json/nullableUnion.ts                    | 10 ++++++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/clients/web/src/test/core/jsonUtils.test.ts b/clients/web/src/test/core/jsonUtils.test.ts
index 6c46a7e6..017a5621 100644
--- a/clients/web/src/test/core/jsonUtils.test.ts
+++ b/clients/web/src/test/core/jsonUtils.test.ts
@@ -281,6 +281,37 @@ describe("JSON Utils", () => {
       expect(convertToolParameters(setTyped, { v: "2" })).toEqual({ v: 2 });
     });
 
+    it("sees through nullable encodings when branches disagree (#2123)", () => {
+      const nullableDisagreement: Tool = {
+        name: "nullable-disagreement",
+        inputSchema: {
+          type: "object",
+          anyOf: [
+            {
+              type: "object",
+              properties: {
+                value: { anyOf: [{ type: "number" }, { type: "null" }] },
+                a: {},
+              },
+            },
+            {
+              type: "object",
+              properties: {
+                value: { anyOf: [{ type: "boolean" }, { type: "null" }] },
+                b: {},
+              },
+            },
+          ],
+        },
+      };
+      // Neither declaration states a top-level `type`, so uncollapsed they
+      // would both read as "no type" and agree — and `value=true` would come
+      // back as `NaN` through the first branch's number.
+      expect(
+        convertToolParameters(nullableDisagreement, { value: "true" }),
+      ).toEqual({ value: "true" });
+    });
+
     it("leaves an ambiguously typed argument uncoerced (#2123)", () => {
       const ambiguous: Tool = {
         name: "ambiguous",
diff --git a/clients/web/src/test/core/nullableUnion.test.ts b/clients/web/src/test/core/nullableUnion.test.ts
index 48b4a9bb..75589d9b 100644
--- a/clients/web/src/test/core/nullableUnion.test.ts
+++ b/clients/web/src/test/core/nullableUnion.test.ts
@@ -784,6 +784,13 @@ describe("admitsNull", () => {
       );
     });
 
+    it("reads an enum that offers null", () => {
+      expect(admitsNull({ enum: [null] })).toBe(true);
+      expect(admitsNull({ const: null, anyOf: [{ enum: [null] }] })).toBe(true);
+      // …under the same sibling conditions a `const: null` answers to.
+      expect(admitsNull({ type: "string", enum: [null] })).toBe(false);
+    });
+
     it("does not override a sibling that rejects null", () => {
       // `const` is conjunctive with its siblings, not an override: both of
       // these reject every value, so claiming nullability would let the
diff --git a/core/json/jsonUtils.ts b/core/json/jsonUtils.ts
index 040917aa..00402915 100644
--- a/core/json/jsonUtils.ts
+++ b/core/json/jsonUtils.ts
@@ -201,7 +201,12 @@ function coercionProperties<T extends RootUnionSchema>(
       // A malformed declaration (`properties: { x: null }`) is not a vote about
       // the type, and storing it as the coercion schema would put a value that
       // is not a schema where one is expected.
-      .filter((schema) => typeof schema === "object" && schema !== null);
+      .filter((schema) => typeof schema === "object" && schema !== null)
+      // Collapsed BEFORE the vote: a nullable declaration states its real type
+      // on the surviving branch, so `number | null` and `boolean | null` would
+      // otherwise both read as "no type" and be counted as agreeing — and the
+      // first would then coerce `value=true` to `NaN`.
+      .map((schema) => normalizeNullableUnion(schema as object));
     const types = new Set(declarations.map((schema) => typeNameOf(schema)));
     if (types.size === 1 && declarations.length > 0) {
       Object.defineProperty(properties, name, {
diff --git a/core/json/nullableUnion.ts b/core/json/nullableUnion.ts
index 9232abdd..2a5e73a6 100644
--- a/core/json/nullableUnion.ts
+++ b/core/json/nullableUnion.ts
@@ -406,6 +406,16 @@ export function admitsNull(schema: NullableUnionSchema): boolean {
     return schema.anyOf === undefined || anyOfAdmitsNull(schema);
   }
 
+  // An `enum` offering `null` admits it, the same way a `const: null` does —
+  // and under the same sibling conditions, since the two are equally
+  // conjunctive with whatever else the schema states.
+  if (Array.isArray(schema.enum) && schema.enum.includes(null)) {
+    return (
+      (schema.nullable === true || typeAdmitsNull(schema.type)) &&
+      (schema.anyOf === undefined || anyOfAdmitsNull(schema))
+    );
+  }
+
   if (schema.nullable === true) {
     return true;
   }