fix(mcp): configure client elicitation handler#1911
Conversation
|
| Name | Type |
|---|---|
| agents | Patch |
| @cloudflare/agent-think | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
agents
@cloudflare/ai-chat
@cloudflare/codemode
create-think
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
ce77585 to
c9b4418
Compare
There was a problem hiding this comment.
Pull request overview
Updates the Agents SDK MCP client API so elicitation handling is configured on the MCP client manager (this.mcp) rather than via an Agent lifecycle hook, ensuring handlers configured in onStart() apply to restored connections before capability advertisement.
Changes:
- Added
MCPClientManager.configureElicitationHandler(...)(with connection-level reconfiguration for uninitialized clients) and exportedMCPClientElicitationHandler. - Removed
Agent.onElicitRequest, updated Agent startup ordering (run useronStart()before MCP restore), and adjusted dedup logic to reuse stored server IDs pre-restore. - Updated conformance worker, example app, docs, changesets, and MCP client tests to match the new elicitation API.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/agents/src/tests/mcp/http-dedup.test.ts | Adds coverage for reusing stored HTTP server IDs before connection restore. |
| packages/agents/src/tests/mcp/client-elicitation.test.ts | Updates/expands tests for manager-scoped elicitation handler behavior. |
| packages/agents/src/tests/agents/mcp.ts | Adds an agent test helper for the “stored server without restored connection” scenario. |
| packages/agents/src/mcp/index.ts | Re-exports the new MCPClientElicitationHandler type. |
| packages/agents/src/mcp/client.ts | Implements manager-scoped elicitation configuration and wiring into connections. |
| packages/agents/src/mcp/client-connection.ts | Adds connection-level handler reconfiguration for pre-initialized clients and updates messaging. |
| packages/agents/src/index.ts | Removes onElicitRequest, adjusts Agent startup sequencing, and reuses stored server IDs when adding MCP servers. |
| packages/agents/conformance/worker.ts | Migrates conformance agent to configure elicitation via this.mcp in onStart(). |
| examples/mcp-client/src/server.ts | Migrates example agent to configure elicitation via this.mcp in onStart(). |
| examples/mcp-client/README.md | Updates example documentation to reference configureElicitationHandler. |
| docs/agents/mcp-client.md | Updates public docs to the new elicitation configuration flow and lifecycle ordering. |
| .changeset/no-elicitation-capability-without-handler.md | Updates release notes to reflect handler configuration via this.mcp. |
| .changeset/mcp-client-elicitation-options.md | Updates release notes for the new API and capability-advertisement semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.mcp.configureElicitationHandler(async (request, serverId) => { | ||
| // e.g. deliver a url-mode elicitation link out-of-band | ||
| return { action: "accept" as const, content: {} }; | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we have different handlers for url and form would that make sense. Like grouped tho id like configure callback has custom something. I only want one function tho configureElicitationHandler
Client.registerCapabilities is merge-only, so a rebuild is the only way to un-advertise a mode when handlers are cleared before connecting.
Handlers are functions and cannot survive hibernation, but the capabilities they advertise can: configuring handlers stamps them onto each stored server row (server_options.capabilities), and the manager consumes the stamp as a capability seed when recreating a known server. Restored connections re-advertise the same modes at the handshake before onStart reconfigures the handlers, so automatic MCP restore keeps its original position before fiber/chat recovery and recovered turns see MCP tools. The stamp is valid for one restore: any configure call re-stamps every row, so a deploy that stops configuring handlers stops advertising stale modes after a single wake. init() re-entry rebuilds the SDK client so reconnects pick up handler changes. Seeding is self-contained in createConnection, so storage restore, the Agent RPC restore, and addMcpServer re-adds all get it uniformly.
Follow-up to #1903/#1910 (none of this has shipped — the pending Version Packages release absorbs all of it, so no compatibility path is kept).
What changed
Elicitation handling moves off the Agent class onto the MCP client manager, matching the
configureOAuthCallbackpattern.Agent.onElicitRequestis removed.How to use
Register handlers in
onStart(). The handler keys are the capability declaration: connections advertise exactly the modes with configured handlers at theinitializehandshake —formonly,urlonly, or both. No handlers → no elicitation capability advertised, so servers use their non-elicitation fallbacks.An explicit
client.capabilities.elicitationonaddMcpServerstill wins wholesale (persisted, survives hibernation), e.g. to declareform: { applyDefaults: true }.Surviving hibernation: persisted capabilities
Handlers are functions and cannot be persisted, but the capabilities they advertise can. Whenever handlers are configured (and whenever a server is registered), the handler-derived client capabilities are recorded on each stored server row (
server_options.capabilities, a generalClientCapabilitiesobject — onlyelicitationis populated today, but future handler-driven capabilities such as sampling reuse the same field with no storage migration).On wake, restored connections re-advertise the same modes at the
initializehandshake beforeonStart()has run. Elicitation dispatch looks handlers up at request time, so whenonStart()callsconfigureElicitationHandler, the handlers attach to the already-live connections — no reconnect needed. The seeding is self-contained in the manager:createConnectionconsumes the stamp from its own stored row, so every path that recreates a known server (storage restore, the Agent RPC restore,addMcpServerre-adds) gets it uniformly with no parameter threading.The stamp is valid for one restore: consuming it clears it, and any
configureElicitationHandlercall re-stamps every row. Apps that configure handlers each wake see no difference; a deploy that deletes the configure call stops advertising the stale modes after a single wake instead of forever (adversarial review caught the forever-stale variant). Configuring handlers also clears the in-memory seed (handlers become the source of truth). Explicitclient.capabilitiesdeclarations win over the seed, and reconnects rebuild the SDK client so handler changes on a live connection take effect at the next handshake.Precedence at client build:
client.capabilities.elicitation(declared) → handler-derived → persisted seed.Lifecycle
Because restored connections no longer depend on handlers existing at connect time, automatic MCP restore keeps its original position: before fiber/chat recovery and user
onStart(). Recovered turns (waitForMcpConnections) see the pending connections and run with MCP tools — an earlier revision of this PR moved restore afteronStart()and traded that away; the persisted-capability seed removes the tradeoff. An end-to-end test replays the wake sequence and pins the ordering (mutation-tested: reordering restore after recovery fails it).addMcpServerreuses the stored server id when re-adding a known server (requestedId ?? existingServer?.id ?? nanoid(8)), keeping registration idempotent during the onStart window and preserving OAuth token storage keyed by server id across hibernation.Two windows remain by construction: an elicitation request arriving after a restored connection connects but before
onStart()configures handlers (including during a recovered turn) is answered with a JSON-RPC error — servers handle elicitation failure per spec, and this equals main's behavior for form mode. And the first wake after handlers are first introduced has no stored record yet, so restored connections advertise nothing for that boot; the record is written during that wake and every later wake is correct.Implementation notes
MCPClientConnection.configureElicitationHandlerrebuilds the SDK client for not-yet-connected connections because SDKregisterCapabilitiesis merge-only — a rebuild is the only way to un-advertise a mode when handlers are cleared before connecting (evaluated and rejected theregisterCapabilitiesapproach for exactly this reason).init()re-entry (mid-session 401 → OAuth → reconnect) also rebuilds, so reconnects genuinely refresh advertised capabilities.migrateServerIdrescopes handlers to the new server id; when the manager has no handlers it leaves the connection untouched so an id migration cannot wipe a restored capability seed (mutation-tested).elicitationHandlersoption; conformance host and the mcp-client example configure viaonStart().Tests
Full agents suite (114 files / 2,267 tests), full MCP suite,
npm run check, and all conformance jobs green. New coverage: seed precedence (seed vs handlers vs explicit declaration), capability persistence on stored rows, restore-then-configure across a simulated hibernation (the fiber-recovery window), a live RPC connection seeded at connect with handlers attached afterwards (full elicitation round-trip), and the wake-ordering test above. Verified end-to-end against the runningmcp-client+mcp-elicitationexamples: both form- and url-mode round-trips complete throughconfigureElicitationHandler.