refactor: consolidate agent MCP onto a single persistent engine#26599
Merged
Conversation
Two MCP paths both spawned the servers declared in .mcp.json: the persistent engine in agent/x/agentmcp, which owns tool-call execution, and an ephemeral one-shot runner in agent/agentcontext that spawned, listed tools, and immediately closed servers just for discovery. Every declared server was therefore launched twice. Make agent/x/agentmcp the single persistent MCP engine. The agentcontext manager now reads that engine's per-server catalog in-process through an injected MCPCatalog option and surfaces it as KindMCPServer resources. The engine fires SetOnReload into Manager.Trigger, so a reload re-resolves and re-pushes. Tool-call execution still flows through the engine's CallTool over /api/v0/mcp/call-tool. - agentmcp.Manager: replace the flat prefixed tool cache with a per-server catalog (Catalog/ServerStatus/ToolInfo) plus a change hook (SetOnReload/fireOnChange). refreshCatalog lists tools per connected server and surfaces failed servers as unreadable entries. - agentcontext: drop the ephemeral mcprunner, a duplicate parser/transport/env/connect stack, and the MCPExecer/MCPUpdateEnv options; consume the injected catalog instead. - Remove the now-dead discovery surface: the agent GET /api/v0/mcp/tools route with agentmcp.API.handleListTools, and workspacesdk.AgentConn.ListMCPTools with ListMCPToolsResponse (mock regenerated). Coder Agents generated on behalf of @kylecarbs
This comment has been minimized.
This comment has been minimized.
…atch make gen go generate alone leaves the generated mock's imports ungrouped. The make target additionally runs scripts/format_go_file.sh, which applies the repo's gci local-prefix grouping (cdr.dev and github.com/coder in a trailing group). Run that step so the committed mock matches CI's make gen output. Coder Agents generated on behalf of @kylecarbs
…ol names The flatTools test helper re-implemented chatd's server__tool prefixing to flatten the manager's catalog. Joining per-server tools into a single namespace is the control plane's concern; the agent exposes raw per-server tool names. Replace flatTools with connectedTools, which returns (server, tool) pairs from Catalog(), and assert on those fields. The one execution-path test that feeds CallTool still builds a prefixed name via ToolNameSep, because that mirrors what coderd sends to the agent's tool-call endpoint. Coder Agents generated on behalf of @kylecarbs
Contributor
Documentation CheckUpdates Needed
Automated review via Coder Agents |
sreya
approved these changes
Jun 23, 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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Two MCP code paths both spawned the servers declared in a workspace's
.mcp.json: the persistent engine inagent/x/agentmcp(which owns tool-call execution viaCallTool) and an ephemeral one-shot runner inagent/agentcontext(mcprunner.go) that connected, listed tools, and immediately closed each server purely for discovery. Every declared server was launched twice, and the discovery path duplicated the engine's.mcp.jsonparse, transport-build, env-resolve, and connect logic.This makes
agent/x/agentmcpthe single persistent MCP engine. Theagentcontextmanager now reads that engine's per-server catalog in-process through an injectedMCPCatalogoption and surfaces each server as aKindMCPServerresource. The engine wiresSetOnReloadto the manager'sTrigger, so a reload (startup connect or.mcp.jsonedit) re-resolves and re-pushes the pinned resources. Tool-call execution is unchanged: it still flows through the engine'sCallTooloverPOST /api/v0/mcp/call-tool.The now-dead HTTP discovery surface is removed: the agent
GET /api/v0/mcp/toolsroute withagentmcp.API.handleListTools, andworkspacesdk.AgentConn.ListMCPToolswithListMCPToolsResponse(mock regenerated). The change nets roughly-1370lines, mostly the deleted duplicate runner and its tests.Decision log
The merge of #26585 made pinned
chat_context_resourcesthe sole source of workspace context, which surfaced the duplicate spawning. Two options were considered:agent/x/agentmcpas the single persistent engine;agentcontextconsumes its catalog in-process and stays the orchestrator/owner at the API boundary (it still pushesKindMCPServerresources). This is low-risk becauseagentcontextalready exposed theresolver.MCPResourcesseam, so the change just rebinds it from the ephemeral runner to the shared engine.agentcontextand deleteagentmcp. Too broad, and it discards the engine's tested lifecycle for no behavioral gain.agentcontext's discovery was never what kept servers alive; its runner closed each server immediately after listing tools. The component holding persistent connections was alwaysagentmcp, which is why execution already lived there. Consolidating onto it removes the duplicated stack rather than a whole package: both packages survive with distinct roles (agentmcpis the engine,agentcontextis the orchestrator/owner).Coder Agents generated on behalf of @kylecarbs