From 1ae94a4cb9abd6150b8f2cda7560f9bb308f11fa Mon Sep 17 00:00:00 2001 From: Simon Pamies Date: Fri, 29 May 2026 15:28:55 +0200 Subject: [PATCH 1/2] Rewrite terminal redraw: pipe PTY output straight to xterm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal reconstructed a byte delta on every render by string-diffing an accumulated rawOutput buffer against its previous value, then writing that guess to xterm. For a redraw-heavy TUI like Claude Code this fell apart: once rawOutput crossed the 2MB cap it was front-trimmed, the truncation probe was defeated by repeated escape sequences, and the fallback was terminal.reset() + a full rewrite — a full-screen flush, i.e. flicker. Switch to Hyper's model: xterm owns the screen buffer and PTY chunks are written to it directly, in order, once. - Store delivers output through a per-terminal command channel ({write,clear}) with a small pre-mount backlog; rawOutput is no longer the screen source of truth (a hasOutput flag drives empty-state UI). - Viewport subscribes and calls terminal.write() directly; deletes the diff/probe/reset effect and the watermark write-queue. - Reattach uses @xterm/addon-serialize: snapshot restored on mount, saved on unmount and debounced, mirrored to safeStorage for reload survival. - Backend decodes PTY reads with a carry buffer (StringDecoder-style) so multi-byte UTF-8 split across a 4KB read boundary no longer corrupts box-drawing/emoji/powerline glyphs into U+FFFD. - AIAuthTerminalModal adopts the new TerminalSessionView interface with a local emitter, keeping its text accumulator only for success scanning. This also folds in prior terminal work that had not reached upstream: Shift+Enter inserting a newline, the immediate first PTY resize, and the dictation overlay. The earlier in-place patch (truncation probe) is removed — it was a workaround for the accumulator the rewrite deletes. Tests: remove the obsolete probe/watermark tests; add coverage for the output channel, clear, replay snapshot persistence, channel teardown, and the UTF-8 carry decoder. Full desktop suite green; lint clean. --- Cargo.lock | 2 +- apps/desktop/native-backend/src/devtools.rs | 106 ++++++- apps/desktop/package-lock.json | 7 + apps/desktop/package.json | 1 + .../ai/components/AIAuthTerminalModal.tsx | 76 ++++- .../editor/EditorPaneContent.test.tsx | 10 +- .../terminal/TerminalViewport.test.tsx | 261 ++++++++++++------ .../features/terminal/TerminalViewport.tsx | 252 +++++++++-------- .../terminal/WorkspaceTerminalHost.test.tsx | 31 ++- .../terminal/WorkspaceTerminalHost.tsx | 10 +- .../terminal/claudeCodeTerminal.test.ts | 2 +- .../terminal/terminalRuntimeStore.test.ts | 140 +++++++++- .../features/terminal/terminalRuntimeStore.ts | 217 ++++++++++++--- .../src/features/terminal/terminalTypes.ts | 21 +- apps/desktop/src/test/setup.ts | 30 +- apps/desktop/src/test/test-utils.tsx | 6 + 16 files changed, 912 insertions(+), 260 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index abd1c84b..fad3fb60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1713,7 +1713,7 @@ dependencies = [ [[package]] name = "neverwrite-native-backend" -version = "0.3.0" +version = "0.3.1" dependencies = [ "agent-client-protocol", "keyring", diff --git a/apps/desktop/native-backend/src/devtools.rs b/apps/desktop/native-backend/src/devtools.rs index e5f2c8ef..dae10994 100644 --- a/apps/desktop/native-backend/src/devtools.rs +++ b/apps/desktop/native-backend/src/devtools.rs @@ -491,6 +491,46 @@ fn release_session_runtime_resources( } } +/// Decode a PTY read into a UTF-8 string, carrying any incomplete trailing +/// multi-byte sequence across to the next read via `carry`. Genuinely invalid +/// bytes are replaced with U+FFFD; only an unfinished-but-valid prefix is held +/// back. Mirrors Node's `StringDecoder('utf8')`. +fn decode_utf8_with_carry(carry: &mut Vec, bytes: &[u8]) -> String { + carry.extend_from_slice(bytes); + let mut out = String::new(); + let mut start = 0; + loop { + match std::str::from_utf8(&carry[start..]) { + Ok(valid) => { + out.push_str(valid); + carry.clear(); + return out; + } + Err(error) => { + let valid_up_to = error.valid_up_to(); + // SAFETY: `valid_up_to` is, by contract, a valid UTF-8 boundary. + out.push_str(unsafe { + std::str::from_utf8_unchecked(&carry[start..start + valid_up_to]) + }); + match error.error_len() { + // A real invalid sequence: emit one replacement char and + // advance past it, then keep decoding the rest. + Some(len) => { + out.push('\u{FFFD}'); + start += valid_up_to + len; + } + // Input ended mid-sequence: hold the tail for the next read. + None => { + let remainder = carry[start + valid_up_to..].to_vec(); + *carry = remainder; + return out; + } + } + } + } + } +} + fn spawn_output_reader( mut reader: Box, closed: Arc, @@ -499,6 +539,12 @@ fn spawn_output_reader( ) { thread::spawn(move || { let mut buffer = [0_u8; OUTPUT_CHUNK_SIZE]; + // Holds the bytes of a multi-byte UTF-8 sequence that was split across a + // read boundary. Without this, `from_utf8_lossy` would replace the split + // codepoint with U+FFFD, corrupting box-drawing / emoji / powerline + // glyphs that TUIs like Claude Code emit heavily. This mirrors Node's + // StringDecoder: decode the valid prefix, carry the incomplete tail. + let mut carry: Vec = Vec::new(); loop { if closed.load(Ordering::Relaxed) { break; @@ -509,8 +555,10 @@ fn spawn_output_reader( if closed.load(Ordering::Relaxed) { break; } - let chunk = String::from_utf8_lossy(&buffer[..read]).into_owned(); - emit_terminal_output(&event_tx, &session_id, chunk); + let chunk = decode_utf8_with_carry(&mut carry, &buffer[..read]); + if !chunk.is_empty() { + emit_terminal_output(&event_tx, &session_id, chunk); + } } Err(error) => { if !closed.load(Ordering::Relaxed) { @@ -817,6 +865,60 @@ mod tests { use super::*; use std::fs; + #[test] + fn decodes_utf8_sequence_split_across_reads() { + // "é" (U+00E9) is 0xC3 0xA9. Split it across two reads. + let mut carry = Vec::new(); + let first = decode_utf8_with_carry(&mut carry, &[b'a', 0xC3]); + assert_eq!(first, "a"); + assert_eq!(carry, vec![0xC3]); + let second = decode_utf8_with_carry(&mut carry, &[0xA9, b'b']); + assert_eq!(second, "\u{00E9}b"); + assert!(carry.is_empty()); + } + + #[test] + fn decodes_three_byte_sequence_split_at_every_boundary() { + // "→" (U+2192) is 0xE2 0x86 0x92. Feed one byte per read. + let mut carry = Vec::new(); + assert_eq!(decode_utf8_with_carry(&mut carry, &[0xE2]), ""); + assert_eq!(decode_utf8_with_carry(&mut carry, &[0x86]), ""); + assert_eq!(decode_utf8_with_carry(&mut carry, &[0x92]), "\u{2192}"); + assert!(carry.is_empty()); + } + + #[test] + fn decodes_four_byte_emoji_split_across_reads() { + // "😀" (U+1F600) is 0xF0 0x9F 0x98 0x80. Split it across three reads. + let mut carry = Vec::new(); + assert_eq!(decode_utf8_with_carry(&mut carry, &[0xF0, 0x9F]), ""); + assert_eq!(decode_utf8_with_carry(&mut carry, &[0x98]), ""); + assert_eq!( + decode_utf8_with_carry(&mut carry, &[0x80, b'!']), + "\u{1F600}!" + ); + assert!(carry.is_empty()); + } + + #[test] + fn leaves_no_carry_for_complete_input() { + let mut carry = Vec::new(); + assert_eq!( + decode_utf8_with_carry(&mut carry, "plain ascii ✓".as_bytes()), + "plain ascii ✓" + ); + assert!(carry.is_empty()); + } + + #[test] + fn replaces_genuinely_invalid_bytes_without_stalling() { + // 0xFF is never valid UTF-8; it must become U+FFFD and not be carried. + let mut carry = Vec::new(); + let out = decode_utf8_with_carry(&mut carry, &[b'a', 0xFF, b'b']); + assert_eq!(out, "a\u{FFFD}b"); + assert!(carry.is_empty()); + } + #[test] fn resolves_requested_cwd_when_directory_exists() { let dir = tempfile::tempdir().expect("temp dir"); diff --git a/apps/desktop/package-lock.json b/apps/desktop/package-lock.json index 0d006e08..835e64da 100644 --- a/apps/desktop/package-lock.json +++ b/apps/desktop/package-lock.json @@ -33,6 +33,7 @@ "@lezer/highlight": "^1.2.3", "@xterm/addon-fit": "^0.11.0", "@xterm/addon-search": "^0.16.0", + "@xterm/addon-serialize": "^0.14.0", "@xterm/addon-web-links": "^0.12.0", "@xterm/addon-webgl": "^0.19.0", "@xterm/xterm": "^6.0.0", @@ -4600,6 +4601,12 @@ "integrity": "sha512-9OeuBFu0/uZJPu+9AHKY6g/w0Czyb/Ut0A5t79I4ULoU4IfU5BEpPFVGQxP4zTTMdfZEYkVIRYbHBX1xWwjeSA==", "license": "MIT" }, + "node_modules/@xterm/addon-serialize": { + "version": "0.14.0", + "resolved": "https://registry.npmjs.org/@xterm/addon-serialize/-/addon-serialize-0.14.0.tgz", + "integrity": "sha512-uteyTU1EkrQa2Ux6P/uFl2fzmXI46jy5uoQMKEOM0fKTyiW7cSn0WrFenHm5vO5uEXX/GpwW/FgILvv3r0WbkA==", + "license": "MIT" + }, "node_modules/@xterm/addon-web-links": { "version": "0.12.0", "resolved": "https://registry.npmjs.org/@xterm/addon-web-links/-/addon-web-links-0.12.0.tgz", diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 9c3f8add..dfac98fe 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -60,6 +60,7 @@ "@lezer/highlight": "^1.2.3", "@xterm/addon-fit": "^0.11.0", "@xterm/addon-search": "^0.16.0", + "@xterm/addon-serialize": "^0.14.0", "@xterm/addon-web-links": "^0.12.0", "@xterm/addon-webgl": "^0.19.0", "@xterm/xterm": "^6.0.0", diff --git a/apps/desktop/src/features/ai/components/AIAuthTerminalModal.tsx b/apps/desktop/src/features/ai/components/AIAuthTerminalModal.tsx index 6dbe0d75..4cc10441 100644 --- a/apps/desktop/src/features/ai/components/AIAuthTerminalModal.tsx +++ b/apps/desktop/src/features/ai/components/AIAuthTerminalModal.tsx @@ -14,6 +14,7 @@ import { appendTerminalRawOutput } from "../../terminal/terminalRawOutput"; import { TerminalViewport } from "../../terminal/TerminalViewport"; import { EMPTY_TERMINAL_SNAPSHOT, + type TerminalOutputCommand, type TerminalSessionView, } from "../../terminal/terminalTypes"; import { APP_BRAND_NAME } from "../../../app/utils/branding"; @@ -98,6 +99,37 @@ export function AIAuthTerminalModal({ const [rawOutput, setRawOutput] = useState(""); const [busy, setBusy] = useState(false); const onRefreshSetupRef = useRef(onRefreshSetup); + // Local output channel feeding the viewport (it pipes commands straight to + // xterm). rawOutput state is kept separately, only for auth-success scanning. + const outputListenersRef = useRef( + new Set<(command: TerminalOutputCommand) => void>(), + ); + const outputBacklogRef = useRef([]); + const replaySnapshotRef = useRef(null); + + const emitOutputCommand = useCallback((command: TerminalOutputCommand) => { + const listeners = outputListenersRef.current; + if (listeners.size === 0) { + outputBacklogRef.current.push(command); + return; + } + for (const listener of listeners) { + listener(command); + } + }, []); + + // Reset the viewport screen and feed it an initial buffer (e.g. on (re)start). + const resetOutputWithBuffer = useCallback( + (buffer: string) => { + replaySnapshotRef.current = null; + outputBacklogRef.current = []; + emitOutputCommand({ type: "clear" }); + if (buffer) { + emitOutputCommand({ type: "write", data: buffer }); + } + }, + [emitOutputCommand], + ); useEffect(() => { onRefreshSetupRef.current = onRefreshSetup; @@ -137,6 +169,7 @@ export function AIAuthTerminalModal({ if (payload.sessionId !== sessionIdRef.current) return; setSnapshot(payload); setRawOutput(payload.buffer); + resetOutputWithBuffer(payload.buffer); setBusy(false); }), ), @@ -146,6 +179,10 @@ export function AIAuthTerminalModal({ setRawOutput((current) => appendTerminalRawOutput(current, payload.chunk), ); + emitOutputCommand({ + type: "write", + data: payload.chunk, + }); }), ), registerListener( @@ -173,6 +210,7 @@ export function AIAuthTerminalModal({ const startSession = async () => { setBusy(true); setRawOutput(""); + resetOutputWithBuffer(""); setSnapshot((current) => ({ ...current, sessionId: "", @@ -197,6 +235,7 @@ export function AIAuthTerminalModal({ sessionIdRef.current = nextSnapshot.sessionId; setSnapshot(nextSnapshot); setRawOutput(nextSnapshot.buffer); + resetOutputWithBuffer(nextSnapshot.buffer); setBusy(false); } catch (error) { if (disposed) return; @@ -216,6 +255,7 @@ export function AIAuthTerminalModal({ } void startSession(); }); + // emitOutputCommand and resetOutputWithBuffer are stable (useCallback). return () => { disposed = true; @@ -230,7 +270,15 @@ export function AIAuthTerminalModal({ void unlisten(); }); }; - }, [open, runtimeId, methodId, vaultPath, customBinaryPath]); + }, [ + open, + runtimeId, + methodId, + vaultPath, + customBinaryPath, + emitOutputCommand, + resetOutputWithBuffer, + ]); const handleClose = useCallback(() => { const sessionId = sessionIdRef.current; @@ -250,6 +298,7 @@ export function AIAuthTerminalModal({ } setRawOutput(""); + resetOutputWithBuffer(""); setSnapshot(buildInitialSnapshot(runtimeId, runtimeName, vaultPath)); setBusy(true); @@ -263,6 +312,7 @@ export function AIAuthTerminalModal({ sessionIdRef.current = nextSnapshot.sessionId; setSnapshot(nextSnapshot); setRawOutput(nextSnapshot.buffer); + resetOutputWithBuffer(nextSnapshot.buffer); setBusy(false); } catch (error) { setSnapshot((current) => ({ @@ -279,12 +329,13 @@ export function AIAuthTerminalModal({ runtimeId, runtimeName, vaultPath, + resetOutputWithBuffer, ]); const sessionView = useMemo( () => ({ snapshot, - rawOutput, + hasOutput: rawOutput.length > 0, busy, writeInput: async (input) => { const sessionId = sessionIdRef.current; @@ -304,9 +355,28 @@ export function AIAuthTerminalModal({ restart: handleRetry, clearViewport: () => { setRawOutput(""); + resetOutputWithBuffer(""); + }, + subscribeOutput: (listener) => { + const listeners = outputListenersRef.current; + if (listeners.size === 0 && outputBacklogRef.current.length > 0) { + const pending = outputBacklogRef.current; + outputBacklogRef.current = []; + for (const command of pending) { + listener(command); + } + } + listeners.add(listener); + return () => { + listeners.delete(listener); + }; + }, + getReplaySnapshot: () => replaySnapshotRef.current, + saveReplaySnapshot: (serialized) => { + replaySnapshotRef.current = serialized; }, }), - [snapshot, rawOutput, busy, handleRetry], + [snapshot, rawOutput, busy, handleRetry, resetOutputWithBuffer], ); if (!open) return null; diff --git a/apps/desktop/src/features/editor/EditorPaneContent.test.tsx b/apps/desktop/src/features/editor/EditorPaneContent.test.tsx index c9b0a8f9..8ea42266 100644 --- a/apps/desktop/src/features/editor/EditorPaneContent.test.tsx +++ b/apps/desktop/src/features/editor/EditorPaneContent.test.tsx @@ -42,12 +42,20 @@ function seedTerminalRuntime(terminalId: string, rawOutput = "ready\n") { snapshot: makeSnapshot({ sessionId: `session-${terminalId}`, }), - rawOutput, + hasOutput: false, busy: false, launchError: null, }, }, }); + // Output is piped through the channel; emitted now, it buffers until the + // viewport mounts and subscribes, then lands in xterm. + if (rawOutput) { + useTerminalRuntimeStore.getState().handleTerminalOutput({ + sessionId: `session-${terminalId}`, + chunk: rawOutput, + }); + } } function getEditorView() { diff --git a/apps/desktop/src/features/terminal/TerminalViewport.test.tsx b/apps/desktop/src/features/terminal/TerminalViewport.test.tsx index da6d6926..8f5319b2 100644 --- a/apps/desktop/src/features/terminal/TerminalViewport.test.tsx +++ b/apps/desktop/src/features/terminal/TerminalViewport.test.tsx @@ -6,14 +6,30 @@ import { renderComponent, } from "../../test/test-utils"; import { TerminalViewport } from "./TerminalViewport"; -import type { TerminalSessionView } from "./terminalTypes"; +import type { + TerminalOutputCommand, + TerminalSessionView, +} from "./terminalTypes"; const DICTATION_PLACEHOLDER = "Speak or type — Enter to send"; -function createSessionView( - overrides: Partial = {}, -): TerminalSessionView { - return { +// Build a session view backed by a controllable output channel that mirrors the +// runtime store's emitter: commands buffered before the viewport subscribes are +// flushed on subscribe, exactly like production. +function createSession(overrides: Partial = {}) { + const listeners = new Set<(command: TerminalOutputCommand) => void>(); + const backlog: TerminalOutputCommand[] = []; + let replaySnapshot: string | null = null; + + const emit = (command: TerminalOutputCommand) => { + if (listeners.size === 0) { + backlog.push(command); + return; + } + for (const listener of listeners) listener(command); + }; + + const session: TerminalSessionView = { snapshot: { sessionId: "devterm-1", program: "/bin/zsh", @@ -25,42 +41,70 @@ function createSessionView( exitCode: null, errorMessage: null, }, - rawOutput: "hello from terminal\nready", + hasOutput: true, busy: false, writeInput: vi.fn(async () => undefined), resize: vi.fn(async () => undefined), restart: vi.fn(async () => undefined), clearViewport: vi.fn(), + subscribeOutput: (listener) => { + if (listeners.size === 0 && backlog.length > 0) { + const pending = backlog.splice(0, backlog.length); + for (const command of pending) listener(command); + } + listeners.add(listener); + return () => { + listeners.delete(listener); + }; + }, + getReplaySnapshot: () => replaySnapshot, + saveReplaySnapshot: (serialized) => { + replaySnapshot = serialized; + }, ...overrides, }; + + return { session, emit }; +} + +// Thin wrapper for tests that only need the session view itself (e.g. the +// dictation UI), not the output channel. +function createSessionView( + overrides: Partial = {}, +): TerminalSessionView { + return createSession(overrides).session; } describe("TerminalViewport", () => { - it("renders raw output and forwards xterm input and settled resize events", async () => { + it("writes piped output to xterm, fires initial PTY resize immediately, and forwards xterm input", async () => { const writeInput = vi.fn(async () => undefined); const resize = vi.fn(async () => undefined); vi.useFakeTimers(); try { - renderComponent( - , - ); + const { session, emit } = createSession({ writeInput, resize }); + renderComponent(); await flushPromises(); - expect(screen.getByText(/hello from terminal/i)).toBeInTheDocument(); + act(() => { + emit({ type: "write", data: "hello from terminal\nready" }); + }); + + expect( + screen.getByText(/hello from terminal/i), + ).toBeInTheDocument(); expect(screen.getByText(/ready/i)).toBeInTheDocument(); - expect(resize).not.toHaveBeenCalled(); + // First fit fires immediately — no debounce on initial mount. + expect(resize).toHaveBeenCalledOnce(); + expect(resize).toHaveBeenCalledWith(80, 24); + + // Advancing time triggers the rAF-driven syncSize, but the size is + // already recorded so the dedup check prevents a second call. await act(async () => { vi.advanceTimersByTime(100); }); - - expect(resize).toHaveBeenCalledWith(80, 24); + expect(resize).toHaveBeenCalledTimes(1); act(() => { getXtermMockInstances()[0]?.emitData("pwd\r"); @@ -72,6 +116,18 @@ describe("TerminalViewport", () => { } }); + it("flushes output buffered before the viewport subscribes", async () => { + const { session, emit } = createSession(); + + // Emitted before render → goes to the backlog, then flushed on subscribe. + emit({ type: "write", data: "buffered output\n" }); + + renderComponent(); + await flushPromises(); + + expect(screen.getByText(/buffered output/i)).toBeInTheDocument(); + }); + it("coalesces noisy resize observer updates into a single PTY resize", async () => { const resize = vi.fn(async () => undefined); const originalResizeObserver = globalThis.ResizeObserver; @@ -108,23 +164,23 @@ describe("TerminalViewport", () => { }); try { - renderComponent( - , - ); + const { session } = createSession({ resize }); + renderComponent(); await flushPromises(); + // Initial fit fires immediately on mount. + expect(resize).toHaveBeenCalledOnce(); + expect(resize).toHaveBeenCalledWith(80, 24); + act(() => { MockResizeObserver.notifyAll(); MockResizeObserver.notifyAll(); MockResizeObserver.notifyAll(); }); - expect(resize).not.toHaveBeenCalled(); - + // Three noisy ResizeObserver callbacks report the same 80×24 that + // was already sent, so the dedup check absorbs them with no new + // debounce timer. await act(async () => { vi.advanceTimersByTime(100); }); @@ -143,79 +199,124 @@ describe("TerminalViewport", () => { }); it("can keep the first terminal output scrolled to the top", async () => { + const { session, emit } = createSession(); renderComponent( - , + , ); await flushPromises(); + act(() => { + emit({ + type: "write", + data: "first line\nsecond line\nthird line", + }); + }); + expect(getXtermMockInstances()[0]?.scrollToTopCalls).toBe(1); }); - it("queues chunks to a local backlog when the xterm write buffer is above the high watermark", async () => { - const { rerender } = renderComponent( - , - ); + it("restores the replay snapshot on mount before live output", async () => { + const { session, emit } = createSession(); + session.saveReplaySnapshot("restored screen state"); + + renderComponent(); await flushPromises(); - const [term] = getXtermMockInstances(); - if (!term) throw new Error("No xterm instance"); + act(() => { + emit({ type: "write", data: " + live tail" }); + }); - // Make write async so pendingWriteCharsRef doesn't immediately drain. - const pendingCallbacks: Array<() => void> = []; - vi.spyOn(term, "write").mockImplementation( - (text: string, callback?: () => void) => { - if (term.screen) { - term.screen.textContent = - (term.screen.textContent ?? "") + text; - } - if (callback) pendingCallbacks.push(callback); - }, + const [term] = getXtermMockInstances(); + expect(term?.screen?.textContent).toBe( + "restored screen state + live tail", ); + }); - // Feed a chunk just above HIGH_WATERMARK (256_000). The write is - // dispatched (pending was 0) but the callback is held, so - // pendingWriteCharsRef stays at 257_000. - const bigChunk = "x".repeat(257_000); - rerender( - , + it("saves a serialized snapshot when unmounting", async () => { + const saveReplaySnapshot = vi.fn(); + const { session, emit } = createSession({ saveReplaySnapshot }); + const { unmount } = renderComponent( + , ); await flushPromises(); - expect(pendingCallbacks).toHaveLength(1); + act(() => { + emit({ type: "write", data: "persist me" }); + }); - // Push a second chunk while the first is still in-flight. - // pendingWriteCharsRef (257_000) > HIGH_WATERMARK (256_000) → backlog. - const smallChunk = "hello"; - rerender( - , - ); + unmount(); + + // The serialize addon mock returns the xterm screen content, so the + // final snapshot captures what was on screen at teardown. + expect(saveReplaySnapshot).toHaveBeenLastCalledWith("persist me"); + }); + + it("clears the xterm screen on a clear command", async () => { + const { session, emit } = createSession(); + renderComponent(); await flushPromises(); - expect(pendingCallbacks).toHaveLength(1); // no new write call - expect(term.screen?.textContent).toBe(bigChunk); // "hello" not visible yet + const [term] = getXtermMockInstances(); + if (!term) throw new Error("No xterm instance"); + + act(() => { + emit({ type: "write", data: "some output" }); + }); + expect(term.screen?.textContent).toBe("some output"); - // Fire the first write's callback → pendingWriteCharsRef drops to 0, - // queueMicrotask(flushBacklog) drains the "hello" backlog entry. - await act(async () => { - pendingCallbacks.shift()?.(); - await Promise.resolve(); - await Promise.resolve(); // let the queueMicrotask(flushBacklog) run + const resetSpy = vi.spyOn(term, "reset"); + act(() => { + emit({ type: "clear" }); }); - // "hello" is now visible — backlog was drained. - expect(term.screen?.textContent).toBe(bigChunk + smallChunk); - expect(pendingCallbacks).toHaveLength(1); // the backlog write's callback + expect(resetSpy).toHaveBeenCalledOnce(); + expect(term.screen?.textContent).toBe(""); + }); + + it("sends \\n to the PTY on Shift+Enter and does not intercept plain Enter or keyup", async () => { + const writeInput = vi.fn(async () => undefined); + + const { session } = createSession({ writeInput }); + renderComponent(); + await flushPromises(); + + const terminal = getXtermMockInstances()[0]; + expect(terminal).toBeDefined(); + + const makeKey = ( + key: string, + overrides: Partial = {}, + ) => + new KeyboardEvent("keydown", { + key, + shiftKey: false, + bubbles: true, + ...overrides, + }); + + // Shift+Enter keydown → intercepted: writes \n and returns false. + const shiftEnter = makeKey("Enter", { shiftKey: true }); + const shiftEnterResult = terminal!.triggerKeyEvent(shiftEnter); + expect(shiftEnterResult).toBe(false); + expect(writeInput).toHaveBeenCalledWith("\n"); + + writeInput.mockClear(); + + // Plain Enter → not intercepted: xterm handles it normally. + const plainEnter = makeKey("Enter"); + const plainEnterResult = terminal!.triggerKeyEvent(plainEnter); + expect(plainEnterResult).toBe(true); + expect(writeInput).not.toHaveBeenCalled(); + + // Shift+Enter keyup → not intercepted (handler only fires on keydown). + const shiftEnterKeyUp = new KeyboardEvent("keyup", { + key: "Enter", + shiftKey: true, + bubbles: true, + }); + const keyUpResult = terminal!.triggerKeyEvent(shiftEnterKeyUp); + expect(keyUpResult).toBe(true); + expect(writeInput).not.toHaveBeenCalled(); }); it("opens dictation from the context menu, cancels cleanly, and sends entered text", async () => { diff --git a/apps/desktop/src/features/terminal/TerminalViewport.tsx b/apps/desktop/src/features/terminal/TerminalViewport.tsx index d5c658a6..dea4de5b 100644 --- a/apps/desktop/src/features/terminal/TerminalViewport.tsx +++ b/apps/desktop/src/features/terminal/TerminalViewport.tsx @@ -2,6 +2,7 @@ import "@xterm/xterm/css/xterm.css"; import { FitAddon } from "@xterm/addon-fit"; import { SearchAddon } from "@xterm/addon-search"; +import { SerializeAddon } from "@xterm/addon-serialize"; import { WebLinksAddon } from "@xterm/addon-web-links"; import { WebglAddon } from "@xterm/addon-webgl"; import { openUrl } from "@neverwrite/runtime"; @@ -26,7 +27,10 @@ import { import { logError } from "../../app/utils/runtimeLog"; import { getDesktopPlatform } from "../../app/utils/platform"; import { getTerminalTheme } from "./terminalTheme"; -import type { TerminalSessionView } from "./terminalTypes"; +import type { + TerminalOutputCommand, + TerminalSessionView, +} from "./terminalTypes"; function TerminalMessage({ message }: { message: string }) { return ( @@ -77,11 +81,12 @@ function buildSearchSummary(resultIndex: number, resultCount: number) { } const TERMINAL_RESIZE_SETTLE_MS = 80; -// Flow-control watermarks for xterm.js write queue. -// When pending chars exceed HIGH, new chunks are queued locally. -// When pending drops below LOW, the local queue is drained. -const WRITE_HIGH_WATERMARK = 256_000; -const WRITE_LOW_WATERMARK = 64_000; +// Debounce for persisting the xterm buffer snapshot after output settles, so a +// full reload can restore recent screen content without serializing on every +// chunk. Scrollback lines kept in the persisted snapshot are capped to bound +// storage size; in-session remounts use the full in-memory snapshot instead. +const SNAPSHOT_SAVE_DEBOUNCE_MS = 1_000; +const PERSISTED_SNAPSHOT_SCROLLBACK = 1_000; export function TerminalViewport({ active = true, @@ -94,16 +99,23 @@ export function TerminalViewport({ initialScrollPosition?: "top" | "bottom"; session: TerminalSessionView; }) { - const { rawOutput, resize, snapshot, writeInput } = session; + const { hasOutput, resize, snapshot, writeInput } = session; const hostRef = useRef(null); const terminalRef = useRef(null); const fitAddonRef = useRef(null); const searchAddonRef = useRef(null); + const serializeAddonRef = useRef(null); const writeInputRef = useRef(writeInput); const resizeRef = useRef(resize); const snapshotRef = useRef(snapshot); + // Latest session view, so the unmount handler can persist a snapshot without + // capturing a stale closure. + const sessionRef = useRef(session); const syncSizeRef = useRef<() => void>(() => undefined); const resizeTimerRef = useRef | null>(null); + const snapshotSaveTimerRef = useRef | null>( + null, + ); const pendingResizeRef = useRef<{ cols: number; rows: number } | null>( null, ); @@ -111,13 +123,15 @@ export function TerminalViewport({ null, ); const lastSessionIdRef = useRef(null); - const lastRawOutputRef = useRef(""); const lastRestoredFocusSessionIdRef = useRef(null); const shouldApplyInitialScrollRef = useRef(false); const shouldRestoreFocusRef = useRef(false); const webglAddonRef = useRef(null); - const pendingWriteCharsRef = useRef(0); - const writeBacklogRef = useRef([]); + // Set to true after an explicit Shift+Enter write so that onData can drop + // the duplicate \n that Electron's textarea sometimes leaks despite + // event.preventDefault() — the root cause of ghost rows when lines added + // via Shift+Enter are then deleted. + const suppressNextNewlineRef = useRef(false); const searchPanelRef = useRef(null); const searchInputRef = useRef(null); const searchOpenRef = useRef(false); @@ -220,7 +234,8 @@ export function TerminalViewport({ writeInputRef.current = writeInput; resizeRef.current = resize; snapshotRef.current = snapshot; - }, [resize, snapshot, writeInput]); + sessionRef.current = session; + }, [resize, session, snapshot, writeInput]); useEffect(() => { const lastRequestedSize = lastRequestedSizeRef.current; @@ -300,6 +315,26 @@ export function TerminalViewport({ return; } + // First fit after mount: send immediately so the PTY has the + // correct size before Claude Code renders its initial frame. + // Subsequent resizes (pane drags, etc.) still debounce to + // avoid thrashing the PTY on every intermediate pixel. + if (!lastRequestedSizeRef.current) { + if (resizeTimerRef.current) { + clearTimeout(resizeTimerRef.current); + resizeTimerRef.current = null; + } + pendingResizeRef.current = null; + lastRequestedSizeRef.current = { + cols: nextCols, + rows: nextRows, + }; + void resizeRef + .current(nextCols, nextRows) + .catch(() => undefined); + return; + } + pendingResizeRef.current = { cols: nextCols, rows: nextRows }; if (resizeTimerRef.current) { clearTimeout(resizeTimerRef.current); @@ -336,6 +371,12 @@ export function TerminalViewport({ closeSearch(); return false; } + if (event.type === "keydown" && event.key === "Enter" && event.shiftKey) { + event.preventDefault(); + suppressNextNewlineRef.current = true; + void writeInputRef.current("\n").catch(() => undefined); + return false; + } return true; }); let cancelled = false; @@ -350,6 +391,51 @@ export function TerminalViewport({ let handleFocus: (() => void) | null = null; let handleBlur: ((event: FocusEvent) => void) | null = null; let observer: ResizeObserver | null = null; + let unsubscribeOutput: (() => void) | null = null; + + // Persist the xterm buffer (debounced) so a full reload can restore the + // screen. xterm owns the live buffer; this only snapshots it. + const saveSnapshot = () => { + snapshotSaveTimerRef.current = null; + const addon = serializeAddonRef.current; + if (!addon) return; + try { + sessionRef.current.saveReplaySnapshot( + addon.serialize({ + scrollback: PERSISTED_SNAPSHOT_SCROLLBACK, + }), + ); + } catch { + // Serialization is best-effort; a failure just skips this save. + } + }; + const scheduleSnapshotSave = () => { + if (snapshotSaveTimerRef.current) { + clearTimeout(snapshotSaveTimerRef.current); + } + snapshotSaveTimerRef.current = setTimeout( + saveSnapshot, + SNAPSHOT_SAVE_DEBOUNCE_MS, + ); + }; + + // The single output path: PTY chunks are written straight to xterm, in + // order. No accumulation, no diffing, no reset on the hot path. + const handleOutputCommand = (command: TerminalOutputCommand) => { + const t = terminalRef.current; + if (!t) return; + if (command.type === "clear") { + t.reset(); + return; + } + t.write(command.data, () => { + if (shouldApplyInitialScrollRef.current) { + shouldApplyInitialScrollRef.current = false; + t.scrollToTop(); + } + }); + scheduleSnapshotSave(); + }; const finishOpen = () => { if (cancelled) return; @@ -374,11 +460,34 @@ export function TerminalViewport({ ); } + const serializeAddon = new SerializeAddon(); + terminal.loadAddon(serializeAddon); + terminalRef.current = terminal; fitAddonRef.current = fitAddon; searchAddonRef.current = searchAddon; + serializeAddonRef.current = serializeAddon; + + shouldApplyInitialScrollRef.current = + initialScrollPosition === "top"; + + // Restore prior screen content (remount or reload), then subscribe to + // live output. Both go through xterm's write queue in FIFO order, so + // the snapshot lands before any buffered/live chunks. + const replaySnapshot = sessionRef.current.getReplaySnapshot(); + if (replaySnapshot) { + terminal.write(replaySnapshot); + } + unsubscribeOutput = sessionRef.current.subscribeOutput( + handleOutputCommand, + ); onDataDisposable = terminal.onData((data) => { + if (data === "\n" && suppressNextNewlineRef.current) { + suppressNextNewlineRef.current = false; + return; + } + suppressNextNewlineRef.current = false; void writeInputRef .current(data) .catch((error) => @@ -440,6 +549,15 @@ export function TerminalViewport({ return () => { cancelled = true; + // Capture a final snapshot before teardown so a remount restores the + // last screen state. Runs synchronously off the live buffer. + unsubscribeOutput?.(); + unsubscribeOutput = null; + if (snapshotSaveTimerRef.current) { + clearTimeout(snapshotSaveTimerRef.current); + snapshotSaveTimerRef.current = null; + } + saveSnapshot(); observer?.disconnect(); onSearchResultsDisposable?.dispose(); onSelectionDisposable?.dispose(); @@ -450,13 +568,12 @@ export function TerminalViewport({ onDataDisposable?.dispose(); webglAddonRef.current?.dispose(); webglAddonRef.current = null; - pendingWriteCharsRef.current = 0; - writeBacklogRef.current = []; terminal.dispose(); syncSizeRef.current = () => undefined; terminalRef.current = null; fitAddonRef.current = null; searchAddonRef.current = null; + serializeAddonRef.current = null; if (resizeTimerRef.current) { clearTimeout(resizeTimerRef.current); resizeTimerRef.current = null; @@ -464,7 +581,6 @@ export function TerminalViewport({ pendingResizeRef.current = null; lastRequestedSizeRef.current = null; lastSessionIdRef.current = null; - lastRawOutputRef.current = ""; lastRestoredFocusSessionIdRef.current = null; shouldApplyInitialScrollRef.current = false; shouldRestoreFocusRef.current = false; @@ -513,104 +629,24 @@ export function TerminalViewport({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [JSON.stringify(theme)]); + // Reset transient UI state when the underlying session changes (e.g. after a + // restart). The screen wipe itself arrives as a "clear" output command, so + // this only handles viewport bookkeeping, not content. useEffect(() => { - const terminal = terminalRef.current; - if (!terminal) return; - - // Drain the local write backlog, capped at WRITE_LOW_WATERMARK chars per - // iteration so xterm can interleave rendering between flushes. - const flushBacklog = () => { - const t = terminalRef.current; - if (!t || writeBacklogRef.current.length === 0) return; - if (pendingWriteCharsRef.current > WRITE_LOW_WATERMARK) return; - let merged = ""; - while ( - writeBacklogRef.current.length > 0 && - merged.length < WRITE_LOW_WATERMARK - ) { - merged += writeBacklogRef.current.shift()!; - } - const chars = merged.length; - pendingWriteCharsRef.current += chars; - t.write(merged, () => { - pendingWriteCharsRef.current = Math.max( - 0, - pendingWriteCharsRef.current - chars, - ); - queueMicrotask(flushBacklog); - }); - }; - - // Write a chunk to xterm, queueing locally if the write buffer is full. - const writeChunk = (data: string, onDone?: () => void) => { - if (pendingWriteCharsRef.current > WRITE_HIGH_WATERMARK) { - writeBacklogRef.current.push(data); - return; - } - const chars = data.length; - pendingWriteCharsRef.current += chars; - terminal.write(data, () => { - pendingWriteCharsRef.current = Math.max( - 0, - pendingWriteCharsRef.current - chars, - ); - onDone?.(); - queueMicrotask(flushBacklog); - }); - }; - const sessionId = snapshot.sessionId || "__pending-terminal__"; - const previousSessionId = lastSessionIdRef.current; - - if (previousSessionId !== sessionId) { - terminal.reset(); - pendingWriteCharsRef.current = 0; - writeBacklogRef.current = []; - lastSessionIdRef.current = sessionId; - lastRawOutputRef.current = ""; - shouldApplyInitialScrollRef.current = - initialScrollPosition === "top"; - queueMicrotask(() => { - setHasSelection(false); - setSearchResultCount(0); - setSearchResultIndex(-1); - }); - } - - if (rawOutput === lastRawOutputRef.current) { + if (lastSessionIdRef.current === sessionId) { return; } - - if ( - rawOutput.length < lastRawOutputRef.current.length || - !rawOutput.startsWith(lastRawOutputRef.current) - ) { - terminal.reset(); - pendingWriteCharsRef.current = 0; - writeBacklogRef.current = []; - if (rawOutput.length > 0) { - writeChunk(rawOutput, () => { - if (!shouldApplyInitialScrollRef.current) return; - shouldApplyInitialScrollRef.current = false; - terminal.scrollToTop(); - }); - } else { - shouldApplyInitialScrollRef.current = false; - } - lastRawOutputRef.current = rawOutput; + const isFirstSession = lastSessionIdRef.current === null; + lastSessionIdRef.current = sessionId; + if (isFirstSession) { return; } - - const nextChunk = rawOutput.slice(lastRawOutputRef.current.length); - if (nextChunk.length > 0) { - writeChunk(nextChunk, () => { - if (!shouldApplyInitialScrollRef.current) return; - shouldApplyInitialScrollRef.current = false; - terminal.scrollToTop(); - }); - } - lastRawOutputRef.current = rawOutput; - }, [initialScrollPosition, rawOutput, snapshot.sessionId]); + shouldApplyInitialScrollRef.current = initialScrollPosition === "top"; + setHasSelection(false); + setSearchResultCount(0); + setSearchResultIndex(-1); + }, [initialScrollPosition, snapshot.sessionId]); useEffect(() => { if ( @@ -764,12 +800,12 @@ export function TerminalViewport({ { type: "separator" }, { label: "Clear", - disabled: rawOutput.length === 0, + disabled: !hasOutput, action: () => session.clearViewport(), }, ]; - const noOutput = rawOutput.length === 0; + const noOutput = !hasOutput; return (
{ return vi.fn(); }); - // Set up a live terminal session so output is applied to rawOutput. + // Set up a live terminal session so output is delivered to the runtime. vi.mocked(invoke).mockResolvedValue({ sessionId: "devterm-1", program: "/bin/zsh", @@ -191,23 +191,26 @@ describe("WorkspaceTerminalHost", () => { }); expect(capturedRafCallback).not.toBeNull(); - expect( - useTerminalRuntimeStore.getState().runtimesById["terminal-1"] - ?.rawOutput, - ).toBe(""); - // Fire the rAF manually — all three chunks merge into one store update. + // Output is held until the frame fires — no store update yet. + const handleOutputSpy = vi.spyOn( + useTerminalRuntimeStore.getState(), + "handleTerminalOutput", + ); + + // Fire the rAF manually — all three chunks merge into one update. await act(async () => { capturedRafCallback?.(performance.now()); }); - expect( - useTerminalRuntimeStore.getState().runtimesById["terminal-1"] - ?.rawOutput, - ).toBe("hello world!"); + expect(handleOutputSpy).toHaveBeenCalledTimes(1); + expect(handleOutputSpy).toHaveBeenCalledWith({ + sessionId: "devterm-1", + chunk: "hello world!", + }); }); - it("caps pending output while waiting for a delayed rAF frame", async () => { + it("merges pending output for a delayed rAF frame without capping it", async () => { let capturedRafCallback: ((time: DOMHighResTimeStamp) => void) | null = null; vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => { @@ -288,8 +291,10 @@ describe("WorkspaceTerminalHost", () => { expect(handleOutputSpy).toHaveBeenCalledTimes(1); const flushedPayload = handleOutputSpy.mock.calls[0]?.[0]; - expect(flushedPayload?.chunk).toHaveLength(2_000_000); - expect(flushedPayload?.chunk.startsWith("a".repeat(500_000))).toBe( + // No host-side cap — xterm owns the buffer, so all bytes pass through + // intact rather than being trimmed. + expect(flushedPayload?.chunk).toHaveLength(3_000_000); + expect(flushedPayload?.chunk.startsWith("a".repeat(1_500_000))).toBe( true, ); expect(flushedPayload?.chunk.endsWith("b".repeat(1_500_000))).toBe( diff --git a/apps/desktop/src/features/terminal/WorkspaceTerminalHost.tsx b/apps/desktop/src/features/terminal/WorkspaceTerminalHost.tsx index 176a7dd4..ccaca73b 100644 --- a/apps/desktop/src/features/terminal/WorkspaceTerminalHost.tsx +++ b/apps/desktop/src/features/terminal/WorkspaceTerminalHost.tsx @@ -15,7 +15,6 @@ import { type TerminalSessionSnapshot, } from "./terminalTypes"; import { useTerminalRuntimeStore } from "./terminalRuntimeStore"; -import { appendTerminalRawOutput } from "./terminalRawOutput"; import { useEffect } from "react"; import { useShallow } from "zustand/react/shallow"; @@ -53,12 +52,13 @@ export function WorkspaceTerminalHost() { (event) => { if (cancelled) return; const { sessionId, chunk } = event.payload; + // Coalesce same-session chunks within a frame to cut the + // number of handler calls. No cap here — this holds a single + // frame of output, then hands it to xterm, which owns the + // buffer. pendingBySessionId.set( sessionId, - appendTerminalRawOutput( - pendingBySessionId.get(sessionId) ?? "", - chunk, - ), + (pendingBySessionId.get(sessionId) ?? "") + chunk, ); if (rafId === null) { rafId = requestAnimationFrame(flushPending); diff --git a/apps/desktop/src/features/terminal/claudeCodeTerminal.test.ts b/apps/desktop/src/features/terminal/claudeCodeTerminal.test.ts index 77b30dd0..a2eea578 100644 --- a/apps/desktop/src/features/terminal/claudeCodeTerminal.test.ts +++ b/apps/desktop/src/features/terminal/claudeCodeTerminal.test.ts @@ -52,7 +52,7 @@ async function attachOpenedTerminalRuntime() { tabId: tab!.id, sessionId: "devterm-1", snapshot: makeRunningSnapshot(), - rawOutput: "", + hasOutput: false, busy: false, launchError: null, }, diff --git a/apps/desktop/src/features/terminal/terminalRuntimeStore.test.ts b/apps/desktop/src/features/terminal/terminalRuntimeStore.test.ts index 96829e85..ae8b83af 100644 --- a/apps/desktop/src/features/terminal/terminalRuntimeStore.test.ts +++ b/apps/desktop/src/features/terminal/terminalRuntimeStore.test.ts @@ -1,8 +1,13 @@ import { invoke } from "../../app/runtime"; import type { TerminalTab } from "../../app/store/editorStore"; import { useSettingsStore } from "../../app/store/settingsStore"; -import type { TerminalSessionSnapshot } from "./terminalTypes"; +import { safeStorageGetItem } from "../../app/utils/safeStorage"; +import type { + TerminalOutputCommand, + TerminalSessionSnapshot, +} from "./terminalTypes"; import { + createTerminalSessionView, resetTerminalRuntimeStoreForTests, useTerminalRuntimeStore, } from "./terminalRuntimeStore"; @@ -60,6 +65,22 @@ function getRuntime(terminalId = "terminal-1") { return useTerminalRuntimeStore.getState().runtimesById[terminalId] ?? null; } +// Subscribe to a terminal's output channel and collect emitted commands. +function collectOutput(terminalId = "terminal-1") { + const runtime = getRuntime(terminalId); + if (!runtime) throw new Error(`No runtime for ${terminalId}`); + const commands: TerminalOutputCommand[] = []; + const unsubscribe = createTerminalSessionView(runtime).subscribeOutput( + (command) => commands.push(command), + ); + const writes = () => + commands + .filter((c): c is { type: "write"; data: string } => c.type === "write") + .map((c) => c.data) + .join(""); + return { commands, writes, unsubscribe }; +} + describe("terminalRuntimeStore", () => { beforeEach(() => { resetTerminalRuntimeStoreForTests(); @@ -89,9 +110,14 @@ describe("terminalRuntimeStore", () => { expect(getRuntime()).toMatchObject({ sessionId: "devterm-1", - rawOutput: "early output\n", + hasOutput: true, busy: false, }); + + // The buffered output is replayed through the output channel, flushed + // to the first subscriber. + const { writes } = collectOutput(); + expect(writes()).toBe("early output\n"); }); it("adds Claude Code fullscreen rendering env to newly created sessions when enabled", async () => { @@ -173,6 +199,10 @@ describe("terminalRuntimeStore", () => { useTerminalRuntimeStore.getState().ensureTerminal(makeTerminalTab()); await flushPromises(); + + // Watch the output channel live across the restart. + const { commands, writes } = collectOutput(); + useTerminalRuntimeStore.getState().handleTerminalOutput({ sessionId: "devterm-1", chunk: "before restart\n", @@ -181,6 +211,7 @@ describe("terminalRuntimeStore", () => { const restartPromise = useTerminalRuntimeStore .getState() .restart("terminal-1"); + // Output from the dying process is suppressed and never reaches xterm. useTerminalRuntimeStore.getState().handleTerminalOutput({ sessionId: "devterm-1", chunk: "old process output\n", @@ -188,13 +219,114 @@ describe("terminalRuntimeStore", () => { restartSession.resolve(makeSnapshot({ sessionId: "devterm-1" })); await restartPromise; - expect(getRuntime()?.rawOutput).toBe(""); + // Restart wipes the screen via a clear command and resets the flag. + expect(commands.some((c) => c.type === "clear")).toBe(true); + expect(getRuntime()?.hasOutput).toBe(false); useTerminalRuntimeStore.getState().handleTerminalOutput({ sessionId: "devterm-1", chunk: "new process output\n", }); - expect(getRuntime()?.rawOutput).toBe("new process output\n"); + expect(getRuntime()?.hasOutput).toBe(true); + // Old process output dropped; only pre-restart and post-restart remain. + expect(writes()).toBe("before restart\nnew process output\n"); + }); + + it("delivers live output to a subscriber in arrival order", async () => { + vi.mocked(invoke).mockResolvedValue( + makeSnapshot({ sessionId: "devterm-1" }), + ); + useTerminalRuntimeStore.getState().ensureTerminal(makeTerminalTab()); + await flushPromises(); + + const { writes } = collectOutput(); + const emit = (chunk: string) => + useTerminalRuntimeStore + .getState() + .handleTerminalOutput({ sessionId: "devterm-1", chunk }); + + emit("one "); + emit("two "); + emit("three"); + + expect(writes()).toBe("one two three"); + expect(getRuntime()?.hasOutput).toBe(true); + }); + + it("clears the viewport: emits a clear command, resets hasOutput, and drops the snapshot", async () => { + vi.mocked(invoke).mockResolvedValue( + makeSnapshot({ sessionId: "devterm-1" }), + ); + useTerminalRuntimeStore.getState().ensureTerminal(makeTerminalTab()); + await flushPromises(); + + const { commands } = collectOutput(); + useTerminalRuntimeStore.getState().handleTerminalOutput({ + sessionId: "devterm-1", + chunk: "output\n", + }); + expect(getRuntime()?.hasOutput).toBe(true); + + const view = createTerminalSessionView(getRuntime()!); + view.saveReplaySnapshot("snapshot"); + expect(view.getReplaySnapshot()).toBe("snapshot"); + + useTerminalRuntimeStore.getState().clear("terminal-1"); + + expect(commands.some((command) => command.type === "clear")).toBe(true); + expect(getRuntime()?.hasOutput).toBe(false); + expect( + createTerminalSessionView(getRuntime()!).getReplaySnapshot(), + ).toBeNull(); + }); + + it("round-trips the replay snapshot and persists it for reload", async () => { + vi.mocked(invoke).mockResolvedValue( + makeSnapshot({ sessionId: "devterm-1" }), + ); + useTerminalRuntimeStore.getState().ensureTerminal(makeTerminalTab()); + await flushPromises(); + + const view = createTerminalSessionView(getRuntime()!); + expect(view.getReplaySnapshot()).toBeNull(); + + view.saveReplaySnapshot("serialized buffer"); + + expect(view.getReplaySnapshot()).toBe("serialized buffer"); + // Persisted to storage too, so it survives a full reload (the in-memory + // copy is gone once the renderer restarts). + expect( + safeStorageGetItem("neverwrite.terminal.replay:terminal-1"), + ).toBe("serialized buffer"); + }); + + it("tears down the output channel and snapshot when a terminal closes", async () => { + vi.mocked(invoke).mockResolvedValue( + makeSnapshot({ sessionId: "devterm-1" }), + ); + useTerminalRuntimeStore.getState().ensureTerminal(makeTerminalTab()); + await flushPromises(); + + const view = createTerminalSessionView(getRuntime()!); + view.saveReplaySnapshot("snapshot"); + // Output buffered with no subscriber attached. + useTerminalRuntimeStore.getState().handleTerminalOutput({ + sessionId: "devterm-1", + chunk: "buffered\n", + }); + + await useTerminalRuntimeStore.getState().closeTerminal("terminal-1"); + + expect(getRuntime()).toBeNull(); + expect( + safeStorageGetItem("neverwrite.terminal.replay:terminal-1"), + ).toBeNull(); + + // Subscribing to the same id after close yields a fresh channel with no + // leftover buffered output. + const received: TerminalOutputCommand[] = []; + view.subscribeOutput((command) => received.push(command)); + expect(received).toHaveLength(0); }); it("does not spam resize commands for an unchanged or already pending size", async () => { diff --git a/apps/desktop/src/features/terminal/terminalRuntimeStore.ts b/apps/desktop/src/features/terminal/terminalRuntimeStore.ts index dea2c6e8..afc6b45f 100644 --- a/apps/desktop/src/features/terminal/terminalRuntimeStore.ts +++ b/apps/desktop/src/features/terminal/terminalRuntimeStore.ts @@ -1,7 +1,11 @@ import { invoke } from "../../app/runtime"; import type { TerminalTab } from "../../app/store/editorStore"; import { useSettingsStore } from "../../app/store/settingsStore"; -import { appendTerminalRawOutput } from "./terminalRawOutput"; +import { + safeStorageGetItem, + safeStorageRemoveItem, + safeStorageTrySetItem, +} from "../../app/utils/safeStorage"; import { allocateTabSessionVersion, collectSessionIdsToClose, @@ -10,6 +14,7 @@ import { import { EMPTY_TERMINAL_SNAPSHOT, type TerminalErrorEventPayload, + type TerminalOutputCommand, type TerminalOutputEventPayload, type TerminalSessionCreateInput, type TerminalSessionSnapshot, @@ -22,7 +27,9 @@ export interface WorkspaceTerminalRuntime { tabId: string; sessionId: string | null; snapshot: TerminalSessionSnapshot; - rawOutput: string; + // Whether any output has been produced this session. Screen content itself + // lives in the xterm instance — this only drives empty-state UI. + hasOutput: boolean; busy: boolean; launchError: string | null; } @@ -58,6 +65,119 @@ const pendingResizeByTerminalId = new Map< const suppressedOutputSessionIds = new Map(); const nextTerminalSessionVersionRef = { current: 1 }; +// --- Live output delivery ------------------------------------------------- +// Output is piped straight to the mounted xterm viewport via per-terminal +// command channels rather than accumulated in React state and diffed. A small +// backlog buffers commands emitted before the viewport's first subscriber +// attaches (the brief window between session start and mount); it is flushed on +// subscribe. The backlog is bounded so a never-mounted terminal can't grow it +// without limit. +const MAX_BOOTSTRAP_BACKLOG_CHARS = 256_000; + +interface TerminalOutputChannel { + listeners: Set<(command: TerminalOutputCommand) => void>; + backlog: TerminalOutputCommand[]; + backlogChars: number; +} + +const outputChannelsByTerminalId = new Map(); + +function getOutputChannel(terminalId: string): TerminalOutputChannel { + let channel = outputChannelsByTerminalId.get(terminalId); + if (!channel) { + channel = { listeners: new Set(), backlog: [], backlogChars: 0 }; + outputChannelsByTerminalId.set(terminalId, channel); + } + return channel; +} + +function emitOutputCommand(terminalId: string, command: TerminalOutputCommand) { + const channel = getOutputChannel(terminalId); + if (channel.listeners.size === 0) { + channel.backlog.push(command); + if (command.type === "write") { + channel.backlogChars += command.data.length; + } + // Drop oldest commands once the bootstrap window overflows. This only + // bites if a terminal produces output but is never mounted. + while ( + channel.backlogChars > MAX_BOOTSTRAP_BACKLOG_CHARS && + channel.backlog.length > 1 + ) { + const dropped = channel.backlog.shift(); + if (dropped?.type === "write") { + channel.backlogChars -= dropped.data.length; + } + } + return; + } + for (const listener of channel.listeners) { + listener(command); + } +} + +function subscribeOutputChannel( + terminalId: string, + listener: (command: TerminalOutputCommand) => void, +): () => void { + const channel = getOutputChannel(terminalId); + // Flush any commands buffered before the first subscriber attached. + if (channel.listeners.size === 0 && channel.backlog.length > 0) { + const pending = channel.backlog; + channel.backlog = []; + channel.backlogChars = 0; + for (const command of pending) { + listener(command); + } + } + channel.listeners.add(listener); + return () => { + channel.listeners.delete(listener); + }; +} + +function resetOutputChannel(terminalId: string) { + const channel = outputChannelsByTerminalId.get(terminalId); + if (channel) { + channel.backlog = []; + channel.backlogChars = 0; + } +} + +// --- Reattach snapshots --------------------------------------------------- +// A serialized xterm buffer per terminal, written back on mount to restore +// screen content. Kept in memory for fast in-session remounts (pane reshuffle) +// and mirrored to persistent storage so content survives a full reload. The +// terminalId is stable across reloads (it is persisted with the tab). +const REPLAY_SNAPSHOT_STORAGE_PREFIX = "neverwrite.terminal.replay:"; +const replaySnapshotsByTerminalId = new Map(); + +function replaySnapshotStorageKey(terminalId: string) { + return `${REPLAY_SNAPSHOT_STORAGE_PREFIX}${terminalId}`; +} + +function getReplaySnapshot(terminalId: string): string | null { + const inMemory = replaySnapshotsByTerminalId.get(terminalId); + if (inMemory !== undefined) { + return inMemory; + } + return safeStorageGetItem(replaySnapshotStorageKey(terminalId)); +} + +function saveReplaySnapshot(terminalId: string, serialized: string) { + if (!serialized) { + clearReplaySnapshot(terminalId); + return; + } + replaySnapshotsByTerminalId.set(terminalId, serialized); + safeStorageTrySetItem(replaySnapshotStorageKey(terminalId), serialized); +} + +function clearReplaySnapshot(terminalId: string) { + replaySnapshotsByTerminalId.delete(terminalId); + safeStorageRemoveItem(replaySnapshotStorageKey(terminalId)); +} + function createRuntimeSnapshot(cwd: string | null): TerminalSessionSnapshot { return { ...EMPTY_TERMINAL_SNAPSHOT, @@ -73,7 +193,7 @@ function createInitialRuntime(tab: TerminalTab): WorkspaceTerminalRuntime { tabId: tab.id, sessionId: null, snapshot: createRuntimeSnapshot(tab.cwd), - rawOutput: "", + hasOutput: false, busy: true, launchError: null, }; @@ -180,12 +300,7 @@ async function createSessionForTerminal( ...current, sessionId: next.sessionId, snapshot: next, - rawOutput: bufferedRaw - ? appendTerminalRawOutput( - current.rawOutput, - bufferedRaw, - ) - : current.rawOutput, + hasOutput: current.hasOutput || bufferedRaw.length > 0, busy: false, launchError: null, }, @@ -193,6 +308,12 @@ async function createSessionForTerminal( }; }); + // Output that raced ahead of the runtime gaining its sessionId is piped + // through now, in order, ahead of any live chunks. + if (bufferedRaw.length > 0) { + emitOutputCommand(terminalId, { type: "write", data: bufferedRaw }); + } + return next; } catch (error) { useTerminalRuntimeStore.setState((state) => { @@ -381,6 +502,11 @@ export const useTerminalRuntimeStore = create( suppressedOutputSessionIds.set(previousSessionId, true); } pendingResizeByTerminalId.delete(terminalId); + // The previous session's screen is gone — wipe xterm, drop any + // buffered output, and discard the reattach snapshot. + resetOutputChannel(terminalId); + clearReplaySnapshot(terminalId); + emitOutputCommand(terminalId, { type: "clear" }); set((state) => { const current = state.runtimesById[terminalId]; @@ -391,7 +517,7 @@ export const useTerminalRuntimeStore = create( ...state.runtimesById, [terminalId]: { ...current, - rawOutput: "", + hasOutput: false, busy: true, launchError: null, snapshot: { @@ -437,7 +563,7 @@ export const useTerminalRuntimeStore = create( ...current, sessionId: next.sessionId, snapshot: next, - rawOutput: "", + hasOutput: false, busy: false, launchError: null, }, @@ -471,16 +597,19 @@ export const useTerminalRuntimeStore = create( }, clear: (terminalId) => { + resetOutputChannel(terminalId); + clearReplaySnapshot(terminalId); + emitOutputCommand(terminalId, { type: "clear" }); set((state) => { const runtime = state.runtimesById[terminalId]; - if (!runtime || runtime.rawOutput.length === 0) return state; + if (!runtime || !runtime.hasOutput) return state; return { runtimesById: { ...state.runtimesById, [terminalId]: { ...runtime, - rawOutput: "", + hasOutput: false, }, }, }; @@ -494,6 +623,8 @@ export const useTerminalRuntimeStore = create( allocateTerminalSessionVersion(terminalId); deleteTabSessionVersions(terminalSessionVersions, [terminalId]); pendingResizeByTerminalId.delete(terminalId); + outputChannelsByTerminalId.delete(terminalId); + clearReplaySnapshot(terminalId); set((state) => { const { [terminalId]: _removed, ...remaining } = @@ -529,34 +660,35 @@ export const useTerminalRuntimeStore = create( if (retiredSessionIds.has(sessionId)) return; if (suppressedOutputSessionIds.has(sessionId)) return; - set((state) => { - const runtime = getRuntimeBySessionId( - state.runtimesById, - sessionId, - ); + const runtime = getRuntimeBySessionId(get().runtimesById, sessionId); - if (!runtime) { - const existing = pendingOutputBySessionId.get(sessionId) ?? ""; - pendingOutputBySessionId.set( - sessionId, - appendTerminalRawOutput(existing, chunk), - ); - return state; - } + if (!runtime) { + // Output arrived before the runtime learned its sessionId. Hold + // it briefly; createSessionForTerminal replays it on attach. + const existing = pendingOutputBySessionId.get(sessionId) ?? ""; + pendingOutputBySessionId.set(sessionId, existing + chunk); + return; + } - return { - runtimesById: { - ...state.runtimesById, - [runtime.terminalId]: { - ...runtime, - rawOutput: appendTerminalRawOutput( - runtime.rawOutput, - chunk, - ), + // Pipe straight to the mounted viewport. No accumulation in state. + emitOutputCommand(runtime.terminalId, { type: "write", data: chunk }); + + // Flip the empty-state flag exactly once, on the first chunk. + if (!runtime.hasOutput) { + set((state) => { + const current = state.runtimesById[runtime.terminalId]; + if (!current || current.hasOutput) return state; + return { + runtimesById: { + ...state.runtimesById, + [runtime.terminalId]: { + ...current, + hasOutput: true, + }, }, - }, - }; - }); + }; + }); + } }, handleTerminalStarted: (snapshot) => { @@ -608,6 +740,8 @@ export function resetTerminalRuntimeStoreForTests() { retiredSessionIds.clear(); pendingResizeByTerminalId.clear(); suppressedOutputSessionIds.clear(); + outputChannelsByTerminalId.clear(); + replaySnapshotsByTerminalId.clear(); nextTerminalSessionVersionRef.current = 1; useTerminalRuntimeStore.setState({ runtimesById: {} }); } @@ -617,7 +751,7 @@ export function createTerminalSessionView( ): TerminalSessionView { return { snapshot: runtime.snapshot, - rawOutput: runtime.rawOutput, + hasOutput: runtime.hasOutput, busy: runtime.busy, writeInput: (input: string) => useTerminalRuntimeStore @@ -631,5 +765,10 @@ export function createTerminalSessionView( useTerminalRuntimeStore.getState().restart(runtime.terminalId), clearViewport: () => useTerminalRuntimeStore.getState().clear(runtime.terminalId), + subscribeOutput: (listener) => + subscribeOutputChannel(runtime.terminalId, listener), + getReplaySnapshot: () => getReplaySnapshot(runtime.terminalId), + saveReplaySnapshot: (serialized: string) => + saveReplaySnapshot(runtime.terminalId, serialized), }; } diff --git a/apps/desktop/src/features/terminal/terminalTypes.ts b/apps/desktop/src/features/terminal/terminalTypes.ts index 3cbfb1c3..a3ccc3d3 100644 --- a/apps/desktop/src/features/terminal/terminalTypes.ts +++ b/apps/desktop/src/features/terminal/terminalTypes.ts @@ -39,14 +39,33 @@ export const DEV_TERMINAL_STARTED_EVENT = "devtools://terminal-started"; export const DEV_TERMINAL_EXITED_EVENT = "devtools://terminal-exited"; export const DEV_TERMINAL_ERROR_EVENT = "devtools://terminal-error"; +// A command delivered to a mounted terminal viewport. Output is piped straight +// to xterm via "write"; "clear" wipes the screen. The viewport — not app state — +// is the source of truth for screen content, so these are transient. +export type TerminalOutputCommand = + | { type: "write"; data: string } + | { type: "clear" }; + export interface TerminalSessionView { snapshot: TerminalSessionSnapshot; - rawOutput: string; + // Whether any output has been produced this session. Drives empty-state UI + // only; the actual content lives in the xterm instance, not here. + hasOutput: boolean; busy: boolean; writeInput: (input: string) => Promise; resize: (cols: number, rows: number) => Promise; restart: () => Promise; clearViewport: () => void; + // Subscribe to live output commands for this terminal. Any commands buffered + // before the first subscriber attaches are flushed on subscribe. Returns an + // unsubscribe function. + subscribeOutput: ( + listener: (command: TerminalOutputCommand) => void, + ) => () => void; + // Reattach support: a serialized xterm buffer snapshot, written back on mount + // to restore screen content across remounts and reloads. + getReplaySnapshot: () => string | null; + saveReplaySnapshot: (serialized: string) => void; } export const EMPTY_TERMINAL_SNAPSHOT: TerminalSessionSnapshot = { diff --git a/apps/desktop/src/test/setup.ts b/apps/desktop/src/test/setup.ts index b79697ba..c0fb523d 100644 --- a/apps/desktop/src/test/setup.ts +++ b/apps/desktop/src/test/setup.ts @@ -11,6 +11,7 @@ type XtermMockInstance = { selectAll: () => void; getSelection: () => string; emitData: (data: string) => void; + triggerKeyEvent: (event: KeyboardEvent) => boolean; }; const xtermMockInstances: XtermMockInstance[] = []; @@ -262,8 +263,17 @@ vi.mock("@xterm/xterm", () => ({ }; } - attachCustomKeyEventHandler(_: (event: KeyboardEvent) => boolean) { - // No-op in tests; keyboard interception is exercised through UI state. + private keyEventHandler: ((event: KeyboardEvent) => boolean) | null = + null; + + attachCustomKeyEventHandler( + handler: (event: KeyboardEvent) => boolean, + ) { + this.keyEventHandler = handler; + } + + triggerKeyEvent(event: KeyboardEvent): boolean { + return this.keyEventHandler?.(event) ?? true; } emitData(data: string) { @@ -386,6 +396,22 @@ vi.mock("@xterm/addon-webgl", () => ({ }, })); +vi.mock("@xterm/addon-serialize", () => ({ + SerializeAddon: class MockSerializeAddon { + private terminal: { screen: HTMLDivElement | null } | null = null; + + activate(terminal: { screen: HTMLDivElement | null }) { + this.terminal = terminal; + } + + serialize() { + return this.terminal?.screen?.textContent ?? ""; + } + + dispose() {} + }, +})); + vi.mock("react-datasheet-grid", async () => { const React = await import("react"); diff --git a/apps/desktop/src/test/test-utils.tsx b/apps/desktop/src/test/test-utils.tsx index a905f5f6..34bcaf59 100644 --- a/apps/desktop/src/test/test-utils.tsx +++ b/apps/desktop/src/test/test-utils.tsx @@ -153,6 +153,12 @@ export function getXtermMockInstances() { globalThis as typeof globalThis & { __xtermMockInstances: Array<{ emitData: (data: string) => void; + triggerKeyEvent: (event: KeyboardEvent) => boolean; + write: (text: string, callback?: () => void) => void; + reset: () => void; + clear: () => void; + scrollToTop: () => void; + screen: HTMLDivElement | null; focusCalls: number; scrollToTopCalls: number; }>; From ccd851fd4f8a015ba310e008cd48480286e37f79 Mon Sep 17 00:00:00 2001 From: jsgerrchg Date: Sat, 30 May 2026 08:55:50 -0400 Subject: [PATCH 2/2] Fix terminal host test unsubscribe mock typing --- .../src/features/terminal/WorkspaceTerminalHost.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/features/terminal/WorkspaceTerminalHost.test.tsx b/apps/desktop/src/features/terminal/WorkspaceTerminalHost.test.tsx index 3f67fe83..31e0a520 100644 --- a/apps/desktop/src/features/terminal/WorkspaceTerminalHost.test.tsx +++ b/apps/desktop/src/features/terminal/WorkspaceTerminalHost.test.tsx @@ -134,7 +134,8 @@ describe("WorkspaceTerminalHost", () => { payload: TerminalOutputEventPayload; }) => void, ); - return vi.fn(); + const unlisten: () => void = vi.fn(); + return unlisten; }); // Set up a live terminal session so output is delivered to the runtime. @@ -232,7 +233,8 @@ describe("WorkspaceTerminalHost", () => { payload: TerminalOutputEventPayload; }) => void, ); - return vi.fn(); + const unlisten: () => void = vi.fn(); + return unlisten; }); vi.mocked(invoke).mockResolvedValue({