diff --git a/packages/cli/src/ui/layouts/DefaultAppLayout.test.tsx b/packages/cli/src/ui/layouts/DefaultAppLayout.test.tsx index 8f6aa7481..fd8396fc4 100644 --- a/packages/cli/src/ui/layouts/DefaultAppLayout.test.tsx +++ b/packages/cli/src/ui/layouts/DefaultAppLayout.test.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render } from 'ink-testing-library'; import { Text } from 'ink'; import { DefaultAppLayout } from './DefaultAppLayout.js'; @@ -17,15 +17,13 @@ import { AgentViewProvider } from '../contexts/AgentViewContext.js'; import { ConfigContext } from '../contexts/ConfigContext.js'; import { StreamingState } from '../types.js'; -// Drive terminal size deterministically: the real useTerminalSize listens on -// process.stdout and updates via React state, which doesn't flush predictably -// under fake timers. Mocking it lets us change width via an explicit rerender. -let mockSize = { columns: 100, rows: 40 }; +// Fixed size for layout; the resize handler under test reads process.stdout +// directly, so we drive it via stdout events rather than this hook. vi.mock('../hooks/useTerminalSize.js', () => ({ - useTerminalSize: () => mockSize, + useTerminalSize: () => ({ columns: 100, rows: 40 }), })); -// Heavy children are irrelevant to the resize→reflow wiring under test; stub +// Heavy children are irrelevant to the resize→repaint wiring under test; stub // them so the layout renders without pulling in their dependency graphs. vi.mock('../components/MainContent.js', () => ({ MainContent: () => MainContent, @@ -61,7 +59,7 @@ vi.mock('../components/TranscriptOverlay.js', () => ({ TranscriptOverlay: () => null, })); -describe('DefaultAppLayout resize reflow', () => { +describe('DefaultAppLayout resize repaint', () => { const refreshStatic = vi.fn(); const mockUIActions = { @@ -90,9 +88,6 @@ describe('DefaultAppLayout resize reflow', () => { }, }; - // A fresh element per render: React bails out of re-rendering when handed the - // same element reference, which would stop the mocked width change from ever - // reaching the component. const makeTree = () => ( @@ -105,55 +100,92 @@ describe('DefaultAppLayout resize reflow', () => { ); - // Real timers: Ink flushes passive effects through its own reconciler - // scheduler, which doesn't advance predictably under fake timers. A short - // wait lets an effect (and its debounce timer) run. FLUSH < 150ms debounce - // (effect scheduled, timer not yet fired); SETTLE > 150ms (timer fired). - const FLUSH_MS = 10; - const SETTLE_MS = 250; + // Each render attaches a resize listener to process.stdout; track instances + // so afterEach can unmount them and listeners don't leak across tests. + const instances: Array> = []; + const mount = () => { + const instance = render(makeTree()); + instances.push(instance); + return instance; + }; + + // The resize listener attaches in a passive effect; a short real wait lets it + // run. SETTLE > RESIZE_THROTTLE_MS (80ms) so the trailing timer has fired. + const MOUNT_MS = 20; + const SETTLE_MS = 160; const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + const originalColumns = process.stdout.columns; + const originalRows = process.stdout.rows; + + const resize = (columns: number, rows = process.stdout.rows) => { + process.stdout.columns = columns; + process.stdout.rows = rows; + process.stdout.emit('resize'); + }; + beforeEach(() => { refreshStatic.mockClear(); - mockSize = { columns: 100, rows: 40 }; + process.stdout.columns = 100; + process.stdout.rows = 40; }); - it('does not refresh static on first mount', async () => { - render(makeTree()); + afterEach(() => { + while (instances.length) { + instances.pop()?.unmount(); + } + process.stdout.columns = originalColumns; + process.stdout.rows = originalRows; + }); + + it('does not repaint on first mount', async () => { + mount(); await wait(SETTLE_MS); expect(refreshStatic).not.toHaveBeenCalled(); }); - it('refreshes static once after a width change settles', async () => { - const { rerender } = render(makeTree()); - await wait(FLUSH_MS); - mockSize = { columns: 60, rows: 40 }; - rerender(makeTree()); - await wait(FLUSH_MS); - // Debounced: the effect scheduled the timer but it hasn't fired yet. - expect(refreshStatic).not.toHaveBeenCalled(); - await wait(SETTLE_MS); + it('repaints immediately on a resize (leading edge)', async () => { + mount(); + await wait(MOUNT_MS); + resize(60); + // Leading edge fires synchronously in the resize handler — no wait needed. expect(refreshStatic).toHaveBeenCalledTimes(1); }); - it('debounces a burst of width changes into a single refresh', async () => { - const { rerender } = render(makeTree()); - await wait(FLUSH_MS); - for (const columns of [90, 80, 70]) { - mockSize = { columns, rows: 40 }; - rerender(makeTree()); - await wait(FLUSH_MS); // < debounce, so each change resets the timer - } + it('throttles a burst of resizes into a leading + trailing repaint', async () => { + mount(); + await wait(MOUNT_MS); + resize(90); + resize(85); + resize(80); + // Only the leading edge has fired so far; the rest coalesce. + expect(refreshStatic).toHaveBeenCalledTimes(1); await wait(SETTLE_MS); + // ...plus a single trailing repaint at the throttle boundary. + expect(refreshStatic).toHaveBeenCalledTimes(2); + }); + + it('ignores a resize event that does not change dimensions', async () => { + mount(); + await wait(MOUNT_MS); + process.stdout.emit('resize'); // same 100x40 as mount + await wait(SETTLE_MS); + expect(refreshStatic).not.toHaveBeenCalled(); + }); + + it('repaints on a height-only change', async () => { + mount(); + await wait(MOUNT_MS); + resize(100, 20); // width unchanged, height differs expect(refreshStatic).toHaveBeenCalledTimes(1); }); - it('does not refresh static when only the height changes', async () => { - const { rerender } = render(makeTree()); - await wait(FLUSH_MS); - mockSize = { columns: 100, rows: 20 }; // width unchanged, height differs - rerender(makeTree()); + it('removes the resize listener on unmount', async () => { + const { unmount } = mount(); + await wait(MOUNT_MS); + unmount(); + resize(50); await wait(SETTLE_MS); expect(refreshStatic).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx index 3be3a8d97..64b6b38bb 100644 --- a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx +++ b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx @@ -25,6 +25,13 @@ import { useAgentViewState } from '../contexts/AgentViewContext.js'; import { useConfig } from '../contexts/ConfigContext.js'; import { useTerminalSize } from '../hooks/useTerminalSize.js'; +/** + * Max repaint cadence while the terminal is being resized. Small enough that a + * drag never accumulates more than a frame or two of stale output before it's + * cleared, large enough not to clear+reprint on every single resize event. + */ +const RESIZE_THROTTLE_MS = 80; + export const DefaultAppLayout: React.FC = () => { const uiState = useUIState(); const { refreshStatic, closeTranscript } = useUIActions(); @@ -59,26 +66,72 @@ export const DefaultAppLayout: React.FC = () => { } }, [isTranscriptOpen, refreshStatic]); - // Reflow committed output on terminal resize. Ink's is - // append-only and tracked by array index, so it never redraws already-emitted - // history when the width changes — the terminal re-wraps those stale lines - // into garbage. refreshStatic() clears the screen and bumps historyRemountKey - // so MainContent's remounts and reprints history at the new width. - // Width is the reflow driver (height changes don't mis-wrap committed lines). - // Debounced so a drag-resize doesn't thrash clear/redraw on every intermediate - // size — we only reflow once the drag settles. The initial width is captured - // by useRef so first mount never triggers a clear. - const prevWidthRef = useRef(terminalWidth); + // Clear + reprint the whole frame on terminal resize. + // + // Two things break on resize, both fixed by refreshStatic() (clear the + // screen + remount so history and the live region reprint cleanly + // at the new size): + // 1. history is append-only and index-tracked — it never redraws + // already-committed lines, so the terminal re-wraps them into garbage. + // 2. Ink's live region (composer, sticky task list, status bar) is redrawn + // by erasing the previous frame's *stored* line count. After a resize + // the terminal has reflowed that frame to a different height, so the + // erase misses lines and the stale frame is pushed into scrollback — + // one duplicate per resize event, i.e. an avalanche during a drag. + // + // Throttled (leading + trailing), NOT debounced: a continuous drag emits + // resize events faster than it ever settles, so a trailing-only debounce + // would let the avalanche build until the drag stops. The throttle repaints + // at a bounded cadence *during* the drag and once more when it settles. + // Listens on stdout directly (not via useTerminalSize state) so the handler + // runs synchronously on the event and is immune to React batching. useEffect(() => { - if (prevWidthRef.current === terminalWidth) { - return; - } - const timer = setTimeout(() => { - prevWidthRef.current = terminalWidth; + let lastRun = 0; + let trailing: ReturnType | undefined; + let lastCols = process.stdout.columns; + let lastRows = process.stdout.rows; + + const fire = () => { + lastRun = Date.now(); refreshStatic(); - }, 150); - return () => clearTimeout(timer); - }, [terminalWidth, refreshStatic]); + }; + + const onResize = () => { + const cols = process.stdout.columns; + const rows = process.stdout.rows; + // Some terminals emit 'resize' for non-size events; ignore no-ops. + if (cols === lastCols && rows === lastRows) { + return; + } + lastCols = cols; + lastRows = rows; + + const sinceLast = Date.now() - lastRun; + if (sinceLast >= RESIZE_THROTTLE_MS) { + if (trailing) { + clearTimeout(trailing); + trailing = undefined; + } + fire(); // leading edge — repaint immediately + } else if (!trailing) { + // Coalesce the rest of this burst into one repaint at the throttle + // boundary; keep the pending timer rather than resetting it so a + // sustained drag still repaints every RESIZE_THROTTLE_MS. + trailing = setTimeout(() => { + trailing = undefined; + fire(); + }, RESIZE_THROTTLE_MS - sinceLast); + } + }; + + process.stdout.on('resize', onResize); + return () => { + process.stdout.off('resize', onResize); + if (trailing) { + clearTimeout(trailing); + } + }; + }, [refreshStatic]); if (isTranscriptOpen) { return ;