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 deleted file mode 100644 index b606b9a5ca7..00000000000 --- a/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts +++ /dev/null @@ -1,172 +0,0 @@ -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 a4949090d2f..ac102135d15 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-scroll.ts @@ -89,16 +89,10 @@ export function scheduleSplitScrollRestore( return } const live = getPaneById(paneId) - if (!live?.pendingSplitScrollState) { - return + if (live?.pendingSplitScrollState) { + restoreScrollState(live.terminal, scrollState) + refreshAfterReparent(live) } - // 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) }) }) @@ -114,18 +108,6 @@ 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)