From 03bcd73c10eb10f14184261a7a1749ef5934b288 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 7 Jun 2026 13:36:18 -0700 Subject: [PATCH] Defer inactive hidden terminal restores --- .../hidden-output-restore-scheduler.test.ts | 70 +++++++++++++++++++ .../hidden-output-restore-scheduler.ts | 67 ++++++++++++++++++ .../terminal-pane/pty-connection.test.ts | 61 ++++++++++++++++ .../terminal-pane/pty-connection.ts | 31 +++++++- 4 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts create mode 100644 src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts diff --git a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts new file mode 100644 index 00000000000..354fff67009 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts @@ -0,0 +1,70 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { + cancelScheduledHiddenOutputRestore, + resetHiddenOutputRestoreSchedulerForTests, + scheduleHiddenOutputRestore +} from './hidden-output-restore-scheduler' + +describe('hidden output restore scheduler', () => { + beforeEach(() => { + vi.useFakeTimers() + resetHiddenOutputRestoreSchedulerForTests() + }) + + afterEach(() => { + resetHiddenOutputRestoreSchedulerForTests() + vi.useRealTimers() + }) + + it('runs active restores immediately', () => { + const target = {} + const requestRestore = vi.fn() + + scheduleHiddenOutputRestore(target, requestRestore, 'active') + + expect(requestRestore).toHaveBeenCalledTimes(1) + }) + + it('spreads inactive restores across timer ticks', () => { + const firstRestore = vi.fn() + const secondRestore = vi.fn() + + scheduleHiddenOutputRestore({}, firstRestore, 'inactive') + scheduleHiddenOutputRestore({}, secondRestore, 'inactive') + + expect(firstRestore).not.toHaveBeenCalled() + expect(secondRestore).not.toHaveBeenCalled() + + vi.advanceTimersByTime(16) + expect(firstRestore).toHaveBeenCalledTimes(1) + expect(secondRestore).not.toHaveBeenCalled() + + vi.advanceTimersByTime(16) + expect(secondRestore).toHaveBeenCalledTimes(1) + }) + + it('cancels pending inactive restore when a target is promoted', () => { + const target = {} + const inactiveRestore = vi.fn() + const activeRestore = vi.fn() + + scheduleHiddenOutputRestore(target, inactiveRestore, 'inactive') + scheduleHiddenOutputRestore(target, activeRestore, 'active') + vi.runOnlyPendingTimers() + + expect(inactiveRestore).not.toHaveBeenCalled() + expect(activeRestore).toHaveBeenCalledTimes(1) + }) + + it('can cancel pending inactive restores', () => { + const target = {} + const requestRestore = vi.fn() + + scheduleHiddenOutputRestore(target, requestRestore, 'inactive') + cancelScheduledHiddenOutputRestore(target) + vi.runOnlyPendingTimers() + + expect(requestRestore).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts new file mode 100644 index 00000000000..909dfe29d59 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts @@ -0,0 +1,67 @@ +type HiddenOutputRestorePriority = 'active' | 'inactive' + +type HiddenOutputRestoreRequest = () => void + +type HiddenOutputRestoreEntry = { + requestRestore: HiddenOutputRestoreRequest +} + +// Why: one inactive xterm scrollback replay per frame keeps tab return focused +// on the active pane while still catching watched split panes up quickly. +const INACTIVE_RESTORE_INTERVAL_MS = 16 + +const inactiveRestoreQueue = new Map() +let inactiveRestoreTimer: ReturnType | null = null + +function clearInactiveRestoreTimer(): void { + if (inactiveRestoreTimer === null) { + return + } + clearTimeout(inactiveRestoreTimer) + inactiveRestoreTimer = null +} + +function scheduleInactiveRestoreDrain(): void { + if (inactiveRestoreTimer !== null || inactiveRestoreQueue.size === 0) { + return + } + inactiveRestoreTimer = setTimeout(drainInactiveRestoreQueue, INACTIVE_RESTORE_INTERVAL_MS) +} + +function drainInactiveRestoreQueue(): void { + inactiveRestoreTimer = null + const next = inactiveRestoreQueue.entries().next() + if (next.done) { + return + } + const [target, entry] = next.value + inactiveRestoreQueue.delete(target) + entry.requestRestore() + scheduleInactiveRestoreDrain() +} + +export function scheduleHiddenOutputRestore( + target: object, + requestRestore: HiddenOutputRestoreRequest, + priority: HiddenOutputRestorePriority +): void { + if (priority === 'active') { + cancelScheduledHiddenOutputRestore(target) + requestRestore() + return + } + inactiveRestoreQueue.set(target, { requestRestore }) + scheduleInactiveRestoreDrain() +} + +export function cancelScheduledHiddenOutputRestore(target: object): void { + inactiveRestoreQueue.delete(target) + if (inactiveRestoreQueue.size === 0) { + clearInactiveRestoreTimer() + } +} + +export function resetHiddenOutputRestoreSchedulerForTests(): void { + inactiveRestoreQueue.clear() + clearInactiveRestoreTimer() +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index ea82074ac3e..bbce562674f 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -3230,6 +3230,67 @@ describe('connectPanePty', () => { disposable.dispose() }) + it('defers inactive split-pane hidden restores', async () => { + const { resetHiddenOutputRestoreSchedulerForTests } = + await import('./hidden-output-restore-scheduler') + let disposable: { dispose: () => void } | null = null + try { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-id') + const capturedDataCallback: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + } + ) + transportFactoryQueue.push(transport) + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + getMainBufferSnapshot.mockResolvedValue({ + data: 'inactive snapshot\r\n', + cols: 100, + rows: 30, + seq: 64 + }) + + const pane = createPane(1) + const manager = createManager(2) + manager.getActivePane.mockReturnValue({ id: 2 }) + const deps = createDeps({ isVisibleRef: { current: false } }) + disposable = connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + const hidden = 'hidden inactive output\r\n' + const live = 'visible inactive output\r\n' + expect(capturedDataCallback.current).not.toBeNull() + capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length }) + ;(deps.isVisibleRef as { current: boolean }).current = true + capturedDataCallback.current?.(live, { + seq: hidden.length + live.length, + rawLength: live.length + }) + await flushAsyncTicks(4) + + expect(getMainBufferSnapshot).not.toHaveBeenCalled() + + await new Promise((resolve) => setTimeout(resolve, 30)) + await flushAsyncTicks(20) + + expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 }) + expect(pane.terminal.write).toHaveBeenCalledWith( + 'inactive snapshot\r\n', + expect.any(Function) + ) + } finally { + disposable?.dispose() + resetHiddenOutputRestoreSchedulerForTests() + } + }) + it('retries hidden remote runtime restore after a null transport snapshot', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('remote:env-1@@terminal-1') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 9a88f4e800d..5e67190b76d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -77,6 +77,10 @@ import { createCommandCodeOutputStatusDetector } from './command-code-output-sta import type { PtyDataMeta } from './pty-dispatcher' import { createTerminalGitHubPRLinkDetector } from '@/lib/terminal-github-pr-link-detector' import { installConptyDeviceAttributesHandler } from './terminal-conpty-device-attributes' +import { + cancelScheduledHiddenOutputRestore, + scheduleHiddenOutputRestore +} from './hidden-output-restore-scheduler' const pendingSpawnByPaneKey = new Map>() const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED' @@ -1850,6 +1854,7 @@ export function connectPanePty( let hiddenOutputRestorePendingOverflow = false let hiddenOutputRestoreFreshSnapshotNeeded = false let hiddenOutputRestoreRetryDeferred = false + let hiddenOutputRestoreScheduled = false let hiddenOutputRestoreDeferredRetryTimer: ReturnType | null = null let hiddenOutputRestoreDeferredRetryAttempts = 0 // Why: hidden recovery state belongs to one PTY stream. Reattach/restart @@ -2204,6 +2209,8 @@ export function connectPanePty( hiddenOutputRestorePendingOverflow = false hiddenOutputRestoreFreshSnapshotNeeded = false hiddenOutputRestoreRetryDeferred = false + hiddenOutputRestoreScheduled = false + cancelScheduledHiddenOutputRestore(pane.terminal) clearHiddenOutputRestoreDeferredRetryTimer() hiddenOutputRestoreDeferredRetryAttempts = 0 } @@ -2334,7 +2341,7 @@ export function connectPanePty( restoreScrollStateAfterSnapshotReplay(scrollState) } - function requestHiddenOutputRestoreIfNeeded(): boolean { + function requestHiddenOutputRestoreIfNeeded(opts?: { bypassScheduler?: boolean }): boolean { resetHiddenOutputRestoreIfPtyChanged() const ptyId = hiddenOutputRestorePtyId ?? transport.getPtyId() if (!hiddenOutputRestoreNeeded && hiddenOutputRestorePendingChunks.length === 0) { @@ -2347,6 +2354,28 @@ export function connectPanePty( if (hiddenOutputRestoreInFlight) { return true } + if (!opts?.bypassScheduler) { + const priority = isActiveSplitPane() ? 'active' : 'inactive' + if (priority === 'inactive') { + if (!hiddenOutputRestoreScheduled) { + hiddenOutputRestoreScheduled = true + // Why: tab/worktree resume can make many split panes visible at once. + // Restore the focused pane immediately and spread inactive replays + // across frames so xterm scrollback replay does not block return. + scheduleHiddenOutputRestore( + pane.terminal, + () => { + hiddenOutputRestoreScheduled = false + requestHiddenOutputRestoreIfNeeded({ bypassScheduler: true }) + }, + priority + ) + } + return true + } + cancelScheduledHiddenOutputRestore(pane.terminal) + hiddenOutputRestoreScheduled = false + } clearHiddenOutputRestoreDeferredRetryTimer() hiddenOutputRestoreRetryDeferred = false