From f1e52b0cbcb9e3fc6dc63f5f9466ced1ca43e25a Mon Sep 17 00:00:00 2001 From: Serhii Dziupin Date: Thu, 30 Jul 2026 13:48:24 +0300 Subject: [PATCH] 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. --- .../layout/ProjectActionsButton.tsx | 12 +- .../components/terminal/TerminalViewport.tsx | 46 +++- .../ui/src/components/views/TerminalView.tsx | 19 +- .../__tests__/terminalViewportRemount.test.ts | 64 +++++ packages/ui/src/lib/terminalApi.test.ts | 45 ++++ packages/ui/src/lib/terminalApi.ts | 34 ++- packages/ui/src/stores/DOCUMENTATION.md | 23 ++ .../ui/src/stores/useTerminalStore.test.ts | 87 ++++++- packages/ui/src/stores/useTerminalStore.ts | 232 ++++++++++++------ 9 files changed, 460 insertions(+), 102 deletions(-) create mode 100644 packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts diff --git a/packages/ui/src/components/layout/ProjectActionsButton.tsx b/packages/ui/src/components/layout/ProjectActionsButton.tsx index aa0bd9fe..8bc3602d 100644 --- a/packages/ui/src/components/layout/ProjectActionsButton.tsx +++ b/packages/ui/src/components/layout/ProjectActionsButton.tsx @@ -308,8 +308,9 @@ export const ProjectActionsButton = ({ React.useEffect(() => { const monitorRuns = () => { - const terminalSessions = useTerminalStore.getState().sessions; - const currentRuns = useTerminalStore.getState().projectActionRuns; + const terminalStore = useTerminalStore.getState(); + const terminalSessions = terminalStore.sessions; + const currentRuns = terminalStore.projectActionRuns; for (const [runKey, entry] of Object.entries(currentRuns)) { const directoryState = terminalSessions.get(entry.directory); const tab = directoryState?.tabs.find((item) => item.id === entry.tabId); @@ -321,9 +322,10 @@ export const ProjectActionsButton = ({ const watch = urlWatchByRunKeyRef.current[runKey] ?? { lastSeenChunkId: null, openedUrl: false, tail: '', openInPreview: false }; urlWatchByRunKeyRef.current[runKey] = watch; const action = displayActions.find((item) => item.id === entry.actionId); - if (!action || !Array.isArray(tab.bufferChunks) || tab.bufferChunks.length === 0) continue; + const bufferChunks = terminalStore.getBuffer(entry.directory, entry.tabId).chunks; + if (!action || bufferChunks.length === 0) continue; - const nextChunks = tab.bufferChunks.filter((chunk) => watch.lastSeenChunkId === null || chunk.id > watch.lastSeenChunkId); + const nextChunks = bufferChunks.filter((chunk) => watch.lastSeenChunkId === null || chunk.id > watch.lastSeenChunkId); if (nextChunks.length === 0) continue; const combined = nextChunks.map((chunk) => chunk.data).join(''); @@ -364,7 +366,7 @@ export const ProjectActionsButton = ({ monitorRuns(); return useTerminalStore.subscribe((state, previousState) => { - if (state.sessions !== previousState.sessions) monitorRuns(); + if (state.sessions !== previousState.sessions || state.buffers !== previousState.buffers) monitorRuns(); }); }, [displayActions, openContextPreview, openExternal, projectActionRuns, removeProjectActionRun, setTabPreviewUrl, t, updateProjectActionRunStatus]); diff --git a/packages/ui/src/components/terminal/TerminalViewport.tsx b/packages/ui/src/components/terminal/TerminalViewport.tsx index ef0c1ef1..90b834d3 100644 --- a/packages/ui/src/components/terminal/TerminalViewport.tsx +++ b/packages/ui/src/components/terminal/TerminalViewport.tsx @@ -53,6 +53,9 @@ const TerminalViewport = React.forwardRef(({ const outputRewriteCarryRef = React.useRef(''); const safeResetRef = React.useRef(getGhosttySafeResetSequence(theme.background)); const writingRef = React.useRef(false); + // Incremented whenever the replay stream restarts, so a write completing from + // before the restart cannot clear the in-flight flag of a newer write. + const writeEpochRef = React.useRef(0); const visibleRef = React.useRef(isVisible); const rendererReadyRef = React.useRef(false); const [ready, setReady] = React.useState(0); @@ -98,19 +101,39 @@ const TerminalViewport = React.forwardRef(({ return; } writingRef.current = true; + const epoch = writeEpochRef.current; terminal.write(rewritten.data, () => { - if (terminalRef.current !== terminal) return; + if (terminalRef.current !== terminal || writeEpochRef.current !== epoch) return; writingRef.current = false; if (writeQueueRef.current) flush(); }); }, []); + /** + * Replay discontinuities (restart, reconnect, buffer reset) only need the VT + * state cleared. `Terminal.reset()` frees and rebuilds the WASM terminal while + * keeping the canvas, renderer and font atlas, so prefer it over remounting the + * whole terminal; the generation bump remains the fallback before the terminal + * exists. + */ const recreateRenderer = React.useCallback(() => { lastChunkRef.current = null; writeQueueRef.current = ''; outputRewriteCarryRef.current = ''; writingRef.current = false; - setRendererGeneration((value) => value + 1); + writeEpochRef.current += 1; + const terminal = terminalRef.current; + if (!terminal) { + setRendererGeneration((value) => value + 1); + return; + } + try { + terminal.reset(); + const safeReset = safeResetRef.current; + if (safeReset) terminal.write(`${safeReset}\u001b[2J\u001b[H`); + } catch { + setRendererGeneration((value) => value + 1); + } }, []); React.useEffect(() => { @@ -195,6 +218,7 @@ const TerminalViewport = React.forwardRef(({ writeQueueRef.current = ''; outputRewriteCarryRef.current = ''; writingRef.current = false; + writeEpochRef.current += 1; rendererReadyRef.current = false; }; }, [fit, fontFamily, fontSize, rendererGeneration, theme]); @@ -214,10 +238,20 @@ const TerminalViewport = React.forwardRef(({ return; } const previous = lastChunkRef.current; - const previousIndex = previous === null ? -1 : chunks.findIndex((chunk) => chunk.id === previous); - if (previous !== null && previousIndex < 0) { - recreateRenderer(); - return; + // Chunk ids are monotonic and the store appends, so the already-written chunk + // is normally the last one. Scanning from the end keeps this O(1) per chunk + // instead of O(chunks) on every streamed write. + let previousIndex = -1; + if (previous !== null) { + for (let index = chunks.length - 1; index >= 0; index -= 1) { + const id = chunks[index].id; + if (id === previous) { previousIndex = index; break; } + if (id < previous) break; + } + if (previousIndex < 0) { + recreateRenderer(); + return; + } } const isReplay = previousIndex < 0; const pending = previousIndex >= 0 ? chunks.slice(previousIndex + 1) : chunks; diff --git a/packages/ui/src/components/views/TerminalView.tsx b/packages/ui/src/components/views/TerminalView.tsx index e91fd081..a373f880 100644 --- a/packages/ui/src/components/views/TerminalView.tsx +++ b/packages/ui/src/components/views/TerminalView.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { useSessionUIStore } from '@/sync/session-ui-store'; -import { useTerminalStore } from '@/stores/useTerminalStore'; +import { EMPTY_TERMINAL_BUFFER, useTerminalStore } from '@/stores/useTerminalStore'; import { useEffectiveDirectory } from '@/hooks/useEffectiveDirectory'; import { type TerminalStreamEvent } from '@/lib/api/types'; import { useThemeSystem } from '@/contexts/useThemeSystem'; @@ -97,7 +97,10 @@ export const TerminalView: React.FC = ({ visible }) => { const terminalSessionId = activeTab?.terminalSessionId ?? null; const terminalLifecycle = activeTab?.lifecycle ?? 'idle'; - const bufferChunks = activeTab?.bufferChunks ?? []; + // Scrollback is a leaf subscription: streaming output must not rerender the tab strip. + const bufferChunks = useTerminalStore((s) => ( + effectiveDirectory && activeTabId ? s.getBuffer(effectiveDirectory, activeTabId).chunks : EMPTY_TERMINAL_BUFFER.chunks + )); const isConnecting = activeTab?.isConnecting ?? false; const previewUrl = activeTab?.previewUrl ?? null; @@ -424,7 +427,8 @@ export const TerminalView: React.FC = ({ visible }) => { let terminalId = tab?.terminalSessionId ?? null; const terminalLifecycle = tab?.lifecycle ?? 'idle'; const isActionTab = Boolean(tab?.label?.startsWith('Action:')); - const hasBufferedOutput = (tab?.bufferLength ?? 0) > 0 || (tab?.bufferChunks?.length ?? 0) > 0; + const buffer = useTerminalStore.getState().getBuffer(directory, tabId); + const hasBufferedOutput = buffer.byteLength > 0 || buffer.chunks.length > 0; if (!terminalId) { if (terminalLifecycle === 'exited') { @@ -793,12 +797,15 @@ export const TerminalView: React.FC = ({ visible }) => { const xtermTheme = React.useMemo(() => convertThemeToXterm(currentTheme), [currentTheme]); + // Viewport identity is the tab, not the PTY session. Including the session id + // here tore down and rebuilt the Ghostty terminal (WASM VT + canvas + font + // atlas) a second time the moment `createSession` resolved, doubling the cost + // of every terminal open. Session changes are handled by the chunk replay path. const terminalViewportKey = React.useMemo(() => { const directoryPart = effectiveDirectory ?? 'no-dir'; const tabPart = activeTabId ?? 'no-tab'; - const terminalPart = terminalSessionId ?? 'no-terminal'; - return `${directoryPart}::${tabPart}::${terminalPart}`; - }, [effectiveDirectory, activeTabId, terminalSessionId]); + return `${directoryPart}::${tabPart}`; + }, [effectiveDirectory, activeTabId]); React.useEffect(() => { if (!isTerminalVisible || useTouchTerminalInput) { diff --git a/packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts b/packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts new file mode 100644 index 00000000..f33bd2cd --- /dev/null +++ b/packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts @@ -0,0 +1,64 @@ +/** + * Regression guard for slow terminal opening on Linux. + * + * `TerminalViewport` is keyed by `terminalViewportKey`. That key used to include + * the PTY session id, which is null until `createSession` resolves. Because the + * viewport must mount first to report its size before a session can be created, + * every terminal open built a Ghostty terminal (WASM VT + 2D canvas renderer + + * font atlas), threw it away when the session id arrived, and built a second one. + * The same churn repeated on reconnect and on every incidental session-id change, + * and the repeated WASM terminal allocate/free cycles are the suspected source of + * the reported crashes. + * + * Viewport identity must therefore be directory + tab only. Session changes are + * handled by the chunk replay path, which resets the existing terminal in place. + */ +import { describe, expect, test } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const terminalViewSource = readFileSync(join(__dirname, '..', 'TerminalView.tsx'), 'utf-8'); +const terminalViewportSource = readFileSync( + join(__dirname, '..', '..', 'terminal', 'TerminalViewport.tsx'), + 'utf-8', +); + +const viewportKeyBlock = (() => { + const start = terminalViewSource.indexOf('const terminalViewportKey = React.useMemo('); + expect(start).toBeGreaterThan(-1); + const end = terminalViewSource.indexOf('}, [', start); + expect(end).toBeGreaterThan(start); + return terminalViewSource.slice(start, terminalViewSource.indexOf(');', end)); +})(); + +describe('terminal viewport remount guard', () => { + test('viewport identity excludes the PTY session id', () => { + expect(viewportKeyBlock).toContain('effectiveDirectory'); + expect(viewportKeyBlock).toContain('activeTabId'); + expect(viewportKeyBlock).not.toContain('terminalSessionId'); + }); + + test('viewport key memo does not depend on the PTY session id', () => { + const dependencyStart = terminalViewSource.indexOf('}, [', terminalViewSource.indexOf('const terminalViewportKey')); + const dependencies = terminalViewSource.slice(dependencyStart, terminalViewSource.indexOf(']', dependencyStart)); + expect(dependencies).toContain('effectiveDirectory'); + expect(dependencies).toContain('activeTabId'); + expect(dependencies).not.toContain('terminalSessionId'); + }); + + test('replay discontinuities reset the terminal in place instead of remounting it', () => { + const start = terminalViewportSource.indexOf('const recreateRenderer = React.useCallback('); + expect(start).toBeGreaterThan(-1); + const body = terminalViewportSource.slice(start, terminalViewportSource.indexOf('}, []);', start)); + expect(body).toContain('terminal.reset()'); + // The generation bump stays only as the fallback when no terminal exists yet. + expect(body.indexOf('if (!terminal)')).toBeLessThan(body.indexOf('terminal.reset()')); + }); + + test('scrollback is read from the buffer slice, not from the tab', () => { + expect(terminalViewSource).toContain('getBuffer('); + expect(terminalViewSource).not.toContain('activeTab?.bufferChunks'); + }); +}); diff --git a/packages/ui/src/lib/terminalApi.test.ts b/packages/ui/src/lib/terminalApi.test.ts index 1423ec6a..b02aa0ac 100644 --- a/packages/ui/src/lib/terminalApi.test.ts +++ b/packages/ui/src/lib/terminalApi.test.ts @@ -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({ diff --git a/packages/ui/src/lib/terminalApi.ts b/packages/ui/src/lib/terminalApi.ts index 0927837e..3e555bf7 100644 --- a/packages/ui/src/lib/terminalApi.ts +++ b/packages/ui/src/lib/terminalApi.ts @@ -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(); private reconnectTimer: ReturnType | null = null; private keepaliveTimer: ReturnType | null = null; + private idleCloseTimer: ReturnType | 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(); 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; } diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index 3e1e7b8c..045fa43f 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -79,6 +79,29 @@ Chat composer drafts, confirmed mentions, inline-comment drafts, and pinned sess Composer draft edits remain immediate in memory and use a trailing durable-write debounce. Pending text and confirmed mentions flush synchronously when the document becomes hidden, freezes, receives `pagehide`, switches identity, or unmounts; authoritative deletion cancels pending work before any lifecycle flush can run. The shared chat-draft envelope reuses its parsed snapshot until the storage value changes. Inline-comment draft byte accounting indexes serialized buckets and recalculates only the changed session bucket during normal edits; deferred storage still performs the final full-envelope serialization and lifecycle flush. +### `useTerminalStore.ts` + +`useTerminalStore` owns terminal tab arrangement per directory plus PTY scrollback. + +Scrollback is deliberately **not** stored on the tab. `buffers` is a separate map keyed by +directory and tab id, and `getBuffer()` returns a shared frozen empty buffer for tabs that +have produced no output. PTY output arrives at streaming frequency, so keeping it inside +`sessions` made every output chunk allocate a new tab, a new directory entry and a new +`sessions` map. That invalidated every tab-strip subscription, re-ran the project-action +run monitor, and made Zustand persist rewrite the session-storage snapshot per chunk. + +Invariants to preserve when editing: + +- Output actions (`appendToBuffer`, `replaceBuffer`) must leave `sessions` referentially + unchanged; only `buffers` and `nextChunkId` may change. +- Buffer entries are owned by their tab. `closeTab`, `removeDirectory`, `clearAll`, and + rebinding a tab to a different terminal session must drop the entry. +- Output for an unknown tab is ignored rather than creating an orphan buffer. +- Only `sessions` and `nextTabId` are persisted. `partialize` reuses its previous + projection while both are referentially unchanged, and the storage adapter skips a write + for an unchanged projection, so streaming output performs no persistence work. +- Consumers that react to output must subscribe to `buffers`, not `sessions`. + ## Git / PR Stores The Git and PR stores are the most important stores to understand before editing this directory. diff --git a/packages/ui/src/stores/useTerminalStore.test.ts b/packages/ui/src/stores/useTerminalStore.test.ts index 711604ec..773f1152 100644 --- a/packages/ui/src/stores/useTerminalStore.test.ts +++ b/packages/ui/src/stores/useTerminalStore.test.ts @@ -7,6 +7,8 @@ const setup = () => { return useTerminalStore.getState().getDirectoryState('/repo')!.tabs[0].id; }; +const buffer = (tabId: string) => useTerminalStore.getState().getBuffer('/repo', tabId); + describe('terminal state reconciliation', () => { afterEach(() => useTerminalStore.getState().clearAll()); @@ -15,15 +17,14 @@ describe('terminal state reconciliation', () => { useTerminalStore.getState().replaceBuffer('/repo', tabId, 'prompt', 4); useTerminalStore.getState().appendToBuffer('/repo', tabId, ' output', 5); useTerminalStore.getState().appendToBuffer('/repo', tabId, ' duplicate', 5); - const tab = useTerminalStore.getState().getDirectoryState('/repo')!.tabs[0]; - expect(tab.bufferChunks.map((chunk) => chunk.data).join('')).toBe('prompt output'); - expect(tab.lastSequence).toBe(5); + expect(buffer(tabId).chunks.map((chunk) => chunk.data).join('')).toBe('prompt output'); + expect(buffer(tabId).lastSequence).toBe(5); }); test('keeps raw live bytes separate from replay-safe bytes', () => { const tabId = setup(); useTerminalStore.getState().appendToBuffer('/repo', tabId, 'prompt\u001b[6n', 1, 'prompt'); - const chunk = useTerminalStore.getState().getDirectoryState('/repo')!.tabs[0].bufferChunks[0]; + const chunk = buffer(tabId).chunks[0]; expect(chunk.data).toBe('prompt\u001b[6n'); expect(chunk.replayData).toBe('prompt'); }); @@ -37,22 +38,88 @@ describe('terminal state reconciliation', () => { const tabId = setup(); useTerminalStore.getState().replaceBuffer('/repo', tabId, 'new', 8); useTerminalStore.getState().replaceBuffer('/repo', tabId, 'stale', 7); - expect(useTerminalStore.getState().getDirectoryState('/repo')!.tabs[0].bufferChunks[0].data).toBe('new'); + expect(buffer(tabId).chunks[0].data).toBe('new'); }); test('preserves buffer identity for an identical snapshot', () => { const tabId = setup(); useTerminalStore.getState().replaceBuffer('/repo', tabId, 'prompt', 8); - const previous = useTerminalStore.getState().getDirectoryState('/repo')!.tabs[0].bufferChunks; + const previous = buffer(tabId).chunks; useTerminalStore.getState().replaceBuffer('/repo', tabId, 'prompt', 8); - expect(useTerminalStore.getState().getDirectoryState('/repo')!.tabs[0].bufferChunks).toBe(previous); + expect(buffer(tabId).chunks).toBe(previous); }); test('caps multibyte scrollback by UTF-8 bytes', () => { const tabId = setup(); useTerminalStore.getState().appendToBuffer('/repo', tabId, '界'.repeat(200_000), 1); - const tab = useTerminalStore.getState().getDirectoryState('/repo')!.tabs[0]; - expect(tab.bufferLength <= 512 * 1024).toBe(true); - expect(new TextEncoder().encode(tab.bufferChunks[0].data).byteLength).toBe(tab.bufferLength); + expect(buffer(tabId).byteLength <= 512 * 1024).toBe(true); + expect(new TextEncoder().encode(buffer(tabId).chunks[0].data).byteLength).toBe(buffer(tabId).byteLength); + }); + + test('returns a stable empty buffer for tabs that produced no output', () => { + const tabId = setup(); + expect(buffer(tabId).chunks.length).toBe(0); + expect(buffer(tabId)).toBe(useTerminalStore.getState().getBuffer('/repo', 'unknown-tab')); + }); + + // Scale guard: everything `partialize` reads must stay referentially unchanged + // while output streams, otherwise persistence and the tab strip go back to + // doing per-chunk work that grows with the number of open terminals. + test('streaming output keeps tab metadata and persisted inputs referentially stable', () => { + const first = setup(); + const second = useTerminalStore.getState().createTab('/repo'); + useTerminalStore.getState().ensureDirectory('/other'); + const otherTab = useTerminalStore.getState().getDirectoryState('/other')!.tabs[0].id; + + const sessionsBefore = useTerminalStore.getState().sessions; + const repoBefore = useTerminalStore.getState().getDirectoryState('/repo'); + const otherBefore = useTerminalStore.getState().getDirectoryState('/other'); + const nextTabIdBefore = useTerminalStore.getState().nextTabId; + + const tabs: Array<[string, string]> = [['/repo', first], ['/repo', second], ['/other', otherTab]]; + for (let index = 0; index < 90; index += 1) { + const [directory, tabId] = tabs[index % tabs.length]; + useTerminalStore.getState().appendToBuffer(directory, tabId, `line ${index}\n`, index + 1); + } + + expect(useTerminalStore.getState().sessions).toBe(sessionsBefore); + expect(useTerminalStore.getState().getDirectoryState('/repo')).toBe(repoBefore); + expect(useTerminalStore.getState().getDirectoryState('/other')).toBe(otherBefore); + expect(useTerminalStore.getState().nextTabId).toBe(nextTabIdBefore); + expect(buffer(first).chunks.length).toBe(30); + + useTerminalStore.getState().removeDirectory('/other'); + }); + + test('drops scrollback when a tab is closed or its directory is removed', () => { + const first = setup(); + const second = useTerminalStore.getState().createTab('/repo'); + useTerminalStore.getState().appendToBuffer('/repo', first, 'first', 1); + useTerminalStore.getState().appendToBuffer('/repo', second, 'second', 1); + expect(useTerminalStore.getState().buffers.size).toBe(2); + + useTerminalStore.getState().closeTab('/repo', second); + expect(useTerminalStore.getState().buffers.size).toBe(1); + expect(buffer(first).chunks[0].data).toBe('first'); + + useTerminalStore.getState().removeDirectory('/repo'); + expect(useTerminalStore.getState().buffers.size).toBe(0); + }); + + test('resets scrollback when a tab is bound to a different terminal session', () => { + const tabId = setup(); + useTerminalStore.getState().setTabSessionId('/repo', tabId, 'session-a'); + useTerminalStore.getState().appendToBuffer('/repo', tabId, 'from a', 1); + expect(buffer(tabId).chunks.length).toBe(1); + + useTerminalStore.getState().setTabSessionId('/repo', tabId, 'session-b'); + expect(buffer(tabId)).toBe(useTerminalStore.getState().getBuffer('/repo', 'never-used')); + }); + + test('ignores output for tabs that no longer exist', () => { + setup(); + useTerminalStore.getState().appendToBuffer('/repo', 'ghost-tab', 'output', 1); + useTerminalStore.getState().replaceBuffer('/repo', 'ghost-tab', 'snapshot', 1); + expect(useTerminalStore.getState().buffers.size).toBe(0); }); }); diff --git a/packages/ui/src/stores/useTerminalStore.ts b/packages/ui/src/stores/useTerminalStore.ts index 6bcd39cd..6b68d32a 100644 --- a/packages/ui/src/stores/useTerminalStore.ts +++ b/packages/ui/src/stores/useTerminalStore.ts @@ -1,5 +1,6 @@ import { create } from 'zustand'; import { devtools, persist, createJSONStorage } from 'zustand/middleware'; +import type { PersistStorage } from 'zustand/middleware'; import { getSafeSessionStorage } from '@/stores/utils/safeStorage'; @@ -10,6 +11,23 @@ export interface TerminalChunk { byteLength: number; } +/** + * Scrollback lives outside `sessions` because PTY output arrives at streaming + * frequency. Keeping it here leaves tab metadata referentially stable, so + * output cannot rerender the tab strip or rewrite the persisted snapshot. + */ +export type TerminalBuffer = { + chunks: TerminalChunk[]; + byteLength: number; + lastSequence: number; +}; + +export const EMPTY_TERMINAL_BUFFER: TerminalBuffer = Object.freeze({ + chunks: Object.freeze([]) as unknown as TerminalChunk[], + byteLength: 0, + lastSequence: -1, +}); + export type TerminalTabLifecycle = 'idle' | 'running' | 'exited'; export type TerminalTab = { @@ -18,9 +36,6 @@ export type TerminalTab = { lifecycle: TerminalTabLifecycle; label: string; iconKey: string | null; - bufferChunks: TerminalChunk[]; - bufferLength: number; - lastSequence: number; isConnecting: boolean; createdAt: number; previewUrl: string | null; @@ -44,6 +59,7 @@ export type TerminalProjectActionRun = { interface TerminalStore { sessions: Map; + buffers: Map; projectActionRuns: Record; nextChunkId: number; nextTabId: number; @@ -52,6 +68,7 @@ interface TerminalStore { ensureDirectory: (directory: string) => void; getDirectoryState: (directory: string) => DirectoryTerminalState | undefined; getActiveTab: (directory: string) => TerminalTab | undefined; + getBuffer: (directory: string, tabId: string) => TerminalBuffer; createTab: (directory: string) => string; setActiveTab: (directory: string, tabId: string) => void; @@ -77,13 +94,28 @@ interface TerminalStore { const TERMINAL_BUFFER_LIMIT = 512 * 1024; const textEncoder = new TextEncoder(); const textDecoder = new TextDecoder(); -const byteLength = (value: string): number => textEncoder.encode(value).byteLength; -const trimToBufferLimit = (value: string): string => { +/** One encode per chunk: the trimmed text and its UTF-8 size are needed together. */ +const trimToBufferLimit = (value: string): { text: string; byteLength: number } => { const bytes = textEncoder.encode(value); - if (bytes.byteLength <= TERMINAL_BUFFER_LIMIT) return value; + if (bytes.byteLength <= TERMINAL_BUFFER_LIMIT) return { text: value, byteLength: bytes.byteLength }; let start = bytes.byteLength - TERMINAL_BUFFER_LIMIT; while (start < bytes.byteLength && (bytes[start] & 0xc0) === 0x80) start += 1; - return textDecoder.decode(bytes.subarray(start)); + const retained = bytes.subarray(start); + return { text: textDecoder.decode(retained), byteLength: retained.byteLength }; +}; +// NUL cannot appear in a directory path or a tab id, so the composite key is unambiguous. +const bufferKey = (directory: string, tabId: string): string => `${directory}\u0000${tabId}`; +const dropBufferKeys = ( + buffers: Map, + matches: (key: string) => boolean, +): Map | null => { + let next: Map | null = null; + for (const key of buffers.keys()) { + if (!matches(key)) continue; + next ??= new Map(buffers); + next.delete(key); + } + return next; }; const TERMINAL_STORE_NAME = 'terminal-store'; let hydrationListenerAttached = false; @@ -131,9 +163,6 @@ const createEmptyTab = (id: string, label: string): TerminalTab => ({ lifecycle: 'idle', label, iconKey: null, - bufferChunks: [], - bufferLength: 0, - lastSequence: -1, isConnecting: false, createdAt: Date.now(), previewUrl: null, @@ -149,11 +178,70 @@ const createEmptyDirectoryState = (firstTab: TerminalTab): DirectoryTerminalStat const findTabIndex = (state: DirectoryTerminalState, tabId: string): number => state.tabs.findIndex((t) => t.id === tabId); +/** + * Zustand persist runs `partialize` and writes storage after every `set`, and + * terminal output calls `set` at streaming frequency. Only `sessions` and + * `nextTabId` are persisted, so reuse the previous projection whenever both are + * referentially unchanged and skip the write for an unchanged projection. + */ +let lastPartializeInput: { sessions: unknown; nextTabId: number } | null = null; +let lastPartializeResult: PersistedTerminalStoreState | null = null; + +const partializeTerminalStore = (state: TerminalStore): PersistedTerminalStoreState => { + if ( + lastPartializeResult + && lastPartializeInput?.sessions === state.sessions + && lastPartializeInput.nextTabId === state.nextTabId + ) { + return lastPartializeResult; + } + + const result: PersistedTerminalStoreState = { + sessions: Array.from(state.sessions.entries()).map(([directory, dirState]) => [ + directory, + { + activeTabId: dirState.activeTabId, + tabs: dirState.tabs.map((tab) => ({ + id: tab.id, + label: tab.label, + iconKey: tab.iconKey, + createdAt: tab.createdAt, + })), + }, + ]), + nextTabId: state.nextTabId, + }; + + lastPartializeInput = { sessions: state.sessions, nextTabId: state.nextTabId }; + lastPartializeResult = result; + return result; +}; + +const createDedupedTerminalStorage = (): PersistStorage | undefined => { + const base = createJSONStorage(() => getSafeSessionStorage()); + if (!base) return undefined; + + let lastWrittenState: PersistedTerminalStoreState | null = null; + return { + getItem: (name) => base.getItem(name), + setItem: (name, value) => { + if (value.state === lastWrittenState) return; + lastWrittenState = value.state; + return base.setItem(name, value); + }, + removeItem: (name) => { + lastWrittenState = null; + return base.removeItem(name); + }, + }; +}; + export const useTerminalStore = create()( devtools( persist( (set, get) => ({ sessions: new Map(), + buffers: new Map(), projectActionRuns: {}, nextChunkId: 1, nextTabId: 1, @@ -191,6 +279,9 @@ export const useTerminalStore = create()( return entry.tabs.find((t) => t.id === activeId) ?? entry.tabs[0]; }, + getBuffer: (directory: string, tabId: string) => + get().buffers.get(bufferKey(normalizeDirectory(directory), tabId)) ?? EMPTY_TERMINAL_BUFFER, + createTab: (directory: string) => { const key = normalizeDirectory(directory); if (!key) { @@ -332,6 +423,10 @@ export const useTerminalStore = create()( Object.entries(state.projectActionRuns).filter(([, run]) => !(run.directory === key && run.tabId === tabId)) ); const runsChanged = Object.keys(nextRuns).length !== Object.keys(state.projectActionRuns).length; + const closedBufferKey = bufferKey(key, tabId); + const nextBuffers = state.buffers.has(closedBufferKey) + ? dropBufferKeys(state.buffers, (bufferEntryKey) => bufferEntryKey === closedBufferKey) + : null; if (nextTabs.length === 0) { const newTabId = createTerminalTabId(); @@ -340,6 +435,7 @@ export const useTerminalStore = create()( return { sessions: newSessions, nextTabId: state.nextTabId + 1, + ...(nextBuffers ? { buffers: nextBuffers } : {}), ...(runsChanged ? { projectActionRuns: nextRuns } : {}), }; } @@ -358,6 +454,7 @@ export const useTerminalStore = create()( return { sessions: newSessions, + ...(nextBuffers ? { buffers: nextBuffers } : {}), ...(runsChanged ? { projectActionRuns: nextRuns } : {}), }; }); @@ -389,13 +486,17 @@ export const useTerminalStore = create()( terminalSessionId: sessionId, lifecycle: nextLifecycle, isConnecting: false, - ...(shouldResetBuffer ? { bufferChunks: [], bufferLength: 0, lastSequence: -1 } : {}), }; + const resetKey = bufferKey(key, tabId); + const nextBuffers = shouldResetBuffer && state.buffers.has(resetKey) + ? dropBufferKeys(state.buffers, (bufferEntryKey) => bufferEntryKey === resetKey) + : null; + const nextTabs = [...existing.tabs]; nextTabs[idx] = nextTab; newSessions.set(key, { ...existing, tabs: nextTabs }); - return { sessions: newSessions }; + return { sessions: newSessions, ...(nextBuffers ? { buffers: nextBuffers } : {}) }; }); }, @@ -445,26 +546,26 @@ export const useTerminalStore = create()( const key = normalizeDirectory(directory); set((state) => { const existing = state.sessions.get(key); - if (!existing) return state; - const idx = findTabIndex(existing, tabId); - if (idx < 0 || existing.tabs[idx].lastSequence > sequence) return state; - const retainedContent = trimToBufferLimit(content); - const bytes = byteLength(retainedContent); - const tab = existing.tabs[idx]; + if (!existing || findTabIndex(existing, tabId) < 0) return state; + const entryKey = bufferKey(key, tabId); + const buffer = state.buffers.get(entryKey) ?? EMPTY_TERMINAL_BUFFER; + if (buffer.lastSequence > sequence) return state; + const retained = trimToBufferLimit(content); if ( - tab.lastSequence === sequence && - tab.bufferLength === bytes && - tab.bufferChunks.map((chunk) => chunk.data).join('') === retainedContent + buffer.lastSequence === sequence && + buffer.byteLength === retained.byteLength && + buffer.chunks.map((chunk) => chunk.data).join('') === retained.text ) { return state; } const chunkId = state.nextChunkId; - const bufferChunks = retainedContent ? [{ id: chunkId, data: retainedContent, byteLength: bytes }] : []; - const nextTabs = [...existing.tabs]; - nextTabs[idx] = { ...nextTabs[idx], bufferChunks, bufferLength: bytes, lastSequence: sequence }; - const sessions = new Map(state.sessions); - sessions.set(key, { ...existing, tabs: nextTabs }); - return { sessions, nextChunkId: retainedContent ? chunkId + 1 : chunkId }; + const buffers = new Map(state.buffers); + buffers.set(entryKey, { + chunks: retained.text ? [{ id: chunkId, data: retained.text, byteLength: retained.byteLength }] : [], + byteLength: retained.byteLength, + lastSequence: sequence, + }); + return { buffers, nextChunkId: retained.text ? chunkId + 1 : chunkId }; }); }, @@ -475,52 +576,45 @@ export const useTerminalStore = create()( const key = normalizeDirectory(directory); set((state) => { - const newSessions = new Map(state.sessions); - const existing = newSessions.get(key); - if (!existing) { + const existing = state.sessions.get(key); + if (!existing || findTabIndex(existing, tabId) < 0) { return state; } - const idx = findTabIndex(existing, tabId); - if (idx < 0) { - return state; - } - - const tab = existing.tabs[idx]; - if (sequence !== undefined && sequence <= tab.lastSequence) return state; + const entryKey = bufferKey(key, tabId); + const buffer = state.buffers.get(entryKey) ?? EMPTY_TERMINAL_BUFFER; + if (sequence !== undefined && sequence <= buffer.lastSequence) return state; const chunkId = state.nextChunkId; const retainedChunk = trimToBufferLimit(chunk); const retainedReplayData = replayData !== undefined && replayData !== chunk - ? trimToBufferLimit(replayData) + ? trimToBufferLimit(replayData).text : undefined; const chunkEntry: TerminalChunk = { id: chunkId, - data: retainedChunk, + data: retainedChunk.text, ...(retainedReplayData !== undefined ? { replayData: retainedReplayData } : {}), - byteLength: byteLength(retainedChunk), + byteLength: retainedChunk.byteLength, }; - const bufferChunks = [...tab.bufferChunks, chunkEntry]; - let bufferLength = tab.bufferLength + chunkEntry.byteLength; + const chunks = [...buffer.chunks, chunkEntry]; + let bufferLength = buffer.byteLength + chunkEntry.byteLength; - while (bufferLength > TERMINAL_BUFFER_LIMIT && bufferChunks.length > 1) { - const removed = bufferChunks.shift(); + while (bufferLength > TERMINAL_BUFFER_LIMIT && chunks.length > 1) { + const removed = chunks.shift(); if (!removed) { break; } bufferLength -= removed.byteLength; } - const nextTabs = [...existing.tabs]; - nextTabs[idx] = { - ...tab, - bufferChunks, - bufferLength, - lastSequence: sequence ?? tab.lastSequence, - }; - newSessions.set(key, { ...existing, tabs: nextTabs }); + const buffers = new Map(state.buffers); + buffers.set(entryKey, { + chunks, + byteLength: bufferLength, + lastSequence: sequence ?? buffer.lastSequence, + }); - return { sessions: newSessions, nextChunkId: chunkId + 1 }; + return { buffers, nextChunkId: chunkId + 1 }; }); }, @@ -629,35 +723,27 @@ export const useTerminalStore = create()( set((state) => { const newSessions = new Map(state.sessions); newSessions.delete(key); + const prefix = bufferKey(key, ''); + const nextBuffers = dropBufferKeys(state.buffers, (entryKey) => entryKey.startsWith(prefix)); const nextRuns = Object.fromEntries( Object.entries(state.projectActionRuns).filter(([, run]) => run.directory !== key) ); - return { sessions: newSessions, projectActionRuns: nextRuns }; + return { + sessions: newSessions, + ...(nextBuffers ? { buffers: nextBuffers } : {}), + projectActionRuns: nextRuns, + }; }); }, clearAll: () => { - set({ sessions: new Map(), projectActionRuns: {}, nextChunkId: 1, nextTabId: 1 }); + set({ sessions: new Map(), buffers: new Map(), projectActionRuns: {}, nextChunkId: 1, nextTabId: 1 }); }, }), { name: TERMINAL_STORE_NAME, - storage: createJSONStorage(() => getSafeSessionStorage()), - partialize: (state): PersistedTerminalStoreState => ({ - sessions: Array.from(state.sessions.entries()).map(([directory, dirState]) => [ - directory, - { - activeTabId: dirState.activeTabId, - tabs: dirState.tabs.map((tab) => ({ - id: tab.id, - label: tab.label, - iconKey: tab.iconKey, - createdAt: tab.createdAt, - })), - }, - ]), - nextTabId: state.nextTabId, - }), + storage: createDedupedTerminalStorage(), + partialize: partializeTerminalStore, merge: (persistedState, currentState) => { if (!isRecord(persistedState)) { return currentState; @@ -708,9 +794,6 @@ export const useTerminalStore = create()( terminalSessionId: null, lifecycle: 'idle', createdAt: typeof rawTab.createdAt === 'number' ? rawTab.createdAt : Date.now(), - bufferChunks: [], - bufferLength: 0, - lastSequence: -1, isConnecting: false, previewUrl: null, previewAutoOpened: false, @@ -743,6 +826,7 @@ export const useTerminalStore = create()( return { ...currentState, sessions, + buffers: new Map(), nextChunkId: 1, nextTabId, hasHydrated: true,