[SDK/Factories] Make The Agent Factories Surface Match The Wire Contract - #2309
Merged
MRayermannMSFT merged 14 commits intoAug 11, 2026
Merged
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
A bare x-opaque-json node now renders as JsonValue and a bare x-opaque-in-process node as OpaqueInProcessValue, instead of both collapsing to an object index signature. Nodes that also carry a real constraint keep it, so declarations like ExternalToolResult and McpServerConfig retain their unions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
The regenerated wire types now type the factory result and argument fields as JsonValue, so the hand-written FactoryRunResult override, the toPublicFactoryRunResult boundary helper, and four casts are all unnecessary. A compile-time assertion pins the result type so the override cannot creep back. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
A factory body could start a second top-level run through any session reference it could reach, escaping the limits the user approved. An AsyncLocalStorage guard now refuses factory.run and factory.resume on the body's call path, before the RPC is dispatched, so no durable run row is created. The guard is per-call-path, so an unrelated concurrent run started elsewhere in the extension still succeeds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
The union exported two codes no runtime path raises and omitted five it does, so a caller could branch on a dead code and receive a raw RpcResponseError for a real one. It now names exactly the codes execute_resume raises before a resumed run starts. permission_denied is deliberately excluded: an SDK-initiated resume dispatches with RunOrigin::default(), so the approval branch never runs and the code is unreachable from this path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
The hand-written FactoryAgentOptions declared only label, schema and model, and the agent implementation rebuilt the request from those three, so agent, reasoningEffort and contextTier were dropped before the request was sent. The options are now declared once as a key tuple and copied from it, and two compile-time assertions pin that tuple to both the public and the wire interface, so a future wire option fails the build instead of being silently dropped. Undeclared keys are still filtered out, because the wire schema forbids them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
…A Run A background progress flush that failed earlier latched its error, and close() rethrew it from the factory execute finally block, so a factory body that succeeded settled as an error. The latched error is now best effort and warns, matching the treatment the final send already had. A mid-body flush failure stays fatal, because a running body that cannot record progress must not continue. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
The guide and four JSDoc comments described behavior that does not exist: a declined SDK-initiated run resolving as cancelled, a single-active-run limit, two error codes no runtime path raises, an unpaginated listRuns, and a three-option ctx.agent. They now match the shipped surface, including that the SDK forwards agent, reasoningEffort and contextTier while the current runtime does not yet honor them. File-content assertions guard both files, which nothing else covers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
The claim that the runtime does not yet honor agent, reasoningEffort and contextTier is a point-in-time fact about another repository. It rots as soon as the runtime lands support, so the SDK docs no longer carry it. Also wraps three over-length test assertions that the prettier check flagged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
This comment has been minimized.
This comment has been minimized.
- Describe ctx.session by what it omits, and point at the extensions_manage guide - Drop the hardcoded active-run limit, which will become a setting - Drop the listRuns paging parenthetical Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
This comment has been minimized.
This comment has been minimized.
The factory awaited its subagent to completion, so the test hung wherever no cached model response exists and timed out at 30s on CI. Only the runtime's acceptance of the option payload is under test, and a refused request rejects before a subagent starts. The factory now races the call against a short timer and returns as soon as the request is accepted. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns the experimental Node.js Agent Factories API and generated types with the runtime wire contract.
Changes:
- Maps opaque schema markers to
JsonValueor in-process values. - Forwards all factory-agent options and updates resume errors.
- Adds nested-run guards, progress-flush handling, documentation, and tests.
Show a summary per file
| File | Description |
|---|---|
scripts/codegen/utils.ts |
Adds opaque-marker helpers. |
scripts/codegen/typescript.ts |
Generates opaque TypeScript aliases. |
nodejs/test/typescript-codegen.test.ts |
Tests opaque-schema generation. |
nodejs/test/session-event-types.test.ts |
Verifies public generated types. |
nodejs/test/factory.test.ts |
Expands factory behavior tests. |
nodejs/test/e2e/fixtures/factory-extension.mjs |
Adds factory E2E scenarios. |
nodejs/test/e2e/factory.e2e.test.ts |
Tests cross-process factory behavior. |
nodejs/src/types.ts |
Narrows tool telemetry values. |
nodejs/src/session.ts |
Updates factory execution behavior. |
nodejs/src/index.ts |
Documents JsonValue re-export. |
nodejs/src/generated/session-events.ts |
Regenerates event JSON types. |
nodejs/src/generated/rpc.ts |
Regenerates RPC opaque types. |
nodejs/src/factory.ts |
Aligns public factory API types. |
nodejs/docs/factories.md |
Updates factory documentation. |
CHANGELOG.md |
Records user-facing changes. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 13/15 changed files
- Comments generated: 3
- Review effort level: Balanced
The context session is a full CopilotSession, so factory.run and factory.resume are present and callable. Saying the APIs are absent contradicted the exported type. The guide and the published comment now say the session refuses those calls. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
The changelog is generated at release time, so an entry added by hand in a feature PR does not fit the file's convention. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ece6b29-8b10-47aa-ab17-b64c47f5fdcd
Contributor
Cross-SDK Consistency Review ✅All 14 changed files are in No cross-SDK consistency issues found. The changes are appropriately scoped to the Node.js implementation.
|
stephentoub
approved these changes
Aug 11, 2026
stephentoub
left a comment
stephentoub
left a comment
Collaborator
There was a problem hiding this comment.
Generated by Copilot
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 11, 2026
MRayermannMSFT
deleted the
mrayermannmsft-factories-match-wire-contract
branch
August 11, 2026 16:49
12 tasks
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This corrects the
@experimentalAgent Factories surface in the Node SDK so its types and behavior match what the runtime actually does. A factory result and factory arguments are now typed asJsonValue, so an array or a scalar fits the value the runtime already sent.ctx.agent()forwardsagent,reasoningEffort, andcontextTier, which the SDK previously dropped. The resume error union now names only codes the runtime raises. A factory body can no longer start a second top-level run. A background progress-flush error no longer turns a completed run into an errored one.Why
The root cause is one gap in the TypeScript generator. The schema marks a value that travels as JSON with
x-opaque-json. It marks one that never serializes withx-opaque-in-process. The generator read neither marker, so both rendered as an object index signature. That is why the SDK carried a hand-written type override and four casts, and why an array result did not fit its own type. Two exported error codes had no raiser anywhere in the runtime, so a caller can branch on a case that never happens. Most seriously, a factory body can callsession.factory.runand start a second run at any limits. That escapes the ceiling the user approved. Fixing the generator removes the workarounds rather than adding more.Related to github/copilot-agent-runtime#14985