perf(mobile): faster thread sync, connects, and reconnects on bad wifi#28487

External image Major · Data Integrity & Integration · Heavy lift

Associate snapshot provenance with each stream item.

subscribeDynamic can emit values buffered by the previous session after activeSubscriptionOmission has been updated for the new session. applyItems then records the new omission key for the old snapshot. The current receivedSnapshot flag also treats enrichment snapshots as authoritative.

Carry the originating subscription’s omission key with each item, and update authoritativeOmission only for full snapshots from that subscription. Use subscribeDynamicWithSession or an equivalent mapping. Add a controlled session-switch test for this ordering.

Not changing this in this PR. The race is real, but main already has it, and it can only cause harm in a case that is very rare.

• main uses the same order with a session ref. makeSubscribeInput sets activeSubscriptionSession, and applyItems then copies it into lastAuthoritativeSession for any snapshot. A buffered snapshot from the old session gets the new session's tag there too. This PR only changes the key from the session to the omission options.
• The wrong tag matters only when the omission options differ between the two sessions. Those options come from server config flags (shellThreadFamilies, shellOlderSettled), so that happens only when the server is upgraded between the drop and the reconnect. With the same options, the old snapshot has the same shape, and resuming from its cursor is correct. The server still sends a full snapshot when the gap is too large.
• Enrichment snapshots come on the same subscription, with the same omission options. So recording that key for them is correct.

If you want this closed, the fix is to switch to subscribeDynamicWithSession (as server.ts does) and key each item by its own session. I would do that as a separate PR, because it also changes the code path main already has.

@Bil0000 Understood. I accept deferring the provenance fix to a separate PR.

The supplied diff retains the existing shared-ref tagging pattern. With unchanged omission options, the wrong session tag does not itself create an omission mismatch. My enrichment-snapshot concern was too broad: an enrichment snapshot from the same subscription does not by itself establish that mismatch.

The provenance race remains unresolved, so I’ll leave this finding open as deferred work rather than mark it fixed.

Would you like me to open a GitHub issue for the follow-up?

_{You are interacting with an AI system.}

Fixed in bb012ea8245 instead of deferring: the shell subscription now uses subscribeDynamicWithSession, each session's resume key is recorded when its input is built, and only full (authoritative) snapshots record their own session's key. Enrichment snapshots no longer count. No dedicated test: reproducing it means controlling switchMap's buffering, which would make a fragile test.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

External image Major · Data Integrity & Integration · Heavy lift

Refresh the older-settled boundary on reconnect.

A matching omission key does not make olderSettled replayable. If a recent settled thread is removed while disconnected, replay removes that thread but does not promote the next omitted thread or update olderSettled.counts. The shell can retain the wrong list and count across later reconnects.

Reload trimmed snapshots on reconnect, or extend the resume protocol to refresh the boundary and affected rows. (raw.githubusercontent.com)

Not changing this. Replay already behaves this way on main, and the client already refreshes the counts.

• On main, a foreground resubscribe on the same session already resumes from the cursor with omitOlderSettled set. A live session also never moves the older-settled boundary when a recent settled thread is removed. The live tail sends the same thread.removed event that replay sends. So replay after a reconnect gives the same result as staying connected the whole time.
• The counts do not go stale. olderSettledAtom (olderSettledThreads.ts) builds a key from removals and older rows (olderSettledCountKey). When that key moves away from the snapshot's key, it reads the counts again from the server. Older rows are paged from the server cursor (cut.before), so a thread that sits just past the boundary still shows on the shelf.
• A new boundary still arrives with each full snapshot. The server sends one when the gap is too large, the sequence went backward, or the omission options changed.

@Bil0000 You’re right. I missed the client-side count refresh and shelf pagination paths.

I checked olderSettledThreads.ts:

• olderSettledCountKey includes the latest removal sequence. Replayed removals therefore invalidate the count key.
• When the key differs from the snapshot key, olderSettledAtom requests fresh counts.
• Shelf pagination starts from cut.before. Omitted threads remain available without moving the snapshot boundary.

The unchanged boundary does not establish the list or count defect I claimed. I withdraw this finding. No change is needed for this concern.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Keep a live socket after it delivers a frame.

If the socket delivers a frame during the host check, SocketAlive confirms it is active. If its probe takes 2 seconds, the new 1.5-second deadline still replaces it and interrupts its RPCs. Keep waiting under the normal remote deadline after a frame proves the socket is active. (raw.githubusercontent.com)

✅ Addressed in commits 791c9f1 to 61981f6

Fixed in 791c9f1332. This was a real problem: after a long resume, a socket that proved it was alive with a frame was still replaced at 1.5 s.

When SocketAlive arrives during a long resume, the probe deadline now extends to QUICK_CONNECTION_PROBE_TIMEOUT (3 s, the value before this PR), counted from the start of the probe. A socket with no frame still gets the short 1.5 s deadline. The test "keeps waiting for the probe when a frame arrives during the host check" now also runs for application-active-long (2 s probe). That case fails on the previous code.

@Bil0000 Thanks for the fix. The supplied code confirms that SocketAlive during a long resume extends the probe deadline to three seconds from the probe start. A socket with no frame retains the 1.5-second deadline. This addresses the reported issue.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Functional Correctness · Quick win

Preserve a fallback when the cwd scan fails.

If a cwd scan fails, serverRefreshProviders returns no workspace snapshot. The mobile composer then falls back to these empty machine catalogs, so provider commands and skills disappear even when the machine catalog is available. OpenCode scans can fail or time out. Preserve a fallback catalog on this path, or disable this opt-in until the fallback is available. (raw.githubusercontent.com)

✅ Addressed in commits 791c9f1 to 61981f6

Fixed in 61981f67f3. This was a real problem: when a cwd scan fails, no cwd snapshot exists, and a client that has opted in to providerCatalogsOnDemand then had no commands or skills.

server.refreshProviders now takes providerCatalogsOnDemand too. For those clients, if no cwd snapshot exists, the server replies with the machine catalog as a snapshot marked slashCommandsPending: true. The composer shows that catalog, and the client keeps its usual retry ladder for the scan. The ws.test.ts case "on-demand clients get provider config without workspace snapshots" now requests a cwd that has no snapshot, and that case fails on the previous code.

@Bil0000 The supplied code addresses this finding. When no matching cwd snapshot exists and providerCatalogsOnDemand is enabled, workspaceSnapshotOrCatalog returns the machine commands and skills with slashCommandsPending: true. Existing cwd snapshots still take precedence.

I did not run the regression test.

📝 Learnings were not changed: only organization members or users with write access to this repository can add or remove learnings.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Keep a silence window for remote sockets.

After a long resume, a fresh host check can return before a live WebSocket probe does. If the probe is delayed by packet loss, this branch replaces the socket at about 500 ms instead of letting it recover. Retain a silence window before replacing a remote socket. (raw.githubusercontent.com)

Keeping this as is. Removing the wait after a long resume is the purpose of this change.

• The check returns "silent" only after a fresh request to the host succeeds and the socket delivers no frame for at least 500 ms. If a frame arrives, the result is "alive" (awaitSilentSocket, socketSilent).
• This path runs only on application-active-long. After a long suspension, iOS often kills the socket without a close event, and the data on screen is already stale. The 2.5 s window then only adds delay before the reconnect that is needed anyway.
• If a live socket on a lossy link gets replaced, the cost is one reconnect. The bench (300 ms RTT, 3% stalls) measured resume going from 6.3 s to about 3.5 s with this trade-off.
• Short resumes (application-active-probe) still use REMOTE_SILENT_SOCKET_AFTER, so the normal case keeps its tolerance for packet loss.

@Bil0000 Understood. The shorter wait after application-active-long is an intentional recovery trade-off. The supplied code requires a successful host check and no new inbound frames after the frame-count baseline. Other remote-socket checks retain REMOTE_SILENT_SOCKET_AFTER.

My recommendation would undo that trade-off. I withdraw the finding. No change is needed for this comment.

📝 Learnings were not changed: only organization members or users with write access to this repository can add or remove learnings.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Performance & Scalability · Quick win

Do not mark a provider without cwd scanning as pending.

When a mobile client refreshes a provider whose instance has no snapshotForCwd, the registry returns without scanning. This branch still creates a pending snapshot. The mobile refresh controller then schedules two more refreshes that cannot produce a complete snapshot. Return the machine catalog without a pending marker for that instance. Keep the pending fallback for a failed scan by an instance that can scan. Update the no-scanner test case accordingly. (raw.githubusercontent.com)

As per coding guidelines, “Make sure all changes are considerate of performance impact.”

Source: Coding guidelines

External image Major · Data Integrity & Integration · Heavy lift

Verify event-store identity before resuming across sessions.

If the server at the same endpoint starts with a different database, the omission key can still match. A new database that has reached the saved snapshotSequence can accept afterSequence without sending a full snapshot. The client then retains projects and threads from the old database because the server checks the replay range, not event-store identity. Tie cross-session cursor reuse to a stable event-store identity, or request a full snapshot when identity cannot be established. (raw.githubusercontent.com)

External image Minor · Functional Correctness · Quick win

Preserve a change signal for omitted provider catalogs.

When an eligible provider’s machine commands or skills change, these lines can project both the old and new catalogs to the same empty arrays. The subscription deduper then sends no update. A previously fetched cwd snapshot also keeps the same client snapshotBasis, so the composer can show the old catalog until its five-minute snapshot TTL expires. Send a compact catalog revision with the filtered provider and include it in snapshot invalidation, without sending the full catalog. (raw.githubusercontent.com)