From 80daa9e01fe782ca07485dcd455b4ddc41600f4b Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 18 Jun 2026 22:58:24 -0700 Subject: [PATCH] fix: settle terminal fit after worktree reveal Co-authored-by: Orca --- .../use-terminal-pane-global-effects.test.ts | 79 +++++++++++++++ .../use-terminal-pane-global-effects.ts | 20 ++++ .../visible-resume-refit-scheduler.ts | 54 ++++++++++ .../worktree-switch-responsiveness.spec.ts | 99 ++++++++++++++++++- 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/components/terminal-pane/visible-resume-refit-scheduler.ts 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 abbddba783e..e129680964c 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 @@ -263,6 +263,85 @@ describe('useTerminalPaneGlobalEffects', () => { expect(isVisibleRef.current).toBe(true) }) + it('schedules settled refits after a hidden terminal becomes visible', () => { + const frameCallbacks: FrameRequestCallback[] = [] + const timerCallbacks: (() => void)[] = [] + ;( + window as unknown as { requestAnimationFrame: typeof requestAnimationFrame } + ).requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => { + frameCallbacks.push(callback) + return frameCallbacks.length + }) + ;( + window as unknown as { cancelAnimationFrame: typeof cancelAnimationFrame } + ).cancelAnimationFrame = vi.fn() + ;( + window as unknown as { setTimeout: (callback: () => void, delay?: number) => number } + ).setTimeout = vi.fn((callback: () => void) => { + timerCallbacks.push(callback) + return timerCallbacks.length + }) + ;(window as unknown as { clearTimeout: (id?: number) => void }).clearTimeout = vi.fn() + + const terminal = { name: 'terminal-a' } + const manager = { + getPanes: vi.fn(() => [{ id: 1, terminal }]), + resumeRendering: vi.fn(), + resetWebglTextureAtlases: vi.fn(), + suspendRendering: vi.fn(), + fitAllPanes: vi.fn(), + getActivePane: vi.fn(() => null), + setActivePane: vi.fn() + } + registerManagerForReset(manager) + + const isActiveRef = { current: false } + const isVisibleRef = { current: false } + + beginHookRender() + useTerminalPaneGlobalEffects({ + tabId: 'tab-1', + worktreeId: 'wt-1', + isActive: false, + isVisible: false, + isSyncFitEnabled: false, + paneCount: 1, + managerRef: { current: manager as never }, + containerRef: { current: null }, + paneTransportsRef: { current: new Map() }, + isActiveRef, + isVisibleRef, + toggleExpandPane: vi.fn() + }) + + beginHookRender() + useTerminalPaneGlobalEffects({ + tabId: 'tab-1', + worktreeId: 'wt-1', + isActive: true, + isVisible: true, + isSyncFitEnabled: true, + paneCount: 1, + managerRef: { current: manager as never }, + containerRef: { current: null }, + paneTransportsRef: { current: new Map() }, + isActiveRef, + isVisibleRef, + toggleExpandPane: vi.fn() + }) + + expect(mocks.fitAndFocusPanes).toHaveBeenCalledTimes(1) + expect(frameCallbacks).toHaveLength(1) + expect(timerCallbacks).toHaveLength(3) + + frameCallbacks[0]?.(16) + for (const callback of timerCallbacks) { + callback() + } + + expect(mocks.fitPanes).toHaveBeenCalledTimes(4) + }) + it('reports the active local PTY to the main output scheduler', () => { const manager = { getPanes: vi.fn(() => [{ id: 1, terminal: { name: 'terminal-a' } }]), 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 b81780d8eb0..943f61aabe7 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 @@ -23,6 +23,7 @@ import { useTerminalScrollVisibilityMemory } from './use-terminal-scroll-visibil import { useTerminalContainerFitSync } from './use-terminal-container-fit-sync' import { pasteTerminalText } from './terminal-bracketed-paste' import { recordTerminalUserInputForLeaf } from './terminal-input-activity' +import { scheduleVisibleResumeSettledRefits } from './visible-resume-refit-scheduler' const VISIBLE_RESUME_FLUSH_CHARS = 256 * 1024 @@ -66,6 +67,7 @@ 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 cancelVisibleResumeSettledRefitsRef = useRef<(() => void) | null>(null) const { captureViewportPositions, withSuppressedScrollTracking, @@ -79,6 +81,13 @@ export function useTerminalPaneGlobalEffects({ }) useTerminalContainerFitSync({ isVisible, isSyncFitEnabled, managerRef, containerRef }) + useEffect(() => { + return () => { + cancelVisibleResumeSettledRefitsRef.current?.() + cancelVisibleResumeSettledRefitsRef.current = null + } + }, []) + useEffect(() => { const manager = managerRef.current if (!manager) { @@ -87,6 +96,7 @@ export function useTerminalPaneGlobalEffects({ isActiveRef.current = isActive isVisibleRef.current = isVisible if (isVisible) { + const resumedFromHidden = !wasVisibleRef.current // 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 @@ -124,10 +134,20 @@ export function useTerminalPaneGlobalEffects({ // terminals; the global reset rebuilds their render models too. resetAllTerminalWebglAtlases() }) + if (resumedFromHidden) { + cancelVisibleResumeSettledRefitsRef.current?.() + cancelVisibleResumeSettledRefitsRef.current = scheduleVisibleResumeSettledRefits({ + manager, + managerRef, + isVisibleRef + }) + } wasVisibleRef.current = true applyPendingFollowOutputRequests() return } else if (wasVisibleRef.current) { + cancelVisibleResumeSettledRefitsRef.current?.() + cancelVisibleResumeSettledRefitsRef.current = null // Why: hidden DOM/layout churn can mutate xterm's viewport before the // pane becomes visible again. Preserve the last visible position. captureViewportPositions(false) diff --git a/src/renderer/src/components/terminal-pane/visible-resume-refit-scheduler.ts b/src/renderer/src/components/terminal-pane/visible-resume-refit-scheduler.ts new file mode 100644 index 00000000000..00da8087b85 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/visible-resume-refit-scheduler.ts @@ -0,0 +1,54 @@ +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { fitPanes } from './pane-helpers' + +const VISIBLE_RESUME_SETTLED_REFIT_DELAYS_MS = [50, 150, 400] as const + +type CurrentRef = { + current: T +} + +export function scheduleVisibleResumeSettledRefits({ + manager, + managerRef, + isVisibleRef +}: { + manager: PaneManager + managerRef: CurrentRef + isVisibleRef: CurrentRef +}): () => void { + let cancelled = false + const animationFrameIds: number[] = [] + const timerIds: number[] = [] + + const fitIfStillVisible = (): void => { + if (cancelled || !isVisibleRef.current || managerRef.current !== manager) { + return + } + fitPanes(manager) + } + + // Why: worktree reveal can briefly report the previous/hidden geometry. + // Retry after paint and layout settle so xterm does not keep stale columns. + if (typeof window.requestAnimationFrame === 'function') { + animationFrameIds.push(window.requestAnimationFrame(fitIfStillVisible)) + } + if (typeof window.setTimeout === 'function') { + for (const delayMs of VISIBLE_RESUME_SETTLED_REFIT_DELAYS_MS) { + timerIds.push(window.setTimeout(fitIfStillVisible, delayMs)) + } + } + + return () => { + cancelled = true + if (typeof window.cancelAnimationFrame === 'function') { + for (const animationFrameId of animationFrameIds) { + window.cancelAnimationFrame(animationFrameId) + } + } + if (typeof window.clearTimeout === 'function') { + for (const timerId of timerIds) { + window.clearTimeout(timerId) + } + } + } +} diff --git a/tests/e2e/worktree-switch-responsiveness.spec.ts b/tests/e2e/worktree-switch-responsiveness.spec.ts index 45a6e6e6b2f..927339af010 100644 --- a/tests/e2e/worktree-switch-responsiveness.spec.ts +++ b/tests/e2e/worktree-switch-responsiveness.spec.ts @@ -1,10 +1,22 @@ import type { Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' -import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + ensureTerminalVisible, + switchToWorktree, + waitForActiveWorktree, + waitForSessionReady +} from './helpers/store' import { worktreeRow } from './worktree-row-locators' const MAX_CLICK_TASK_DURATION_MS = 32 const MAX_CLICK_BACK_TIMER_DRIFT_MS = 32 +const MIN_SETTLED_TERMINAL_COLS = 60 + +type ActiveTerminalGeometry = { + cols: number + rows: number + proposedCols: number | null +} async function prepareSidebarForSwitchTest(page: Page): Promise<[string, string]> { return page.evaluate(async () => { @@ -40,6 +52,55 @@ async function prepareSidebarForSwitchTest(page: Page): Promise<[string, string] }) } +async function getActiveTerminalGeometry(page: Page): Promise { + return page.evaluate(() => { + const visibleOverlayTabId = (() => { + for (const overlay of document.querySelectorAll( + '[data-terminal-overlay-tab-id]' + )) { + const rect = overlay.getBoundingClientRect() + if ( + overlay.dataset.terminalOverlayTabId && + getComputedStyle(overlay).display !== 'none' && + rect.width > 0 && + rect.height > 0 + ) { + return overlay.dataset.terminalOverlayTabId + } + } + return null + })() + const state = window.__store?.getState() + const worktreeId = state?.activeWorktreeId + const storeTabId = + state?.activeTabType === 'terminal' + ? state.activeTabId + : worktreeId + ? (state?.activeTabIdByWorktree?.[worktreeId] ?? null) + : null + const tabId = visibleOverlayTabId ?? storeTabId + const manager = tabId ? window.__paneManagers?.get(tabId) : null + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + if (!pane) { + throw new Error('No active terminal pane is available') + } + const proposed = pane.fitAddon.proposeDimensions() + return { + cols: pane.terminal.cols, + rows: pane.terminal.rows, + proposedCols: proposed?.cols ?? null + } + }) +} + +async function waitForRenderedWorktree(page: Page, worktreeId: string): Promise { + await expect(page.locator('[data-rendered-active-worktree-id]')).toHaveAttribute( + 'data-rendered-active-worktree-id', + worktreeId, + { timeout: 10_000 } + ) +} + test.describe('Worktree switch responsiveness', () => { test.beforeEach(async ({ orcaPage }) => { await waitForSessionReady(orcaPage) @@ -153,4 +214,40 @@ test.describe('Worktree switch responsiveness', () => { renderedWorktreeId: firstWorktreeId }) }) + + test('settles terminal geometry after switching back to a worktree', async ({ orcaPage }) => { + const [firstWorktreeId, secondWorktreeId] = await prepareSidebarForSwitchTest(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForRenderedWorktree(orcaPage, firstWorktreeId) + await expect(orcaPage.locator('.xterm').first()).toBeVisible() + + const initialGeometry = await getActiveTerminalGeometry(orcaPage) + expect(initialGeometry.cols).toBeGreaterThan(MIN_SETTLED_TERMINAL_COLS) + expect(initialGeometry.proposedCols).toBeGreaterThan(MIN_SETTLED_TERMINAL_COLS) + + await switchToWorktree(orcaPage, secondWorktreeId) + await ensureTerminalVisible(orcaPage) + await waitForRenderedWorktree(orcaPage, secondWorktreeId) + await switchToWorktree(orcaPage, firstWorktreeId) + await ensureTerminalVisible(orcaPage) + await waitForRenderedWorktree(orcaPage, firstWorktreeId) + + await expect + .poll( + async () => { + const geometry = await getActiveTerminalGeometry(orcaPage) + return geometry.proposedCols === null + ? geometry.cols + : Math.abs(geometry.cols - geometry.proposedCols) + }, + { + timeout: 5_000, + message: 'terminal columns did not settle back to the measured container width' + } + ) + .toBeLessThanOrEqual(1) + + const finalGeometry = await getActiveTerminalGeometry(orcaPage) + expect(finalGeometry.cols).toBeGreaterThan(MIN_SETTLED_TERMINAL_COLS) + }) })