Skip to content

Commit ab85667

Browse files
committed
fix: address Copilot review round 38 — constants must agree, not just types
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 <cliff@futurescale.com>
1 parent 85b030c commit ab85667

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

‎clients/web/src/test/core/jsonUtils.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,24 @@ describe("JSON Utils", () => {
334334
});
335335
});
336336

337+
it("treats textually equal constants of different types as ambiguous (#2123)", () => {
338+
const indistinguishable: Tool = {
339+
name: "indistinguishable",
340+
inputSchema: {
341+
type: "object",
342+
anyOf: [
343+
{ type: "object", properties: { kind: { const: 1 }, a: {} } },
344+
{ type: "object", properties: { kind: { const: "1" }, b: {} } },
345+
],
346+
},
347+
};
348+
// `kind=1` matches both, so neither typed constant may be assumed —
349+
// the raw string is the honest answer.
350+
expect(convertToolParameters(indistinguishable, { kind: "1" })).toEqual({
351+
kind: "1",
352+
});
353+
});
354+
337355
it("leaves an ambiguously typed argument uncoerced (#2123)", () => {
338356
const ambiguous: Tool = {
339357
name: "ambiguous",

‎core/json/jsonUtils.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,18 @@ function coercionProperties<T extends RootUnionSchema>(
209209
// first would then coerce `value=true` to `NaN`.
210210
.map((schema) => normalizeNullableUnion(schema as object));
211211
const types = new Set(declarations.map((schema) => typeNameOf(schema)));
212-
if (types.size === 1 && declarations.length > 0) {
212+
// The `const` has to agree too, not just the type: `{ const: 1 }` and
213+
// `{ const: "1" }` both state no `type` and both match the text `1`, so
214+
// agreeing on the type alone would send whichever typed constant came
215+
// first rather than falling back to the raw string the user typed.
216+
const pinnedOf = (schema: unknown) =>
217+
typeof schema === "object" && schema !== null && "const" in schema
218+
? (schema as { const?: unknown }).const
219+
: undefined;
220+
const constsAgree = declarations.every((schema) =>
221+
sameJsonValue(pinnedOf(schema), pinnedOf(declarations[0])),
222+
);
223+
if (types.size === 1 && constsAgree && declarations.length > 0) {
213224
Object.defineProperty(properties, name, {
214225
value: declarations[0],
215226
writable: true,

0 commit comments

Comments
 (0)