diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index d5f2f84dd6a..3bc2700abb0 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -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') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 0408f68eded..7b8f4e852be 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -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 | 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)