fix(agent): gate workspace context collection until the agent is ready (#26715)

## Problem

Workspace context surfaced in chat (Coder Agents) is incomplete and racy
on a fresh boot:

- The context panel is missing personal skills (only repo-level skills
under `.claude/skills` show up).
- The MCP section lists `.mcp.json` files but no MCP servers are
registered.
- The Issues panel reports instruction files as unreadable, e.g.
`CLAUDE.md (file: unreadable)` and `.cursorrules (file: unreadable)`
with `symlink resolve: lstat .../AGENTS.md: no such file or directory`.

## Root cause

`agentcontext.Manager` collected and pushed context too eagerly:

- `NewManager` ran an eager resolve at agent `init()`.
- `RunPush` starts as a normal connection routine (`startAgentAPI210`)
with no lifecycle gating, so the first snapshot was pushed
(`Initial=true`) as soon as the agent API connected.

Both happened **before startup scripts finish** and before the lifecycle
reaches `ready`. At that point:

- `CLAUDE.md` / `.cursorrules` symlinks to `AGENTS.md` don't resolve
yet, so `EvalSymlinks` fails and the resolver emits `StatusUnreadable`
"symlink resolve" issues.
- Personal skills haven't synced yet, so they're missing.
- MCP servers connect via `mcpManager.Reload(...)` only **after**
`ready`, so only `.mcp.json` configs appear, with no servers.

That partial, error-laden snapshot is persisted by coderd and can
hydrate a chat.

## Fix

Gate `agentcontext.Manager` until the agent is ready, unconditionally:

- The Manager always starts gated. `NewManager` leaves the zero-value
(version 0) snapshot in place and never walks the filesystem; `RunPush`
withholds version-0 snapshots, so nothing reaches coderd.
- The agent calls `Manager.SetReady()` from the lifecycle transition in
`handleManifest`, right after startup scripts finish (`ready`, or
terminal `start_error` / `start_timeout` so a failed startup still
surfaces whatever context exists).
- On `SetReady`, the Manager performs the first real resolve (version 1)
and broadcasts it; `RunPush` ships it with `Initial=true`. Later changes
(MCP connect, skill edits) re-resolve and push as before.

Eager resolution before `ready` was the bug, not a mode worth
preserving, so the gate is always on rather than an opt-in option. This
aligns the agent-side push with chatd, which already waits for agent
readiness before loading context. No proto/coderd/DB changes: coderd
simply never receives a pre-ready snapshot.

<details>
<summary>Design notes &amp; decisions</summary>

- **Unconditional, not opt-in.** An earlier iteration added the gate as
an opt-in `ManagerOptions.GateUntilReady`. Since the eager
resolve-on-construct was the defect, the option, the eager first
resolve, and the now-dead `resolveLocked` helper were all removed; the
Manager is always gated until `SetReady`.
- **Version 0 is the pre-ready sentinel.** The gated placeholder is just
the zero-value snapshot (version 0); the first real resolve is version
1, so the push loop withholds anything at version 0. An earlier revision
carried a dedicated `Snapshot.Initializing` bool plus an HTTP `/resync`
field, but the push loop was the only consumer and nothing read the HTTP
field, so both were dropped.
- **Defer, don't retry symlinks.** Transient "unreadable" symlinks are
an artifact of collecting before checkout. Deferring until `ready` fixes
all three symptom classes at once and avoids masking genuine post-ready
errors (a broken symlink at `ready` is still reported).
- **Release on terminal startup states too** (`start_error`,
`start_timeout`), so a failed startup still surfaces whatever context
exists instead of gating forever. On reconnect the Manager instance is
reused and stays ready.

</details>

## Tests

- `agentcontext.TestManager_WithholdsCollectionUntilReady` simulates
collection running before startup finishes (broken `CLAUDE.md` /
`.cursorrules` -> `AGENTS.md` symlinks): asserts the gated snapshot is
the empty version-0 placeholder with no resources and no `unreadable`
issues, and that after `SetReady` (target now present) the inventory
resolves cleanly to a single instruction file with no spurious issues.
- `agentcontext.TestRunPush_WaitsForReady` asserts the push loop ships
nothing while gated even when content exists, then ships the full
inventory with `Initial=true` after `SetReady`.
- `agentcontext.TestManager_SetReadyIsIdempotent` covers the version-0
placeholder before ready, the single resolve to version 1 on `SetReady`,
and idempotency across repeated calls.
- Updated `agent.TestAgent_ContextStatePushed`: the first push now
already contains `AGENTS.md` with `Initial=true` and no `UNREADABLE`
resources (no pre-startup empty/partial push).

Validated on the changed packages: `go test -race
./agent/agentcontext/...`, `go test ./agent/ -run
TestAgent_ContextStatePushed`, `golangci-lint run`, `go vet`, `gofmt`
(all clean).

---
🤖 Generated by Coder Agents on behalf of @kylecarbs.

agent/agent.go

@@ -1552,6 +1552,12 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 				a.metrics.startupScriptSeconds.WithLabelValues(label).Set(dur)
 				a.scriptRunner.StartCron()
 
+				// Startup finished (success or terminal failure): release
+				// the context gate. MCP servers connect below and
+				// re-trigger a push once up, so we don't block readiness
+				// on them.
+				a.contextManager.SetReady()
+
 				// Connect to workspace MCP servers after the
 				// lifecycle transition to avoid delaying Ready.
 				// This runs inside the tracked goroutine so it

agent/agent_context_test.go

@@ -16,9 +16,11 @@ import (
 	"github.com/coder/coder/v2/testutil"
 )
 
-// TestAgent_ContextStatePushed verifies the agent's
-// agentcontext.Manager pushes its initial Snapshot to coderd
-// over the v2.10 PushContextState RPC during a normal boot.
+// TestAgent_ContextStatePushed verifies the agent pushes its workspace
+// context over the v2.10 PushContextState RPC, and that the readiness
+// gate (SetReady, wired to the lifecycle transition) holds the push
+// until startup completes. The first push therefore already contains
+// the seeded AGENTS.md with Initial=true and no "unreadable" issues.
 func TestAgent_ContextStatePushed(t *testing.T) {
 	t.Parallel()
 
@@ -35,29 +37,32 @@ func TestAgent_ContextStatePushed(t *testing.T) {
 		},
 	)
 
-	// The first push is the initial empty-workspace snapshot
-	// because the manifest has not been fetched yet. Wait for a
-	// later push that includes the seeded AGENTS.md.
+	// The push is gated until the agent reaches lifecycle ready. Wait
+	// for that first push to land.
 	var pushes []*agentproto.PushContextStateRequest
 	require.Eventually(t, func() bool {
 		pushes = client.ContextStatePushes()
-		for _, push := range pushes {
-			for _, r := range push.GetResources() {
-				if r.GetInstructionFile() != nil &&
-					filepath.Base(r.GetSource()) == "AGENTS.md" {
-					return true
-				}
-			}
-		}
-		return false
+		return len(pushes) > 0
 	}, testutil.WaitMedium, testutil.IntervalFast,
-		"expected the seeded AGENTS.md to appear in a snapshot push; got %d pushes", len(pushes))
+		"expected a context snapshot push after startup; got %d pushes", len(pushes))
 
-	require.NotEmpty(t, pushes)
 	first := pushes[0]
 	assert.True(t, first.GetInitial(), "first push must carry Initial=true")
 	assert.NotEmpty(t, first.GetAggregateHash(), "aggregate_hash must be populated")
 
+	// The first push must already reflect the ready workspace: the
+	// seeded AGENTS.md is present and no resource is UNREADABLE.
+	var foundAgents bool
+	for _, r := range first.GetResources() {
+		if r.GetInstructionFile() != nil &&
+			filepath.Base(r.GetSource()) == "AGENTS.md" {
+			foundAgents = true
+		}
+		assert.NotEqualf(t, agentproto.ContextResource_UNREADABLE, r.GetStatus(),
+			"no resource should be UNREADABLE in the post-ready snapshot: %s", r.GetSource())
+	}
+	assert.True(t, foundAgents, "first push must already include the seeded AGENTS.md")
+
 	// Subsequent pushes must not be Initial.
 	for _, p := range pushes[1:] {
 		assert.False(t, p.GetInitial(), "only the first push must be Initial")

agent/agentcontext/doc.go

@@ -17,6 +17,11 @@
 //     the tree downward or up to a parent directory.
 //   - A fixed-location fsnotify watcher that signals a re-resolve
 //     when any recognized file changes.
+//   - A readiness gate (Manager.SetReady). The Manager starts gated,
+//     publishing only an empty version-0 snapshot until the agent calls
+//     SetReady from the workspace lifecycle transition once startup
+//     scripts finish. This keeps pre-startup partial state out of
+//     coderd and chats.
 //   - An HTTP API at /api/v0/context/sources for source CRUD
 //     and /api/v0/context/resync for synchronous push barriers.
 //   - A Pusher abstraction so the latest Snapshot can be shipped

agent/agentcontext/manager.go

@@ -98,6 +98,12 @@ type Manager struct {
 	// observe a change.
 	trigger chan struct{}
 
+	// ready gates collection. While false (until the first SetReady
+	// call) the Manager does not scan; Snapshot() returns the empty
+	// version-0 value, which the push loop never sends to coderd.
+	// Guarded by mu.
+	ready bool
+
 	// running tracks Run lifetime.
 	running      bool
 	closed       bool
@@ -108,10 +114,10 @@ type Manager struct {
 	watcher *Watcher
 }
 
-// NewManager validates options, canonicalizes initial sources,
-// performs the first resolver pass synchronously, and returns
-// the resulting Manager. Run must be called separately to start
-// the watcher and re-resolve goroutine.
+// NewManager validates options and canonicalizes initial sources. The
+// returned Manager is gated, so its first snapshot is the empty
+// version-0 placeholder and the first real resolve runs on SetReady.
+// Call Run to start the watcher and re-resolve goroutine.
 func NewManager(opts ManagerOptions) *Manager {
 	clock := opts.Clock
 	if clock == nil {
@@ -146,9 +152,8 @@ func NewManager(opts ManagerOptions) *Manager {
 	// resources unless the resolver already has a provider (tests
 	// inject one via Resolver). The engine owns the connection
 	// lifecycle and notifies this Manager via Trigger when its
-	// catalog changes (see agent wiring). The provider must be wired
-	// before the eager first resolve below so the seam is present
-	// from the first snapshot.
+	// catalog changes (see agent wiring). Wire it before SetReady runs
+	// the first resolve.
 	if resolver.MCPResources == nil && opts.MCPCatalog != nil {
 		resolver.MCPResources = func() []Resource {
 			return buildMCPServerResources(opts.MCPCatalog())
@@ -170,12 +175,8 @@ func NewManager(opts ManagerOptions) *Manager {
 		m.addSourceLocked(identity)
 	}
 
-	// First snapshot is computed eagerly. The push protocol
-	// requires a snapshot to be present before the agent signals
-	// lifecycle = ready, so callers can rely on Snapshot() being
-	// populated immediately after NewManager returns.
-	m.resolveLocked()
-
+	// Start gated: m.snapshot stays the zero value (version 0) until
+	// SetReady runs the first resolve.
 	return m
 }
 
@@ -444,6 +445,12 @@ func (m *Manager) Resync(ctx context.Context) (Snapshot, error) {
 		m.mu.Unlock()
 		return m.Snapshot(), ErrManagerClosed
 	}
+	if !m.ready {
+		// Gated until SetReady: return the version-0 placeholder, no scan.
+		snap := m.snapshot
+		m.mu.Unlock()
+		return snap, nil
+	}
 	roots := m.scanRootsLocked()
 	resolver := m.resolver
 	watcher := m.watcher
@@ -535,6 +542,31 @@ func (m *Manager) Trigger() {
 	m.signal()
 }
 
+// SetReady starts context collection: the agent calls it once startup
+// scripts finish (or terminally fail) so context is never collected
+// from a half-built workspace. Idempotent; the first call triggers the
+// first resolve and push.
+func (m *Manager) SetReady() {
+	m.mu.Lock()
+	if m.ready || m.closed {
+		m.mu.Unlock()
+		return
+	}
+	m.ready = true
+	running := m.running
+	m.mu.Unlock()
+
+	if running {
+		// The Run loop owns the watcher; signal it to re-sync and resolve
+		// with ready=true.
+		m.signal()
+		return
+	}
+	// No Run loop yet (embedders or tests driving the Manager directly):
+	// resolve inline.
+	m.resolveAndBroadcast(context.Background())
+}
+
 // scanRootsLocked returns the list of ScanRoots to feed the
 // resolver and watcher. The Manager's mutex must be held.
 func (m *Manager) scanRootsLocked() []ScanRoot {
@@ -599,6 +631,11 @@ func (m *Manager) resolveAndBroadcast(ctx context.Context) {
 	// RemoveSource, Snapshot, and SubscribeChanges for the
 	// duration of the pass.
 	m.mu.Lock()
+	if !m.ready {
+		// Gated until SetReady: no scan, no broadcast.
+		m.mu.Unlock()
+		return
+	}
 	roots := m.scanRootsLocked()
 	resolver := m.resolver
 	watcher := m.watcher
@@ -654,26 +691,6 @@ func (m *Manager) resolveAndBroadcast(ctx context.Context) {
 	}
 }
 
-// resolveLocked runs the resolver inline while m.mu is held.
-// It is used by the synchronous initial resolve in NewManager,
-// where there is no concurrent reader. Background re-resolves
-// must use resolveAndBroadcast, which drops the lock around
-// filesystem I/O.
-func (m *Manager) resolveLocked() {
-	roots := m.scanRootsLocked()
-	snap := m.resolver.Resolve(roots)
-	m.version++
-	snap.Version = m.version
-	// Surface watcher degradation as a snapshot-level error
-	// when the resolver did not already emit one.
-	if snap.SnapshotError == "" && m.watcher != nil {
-		if d := m.watcher.Degraded(); d != "" {
-			snap.SnapshotError = d
-		}
-	}
-	m.snapshot = snap
-}
-
 // ErrSourceNotFound is returned by RemoveSource when the
 // requested path is not in the source list.
 var ErrSourceNotFound = xerrors.New("source not found")

agent/agentcontext/manager_test.go

@@ -43,6 +43,19 @@ func TestMain(m *testing.M) {
 }
 
 func newTestManager(t *testing.T, opts agentcontext.ManagerOptions) *agentcontext.Manager {
+	t.Helper()
+	m := newPendingTestManager(t, opts)
+	// Most tests want the ready behavior; release the gate so the first
+	// snapshot is the resolved inventory at version 1. Gate tests use
+	// newPendingTestManager directly.
+	m.SetReady()
+	return m
+}
+
+// newPendingTestManager builds a gated Manager (first snapshot is the
+// empty version-0 placeholder) for tests that drive SetReady
+// themselves.
+func newPendingTestManager(t *testing.T, opts agentcontext.ManagerOptions) *agentcontext.Manager {
 	t.Helper()
 	opts.Logger = testutil.Logger(t).Named("agentcontext-test")
 	m := agentcontext.NewManager(opts)
@@ -370,6 +383,92 @@ func TestManager_SeedSourcesLateBindsAfterManifest(t *testing.T) {
 	require.Equal(t, late, snap.Resources[0].SourcePath)
 }
 
+// TestManager_WithholdsCollectionUntilReady reproduces the boot-time
+// race: collecting before startup finishes sees instruction-file
+// symlinks (CLAUDE.md / .cursorrules -> AGENTS.md) with no target yet.
+// The gated snapshot is the version-0 placeholder with no resources or
+// errors; after SetReady the symlinks resolve cleanly.
+func TestManager_WithholdsCollectionUntilReady(t *testing.T) {
+	t.Parallel()
+	if runtime.GOOS == "windows" {
+		t.Skip("symlinks require admin privileges on Windows runners")
+	}
+	dir := testutil.TempDirResolved(t)
+	// CLAUDE.md and .cursorrules symlink to an AGENTS.md that does not
+	// exist yet, so an eager resolve would report them unreadable.
+	require.NoError(t, os.Symlink(filepath.Join(dir, "AGENTS.md"), filepath.Join(dir, "CLAUDE.md")))
+	require.NoError(t, os.Symlink(filepath.Join(dir, "AGENTS.md"), filepath.Join(dir, ".cursorrules")))
+
+	m := newPendingTestManager(t, agentcontext.ManagerOptions{
+		WorkingDir: func() string { return dir },
+	})
+
+	// Before SetReady the snapshot is the version-0 placeholder: no
+	// resources and no errors. The broken symlinks are NOT reported.
+	snap := m.Snapshot()
+	require.Zero(t, snap.Version, "gated snapshot must be the version-0 placeholder")
+	require.Empty(t, snap.Resources, "gated snapshot must not collect a partial inventory")
+	require.Empty(t, snap.SnapshotError, "gated snapshot must not surface transient errors")
+
+	// Resync stays gated too, so callers see the version-0 placeholder
+	// instead of a partial result.
+	rs, err := m.Resync(testutil.Context(t, testutil.WaitShort))
+	require.NoError(t, err)
+	require.Zero(t, rs.Version)
+	require.Empty(t, rs.Resources)
+
+	// Startup finishes: AGENTS.md now exists, so the symlinks resolve.
+	mustWriteFile(t, filepath.Join(dir, "AGENTS.md"), "# rules\n")
+
+	// Release the gate. SetReady performs the first real resolve.
+	m.SetReady()
+
+	snap = m.Snapshot()
+	require.NotZero(t, snap.Version, "post-ready snapshot must be a real resolve")
+	require.NotEmpty(t, snap.Resources, "post-ready snapshot must include resolved files")
+	require.Empty(t, snap.SnapshotError)
+	var instr int
+	for _, r := range snap.Resources {
+		require.NotEqualf(t, agentcontext.StatusUnreadable, r.Status,
+			"resolved snapshot must not contain spurious unreadable issues: %s", r.Source)
+		if r.Kind == agentcontext.KindInstructionFile {
+			instr++
+		}
+	}
+	// AGENTS.md and the two symlinks that resolve to it collapse to a
+	// single instruction-file resource.
+	require.Equal(t, 1, instr)
+}
+
+// TestManager_SetReadyIsIdempotent verifies the first SetReady resolves
+// once (version 1) and repeated calls neither panic nor re-resolve.
+func TestManager_SetReadyIsIdempotent(t *testing.T) {
+	t.Parallel()
+	dir := t.TempDir()
+	mustWriteFile(t, filepath.Join(dir, "AGENTS.md"), "rules")
+
+	// Gated: first snapshot is the version-0 placeholder.
+	m := newPendingTestManager(t, agentcontext.ManagerOptions{
+		WorkingDir: func() string { return dir },
+	})
+	snap := m.Snapshot()
+	require.Zero(t, snap.Version)
+	require.Empty(t, snap.Resources)
+
+	// First SetReady resolves once: version 1 with the inventory.
+	m.SetReady()
+	snap = m.Snapshot()
+	require.Equal(t, uint64(1), snap.Version)
+	require.Len(t, snap.Resources, 1)
+
+	// Further calls are no-ops: version and inventory unchanged.
+	m.SetReady()
+	m.SetReady()
+	snap = m.Snapshot()
+	require.Equal(t, uint64(1), snap.Version)
+	require.Len(t, snap.Resources, 1)
+}
+
 func TestManager_CloseIsIdempotent(t *testing.T) {
 	t.Parallel()
 	m := newTestManager(t, agentcontext.ManagerOptions{

agent/agentcontext/push.go

@@ -101,10 +101,20 @@ func (m *Manager) RunPush(ctx context.Context, p Pusher, opts PushOptions) error
 	changes, unsub := m.SubscribeChanges()
 	defer unsub()
 
-	// First push uses the snapshot computed by NewManager.
+	// Until SetReady the snapshot is version 0: wait, don't push it.
 	initial := true
 	for {
 		snap := m.Snapshot()
+		if snap.Version == 0 {
+			select {
+			case <-ctx.Done():
+				return ctx.Err()
+			case <-m.closedCh:
+				return nil
+			case <-changes:
+			}
+			continue
+		}
 		req := snapshotToPushRequest(snap, initial)
 
 		err := pushWithRetry(ctx, p, req, initialBackoff, maxBackoff, clock, logger)

agent/agentcontext/push_test.go

@@ -293,6 +293,60 @@ func TestRunPush_RejectedResponseProceeds(t *testing.T) {
 	require.ErrorIs(t, <-pushDone, context.Canceled)
 }
 
+// TestRunPush_WaitsForReady verifies the push loop ships nothing while
+// the Manager is gated and ships the complete inventory once SetReady
+// fires, so coderd never sees pre-startup partial state.
+func TestRunPush_WaitsForReady(t *testing.T) {
+	t.Parallel()
+	dir := t.TempDir()
+	// Content exists from the start, but the Manager is gated: nothing
+	// is pushed until SetReady.
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("rules"), 0o600))
+
+	m := newPendingTestManager(t, agentcontext.ManagerOptions{
+		WorkingDir: func() string { return dir },
+	})
+
+	p := newFakePusher()
+	ctx, cancel := context.WithCancel(testutil.Context(t, testutil.WaitLong))
+	defer cancel()
+
+	pushDone := make(chan error, 1)
+	go func() {
+		pushDone <- m.RunPush(ctx, p, agentcontext.PushOptions{
+			Logger: testutil.Logger(t).Named("push"),
+		})
+	}()
+
+	// While gated, no push must be sent even though AGENTS.md exists.
+	select {
+	case <-p.signal:
+		t.Fatal("push loop shipped a snapshot before SetReady")
+	case <-time.After(testutil.IntervalMedium):
+	}
+	require.Empty(t, p.snapshot(), "no push must happen before the gate releases")
+
+	// Startup completes; the gate releases and the first real snapshot
+	// is pushed with Initial=true.
+	m.SetReady()
+
+	select {
+	case <-p.signal:
+	case <-time.After(testutil.WaitShort):
+		t.Fatal("expected a push after SetReady")
+	}
+
+	requests := p.snapshot()
+	require.NotEmpty(t, requests)
+	first := requests[0]
+	require.True(t, first.Initial, "first push after the gate releases must be Initial")
+	require.NotEmpty(t, first.Resources, "first push must carry the resolved inventory")
+	require.Empty(t, first.SnapshotError, "first push must not carry a transient snapshot error")
+
+	cancel()
+	require.ErrorIs(t, <-pushDone, context.Canceled)
+}
+
 func TestRunPush_NilPusherErrors(t *testing.T) {
 	t.Parallel()
 	m := newTestManager(t, agentcontext.ManagerOptions{

agent/agentcontext/resolve.go

@@ -953,8 +953,10 @@ type MCPTool struct {
 // Snapshot is the immutable bundle of resources produced by a
 // single resolver pass.
 type Snapshot struct {
-	// Version is monotonically increasing per Manager
-	// instance; resets when the agent process restarts.
+	// Version is monotonically increasing per Manager instance; resets
+	// when the agent process restarts. Version 0 is the gated pre-ready
+	// placeholder (the first real resolve is version 1), which the push
+	// loop withholds.
 	Version uint64
 	// AggregateHash is sha256 over a canonical encoding of
 	// (ID, Kind, Source, ContentHash, Status) for every