diff --git a/src/renderer/src/components/terminal-pane/pty-connection-hidden-reveal-replay.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-hidden-reveal-replay.test.ts new file mode 100644 index 00000000000..ad9658598e5 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-hidden-reveal-replay.test.ts @@ -0,0 +1,289 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type ConnectCallbacks, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { createInitialStoreState } from './pty-connection-test-store-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep +} = vi.hoisted(() => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn() +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync +})) + +vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({ + ...(await importOriginal>()), + resetAndRefreshAllTerminalWebglAtlases +})) + +vi.mock('./terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ + shouldSeedCacheTimerOnInitialTitle +})) + +vi.mock('sonner', () => ({ + toast: { + info: toastInfo + } +})) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep +})) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn((options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./remote-runtime-pty-transport', () => ({ + createRemoteRuntimePtyTransport: vi.fn( + (_environmentId: string, options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + } + ) +})) + +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + getEagerPtyBufferHandle: vi.fn(() => undefined) + } +}) + +function createDeps(overrides: Record = {}) { + return buildPaneConnectionDeps(() => mockStoreState, overrides) +} + +function writtenText(pane: ReturnType): string { + return pane.terminal.write.mock.calls.map((call) => String(call[0])).join('') +} + +function writeCallOrder(pane: ReturnType, fragment: string): number | undefined { + const index = pane.terminal.write.mock.calls.findIndex((call) => + String(call[0]).includes(fragment) + ) + return index !== -1 ? pane.terminal.write.mock.invocationCallOrder[index] : undefined +} + +describe('hidden-to-visible PTY reveal replay', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + mockStoreState.settings = { + ...mockStoreState.settings, + terminalMainSideEffectAuthority: true + } as StoreState['settings'] + installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + it('replays the current buffer on visibility sync, then resumes live frames', async () => { + 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 snapshot = 'authoritative frame 000010' + const live = 'authoritative frame 000011' + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + getMainBufferSnapshot.mockResolvedValue({ + data: `${snapshot}\r\n`, + cols: 100, + rows: 30, + seq: 64 + }) + + const pane = createPane(1) + const manager = createManager(1) + const isVisibleRef = { current: false } + const binding = connectPanePty( + pane as never, + manager as never, + createDeps({ isVisibleRef }) as never + ) as { + syncProcessTracking: () => void + dispose: () => void + } + await flushAsyncTicks(6) + + capturedDataCallback.current?.('stale hidden frame\r\n', { seq: 16, rawLength: 18 }) + pane.terminal.write.mockClear() + getMainBufferSnapshot.mockClear() + + // Reveal through the pane visibility hook only. Production tab reveal can + // race the restore marker and the backlog-recovery call. + isVisibleRef.current = true + binding.syncProcessTracking() + await flushAsyncTicks(20) + + expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 }) + expect(writtenText(pane)).toContain(snapshot) + + capturedDataCallback.current?.(live, { + seq: 64 + live.length, + rawLength: live.length + }) + await flushAsyncTicks(20) + + expect(writtenText(pane)).toContain(live) + expect(writeCallOrder(pane, snapshot)).toBeLessThan(writeCallOrder(pane, live) ?? 0) + + binding.dispose() + }) + + it('retires a duplicate hidden renderer once so the revealed pane can restore', async () => { + const { connectPanePty } = await import('./pty-connection') + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + getMainBufferSnapshot.mockResolvedValue({ + data: 'authoritative frame 000010\r\n', + cols: 100, + rows: 30, + seq: 64 + }) + + const connectGated = async ( + paneId: number, + isVisibleRef: { current: boolean } + ): Promise<{ + pane: ReturnType + binding: { syncProcessTracking: () => void; dispose: () => void } + dataCallback: (data: string, meta?: { seq?: number; rawLength?: number }) => void + }> => { + 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 pane = createPane(paneId) + const manager = createManager(paneId) + const binding = connectPanePty( + pane as never, + manager as never, + createDeps({ isVisibleRef, restoredLeafId: pane.leafId }) as never + ) as { syncProcessTracking: () => void; dispose: () => void } + await flushAsyncTicks(6) + return { pane, binding, dataCallback: capturedDataCallback.current! } + } + + const retainedVisibleRef = { current: false } + const duplicateVisibleRef = { current: false } + const retained = await connectGated(1, retainedVisibleRef) + const duplicate = await connectGated(2, duplicateVisibleRef) + + retained.dataCallback('stale hidden frame\r\n', { seq: 16, rawLength: 18 }) + duplicate.dataCallback('stale hidden frame\r\n', { seq: 16, rawLength: 18 }) + const setHiddenRendererPty = window.api.pty.setHiddenRendererPty as unknown as ReturnType< + typeof vi.fn + > + expect(setHiddenRendererPty).toHaveBeenCalledWith('pty-id', true) + setHiddenRendererPty.mockClear() + getMainBufferSnapshot.mockClear() + retained.pane.terminal.write.mockClear() + + duplicate.binding.dispose() + retainedVisibleRef.current = true + retained.binding.syncProcessTracking() + await flushAsyncTicks(20) + + expect(setHiddenRendererPty).toHaveBeenCalledTimes(1) + expect(setHiddenRendererPty).toHaveBeenCalledWith('pty-id', false) + expect(getMainBufferSnapshot).toHaveBeenCalledTimes(1) + expect(writtenText(retained.pane)).toContain('authoritative frame 000010') + + retained.binding.dispose() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 18641a18793..7add6a9bc61 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -6287,6 +6287,10 @@ export function connectPanePty( } else if (releaseHiddenDeliveryClaim) { releaseHiddenDeliveryClaim() releaseHiddenDeliveryClaim = null + // Why: unhide must latch restore locally. Waiting for main's one-shot + // marker races light-tab reveal, which no-ops when restore isn't needed + // yet, then live TUI frames never paint the gated-stale alt-screen. + markHiddenOutputRestoreNeeded() } else if (isFirstSyncForPty) { // Why: clear unconditionally on first sync — a stale main-side hidden bit can survive a renderer reload for daemon-backed PTYs that keep their session id. declareRendererPtyDeliveryVisible(ptyId) diff --git a/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.test.ts b/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.test.ts index 5da2177f1b2..e2a447601ec 100644 --- a/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.test.ts @@ -59,4 +59,17 @@ describe('renderer PTY delivery claims', () => { setRendererPtyVisibilityClaim({}, PTY_ID, false) expect(setRendererPtyVisible).toHaveBeenCalledWith(PTY_ID, false) }) + + it('ungates a visible renderer even while a stale hidden owner still holds a claim', () => { + const visiblePane = {} + const staleHidden = acquireHiddenRendererPtyDeliveryClaim(PTY_ID) + expect(setHiddenRendererPty).toHaveBeenLastCalledWith(PTY_ID, true) + + setRendererPtyVisibilityClaim(visiblePane, PTY_ID, true) + expect(setHiddenRendererPty).toHaveBeenLastCalledWith(PTY_ID, false) + + staleHidden() + expect(setHiddenRendererPty).toHaveBeenLastCalledWith(PTY_ID, false) + releaseRendererPtyVisibilityClaim(visiblePane) + }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.ts b/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.ts index 57644da8a83..7185eb1f107 100644 --- a/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.ts +++ b/src/renderer/src/components/terminal-pane/pty-renderer-delivery-claims.ts @@ -7,6 +7,7 @@ type VisibilityClaim = { ptyId: string; visible: boolean } const visibilityClaimsByOwner = new Map() const visibleClaimCounts = new Map() +const lastPublishedHiddenGate = new Map() function sendHiddenState(ptyId: string, hidden: boolean): void { recordTerminalFreezeBreadcrumb(hidden ? 'renderer-gate-mark' : 'renderer-gate-unmark', { @@ -15,20 +16,38 @@ function sendHiddenState(ptyId: string, hidden: boolean): void { ;(globalThis as { window?: Window }).window?.api?.pty?.setHiddenRendererPty?.(ptyId, hidden) } +function shouldGateHiddenRendererPty(ptyId: string): boolean { + return (hiddenClaimCounts.get(ptyId) ?? 0) > 0 && (visibleClaimCounts.get(ptyId) ?? 0) === 0 +} + +function publishHiddenRendererGate(ptyId: string): void { + const hidden = shouldGateHiddenRendererPty(ptyId) + if (lastPublishedHiddenGate.get(ptyId) === hidden) { + return + } + if (lastPublishedHiddenGate.get(ptyId) === undefined && !hidden) { + lastPublishedHiddenGate.set(ptyId, false) + return + } + lastPublishedHiddenGate.set(ptyId, hidden) + sendHiddenState(ptyId, hidden) + if (!hidden && !hiddenClaimCounts.has(ptyId) && !visibleClaimCounts.has(ptyId)) { + lastPublishedHiddenGate.delete(ptyId) + } +} + function sendVisibility(ptyId: string, visible: boolean): void { ;(globalThis as { window?: Window }).window?.api?.pty?.setRendererPtyVisible?.(ptyId, visible) } /** * Holds a hidden-delivery claim until the returned release function runs. - * Main sees only the first-acquire/last-release transitions for each PTY. + * Main stays gated only while some hidden owner exists and no visible owner does. */ export function acquireHiddenRendererPtyDeliveryClaim(ptyId: string): () => void { const nextCount = (hiddenClaimCounts.get(ptyId) ?? 0) + 1 hiddenClaimCounts.set(ptyId, nextCount) - if (nextCount === 1) { - sendHiddenState(ptyId, true) - } + publishHiddenRendererGate(ptyId) let released = false return () => { @@ -39,19 +58,22 @@ export function acquireHiddenRendererPtyDeliveryClaim(ptyId: string): () => void const currentCount = hiddenClaimCounts.get(ptyId) ?? 0 if (currentCount <= 1) { hiddenClaimCounts.delete(ptyId) - sendHiddenState(ptyId, false) + publishHiddenRendererGate(ptyId) return } hiddenClaimCounts.set(ptyId, currentCount - 1) + publishHiddenRendererGate(ptyId) } } /** Clears stale main state for a visible PTY without overriding another live * hidden owner that is still completing a pane-to-watcher handoff. */ export function declareRendererPtyDeliveryVisible(ptyId: string): void { - if (!hiddenClaimCounts.has(ptyId)) { - sendHiddenState(ptyId, false) + if (shouldGateHiddenRendererPty(ptyId)) { + return } + lastPublishedHiddenGate.set(ptyId, false) + sendHiddenState(ptyId, false) } function removeVisibleClaim(claim: VisibilityClaim): boolean { @@ -86,6 +108,9 @@ export function setRendererPtyVisibilityClaim( if (becameHidden && previous.ptyId !== ptyId) { sendVisibility(previous.ptyId, false) } + if (previous.ptyId !== ptyId) { + publishHiddenRendererGate(previous.ptyId) + } } visibilityClaimsByOwner.set(owner, { ptyId, visible }) @@ -95,12 +120,14 @@ export function setRendererPtyVisibilityClaim( if (nextCount === 1) { sendVisibility(ptyId, true) } + publishHiddenRendererGate(ptyId) return } if (!visibleClaimCounts.has(ptyId)) { sendVisibility(ptyId, false) } + publishHiddenRendererGate(ptyId) } export function releaseRendererPtyVisibilityClaim(owner: object): void { @@ -112,6 +139,7 @@ export function releaseRendererPtyVisibilityClaim(owner: object): void { if (removeVisibleClaim(previous)) { sendVisibility(previous.ptyId, false) } + publishHiddenRendererGate(previous.ptyId) } /** Test seam: renderer reload naturally clears these module-scoped claims. */ @@ -119,4 +147,5 @@ export function _resetPtyRendererDeliveryClaimsForTest(): void { hiddenClaimCounts.clear() visibilityClaimsByOwner.clear() visibleClaimCounts.clear() + lastPublishedHiddenGate.clear() }