Skip to content
Closed
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
124 changes: 124 additions & 0 deletions apps/desktop/src/main/terminal-host/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,127 @@ describe("Terminal Host Session shell args", () => {
expect(writes.some((message) => message.includes('"hello"'))).toBe(true);
});
});

describe("Terminal Host Session backpressure (#2968)", () => {
/**
* Helper: create a session with a fake socket attached, bypassing the
* spawn/attach lifecycle so we can test broadcastEvent in isolation.
*/
function createSessionWithSocket(socketOverrides: {
write: (message: string) => boolean;
once?: (event: string, listener: () => void) => void;
}) {
const session = new Session({
sessionId: "session-backpressure",
workspaceId: "workspace-1",
paneId: "pane-1",
tabId: "tab-1",
cols: 80,
rows: 24,
cwd: "/tmp",
shell: "/bin/bash",
});

// Directly register the fake socket as an attached client
const socket = socketOverrides as unknown as import("node:net").Socket;
const attachedClients = (
session as unknown as {
attachedClients: Map<
import("node:net").Socket,
{
socket: import("node:net").Socket;
attachedAt: number;
attachToken: symbol;
}
>;
}
).attachedClients;
attachedClients.set(socket, {
socket,
attachedAt: Date.now(),
attachToken: Symbol("test"),
});

const broadcast = (data: string) => {
(
session as unknown as {
broadcastEvent: (
eventType: string,
payload: { type: "data"; data: string },
) => void;
}
).broadcastEvent("data", { type: "data", data });
};

return { session, socket, broadcast };
}

it("stops writing to a backpressured socket instead of growing the buffer", () => {
const writes: string[] = [];
let drainCallback: (() => void) | null = null;

const { broadcast } = createSessionWithSocket({
write(message: string) {
writes.push(message);
// First write succeeds, subsequent ones signal backpressure
return writes.length <= 1;
},
once(event: string, listener: () => void) {
if (event === "drain") drainCallback = listener;
},
});

// First broadcast: write succeeds (returns true)
broadcast("frame-1");
expect(writes).toHaveLength(1);
expect(writes[0]).toContain("frame-1");

// Second broadcast: write returns false → socket becomes backpressured
broadcast("frame-2");
expect(writes).toHaveLength(2);
expect(writes[1]).toContain("frame-2");
expect(drainCallback).not.toBeNull();

// Subsequent broadcasts should be SKIPPED (not written to the socket)
// This is the fix for #2968: previously these would keep writing,
// growing Node's internal buffer without bound.
broadcast("frame-3");
broadcast("frame-4");
broadcast("frame-5");
expect(writes).toHaveLength(2); // No new writes!
});

it("resumes writing after the socket drains", () => {
const writes: string[] = [];
let drainCallback: (() => void) | null = null;

const { broadcast } = createSessionWithSocket({
write(message: string) {
writes.push(message);
// After drain, writes succeed again
return writes.length <= 1 || writes.length > 5;
},
once(event: string, listener: () => void) {
if (event === "drain") drainCallback = listener;
},
});

// Fill up the socket
broadcast("frame-1"); // succeeds
broadcast("frame-2"); // backpressures

// Skipped during backpressure
broadcast("frame-3");
broadcast("frame-4");
expect(writes).toHaveLength(2);

// Simulate drain
expect(drainCallback).not.toBeNull();
drainCallback?.();

// After drain, new broadcasts should write again
broadcast("frame-5");
expect(writes).toHaveLength(3);
expect(writes[2]).toContain("frame-5");
});
});
12 changes: 10 additions & 2 deletions apps/desktop/src/main/terminal-host/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1065,10 +1065,18 @@ export class Session {

for (const { socket } of this.attachedClients.values()) {
try {
// Skip writing to sockets that are already backpressured.
// Continuing to write would grow Node's internal write buffer
// without bound, and when the socket finally drains the massive
// buffer flush causes a visible freeze / catch-up stall (#2968).
// The data is still processed by the emulator, so snapshot state
// stays consistent and the next TUI repaint naturally resyncs.
if (this.clientSocketsWaitingForDrain.has(socket)) {
continue;
}
Comment on lines +1074 to +1076
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P1: The backpressure skip check drops exit/error events as well as data. Restrict skipping to data events so clients still receive terminal lifecycle/error notifications.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/terminal-host/session.ts, line 1074:

<comment>The backpressure skip check drops `exit`/`error` events as well as `data`. Restrict skipping to `data` events so clients still receive terminal lifecycle/error notifications.</comment>

<file context>
@@ -1065,10 +1065,18 @@ export class Session {
+				// buffer flush causes a visible freeze / catch-up stall (#2968).
+				// The data is still processed by the emulator, so snapshot state
+				// stays consistent and the next TUI repaint naturally resyncs.
+				if (this.clientSocketsWaitingForDrain.has(socket)) {
+					continue;
+				}
</file context>
Suggested change
if (this.clientSocketsWaitingForDrain.has(socket)) {
continue;
}
if (
eventType === "data" &&
this.clientSocketsWaitingForDrain.has(socket)
) {
continue;
}
Fix with Cubic


const canWrite = socket.write(message);
if (!canWrite) {
// Socket buffer full - data will be queued but may cause memory pressure
// In production, could track this and pause PTY output temporarily
console.warn(
`[Session ${this.sessionId}] Client socket buffer full, output may be delayed`,
);
Expand Down