Repository navigation
fix(mcp): stop MCP recovery from retrying or minting links on its own - #2758
mattzcarey wants to merge 3 commits into
Conversation
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
✅ agents import sizes: no significant changes ( |
baa16aa to
197c6b0
Compare
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ MCP reconnect API lacks a release changeset
Without a changeset for reconnectMcpServer(), the release workflow has no entry to version this public API change. It cannot trigger an agents release on its own.
…ks on its own - addMcpServer() for a registered server reports its state and never reconnects, so calling it from onStart or onConnect cannot clear a recorded failure, skip the reconnect backoff or reset the step-up budget; a dead link is recorded as expired instead of replaced. New reconnectMcpServer(id) is the explicit retry. - a denied or failed OAuth callback is recorded, so a wake does not start another flow the user just turned down - dynamic client registration rejected with a 4xx (other than 429) counts as refused instead of being retried for a day - the manager remembers the state each callback redeemed; a 401 right after a completed authorization no longer re-shows that spent link and is recorded as refused - an authorization link stays pending for at most 24h even when a custom provider would still redeem its state
…ept one a person must fix addMcpServer() for a registered server reconnects it when it failed for a reason that can pass, or replaces a link that can no longer complete, at most once per first backoff step so calling it on every request cannot hammer a down server. A server stopped for a reason only a person can fix (authorization refused or denied, the step-up limit, reconnecting gave up) throws that reason, naming reconnectMcpServer(id). The reconnect job now records when its latest attempt failed.
197c6b0 to
9f93362
Compare
Closes the remaining ways MCP recovery acted on its own when only a user should.
addMcpServer()on a registered server reconnects it when it isn't connected, at most once per first backoff step, but leaves alone a server stopped for a reason only a person can fix (authorization refused or denied, the step-up limit, reconnecting gave up). The newreconnectMcpServer(id)retries those. Denied callbacks, rejected client registration, a401right after a completed authorization, and links nobody completes now stop with a recorded failure instead of retrying or showing a link that can't work.Stacked on #2751, part of the work on #2658. I found these auditing everything the stack does without a user:
addMcpServer()fromonConnectandonRequest, and two examples call it fromonStart. Because it counted as an explicit reconnect, every wake or connection cleared recorded failures, skipped the reconnect backoff and reset the step-up budget. It still reconnects what can pass, so apps that call it to bring a server back keep working.failedin memory only, so the next wake started a new flow the user had just turned downRegistrationRejectedError, not anOAuthError, so a400from/registerwas retried for a day401got the spent link saved again, which every later click rejected with "State not found or already used"statevalid, potentially foreverCallers will notice:
addMcpServer()reconnects a failed server (onmainit threw) and replaces a dead link (as onmain), at most once per 30 seconds by default. For a server stopped for a reason only a person can fix, it throws that reason and namesreconnectMcpServer(id).reconnectMcpServer(id). It returns whataddMcpServer()returns, including a freshauthUrl, and retries any failed server.pnpm run checkand the full agents workers suite pass. New integration tests cover each case, and each one fails without its fix.