Repository navigation
refactor(mcp): split MCPClientManager into a thin facade and three controllers - #2759
Closed
mattzcarey wants to merge 1 commit into
Closed
mattzcarey wants to merge 1 commit into
mattzcarey wants to merge 1 commit into
Conversation
|
Contributor
|
✅ agents import sizes: no significant changes ( |
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
mattzcarey
force-pushed
the
refactor/mcp-client-controllers
branch
from
October 29, 2026 11:52
0daff73 to
a3a7e91
Compare
![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.
mattzcarey
force-pushed
the
refactor/mcp-client-controllers
branch
from
October 29, 2026 18:58
a3a7e91 to
f822df1
Compare
…ntrollers MCPClientManager was a 2,600-line class holding every decision. It is now the public API and the Lifecycle hooks only, delegating to: - servers.ts: each server's connection life, with afterFailure as the one place that decides retry or stop - authorization.ts: links, OAuth callbacks, request recovery, link expiry, provider creation (one createProvider instead of three copies) - jobs.ts: the durable jobs and reconnect policy (reconnect-policy.ts folded in); schedules, moves and cancels, decides nothing - status.ts: the shared stop / ready / save-session transitions Dependencies point one way. The public exports and MCPClientManager's public members are unchanged; index.ts only re-exports. Unused exports in the stack's modules are no longer exported.
mattzcarey
force-pushed
the
refactor/mcp-client-controllers
branch
from
October 29, 2026 19:28
f822df1 to
c5edcb9
Compare
Member
Author
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.
MCPClientManageris now a thin facade over three controllers, with no behaviour change. It was a 2,600-line class holding every decision; now it holds the public API and the Lifecycle hooks and delegates each call to the module that owns that kind of decision.index.tsonly re-exports.Stacked on #2758, part of the work on #2658. Every review on this stack found its bugs in the gaps between methods of that one class: a failure path that forgot to record, a link shown again after being redeemed, an
addMcpServer()that undid a recorded failure. With each kind of decision in one file, those gaps are easy to see. Dependencies point one way, top to bottom:manager.tsonStart,onRequest,onJob)servers.tsafterFailureis the one place that decides retry or stop.authorization.tsauthenticating(round limit, spent links), callbacks, request recovery, link expiry, provider creationstatus.tsjobs.tsconnectandauth-expiryjobs and the reconnect policy. It schedules, moves and cancels, and decides nothing.elicitation.ts,legacy-connect.ts,types.ts,context.tsconnect(), the public types, and the one context object the controllers shareOther changes:
createProvider()reconnect-policy.tsis folded intojobs.tsservers.tsreachesconnectToServer()anddiscoverIfConnected()through the manager, so subclass overrides and test spies still see internal callsObject.create(MCPClientManager.prototype)now constructs a real oneThe generated
.d.tsforagents/mcp/clientexports the same names, andMCPClientManagerhas the same public members.pnpm run checkand the full agents suite (4,614 tests) pass unchanged, including the MCP integration tests.