fix(ws): take the initial subscribe snapshot from the pool cache under the attach lock

[benitogf]

Closes #152.

What & why

On WS subscribe, the initial snapshot was captured at fetch() time, but the connection was registered later under pool.mutex — after OnSubscribe and the WebSocket upgrade handshake. A storage write whose broadcast ran in that window was neither in the snapshot nor delivered to the still-unregistered conn: the attaching subscriber silently missed it, then applied its next patch on a baseline that skipped an event (corrupting its list/object). attachConn already made snapshot-write + append atomic; the gap was that the snapshot's content and version were fixed before that critical section.

The window exists on every attach, not only pool creation — the conn joins the pool only after the network handshake regardless of where the snapshot came from, so #150 (which serves initialized pools from the cache) did not meaningfully narrow it. #152's text has been scope-corrected accordingly.

The change

Take the initial snapshot from pool.cache inside the same pool.mutex critical section that registers the connection: under the lock, read the cache version, honor the ?v= skip against the live version, else encode the current cache snapshot with that version, then append the conn. The delivered snapshot's version now equals the cache version at the instant the conn joins the pool, so a broadcast landing in the OnSubscribe→upgrade→attach window lands in the cache first and shows up in the snapshot. fetch() shrinks to ensuring the pool is created and initialized from storage; encoding moves under the pool lock for the (infrequent) attach path only — the broadcast path is untouched.

Known residual, pre-existing and tracked in #156: on pool creation, a write racing fetch's storage-read→init sequence can still be clobbered by the unconditional cache init before the snapshot is taken (Broadcast writes uninitialized caches; InitCache* overwrites). It predates this change and needs a reconciliation design, not a mechanical edit.

Reservation — so the pool can't be pruned mid-attach

Deferring the snapshot to attach-time exposed a second race: a pool pruned in the fetch→attach window (last-conn sweep, or a sibling's failed-upgrade prune) would be recreated empty, and the attach would then see Version==0 and skip the snapshot — leaving the subscriber with no baseline. Closed structurally: a pendingAttach counter on the pool, incremented by ReservePool (called first in fetch, atomically with pool creation) and decremented when the conn is appended (or released on any failure path — including a panicking OnSubscribe user callback, via a tightly-scoped recover→release→re-panic in Stream.New; net/http would otherwise swallow the panic with the reservation still held, leaving the pool unprunable forever). The empty-pool sweep in both Close and PruneIfEmpty now also requires pendingAttach == 0, so a pool with an attach in flight is never pruned — attachConn always sees an initialized cache.

Tests

All fault-detecting (each verified failing on the code it guards against), deterministic (bounded GetCacheVersion polls via the OnSubscribe seam, no sleeps), green at -race -count=10:

• TestWsAttachSnapshotIncludesRaceInjectedRecord / …SingleObjectSnapshotIncludesRaceInjectedWrite — inject an event into the fetch→attach window and assert the initial snapshot already contains it (pool-creation variants).
• TestWsSecondAttachSnapshotIncludesRaceInjectedRecord — the live-pool case: subscriber A attached with a baseline record, subscriber B attaches while the write lands in B's window; B's snapshot must contain it AND A must receive it as a patch. Fails at the pre-fix(ws): take the initial subscribe snapshot from the pool cache under the attach lock #154 head.
• TestWsFirstAttachSurvivesPruneRaceInWindow — prunes the pool inside the window; fails without the reservation.
• TestWsOnSubscribePanicReleasesReservation — panicking callback; fails with the recover→release→re-panic guard reverted (probes pool existence via GetCacheVersion, since GetState hides conn-less pools).
• The client list tests, clock/hijack/?v= tests pass unmodified. Full suite green under go test ./... -race -count=1.

Notes

FetchResult.Version (dead after the refactor — only the REST path uses FetchResult) is removed. ServeCache is removed (its only caller was fetch's probe, now a copy-free version check); its empty-glob []-not-null guarantee moved into the attach-path encoder.

🤖 Generated with Claude Code

[CBosch101]

Closes the #152 attach-window snapshot loss cleanly. The initial WS snapshot is now taken from pool.cache inside attachConn under the same pool.mutex that registers the conn, so a broadcast landing in the OnSubscribe→upgrade→attach window is reflected in the delivered snapshot (its version == cache version at registration) instead of being silently dropped. Approve.

Verified

• Reservation accounting is balanced: every ReservePool increment in fetch is matched by exactly one release across all Stream.New exit paths — OnSubscribe error/panic (recover→ReleaseAttach→re-panic), upgrade failure, attachConn encode/write ErrHijacked, and success — no leak, no double-decrement. stream/stream.go:355,466.
• Lock order is sm.mutex→pool.mutex everywhere both are held (ReservePool/ReleaseAttach/PruneIfEmpty/Close); Broadcast/getOrCreatePool never invert it — no deadlock.
• pendingAttach != 0 guard added to both prune predicates (Close, PruneIfEmpty) closes the prune-and-recreate-empty race; because no deletion site removes the pool while pendingAttach>0, attachConn's getOrCreatePool is guaranteed the same reserved *Pool, so the decrement can't hit the wrong instance.
• encodeCacheSnapshot preserves the single-object vs empty-glob-list ([] not null) distinction the removed ServeCache guaranteed; ?v= skip is now evaluated against the live cache version at attach, which is strictly more correct than the old fetch-time version.
• Only two Stream.New callers (ws.go reserved=true, clock.go reserved=false); signature change threaded consistently. No dangling FetchResult.Version / ServeCache references remain. CI green on all three platforms; tests are fault-injecting (each verified failing on the code it guards) and run at -race.

---

🤖 Generated with Claude Code