diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 8d6882ed6..8789be0b3 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -4502,6 +4502,53 @@ describe('registerPtyHandlers', () => { } }) + it('lets active PTY output exceed its old background in-flight cap', async () => { + vi.useFakeTimers() + const activeProc = createMockProc() + spawnMock.mockReturnValue(activeProc.proc) + + try { + registerPtyHandlers(mainWindow as never) + const activeSpawn = (await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd: '/tmp' + })) as { id: string } + const setActiveRendererPty = getPtySetActiveRendererPtyListener() + mainWindow.webContents.send.mockClear() + + activeProc.emitData('x'.repeat(768 * 1024)) + vi.advanceTimersByTime(8) + for (let index = 0; index < 31; index++) { + vi.advanceTimersByTime(1) + } + + expect(mainWindow.webContents.send).toHaveBeenCalledTimes(32) + expect(getPtyRendererDeliveryDebugSnapshot()).toMatchObject({ + pendingPtyCount: 1, + pendingChars: 256 * 1024, + rendererInFlightChars: 512 * 1024, + maxRendererInFlightCharsByPty: 512 * 1024 + }) + + setActiveRendererPty(null, { id: activeSpawn.id, active: true }) + vi.advanceTimersByTime(1) + + expect(mainWindow.webContents.send).toHaveBeenCalledTimes(33) + expect(mainWindow.webContents.send).toHaveBeenNthCalledWith(33, 'pty:data', { + id: activeSpawn.id, + data: 'x'.repeat(16 * 1024) + }) + expect(getPtyRendererDeliveryDebugSnapshot()).toMatchObject({ + pendingChars: 240 * 1024, + rendererInFlightChars: 528 * 1024, + maxRendererInFlightCharsByPty: 528 * 1024 + }) + } finally { + vi.useRealTimers() + } + }) + it('batches stale PTY output after the interactive window expires', async () => { vi.useFakeTimers() const mockProc = createMockProc() diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index e2edb4494..60dad8f05 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -1073,6 +1073,9 @@ export function registerPtyHandlers( const PTY_RENDERER_IN_FLIGHT_HIGH_WATER_CHARS = 512 * 1024 const PTY_RENDERER_TOTAL_IN_FLIGHT_HIGH_WATER_CHARS = 8 * 1024 * 1024 const PTY_RENDERER_INTERACTIVE_RESERVE_CHARS = 256 * 1024 + // Why: active panes need a bounded lane through old hidden bulk output so a + // keystroke redraw can reach the renderer before every background ACK lands. + const PTY_RENDERER_ACTIVE_PTY_IN_FLIGHT_RESERVE_CHARS = 512 * 1024 // Why: keep the immediate path bounded to keystroke-sized TUI redraws; // large output and non-interactive output must still use the batcher. const INTERACTIVE_OUTPUT_WINDOW_MS = 100 @@ -1193,8 +1196,13 @@ export function registerPtyHandlers( const totalLimit = PTY_RENDERER_TOTAL_IN_FLIGHT_HIGH_WATER_CHARS + (options.interactive === true ? PTY_RENDERER_INTERACTIVE_RESERVE_CHARS : 0) + // Why: the reserve is per active PTY, not global; one active pane should + // stay responsive without letting every background pane burst past the cap. + const ptyLimit = + PTY_RENDERER_IN_FLIGHT_HIGH_WATER_CHARS + + (options.interactive === true ? PTY_RENDERER_ACTIVE_PTY_IN_FLIGHT_RESERVE_CHARS : 0) return ( - (rendererInFlightCharsByPty.get(id) ?? 0) < PTY_RENDERER_IN_FLIGHT_HIGH_WATER_CHARS && + (rendererInFlightCharsByPty.get(id) ?? 0) < ptyLimit && rendererInFlightTotalChars < totalLimit ) } 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 000000000..354fff670 --- /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 000000000..909dfe29d --- /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 ea82074ac..40b1acafa 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,127 @@ 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() + + // Why: inactive split restore is frame-spaced, so this waits past one + // scheduler tick without depending on fake timers for xterm callbacks. + 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('drops a deferred inactive hidden restore when the pane is hidden again', 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 + }) + ;(deps.isVisibleRef as { current: boolean }).current = false + await new Promise((resolve) => setTimeout(resolve, 30)) + await flushAsyncTicks(20) + + expect(getMainBufferSnapshot).not.toHaveBeenCalled() + expect(pane.terminal.write).not.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 9a88f4e80..bc5a88273 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,41 @@ export function connectPanePty( if (hiddenOutputRestoreInFlight) { return true } + if (!opts?.bypassScheduler) { + const priority = isActiveSplitPane() ? 'active' : 'inactive' + if (priority === 'inactive') { + if (!hiddenOutputRestoreScheduled) { + hiddenOutputRestoreScheduled = true + const scheduledPtyId = ptyId + const scheduledGeneration = hiddenOutputRestoreGeneration + // 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 + if ( + disposed || + hiddenOutputRestoreGeneration !== scheduledGeneration || + hiddenOutputRestorePtyId !== scheduledPtyId || + transport.getPtyId() !== scheduledPtyId || + !canUseHiddenOutputSnapshot(scheduledPtyId) || + (!hiddenOutputRestoreNeeded && hiddenOutputRestorePendingChunks.length === 0) || + !shouldWritePtyOutputForeground(deps.isVisibleRef.current) + ) { + return + } + requestHiddenOutputRestoreIfNeeded({ bypassScheduler: true }) + }, + priority + ) + } + return true + } + cancelScheduledHiddenOutputRestore(pane.terminal) + hiddenOutputRestoreScheduled = false + } clearHiddenOutputRestoreDeferredRetryTimer() hiddenOutputRestoreRetryDeferred = false