fix(terminal): stop flood-abandon from duplicating replay-covered lines

The 750ms foreground deadline can abandon a hidden-output restore while
a RESOLVED snapshot's replay is still parsing. Those replay writes are
already queued in xterm's FIFO and WILL paint everything at or before
the snapshot seq, but the abandon (a) rewrote its parked live chunks
raw — duplicating every line the replay covers (field: TUI lines
stacked twice on reveal) — and (b) skipped baseline adoption, so main's
ACK-backlog re-deliveries below the seq duplicated again.

Track the in-flight replay snapshot from the moment its writes are
queued; on abandon, seq-slice the pending write-through against it
(raw fallback only for unmappable offsets — availability still wins)
and adopt it as the live reconcile baseline with the drain's continuity
advance. Abandons with no started replay keep the existing raw path.

Deterministic repro: pty-connection.test.ts stages snapshot-resolve →
replay parse held → deadline abandon, asserting no rewrite of covered
chunks, dedup of post-abandon backlog, and write-through of new bytes.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
2026-07-17 03:28:10 -04:00
co-authored by Orca
parent 778fae26b8
commit cd76d0797e
2 changed files with 152 additions and 1 deletions
@@ -11498,6 +11498,113 @@ describe('connectPanePty', () => {
disposable.dispose()
})
it('slices abandoned pending chunks against a replay that already painted', async () => {
// Field bug: the 750ms foreground deadline can fire while a RESOLVED
// snapshot's replay is still parsing (its writes are already in xterm's
// FIFO and WILL paint). The abandon used to rewrite parked chunks raw,
// duplicating every line the replay covers, and left the live reconcile
// baseline stale so main's ACK backlog duplicated again.
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-id')
const capturedDataCallback: {
current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null
} = { current: null }
transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
capturedDataCallback.current = callbacks.onData ?? null
return 'pty-id'
})
transportFactoryQueue.push(transport)
const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType<
typeof vi.fn
>
const snapshot = createDeferred<{
data: string
cols: number
rows: number
seq: number
pendingDeliveryStartSeq: number
}>()
getMainBufferSnapshot.mockReturnValue(snapshot.promise)
const hidden = 'hidden-codex-output\r\n'
const coveredLive = 'LIVE_DUP_LINE\r\n'
const afterAbandon = 'AFTER_ABANDON\r\n'
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps({
isVisibleRef: { current: false },
startup: { command: 'codex' }
})
const disposable = connectPanePty(pane as never, manager as never, deps as never)
await flushAsyncTicks(6)
expect(capturedDataCallback.current).not.toBeNull()
vi.useFakeTimers()
capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length })
;(deps.isVisibleRef as { current: boolean }).current = true
// Parked while the snapshot fetch is in flight; also arms the deadline.
capturedDataCallback.current?.(coveredLive, {
seq: hidden.length + coveredLive.length,
rawLength: coveredLive.length
})
await flushAsyncTicks(4)
// The snapshot resolves covering the parked chunk, but its replay writes
// never finish parsing (writes below hold their callbacks), so the
// deadline fires mid-replay.
const heldCallbacks: (() => void)[] = []
pane.terminal.write.mockImplementation(function write(
data: string,
callback?: () => void
): void {
if (callback) {
heldCallbacks.push(callback)
}
void data
})
snapshot.resolve({
data: 'SNAP_STATE\r\n',
cols: 100,
rows: 30,
seq: hidden.length + coveredLive.length,
pendingDeliveryStartSeq: 0
})
await flushAsyncTicks(6)
vi.advanceTimersByTime(750)
await flushAsyncTicks(10)
// The replay covers the parked chunk — the abandon must not rewrite it.
const writtenAfterAbandon = pane.terminal.write.mock.calls.map(([data]) => data as string)
expect(writtenAfterAbandon.join('')).not.toContain('LIVE_DUP_LINE')
// The abandoned replay still paints up to its seq, so it must become the
// live reconcile baseline: a backlog re-delivery at or below it is a
// duplicate...
pane.terminal.write.mockClear()
capturedDataCallback.current?.(coveredLive, {
seq: hidden.length + coveredLive.length,
rawLength: coveredLive.length
})
await flushAsyncTicks(4)
expect(pane.terminal.write.mock.calls.map(([data]) => data as string).join('')).not.toContain(
'LIVE_DUP_LINE'
)
// ...while genuinely-new bytes past the baseline must still write through.
capturedDataCallback.current?.(afterAbandon, {
seq: hidden.length + coveredLive.length + afterAbandon.length,
rawLength: afterAbandon.length
})
await flushAsyncTicks(4)
expect(pane.terminal.write.mock.calls.map(([data]) => data as string).join('')).toContain(
'AFTER_ABANDON'
)
heldCallbacks.forEach((callback) => callback())
disposable.dispose()
})
it('abandons a stalled hidden restore and drains pending foreground chunks warning-first', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-id')
@@ -5182,6 +5182,14 @@ export function connectPanePty(
let hiddenOutputRestoreGeneration = 0
// Flood-backpressure suppression (HIDDEN_OUTPUT_RESTORE_FLOOD_SUPPRESS_MS).
let hiddenOutputRestoreFloodSuppressedUntil = 0
// Why: once a restore snapshot's replay writes are queued, they WILL paint
// (xterm FIFO) even if the restore is abandoned mid-parse. Abandon must
// slice its pending write-through against this snapshot or every line the
// replay covers renders twice.
let hiddenOutputRestoreReplayingSnapshot: {
seq?: number
pendingDeliveryStartSeq?: number
} | null = null
let hiddenOutputRestoreFloodRepaintTimer: ReturnType<typeof setTimeout> | null = null
// Why: after a snapshot restore, main can still drain ACK-backlog chunks
// whose bytes the snapshot already covers — writing them unguarded
@@ -6278,6 +6286,8 @@ export function connectPanePty(
? []
: hiddenOutputRestorePendingChunks.slice()
const hadPendingOverflow = hiddenOutputRestorePendingOverflow
const replayingSnapshot = hiddenOutputRestoreReplayingSnapshot
hiddenOutputRestoreReplayingSnapshot = null
hiddenOutputRestoreGeneration += 1
if (
hiddenOutputSnapshotScrollRestore?.valid &&
@@ -6313,7 +6323,30 @@ export function connectPanePty(
if (hadPendingOverflow) {
return
}
const pendingData = pendingChunks.map((chunk) => chunk.data).join('')
// Why: an abandoned-but-started replay is already queued in xterm's FIFO
// and will paint everything at or before its seq. Rewriting covered
// chunks raw duplicated TUI lines (field: same line stacked twice on
// reveal); slice against the replay and fall back raw only when offsets
// cannot be mapped — availability still wins over a dropped tail.
const replayedSeq = typeof replayingSnapshot?.seq === 'number' ? replayingSnapshot.seq : null
let pendingData = ''
for (const chunk of pendingChunks) {
const sliced =
replayedSeq === null ? chunk.data : getChunkDataAfterSnapshot(chunk, replayedSeq)
pendingData += sliced ?? chunk.data
}
if (replayingSnapshot && replayedSeq !== null) {
// Why: adopt the painting replay as the live reconcile baseline so
// main's ACK backlog at or below it dedups instead of duplicating.
setRestoredSnapshotBaseline(expectedPtyId, replayingSnapshot)
for (const chunk of pendingChunks) {
// Mirror the drain's continuity: written chunks advance the point
// so later live chunks are neither re-dropped nor misread as gaps.
if (typeof chunk.seq === 'number' && restoredSnapshotExpectedStartSeq !== null) {
restoredSnapshotExpectedStartSeq = Math.max(restoredSnapshotExpectedStartSeq, chunk.seq)
}
}
}
if (pendingData) {
writePtyOutputToXterm(pendingData, true)
}
@@ -6358,6 +6391,7 @@ export function connectPanePty(
resetHiddenRendererRiskState()
hiddenOutputRestoreNeeded = false
hiddenOutputRestorePtyId = null
hiddenOutputRestoreReplayingSnapshot = null
hiddenOutputRestoreGeneration += 1
}
@@ -6449,6 +6483,7 @@ export function connectPanePty(
cols: number
rows: number
seq?: number
pendingDeliveryStartSeq?: number
alternateScreen?: boolean
scrollbackAnsi?: string
pendingEscapeTailAnsi?: string
@@ -6484,6 +6519,14 @@ export function connectPanePty(
return
}
scrollRestore.started = true
if (typeof snapshot.seq === 'number') {
hiddenOutputRestoreReplayingSnapshot = {
seq: snapshot.seq,
...(typeof snapshot.pendingDeliveryStartSeq === 'number'
? { pendingDeliveryStartSeq: snapshot.pendingDeliveryStartSeq }
: {})
}
}
discardTerminalOutput(pane.terminal)
if (
hasSnapshotDimensions &&
@@ -6750,6 +6793,7 @@ export function connectPanePty(
// still draining from main's ACK backlog below that point are
// duplicates the dataCallback reconciliation must suppress.
setRestoredSnapshotBaseline(currentPtyId, snapshot)
hiddenOutputRestoreReplayingSnapshot = null
const needsFreshSnapshot = hiddenOutputRestoreFreshSnapshotNeeded
hiddenOutputRestoreFreshSnapshotNeeded = false
const drainOutcome = drainPendingLiveChunksAfterSnapshot(snapshot.seq)