Repository navigation
fix(mcp): drop background reconnects and link expiry: quick attempts, then failed until reconnected - #2778
fix(mcp): drop background reconnects and link expiry: quick attempts, then failed until reconnected#2778mattzcarey wants to merge 8 commits into
Conversation
🦋 Changeset detectedLatest commit: 5843710 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🟢 agents import sizes: 4 entry points changed, no growth
Changed exports (24)
How this worksEach runtime export is bundled on its own, minified, and gzipped. Changes smaller than 100 B, or smaller than 1% and 1 KiB, are ignored. Growth over 10% or 5 KiB is marked 🔴. This report is informational and does not fail CI. The workflow artifact contains every measurement. Compared |
f518263 to
f5cdc9a
Compare
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
…il reconnected A connect makes the server's quick attempts (3 by default) and then settles. A failure is recorded and stays across wakes; nothing retries on a timer. reconnectMcpServer() is the retry. addMcpServer() throws for a failed server, as on main, and replaces a dead or expired link with a fresh flow, as on main. Removes the durable connect job, its backoff policy, the 24h give-up and the reconnect option; the link-expiry job stays.
… discovery with connect A server in authenticating keeps the link it was given, across wakes and addMcpServer() calls, until someone reconnects it. Whether the link still works is the authorization server's call; the callback reports it when it does not. Removes the link-expiry job (and with it jobs.ts and the manager's onJob), the 24h link cap, the expired-link check on wake, the dead-link refresh in addMcpServer() and the authorization-expired reason. Each quick attempt is now a connect plus discovery, so a catalog that fails to load once does not leave the server failed.
3175815 to
273a474
Compare
…py path Older links after Reconnect, reopened callbacks, tool calls while a step-up waits, Reconnect on a ready server or pressed twice, an unknown server, a server that goes down mid-session, and a handshake that fails once.
…e pending link, and a missing callback says how to fix it
| if ( | ||
| conn.connectionState === MCPConnectionState.AUTHENTICATING && | ||
| !this.isLiveStepUp(serverId, conn) | ||
| ) { | ||
| const authUrl = this.pendingLink(serverId, conn); | ||
| if (authUrl) { | ||
| throw new MCPAuthorizationRequiredError(serverId, authUrl, { |
There was a problem hiding this comment.
🔴 Existing grant blocked after wake
When an agent wakes with a pending step-up, recoverRequest rejects even requests covered by the existing grant. liveStepUps is empty after restore, although the OAuth token and pending link persist.
Learn more
A scope step-up leaves a usable session on the connection while the user decides. restore rebuilds a pending connection as authenticating after a wake, but the in-memory liveStepUps map is not rebuilt. The new guard therefore rejects every call without trying the still-valid grant. A first authorization must still return the pending link immediately, so the guard needs a durable way to distinguish that case from a pending step-up.
Example: Authorize files:read, request write and leave its scope link open, then let the agent wake. read now returns MCPAuthorizationRequiredError instead of succeeding on the existing grant.
Recommended fix: Persist whether the pending flow started on an established session, restore that distinction with the connection, and use it in recoverRequest. Preserve the immediate link response for servers awaiting their first grant.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const declined = params.get("error"); | ||
| if (declined && this.isLiveStepUp(serverId, conn)) { | ||
| await consumeStaleState(serverId, provider, state); | ||
| return this.declineStepUp(serverId, conn, declined); |
There was a problem hiding this comment.
🔴 Old denial cancels current step-up
When two step-up links exist, declining the older link makes declineStepUp clear the newer link. Approving the newer link then reports stale success without exchanging its code, leaving the scope ungranted.
Learn more
The authorization provider can issue several valid links before the user responds; the server saves the latest link as its active continuation. The callback checks that a state is valid but does not compare it to the active link. Denying an earlier link then clears the active link and moves the connection to ready; isPastAuthorization treats the later approval as already completed. That callback reports success without exchanging its authorization code.
Example: Two write requests produce links A and B. The user declines A and approves B; the agent reports B as authorized, but a later write still needs step-up.
Recommended fix: Match the callback state against the current pending authorization link before restoring the previous connection state. Consume earlier denied states without changing the active flow; keep B available until B completes or is declined.
Was this helpful? React with 👍 or 👎 to provide feedback.
Removes the background retry and link-expiry machinery from the MCP client. Servers now behave the way Claude Code's do: a connection makes a few quick attempts (the server's
retryoptions, 3 by default), each one a connect plus discovery. If they all fail, the failure is recorded and the server staysfailedacross wakes until someone callsreconnectMcpServer(id). A server waiting for authorization keeps the link it was given until the user completes it or reconnects.Stacked on #2761, part of the work on #2658. The stack had grown background work nobody asked for:
addMcpServer(): it swapped a dead link for a new one.We can't know when a link expires. That's the authorization server's call, and when the user clicks a dead link the callback already reports it.
mainwent the other way and recorded nothing, so every wake retried a failed server.What goes:
connectjob, its backoff policy, the 24h give-up and thereconnectmanager optionauth-expiryjob, the 24h link cap and the expired-link check on wakeaddMcpServer()'s dead-link refresh and theauthorization-expiredreasonjobs.tsand the manager'sonJob; the MCP client no longer uses the job queueNone of this has been released. Lifecycle logs and drops any leftover
mcpjob from a preview build.What stays:
reconnectMcpServer()addMcpServer()on a registered server returnsreadyor the pending link as is, and throws for a failed server. The error namesreconnectMcpServer(). A stored link that isn't an absolute http(s) URL, which only happens with corrupted storage, still starts a new flow as onmain.The stack now ships one changeset, here. The lower PRs drop theirs, so the release notes describe only what changed for users.
New tests cover the new behaviour:
addMcpServer()reports it without contacting the server either.addMcpServer()after it stops working. Clicking it reports the failure, and reconnecting gives a working link.The MCP suite (700 tests), typecheck and lint pass locally. CI covers the rest.