diff --git a/apps/desktop/src/lib/trpc/routers/terminal/terminal.ts b/apps/desktop/src/lib/trpc/routers/terminal/terminal.ts index 16abeb88746..dc743ff464d 100644 --- a/apps/desktop/src/lib/trpc/routers/terminal/terminal.ts +++ b/apps/desktop/src/lib/trpc/routers/terminal/terminal.ts @@ -146,6 +146,21 @@ export const createTerminalRouter = () => { terminalManager.detach(input); }), + /** + * Save serialized terminal state from renderer. + * Uses xterm.js serialize addon output for clean scrollback persistence. + */ + saveScrollback: publicProcedure + .input( + z.object({ + tabId: z.string(), + serialized: z.string(), + }), + ) + .mutation(async ({ input }) => { + await terminalManager.saveScrollback(input); + }), + getSession: publicProcedure .input(z.string()) .query(async ({ input: tabId }) => { diff --git a/apps/desktop/src/main/lib/terminal-escape-filter.test.ts b/apps/desktop/src/main/lib/terminal-escape-filter.test.ts index ff185ee800c..ca3e8906ca0 100644 --- a/apps/desktop/src/main/lib/terminal-escape-filter.test.ts +++ b/apps/desktop/src/main/lib/terminal-escape-filter.test.ts @@ -417,20 +417,28 @@ describe("TerminalEscapeFilter (stateful)", () => { expect(result2).toBe(`${ESC}[32mgreen`); }); - it("should NOT buffer ESC alone at end", () => { + it("should buffer ESC alone at end", () => { const filter = new TerminalEscapeFilter(); const chunk1 = `text${ESC}`; const result1 = filter.filter(chunk1); - // ESC alone should pass through (conservative - don't buffer) - expect(result1).toBe(`text${ESC}`); + // ESC alone must be buffered to check if next chunk forms a query response + // This prevents orphaned sequences like "2R" when ESC is at chunk boundary + expect(result1).toBe("text"); + // If next chunk completes a query response, it gets filtered + const result2 = filter.filter("[2R"); + expect(result2).toBe(""); }); - it("should NOT buffer ESC [ alone at end", () => { + it("should buffer ESC [ alone at end", () => { const filter = new TerminalEscapeFilter(); const chunk1 = `text${ESC}[`; const result1 = filter.filter(chunk1); - // ESC [ alone should pass through (could be any CSI) - expect(result1).toBe(`text${ESC}[`); + // ESC [ alone must be buffered to see what follows + // This prevents ";1R" from leaking when next chunk is ";1R..." + expect(result1).toBe("text"); + // Complete with a query response + const result2 = filter.filter(";1R"); + expect(result2).toBe(""); }); it("should buffer ESC [ digit (could be CPR/mode report/DA)", () => { @@ -475,14 +483,14 @@ describe("TerminalEscapeFilter (stateful)", () => { }); describe("flush behavior", () => { - it("should flush buffered incomplete sequence", () => { + it("should discard incomplete query response on flush", () => { const filter = new TerminalEscapeFilter(); const chunk = `hello${ESC}[?1;0`; // Incomplete DA1 const result = filter.filter(chunk); expect(result).toBe("hello"); - // Flush returns the buffered data (filtered) + // Flush discards incomplete query responses to prevent garbage on restore const flushed = filter.flush(); - expect(flushed).toBe(`${ESC}[?1;0`); // Not filtered because incomplete + expect(flushed).toBe(""); }); it("should return empty on flush when no buffer", () => { @@ -490,6 +498,51 @@ describe("TerminalEscapeFilter (stateful)", () => { filter.filter("complete data"); expect(filter.flush()).toBe(""); }); + + it("should discard lone ESC on flush", () => { + const filter = new TerminalEscapeFilter(); + const chunk = `prompt ❯ ${ESC}`; + const result = filter.filter(chunk); + expect(result).toBe("prompt ❯ "); + // Lone ESC is discarded on flush + expect(filter.flush()).toBe(""); + }); + }); + + describe("TUI app scenarios", () => { + it("should filter rapid query responses split across chunks", () => { + // Simulates TUI apps like lazygit sending many query responses + const filter = new TerminalEscapeFilter(); + const chunks = [ + `prompt ❯ ${ESC}`, + `[2R${ESC}`, + `[1R${ESC}`, + `[12;2$y more text`, + ]; + + let output = ""; + for (const chunk of chunks) { + output += filter.filter(chunk); + } + output += filter.flush(); + + // All query responses should be filtered, only text remains + expect(output).toBe("prompt ❯ more text"); + }); + + it("should not leak orphaned sequence parts", () => { + // This was the bug: ESC at chunk end caused "[2R" to leak through + const filter = new TerminalEscapeFilter(); + const chunk1 = `text${ESC}`; + const chunk2 = `[2R${ESC}[1R`; + + const out1 = filter.filter(chunk1); + const out2 = filter.filter(chunk2); + const flushed = filter.flush(); + + // Should not contain any orphaned "2R", "1R", etc. + expect(out1 + out2 + flushed).toBe("text"); + }); }); describe("reset behavior", () => { diff --git a/apps/desktop/src/main/lib/terminal-escape-filter.ts b/apps/desktop/src/main/lib/terminal-escape-filter.ts index 9ba7e5bef54..453d8738c25 100644 --- a/apps/desktop/src/main/lib/terminal-escape-filter.ts +++ b/apps/desktop/src/main/lib/terminal-escape-filter.ts @@ -18,11 +18,13 @@ const FILTER_PATTERNS = { /** * Cursor Position Report (CPR): ESC [ Pl ; Pc R or ESC [ Pl R * Response to DSR (Device Status Report) query ESC [ 6 n + * Some terminals omit row/column, sending ESC[;1R or ESC[R * Examples: * - ESC[24;1R (cursor at row 24, column 1) * - ESC[2R (cursor at row 2, column defaults to 1) + * - ESC[;1R (row omitted, column 1) */ - cursorPositionReport: `${ESC}\\[\\d+(?:;\\d+)?R`, + cursorPositionReport: `${ESC}\\[\\d*(?:;\\d*)?R`, /** * Primary Device Attributes (DA1): ESC [ ? Ps c @@ -146,19 +148,25 @@ export class TerminalEscapeFilter { * at chunk boundaries. If the complete sequence doesn't match our filter, it passes through. */ private looksLikeQueryResponse(str: string): boolean { - if (str.length < 2) return false; // Just ESC alone - don't buffer, could be anything + // Buffer lone ESC - we need to see the next char to decide + if (str.length === 1 && str[0] === ESC) return true; + if (str.length < 2) return false; const secondChar = str[1]; // CSI query responses we want to buffer: + // - ESC [ alone (need to see next char) // - ESC [ ? (DA1, DECRPM private mode) // - ESC [ > (DA2 secondary) // - ESC [ digit (CPR, standard mode reports, device attributes) + // - ESC [ ; (CPR with omitted row, e.g., ESC[;1R) if (secondChar === "[") { - if (str.length < 3) return false; // ESC [ alone - don't buffer + if (str.length < 3) return true; // ESC [ alone - buffer to see what follows const thirdChar = str[2]; // Buffer ? (private mode) or > (secondary DA) if (thirdChar === "?" || thirdChar === ">") return true; + // Buffer ; for CPR with omitted parameters (ESC[;1R) + if (thirdChar === ";") return true; // Buffer digit-starting CSI sequences that could be query responses: // - CPR: ESC[24;1R or ESC[1R // - Standard mode report: ESC[12;2$y @@ -192,6 +200,7 @@ export class TerminalEscapeFilter { * Check if a potential query response sequence is incomplete. */ private isIncomplete(str: string): boolean { + // Lone ESC is incomplete - need to see what follows if (str.length < 2) return true; const secondChar = str[1]; @@ -224,10 +233,20 @@ export class TerminalEscapeFilter { /** * Flush any remaining buffered data. * Call this when the terminal session ends. + * + * If the buffer contains what looks like an incomplete query response, + * discard it entirely to prevent garbage output on restore. */ flush(): string { const remaining = this.buffer; this.buffer = ""; + + // If the buffer looks like an incomplete query response, discard it + // This prevents orphaned sequences like "2R1R12;2$y" from appearing + if (remaining && this.looksLikeQueryResponse(remaining)) { + return ""; + } + return remaining.replace(COMBINED_PATTERN, ""); } diff --git a/apps/desktop/src/main/lib/terminal-history.ts b/apps/desktop/src/main/lib/terminal-history.ts index 98b0e2c2842..72161e02130 100644 --- a/apps/desktop/src/main/lib/terminal-history.ts +++ b/apps/desktop/src/main/lib/terminal-history.ts @@ -56,6 +56,7 @@ export class HistoryWriter { await fs.mkdir(dir, { recursive: true }); // Write initial scrollback (recovered from previous session) or truncate + // Serialized data from xterm.js is already clean, no filtering needed if (initialScrollback) { await fs.writeFile(this.filePath, Buffer.from(initialScrollback)); } else { @@ -80,6 +81,36 @@ export class HistoryWriter { } } + /** + * Save a complete serialized terminal snapshot. + * This replaces the entire scrollback file with clean serialized data + * from xterm.js serialize addon, which doesn't contain query responses. + */ + async saveSnapshot(serialized: string): Promise { + // Close existing stream before replacing file + if (this.stream && !this.streamErrored) { + try { + await new Promise((resolve) => { + this.stream?.end(() => resolve()); + }); + } catch { + // Ignore close errors + } + } + this.stream = null; + this.streamErrored = false; + + // Write serialized data (already clean, no filtering needed) + await fs.writeFile(this.filePath, Buffer.from(serialized)); + + // Reopen stream for any subsequent writes + this.stream = createWriteStream(this.filePath, { flags: "a" }); + this.stream.on("error", () => { + this.streamErrored = true; + this.stream = null; + }); + } + async close(exitCode?: number): Promise { if (this.stream && !this.streamErrored) { try { diff --git a/apps/desktop/src/main/lib/terminal-manager.test.ts b/apps/desktop/src/main/lib/terminal-manager.test.ts index b46396a66ce..d1a2b04c6c8 100644 --- a/apps/desktop/src/main/lib/terminal-manager.test.ts +++ b/apps/desktop/src/main/lib/terminal-manager.test.ts @@ -372,6 +372,12 @@ describe("TerminalManager", () => { onDataCallback("Preserved output\n"); } + // Simulate what the renderer does: save serialized scrollback before detach + await manager.saveScrollback({ + tabId: "tab-preserve", + serialized: "Preserved output\n", + }); + const exitPromise = new Promise((resolve) => { manager.once("exit:tab-preserve", () => resolve()); }); @@ -753,6 +759,12 @@ describe("TerminalManager", () => { onDataCallback1("Session 1 output\n"); } + // Simulate renderer saving scrollback before exit + await manager.saveScrollback({ + tabId: "tab-multi", + serialized: "Session 1 output\n", + }); + const exitPromise1 = new Promise((resolve) => { manager.once("exit:tab-multi", () => resolve()); }); @@ -784,6 +796,12 @@ describe("TerminalManager", () => { onDataCallback2("Session 2 output\n"); } + // Simulate renderer saving scrollback with both sessions' output + await manager.saveScrollback({ + tabId: "tab-multi", + serialized: "Session 1 output\nSession 2 output\n", + }); + const exitPromise2 = new Promise((resolve) => { manager.once("exit:tab-multi", () => resolve()); }); diff --git a/apps/desktop/src/main/lib/terminal-manager.ts b/apps/desktop/src/main/lib/terminal-manager.ts index 251b38cedf8..cf8c0c97b76 100644 --- a/apps/desktop/src/main/lib/terminal-manager.ts +++ b/apps/desktop/src/main/lib/terminal-manager.ts @@ -14,12 +14,14 @@ interface TerminalSession { cols: number; rows: number; lastActive: number; + /** Raw accumulated PTY output - may contain query responses */ scrollback: string; + /** Last clean serialized snapshot from renderer - used for reattach/recovery */ + serializedScrollback: string; isAlive: boolean; deleteHistoryOnExit?: boolean; wasRecovered: boolean; historyWriter?: HistoryWriter; - escapeFilter: TerminalEscapeFilter; } export interface TerminalDataEvent { @@ -71,15 +73,46 @@ export class TerminalManager extends EventEmitter { if (cols !== undefined && rows !== undefined) { this.resize({ tabId, cols, rows }); } + // For live sessions, use the last serialized snapshot as the base. + // If raw scrollback has grown since then, filter and append the new data. + let scrollback = existing.serializedScrollback || ""; + const rawLen = existing.scrollback.length; + const serializedLen = existing.serializedScrollback.length; + console.log( + "[TerminalManager] reattach:", + tabId, + "serializedLen:", + serializedLen, + "rawLen:", + rawLen, + "delta:", + rawLen - serializedLen, + ); + if (rawLen > serializedLen) { + // There's new data since last serialization - filter it + const newData = existing.scrollback.slice(serializedLen); + console.log( + "[TerminalManager] filtering delta, first 200:", + newData.slice(0, 200), + ); + const filter = new TerminalEscapeFilter(); + scrollback += filter.filter(newData) + filter.flush(); + } + console.log( + "[TerminalManager] returning scrollback, first 200:", + scrollback.slice(0, 200), + ); return { isNew: false, - scrollback: existing.scrollback, + scrollback, wasRecovered: existing.wasRecovered, }; } // Use in-memory scrollback from dead session if available - const existingScrollback = existing?.scrollback || null; + // Prefer serialized (clean) over raw scrollback + const existingScrollback = + existing?.serializedScrollback || existing?.scrollback || null; const shell = this.getDefaultShell(); const workingDir = cwd || os.homedir(); @@ -115,8 +148,10 @@ export class TerminalManager extends EventEmitter { } } + // Filter escape sequences from recovered scrollback for legacy data + // New serialized data from xterm.js is already clean, but old raw + // scrollback files may still contain query responses if (recoveredScrollback) { - // Strip protocol responses from recovered history so replays stay clean const recoveryFilter = new TerminalEscapeFilter(); recoveredScrollback = recoveryFilter.filter(recoveredScrollback) + recoveryFilter.flush(); @@ -151,32 +186,22 @@ export class TerminalManager extends EventEmitter { rows: terminalRows, lastActive: Date.now(), scrollback: recoveredScrollback, + serializedScrollback: recoveredScrollback, // Initially same as recovered isAlive: true, wasRecovered, historyWriter, - escapeFilter: new TerminalEscapeFilter(), }; ptyProcess.onData((data) => { - // Filter terminal query responses for storage only - // xterm needs raw data for proper terminal behavior (DA/DSR/OSC responses) - const filteredData = session.escapeFilter.filter(data); - session.scrollback += filteredData; - session.historyWriter?.write(filteredData); - // Emit ORIGINAL data to xterm - it needs to process query responses + // Accumulate raw data in memory for quick reattach + // This is NOT persisted to disk - only saveScrollback() persists + // The renderer uses xterm.js serialize addon for clean persistence + session.scrollback += data; this.emit(`data:${tabId}`, data); }); ptyProcess.onExit(async ({ exitCode, signal }) => { session.isAlive = false; - - // Flush any buffered data from the escape filter - const remaining = session.escapeFilter.flush(); - if (remaining) { - session.scrollback += remaining; - session.historyWriter?.write(remaining); - } - await this.closeHistory(session, exitCode); this.emit(`exit:${tabId}`, exitCode, signal); @@ -280,6 +305,42 @@ export class TerminalManager extends EventEmitter { session.lastActive = Date.now(); } + /** + * Save serialized terminal state from renderer. + * This replaces the raw scrollback with clean xterm.js serialized output + * that doesn't contain terminal query responses. + */ + async saveScrollback(params: { + tabId: string; + serialized: string; + }): Promise { + const { tabId, serialized } = params; + const session = this.sessions.get(tabId); + + if (!session) { + console.warn( + `Cannot save scrollback for terminal ${tabId}: session not found`, + ); + return; + } + + // Update the clean serialized snapshot (used for reattach) + console.log( + "[TerminalManager] saveScrollback:", + tabId, + "length:", + serialized.length, + "first 200:", + serialized.slice(0, 200), + ); + session.serializedScrollback = serialized; + + // Save to disk via history writer + if (session.historyWriter) { + await session.historyWriter.saveSnapshot(serialized); + } + } + getSession(tabId: string): { isAlive: boolean; cwd: string; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx index 0f801aa0654..eb5acce1301 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx @@ -1,10 +1,12 @@ import "@xterm/xterm/css/xterm.css"; import type { FitAddon } from "@xterm/addon-fit"; import type { SearchAddon } from "@xterm/addon-search"; +import type { SerializeAddon } from "@xterm/addon-serialize"; import type { Terminal as XTerm } from "@xterm/xterm"; import { useEffect, useRef, useState } from "react"; import { useHotkeys } from "react-hotkeys-hook"; import { trpc } from "renderer/lib/trpc"; +import { trpcClient } from "renderer/lib/trpc-client"; import { useWindowsStore } from "renderer/stores/tabs/store"; import { useTerminalTheme } from "renderer/stores/theme"; import { HOTKEYS } from "shared/hotkeys"; @@ -29,6 +31,7 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { const xtermRef = useRef(null); const fitAddonRef = useRef(null); const searchAddonRef = useRef(null); + const serializeAddonRef = useRef(null); const isExitedRef = useRef(false); const pendingEventsRef = useRef([]); const [subscriptionEnabled, setSubscriptionEnabled] = useState(false); @@ -140,10 +143,12 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { const { xterm, fitAddon, + serializeAddon, cleanup: cleanupQuerySuppression, } = createTerminalInstance(container, workspaceCwd, terminalTheme); xtermRef.current = xterm; fitAddonRef.current = fitAddon; + serializeAddonRef.current = serializeAddon; isExitedRef.current = false; // Autofocus on initial render if this terminal is the focused pane @@ -279,12 +284,45 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { cleanupFocus?.(); cleanupResize(); cleanupQuerySuppression(); + + // Serialize terminal state before detaching for clean persistence + // This captures the rendered buffer state without query response garbage + if (serializeAddon && !isExitedRef.current) { + try { + const serialized = serializeAddon.serialize({ + excludeAltBuffer: true, // Don't save TUI app alternate screen + }); + console.log( + "[Terminal] Serialized scrollback:", + serialized?.length, + "chars, first 200:", + serialized?.slice(0, 200), + ); + if (serialized) { + // Fire and forget - don't block unmount on save + trpcClient.terminal.saveScrollback + .mutate({ tabId: paneId, serialized }) + .catch((err) => { + console.error("[Terminal] Failed to save scrollback:", err); + }); + } + } catch (err) { + console.error("[Terminal] Failed to serialize terminal:", err); + } + } else { + console.log( + "[Terminal] Skipping serialize:", + !serializeAddon ? "no addon" : "exited", + ); + } + // Keep PTY running for reattachment detachRef.current({ tabId: paneId }); setSubscriptionEnabled(false); xterm.dispose(); xtermRef.current = null; searchAddonRef.current = null; + serializeAddonRef.current = null; }; }, [paneId, workspaceId, workspaceCwd, paneName, terminalTheme]); diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts index 7700ad24f6c..965a1ebfeb3 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts @@ -1,5 +1,6 @@ import { ClipboardAddon } from "@xterm/addon-clipboard"; import { FitAddon } from "@xterm/addon-fit"; +import { SerializeAddon } from "@xterm/addon-serialize"; import { Unicode11Addon } from "@xterm/addon-unicode11"; import { WebLinksAddon } from "@xterm/addon-web-links"; import type { ITheme } from "@xterm/xterm"; @@ -56,6 +57,7 @@ export function createTerminalInstance( ): { xterm: XTerm; fitAddon: FitAddon; + serializeAddon: SerializeAddon; cleanup: () => void; } { // Use provided theme, or fall back to localStorage-based default to prevent flash @@ -80,6 +82,9 @@ export function createTerminalInstance( // Unicode 11 provides better emoji and unicode rendering than default const unicode11Addon = new Unicode11Addon(); + // Serialize addon for saving terminal state (scrollback, colors, cursor) + const serializeAddon = new SerializeAddon(); + xterm.open(container); // Addons must be loaded after terminal is opened, otherwise they won't attach properly @@ -87,6 +92,7 @@ export function createTerminalInstance( xterm.loadAddon(webLinksAddon); xterm.loadAddon(clipboardAddon); xterm.loadAddon(unicode11Addon); + xterm.loadAddon(serializeAddon); // Suppress terminal query responses (DA1, DA2, CPR, OSC color responses, etc.) // These are protocol-level responses that should be handled internally, not displayed @@ -123,6 +129,7 @@ export function createTerminalInstance( return { xterm, fitAddon, + serializeAddon, cleanup: cleanupQuerySuppression, }; }