diff --git a/src/main/ipc/pty-hidden-at-spawn-mark.test.ts b/src/main/ipc/pty-hidden-at-spawn-mark.test.ts index 37d301d3af7..0904f47fab3 100644 --- a/src/main/ipc/pty-hidden-at-spawn-mark.test.ts +++ b/src/main/ipc/pty-hidden-at-spawn-mark.test.ts @@ -115,6 +115,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: 'daemon-session', + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop', markerSeq: 42 }) @@ -164,6 +165,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop' }) } finally { diff --git a/src/main/ipc/pty-hidden-delivery-gate.test.ts b/src/main/ipc/pty-hidden-delivery-gate.test.ts index d70eac4c87e..bf32562bc8d 100644 --- a/src/main/ipc/pty-hidden-delivery-gate.test.ts +++ b/src/main/ipc/pty-hidden-delivery-gate.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it } from 'vitest' import { _resetHiddenRendererPtyDeliveryGateForTest, clearHiddenRendererPtyDeliveryState, + consumeHiddenRendererPtyDropMemory, getHiddenRendererPtyDeliveryDebug, isHiddenPtyDeliveryGateEnabled, markHiddenRendererPty, @@ -42,18 +43,91 @@ describe('pty hidden delivery gate', () => { expect(shouldDropHiddenRendererPtyData(PTY_ID, {})).toBe(true) }) - it('requests the restore marker exactly once per drop episode, re-armed by unmark', () => { + it('requests the restore marker exactly once per drop episode, re-armed by the applied ack', () => { markHiddenRendererPty(PTY_ID) - expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(true) + const drop = recordHiddenRendererPtyDataDrop(PTY_ID, 10) + expect(drop.shouldEmitRestoreMarker).toBe(true) expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(false) - // Why: unmark consumes the latch (and re-emits via its own return value); - // the next hidden period's first drop reports again. + // An ack before any reveal belongs to an earlier restore and retires nothing. + consumeHiddenRendererPtyDropMemory(PTY_ID, drop.hiddenDropEpisode) + expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(false) + + // The reveal painted the snapshot: the next episode reports again. unmarkHiddenRendererPty(PTY_ID) + consumeHiddenRendererPtyDropMemory(PTY_ID, drop.hiddenDropEpisode) markHiddenRendererPty(PTY_ID) expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(true) }) + it('keeps drop memory across unhide until the renderer acks the paint (STA-4869)', () => { + markHiddenRendererPty(PTY_ID) + const { hiddenDropEpisode } = recordHiddenRendererPtyDataDrop(PTY_ID, 10) + + // A pane retiring while hidden unhides after unregistering its marker + // handler; every later unhide must still be able to re-emit. + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + + // An ack that names no episode cannot prove what it covered. + consumeHiddenRendererPtyDropMemory(PTY_ID) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + + consumeHiddenRendererPtyDropMemory(PTY_ID, hiddenDropEpisode) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(false) + }) + + it('a drop after the reveal survives the in-flight restore ack and re-emits its marker', () => { + markHiddenRendererPty(PTY_ID) + const first = recordHiddenRendererPtyDataDrop(PTY_ID, 10).hiddenDropEpisode + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + + // The user switches away again before the reveal's snapshot is painted. + markHiddenRendererPty(PTY_ID) + const second = recordHiddenRendererPtyDataDrop(PTY_ID, 10) + expect(second.shouldEmitRestoreMarker).toBe(true) + expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(false) + // That snapshot may predate the new drop, so its ack must not retire the latch. + consumeHiddenRendererPtyDropMemory(PTY_ID, first) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + // Nor after the second reveal: the older restore still covers only episode one. + consumeHiddenRendererPtyDropMemory(PTY_ID, first) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + + consumeHiddenRendererPtyDropMemory(PTY_ID, second.hiddenDropEpisode) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(false) + }) + + it('tags every marker of one drop episode with the same episode id', () => { + markHiddenRendererPty(PTY_ID) + const first = recordHiddenRendererPtyDataDrop(PTY_ID, 10).hiddenDropEpisode + expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).hiddenDropEpisode).toBe(first) + expect(unmarkHiddenRendererPty(PTY_ID).hiddenDropEpisode).toBe(first) + expect(unmarkHiddenRendererPty(PTY_ID).hiddenDropEpisode).toBe(first) + + markHiddenRendererPty(PTY_ID) + const second = recordHiddenRendererPtyDataDrop(PTY_ID, 10).hiddenDropEpisode + expect(second).not.toBe(first) + expect(unmarkHiddenRendererPty(PTY_ID).hiddenDropEpisode).toBe(second) + }) + + it('retires drop memory on teardown', () => { + markHiddenRendererPty(PTY_ID) + recordHiddenRendererPtyDataDrop(PTY_ID, 10) + + clearHiddenRendererPtyDeliveryState(PTY_ID) + markHiddenRendererPty(PTY_ID) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(false) + }) + + it('consuming drop memory it never had is a no-op for other PTYs', () => { + markHiddenRendererPty(PTY_ID) + recordHiddenRendererPtyDataDrop(PTY_ID, 10) + + consumeHiddenRendererPtyDropMemory('pty-2', 1) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + }) + it('keeps drop memory when an already-dropped PTY is re-marked hidden', () => { // Why: a hidden remount or renderer reload re-marks without an unhide in // between — clearing the latch there would make reveal skip the restore. diff --git a/src/main/ipc/pty-hidden-delivery-gate.ts b/src/main/ipc/pty-hidden-delivery-gate.ts index c0d2030cf1b..212860b7ab8 100644 --- a/src/main/ipc/pty-hidden-delivery-gate.ts +++ b/src/main/ipc/pty-hidden-delivery-gate.ts @@ -22,11 +22,18 @@ const hiddenRendererPtys = new Set() // registered interest suppresses the gate for that PTY. const deliveryInterestRendererPtys = new Set() // Why: reveal must restore from the model only when bytes were actually -// dropped. Doubles as the one-shot marker latch: the first gated drop emits a -// restore marker, and the latch is consumed only by unmark (which re-emits) -// or full PTY teardown — never by re-marking hidden, so drop memory survives -// hidden remounts and renderer reloads. -const droppedSinceHiddenPtys = new Set() +// dropped. Doubles as the marker latch: the first gated drop emits a restore +// marker and every unhide re-emits one. It retires only when the renderer acks +// the restore painted, or on PTY teardown — a retiring hidden pane unhides after +// unregistering its marker handler, so spending it on unhide loses the gap. +// 'awaiting-paint' = an unhide re-emitted the marker; a drop after that starts a +// new episode the in-flight restore's snapshot may predate, so its ack must not +// retire it. +const droppedSinceHiddenPtys = new Map< + string, + { state: 'unrevealed' | 'awaiting-paint'; episode: number } +>() +let nextHiddenDropEpisode = 1 // Why: a runtime background spawn has no renderer party to re-mark it after a // reload/crash, so its hidden mark must outlive renderer-scoped resets until a // renderer unmarks it (visible mount) or the PTY is torn down. @@ -49,18 +56,36 @@ export function isHiddenPtyDeliveryGateEnabled( /** Renderer-reported "no visible view needs bytes" bit. Never clears drop * memory: a hidden remount or renderer reload re-marks an already-dropped * PTY, and erasing the latch there would make the eventual reveal skip the - * restore. Unmark is the only consumer of the latch. */ + * restore. */ export function markHiddenRendererPty(id: string): void { hiddenRendererPtys.add(id) } /** Clears the hidden bit. Returns whether bytes were dropped while hidden so - * the caller can emit a restore marker to the now-visible renderer. */ -export function unmarkHiddenRendererPty(id: string): { droppedWhileHidden: boolean } { + * the caller can emit a restore marker. Reads the latch without spending it: + * the marker may reach no handler (see droppedSinceHiddenPtys). */ +export function unmarkHiddenRendererPty(id: string): { + droppedWhileHidden: boolean + hiddenDropEpisode?: number +} { hiddenRendererPtys.delete(id) runtimeOwnedHiddenRendererPtys.delete(id) - const droppedWhileHidden = droppedSinceHiddenPtys.delete(id) - return { droppedWhileHidden } + const latch = droppedSinceHiddenPtys.get(id) + if (!latch) { + return { droppedWhileHidden: false } + } + latch.state = 'awaiting-paint' + return { droppedWhileHidden: true, hiddenDropEpisode: latch.episode } +} + +/** Retires drop memory once the renderer reports painting a restore requested + * for this episode. An older restore's ack must not retire a newer episode, + * and an ack naming no episode retires nothing (fails toward recovery). */ +export function consumeHiddenRendererPtyDropMemory(id: string, hiddenDropEpisode?: number): void { + const latch = droppedSinceHiddenPtys.get(id) + if (latch?.state === 'awaiting-paint' && latch.episode === hiddenDropEpisode) { + droppedSinceHiddenPtys.delete(id) + } } export function isHiddenRendererPty(id: string): boolean { @@ -104,18 +129,20 @@ export function shouldDropHiddenRendererPtyData( } /** Record one gated drop. Returns whether the caller should emit the one-shot - * empty restore-marker chunk (first drop since this PTY went hidden). */ + * empty restore-marker chunk (first drop since the latch was retired or last revealed). */ export function recordHiddenRendererPtyDataDrop( id: string, chars: number -): { shouldEmitRestoreMarker: boolean } { +): { shouldEmitRestoreMarker: boolean; hiddenDropEpisode: number } { droppedHiddenDeliveryChars += chars droppedHiddenDeliveryChunks += 1 - if (droppedSinceHiddenPtys.has(id)) { - return { shouldEmitRestoreMarker: false } + const latch = droppedSinceHiddenPtys.get(id) + if (latch?.state === 'unrevealed') { + return { shouldEmitRestoreMarker: false, hiddenDropEpisode: latch.episode } } - droppedSinceHiddenPtys.add(id) - return { shouldEmitRestoreMarker: true } + const episode = nextHiddenDropEpisode++ + droppedSinceHiddenPtys.set(id, { state: 'unrevealed', episode }) + return { shouldEmitRestoreMarker: true, hiddenDropEpisode: episode } } /** Renderer process replaced (reload / crash): its ref-counted interest diff --git a/src/main/ipc/pty-ipc-hidden-delivery-gate.test.ts b/src/main/ipc/pty-ipc-hidden-delivery-gate.test.ts index 6ad9a5e4f94..03be77d113d 100644 --- a/src/main/ipc/pty-ipc-hidden-delivery-gate.test.ts +++ b/src/main/ipc/pty-ipc-hidden-delivery-gate.test.ts @@ -150,6 +150,7 @@ describe('registerPtyHandlers', () => { // Why out-of-band: an in-band empty pty:data chunk is ambiguous with chunks fully consumed by renderer OSC-9999 stripping. expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: result.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop', markerSeq: 42 }) @@ -280,6 +281,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop' }) } finally { @@ -318,6 +320,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop' }) } finally { @@ -364,6 +367,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop' }) expect(getPtyRendererDeliveryDebugSnapshot()).toMatchObject({ @@ -441,6 +445,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop' }) expect(getPtyRendererDeliveryDebugSnapshot()).toMatchObject({ @@ -475,6 +480,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop' }) vi.advanceTimersByTime(2) @@ -509,6 +515,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(2) expect(mainWindow.webContents.send).toHaveBeenLastCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'unhide' }) @@ -600,6 +607,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(2) expect(mainWindow.webContents.send).toHaveBeenLastCalledWith('pty:modelRestoreNeeded', { id: spawnResult.id, + hiddenDropEpisode: expect.any(Number), reason: 'unhide' }) } finally { @@ -652,6 +660,7 @@ describe('registerPtyHandlers', () => { setHidden(null, { id: result.id, hidden: false }) expect(mainWindow.webContents.send).toHaveBeenLastCalledWith('pty:modelRestoreNeeded', { id: result.id, + hiddenDropEpisode: expect.any(Number), reason: 'unhide', markerSeq: 42 }) @@ -709,6 +718,7 @@ describe('registerPtyHandlers', () => { expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { id: result.id, + hiddenDropEpisode: expect.any(Number), reason: 'hidden-drop', markerSeq: 42 }) diff --git a/src/main/ipc/pty-ipc-hidden-restore-ack.test.ts b/src/main/ipc/pty-ipc-hidden-restore-ack.test.ts new file mode 100644 index 00000000000..bf1a560a141 --- /dev/null +++ b/src/main/ipc/pty-ipc-hidden-restore-ack.test.ts @@ -0,0 +1,277 @@ +import { describe, expect, it, vi } from 'vitest' +import { setupPtyIpcSuite } from './pty-ipc-test-harness' +import { registerPtyHandlers } from './pty/register-handlers' + +// STA-4869 — main's hidden-delivery drop latch retires on the renderer's +// applied-ack, never on serving the snapshot. Serving proves only that main +// answered: the pane can dispose across the multi-MB serialize + IPC round trip +// (tab/split/worktree switch, renderer reload), and a snapshot nobody painted +// heals nothing. Anything that retires the latch earlier spends the last +// recovery signal on a marker no view will ever receive. + +vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) +vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock())) +vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock())) +vi.mock('node:child_process', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal()) +) +vi.mock('../opencode/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock()) +) +vi.mock('../mimo/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock()) +) +vi.mock('../agent-hooks/server', () => + import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock()) +) +vi.mock('../pi/titlebar-extension-service', () => + import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock()) +) +vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock())) +vi.mock('../wsl', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal()) +) +vi.mock('../telemetry/client', () => + import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock()) +) +vi.mock('../telemetry/classify-error', () => + import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock()) +) +vi.mock('../cli/linux-terminal-orca-cli-shim', () => + import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock()) +) +vi.mock('../memory/pty-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock()) +) +vi.mock('../agent-hooks/migration-unsupported-pty-state', () => + import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock()) +) +vi.mock('../codex/codex-pane-account-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock()) +) +vi.mock('../codex/codex-state-db-backfill-recovery', () => + import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock()) +) + +describe('registerPtyHandlers hidden-output restore ack', () => { + const { + handlers, + mainWindow, + installObservableDaemonTestProvider, + getPtySetHiddenRendererPtyListener, + getPtyHiddenOutputRestoreAppliedListener + } = setupPtyIpcSuite() + + function buildRuntime(): Record { + return { + setPtyController: vi.fn(), + registerPty: vi.fn(), + noteTerminalSpawnCommand: vi.fn(), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: vi.fn(() => 42), + getPtyOutputSequence: vi.fn(() => 42), + hasRemoteTerminalViewSubscriber: vi.fn(() => false), + createPreAllocatedTerminalHandle: vi.fn(() => 'terminal-handle-1'), + registerPreAllocatedHandleForPty: vi.fn(), + serializeHiddenOutputRecoveryBuffer: vi.fn(async () => ({ + data: 'retained while hidden', + cols: 80, + rows: 24, + seq: 42 + })) + } + } + + /** Registers the handlers against the suite's mock window and spawns one daemon PTY. */ + async function registerAndSpawnDaemonPty(runtime: Record): Promise { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the suite's mock window and this runtime stub implement every member the spawn path touches. + registerPtyHandlers(mainWindow as never, runtime as never) + const spawn = handlers.get('pty:spawn') + if (!spawn) { + throw new Error('missing pty:spawn handler') + } + const result: unknown = await spawn(null, { + cols: 80, + rows: 24, + sessionId: 'daemon-session' + }) + if ( + !result || + typeof result !== 'object' || + !('id' in result) || + typeof result.id !== 'string' + ) { + throw new Error('pty:spawn returned no id') + } + return result.id + } + + /** The drop episode main put on its most recent restore marker. */ + function lastMarkerEpisode(): number { + const markers = mainWindow.webContents.send.mock.calls.filter( + (call: unknown[]) => call[0] === 'pty:modelRestoreNeeded' + ) + const payload: unknown = markers.at(-1)?.[1] + if ( + !payload || + typeof payload !== 'object' || + !('hiddenDropEpisode' in payload) || + typeof payload.hiddenDropEpisode !== 'number' + ) { + throw new Error('no restore marker carried a drop episode') + } + return payload.hiddenDropEpisode + } + + /** One gated drop episode on a fresh daemon PTY, with the marker count reset. */ + async function armDropLatch(): Promise<{ + id: string + setHidden: (event: unknown, args: { id: string; hidden: boolean }) => void + restoreMarkers: () => number + }> { + const daemon = installObservableDaemonTestProvider() + const result = { id: await registerAndSpawnDaemonPty(buildRuntime()) } + const setHidden = getPtySetHiddenRendererPtyListener() + mainWindow.webContents.send.mockClear() + + setHidden(null, { id: result.id, hidden: true }) + daemon.emitData(result.id, 'hidden output') + vi.advanceTimersByTime(50) + const restoreMarkers = (): number => + mainWindow.webContents.send.mock.calls.filter( + (call: unknown[]) => call[0] === 'pty:modelRestoreNeeded' + ).length + expect(restoreMarkers()).toBe(1) + return { id: result.id, setHidden, restoreMarkers } + } + + it('retires drop memory on the applied ack, not on unhide or on serving the snapshot', async () => { + vi.useFakeTimers() + try { + const { id, setHidden, restoreMarkers } = await armDropLatch() + const getSnapshot = handlers.get('pty:getMainBufferSnapshot')! + const ackApplied = getPtyHiddenOutputRestoreAppliedListener() + + // A pane retiring while hidden unhides after unregistering its marker + // handler, so the first unhide marker reaches nobody. The replacement's + // own unhide must still be able to ask for the restore. + setHidden(null, { id, hidden: false }) + setHidden(null, { id, hidden: false }) + expect(restoreMarkers()).toBe(3) + expect(mainWindow.webContents.send).toHaveBeenLastCalledWith('pty:modelRestoreNeeded', { + id, + hiddenDropEpisode: expect.any(Number), + reason: 'unhide', + markerSeq: 42 + }) + + // Serving the snapshot proves only that main answered — the asking pane + // can still die before it paints. + await getSnapshot(null, { id }) + setHidden(null, { id, hidden: false }) + expect(restoreMarkers()).toBe(4) + + // The ack says the bytes are on screen: no marker storm afterwards. + ackApplied(null, { id, hiddenDropEpisode: lastMarkerEpisode() }) + setHidden(null, { id, hidden: false }) + setHidden(null, { id, hidden: false }) + expect(restoreMarkers()).toBe(4) + } finally { + vi.useRealTimers() + } + }) + + it('re-arms the marker for a fresh drop episode after the ack', async () => { + vi.useFakeTimers() + try { + const daemon = installObservableDaemonTestProvider() + const result = { id: await registerAndSpawnDaemonPty(buildRuntime()) } + const setHidden = getPtySetHiddenRendererPtyListener() + const ackApplied = getPtyHiddenOutputRestoreAppliedListener() + mainWindow.webContents.send.mockClear() + + setHidden(null, { id: result.id, hidden: true }) + daemon.emitData(result.id, 'hidden output') + vi.advanceTimersByTime(50) + setHidden(null, { id: result.id, hidden: false }) + ackApplied(null, { id: result.id, hiddenDropEpisode: lastMarkerEpisode() }) + + setHidden(null, { id: result.id, hidden: true }) + daemon.emitData(result.id, 'hidden again') + vi.advanceTimersByTime(50) + + // hidden-drop, unhide, then the fresh episode's hidden-drop. + expect( + mainWindow.webContents.send.mock.calls.filter( + (call: unknown[]) => call[0] === 'pty:modelRestoreNeeded' + ).length + ).toBe(3) + } finally { + vi.useRealTimers() + } + }) + + it('ignores a repeat ack and an ack for a PTY it knows nothing about', async () => { + vi.useFakeTimers() + try { + const { id, setHidden, restoreMarkers } = await armDropLatch() + const ackApplied = getPtyHiddenOutputRestoreAppliedListener() + + // Another pane's ack must never retire this PTY's drop memory. + ackApplied(null, { id: 'some-other-pty' }) + ackApplied(null, {}) + ackApplied(null, { id: '' }) + ackApplied(null, { id: 42 }) + setHidden(null, { id, hidden: false }) + expect(restoreMarkers()).toBe(2) + + // An ack naming no episode, or a stale one, cannot prove it covered this gap. + ackApplied(null, { id }) + ackApplied(null, { id, hiddenDropEpisode: lastMarkerEpisode() - 1 }) + setHidden(null, { id, hidden: false }) + expect(restoreMarkers()).toBe(3) + + const episode = lastMarkerEpisode() + ackApplied(null, { id, hiddenDropEpisode: episode }) + ackApplied(null, { id, hiddenDropEpisode: episode }) + setHidden(null, { id, hidden: false }) + expect(restoreMarkers()).toBe(3) + } finally { + vi.useRealTimers() + } + }) + + it('keeps drop memory armed when the snapshot is refused', async () => { + vi.useFakeTimers() + try { + const daemon = installObservableDaemonTestProvider() + const runtime = buildRuntime() + runtime.serializeHiddenOutputRecoveryBuffer = vi.fn(async () => null) + const result = { id: await registerAndSpawnDaemonPty(runtime) } + const setHidden = getPtySetHiddenRendererPtyListener() + mainWindow.webContents.send.mockClear() + + setHidden(null, { id: result.id, hidden: true }) + daemon.emitData(result.id, 'hidden output') + vi.advanceTimersByTime(50) + + // Why: a refused snapshot painted nothing, so the next reveal still owes + // the renderer a restore signal. + expect( + await handlers.get('pty:getMainBufferSnapshot')!(null, { + id: result.id + }) + ).toBeNull() + setHidden(null, { id: result.id, hidden: false }) + expect(mainWindow.webContents.send).toHaveBeenLastCalledWith('pty:modelRestoreNeeded', { + id: result.id, + hiddenDropEpisode: expect.any(Number), + reason: 'unhide', + markerSeq: 42 + }) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/main/ipc/pty-ipc-listener-accessors.ts b/src/main/ipc/pty-ipc-listener-accessors.ts index 22d60b63985..ee1340f19bb 100644 --- a/src/main/ipc/pty-ipc-listener-accessors.ts +++ b/src/main/ipc/pty-ipc-listener-accessors.ts @@ -113,6 +113,23 @@ export function createPtyIpcListenerAccessors(ctx: { return hiddenCall[1] as (event: unknown, args: { id: string; hidden: boolean }) => void } + function getPtyHiddenOutputRestoreAppliedListener(): ( + event: unknown, + args: { id?: unknown; hiddenDropEpisode?: unknown } + ) => void { + const appliedCall = onMock.mock.calls.find( + (call: unknown[]) => call[0] === 'pty:hiddenOutputRestoreApplied' + ) + if (!appliedCall) { + throw new Error('missing pty:hiddenOutputRestoreApplied listener') + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: found by channel name, so this is the listener resize-visibility.ts registered with this signature. + return appliedCall[1] as ( + event: unknown, + args: { id?: unknown; hiddenDropEpisode?: unknown } + ) => void + } + function getPtySetDeliveryInterestListener(): ( event: unknown, args: { id: string; interested: boolean } @@ -188,6 +205,7 @@ export function createPtyIpcListenerAccessors(ctx: { getMainFrameNavigationListener, getPtyResizeListener, getPtySetHiddenRendererPtyListener, + getPtyHiddenOutputRestoreAppliedListener, getPtySetDeliveryInterestListener, DELIVERY_RESYNC_UNANSWERED_WARNING, countResyncUnansweredWarnings, diff --git a/src/main/ipc/pty/delivery/accept.ts b/src/main/ipc/pty/delivery/accept.ts index 44e1544e7d5..4b266a7df52 100644 --- a/src/main/ipc/pty/delivery/accept.ts +++ b/src/main/ipc/pty/delivery/accept.ts @@ -59,7 +59,13 @@ export function acceptPtyDataForRenderer( const drop = recordHiddenRendererPtyDataDrop(payload.id, droppedChars) warnIfDroppingHiddenBytesForVisiblePty(session, payload.id, droppedChars) if (drop.shouldEmitRestoreMarker) { - sendModelRestoreNeededMarker(session, payload.id, 'hidden-drop', outputSeq) + sendModelRestoreNeededMarker( + session, + payload.id, + 'hidden-drop', + outputSeq, + drop.hiddenDropEpisode + ) } return } diff --git a/src/main/ipc/pty/delivery/flush.ts b/src/main/ipc/pty/delivery/flush.ts index 917825418c1..8de4463bb35 100644 --- a/src/main/ipc/pty/delivery/flush.ts +++ b/src/main/ipc/pty/delivery/flush.ts @@ -115,7 +115,8 @@ export function flushPendingData(session: PtyIpcSession): void { session, id, 'hidden-drop', - session.runtime?.getPtyOutputSequence(id) + session.runtime?.getPtyOutputSequence(id), + drop.hiddenDropEpisode ) } continue diff --git a/src/main/ipc/pty/delivery/hidden-transition.ts b/src/main/ipc/pty/delivery/hidden-transition.ts index a6ca1204875..a6026e239b9 100644 --- a/src/main/ipc/pty/delivery/hidden-transition.ts +++ b/src/main/ipc/pty/delivery/hidden-transition.ts @@ -10,17 +10,22 @@ export function transitionHiddenRendererPtyDeliveryState( session: PtyIpcSession, id: string, hidden: boolean -): { droppable: boolean; droppedWhileHidden: boolean; policyChanged: boolean } { +): { + droppable: boolean + droppedWhileHidden: boolean + hiddenDropEpisode?: number + policyChanged: boolean +} { const settings = session.getSettings?.() const wasDroppable = shouldDropHiddenRendererPtyData(id, settings) - let droppedWhileHidden = false + let unmarked: ReturnType = { droppedWhileHidden: false } if (hidden) { markHiddenRendererPty(id) } else { - droppedWhileHidden = unmarkHiddenRendererPty(id).droppedWhileHidden + unmarked = unmarkHiddenRendererPty(id) } const droppable = shouldDropHiddenRendererPtyData(id, settings) - return { droppable, droppedWhileHidden, policyChanged: wasDroppable !== droppable } + return { droppable, ...unmarked, policyChanged: wasDroppable !== droppable } } export function transitionSpawnHiddenRendererPtyDeliveryState( diff --git a/src/main/ipc/pty/delivery/payload.ts b/src/main/ipc/pty/delivery/payload.ts index ed2fc87da88..28782310346 100644 --- a/src/main/ipc/pty/delivery/payload.ts +++ b/src/main/ipc/pty/delivery/payload.ts @@ -36,7 +36,8 @@ export function sendModelRestoreNeededMarker( session: PtyIpcSession, id: string, reason: PtyModelRestoreReason, - markerSeq: number | undefined + markerSeq: number | undefined, + hiddenDropEpisode?: number ): boolean { if (!session.mainWindow || session.mainWindow.isDestroyed()) { return false @@ -45,7 +46,8 @@ export function sendModelRestoreNeededMarker( session.mainWindow.webContents.send('pty:modelRestoreNeeded', { id, reason, - ...(typeof markerSeq === 'number' ? { markerSeq } : {}) + ...(typeof markerSeq === 'number' ? { markerSeq } : {}), + ...(typeof hiddenDropEpisode === 'number' ? { hiddenDropEpisode } : {}) }) } catch (error) { // Why: a disposed render frame throws synchronously here, and this rides the data path. diff --git a/src/main/ipc/pty/ipc/resize-visibility.ts b/src/main/ipc/pty/ipc/resize-visibility.ts index 35b08d10fd6..af8a88241f2 100644 --- a/src/main/ipc/pty/ipc/resize-visibility.ts +++ b/src/main/ipc/pty/ipc/resize-visibility.ts @@ -7,6 +7,7 @@ import type { import { redactPtyIdForDiagnostics } from '../../../../shared/pty-delivery-diagnostics' import { installTerminalViewAttributesIpc } from './terminal-view-attributes-ipc' import { + consumeHiddenRendererPtyDropMemory, recordHiddenRendererPtyDataDrop, setRendererPtyDeliveryInterest, shouldDropHiddenRendererPtyData @@ -78,6 +79,21 @@ export function installPtyResizeVisibilityIpc(session: PtyIpcSession): void { } }) + // Why the ack and not the serve: the pane can dispose across the serialize + IPC round trip, + // so the drop latch stays armed until the restored bytes are actually painted (#15504). + ipcMain.removeAllListeners('pty:hiddenOutputRestoreApplied') + ipcMain.on( + 'pty:hiddenOutputRestoreApplied', + (_event, args: { id?: unknown; hiddenDropEpisode?: unknown }) => { + if (typeof args?.id !== 'string' || !args.id) { + return + } + const episode = + typeof args.hiddenDropEpisode === 'number' ? args.hiddenDropEpisode : undefined + consumeHiddenRendererPtyDropMemory(args.id, episode) + } + ) + // Why: renderer ACKs bound main→renderer delivery without stopping PTY ingestion — agent/status consumers still see every chunk via the provider/runtime path. ipcMain.removeAllListeners('pty:ackData') ipcMain.on( @@ -252,7 +268,8 @@ export function installPtyResizeVisibilityIpc(session: PtyIpcSession): void { session, args.id, 'hidden-drop', - runtime?.getPtyOutputSequence(args.id) + runtime?.getPtyOutputSequence(args.id), + drop.hiddenDropEpisode ) } } @@ -272,7 +289,8 @@ export function installPtyResizeVisibilityIpc(session: PtyIpcSession): void { session, args.id, 'unhide', - runtime?.getPtyOutputSequence(args.id) + runtime?.getPtyOutputSequence(args.id), + transition.hiddenDropEpisode ) } }) diff --git a/src/main/ipc/pty/register-handlers.ts b/src/main/ipc/pty/register-handlers.ts index b40aececc21..297c9b58ec5 100644 --- a/src/main/ipc/pty/register-handlers.ts +++ b/src/main/ipc/pty/register-handlers.ts @@ -153,6 +153,7 @@ export function registerPtyHandlers( ipcMain.removeHandler('pty:writeAccepted') ipcMain.removeAllListeners('pty:write') ipcMain.removeAllListeners('pty:ackColdRestore') + ipcMain.removeAllListeners('pty:hiddenOutputRestoreApplied') ipcMain.removeAllListeners('pty:ackData') ipcMain.removeAllListeners('pty:deliveryResyncResponse') ipcMain.removeAllListeners('pty:serializeBuffer:response') diff --git a/src/main/ipc/pty/session.ts b/src/main/ipc/pty/session.ts index 299215d3c4e..63d97259ce9 100644 --- a/src/main/ipc/pty/session.ts +++ b/src/main/ipc/pty/session.ts @@ -156,7 +156,12 @@ export type PtyIpcSession = { transitionHiddenRendererPtyDeliveryState: ( id: string, hidden: boolean - ) => { droppable: boolean; droppedWhileHidden: boolean; policyChanged: boolean } + ) => { + droppable: boolean + droppedWhileHidden: boolean + hiddenDropEpisode?: number + policyChanged: boolean + } transitionSpawnHiddenRendererPtyDeliveryState: (id: string, hidden: boolean) => void rendererPtyIsKnownHidden: (id: string) => boolean schedulePendingDataAfterCreditReport: (creditedAny: boolean) => void diff --git a/src/preload/api/pty-api.ts b/src/preload/api/pty-api.ts index ad6663256e7..3ca060012e0 100644 --- a/src/preload/api/pty-api.ts +++ b/src/preload/api/pty-api.ts @@ -108,6 +108,8 @@ export type PtyApi = { resetInputModes: (id: string) => void kill: (id: string, opts?: { keepHistory?: boolean }) => Promise ackColdRestore: (id: string) => void + /** Hidden-output recovery painted main's snapshot; retires the hidden-delivery drop latch. */ + ackHiddenOutputRestoreApplied: (id: string, hiddenDropEpisode?: number) => void ackData: (id: string, charCount: number, processedChars?: number) => void onDeliveryResyncRequest: (callback: (payload: { requestId: number }) => void) => () => void respondDeliveryResync: (payload: { diff --git a/src/preload/api/pty-bridge-session-control.ts b/src/preload/api/pty-bridge-session-control.ts index ac235e62b63..1bd100352d0 100644 --- a/src/preload/api/pty-bridge-session-control.ts +++ b/src/preload/api/pty-bridge-session-control.ts @@ -122,6 +122,9 @@ export const ptySessionControlApi = { ackColdRestore: (id: string): void => { ipcRenderer.send('pty:ackColdRestore', { id }) }, + ackHiddenOutputRestoreApplied: (id: string, hiddenDropEpisode?: number): void => { + ipcRenderer.send('pty:hiddenOutputRestoreApplied', { id, hiddenDropEpisode }) + }, ackData: (id: string, charCount: number, processedChars?: number): void => { ipcRenderer.send('pty:ackData', { id, diff --git a/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-flood-abandon.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-flood-abandon.test.ts new file mode 100644 index 00000000000..a0a13b1a5aa --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-flood-abandon.test.ts @@ -0,0 +1,361 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks, createDeferred } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type ConnectCallbacks, + type MockPane, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps, type PaneConnectionDeps } 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' +import { connectMockPanePty } from './pty-connection-test-connect-mock-pane' + +// STA-4869 — routes R3 (pending-queue overflow, pty-connection.ts:7696) and R4 +// (loop iteration cap, 7702) end in the QUIET abandon, which trades the loss +// banner for one deferred post-flood repaint. Same invariant as the non-quiet +// routes: every retained byte is reconciled exactly once, or the loss is +// disclosed. These cases must also survive the pane being scrolled back or +// hidden when the deferred repaint comes due. + +const { + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep, + warnTerminalLifecycleAnomaly +} = vi.hoisted(() => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn(), + warnTerminalLifecycleAnomaly: vi.fn() +})) + +// The iteration-cap warning is the only externally visible proof that R4's +// backstop fired rather than the loop finishing on its own. +vi.mock('./terminal-lifecycle-diagnostics', async (importOriginal) => ({ + ...(await importOriginal>()), + warnTerminalLifecycleAnomaly +})) + +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) => ({ + ...(await importOriginal>()), + getEagerPtyBufferHandle: vi.fn(() => undefined) +})) + +const PTY_ID = 'pty-id' +const BANNER_FRAGMENT = 'main recovery was unavailable' +const R3_MARKER = 'R3-LOST-4d17' +const R4_MARKER = 'R4-LOST-8ba0' +// One char over HIDDEN_OUTPUT_RESTORE_PENDING_CHARS (512KB): the queue discards +// its whole backlog and latches overflow. +const OVERFLOW_CHUNK = 'o'.repeat(512 * 1024 + 1) + +type MainSnapshot = { data: string; cols: number; rows: number; seq: number } + +function modelSnapshot(marker: string, seq: number): MainSnapshot { + return { data: `${marker}\r\n`, cols: 120, rows: 40, seq } +} + +type PaneDrive = { + pane: MockPane + deps: PaneConnectionDeps + disposable: { dispose: () => void } + deliver: (data: string, seq: number) => void + setVisible: (visible: boolean) => void + writtenChunks: () => string[] +} + +async function connectVisiblePane(): Promise { + const transport = createMockTransport(PTY_ID) + const captured: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => { + captured.current = callbacks?.onData ?? null + return PTY_ID + }) + transportFactoryQueue.push(transport) + const pane = createPane(1) + const deps = buildPaneConnectionDeps(() => mockStoreState, { + isVisibleRef: { current: true } + }) + const disposable = await connectMockPanePty(pane, createManager(1), deps) + await flushAsyncTicks(6) + expect(captured.current).not.toBeNull() + return { + pane, + deps, + disposable, + deliver: (data, seq) => captured.current?.(data, { seq, rawLength: data.length }), + setVisible: (visible) => { + deps.isVisibleRef.current = visible + }, + writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data)) + } +} + +async function announceMainDrop(markerSeq: number): Promise { + const { _dispatchPtyModelRestoreNeededForTest } = await import('./pty-model-restore-channel') + _dispatchPtyModelRestoreNeededForTest({ + id: PTY_ID, + reason: 'hidden-drop', + markerSeq + }) +} + +function countOccurrences(haystack: string, needle: string): number { + return haystack.split(needle).length - 1 +} + +type Disclosure = { + bannerWrites: number + markerWrites: number + disclosedOrRecovered: boolean +} + +function observeDisclosure(drive: PaneDrive, marker: string): Disclosure { + const written = drive.writtenChunks() + const bannerWrites = written.filter((data) => data.includes(BANNER_FRAGMENT)).length + const markerWrites = countOccurrences(written.join(''), marker) + return { + bannerWrites, + markerWrites, + disclosedOrRecovered: bannerWrites > 0 || markerWrites === 1 + } +} + +function expectDisclosedOrRecovered(disclosure: Disclosure): void { + expect( + disclosure.disclosedOrRecovered, + `hidden output was neither reconciled nor disclosed: ${JSON.stringify(disclosure)}` + ).toBe(true) +} + +describe('hidden-output restore quiet abandonment (STA-4869)', () => { + beforeEach(async () => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + await installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + // ── R3: foreground pending-queue overflow ─────────────────────────────── + describe('R3 pending-queue overflow', () => { + /** Drives one restore into a 512KB queue overflow and returns once the + * quiet abandon has run. */ + async function overflowRestoreQueue(drive: PaneDrive): Promise { + const getMainBufferSnapshot = vi.mocked(window.api.pty.getMainBufferSnapshot) + const firstSnapshot = createDeferred() + getMainBufferSnapshot.mockReturnValueOnce(firstSnapshot.promise) + await announceMainDrop(64) + await flushAsyncTicks(20) + expect(getMainBufferSnapshot).toHaveBeenCalledTimes(1) + + // The live stream outruns fetch+replay and blows the queue cap. + drive.deliver(OVERFLOW_CHUNK, 64 + OVERFLOW_CHUNK.length) + drive.deliver(OVERFLOW_CHUNK, 64 + 2 * OVERFLOW_CHUNK.length) + // Resolve inside the 750ms foreground deadline so the drain, not the + // deadline timer, owns the abandonment. + firstSnapshot.resolve(modelSnapshot('pre-overflow-model', 64)) + await flushAsyncTicks(20) + + // Proof the overflow arm ran: the snapshot replayed, and the queued + // backlog was discarded rather than drained under it. + const written = drive.writtenChunks().join('') + expect(written).toContain('pre-overflow-model') + expect(written).not.toContain('o'.repeat(1024)) + } + + it('[invariant] reconciles the retained bytes through the deferred post-flood repaint', async () => { + const drive = await connectVisiblePane() + vi.useFakeTimers() + const getMainBufferSnapshot = vi.mocked(window.api.pty.getMainBufferSnapshot) + await overflowRestoreQueue(drive) + + expect(drive.writtenChunks().join('')).not.toContain(BANNER_FRAGMENT) + getMainBufferSnapshot.mockResolvedValue(modelSnapshot(R3_MARKER, 4096)) + await vi.advanceTimersByTimeAsync(2_100) + await flushAsyncTicks(20) + + const disclosure = observeDisclosure(drive, R3_MARKER) + expect(disclosure.markerWrites).toBeLessThanOrEqual(1) + expectDisclosedOrRecovered(disclosure) + drive.disposable.dispose() + }) + + it('[invariant] keeps recovery armed while the user reads scrollback', async () => { + const { markTerminalFollowOutput, markTerminalPinnedViewport } = + await import('@/lib/pane-manager/terminal-scroll-intent') + const drive = await connectVisiblePane() + drive.pane.terminal.buffer.active.baseY = 100 + drive.pane.terminal.buffer.active.viewportY = 42 + markTerminalPinnedViewport(drive.pane.terminal) + vi.useFakeTimers() + const getMainBufferSnapshot = vi.mocked(window.api.pty.getMainBufferSnapshot) + await overflowRestoreQueue(drive) + + getMainBufferSnapshot.mockResolvedValue(modelSnapshot(R4_MARKER, 4096)) + await vi.advanceTimersByTimeAsync(60_000) + await flushAsyncTicks(20) + // Parked, not abandoned: the repaint must not yank the viewport, and must + // not claim a loss it can still heal. + expect(drive.writtenChunks().join('')).not.toContain(BANNER_FRAGMENT) + expect(drive.pane.terminal.buffer.active.viewportY).toBe(42) + + // A background stint must not consume the parked recovery either. + drive.setVisible(false) + await vi.advanceTimersByTimeAsync(60_000) + drive.setVisible(true) + await flushAsyncTicks(20) + + drive.pane.terminal.buffer.active.viewportY = drive.pane.terminal.buffer.active.baseY + markTerminalFollowOutput(drive.pane.terminal) + await vi.advanceTimersByTimeAsync(2_100) + await flushAsyncTicks(20) + + const disclosure = observeDisclosure(drive, R4_MARKER) + expect(disclosure.markerWrites).toBeLessThanOrEqual(1) + expectDisclosedOrRecovered(disclosure) + drive.disposable.dispose() + }) + }) + + // ── R4: restore loop iteration cap ────────────────────────────────────── + describe('R4 loop iteration cap', () => { + it('[invariant] reconciles the retained bytes after the iteration backstop fires', async () => { + const drive = await connectVisiblePane() + vi.useFakeTimers() + const getMainBufferSnapshot = vi.mocked(window.api.pty.getMainBufferSnapshot) + const rounds = [0, 1, 2].map(() => createDeferred()) + for (const round of rounds) { + getMainBufferSnapshot.mockReturnValueOnce(round.promise) + } + await announceMainDrop(64) + await flushAsyncTicks(20) + + // Each round: the pane goes background long enough for one chunk to arrive + // with a restore in flight, which marks the in-flight snapshot stale and + // sends the loop around again. + for (const [index, round] of rounds.entries()) { + expect(getMainBufferSnapshot).toHaveBeenCalledTimes(index + 1) + drive.setVisible(false) + drive.deliver(`stale-round-${index}\r\n`, 128 + index * 32) + drive.setVisible(true) + round.resolve(modelSnapshot(`round-${index}-model`, 64 + index)) + await flushAsyncTicks(20) + } + + // The backstop cut the loop without a banner; the deferred repaint owes + // the pane its bytes. + expect(getMainBufferSnapshot).toHaveBeenCalledTimes(3) + expect(warnTerminalLifecycleAnomaly).toHaveBeenCalledWith( + 'hidden output restore hit its iteration cap', + expect.objectContaining({ ptyId: PTY_ID }) + ) + expect(drive.writtenChunks().join('')).not.toContain(BANNER_FRAGMENT) + getMainBufferSnapshot.mockResolvedValue(modelSnapshot(R4_MARKER, 8192)) + await vi.advanceTimersByTimeAsync(2_100) + await flushAsyncTicks(20) + + const disclosure = observeDisclosure(drive, R4_MARKER) + expect(disclosure.markerWrites).toBeLessThanOrEqual(1) + expectDisclosedOrRecovered(disclosure) + drive.disposable.dispose() + }) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-loss-disclosure.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-loss-disclosure.test.ts new file mode 100644 index 00000000000..6cb2d916153 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-loss-disclosure.test.ts @@ -0,0 +1,689 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RESET_AFTER_BYTE_GAP } from '../../../../shared/terminal-mode-reset-profiles' +import { flushAsyncTicks, createDeferred } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type ConnectCallbacks, + type MockPane, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps, type PaneConnectionDeps } 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' +import { connectMockPanePty } from './pty-connection-test-connect-mock-pane' + +// STA-4869 — routes R1 (host retry budget), R2 (permanently-unavailable) and R5 +// (local deferred-retry exhaustion) all end in the NON-QUIET abandon at +// pty-connection.ts:7106, whose only loss disclosure is +// writeRestoreUnavailableWarning (7302). That function early-returns the banner +// when the pane is not foreground, and the abandon has already zeroed +// hiddenOutputRestoreNeeded/PtyId and cleared the flood repaint timer. +// +// Invariant asserted here: when renderer delivery was withheld while hidden, +// reveal must either reconcile every retained byte exactly once or leave a +// durable loss banner on the pane. Never neither. + +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) => ({ + ...(await importOriginal>()), + getEagerPtyBufferHandle: vi.fn(() => undefined) +})) + +const REMOTE_PTY_ID = 'remote:env-1@@terminal-sta4869' +const LOCAL_PTY_ID = 'pty-id' +// Overruns the renderer's 2MB hidden background queue: the bytes exist only in +// the authority's buffer afterwards, which is what reveal has to reconcile. +const HIDDEN_FLOOD = 'x'.repeat(2 * 1024 * 1024 + 1) +const LIVE_CHUNK = 'live-after-reveal\r\n' +const BANNER_FRAGMENT = 'main recovery was unavailable' +const R1_MARKER = 'R1-LOST-7f3a' +const R2_MARKER = 'R2-LOST-91c4' +const R5_MARKER = 'R5-LOST-b28e' + +type RetainedSnapshot = { + data: string + cols: number + rows: number + seq: number +} + +/** The authority's retained content — the only carrier of the dropped bytes. */ +function retainedSnapshot(marker: string): RetainedSnapshot { + return { + data: `${marker}\r\n`, + cols: 120, + rows: 40, + seq: HIDDEN_FLOOD.length + LIVE_CHUNK.length + } +} + +function hostAnswers(snapshot: RetainedSnapshot): unknown { + return { availability: { kind: 'snapshot' }, snapshot } +} + +type PaneDrive = { + pane: MockPane + deps: PaneConnectionDeps + disposable: { dispose: () => void } + deliver: (data: string, seq: number) => void + setVisible: (visible: boolean) => void + setOutputPaused: (paused: boolean, dropsReported?: boolean) => void + reportPausedOutputDropped: () => void + writtenChunks: () => string[] +} + +async function connectHiddenPane( + ptyId: string, + configureTransport: (transport: MockTransport) => void +): Promise { + const transport = createMockTransport(ptyId) + configureTransport(transport) + const captured: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + const capturedPause: { + current: ((paused: boolean, supported: boolean, dropsReported?: boolean) => void) | null + } = { + current: null + } + const capturedDropped: { current: (() => void) | null } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => { + captured.current = callbacks?.onData ?? null + capturedPause.current = callbacks?.onOutputPauseChanged ?? null + capturedDropped.current = callbacks?.onPausedOutputDropped ?? null + return ptyId + }) + transportFactoryQueue.push(transport) + const pane = createPane(1) + const deps = buildPaneConnectionDeps(() => mockStoreState, { + isVisibleRef: { current: false } + }) + const disposable = await connectMockPanePty(pane, createManager(1), deps) + await flushAsyncTicks(6) + expect(captured.current).not.toBeNull() + return { + pane, + deps, + disposable, + deliver: (data, seq) => captured.current?.(data, { seq, rawLength: data.length }), + setVisible: (visible) => { + deps.isVisibleRef.current = visible + }, + setOutputPaused: (paused, dropsReported) => + capturedPause.current?.(paused, true, dropsReported), + reportPausedOutputDropped: () => capturedDropped.current?.(), + writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data)) + } +} + +/** Hidden-time bytes are dropped, then the pane is revealed with live output + * still streaming — the reveal path the gate's recovery loop owns. */ +function floodWhileHiddenThenReveal(drive: PaneDrive): void { + drive.deliver(HIDDEN_FLOOD, HIDDEN_FLOOD.length) + drive.setVisible(true) + drive.deliver(LIVE_CHUNK, HIDDEN_FLOOD.length + LIVE_CHUNK.length) +} + +async function revealPane(drive: PaneDrive): Promise { + drive.setVisible(true) + const { requestTerminalBacklogRecovery } = + await import('@/lib/pane-manager/pane-terminal-output-scheduler') + requestTerminalBacklogRecovery(drive.pane.terminal) + await flushAsyncTicks(20) +} + +function countOccurrences(haystack: string, needle: string): number { + return haystack.split(needle).length - 1 +} + +type Disclosure = { + bannerWrites: number + markerWrites: number + disclosedOrRecovered: boolean +} + +function observeDisclosure(drive: PaneDrive, marker: string): Disclosure { + const written = drive.writtenChunks() + const bannerWrites = written.filter((data) => data.includes(BANNER_FRAGMENT)).length + const markerWrites = countOccurrences(written.join(''), marker) + return { + bannerWrites, + markerWrites, + disclosedOrRecovered: bannerWrites > 0 || markerWrites === 1 + } +} + +function expectDisclosedOrRecovered(disclosure: Disclosure): void { + expect( + disclosure.disclosedOrRecovered, + `hidden output was neither reconciled nor disclosed: ${JSON.stringify(disclosure)}` + ).toBe(true) +} + +/** Long enough for every deferred repaint, re-arm cycle and retry tick. */ +async function allowRecoveryWindow(): Promise { + await vi.advanceTimersByTimeAsync(60_000) + await flushAsyncTicks(20) +} + +describe('hidden-output restore loss disclosure (STA-4869)', () => { + beforeEach(async () => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + await installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + // ── R1: host retry-worthy budget (7 answers) ──────────────────────────── + describe('R1 host retry budget exhaustion', () => { + const retryWorthy = { + availability: { + kind: 'retry-worthy', + cause: 'host-pending-output-overflowed' + }, + snapshot: null + } + + async function driveToSeventhAnswer( + serializeBufferOutcome: ReturnType + ): Promise { + const drive = await connectHiddenPane(REMOTE_PTY_ID, (transport) => { + transport.serializeBuffer = vi.fn() + transport.serializeBufferOutcome = serializeBufferOutcome + }) + vi.useFakeTimers() + floodWhileHiddenThenReveal(drive) + await flushAsyncTicks(20) + expect(serializeBufferOutcome).toHaveBeenCalledTimes(1) + // Each retry-worthy answer quietly re-arms one post-suppression repaint. + for (let request = 2; request <= 6; request += 1) { + await vi.advanceTimersByTimeAsync(2_000) + await flushAsyncTicks(20) + expect(serializeBufferOutcome).toHaveBeenCalledTimes(request) + } + expect(drive.writtenChunks().join('')).not.toContain(BANNER_FRAGMENT) + return drive + } + + it('[control] discloses the loss when the budget ends while the pane is visible', async () => { + const serializeBufferOutcome = vi.fn().mockResolvedValue(retryWorthy) + const drive = await driveToSeventhAnswer(serializeBufferOutcome) + + await vi.advanceTimersByTimeAsync(2_000) + await flushAsyncTicks(20) + serializeBufferOutcome.mockResolvedValue(hostAnswers(retainedSnapshot(R1_MARKER))) + await allowRecoveryWindow() + + expect(observeDisclosure(drive, R1_MARKER)).toMatchObject({ + disclosedOrRecovered: true + }) + drive.disposable.dispose() + }) + + it('[invariant] discloses or recovers when the budget ends while the pane is hidden', async () => { + const seventhAnswer = createDeferred() + const serializeBufferOutcome = vi.fn().mockResolvedValue(retryWorthy) + const drive = await driveToSeventhAnswer(serializeBufferOutcome) + + serializeBufferOutcome.mockReturnValueOnce(seventhAnswer.promise) + await vi.advanceTimersByTimeAsync(2_000) + await flushAsyncTicks(20) + expect(serializeBufferOutcome).toHaveBeenCalledTimes(7) + + // The user switches tabs while the seventh request is still outstanding; + // the restore loop keeps running across the await. + drive.setVisible(false) + seventhAnswer.resolve(retryWorthy) + await flushAsyncTicks(20) + + // The host is healthy again and still holds the dropped bytes. + serializeBufferOutcome.mockResolvedValue(hostAnswers(retainedSnapshot(R1_MARKER))) + const requestsBeforeReveal = serializeBufferOutcome.mock.calls.length + await revealPane(drive) + await allowRecoveryWindow() + + const disclosure = observeDisclosure(drive, R1_MARKER) + expect(disclosure.markerWrites).toBeLessThanOrEqual(1) + expectDisclosedOrRecovered(disclosure) + // Whichever way the gap is closed, silence is not one of them: recovering + // it costs a fresh request, disclosing it costs a banner. + expect( + serializeBufferOutcome.mock.calls.length - requestsBeforeReveal + ).toBeGreaterThanOrEqual(disclosure.bannerWrites > 0 ? 0 : 1) + drive.disposable.dispose() + }) + + async function exhaustBudgetWhileHidden( + serializeBufferOutcome: ReturnType + ): Promise { + const seventhAnswer = createDeferred() + const drive = await driveToSeventhAnswer(serializeBufferOutcome) + serializeBufferOutcome.mockReturnValueOnce(seventhAnswer.promise) + await vi.advanceTimersByTimeAsync(2_000) + await flushAsyncTicks(20) + drive.setVisible(false) + seventhAnswer.resolve(retryWorthy) + await flushAsyncTicks(20) + return drive + } + + it('[fix] heals exactly once on one fresh snapshot when the host recovered', async () => { + const serializeBufferOutcome = vi.fn().mockResolvedValue(retryWorthy) + const drive = await exhaustBudgetWhileHidden(serializeBufferOutcome) + + serializeBufferOutcome.mockResolvedValue(hostAnswers(retainedSnapshot(R1_MARKER))) + const requestsBeforeReveal = serializeBufferOutcome.mock.calls.length + await revealPane(drive) + await allowRecoveryWindow() + + expect(observeDisclosure(drive, R1_MARKER)).toMatchObject({ + bannerWrites: 0, + markerWrites: 1 + }) + // Reveal is the bounded retry: one fresh snapshot, not a fresh budget. + expect(serializeBufferOutcome.mock.calls.length - requestsBeforeReveal).toBe(1) + drive.disposable.dispose() + }) + + it('[fix] banners once when the host is still broken at reveal', async () => { + const serializeBufferOutcome = vi.fn().mockResolvedValue(retryWorthy) + const drive = await exhaustBudgetWhileHidden(serializeBufferOutcome) + expect(drive.writtenChunks().join('')).not.toContain(BANNER_FRAGMENT) + + const requestsBeforeReveal = serializeBufferOutcome.mock.calls.length + await revealPane(drive) + await allowRecoveryWindow() + + // The spent budget abandons on the first foreground answer, so the + // disclosure now lands where it can be read. + expect(observeDisclosure(drive, R1_MARKER)).toMatchObject({ + bannerWrites: 1, + markerWrites: 0 + }) + // Reveal is a bounded retry, not a fresh budget: one host request, then + // the banner. Carrying the spent counters across the deferral is what + // makes this hold — a reset would re-run all seven against a dead host. + expect(serializeBufferOutcome.mock.calls.length - requestsBeforeReveal).toBe(1) + drive.disposable.dispose() + }) + }) + + // ── R2: permanently-unavailable ───────────────────────────────────────── + describe('R2 permanently-unavailable answer', () => { + const permanentlyUnavailable = { + availability: { + kind: 'permanently-unavailable', + reason: 'exceeds-client-replay-limit' + }, + snapshot: null + } + + it('[control] banners once when the answer lands while the pane is visible', async () => { + const serializeBufferOutcome = vi.fn().mockResolvedValue(permanentlyUnavailable) + const drive = await connectHiddenPane(REMOTE_PTY_ID, (transport) => { + transport.serializeBuffer = vi.fn() + transport.serializeBufferOutcome = serializeBufferOutcome + }) + vi.useFakeTimers() + floodWhileHiddenThenReveal(drive) + await flushAsyncTicks(20) + await allowRecoveryWindow() + + expect(observeDisclosure(drive, R2_MARKER)).toMatchObject({ + bannerWrites: 1, + disclosedOrRecovered: true + }) + drive.disposable.dispose() + }) + + it('[invariant] discloses or recovers when the answer lands while the pane is hidden', async () => { + const answer = createDeferred() + const serializeBufferOutcome = vi.fn().mockReturnValue(answer.promise) + const drive = await connectHiddenPane(REMOTE_PTY_ID, (transport) => { + transport.serializeBuffer = vi.fn() + transport.serializeBufferOutcome = serializeBufferOutcome + }) + vi.useFakeTimers() + floodWhileHiddenThenReveal(drive) + await flushAsyncTicks(20) + expect(serializeBufferOutcome).toHaveBeenCalledTimes(1) + + drive.setVisible(false) + answer.resolve(permanentlyUnavailable) + await flushAsyncTicks(20) + + serializeBufferOutcome.mockResolvedValue(hostAnswers(retainedSnapshot(R2_MARKER))) + await revealPane(drive) + await allowRecoveryWindow() + + const disclosure = observeDisclosure(drive, R2_MARKER) + expect(disclosure.markerWrites).toBeLessThanOrEqual(1) + expectDisclosedOrRecovered(disclosure) + drive.disposable.dispose() + }) + }) + + // SSH (orcad) and paired `orca serve` panes both reveal through the host's output + // unpause, not the local backlog recovery, so pin that path on its own. + describe('remote output-pause reveal', () => { + const REMOTE_MARKER = 'REMOTE-LOST-4d19' + const answersWhileHidden = [ + { + host: 'modern host, permanently unavailable', + failure: { + availability: { + kind: 'permanently-unavailable', + reason: 'exceeds-client-replay-limit' + }, + snapshot: null + }, + legacy: false, + dropsReported: false + }, + { + // #29253: the host reports drops, so only its drop frame arms the restore. + host: 'drop-reporting host, permanently unavailable', + failure: { + availability: { + kind: 'permanently-unavailable', + reason: 'exceeds-client-replay-limit' + }, + snapshot: null + }, + legacy: false, + dropsReported: true + }, + { host: 'legacy host, no snapshot yet', failure: null, legacy: true, dropsReported: false } + ] + it.each(answersWhileHidden)( + '[invariant] restores on the unpause reveal after a restore failed while hidden ($host)', + async ({ failure, legacy, dropsReported }) => { + const answer = createDeferred() + const request = vi.fn().mockReturnValue(answer.promise) + const drive = await connectHiddenPane(REMOTE_PTY_ID, (transport) => { + transport.serializeBuffer = legacy ? request : vi.fn() + transport.serializeBufferOutcome = legacy ? undefined : request + }) + vi.useFakeTimers() + drive.setOutputPaused(true, dropsReported) + if (dropsReported) { + drive.reportPausedOutputDropped() + } + drive.setVisible(true) + drive.setOutputPaused(false, dropsReported) + await flushAsyncTicks(20) + expect(request).toHaveBeenCalledTimes(1) + + // The user switches away before the answer lands. A drop-reporting host sends no + // second drop frame for bytes it already reported, so the pending restore must survive. + drive.setVisible(false) + drive.setOutputPaused(true, dropsReported) + answer.resolve(failure) + await flushAsyncTicks(20) + await allowRecoveryWindow() + + const snapshot = retainedSnapshot(REMOTE_MARKER) + request.mockResolvedValue(legacy ? snapshot : hostAnswers(snapshot)) + drive.setVisible(true) + drive.setOutputPaused(false, dropsReported) + await flushAsyncTicks(20) + + expect(request).toHaveBeenCalledTimes(2) + expect(observeDisclosure(drive, REMOTE_MARKER)).toMatchObject({ + bannerWrites: 0, + markerWrites: 1 + }) + drive.disposable.dispose() + } + ) + }) + + // ── R5: local snapshot unavailable, deferred retries exhausted ────────── + describe('R5 local deferred-retry exhaustion', () => { + async function connectLocalHiddenPane(): Promise { + // A codex startup makes the renderer skip hidden bytes locally, which is + // the local mirror of the remote pause the other routes model. + const transport = createMockTransport(LOCAL_PTY_ID) + const captured: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks?: ConnectCallbacks }) => { + captured.current = callbacks?.onData ?? null + return LOCAL_PTY_ID + } + ) + transportFactoryQueue.push(transport) + const pane = createPane(1) + const deps = buildPaneConnectionDeps(() => mockStoreState, { + isVisibleRef: { current: false }, + startup: { command: 'codex' } + }) + const disposable = await connectMockPanePty(pane, createManager(1), deps) + await flushAsyncTicks(6) + expect(captured.current).not.toBeNull() + return { + pane, + deps, + disposable, + deliver: (data, seq) => captured.current?.(data, { seq, rawLength: data.length }), + setVisible: (visible) => { + deps.isVisibleRef.current = visible + }, + // Local panes are gated in main, not paused on a host. + setOutputPaused: () => {}, + reportPausedOutputDropped: () => {}, + writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data)) + } + } + + it('[control] banners once when the retries run out while the pane is visible', async () => { + const getMainBufferSnapshot = vi.mocked(window.api.pty.getMainBufferSnapshot) + getMainBufferSnapshot.mockResolvedValue(null) + const drive = await connectLocalHiddenPane() + vi.useFakeTimers() + floodWhileHiddenThenReveal(drive) + await flushAsyncTicks(20) + + for (let attempt = 0; attempt < 3; attempt += 1) { + await vi.advanceTimersByTimeAsync(50) + await flushAsyncTicks(20) + } + + expect(getMainBufferSnapshot).toHaveBeenCalledTimes(4) + expect(observeDisclosure(drive, R5_MARKER)).toMatchObject({ + bannerWrites: 1, + disclosedOrRecovered: true + }) + drive.disposable.dispose() + }) + + it('[invariant] keeps recovery armed when the pane hides before the retries run out', async () => { + const getMainBufferSnapshot = vi.mocked(window.api.pty.getMainBufferSnapshot) + getMainBufferSnapshot.mockResolvedValue(null) + const drive = await connectLocalHiddenPane() + vi.useFakeTimers() + floodWhileHiddenThenReveal(drive) + await flushAsyncTicks(20) + + // The pane goes background one retry into the budget. + await vi.advanceTimersByTimeAsync(50) + await flushAsyncTicks(20) + drive.setVisible(false) + await allowRecoveryWindow() + + // Main still holds the dropped bytes; reveal must spend them. + getMainBufferSnapshot.mockResolvedValue(retainedSnapshot(R5_MARKER)) + await revealPane(drive) + await allowRecoveryWindow() + + const disclosure = observeDisclosure(drive, R5_MARKER) + expect(disclosure.markerWrites).toBeLessThanOrEqual(1) + expectDisclosedOrRecovered(disclosure) + drive.disposable.dispose() + }) + }) + + // ── The restore loop's PTY-swap arm (pty-connection.ts ~7660) ──────────── + // The one writeRestoreUnavailableWarning caller the defer guard does not + // cover. It is reachable only from loop iteration 2+, which the hidden-pause + // guard after the replay makes foreground-only, and it is remote-only (a + // local ptyId always satisfies canUseHiddenOutputSnapshot) — so the re-arm + // budget clearHiddenOutputRestoreState() just reset always wins and the + // banner call is dead. Entering it with the budget already spent is not + // constructible: iteration 2 requires a successful snapshot first, and that + // zeroes hiddenOutputRestoreRemoteAbandonCycles on the way through. + describe('restore-loop PTY-swap arm', () => { + it('[fix] re-arms instead of declaring a loss when the host loses snapshot capability', async () => { + const swapMarker = 'R7-LOST-3e91' + const answer = createDeferred() + const serializeBufferOutcome = vi.fn().mockReturnValue(answer.promise) + let paneTransport: MockTransport | null = null + const drive = await connectHiddenPane(REMOTE_PTY_ID, (transport) => { + paneTransport = transport + transport.serializeBuffer = vi.fn() + transport.serializeBufferOutcome = serializeBufferOutcome + }) + if (!paneTransport) { + throw new Error('transport was never configured') + } + const transport: MockTransport = paneTransport + const { resetTerminalFreezeBreadcrumbsForTesting, getTerminalFreezeBreadcrumbs } = + await import('./terminal-freeze-breadcrumbs') + vi.useFakeTimers() + floodWhileHiddenThenReveal(drive) + await flushAsyncTicks(20) + expect(serializeBufferOutcome).toHaveBeenCalledTimes(1) + + // A chunk landing while the pane is momentarily background marks the + // in-flight snapshot stale, which is what sends the loop around again. + drive.setVisible(false) + drive.deliver('stale-during-restore\r\n', HIDDEN_FLOOD.length + LIVE_CHUNK.length + 32) + drive.setVisible(true) + + // The host drops snapshot support underneath the in-flight request. + transport.serializeBuffer = undefined + resetTerminalFreezeBreadcrumbsForTesting() + answer.resolve(hostAnswers(retainedSnapshot(swapMarker))) + await flushAsyncTicks(20) + + const disclosure = observeDisclosure(drive, swapMarker) + expect(disclosure.bannerWrites).toBe(0) + // Route proof: the re-arm reason names this arm, not the deadline abandon. + expect( + getTerminalFreezeBreadcrumbs() + .filter((crumb) => crumb.kind === 'restore-abandon-rearm') + .map((crumb) => crumb.detail?.reason) + ).toEqual(['restore-pty-swapped']) + // The gap is still grounded, and the deferred repaint keeps recovery armed. + expect(drive.writtenChunks().some((chunk) => chunk.startsWith(RESET_AFTER_BYTE_GAP))).toBe( + true + ) + drive.disposable.dispose() + }) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-marker-handoff.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-marker-handoff.test.ts new file mode 100644 index 00000000000..a34a9ac4b03 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-marker-handoff.test.ts @@ -0,0 +1,525 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createDeferred, flushAsyncTicks } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type ConnectCallbacks, + type MockPane, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps, type PaneConnectionDeps } 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' +import { connectMockPanePty } from './pty-connection-test-connect-mock-pane' + +// STA-4869 route R6 — the renderer channel has no latch of its own: +// dispatchPtyModelRestoreNeeded (pty-model-restore-channel.ts) drops a marker +// for which no handler is registered. A pane that releases the last hidden claim +// while retiring unhides after unregistering its handler, so a drop latch +// consumed by unmark would be spent on a marker nobody can receive. The fix +// keeps the latch armed across unhide and retires it where recovery actually +// paints the bytes — the renderer's post-apply ack. +// +// This suite drives the REAL renderer channel from behind the +// setHiddenRendererPty IPC, delivering main's marker one turn later the way +// Electron does. Main's gate is modelled by mainDropLatch below rather than +// imported — src/main is outside the renderer tsconfig project — and its two +// rules are pinned against the real module in +// src/main/ipc/pty-hidden-delivery-gate.test.ts. + +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) => ({ + ...(await importOriginal>()), + getEagerPtyBufferHandle: vi.fn(() => undefined) +})) + +const PTY_ID = 'pty-id' +const BANNER_FRAGMENT = 'main recovery was unavailable' +const R6_MARKER = 'R6-LOST-c5e2' +const HIDDEN_CHUNK = 'hidden-agent-output\r\n' + +type PaneDrive = { + pane: MockPane + deps: PaneConnectionDeps + binding: { syncProcessTracking: () => void; dispose: () => void } + deliver: (data: string, seq: number) => void + setVisible: (visible: boolean) => void + writtenChunks: () => string[] +} + +/** Main's droppedSinceHiddenPtys latch (pty-hidden-delivery-gate.ts): the first + * gated drop arms it, neither re-marking hidden nor unmarking clears it, and + * only the renderer's applied-ack (or teardown) retires it. */ +const mainDropLatch = { + hidden: false, + dropped: false, + markHidden(): void { + this.hidden = true + }, + /** @returns whether main owes the renderer a restore marker. */ + recordDrop(): boolean { + if (this.dropped) { + return false + } + this.dropped = true + return true + }, + /** @returns droppedWhileHidden — read, not spent. */ + unmarkHidden(): boolean { + this.hidden = false + return this.dropped + }, + /** consumeHiddenRendererPtyDropMemory: the renderer painted the snapshot. */ + consumeDropMemory(): void { + this.dropped = false + }, + reset(): void { + this.hidden = false + this.dropped = false + } +} + +/** Routes the renderer's hidden-state IPC and its applied-ack into main's gate, + * delivering main's marker back one turn later as the IPC round trip does. */ +async function wireMainGateToRendererChannel(): Promise<{ + markersDelivered: () => number +}> { + const { _dispatchPtyModelRestoreNeededForTest } = await import('./pty-model-restore-channel') + let markersDelivered = 0 + vi.mocked(window.api.pty.setHiddenRendererPty).mockImplementation( + (id: string, hidden: boolean) => { + if (hidden) { + mainDropLatch.markHidden() + return + } + if (!mainDropLatch.unmarkHidden()) { + return + } + markersDelivered += 1 + // Emit-time seq advances with visible output between reveals; the episode does not. + const markerSeq = 4096 + markersDelivered + queueMicrotask(() => { + _dispatchPtyModelRestoreNeededForTest({ + id, + reason: 'unhide', + markerSeq, + hiddenDropEpisode: 1 + }) + }) + } + ) + vi.mocked(window.api.pty.ackHiddenOutputRestoreApplied).mockImplementation(() => { + mainDropLatch.consumeDropMemory() + }) + return { markersDelivered: () => markersDelivered } +} + +/** Main ingests the chunk into its model and drops the renderer copy. */ +async function dropHiddenBytesInMain(): Promise { + const { _dispatchPtyModelRestoreNeededForTest } = await import('./pty-model-restore-channel') + expect(mainDropLatch.hidden).toBe(true) + if (mainDropLatch.recordDrop()) { + _dispatchPtyModelRestoreNeededForTest({ + id: PTY_ID, + reason: 'hidden-drop', + markerSeq: 2048, + hiddenDropEpisode: 1 + }) + } + await flushAsyncTicks(6) +} + +async function connectPane(visible: boolean): Promise { + const transport = createMockTransport(PTY_ID) + const captured: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => { + captured.current = callbacks?.onData ?? null + return PTY_ID + }) + transportFactoryQueue.push(transport) + const pane = createPane(1) + const deps = buildPaneConnectionDeps(() => mockStoreState, { + isVisibleRef: { current: visible } + }) + const binding = await connectMockPanePty(pane, createManager(1), deps) + await flushAsyncTicks(6) + expect(captured.current).not.toBeNull() + return { + pane, + deps, + binding, + deliver: (data, seq) => captured.current?.(data, { seq, rawLength: data.length }), + setVisible: (nextVisible) => { + deps.isVisibleRef.current = nextVisible + }, + writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data)) + } +} + +function observeDisclosure(drive: PaneDrive): { + bannerWrites: number + markerWrites: number + disclosedOrRecovered: boolean +} { + const written = drive.writtenChunks() + const bannerWrites = written.filter((data) => data.includes(BANNER_FRAGMENT)).length + const markerWrites = written.join('').split(R6_MARKER).length - 1 + return { + bannerWrites, + markerWrites, + disclosedOrRecovered: bannerWrites > 0 || markerWrites === 1 + } +} + +describe('hidden-output restore marker handoff (STA-4869 R6)', () => { + beforeEach(async () => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mainDropLatch.reset() + mockStoreState = createInitialStoreState(() => mockStoreState) + mockStoreState.settings = { + ...mockStoreState.settings, + terminalMainSideEffectAuthority: true + } + await installTerminalTestGlobals() + vi.mocked(window.api.pty.getMainBufferSnapshot).mockResolvedValue({ + data: `${R6_MARKER}\r\n`, + cols: 120, + rows: 40, + seq: 4096 + }) + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + const { _resetPtyRendererDeliveryClaimsForTest } = + await import('./pty-renderer-delivery-claims') + _resetPtyRendererDeliveryClaimsForTest() + }) + + it('[mechanism] the renderer channel drops a marker with no registered handler and never replays it', async () => { + const { registerPtyModelRestoreNeededHandler, _dispatchPtyModelRestoreNeededForTest } = + await import('./pty-model-restore-channel') + const retiringHandler = vi.fn() + const unregister = registerPtyModelRestoreNeededHandler(PTY_ID, retiringHandler) + unregister() + + _dispatchPtyModelRestoreNeededForTest({ + id: PTY_ID, + reason: 'unhide', + markerSeq: 4096 + }) + expect(retiringHandler).not.toHaveBeenCalled() + + // The replacement view registers a moment later: nothing is held for it. + const replacementHandler = vi.fn() + registerPtyModelRestoreNeededHandler(PTY_ID, replacementHandler) + await flushAsyncTicks(6) + expect(replacementHandler).not.toHaveBeenCalled() + }) + + it('[control] a revealed pane consumes the unhide marker and repaints from the model', async () => { + const gate = await wireMainGateToRendererChannel() + const drive = await connectPane(false) + drive.deliver(HIDDEN_CHUNK, HIDDEN_CHUNK.length) + await dropHiddenBytesInMain() + + drive.setVisible(true) + drive.binding.syncProcessTracking() + await flushAsyncTicks(20) + + expect(gate.markersDelivered()).toBe(1) + expect(observeDisclosure(drive)).toMatchObject({ markerWrites: 1 }) + drive.binding.dispose() + }) + + it('[invariant] a hidden pane that retires must not spend the drop latch on a marker nobody receives', async () => { + const gate = await wireMainGateToRendererChannel() + const retiring = await connectPane(false) + retiring.deliver(HIDDEN_CHUNK, HIDDEN_CHUNK.length) + await dropHiddenBytesInMain() + expect(vi.mocked(window.api.pty.setHiddenRendererPty)).toHaveBeenCalledWith(PTY_ID, true) + + // The pane retires (tab remount / recovery): releasing the last hidden claim + // unhides the PTY in main, which answers with the one restore marker. + retiring.binding.dispose() + await flushAsyncTicks(20) + expect(gate.markersDelivered()).toBe(1) + + // The replacement pane binds the same PTY while visible. + const replacement = await connectPane(true) + replacement.binding.syncProcessTracking() + await flushAsyncTicks(20) + + // Main still holds the dropped bytes and answers any snapshot request with + // them, so the replacement pane owes the user either those bytes or a banner. + const disclosure = { + ...observeDisclosure(replacement), + modelSnapshotRequests: vi.mocked(window.api.pty.getMainBufferSnapshot).mock.calls.length + } + expect(disclosure.markerWrites).toBeLessThanOrEqual(1) + expect( + disclosure.disclosedOrRecovered, + `hidden output was neither reconciled nor disclosed after remount: ${JSON.stringify(disclosure)}` + ).toBe(true) + replacement.binding.dispose() + }) + + it('[fix] the remount re-emits the unhide marker and the applied ack retires the latch', async () => { + const gate = await wireMainGateToRendererChannel() + const retiring = await connectPane(false) + retiring.deliver(HIDDEN_CHUNK, HIDDEN_CHUNK.length) + await dropHiddenBytesInMain() + retiring.binding.dispose() + await flushAsyncTicks(20) + expect(gate.markersDelivered()).toBe(1) + + const replacement = await connectPane(true) + replacement.binding.syncProcessTracking() + await flushAsyncTicks(20) + + // The latch outlived the marker nobody received, so the remount's own unhide + // re-emits it; the paint that heals the gap is what finally spends it. + expect(gate.markersDelivered()).toBe(2) + expect(window.api.pty.ackHiddenOutputRestoreApplied).toHaveBeenCalledWith(PTY_ID, 1) + expect(observeDisclosure(replacement)).toMatchObject({ markerWrites: 1 }) + expect(mainDropLatch.dropped).toBe(false) + + // No marker storm: hide/reveal cycles after consumption ask for nothing. + const snapshotRequests = vi.mocked(window.api.pty.getMainBufferSnapshot).mock.calls.length + replacement.setVisible(false) + replacement.binding.syncProcessTracking() + replacement.setVisible(true) + replacement.binding.syncProcessTracking() + await flushAsyncTicks(20) + + expect(gate.markersDelivered()).toBe(2) + expect(vi.mocked(window.api.pty.getMainBufferSnapshot).mock.calls.length).toBe(snapshotRequests) + replacement.binding.dispose() + }) + + it('[fix] acks only the drop episode its snapshot was requested for', async () => { + const snapshot = { data: `${R6_MARKER}\r\n`, cols: 120, rows: 40, seq: 4096 } + const served = createDeferred() + let servedCalls = 0 + vi.mocked(window.api.pty.getMainBufferSnapshot).mockImplementation(async () => { + servedCalls += 1 + return servedCalls === 1 ? await served.promise : snapshot + }) + await wireMainGateToRendererChannel() + const drive = await connectPane(false) + drive.deliver(HIDDEN_CHUNK, HIDDEN_CHUNK.length) + await dropHiddenBytesInMain() + drive.setVisible(true) + drive.binding.syncProcessTracking() + await flushAsyncTicks(10) + expect(servedCalls).toBe(1) + + // A newer episode is reported while the first snapshot is still in flight. + const { _dispatchPtyModelRestoreNeededForTest } = await import('./pty-model-restore-channel') + _dispatchPtyModelRestoreNeededForTest({ + id: PTY_ID, + reason: 'hidden-drop', + markerSeq: 8192, + hiddenDropEpisode: 2 + }) + served.resolve(snapshot) + await flushAsyncTicks(30) + + // The older snapshot may predate episode 2's bytes, so it can only retire episode 1. + const acks = vi.mocked(window.api.pty.ackHiddenOutputRestoreApplied).mock.calls + expect(acks[0]).toEqual([PTY_ID, 1]) + expect(acks.slice(1).every(([, episode]) => episode === 2)).toBe(true) + drive.binding.dispose() + }) + + it('[fix] a snapshot served into a disposing pane leaves the latch armed for the remount', async () => { + // Main serializes multi-MB buffers across an IPC round trip; a tab/split + // switch or renderer reload inside that window disposes the pane that asked. + const snapshot = { + data: `${R6_MARKER}\r\n`, + cols: 120, + rows: 40, + seq: 4096 + } + const served = createDeferred() + let servedCalls = 0 + vi.mocked(window.api.pty.getMainBufferSnapshot).mockImplementation(async () => { + servedCalls += 1 + return servedCalls === 1 ? await served.promise : snapshot + }) + const gate = await wireMainGateToRendererChannel() + const retiring = await connectPane(false) + retiring.deliver(HIDDEN_CHUNK, HIDDEN_CHUNK.length) + await dropHiddenBytesInMain() + retiring.setVisible(true) + retiring.binding.syncProcessTracking() + await flushAsyncTicks(10) + expect(servedCalls).toBe(1) + + retiring.binding.dispose() + served.resolve(snapshot) + await flushAsyncTicks(20) + + // Nobody painted those bytes, so main still owes the next view a marker. + expect(window.api.pty.ackHiddenOutputRestoreApplied).not.toHaveBeenCalled() + expect(mainDropLatch.dropped).toBe(true) + + const replacement = await connectPane(true) + replacement.binding.syncProcessTracking() + await flushAsyncTicks(20) + + expect(gate.markersDelivered()).toBeGreaterThanOrEqual(2) + expect(observeDisclosure(replacement)).toMatchObject({ + bannerWrites: 0, + markerWrites: 1 + }) + expect(mainDropLatch.dropped).toBe(false) + replacement.binding.dispose() + }) + + it('[fix] a permanently refused source discloses the gap once across repeated reveals', async () => { + // The longer-lived latch re-marks on every unhide, so without disclosed-gap + // dedupe each reveal re-banners and re-fetches the same historical gap. + vi.mocked(window.api.pty.getMainBufferSnapshot).mockResolvedValue(null) + const gate = await wireMainGateToRendererChannel() + const drive = await connectPane(false) + drive.deliver(HIDDEN_CHUNK, HIDDEN_CHUNK.length) + await dropHiddenBytesInMain() + + vi.useFakeTimers() + try { + drive.setVisible(true) + drive.binding.syncProcessTracking() + await vi.advanceTimersByTimeAsync(5_000) + await flushAsyncTicks(20) + expect(observeDisclosure(drive)).toMatchObject({ bannerWrites: 1 }) + const snapshotsAtDisclosure = vi.mocked(window.api.pty.getMainBufferSnapshot).mock.calls + .length + + for (let cycle = 0; cycle < 4; cycle += 1) { + // Ordinary visible output between reveals is not a new hidden drop. + drive.deliver(`visible-${cycle}\r\n`, 8192 + cycle) + drive.setVisible(false) + drive.binding.syncProcessTracking() + await vi.advanceTimersByTimeAsync(1_000) + await flushAsyncTicks(20) + drive.setVisible(true) + drive.binding.syncProcessTracking() + await vi.advanceTimersByTimeAsync(5_000) + await flushAsyncTicks(20) + } + + // Every reveal re-emits main's marker; none of them re-open the same gap. + expect(gate.markersDelivered()).toBeGreaterThan(1) + expect(observeDisclosure(drive)).toMatchObject({ bannerWrites: 1 }) + expect(vi.mocked(window.api.pty.getMainBufferSnapshot).mock.calls.length).toBe( + snapshotsAtDisclosure + ) + } finally { + vi.useRealTimers() + } + drive.binding.dispose() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-test-connect-mock-pane.ts b/src/renderer/src/components/terminal-pane/pty-connection-test-connect-mock-pane.ts new file mode 100644 index 00000000000..89d7a8c008e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-test-connect-mock-pane.ts @@ -0,0 +1,22 @@ +import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager' +import type { PtyConnectionDeps } from './pty-connection-types' +import type { PanePtyBinding } from './pty-connection/pane-pty-binding' +import type { PaneConnectionDeps } from './pty-connection-test-deps' +import type { MockPane, MockPaneManager } from './pty-connection-test-pane-fixtures' + +/** Connects a mock pane through the real connectPanePty. Imported lazily so a + * test's vi.resetModules() hands back the freshly mocked module graph. */ +export async function connectMockPanePty( + pane: MockPane, + manager: MockPaneManager, + deps: PaneConnectionDeps +): Promise { + const { connectPanePty } = await import('./pty-connection') + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixtures stub every pane member the connection path reads. + const managedPane = pane as unknown as ManagedPane + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mock manager stubs every manager member the connection path calls. + const paneManager = manager as unknown as PaneManager + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: buildPaneConnectionDeps mirrors PtyConnectionDeps with mock refs and callbacks. + const connectionDeps = deps as unknown as PtyConnectionDeps + return connectPanePty(managedPane, paneManager, connectionDeps) +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts b/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts index c4b1838bb25..775b37d5d92 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts @@ -68,6 +68,7 @@ export async function installTerminalTestGlobals(): Promise { setHiddenRendererPty: vi.fn(), setPtyDeliveryInterest: vi.fn(), ackColdRestore: vi.fn(), + ackHiddenOutputRestoreApplied: vi.fn(), onClearBufferRequest: vi.fn(() => vi.fn()), onResetInputModesRequest: vi.fn(() => vi.fn()), resetInputModes: vi.fn(), diff --git a/src/renderer/src/components/terminal-pane/pty-connection/foreground-output-refresh.ts b/src/renderer/src/components/terminal-pane/pty-connection/foreground-output-refresh.ts index 662399532d5..a836d6712b9 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/foreground-output-refresh.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/foreground-output-refresh.ts @@ -10,6 +10,7 @@ import { RESET_AFTER_BYTE_GAP } from '../../../../../shared/terminal-mode-reset- import { recordTerminalOutput } from '@/lib/pane-manager/pane-scroll' import { ensureArabicShapingJoinerForText } from '@/lib/pane-manager/terminal-arabic-shaping-joiner' import { registerPtyModelRestoreNeededHandler } from '../pty-model-restore-channel' +import type { PtyModelRestoreNeededEvent } from '../../../../../shared/pty-model-restore-marker' import { acquireHiddenRendererPtyDeliveryClaim, declareRendererPtyDeliveryVisible @@ -51,10 +52,26 @@ export function bindForegroundOutputRefresh(session: ConnectPanePtySession): voi } // Why: main reports dropped renderer-bound bytes out-of-band, routed per PTY by pty-model-restore-channel.ts. - function handleModelRestoreNeededMarker(): void { + function handleModelRestoreNeededMarker(event: PtyModelRestoreNeededEvent): void { if (session.disposed) { return } + const episode = typeof event.hiddenDropEpisode === 'number' ? event.hiddenDropEpisode : null + // Why: main's latch stays armed until a restore paints, so a gap it can never serve re-marks + // on every unhide. A repeat of the disclosed episode is that same gap; any other marker may + // be new loss and proceeds, failing toward recovery. + const disclosed = session.hiddenOutputRestoreDisclosedGap + if ( + episode !== null && + disclosed !== null && + disclosed.ptyId === session.transport.getPtyId() && + disclosed.episode === episode + ) { + return + } + if (episode !== null) { + session.hiddenOutputRestoreLatestDropEpisode = episode + } recordTerminalFreezeBreadcrumb('restore-marker', { id: redactPtyIdForDiagnostics(session.transport.getPtyId() ?? '') }) @@ -90,6 +107,9 @@ export function bindForegroundOutputRefresh(session: ConnectPanePtySession): voi } session.unregisterModelRestoreNeeded?.() session.unregisterModelRestoreNeeded = null + // Marker bookkeeping is per PTY stream, like the restore state itself. + session.hiddenOutputRestoreLatestDropEpisode = null + session.hiddenOutputRestoreDisclosedGap = null session.modelRestoreSubscribedPtyId = ptyId // Why: markers exist only for PTYs whose bytes transit local main; // remote-runtime transports are structurally unaffected. diff --git a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-abandon.ts b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-abandon.ts index 3e30289c2f6..29238650f7d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-abandon.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-abandon.ts @@ -1,8 +1,23 @@ import { cancelScheduledHiddenOutputRestore } from '../hidden-output-restore-scheduler' +import { shouldWritePtyOutputForeground } from './foreground-output-scan' import type { ConnectPanePtySession } from './connect-pane-pty-session' export function bindAbandonHiddenOutputRestore(session: ConnectPanePtySession): void { + // Why: a loss finalized while hidden swallows the banner and zeroes recovery, so no reveal + // can reach the gap. Stay armed; reveal spends one fresh snapshot (attempt counters are kept), + // which heals a recovered source or abandons in the foreground where the banner lands. + session.deferHiddenOutputRestoreLossToReveal = function (): boolean { + if (shouldWritePtyOutputForeground(session.deps.isVisibleRef.current)) { + return false + } + // A deadline armed while foreground fires after reveal under hidden-timer throttling and + // would bump the generation, discarding the reveal's fresh snapshot mid-flight. + session.clearHiddenOutputRestoreForegroundDeadlineTimer() + session.hiddenOutputRestoreNeeded = true + return true + } + session.abandonHiddenOutputRestoreAndDrainPendingForeground = function ( expectedPtyId: string, opts: { quiet?: boolean; rearmRemote?: boolean } = {} diff --git a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts index e89c14f432a..8acca6423f3 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts @@ -131,6 +131,8 @@ export function bindHiddenOutputRestoreRequest(session: ConnectPanePtySession): return } const restoreGeneration = session.hiddenOutputRestoreGeneration + // Captured before the fetch: the snapshot can only cover drops main reported by now. + const coveredDropEpisode: number | null = session.hiddenOutputRestoreLatestDropEpisode session.hiddenOutputRestoreNeeded = false let snapshotResult: HiddenOutputSnapshotResult try { @@ -170,9 +172,11 @@ export function bindHiddenOutputRestoreRequest(session: ConnectPanePtySession): HIDDEN_OUTPUT_RESTORE_LOCAL_GATE_MAX_ATTEMPTS } if (budgetExhausted) { - session.abandonHiddenOutputRestoreAndDrainPendingForeground(currentPtyId, { - rearmRemote: false - }) + if (!session.deferHiddenOutputRestoreLossToReveal()) { + session.abandonHiddenOutputRestoreAndDrainPendingForeground(currentPtyId, { + rearmRemote: false + }) + } return } session.hiddenOutputRestoreNeeded = true @@ -182,9 +186,11 @@ export function bindHiddenOutputRestoreRequest(session: ConnectPanePtySession): return } if (snapshotResult.kind === 'permanently-unavailable') { - session.abandonHiddenOutputRestoreAndDrainPendingForeground(currentPtyId, { - rearmRemote: false - }) + if (!session.deferHiddenOutputRestoreLossToReveal()) { + session.abandonHiddenOutputRestoreAndDrainPendingForeground(currentPtyId, { + rearmRemote: false + }) + } return } if (snapshotResult.kind === 'unknown-legacy-host') { @@ -219,6 +225,7 @@ export function bindHiddenOutputRestoreRequest(session: ConnectPanePtySession): snapshot, restoredSnapshotPaintsPrintableContent(snapshot) ) + session.ackHiddenOutputRestoreApplied(currentPtyId, coveredDropEpisode) session.hiddenOutputRestoreReplayingSnapshot = null const needsFreshSnapshot = session.hiddenOutputRestoreFreshSnapshotNeeded session.hiddenOutputRestoreFreshSnapshotNeeded = false diff --git a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-snapshot.ts b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-snapshot.ts index 4db74de58b6..981bed321ca 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-snapshot.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-snapshot.ts @@ -50,6 +50,25 @@ export function bindHiddenOutputRestoreSnapshot(session: ConnectPanePtySession): foreground: true, beforeWrite: session.beforeTerminalOutputWrite }) + const ptyId = session.transport.getPtyId() + const episode = session.hiddenOutputRestoreLatestDropEpisode + if (ptyId !== null && episode !== null) { + // Repeat markers for the drop episode this banner disclosed are ignored. + session.hiddenOutputRestoreDisclosedGap = { ptyId, episode } + } + } + + // Why an ack instead of retiring on the serve: the pane can dispose across the serialize + + // IPC round trip, and a snapshot nobody painted heals nothing. Only PTYs whose bytes transit + // local main have a drop latch, so remote-runtime streams send nothing. + session.ackHiddenOutputRestoreApplied = function ( + ptyId: string, + hiddenDropEpisode: number | null + ): void { + if (!session.canUseMainBufferSnapshot(ptyId)) { + return + } + window.api.pty.ackHiddenOutputRestoreApplied?.(ptyId, hiddenDropEpisode ?? undefined) } session.applyMainBufferSnapshot = async function (snapshot: { diff --git a/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts b/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts index 6d142305be7..3df0803e532 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts @@ -134,6 +134,10 @@ export function bindCaptureTransportOutputCallbacks(session: ConnectPanePtySessi session.hiddenOutputRestoreLegacyPtyId = null // Bounded remote re-arms spent instead of the loss banner (per PTY stream). session.hiddenOutputRestoreRemoteAbandonCycles = 0 + // Latest hidden-drop episode main reported, and the one a banner already disclosed: + // main's drop latch outlives unhide, so an unservable gap re-marks on every reveal. + session.hiddenOutputRestoreLatestDropEpisode = null + session.hiddenOutputRestoreDisclosedGap = null session.hiddenOutputSnapshotScrollRestore = null // Why: hidden recovery state belongs to one PTY stream. Reattach/restart // can reuse the pane object for a different session before visibility. diff --git a/src/renderer/src/web/preload-api/web-terminal-api.ts b/src/renderer/src/web/preload-api/web-terminal-api.ts index edbdde18363..0b0b474287a 100644 --- a/src/renderer/src/web/preload-api/web-terminal-api.ts +++ b/src/renderer/src/web/preload-api/web-terminal-api.ts @@ -21,6 +21,8 @@ export function createPtyApi(): NonNullable['pty']> { resetInputModes: () => {}, kill: () => Promise.resolve(), ackColdRestore: () => {}, + // Web panes never transit main's hidden-delivery gate, so there is no drop latch. + ackHiddenOutputRestoreApplied: () => {}, ackData: () => {}, onDeliveryResyncRequest: () => noopUnsubscribe, respondDeliveryResync: () => {}, diff --git a/src/shared/pty-model-restore-marker.ts b/src/shared/pty-model-restore-marker.ts index a182226a0d0..f43201dfe4e 100644 --- a/src/shared/pty-model-restore-marker.ts +++ b/src/shared/pty-model-restore-marker.ts @@ -16,4 +16,7 @@ export type PtyModelRestoreNeededEvent = { /** Main's PTY output sequence at emit time — everything at or before this * point is only recoverable from the model snapshot. */ markerSeq?: number + /** Hidden-delivery drop episode this marker reports (gate markers only). A + * repeat for an episode the renderer already disclosed as lost is the same gap. */ + hiddenDropEpisode?: number }