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>
This commit is contained in:
parent
85b030cbde
commit
ab85667dee
2 changed files with 30 additions and 1 deletions
|
|
@ -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)", () => {
|
it("leaves an ambiguously typed argument uncoerced (#2123)", () => {
|
||||||
const ambiguous: Tool = {
|
const ambiguous: Tool = {
|
||||||
name: "ambiguous",
|
name: "ambiguous",
|
||||||
|
|
|
||||||
|
|
@ -209,7 +209,18 @@ function coercionProperties<T extends RootUnionSchema>(
|
||||||
// first would then coerce `value=true` to `NaN`.
|
// first would then coerce `value=true` to `NaN`.
|
||||||
.map((schema) => normalizeNullableUnion(schema as object));
|
.map((schema) => normalizeNullableUnion(schema as object));
|
||||||
const types = new Set(declarations.map((schema) => typeNameOf(schema)));
|
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, {
|
Object.defineProperty(properties, name, {
|
||||||
value: declarations[0],
|
value: declarations[0],
|
||||||
writable: true,
|
writable: true,
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue