diff --git a/agent/agentcontext/export_test.go b/agent/agentcontext/export_test.go new file mode 100644 index 0000000000..3d7da1e96c --- /dev/null +++ b/agent/agentcontext/export_test.go @@ -0,0 +1,7 @@ +package agentcontext + +// ManagerStarted exposes the unexported started() channel for +// use by external _test packages. Production code does not need +// this signal; the agent calls Run synchronously after wiring +// the Manager. Tests use it to coordinate without polling. +func ManagerStarted(m *Manager) <-chan struct{} { return m.started() } diff --git a/agent/agentcontext/manager.go b/agent/agentcontext/manager.go index 55ed98a1c2..a781318d61 100644 --- a/agent/agentcontext/manager.go +++ b/agent/agentcontext/manager.go @@ -40,7 +40,7 @@ type ManagerOptions struct { InitialSources []Source // AllowedRoots restricts which paths may be added as // sources at runtime. Defaults to [~, ~/.coder, ~/.claude, - // workingDir]. Empty disables validation. + // workingDir]. Empty falls back to the home directory. AllowedRoots []string // Resolver, when non-nil, replaces the default resolver. // Tests use this to inject MCP providers and tighten @@ -232,11 +232,11 @@ func (m *Manager) Run(ctx context.Context) error { } } -// Started returns a channel that is closed once Run has -// claimed the running flag. Callers waiting to coordinate with -// the watcher loop can select on it; a closed channel never +// started returns a channel that is closed once Run has +// claimed the running flag. Tests use it to coordinate with +// the watcher loop without polling; a closed channel never // blocks, so this is safe to call repeatedly. -func (m *Manager) Started() <-chan struct{} { +func (m *Manager) started() <-chan struct{} { return m.runStartedCh } @@ -405,6 +405,14 @@ func (m *Manager) Resync(ctx context.Context) (Snapshot, error) { return m.Snapshot(), ctxErr } snap := resolver.ResolveContext(ctx, roots) + if ctxErr := ctx.Err(); ctxErr != nil { + // Cancellation mid-walk yields a partial or empty + // Snapshot whose SnapshotError is set to + // "context canceled". Publishing it would replace + // the live Snapshot with empty resources until the + // next trigger, so bail without touching state. + return m.Snapshot(), ctxErr + } if snap.SnapshotError == "" && watcher != nil { if d := watcher.Degraded(); d != "" { snap.SnapshotError = d @@ -545,6 +553,15 @@ func (m *Manager) resolveAndBroadcast(ctx context.Context) { return } snap := resolver.ResolveContext(ctx, roots) + if err := ctx.Err(); err != nil { + // Cancellation mid-walk yields a partial or empty + // Snapshot. Publishing it would replace the live + // Snapshot with empty resources, so bail without + // touching state. The Run loop's gracefulCtx is + // canceled only at shutdown, but defensive checks + // keep the publish contract uniform with Resync. + return + } // Surface watcher degradation as a snapshot-level error // when the resolver did not already emit one. if snap.SnapshotError == "" && watcher != nil { diff --git a/agent/agentcontext/manager_test.go b/agent/agentcontext/manager_test.go index 0f11106a57..abf3904505 100644 --- a/agent/agentcontext/manager_test.go +++ b/agent/agentcontext/manager_test.go @@ -217,6 +217,44 @@ func TestManager_ResyncReturnsLatestSnapshot(t *testing.T) { require.Equal(t, "second content edit", string(snap.Resources[0].Payload)) } +// TestManager_ResyncCanceledKeepsLiveSnapshot guards CRF-44: +// a context cancellation mid-walk must not replace the live +// Snapshot with an empty one. Resync returns the existing +// Snapshot and ctx.Err() instead of publishing a stub. +func TestManager_ResyncCanceledKeepsLiveSnapshot(t *testing.T) { + t.Parallel() + wd := t.TempDir() + mustWriteFile(t, filepath.Join(wd, "AGENTS.md"), "live content") + + m := newTestManager(t, agentcontext.ManagerOptions{ + WorkingDir: func() string { return wd }, + }) + + // Capture the live snapshot the Manager populated at + // construction time. + live := m.Snapshot() + require.Len(t, live.Resources, 1) + require.Equal(t, "live content", string(live.Resources[0].Payload)) + + // Cancel the context before calling Resync so + // ResolveContext observes the cancellation. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + snap, err := m.Resync(ctx) + require.ErrorIs(t, err, context.Canceled) + // The returned snapshot must still expose the live + // resources, not an empty result from the canceled walk. + require.Len(t, snap.Resources, 1) + require.Equal(t, "live content", string(snap.Resources[0].Payload)) + + // The next Snapshot call must also return live content; + // no stub was published. + after := m.Snapshot() + require.Equal(t, live.Version, after.Version) + require.Len(t, after.Resources, 1) +} + func TestManager_InitialSourcesSeeded(t *testing.T) { t.Parallel() wd := t.TempDir() @@ -260,7 +298,7 @@ func TestManager_RunOnce(t *testing.T) { // second call rejects with a deterministic error rather than // racing the scheduler. select { - case <-m.Started(): + case <-agentcontext.ManagerStarted(m): case <-ctx.Done(): t.Fatalf("manager never started: %v", ctx.Err()) } diff --git a/agent/agentcontext/resolve.go b/agent/agentcontext/resolve.go index c78769eb7e..4bf60e6a2e 100644 --- a/agent/agentcontext/resolve.go +++ b/agent/agentcontext/resolve.go @@ -385,9 +385,8 @@ func (r *Resolver) readInstructionFile(path string, info fs.FileInfo, userSource // readMCPConfig reads a .mcp.json file and produces a // KindMCPConfig resource. Parsing is left to consumers; the -// resolver only enforces JSON shape lightly via size and Unix -// newline conversion. Future work: detect malformed JSON and -// surface StatusInvalid. +// resolver only enforces the per-resource size cap. Future +// work: detect malformed JSON and surface StatusInvalid. func (r *Resolver) readMCPConfig(path string, info fs.FileInfo, userSource string) Resource { return r.readFileResource(KindMCPConfig, path, info, userSource) }