mirror of
https://github.com/stablyai/orca.git
synced 2026-10-26 16:02:42 +00:00
fix(terminal): retire the hidden-drop latch on the applied ack, not the serve
Review round on the STA-4869 fix (three adversarial reviews). Two reviewers found the same window: retiring main's droppedSinceHiddenPtys latch when pty:getMainBufferSnapshot served the snapshot spends it before anyone paints. The reply is a multi-MB serialize plus an IPC round trip, and a tab/split/worktree switch or renderer reload inside that window disposes the asking pane 0-3 async ticks later. Move retirement to a renderer->main fire-and-forget ack sent after the snapshot is applied (pty:hiddenOutputRestoreApplied), and drop the hiddenOutputRestore opt entirely: the ack is one structural seam with no caller flag to police. The parked-SSH prepaint sends it too, behind its own guards, because that repaint also paints main's retained bytes. Also from the round: - deferHiddenOutputRestoreLossToReveal now clears the foreground deadline. A deadline armed while foreground survived the deferral, and Chromium throttles hidden timers to >=1s, so it typically fired AFTER reveal, passed the foreground guard, bannered, and bumped the generation - discarding the reveal's fresh snapshot mid-flight. - Disclosed-gap dedupe. The longer-lived latch re-marks on every unhide, so a permanently refused source turned four reveals into four banners and sixteen host serializes. A written banner now records the highest markerSeq it covered; later markers at or below that seq describe the same gap and are ignored. A higher seq (new bytes dropped since) or a seq-less marker proceeds, failing toward recovery. Cleared per PTY stream. Tests: apply-ack lifetime on both sides (served-but-unacked stays armed, ack retires, repeat/unknown-id acks are no-ops, dispose-mid-serve recovers on the remount), the storm bound, the reveal-spends-one-request property, the restore-loop PTY-swap arm's re-arm, and the missing markerWrites guard on the marker-handoff invariant. writeRestoreUnavailableWarning's precondition comment now names the swap arm and why its banner call is unreachable. Refs STA-4869
This commit is contained in:
@@ -42,7 +42,7 @@ describe('pty hidden delivery gate', () => {
|
||||
expect(shouldDropHiddenRendererPtyData(PTY_ID, {})).toBe(true)
|
||||
})
|
||||
|
||||
it('requests the restore marker exactly once per drop episode, re-armed by recovery', () => {
|
||||
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)
|
||||
expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(false)
|
||||
@@ -53,12 +53,12 @@ describe('pty hidden delivery gate', () => {
|
||||
markHiddenRendererPty(PTY_ID)
|
||||
expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(false)
|
||||
|
||||
// Recovery took delivery of the retained bytes: the next episode reports again.
|
||||
// The renderer painted the snapshot: the next episode reports again.
|
||||
consumeHiddenRendererPtyDropMemory(PTY_ID)
|
||||
expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps drop memory across unhide until recovery takes the bytes (STA-4869)', () => {
|
||||
it('keeps drop memory across unhide until the renderer acks the paint (STA-4869)', () => {
|
||||
markHiddenRendererPty(PTY_ID)
|
||||
recordHiddenRendererPtyDataDrop(PTY_ID, 10)
|
||||
|
||||
|
||||
@@ -23,8 +23,8 @@ const hiddenRendererPtys = new Set<string>()
|
||||
const deliveryInterestRendererPtys = new Set<string>()
|
||||
// Why: reveal must restore from the model only when bytes were actually
|
||||
// dropped. Doubles as the marker latch: the first gated drop emits a restore
|
||||
// marker, every unhide re-emits one, and the latch retires only when recovery
|
||||
// actually has the bytes (a served model snapshot) or the PTY tears down.
|
||||
// marker, every unhide re-emits one, and the latch retires only when the
|
||||
// renderer reports the retained bytes painted, or the PTY tears down.
|
||||
// Neither re-marking hidden nor unmarking clears it — a marker only reaches a
|
||||
// pane that is listening when it lands, and a retiring hidden pane unhides into
|
||||
// a renderer with no handler registered (STA-4869).
|
||||
@@ -62,9 +62,9 @@ export function unmarkHiddenRendererPty(id: string): { droppedWhileHidden: boole
|
||||
return { droppedWhileHidden: droppedSinceHiddenPtys.has(id) }
|
||||
}
|
||||
|
||||
/** Retires drop memory once recovery holds the bytes — main served a model
|
||||
* snapshot, which carries everything ingested before it. A refused snapshot
|
||||
* must leave the latch armed so the next unhide re-emits. */
|
||||
/** Retires drop memory once the renderer reports the snapshot applied. Serving
|
||||
* the snapshot is not enough: the pane can dispose across the serialize+IPC
|
||||
* round trip, and a snapshot nobody painted heals nothing. */
|
||||
export function consumeHiddenRendererPtyDropMemory(id: string): void {
|
||||
droppedSinceHiddenPtys.delete(id)
|
||||
}
|
||||
|
||||
@@ -170,128 +170,6 @@ describe('registerPtyHandlers', () => {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
it('retires drop memory on the recovery snapshot, not on unhide (STA-4869)', async () => {
|
||||
vi.useFakeTimers()
|
||||
const runtime = {
|
||||
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
|
||||
}))
|
||||
}
|
||||
const daemon = installObservableDaemonTestProvider()
|
||||
const restoreMarkers = (): number =>
|
||||
mainWindow.webContents.send.mock.calls.filter(
|
||||
(call: unknown[]) => call[0] === 'pty:modelRestoreNeeded'
|
||||
).length
|
||||
try {
|
||||
registerPtyHandlers(mainWindow as never, runtime as never)
|
||||
const result = (await handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: 'daemon-session'
|
||||
})) as { id: string }
|
||||
const setHidden = getPtySetHiddenRendererPtyListener()
|
||||
const getSnapshot = handlers.get('pty:getMainBufferSnapshot')!
|
||||
mainWindow.webContents.send.mockClear()
|
||||
|
||||
setHidden(null, { id: result.id, hidden: true })
|
||||
daemon.emitData(result.id, 'hidden output')
|
||||
vi.advanceTimersByTime(50)
|
||||
expect(restoreMarkers()).toBe(1)
|
||||
|
||||
// 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: result.id, hidden: false })
|
||||
setHidden(null, { id: result.id, hidden: false })
|
||||
expect(restoreMarkers()).toBe(3)
|
||||
expect(mainWindow.webContents.send).toHaveBeenLastCalledWith('pty:modelRestoreNeeded', {
|
||||
id: result.id,
|
||||
reason: 'unhide',
|
||||
markerSeq: 42
|
||||
})
|
||||
|
||||
// Sidecar readers (native chat, the SSH reattach probe) fetch the same
|
||||
// snapshot without claiming recovery, so they cannot spend the latch.
|
||||
await getSnapshot(null, { id: result.id })
|
||||
setHidden(null, { id: result.id, hidden: false })
|
||||
expect(restoreMarkers()).toBe(4)
|
||||
|
||||
// Recovery's own fetch retires it: no marker storm afterwards.
|
||||
await getSnapshot(null, { id: result.id, opts: { hiddenOutputRestore: true } })
|
||||
setHidden(null, { id: result.id, hidden: false })
|
||||
setHidden(null, { id: result.id, hidden: false })
|
||||
expect(restoreMarkers()).toBe(4)
|
||||
|
||||
// A fresh hidden episode reports again.
|
||||
setHidden(null, { id: result.id, hidden: true })
|
||||
daemon.emitData(result.id, 'hidden again')
|
||||
vi.advanceTimersByTime(50)
|
||||
expect(restoreMarkers()).toBe(5)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
it('keeps drop memory armed when the recovery snapshot is refused', async () => {
|
||||
vi.useFakeTimers()
|
||||
const runtime = {
|
||||
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 () => null)
|
||||
}
|
||||
const daemon = installObservableDaemonTestProvider()
|
||||
try {
|
||||
registerPtyHandlers(mainWindow as never, runtime as never)
|
||||
const result = (await handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: 'daemon-session'
|
||||
})) as { id: string }
|
||||
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 delivered nothing, so the next reveal still owes
|
||||
// the renderer a restore signal.
|
||||
expect(
|
||||
await handlers.get('pty:getMainBufferSnapshot')!(null, {
|
||||
id: result.id,
|
||||
opts: { hiddenOutputRestore: true }
|
||||
})
|
||||
).toBeNull()
|
||||
setHidden(null, { id: result.id, hidden: false })
|
||||
expect(mainWindow.webContents.send).toHaveBeenLastCalledWith('pty:modelRestoreNeeded', {
|
||||
id: result.id,
|
||||
reason: 'unhide',
|
||||
markerSeq: 42
|
||||
})
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
it('surfaces the hidden-yet-visible contradiction in the snapshot and warns on drop', async () => {
|
||||
// Why: field snapshot v1.4.124-rc.2.perf — aggregates couldn't tell if the visible pane was hidden-gated; overlap counter + warn makes it decisive.
|
||||
vi.useFakeTimers()
|
||||
|
||||
@@ -0,0 +1,237 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { setupPtyIpcSuite } from './pty-ipc-test-harness'
|
||||
import { registerPtyHandlers } from './pty'
|
||||
|
||||
// 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<string, unknown> {
|
||||
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
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
/** 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()
|
||||
registerPtyHandlers(mainWindow as never, buildRuntime() as never)
|
||||
const result = (await handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: 'daemon-session'
|
||||
})) as { id: string }
|
||||
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,
|
||||
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 })
|
||||
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()
|
||||
registerPtyHandlers(mainWindow as never, buildRuntime() as never)
|
||||
const result = (await handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: 'daemon-session'
|
||||
})) as { id: string }
|
||||
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)
|
||||
ackApplied(null, { id: result.id })
|
||||
setHidden(null, { id: result.id, hidden: false })
|
||||
|
||||
setHidden(null, { id: result.id, hidden: true })
|
||||
daemon.emitData(result.id, 'hidden again')
|
||||
vi.advanceTimersByTime(50)
|
||||
|
||||
expect(
|
||||
mainWindow.webContents.send.mock.calls.filter(
|
||||
(call: unknown[]) => call[0] === 'pty:modelRestoreNeeded'
|
||||
).length
|
||||
).toBe(2)
|
||||
} 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)
|
||||
|
||||
ackApplied(null, { id })
|
||||
ackApplied(null, { id })
|
||||
setHidden(null, { id, hidden: false })
|
||||
expect(restoreMarkers()).toBe(2)
|
||||
} 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)
|
||||
registerPtyHandlers(mainWindow as never, runtime as never)
|
||||
const result = (await handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: 'daemon-session'
|
||||
})) as { id: string }
|
||||
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,
|
||||
reason: 'unhide',
|
||||
markerSeq: 42
|
||||
})
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -113,6 +113,19 @@ export function createPtyIpcListenerAccessors(ctx: {
|
||||
return hiddenCall[1] as (event: unknown, args: { id: string; hidden: boolean }) => void
|
||||
}
|
||||
|
||||
function getPtyHiddenOutputRestoreAppliedListener(): (
|
||||
event: unknown,
|
||||
args: { id?: unknown }
|
||||
) => void {
|
||||
const appliedCall = onMock.mock.calls.find(
|
||||
(call: unknown[]) => call[0] === 'pty:hiddenOutputRestoreApplied'
|
||||
)
|
||||
if (!appliedCall) {
|
||||
throw new Error('missing pty:hiddenOutputRestoreApplied listener')
|
||||
}
|
||||
return appliedCall[1] as (event: unknown, args: { id?: unknown }) => void
|
||||
}
|
||||
|
||||
function getPtySetDeliveryInterestListener(): (
|
||||
event: unknown,
|
||||
args: { id: string; interested: boolean }
|
||||
@@ -188,6 +201,7 @@ export function createPtyIpcListenerAccessors(ctx: {
|
||||
getMainFrameNavigationListener,
|
||||
getPtyResizeListener,
|
||||
getPtySetHiddenRendererPtyListener,
|
||||
getPtyHiddenOutputRestoreAppliedListener,
|
||||
getPtySetDeliveryInterestListener,
|
||||
DELIVERY_RESYNC_UNANSWERED_WARNING,
|
||||
countResyncUnansweredWarnings,
|
||||
|
||||
+14
-10
@@ -6007,23 +6007,15 @@ export function registerPtyHandlers(
|
||||
'pty:getMainBufferSnapshot',
|
||||
async (
|
||||
_event,
|
||||
args: { id?: unknown; opts?: { scrollbackRows?: unknown; hiddenOutputRestore?: unknown } }
|
||||
args: { id?: unknown; opts?: { scrollbackRows?: unknown } }
|
||||
): Promise<MainBufferSnapshotReply> => {
|
||||
if (!runtime || typeof args?.id !== 'string' || args.id.length === 0) {
|
||||
return null
|
||||
}
|
||||
const snapshot = await resolveMainBufferSnapshot(
|
||||
return resolveMainBufferSnapshot(
|
||||
args.id,
|
||||
normalizeSnapshotScrollbackRows(args.opts?.scrollbackRows)
|
||||
)
|
||||
// Why here and not on unhide: this is where hidden-output recovery actually
|
||||
// takes delivery of the retained bytes. A refused snapshot leaves the latch
|
||||
// armed so the next unhide re-emits, and sidecar readers (native chat, the
|
||||
// SSH reattach probe) never pass the flag, so they cannot spend it (STA-4869).
|
||||
if (snapshot && args.opts?.hiddenOutputRestore === true) {
|
||||
consumeHiddenRendererPtyDropMemory(args.id)
|
||||
}
|
||||
return snapshot
|
||||
}
|
||||
)
|
||||
|
||||
@@ -7700,6 +7692,18 @@ export function registerPtyHandlers(
|
||||
}
|
||||
})
|
||||
|
||||
ipcMain.removeAllListeners('pty:hiddenOutputRestoreApplied')
|
||||
ipcMain.on('pty:hiddenOutputRestoreApplied', (_event, args: { id?: unknown }) => {
|
||||
if (typeof args?.id !== 'string' || !args.id) {
|
||||
return
|
||||
}
|
||||
// Why the ack and not the serve: the pane can dispose across the multi-MB
|
||||
// serialize + IPC round trip, and a snapshot nobody painted heals nothing.
|
||||
// Retiring here keeps the latch armed until the bytes are on screen, so the
|
||||
// replacement pane's unhide still re-emits the marker (STA-4869).
|
||||
consumeHiddenRendererPtyDropMemory(args.id)
|
||||
})
|
||||
|
||||
ipcMain.removeAllListeners('pty:terminalViewAttributes')
|
||||
ipcMain.on('pty:terminalViewAttributes', (_event, args: unknown) => {
|
||||
// Why validate-or-drop: a malformed palette gives a wrong color reply that breaks TUI theme detection worse than the silent-until-first-push default.
|
||||
|
||||
@@ -76,6 +76,10 @@ export type PtyApi = {
|
||||
clearBuffer: (id: string) => void
|
||||
kill: (id: string, opts?: { keepHistory?: boolean }) => Promise<void>
|
||||
ackColdRestore: (id: string) => void
|
||||
/** Hidden-output recovery painted main's snapshot: retires the drop latch
|
||||
* behind the hidden-delivery gate. Serving the snapshot is not enough — the
|
||||
* pane can dispose across the round trip (STA-4869). */
|
||||
ackHiddenOutputRestoreApplied: (id: string) => void
|
||||
ackData: (id: string, charCount: number, processedChars?: number) => void
|
||||
onDeliveryResyncRequest: (callback: (payload: { requestId: number }) => void) => () => void
|
||||
respondDeliveryResync: (payload: {
|
||||
@@ -121,12 +125,7 @@ export type PtyApi = {
|
||||
hasPty: (id: string) => Promise<boolean | null>
|
||||
getMainBufferSnapshot: (
|
||||
id: string,
|
||||
opts?: {
|
||||
scrollbackRows?: number
|
||||
/** Marks this fetch as hidden-output recovery, which retires main's
|
||||
* drop latch; sidecar readers must leave it unset. */
|
||||
hiddenOutputRestore?: boolean
|
||||
}
|
||||
opts?: { scrollbackRows?: number }
|
||||
) => Promise<{
|
||||
data: string
|
||||
frameRestoreAnsi?: string
|
||||
|
||||
@@ -1085,6 +1085,10 @@ const api = {
|
||||
ackColdRestore: (id: string): void => {
|
||||
ipcRenderer.send('pty:ackColdRestore', { id })
|
||||
},
|
||||
/** Hidden-output recovery painted main's snapshot — retires the hidden-delivery drop latch. Fire-and-forget. */
|
||||
ackHiddenOutputRestoreApplied: (id: string): void => {
|
||||
ipcRenderer.send('pty:hiddenOutputRestoreApplied', { id })
|
||||
},
|
||||
/** charCount is the legacy per-chunk delta; processedChars is the cumulative per-pty total (self-heals under lost ACKs). */
|
||||
ackData: (id: string, charCount: number, processedChars?: number): void => {
|
||||
ipcRenderer.send('pty:ackData', {
|
||||
@@ -1147,7 +1151,7 @@ const api = {
|
||||
|
||||
getMainBufferSnapshot: (
|
||||
id: string,
|
||||
opts?: { scrollbackRows?: number; hiddenOutputRestore?: boolean }
|
||||
opts?: { scrollbackRows?: number }
|
||||
): Promise<{
|
||||
data: string
|
||||
frameRestoreAnsi?: string
|
||||
|
||||
+1
@@ -429,6 +429,7 @@ describe('fish never receives a color-scheme report it did not query (#9993)', (
|
||||
setHiddenRendererPty: vi.fn(),
|
||||
setPtyDeliveryInterest: vi.fn(),
|
||||
ackColdRestore: vi.fn(),
|
||||
ackHiddenOutputRestoreApplied: vi.fn(),
|
||||
onClearBufferRequest: vi.fn(() => vi.fn()),
|
||||
onSerializeBufferRequest: vi.fn(() => vi.fn()),
|
||||
sendSerializedBuffer: vi.fn(),
|
||||
|
||||
+3
-12
@@ -317,10 +317,7 @@ describe('connectPanePty', () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 30))
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.write).not.toHaveBeenCalledWith(hidden)
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(
|
||||
expect.stringContaining('inactive snapshot'),
|
||||
@@ -541,10 +538,7 @@ describe('connectPanePty', () => {
|
||||
})
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.resize).toHaveBeenCalledWith(100, 30)
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(NORMAL_BUFFER_PROLOGUE, expect.any(Function))
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith('snapshot-state\r\n', expect.any(Function))
|
||||
@@ -595,10 +589,7 @@ describe('connectPanePty', () => {
|
||||
})
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(NORMAL_BUFFER_PROLOGUE, expect.any(Function))
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(
|
||||
'preserved-shell-history\r\n',
|
||||
|
||||
+2
-8
@@ -243,10 +243,7 @@ describe('connectPanePty', () => {
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(setHiddenRendererPty).toHaveBeenLastCalledWith('pty-id', false)
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', {
|
||||
scrollbackRows: 50_000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 50_000 })
|
||||
// The unhide IPC must precede the snapshot request (seq-guard contract).
|
||||
const unhideOrder = setHiddenRendererPty.mock.invocationCallOrder.at(-1)!
|
||||
const snapshotOrder = getMainBufferSnapshot.mock.invocationCallOrder[0]!
|
||||
@@ -428,10 +425,7 @@ describe('connectPanePty', () => {
|
||||
requestTerminalBacklogRecovery(pane.terminal as never)
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(
|
||||
expect.stringContaining('dropped bytes snapshot'),
|
||||
expect.any(Function)
|
||||
|
||||
+60
@@ -1,5 +1,6 @@
|
||||
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,
|
||||
@@ -369,6 +370,7 @@ describe('hidden-output restore loss disclosure (STA-4869)', () => {
|
||||
const drive = await exhaustBudgetWhileHidden(serializeBufferOutcome)
|
||||
expect(drive.writtenChunks().join('')).not.toContain(BANNER_FRAGMENT)
|
||||
|
||||
const requestsBeforeReveal = serializeBufferOutcome.mock.calls.length
|
||||
await revealPane(drive)
|
||||
await allowRecoveryWindow()
|
||||
|
||||
@@ -378,6 +380,10 @@ describe('hidden-output restore loss disclosure (STA-4869)', () => {
|
||||
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()
|
||||
})
|
||||
})
|
||||
@@ -517,4 +523,58 @@ describe('hidden-output restore loss disclosure (STA-4869)', () => {
|
||||
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<unknown>()
|
||||
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
|
||||
})
|
||||
const transport = paneTransport as unknown as MockTransport
|
||||
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()).toContain(RESET_AFTER_BYTE_GAP)
|
||||
drive.disposable.dispose()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
+98
-17
@@ -1,6 +1,6 @@
|
||||
import type * as React from 'react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { flushAsyncTicks } from './pty-connection-test-async'
|
||||
import { createDeferred, flushAsyncTicks } from './pty-connection-test-async'
|
||||
import {
|
||||
createMockTransport,
|
||||
createPane,
|
||||
@@ -23,7 +23,7 @@ import {
|
||||
// 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
|
||||
// takes delivery of the bytes — main serving a model snapshot.
|
||||
// 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
|
||||
@@ -138,7 +138,7 @@ type PaneDrive = {
|
||||
|
||||
/** 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 a served model snapshot (or teardown) retires it. */
|
||||
* only the renderer's applied-ack (or teardown) retires it. */
|
||||
const mainDropLatch = {
|
||||
hidden: false,
|
||||
dropped: false,
|
||||
@@ -158,7 +158,7 @@ const mainDropLatch = {
|
||||
this.hidden = false
|
||||
return this.dropped
|
||||
},
|
||||
/** consumeHiddenRendererPtyDropMemory: recovery has the bytes now. */
|
||||
/** consumeHiddenRendererPtyDropMemory: the renderer painted the snapshot. */
|
||||
consumeDropMemory(): void {
|
||||
this.dropped = false
|
||||
},
|
||||
@@ -168,8 +168,8 @@ const mainDropLatch = {
|
||||
}
|
||||
}
|
||||
|
||||
/** Routes the renderer's hidden-state IPC into main's real gate and delivers
|
||||
* main's marker back one turn later, as the IPC round trip does. */
|
||||
/** 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
|
||||
@@ -188,6 +188,9 @@ async function wireMainGateToRendererChannel(): Promise<{ markersDelivered: () =
|
||||
})
|
||||
}
|
||||
)
|
||||
vi.mocked(window.api.pty.ackHiddenOutputRestoreApplied).mockImplementation(() => {
|
||||
mainDropLatch.consumeDropMemory()
|
||||
})
|
||||
return { markersDelivered: () => markersDelivered }
|
||||
}
|
||||
|
||||
@@ -263,15 +266,12 @@ describe('hidden-output restore marker handoff (STA-4869 R6)', () => {
|
||||
terminalMainSideEffectAuthority: true
|
||||
} as StoreState['settings']
|
||||
installTerminalTestGlobals()
|
||||
vi.mocked(window.api.pty.getMainBufferSnapshot).mockImplementation(
|
||||
async (_id: string, opts?: { hiddenOutputRestore?: boolean }) => {
|
||||
// Main retires the drop latch when it actually serves recovery its bytes.
|
||||
if (opts?.hiddenOutputRestore === true) {
|
||||
mainDropLatch.consumeDropMemory()
|
||||
}
|
||||
return { data: `${R6_MARKER}\r\n`, cols: 120, rows: 40, seq: 4096 } as never
|
||||
}
|
||||
)
|
||||
vi.mocked(window.api.pty.getMainBufferSnapshot).mockResolvedValue({
|
||||
data: `${R6_MARKER}\r\n`,
|
||||
cols: 120,
|
||||
rows: 40,
|
||||
seq: 4096
|
||||
} as never)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
@@ -337,6 +337,7 @@ describe('hidden-output restore marker handoff (STA-4869 R6)', () => {
|
||||
...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)}`
|
||||
@@ -344,7 +345,7 @@ describe('hidden-output restore marker handoff (STA-4869 R6)', () => {
|
||||
replacement.binding.dispose()
|
||||
})
|
||||
|
||||
it('[fix] the remount re-emits the unhide marker and one snapshot retires the latch', async () => {
|
||||
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)
|
||||
@@ -358,8 +359,9 @@ describe('hidden-output restore marker handoff (STA-4869 R6)', () => {
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
// The latch outlived the marker nobody received, so the remount's own unhide
|
||||
// re-emits it; the snapshot that heals the gap is what finally spends it.
|
||||
// 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)
|
||||
expect(observeDisclosure(replacement)).toMatchObject({ markerWrites: 1 })
|
||||
expect(mainDropLatch.dropped).toBe(false)
|
||||
|
||||
@@ -375,4 +377,83 @@ describe('hidden-output restore marker handoff (STA-4869 R6)', () => {
|
||||
expect(vi.mocked(window.api.pty.getMainBufferSnapshot).mock.calls.length).toBe(snapshotRequests)
|
||||
replacement.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 served = createDeferred<unknown>()
|
||||
const snapshot = { data: `${R6_MARKER}\r\n`, cols: 120, rows: 40, seq: 4096 }
|
||||
let servedCalls = 0
|
||||
vi.mocked(window.api.pty.getMainBufferSnapshot).mockImplementation(async () => {
|
||||
servedCalls += 1
|
||||
return (servedCalls === 1 ? await served.promise : snapshot) as never
|
||||
})
|
||||
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 as never)
|
||||
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) {
|
||||
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()
|
||||
})
|
||||
})
|
||||
|
||||
+1
-4
@@ -200,10 +200,7 @@ describe('connectPanePty', () => {
|
||||
})
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.resize).toHaveBeenCalledWith(96, 18)
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(richSnapshot, expect.any(Function))
|
||||
expect(pane.terminal.write).not.toHaveBeenCalledWith(visibleTrigger, expect.any(Function))
|
||||
|
||||
+1
-4
@@ -559,10 +559,7 @@ describe('connectPanePty', () => {
|
||||
}
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledOnce()
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith(localPtyId, {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith(localPtyId, { scrollbackRows: 5000 })
|
||||
expect(transport.connect).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ sessionId: localPtyId })
|
||||
)
|
||||
|
||||
+1
-2
@@ -761,8 +761,7 @@ describe('connectPanePty', () => {
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-large-live', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
scrollbackRows: 5000
|
||||
})
|
||||
expect(writes.some((write) => write.startsWith('L'))).toBe(false)
|
||||
expect(acknowledgeDroppedFrame).toHaveBeenCalledOnce()
|
||||
|
||||
+2
-8
@@ -187,10 +187,7 @@ describe('connectPanePty', () => {
|
||||
rawLength: secondLive.length
|
||||
})
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.write).not.toHaveBeenCalledWith(firstLive, expect.any(Function))
|
||||
expect(pane.terminal.write).not.toHaveBeenCalledWith(secondLive, expect.any(Function))
|
||||
|
||||
@@ -751,10 +748,7 @@ describe('connectPanePty', () => {
|
||||
})
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('old-pty-id', {
|
||||
scrollbackRows: 5000,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('old-pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.write).not.toHaveBeenCalledWith(
|
||||
'old-snapshot-state\r\n',
|
||||
expect.any(Function)
|
||||
|
||||
@@ -53,6 +53,7 @@ export function installTerminalTestGlobals(): void {
|
||||
setHiddenRendererPty: vi.fn(),
|
||||
setPtyDeliveryInterest: vi.fn(),
|
||||
ackColdRestore: vi.fn(),
|
||||
ackHiddenOutputRestoreApplied: vi.fn(),
|
||||
onClearBufferRequest: vi.fn(() => vi.fn()),
|
||||
onSerializeBufferRequest: vi.fn(() => vi.fn()),
|
||||
sendSerializedBuffer: vi.fn(),
|
||||
|
||||
@@ -106,6 +106,7 @@ import {
|
||||
} from './stale-document-visibility'
|
||||
import { recordTerminalFreezeBreadcrumb } from './terminal-freeze-breadcrumbs'
|
||||
import { redactPtyIdForDiagnostics } from '../../../../shared/pty-delivery-diagnostics'
|
||||
import type { PtyModelRestoreNeededEvent } from '../../../../shared/pty-model-restore-marker'
|
||||
import {
|
||||
nativeWindowsRewriteNeedsFollowupRenderRefresh,
|
||||
terminalOutputPrefersRenderRefresh,
|
||||
@@ -5913,6 +5914,11 @@ export function connectPanePty(
|
||||
let hiddenOutputRestoreLegacyPtyId: string | null = null
|
||||
// Bounded remote re-arms spent instead of the loss banner (per PTY stream).
|
||||
let hiddenOutputRestoreRemoteAbandonCycles = 0
|
||||
// Highest markerSeq main has reported for the subscribed PTY, and the gap a
|
||||
// banner has already disclosed — main keeps its drop latch armed until the
|
||||
// restore lands, so an unrecoverable gap re-marks on every unhide.
|
||||
let hiddenOutputRestoreMarkerSeqHighWater: number | null = null
|
||||
let hiddenOutputRestoreDisclosedGap: { ptyId: string; seq: number | null } | null = null
|
||||
let hiddenOutputSnapshotScrollRestore: {
|
||||
ptyId: string | null
|
||||
generation: number
|
||||
@@ -6011,6 +6017,17 @@ export function connectPanePty(
|
||||
return transport.getPtyId() === ptyId && typeof transport.serializeBuffer === 'function'
|
||||
}
|
||||
|
||||
// Why an ack instead of retiring on the serve: main's drop latch must outlive
|
||||
// the multi-MB serialize + IPC round trip, because the pane can dispose in it
|
||||
// and a snapshot nobody painted heals nothing. Only PTYs whose bytes transit
|
||||
// local main have a latch, so remote-runtime streams stay untouched (STA-4869).
|
||||
function ackHiddenOutputRestoreApplied(ptyId: string): void {
|
||||
if (!canUseMainBufferSnapshot(ptyId)) {
|
||||
return
|
||||
}
|
||||
window.api.pty.ackHiddenOutputRestoreApplied(ptyId)
|
||||
}
|
||||
|
||||
type HiddenOutputSnapshotResult =
|
||||
| { kind: 'snapshot'; snapshot: PtyBufferSnapshot }
|
||||
// `source` picks the budget: only 'host' answers cost the host a serialize attempt.
|
||||
@@ -6029,10 +6046,7 @@ export function connectPanePty(
|
||||
return snapshot ? { kind: 'snapshot', snapshot } : { kind: 'unavailable' }
|
||||
}
|
||||
if (canUseMainBufferSnapshot(ptyId)) {
|
||||
const snapshot = await window.api.pty.getMainBufferSnapshot(ptyId, {
|
||||
...opts,
|
||||
hiddenOutputRestore: true
|
||||
})
|
||||
const snapshot = await window.api.pty.getMainBufferSnapshot(ptyId, opts)
|
||||
return snapshot ? { kind: 'snapshot', snapshot } : { kind: 'unavailable' }
|
||||
}
|
||||
if (transport.getPtyId() !== ptyId || typeof transport.serializeBuffer !== 'function') {
|
||||
@@ -6163,10 +6177,32 @@ export function connectPanePty(
|
||||
}
|
||||
|
||||
// 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 (disposed) {
|
||||
return
|
||||
}
|
||||
const markerSeq = typeof event.markerSeq === 'number' ? event.markerSeq : null
|
||||
if (markerSeq !== null) {
|
||||
hiddenOutputRestoreMarkerSeqHighWater = Math.max(
|
||||
hiddenOutputRestoreMarkerSeqHighWater ?? markerSeq,
|
||||
markerSeq
|
||||
)
|
||||
}
|
||||
// Why: main's latch stays armed until a restore lands, so a gap it can
|
||||
// never serve re-marks on every unhide. Once the banner has disclosed
|
||||
// everything up to that seq, repeats describe the same gap — answering them
|
||||
// re-banners and re-fetches per reveal. A higher seq means bytes were
|
||||
// dropped since the disclosure, and a seq-less marker cannot be compared;
|
||||
// both proceed, failing toward recovery (STA-4869).
|
||||
if (
|
||||
hiddenOutputRestoreDisclosedGap !== null &&
|
||||
hiddenOutputRestoreDisclosedGap.ptyId === transport.getPtyId() &&
|
||||
hiddenOutputRestoreDisclosedGap.seq !== null &&
|
||||
markerSeq !== null &&
|
||||
markerSeq <= hiddenOutputRestoreDisclosedGap.seq
|
||||
) {
|
||||
return
|
||||
}
|
||||
recordTerminalFreezeBreadcrumb('restore-marker', {
|
||||
id: redactPtyIdForDiagnostics(transport.getPtyId() ?? '')
|
||||
})
|
||||
@@ -6202,6 +6238,9 @@ export function connectPanePty(
|
||||
}
|
||||
unregisterModelRestoreNeeded?.()
|
||||
unregisterModelRestoreNeeded = null
|
||||
// Marker bookkeeping is per PTY stream, like the restore state itself.
|
||||
hiddenOutputRestoreMarkerSeqHighWater = null
|
||||
hiddenOutputRestoreDisclosedGap = null
|
||||
modelRestoreSubscribedPtyId = ptyId
|
||||
// Why: markers exist only for PTYs whose bytes transit local main;
|
||||
// remote-runtime transports are structurally unaffected.
|
||||
@@ -7115,6 +7154,12 @@ export function connectPanePty(
|
||||
if (shouldWritePtyOutputForeground(deps.isVisibleRef.current)) {
|
||||
return false
|
||||
}
|
||||
// A deadline armed while this pane was still foreground would survive the
|
||||
// deferral, and Chromium throttles hidden timers to >=1s so it typically
|
||||
// fires AFTER reveal: it passes the foreground guard, banners, and bumps the
|
||||
// generation — discarding the reveal's fresh snapshot mid-flight. Reveal
|
||||
// re-arms its own deadline once it has pending chunks again.
|
||||
clearHiddenOutputRestoreForegroundDeadlineTimer()
|
||||
hiddenOutputRestoreNeeded = true
|
||||
return true
|
||||
}
|
||||
@@ -7315,9 +7360,16 @@ export function connectPanePty(
|
||||
}
|
||||
}
|
||||
|
||||
// Precondition: every caller must already be foreground. A background pane
|
||||
// swallows the banner, so declaring a loss there discloses it to nobody —
|
||||
// deferHiddenOutputRestoreLossToReveal keeps the abandon out of that state.
|
||||
// Precondition: every caller must already be foreground, or a background pane
|
||||
// swallows the banner and the loss is declared to nobody. The abandon and the
|
||||
// deferred-retry exhaustion get there via deferHiddenOutputRestoreLossToReveal
|
||||
// and the foreground guard in scheduleHiddenOutputRestoreDeferredRetry. The
|
||||
// restore loop's PTY-swap arm is foreground by construction: it only runs on
|
||||
// iteration 2+, and iteration 1 returns at the hidden-pause guard after the
|
||||
// replay. That arm also cannot reach the banner at all — it is remote-only
|
||||
// (a local ptyId always satisfies canUseHiddenOutputSnapshot), and the
|
||||
// clearHiddenOutputRestoreState() right before it resets the re-arm budget,
|
||||
// so rearmRemoteHiddenOutputRestoreInsteadOfWarning always wins.
|
||||
function writeRestoreUnavailableWarning(): void {
|
||||
// The reset must parse before both the warning and any foreground drain.
|
||||
writePtyOutputToXterm(RESET_AFTER_BYTE_GAP, true)
|
||||
@@ -7328,6 +7380,12 @@ export function connectPanePty(
|
||||
foreground: true,
|
||||
beforeWrite: beforeTerminalOutputWrite
|
||||
})
|
||||
const ptyId = transport.getPtyId()
|
||||
if (ptyId !== null) {
|
||||
// The banner covers every byte main has reported dropped so far; markers
|
||||
// repeating that gap are answered by this record, not by a second banner.
|
||||
hiddenOutputRestoreDisclosedGap = { ptyId, seq: hiddenOutputRestoreMarkerSeqHighWater }
|
||||
}
|
||||
}
|
||||
|
||||
async function applyMainBufferSnapshot(snapshot: {
|
||||
@@ -7701,6 +7759,7 @@ export function connectPanePty(
|
||||
}
|
||||
// Why: everything at/before snapshot.seq is now painted; chunks still draining from main's ACK backlog below it are duplicates to suppress.
|
||||
setRestoredSnapshotBaseline(currentPtyId, snapshot)
|
||||
ackHiddenOutputRestoreApplied(currentPtyId)
|
||||
hiddenOutputRestoreReplayingSnapshot = null
|
||||
const needsFreshSnapshot = hiddenOutputRestoreFreshSnapshotNeeded
|
||||
hiddenOutputRestoreFreshSnapshotNeeded = false
|
||||
@@ -8151,6 +8210,9 @@ export function connectPanePty(
|
||||
recordTerminalOutput(pane.terminal)
|
||||
await waitForTerminalReplayWritesParsed(pane.terminal)
|
||||
if (isCurrent()) {
|
||||
// This repaint is a restore too: it painted main's retained bytes,
|
||||
// so it owes the same latch retirement the recovery loop sends.
|
||||
ackHiddenOutputRestoreApplied(ptyId)
|
||||
manager.rebuildPaneWebgl(pane.id)
|
||||
}
|
||||
},
|
||||
|
||||
@@ -3337,6 +3337,8 @@ function createPtyApi(): NonNullable<Partial<PreloadApi>['pty']> {
|
||||
clearBuffer: () => {},
|
||||
kill: () => Promise.resolve(),
|
||||
ackColdRestore: () => {},
|
||||
// Web panes never transit main's hidden-delivery gate, so there is no drop latch to retire.
|
||||
ackHiddenOutputRestoreApplied: () => {},
|
||||
ackData: () => {},
|
||||
onDeliveryResyncRequest: () => noopUnsubscribe,
|
||||
respondDeliveryResync: () => {},
|
||||
|
||||
Reference in New Issue
Block a user