Repository navigation
fix(mobile): recover when half-open sockets omit close events - #11368
Conversation
📝 WalkthroughWalkthroughThe RPC client now prevents overlapping foreground activity probes and clears the in-flight state across completion, failure, and timeout paths. Probe timeouts explicitly invoke socket-closure recovery after closing the active socket, including half-open sockets without a close callback. New tests verify reconnection to an authenticated socket, reset of the reconnect attempt counter, and coalescing of repeated foreground notifications into one probe. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mobile/src/transport/rpc-client.test.ts (1)
722-725: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that reconnect attempts reset after recovery.
The test verifies
connectedbut notclient.getReconnectAttempt(). Add the assertion to cover the stated recovery contract.Suggested assertion
openAndAuthenticate(mockSockets[1]!) expect(client.getState()).toBe('connected') + expect(client.getReconnectAttempt()).toBe(0)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3279c173-925c-4353-8b64-042c38501164
📥 Commits
Reviewing files that changed from the base of the PR and between 4543bb6 and bdae8f0fc27fb06b64d3ccd4547007c77e5193ee.
📒 Files selected for processing (2)
mobile/src/transport/rpc-client.test.tsmobile/src/transport/rpc-client.ts
Greptile SummaryThis PR fixes a false-connected state on iOS where a half-open WebSocket's liveness probe correctly times out and calls
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to the activity-probe timeout path, reuses the existing close/reconnect lifecycle, and cannot trigger on a healthy connection. The recovery logic correctly mirrors the pre-existing connect-timeout pattern. The double probeWs === ws guard ensures the direct handleSocketClosed call is skipped whenever the normal onclose already ran and nulled ws, and the stale-socket check inside handleSocketClosed prevents a second run if a late onclose arrives afterward. The activityProbeInFlight flag is reset in every exit path. Two deterministic regression tests cover the missing-onclose scenario and probe-coalescing behaviour. No unrelated paths are touched. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| mobile/src/transport/rpc-client.ts | Adds activityProbeInFlight guard to prevent concurrent probes, and directly calls handleSocketClosed after probeWs.close() to handle the iOS missing-onclose case — logic mirrors the existing connectTimer recovery path; stale-event fencing via the ws === probeWs double-check is correctly reasoned. |
| mobile/src/transport/rpc-client.test.ts | Adds two focused regression tests: one that suppresses onclose and verifies a single reconnect with late-event fencing, and one that verifies probe coalescing under repeated notifyForeground calls; MockWebSocket gains emitCloseOnClose flag to simulate iOS behaviour. |
Reviews (3): Last reviewed commit: "fix(mobile): coalesce half-open recovery..." | Re-trigger Greptile
|
Addressed the CodeRabbit nit in 17a18b4ad by explicitly asserting that the reconnect attempt counter resets to 0 after the replacement socket authenticates. Re-ran the focused regression, all 2,599 mobile tests, mobile typecheck, and mobile lint; all passed. |
17a18b4 to
d421ca4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35883623-7d80-4eb3-8d41-999f1cb8b023
📥 Commits
Reviewing files that changed from the base of the PR and between 17a18b4ad2e69bb52607836070fe7086252a2877 and d421ca4.
📒 Files selected for processing (2)
mobile/src/transport/rpc-client.test.tsmobile/src/transport/rpc-client.ts
| // Why: React Native can omit onclose for a wedged iOS transport. | ||
| if (probeWs === ws) { | ||
| handleSocketClosed(probeWs, { timedOut: true }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Recover when the current socket is no longer OPEN.
The fallback is nested under the OPEN check. If the silent socket is already CLOSING/CLOSED and omits onclose, this leaves state === 'connected' indefinitely. Keep the stale-socket check, close only when OPEN, then invoke handleSocketClosed for any still-current probe socket. Add a regression for a non-OPEN socket with no close callback.
Proposed fix
- if (probeWs === ws && probeWs.readyState === WebSocket.OPEN) {
+ if (probeWs !== ws) {
+ return
+ }
+ if (probeWs.readyState === WebSocket.OPEN) {
probeWs.close()
- // Why: React Native can omit onclose for a wedged iOS transport.
- if (probeWs === ws) {
- handleSocketClosed(probeWs, { timedOut: true })
- }
+ }
+ // Why: React Native can omit onclose for a wedged iOS transport.
+ if (probeWs === ws) {
+ handleSocketClosed(probeWs, { timedOut: true })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Why: React Native can omit onclose for a wedged iOS transport. | |
| if (probeWs === ws) { | |
| handleSocketClosed(probeWs, { timedOut: true }) | |
| } | |
| if (probeWs !== ws) { | |
| return | |
| } | |
| if (probeWs.readyState === WebSocket.OPEN) { | |
| probeWs.close() | |
| } | |
| // Why: React Native can omit onclose for a wedged iOS transport. | |
| if (probeWs === ws) { | |
| handleSocketClosed(probeWs, { timedOut: true }) | |
| } |
Completes the 0.0.37 release attempted on 2026-08-03 (run 30791649691 failed on the version assertion). Ships the post-0.0.36 transport fixes: relay session recovery when the LAN endpoint is unreachable (#12344, #11368, #11465, #11690) and honest worktree-catalog failure states (#12235) — the released-app defect class verified live tonight. Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
…ai#11368) * fix(mobile): recover half-open RPC sockets * test(mobile): assert reconnect attempt reset * fix(mobile): coalesce half-open recovery probes --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Completes the 0.0.37 release attempted on 2026-08-03 (run 30791649691 failed on the version assertion). Ships the post-0.0.36 transport fixes: relay session recovery when the LAN endpoint is unreachable (stablyai#12344, stablyai#11368, stablyai#11465, stablyai#11690) and honest worktree-catalog failure states (stablyai#12235) — the released-app defect class verified live tonight. Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Summary
Recover the mobile RPC transport when its existing liveness probe detects a half-open WebSocket but React Native does not deliver the socket's
onclosecallback.The underlying MagicSock receive-loop failure remains outside Orca. Public Tailscale reports include the exact closed ReceiveDERP issue and the similar open iOS ReceiveIPv4/DERP issue; neither currently has a linked fix PR. A search of
stablyai/orcafound no exactReceiveDERPissue or PR.Orca already sends an application-level
status.getprobe every 20 seconds and waits 8 seconds for validated inbound activity. Previously, the timeout only calledWebSocket.close(). If iOS kept the transport wedged and omittedonclose, Orca stayed visiblyconnectedwhile RPC traffic was dead.This change mirrors the existing connect-timeout recovery path: after closing the silent socket, it enters the normal close lifecycle only if that socket is still current. The existing lifecycle provides stream replay, delivery-ambiguity handling, bounded backoff, the retry cap/trickle policy, and stale-event fencing. Synchronous or late close events cannot create a second reconnect.
User impact: Orca can move from a false connected state into its existing bounded reconnect flow without requiring the user to restart the app. Manual Force Reconnect, pairing, relay selection, SSH, and folder workspaces are unchanged.
Screenshots
No visual change.
Testing
cd mobile && pnpm lintcd mobile && pnpm typecheckcd mobile && pnpm test— 354 files; 2,599 passed, 2 skippedcd mobile && pnpm exec expo export --platform ios— iOS Hermes bundle exported successfullyonclose, proves the pre-fix client remains falsely connected, and verifies one replacement socket with no reconnect stormgit diff --checkThe mobile workspace has no
buildscript; the Expo iOS export is the available target bundle build used here. A signed native install was not performed.AI Review Report
The same read-only reviewer assessed the baseline design and the frozen final diff. It independently reproduced the architectural gap, verified the binary diff SHA-256 and changed blob IDs, and returned PASS with no P0-P3 findings.
The review checked:
Security Audit
No new input surface, command execution, filesystem path, IPC method, dependency, credential, token, encryption, or authorization behavior is introduced. The patch does not log secrets or endpoints beyond existing redaction. It reuses the authenticated RPC client's existing close/reconnect path and its delivery-unknown marking for frames that may already have reached the host.
Notes
onclosebehavior is covered deterministically with a mocked React Native WebSocket.ReceiveDERPoutage.Contributor