diff --git a/apps/desktop/src/features/terminal/TerminalViewport.test.tsx b/apps/desktop/src/features/terminal/TerminalViewport.test.tsx index ca341fef..b530b672 100644 --- a/apps/desktop/src/features/terminal/TerminalViewport.test.tsx +++ b/apps/desktop/src/features/terminal/TerminalViewport.test.tsx @@ -34,7 +34,7 @@ function createSessionView( } describe("TerminalViewport", () => { - it("renders raw output and forwards xterm input and settled resize events", async () => { + it("renders raw output, fires initial PTY resize immediately, and forwards xterm input", async () => { const writeInput = vi.fn(async () => undefined); const resize = vi.fn(async () => undefined); vi.useFakeTimers(); @@ -52,13 +52,17 @@ describe("TerminalViewport", () => { 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"); @@ -115,14 +119,19 @@ describe("TerminalViewport", () => { ); 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); }); @@ -215,4 +224,236 @@ describe("TerminalViewport", () => { expect(term.screen?.textContent).toBe(bigChunk + smallChunk); expect(pendingCallbacks).toHaveLength(1); // the backlog write's callback }); + + it("sends \\n to the PTY on Shift+Enter and does not intercept plain Enter or keyup", async () => { + const writeInput = vi.fn(async () => undefined); + + 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("does not reset xterm when rawOutput is truncated at the buffer cap", async () => { + // Simulate what appendTerminalRawOutput does once rawOutput exceeds + // MAX_RAW_OUTPUT_CHARS: the front is trimmed so rawOutput no longer + // starts with the previous value. The probe-based truncation detector + // must recognise this and avoid calling terminal.reset(), which would + // blank the screen on every subsequent write. + // + // OVERLAP must be >= PROBE_LEN * 2 (64 chars) so the 32-byte probe + // AND the 32-byte verification window both land inside the shared + // portion of lastRef and rawOutput. + const OVERLAP = + // 32 chars for the probe window — keep them unique so indexOf + // returns the correct position and not an earlier false hit. + "1234567890ABCDEFGHIJKLMNOPQRSTUV" + + // 48 more chars for the verification window. + "WXYZabcdefghijklmnopqrstuvwxyz!@#$%^&*()-_+=[]{}"; + const PREFIX = "AAA"; // bytes trimmed from the front + const initial = PREFIX + OVERLAP; + // postTruncation = initial.slice(PREFIX.length) + newChunk + // — same length, does not start with initial. + const postTruncation = OVERLAP + "XYZ"; + + const { rerender } = renderComponent( + , + ); + await flushPromises(); + + const [term] = getXtermMockInstances(); + if (!term) throw new Error("No xterm instance"); + + // Spy after the initial render — which legitimately calls reset once + // for the new session — so we only count resets during truncation. + const resetSpy = vi.spyOn(term, "reset"); + + rerender( + , + ); + await flushPromises(); + + expect(resetSpy).not.toHaveBeenCalled(); + // The new tail must reach xterm without a full rewrite. + expect(term.screen?.textContent).toContain("XYZ"); + }); + + it("writes the new tail to xterm when output stops exactly at the truncation boundary", async () => { + // Verify the tail bytes ("ENDOFOUTPUT") reach xterm even if no further + // output arrives after the truncating write — ruling out any approach + // that would skip writing the new chunk on truncation. + const OVERLAP = + "1234567890ABCDEFGHIJKLMNOPQRSTUV" + + "WXYZabcdefghijklmnopqrstuvwxyz!@#$%^&*()-_+=[]{}"; + const PREFIX = "AAA"; + const TAIL = "ENDOFOUTPUT"; + const initial = PREFIX + OVERLAP; + const postTruncation = OVERLAP + TAIL; + + const { rerender } = renderComponent( + , + ); + await flushPromises(); + + rerender( + , + ); + await flushPromises(); + + const [term] = getXtermMockInstances(); + if (!term) throw new Error("No xterm instance"); + + expect(term.screen?.textContent).toContain(TAIL); + }); + + it("writes the new tail without reset when the overlap region is plain repeated characters", async () => { + // Covers the case where a terminal outputs monotonous content (progress + // bars, spinner lines, repeated padding) so the overlap between lastRef + // and rawOutput is all the same character. The unique prefix before the + // repeated block ensures the probe's first indexOf hit lands at the true + // truncation offset, not at a false-positive earlier position. + // + // Geometry assumes PROBE_LEN=32. The probe is "a"*32; PREFIX has no + // 'a', so the first indexOf hit in searchArea is at the true offset. + // + // Length invariant: postTruncation.length must be >= initial.length so + // the rawOutput.length >= lastRef.length gate is satisfied and the probe + // path is entered. We preserve length by adding as many tail chars as + // were trimmed from the front. + const PREFIX = "UNIQUE_PREF"; // 11 unique non-'a' chars, trimmed by the cap + const OVERLAP = "a".repeat(200); // repeated content — the probe "a"*32 lives here + const TAIL = "N".repeat(PREFIX.length); // same length as PREFIX to keep total length equal + const initial = PREFIX + OVERLAP; // 211 chars + const postTruncation = OVERLAP + TAIL; // 211 chars — does not start with initial + + const { rerender } = renderComponent( + , + ); + await flushPromises(); + + const [term] = getXtermMockInstances(); + if (!term) throw new Error("No xterm instance"); + + // Spy after the initial render (which legitimately resets for the new + // session) so we only count resets that happen during truncation. + const resetSpy = vi.spyOn(term, "reset"); + + rerender( + , + ); + await flushPromises(); + + expect(resetSpy).not.toHaveBeenCalled(); + // Exact screen state: initial content (written on first render) followed + // by only the delta tail — not a full reset+rewrite of postTruncation. + // If reset+replay had fired, PREFIX would be absent (reset clears screen, + // then the full replay starts with OVERLAP). + expect(term.screen?.textContent).toBe(initial + TAIL); + }); + + it("falls back to full reset when the probe hits an earlier repeated-content position and verification rejects it", async () => { + // When rawOutput starts with a repeated pattern that also appears + // earlier in lastRef, indexOf finds the wrong (earlier) offset. The + // 32-char verification window catches the mismatch and the code falls + // through to terminal.reset() + full replay — no silent output drop. + // + // Geometry assumes PROBE_LEN=32. + // + // Layout (all lengths preserved so the length gate is satisfied): + // initial = "a"*100 + SEPARATOR + "a"*100 (213 chars) + // postTrunc = "a"*40 + SEPARATOR + "a"*100 + "N"*60 (213 chars) + // + // Probe = "a"*32. searchArea = initial[1:] starts with "a"*99 so the + // first indexOf hit is at index 0 → delta = 1 (wrong; true offset = 60). + // Verification: initial[33:65] = "a"*32 but postTrunc[32:64] contains + // SEPARATOR (which starts at index 40), so the windows differ and the + // verification step rejects the early hit. + // + // SEPARATOR must appear within postTrunc[32:64] (starts at 40, ends at + // 52) — i.e. it must start after the 32-char probe window AND end before + // index 64. Shifting it entirely before index 32 would put it inside the + // probe (changing the probe content), and shifting it past index 64 would + // place it outside the verification window and allow a false acceptance. + const SEPARATOR = "SEP_UNIQUE_XZ"; // 13 chars, no 'a', starts at postTrunc[40] + const initial = + "a".repeat(100) + SEPARATOR + "a".repeat(100); // 213 chars + const postTruncation = + initial.slice(60) + "N".repeat(60); // 153 + 60 = 213 chars + + const { rerender } = renderComponent( + , + ); + await flushPromises(); + + const [term] = getXtermMockInstances(); + if (!term) throw new Error("No xterm instance"); + + const resetSpy = vi.spyOn(term, "reset"); + + rerender( + , + ); + await flushPromises(); + + // Verification rejected the early probe hit → graceful reset fallback. + expect(resetSpy).toHaveBeenCalledOnce(); + // Exact screen state after reset+full replay: postTruncation in full. + // toContain alone would miss a regression where only part of the output + // was replayed after the reset. + expect(term.screen?.textContent).toBe(postTruncation); + }); }); diff --git a/apps/desktop/src/features/terminal/TerminalViewport.tsx b/apps/desktop/src/features/terminal/TerminalViewport.tsx index b693ba76..8f1afbb1 100644 --- a/apps/desktop/src/features/terminal/TerminalViewport.tsx +++ b/apps/desktop/src/features/terminal/TerminalViewport.tsx @@ -117,6 +117,11 @@ export function TerminalViewport({ 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); @@ -261,6 +266,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); @@ -297,6 +322,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; @@ -340,6 +371,11 @@ export function TerminalViewport({ searchAddonRef.current = searchAddon; onDataDisposable = terminal.onData((data) => { + if (data === "\n" && suppressNextNewlineRef.current) { + suppressNextNewlineRef.current = false; + return; + } + suppressNextNewlineRef.current = false; void writeInputRef .current(data) .catch((error) => @@ -541,6 +577,64 @@ export function TerminalViewport({ rawOutput.length < lastRawOutputRef.current.length || !rawOutput.startsWith(lastRawOutputRef.current) ) { + // Check whether this is a truncation event rather than a genuine + // content mismatch. appendTerminalRawOutput caps rawOutput at + // MAX_RAW_OUTPUT_CHARS by slicing off the front; once the session + // crosses that threshold every new chunk lands here, triggering + // terminal.reset() on every write and causing continuous flicker. + // + // Truncation signature: rawOutput didn't shrink (it stayed at the + // MAX cap), and rawOutput = lastRef[delta:] + newChunk. We recover + // the new tail by locating the start of rawOutput inside lastRef + // with a short probe, then writing only the new bytes. + if (rawOutput.length >= lastRawOutputRef.current.length) { + const PROBE_LEN = 32; + // Search up to MAX_SEARCH chars into lastRef for the delta + // offset. The inter-render delta can be large when the store + // coalesces multiple PTY events (paste echo, large file cat, + // TUI redraws) — 512KB covers most realistic bursts without + // meaningful cost since String.indexOf is native. + const MAX_SEARCH = 512_000; + const probe = rawOutput.slice(0, PROBE_LEN); + const searchArea = lastRawOutputRef.current.slice( + 1, + MAX_SEARCH + PROBE_LEN, + ); + const hit = searchArea.indexOf(probe); + if (hit >= 0) { + const delta = hit + 1; // +1 because searchArea starts at 1 + // Verify the match extends well past the probe to reduce + // (not eliminate) false positives from repeated ANSI + // sequences — TUI spinners and redraws can emit long runs + // of identical escape codes. A wrong delta produces one + // garbled frame before the next reset corrects it. + if ( + lastRawOutputRef.current.slice( + delta + PROBE_LEN, + delta + PROBE_LEN * 2, + ) === rawOutput.slice(PROBE_LEN, PROBE_LEN * 2) + ) { + const overlapLen = + lastRawOutputRef.current.length - delta; + const newChunk = + overlapLen < rawOutput.length + ? rawOutput.slice(overlapLen) + : ""; + if (newChunk.length > 0) { + writeChunk(newChunk, () => { + if (!shouldApplyInitialScrollRef.current) + return; + shouldApplyInitialScrollRef.current = false; + terminal.scrollToTop(); + }); + } + lastRawOutputRef.current = rawOutput; + return; + } + } + // Probe not found or verification failed — fall through to + // full reset (genuine mismatch or enormous chunk > MAX_SEARCH). + } terminal.reset(); pendingWriteCharsRef.current = 0; writeBacklogRef.current = []; diff --git a/apps/desktop/src/test/setup.ts b/apps/desktop/src/test/setup.ts index b79697ba..c87242e7 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) {