fix(compass): SEA-1423 assert the snapshot-boundary frame before the seeded event

[seal-agent]

TestServeShutdownWithLiveCommsSubscriberReturnsClean has failed against a live Postgres since #825 ratified the since_seq=0 snapshot boundary. It is the known blocker recorded in the Compass test-strategy record (#740), gating that lane to reporting-only.

It was never a shutdown-ordering defect

The test read frame 1 as the seeded ChannelChanged. Frame 1 is the snapshot boundary, sent unconditionally ahead of any event on since_seq=0 (subscribe.go:72-80), so the id compare saw "" and the gate fataled. The observed failure is exactly that:

serve_pgtest_test.go:213: subscriber gate: first event channel id = "",
                          want the seeded "99d657ba08907863b6aaba50d877c143"

The old test encoded a two-part claim: the wire starts with the seeded event and shutdown drains clean. #825 falsified the first. The second was never in question — which matters, because the record previously carried this as a suspected drain defect, and anyone acting on that would have hunted a bug that does not exist.

The fix asserts the boundary rather than skipping it

Read frame 1, assert it, then gate on the event. A bare Recv() discarding frame 1 would also go green against a server that stopped sending the boundary entirely — the regression #825 pinned. So the frame is checked for:

Two discriminators, both documented as such by the frame's own contract (subscribe.go:220-224 — "discriminates it from a positioned event by its zero seq and absent payload"):

• nil payload — an event frame always carries a payload oneof.
• seq == 0 — the boundary is a control frame at commsResyncSeq, never positioned in bus-seq space, where a real event carries the seq stamped at publish (events.go:169-172, off a nextSeq that starts at 1 — events.go:155 — so no event is ever seq 0).

Two weaker checks, labelled in the code as not discriminators so a later reader does not overtrust them:

• instance_epoch, checked twice — on the boundary alone it can only be a field-stamping check (!= 0), since every event carries the epoch too (subscribe.go:201); what it catches is a boundary built with a literal 0 instead of sub.Epoch. Against the seeded event it becomes an equality check: both frames come from one Bus and carry one per-boot nonce (events.go:174, :242, :288), so a boundary stamped from any other source reddens — which the != 0 form passes silently.
• snapshot_seq == 0 — a no-drift check on the empty-store head, explicitly not a presence check. See below.

Every drain assertion is byte-for-byte unchanged, so a genuine ordering defect still reddens the test. This is strictly more coverage than before: the boundary contract now has a serve-level assertion it did not have.

One deviation from the ruled shape, forced by the code

The ruling asked to assert GetSnapshotSeq()/GetInstanceEpoch() are set. snapshot_seq is COALESCE(MAX(seq), 0) over messages (messages.go:134-142), and this fixture seeds a channel, never a message — so 0 is the correct head, and asserting it nonzero would red-bar on a value the fixture cannot produce.

Asserted == 0 instead, and the code says plainly what that is and is not: the getter returns 0 for an unset field and the true head is also 0, so a server that dropped the SnapshotSeq assignment entirely would still pass. This is a no-drift check, not presence. Presence is pinned where it can be — subscribe_test.go:243-244, against a populated store. Seeding a message here to force a nonzero head would publish a MessagePosted onto the bus and reorder frame 2, complicating the registration gate this test exists for.

Verification

Against a live postgresql-17.10:

state	result
before (main)	FAIL at the id compare
after	ok ./server 3.743s
boundary send deleted	FAIL — frame 1 carries payload *SubscribeCommsResponse_ChannelChanged, want the payload-less snapshot boundary
boundary sent at Seq: 42	FAIL — snapshot boundary seq = 42, want 0 (control frame, not positioned)
epoch passed as 0 not sub.Epoch	FAIL — snapshot boundary instance_epoch = 0, want the per-boot nonce
epoch stamped instanceEpoch + 1 — nonzero but wrong	FAIL — seeded event instance_epoch = …, want the boundary's … (only the cross-frame equality catches this)

Each mutation reddens with its own message, so the assertions are independently load-bearing rather than one check carrying the rest. The first is what makes this added coverage rather than a swallowed frame; the second catches a defect class the old test could not see at all.

Refs SEA-1423

Spec-impact: none — the boundary frame's shape and ordering are already ratified (SEA-1333 OQ4) and already what subscribe.go:72-80 does; this PR pins that behavior in a test rather than changing it. No observable behavior moves.

---

Ported from sealedsecurity/sealed#973, unchanged in content — Compass code now lives here, so this is where it ships. The sealed PR is closed in favour of this one.

Verification (this repo has no CI yet, so a local run is the only evidence there is): go build ./..., go vet -tags "pgtest unix" ./..., gofmt -l clean, go test -race ./internal/... ./server/..., and the pgtest suites for ./internal/store/, ./internal/comms/, ./server/ against a live postgres:17 — all green on this branch.

[linear-code]

SEA-1423

[greptile-apps]

Greptile Summary

Updates the live-Postgres shutdown regression test to:

• Assert the since_seq=0 snapshot-boundary frame before reading the seeded event.
• Validate boundary sequence, payload, snapshot head, and instance epoch.
• Compare the boundary and event epochs.
• Ensure the server context is canceled on early test failure.

Confidence Score: 5/5

The PR appears safe to merge.

The change is confined to test assertions and cleanup, correctly accounting for the unconditional snapshot boundary while retaining the seeded-event and clean-shutdown checks.

Important Files Changed

Filename	Overview
go/server/serve_pgtest_test.go	The revised test matches the stream’s boundary-first contract and preserves the existing shutdown-drain assertions without introducing an actionable defect.

_{Reviews (1): Last reviewed commit: "fix(compass): SEA-1423 assert the snapsh..." | Re-trigger Greptile}

[seal-agent]

Cross-branch integration verified locally. This repo has no CI yet, so this is the only integration evidence there is — and it is the check that caught a real defect in the sealed copies of these same diffs (a deleted symbol still cited by a sibling branch, invisible to any single-diff review).

Merged all five of this lane's branches together onto main — #29, #30, #31, #32, #33 — in one tree:

merge boundary rc=0   merge drop rc=0   merge ask rc=0   merge t3 rc=0   merge schema rc=0

All five merge clean, no conflicts. On the combined tree, against a live postgres:17:

gofmt -l           clean
go build ./...     BUILD_OK
go vet -tags…      VET_OK
go test -race ./internal/... ./server/...        all pass
go test -tags pgtest ./internal/store/...        ok  39.358s
go test -tags pgtest ./internal/comms/...        ok  23.879s
go test -tags pgtest ./server/...                ok  20.286s

Worth stating plainly: ./server/ only goes green here because #29 is in the tree. On main today TestServeShutdownWithLiveCommsSubscriberReturnsClean fails against a live Postgres — pre-existing, confirmed on a clean main checkout — and #29 is the fix. The other four each show that same failure alone, and it is not theirs.

A per-package throwaway Postgres container also starves under this suite's parallelism (the store package hit its 600s timeout four times over); one shared database runs the same suite in ~30s. Relevant when CI is set up here.

[seal-agent]

Closing as a duplicate of #21, which is the better fix and got here first.

I ported this from sealed#973 without checking for an existing compass PR on the same test — my mistake. #21 covers the same single file and the same TestServeShutdownWithLiveCommsSubscriberReturnsClean gate, and does strictly more:

• it asserts the boundary on the same discriminators (seq == 0, nil payload);
• it adds a defer cancel() fixing a Serve goroutine leak on the failure paths — and explicitly credits comparing against sealed#973, so my lane's contribution is already folded in;
• it verifies its own claim by deleting the drain's commsBus.Close() and confirming the test then fails at the drain assertion rather than the gate. That's the mutation check that proves the test defends what its comment says.

Deleting the branch. SEA-1423 is already closed as a duplicate of SEA-1530, which tracks #21 — so no tracker change needed either.