fix: hold pty batches during reveal snapshots (#2707)

This commit is contained in:
Neil 2026-05-23 13:19:38 -07:00 committed by GitHub
parent 1f1171e0de
commit f286702c62
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 223 additions and 50 deletions

View File

@ -3381,24 +3381,23 @@ describe('registerPtyHandlers', () => {
)
})
it('flushes pending renderer PTY batches around headless serialization', async () => {
it('discards pending renderer PTY batches when the headless snapshot includes them', async () => {
vi.useFakeTimers()
try {
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
let resolveSnapshot!: (snapshot: { data: string; cols: number; rows: number }) => void
const runtime = {
setPtyController: vi.fn(),
onPtySpawned: vi.fn(),
onPtyData: vi.fn(),
onPtyExit: vi.fn(),
preAllocateHandleForPty: vi.fn(),
serializeHeadlessTerminalBufferForRenderer: vi.fn(async () => {
serializeHeadlessTerminalBufferForRenderer: vi.fn(() => {
mockProc.emitData('during-serialize')
return {
data: 'headless',
cols: 100,
rows: 30
}
return new Promise<{ data: string; cols: number; rows: number }>((resolve) => {
resolveSnapshot = resolve
})
})
}
handlers.clear()
@ -3411,16 +3410,23 @@ describe('registerPtyHandlers', () => {
mainWindow.webContents.send.mockClear()
mockProc.emitData('before-serialize')
await handlers.get('pty:serializeHeadlessBuffer')!(null, { id: spawnResult.id })
const pending = handlers.get('pty:serializeHeadlessBuffer')!(null, { id: spawnResult.id })
vi.advanceTimersByTime(8)
expect(mainWindow.webContents.send).not.toHaveBeenCalledWith(
'pty:data',
expect.objectContaining({ id: spawnResult.id })
)
resolveSnapshot({
data: 'headless',
cols: 100,
rows: 30
})
await pending
expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', {
id: spawnResult.id,
data: 'before-serialize'
})
expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', {
id: spawnResult.id,
data: 'during-serialize'
})
expect(mainWindow.webContents.send).not.toHaveBeenCalledWith(
'pty:data',
expect.objectContaining({ id: spawnResult.id })
)
expect(runtime.serializeHeadlessTerminalBufferForRenderer).toHaveBeenCalledWith(
spawnResult.id,
{}
@ -3432,5 +3438,97 @@ describe('registerPtyHandlers', () => {
vi.useRealTimers()
}
})
it('releases pending renderer PTY batches when headless serialization cannot snapshot', async () => {
vi.useFakeTimers()
try {
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
let resolveSnapshot!: (snapshot: null) => void
const runtime = {
setPtyController: vi.fn(),
onPtySpawned: vi.fn(),
onPtyData: vi.fn(),
onPtyExit: vi.fn(),
preAllocateHandleForPty: vi.fn(),
serializeHeadlessTerminalBufferForRenderer: vi.fn(() => {
mockProc.emitData('during-serialize')
return new Promise<null>((resolve) => {
resolveSnapshot = resolve
})
})
}
handlers.clear()
registerPtyHandlers(mainWindow as never, runtime as never)
const spawnResult = (await handlers.get('pty:spawn')!(null, {
cols: 80,
rows: 24,
cwd: '/tmp'
})) as { id: string }
mainWindow.webContents.send.mockClear()
mockProc.emitData('before-serialize')
const pending = handlers.get('pty:serializeHeadlessBuffer')!(null, { id: spawnResult.id })
vi.advanceTimersByTime(8)
expect(mainWindow.webContents.send).not.toHaveBeenCalledWith(
'pty:data',
expect.objectContaining({ id: spawnResult.id })
)
resolveSnapshot(null)
await pending
expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', {
id: spawnResult.id,
data: 'before-serializeduring-serialize'
})
mainWindow.webContents.send.mockClear()
vi.advanceTimersByTime(8)
expect(mainWindow.webContents.send).not.toHaveBeenCalled()
} finally {
vi.useRealTimers()
}
})
it('releases pending renderer PTY batches when headless serialization throws', async () => {
vi.useFakeTimers()
try {
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
const runtime = {
setPtyController: vi.fn(),
onPtySpawned: vi.fn(),
onPtyData: vi.fn(),
onPtyExit: vi.fn(),
preAllocateHandleForPty: vi.fn(),
serializeHeadlessTerminalBufferForRenderer: vi.fn(async () => {
mockProc.emitData('during-serialize')
throw new Error('snapshot failed')
})
}
handlers.clear()
registerPtyHandlers(mainWindow as never, runtime as never)
const spawnResult = (await handlers.get('pty:spawn')!(null, {
cols: 80,
rows: 24,
cwd: '/tmp'
})) as { id: string }
mainWindow.webContents.send.mockClear()
mockProc.emitData('before-serialize')
await expect(
handlers.get('pty:serializeHeadlessBuffer')!(null, { id: spawnResult.id })
).rejects.toThrow('snapshot failed')
expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', {
id: spawnResult.id,
data: 'before-serializeduring-serialize'
})
mainWindow.webContents.send.mockClear()
vi.advanceTimersByTime(8)
expect(mainWindow.webContents.send).not.toHaveBeenCalled()
} finally {
vi.useRealTimers()
}
})
})
})

View File

@ -659,6 +659,7 @@ export function registerPtyHandlers(
// throughput. Keystroke echo/redraws bypass this below because agent TUIs
// already spend tens of ms producing their redraw.
const pendingData = new Map<string, string>()
const headlessSnapshotHeldPtyIds = new Set<string>()
const trustedTerminalHandleEnv = new Set<string>()
let flushTimer: ReturnType<typeof setTimeout> | null = null
const PTY_BATCH_INTERVAL_MS = 8
@ -667,6 +668,15 @@ export function registerPtyHandlers(
const INTERACTIVE_OUTPUT_WINDOW_MS = 100
const INTERACTIVE_OUTPUT_MAX_CHARS = 1024
const hasFlushablePendingData = (): boolean => {
for (const id of pendingData.keys()) {
if (!headlessSnapshotHeldPtyIds.has(id)) {
return true
}
}
return false
}
const flushPendingData = (): void => {
flushTimer = null
if (mainWindow.isDestroyed()) {
@ -674,29 +684,40 @@ export function registerPtyHandlers(
return
}
for (const [id, data] of pendingData) {
if (headlessSnapshotHeldPtyIds.has(id)) {
continue
}
mainWindow.webContents.send('pty:data', { id, data })
pendingData.delete(id)
}
if (hasFlushablePendingData()) {
flushTimer = setTimeout(flushPendingData, PTY_BATCH_INTERVAL_MS)
}
pendingData.clear()
}
const clearFlushTimerIfIdle = (): void => {
if (pendingData.size > 0 || flushTimer === null) {
if (hasFlushablePendingData() || flushTimer === null) {
return
}
clearTimeout(flushTimer)
flushTimer = null
}
const flushPendingDataForPty = (id: string): void => {
const data = pendingData.get(id)
if (!data) {
const sendPtyDataToRenderer = (id: string, data: string): void => {
if (!data || mainWindow.isDestroyed()) {
return
}
pendingData.delete(id)
if (!mainWindow.isDestroyed()) {
mainWindow.webContents.send('pty:data', { id, data })
mainWindow.webContents.send('pty:data', { id, data })
}
const takePendingDataForPty = (id: string): string => {
const data = pendingData.get(id)
if (!data) {
return ''
}
pendingData.delete(id)
clearFlushTimerIfIdle()
return data
}
// Why: extracted so the "Restart daemon" flow can rebind against the fresh
@ -737,7 +758,7 @@ export function registerPtyHandlers(
nextData.length <= INTERACTIVE_OUTPUT_MAX_CHARS &&
lastInputAt !== undefined &&
performance.now() - lastInputAt <= INTERACTIVE_OUTPUT_WINDOW_MS
if (isInteractiveOutput) {
if (isInteractiveOutput && !headlessSnapshotHeldPtyIds.has(payload.id)) {
pendingData.delete(payload.id)
clearFlushTimerIfIdle()
// Why: agent TUIs redraw small prompt regions after every keystroke.
@ -749,7 +770,7 @@ export function registerPtyHandlers(
return
}
pendingData.set(payload.id, nextData)
if (!flushTimer) {
if (!flushTimer && hasFlushablePendingData()) {
flushTimer = setTimeout(flushPendingData, PTY_BATCH_INTERVAL_MS)
}
})
@ -1848,12 +1869,25 @@ export function registerPtyHandlers(
opts.scrollbackRows = Math.floor(args.scrollbackRows)
}
// Why: hidden-pane reveal uses the headless snapshot as the authoritative
// paint. Flush any ≤8ms main-process PTY batch before and after the
// snapshot so renderer-side hydration can drop its duplicate fallback
// queue without losing bytes that already reached the headless model.
flushPendingDataForPty(args.id)
const snapshot = await runtime.serializeHeadlessTerminalBufferForRenderer(args.id, opts)
flushPendingDataForPty(args.id)
// paint. Hold any ≤8ms main-process PTY batches while serializing: a
// successful headless snapshot already contains them, while a null
// snapshot must release them so the renderer fallback can replay them.
const pendingBeforeSnapshot = takePendingDataForPty(args.id)
headlessSnapshotHeldPtyIds.add(args.id)
let snapshot: { data: string; cols: number; rows: number } | null
try {
snapshot = await runtime.serializeHeadlessTerminalBufferForRenderer(args.id, opts)
} catch (err) {
const pendingDuringSnapshot = takePendingDataForPty(args.id)
headlessSnapshotHeldPtyIds.delete(args.id)
sendPtyDataToRenderer(args.id, pendingBeforeSnapshot + pendingDuringSnapshot)
throw err
}
const pendingDuringSnapshot = takePendingDataForPty(args.id)
headlessSnapshotHeldPtyIds.delete(args.id)
if (!snapshot) {
sendPtyDataToRenderer(args.id, pendingBeforeSnapshot + pendingDuringSnapshot)
}
return snapshot
}
)

View File

@ -0,0 +1,27 @@
import { describe, expect, it, vi } from 'vitest'
import {
consumeHiddenTerminalHydration,
markHiddenTerminalFallbackReplayed,
queueHiddenTerminalOutput
} from './hidden-terminal-output-state'
function createTerminal() {
return {
write: vi.fn()
}
}
describe('hidden terminal output state', () => {
it('preserves reveal-time fallback output when trimming drops pre-hydration chunks', () => {
const terminal = createTerminal()
queueHiddenTerminalOutput(terminal, 'pty-1', 'before-reveal')
const hydration = consumeHiddenTerminalHydration(terminal)
expect(hydration).not.toBeNull()
const retainedDuringHydration = 'x'.repeat(512 * 1024)
queueHiddenTerminalOutput(terminal, 'pty-1', `dropped-prefix${retainedDuringHydration}`)
expect(markHiddenTerminalFallbackReplayed(terminal, hydration!)).toBe(retainedDuringHydration)
})
})

View File

@ -6,11 +6,17 @@ type TerminalOutputTarget = {
type HiddenTerminalState = {
ptyId: string
chunks: string[]
chunks: HiddenTerminalChunk[]
bytes: number
needsHydration: boolean
hydrating: boolean
hydrationToken: number
nextChunkSeq: number
}
type HiddenTerminalChunk = {
seq: number
data: string
}
type HiddenTerminalOutputDebugSnapshot = {
@ -31,7 +37,7 @@ export type HiddenTerminalHydration = {
ptyId: string
fallbackData: string
token: number
fallbackChunkCount: number
fallbackLastSeq: number
}
const MAX_FALLBACK_BYTES = 512 * 1024
@ -78,17 +84,17 @@ function trimFallback(state: HiddenTerminalState): void {
if (!dropped) {
continue
}
state.bytes -= dropped.length
state.bytes -= dropped.data.length
if (debugEnabled) {
debugState.droppedBytes += dropped.length
debugState.droppedBytes += dropped.data.length
}
}
if (state.bytes > MAX_FALLBACK_BYTES && state.chunks.length === 1) {
const chunk = state.chunks[0]
const keepFrom = Math.max(0, chunk.length - MAX_FALLBACK_BYTES)
const keepFrom = Math.max(0, chunk.data.length - MAX_FALLBACK_BYTES)
if (keepFrom > 0) {
state.chunks[0] = chunk.slice(keepFrom)
state.bytes = state.chunks[0].length
state.chunks[0] = { ...chunk, data: chunk.data.slice(keepFrom) }
state.bytes = state.chunks[0].data.length
if (debugEnabled) {
debugState.droppedBytes += keepFrom
}
@ -113,12 +119,13 @@ export function queueHiddenTerminalOutput(
bytes: 0,
needsHydration: false,
hydrating: false,
hydrationToken: 0
hydrationToken: 0,
nextChunkSeq: 1
}
hiddenStateByTerminal.set(terminal, state)
}
state.needsHydration = true
state.chunks.push(data)
state.chunks.push({ seq: state.nextChunkSeq++, data })
state.bytes += data.length
trimFallback(state)
if (debugEnabled) {
@ -143,23 +150,29 @@ export function consumeHiddenTerminalHydration(
}
return {
ptyId: state.ptyId,
fallbackData: state.chunks.join(''),
fallbackData: state.chunks.map((chunk) => chunk.data).join(''),
token: state.hydrationToken,
fallbackChunkCount: state.chunks.length
fallbackLastSeq: state.chunks.at(-1)?.seq ?? 0
}
}
function finishHydration(
terminal: TerminalOutputTarget,
token: number,
consumedChunkCount: number
fallbackLastSeq: number,
collectQueuedDuringHydration: boolean
): string {
exposeDebugApi()
const state = hiddenStateByTerminal.get(terminal)
if (!state || state.hydrationToken !== token) {
return ''
}
const queuedDuringHydration = state.chunks.slice(consumedChunkCount).join('')
const queuedDuringHydration = collectQueuedDuringHydration
? state.chunks
.filter((chunk) => chunk.seq > fallbackLastSeq)
.map((chunk) => chunk.data)
.join('')
: ''
state.chunks.length = 0
state.bytes = 0
state.needsHydration = false
@ -175,7 +188,8 @@ export function markHiddenTerminalFallbackReplayed(
const queuedDuringHydration = finishHydration(
terminal,
hydration.token,
hydration.fallbackChunkCount
hydration.fallbackLastSeq,
true
)
if (debugEnabled) {
debugState.fallbackReplayCount++
@ -187,7 +201,7 @@ export function markHiddenTerminalHydrated(
terminal: TerminalOutputTarget,
hydration: HiddenTerminalHydration
): string {
return finishHydration(terminal, hydration.token, hydration.fallbackChunkCount)
return finishHydration(terminal, hydration.token, hydration.fallbackLastSeq, false)
}
export function cancelHiddenTerminalHydration(

View File

@ -122,9 +122,9 @@ export function useTerminalPaneGlobalEffects({
// guard, so terminal query replies do not leak into the shell.
replayIntoTerminal(pane, replayingPanesRef, '\x1b[2J\x1b[3J\x1b[H')
replayIntoTerminal(pane, replayingPanesRef, snapshot.data)
// Why: the main serializer flushes pending PTY batches around the
// headless snapshot. Any renderer bytes queued during this window
// are already represented in the authoritative snapshot.
// Why: the main serializer holds pending PTY batches while taking
// the headless snapshot and discards them only after a successful
// snapshot, so this fallback queue would duplicate the replay.
markHiddenTerminalHydrated(pane.terminal, hydration)
return
}