diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3f3a5b5d..d1b55f36 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -346,6 +346,147 @@ jobs: echo "pgtest: checked $(printf '%s\n' "$pkgs" | wc -l) real-Postgres packages against the service database" exit "$rc" + - name: Detect whether this PR changes the compass-agent image + id: image_affected + # PRs only. Whether the e2e gate must test a LOCALLY-BUILT image or the + # published `:latest` turns on one question — does this PR change the + # image's inputs? — and the answer already lives in one place: the + # `compass-agent-image` moon project's `inputs` globs (agent-image/moon.yml). + # Asking moon `--affected` reuses that declaration as the single source of + # truth for the image closure, so this detection cannot drift from what + # actually rebuilds the image — the same anti-silent-staleness doctrine the + # matrix rationale in this file's header rejects a hand-kept project list + # for. moon resolves the base from GITHUB_BASE_REF exactly as the "CI + # (affected)" step above does. `moon query projects` prints a JSON + # envelope on stdout UNCONDITIONALLY — an unaffected project yields + # `{"projects": [], "options": {…}}`, not empty output — so the signal is + # the discriminator: populated = affected (build from tree), empty = not + # (fall to the pull branch below); the non-PR path never runs this step, + # so image_affected is unset and the seed step's `!= true` also pulls. + # moon's own exit is checked SEPARATELY from jq: under pipefail a + # `moon query` that ITSELF exits non-zero (moon error, renamed id, + # transient) would sink the pipe, fall to the else, and set + # image_affected=false — silently pulling published :latest and testing + # the WRONG image with no signal. A broken moon means the detection + # cannot be trusted, so fail the step loudly rather than guess. + if: github.event_name == 'pull_request' + run: | + if ! moon_out=$(moon query projects --affected --id compass-agent-image); then + echo "::error::moon query for compass-agent-image affected-detection failed; cannot trust the image-affected signal, so failing rather than testing a possibly-stale published image" + exit 1 + fi + if printf '%s' "$moon_out" | jq -e '.projects | length > 0' >/dev/null; then + echo "image_affected=true" >>"$GITHUB_OUTPUT" + else + echo "image_affected=false" >>"$GITHUB_OUTPUT" + fi + + - name: Seed the compass-agent image into local containers-storage + # The dogfood e2e fixture resolves its agent image as the bare ref + # `compass-agent:latest` (go/e2e/fixture.go's agentImage const), and its + # EnsureImage present-checks the LOCAL containers-storage + # (internal/stack/adapters/image.go) — it does not pull at test time. So + # seeding that exact tag into local storage here is what satisfies the + # ensure: the run below finds it present and no registry round-trip + # happens mid-test. The bare `compass-agent:latest` ref is also what + # podmanUsable() runs as its probe, so the same seed makes that probe + # true rather than skipping the suite. + # + # Where that tag COMES FROM branches on the detection above: + # + # image-affected PR → build+load from THIS tree. On a PR that changes + # the image's inputs the gate must prove the image THIS PR produces, not + # the last-published `:latest`. `nix run path:../forks/devenv#devenv -- + # container copy agent`, run from agent-image/, builds the image and + # copies it into local containers-storage (agent-image/devenv.nix's + # `registry = "containers-storage:"`) — the exact ref the fixture + # resolves — with no registry round-trip and no `:pr-` tag. Because + # this branch now fires ONLY when the image is affected (see the fixed + # detection above), `moon ci :ci` on this same PR already built + # compass-agent-image, so the copy reuses that warm nix store and its + # realize is near-free. + # + # otherwise → pull the published image. On a PR that leaves the image + # untouched (and on every push to main, where the detection step does + # not run and image_affected is unset), test the last-published + # `:latest`. The image is public (SEA-1690; Matt-ruled public), so this + # pulls with no login, no credential, and no `packages: read` — + # `permissions: contents: read` above stays untouched. The mutable + # `:latest` tag is intentional (Matt-ruled always-fresh) — do NOT + # "fix" it to a digest pin; the gate is meant to test whatever main + # last published. + run: | + if [ "${{ steps.image_affected.outputs.image_affected }}" = "true" ]; then + ( cd agent-image && nix run path:../forks/devenv#devenv -- container copy agent ) + else + podman pull ghcr.io/rigelbuild/compass-agent:latest + podman tag ghcr.io/rigelbuild/compass-agent:latest compass-agent:latest + fi + + - name: Dogfood e2e (deterministic full-stack tier) + working-directory: go + env: + # -race needs cgo, matching the pgtest sibling above (go/moon.yml's race + # lane and the Real-Postgres step both build with CGO_ENABLED=1). Without + # it `go test -race` refuses to build. + CGO_ENABLED: '1' + # The deterministic tier: a REAL full stack — compass-server + runner + + # agent-container + a PRIVATE postgres the suite stands up itself (not the + # job's `services: postgres`) — brought up via stack.Up, driven against a + # canned in-process model. No live model egress and no secrets (Decision + # D2), so it is reproducible and safe on every PR. It is build-tagged + # `podman`, so the moon `go test ./...` battery never compiles it — this + # step is the ONLY thing that runs it. + # + # Same capture-replay-exit shape as the Real-Postgres step above and for + # the same reason: redirect (not a `| tee` pipeline, whose exit status is + # tee's 0 and would swallow a FAIL), replay the log, then exit on go + # test's own status. `|| rc=$?` because the step runs under `bash -e`, + # which would otherwise abort before the log is printed. -race matches the + # pgtest step; the 20m timeout is finite headroom for a run that builds 3 + # binaries, stands up a real stack, and runs a container turn. + run: | + rc=0 + go test -tags podman -race -v -timeout 20m ./e2e/... >/tmp/e2e.log 2>&1 || rc=$? + cat /tmp/e2e.log + exit "$rc" + + - name: Assert the dogfood e2e ran rather than skipped + working-directory: go + # The e2e legs t.Skip (never fail) when podmanUsable() is false — correct + # for a container-less sandbox, but a silent no-op here. A required check + # that let that skip pass would be VACUOUSLY green, so this guard makes an + # unavailable-podman run loud. + # + # Both halves are derived from source rather than hardcoded, matching the + # pgtest guard's discipline: a guard that drifts out of step with what it + # guards passes silently. + # - the skip text is read from go/e2e/harness_test.go, so rewording the + # skip cannot leave this grep matching nothing and reporting success; + # - the ok line is required for the e2e package specifically, so "the + # deterministic tier actually ran and passed" is what green means. + # This matters because go/e2e/cannedmodel_test.go is deliberately + # UNTAGGED and runs under a bare `go test`, so an `ok`-line grep alone + # could pass on that file while the real podman legs silently skipped + # — the skip-string check above is what closes that gap. + run: | + skip=$(sed -n 's/.*t\.Skip("\(rootless podman[^";]*\)[^"]*").*/\1/p' \ + e2e/harness_test.go) + if [ -z "$skip" ]; then + echo "::error::could not read the skip message out of go/e2e/harness_test.go — this guard has drifted from the harness and is no longer checking anything" + exit 1 + fi + if grep -qF "$skip" /tmp/e2e.log; then + echo "::error::dogfood e2e skipped — podman could not run compass-agent:latest, so nothing was asserted" + grep -nF "$skip" /tmp/e2e.log | head + exit 1 + fi + if ! grep -qE "^ok[[:space:]]+github\.com/sealedsecurity/compass/go/e2e[[:space:]]" /tmp/e2e.log; then + echo "::error::the dogfood e2e package did not report ok — it failed, skipped, or never ran" + exit 1 + fi + echo "dogfood e2e: the deterministic full-stack tier ran and reported ok" + - name: Retrospect # Collapse the single job's flat task fan-out into per-task sections in # the Actions log, so a failure is one expand instead of a scroll. Reads diff --git a/devenv.nix b/devenv.nix index d65c780f..c1d160fc 100644 --- a/devenv.nix +++ b/devenv.nix @@ -84,6 +84,22 @@ in # resolves and is cross-platform — harmless on macOS, where the app links # the system WebKit framework and pkg-config goes unused. pkg-config + + # postgresql: the dogfood e2e harness's private postgres + # (go/cmd/compass-postgres/main.go) shells out to `initdb`/`postgres`/`createdb` + # via exec.LookPath, so those binaries must be on PATH wherever the e2e suite + # runs. In the dev shell they arrive free from `services.postgres` (a devenv + # service), but CI's gate-tools (tools/toolchain/gate-tools.nix, fed by + # `parity.ts --print-nix-attrs` off THIS list) builds its PATH env ONLY from + # `packages` — service-provided binaries never reach a CI runner. So the + # harness prereq has to live here for the CI e2e gate to find `initdb`. + # + # Bare `postgresql`, not a version-suffixed attr, for strict parity: + # `services.postgres.package` defaults to bare `pkgs.postgresql` + # (forks/devenv/src/modules/services/postgres.nix), which at this devenv.lock + # pin resolves to postgresql-18.4 — the SAME derivation the service uses, so + # CI and the dev shell exercise one postgres, not two. + postgresql ]; env = { diff --git a/docs/designs/platform/compass-dogfood-e2e/design.md b/docs/designs/platform/compass-dogfood-e2e/design.md index 41d6298c..0fa114ab 100644 --- a/docs/designs/platform/compass-dogfood-e2e/design.md +++ b/docs/designs/platform/compass-dogfood-e2e/design.md @@ -62,14 +62,17 @@ its own. merged compass-stack integration test: `//go:build podman` + `podmanUsable()`-guarded skip — "a missing binary or broken rootless setup means skip, not fail" - (`go/cmd/compass-stack/integration_podman_test.go:1,77-81`). Today the runner - additionally refuses any uid but 1000 (`verifyRunnerUID(os.Getuid())`, - `go/cmd/compass-runner/main.go:93`; `const defaultAgentUID uint32 = 1000`, - `:163`), which `podmanUsable()` does not probe. The capstone gate runs its CI - runner AT uid 1000, so this does not block the Dogfood full-stack tier; - lifting it to an arbitrary uid — to run on ordinary CI runners — is a - GA-milestone follow-up (SEA-1691, milestone GA), not a capstone prerequisite - (D2). The interim uid handling for embedded Dogfood is preflight-and-refuse + (`go/cmd/compass-stack/integration_podman_test.go:1,77-81`). The runner runs + the agent as a baked in-container uid 1000 but no longer requires the HOST + uid to be 1000: containers launch with + `--userns=keep-id:uid=,gid=` + (`go/internal/runtime/podman.go:389`, `spec.UID` = `defaultAgentUID` = 1000, + `go/cmd/compass-runner/main.go:140,167`), which remaps any host uid onto the + baked uid, and the engine's support for that remap is floor-checked by + `VerifyUsernsRemapSupport` (`podman.go:415-438`, called from `main.go:97`). The + once-blocking uid-1000-host requirement (`verifyRunnerUID`) has since been + lifted (SEA-1691), so the full-stack tier runs on ordinary arbitrary-uid CI + runners. The interim uid handling for embedded Dogfood is preflight-and-refuse (compass-native T4, SEA-1685). - **AF_UNIX sun_path budget.** `stack.Config.Validate` rejects a `RuntimeDir` whose per-container agent-socket tail would overflow the platform sun_path @@ -105,12 +108,12 @@ its own. - **Deterministic tier gates PRs; live/UI tiers are on-demand.** Per D1: the backend-only + deterministic-model configuration is the per-PR CI gate and regression base; live-model and UI-inclusive runs are on-demand/nightly - (nondeterministic, keys + cost). The Dogfood capstone gate's one feasibility - prerequisite is agent-image distribution (SEA-1690, GHCR publish); until it - lands, the interim cadence keeps every PR gated by the in-process pgtest e2e - suite while the full-stack tier runs on the merge queue + nightly. Promoting - that gate to ordinary (arbitrary-uid) CI runners is a further GA-milestone - step (SEA-1691); both are the subject of Decision D2. + (nondeterministic, keys + cost). The two once-blocking feasibility + prerequisites have since landed — agent-image distribution (SEA-1690, GHCR + publish) and the host-uid lift (SEA-1691, the userns keep-id remap) — so the + full-stack deterministic tier now runs as the required per-PR check directly + on ordinary arbitrary-uid CI runners, with no interim merge-queue/nightly + staging. This is the subject of Decision D2. - **Commits** authored as Matt with the seal co-author trailer (rule://commit-conventions); the spawning agent ships the PR. @@ -732,44 +735,60 @@ UI-tier scenario passes against the harness fixture. Wire the deterministic backend-only tier into the repo's ACTUAL CI — GitHub Actions (`.github/workflows/ci.yml`; the repo has no Woodpecker config, so any Woodpecker migration is out of scope for this record). Per Decision D2 the -Dogfood end state is a required per-PR check running the full-stack -deterministic tier — on a uid-1000 runner, its one prerequisite being SEA-1690, -which publishes `compass-agent` to GHCR so `EnsureImage`'s unconditional pull -resolves in CI (needed regardless, part of getting this e2e test into CI). -Until it lands, this task wires the interim cadence: the full-stack -deterministic suite runs on the merge queue and nightly, and the existing -in-process pgtest e2e step remains the per-PR gate -(`.github/workflows/ci.yml:296-316`) — never a skip-configured required check, -which would pass vacuously green. Promoting the check further to ORDINARY -(arbitrary-uid) runners is a GA-milestone follow-up (SEA-1691), which lifts the -uid-1000 requirement (`verifyRunnerUID`, -`go/cmd/compass-runner/main.go:93,178-187` — which `podmanUsable()` does not -probe, `integration_podman_test.go:74-81`, so on a podman-capable runner at -uid ≠ 1000 the suite would otherwise go RED, not skip); it is not a Dogfood -prerequisite. +Dogfood end state — which this task now implements — is a required per-PR check +running the full-stack deterministic tier on an ORDINARY arbitrary-uid +ubuntu-latest runner. Two once-blocking prerequisites have landed: SEA-1690 +published `compass-agent` to GHCR, and the uid-1000 requirement was lifted by +the userns keep-id remap (`go/internal/runtime/podman.go`'s +`--userns=keep-id:uid=%d,gid=%d`, `:389`, floor-checked by +`VerifyUsernsRemapSupport`, `:415-438` / `go/cmd/compass-runner/main.go:97` — +`verifyRunnerUID` no longer exists), so the runner no longer has to be uid 1000 +and no interim merge-queue/nightly staging is needed. The check runs on every +PR directly, never a skip-configured required check (which would pass vacuously +green). + +The image the gate tests comes from one of two sources, decided per-PR by +whether the PR changes the image's inputs (`.github/workflows/ci.yml`'s +`compass-agent-image` moon `--affected` detection): an image-input-changing PR +builds+loads the image from THIS tree into local containers-storage (`nix run +path:../forks/devenv#devenv -- container copy agent` from `agent-image/`, the +ref the fixture resolves), so the gate proves the image the PR produces; any +other PR (and every push to main, where main's own publish already rebuilt it) +pulls the published `ghcr.io/rigelbuild/compass-agent:latest`. `:latest` is kept +mutable deliberately (Matt-ruled always-fresh), not digest-pinned. Either way +the fixture's `EnsureImage` present-checks local containers-storage and does not +pull at test time, so the seed step above is what satisfies it. + +The e2e harness additionally needs a postgres toolchain on PATH: its private +postgres (`go/cmd/compass-postgres/main.go`) shells out to `initdb`/`postgres`/ +`createdb` via `exec.LookPath`. In CI those binaries come from the devenv +`packages` list (`devenv.nix`), which gate-tools carries onto PATH; in the dev +shell they come from `services.postgres`. Without them on PATH the full-stack +bring-up cannot stand up its database and the suite fails rather than runs. Interfaces: - Consumes: `go test -tags podman ./go/e2e/...` (mirroring the pgtest step - shape, `.github/workflows/ci.yml:296-316`); H3's documented `COMPASS_MODEL` - selector; the GHCR image ref (SEA-1690); the uid lift (SEA-1691); live-mode + shape, `.github/workflows/ci.yml`'s Real-Postgres suites); H3's documented + `COMPASS_MODEL` selector; the published `compass-agent:latest` image (SEA-1690) + OR — on an image-input-changing PR — the image built+loaded from the tree via + `container copy agent`; the postgres toolchain (`initdb`/`postgres`/`createdb`) + carried onto PATH from the devenv `packages` list (`devenv.nix`); live-mode secrets (LiteLLM key) injected only in the nightly workflow, never per-PR. -- Produces: interim (pre-SEA-1690) — a merge-queue/nightly full-stack job with - the per-PR gate remaining the in-process pgtest e2e suite; Dogfood end state - (post SEA-1690) — a required per-PR check running the deterministic full-stack - tier on a uid-1000 runner; GA end state (post SEA-1691) — the same check - promoted to ordinary arbitrary-uid runners. Either way: an on-demand/nightly - live-mode job, and documented runner prereqs (Linux, rootless podman, - subuid/subgid). +- Produces: a required per-PR full-stack e2e check on an ordinary arbitrary-uid + ubuntu-latest runner, seeding the agent image per-PR (built-from-tree when the + PR changes image inputs, else pulled `:latest`); plus an on-demand/nightly + live-mode job. Documented runner prereqs: Linux, rootless podman, + subuid/subgid, and the postgres binaries on PATH (via devenv `packages` in CI, + `services.postgres` in the dev shell). Test cycle: red — no CI job compiles the podman-tagged suite (build tags keep -it out of `go test ./...`); a naive required check on a GitHub-hosted runner -(uid ≠ 1000, pre-SEA-1691) reds on `verifyRunnerUID`, and a skip-configured -one passes vacuously. Green — the interim cadence runs the full-stack tier on -the merge queue + nightly and reports honestly while the pgtest e2e suite -gates per-PR; once SEA-1690 lands the full-stack tier promotes to the required -per-PR check (uid-1000 runner), and SEA-1691 later widens it to ordinary -runners; the nightly live run reports without gating throughout. +it out of `go test ./...`), so the deterministic tier never runs and the gate is +vacuously green; a skip-configured required check would pass vacuously the same +way. Green — the full-stack tier runs as the required per-PR check on +ubuntu-latest, seeding the agent image per-PR, with the assert-ran guard making +a podman-unavailable skip loud rather than silently green; the nightly live run +reports without gating. ## Tasks @@ -780,7 +799,7 @@ runners; the nightly live run reports without gating throughout. - [ ] H5 [harness] leg-5 scenario: remove → re-provision → resume across a real container boundary (red-green) - [ ] H6 [harness] teardown + idempotence: exact-name preflight/cleanup; double-run gate (red-green via second-run provision) - [ ] H7 [ui] UI-inclusive tier scenario — gated on OQ3 + compass-ui/compass-native coordination (fast-follow unless ruled otherwise) -- [ ] H8 [ci] GitHub Actions wiring (D2): interim merge-queue/nightly full-stack + pgtest e2e per-PR gate; promotes to a required per-PR full-stack check (uid-1000 runner) once SEA-1690 (GHCR image) lands; widens to ordinary arbitrary-uid runners once SEA-1691 (uid lift, GA) lands; live tier on-demand/nightly +- [ ] H8 [ci] GitHub Actions wiring (D2): a required per-PR full-stack e2e check (`go test -tags podman ./e2e/...`) on ordinary arbitrary-uid ubuntu-latest — SEA-1690 (GHCR image) + SEA-1691 (host-uid lift) both landed, so no interim staging; seeds the agent image per-PR (built-from-tree when the PR changes image inputs, else pull `:latest`); postgres toolchain on PATH via devenv `packages`; live tier on-demand/nightly ## Decisions @@ -797,16 +816,15 @@ runners; the nightly live run reports without gating throughout. deterministic tier is the per-PR gate; the feasibility constraints in its way are FIXED, not worked around. Agent-image distribution rides SEA-1690 (publish `compass-agent` to GHCR), which is needed regardless and becomes - part of getting this e2e test into CI — it is the one prerequisite for the - Dogfood per-PR gate, which runs its CI runner at uid 1000. The runner's - uid-1000 requirement is a known limitation that "can't be required long - term"; lifting it to arbitrary uids (SEA-1691, milestone GA) promotes the - gate to ordinary CI runners, a GA-timeframe step — NOT a Dogfood-capstone - prerequisite (the interim uid handling for embedded Dogfood is the - preflight-and-refuse of compass-native T4, SEA-1685). Until SEA-1690 lands, - the interim cadence runs the full-stack tier on the merge queue + nightly - while the existing in-process pgtest e2e suite remains the per-PR gate; once - it lands, the full-stack tier promotes to the required per-PR check. + part of getting this e2e test into CI. The runner's original + uid-1000-on-the-HOST requirement was a known limitation that "can't be + required long term"; lifting it to arbitrary host uids (SEA-1691, the userns + keep-id remap) lets the gate run on ordinary CI runners. Both once-blocking + prerequisites have since landed, so the full-stack deterministic tier now + runs as the required per-PR check directly on ordinary arbitrary-uid runners + — no interim merge-queue/nightly staging. (The interim uid handling for + embedded Dogfood remains the preflight-and-refuse of compass-native T4, + SEA-1685.) Supersedes the drafted OQ5 arms: neither a bespoke uid-1000 runner ([A]) nor a permanent nightly-only fallback ([B]) — fix the constraints, gate per-PR. Folded through Global Constraints, Approach A1, and task H8. diff --git a/go/e2e/agent_ops.go b/go/e2e/agent_ops.go index 2d4fcf8a..748d08e8 100644 --- a/go/e2e/agent_ops.go +++ b/go/e2e/agent_ops.go @@ -11,6 +11,7 @@ import ( "connectrpc.com/connect" compassv1 "github.com/sealedsecurity/compass/go/gen/compass/v1" + "github.com/sealedsecurity/compass/go/internal/store" ) // CreateAgent creates a first-party agent account over CommsService and returns @@ -174,6 +175,71 @@ func (f *Fixture) waitRunnerEnrolled(ctx context.Context) error { } } +// waitDeliveryCursorPast blocks until post1 is no longer OWED to the agent on +// its home channel — the moment the agent's delivery cursor has advanced past +// it — or the budget elapses. leg-5's resume is only correct if this ordering +// holds BEFORE the resumed lifetime's server-side start-sweep runs: the cursor +// advances on the agent's delivery_ack (runnerhub deliverAck), NOT on the +// WORKING→READY settle AwaitSessionSettled observes, so a settle can return +// with post1 still owed. The resume start-sweep reads UndeliveredMessages for +// the agent and redelivers anything still owed into the fresh container2; were +// post1 still owed at that point the sweep would redeliver it, consume the +// resumed lifetime's canned turn, and desync the 2-turn script (a +// hang-to-timeout flake OR a mis-attributed green). Gating on the cursor — the +// real cross-process ack signal — instead of trusting the settle-implies-acked +// ordering closes that race. +// +// It is an event-gated bounded poll on UndeliveredMessages, mirroring +// waitRunnerEnrolled: a ticker-driven bound off f.now() with a deadline check +// that returns a legible timeout error, and a select on ctx.Done() vs the +// ticker so it respects cancellation — no sleeps, no retry-as-sync. It returns +// nil the instant post1ID is absent from the home-channel slice of the map +// UndeliveredMessages returns; a store error, ctx cancellation, or a budget +// timeout is a legible error. +func (f *Fixture) waitDeliveryCursorPast(ctx context.Context, st *store.Store, agent store.AccountID, home store.ChannelID, post1ID store.MessageID) error { + deadline := f.now().Add(cursorPollBudget) + ticker := time.NewTicker(cursorPollInterval) + defer ticker.Stop() + for { + if !f.now().Before(deadline) { + return fmt.Errorf("delivery cursor did not advance past message %s within %s", post1ID, cursorPollBudget) + } + owed, err := st.UndeliveredMessages(ctx, agent) + if err != nil { + return err + } + stillOwed := false + for _, m := range owed[home] { + if m.ID == post1ID { + stillOwed = true + break + } + } + if !stillOwed { + return nil + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + } + } +} + +// cursorPollInterval and cursorPollBudget bound waitDeliveryCursorPast: the +// wait between leg-5's pre-teardown settle and the container1 teardown for the +// agent's delivery cursor to advance past post1. The cursor advances on the +// agent's delivery_ack (runnerhub deliverAck), which trails the WORKING→READY +// settle by a bus round-trip, not by an agent turn — so like enrollment this is +// a fast one-time transition and the budget can be far smaller than +// settleTimeout while still failing a genuinely stuck ack legibly rather than +// hanging to the go-test timeout; the interval matches enrollPollInterval's +// magnitude. A deterministic deadline, never a retry loop. +const ( + cursorPollInterval = 100 * time.Millisecond + cursorPollBudget = 15 * time.Second +) + // 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. diff --git a/go/e2e/legfive_test.go b/go/e2e/legfive_test.go index d11d055a..bcc0b76c 100644 --- a/go/e2e/legfive_test.go +++ b/go/e2e/legfive_test.go @@ -77,11 +77,46 @@ func TestLegFivePersistAndResume(t *testing.T) { t.Fatalf("StartSession (container1): %v", err) } + st, err := store.Open(ctx, f.DSN()) + if err != nil { + t.Fatalf("store.Open: %v", err) + } + defer st.Close() + + // Resolve the agent's home channel once; both the pre-teardown and resumed + // turns are driven by posts to it. Each post lands on the already-live + // session and is delivered via the LIVE delivery path — the delivery + // consumer tailing the comms bus dispatches it to the live session, which + // fires the turn AwaitSessionSettled waits on. The session-start sweep only + // redelivers messages left UNDELIVERED from a prior lifetime, which is + // exactly why post1 below must be acked (cursor advanced) before the resume: + // otherwise container2's start-sweep would redeliver it and consume the + // resumed lifetime's canned turn. So each post must precede its settle wait. + acc, err := st.AgentByHandle(ctx, "leg5-persistresume") + if err != nil { + t.Fatalf("AgentByHandle: %v", err) + } + homeChannelID := string(acc.Agent.HomeChannelID) + post1ID, err := f.PostMessage(ctx, homeChannelID, "general", "say the pre-teardown reply and stop") + if err != nil { + t.Fatalf("PostMessage(home, pre-teardown): %v", err) + } + // Event-gated settle on the first session — the canned turn0 (reply1) runs. if err := f.AwaitSessionSettled(ctx, originalSessionID); err != nil { t.Fatalf("AwaitSessionSettled (original): %v", err) } + // Gate on the delivery cursor advancing past post1 BEFORE the container1 + // teardown, so the resume start-sweep in container2 does not observe post1 + // still owed and redeliver it. The cursor advances on the agent's + // delivery_ack, NOT on the settle above (AwaitSessionSettled returns on the + // first READY frame; it does not gate on the ack), so this is the event that + // actually proves post1 is consumed. See waitDeliveryCursorPast for the WHY. + if err := f.waitDeliveryCursorPast(ctx, st, acc.ID, acc.Agent.HomeChannelID, store.MessageID(post1ID)); err != nil { + t.Fatalf("waitDeliveryCursorPast (post1): %v", err) + } + // The persist boundary: tear container1 down mid-test (the leg-5 teardown // step). The logical session's transcript survives in the store keyed under // originalSessionID; container1's Cleanup above then no-ops idempotently. @@ -110,6 +145,11 @@ func TestLegFivePersistAndResume(t *testing.T) { t.Fatalf("Resume (container2): %v", err) } + // Post 2 drives the resumed turn: same home channel, resolved once above. + if _, err := f.PostMessage(ctx, homeChannelID, "general", "say the resumed reply and stop"); err != nil { + t.Fatalf("PostMessage(home, resumed): %v", err) + } + // Event-gated settle on the RESUMED session: the frame stream keys on the // minted live id, so wait on resumedSessionID — the canned turn1 (reply2) // runs. If the resumed turn never settled, this errors (the design.md:687 @@ -118,12 +158,6 @@ func TestLegFivePersistAndResume(t *testing.T) { t.Fatalf("AwaitSessionSettled (resumed): %v", err) } - st, err := store.Open(ctx, f.DSN()) - if err != nil { - t.Fatalf("store.Open: %v", err) - } - defer st.Close() - // The carried transcript lives under the ORIGINAL logical session id, NOT the // minted resumedSessionID: the persisted entry_seq is monotonic per session // across resumes and the transcript stays keyed under the logical id diff --git a/go/e2e/legthreefour_test.go b/go/e2e/legthreefour_test.go index c220184e..149256b0 100644 --- a/go/e2e/legthreefour_test.go +++ b/go/e2e/legthreefour_test.go @@ -46,9 +46,9 @@ func TestLegThreeFourSpawnAndMessaging(t *testing.T) { ctx := context.Background() // test root, threaded into NewFixture + every primitive // The peer the scripted spawn mints: a unique handle the leg-3 assertions - // resolve the fresh account and its container by. The peer provisions and - // idles (no live-model egress: the peer has no canned backend of its own, - // like the leg-2 primitives path). + // resolve the fresh account and its container by. The peer stays idle by + // default (no live-model egress: the peer has no canned backend of its own; + // it simply provisions and idles, like the leg-2 primitives path). const peerHandle = "leg34-peer" const peerDisplayName = "Leg Three-Four Peer" // The spawn tool's arguments, serialized JSON (the OpenAI tool-call @@ -115,18 +115,33 @@ func TestLegThreeFourSpawnAndMessaging(t *testing.T) { t.Fatalf("StartSession (spawner): %v", err) } - // Event-gated settle on the spawner's session: the scripted spawn tool-call - // executes and the closing text turn settles — no sleeps. - if err := f.AwaitSessionSettled(ctx, sessionID); err != nil { - t.Fatalf("AwaitSessionSettled (spawner): %v", err) - } - st, err := store.Open(ctx, f.DSN()) if err != nil { t.Fatalf("store.Open: %v", err) } defer st.Close() + // Post to the SPAWNER's home channel: this post lands on the already-live + // session and is delivered via the live fan-out (the delivery consumer + // tailing the comms bus), which fires the spawner's first turn (the one that + // issues the spawn tool-call) — the turn AwaitSessionSettled waits on. The + // session-start sweep only redelivers messages left undelivered from a prior + // lifetime (relevant only to leg-5's post1), not this one. Must precede the + // settle wait. + spawner, err := st.AgentByHandle(ctx, "leg34-spawner") + if err != nil { + t.Fatalf("AgentByHandle(spawner): %v", err) + } + if _, err := f.PostMessage(ctx, string(spawner.Agent.HomeChannelID), "general", "spawn a peer and stand by"); err != nil { + t.Fatalf("PostMessage(home): %v", err) + } + + // Event-gated settle on the spawner's session: the scripted spawn tool-call + // executes and the closing text turn settles — no sleeps. + if err := f.AwaitSessionSettled(ctx, sessionID); err != nil { + t.Fatalf("AwaitSessionSettled (spawner): %v", err) + } + // ── Leg 3: fresh peer account (F2 ownership) + a second real container ── // The spawn minted a fresh agent account resolvable by its handle. diff --git a/go/e2e/legtwo_test.go b/go/e2e/legtwo_test.go index cd13e618..11f5ef9e 100644 --- a/go/e2e/legtwo_test.go +++ b/go/e2e/legtwo_test.go @@ -63,7 +63,7 @@ func TestLegTwoPrimitives(t *testing.T) { } // TestLegTwoRealTurn is the full leg-2 scenario: CreateAgent -> Provision -> -// StartSession -> AwaitSessionSettled -> assert the session's +// StartSession -> PostMessage(home) drives the turn -> AwaitSessionSettled -> assert the session's // transcript is non-empty. On H2 it was PRESENT-BUT-SKIPPED: the leg-2 turn // cannot complete without a deterministic model backend, so on the bare stack // AwaitSessionSettled would hang and the transcript stay empty. H3 (SEA-1787) @@ -110,16 +110,30 @@ func TestLegTwoRealTurn(t *testing.T) { t.Fatalf("StartSession: %v", err) } - if err := f.AwaitSessionSettled(ctx, sessionID); err != nil { - t.Fatalf("AwaitSessionSettled: %v", err) - } - st, err := store.Open(ctx, f.DSN()) if err != nil { t.Fatalf("store.Open: %v", err) } defer st.Close() + // Post to the agent's home channel: this post lands on the already-live + // session and is delivered via the live fan-out (the delivery consumer + // tailing the comms bus), which fires the agent's first turn — the turn + // AwaitSessionSettled waits on. The session-start sweep only redelivers + // messages left undelivered from a prior lifetime (relevant only to leg-5's + // post1), not this one. Must precede the settle wait. + acc, err := st.AgentByHandle(ctx, "leg2-realturn") + if err != nil { + t.Fatalf("AgentByHandle: %v", err) + } + if _, err := f.PostMessage(ctx, string(acc.Agent.HomeChannelID), "general", "say hello and stop"); err != nil { + t.Fatalf("PostMessage(home): %v", err) + } + + if err := f.AwaitSessionSettled(ctx, sessionID); err != nil { + t.Fatalf("AwaitSessionSettled: %v", err) + } + transcript, err := st.SessionTranscript(ctx, sessionID) if err != nil { t.Fatalf("SessionTranscript: %v", err) diff --git a/go/internal/runner/e2e_transport_test.go b/go/internal/runner/e2e_transport_test.go index c9fa1483..f4b10192 100644 --- a/go/internal/runner/e2e_transport_test.go +++ b/go/internal/runner/e2e_transport_test.go @@ -314,3 +314,111 @@ func TestE2EInFlightCallForceClosedAtTeardown(t *testing.T) { t.Fatalf("host teardown must remove the agent socket: Lstat = %v", err) } } + +// TestFreshStartSendsReplayCompleteFirst — a fresh (non-resume) Start lifts the +// agent's replay barrier by sending replay_complete as the FIRST control op, so +// the first idle-deliver that starts the agent's turn is dispatched rather than +// refused. Event-gated with no clock: the producer retains ops until acked, so a +// sentinel prompt enqueued via Deliver AFTER Start sits behind the Start-sent +// replay_complete; a subscriber attaching later reads them in order. Contract: +// the first op is replay_complete and ordinary ops follow it. Mutation that +// reddens it: dropping the fresh-start SendControl in host.Start (the first op +// would then be the sentinel, GetReplayComplete() == nil). +func TestFreshStartSendsReplayCompleteFirst(t *testing.T) { + fake := &recordingRelay{} + h := newTransportFixture(t, fake) + ctx := context.Background() + + name, err := h.Provision(ctx, &compassv1.ProvisionAgentWorkspaceRequest{AgentAccountId: "0123456789abcdef0123456789abcdef"}) + if err != nil { + t.Fatalf("Provision = %v", err) + } + sessionID, err := h.Start(ctx, &compassv1.StartAgentSessionRequest{ContainerName: name}, "") + if err != nil { + t.Fatalf("Start = %v", err) + } + t.Cleanup(func() { _ = h.Stop(context.Background(), sessionID) }) + + sentinel := &compassv1internal.AgentControl{ + Control: &compassv1internal.AgentControl_Prompt{ + Prompt: &compassv1internal.PromptControl{Input: "after-barrier"}, + }, + } + if err := h.Deliver(ctx, sessionID, sentinel); err != nil { + t.Fatalf("Deliver sentinel = %v", err) + } + + subCtx, cancel := context.WithTimeout(ctx, testTimeout) + defer cancel() + stream, err := dialAgent(t, listenerPath(t, h, name)).Control(subCtx, + connect.NewRequest(&compassv1internal.ControlSubscribeRequest{})) + if err != nil { + t.Fatalf("Control over the socket = %v, want a bound subscription", err) + } + defer func() { _ = stream.Close() }() + + if !stream.Receive() { + t.Fatalf("no first op reached the agent (stream err %v): fresh Start did not send replay_complete", stream.Err()) + } + if stream.Msg().GetReplayComplete() == nil { + t.Fatalf("first op = %v, want replay_complete first on a fresh start", stream.Msg()) + } + if !stream.Receive() { + t.Fatalf("no second op reached the agent (stream err %v): sentinel did not follow replay_complete", stream.Err()) + } + if input := stream.Msg().GetPrompt().GetInput(); input != "after-barrier" { + t.Fatalf("second op input = %q, want the sentinel %q (ordinary ops follow the barrier lift)", input, "after-barrier") + } +} + +// TestResumeStartSendsNoReplayComplete — a resume Start (non-empty +// resume_session_id) NEVER sends replay_complete: the restart replay path drives +// the barrier there, so the lifecycle must not double-lift it. Same clockless +// event-gate as the fresh case: a sentinel prompt enqueued after Start is the +// only retained op, so the first op the subscriber reads is the sentinel. +// Contract: the first op is the ordinary sentinel, not replay_complete. Mutation +// that reddens it: sending replay_complete on resume (dropping the +// resume-session-id guard in host.Start) would put replay_complete first. +func TestResumeStartSendsNoReplayComplete(t *testing.T) { + fake := &recordingRelay{} + h := newTransportFixture(t, fake) + ctx := context.Background() + + name, err := h.Provision(ctx, &compassv1.ProvisionAgentWorkspaceRequest{AgentAccountId: "0123456789abcdef0123456789abcdef"}) + if err != nil { + t.Fatalf("Provision = %v", err) + } + sessionID, err := h.Start(ctx, &compassv1.StartAgentSessionRequest{ContainerName: name, ResumeSessionId: "resume-1"}, "some transcript body") + if err != nil { + t.Fatalf("Start = %v", err) + } + t.Cleanup(func() { _ = h.Stop(context.Background(), sessionID) }) + + sentinel := &compassv1internal.AgentControl{ + Control: &compassv1internal.AgentControl_Prompt{ + Prompt: &compassv1internal.PromptControl{Input: "after-barrier"}, + }, + } + if err := h.Deliver(ctx, sessionID, sentinel); err != nil { + t.Fatalf("Deliver sentinel = %v", err) + } + + subCtx, cancel := context.WithTimeout(ctx, testTimeout) + defer cancel() + stream, err := dialAgent(t, listenerPath(t, h, name)).Control(subCtx, + connect.NewRequest(&compassv1internal.ControlSubscribeRequest{})) + if err != nil { + t.Fatalf("Control over the socket = %v, want a bound subscription", err) + } + defer func() { _ = stream.Close() }() + + if !stream.Receive() { + t.Fatalf("no op reached the agent (stream err %v): sentinel was not delivered", stream.Err()) + } + if stream.Msg().GetReplayComplete() != nil { + t.Fatal("resume Start sent replay_complete; want none (the restart replay path drives the barrier)") + } + if input := stream.Msg().GetPrompt().GetInput(); input != "after-barrier" { + t.Fatalf("first op input = %q, want the sentinel %q first on a resume start", input, "after-barrier") + } +} diff --git a/go/internal/runner/host.go b/go/internal/runner/host.go index a51b8114..aeaef343 100644 --- a/go/internal/runner/host.go +++ b/go/internal/runner/host.go @@ -412,10 +412,35 @@ func (h *agentHost) Start(ctx context.Context, req *compassv1.StartAgentSessionR // agent that subscribes or acks against a session the lifecycle never bound // (or already retired) is turned away, instead of minting state nothing // would ever reclaim. - if listener, served := h.sockets[name]; served { + listener, served := h.sockets[name] + if served { listener.BindSession(sessionID) } h.mu.Unlock() + + // On a fresh (non-resume) start, lift the agent's replay barrier so the + // first idle-deliver that starts the agent's turn is dispatched rather than + // refused: the barrier defaults closed and only the arrival of + // replay_complete lifts it. A resume start never sends it — the restart + // replay path drives the barrier there. Sent after the h.mu release, + // mirroring Deliver's resolve-under-lock / send-outside discipline; the + // per-container transition lock held across Start guarantees no Stop/Retire + // races between the bind above and this send. See + // docs/designs/product/compass-system-sender-first-turn/design.md. + if served && req.GetResumeSessionId() == "" { + op := &compassv1internal.AgentControl{ + Control: &compassv1internal.AgentControl_ReplayComplete{ + ReplayComplete: &compassv1internal.ReplayComplete{}, + }, + } + if err := listener.SendControl(sessionID, op); err != nil { + // A served listener always has a wired producer (gateway.Serve wires + // it), so this is an unreachable wiring fault in production, not a + // reason to fail an already-recorded Start (recorded => success). Log + // and continue, matching Start's other degraded-posture logging. + h.log.Error("sending fresh-start replay_complete", slog.String("container", name), slog.String("session_id", sessionID), slog.Any("error", err)) + } + } return sessionID, nil }