Skip to content

feat: gate chat turns on reported workspace context#27181

Draft
kylecarbs wants to merge 2 commits into
mainfrom
context-report-gating
Draft

feat: gate chat turns on reported workspace context#27181
kylecarbs wants to merge 2 commits into
mainfrom
context-report-gating

Conversation

@kylecarbs

Copy link
Copy Markdown
Member

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

  1. agent/agent.go released the context gate (SetReady) before connecting MCP servers, so the first push carried no tools and a second push followed up to ~60s later.
  2. chatd's turn prep pinned context best-effort; a turn against a not-yet-reported agent ran with empty context and no workspace MCP tools.
  3. driftResources excluded 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 pinned chat_context_resources rows. The tool-bearing second push therefore neither re-pinned nor dirtied already-hydrated chats, and the tools were permanently missed.
  4. Relatedly, turn prep resolved the bound agent by ID, and old-build agent rows persist, so after a workspace rebuild a chat that never dialed kept the dead agent's binding forever and the new agent's pushes hydrated and dirtied nothing for it.

Fix

  • agent: connect workspace MCP servers before releasing the context gate, bounded by a 30s first-sync wait (agentmcp.ReloadWithTimeout). SetReady runs unconditionally afterwards; late-settling servers still re-push via the catalog-change callback. Lifecycle Ready is reported before the wait and is unaffected.
  • agentcontext: the aggregate hash now covers every resource, including MCP config and live MCP server state, so MCP changes (server added/removed, tool list changed, connect state changed) surface as chat-context drift.
  • chatd: the context report is a hard precondition for workspace turns. Turn prep rebinds stale bindings to the latest start build (re-pinning context), proceeds immediately for pinned chats, and otherwise waits on a 1s poll up to a 15m ceiling for the agent's first report. It fails fast, with actionable errors surfaced as the chat's visible error state, when the workspace's latest build is not a start transition or the agent connected with Agent API < 2.10 (pre-PushContextState). Interrupts keep working during the wait.
  • surface: ChatContext gains state: waiting | ready, first hydration and gate-exit pinning publish a context_ready watch event, and the agents UI renders a waiting indicator.

Behavior changes to review

  • An unpinned chat on a stopped workspace now fails fast ("workspace must be started to report chat context") instead of running a turn in which the model could call start_workspace. TestStartWorkspaceTool_EndToEnd was repurposed to cover the gate error end to end; the tool itself remains covered by chattool tests. A follow-up could have the gate auto-start the workspace via the existing startWorkspaceFn instead of erroring.
  • Chats on pre-2.10 agents fail fast with "update Coder and rebuild the workspace" instead of silently running without workspace context.
  • After the agent upgrade, an identical workspace produces a different aggregate hash (MCP now included), so previously pinned chats flip dirty once; a refresh picks up the tools.
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:

  1. The workspace agent reports lifecycle 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: connectTimeout is 30s each for connect and list-tools in agent/x/agentmcp/manager.go).
  2. chatd never waits for a push. 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.
  3. MCP config/server resources are deliberately excluded from the aggregate drift hash (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 driftResources exclusion 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 pinned chat_context_resources rows (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:

The agent's first non-gated push is the complete initial context (MCP settled or timed out). A snapshot row in workspace_agent_context_snapshots is the "context reported" indicator, even when the context is empty. Chats wait for that indicator before their first turn against an agent.

A. Agent: first push waits for MCP settle-or-timeout

Reorder the startup sequence in agent/agent.go (inside the existing tracked goroutine, after setLifecycle(Ready) so workspace readiness is unaffected):

before: StartCron -> SetReady -> mcpManager.Reload (unbounded wait)
after:  StartCron -> mcpManager.Reload (bounded wait) -> SetReady (always)
  • agentmcp.Manager.Reload is already synchronous and singleflight; add a bounded-wait entry point (ReloadWithTimeout) reusing the existing waitReload timeout parameter and quartz clock.
  • Gate timeout: 30s constant (aligned with connectTimeout). Servers that miss the window keep connecting in the background; the existing SetOnReload -> contextManager.Trigger path still emits a follow-up push.
  • SetReady() runs unconditionally after the bounded wait.
  • Failed servers become failed KindMCPServer resources, so the first push reflects reality even when servers are broken.
  • No proto change.

B. Agent: include MCP in the aggregate hash (cleanup)

Delete driftResources and 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:

  1. Snapshot exists: proceed and pin. Pinned chats skip the gate entirely.
  2. Provably never: fail the turn immediately with an actionable error (workspace's latest build is not a start transition; agent connected with Agent API < 2.10).
  3. Wait: poll on a quartz ticker (1s); each tick re-resolves the agent (rebinding stale bindings) and checks 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), ChatWatchEventKindContextReady published on first hydration and gate-exit pinning, and the agents UI waiting indicator with a Storybook story.

Compatibility

Agent coderd Behavior
new new Single complete first push; chats wait for it; late MCP = dirty
new old (<2.10 API) Push unimplemented; RunPush exits; no change
old (2.10, two-push) new Gate waits for first (tool-less) push; second push flips chats dirty via hash change, so tools are visible instead of missed
old (<2.10) new Turn fails fast with an actionable error (deliberate under the no-degraded-path policy)

Risks and mitigations

  • Long waits hold chat worker slots: only unpinned chats wait, the wait is a single goroutine on a ticker, provably-dead cases error immediately, interrupt always works, and the ceiling bounds the rest.
  • First turns now wait for startup scripts (SetReady semantics): intended correctness tradeoff; the UI waiting state makes it legible.
  • Stopped-workspace chats: previously hydrated chats keep working via the pinned short-circuit; only never-reported workspaces fail, with an error saying to start the workspace.
  • Hash flapping from unstable MCP servers: catalog only changes on reload/config events; a flap is real drift the user should see.

🤖 This PR was generated by Coder Agents on behalf of @kylecarbs.

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.
@github-actions

Copy link
Copy Markdown

Docs preview

📖 View docs preview for docs/reference/api/chats.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant