fix: address Copilot review round 23 — inherited constants, branch null forms, test cast
- matchesConstants checks own-property presence before reading a supplied
value, so an absent argument legally named `constructor` no longer reads the
inherited one and rules out every branch pinning that name.
- anyOfAdmitsNull asks `admitsNull` about each composition-free branch instead
of testing `type` alone, so a branch spelling its nullability as
`{ const: null }` or `{ nullable: true }` is recognized. Refusing composed
branches is what bounds the recursion.
- Drop a double cast from the malformed-applicator fixture.
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
0be450a911
commit
4cea4e1985
3 changed files with 57 additions and 11 deletions
|
|
@ -181,6 +181,36 @@ describe("JSON Utils", () => {
|
|||
).toEqual({ a: "x", value: 3 });
|
||||
});
|
||||
|
||||
it("does not read an inherited property as a supplied constant (#2123)", () => {
|
||||
const pinnedOnInherited: Tool = {
|
||||
name: "pinned-on-inherited",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: Object.fromEntries([
|
||||
["constructor", { const: "a" }],
|
||||
["count", { type: "number" }],
|
||||
]),
|
||||
},
|
||||
{
|
||||
type: "object",
|
||||
properties: Object.fromEntries([
|
||||
["constructor", { const: "b" }],
|
||||
["other", { type: "string" }],
|
||||
]),
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
// `constructor` was not supplied, so it rules nothing out — `count`
|
||||
// belongs to the first branch alone and settles it.
|
||||
expect(convertToolParameters(pinnedOnInherited, { count: "3" })).toEqual({
|
||||
count: 3,
|
||||
});
|
||||
});
|
||||
|
||||
it("leaves an ambiguously typed argument uncoerced (#2123)", () => {
|
||||
const ambiguous: Tool = {
|
||||
name: "ambiguous",
|
||||
|
|
|
|||
|
|
@ -751,6 +751,14 @@ describe("admitsNull", () => {
|
|||
expect(admitsNull({ const: null, anyOf: [{ type: "null" }] })).toBe(true);
|
||||
});
|
||||
|
||||
it("recognizes a branch that spells its nullability another way", () => {
|
||||
// The branch admits null through `const`, not through `type`.
|
||||
expect(admitsNull({ const: null, anyOf: [{ const: null }] })).toBe(true);
|
||||
expect(admitsNull({ const: null, anyOf: [{ nullable: true }] })).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -668,10 +668,13 @@ describe("resolveRootUnion", () => {
|
|||
// These schemas describe the wire, and every member arrives as `unknown`
|
||||
// — reading `anyOf: {}` as a list would throw and take all three clients
|
||||
// down rather than declining one malformed tool.
|
||||
// A single assertion, from an `unknown`-typed value: the point is a wire
|
||||
// shape TypeScript would never produce, not a cast chain.
|
||||
const malformed: unknown = {};
|
||||
const { base, branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { a: { type: "string" } },
|
||||
anyOf: {} as unknown as unknown[],
|
||||
anyOf: malformed as unknown[],
|
||||
});
|
||||
expect(branches).toEqual([]);
|
||||
expect(Object.keys(base.properties ?? {})).toEqual(["a"]);
|
||||
|
|
|
|||
Loading…
Reference in a new issue