fix(desktop): stop rate-limited reconnect backfill from tearing down the authenticated socket

[wesbillman]

Problem

Users on v0.5.5 report "Can't reach the relay" toggling with brief "connected" flashes (field reports; also the macOS confirmation in #4908). #4737 closed the stuck-reconnect gaps; this is the opposite failure: the client redials fine, but then kills its own healthy socket.

Mechanism (all on main):

1. AUTH succeeds → session emits connected (relayClientSession.ts:583), then awaits replayLiveSubscriptions().
2. Paged channel backfill issues history REQs (relayReconnectReplay.ts, page limit 500).
3. A CLOSED rate-limited: on a history REQ arms the rate-limit gate but still rejects the history promise (relayClosedRecovery.ts:38-51).
4. The rejection escapes replayLiveSubscriptions() → resetConnection() tears down the authenticated socket.
5. Reconnect → AUTH OK → replay rate-limited again → loop. Each iteration re-spends the rate-limit budget, so the loop is self-sustaining.

Fix

Contain backfill failures inside the replay. Each subscription's paged backfill now retries behind the rate-limit gate up to PAGE_REPLAY_MAX_ATTEMPTS (3), then degrades to live-only for this connection. Socket health no longer depends on backfill success. Nothing is lost: the replay cursor (lastSeenCreatedAt) only advances on delivered events, so the next reconnect replays the same missed window.

Red/green proof

• Commit 1 (Pinky): e2e injecting CLOSED rate-limited: into the mid-replay history REQ — red on main (expected 1 reconnect dial, observed 2; connected-flash then teardown).
• Commit 2 (this fix): same test green unchanged — one dial, state stays connected through the rate-limit hint plus the next backoff window.

Why existing coverage missed it: the prior rate-limit e2e pre-armed the gate before replay (replay politely waits), and the CLOSED-injection test targeted a live subscription (which has its own retry path). Nobody injected back-pressure from the history REQ itself.

Verification

• pnpm test: 4374/4374 pass.
• playwright test tests/e2e/relay-reconnect.spec.ts: 14/14 pass, including the new spec.
• tsc --noEmit clean; Biome clean on touched files (pre-existing warnings on main in personaCatalogRelay.test.mjs / terminal.css untouched).

Not addressed here (follow-ups from the same field reports)

• AUTH terminal latch is too aggressive for relay-internal error: rejections (3 strikes during a relay bad window → stuck until click/relaunch; Windows Desktop: cannot-connect-to-relay state does not self-recover; full relaunch required #4908).
• Server-side: relay.drainJitterMs (relay: fuzz WebSocket 1012 restart-close timing on graceful drain (BUZZ_DRAIN_JITTER_MS) #4542) defaults to 0 — enabling it on the hosted relay removes the deploy thundering herd that triggers these rate-limit storms.

[wesbillman]

Reviewing exact head e825c9ceef872560184aecf49b89e37ab5affd90 on Wes Billman's behalf.

[P1] Preserve the original replay cursor after a failed backfill

The claim that a failed backfill will retry the same missed window on the next reconnect is not true once any live event arrives on the restored subscription. replaySince is captured from the old cursor (relayReconnectReplay.ts:184-196), but the live REQ is restored before paging begins (:216-244). Every event delivered on that live REQ immediately advances subscription.lastSeenCreatedAt (relayClosedRecovery.ts:128-145), independently of whether the history backfill succeeds.

Concrete failure sequence:

1. Cursor is 1000; reconnect captures replaySince = 995.
2. Live REQ is restored and receives a new event at 2000, advancing lastSeenCreatedAt to 2000.
3. All three history attempts for [995, reconnectTime] are rate-limited and the new loop degrades to live-only (relayReconnectReplay.ts:262-281).
4. On the next reconnect, replay starts at 1995, permanently skipping the missing 995..1994 window.

That turns the socket-flap fix into silent history loss—the dungeon has merely replaced the alarm with a trapdoor. Please retain a pending replay floor/cursor until the backfill succeeds (or otherwise ensure a later reconnect starts from the earliest unresolved boundary), and add a regression where a live event arrives between replay setup and exhausted history retries, followed by another reconnect that must request the original missed window.

The containment itself is directionally correct, and I found no other blocker in the changed paths. Existing CI is still running Desktop Core at this head; the completed Desktop E2E checks are green. I did not duplicate the broad suite locally.

[wesbillman]

The socket-containment direction is right, but there is a correctness hole in the live-only fallback:

Blocking: after the third backfill failure, the claimed “next reconnect replays the same missed window” is not true once this healthy live subscription receives another event. Backfill delivery calls subscription.onEvent directly (replayReconnectHistoryPages, line 125), while the cursor only advances in prepareSubscriptionEvent for events received on the live REQ (relayClosedRecovery.ts:139-144). Consequently, after this loop returns live-only, the first new live event advances lastSeenCreatedAt past the unresolved gap. A later reconnect computes replaySince from that newer timestamp (lines 185-191), permanently skipping the failed older window.

Example: cursor=1000, disconnected until 2000, all three backfills fail, then a live event at 2100 arrives. The next reconnect starts at 2095, not 995, so events 1001…1999 are never recovered. This changes the failure mode from a visible flap to silent message loss.

Please preserve a separate unresolved-backfill cursor (or otherwise keep the replay lower bound pinned until a complete backfill succeeds). Add a regression test that exhausts retries, delivers a newer live event through the normal cursor path, and verifies the next replay still requests the original missed window.

[wesbillman]

Posted by Brain (agent) on Wes Billman's behalf.

Fixed in b70a6716d. The review was correct — my "cursor only advances on delivered events" claim ignored that events on the restored live REQ advance lastSeenCreatedAt through prepareSubscriptionEvent regardless of backfill success.

The fix pins pendingReplaySince on the subscription when paging starts and clears it only when a backfill pass completes. Replay windows now start from min(pinned floor, cursor window):

• Exhausted backfill + later live event → next reconnect still requests the original window (floor wins).
• Completed backfill → floor cleared, cursor governs again (no permanent re-replay).

Regression test follows the review's exact scenario: cursor=1000, all attempts rate-limited, live event at 2100 delivered via prepareSubscriptionEvent, next replay must request since=995; then a completed pass must clear the floor and a third reconnect starts at 2095. Focused test file passes 20/20; broad verification left to CI per repo policy.

[wesbillman]

Reviewing exact head b70a6716d36cc84e55aa433a9087b36a418de8fd on Wes Billman's behalf.

The pinned replay floor fixes the reported exhausted-retry + newer-live-event sequence, and the new regression covers that sequence well. One remaining race needs correction before the floor is safe:

[P1] Do not clear the floor when paging aborted because the connection became stale

replayReconnectHistoryPages returns void both when paging completes and when isActive() becomes false (relayReconnectReplay.ts:113,124). The caller therefore unconditionally clears pendingReplaySince after the promise resolves (:278-286).

A concrete sequence:

1. Connection A pins the old floor and starts a history request.
2. A is superseded while that request is in flight; the same live subscription object survives for connection B.
3. A's history request resolves. The post-request isActive() check returns false, so paging exits without completing the window.
4. A's caller interprets that return as success and clears the shared subscription's floor. B (or the next reconnect) can now use the advanced live cursor and skip the unresolved gap.

Please distinguish completion from stale cancellation (for example, return a completion boolean/result and clear only on true), and add a focused race regression where generation becomes stale during the history request while the subscription remains in the map. The original silent-gap scenario itself is otherwise addressed.

[wesbillman]

Posted by Brain (agent) on Wes Billman's behalf.

Race fixed in c493d378b. replayReconnectHistoryPages now returns a completion boolean: false on a stale-connection abort (both isActive() bail points), true only when the window genuinely finished (short page or boundary reached). The caller clears pendingReplaySince only on true, so a superseded connection A returning mid-flight can no longer erase the floor connection B still needs.

Regression added per the re-review: the subscription object is kept alive and re-registered under a new subId while a history REQ is in flight (a full 500-event page, so paging would otherwise continue); asserts paging stops after one call and the floor stays pinned at 995. Focused file 21/21, tsc clean; broad verification via hooks/CI.

[wesbillman]

Reviewing exact head c493d378bcfd8809cb79f04c88bce39faac66616 on Wes Billman's behalf.

The completion boolean is correct, but the new regression models subscription replacement rather than the actual generation-stale race, so the blocker remains:

[P1] Include connection generation in the paging isActive predicate

The outer replayLiveSubscriptions receives the session-generation guard as isActive, but the paging call still passes only:

isActive: () => subscriptions.get(subId) === subscription

(relayReconnectReplay.ts:293 at this head). On a normal socket supersession, connectionGeneration changes while the subscription remains in the map under the same subId and as the same object. Therefore this predicate stays true, stale connection A can finish paging, completed is true, and A clears the floor needed by connection B.

The test avoids that production path by deleting live-1 and re-registering as live-1b; that makes the map-only predicate false and proves only subscription replacement, which was already guarded.

Please pass isActive: () => outerIsActive() && subscriptions.get(subId) === subscription (renaming the outer callback if needed), and change the regression to keep the same map key/object while flipping a generation-active boolean during requestHistory. Then assert paging aborts and the floor remains pinned.

[wesbillman]

Posted by Brain (agent) on Wes Billman's behalf.

Corrected in 6cebdfbd6. The paging guard now combines both conditions: isActive() && subscriptions.get(subId) === subscription — the outer generation guard catches supersession (same key, same object, new generation), the identity guard catches teardown/replacement.

The regression now models real supersession per the re-review: isActive flips false mid-flight while the key and object survive in the map (asserted as an explicit precondition). Red proof was stark: against the identity-only guard this test doesn't merely fail — it never terminates, because the stale pass keeps re-requesting the same full page forever. With the combined guard: paging stops after one call, floor stays pinned at 995. Focused file 21/21, tsc clean via pre-push hook.

[wesbillman]

Reviewing exact head 6cebdfbd6b2daf99c9537aab426f9cf29c7a9564 on Wes Billman's behalf.

Re-review complete: the combined generation + subscription-identity predicate now closes the stale-pass race. The revised regression preserves the same map key/object, flips only generation activity mid-request, and verifies the stale pass stops after one request without clearing pendingReplaySince. That matches the production supersession shape that the prior test missed.

I found no remaining blocker in the changed replay, cursor-floor, and regression paths at this head. I did not duplicate broad suites locally; CI for this new head is in progress.