mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: push MCP server context and tools from agentcontext (#26533)
## What Live MCP servers and their tools now flow into the `agentcontext` snapshot and are pushed to coderd via `PushContextState`, stored alongside instruction files and skills. Previously the resolver's MCP seam was unimplemented, so live MCP tool lists never reached the pushed snapshot. `agentcontext` is now **fully self-contained** for MCP: it connects to the MCP servers declared in the `.mcp.json` files its own watcher already discovers, lists their tools, and emits `KindMCPServer` resources. It does **not** depend on or modify `agent/x/agentmcp` — that package is left pristine and keeps serving the agent's MCP HTTP API. The two MCP paths run independently, which means the legacy package can be deleted later without touching this code. ## How - **Self-contained runner** (`agentcontext/mcprunner.go`): a one-shot MCP client (connect → initialize → list tools → close) with its own `.mcp.json` parser. A Manager goroutine (`runMCPSync`) reloads it whenever the discovered `KindMCPConfig` `path:contenthash` set changes, then re-resolves so the new tools are published. Per-server connects run in parallel (bounded) with a per-server timeout; a server that fails to connect is recorded as a failure rather than aborting the batch. Each connect also force-kills its subprocess on close, because mcp-go's stdio `Close()` closes stdin and then blocks on `cmd.Wait()` with no kill — a server that ignores stdin-close would otherwise stall the whole reload loop. - **Resource production** (`agentcontext/mcp.go`): `buildMCPServerResources` turns the runner's non-blocking per-server snapshot into `KindMCPServer` resources. Connected servers carry their sorted tools (`StatusOK`); failed servers surface as `StatusUnreadable` issues instead of vanishing; connected-but-no-tools-yet are skipped until a later reload. The content hash is tool-set sensitive. The resolver consumes this through a plain `MCPResources func() []Resource` field (no `MCPProvider` interface). - **Tool names**: emitted exactly as the server reports them. Flattening into a single namespace (e.g. `server__tool`) is left to the control plane in the next step, since each resource already carries the server name. - **Drift**: MCP resources are excluded from the snapshot aggregate/drift hash (`driftResources`). MCP servers connect asynchronously after boot; without this, a server finishing its connect would dirty every hydrated chat even though nothing the user pinned changed. - **Wiring** (`agent.go`): the manager is given `ManagerOptions.MCPExecer`/`MCPUpdateEnv`; `agent/x/agentmcp` is untouched. - **Config validation**: a structurally broken `.mcp.json` surfaces as `StatusInvalid` rather than silently dropping all its servers. coderd already persists `mcp_server`/`mcp_config` resource bodies (including tools), so no coderd or proto changes were required. ## Testing - **Unit**: `buildMCPServerResources` (grouping/sort/skip/failed/hash sensitivity), MCP resources applied via the resolver seam, MCP exclusion from the aggregate hash, `.mcp.json` parsing (transport inference, env expansion), `toolInputSchema`, and `mcpConfigSet` change detection. - **Proto serialization** (`TestDRPCPusher_HappyPathSerializesAllFields`): a `KindMCPServer` resource (tools + input schema) round-trips through `PushContextState` into the `MCPServerBody` wire form, asserting the server name, tool name/description, and the decoded `input_schema`. - **Manager-level, real subprocess** (`TestManager_MCPServerToolsInSnapshot`): a `.mcp.json` points at a re-exec'd fake stdio MCP server; the runner connects it and its `echo` tool surfaces as a `KindMCPServer` resource in the Manager snapshot — the same snapshot pushed to coderd — exercising `runMCPSync` and the resolver wiring end to end. - **Regression** (`TestManager_MCPServerHangingCloseDoesNotStall`): the fake server ignores stdin-close; the test asserts its tool still surfaces, proving the runner force-kills the subprocess instead of stalling the reload. Verified to fail without the fix. - All pass under `-race`; `go build ./...`, `go vet`, and `golangci-lint` are clean on the touched packages. ## Scope / follow-ups This is the agent-side production+push half. The chatd consumer (reading the pinned MCP resources for prompt/tool injection, including any server-prefix flattening of tool names) and removing the legacy `workspaceMCPToolsCache` pull path remain follow-ups, per the RFC rollout. While both `agent/x/agentmcp` and `agentcontext` exist, stdio MCP servers are spawned by both; this is intentional and temporary until `agentmcp` is removed. <details> <summary>Implementation plan and decisions</summary> **Goal:** produce live MCP server resources (with tools) from `agentcontext` and push them to coderd. **Starting state (main):** proto (`PushContextState`, `MCPServerBody`, `MCPTool`), the drpc adapter, coderd storage (`workspace_agent_context_resources`, body kind `mcp_server`), and the resolver's MCP seam already existed; nothing implemented the seam or fed live tools into the snapshot. **Decision (agentcontext fully separate from agentmcp):** `agentcontext` starts and lists its own MCP servers using only the connect-and-list half of an mcp-go client, driven by the `.mcp.json` files its existing watcher discovers. It shares no state with `agent/x/agentmcp` and does not import it. Two earlier revisions of this branch were discarded: (1) relocating `agentmcp` into `agentcontext` (rejected — it duplicates config parsing and file watching `agentcontext` already does); (2) reading `agentmcp`'s cached server snapshot via new accessors (rejected — unnecessary coupling between two packages that should simply run independently while one is being retired). The temporary double-spawn of stdio servers is the accepted cost of keeping the two paths cleanly separated until `agentmcp` is removed. **Decision (no tool-name prefixing, no MCPProvider interface):** the agent pushes raw, unflattened data — server name plus verbatim tool names — and lets the control plane own any `server__tool` flattening. With a single self-contained producer, the `MCPProvider` interface was collapsed into a `func() []Resource` field on the resolver. **Invariants held:** no secrets (env/headers) in pushed resources, only server/tool metadata; MCP excluded from the drift hash; the seam is non-blocking so the resolver never stalls on MCP I/O. </details> --- *This PR was created by Coder Agents on behalf of @kylecarbs.*
This commit is contained in:
@@ -9,6 +9,7 @@ import (
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/quartz"
|
||||
)
|
||||
|
||||
@@ -35,9 +36,21 @@ type ManagerOptions struct {
|
||||
// directly; production callers leave it unset.
|
||||
AllowedRoots []string
|
||||
// Resolver, when non-nil, replaces the default resolver.
|
||||
// Tests use this to inject MCP providers and tighten
|
||||
// caps.
|
||||
// Tests use this to inject MCP resources (via
|
||||
// Resolver.MCPResources) and tighten caps.
|
||||
Resolver *Resolver
|
||||
// MCPExecer, when non-nil, enables the self-contained MCP
|
||||
// runner: the Manager connects to the MCP servers declared
|
||||
// in the .mcp.json files it discovers, lists their tools,
|
||||
// and surfaces them as KindMCPServer resources in every
|
||||
// snapshot. The runner uses this Execer to launch stdio MCP
|
||||
// servers. It is ignored when the resolver already has an
|
||||
// MCP provider (e.g. a test injecting one via Resolver).
|
||||
MCPExecer agentexec.Execer
|
||||
// MCPUpdateEnv optionally enriches the environment handed to
|
||||
// stdio MCP servers (typically the agent's per-command env).
|
||||
// Used only when MCPExecer is set; may be nil.
|
||||
MCPUpdateEnv func([]string) ([]string, error)
|
||||
// Debounce overrides the watcher's debounce window.
|
||||
Debounce time.Duration
|
||||
}
|
||||
@@ -60,7 +73,11 @@ type Manager struct {
|
||||
workingDir func() string
|
||||
allowedRoots []string
|
||||
resolver *Resolver
|
||||
debounce time.Duration
|
||||
// mcpRunner, when non-nil, owns the agent's self-contained
|
||||
// MCP connection lifecycle and feeds the resolver's MCP
|
||||
// provider. runMCPSync (started by Run) drives its reloads.
|
||||
mcpRunner *mcpRunner
|
||||
debounce time.Duration
|
||||
|
||||
mu sync.Mutex
|
||||
sources []Source
|
||||
@@ -135,6 +152,20 @@ func NewManager(opts ManagerOptions) *Manager {
|
||||
runStartedCh: make(chan struct{}),
|
||||
}
|
||||
|
||||
// Enable the self-contained MCP runner unless the resolver
|
||||
// already has a provider (tests inject one via Resolver). The
|
||||
// runner connects to the .mcp.json servers the resolver
|
||||
// discovers and surfaces their tools as KindMCPServer
|
||||
// resources; runMCPSync (started in Run) drives its reloads.
|
||||
// The provider must be wired before the eager first resolve
|
||||
// below so the seam is present from the first snapshot.
|
||||
if resolver.MCPResources == nil && opts.MCPExecer != nil {
|
||||
m.mcpRunner = newMCPRunner(m.logger.Named("mcp"), opts.MCPExecer, opts.MCPUpdateEnv, m.Trigger)
|
||||
resolver.MCPResources = func() []Resource {
|
||||
return buildMCPServerResources(m.mcpRunner.Servers())
|
||||
}
|
||||
}
|
||||
|
||||
for _, s := range opts.InitialSources {
|
||||
canonical, err := CanonicalizePath(s.Path)
|
||||
if err != nil {
|
||||
@@ -205,6 +236,14 @@ func (m *Manager) Run(ctx context.Context) error {
|
||||
|
||||
defer watcher.Close()
|
||||
|
||||
// Drive MCP server reloads from discovered .mcp.json files for
|
||||
// the lifetime of Run. Started here (not in NewManager) so it
|
||||
// runs alongside the trigger loop that consumes its re-resolve
|
||||
// signals.
|
||||
if m.mcpRunner != nil {
|
||||
go m.runMCPSync(ctx)
|
||||
}
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
|
||||
Reference in New Issue
Block a user