test(e2e): gate the dogfood fixture on runner enrollment - #270
Merged
Conversation
`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 <matt@sealedsecurity.com>
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 <matt@sealedsecurity.com>
…ranch 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 <matt@sealedsecurity.com>
seal-agent
force-pushed
the
seal-h8-enroll-gate
branch
from
August 10, 2026 20:12
95f3110 to
dc1a4c7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is part of a stack containing 2 PRs:
mainstack.Upreturns as soon as the compass-runner child is spawned, but the runner enrolls with the server asynchronously over the TLS door afterUpreturns. A leg that Provisions immediately therefore races that enrollment and failsunavailable: no runner enrolled to serve session.TestHarnessCoreonly passed because two incidental authed RPCs betweenNewFixtureand 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 ownwaitReady/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 issuesStopAgentSessionon a synthetic session id (relays throughrouterForexactly as Provision does; an idempotent runner-side no-op once enrolled, with no container or session side effect), treating only that specificCodeUnavailable/no runner enrolledcondition as not-yet-ready and surfacing any other error immediately. It is wired intoNewFixture's post-Upreadiness so every Provisioning leg starts against an enrolled runner. Not a sleep and not a retry-as-sync — the same readiness idiomwaitReady/waitPostgresalready 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):TestLegTwoPrimitivesred -> green (wasProvision: unavailable: no runner enrolled, now PASS) andTestHarnessCorestill green (no regression).Spec-impact: none.
Co-authored-by: Matt Wilkinson matt@sealedsecurity.com