Event feed connector: the run loop and tier-2 driver (2/3)

[jeremy]

This PR has been split and force-pushed. It now carries the state machine
only, and is stacked on #777 (foundations). #778 (conformance corrections)
stacks on this. The pre-split head is preserved at tag pre-split/705-head;
the exact commit this PR pointed at before the force-push is
pre-split/705-remote-head (d379f2e11). All 63 review threads are intact,
but line anchors on foundation files now resolve against #777.

Why: eight bot rounds here did not converge (12→3→5→2→2→1→3 threads, with late
findings in files no earlier round had touched), and a review pass then found a
P1 credential defect all eight missed because it composes two files across a
package boundary. That defect is fixed in #777.

The Go reference implementation of the SPEC.md §23 Event Feed connector — BC3's
account-wide event feed over Action Cable push plus polling catch-up — and the
tier-2 conformance driver. All 22 fixtures pass (23 with #778's addition).

Layer 2 only: the connector reaches the wire through TicketMinter /
PollSource seams and one sanctioned cable dial (AGENTS.md Hard Rule 2). The
Layer-1 adapters are deferred to G1b, so the package is experimental and a
consumer supplies the seams today.

What is here

connector.go (New, the options, Events, Close), loop.go (states,
transitions, timers, live buffer), catchup.go (the poll walk, its page
boundary, the drain), recovery.go (the 400/409/410 matrix), and the tier-2
driver with its fixture model, harness and self-tests.

The foundations — seams, wire types, transports, filters, checkpoint identity,
the file store, dedupe, backoff, clock, cable codec, feedtest/ — are #777.

Rebase note

The 29 commits were re-cut, not rebased. Rebasing onto #777 was attempted
and abandoned: 8 of them touch only foundation files and would replay empty, and
21 of the remaining 22 mix both halves, so every one conflicts against the four
fixes #777 applies. Resolving 22 interleaved conflicts by hand is a worse
guarantee than re-cutting, which is byte-exact by construction — the foundation
paths come from #777's tip, these 17 from the pre-split head, verified with an
empty git diff against both.

The five fixes on top

Blocker 2 — a poll page carrying no position is malformed

The walk took page.Position on trust, and an empty one silently skipped
history in two different ways.

Position-resume: acceptPosition("") sets l.position = "", and
entryCursor selects on l.position != "" — so it does not preserve the old
cursor, it falls through to a bare present entry. The feed resumes at the
server's head with everything between skipped, reporting nothing.

Present-class is worse. held uses "" as its sentinel for "the final entry
was not present-class", so an empty position is not saved as empty — it
collapses into the sentinel, the held != "" guard skips acceptPosition and
saveCheckpoint outright, and caught_up announces anyway. The position is
discarded, with the drain's deliveries already handed to the consumer.

Refused before delivery, counter resets, and every mutation. That placement is
what the mutation check exercises: moving the guard after the delivery loop
fails three of four subtests on delivered ids = [101], want [].

Blocker 6 — narrow the promise, and make Wait the quiescence point

Close's doc claimed "no seam call and no delivery can begin after Close has
returned". That is not true and cannot be made true: the run goroutine checks
its context at each dispatch point and then acts, so a Close landing in between
cannot stop the call from starting — only from starting on a live context.
Closing that window means holding a lock across arbitrary host code, trading a
benign race for a deadlock reachable from any callback.

The one effect that outlives the process is the checkpoint save, and this PR
deliberately does not gate it. An accepted page's position saves even when
Close lands first — its events were already delivered, and dropping the write
would silently re-deliver them — and the save runs under a context detached
from the run's cancellation (context.WithoutCancel: values kept, cancellation
dropped), because a store that honors its ctx is compliant and would
otherwise lose the position Close raced. A save decided just before Close can
land just after it; that is intended, not residual.

Ordering a second connector over the same store is therefore the consumer's
to do, and Wait is the tool: it blocks until the run goroutine has exited, so
no save can be in flight — by construction, not by a narrow window. Await the
iterator's termination, or Wait, before opening a second connector over the
same checkpoint store. (An earlier revision ordered saves through a
durableGate claimed inside Close and tracked the unclosable [claim, write]
residual in #784; the gate is gone, Wait replaces it, and #784 is closed with
it.)

Also: a cancelled checkpoint load is no longer Terminal(checkpoint_load).

Blocker 7 — observers see origins only

#777's redactor applied to Observer.Gap and both CatchUpStarted sites. An
accepted 410 latches the server's resume URL as reconnect state, so the
reconnect announces its walk carrying it — redacting Gap alone would have left
the identical URL leaving through a different callback one reconnect later.

Observer.Disconnected is redacted, and this paragraph used to say the
opposite. The original reasoning — an error is opaque text, and stripping a
credential out of arbitrary text means modelling the credential, which §23's
"opaque bearer" contract forbids — argued for leaving it alone. A later round
found a ticket reaching the callback through the raw seam read error, which no
seam obligation can repair from the connector's side, so both arguments now go
through closed vocabularies: observableDisconnectReason maps every
unrecognized peer reason to "other", and observableSocketError reduces every
cause to one the connector owns, degrading anything unrecognized to
errSocketFailed.

"Reduces" is load-bearing and was the last hole: matching a sentinel is not the
same as being one, so a seam returning fmt.Errorf("read %s: %w", cableURL, context.Canceled) matched a recognized arm while its text carried the ticket.
Every arm now returns the connector's own value rather than its argument;
errors.Is still matches, and the wrapper does not survive.

The seam obligation remains on Dial, ReadFrame and WriteFrame — it is what
keeps a preserved typed error safe — but the connector no longer depends on it
being honored.

#763 — staleness arms at socket open

Observer.Connected fired between the socket opening and the window that
measures silence on it. Whatever a host's callback spent was time the window
never counted. Observed from inside the callback, which is the only place the
ordering is visible.

#760 — an occupied deferral slot no longer blinds the drain's fatal scan

drainScan returned the moment it found the slot occupied, on the reasoning
that everything queued "arrived behind this one". That is a claim about arrival
order, where §23's carve-out is a claim about which verdict governs. With
any non-fatal outcome parked ahead of it the scan looked at nothing — and the
budget it runs under is pumpDepth+1, sized in drain's own comment to reach
every frame the pump had already read. An occupied slot spent none of it.

The planned fix was a bounded deferral queue carved out of pumpDepth; that
turned out to be unnecessary. Only one deferral is ever dispatched —
pumpExited, dispatchDisconnect and the invalid-frame teardown all end the
cycle — so a queue would be a buffer sized for a delivery that cannot happen.
The scan keeps the first outcome and discards the rest as it passes them, which
removes the capacity question entirely: no share of pumpDepth, no channel
resize, no change to the published memory bound, no fixture sweep.

The new subtest needs both halves of the two around it: deferring during the
entry poll occupies the slot, and queuing the fatal frame mid-drain puts it
where only the scan can find it. Queued earlier, the ownership cut consumes it
first — which is how the first draft passed against un-fixed code.

On #758/#759: I could not reproduce the missing-wake-source hang. Every path
leaves a wake. The overshoot is real (close to two staleness windows) and
provably cannot exceed two, so the bound is documented in place rather than
patched, and carried to bc3 as an open §23 contract question.

Verification

Pristine worktree, one pass, clean tree before and after:

• go build / go vet / -race -count=1 / -count=5 — pass
• make go-lint 0 issues; gosec on the CI-pinned v2.23.0 (hash-verified) 0 issues
• full make check — exit 0
• 22/22 fixtures
• TestWalkFailureBetweenPages both subtests — the invariant that killed the
  reviewers' one-liner survives
• staleness soak 8 × 500 under -race: 0 failures, 0 data races, re-earned
  because this PR rewrites the files that wait

Kill-matrix correction

Row 15's claim was inherited from the family README and is wrong; #778
corrects it. Tier 2 cannot prove zero egress to a foreign redirect target,
because the driver is the seam and manufactures the verdict. That is a
Layer-1 property, tracked for G1b.

[Copilot]

Pull request overview

Adds the experimental Go Event Feed reference connector, deterministic conformance infrastructure, WebSocket transport, and checkpoint persistence.

Changes:

• Implements the push/poll state machine, recovery, deduplication, and checkpointing.
• Adds real and fake transports plus tier-2/tier-3 conformance tests.
• Documents the experimental API and architecture exception.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 54 out of 55 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
AGENTS.md	Registers Event Feed infrastructure.
CONTRIBUTING.md	Documents conformance verification.
go/README.md	Adds Event Feed usage guidance.
go/go.mod	Adds WebSocket dependency.
go/go.sum	Locks WebSocket dependency.
eventfeed/backoff.go	Implements retry timing.
eventfeed/backoff_test.go	Tests retry timing.
eventfeed/cable.go	Implements cable framing.
eventfeed/cable_test.go	Tests cable framing.
eventfeed/catchup.go	Implements catch-up and streaming.
eventfeed/catchup_test.go	Tests catch-up behavior.
eventfeed/checkpoint.go	Defines checkpoint contracts.
eventfeed/clock.go	Implements production timers.
eventfeed/clock_test.go	Tests production timers.
eventfeed/connector.go	Defines the public connector.
eventfeed/connector_test.go	Tests connector construction.
eventfeed/continuation.go	Validates continuation URLs.
eventfeed/dedupe.go	Implements delivered-ID deduplication.
eventfeed/dedupe_test.go	Tests deduplication.
eventfeed/digest.go	Implements filter digests.
eventfeed/digest_test.go	Tests shared digest vectors.
eventfeed/doc.go	Documents the package contract.
eventfeed/errors.go	Defines terminal errors.
eventfeed/errors_test.go	Tests error taxonomy.
eventfeed/event.go	Defines feed events.
eventfeed/event_test.go	Tests event decoding.
eventfeed/export_test.go	Exposes test-only hooks.
eventfeed/filestore.go	Implements file checkpoints.
eventfeed/filestore_test.go	Tests file checkpoints.
eventfeed/filters.go	Defines filter validation.
eventfeed/filters_test.go	Tests filter validation.
eventfeed/loop.go	Implements connector lifecycle.
eventfeed/loop_test.go	Tests lifecycle behavior.
eventfeed/reconnect_test.go	Tests reconnect and staleness.
eventfeed/recovery.go	Implements poll recovery.
eventfeed/recovery_test.go	Tests recovery paths.
eventfeed/scenario_conformance_test.go	Replays conformance fixtures.
eventfeed/scenario_fixture_test.go	Decodes fixture contracts.
eventfeed/scenario_harness_test.go	Provides scenario harnessing.
eventfeed/scenario_selftest_test.go	Tests driver strictness.
eventfeed/seams.go	Defines connector seams.
eventfeed/transport.go	Enforces cable URL policy.
eventfeed/transport_contract_test.go	Defines transport contract tests.
eventfeed/transport_test.go	Tests cable URL policy.
eventfeed/websocket_transport.go	Implements WebSocket transport.
eventfeed/websocket_transport_test.go	Tests real WebSocket behavior.
eventfeed/feedtest/clock.go	Adds deterministic virtual time.
eventfeed/feedtest/clock_test.go	Tests virtual time.
eventfeed/feedtest/minter.go	Adds scripted ticket minting.
eventfeed/feedtest/minter_test.go	Tests scripted minting.
eventfeed/feedtest/polls.go	Adds scripted polling.
eventfeed/feedtest/polls_test.go	Tests scripted polling.
eventfeed/feedtest/store.go	Adds scripted checkpoints.
eventfeed/feedtest/transport.go	Adds scripted cable transport.
eventfeed/feedtest/transport_test.go	Tests scripted transport.

Suppressed comments (1)

go/pkg/basecamp/eventfeed/filestore.go:230

• The rename is atomic but not durable without syncing the staged file and parent directory. After a system crash, the first checkpoint file can disappear; the next Load then reports Missing and starts at the present, which can skip history rather than merely replay from an older position. Since Save and the package advertise durable checkpointing, sync the file before rename and the directory after rename, or stop claiming crash durability and avoid treating disappearance as a safe present entry.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ceb8398f4c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 54 out of 55 changed files in this pull request and generated no new comments.

Suppressed comments (4)

go/pkg/basecamp/eventfeed/websocket_transport.go:73

• The offered subprotocol is not verified after the handshake. coder/websocket v1.8.15 accepts a 101 response with an empty Sec-WebSocket-Protocol, so this can return a connection even though actioncable-v1-json was never negotiated, contrary to the CableTransport contract. Check conn.Subprotocol() and reject/close a missing selection; add a server case that intentionally selects none.
  go/pkg/basecamp/eventfeed/catchup.go:302
• This parks an already-observed overflow until the poll returns. SPEC §23 requires semantic signals at the first consumer-context opportunity after the condition arises (with “before the next save” only as the outer bound), so a stalled poll can postpone the handler forever even though this goroutine has received the dropping frame. Dispatch the overflow immediately here; an Accept disposition can continue awaiting the poll, while Terminate should cancel the attempt/poll.

			} else if over {
				// The buffer, not the socket: the call is unaffected and is
				// awaited to completion, and the drop's disposition runs
				// before this page's position moves anything durable.
				l.deferred = &deferredFrame{item: item, overflow: true}
				if l.hooks.frameDeferred != nil {
					l.hooks.frameDeferred(true)
				}
				r := <-done
				return r.page, false, r.err

go/pkg/basecamp/eventfeed/websocket_transport.go:73

• checkCableURL explicitly accepts case-insensitive schemes (the new unit test includes WSS://), but this passes that original spelling to coder/websocket. In v1.8.15 its handshake switch recognizes only lowercase ws/wss, so a URL accepted by policy fails as a transient dial and is retried indefinitely. Normalize only the scheme before dialing, while leaving the ticket-bearing remainder unchanged.

This issue also appears on line 70 of the same file.
go/pkg/basecamp/eventfeed/websocket_transport.go:183

• This synchronous graceful close can block teardown for several seconds: coder/websocket v1.8.15 waits up to 5 seconds to write the close frame and another 5 seconds for the peer response. Because dispose calls this before cancelling the attempt, caller cancellation, Connector.Close, terminal outcomes, and reconnects can all stall on a live peer that ignores the handshake. Use a bounded teardown strategy that preserves the required close frame without letting the library's full handshake timeout delay the universal Closed edge.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 55 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

go/pkg/basecamp/eventfeed/catchup.go:504

• A live event admitted by drainScan is stranded when the buffer was empty at the start of this iteration: batch stays empty, so this returns even though the scan just repopulated l.buffer. Streaming never drains that buffer, delaying the event until a later repair walk and allowing caught_up (and a held save) to happen first. Continue the loop when the scan added events.

		batch := l.buffer.take()
		if out, done := l.drainScan(at, &budget); done {
			return out, true
		}
		if len(batch) == 0 {
			return cycleOutcome{}, false

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6eb58e6010

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jeremy]

Round-2 note on the suppressed comments, since three of them are load-bearing and one changed the design.

catchup.go:302 (overflow parked until the poll returns) — taken, and it reframed the visible finding above it. Rather than add a third dispatch site I deleted the overflow deferral outright; admitDuringPoll now dispatches at the drop's instant. Details in the thread. Fixture 22's own description already called drop-time dispatch normative and its post-poll rendezvous "scheduling slack only", so the implementation now matches what the merged fixture says it should do.

catchup.go:504 (live event stranded by drainScan) — real, and a defect introduced by my own round-1 change: drain returned on len(batch) == 0 even when the scan had just repopulated the buffer, so the event waited for a later repair walk while caught_up and the held save went first. Now gated on l.buffer.empty() too; red proof showed [event 41 save pos-1] where [event 41 event 42 save pos-1] was required.

websocket_transport.go:73 ×2 and :183 — all three taken and in progress: the negotiated subprotocol is never verified after the handshake, a WSS://-spelled URL passes policy but the library's dial switch is lowercase-only (so it fails transient and retries forever), and the graceful close can block teardown on a peer that ignores the handshake. That last one has to keep the close frame that nine fixtures pin — an earlier round caught the opposite bug — so it is getting a bound rather than a reordering.

Thanks for putting the sharp ones in the suppressed block; they've been the most useful part of both rounds.

[jeremy]

Closing out the remaining suppressed comments from round 2 (02fec5f). Two fixed, one declined with evidence.

Unverified subprotocol — real, fixed. Confirmed in the library rather than assumed: coder/websocket@v1.8.15/dial.go:270-283, verifySubprotocol returns nil when Sec-WebSocket-Protocol is absent, and the server side (accept.go, selectSubprotocol) sets no header at all when it selects nothing — so a 101 that negotiated nothing yielded a live connection. A mismatched selection the library does reject, leaving the empty case as the only reachable one, exactly as you said. Now conn.Subprotocol() must match actioncable-v1-json after the handshake; on mismatch the socket is torn down and the dial fails DialPolicy. Policy rather than transient is a deliberate call: a fresh mint returns a URL pointing at the same server, which will select the same nothing, so retrying forever against a server that cannot speak the protocol is the wrong shape — the redirect refusal already lands there for the same structural reason. Test includes the server-selects-none case you asked for.

Unbounded graceful close — real, fixed. close.go:99-128,157-228: Close writes the close frame under a hardcoded 5s context, then waits another 5s for the peer, then waitGoroutines. The phases can't be bounded separately (closeHandshake is unexported, no exported close-frame writer), and CloseNow is not an escape hatch once Close is in flight — casClosing has already flipped, so it just waits too. Bound is 1s, run off-caller: only the write is contractual (§23 needs the peer to see the frame, which is why dispose closes before cancelling — this is a bound, not the reordering an earlier round rejected), and that write is a control frame to an open socket bounded by the kernel send buffer, not by the peer. All 12 fixtures carrying expectClientClose still pass. Red proof: Close blocked past 3s against a peer that never answers; green returns at exactly the 1s budget, so it passes via the timeout path rather than a lucky response.

Case-insensitive scheme — not a defect, declined. net/url.Parse lowercases the scheme before anything downstream sees it ($(go env GOROOT)/src/net/url/url.go:454), and coder/websocket's dial switch runs on u.Scheme from its own url.Parse, so a WSS:// spelling arrives there already wss. Verified empirically, not just by reading: the new test dials a WS://-spelled loopback URL and passed against un-fixed code, with the ticket-bearing remainder byte-identical. No normalization added; the test stays as a regression pin binding checkCableURL's deliberate case-insensitivity to what actually dials.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 55 changed files in this pull request and generated no new comments.

Suppressed comments (3)

go/pkg/basecamp/eventfeed/catchup.go:158

• A socket outcome deferred while this poll was in flight is skipped when the poll itself fails. For terminal poll branches, recoverPoll calls disposeAttempt, which clears l.deferred; for retryable failures, the deferred frame can remain undispatched through arbitrarily many retries. In particular, an invalid_event_stream_command observed during CatchingUp can be replaced by poll_failed/authorization_failed or delayed indefinitely, despite SPEC §23 requiring protocol-fatal to terminate from every socket-open state. Dispatch the already-observed socket outcome here before applying poll recovery; a failed poll has no successful page boundary left to finish.

		if p.err != nil {
			step, out, done := l.recoverPoll(at, cursor, p.err)

go/pkg/basecamp/eventfeed/transport.go:40

• Checking u.Host does not ensure that the URL has a hostname. For example, wss://:443/cable has Host == ":443" but an empty Hostname(), so it passes the policy check and is classified as a transient dial failure instead of terminal invalid_cable_url, causing repeated re-mints/dials for a structurally unusable URL.
  go/pkg/basecamp/eventfeed/cable.go:250
• The tier-2 push-event schema requires all nine keys, including presence-bearing visible_to_clients, and SPEC §23 treats a correlated message missing a required event key as an invalid frame. Omitting this field from the presence checks accepts both an absent value and JSON null, exposing a nil value on a push event instead of taking the socket-failure recovery edge.

		{"creator_id", p.CreatorID != nil},
		{"recording_id", p.RecordingID != nil},

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02fec5ff1f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 54 out of 55 changed files in this pull request and generated no new comments.

Suppressed comments (1)

go/pkg/basecamp/eventfeed/catchup.go:160

• A deferred socket outcome is skipped when the in-flight poll returns an error. recoverPoll may retry, increment authorization failures, or terminate, and disposal then clears l.deferred; this can even swallow an already-observed invalid_event_stream_command instead of producing protocol_fatal. Since no page succeeded, dispatch the deferred outcome before classifying the poll error (the finish-page ordering only applies to successful pages).

		if p.err != nil {
			step, out, done := l.recoverPoll(at, cursor, p.err)
			if done {
				return out, "", true

[jeremy]

Holding two findings open deliberately: the in-flight-poll mechanism has drawn four rounds

Two round-4 findings — the suspendable bound in awaitSupersededPoll (thread-adjacent) and the deferred socket outcome swallowed on the poll-error path (suppressed, catchup.go:160) — are not being fixed in this round. Both are correct. I am not writing the next patch on that mechanism until its shape is settled, because the pattern is now the finding.

The ledger on one mechanism, in order:

1. Round 1 (Codex P1): a stalled PollSource holds the consumer's goroutine when the socket dies → added awaitSupersededPoll, bounding the wait by the staleness window.
2. Round 2 (Copilot + Codex, P1): a deferred overflow dispatches after the save, and is dropped entirely on failed-poll paths → removed the overflow deferral; admitDuringPoll dispatches at the drop's instant.
3. Round 3 (Codex): the fatal-frame scan's budget was sized by the live-buffer capacity, so a fatal could hide behind one ping → rebounded on the pump queue's own depth.
4. Round 4 (Codex, this round): the staleness bound from step 1 is suspendable by the very pump backpressure that creates the problem, so a misbehaving peer can keep a compliant stalled poll alive indefinitely. Plus (suppressed): the socket deferral is cleared by disposal when the poll returns an error, which can swallow an already-observed invalid_event_stream_command instead of producing protocol_fatal.

Each fix was principled and each was verified, but four rounds of edge-findings on one structure is evidence about the structure. What they all orbit: the state machine both awaits a poll seam call and services the frame queue during it, parking one out-of-band frame in a single l.deferred slot, with a bound borrowed from a timer whose evaluation the queue itself can suspend.

The concurrency is not optional — fixtures 01 and 19 require a live frame admitted after confirm but before the entry page is served, and transition 21 requires the in-flight page to be finished before the dying socket is observed. So this is not a mechanism that can simply be deleted, which is precisely why it wants a decision rather than a fifth patch. The candidate shapes:

• (a) Patch in place — give the superseded-poll wait its own deadline from the injected clock (unsuspendable), and dispatch a deferred socket outcome before recoverPoll classifies a poll error, since the finish-page ordering only applies to pages that succeeded. Small, local, and both findings close.
• (b) One event loop — stop blocking on the poll at all: make the poll result just another case in the same select that reads frames, so there is no deferral slot, no superseded wait, and no borrowed bound. Larger, restructures the core of catchup.go, and dissolves this whole class.

I lean (a) plus a note, because the deferral that remains is the one SPEC explicitly sanctions (transition 21's deferred consumption) and (b) risks the delivery-ordering guarantees that nine fixtures and the save-ordering invariant pin. But it is a judgment call about this PR's core, so it goes to a human rather than to me.

The other three round-4 findings (synchronous cancellation on Close, visible_to_clients required on push frames, and the drain batch escaping the live-buffer ceiling) are independent of this and are being fixed now.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9103dd83a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 54 out of 55 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

go/pkg/basecamp/eventfeed/loop.go:152

• ctx is the attempt context, which is a child of runCtx. Connector.Close (or caller cancellation) therefore cancels the pump immediately, before the state machine can reach dispose and call conn.Close. The default WebSocket read may abort the socket on that cancellation, so normal shutdown can still produce the abrupt disconnect that the close-before-cancel ordering is meant to prevent. Give the pump a cancellation scope that is canceled only by disposal after the socket close; HTTP seam calls can continue using the attempt context.
  go/README.md:615
• This says the connector performs no wire I/O, but the package directly performs the sanctioned WebSocket cable dial. That contradicts both the implementation and the architecture rule. Limit the claim to HTTP requests and explicitly name the cable dial exception.

**Experimental: the Layer-1 seam adapters have not landed yet.** The connector performs
no wire I/O of its own — every HTTP exchange reaches the wire through a seam backed by
a generated operation — and the adapters that build those seams over the generated
`CreateStreamTicket` and `PollEvents` operations are still to come. Until they do, a
consumer must supply the `TicketMinter` and `PollSource` implementations itself, and the
exported surface may still change as they land.

go/pkg/basecamp/eventfeed/loop.go:638

• Cancellation is not re-checked after Load. A custom store that honors the supplied context can return context.Canceled when the caller cancels or calls Close, and this path then yields checkpoint_load even though cancellation is documented to end iteration cleanly. Match the mint/poll paths by giving runCtx cancellation precedence over the load error.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93a3d5a457

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jeremy]

@codex review

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

go/pkg/basecamp/eventfeed/scenario_conformance_test.go:730

• This boundary is captured too late. For example, expectPoll advances the pointer and then releases its response; the connector can reach and leave the scripted buffer occupancy before expectPoll returns, after which from excludes that transition and this rendezvous times out despite compliant behavior. Capture the occupancy-history index atomically when the pointer advances onto the expectBuffered step, not when its executor is eventually scheduled.

	from := len(d.h.occupancyHistory)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a466f3896f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jeremy]

@codex review

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.

Suppressed comments (1)

go/pkg/basecamp/eventfeed/scenario_conformance_test.go:1077

• The poll lane has the same status-keyed mapping error: a 429/503 without a parsed header is emitted as PollThrottled with zero. With the new pollRetryDelay branch, that becomes an immediate retry loop instead of transient jitter. SPEC.md:682-687 requires PollThrottled only when a usable parsed Retry-After is present, regardless of status.

		kind := eventfeed.PollTransient
		if status == 429 || status == 503 {
			kind = eventfeed.PollThrottled
		}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 86ec2cd547

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jeremy]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 67e908aa19

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.

[jeremy]

@codex review

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (2)

go/pkg/basecamp/eventfeed/catchup.go:373

• Dispatch an already-parked outcome before scanning the socket again. When the in-flight poll returns a successful page with Next, pollPage may already have stored a protocol-fatal disconnect in l.deferred; socketCheck can then consume the pump's later read error/closed channel and end the cycle as a retryable failure before the next iteration reaches probeFatal. That turns the required Terminal(protocol_fatal) into Backoff and loses the first outcome's ordering.

		if out, done := l.socketCheck(at); done {
			return out, "", true
		}
		cursor = Cursor{PageURL: page.Next}

go/pkg/basecamp/eventfeed/catchup.go:530

• The fixed grace deadline is not actually enforced for ordinary deferred frames/read errors. graceWake is armed only for a deferred staleness verdict, so a frame reset just before deadline can move the next staleness firing almost a full window later; awaitSupersededPoll checks the deadline only after a wake and therefore remains blocked past it. This contradicts the new SPEC.md:3235-3243 requirement that the grace phase be measured from deferral and be immune to frame resets. Please provide a deadline wake for every deferral (without letting it be mistaken for a new staleness verdict) and cover a reset immediately before the deadline.

		if stale {
			// The firing this branch consumed was the wait's own wake source,
			// and an authoritative expiry latches — so without this there is
			// nothing left to wake the grace phase at all, and the bounded wait
			// becomes unbounded again. Armed BEFORE the hook, so a test that
			// rendezvouses on the deferral cannot advance past a wake that is
			// not yet armed. The distance is the remainder to the deadline —
			// here, at the deferral itself, the full window.
			at.lc.stale.graceWake(l.cfg.staleAfter)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 86dd27a4af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jeremy]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93c714ca1f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

go/pkg/basecamp/eventfeed/scenario_conformance_test.go:1031

• The driver attaches the parsed header before classifying the status, so scripted 401/403 (and unrecoverable 404/422) mint errors carry RetryAfter. This contradicts the §23 mapping that only retryable outcomes carry that value and makes tier 2 encode the same unauthorized-backoff bug as the connector. Assign RetryAfter only in the retryable default branch.

	retryAfter, present := d.retryAfterFrom(respond.Headers)
	mintErr := &eventfeed.MintError{RetryAfter: retryAfter, Err: fmt.Errorf("mint responded %d", *respond.Status)}
	switch *respond.Status {
	case 401, 403:
		mintErr.Kind = eventfeed.MintUnauthorized

[jeremy]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 16811f1200

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

go/pkg/basecamp/eventfeed/catchup.go:530

• The fixed grace deadline is only given an immutable wake when the deferred outcome is staleness. For a deferred disconnect/invalid frame, a later frame can reset the ordinary staleness timer near deadline; the rearmed case wakes before the deadline, then the next wake is delayed until frameTime + staleAfter, so the abandoned poll can outlive the one-window grace phase. This contradicts SPEC.md:3235-3243 and #758’s accepted “immune to frame resets” criterion. The existing TestGracePhaseIsImmuneToFrameResets uses staleDeferralHarness, so it cannot exercise this branch. Please add equivalent coverage for a frame-based deferral and provide a fixed deadline wake for that path without allowing socketCheck to replace the parked disconnect reason with staleness.

		if stale {
			// The firing this branch consumed was the wait's own wake source,
			// and an authoritative expiry latches — so without this there is
			// nothing left to wake the grace phase at all, and the bounded wait
			// becomes unbounded again. Armed BEFORE the hook, so a test that
			// rendezvouses on the deferral cannot advance past a wake that is
			// not yet armed. The distance is the remainder to the deadline —
			// here, at the deferral itself, the full window.
			at.lc.stale.graceWake(l.cfg.staleAfter)