From 8faa6842212515b20ec6423ebb45be01535ce262 Mon Sep 17 00:00:00 2001 From: seal Date: Mon, 10 Aug 2026 11:50:24 -0400 Subject: [PATCH 1/3] test(e2e): gate the dogfood fixture on runner enrollment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stack.Up` returns as soon as the compass-runner child is spawned, but the runner enrolls with the server asynchronously over the TLS door *after* `Up` returns. A leg that Provisions immediately therefore races that enrollment and fails `unavailable: no runner enrolled to serve session`. `TestHarnessCore` only passed because two incidental authed RPCs between `NewFixture` and its first Provision gave the runner time to enroll — masking the race rather than closing it. This adds `waitRunnerEnrolled`, the enrollment counterpart to the stack's own `waitReady`/`waitPostgres`: a bounded, event-gated readiness poll that returns the instant the runner is enrolled and errors legibly on a wedged enrollment. Enrollment is a monotonic one-time transition, so the observable cross-process signal is that an enrollment-gated relay stops returning the no-runner error — the probe issues `StopAgentSession` on a synthetic session id (relays through `routerFor` exactly as Provision does; an idempotent runner-side no-op once enrolled, with no container or session side effect), treating only that specific `CodeUnavailable`/`no runner enrolled` condition as not-yet-ready and surfacing any other error immediately. It is wired into `NewFixture`'s post-`Up` readiness so every Provisioning leg starts against an enrolled runner. Not a sleep and not a retry-as-sync — the same readiness idiom `waitReady`/`waitPostgres` already use. Placement is the fixture, not `stack.Up`: the stack has no in-process enrollment signal (no authed CompassService client, and GetServerInfo does not report runner presence), whereas the fixture already holds the authed client. Putting it in the stack would duplicate CA-trust/admin-token/authed-client construction into production CLI code. Verified against the real agent image (`compass-agent:latest`, podman 5.8.4): `TestLegTwoPrimitives` red -> green (was `Provision: unavailable: no runner enrolled`, now PASS) and `TestHarnessCore` still green (no regression). Spec-impact: none. Co-authored-by: Matt Wilkinson --- go/e2e/agent_ops.go | 74 +++++++++++++++++++++++++++++++++++++++++++++ go/e2e/fixture.go | 17 ++++++++++- go/e2e/timeouts.go | 17 +++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/go/e2e/agent_ops.go b/go/e2e/agent_ops.go index 193cc35e..1e4c8338 100644 --- a/go/e2e/agent_ops.go +++ b/go/e2e/agent_ops.go @@ -5,6 +5,8 @@ package e2e import ( "context" "fmt" + "strings" + "time" "connectrpc.com/connect" @@ -127,3 +129,75 @@ func (f *Fixture) RemoveWorkspace(ctx context.Context, containerName, clientRequ } return nil } + +// waitRunnerEnrolled blocks until the embedded compass-runner has enrolled with +// the server, or the budget elapses. It is the enrollment counterpart to the +// stack's own waitReady/waitPostgres poll (stack.go): stack.Up returns as soon +// as the runner CHILD is spawned, but the runner enrolls ASYNCHRONOUSLY over the +// TLS door AFTER Up returns, so a leg that Provisions immediately races that +// enrollment and fails `unavailable: no runner enrolled to serve session`. This +// gate closes that race so every Provisioning leg starts against an enrolled +// runner. +// +// The observable enrollment signal available to the cross-process fixture is a +// lightweight enrollment-gated probe. The client GetAgentStatus is served off +// the Server's board projection (server/service.go), NOT a Runner relay, so it +// answers even with no Runner and cannot observe enrollment. StopAgentSession, +// by contrast, relays through the hub's routerFor exactly as Provision does, so +// it returns the CodeUnavailable `no runner enrolled` error until a Runner has +// enrolled — and once one has, a Stop of a synthetic never-started session id is +// an idempotent Runner-side no-op (host.Stop returns success for an unknown +// session; the session-end transcript flush is skipped since the id has no +// entries), so the probe has NO container or session side effect. ONLY that +// specific unavailable-no-runner condition is treated as not-yet-ready; any +// other error is a real failure and is returned immediately. Enrollment is a +// MONOTONIC one-time transition, so this is an event-gated readiness poll on a +// real cross-process signal, not a retry-as-sync: it returns the instant the +// probe stops reporting no-runner. The poll respects ctx cancellation; a budget +// timeout is a legible error. +func (f *Fixture) waitRunnerEnrolled(ctx context.Context) error { + deadline := time.Now().Add(enrollPollBudget) + ticker := time.NewTicker(enrollPollInterval) + defer ticker.Stop() + for { + if ready, err := f.runnerEnrolledProbe(ctx); err != nil { + return err + } else if ready { + return nil + } + if !time.Now().Before(deadline) { + return fmt.Errorf("runner did not enroll within %s", enrollPollBudget) + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + } + } +} + +// enrollProbeSessionID is the synthetic, never-started session id the enrollment +// probe Stops. It is namespaced so it can never collide with a real +// Server-minted session id; a Stop of it is an idempotent Runner-side no-op. +const enrollProbeSessionID = "e2e-enroll-probe-nonexistent-session" + +// runnerEnrolledProbe runs one lightweight enrollment-gated probe. It reports +// ready=true once the Runner is enrolled (the Stop relay no longer returns the +// no-runner error), ready=false while enrollment is still pending (the specific +// CodeUnavailable `no runner enrolled` condition), and a non-nil error for any +// other failure — which waitRunnerEnrolled surfaces immediately rather than +// polling through. +func (f *Fixture) runnerEnrolledProbe(ctx context.Context) (ready bool, err error) { + rctx, cancel := context.WithTimeout(ctx, rpcTimeout) + defer cancel() + _, err = f.Compass().StopAgentSession(rctx, connect.NewRequest(&compassv1.StopAgentSessionRequest{ + SessionId: enrollProbeSessionID, + })) + if err == nil { + return true, nil + } + if connect.CodeOf(err) == connect.CodeUnavailable && strings.Contains(err.Error(), "no runner enrolled") { + return false, nil + } + return false, fmt.Errorf("runner enrollment probe (StopAgentSession): %w", err) +} diff --git a/go/e2e/fixture.go b/go/e2e/fixture.go index 474223ce..83ab1c63 100644 --- a/go/e2e/fixture.go +++ b/go/e2e/fixture.go @@ -262,7 +262,7 @@ func NewFixture(ctx context.Context, t *testing.T, opts ...fixtureOption) *Fixtu t.Fatalf("build authed clients: %v", err) } - return &Fixture{ + f := &Fixture{ compass: compass, comms: comms, stack: st, @@ -272,6 +272,21 @@ func NewFixture(ctx context.Context, t *testing.T, opts ...fixtureOption) *Fixtu runtimeDir: runtimeDir, stub: stub, } + + // stack.Up returns as soon as the compass-runner CHILD is spawned, but the + // runner enrolls with the server ASYNCHRONOUSLY over the TLS door AFTER Up + // returns. A leg that Provisions immediately would otherwise race that + // enrollment and fail `unavailable: no runner enrolled to serve session`. + // Gate the fixture's post-Up readiness on the runner being enrolled — the + // enrollment counterpart to the stack's own waitReady/waitPostgres — so every + // leg starts against an enrolled runner. Event-gated on a real cross-process + // signal (an enrollment-gated probe), never a sleep. On the WithSite re-attach + // path the runner is already enrolled, so the first probe passes immediately. + if err := f.waitRunnerEnrolled(ctx); err != nil { + t.Fatalf("wait for runner enrollment: %v", err) + } + + return f } // cannedAgentDir is the in-container path the canned models.yml is delivered diff --git a/go/e2e/timeouts.go b/go/e2e/timeouts.go index d4544922..07fc08d2 100644 --- a/go/e2e/timeouts.go +++ b/go/e2e/timeouts.go @@ -23,3 +23,20 @@ const settleTimeout = 2 * time.Minute // carries the matching MessagePosted fails visibly here instead of blocking to // the go-test timeout. A deterministic deadline, never a retry loop. const deliverTimeout = 1 * time.Minute + +// enrollPollInterval and enrollPollBudget bound the runner-enrollment readiness +// poll NewFixture runs after stack.Up returns. Up returns as soon as the +// compass-runner CHILD is spawned (stack spawnChain step 7), but the runner +// enrolls ASYNCHRONOUSLY — it dials the server over the TLS door and enrolls +// after Up has already returned — so a leg that Provisions immediately races +// that enrollment. These bound the enrollment counterpart to the stack's own +// waitReady/waitPostgres poll: enrollment is a fast one-time transition (a +// single server dial once the server is already answering), so the budget is +// far smaller than readyPollBudget while still failing a genuinely wedged +// enrollment legibly rather than hanging to the go-test timeout; the interval +// matches readyPollInterval's magnitude. A deterministic deadline, never a +// retry loop. +const ( + enrollPollInterval = 100 * time.Millisecond + enrollPollBudget = 15 * time.Second +) From aa8e19fafbc3c5a6f34db504b28e8cdedc7d3941 Mon Sep 17 00:00:00 2001 From: seal Date: Mon, 10 Aug 2026 13:27:28 -0400 Subject: [PATCH 2/3] test(e2e): unit-test the enrollment-probe classifier + honest budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 review of the enrollment gate (medium + two lows). Additive, no behavior change on the happy/not-ready paths (re-verified green: TestLegTwoPrimitives + TestHarnessCore against the real agent image). - Extract the pure error-classification out of runnerEnrolledProbe into a free classifyEnrollProbe(err) (ready, retry bool, cerr error) and unit-test its four branches (enrolled / not-yet-enrolled retry / real error surfaced / wrong-message CodeUnavailable surfaced-not-retried) without a live client (TestClassifyEnrollProbe) — the failure branches were previously reachable only through a full real-stack run. - Add an injectable now func() time.Time clock seam to the Fixture (default time.Now), used by waitRunnerEnrolled's deadline, mirroring the sibling stack waitReady/waitPostgres s.deps.now() seam so the budget-timeout branch is deterministically testable. - Bound each probe by min(rpcTimeout, remaining-budget) so enrollPollBudget is an honest ceiling on waitRunnerEnrolled's total runtime. - Note the load-bearing "no runner enrolled" cross-package coupling at the probe (const-centralization in runnerhub tracked as SEA-1948). Spec-impact: none Co-authored-by: Matt Wilkinson --- go/e2e/agent_ops.go | 42 ++++++++++++++---- go/e2e/agent_ops_enroll_test.go | 76 +++++++++++++++++++++++++++++++++ go/e2e/fixture.go | 6 +++ 3 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 go/e2e/agent_ops_enroll_test.go diff --git a/go/e2e/agent_ops.go b/go/e2e/agent_ops.go index 1e4c8338..02a073c2 100644 --- a/go/e2e/agent_ops.go +++ b/go/e2e/agent_ops.go @@ -156,16 +156,16 @@ func (f *Fixture) RemoveWorkspace(ctx context.Context, containerName, clientRequ // probe stops reporting no-runner. The poll respects ctx cancellation; a budget // timeout is a legible error. func (f *Fixture) waitRunnerEnrolled(ctx context.Context) error { - deadline := time.Now().Add(enrollPollBudget) + deadline := f.now().Add(enrollPollBudget) ticker := time.NewTicker(enrollPollInterval) defer ticker.Stop() for { - if ready, err := f.runnerEnrolledProbe(ctx); err != nil { + if ready, err := f.runnerEnrolledProbe(ctx, deadline); err != nil { return err } else if ready { return nil } - if !time.Now().Before(deadline) { + if !f.now().Before(deadline) { return fmt.Errorf("runner did not enroll within %s", enrollPollBudget) } select { @@ -187,17 +187,43 @@ const enrollProbeSessionID = "e2e-enroll-probe-nonexistent-session" // CodeUnavailable `no runner enrolled` condition), and a non-nil error for any // other failure — which waitRunnerEnrolled surfaces immediately rather than // polling through. -func (f *Fixture) runnerEnrolledProbe(ctx context.Context) (ready bool, err error) { - rctx, cancel := context.WithTimeout(ctx, rpcTimeout) +func (f *Fixture) runnerEnrolledProbe(ctx context.Context, deadline time.Time) (ready bool, err error) { + perProbe := rpcTimeout + if !deadline.IsZero() { + if remaining := time.Until(deadline); remaining < perProbe { + perProbe = remaining + } + } + if perProbe <= 0 { + perProbe = time.Millisecond + } + rctx, cancel := context.WithTimeout(ctx, perProbe) defer cancel() _, err = f.Compass().StopAgentSession(rctx, connect.NewRequest(&compassv1.StopAgentSessionRequest{ SessionId: enrollProbeSessionID, })) + ready, retry, cerr := classifyEnrollProbe(err) + if retry { + return false, nil + } + return ready, cerr +} + +// classifyEnrollProbe classifies a StopAgentSession probe result into the +// enrollment-readiness signal, as a pure function so its branches are unit +// testable without a live client. A nil error means the Runner is enrolled. The +// substring "no runner enrolled" is a load-bearing cross-package coupling to the +// production error raised by routerFor at go/internal/runnerhub/hub.go; only that specific +// CodeUnavailable condition is treated as not-yet-ready (retry). A CodeUnavailable +// that does NOT carry that message — a transient transport flap — is intentionally +// surfaced as fatal rather than retried, which is acceptable for a deterministic +// e2e readiness gate. Any other error is a real failure surfaced immediately. +func classifyEnrollProbe(err error) (ready bool, retry bool, cerr error) { if err == nil { - return true, nil + return true, false, nil } if connect.CodeOf(err) == connect.CodeUnavailable && strings.Contains(err.Error(), "no runner enrolled") { - return false, nil + return false, true, nil } - return false, fmt.Errorf("runner enrollment probe (StopAgentSession): %w", err) + return false, false, fmt.Errorf("runner enrollment probe (StopAgentSession): %w", err) } diff --git a/go/e2e/agent_ops_enroll_test.go b/go/e2e/agent_ops_enroll_test.go new file mode 100644 index 00000000..ad92bd24 --- /dev/null +++ b/go/e2e/agent_ops_enroll_test.go @@ -0,0 +1,76 @@ +//go:build podman + +package e2e + +import ( + "errors" + "strings" + "testing" + + "connectrpc.com/connect" +) + +// TestClassifyEnrollProbe pins the pure classification of a StopAgentSession +// enrollment probe result into (ready, retry, cerr) without a live client. +func TestClassifyEnrollProbe(t *testing.T) { + tests := []struct { + name string + err error + wantReady bool + wantRetry bool + wantErr bool + }{ + { + name: "nil error means enrolled", + err: nil, + wantReady: true, + wantRetry: false, + wantErr: false, + }, + { + name: "unavailable no runner enrolled retries", + err: connect.NewError(connect.CodeUnavailable, errors.New("unavailable: no runner enrolled to serve session")), + wantReady: false, + wantRetry: true, + wantErr: false, + }, + { + name: "internal error is surfaced", + err: connect.NewError(connect.CodeInternal, errors.New("boom")), + wantReady: false, + wantRetry: false, + wantErr: true, + }, + { + name: "unavailable with other message is surfaced not retried", + err: connect.NewError(connect.CodeUnavailable, errors.New("some other unavailable")), + wantReady: false, + wantRetry: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ready, retry, cerr := classifyEnrollProbe(tt.err) + if ready != tt.wantReady { + t.Errorf("ready = %v, want %v", ready, tt.wantReady) + } + if retry != tt.wantRetry { + t.Errorf("retry = %v, want %v", retry, tt.wantRetry) + } + if tt.wantErr { + if cerr == nil { + t.Fatalf("cerr = nil, want non-nil") + } + if !errors.Is(cerr, tt.err) { + t.Errorf("cerr = %v, want to wrap %v", cerr, tt.err) + } + if !strings.Contains(cerr.Error(), "runner enrollment probe (StopAgentSession)") { + t.Errorf("cerr = %q, want probe prefix", cerr.Error()) + } + } else if cerr != nil { + t.Errorf("cerr = %v, want nil", cerr) + } + }) + } +} diff --git a/go/e2e/fixture.go b/go/e2e/fixture.go index 83ab1c63..a78de487 100644 --- a/go/e2e/fixture.go +++ b/go/e2e/fixture.go @@ -49,6 +49,11 @@ type Fixture struct { // WithCannedModel, else nil. Its lifecycle rides a t.Cleanup registered at // startup, so a consumer never closes it directly. stub *cannedModelServer + // now is the injectable wall-clock for the enrollment-readiness poll; + // defaults to time.Now. A test overrides it to drive the budget-timeout + // branch of waitRunnerEnrolled — the enrollment counterpart to the stack's + // s.deps.now() seam. + now func() time.Time } // fixtureConfig holds the optional knobs a caller flips through fixtureOption @@ -271,6 +276,7 @@ func NewFixture(ctx context.Context, t *testing.T, opts ...fixtureOption) *Fixtu serverURL: serverURL, runtimeDir: runtimeDir, stub: stub, + now: time.Now, } // stack.Up returns as soon as the compass-runner CHILD is spawned, but the From dc1a4c773ab932f53d57ddcb3b7918ed58756107 Mon Sep 17 00:00:00 2001 From: seal Date: Mon, 10 Aug 2026 13:48:10 -0400 Subject: [PATCH 3/3] test(e2e): fully wire the enrollment clock seam + cover the timeout branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review of the enrollment-probe fix found the injectable clock seam was only half-wired: waitRunnerEnrolled computed the deadline and its budget check through f.now(), but runnerEnrolledProbe still bounded each probe with time.Until(deadline) (the real wall clock). A test that fast-forwards the fake clock past the deadline would diverge from time.Until, drive the per-probe budget to the 1ms floor, and cut off a live StopAgentSession probe with a DeadlineExceeded surfaced as a fatal error — instead of the clean "did not enroll within" budget message the seam exists to make testable. - Wire the seam into the per-probe bound: deadline.Sub(f.now()) in place of time.Until(deadline), so the probe honors the injected clock exactly as the loop does. - Reorder waitRunnerEnrolled to check the budget at the top of the loop before probing. The budget is now reported without a final, doomed probe firing against an expired deadline (the round-2 LOW), and the timeout branch is reachable through the fake clock with no live client. Live semantics are preserved: at t~0 the budget check trivially passes so the first probe still fires immediately; enrolled and not-ready paths are unchanged. - Add TestWaitRunnerEnrolledBudgetTimeout: drives the budget-timeout branch through the now seam on a bare Fixture and asserts the canonical message — the missing timeout-branch coverage. - Add a context.DeadlineExceeded case to TestClassifyEnrollProbe pinning that a non-connect error is surfaced-not-retried with the probe prefix. Co-authored-by: Matt Wilkinson --- go/e2e/agent_ops.go | 8 ++++---- go/e2e/agent_ops_enroll_test.go | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/go/e2e/agent_ops.go b/go/e2e/agent_ops.go index 02a073c2..cdf2e310 100644 --- a/go/e2e/agent_ops.go +++ b/go/e2e/agent_ops.go @@ -160,14 +160,14 @@ func (f *Fixture) waitRunnerEnrolled(ctx context.Context) error { ticker := time.NewTicker(enrollPollInterval) defer ticker.Stop() for { + if !f.now().Before(deadline) { + return fmt.Errorf("runner did not enroll within %s", enrollPollBudget) + } if ready, err := f.runnerEnrolledProbe(ctx, deadline); err != nil { return err } else if ready { return nil } - if !f.now().Before(deadline) { - return fmt.Errorf("runner did not enroll within %s", enrollPollBudget) - } select { case <-ctx.Done(): return ctx.Err() @@ -190,7 +190,7 @@ const enrollProbeSessionID = "e2e-enroll-probe-nonexistent-session" func (f *Fixture) runnerEnrolledProbe(ctx context.Context, deadline time.Time) (ready bool, err error) { perProbe := rpcTimeout if !deadline.IsZero() { - if remaining := time.Until(deadline); remaining < perProbe { + if remaining := deadline.Sub(f.now()); remaining < perProbe { perProbe = remaining } } diff --git a/go/e2e/agent_ops_enroll_test.go b/go/e2e/agent_ops_enroll_test.go index ad92bd24..914ebcd3 100644 --- a/go/e2e/agent_ops_enroll_test.go +++ b/go/e2e/agent_ops_enroll_test.go @@ -3,9 +3,11 @@ package e2e import ( + "context" "errors" "strings" "testing" + "time" "connectrpc.com/connect" ) @@ -48,6 +50,13 @@ func TestClassifyEnrollProbe(t *testing.T) { wantRetry: false, wantErr: true, }, + { + name: "deadline exceeded is surfaced not retried", + err: context.DeadlineExceeded, + wantReady: false, + wantRetry: false, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -74,3 +83,29 @@ func TestClassifyEnrollProbe(t *testing.T) { }) } } + +// TestWaitRunnerEnrolledBudgetTimeout drives the budget-timeout branch of +// waitRunnerEnrolled through the injectable clock seam: the fake clock jumps +// past the deadline on the loop-top check, so the poll returns the clean +// budget-exhausted error without ever firing a live probe (a bare Fixture with +// only now set — Compass() is never reached). +func TestWaitRunnerEnrolledBudgetTimeout(t *testing.T) { + base := time.Now() + calls := 0 + f := &Fixture{ + now: func() time.Time { + calls++ + if calls == 1 { + return base // deadline = base + enrollPollBudget + } + return base.Add(enrollPollBudget + time.Second) // past the deadline + }, + } + err := f.waitRunnerEnrolled(context.Background()) + if err == nil { + t.Fatal("waitRunnerEnrolled() = nil, want budget-timeout error") + } + if !strings.Contains(err.Error(), "did not enroll within") { + t.Errorf("err = %q, want budget-timeout message", err.Error()) + } +}