fix: address Copilot review round 40 — a required name counts as a field
A schema may require a property it never declares: `{ type: "object",
required: ["token"] }` is legal and the tool plainly takes an argument.
declaresAnyFields counted only `properties`, so an App tool shaped that way —
including nested under a composition — was reported input-free and
auto-invoked with `{}`.
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
48651956e5
commit
8f935adaad
3 changed files with 25 additions and 0 deletions
|
|
@ -901,6 +901,20 @@ describe("resolveRootUnion", () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it("counts a required name a schema never declares", () => {
|
||||
// Legal, and the tool plainly takes an argument — an App tool shaped this
|
||||
// way must ask rather than being auto-invoked with `{}`.
|
||||
expect(declaresAnyFields({ type: "object", required: ["token"] })).toBe(
|
||||
true,
|
||||
);
|
||||
expect(
|
||||
declaresAnyFields({
|
||||
type: "object",
|
||||
allOf: [{ type: "object", required: ["token"] }],
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("reports none for a bare object schema", () => {
|
||||
expect(declaresAnyFields({ type: "object" })).toBe(false);
|
||||
expect(declaresAnyFields(undefined)).toBe(false);
|
||||
|
|
|
|||
|
|
@ -145,6 +145,12 @@ describe("hasInputFields with root composition (#2123)", () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it("counts a required name the schema never declares", () => {
|
||||
expect(hasInputFields(tool({ type: "object", required: ["token"] }))).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it("still reports no fields for a bare object schema", () => {
|
||||
expect(hasInputFields(tool({ type: "object" }))).toBe(false);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -498,6 +498,11 @@ export function declaresAnyFields(
|
|||
): boolean {
|
||||
if (schema === undefined) return false;
|
||||
if (Object.keys(propertiesOf(schema)).length > 0) return true;
|
||||
// A schema may require a name it never declares — `{ required: ["token"] }`
|
||||
// is a legal object schema, and the tool plainly takes an argument. Counting
|
||||
// it here is what stops an App tool with that shape being auto-invoked with
|
||||
// `{}` for want of a `properties` map.
|
||||
if (requiredOf(schema).length > 0) return true;
|
||||
// A `$ref`'s shape is unknown rather than empty, so it counts. Reporting "no
|
||||
// fields" for `anyOf: [{ $ref: … }, { $ref: … }]` would auto-invoke an App
|
||||
// tool with `{}` on the strength of something never read.
|
||||
|
|
|
|||
Loading…
Reference in a new issue