fix: stop terminal open/switch from rewriting state per output chunk (#2536)
Opening a terminal rebuilt the WASM terminal twice and rewrote the persisted session snapshot on every streamed output chunk, so the cost grew with each open terminal and could crash under load. - Move PTY scrollback to a standalone buffers map keyed by directory and tab id; output no longer touches sessions, so the tab strip, the project-action monitor and the persist projection stay referentially stable while chunks stream. - Memoize partialize and add a dedup storage adapter that skips writes when the persisted projection is unchanged. - Reuse the idle terminal WebSocket across tab switches (15s grace) instead of re-authenticating and replaying the snapshot on every attach. - Key the viewport by directory + tab only so createSession no longer tears down and rebuilds the Ghostty terminal. - Reset the terminal in place on replay discontinuities instead of bumping the renderer generation. - Scan the chunk array from the end (O(1) per write) instead of findIndex. Adds regression tests for the buffer map, socket reuse and viewport key, and documents the store invariants.
This commit is contained in:
@@ -132,6 +132,51 @@ describe('terminal transport', () => {
|
||||
transport.dispose();
|
||||
});
|
||||
|
||||
test('reuses the open socket when switching between terminals', async () => {
|
||||
const sockets: FakeSocket[] = [];
|
||||
let authCalls = 0;
|
||||
const transport = new TerminalTransport({
|
||||
refreshAuth: async () => { authCalls += 1; },
|
||||
openSocket: () => { const socket = new FakeSocket(); sockets.push(socket); return socket; },
|
||||
});
|
||||
|
||||
const unsubscribeFirst = transport.subscribe('term-1', { onEvent: () => {} });
|
||||
await tick();
|
||||
sockets[0].open();
|
||||
await tick();
|
||||
expect(authCalls).toBe(1);
|
||||
|
||||
// Switching tabs detaches the old terminal before attaching the new one.
|
||||
unsubscribeFirst();
|
||||
transport.subscribe('term-2', { onEvent: () => {} });
|
||||
await tick();
|
||||
|
||||
expect(sockets).toHaveLength(1);
|
||||
expect(sockets[0].readyState).toBe(1);
|
||||
expect(authCalls).toBe(1);
|
||||
expect(sockets[0].sent.some((message) => message.t === 'detach' && message.s === 'term-1')).toBe(true);
|
||||
expect(sockets[0].sent.some((message) => message.t === 'attach' && message.s === 'term-2')).toBe(true);
|
||||
transport.dispose();
|
||||
});
|
||||
|
||||
test('disposing closes a socket that was being held for reuse', async () => {
|
||||
const sockets: FakeSocket[] = [];
|
||||
const transport = new TerminalTransport({
|
||||
refreshAuth: async () => '',
|
||||
openSocket: () => { const socket = new FakeSocket(); sockets.push(socket); return socket; },
|
||||
});
|
||||
|
||||
const unsubscribe = transport.subscribe('term-1', { onEvent: () => {} });
|
||||
await tick();
|
||||
sockets[0].open();
|
||||
await tick();
|
||||
|
||||
unsubscribe();
|
||||
expect(sockets[0].readyState).toBe(1);
|
||||
transport.dispose();
|
||||
expect(sockets[0].readyState).toBe(3);
|
||||
});
|
||||
|
||||
test('does not reconnect after the last subscriber detaches', async () => {
|
||||
let attempts = 0;
|
||||
const transport = new TerminalTransport({
|
||||
|
||||
@@ -21,6 +21,13 @@ const TAG = 1;
|
||||
const MAX_PROJECTION_BYTES = 512 * 1024;
|
||||
const SOCKET_CONNECTING = 0;
|
||||
const SOCKET_OPEN = 1;
|
||||
/**
|
||||
* Switching terminal tabs detaches the old terminal before attaching the new one,
|
||||
* which momentarily leaves zero subscribers. Closing the socket there forced a
|
||||
* token refresh, a fresh upgrade and a full snapshot replay on every switch, so
|
||||
* hold the idle socket briefly and reuse it instead.
|
||||
*/
|
||||
const IDLE_SOCKET_GRACE_MS = 15_000;
|
||||
const encoder = new TextEncoder();
|
||||
const decoder = new TextDecoder();
|
||||
|
||||
@@ -69,6 +76,7 @@ export class TerminalTransport {
|
||||
private projections = new Map<string, TerminalProjection>();
|
||||
private reconnectTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
private keepaliveTimer: ReturnType<typeof setInterval> | null = null;
|
||||
private idleCloseTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
private failures = 0;
|
||||
private wakeCleanup: (() => void) | null = null;
|
||||
private generation = 0;
|
||||
@@ -80,6 +88,7 @@ export class TerminalTransport {
|
||||
}) {}
|
||||
|
||||
subscribe(sessionId: string, handlers: TerminalHandlers): () => void {
|
||||
this.cancelIdleClose();
|
||||
const subscriber = { handlers, lastSequence: -1 };
|
||||
const set = this.subscribers.get(sessionId) ?? new Set<Subscriber>();
|
||||
const first = set.size === 0;
|
||||
@@ -104,8 +113,14 @@ export class TerminalTransport {
|
||||
this.send({ t: 'detach', v: 3, s: sessionId });
|
||||
}
|
||||
if (this.subscribers.size === 0) {
|
||||
this.generation += 1;
|
||||
this.cancelReconnect();
|
||||
if (this.socket?.readyState === SOCKET_OPEN) {
|
||||
// Healthy socket: hold it briefly so a tab switch can reattach to it.
|
||||
this.scheduleIdleClose();
|
||||
return;
|
||||
}
|
||||
// Nothing to reuse, so abandon any dial that is still in flight.
|
||||
this.generation += 1;
|
||||
this.closeSocket();
|
||||
}
|
||||
};
|
||||
@@ -127,6 +142,7 @@ export class TerminalTransport {
|
||||
this.projections.clear();
|
||||
if (this.reconnectTimer) clearTimeout(this.reconnectTimer);
|
||||
this.reconnectTimer = null;
|
||||
this.cancelIdleClose();
|
||||
this.wakeCleanup?.();
|
||||
this.wakeCleanup = null;
|
||||
this.closeSocket();
|
||||
@@ -282,6 +298,22 @@ export class TerminalTransport {
|
||||
this.reconnectTimer = setTimeout(wake, delay);
|
||||
}
|
||||
|
||||
private scheduleIdleClose(): void {
|
||||
if (this.idleCloseTimer || this.disposed) return;
|
||||
this.idleCloseTimer = setTimeout(() => {
|
||||
this.idleCloseTimer = null;
|
||||
if (this.disposed || this.subscribers.size > 0) return;
|
||||
this.generation += 1;
|
||||
this.closeSocket();
|
||||
}, IDLE_SOCKET_GRACE_MS);
|
||||
}
|
||||
|
||||
private cancelIdleClose(): void {
|
||||
if (!this.idleCloseTimer) return;
|
||||
clearTimeout(this.idleCloseTimer);
|
||||
this.idleCloseTimer = null;
|
||||
}
|
||||
|
||||
private startKeepalive(): void { this.stopKeepalive(); this.keepaliveTimer = setInterval(() => this.send({ t: 'ping', v: 3 }), 20_000); }
|
||||
private stopKeepalive(): void { if (this.keepaliveTimer) clearInterval(this.keepaliveTimer); this.keepaliveTimer = null; }
|
||||
private cancelReconnect(): void { if (this.reconnectTimer) clearTimeout(this.reconnectTimer); this.reconnectTimer = null; this.wakeCleanup?.(); this.wakeCleanup = null; }
|
||||
|
||||
Reference in New Issue
Block a user