feat: mobile lifecycle handling for robust session persistence#724
Open
leighstillard wants to merge 3 commits intositeboon:mainfrom
Open
feat: mobile lifecycle handling for robust session persistence#724leighstillard wants to merge 3 commits intositeboon:mainfrom
leighstillard wants to merge 3 commits intositeboon:mainfrom
Conversation
On mobile browsers (Android/iOS), backgrounding the app kills WebSocket connections silently. Previously, sessions would appear frozen when returning to the app, requiring a manual refresh. This adds: - Page Visibility API detection (useAppLifecycle hook) for immediate reconnect when the app returns to foreground - Server-side WebSocket heartbeat (ping every 30s) to detect dead connections promptly instead of waiting for TCP timeout - Client-side application-level ping/pong with stale connection probing after background periods >5s - Shell WebSocket auto-reconnect with server-side output buffer replay (terminal no longer clears on disconnect) - Wake Lock API integration to keep connections alive during brief app switches on mobile when an agent session is actively processing - Reconnect jitter to prevent thundering herd on mass reconnection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds heartbeat-based WebSocket liveness (app-level ping/pong), background-aware probes and reconnection logic, automatic shell WebSocket reconnection with jitter and foreground fast-path, a page-visibility lifecycle hook, and a wake-lock hook used while processing sessions are active. Changes
Sequence DiagramssequenceDiagram
participant Client as Client<br/>(WebSocketContext)
participant Timer as Heartbeat<br/>Timer
participant Server as Server<br/>(WS)
participant App as App<br/>(Visibility)
rect rgba(100, 150, 200, 0.5)
Note over Client,Server: Normal Heartbeat Loop
Timer->>Client: heartbeat interval
Client->>Server: {type:'ping'}
Server->>Client: {type:'pong'}
Client->>Client: update lastPong timestamp
end
rect rgba(200, 100, 100, 0.5)
Note over Client,Server: Stale Connection Detection
Timer->>Client: heartbeat interval
Client->>Client: check lastPong age
alt lastPong too old
Client->>Client: force close & schedule reconnect
end
end
rect rgba(150, 200, 100, 0.5)
Note over App,Client: Foreground Resume Probe
App->>Client: onForeground(backgroundDuration)
Client->>Server: probe {type:'ping'}
Timer->>Client: wait PING_PROBE_TIMEOUT
alt no pong
Client->>Client: trigger reconnect
end
end
sequenceDiagram
participant Shell as Shell Hook<br/>(useShellConnection)
participant WS as WebSocket
participant App as App Lifecycle
participant Timer as Reconnect<br/>Timer
rect rgba(100, 150, 200, 0.5)
Note over Shell,WS: Connection Lost
WS->>Shell: onclose
Shell->>Shell: mark wasConnectedRef=true
Shell->>Timer: schedule reconnect with jitter
end
rect rgba(150, 100, 200, 0.5)
Note over App,Shell: Foreground Fast-Path
App->>Shell: onForeground
Shell->>Timer: cancel pending reconnect
Shell->>WS: immediate reconnect attempt
end
rect rgba(200, 100, 100, 0.5)
Note over Shell,WS: Explicit Disconnect
Shell->>Shell: disconnectFromShell()
Shell->>Timer: cancel pending reconnect
Shell->>Shell: reset wasConnectedRef=false
Shell->>WS: close socket
end
Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
leighstillard
force-pushed
the
feat/mobile-lifecycle-reconnect
branch
from
c8d8925 to
1fcb9a2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/shell/hooks/useShellConnection.ts (1)
169-182:⚠️ Potential issue | 🟠 MajorDon't auto-reconnect intentional closes.
This
onclosepath always flipswasConnectedRefback totrueand arms a retry, sodisconnectFromShell()can still reconnect a few seconds later. It also leaves the existing auto-connect effect at Lines 236-242 free to reconnect immediately, which bypasses the new backoff. This needs an explicitshouldReconnect/reconnectScheduledguard.♻️ One way to gate reconnects
+ const shouldReconnectRef = useRef(true); + const reconnectScheduledRef = useRef(false); + socket.onclose = () => { setIsConnected(false); setIsConnecting(false); connectingRef.current = false; - // Don't clear terminal — server will replay buffered output on reconnect. - // Track that we were connected so foreground resume can auto-reconnect. - wasConnectedRef.current = true; + if (!shouldReconnectRef.current) return; + // Don't clear terminal — server will replay buffered output on reconnect. + // Track that we were connected so foreground resume can auto-reconnect. + wasConnectedRef.current = true; + reconnectScheduledRef.current = true; // Auto-reconnect after delay (with jitter) const jitter = Math.random() * 1000; - reconnectTimeoutRef.current = setTimeout(() => { + reconnectTimeoutRef.current = window.setTimeout(() => { + reconnectScheduledRef.current = false; if (wsRef.current?.readyState === WebSocket.OPEN) return; connectWebSocket(); }, SHELL_RECONNECT_DELAY_MS + jitter); };// Also set `shouldReconnectRef.current = false` before `closeSocket()` in // `disconnectFromShell()`, and have the auto-connect effect return early while // `reconnectScheduledRef.current` is true.Also applies to: 223-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/shell/hooks/useShellConnection.ts` around lines 169 - 182, The onclose handler currently always sets wasConnectedRef.current = true and schedules a reconnect; add a guard using a new shouldReconnectRef and reconnectScheduledRef so intentional closes don't auto-reconnect: in the socket.onclose callback (the block that calls setIsConnected/setIsConnecting/wasConnectedRef and sets reconnectTimeoutRef) only set wasConnectedRef.current = true and schedule reconnect if shouldReconnectRef.current is true, and set reconnectScheduledRef.current = true when you arm reconnectTimeoutRef and clear it when the timeout runs or connection succeeds; in disconnectFromShell(), set shouldReconnectRef.current = false (and clear any reconnectScheduledRef/current timeout) before calling closeSocket(); and update the auto-connect effect to return early while reconnectScheduledRef.current is true so the existing effect does not bypass backoff.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/contexts/WebSocketContext.tsx`:
- Around line 139-153: The cleanup in the token-dependent useEffect incorrectly
sets unmountedRef.current = true on every token change causing connect() to bail
forever; remove setting unmountedRef.current from that effect and instead set it
only in a separate effect that runs on real unmount (e.g., useEffect(() => () =>
{ unmountedRef.current = true }, [])). Keep the existing clearing logic
(clearHeartbeat, clearTimeout, wsRef.current.close) in the token effect cleanup
so reconnection on token refresh still happens, and ensure connect() continues
to check unmountedRef as before.
In `@src/hooks/useWakeLock.ts`:
- Around line 19-25: The requestLock async function can race with cleanup and
other requests: change requestLock so it stores the result of
navigator.wakeLock.request('screen') in a local sentinel variable, then re-check
the released flag after the await and if released immediately call
sentinel.release() and return; assign wakeLockRef.current = sentinel only if
still appropriate; when adding the 'release' listener, have it clear
wakeLockRef.current only if wakeLockRef.current === sentinel to avoid clearing a
newer sentinel; ensure cleanup logic releases the current sentinel
(wakeLockRef.current) if present.
---
Outside diff comments:
In `@src/components/shell/hooks/useShellConnection.ts`:
- Around line 169-182: The onclose handler currently always sets
wasConnectedRef.current = true and schedules a reconnect; add a guard using a
new shouldReconnectRef and reconnectScheduledRef so intentional closes don't
auto-reconnect: in the socket.onclose callback (the block that calls
setIsConnected/setIsConnecting/wasConnectedRef and sets reconnectTimeoutRef)
only set wasConnectedRef.current = true and schedule reconnect if
shouldReconnectRef.current is true, and set reconnectScheduledRef.current = true
when you arm reconnectTimeoutRef and clear it when the timeout runs or
connection succeeds; in disconnectFromShell(), set shouldReconnectRef.current =
false (and clear any reconnectScheduledRef/current timeout) before calling
closeSocket(); and update the auto-connect effect to return early while
reconnectScheduledRef.current is true so the existing effect does not bypass
backoff.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6ec7b3da-bdf5-457f-891d-049ded33d5b9
📒 Files selected for processing (6)
server/index.jssrc/components/app/AppContent.tsxsrc/components/shell/hooks/useShellConnection.tssrc/contexts/WebSocketContext.tsxsrc/hooks/useAppLifecycle.tssrc/hooks/useWakeLock.ts
- Fix unmountedRef in WebSocketContext: token changes no longer mark the provider as unmounted, which was preventing reconnection after login/logout cycles. Unmount flag now only set on true component unmount via a separate effect. - Fix Wake Lock async race: store sentinel in local variable and re-check released flag after await. Guard release listener to only clear ref if it still holds the same sentinel instance. - Fix shell auto-reconnect on intentional disconnect: add shouldReconnectRef guard so disconnectFromShell() prevents the onclose handler from scheduling a reconnect. Guard is reset when connectToShell() is called intentionally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/contexts/WebSocketContext.tsx`:
- Around line 188-196: The stale-probe timeout set via pingProbeTimeoutRef and
PING_PROBE_TIMEOUT_MS is being cleared immediately because startHeartbeat()
calls clearHeartbeat() (which clears pingProbeTimeoutRef), so the 2s probe never
fires; fix by only restarting the heartbeat when no probe is active: after
scheduling the probe (pingProbeTimeoutRef.current = setTimeout(...)), avoid
calling startHeartbeat() unconditionally—instead call startHeartbeat() only when
pingProbeTimeoutRef.current is falsy (or move startHeartbeat() into an else
branch), so the fast stale-check can run and ws.close() can trigger before the
regular heartbeat timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c61a15aa-218c-4eb9-b649-8354b94d8ab5
📒 Files selected for processing (3)
src/components/shell/hooks/useShellConnection.tssrc/contexts/WebSocketContext.tsxsrc/hooks/useWakeLock.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/useWakeLock.ts
- src/components/shell/hooks/useShellConnection.ts
startHeartbeat() calls clearHeartbeat() which was clearing the ping probe timeout before it could fire, defeating the fast 2s stale-check on foreground resume. Now returns early when a probe is pending so it isn't immediately cancelled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Problem
On mobile browsers, backgrounding the app kills WebSocket connections silently. The browser freezes JS execution, so
onclosetimers don't fire until the user returns. This left sessions appearing frozen with no data flowing, requiring a manual page refresh.Solution
New files
src/hooks/useAppLifecycle.ts— Reusable hook wrapping the Page Visibility API andpageshowevent (for Safari bfcache). ProvidesonForeground(callback)andonBackground(callback)with background duration tracking.src/hooks/useWakeLock.ts— Screen Wake Lock API hook that prevents the device from sleeping while an agent session is actively processing. Only activates on mobile. Auto-reacquires on foreground resume.Modified files
server/index.js— WebSocket-level heartbeat (ping/pong every 30s withisAlivetracking) on both chat and shell connections. Application-levelping/pongmessage handler for client-initiated health checks.src/contexts/WebSocketContext.tsx— Foreground-triggered reconnect (immediate if socket is dead, ping-probe if backgrounded >5s). Client heartbeat every 25s, paused during background. Reconnect delay includes random jitter.src/components/shell/hooks/useShellConnection.ts— Auto-reconnect on close (5s + jitter) and on foreground resume. No longer clears terminal on disconnect so server can replay buffered output.src/components/app/AppContent.tsx— WiresuseWakeLockto activate when mobile + sessions are processing.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes