From ab85667deeeccc1255e9363fbc90fb65438fbcfe Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 26 Aug 2026 15:20:23 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20address=20Copilot=20review=20round=2038?= =?UTF-8?q?=20=E2=80=94=20constants=20must=20agree,=20not=20just=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cross-branch agreement check ignored `const`, so two branches pinned to `1` and `"1"` both stated no `type`, both matched the text `1`, and the first one's typed constant was sent. Textually indistinguishable constants of different types are ambiguous, and the raw string the user typed is the honest answer. Signed-off-by: cliffhall --- clients/web/src/test/core/jsonUtils.test.ts | 18 ++++++++++++++++++ core/json/jsonUtils.ts | 13 ++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/clients/web/src/test/core/jsonUtils.test.ts b/clients/web/src/test/core/jsonUtils.test.ts index 68c4190b..29f60d56 100644 --- a/clients/web/src/test/core/jsonUtils.test.ts +++ b/clients/web/src/test/core/jsonUtils.test.ts @@ -334,6 +334,24 @@ describe("JSON Utils", () => { }); }); + it("treats textually equal constants of different types as ambiguous (#2123)", () => { + const indistinguishable: Tool = { + name: "indistinguishable", + inputSchema: { + type: "object", + anyOf: [ + { type: "object", properties: { kind: { const: 1 }, a: {} } }, + { type: "object", properties: { kind: { const: "1" }, b: {} } }, + ], + }, + }; + // `kind=1` matches both, so neither typed constant may be assumed — + // the raw string is the honest answer. + expect(convertToolParameters(indistinguishable, { kind: "1" })).toEqual({ + kind: "1", + }); + }); + it("leaves an ambiguously typed argument uncoerced (#2123)", () => { const ambiguous: Tool = { name: "ambiguous", diff --git a/core/json/jsonUtils.ts b/core/json/jsonUtils.ts index cecce758..205db2ab 100644 --- a/core/json/jsonUtils.ts +++ b/core/json/jsonUtils.ts @@ -209,7 +209,18 @@ function coercionProperties( // 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) { + // The `const` has to agree too, not just the type: `{ const: 1 }` and + // `{ const: "1" }` both state no `type` and both match the text `1`, so + // agreeing on the type alone would send whichever typed constant came + // first rather than falling back to the raw string the user typed. + const pinnedOf = (schema: unknown) => + typeof schema === "object" && schema !== null && "const" in schema + ? (schema as { const?: unknown }).const + : undefined; + const constsAgree = declarations.every((schema) => + sameJsonValue(pinnedOf(schema), pinnedOf(declarations[0])), + ); + if (types.size === 1 && constsAgree && declarations.length > 0) { Object.defineProperty(properties, name, { value: declarations[0], writable: true,