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 — order the one durable effect against Close, and narrow the promise

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.

Narrowed to what holds. One effect is not self-limiting and is now ordered: a
checkpoint save. Cancellation cannot stop it — a CheckpointStore may ignore
ctx, and the built-in one documents that it does, so the cancelled context
Close publishes is precisely the signal that store is specified to ignore. A
A durableGate narrows it. The failure is sequential, not adversarial —
Close, then a second connector over the same store, then a late save landing on
a lineage the new run already loaded.

Stated precisely, because an earlier version of this paragraph claimed more than
the gate delivers: the guarantee is that no save commences after Close
returns, where commencing means claiming the gate — an act that is atomic
against Close and takes no host code with it. Close does not wait, and must
not: holding the lock across CheckpointStore.Save self-deadlocks a store whose
Save calls Close (documented callable from anywhere), and a store that merely
stalls would block every Close for as long as it stalls.

So a save that claimed the gate and was then descheduled can still land after a
replacement connector loaded — the window is narrowed from [decision, write] to
[claim, write], not closed. That residual moves a checkpoint backward against
§23's "checkpoints only move forward"; its cost is bounded replay, not skipped
events. Closing it needs Close to wait (rejected above) or a fencing token on
CheckpointStore — a six-SDK contract change. Tracked in #784.

Released before the observer callbacks, because Close-from-a-callback is
documented as supported and holding the gate across host code deadlocks
Observer.Checkpoint against the very save that fired it — verified by mutation.

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".

[Copilot]

Pull request overview

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

Suppressed comments (5)

go/pkg/basecamp/eventfeed/scenario_fixture_test.go:737

• Decoder.More is not a top-level EOF check: trailing ] or } makes it return false, so malformed fixture input is accepted despite this helper's strict-decoding contract. Check the unconsumed bytes (or require a second decode to return io.EOF) instead.

	if dec.More() {
		return fmt.Errorf("trailing JSON content")

go/pkg/basecamp/eventfeed/connector.go:535

• The restriction is broader than consumer callbacks. TicketMinter and CheckpointStore.Load/Save are also invoked synchronously on the run goroutine, so calling Wait from one of those implementations waits for itself; consequently “safe from anywhere ELSE” is incorrect and can lead custom seam authors into a deadlock.

// It is not callable from a consumer callback. Every callback — an observer, a
// signal handler, the loop body — runs ON the run goroutine, so waiting for
// that goroutine from inside one waits for itself. Close is the call that is
// safe from anywhere; this is the one that is safe from anywhere ELSE.

SPEC.md:2518

• This constraint must cover all code running synchronously on the connector's execution context, not only consumer callbacks. For example, a custom checkpoint store's Save runs there too and self-deadlocks if it calls wait(); the cross-language contract should state the broader restriction.

  `wait()` is not callable from a consumer callback, for the reason `close()` does not wait.

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

• This header contradicts both the implementation and the new README disclosure: outbound frames (and saves outside the delivery witness) are queued and can satisfy a later expect step, so they are not arrival-strict. Please describe the tracked gap accurately here rather than claiming the driver follows the rule exactly.

// Strictness follows the family README's per-action-class rules exactly.
// Checkpoint saves and outbound frames are ARRIVAL-STRICT: a save carries the

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

• The PR description still says a durableGate prevents saves from commencing after Close, but this implementation deliberately removed that gate, permits an accepted save after Close, and uses Wait as the quiescence boundary. Please update the description (and the #784 status it cites) so reviewers and consumers are evaluating the lifecycle contract this diff actually implements.

	// A position that reached here was ACCEPTED, and its events were delivered
	// to the consumer before it was. It is written unconditionally, including
	// after Close: the store is what tells the next run where those deliveries
	// stopped, and dropping the write because the consumer closed a moment
	// earlier silently re-delivers them. Ordering a second connector against

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2cb78a250

ℹ️ 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 22 out of 22 changed files in this pull request and generated no new comments.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9fe0a8798a

ℹ️ 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 22 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• A non-staleness outcome deferred late in the current staleness window does not receive a fresh grace-phase wake. When the pre-existing timer fires, awaitSupersededPoll treats evaluate(...) == ok as the verdict and returns immediately, even if this fixed deadline is still almost a full window away. That contradicts the documented full grace phase measured from deferral and can abandon a poll prematurely. Arm graceWake for every deferred socket outcome (not only an already-fired staleness outcome), so the wake is fixed at this new deadline and cannot be inherited from the older detection window.

		deadline := l.cfg.clock.Now().Add(l.cfg.staleAfter)
		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.
			at.lc.stale.graceWake()
		}

[Copilot]

Pull request overview

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

Suppressed comments (3)

go/pkg/basecamp/eventfeed/connector.go:495

• The PR description still says durableGate prevents a checkpoint save from commencing after Close and frames #784 as the remaining post-claim race. This implementation intentionally removes that guarantee: a pre-close decision may start writing after Close, and Wait is now the ordering primitive. Please update the Blocker 6 / #784 narrative so reviewers and release notes describe the lifecycle contract actually shipped here.

// just before Close can still be written just after. That is intended. The

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

• A schema-valid expectPoll may contain both url and query, but this early return silently skips all query/filter assertions whenever url is present. Such a fixture can therefore pass even if the connector supplies the wrong filters while following the correct continuation URL. Validate the URL and then continue into query validation when both pins are supplied.

	if step.URL != "" {
		if call.cursor.PageURL != step.URL {
			return fmt.Errorf("the poll targeted %+v, want the URL followed verbatim: %s", call.cursor, step.URL)
		}
		return nil

go/README.md:688

• This lifecycle summary omits the new quiescence requirement. Close can return while a detached checkpoint save is still pending, so a consumer that immediately opens a replacement over the same store can race that save. Document that callers must await iteration termination or call Wait before replacement; otherwise “the feed is resumable” suggests Close alone provides that ordering.

`Close` stops the feed without draining, and cancelling the context, calling `Close`, or
breaking out of the loop all end iteration with **no** error element — a clean stop, and
the feed is resumable by design.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 049cfc5ca2

ℹ️ 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".

[chatgpt-codex-connector]

Honor caller cancellation before publishing a terminal

When the caller cancels ctx after a terminal outcome is selected but before emitTerminal runs—for example, while a subscription-rejection teardown is waiting for the socket close—claimTerminal checks only the connector's isClosed latch, so this still yields the terminal error. Caller cancellation is documented as the same clean, no-error universal edge as Close; fold cancellation into the terminal-publication ordering rather than allowing an already-canceled run to emit an error.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Validate origin UTF-8 before canonicalizing it

When an origin contains invalid UTF-8 in its hostname, url.Parse accepts the bytes and CanonicalOrigin passes them through strings.ToLower, which replaces each invalid byte with U+FFFD; validating only the resulting canonical string therefore succeeds. Distinct inputs such as hosts containing \xff and \xfe collapse to the same canonical origin and consequently the same checkpoint identity, allowing separate configured origins to load and overwrite one lineage; validate cfg.origin before this lossy canonicalization.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Check the deadline before accepting a rejection

When a matching reject_subscription and the confirmation deadline are both ready, the frame can win the select and this teardown ignores the deadline timer's failed Stop, producing the permanent subscription_rejected terminal instead of transition 14's teardown and reconnect. Apply the same expired-timer ordering check used by the neighboring confirm_subscription branch so a rejection processed after its confirmation window cannot terminate the feed.

Useful? React with 👍 / 👎.