From a4c4837a5635dc642e77ecf2bc7c3bd3aabdf115 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 19 May 2026 17:44:18 -0700 Subject: [PATCH] Fix terminal scroll restore behavior (#2368) * Fix terminal scroll restore behavior * Fix terminal pane global effects test args --- src/main/ipc/notifications.test.ts | 3 +- src/main/ipc/notifications.ts | 3 +- src/preload/api-types.ts | 1 + src/preload/index.ts | 2 + .../activity/ActivityPrototypePage.tsx | 3 +- .../components/sidebar/WorktreeCardAgents.tsx | 3 +- .../resource-session-navigation.test.ts | 12 +- .../status-bar/resource-session-navigation.ts | 5 +- .../components/terminal-pane/TerminalPane.tsx | 1 + .../focus-terminal-pane-event.test.ts | 23 +++ .../focus-terminal-pane-event.ts | 12 +- .../terminal-pane/pty-connection.ts | 2 + .../use-terminal-container-fit-sync.ts | 75 +++++++ .../use-terminal-pane-global-effects.test.ts | 17 +- .../use-terminal-pane-global-effects.ts | 163 +++++---------- .../use-terminal-scroll-visibility-memory.ts | 186 ++++++++++++++++++ src/renderer/src/constants/terminal.ts | 2 + src/renderer/src/hooks/useIpcEvents.ts | 16 +- .../src/lib/activate-tab-and-focus-pane.ts | 11 +- .../src/lib/pane-manager/pane-scroll.test.ts | 87 +++++++- .../src/lib/pane-manager/pane-scroll.ts | 89 +++++++++ .../src/lib/pane-manager/pane-tree-ops.ts | 4 +- 22 files changed, 586 insertions(+), 134 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts create mode 100644 src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts diff --git a/src/main/ipc/notifications.test.ts b/src/main/ipc/notifications.test.ts index e01bb6576..2dec1fd2f 100644 --- a/src/main/ipc/notifications.test.ts +++ b/src/main/ipc/notifications.test.ts @@ -251,7 +251,8 @@ describe('registerNotificationHandlers', () => { worktreeId: 'repo::wt1', leafId: '11111111-1111-4111-8111-111111111111', ackPaneKeyOnSuccess: paneKey, - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) }) diff --git a/src/main/ipc/notifications.ts b/src/main/ipc/notifications.ts index 2eae2e4bb..6d24106c5 100644 --- a/src/main/ipc/notifications.ts +++ b/src/main/ipc/notifications.ts @@ -182,7 +182,8 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime worktreeId: args.worktreeId, leafId: paneTarget.leafId, ackPaneKeyOnSuccess: args.paneKey, - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) } }) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 9ae47c883..08a1efaf4 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1708,6 +1708,7 @@ export type PreloadApi = { leafId?: string | null ackPaneKeyOnSuccess?: string flashFocusedPane?: boolean + scrollToBottomIfOutputSinceLastView?: boolean }) => void ) => () => void onFocusEditorTab: ( diff --git a/src/preload/index.ts b/src/preload/index.ts index 6415ee39d..31d1d6537 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2460,6 +2460,7 @@ const api = { leafId?: string | null ackPaneKeyOnSuccess?: string flashFocusedPane?: boolean + scrollToBottomIfOutputSinceLastView?: boolean }) => void ): (() => void) => { const listener = ( @@ -2470,6 +2471,7 @@ const api = { leafId?: string | null ackPaneKeyOnSuccess?: string flashFocusedPane?: boolean + scrollToBottomIfOutputSinceLastView?: boolean } ) => callback(data) ipcRenderer.on('ui:focusTerminal', listener) diff --git a/src/renderer/src/components/activity/ActivityPrototypePage.tsx b/src/renderer/src/components/activity/ActivityPrototypePage.tsx index d9977d65a..3d5937ea6 100644 --- a/src/renderer/src/components/activity/ActivityPrototypePage.tsx +++ b/src/renderer/src/components/activity/ActivityPrototypePage.tsx @@ -1522,7 +1522,8 @@ export default function ActivityPrototypePage(): React.JSX.Element { const parsed = parsePaneKey(thread.paneKey) activateTabAndFocusPane( thread.tab.id, - parsed && parsed.tabId === thread.tab.id ? parsed.leafId : null + parsed && parsed.tabId === thread.tab.id ? parsed.leafId : null, + { scrollToBottomIfOutputSinceLastView: true } ) } diff --git a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx index 6c8ac584c..11e616083 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx @@ -108,7 +108,8 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({ if (tabs.some((t) => t.id === tabId)) { activateTabAndFocusPane(tabId, parsed.leafId, { ackPaneKeyOnSuccess: paneKey, - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) } else { dismissStaleAgentRowByKey(paneKey) diff --git a/src/renderer/src/components/status-bar/resource-session-navigation.test.ts b/src/renderer/src/components/status-bar/resource-session-navigation.test.ts index 6ef4cea36..1fe69c6da 100644 --- a/src/renderer/src/components/status-bar/resource-session-navigation.test.ts +++ b/src/renderer/src/components/status-bar/resource-session-navigation.test.ts @@ -40,7 +40,8 @@ describe('resource session navigation', () => { `tab:${TAB_ID}:${LEAF_ID}` ]) expect(deps.activateTabAndFocusPane).toHaveBeenCalledWith(TAB_ID, LEAF_ID, { - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) }) @@ -48,13 +49,15 @@ describe('resource session navigation', () => { const malformed = makeDeps() navigateResourceSessionToTab(TAB_ID, `${TAB_ID}:1`, malformed) expect(malformed.activateTabAndFocusPane).toHaveBeenCalledWith(TAB_ID, null, { - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) const mismatched = makeDeps() navigateResourceSessionToTab(TAB_ID, `${OTHER_TAB_ID}:${LEAF_ID}`, mismatched) expect(mismatched.activateTabAndFocusPane).toHaveBeenCalledWith(TAB_ID, null, { - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) }) @@ -67,7 +70,8 @@ describe('resource session navigation', () => { expect(deps.activateAndRevealWorktree).not.toHaveBeenCalled() expect(deps.setActiveView).toHaveBeenCalledWith('terminal') expect(deps.activateTabAndFocusPane).toHaveBeenCalledWith(TAB_ID, LEAF_ID, { - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) }) diff --git a/src/renderer/src/components/status-bar/resource-session-navigation.ts b/src/renderer/src/components/status-bar/resource-session-navigation.ts index 3d40edfe3..638d613e1 100644 --- a/src/renderer/src/components/status-bar/resource-session-navigation.ts +++ b/src/renderer/src/components/status-bar/resource-session-navigation.ts @@ -10,7 +10,7 @@ export type ResourceSessionNavigationDeps = { activateTabAndFocusPane: ( tabId: string, leafId: string | null, - opts: { flashFocusedPane: true } + opts: { flashFocusedPane: true; scrollToBottomIfOutputSinceLastView: true } ) => void } @@ -42,6 +42,7 @@ export function navigateResourceSessionToTab( // Legacy numeric keys degrade to tab-only activation instead of guessing. const parsed = paneKey ? parsePaneKey(paneKey) : null deps.activateTabAndFocusPane(tabId, parsed?.tabId === tabId ? parsed.leafId : null, { - flashFocusedPane: true + flashFocusedPane: true, + scrollToBottomIfOutputSinceLastView: true }) } diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 13cbd6033..8c8cd08f1 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -980,6 +980,7 @@ export default function TerminalPane({ cwd, isActive, isVisible, + paneCount, managerRef, containerRef, paneTransportsRef, diff --git a/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.test.ts b/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.test.ts index 5253c9e28..43a295a10 100644 --- a/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.test.ts +++ b/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.test.ts @@ -72,6 +72,29 @@ describe('handleFocusTerminalPaneDetail', () => { expect(surfaceStaleAgentRow).not.toHaveBeenCalled() }) + it('requests follow-output scrolling after resolving the target leaf', () => { + const { manager } = createManager() + const scrollToBottomIfOutputSinceLastView = vi.fn() + + handleFocusTerminalPaneDetail( + { + tabId: 'tab-1', + leafId: LEAF_ID, + scrollToBottomIfOutputSinceLastView: true + }, + { + tabId: 'tab-1', + manager, + acknowledgeAgents: vi.fn(), + surfaceStaleAgentRow: vi.fn(), + scrollToBottomIfOutputSinceLastView + } + ) + + expect(manager.setActivePane).toHaveBeenCalledWith(7, { focus: true }) + expect(scrollToBottomIfOutputSinceLastView).toHaveBeenCalledWith(7) + }) + it('does not focus, flash, or ack when the numeric pane no longer owns the leaf', () => { const { container, manager } = createManager({ leafId: OTHER_LEAF_ID }) const acknowledgeAgents = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.ts b/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.ts index 72497950d..45b9a4f6a 100644 --- a/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.ts +++ b/src/renderer/src/components/terminal-pane/focus-terminal-pane-event.ts @@ -14,11 +14,18 @@ type FocusTerminalPaneEventDeps = { manager: FocusTerminalPaneManager | null acknowledgeAgents: (paneKeys: string[]) => void surfaceStaleAgentRow: (tabId: string, leafId: string) => void + scrollToBottomIfOutputSinceLastView?: (paneId: number) => void } export function handleFocusTerminalPaneDetail( detail: FocusTerminalPaneDetail | undefined, - { tabId, manager, acknowledgeAgents, surfaceStaleAgentRow }: FocusTerminalPaneEventDeps + { + tabId, + manager, + acknowledgeAgents, + surfaceStaleAgentRow, + scrollToBottomIfOutputSinceLastView + }: FocusTerminalPaneEventDeps ): void { if (!detail?.tabId || detail.tabId !== tabId) { return @@ -40,6 +47,9 @@ export function handleFocusTerminalPaneDetail( return } manager.setActivePane(resolution.numericPaneId, { focus: true }) + if (detail.scrollToBottomIfOutputSinceLastView) { + scrollToBottomIfOutputSinceLastView?.(resolution.numericPaneId) + } if (detail.flashFocusedPane) { const pane = manager.getPanes().find((candidate) => candidate.id === resolution.numericPaneId) if (pane) { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 09e3aaf73..71c4e3ec1 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -29,6 +29,7 @@ import { waitForTerminalOutputParsed, writeTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-scheduler' +import { recordTerminalOutput } from '@/lib/pane-manager/pane-scroll' import { makePaneKey } from '../../../../shared/stable-pane-id' import { createTerminalCommandLifecycle } from './terminal-command-lifecycle' import { e2eConfig } from '@/lib/e2e-config' @@ -1137,6 +1138,7 @@ export function connectPanePty( if (terminalOutputPrefersDomRenderer(data)) { manager.markPaneHasComplexScriptOutput(pane.id) } + recordTerminalOutput(pane.terminal) // Why: the active split pane owns keyboard latency. Visible inactive // panes still drain, but through the shared scheduler so a build log in // another split cannot monopolize xterm writes while the user types. diff --git a/src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts b/src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts new file mode 100644 index 000000000..b7a2fb963 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts @@ -0,0 +1,75 @@ +import { useEffect } from 'react' +import { SYNC_FIT_PANES_EVENT } from '@/constants/terminal' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { fitPanes } from './pane-helpers' + +type UseTerminalContainerFitSyncArgs = { + isVisible: boolean + managerRef: React.RefObject + containerRef: React.RefObject +} + +export function useTerminalContainerFitSync({ + isVisible, + managerRef, + containerRef +}: UseTerminalContainerFitSyncArgs): void { + // Why: sidebar open/close toggles dispatch SYNC_FIT_PANES_EVENT from a + // useLayoutEffect (pre-paint, same frame as the width change) so the + // terminal fits synchronously with the new container size, eliminating the + // ~16ms "old cols, new container width" flash that a deferred + // ResizeObserver rAF would otherwise produce. The subsequent per-pane + // ResizeObserver rAF and the 150ms debounced global fit become no-ops + // because proposeDimensions() will match current cols/rows (early-return + // branch in safeFit). Listener is global (not gated on isVisible/isActive) + // so background tabs also fit, keeping their scroll position intact for + // when the user switches back. + useEffect(() => { + const onSyncFit = (): void => { + managerRef.current?.fitAllPanes() + } + window.addEventListener(SYNC_FIT_PANES_EVENT, onSyncFit) + return () => { + window.removeEventListener(SYNC_FIT_PANES_EVENT, onSyncFit) + } + }, [managerRef]) + + useEffect(() => { + if (!isVisible) { + return + } + const container = containerRef.current + if (!container) { + return + } + // Why: ResizeObserver fires on every incremental size change during + // continuous window resizes or layout animations. Each fitPanes() call + // triggers fitAddon.fit() -> terminal.resize() which, when the column + // count changes, reflows the entire scrollback buffer and recalculates + // the viewport scroll position. On Windows, a single reflow of 10 000 + // scrollback lines can block the renderer for 500 ms-2 s, freezing the + // UI while a sidebar opens or a window resizes. + const RESIZE_DEBOUNCE_MS = 150 + let timerId: ReturnType | null = null + const resizeObserver = new ResizeObserver(() => { + if (timerId !== null) { + clearTimeout(timerId) + } + timerId = setTimeout(() => { + timerId = null + const manager = managerRef.current + if (manager) { + fitPanes(manager) + } + }, RESIZE_DEBOUNCE_MS) + }) + resizeObserver.observe(container) + return () => { + resizeObserver.disconnect() + if (timerId !== null) { + clearTimeout(timerId) + } + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isVisible]) +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts index 66500f56b..9e556bead 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts @@ -7,8 +7,10 @@ const mocks = vi.hoisted(() => ({ fitAndFocusPanes: vi.fn(), fitPanes: vi.fn(), flushTerminalOutput: vi.fn(), + getTerminalOutputEpoch: vi.fn(() => 0), handleTerminalFileDrop: vi.fn(), - restoreScrollState: vi.fn() + restoreScrollState: vi.fn(), + restoreScrollStateAfterLayout: vi.fn() })) const reactRefState = vi.hoisted(() => ({ @@ -29,6 +31,7 @@ vi.mock('react', async (importOriginal) => { const actual = await importOriginal() return { ...actual, + useCallback: unknown>(callback: T) => callback, useEffect: (effect: () => void | (() => void)) => { effect() }, @@ -54,7 +57,9 @@ vi.mock('@/lib/pane-manager/pane-terminal-output-scheduler', () => ({ vi.mock('@/lib/pane-manager/pane-scroll', () => ({ captureScrollState: mocks.captureScrollState, - restoreScrollState: mocks.restoreScrollState + getTerminalOutputEpoch: mocks.getTerminalOutputEpoch, + restoreScrollState: mocks.restoreScrollState, + restoreScrollStateAfterLayout: mocks.restoreScrollStateAfterLayout })) vi.mock('./terminal-drop-handler', () => ({ @@ -75,6 +80,7 @@ function useMountForFileDrop( cwd?: string isActive?: boolean isVisible?: boolean + paneCount?: number } = {} ): { onFileDrop: DropCallback @@ -108,6 +114,7 @@ function useMountForFileDrop( cwd: options.cwd, isActive: options.isActive ?? true, isVisible: options.isVisible ?? true, + paneCount: options.paneCount ?? 0, managerRef: { current: manager as never }, containerRef: { current: null }, paneTransportsRef: { current: paneTransports }, @@ -162,7 +169,7 @@ describe('useTerminalPaneGlobalEffects', () => { order.push(`capture:${terminal.name}`) return { terminalName: terminal.name } }) - mocks.restoreScrollState.mockImplementation((terminal: { name: string }) => { + mocks.restoreScrollStateAfterLayout.mockImplementation((terminal: { name: string }) => { order.push(`restore:${terminal.name}`) }) mocks.fitAndFocusPanes.mockImplementation(() => order.push('fit-focus')) @@ -175,6 +182,7 @@ describe('useTerminalPaneGlobalEffects', () => { worktreeId: 'wt-1', isActive: true, isVisible: true, + paneCount: 2, managerRef: { current: manager as never }, containerRef: { current: null }, paneTransportsRef: { current: new Map() }, @@ -222,6 +230,7 @@ describe('useTerminalPaneGlobalEffects', () => { paneTransportsRef: { current: new Map() }, isActiveRef: { current: false }, isVisibleRef: { current: false }, + paneCount: 1, toggleExpandPane: vi.fn() } @@ -250,7 +259,7 @@ describe('useTerminalPaneGlobalEffects', () => { expect(mocks.captureScrollState).toHaveBeenCalledTimes(2) expect(manager.suspendRendering).toHaveBeenCalledTimes(1) - expect(mocks.restoreScrollState).toHaveBeenLastCalledWith(terminalA, preHideState) + expect(mocks.restoreScrollStateAfterLayout).toHaveBeenLastCalledWith(terminalA, preHideState) }) it('ignores terminal file drops for another terminal tab', () => { diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index 4f5048e4e..af84cc9bb 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -2,7 +2,6 @@ import { useEffect, useRef } from 'react' import { FOCUS_TERMINAL_PANE_EVENT, PASTE_TERMINAL_TEXT_EVENT, - SYNC_FIT_PANES_EVENT, TOGGLE_TERMINAL_PANE_EXPAND_EVENT, type FocusTerminalPaneDetail, type PasteTerminalTextDetail @@ -15,8 +14,9 @@ import { flushTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-sch import { handleFocusTerminalPaneDetail } from './focus-terminal-pane-event' import { surfaceStaleAgentRow } from './stale-agent-row' import { useAppStore } from '@/store' -import { captureScrollState, restoreScrollState } from '@/lib/pane-manager/pane-scroll' -import type { ScrollState } from '@/lib/pane-manager/pane-manager-types' +import { restoreScrollStateAfterLayout } from '@/lib/pane-manager/pane-scroll' +import { useTerminalScrollVisibilityMemory } from './use-terminal-scroll-visibility-memory' +import { useTerminalContainerFitSync } from './use-terminal-container-fit-sync' type UseTerminalPaneGlobalEffectsArgs = { tabId: string @@ -24,6 +24,7 @@ type UseTerminalPaneGlobalEffectsArgs = { cwd?: string isActive: boolean isVisible: boolean + paneCount: number managerRef: React.RefObject containerRef: React.RefObject paneTransportsRef: React.RefObject> @@ -38,6 +39,7 @@ export function useTerminalPaneGlobalEffects({ cwd, isActive, isVisible, + paneCount, managerRef, containerRef, paneTransportsRef, @@ -54,59 +56,71 @@ export function useTerminalPaneGlobalEffects({ // otherwise leak WebGL contexts — openTerminal() unconditionally creates // one — and exhaust Chromium's ~8-context budget across worktrees. const wasVisibleRef = useRef(true) - const scrollStatesBeforeHideRef = useRef | null>(null) + const { + captureViewportPositions, + withSuppressedScrollTracking, + applyPendingFollowOutputRequests, + scheduleFollowOutputIfNeeded + } = useTerminalScrollVisibilityMemory({ + managerRef, + isVisibleRef, + visibleResumeCompleteRef: wasVisibleRef, + paneCount + }) + useTerminalContainerFitSync({ isVisible, managerRef, containerRef }) useEffect(() => { const manager = managerRef.current if (!manager) { return } + isActiveRef.current = isActive + isVisibleRef.current = isVisible if (isVisible) { // Why: WebGL resume can disturb xterm's viewport bookkeeping before the // post-resume fit runs. Capture numeric viewport positions first; the // restore path avoids content matching so duplicate agent log lines do // not jump to the wrong history entry. - const viewportPositions = - scrollStatesBeforeHideRef.current && scrollStatesBeforeHideRef.current.size > 0 - ? scrollStatesBeforeHideRef.current - : capturePaneScrollStates(manager) - scrollStatesBeforeHideRef.current = null - // Why: background PTY output is throttled while a pane is not focused; - // flush it before fitting so newly visible terminals paint current state. - for (const pane of manager.getPanes()) { - flushTerminalOutput(pane.terminal) - } - // Resume WebGL immediately so the terminal shows its last-known state - // on the first painted frame. macOS context creation is ~5 ms; on - // Windows (ANGLE → D3D11) it can be 100–500 ms but a deferred resume - // would paint a stretched DOM-fallback flash, which is worse UX. - manager.resumeRendering() - // Single fit on resume. Background bytes have been pushed into xterm - // above, so this fit only absorbs container dimension changes that - // happened while hidden (e.g. sidebar toggle on another worktree). - if (isActive) { - fitAndFocusPanes(manager) - } else { - fitPanes(manager) - } - for (const pane of manager.getPanes()) { - const position = viewportPositions.get(pane.id) - if (position) { - restoreScrollState(pane.terminal, position) + const viewportPositions = captureViewportPositions(!wasVisibleRef.current) + withSuppressedScrollTracking(() => { + // Why: background PTY output is throttled while a pane is not focused; + // flush it before fitting so newly visible terminals paint current state. + for (const pane of manager.getPanes()) { + flushTerminalOutput(pane.terminal) } - } + // Resume WebGL immediately so the terminal shows its last-known state + // on the first painted frame. macOS context creation is ~5 ms; on + // Windows (ANGLE → D3D11) it can be 100–500 ms but a deferred resume + // would paint a stretched DOM-fallback flash, which is worse UX. + manager.resumeRendering() + // Single fit on resume. Background bytes have been pushed into xterm + // above, so this fit only absorbs container dimension changes that + // happened while hidden (e.g. sidebar toggle on another worktree). + if (isActive) { + fitAndFocusPanes(manager) + } else { + fitPanes(manager) + } + for (const pane of manager.getPanes()) { + const position = viewportPositions.get(pane.id) + if (position) { + restoreScrollStateAfterLayout(pane.terminal, position) + } + } + }) + wasVisibleRef.current = true + applyPendingFollowOutputRequests() + return } else if (wasVisibleRef.current) { // Why: hidden DOM/layout churn can mutate xterm's viewport before the // pane becomes visible again. Preserve the last visible position. - scrollStatesBeforeHideRef.current = capturePaneScrollStates(manager) + captureViewportPositions(false) // Suspend WebGL when going hidden. xterm.write() continues to land in // the (now DOM-renderer-fallback or paused-canvas) terminal; the // suspend is purely a GPU resource decision. manager.suspendRendering() } - wasVisibleRef.current = isVisible - isActiveRef.current = isActive - isVisibleRef.current = isVisible + wasVisibleRef.current = false // eslint-disable-next-line react-hooks/exhaustive-deps }, [isActive, isVisible]) @@ -142,12 +156,13 @@ export function useTerminalPaneGlobalEffects({ tabId, manager: managerRef.current, acknowledgeAgents: (paneKeys) => useAppStore.getState().acknowledgeAgents(paneKeys), - surfaceStaleAgentRow + surfaceStaleAgentRow, + scrollToBottomIfOutputSinceLastView: scheduleFollowOutputIfNeeded }) } window.addEventListener(FOCUS_TERMINAL_PANE_EVENT, onFocusPane) return () => window.removeEventListener(FOCUS_TERMINAL_PANE_EVENT, onFocusPane) - }, [tabId, managerRef]) + }, [tabId, managerRef, scheduleFollowOutputIfNeeded]) useEffect(() => { const onPasteText = (event: Event): void => { @@ -170,74 +185,6 @@ export function useTerminalPaneGlobalEffects({ return () => window.removeEventListener(PASTE_TERMINAL_TEXT_EVENT, onPasteText) }, [tabId, managerRef]) - // Why: sidebar open/close toggles dispatch SYNC_FIT_PANES_EVENT from a - // useLayoutEffect (pre-paint, same frame as the width change) so the - // terminal fits synchronously with the new container size, eliminating the - // ~16ms "old cols, new container width" flash that a deferred - // ResizeObserver rAF would otherwise produce. The subsequent per-pane - // ResizeObserver rAF and the 150ms debounced global fit become no-ops - // because proposeDimensions() will match current cols/rows (early-return - // branch in safeFit). Listener is global (not gated on isVisible/isActive) - // so background tabs also fit, keeping their scroll position intact for - // when the user switches back. - useEffect(() => { - const onSyncFit = (): void => { - managerRef.current?.fitAllPanes() - } - window.addEventListener(SYNC_FIT_PANES_EVENT, onSyncFit) - return () => { - window.removeEventListener(SYNC_FIT_PANES_EVENT, onSyncFit) - } - }, [managerRef]) - - useEffect(() => { - if (!isVisible) { - return - } - const container = containerRef.current - if (!container) { - return - } - // Why: ResizeObserver fires on every incremental size change during - // continuous window resizes or layout animations. Each fitPanes() call - // triggers fitAddon.fit() → terminal.resize() which, when the column - // count changes, reflows the entire scrollback buffer and recalculates - // the viewport scroll position. On Windows, a single reflow of 10 000 - // scrollback lines can block the renderer for 500 ms–2 s, freezing the - // UI while a sidebar opens or a window resizes. - // - // A trailing-edge debounce (150 ms) coalesces bursts into one reflow - // after the layout settles. This is longer than the previous RAF-only - // batch (≈16 ms) but still short enough that the user never notices the - // terminal running at a stale column count. - const RESIZE_DEBOUNCE_MS = 150 - let timerId: ReturnType | null = null - const resizeObserver = new ResizeObserver(() => { - if (timerId !== null) { - clearTimeout(timerId) - } - timerId = setTimeout(() => { - timerId = null - const manager = managerRef.current - if (!manager) { - return - } - // safeFit early-returns when proposeDimensions matches current - // cols/rows, so a no-op resize is cheap. Always-live writes mean - // there is no "deferred drain" race; fit can run unconditionally. - fitPanes(manager) - }, RESIZE_DEBOUNCE_MS) - }) - resizeObserver.observe(container) - return () => { - resizeObserver.disconnect() - if (timerId !== null) { - clearTimeout(timerId) - } - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isVisible]) - // Why: dictation events are dispatched globally; gate on isActiveRef so only // the foreground terminal pane consumes the inserted text — otherwise text // would be duplicated across all mounted but inactive tabs. @@ -315,7 +262,3 @@ export function useTerminalPaneGlobalEffects({ }) }, [isActive, isVisible, managerRef, paneTransportsRef, tabId]) } - -function capturePaneScrollStates(manager: PaneManager): Map { - return new Map(manager.getPanes().map((pane) => [pane.id, captureScrollState(pane.terminal)])) -} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts new file mode 100644 index 000000000..a6114b5b9 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts @@ -0,0 +1,186 @@ +import { useCallback, useEffect, useRef } from 'react' +import type { IDisposable, Terminal } from '@xterm/xterm' +import { flushTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-scheduler' +import { + cancelDeferredScrollRestore, + captureScrollState, + getTerminalOutputEpoch +} 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' + +type VisibleScrollSnapshot = { + scrollState: ScrollState + outputEpoch: number +} + +type UseTerminalScrollVisibilityMemoryArgs = { + managerRef: React.RefObject + isVisibleRef: React.RefObject + visibleResumeCompleteRef: React.RefObject + paneCount: number +} + +type TerminalScrollVisibilityMemory = { + captureViewportPositions: (useRememberedSnapshots: boolean) => Map + withSuppressedScrollTracking: (callback: () => void) => void + applyPendingFollowOutputRequests: () => boolean + scheduleFollowOutputIfNeeded: (paneId: number) => void +} + +export function useTerminalScrollVisibilityMemory({ + managerRef, + isVisibleRef, + visibleResumeCompleteRef, + paneCount +}: UseTerminalScrollVisibilityMemoryArgs): TerminalScrollVisibilityMemory { + const visibleScrollSnapshotsRef = useRef>(new Map()) + const scrollDisposablesRef = useRef>(new Map()) + const suppressScrollTrackingRef = useRef(false) + const pendingFollowOutputPaneIdsRef = useRef>(new Set()) + + const captureVisibleScrollSnapshot = useCallback( + (terminal: Terminal): VisibleScrollSnapshot => ({ + scrollState: captureScrollState(terminal), + outputEpoch: getTerminalOutputEpoch(terminal) + }), + [] + ) + + const rememberVisibleScrollSnapshot = useCallback( + (paneId: number, terminal: Terminal): void => { + visibleScrollSnapshotsRef.current.set(paneId, captureVisibleScrollSnapshot(terminal)) + }, + [captureVisibleScrollSnapshot] + ) + + const captureViewportPositions = useCallback( + (useRememberedSnapshots: boolean): Map => { + const manager = managerRef.current + if (!manager) { + return new Map() + } + return new Map( + manager.getPanes().map((pane) => { + const remembered = visibleScrollSnapshotsRef.current.get(pane.id) + if (useRememberedSnapshots && remembered) { + return [pane.id, remembered.scrollState] as const + } + const state = captureScrollState(pane.terminal) + if (!useRememberedSnapshots || !remembered) { + visibleScrollSnapshotsRef.current.set(pane.id, { + scrollState: state, + outputEpoch: getTerminalOutputEpoch(pane.terminal) + }) + } + return [pane.id, state] as const + }) + ) + }, + [managerRef] + ) + + const withSuppressedScrollTracking = useCallback((callback: () => void): void => { + suppressScrollTrackingRef.current = true + try { + callback() + } finally { + suppressScrollTrackingRef.current = false + } + }, []) + + const applyPendingFollowOutputRequests = useCallback((): boolean => { + const pending = pendingFollowOutputPaneIdsRef.current + if (pending.size === 0) { + return false + } + if (!isVisibleRef.current || !visibleResumeCompleteRef.current) { + return false + } + const manager = managerRef.current + if (!manager) { + return false + } + let didScroll = false + for (const pane of manager.getPanes()) { + if (!pending.has(pane.id)) { + continue + } + const previous = visibleScrollSnapshotsRef.current.get(pane.id) + flushTerminalOutput(pane.terminal) + const currentEpoch = getTerminalOutputEpoch(pane.terminal) + const hasNewOutput = previous ? currentEpoch > previous.outputEpoch : currentEpoch > 0 + if (hasNewOutput) { + cancelDeferredScrollRestore(pane.terminal) + pane.terminal.scrollToBottom() + rememberVisibleScrollSnapshot(pane.id, pane.terminal) + didScroll = true + } + pending.delete(pane.id) + } + return didScroll + }, [isVisibleRef, managerRef, rememberVisibleScrollSnapshot, visibleResumeCompleteRef]) + + const scheduleFollowOutputIfNeeded = useCallback( + (paneId: number): void => { + pendingFollowOutputPaneIdsRef.current.add(paneId) + requestAnimationFrame(() => { + requestAnimationFrame(applyPendingFollowOutputRequests) + }) + }, + [applyPendingFollowOutputRequests] + ) + + useEffect(() => { + const manager = managerRef.current + if (!manager) { + return + } + const disposables = scrollDisposablesRef.current + const panes = manager.getPanes() + const livePaneIds = new Set(panes.map((pane) => pane.id)) + for (const [paneId, disposable] of disposables) { + if (!livePaneIds.has(paneId)) { + disposable.dispose() + disposables.delete(paneId) + visibleScrollSnapshotsRef.current.delete(paneId) + pendingFollowOutputPaneIdsRef.current.delete(paneId) + } + } + for (const pane of panes) { + if (disposables.has(pane.id)) { + continue + } + const onScroll = ( + pane.terminal as Terminal & { + onScroll?: (listener: (position: number) => void) => IDisposable + } + ).onScroll + if (typeof onScroll !== 'function') { + continue + } + disposables.set( + pane.id, + onScroll.call(pane.terminal, () => { + if (!isVisibleRef.current || suppressScrollTrackingRef.current) { + return + } + rememberVisibleScrollSnapshot(pane.id, pane.terminal) + }) + ) + } + return () => { + for (const disposable of disposables.values()) { + disposable.dispose() + } + disposables.clear() + } + }, [isVisibleRef, managerRef, paneCount, rememberVisibleScrollSnapshot]) + + return { + captureViewportPositions, + withSuppressedScrollTracking, + applyPendingFollowOutputRequests, + scheduleFollowOutputIfNeeded + } +} diff --git a/src/renderer/src/constants/terminal.ts b/src/renderer/src/constants/terminal.ts index a6c0cabe2..9372493df 100644 --- a/src/renderer/src/constants/terminal.ts +++ b/src/renderer/src/constants/terminal.ts @@ -30,6 +30,8 @@ export type FocusTerminalPaneDetail = { ackPaneKeyOnSuccess?: string /** Briefly lights the resolved pane rim after focus for click-to-locate flows. */ flashFocusedPane?: boolean + /** Follow live agent output when activation is explicitly about that agent. */ + scrollToBottomIfOutputSinceLastView?: boolean } export type PasteTerminalTextDetail = { diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index a580de095..e676bdd28 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -987,7 +987,14 @@ export function useIpcEvents(): void { unsubs.push( window.api.ui.onFocusTerminal( - ({ tabId, worktreeId, leafId, ackPaneKeyOnSuccess, flashFocusedPane }) => { + ({ + tabId, + worktreeId, + leafId, + ackPaneKeyOnSuccess, + flashFocusedPane, + scrollToBottomIfOutputSinceLastView + }) => { const store = useAppStore.getState() store.setActiveWorktree(worktreeId) // Why: CLI-driven focus is a user-initiated switch; stamp focus @@ -996,10 +1003,13 @@ export function useIpcEvents(): void { store.setActiveView('terminal') store.setActiveTab(tabId) store.revealWorktreeInSidebar(worktreeId) - if (ackPaneKeyOnSuccess || flashFocusedPane) { + if (ackPaneKeyOnSuccess || flashFocusedPane || scrollToBottomIfOutputSinceLastView) { activateTabAndFocusPane(tabId, leafId ?? null, { ...(ackPaneKeyOnSuccess ? { ackPaneKeyOnSuccess } : {}), - ...(flashFocusedPane ? { flashFocusedPane: true } : {}) + ...(flashFocusedPane ? { flashFocusedPane: true } : {}), + ...(scrollToBottomIfOutputSinceLastView + ? { scrollToBottomIfOutputSinceLastView: true } + : {}) }) return } diff --git a/src/renderer/src/lib/activate-tab-and-focus-pane.ts b/src/renderer/src/lib/activate-tab-and-focus-pane.ts index f89872855..8f7860bcb 100644 --- a/src/renderer/src/lib/activate-tab-and-focus-pane.ts +++ b/src/renderer/src/lib/activate-tab-and-focus-pane.ts @@ -4,7 +4,11 @@ import { FOCUS_TERMINAL_PANE_EVENT, type FocusTerminalPaneDetail } from '@/const export function activateTabAndFocusPane( tabId: string, leafId: string | null, - opts?: { ackPaneKeyOnSuccess?: string; flashFocusedPane?: boolean } + opts?: { + ackPaneKeyOnSuccess?: string + flashFocusedPane?: boolean + scrollToBottomIfOutputSinceLastView?: boolean + } ): void { useAppStore.getState().setActiveTab(tabId) if (leafId === null) { @@ -17,7 +21,10 @@ export function activateTabAndFocusPane( tabId, leafId, ...(opts?.ackPaneKeyOnSuccess ? { ackPaneKeyOnSuccess: opts.ackPaneKeyOnSuccess } : {}), - ...(opts?.flashFocusedPane ? { flashFocusedPane: true } : {}) + ...(opts?.flashFocusedPane ? { flashFocusedPane: true } : {}), + ...(opts?.scrollToBottomIfOutputSinceLastView + ? { scrollToBottomIfOutputSinceLastView: true } + : {}) } window.dispatchEvent( new CustomEvent(FOCUS_TERMINAL_PANE_EVENT, { diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.test.ts b/src/renderer/src/lib/pane-manager/pane-scroll.test.ts index a1beb6563..c1cd9872c 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.test.ts @@ -1,6 +1,12 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { Terminal } from '@xterm/xterm' -import { captureScrollState, restoreScrollState } from './pane-scroll' +import { + captureScrollState, + getTerminalOutputEpoch, + recordTerminalOutput, + restoreScrollState, + restoreScrollStateAfterLayout +} from './pane-scroll' import type { ScrollState } from './pane-manager-types' function createTerminal(args: { @@ -28,6 +34,11 @@ function createTerminal(args: { } describe('scroll state', () => { + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + it('captures the numeric viewport position', () => { const terminal = createTerminal({ viewportY: 42, baseY: 100 }) @@ -39,6 +50,18 @@ describe('scroll state', () => { }) }) + it('tracks output epochs per terminal', () => { + const terminalA = createTerminal({ viewportY: 0, baseY: 0 }) + const terminalB = createTerminal({ viewportY: 0, baseY: 0 }) + + recordTerminalOutput(terminalA) + recordTerminalOutput(terminalA) + recordTerminalOutput(terminalB) + + expect(getTerminalOutputEpoch(terminalA)).toBe(2) + expect(getTerminalOutputEpoch(terminalB)).toBe(1) + }) + it('restores the captured viewport line', () => { const terminal = createTerminal({ viewportY: 10, baseY: 100 }) const state: ScrollState = { @@ -54,6 +77,66 @@ describe('scroll state', () => { expect(terminal.buffer.active.viewportY).toBe(42) }) + it('reapplies a layout restore after xterm settles asynchronously', () => { + vi.useFakeTimers() + const rafCallbacks: FrameRequestCallback[] = [] + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + rafCallbacks.push(callback) + return rafCallbacks.length + }) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + const terminal = createTerminal({ viewportY: 10, baseY: 100 }) + const state: ScrollState = { + bufferType: 'normal', + wasAtBottom: false, + viewportY: 42, + baseY: 100 + } + + restoreScrollStateAfterLayout(terminal, state) + const activeBuffer = terminal.buffer.active as { viewportY: number } + activeBuffer.viewportY = 0 + rafCallbacks.shift()?.(0) + activeBuffer.viewportY = 0 + vi.advanceTimersByTime(80) + + expect(terminal.buffer.active.viewportY).toBe(42) + expect(terminal.scrollToLine).toHaveBeenCalledWith(42) + }) + + it('does not run stale animation-frame restores after the timeout restore completes', () => { + vi.useFakeTimers() + const rafCallbacks: FrameRequestCallback[] = [] + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + rafCallbacks.push(callback) + return rafCallbacks.length + }) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + const terminal = createTerminal({ viewportY: 10, baseY: 100 }) + const state: ScrollState = { + bufferType: 'normal', + wasAtBottom: false, + viewportY: 42, + baseY: 100 + } + + restoreScrollStateAfterLayout(terminal, state) + vi.advanceTimersByTime(80) + expect(terminal.buffer.active.viewportY).toBe(42) + + const activeBuffer = terminal.buffer.active as { viewportY: number } + activeBuffer.viewportY = 7 + rafCallbacks.shift()?.(0) + + expect(terminal.buffer.active.viewportY).toBe(7) + }) + it('clamps the restored viewport line to the current buffer bottom', () => { const terminal = createTerminal({ viewportY: 10, baseY: 30 }) const state: ScrollState = { diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.ts b/src/renderer/src/lib/pane-manager/pane-scroll.ts index d4d1bb449..d664bb5a1 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.ts @@ -1,6 +1,41 @@ import type { Terminal } from '@xterm/xterm' import type { ScrollState } from './pane-manager-types' +const terminalOutputEpochs = new WeakMap() +const deferredScrollRestores = new WeakMap< + Terminal, + { + cancelled: boolean + rafIds: number[] + timeoutIds: ReturnType[] + } +>() + +export function recordTerminalOutput(terminal: Terminal): void { + terminalOutputEpochs.set(terminal, getTerminalOutputEpoch(terminal) + 1) +} + +export function getTerminalOutputEpoch(terminal: Terminal): number { + return terminalOutputEpochs.get(terminal) ?? 0 +} + +export function cancelDeferredScrollRestore(terminal: Terminal): void { + const pending = deferredScrollRestores.get(terminal) + if (!pending) { + return + } + pending.cancelled = true + if (typeof cancelAnimationFrame === 'function') { + for (const rafId of pending.rafIds) { + cancelAnimationFrame(rafId) + } + } + for (const timeoutId of pending.timeoutIds) { + clearTimeout(timeoutId) + } + deferredScrollRestores.delete(terminal) +} + export function captureScrollState(terminal: Terminal): ScrollState { const buf = terminal.buffer.active return { @@ -12,6 +47,60 @@ export function captureScrollState(terminal: Terminal): ScrollState { } export function restoreScrollState(terminal: Terminal, state: ScrollState): void { + cancelDeferredScrollRestore(terminal) + restoreScrollStateNow(terminal, state) +} + +export function restoreScrollStateAfterLayout(terminal: Terminal, state: ScrollState): void { + cancelDeferredScrollRestore(terminal) + restoreScrollStateNow(terminal, state) + if (typeof requestAnimationFrame !== 'function') { + return + } + + const pending = { + cancelled: false, + rafIds: [] as number[], + timeoutIds: [] as ReturnType[] + } + const restore = (): void => { + if (!pending.cancelled) { + restoreScrollStateNow(terminal, state) + } + } + const cancelPendingRafs = (): void => { + pending.cancelled = true + if (typeof cancelAnimationFrame !== 'function') { + return + } + for (const rafId of pending.rafIds) { + cancelAnimationFrame(rafId) + } + } + const firstRaf = requestAnimationFrame(() => { + restore() + if (pending.cancelled) { + return + } + const secondRaf = requestAnimationFrame(restore) + pending.rafIds.push(secondRaf) + }) + const timeoutId = setTimeout(() => { + if (!pending.cancelled) { + restoreScrollStateNow(terminal, state) + } + // Why: background tabs can throttle rAF past the timeout. Once the + // authoritative timeout restore has run, stale frame callbacks must not + // later rewind a user-initiated scroll or follow-output jump. + cancelPendingRafs() + deferredScrollRestores.delete(terminal) + }, 80) + pending.rafIds.push(firstRaf) + pending.timeoutIds.push(timeoutId) + deferredScrollRestores.set(terminal, pending) +} + +function restoreScrollStateNow(terminal: Terminal, state: ScrollState): void { const buf = terminal.buffer.active if (state.bufferType === 'alternate' || buf.type !== state.bufferType) { return diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts index 377d7fee3..490bf3e8d 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts @@ -8,7 +8,7 @@ import type { import { createDivider } from './pane-divider' import { getFitOverrideForPty } from './mobile-fit-overrides' import { disposeWebgl, attachWebgl } from './pane-webgl-renderer' -import { captureScrollState, restoreScrollState } from './pane-scroll' +import { captureScrollState, restoreScrollStateAfterLayout } from './pane-scroll' export { captureScrollState, restoreScrollState } from './pane-scroll' @@ -73,7 +73,7 @@ export function safeFit(pane: ManagedPane): void { // Container may not have dimensions yet } finally { if (shouldRestoreScroll && scrollState) { - restoreScrollState(pane.terminal, scrollState) + restoreScrollStateAfterLayout(pane.terminal, scrollState) } } }