Ignore restore scroll events in visibility memory

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts

@@ -11,7 +11,8 @@ const mocks = vi.hoisted(() => ({
     baseY: 0
   })),
   flushTerminalOutput: vi.fn(),
-  getTerminalOutputEpoch: vi.fn(() => 1)
+  getTerminalOutputEpoch: vi.fn(() => 1),
+  isTerminalScrollRestoreInProgress: vi.fn(() => false)
 }))
 
 const reactRefState = vi.hoisted(() => ({
@@ -65,7 +66,8 @@ vi.mock('@/lib/pane-manager/pane-terminal-output-scheduler', () => ({
 vi.mock('@/lib/pane-manager/pane-scroll', () => ({
   cancelDeferredScrollRestore: mocks.cancelDeferredScrollRestore,
   captureScrollState: mocks.captureScrollState,
-  getTerminalOutputEpoch: mocks.getTerminalOutputEpoch
+  getTerminalOutputEpoch: mocks.getTerminalOutputEpoch,
+  isTerminalScrollRestoreInProgress: mocks.isTerminalScrollRestoreInProgress
 }))
 
 describe('useTerminalScrollVisibilityMemory', () => {
@@ -149,4 +151,51 @@ describe('useTerminalScrollVisibilityMemory', () => {
     expect(cancelAnimationFrame).toHaveBeenCalledWith(7)
     expect(mocks.flushTerminalOutput).not.toHaveBeenCalled()
   })
+
+  it('does not remember scroll events caused by a deferred restore', () => {
+    const onScrollListeners: (() => void)[] = []
+    const terminal = {
+      onScroll: vi.fn((listener: () => void) => {
+        onScrollListeners.push(listener)
+        return { dispose: vi.fn() }
+      })
+    }
+    const manager = {
+      getPanes: vi.fn(() => [{ id: 1, terminal }])
+    }
+    const userScrolledState = {
+      bufferType: 'normal',
+      wasAtBottom: false,
+      viewportY: 42,
+      baseY: 100
+    }
+    const restoreBottomState = {
+      bufferType: 'normal',
+      wasAtBottom: true,
+      viewportY: 100,
+      baseY: 100
+    }
+    mocks.captureScrollState
+      .mockReturnValueOnce(userScrolledState)
+      .mockReturnValueOnce(restoreBottomState)
+
+    beginHookRender()
+    const visibilityMemory = useTerminalScrollVisibilityMemory({
+      managerRef: { current: manager as never },
+      isVisibleRef: { current: true },
+      visibleResumeCompleteRef: { current: true },
+      paneCount: 1
+    })
+
+    const listener = onScrollListeners[0]
+    if (!listener) {
+      throw new Error('expected onScroll listener to be registered')
+    }
+    listener()
+    mocks.isTerminalScrollRestoreInProgress.mockReturnValueOnce(true)
+    listener()
+
+    expect(visibilityMemory.captureViewportPositions(true).get(1)).toBe(userScrolledState)
+    expect(mocks.captureScrollState).toHaveBeenCalledTimes(1)
+  })
 })

src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts

@@ -4,7 +4,8 @@ import { flushTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-sch
 import {
   cancelDeferredScrollRestore,
   captureScrollState,
-  getTerminalOutputEpoch
+  getTerminalOutputEpoch,
+  isTerminalScrollRestoreInProgress
 } from '@/lib/pane-manager/pane-scroll'
 import type { PaneManager } from '@/lib/pane-manager/pane-manager'
 import type { ScrollState } from '@/lib/pane-manager/pane-manager-types'
@@ -190,7 +191,11 @@ export function useTerminalScrollVisibilityMemory({
       disposables.set(
         pane.id,
         onScroll.call(pane.terminal, () => {
-          if (!isVisibleRef.current || suppressScrollTrackingRef.current) {
+          if (
+            !isVisibleRef.current ||
+            suppressScrollTrackingRef.current ||
+            isTerminalScrollRestoreInProgress(pane.terminal)
+          ) {
             return
           }
           rememberVisibleScrollSnapshot(pane.id, pane.terminal)

src/renderer/src/lib/pane-manager/pane-scroll.test.ts

@@ -3,6 +3,7 @@ import type { IMarker, Terminal } from '@xterm/xterm'
 import {
   captureScrollState,
   getTerminalOutputEpoch,
+  isTerminalScrollRestoreInProgress,
   recordTerminalOutput,
   restoreScrollState,
   restoreScrollStateAfterLayout
@@ -104,6 +105,27 @@ describe('scroll state', () => {
     expect(terminal.buffer.active.viewportY).toBe(42)
   })
 
+  it('marks imperative restore scrolls through the current task', () => {
+    vi.useFakeTimers()
+    const terminal = createTerminal({ viewportY: 10, baseY: 100 })
+    ;(terminal.scrollToLine as ReturnType<typeof vi.fn>).mockImplementation((line: number) => {
+      expect(isTerminalScrollRestoreInProgress(terminal)).toBe(true)
+      ;(terminal.buffer.active as { viewportY: number }).viewportY = line
+    })
+    const state: ScrollState = {
+      bufferType: 'normal',
+      wasAtBottom: false,
+      viewportY: 42,
+      baseY: 100
+    }
+
+    restoreScrollState(terminal, state)
+
+    expect(isTerminalScrollRestoreInProgress(terminal)).toBe(true)
+    vi.advanceTimersByTime(0)
+    expect(isTerminalScrollRestoreInProgress(terminal)).toBe(false)
+  })
+
   it('skips restore when the terminal element is gone (post WebGL teardown)', () => {
     const terminal = createTerminal({ viewportY: 10, baseY: 100 })
     ;(terminal as unknown as { element: HTMLElement | undefined }).element = undefined

src/renderer/src/lib/pane-manager/pane-scroll.ts

@@ -7,6 +7,7 @@ import {
 } from './terminal-scroll-restore-debug'
 
 const terminalOutputEpochs = new WeakMap<Terminal, number>()
+const terminalScrollRestoreDepths = new WeakMap<Terminal, number>()
 const deferredScrollRestores = new WeakMap<
   Terminal,
   {
@@ -31,6 +32,10 @@ export function getTerminalOutputEpoch(terminal: Terminal): number {
   return terminalOutputEpochs.get(terminal) ?? 0
 }
 
+export function isTerminalScrollRestoreInProgress(terminal: Terminal): boolean {
+  return (terminalScrollRestoreDepths.get(terminal) ?? 0) > 0
+}
+
 export function cancelDeferredScrollRestore(terminal: Terminal): void {
   const pending = deferredScrollRestores.get(terminal)
   if (!pending) {
@@ -170,7 +175,7 @@ function restoreScrollStateNow(
   // window quietly — the next visibility flip re-fits and re-restores.
   if (state.wasAtBottom) {
     const before = terminalViewportForDebug(terminal)
-    if (safeScrollCall(() => terminal.scrollToBottom())) {
+    if (safeScrollRestoreCall(terminal, () => terminal.scrollToBottom())) {
       if (options.syncScrollbar) {
         forceViewportScrollbarSync(terminal)
       }
@@ -197,7 +202,7 @@ function restoreScrollStateNow(
   // (restoreScrollState, the timeout in
   // restoreScrollStateAfterLayout, cancelDeferredScrollRestore) own disposal.
   const before = terminalViewportForDebug(terminal)
-  if (safeScrollCall(() => terminal.scrollToLine(targetLine))) {
+  if (safeScrollRestoreCall(terminal, () => terminal.scrollToLine(targetLine))) {
     if (options.syncScrollbar) {
       forceViewportScrollbarSync(terminal)
     }
@@ -229,6 +234,26 @@ function safeScrollCall(fn: () => void): boolean {
   }
 }
 
+function safeScrollRestoreCall(terminal: Terminal, fn: () => void): boolean {
+  const depth = terminalScrollRestoreDepths.get(terminal) ?? 0
+  terminalScrollRestoreDepths.set(terminal, depth + 1)
+  try {
+    return safeScrollCall(fn)
+  } finally {
+    // Why: xterm can notify scroll listeners just after the imperative scroll
+    // returns. Keep the restore marker through the current task so those
+    // notifications do not become remembered user scroll positions.
+    setTimeout(() => {
+      const nextDepth = (terminalScrollRestoreDepths.get(terminal) ?? 1) - 1
+      if (nextDepth > 0) {
+        terminalScrollRestoreDepths.set(terminal, nextDepth)
+      } else {
+        terminalScrollRestoreDepths.delete(terminal)
+      }
+    }, 0)
+  }
+}
+
 export function releaseScrollStateMarker(state: ScrollState): void {
   state.firstVisibleLineMarker?.dispose()
   state.firstVisibleLineMarker = undefined
@@ -244,10 +269,10 @@ function forceViewportScrollbarSync(terminal: Terminal): void {
     return
   }
   if (buf.viewportY > 0) {
-    safeScrollCall(() => terminal.scrollLines(-1))
-    safeScrollCall(() => terminal.scrollLines(1))
+    safeScrollRestoreCall(terminal, () => terminal.scrollLines(-1))
+    safeScrollRestoreCall(terminal, () => terminal.scrollLines(1))
   } else if (buf.viewportY < buf.baseY) {
-    safeScrollCall(() => terminal.scrollLines(1))
-    safeScrollCall(() => terminal.scrollLines(-1))
+    safeScrollRestoreCall(terminal, () => terminal.scrollLines(1))
+    safeScrollRestoreCall(terminal, () => terminal.scrollLines(-1))
   }
 }