feat: gate chat turns on reported workspace context#27181
Draft
kylecarbs wants to merge 2 commits into
Draft
Conversation
A workspace-bound chat turn no longer runs before the workspace agent has reported its context snapshot, and the first snapshot now already contains the MCP catalog: - agent: connect workspace MCP servers before releasing the context gate, bounded by a 30s first-sync wait, so the initial push carries MCP servers and tools. Late-settling servers still re-push via the catalog-change callback. Lifecycle Ready is unaffected. - agentcontext: include MCP config and live MCP server state in the aggregate hash, so MCP changes surface as chat-context drift instead of being silently missed by pinned chats. - chatd: treat the context report as a hard precondition for workspace turns. Turn prep rebinds stale agent bindings to the latest start build (re-pinning context), waits on a 1s poll up to a 15m ceiling for the agent's first report, and fails fast with actionable errors when the workspace is not started or the agent predates Agent API 2.10. Gate failures surface as the chat's visible error state and interrupts keep working during the wait. - surface: ChatContext gains a waiting/ready state, first hydration and gate-exit pinning publish a context_ready watch event, and the agents UI renders the waiting state.
Docs preview📖 View docs preview for |
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.
Summary
Workspace context (instruction files, skills, MCP tools) reached chats through a race: the agent's first context push almost never contained MCP tools, chats never waited for any push, and MCP was excluded from the drift hash, so chats hydrated from the first push silently missed workspace MCP tools until a manual refresh. This PR replaces that with an explicit readiness contract: the agent's first push is the complete initial context (MCP settled or timed out), a snapshot row is the "context reported" indicator (even when empty), and a workspace-bound turn does not run before its agent has reported.
Problem
agent/agent.goreleased the context gate (SetReady) before connecting MCP servers, so the first push carried no tools and a second push followed up to ~60s later.driftResourcesexcluded MCP from the aggregate hash on the stale assumption that tools are "discovered live at turn time". They are not: chatd builds workspace MCP tools from the pinnedchat_context_resourcesrows. The tool-bearing second push therefore neither re-pinned nor dirtied already-hydrated chats, and the tools were permanently missed.Fix
agentmcp.ReloadWithTimeout).SetReadyruns unconditionally afterwards; late-settling servers still re-push via the catalog-change callback. LifecycleReadyis reported before the wait and is unaffected.PushContextState). Interrupts keep working during the wait.ChatContextgainsstate: waiting | ready, first hydration and gate-exit pinning publish acontext_readywatch event, and the agents UI renders a waiting indicator.Behavior changes to review
start_workspace.TestStartWorkspaceTool_EndToEndwas repurposed to cover the gate error end to end; the tool itself remains covered bychattooltests. A follow-up could have the gate auto-start the workspace via the existingstartWorkspaceFninstead of erroring.Implementation plan (as reviewed)
Plan: Context-ready gating for /agents workspace context
Problem
Workspace context reaches chats through a race, and MCP tools lose it.
Current flow on
main:Ready, then releases the context gate (contextManager.SetReady()), and only then starts connecting MCP servers (agent/agent.go:1545-1567). The first real context push (initial=true) therefore almost never contains MCP servers or tools. A second push follows once the MCP catalog settles (up to ~60s later:connectTimeoutis 30s each for connect and list-tools inagent/x/agentmcp/manager.go).prepareGeneration(coderd/x/chatd/generation_preparer.go:225-249) does best-effort pinning (ensureChatContextPinnedOnFirstTurn); if the agent has not pushed yet, the turn runs with empty context and no workspace MCP tools.driftResources,agent/agentcontext/resolve.go:981-1000), so the tool-bearing second push neither re-pins nor dirties chats hydrated from the first push. Those chats permanently miss the tools unless the user manually refreshes context.The
driftResourcesexclusion is justified by a stale assumption: its comment says "the chat model discovers their tools live at turn time, not from pinned prompt content". That is no longer true. chatd builds workspace MCP tools from the pinnedchat_context_resourcesrows (pinnedWorkspaceMCPTools,coderd/x/chatd/chatd.go:445-464); only execution is proxied live.Design
Three coordinated changes plus a UI surface. Together they replace the racy two-push choreography with one explicit readiness contract:
A. Agent: first push waits for MCP settle-or-timeout
Reorder the startup sequence in
agent/agent.go(inside the existing tracked goroutine, aftersetLifecycle(Ready)so workspace readiness is unaffected):agentmcp.Manager.Reloadis already synchronous and singleflight; add a bounded-wait entry point (ReloadWithTimeout) reusing the existingwaitReloadtimeout parameter and quartz clock.connectTimeout). Servers that miss the window keep connecting in the background; the existingSetOnReload -> contextManager.Triggerpath still emits a follow-up push.SetReady()runs unconditionally after the bounded wait.KindMCPServerresources, so the first push reflects reality even when servers are broken.B. Agent: include MCP in the aggregate hash (cleanup)
Delete
driftResourcesand hash all resources uniformly. MCP tools are pinned prompt/tool content now, so MCP changes are genuine drift. Change A makes this safe: the original reason for the exclusion (the expected connect-after-startup push flipping every fresh chat to dirty) disappears when MCP state is already in the first push.C. chatd: context report is a hard precondition for workspace turns
No degraded path: a workspace-bound turn never runs without reported context. The gate has exactly three exits:
GetLatestWorkspaceAgentContextSnapshot. Interrupt keeps working; 15-minute ceiling then a visible error.Rebind on new builds happens at turn prep, before the gate. Old-build agent rows persist, so after a restart the stale binding would stick and the pinned short-circuit would pass with the previous build's context; rebind previously happened only on the dial path, so a turn that never dials kept the dead agent's binding forever and the new agent's pushes hydrated and dirtied nothing. Turn prep now detects a newer start build, rebinds via
persistBuildAgentBinding(which re-pins), and the gate applies to the new agent, so a restart re-gates exactly like a first start. Stop builds keep the old binding and pinned context.Wait ceiling rationale: the agent pushes only after startup scripts finish (deliberate: scripts clone the repos that contain AGENTS.md and .mcp.json), so first turns against a cold workspace legitimately wait for startup-script duration; a short ceiling is therefore wrong.
D. Surface the ready state
codersdk.ChatContext.State(waiting | ready),ChatWatchEventKindContextReadypublished on first hydration and gate-exit pinning, and the agents UI waiting indicator with a Storybook story.Compatibility
Risks and mitigations
🤖 This PR was generated by Coder Agents on behalf of @kylecarbs.