fix: address Copilot review round 32 — normalize before the type vote, enum-spelled null
- 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>
This commit is contained in:
parent
c9162d0e73
commit
00d9905781
4 changed files with 54 additions and 1 deletions
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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, {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue