From 8a4e740250b60cf6feea72c02dd3b59ada6a8f18 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Thu, 4 Dec 2025 13:16:52 -0800 Subject: [PATCH 1/4] fix terminal scrolling bug --- .../TabsContent/Terminal/Terminal.tsx | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 1457232ca71..13aa29869f0 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 @@ -76,7 +76,18 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { } if (event.type === "data") { - xtermRef.current.write(event.data); + const xterm = xtermRef.current; + // Check if viewport is at the bottom before writing + const buffer = xterm.buffer.active; + const isAtBottom = + buffer.viewportY >= buffer.baseY + buffer.cursorY - xterm.rows + 1; + + xterm.write(event.data); + + // Scroll to bottom after write if user was at bottom (prevents scroll-to-top on cursor moves) + if (isAtBottom) { + xterm.scrollToBottom(); + } } else if (event.type === "exit") { isExitedRef.current = true; setSubscriptionEnabled(false); @@ -159,6 +170,8 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { xterm.writeln("[Press any key to restart]"); } } + // Scroll to bottom after flushing pending events + xterm.scrollToBottom(); }; const applyInitialScrollback = (result: { @@ -167,6 +180,8 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { scrollback: string; }) => { xterm.write(result.scrollback); + // Scroll to bottom after applying scrollback to show latest content + xterm.scrollToBottom(); }; const restartTerminal = () => { From a29aa9c5a7098f44d735f48d80d9a84681371d06 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Thu, 4 Dec 2025 13:47:31 -0800 Subject: [PATCH 2/4] resize fix --- .../ContentView/TabsContent/Terminal/Terminal.tsx | 5 ++--- .../ContentView/TabsContent/Terminal/helpers.ts | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) 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 13aa29869f0..c5c2a508b93 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 @@ -77,10 +77,9 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { if (event.type === "data") { const xterm = xtermRef.current; - // Check if viewport is at the bottom before writing + // Check if viewport is at the bottom before writing (xterm.js recommended pattern) const buffer = xterm.buffer.active; - const isAtBottom = - buffer.viewportY >= buffer.baseY + buffer.cursorY - xterm.rows + 1; + const isAtBottom = buffer.viewportY === buffer.baseY; xterm.write(event.data); 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 1b1914a69f7..5df7fe2c717 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 @@ -229,6 +229,10 @@ export function setupResizeHandlers( const handleResize = () => { fitAddon.fit(); + // Defer scroll to next frame to ensure xterm has finished reflowing the buffer + requestAnimationFrame(() => { + xterm.scrollToBottom(); + }); debouncedResize(xterm.cols, xterm.rows); }; From 9687381541ed48eaeec3669e6f37ff6188514eb5 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Thu, 4 Dec 2025 14:14:39 -0800 Subject: [PATCH 3/4] fixing 1R7;1R bug with filter --- .../main/lib/terminal-escape-filter.test.ts | 51 ++++++++++++++++--- .../src/main/lib/terminal-escape-filter.ts | 19 +++++++ 2 files changed, 63 insertions(+), 7 deletions(-) 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..2b40df26bc8 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,36 @@ describe("TerminalEscapeFilter (stateful)", () => { expect(result2).toBe(`${ESC}[32mgreen`); }); - it("should NOT buffer ESC alone at end", () => { + it("should buffer ESC alone at end (could be start of query response)", () => { 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 should be buffered - could be start of query response + expect(result1).toBe("text"); }); - it("should NOT buffer ESC [ alone at end", () => { + it("should buffer ESC [ alone at end (could be start of query response)", () => { 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 should be buffered - could be start of CPR/DA/etc + expect(result1).toBe("text"); + }); + + it("should reassemble CPR split with ESC at chunk boundary", () => { + const filter = new TerminalEscapeFilter(); + // CPR response ESC[1;1R split with just ESC at boundary + const result1 = filter.filter(`text${ESC}`); + const result2 = filter.filter("[1;1R"); + expect(result1 + result2).toBe("text"); + }); + + it("should reassemble CPR split with ESC[ at chunk boundary", () => { + const filter = new TerminalEscapeFilter(); + // CPR response ESC[1;1R split with ESC[ at boundary + const result1 = filter.filter(`text${ESC}[`); + const result2 = filter.filter("1;1R"); + expect(result1 + result2).toBe("text"); }); it("should buffer ESC [ digit (could be CPR/mode report/DA)", () => { @@ -490,6 +506,24 @@ describe("TerminalEscapeFilter (stateful)", () => { filter.filter("complete data"); expect(filter.flush()).toBe(""); }); + + it("should preserve standalone ESC on flush (not a query response)", () => { + const filter = new TerminalEscapeFilter(); + const result = filter.filter(`text${ESC}`); + expect(result).toBe("text"); + // Flush should return the standalone ESC - it never formed a query response + const flushed = filter.flush(); + expect(flushed).toBe(ESC); + }); + + it("should preserve standalone ESC[ on flush (not a query response)", () => { + const filter = new TerminalEscapeFilter(); + const result = filter.filter(`text${ESC}[`); + expect(result).toBe("text"); + // Flush should return ESC[ - it never formed a query response + const flushed = filter.flush(); + expect(flushed).toBe(`${ESC}[`); + }); }); describe("reset behavior", () => { @@ -514,7 +548,10 @@ describe("TerminalEscapeFilter (stateful)", () => { const chunk2 = `[0mnormal`; const result1 = filter.filter(chunk1); const result2 = filter.filter(chunk2); - // Colors should pass through + // Trailing ESC is buffered, then reassembled with next chunk + // Colors should pass through (not matching query response patterns) + expect(result1).toBe(`${ESC}[31mred`); + expect(result2).toBe(`${ESC}[0mnormal`); expect(result1 + result2).toBe(`${ESC}[31mred${ESC}[0mnormal`); }); }); diff --git a/apps/desktop/src/main/lib/terminal-escape-filter.ts b/apps/desktop/src/main/lib/terminal-escape-filter.ts index 9ba7e5bef54..4bc38fe0775 100644 --- a/apps/desktop/src/main/lib/terminal-escape-filter.ts +++ b/apps/desktop/src/main/lib/terminal-escape-filter.ts @@ -117,6 +117,19 @@ export class TerminalEscapeFilter { const combined = this.buffer + data; this.buffer = ""; + // Pre-check: Buffer trailing prefix fragments (ESC or ESC[) at chunk boundaries + // These could be the start of query responses split across chunks + if (combined.endsWith(ESC)) { + this.buffer = ESC; + const toFilter = combined.slice(0, -1); + return toFilter.replace(COMBINED_PATTERN, ""); + } + if (combined.endsWith(`${ESC}[`)) { + this.buffer = `${ESC}[`; + const toFilter = combined.slice(0, -2); + return toFilter.replace(COMBINED_PATTERN, ""); + } + // Check if the data ends with a potential incomplete query response const lastEscIndex = combined.lastIndexOf(ESC); @@ -224,10 +237,16 @@ export class TerminalEscapeFilter { /** * Flush any remaining buffered data. * Call this when the terminal session ends. + * Preserves standalone ESC or ESC[ that never formed a query response. */ flush(): string { const remaining = this.buffer; this.buffer = ""; + // Preserve prefix fragments that never completed into a query response + // These are genuine ESC bytes, not query responses to filter + if (remaining === ESC || remaining === `${ESC}[`) { + return remaining; + } return remaining.replace(COMBINED_PATTERN, ""); } From 69f51c7d1673a3c9742dd26e96f6bcf3c0484bc9 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Thu, 4 Dec 2025 14:21:34 -0800 Subject: [PATCH 4/4] tests and escape filter handled --- .../main/lib/terminal-escape-filter.test.ts | 50 +++++++++++++++++++ .../src/main/lib/terminal-escape-filter.ts | 18 +++++-- 2 files changed, 65 insertions(+), 3 deletions(-) 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 2b40df26bc8..229e1154dce 100644 --- a/apps/desktop/src/main/lib/terminal-escape-filter.test.ts +++ b/apps/desktop/src/main/lib/terminal-escape-filter.test.ts @@ -449,6 +449,38 @@ describe("TerminalEscapeFilter (stateful)", () => { expect(result1 + result2).toBe("text"); }); + it("should buffer ESC ] alone at end (could be start of OSC response)", () => { + const filter = new TerminalEscapeFilter(); + const chunk1 = `text${ESC}]`; + const result1 = filter.filter(chunk1); + // ESC ] alone should be buffered - could be start of OSC color response + expect(result1).toBe("text"); + }); + + it("should reassemble OSC split with ESC] at chunk boundary", () => { + const filter = new TerminalEscapeFilter(); + // OSC 10 response split with ESC] at boundary + const result1 = filter.filter(`text${ESC}]`); + const result2 = filter.filter(`10;rgb:ffff/ffff/ffff${BEL}more`); + expect(result1 + result2).toBe("textmore"); + }); + + it("should buffer ESC P alone at end (could be start of DCS response)", () => { + const filter = new TerminalEscapeFilter(); + const chunk1 = `text${ESC}P`; + const result1 = filter.filter(chunk1); + // ESC P alone should be buffered - could be start of XTVERSION/DA3 + expect(result1).toBe("text"); + }); + + it("should reassemble XTVERSION split with ESC P at chunk boundary", () => { + const filter = new TerminalEscapeFilter(); + // XTVERSION response split with ESC P at boundary + const result1 = filter.filter(`text${ESC}P`); + const result2 = filter.filter(`>|XTerm(354)${ESC}\\more`); + expect(result1 + result2).toBe("textmore"); + }); + it("should buffer ESC [ digit (could be CPR/mode report/DA)", () => { const filter = new TerminalEscapeFilter(); const chunk1 = `text${ESC}[2`; @@ -524,6 +556,24 @@ describe("TerminalEscapeFilter (stateful)", () => { const flushed = filter.flush(); expect(flushed).toBe(`${ESC}[`); }); + + it("should preserve standalone ESC] on flush (not a query response)", () => { + const filter = new TerminalEscapeFilter(); + const result = filter.filter(`text${ESC}]`); + expect(result).toBe("text"); + // Flush should return ESC] - it never formed a query response + const flushed = filter.flush(); + expect(flushed).toBe(`${ESC}]`); + }); + + it("should preserve standalone ESC P on flush (not a query response)", () => { + const filter = new TerminalEscapeFilter(); + const result = filter.filter(`text${ESC}P`); + expect(result).toBe("text"); + // Flush should return ESC P - it never formed a query response + const flushed = filter.flush(); + expect(flushed).toBe(`${ESC}P`); + }); }); 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 4bc38fe0775..394f82e036c 100644 --- a/apps/desktop/src/main/lib/terminal-escape-filter.ts +++ b/apps/desktop/src/main/lib/terminal-escape-filter.ts @@ -117,8 +117,9 @@ export class TerminalEscapeFilter { const combined = this.buffer + data; this.buffer = ""; - // Pre-check: Buffer trailing prefix fragments (ESC or ESC[) at chunk boundaries + // Pre-check: Buffer trailing prefix fragments at chunk boundaries // These could be the start of query responses split across chunks + // Covers: ESC (any), ESC[ (CSI), ESC] (OSC), ESC P (DCS) if (combined.endsWith(ESC)) { this.buffer = ESC; const toFilter = combined.slice(0, -1); @@ -129,6 +130,16 @@ export class TerminalEscapeFilter { const toFilter = combined.slice(0, -2); return toFilter.replace(COMBINED_PATTERN, ""); } + if (combined.endsWith(`${ESC}]`)) { + this.buffer = `${ESC}]`; + const toFilter = combined.slice(0, -2); + return toFilter.replace(COMBINED_PATTERN, ""); + } + if (combined.endsWith(`${ESC}P`)) { + this.buffer = `${ESC}P`; + const toFilter = combined.slice(0, -2); + return toFilter.replace(COMBINED_PATTERN, ""); + } // Check if the data ends with a potential incomplete query response const lastEscIndex = combined.lastIndexOf(ESC); @@ -237,14 +248,15 @@ export class TerminalEscapeFilter { /** * Flush any remaining buffered data. * Call this when the terminal session ends. - * Preserves standalone ESC or ESC[ that never formed a query response. + * Preserves standalone prefix fragments that never formed a query response. */ flush(): string { const remaining = this.buffer; this.buffer = ""; // Preserve prefix fragments that never completed into a query response // These are genuine ESC bytes, not query responses to filter - if (remaining === ESC || remaining === `${ESC}[`) { + const prefixFragments = [ESC, `${ESC}[`, `${ESC}]`, `${ESC}P`]; + if (prefixFragments.includes(remaining)) { return remaining; } return remaining.replace(COMBINED_PATTERN, "");