Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import {
afterEach,
beforeEach,
describe,
expect,
jest,
setSystemTime,
test,
} from "bun:test";
import type { Terminal as XTerm } from "@xterm/xterm";
import { connect, createTransport } from "./terminal-ws-transport";

Expand Down Expand Up @@ -90,6 +98,8 @@ beforeEach(() => {

afterEach(() => {
globalThis.WebSocket = originalWebSocket;
setSystemTime();
jest.useRealTimers();
});

describe("terminal-ws-transport", () => {
Expand Down Expand Up @@ -156,4 +166,28 @@ describe("terminal-ws-transport", () => {
{ type: "input", data: "b" },
]);
});

test("recovers a half-open socket after the machine resumes from sleep", () => {
jest.useFakeTimers();
setSystemTime(new Date("2026-01-01T00:00:00Z"));

const transport = createTransport();
connect(transport, createMockTerminal(), "ws://host/terminal/t1");

const socket = MockWebSocket.instances[0];
if (!socket) throw new Error("expected websocket instance");
socket.open();
socket.message(JSON.stringify({ type: "attached", terminalId: "t1" }));
expect(transport.connectionState).toBe("open");

// Laptop sleeps: the socket dies but never observes it. readyState stays
// OPEN and no `close` is delivered — that silent death is the bug. Two
// minutes pass (clock jumps), then the watchdog tick runs on wake.
setSystemTime(new Date("2026-01-01T00:02:00Z"));
jest.advanceTimersByTime(120_000);

// Recovery: the wall-clock-gap watchdog drops the wedged socket and dials
// a fresh one. Without it, only the original socket would ever exist.
expect(MockWebSocket.instances.length).toBe(2);
});
});
104 changes: 104 additions & 0 deletions apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ export interface TerminalTransport {
* with no output still gets replay on the next connect.
*/
_hasReceivedBytes: boolean;
/** Internal: wall-clock-gap watchdog for laptop sleep/wake detection. */
_livenessTimer: ReturnType<typeof setInterval> | null;
/** Internal: Date.now() at the last watchdog tick. */
_lastLivenessTick: number;
/** Internal: bound resume handler shared by the online/focus/visibility
* listeners, so they can be removed on teardown. */
_resumeListener: (() => void) | null;
}

const MAX_LOG_ENTRIES = 200;
Expand Down Expand Up @@ -153,9 +160,103 @@ export function createTransport(): TerminalTransport {
_terminal: null,
_hasReceivedBytes: false,
_terminated: false,
_livenessTimer: null,
_lastLivenessTick: 0,
_resumeListener: null,
};
}

// Wall-clock watchdog cadence and the gap that counts as a suspend. A tick gap
// far larger than the interval means the process was paused (laptop sleep), so
// any socket still reporting OPEN is almost certainly half-open — dead, but
// without a `close` event ever firing. This is the dependable desktop signal:
// app-suspend doesn't reliably fire focus/visibility when the window was
// focused both before and after sleep.
const LIVENESS_CHECK_INTERVAL_MS = 5_000;
const LIVENESS_SUSPEND_GAP_MS = 20_000;

// Drop the current socket and immediately reconnect, without waiting for a
// `close` event that a half-open socket will never deliver. The host keeps the
// PTY alive, so this just re-attaches (and replays anything missed).
function reconnectNow(transport: TerminalTransport) {
if (transport._terminated) return;
if (!transport.currentUrl || !transport._terminal) return;
cancelReconnect(transport);
if (transport.socket) {
const dead = transport.socket;
transport.socket = null;
try {
dead.close();
} catch {
// best-effort; the close handler is a no-op once socket is detached
}
}
transport._reconnectAttempt = 0;
// connect() is idempotent while "open"/"connecting"; force "closed" so it
// actually re-dials the now-detached socket.
setConnectionState(transport, "closed");
connect(transport, transport._terminal, transport.currentUrl);
}

// DOM resume signal (online/focus/visibilitychange). Reset backoff and
// reconnect only if the socket is actually dead — a healthy or still-connecting
// socket is left alone. Mirrors TerminalConnection.handleResume on web.
function handleResume(transport: TerminalTransport) {
if (transport._terminated) return;
if (!transport.currentUrl || !transport._terminal) return;
transport._reconnectAttempt = 0;
const socket = transport.socket;
if (
socket &&
(socket.readyState === WebSocket.OPEN ||
socket.readyState === WebSocket.CONNECTING)
) {
return;
}
reconnectNow(transport);
Comment on lines +208 to +216

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

}

function setupLiveness(transport: TerminalTransport) {
if (transport._livenessTimer === null) {
transport._lastLivenessTick = Date.now();
transport._livenessTimer = setInterval(() => {
const now = Date.now();
const gap = now - transport._lastLivenessTick;
transport._lastLivenessTick = now;
if (gap > LIVENESS_SUSPEND_GAP_MS) reconnectNow(transport);
}, LIVENESS_CHECK_INTERVAL_MS);
}
if (!transport._resumeListener) {
const listener = () => handleResume(transport);
transport._resumeListener = listener;
if (typeof window !== "undefined") {
window.addEventListener("online", listener);
window.addEventListener("focus", listener);
}
if (typeof document !== "undefined") {
document.addEventListener("visibilitychange", listener);
}
}
}

function teardownLiveness(transport: TerminalTransport) {
if (transport._livenessTimer !== null) {
clearInterval(transport._livenessTimer);
transport._livenessTimer = null;
}
const listener = transport._resumeListener;
if (listener) {
if (typeof window !== "undefined") {
window.removeEventListener("online", listener);
window.removeEventListener("focus", listener);
}
if (typeof document !== "undefined") {
document.removeEventListener("visibilitychange", listener);
}
transport._resumeListener = null;
}
}

function scheduleReconnect(transport: TerminalTransport) {
if (transport._reconnectTimer) return;
if (transport._terminated) return;
Expand Down Expand Up @@ -235,6 +336,7 @@ export function connect(
transport.currentUrl = wsUrl;
transport._terminal = terminal;
transport._terminated = false;
setupLiveness(transport);
setConnectionState(transport, "connecting");
const actualUrl = transport._hasReceivedBytes
? appendQueryParam(wsUrl, "replay", "0")
Expand Down Expand Up @@ -391,6 +493,7 @@ function attachSocketListeners(

export function disconnect(transport: TerminalTransport) {
cancelReconnect(transport);
teardownLiveness(transport);
if (transport.socket) {
transport.socket.close();
transport.socket = null;
Expand Down Expand Up @@ -430,6 +533,7 @@ export function sendDispose(transport: TerminalTransport) {

export function disposeTransport(transport: TerminalTransport) {
cancelReconnect(transport);
teardownLiveness(transport);
if (transport.socket) {
transport.socket.close();
transport.socket = null;
Expand Down
Loading