diff --git a/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts b/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts new file mode 100644 index 000000000..b606b9a5c --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts @@ -0,0 +1,172 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { Terminal } from '@xterm/headless' +import type { Terminal as XtermTerminal } from '@xterm/xterm' +import type { ManagedPaneInternal, ScrollState } from './pane-manager-types' +import { scheduleSplitScrollRestore } from './pane-split-scroll' + +/** + * Why this suite exists: + * PR #1298 added `scheduleSplitScrollRestore` to preserve scroll position + * through split-induced DOM reparenting. The 200ms authoritative phase + * (and the earlier double-rAF phase) each run scroll restore + + * `terminal.refresh(0, rows-1)` on the pane. When a full-screen TUI + * (Claude Code, vim, less) is drawing at the moment the phase fires, the + * restore path repaints rows from xterm's buffer mid-draw and knocks the + * TUI's cursor one row off — the bug surfaced as typed input appearing + * one row below where the TUI expected, with residual fragments left at + * the old cursor row. + * + * What this suite locks down: + * - When the terminal is on the alt-buffer (TUI active) the restore/ + * refresh pair is skipped at *both* phases. The TUI keeps full control + * of its cursor. + * - When the terminal is on the normal buffer, the restore/refresh pair + * still runs — that is the path the #1298 fix originally needed for a + * post-split scrollback repaint. + * - WebGL reattach still fires on alt-buffer: the dead-canvas symptom + * that #1298 fixed still matters; only the scroll-restore side effect + * needs to be suppressed. + */ + +function writeSync(term: Terminal, data: string): Promise { + return new Promise((resolve) => term.write(data, resolve)) +} + +type TestPane = { + id: number + terminal: XtermTerminal + pendingSplitScrollState: ScrollState | null + refreshSpy: ReturnType +} + +function makePane(term: Terminal, id = 1): TestPane { + const refreshSpy = vi.fn() + // Why: the DOM Terminal exposes `refresh(start, end)` for external repaint; + // the headless build does not, because it has no renderer. Stub it in so we + // can assert whether the scheduler chose to call refresh on this pane — + // that is the exact call that was clobbering TUI cursor state before the + // alt-buffer guard landed. + const patched = term as unknown as XtermTerminal & { refresh: typeof refreshSpy } + patched.refresh = refreshSpy + return { + id, + terminal: patched, + pendingSplitScrollState: null, + refreshSpy + } +} + +function toInternal(pane: TestPane): ManagedPaneInternal { + return pane as unknown as ManagedPaneInternal +} + +const idleScrollState: ScrollState = { + wasAtBottom: true, + firstVisibleLineContent: '', + viewportY: 0, + totalLines: 0 +} + +// Why: xterm's write() schedules its own async flush on a timer, which +// deadlocks with vi.useFakeTimers(). Drive escape sequences synchronously by +// briefly switching to real timers. +async function writeWithRealTimers(term: Terminal, data: string): Promise { + vi.useRealTimers() + try { + await writeSync(term, data) + } finally { + vi.useFakeTimers() + } +} + +beforeEach(() => { + // Why: vitest runs in node env (see config/vitest.config.ts), which has no + // requestAnimationFrame. The scheduler uses a double-rAF for its first + // phase; polyfill it so the scheduled work is reachable under fake timers. + ;( + globalThis as unknown as { requestAnimationFrame: (cb: FrameRequestCallback) => number } + ).requestAnimationFrame = (cb) => setTimeout(() => cb(performance.now()), 16) as unknown as number +}) + +afterEach(() => { + delete (globalThis as unknown as { requestAnimationFrame?: unknown }).requestAnimationFrame + vi.useRealTimers() +}) + +describe('scheduleSplitScrollRestore', () => { + it('skips scroll restore + refresh at the 200ms phase when the TUI alt-buffer is active', async () => { + vi.useFakeTimers() + const term = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + // Enter alt-buffer — xterm switches buffer.active.type to 'alternate'. + await writeWithRealTimers(term, '\x1b[?1049h') + expect(term.buffer.active.type).toBe('alternate') + + const pane = makePane(term) + pane.pendingSplitScrollState = idleScrollState + + scheduleSplitScrollRestore( + () => toInternal(pane), + pane.id, + idleScrollState, + () => false + ) + + await vi.advanceTimersByTimeAsync(250) + + expect(pane.refreshSpy).not.toHaveBeenCalled() + // The scroll lock must still be cleared — otherwise safeFit / + // ResizeObserver restores would stay disabled forever on this pane. + expect(pane.pendingSplitScrollState).toBeNull() + + term.dispose() + }) + + it('still reattaches WebGL on alt-buffer (the dead-canvas fix from #1298 must not regress)', async () => { + vi.useFakeTimers() + const term = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + await writeWithRealTimers(term, '\x1b[?1049h') + + const pane = makePane(term) + pane.pendingSplitScrollState = idleScrollState + const reattach = vi.fn() + + scheduleSplitScrollRestore( + () => toInternal(pane), + pane.id, + idleScrollState, + () => false, + reattach + ) + + await vi.advanceTimersByTimeAsync(250) + + expect(reattach).toHaveBeenCalledWith(toInternal(pane)) + + term.dispose() + }) + + it('runs scroll restore + refresh at the 200ms phase on the normal buffer', async () => { + vi.useFakeTimers() + const term = new Terminal({ cols: 80, rows: 24, scrollback: 1000, allowProposedApi: true }) + // Leave on normal buffer so the guard path exercises the non-alt branch. + await writeWithRealTimers(term, 'hello\r\n') + expect(term.buffer.active.type).toBe('normal') + + const pane = makePane(term) + pane.pendingSplitScrollState = idleScrollState + + scheduleSplitScrollRestore( + () => toInternal(pane), + pane.id, + idleScrollState, + () => false + ) + + await vi.advanceTimersByTimeAsync(250) + + expect(pane.refreshSpy).toHaveBeenCalled() + expect(pane.pendingSplitScrollState).toBeNull() + + term.dispose() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-split-scroll.ts b/src/renderer/src/lib/pane-manager/pane-split-scroll.ts index ac102135d..a4949090d 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-scroll.ts @@ -89,10 +89,16 @@ export function scheduleSplitScrollRestore( return } const live = getPaneById(paneId) - if (live?.pendingSplitScrollState) { - restoreScrollState(live.terminal, scrollState) - refreshAfterReparent(live) + if (!live?.pendingSplitScrollState) { + return + } + // Why: see the 200ms timer below — the alt-screen buffer belongs to a + // TUI and restore-during-draw knocks its cursor one row off. + if (live.terminal.buffer.active.type === 'alternate') { + return } + restoreScrollState(live.terminal, scrollState) + refreshAfterReparent(live) }) }) @@ -108,6 +114,18 @@ export function scheduleSplitScrollRestore( if (reattachWebgl) { reattachWebgl(live) } + // Why: the alt-screen buffer belongs to a full-screen TUI (Claude Code, + // vim, less) that owns its cursor position. Re-running scroll restore + // and a full refresh here clobbers an in-progress draw — the + // forceViewportScrollbarSync scrollLines(-1)+scrollLines(1) and the + // refresh(0, rows-1) both repaint rows from xterm's buffer, which + // races the TUI's next write and leaves its cursor one row off (#1298 + // regression). Alt-screen has no scrollback, so scroll restore has + // nothing legitimate to do — skip it, and trust the TUI to repaint + // itself. Normal-screen splits still need the restore. + if (live.terminal.buffer.active.type === 'alternate') { + return + } restoreScrollState(live.terminal, scrollState) refreshAfterReparent(live) }, 200)