mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
Adds the agent half of the workspace context sources RFC. The agent now resolves instruction files, skills, and MCP configs into a typed `Snapshot`, watches the relevant paths recursively, exposes the source list over a workspace-agent HTTP API, and pushes each `Snapshot` to coderd over a new `PushContextState` RPC on Agent API v2.10. The coderd-side handler is a stub returning `Unimplemented` for now. Real persistence to `workspace_agent_context`, chatd hydration on dirty events, and the `KindMCPServer` MCP provider are tracked by [CODAGT-569](https://linear.app/codercom/issue/CODAGT-569/enable-agent-api-v210-pushcontextstate-bump-currentminor-wire-coderd). This matches the pattern used for v2.7 `ReportBoundaryLogs` in [#21293](https://github.com/coder/coder/pull/21293), which bumped the version and shipped a stub server so the wire and client could iterate before the persistence layer landed. ## What ships ### agent/agentcontext (new package) - `Source`, `Resource` (kinds `instruction_file`, `skill`, `mcp_config`, `mcp_server` plus reserved `plugin`/`hook`/`subagent`/`command`), `ResourceStatus`, `Snapshot`, `ComputeAggregateHash`. - `Manager` owns the in-memory source list, performs the initial resolve synchronously in `NewManager`, runs a re-resolve/watcher loop in `Run`, exposes `AddSource`/`RemoveSource`/`Sources`/`HasSource`/`Snapshot`/`SubscribeChanges`/`Resync`/`SeedSources`/`Close`. - `Resolver` walks scan roots, classifies recognized files, enforces 64 KiB per-resource, 2 MiB aggregate, and 500-resource caps with `StatusOversize`/`StatusExcluded`/`StatusUnreadable`/`StatusInvalid` outcomes, skips `node_modules`/`vendor`/etc., validates symlink targets stay inside the scan root, stamps `SourcePath` on user-derived resources, and optionally pulls MCP server tool lists via an `MCPProvider` interface. MCP config resources ship metadata only (size, hash) so secrets in env blocks never leave the agent. - `Watcher` is a recursive `fsnotify` wrapper with a 250 ms debounce, dynamic arming of newly created directories, and an ENOSPC-tolerant degraded mode that no-ops further syncs until the manager resyncs explicitly. - HTTP API for `GET/POST /sources`, `GET/DELETE /sources/{path}`, `POST /resync` mounted at `/api/v0/context`. - `Pusher` interface plus `RunPush` goroutine with exponential backoff capped at 30 s. `DRPCPusher` adapts the generated `DRPCAgentClient210` to `Pusher` and translates `drpcerr.Unimplemented` to `ErrPushUnimplemented` so the push loop exits cleanly when talking to coderd deployments that have not enabled the real handler. ### agent/proto (v2.10) - New messages `ContextResource`, `PushContextStateRequest`, `PushContextStateResponse` and the `PushContextState` RPC on `service Agent`. - Generated `DRPCAgentClient210` interface and `codersdk/agentsdk.Client.ConnectRPC210` / `ConnectRPC210WithRole`. - `tailnet/proto.CurrentMinor` bumped from `9` to `10`. ### Agent wiring - `agent.Options.Client` declares both v2.9 and v2.10 connectors; `run()` dials with `ConnectRPC210WithRole`. - `apiConnRoutineManager` holds a `DRPCAgentClient210`. Existing v2.8 routines keep their narrower `DRPCAgentClient28` signature thanks to interface embedding. - `startAgentAPI210` is the v2.10 counterpart to `startAgentAPI` for routines that need the new client. The push context state routine uses it. - A `contextManager` is constructed in `agent.init()`, seeded from the existing `CODER_AGENT_EXP_*_DIRS` env vars, started in its own goroutine under `gracefulCtx`, and closed in `agent.Close`. - `handleManifest` calls `Manager.SeedSources` for sources rooted at the manifest directory, then `Resync` after `manifest.Swap`, so the snapshot reflects the workspace working directory immediately instead of waiting for the next filesystem event. - HTTP routes mounted at `/api/v0/context` when the manager is up. ### Coderd stub `coderd/agentapi/context.go` returns `drpcerr.Unimplemented` for `PushContextState`. The real handler that persists `workspace_agent_context` rows, hydrates chats, and emits dirty events lives in CODAGT-569. ## Tests 24 tests across `agent/agentcontext` cover types, paths, resolver behavior with file caps, skill containers, MCP secret omission, symlink target validation, the recursive watcher firing on real fsnotify events, manager source CRUD / `Resync` / `SeedSources` / `Run` lifetime, the HTTP API, the DRPC adapter, and the push retry / initial-flag / unimplemented paths. Passes `go test -race -count=2`. `TestAgent_ContextStatePushed` boots a full agent against `agenttest.FakeAgentAPI` (which now records `PushContextState` traffic) and asserts the seeded `AGENTS.md` appears in a snapshot push with `schema_version = 1`. <details> <summary>Notes for reviewers</summary> - Source CRUD is workspace-agent-token only; coderd is not in the path for source mutation. - Per-resource cap 64 KiB, aggregate 2 MiB, count cap 500; resources past the cap ship with `StatusExcluded` and an empty payload so the aggregate hash still detects content edits. MCP-emitted resources enforce both a per-provider count cap and the aggregate byte cap. - Symlinks inside the scan root are followed; symlinks pointing outside (or broken) are rejected with `StatusExcluded` so credentials reachable via a stray symlink stay off the wire. - The initial push gates `lifecycle = ready` in the eventual full design. For this PR the `SeedSources` plus `handleManifest`-driven `Resync` keeps the snapshot fresh; the live push loop ships now and DRPCPusher translates the coderd `Unimplemented` stub into a clean exit. - The `PLUGIN`/`HOOK`/`SUBAGENT`/`COMMAND` kinds are reserved in proto and Go enums but unused; the Claude Code plugin resolver ships in a follow-up that does not need a schema migration. - Two follow-ups remain, both tracked by [CODAGT-569](https://linear.app/codercom/issue/CODAGT-569/enable-agent-api-v210-pushcontextstate-bump-currentminor-wire-coderd): (1) the chatd-side handler that persists snapshots and dirties chats; (2) the `coder exp chat context` CLI command set for `list`/`show`/`add`/`remove`/`refresh`. </details> _This PR was authored by Coder Agents on Kyle Carberry's behalf._
306 lines
8.2 KiB
Go
306 lines
8.2 KiB
Go
package agentcontext_test
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"path/filepath"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
"golang.org/x/xerrors"
|
|
|
|
"github.com/coder/coder/v2/agent/agentcontext"
|
|
"github.com/coder/coder/v2/testutil"
|
|
"github.com/coder/quartz"
|
|
)
|
|
|
|
// fakePusher records every push and lets the test control the
|
|
// returned response and error.
|
|
type fakePusher struct {
|
|
mu sync.Mutex
|
|
requests []*agentcontext.PushRequest
|
|
resp *agentcontext.PushResponse
|
|
err error
|
|
// errOnce is non-nil to simulate a single transient
|
|
// failure followed by success.
|
|
errOnce error
|
|
signal chan struct{}
|
|
}
|
|
|
|
func newFakePusher() *fakePusher {
|
|
return &fakePusher{
|
|
resp: &agentcontext.PushResponse{Accepted: true},
|
|
signal: make(chan struct{}, 16),
|
|
}
|
|
}
|
|
|
|
func (p *fakePusher) PushContextState(_ context.Context, req *agentcontext.PushRequest) (*agentcontext.PushResponse, error) {
|
|
p.mu.Lock()
|
|
defer p.mu.Unlock()
|
|
p.requests = append(p.requests, req)
|
|
if p.errOnce != nil {
|
|
err := p.errOnce
|
|
p.errOnce = nil
|
|
return nil, err
|
|
}
|
|
select {
|
|
case p.signal <- struct{}{}:
|
|
default:
|
|
}
|
|
return p.resp, p.err
|
|
}
|
|
|
|
func (p *fakePusher) snapshot() []*agentcontext.PushRequest {
|
|
p.mu.Lock()
|
|
defer p.mu.Unlock()
|
|
out := make([]*agentcontext.PushRequest, len(p.requests))
|
|
copy(out, p.requests)
|
|
return out
|
|
}
|
|
|
|
func TestRunPush_FirstPushIsInitial(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("v1"), 0o600))
|
|
|
|
m := newTestManager(t, agentcontext.ManagerOptions{
|
|
WorkingDir: func() string { return dir },
|
|
})
|
|
|
|
p := newFakePusher()
|
|
ctx, cancel := context.WithCancel(testutil.Context(t, testutil.WaitShort))
|
|
defer cancel()
|
|
|
|
pushDone := make(chan error, 1)
|
|
go func() {
|
|
pushDone <- m.RunPush(ctx, p, agentcontext.PushOptions{
|
|
Logger: testutil.Logger(t).Named("push"),
|
|
})
|
|
}()
|
|
|
|
// Wait for the first push.
|
|
select {
|
|
case <-p.signal:
|
|
case <-time.After(testutil.WaitShort):
|
|
t.Fatalf("expected initial push")
|
|
}
|
|
|
|
requests := p.snapshot()
|
|
require.Len(t, requests, 1)
|
|
require.True(t, requests[0].Initial, "first push must be initial")
|
|
require.Equal(t, uint64(1), requests[0].Version)
|
|
|
|
cancel()
|
|
require.ErrorIs(t, <-pushDone, context.Canceled)
|
|
}
|
|
|
|
func TestRunPush_SubsequentPushOnChange(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("v1"), 0o600))
|
|
|
|
m := newTestManager(t, agentcontext.ManagerOptions{
|
|
WorkingDir: func() string { return dir },
|
|
})
|
|
|
|
p := newFakePusher()
|
|
ctx, cancel := context.WithCancel(testutil.Context(t, testutil.WaitShort))
|
|
defer cancel()
|
|
|
|
pushDone := make(chan error, 1)
|
|
go func() {
|
|
pushDone <- m.RunPush(ctx, p, agentcontext.PushOptions{
|
|
Logger: testutil.Logger(t).Named("push"),
|
|
})
|
|
}()
|
|
|
|
// Initial push.
|
|
<-p.signal
|
|
|
|
// Trigger a resync via Resync.
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("v2"), 0o600))
|
|
_, err := m.Resync(ctx)
|
|
require.NoError(t, err)
|
|
|
|
// Second push.
|
|
select {
|
|
case <-p.signal:
|
|
case <-time.After(testutil.WaitShort):
|
|
t.Fatalf("expected second push after resync")
|
|
}
|
|
|
|
requests := p.snapshot()
|
|
require.GreaterOrEqual(t, len(requests), 2)
|
|
require.False(t, requests[1].Initial, "subsequent pushes must not be Initial")
|
|
require.NotEqual(t, requests[0].AggregateHash, requests[1].AggregateHash,
|
|
"second push must reflect the v2 content, not a duplicate of the first snapshot")
|
|
require.Greater(t, requests[1].Version, requests[0].Version,
|
|
"version must advance between snapshots")
|
|
|
|
cancel()
|
|
require.ErrorIs(t, <-pushDone, context.Canceled)
|
|
}
|
|
|
|
func TestRunPush_StopsOnUnimplemented(t *testing.T) {
|
|
t.Parallel()
|
|
m := newTestManager(t, agentcontext.ManagerOptions{
|
|
WorkingDir: func() string { return t.TempDir() },
|
|
})
|
|
|
|
p := newFakePusher()
|
|
p.err = agentcontext.ErrPushUnimplemented
|
|
|
|
ctx := testutil.Context(t, testutil.WaitShort)
|
|
err := m.RunPush(ctx, p, agentcontext.PushOptions{
|
|
Logger: testutil.Logger(t).Named("push"),
|
|
})
|
|
require.NoError(t, err, "Unimplemented must stop the loop cleanly")
|
|
}
|
|
|
|
func TestRunPush_RetriesTransientError(t *testing.T) {
|
|
t.Parallel()
|
|
mClock := quartz.NewMock(t)
|
|
trap := mClock.Trap().NewTimer()
|
|
defer trap.Close()
|
|
|
|
m := newTestManager(t, agentcontext.ManagerOptions{
|
|
WorkingDir: func() string { return t.TempDir() },
|
|
})
|
|
|
|
p := newFakePusher()
|
|
p.errOnce = xerrors.New("transient")
|
|
|
|
ctx, cancel := context.WithCancel(testutil.Context(t, testutil.WaitShort))
|
|
defer cancel()
|
|
pushDone := make(chan error, 1)
|
|
go func() {
|
|
pushDone <- m.RunPush(ctx, p, agentcontext.PushOptions{
|
|
Logger: testutil.Logger(t).Named("push"),
|
|
InitialBackoff: time.Second,
|
|
Clock: mClock,
|
|
})
|
|
}()
|
|
|
|
// First push hits transient and arms the retry timer. Wait for
|
|
// the timer creation, then advance the clock past the backoff.
|
|
call := trap.MustWait(ctx)
|
|
call.MustRelease(ctx)
|
|
mClock.Advance(time.Second).MustWait(ctx)
|
|
|
|
select {
|
|
case <-p.signal:
|
|
case <-time.After(testutil.WaitShort):
|
|
t.Fatalf("expected push after transient error")
|
|
}
|
|
require.GreaterOrEqual(t, len(p.snapshot()), 2)
|
|
|
|
cancel()
|
|
<-pushDone
|
|
}
|
|
|
|
// TestRunPush_ClosesOnManagerClose verifies that calling
|
|
// Manager.Close terminates an in-flight RunPush even when the
|
|
// caller's context is still live. Without this guarantee the
|
|
// agent shutdown would leak a push goroutine until the
|
|
// surrounding ctx expired.
|
|
func TestRunPush_ClosesOnManagerClose(t *testing.T) {
|
|
t.Parallel()
|
|
m := newTestManager(t, agentcontext.ManagerOptions{
|
|
WorkingDir: func() string { return t.TempDir() },
|
|
})
|
|
|
|
p := newFakePusher()
|
|
ctx := testutil.Context(t, testutil.WaitShort)
|
|
done := make(chan error, 1)
|
|
go func() {
|
|
done <- m.RunPush(ctx, p, agentcontext.PushOptions{
|
|
Logger: testutil.Logger(t).Named("push"),
|
|
})
|
|
}()
|
|
|
|
// Wait for the initial push so the loop is parked on the
|
|
// change channel, then close the Manager and assert that
|
|
// RunPush returns promptly with a nil error.
|
|
select {
|
|
case <-p.signal:
|
|
case <-ctx.Done():
|
|
t.Fatalf("initial push never landed: %v", ctx.Err())
|
|
}
|
|
require.NoError(t, m.Close())
|
|
|
|
select {
|
|
case err := <-done:
|
|
require.NoError(t, err)
|
|
case <-ctx.Done():
|
|
t.Fatalf("RunPush did not return after Manager.Close: %v", ctx.Err())
|
|
}
|
|
}
|
|
|
|
// TestRunPush_RejectedResponseProceeds verifies the contract
|
|
// that an Accepted=false response is not retried: pushWithRetry
|
|
// returns success and RunPush parks on the next change instead
|
|
// of re-sending the same snapshot. A regression that added
|
|
// retry-on-reject logic would loop here and fail the test.
|
|
func TestRunPush_RejectedResponseProceeds(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("v1"), 0o600))
|
|
|
|
m := newTestManager(t, agentcontext.ManagerOptions{
|
|
WorkingDir: func() string { return dir },
|
|
})
|
|
|
|
p := newFakePusher()
|
|
p.resp = &agentcontext.PushResponse{Accepted: false}
|
|
|
|
ctx, cancel := context.WithCancel(testutil.Context(t, testutil.WaitShort))
|
|
defer cancel()
|
|
pushDone := make(chan error, 1)
|
|
go func() {
|
|
pushDone <- m.RunPush(ctx, p, agentcontext.PushOptions{
|
|
Logger: testutil.Logger(t).Named("push"),
|
|
})
|
|
}()
|
|
|
|
// Initial push delivered and accepted=false; loop must park
|
|
// on changes, not retry the same payload.
|
|
select {
|
|
case <-p.signal:
|
|
case <-ctx.Done():
|
|
t.Fatalf("initial push never landed: %v", ctx.Err())
|
|
}
|
|
|
|
// Trigger a content change so a second push lands. Without
|
|
// the change, the loop should remain parked.
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("v2"), 0o600))
|
|
_, err := m.Resync(ctx)
|
|
require.NoError(t, err)
|
|
|
|
select {
|
|
case <-p.signal:
|
|
case <-ctx.Done():
|
|
t.Fatalf("second push never landed after change: %v", ctx.Err())
|
|
}
|
|
|
|
requests := p.snapshot()
|
|
require.GreaterOrEqual(t, len(requests), 2,
|
|
"exactly one push per snapshot; rejection must not double-fire")
|
|
require.NotEqual(t, requests[0].AggregateHash, requests[1].AggregateHash)
|
|
|
|
cancel()
|
|
require.ErrorIs(t, <-pushDone, context.Canceled)
|
|
}
|
|
|
|
func TestRunPush_NilPusherErrors(t *testing.T) {
|
|
t.Parallel()
|
|
m := newTestManager(t, agentcontext.ManagerOptions{
|
|
WorkingDir: func() string { return t.TempDir() },
|
|
})
|
|
err := m.RunPush(context.Background(), nil, agentcontext.PushOptions{
|
|
Logger: testutil.Logger(t).Named("push"),
|
|
})
|
|
require.Error(t, err)
|
|
}
|