From b5fe21a28eeed870cce2bf624807c4f2c753e2d7 Mon Sep 17 00:00:00 2001 From: Matthew Rayermann Date: Tue, 11 Aug 2026 21:07:13 -0700 Subject: [PATCH 1/2] [SDK/Factories] Add argsSchema To The Factory Authoring Surface FactoryMeta now declares an optional argsSchema, typed as the existing FactoryJsonSchema. The field already crossed the wire because defineFactory snapshots meta whole, so this is additive and type-level: it makes a runtime feature discoverable to extension authors writing against the published types. Without a declared schema nothing validates a caller's args. A malformed call starts a run, takes a user approval, spends credits, and then fails inside the factory body. With one, the CLI rejects it before the run row exists and the model retries against a correction hint. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- nodejs/docs/factories.md | 21 ++++- nodejs/src/factory.ts | 12 ++- nodejs/src/types.ts | 21 ++++- .../test/e2e/fixtures/factory-extension.mjs | 3 + nodejs/test/factory.test.ts | 91 +++++++++++++++++++ 5 files changed, 140 insertions(+), 8 deletions(-) diff --git a/nodejs/docs/factories.md b/nodejs/docs/factories.md index a22767905..a763bfb3b 100644 --- a/nodejs/docs/factories.md +++ b/nodejs/docs/factories.md @@ -16,6 +16,13 @@ const reviewChanged = defineFactory({ "Review changed files and verify the findings. " + "args: { files: string[] } — the paths to review.", phases: [{ title: "Review" }, { title: "Verify" }], + argsSchema: { + type: "object", + required: ["files"], + properties: { + files: { type: "array", items: { type: "string" } }, + }, + }, limits: { maxConcurrentSubagents: 3, maxTotalSubagents: 10, @@ -41,9 +48,17 @@ const reviewChanged = defineFactory({ const session = await joinSession({ factories: [reviewChanged] }); ``` -Factory metadata contains a stable `name`, a human-readable `description`, declared `phases`, and optional `limits`. Phase entries contain a `title` and optional `detail`. +Factory metadata contains a stable `name`, a human-readable `description`, declared `phases`, an optional `argsSchema`, and optional `limits`. Phase entries contain a `title` and optional `detail`. + +## Declaring an argument shape + +A factory that reads `ctx.args` should declare `meta.argsSchema`, as the example above does. The CLI validates the caller's `args` against it **before** the run starts. + +Declaring one turns an expensive failure into a cheap one. With a schema, a malformed call is rejected up front — the model gets a correction hint and retries, and no run row, permission prompt, or credit spend happens. Without one, nothing validates: the run starts, takes a user approval, spends credits, and then dies inside the factory body with a confusing error. Agents can read the declared shape with `factories_manage` using `operation: "inspect"`. + +Enforcement covers structure — types, required properties, and enum or const values. Finer constraints such as `minLength`, `pattern`, or `additionalProperties` are recorded in the declaration but not enforced. The accepted vocabulary is the `FactoryJsonSchema` subset also used for subagent structured output: `type`, `required`, `enum`, `const`, recursive `properties`/`items`, and `anyOf`/`oneOf`/`allOf`. A `type` is one of `null`, `boolean`, `integer`, `number`, `string`, `array`, or `object`, or a non-empty array of those such as `["object", "null"]`. A declaration outside that subset is rejected at registration. -There is no declared schema for `ctx.args`. The `run_factory` tool forwards `args` verbatim and its parameter is untyped, so **the `description` is the only thing telling an agent what arguments to supply** — state the expected shape there whenever a factory reads `ctx.args`, as the example above does. Arguments supplied by an extension calling `session.factory.run(...)` directly are typed through `defineFactory`, but that typing does not reach the model. A factory that reads `ctx.args` should validate it rather than assume a shape. +`argsSchema` is optional and backward compatible. A factory that omits it behaves exactly as before, so **the `description` is then the only thing telling an agent what arguments to supply** — state the expected shape there. Arguments supplied by an extension calling `session.factory.run(...)` directly are typed through `defineFactory`, but that typing does not reach the model. A factory that reads `ctx.args` should still validate it rather than assume a shape, because the declared subset does not enforce every constraint. `defineFactory` accepts a `run(context)` function returning `Promise`, where `TResult` is `JsonValue | void`. Objects, arrays, strings, numbers, booleans, and `null` are valid results. Returning `undefined` completes the factory with no result. Other non-JSON values are rejected. @@ -193,7 +208,7 @@ async ({ args, agent, phase }) => { }; ``` -Authoring registers the factory but does not run it. Invoke it afterwards with `run_factory`. Use `factories_manage` with `operation: "list"` to see the factories already registered in the session and `operation: "inspect"` to read one factory's description, phases, and limits before running it. +Authoring registers the factory but does not run it. Invoke it afterwards with `run_factory`. Use `factories_manage` with `operation: "list"` to see the factories already registered in the session and `operation: "inspect"` to read one factory's description, phases, declared argument shape, and limits before running it. ## Observe a run diff --git a/nodejs/src/factory.ts b/nodejs/src/factory.ts index 8ad1c7acb..8a6c78747 100644 --- a/nodejs/src/factory.ts +++ b/nodejs/src/factory.ts @@ -62,12 +62,16 @@ export type JsonValue = | { [key: string]: JsonValue }; /** - * Conservative JSON shape language accepted for structured factory agent output. + * Conservative JSON shape language accepted by the Agent Factories surface, for + * both structured factory agent output and a factory's declared `argsSchema`. * - * This is a best-effort structural guard used to decide whether a subagent's - * structured output should be accepted or retried — **not** a full JSON Schema + * This is a best-effort structural guard — used to decide whether a subagent's + * structured output should be accepted or retried, and whether a caller's + * factory `args` match the declared shape — **not** a full JSON Schema * validator. Only these keywords are honored: `type`, `required`, `enum`, - * `const`, recursive `properties`/`items`, and `anyOf`/`oneOf`/`allOf`. + * `const`, recursive `properties`/`items`, and `anyOf`/`oneOf`/`allOf`. A `type` + * is one of `null`, `boolean`, `integer`, `number`, `string`, `array`, or + * `object`, or a non-empty array of those (for example `["object", "null"]`). * * Everything else is **ignored, not enforced**. In particular, string * constraints (`pattern`, `minLength`, `maxLength`, `format`), numeric ranges diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 3a5f7714b..857b08391 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -19,7 +19,7 @@ import type { SessionEvent as GeneratedSessionEvent, } from "./generated/session-events.js"; import type { CopilotSession } from "./session.js"; -import type { JsonValue } from "./factory.js"; +import type { FactoryJsonSchema, JsonValue } from "./factory.js"; import type { GitHubTelemetryNotification, ModelBillingTokenPrices, @@ -2007,6 +2007,25 @@ export interface FactoryMeta { description: string; /** Display metadata for the progress phases the factory may report. */ phases: Array<{ title: string; detail?: string }>; + /** + * Optional declared shape of the arguments this factory expects as `ctx.args`. + * + * Declaring one is strongly recommended for any factory that reads `ctx.args`. + * The CLI validates the caller's `args` against it **before** the run starts, so a + * malformed call from the model is rejected with a correction hint and retried + * without ever creating a run row, prompting the user for permission, or spending + * credits. A factory that declares nothing is never validated: a malformed call + * starts, takes an approval, spends credits, and then fails inside the factory + * body. `factories_manage` with `operation: "inspect"` reports the declared shape + * so an agent can read it before invoking. + * + * Enforcement covers structure — types, required properties, and enum/const + * values. Finer constraints such as `minLength`, `pattern`, and + * `additionalProperties` are recorded in the declaration but not enforced. See + * {@link FactoryJsonSchema} for the accepted subset. A declaration outside that + * subset is rejected at registration. + */ + argsSchema?: FactoryJsonSchema; /** Optional resource ceilings presented to the user before execution. */ limits?: FactoryLimits; } diff --git a/nodejs/test/e2e/fixtures/factory-extension.mjs b/nodejs/test/e2e/fixtures/factory-extension.mjs index 5f6c19e2b..0f4985d4b 100644 --- a/nodejs/test/e2e/fixtures/factory-extension.mjs +++ b/nodejs/test/e2e/fixtures/factory-extension.mjs @@ -18,6 +18,9 @@ const argumentEcho = defineFactory({ name: "argument-echo", description: "Return the invocation arguments verbatim.", phases: [], + // A declared shape has to survive the SDK boundary and reach the runtime, + // which validates `args` against it before a run row exists. + argsSchema: { type: ["object", "null"] }, }, run: async ({ args }) => args, }); diff --git a/nodejs/test/factory.test.ts b/nodejs/test/factory.test.ts index f550c3bc9..3b622d755 100644 --- a/nodejs/test/factory.test.ts +++ b/nodejs/test/factory.test.ts @@ -15,6 +15,7 @@ import { type FactoryAgentOptions, type FactoryContext, type FactoryDefinition, + type FactoryJsonSchema, type JsonValue, } from "../src/factory.js"; @@ -461,6 +462,96 @@ describe("factories", () => { ); }); + it("carries a declared argsSchema through defineFactory into the registration payload", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => stopClient(client)); + + const argsSchema = { + type: "object", + required: ["repoPath"], + properties: { + repoPath: { type: "string" }, + depth: { type: ["integer", "null"] }, + mode: { enum: ["fast", "thorough"] }, + }, + } satisfies FactoryJsonSchema; + const meta = { + name: "declares-args", + description: "Declares the argument shape it expects", + phases: [], + argsSchema, + }; + const factory = defineFactory({ meta, run: async () => ({ ok: true }) }); + + // The declaration is snapshotted and deep-frozen like the rest of the + // metadata, so it cannot be mutated after registration. + expect(factory.meta.argsSchema).toEqual(argsSchema); + expect(factory.meta.argsSchema).not.toBe(argsSchema); + expect(Object.isFrozen(factory.meta.argsSchema)).toBe(true); + expect(() => { + // @ts-expect-error handle.meta.argsSchema is deeply readonly. + factory.meta.argsSchema!.type = "array"; + }).toThrow(TypeError); + + const omitted = defineFactory({ + meta: { name: "omits-args", description: "Declares nothing", phases: [] }, + run: async () => ({ ok: true }), + }); + expect(omitted.meta.argsSchema).toBeUndefined(); + expect("argsSchema" in omitted.meta).toBe(false); + + const sendRequest = vi + .spyOn( + (client as never as { connection: { sendRequest: Function } }).connection, + "sendRequest" + ) + .mockImplementation(async (method: string, params: Record) => { + if (method === "session.resume") { + return { sessionId: params.sessionId }; + } + throw new Error(`Unexpected method: ${method}`); + }); + + await client.resumeSessionForExtension( + "session-args-schema", + { onPermissionRequest: () => ({ kind: "approved" }) }, + [factory, omitted] + ); + + const payload = sendRequest.mock.calls.find( + ([method]) => method === "session.resume" + )![1] as { factories: Array> }; + // The schema has to survive JSON serialization to reach the runtime, which + // validates `args` against it before a run row exists. + expect(JSON.parse(JSON.stringify(payload.factories))[0].argsSchema).toEqual(argsSchema); + expect(payload.factories[1]).not.toHaveProperty("argsSchema"); + }); + + it("documents argsSchema consistently with the runtime's enforced subset", () => { + const publicTypes = readFileSync(new URL("../src/types.ts", import.meta.url), "utf8"); + const publicApi = readFileSync(new URL("../src/factory.ts", import.meta.url), "utf8"); + const guide = readFileSync(new URL("../docs/factories.md", import.meta.url), "utf8"); + const normalizeJSDoc = (document: string) => + document.replace(/\r?\n\s*\* ?/g, " ").replace(/\s+/g, " "); + + expect(publicTypes).toContain("argsSchema?: FactoryJsonSchema;"); + + // The `run_factory` tool tells the model exactly this. The two surfaces + // have to agree about what a declaration does and does not enforce. + for (const document of [normalizeJSDoc(publicTypes), guide]) { + expect(document).toContain("types, required properties, and enum"); + expect(document).toMatch( + /`minLength`, `pattern`,? (?:and|or) `additionalProperties` are recorded/ + ); + } + expect(normalizeJSDoc(publicTypes)).toContain("before** the run starts"); + expect(normalizeJSDoc(publicApi)).toContain( + "`null`, `boolean`, `integer`, `number`, `string`, `array`, or `object`" + ); + expect(guide).toContain("no run row, permission prompt, or credit spend happens"); + }); + it("serializes only factory metadata in the extension resume payload", async () => { const client = new CopilotClient(); await client.start(); From 82cb80c8818274dc2440a119ee9ce8c4b98bfeca Mon Sep 17 00:00:00 2001 From: Matthew Rayermann Date: Thu, 13 Aug 2026 13:51:43 -0700 Subject: [PATCH 2/2] Scope argsSchema docs to the run_factory path and widen the E2E fixture Enforcement lives behind toolRunFactoryValidateArgs, which the runtime calls only from runFactoryTool. session.factory.run does not validate, so the docs and JSDoc now say which caller is checked instead of implying all of them are. The argument-echo fixture declared ["object","null"] while its contract is to echo any JsonValue, and it is invoked with an array. Nothing broke, because the SDK path does not validate, but the narrow declaration was dishonest and would have become load-bearing if that path ever gained validation. Widened it to the factory's real contract and corrected the comment, which claimed the fixture exercised enforcement when it only exercises registration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- nodejs/docs/factories.md | 6 ++++-- nodejs/src/types.ts | 19 ++++++++++++------- .../test/e2e/fixtures/factory-extension.mjs | 12 +++++++++--- nodejs/test/factory.test.ts | 8 ++++++++ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/nodejs/docs/factories.md b/nodejs/docs/factories.md index a763bfb3b..e6d9ce2bc 100644 --- a/nodejs/docs/factories.md +++ b/nodejs/docs/factories.md @@ -52,13 +52,15 @@ Factory metadata contains a stable `name`, a human-readable `description`, decla ## Declaring an argument shape -A factory that reads `ctx.args` should declare `meta.argsSchema`, as the example above does. The CLI validates the caller's `args` against it **before** the run starts. +A factory that reads `ctx.args` should declare `meta.argsSchema`, as the example above does. When the model invokes the factory through the `run_factory` tool, the CLI validates `args` against the declaration **before** the run starts. Declaring one turns an expensive failure into a cheap one. With a schema, a malformed call is rejected up front — the model gets a correction hint and retries, and no run row, permission prompt, or credit spend happens. Without one, nothing validates: the run starts, takes a user approval, spends credits, and then dies inside the factory body with a confusing error. Agents can read the declared shape with `factories_manage` using `operation: "inspect"`. Enforcement covers structure — types, required properties, and enum or const values. Finer constraints such as `minLength`, `pattern`, or `additionalProperties` are recorded in the declaration but not enforced. The accepted vocabulary is the `FactoryJsonSchema` subset also used for subagent structured output: `type`, `required`, `enum`, `const`, recursive `properties`/`items`, and `anyOf`/`oneOf`/`allOf`. A `type` is one of `null`, `boolean`, `integer`, `number`, `string`, `array`, or `object`, or a non-empty array of those such as `["object", "null"]`. A declaration outside that subset is rejected at registration. -`argsSchema` is optional and backward compatible. A factory that omits it behaves exactly as before, so **the `description` is then the only thing telling an agent what arguments to supply** — state the expected shape there. Arguments supplied by an extension calling `session.factory.run(...)` directly are typed through `defineFactory`, but that typing does not reach the model. A factory that reads `ctx.args` should still validate it rather than assume a shape, because the declared subset does not enforce every constraint. +`argsSchema` is optional and backward compatible. A factory that omits it behaves exactly as before, so **the `description` is then the only thing telling an agent what arguments to supply** — state the expected shape there. + +Validation covers the model's `run_factory` path only. An extension calling `session.factory.run(...)` directly is not validated against `argsSchema`; those arguments are typed through `defineFactory` instead, and that typing does not reach the model. So a factory that reads `ctx.args` should still validate it rather than assume a shape — the declared subset does not enforce every constraint, and it does not run at all on the SDK path. `defineFactory` accepts a `run(context)` function returning `Promise`, where `TResult` is `JsonValue | void`. Objects, arrays, strings, numbers, booleans, and `null` are valid results. Returning `undefined` completes the factory with no result. Other non-JSON values are rejected. diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 857b08391..6eaa65a6b 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -2011,13 +2011,18 @@ export interface FactoryMeta { * Optional declared shape of the arguments this factory expects as `ctx.args`. * * Declaring one is strongly recommended for any factory that reads `ctx.args`. - * The CLI validates the caller's `args` against it **before** the run starts, so a - * malformed call from the model is rejected with a correction hint and retried - * without ever creating a run row, prompting the user for permission, or spending - * credits. A factory that declares nothing is never validated: a malformed call - * starts, takes an approval, spends credits, and then fails inside the factory - * body. `factories_manage` with `operation: "inspect"` reports the declared shape - * so an agent can read it before invoking. + * When the model invokes the factory through the `run_factory` tool, the CLI + * validates `args` against this declaration **before** the run starts, so a + * malformed call is rejected with a correction hint and retried without ever + * creating a run row, prompting the user for permission, or spending credits. A + * factory that declares nothing is never validated: a malformed call starts, + * takes an approval, spends credits, and then fails inside the factory body. + * `factories_manage` with `operation: "inspect"` reports the declared shape so an + * agent can read it before invoking. + * + * This covers the model's `run_factory` path only. `session.factory.run(...)` is + * not validated against the declaration, so a factory should still check + * `ctx.args` rather than assume the declared shape held. * * Enforcement covers structure — types, required properties, and enum/const * values. Finer constraints such as `minLength`, `pattern`, and diff --git a/nodejs/test/e2e/fixtures/factory-extension.mjs b/nodejs/test/e2e/fixtures/factory-extension.mjs index 0f4985d4b..45227a1be 100644 --- a/nodejs/test/e2e/fixtures/factory-extension.mjs +++ b/nodejs/test/e2e/fixtures/factory-extension.mjs @@ -18,9 +18,15 @@ const argumentEcho = defineFactory({ name: "argument-echo", description: "Return the invocation arguments verbatim.", phases: [], - // A declared shape has to survive the SDK boundary and reach the runtime, - // which validates `args` against it before a run row exists. - argsSchema: { type: ["object", "null"] }, + // Proves a declared shape survives the SDK boundary and registers against a + // real runtime. It does not exercise enforcement: `argsSchema` is checked by + // the model's `run_factory` tool, and these tests invoke `session.factory.run`, + // which does not validate. The declaration stays as wide as this factory's + // actual contract — it echoes any JsonValue, and is called with an array, an + // object, and nothing — so it cannot constrain the runs below. + argsSchema: { + type: ["object", "array", "string", "number", "integer", "boolean", "null"], + }, }, run: async ({ args }) => args, }); diff --git a/nodejs/test/factory.test.ts b/nodejs/test/factory.test.ts index 3b622d755..3d85b972d 100644 --- a/nodejs/test/factory.test.ts +++ b/nodejs/test/factory.test.ts @@ -546,6 +546,14 @@ describe("factories", () => { ); } expect(normalizeJSDoc(publicTypes)).toContain("before** the run starts"); + // Enforcement is tool-path only: `toolRunFactoryValidateArgs` is called from + // the runtime's runFactoryTool, and never from `session.factory.run`. Both + // surfaces must keep saying so, or authors will assume their own SDK-initiated + // runs are checked. + expect(normalizeJSDoc(publicTypes)).toContain( + "`session.factory.run(...)` is not validated against the declaration" + ); + expect(guide).toContain("Validation covers the model's `run_factory` path only"); expect(normalizeJSDoc(publicApi)).toContain( "`null`, `boolean`, `integer`, `number`, `string`, `array`, or `object`" );