perf(mobile): stop replaying stored read state on every resume - #9090

Wait for live readiness before taking the snapshot

When initialization starts before WebSocket authentication completes, or EOSE takes over 500 ms, RelaySessionNotifier.subscribe returns via its timeout while the REQ may still be held (relay_session.dart lines 382–390). The subsequent HTTP snapshot can therefore finish before the live subscription is installed, and a connection arriving during initialization may skip another resync because _initialSyncSucceeded was set. Any read-state event published in that gap is absent from both the snapshot and this limit: 0 replay, leaving stale unread state until a later resync; require confirmed live readiness or run another snapshot after delayed readiness.

AGENTS.md reference: AGENTS.md:L235-L238

Useful? React with 👍 / 👎.

Backfill after retryable subscription closures

When the relay sends a retryable CLOSED while the socket remains connected—for example a deadline or rate-limit closure—RelaySessionNotifier retries the same filter after backoff (relay_session.dart lines 901–921), while the read-state provider only requests a snapshot on a transition to SessionStatus.connected. With limit: 0, read-state events published during that backoff are not replayed by the retry and are silently missed; trigger a fresh snapshot when this subscription recovers or retain a bounded replay for this recovery path.

AGENTS.md reference: AGENTS.md:L235-L238

Useful? React with 👍 / 👎.

🤖 Blocker: read marks published while this subscription is CLOSED-and-retrying are never received.

RelaySession retries a retryable CLOSED (rate limit, deadline, capacity) on the same socket after backoff (relay_session.dart _handleClosed). Its own comment says the design relies on the retry replaying stored events: "since is not advanced, so events missed while closed aren't skipped." With limit: 0 the retry replays nothing. The manager uses plain subscribe, so it never learns that the stream was interrupted. No snapshot runs until the next session transition to connected.

Scenario: the relay closes this REQ with a rate-limit CLOSED, and the phone's socket stays up. During the backoff, the desktop marks a channel read. The retry registers with LIMIT 0, so that mark never arrives. The phone shows the channel as unread until the next background/resume or reconnect. A closely related window: if subscribe() completes on its 500 ms fallback rather than EOSE, the HTTP snapshot can be read before the relay has registered the REQ.

Fix: follow the existing pattern for limit: 0 subscribers (channel_list_changes.dart _subscribeChannelListNotices). Use subscribeWithStatus, and on retrying → ready, run _fetchAndMerge() (serialized with reinitializeRemote). Please add one journey test: a CLOSED retry with a peer write during the backoff, then the merged state after the retry is ready.

🤖 P2 · Notify readers after the publish-only snapshot retry

After a resume snapshot fails, the new condition lets the scheduled publish retry it even when all local slots are already acknowledged. If that successful retry reads newer peer marks, _mergeEvents() updates and persists the manager, but _publishOnce() never calls _onChanged(). Unchanged local slots send no event, so no relay echo supplies the missing notification.

User/system effect: ReadStateNotifier keeps its copied contexts, leaving unread badges and highlights stale until another notification or its one-minute timer, despite the snapshot having recovered.

Test: With acknowledged local slots, fail the resume snapshot, then let the scheduled publish snapshot return a newer peer mark without outgoing writes. Check that consumers receive the recovered mark immediately.

🤖 Blocker: on cold start, a peer mark published before the held REQ registers is never received.

If initialize() runs before WS AUTH completes, _subscribe holds the REQ and returns on its 500 ms fallback (relay_session.dart _subscribe). The HTTP snapshot does not wait for the socket (relay_session_snapshot.dart), so it can read before the REQ registers. initialize() then sets _initialSyncSucceeded, so the connect that lands mid-initialize skips its resync (reinitializeRemote). When the held REQ's EOSE finally arrives, this callback ignores the ready because nothing set interrupted. On main, the 500-event stored replay covered this window. With limit: 0, nothing covers it.

Scenario: the user cold-launches the phone with slow AUTH. The snapshot reads old state, then the desktop marks a channel read, then AUTH completes and the REQ registers. The phone shows that channel unread until the next resume or a publish that reads the snapshot. The same 500 ms fallback applies to the subscription _resyncRemote opens if EOSE is slow, but that window is much smaller.

Fix: also catch up when the first ready arrives after subscribeWithStatus has returned. That is the case where the call fell back to its timeout instead of reaching EOSE. Normal starts get their EOSE inside the await, so they keep a single snapshot. For example, set a returned flag after the await and track seenReady in the callback, then call _catchUpAfterRetry() when interrupted || (returned && !seenReady). Please add one journey test where the fake withholds ready until after initialize() finishes, a peer mark lands, and the late ready merges it. The current ordering test's fake returns immediately, so it can't show relay registration.

🤖 Blocker: every plain reconnect now reads two full snapshots instead of one.

On reconnect, _handleConnected sets state = connected before _replayLiveSubscriptions. The provider's listener runs synchronously, so reinitializeRemote → _resyncRemote has already opened the replacement subscription (and sent its REQ) when the replay collects _liveSubscriptions and sends that same REQ again. The new socket sees REQ l-2 twice:

1. The first EOSE fires ready with returned == false, then resolves subscribe.
2. The second EOSE fires ready with returned == true and the current _liveGeneration. _catchUpAfterLateReady waits for _runningResync, the generation still matches, and it reads the whole snapshot again.

Repro: real RelaySessionNotifier with a controlled socket, real ReadStateManager, and the provider's connected listener. One socket reconnect logs REQs on new socket: [l-2, l-2]. Snapshots per reconnect: 1 on base, 2 on this head. On the 28-slot account in the description that's roughly an extra 1.4 MB per reconnect, which is the cost this PR sets out to remove. The PR's tests miss it because _FakeRelaySession.subscribeWithStatus never emits ready before returning.

Suggested fix: catch up only on the first ready after the subscription was interrupted, not on every ready. For example:

var live = false;
onStatusChanged: (status) {
  if (status == RelaySubscriptionStatus.retrying) {
    live = false;
    return;
  }
  final wasLive = live;
  live = true;
  if (returned && !wasLive) unawaited(_catchUpAfterLateReady(generation));
},

With that change the repro reads one snapshot per reconnect, and the read-state suite passes, including the slow cold-start and CLOSED-retry tests added here.

🤖 Fixed in 29be4ea. The catch-up now runs only when the subscription becomes ready after it was not (a ready after subscribe returned on its fallback, or after a CLOSED retry). A second ready while it is already live, as from the duplicate REQ on reconnect, reads nothing, so each reconnect reads one snapshot. The reconnect test's fake now reports ready before subscribe returns and repeats it; the test fails (4 snapshots instead of 3) without the check. The PR text now notes the table counts websocket bytes only and that each resume reads the snapshot once over HTTP.

🤖 Non-blocking: this path is less careful than _resyncRemote, the other snapshot-after-subscribe path:

• If this _fetchAndMerge fails, nothing schedules a retry, so a mark missed during the gap stays missing until the next publish, pause or reconnect. _resyncRemote calls _schedulePublish() after its fetch. Since a failed snapshot now forces the publish to run in full, calling _schedulePublish() here would make that publish the retry. This sits close to your accepted gap, so treat it as optional.
• It doesn't wait for an in-flight publish (_resyncRemote awaits _publishCompleter). A snapshot fetched before that publish lands can merge our own older blob. That costs an extra full snapshot on the next publish and can set syncError transiently. The journal is kept, so nothing is lost.

🤖 Took the first point in 29be4ea: a failed catch-up now calls _schedulePublish(), so that publish retries the snapshot in full. Covered by a failed late-ready read is retried by the next publish. Left the second as is: the journal keeps the mark, and the next publish heals it.

🤖 Blocker: a socket reconnect during the initial snapshot leaves a gap that nothing reads.

live never resets when the socket drops. RelaySession emits no status on disconnect (_handleDisconnected), only retrying on a CLOSED. So the replay's EOSE on a new socket looks exactly like the duplicate-REQ ready this guard ignores.

Ordering:

1. initialize() subscribes and EOSE arrives in time, so live = true. The HTTP snapshot is still in flight (megabytes on a busy account).
2. The socket drops and reconnects. The provider's connected listener calls reinitializeRemote, which waits for initialize. Because _initialSyncSucceeded is set, it returns early (line 313, "it already used this connection"), and that assumption no longer holds.
3. The session replays this REQ with limit: 0, and this callback ignores its ready.
4. The snapshot's fence checks the session context, not the socket (relay_session_snapshot.dart checkContext), so the manager accepts the snapshot it started before the drop.

Another device's mark published while the socket was down is never read. On main the 500-event replay covered it, and at f0cc23bab6 this ready queued the catch-up. With nothing unpublished, the scheduled publish also skips its snapshot. The channel stays unread until the next resume or reconnect.

Repro (throwaway, existing fake): set readyOnSubscribe = true and hold fetchGate. Call initialize() and pump, then call reinitializeRemote(). Fire statusListeners.last(ready) and release the gate. Snapshot reads: 1 at this head, 2 at f0cc23bab6.

Fix: the manager needs to learn that the socket changed. For example, skip the _initialSyncSucceeded shortcut when the connection changed during initialize, or clear live when the socket drops. Add this ordering as a step in a connect during a successful initialize does not sync again rather than a new test.

🤖 Fixed in 4eff9e4. reinitializeRemote now skips a connect during initialize only when the live subscription had not yet reported ready, meaning that connect is the one initialize was waiting for. A connect after the subscription went ready is a reconnect, so it reads the snapshot again. New test a reconnect during the first snapshot syncs again fails with the old skip (red-checked) and passes now. The duplicate-ready test still holds.

🤖 P2 · A queued cold-start pass loses a later reconnect

There is a residual ordering after the reconnect fix: the first connected callback queues this pass while _liveReady is false; EOSE then arrives and the initial HTTP snapshot starts; a socket reconnect during that fetch returns the existing _queuedResync without updating its captured reconnected. After successful initialization this guard skips the new snapshot. The replayed EOSE also skips catch-up because disconnect does not reset _liveReady.

User/system effect: A peer read mark published during the outage, after the initial snapshot's cut, reaches neither that snapshot nor the limit: 0 replay. Unread state stays incorrect until another independent sync despite successful recovery.

Test: Queue the initial connected pass before EOSE, hold the first snapshot, then reconnect and publish a peer mark before releasing it. Recovery should fetch and merge that mark.

[P2] Preserve reconnect demand while a cold-start pass is queued

The queued-future return happens before _liveReady is read, so a second connection cannot upgrade the first pass's captured reconnected = false. Initialization then skips recovery, while the already-ready subscription suppresses catch-up. Could the queued pass retain a sticky reconnect requirement or connection generation?

Posted by Codex (gpt-6.1-sol) on behalf of kylez

🤖 Fixed in 824494224a. The reconnect and generation logic is gone. Initialize, every reconnect and every live-subscription ready now ask one sync loop for a snapshot. A request made before the current pass starts its read joins that read, and a request made while the read is in flight runs one more pass, so a queued cold-start pass can no longer swallow a later reconnect. The cost is an occasional extra snapshot read.

New test a reconnect while the first snapshot downloads is not swallowed by the cold-start connect reconnects while the first read is held and has a peer publish a mark after its cut. It fails against the previous head and passes now.

🤖 P2 · An older catch-up can suppress a newer recovery retry

This generation check runs only before the awaited fetch, and catch-up is not registered in _runningResync. A CLOSED-recovery catch-up can therefore overlap a reconnect snapshot. If the newer snapshot fails and the older catch-up then succeeds, _fetchAndMerge restores _lastSnapshotSucceeded = true. The reconnect's scheduled publish sees no unpublished local changes, skips its retry snapshot and clears the error. The HTTP transport fences community/session changes, but not socket reconnects.

User/system effect: Peer read marks missed during the newer outage remain stale while the manager considers itself synchronized. This loses the retry intended after the failed recovery, beyond the documented temporary HTTP-failure gap.

Test: Hold an old catch-up response, reconnect and fail the new snapshot, then release the old response before the scheduled publish. The publish should still retry the newer recovery.

[P2] Keep older snapshot completion from clearing newer retry debt

The generation check precedes the awaited fetch, and catch-up is not registered as a running resync. An older successful response can therefore follow a newer failure, restore _lastSnapshotSucceeded, and make the scheduled publish skip recovery. Could completion be generation-fenced or these fetches share serialization?

Posted by Codex (gpt-6.1-sol) on behalf of kylez

🤖 Fixed in 824494224a. Snapshot reads are now numbered in start order, and only a read newer than every settled one decides whether the last snapshot succeeded, so an older read finishing late can't clear the retry a newer failed read needs. Catch-up reads also go through the single sync loop now, so they're serialized with initialize and reconnect passes.

New test an older snapshot finishing late keeps a newer failed read's retry holds an older read, fails a newer one, lets the older one succeed, and checks that the next publish still reads a newer peer mark. It fails against the previous head and passes now.

Blocker: every clean reconnect reads the full snapshot twice.

The provider's connected listener runs synchronously inside state = in _handleConnected, before _replayLiveSubscriptions. So this resubscribe sends REQ l-2 on the new socket, and the replay right after it sends l-2 again (marked ambiguous in _sendLiveReq). Both EOSEs emit ready. The first one releases subscribe and the loop starts snapshot A. The second lands while A downloads, sets _snapshotWanted, and the loop reads the whole snapshot again.

Repro: a real RelaySessionNotifier with a recording socket and a gated fake fetchReadStateSnapshot. Call reinitializeRemote(), then debugReplayLiveSubscriptions(), then EOSE each REQ, the second while the read is held.

A separate reviewer probe got the same result through the provider listener. With only this resubscribe removed, it got one REQ ([l-1]) and one read.

That is a full read-state download (discovery plus POST) on every resume, on the accounts this PR is meant to help. The PR text's "Each resume reads the read-state snapshot once over HTTP" no longer holds.

Suggested fix (simplest): let the session own resubscription. It already keeps live subscriptions across reconnects and replays them, and a subscription opened before AUTH is held and sent on connect. Pass onClosed to mark the subscription lost, and resubscribe here only when it is lost (terminal CLOSED, or a failed subscribe). Otherwise the replay's ready already asks the loop for the read. That also removes _resubscribeWanted, the wait for the publish before resubscribing, and the cold start's extra pass, and it stops a terminal CLOSED from dropping the subscription silently. The alternative is to restore "a repeated ready while already live reads nothing", reset on retrying and on each new subscribe. That works, but it keeps the duplicate REQ.

Please cover it with a test through the real session replay. The _FakeRelaySession path can't show the duplicate EOSE.

🤖 Fixed in c620e9d4b5, with your suggested fix. A reconnect no longer resubscribes: the session's replay of the live subscription is the one REQ on the new socket, and its ready asks the sync loop for the one snapshot read. The manager opens a new subscription only after losing one (onClosed on a terminal CLOSED, or a failed subscribe), on the next reconnect. That also removed the resubscribe flag and the wait for an in-flight publish; the sync loop now just opens the subscription when it is missing. No generation or ready-while-live check came back.

New journey test a clean reconnect through the real session replays one REQ and reads the snapshot once, and a lost subscription is reopened drives the real RelaySessionNotifier with a recording socket: reinitializeRemote(), then debugReplayLiveSubscriptions(), then EOSE while the read is held. It fails against 824494224a ([l-2, l-2], two reads) and passes now ([l-1], one read). It also checks that after a terminal CLOSED the next reconnect opens l-2 and reads the snapshot. In that rare lost case the replay can still send the new REQ a second time.

The cold start keeps its occasional extra read when ready lands after the 500 ms fallback.

Non-blocking, wording only. The retry chain here is one publish deep. If that publish's snapshot also fails, _publishOnce's catch records syncError and schedules nothing. A mark another device published while the subscription was down then stays unread here until the next local mark, pause flush, resume or reconnect. That fits the PR's "Accepted gap" (it heals on its own, and syncError reaches no UI), so I'm not asking for backoff. But "A failed read is retried by the next publish" reads as a guarantee. Please say instead that it waits for the next trigger.

Related nit: the publish read at _publishOnce increments _snapshotsStarted but never settles a failure, so the invariant "only the newest settled read decides" has one unrecorded path. I can't build a user-visible failure from it, because a failed publish leaves unpublished state that forces the next full publish anyway. Settling the failure there would keep the accounting honest.

🤖 Both done in c620e9d4b5. The comment now says a failed read waits for the next trigger (the publish it schedules, or a later ready, resume or reconnect), and _publishOnce settles a failed snapshot read too.