Repository navigation
fix(desktop): recover remote terminal after laptop sleep/wake - #5135
Merged
Merged
Conversation
The terminal WebSocket goes half-open on suspend: the TCP connection is dead but readyState stays OPEN and no close event fires. The transport only reconnects from its close handler, so it sat wedged on the dead pipe reporting connectionState "open" and the pane stayed frozen until killed. Add a liveness/wake path to the transport, set up in connect() and torn down in disconnect()/disposeTransport(): - A wall-clock-gap watchdog (setInterval) that recycles a presumed half-open socket when a tick gap shows the process was suspended. This is the dependable desktop signal; app-suspend doesn't reliably fire focus/visibility. - online/focus/visibilitychange listeners that reset backoff and reconnect a dead socket immediately, mirroring the web TerminalConnection. Re-attach is cheap: the host keeps the PTY alive, so reconnecting just re-subscribes and replays. Closes #5130
|
Ready to review this PR? Stage has broken it down into 3 individual chapters for you:
Chapters generated by Stage for commit e352f3d on Jun 5, 2026 3:05am UTC. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTerminalTransport gains a wall-clock watchdog and DOM resume event listeners to detect and recover from half-open WebSockets after laptop sleep/wake. The watchdog measures gaps between timer ticks; a large gap indicates suspension and triggers immediate reconnect. DOM events reset backoff and reconnect if the socket is unhealthy. Setup and teardown are wired into connect, disconnect, and disposeTransport. Tests validate the sleep/wake recovery with system-time mocking. ChangesSleep/Wake Detection and Terminal Reconnection
Sequence DiagramsequenceDiagram
participant App
participant WatchdogInterval as Watchdog<br/>Interval
participant DOMEvents as DOM Events<br/>(online, focus, etc.)
participant TransportLogic as TerminalTransport
participant WebSocket
App->>TransportLogic: connect()
TransportLogic->>TransportLogic: setupLiveness()
TransportLogic->>WatchdogInterval: start interval (check wall-clock gap)
TransportLogic->>DOMEvents: attach listeners
Note over WatchdogInterval: Normal operation: gap < threshold
WatchdogInterval->>TransportLogic: tick (no action)
Note over App: Laptop sleeps → socket half-open, no close event
Note over WatchdogInterval: Gap > SUSPEND_THRESHOLD detected
WatchdogInterval->>TransportLogic: reconnectNow()
TransportLogic->>WebSocket: close dead socket
TransportLogic->>TransportLogic: scheduleReconnect()
Note over App: OR: DOM resume event fires
DOMEvents->>TransportLogic: handleResume()
TransportLogic->>TransportLogic: check if socket is dead
alt Socket not OPEN/CONNECTING
TransportLogic->>TransportLogic: reconnectNow()
end
App->>TransportLogic: disconnect()
TransportLogic->>TransportLogic: teardownLiveness()
TransportLogic->>WatchdogInterval: stop interval
TransportLogic->>DOMEvents: remove listeners
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 docstrings
🧪 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. Comment |
Contributor
Greptile SummaryThis PR adds a sleep/wake recovery mechanism to the desktop remote terminal WebSocket transport, fixing a half-open socket problem where the TCP connection dies silently (no
Confidence Score: 3/5The core watchdog is sound but handleResume can race with the async pre-flight on remote /hosts/ workspaces, producing an orphaned socket on every sleep/wake cycle. The wall-clock-gap watchdog and teardown wiring are solid. handleResume guards against a live socket by inspecting readyState but not connectionState. For /hosts/ remote URLs the watchdog triggers connect() which starts an async pre-flight before creating a WebSocket — during that window transport.socket is null. If online fires, handleResume sees a null socket, skips the early-return, and calls reconnectNow a second time. Both pre-flights race to openSocket, both pass the currentUrl/connectionState guards, and two WebSockets are opened to the same terminal. The first is silently overwritten without being closed, leaking a connection. The test covers only ws://host/terminal/t1 (non-hosts, synchronous open) so this path is not caught. terminal-ws-transport.ts — specifically handleResume and its interaction with the pre-flight path in connect() for /hosts/ remote URLs.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts | Adds liveness watchdog and DOM resume listeners; handleResume lacks a guard for connectionState === connecting, which can create a duplicate WebSocket for remote /hosts/ URLs when an online/focus event fires while a pre-flight HTTP request is still in flight. |
| apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.test.ts | Adds a sleep/wake recovery test using fake timers and fake system time; covers the wall-clock gap path cleanly. Teardown properly restores real timers and system time. |
Sequence Diagram
sequenceDiagram
participant WD as Watchdog (setInterval)
participant RL as ResumeListeners (online/focus/vis)
participant RT as reconnectNow()
participant HR as handleResume()
participant T as Transport
participant WS as WebSocket
Note over T,WS: Normal connected state (socket OPEN)
Note over WD: Laptop sleeps - JS paused, socket goes half-open
Note over WD: Laptop wakes - JS resumes
WD->>WD: "tick fires: gap > 20s"
WD->>RT: reconnectNow()
RT->>T: "socket = null"
RT->>WS: dead.close() best-effort
RT->>T: setConnectionState closed
RT->>T: connect() pre-flight for /hosts/ URLs async
T->>T: "connectionState = connecting, socket = null"
RL->>HR: online event fires on wake
HR->>HR: "socket = transport.socket null"
HR->>HR: null check fails, falls through
HR->>RT: reconnectNow() again - duplicate race
Note over T,WS: Two pre-flights complete, two WebSockets opened, first orphaned
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts:208-216
`handleResume` doesn't guard against `connectionState === "connecting"`, so it calls `reconnectNow` even while a pre-flight HTTP request is already in progress. For remote `/hosts/` URLs, the waketime sequence is: watchdog fires → `reconnectNow` → `connect()` starts async `primeRelayAffinity` → socket is still `null`, state is `"connecting"` → `online` event fires → `handleResume` sees `null` socket → skips the early-return → calls `reconnectNow` again. Both pre-flights resolve and each calls `openSocket()`, passing the `currentUrl`/`connectionState` guards, creating two simultaneous WebSocket connections. The first socket is overwritten in `transport.socket` without being closed, leaving it orphaned on both the client and server.
```suggestion
const socket = transport.socket;
if (
socket &&
(socket.readyState === WebSocket.OPEN ||
socket.readyState === WebSocket.CONNECTING)
) {
return;
}
// Also skip if a connection attempt is already in progress (e.g., a
// pre-flight HTTP request for /hosts/ URLs where transport.socket is null
// while connectionState is still "connecting"). Without this guard, an
// `online` event firing during the watchdog's pre-flight would call
// reconnectNow() a second time, producing two concurrent openSocket()
// closures and an orphaned socket.
if (transport.connectionState === "connecting") return;
reconnectNow(transport);
```
Reviews (1): Last reviewed commit: "fix(desktop): recover remote terminal af..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+208
to
+216
| const socket = transport.socket; | ||
| if ( | ||
| socket && | ||
| (socket.readyState === WebSocket.OPEN || | ||
| socket.readyState === WebSocket.CONNECTING) | ||
| ) { | ||
| return; | ||
| } | ||
| reconnectNow(transport); |
Contributor
There was a problem hiding this comment.
handleResume doesn't guard against connectionState === "connecting", so it calls reconnectNow even while a pre-flight HTTP request is already in progress. For remote /hosts/ URLs, the waketime sequence is: watchdog fires → reconnectNow → connect() starts async primeRelayAffinity → socket is still null, state is "connecting" → online event fires → handleResume sees null socket → skips the early-return → calls reconnectNow again. Both pre-flights resolve and each calls openSocket(), passing the currentUrl/connectionState guards, creating two simultaneous WebSocket connections. The first socket is overwritten in transport.socket without being closed, leaving it orphaned on both the client and server.
Suggested change
| const socket = transport.socket; | |
| if ( | |
| socket && | |
| (socket.readyState === WebSocket.OPEN || | |
| socket.readyState === WebSocket.CONNECTING) | |
| ) { | |
| return; | |
| } | |
| reconnectNow(transport); | |
| const socket = transport.socket; | |
| if ( | |
| socket && | |
| (socket.readyState === WebSocket.OPEN || | |
| socket.readyState === WebSocket.CONNECTING) | |
| ) { | |
| return; | |
| } | |
| // Also skip if a connection attempt is already in progress (e.g., a | |
| // pre-flight HTTP request for /hosts/ URLs where transport.socket is null | |
| // while connectionState is still "connecting"). Without this guard, an | |
| // `online` event firing during the watchdog's pre-flight would call | |
| // reconnectNow() a second time, producing two concurrent openSocket() | |
| // closures and an orphaned socket. | |
| if (transport.connectionState === "connecting") return; | |
| reconnectNow(transport); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts
Line: 208-216
Comment:
`handleResume` doesn't guard against `connectionState === "connecting"`, so it calls `reconnectNow` even while a pre-flight HTTP request is already in progress. For remote `/hosts/` URLs, the waketime sequence is: watchdog fires → `reconnectNow` → `connect()` starts async `primeRelayAffinity` → socket is still `null`, state is `"connecting"` → `online` event fires → `handleResume` sees `null` socket → skips the early-return → calls `reconnectNow` again. Both pre-flights resolve and each calls `openSocket()`, passing the `currentUrl`/`connectionState` guards, creating two simultaneous WebSocket connections. The first socket is overwritten in `transport.socket` without being closed, leaving it orphaned on both the client and server.
```suggestion
const socket = transport.socket;
if (
socket &&
(socket.readyState === WebSocket.OPEN ||
socket.readyState === WebSocket.CONNECTING)
) {
return;
}
// Also skip if a connection attempt is already in progress (e.g., a
// pre-flight HTTP request for /hosts/ URLs where transport.socket is null
// while connectionState is still "connecting"). Without this guard, an
// `online` event firing during the watchdog's pre-flight would call
// reconnectNow() a second time, producing two concurrent openSocket()
// closures and an orphaned socket.
if (transport.connectionState === "connecting") return;
reconnectNow(transport);
```
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts:216">
P2: `handleResume` doesn't guard against `connectionState === "connecting"`, so it calls `reconnectNow` even when a pre-flight connection attempt is already in progress (socket is still `null` during async relay affinity). This creates a race where both the watchdog-initiated and the online-event-initiated `connect()` calls proceed to `openSocket()`, producing two simultaneous WebSocket connections with the first being overwritten without close — leaving an orphaned socket on client and server.
Add a guard before calling `reconnectNow`:
```ts
if (transport.connectionState === "connecting") return;
```</violation>
</file>
Architecture diagram
sequenceDiagram
participant App as Desktop Terminal
participant Transport as TerminalTransport
participant WS as WebSocket
participant Backend as pty-daemon
participant OS as OS
Note over App,Backend: NEW: Liveness detection for sleep/wake recovery
App->>Transport: connect(terminal, url)
Transport->>Transport: setupLiveness() (setInterval 5s, add listeners)
Transport->>WS: new WebSocket(url)
WS-->>Transport: onopen
Transport->>Transport: setConnectionState("open")
Transport->>Transport: attach socket listeners
Note over Transport,Backend: Normal operation…
Note over OS: Laptop sleeps – socket goes half‑open (no close event)
Transport->>Transport: [setInterval tick] compare Date.now()
alt gap > 20s (process was suspended)
Transport->>Transport: reconnectNow()
Transport->>WS: dead.close() (force close old socket)
Transport->>WS: new WebSocket(url)
WS-->>Transport: onopen
Transport->>Backend: re‑subscribe
Backend-->>Transport: replay (if any)
Transport-->>App: terminal resumed
else gap ≤ 20s (normal tick)
Transport-->>Transport: nothing
end
Note over OS: Laptop wakes – resume events fire (online / focus / visibilitychange)
opt Resume event received
Transport->>Transport: handleResume()
Transport->>Transport: reset backoff
alt socket is dead (readyState not OPEN or CONNECTING)
Transport->>Transport: reconnectNow()
Transport->>WS: new WebSocket…
else socket healthy
Transport-->>Transport: no‑op (leave existing connection)
end
end
Note over Transport: Cleanup on explicit disconnect or dispose
App->>Transport: disconnect()
Transport->>Transport: teardownLiveness() (clearInterval, remove listeners)
Transport->>WS: close()
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) { | ||
| return; | ||
| } | ||
| reconnectNow(transport); |
Contributor
There was a problem hiding this comment.
P2: handleResume doesn't guard against connectionState === "connecting", so it calls reconnectNow even when a pre-flight connection attempt is already in progress (socket is still null during async relay affinity). This creates a race where both the watchdog-initiated and the online-event-initiated connect() calls proceed to openSocket(), producing two simultaneous WebSocket connections with the first being overwritten without close — leaving an orphaned socket on client and server.
Add a guard before calling reconnectNow:
if (transport.connectionState === "connecting") return;Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts, line 216:
<comment>`handleResume` doesn't guard against `connectionState === "connecting"`, so it calls `reconnectNow` even when a pre-flight connection attempt is already in progress (socket is still `null` during async relay affinity). This creates a race where both the watchdog-initiated and the online-event-initiated `connect()` calls proceed to `openSocket()`, producing two simultaneous WebSocket connections with the first being overwritten without close — leaving an orphaned socket on client and server.
Add a guard before calling `reconnectNow`:
```ts
if (transport.connectionState === "connecting") return;
```</comment>
<file context>
@@ -153,9 +160,103 @@ export function createTransport(): TerminalTransport {
+ ) {
+ return;
+ }
+ reconnectNow(transport);
+}
+
</file context>
Suggested change
| reconnectNow(transport); | |
| if (transport.connectionState === "connecting") return; | |
| reconnectNow(transport); |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
added a commit
that referenced
this pull request
Jun 5, 2026
handleResume only skipped when transport.socket was OPEN/CONNECTING, but for /hosts/ URLs connect() runs primeRelayAffinity async, leaving transport.socket null while connectionState is "connecting". A resume event (online/focus/visibilitychange) firing in that window saw the null socket, fell through, and called reconnectNow a second time — both pre-flights resolved into openSocket() and the first WebSocket was overwritten without being closed, orphaning it on client and server. Skip when connectionState is "connecting" too. reconnectNow stays unguarded so the watchdog can still force-recycle a half-open "open" socket (or a hung connect). Follow-up to #5135 (flagged in review).
This was referenced Jul 9, 2026
1 of 4 tasks
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
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.
Fixes #5130.
Problem
A desktop workspace terminal on a remote host freezes after the laptop sleeps and wakes — no output, keystrokes do nothing, and the only fix is to kill the pane and open a new one. The PTY itself keeps running in
pty-daemonon the host; the break is client-side.On suspend the terminal WebSocket goes half-open: the TCP connection is dead but
readyStatestaysOPENand nocloseevent ever fires.terminal-ws-transport.tsonly reconnects from itsclosehandler (scheduleReconnect()is reached from a WS construction failure and theclosehandler, nowhere else), so it sat wedged on the dead pipe reportingconnectionState === "open".The web workspace terminal (
apps/web/.../WebTerminal/TerminalConnection.ts) already recovers on resume viavisibilitychange/resume/pageshow/online+ a backoff reset inhandleResume. The desktop transport never got the equivalent — and DOM resume events alone aren't enough on desktop, since app-suspend doesn't reliably firevisibilitychange/focuswhen the window was focused both before and after sleep. The dependable signal is a wall-clock gap.Fix
Add a liveness/wake path to the transport, set up in
connect()and torn down indisconnect()/disposeTransport():setIntervalcomparesDate.now()between ticks; a gap > 20s means the process was suspended, so it force-recycles the (presumed half-open) socket. This catches the dead-but-OPENcase.online/focus/visibilitychangelisteners — reset backoff and reconnect a dead socket immediately (a healthy/connecting one is left alone), mirroringTerminalConnection.handleResumeon web.reconnectNow()drops the dead socket without waiting for acloseand re-dials. Re-attach is cheap: the host keeps the PTY alive, so reconnecting just re-subscribes and the protocol replays anything missed. The existingtransport.socket !== socketguard means a late OS-deliveredcloseon the old socket is harmless.Testing
terminal-ws-transport.test.ts. It drives the real watchdogsetInterval+Date.now()-gap path: fails onmain(Expected: 2, Received: 1— only one socket ever created) and passes with the fix.bun test(3 pass),bun run lint(clean),bun run typecheck(pass).Manual verification (real half-open socket) is a lid-close test: open a remote-host terminal running
top, close the lid ≥1 min, reopen → recovers within ~5s.Summary by cubic
Recover remote desktop terminals after laptop sleep/wake by detecting suspension and force-reconnecting the WebSocket. Fixes frozen panes caused by half-open sockets where output and keystrokes stopped.
online,focus,visibilitychange) to reset backoff and reconnect only if the socket is dead.reconnectNow()and wired liveness setup/teardown inconnect()anddisconnect()/disposeTransport(); added a test that simulates half-open recovery.Written for commit e352f3d. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests