From d64ccc71bd7daf7ff64fe182b9bf522b90293397 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 8 Aug 2026 02:54:16 -0700 Subject: [PATCH] fix(terminal): break the ConPTY foreground livelock that froze a repainting TUI (#12463) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `holdForeground` and `coalesceForeground` each cancelled the other's fallback timer on the Windows ConPTY DEC 2026 (synchronized output) path. A continuously repainting TUI such as Codex therefore left the foreground latch stuck open, so every later chunk was held instead of coalesced and output never reached the visible pane until the tab was refreshed. Break the mutual cancellation and mirror the hidden path's scan on the foreground path, carrying a marker tail so a ConPTY-split DEC 2026 marker is still detected. Nothing was wrong with Flutter — it was simply a long-running command behind a repainting TUI. Fixes #8754 Co-authored-by: Orca --- .../terminal-pane/pty-connection.test.ts | 45 ++++++++++ .../terminal-pane/pty-connection.ts | 86 +++++++++++++------ .../pane-terminal-output-scheduler.test.ts | 44 ++++++++++ .../pane-terminal-output-scheduler.ts | 77 ++++++++++++----- 4 files changed, 204 insertions(+), 48 deletions(-) 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 bcd4066bf0f..5e2dc95dcfc 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -17978,6 +17978,51 @@ describe('connectPanePty', () => { } }) + it('clears the synchronized latch when ConPTY splits the frame end marker', async () => { + // Why: issue #8754 — a split \x1b[?2026l left the foreground latch armed, so every later + // chunk was held as frame body and the visible pane froze until the tab was blurred. + const restoreNavigator = temporarilySetNavigatorUserAgent( + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' + ) + try { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + } + ) + transportFactoryQueue.push(transport) + + const pane = createPane(1) + connectPanePty(pane as never, createManager(1) as never, createDeps() as never) + await flushAsyncTicks(6) + + vi.useFakeTimers() + const repaintBody = 'codex spinner '.repeat(200) + capturedDataCallback.current?.(`\x1b[?2026h${repaintBody}`) + vi.advanceTimersByTime(300) + pane.terminal.write.mockClear() + + // ConPTY splits the closing marker across two chunks. + capturedDataCallback.current?.(`${repaintBody}\x1b[?25l\x1b[13;14H\x1b[?25h\x1b[?202`) + capturedDataCallback.current?.('6l') + vi.advanceTimersByTime(1100) + expect(pane.terminal.write).toHaveBeenCalled() + pane.terminal.write.mockClear() + + // The frame is closed, so ordinary output must paint instead of being held as frame body. + capturedDataCallback.current?.('command finished\r\n') + vi.advanceTimersByTime(20) + expect(pane.terminal.write).toHaveBeenCalled() + } finally { + vi.useRealTimers() + restoreNavigator() + } + }) + it('does not leak the interactive latch across a same-chunk close+open to a stale frame', async () => { // Why: a same-chunk close+open re-evaluates the new frame from its own open time so it can't inherit the prior frame's fast path. const restoreNavigator = temporarilySetNavigatorUserAgent( diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 9ac30d4f9f6..405c8d73939 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -938,21 +938,56 @@ function shouldWritePtyOutputForeground(isPaneVisible: boolean): boolean { return isDocumentVisibilityProvenStale() } -function containsSynchronizedOutputStart(data: string): boolean { - return data.includes(SYNCHRONIZED_OUTPUT_START_SEQUENCE) +type SynchronizedForegroundScan = { + started: boolean + ended: boolean + active: boolean + markerTail: string } -function containsSynchronizedOutputEnd(data: string): boolean { - return data.includes(SYNCHRONIZED_OUTPUT_END_SEQUENCE) -} +// Why the carried tail: ConPTY can split \x1b[?2026l across chunks; scanning the raw +// chunk alone left the foreground DEC 2026 latch stuck open so every later chunk was +// held instead of coalesced, freezing the visible pane (#8754). Mirrors the hidden path. +function scanSynchronizedForegroundOutput( + data: string, + markerTail: string, + wasActive: boolean +): SynchronizedForegroundScan { + const scanData = markerTail ? `${markerTail}${data}` : data + const currentChunkStartIndex = scanData.length - data.length + let active = wasActive + let started = false + let ended = false + let offset = 0 -function shouldSynchronizedOutputRemainActive(data: string, wasActive: boolean): boolean { - const lastStartIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_START_SEQUENCE) - const lastEndIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE) - if (lastStartIndex === -1 && lastEndIndex === -1) { - return wasActive + while (offset < scanData.length) { + const startIndex = scanData.indexOf(SYNCHRONIZED_OUTPUT_START_SEQUENCE, offset) + const endIndex = scanData.indexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE, offset) + if (startIndex === -1 && endIndex === -1) { + break + } + if (endIndex !== -1 && (startIndex === -1 || endIndex < startIndex)) { + active = false + if (endIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length > currentChunkStartIndex) { + ended = true + } + offset = endIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length + continue + } + active = true + if (startIndex + SYNCHRONIZED_OUTPUT_START_SEQUENCE.length > currentChunkStartIndex) { + started = true + } + offset = startIndex + SYNCHRONIZED_OUTPUT_START_SEQUENCE.length + } + + return { + started, + ended, + active, + // Why length-1: a full marker can never hide in the tail, so no marker is counted twice. + markerTail: scanData.slice(-SYNCHRONIZED_OUTPUT_MARKER_TAIL_CHARS) } - return lastStartIndex > lastEndIndex } function containsCursorPositionSequence(data: string): boolean { @@ -1108,6 +1143,8 @@ export function connectPanePty( let alternateScreenBackgroundRepaintTimer: ReturnType | null = null let shiftEnterReconfirmTimer: ReturnType | null = null let synchronizedForegroundOutputActive = false + // Why: carries up to one marker-length-1 of trailing bytes so a ConPTY-split DEC 2026 marker is still detected (#8754). + let synchronizedForegroundMarkerTail = '' // Why: tracks the keystroke proximity captured when the current synchronized // foreground frame opened, so a split end marker that lands after the redraw // window still drains on the fast path instead of the 1s coalesce fallback. @@ -6425,22 +6462,20 @@ export function connectPanePty( canUseHiddenOutputSnapshot(transport.getPtyId()) && shouldSnapshotHiddenCodexOutput && (opts?.hiddenStartupRendererQuery === true || containsHiddenStartupRendererQuery(data)) - const synchronizedOutputStarted = - shouldProtectNativeWindowsSynchronizedOutput && - foreground && - containsSynchronizedOutputStart(data) - const synchronizedOutputEnded = - shouldProtectNativeWindowsSynchronizedOutput && - foreground && - containsSynchronizedOutputEnd(data) + const synchronizedForegroundScan = + shouldProtectNativeWindowsSynchronizedOutput && foreground + ? scanSynchronizedForegroundOutput( + data, + synchronizedForegroundMarkerTail, + synchronizedForegroundOutputActive + ) + : null + const synchronizedOutputStarted = synchronizedForegroundScan?.started === true + const synchronizedOutputEnded = synchronizedForegroundScan?.ended === true const synchronizedForegroundOutput = - shouldProtectNativeWindowsSynchronizedOutput && - foreground && + synchronizedForegroundScan !== null && (synchronizedForegroundOutputActive || synchronizedOutputStarted || synchronizedOutputEnded) - const nextSynchronizedForegroundOutputActive = - shouldProtectNativeWindowsSynchronizedOutput && - foreground && - shouldSynchronizedOutputRemainActive(data, synchronizedForegroundOutputActive) + const nextSynchronizedForegroundOutputActive = synchronizedForegroundScan?.active === true // Why: xterm's DOM renderer draws the cursor as row content, so Windows cursor-only restores need row invalidation even outside DEC 2026. const nativeWindowsCursorRestore = shouldProtectNativeWindowsSynchronizedOutput && foreground && containsCursorRestore(data) @@ -6484,6 +6519,7 @@ export function connectPanePty( const synchronizedFrameLatencySensitive = synchronizedForegroundOutput && synchronizedForegroundFrameInteractive synchronizedForegroundOutputActive = nextSynchronizedForegroundOutputActive + synchronizedForegroundMarkerTail = synchronizedForegroundScan?.markerTail ?? '' writeTerminalOutput(pane.terminal, data, { foreground: foregroundOutput, beforeWrite: beforeTerminalOutputWrite, diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts index aaefe89f872..c3f0c4df330 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts @@ -1038,6 +1038,50 @@ describe('pane terminal output scheduler', () => { expect(terminal.write).toHaveBeenCalledWith('\x1b[?2026h\x1b[?25lpartial', expect.any(Function)) }) + // Why: issue #8754 — ConPTY splits Codex spinner frames into an open chunk and a + // close chunk; hold and coalesce each cancelled the other's fallback timer, so a + // visible pane never repainted until the tab was blurred. + it('keeps repainting when synchronized frames alternate hold and coalesce chunks', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const terminal = createTerminal() + + const writeFrameOpen = (frame: number): void => { + writeTerminalOutput(terminal, `\x1b[?2026h\x1b[?25l\x1b[10;5HWorking ${frame}`, { + foreground: true, + forceForegroundRefresh: true, + stripTransientCursorShows: true, + holdForeground: true + }) + } + // Codex shows the cursor before the end marker, so this never hits the immediate-drain escape. + const writeFrameClose = (): void => { + writeTerminalOutput(terminal, '\x1b[10;8H\x1b[?25h\x1b[?2026l', { + foreground: true, + forceForegroundRefresh: true, + stripTransientCursorShows: true, + coalesceForeground: true + }) + } + + writeFrameOpen(0) + vi.advanceTimersByTime(100) + writeFrameClose() + vi.advanceTimersByTime(100) + writeFrameOpen(1) + vi.advanceTimersByTime(60) + expect(terminal.write).toHaveBeenCalledTimes(1) + + for (let frame = 2; frame < 8; frame += 1) { + writeFrameClose() + vi.advanceTimersByTime(100) + writeFrameOpen(frame) + vi.advanceTimersByTime(100) + } + + expect(terminal.write.mock.calls.length).toBeGreaterThanOrEqual(4) + }) + it('safety-flushes latency-sensitive synchronized holds without a visible input delay', async () => { vi.useFakeTimers() const { writeTerminalOutput } = await loadScheduler() diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts index 4272d4c0913..8acb768e635 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -85,6 +85,10 @@ type QueueEntry = { foregroundCoalesceDelayMs: number foregroundHoldSafetyTimer: ReturnType | null foregroundCoalesceTimer: ReturnType | null + // Why: hold and coalesce cancel each other's fallback timer, so an alternating DEC 2026 stream could re-arm both forever and freeze a visible pane (#8754). This caps one non-drainable episode. + foregroundReleaseDeadlineAt: number | null + // Why: an open frame's own hold chunks may still push the deadline out, but once coalesce has taken the entry over the deadline stops moving so the two mechanisms can't re-arm each other. + foregroundReleaseDeadlineFixed: boolean } const BACKGROUND_FLUSH_DELAY_MS = 50 @@ -323,10 +327,40 @@ function createQueueEntry( foregroundCoalesce: false, foregroundCoalesceDelayMs: FOREGROUND_COALESCE_DELAY_MS, foregroundHoldSafetyTimer: null, - foregroundCoalesceTimer: null + foregroundCoalesceTimer: null, + foregroundReleaseDeadlineAt: null, + foregroundReleaseDeadlineFixed: false } } +// Returns the delay the caller's timer must use so it never outlives the episode deadline. +function armForegroundReleaseDeadline( + entry: QueueEntry, + delayMs: number, + mayExtend: boolean +): number { + const now = getDrainNow() + const requested = now + delayMs + entry.foregroundReleaseDeadlineAt = + entry.foregroundReleaseDeadlineAt === null || + (mayExtend && !entry.foregroundReleaseDeadlineFixed) + ? requested + : Math.min(entry.foregroundReleaseDeadlineAt, requested) + return Math.max(0, entry.foregroundReleaseDeadlineAt - now) +} + +// Why: reopen the gate only once the entry is drainable again, so the next synchronized frame gets a full budget. +function resetForegroundReleaseGate(entry: QueueEntry): void { + entry.foregroundReleaseDeadlineAt = null + entry.foregroundReleaseDeadlineFixed = false +} + +function clearForegroundRelease(entry: QueueEntry): void { + clearForegroundHoldSafety(entry) + clearForegroundCoalesce(entry) + resetForegroundReleaseGate(entry) +} + function clearForegroundHoldSafety(entry: QueueEntry): void { if (entry.foregroundHoldSafetyTimer === null) { return @@ -347,14 +381,16 @@ function clearForegroundCoalesce(entry: QueueEntry): void { function scheduleForegroundHoldSafety(entry: QueueEntry): void { clearForegroundHoldSafety(entry) + const delayMs = armForegroundReleaseDeadline(entry, entry.foregroundHoldSafetyDelayMs, true) entry.foregroundHoldSafetyTimer = setTimeout(() => { entry.foregroundHoldSafetyTimer = null entry.foregroundHold = false clearForegroundCoalesce(entry) + resetForegroundReleaseGate(entry) if (queuedByTerminal.has(entry.terminal)) { scheduleDrain(0) } - }, entry.foregroundHoldSafetyDelayMs) + }, delayMs) } function scheduleForegroundCoalesceRelease( @@ -370,13 +406,17 @@ function scheduleForegroundCoalesceRelease( entry.foregroundCoalesceTimer = null } entry.foregroundCoalesce = true + // Why fixed from here: a later hold chunk must clamp to this deadline instead of restarting the pair's mutual re-arm (#8754). + entry.foregroundReleaseDeadlineFixed = true + const delayMs = armForegroundReleaseDeadline(entry, entry.foregroundCoalesceDelayMs, false) entry.foregroundCoalesceTimer = setTimeout(() => { entry.foregroundCoalesceTimer = null entry.foregroundCoalesce = false + resetForegroundReleaseGate(entry) if (queuedByTerminal.has(entry.terminal)) { scheduleDrain(0) } - }, entry.foregroundCoalesceDelayMs) + }, delayMs) } function isEntryDrainable(entry: QueueEntry): boolean { @@ -708,8 +748,7 @@ function discardDetachedQueueEntry(entry: QueueEntry): void { entry.chunkIndex = 0 entry.queuedChars = 0 entry.highPriority = false - clearForegroundHoldSafety(entry) - clearForegroundCoalesce(entry) + clearForegroundRelease(entry) } function queueCapExceeded(entry: QueueEntry): boolean { @@ -760,7 +799,7 @@ function replaceBacklogWithWarning( if (debugEnabled && shouldNotify) { debugState.droppedBacklogCount++ } - clearForegroundCoalesce(entry) + clearForegroundRelease(entry) recordQueueDebugPressure() if (shouldNotify) { entry.onBackgroundBacklogDropped?.() @@ -945,8 +984,7 @@ function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background' | null entry.chunks.length = 0 entry.chunkIndex = 0 entry.queuedChars = 0 - clearForegroundHoldSafety(entry) - clearForegroundCoalesce(entry) + clearForegroundRelease(entry) recordQueueDebugPressure() return null } @@ -958,8 +996,7 @@ function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background' | null entry.chunks.length = 0 entry.chunkIndex = 0 entry.queuedChars = 0 - clearForegroundHoldSafety(entry) - clearForegroundCoalesce(entry) + clearForegroundRelease(entry) recordQueueDebugPressure() return null } @@ -1003,8 +1040,7 @@ function drainQueuedOutput(): void { queuedByTerminal.set(entry.terminal, entry) } else { entry.highPriority = false - clearForegroundCoalesce(entry) - clearForegroundHoldSafety(entry) + clearForegroundRelease(entry) } // Why: xterm parsing and DOM work share the renderer thread with input; keep draining cooperative so WSL/agent output can't pin the UI. if (writes > 0 && getDrainNow() - startedAt >= DRAIN_TIME_BUDGET_MS) { @@ -1103,7 +1139,7 @@ export function writeTerminalOutput( shouldShortenCoalesceForLatencySensitiveForeground && !coalescedQueuedDataNeedsCursorRestore(queued) if (containsDrainableCursorRestore(data) || shouldDrainForLatencySensitiveForeground) { - clearForegroundCoalesce(queued) + clearForegroundRelease(queued) scheduleDrain(0) return } @@ -1114,8 +1150,7 @@ export function writeTerminalOutput( return } queued.foregroundHold = false - clearForegroundCoalesce(queued) - clearForegroundHoldSafety(queued) + clearForegroundRelease(queued) scheduleDrain(0) return } @@ -1256,8 +1291,7 @@ export function flushTerminalOutput( entry.chunkIndex = 0 entry.queuedChars = 0 entry.highPriority = false - clearForegroundHoldSafety(entry) - clearForegroundCoalesce(entry) + clearForegroundRelease(entry) recordQueueDebugPressure() return } @@ -1302,8 +1336,7 @@ export function flushTerminalOutput( ) if (!writeAccepted) { fireQueuedAckCredits(entry) - clearForegroundHoldSafety(entry) - clearForegroundCoalesce(entry) + clearForegroundRelease(entry) recordQueueDebugPressure() return } @@ -1312,8 +1345,7 @@ export function flushTerminalOutput( cancelTerminalWriteStallWatch(terminal) ackCreditsParsed?.() fireQueuedAckCredits(entry) - clearForegroundHoldSafety(entry) - clearForegroundCoalesce(entry) + clearForegroundRelease(entry) recordQueueDebugPressure() return } @@ -1328,8 +1360,7 @@ export function flushTerminalOutput( scheduleDrain(0) } else { entry.highPriority = false - clearForegroundCoalesce(entry) - clearForegroundHoldSafety(entry) + clearForegroundRelease(entry) } recordQueueDebugPressure() }