From 12c6e6ed7da169a72943c6a8828aeaa0d5b802ee Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:41:37 -0700 Subject: [PATCH] fix(terminal): release an abandoned synchronized-output latch on restore and reveal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Alt-screen agent TUIs bracket every repaint in `?2026h … ?2026l`. A pane that stops presenting mid-bracket leaves xterm's `decPrivateModes.synchronizedOutput` latched, and `RenderService.refreshRows` checks that latch before rendering — so every repaint Orca owns renders zero rows while the xterm buffer is perfectly correct: the forced render-pause repaint, the plain `refresh()` fallback, and the shared glyph-atlas rebuild alike. xterm's 1s watchdog does not bound this. It is armed only inside `bufferRows`, and `refreshRows` returns at its `_isPaused` check first, so a pane that is not presenting holds the latch with no timer behind it. Resizing the window is the repair users find by hand: SIGWINCH makes the TUI repaint, and that repaint's closing `?2026l` clears the latch. This restores the release helper from #10907, which #11338 reverted as part of a batch whose stated reason ("terminal rendering regressions (flashing, lost content)") attaches to the #10871 hunk; no defect was ever attributed to the synchronized-output release, and it was never relanded. STA-2694 is still marked Done with its fix absent from the tree. Two changes versus #10907: - Drop the plain-refocus call site. #10907 also released from `schedulePaneRevealPresent`, but a refocus never hid its panes, so a latch there is a LIVE frame mid-flight and clearing it presents a half-drawn frame. Agent TUIs write these brackets many times a second, so that path could tear a frame on ordinary window refocus — a plausible source of the flashing the revert batch was chasing. Release only on the reveal repaint. - Add the post-replay restore boundary. A restored-open floating workspace never crosses hide -> reveal (its terminals mount visible from birth), so the reveal release never runs for it. Live bytes are still deferred at that point, so a latch there can only have come from the replay and cannot belong to a frame the TUI is mid-way through. Tests are mutation-verified: neutralizing the release fails 4 of the 8. --- .../terminal-pane/pty-connection.ts | 7 ++ .../lib/pane-manager/pane-reveal-repaint.ts | 5 + ...reveal-repaint-synchronized-output.test.ts | 83 ++++++++++++++++ ...rminal-synchronized-output-release.test.ts | 94 +++++++++++++++++++ .../terminal-synchronized-output-release.ts | 83 ++++++++++++++++ 5 files changed, 272 insertions(+) create mode 100644 src/renderer/src/lib/pane-manager/reveal-repaint-synchronized-output.test.ts create mode 100644 src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.test.ts create mode 100644 src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 1010bb579d8..3afbe2a40ef 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -71,6 +71,7 @@ import { type SafeFitContinuationHandle } from '@/lib/pane-manager/pane-tree-ops' import { requestStablePaneFit } from '@/lib/pane-manager/pane-fit-resize-observer' +import { releaseAbandonedSynchronizedOutput } from '@/lib/pane-manager/terminal-synchronized-output-release' import { bindPanePtyId, getFitOverrideForPty, @@ -8485,6 +8486,12 @@ export function connectPanePty( if (!isCurrentReattachPayload()) { return } + // Why here: a restored-open pane never crosses hide→reveal, so the + // reveal repaint's release never runs for it. Live bytes are still + // deferred at this point, so a latch can only have come from the + // replay — releasing it cannot tear a frame the TUI is mid-way + // through. Without this the fit below repaints zero rows. + releaseAbandonedSynchronizedOutput(pane.terminal) reattachPayloadApplied = true } } diff --git a/src/renderer/src/lib/pane-manager/pane-reveal-repaint.ts b/src/renderer/src/lib/pane-manager/pane-reveal-repaint.ts index bfc4797aa52..b3bec464410 100644 --- a/src/renderer/src/lib/pane-manager/pane-reveal-repaint.ts +++ b/src/renderer/src/lib/pane-manager/pane-reveal-repaint.ts @@ -1,5 +1,6 @@ import type { ManagedPaneInternal } from './pane-manager-types' import { reattachWebglIfNeeded } from './pane-webgl-reattach' +import { releaseAbandonedSynchronizedOutput } from './terminal-synchronized-output-release' import { resetAndRefreshAllTerminalWebglAtlases } from './pane-manager-registry' type PaneGetter = () => Iterable @@ -54,6 +55,10 @@ function flushPaneRevealRepaints(): void { for (const pane of livePanes) { try { reattachWebglIfNeeded(pane) + // Why before the reset+refresh below: a TUI hidden mid-`?2026h` leaves + // xterm's synchronized-output latch set, and RenderService buffers every + // refresh while it holds — so the repaint would render zero rows. + releaseAbandonedSynchronizedOutput(pane.terminal) } catch { /* ignore — one pane's teardown must not block global recovery */ } diff --git a/src/renderer/src/lib/pane-manager/reveal-repaint-synchronized-output.test.ts b/src/renderer/src/lib/pane-manager/reveal-repaint-synchronized-output.test.ts new file mode 100644 index 00000000000..3056ba7c3f9 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/reveal-repaint-synchronized-output.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' +import { schedulePaneRevealRepaint, schedulePaneRevealPresent } from './pane-reveal-repaint' +import type { ManagedPaneInternal } from './pane-manager-types' + +vi.mock('./pane-webgl-reattach', () => ({ + reattachWebglIfNeeded: vi.fn() +})) +vi.mock('./pane-manager-registry', () => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn() +})) + +/** + * Why these tests exist: xterm's `RenderService.refreshRows` returns at its + * `_isPaused` check BEFORE it reaches `bufferRows`, and `bufferRows` is the only + * place the 1s synchronized-output watchdog is armed. So a pane hidden mid + * `?2026h` holds the latch with no timer behind it — indefinitely — and every + * repaint Orca owns renders zero rows against a perfectly correct buffer. + */ +function createPane(synchronizedOutput: boolean): ManagedPaneInternal { + const decPrivateModes = { synchronizedOutput } + return { + terminal: { + rows: 24, + refresh: vi.fn(), + _core: { + coreService: { decPrivateModes }, + _renderService: { _syncOutputHandler: { flush: vi.fn() } } + } + } + } as unknown as ManagedPaneInternal +} + +function modesOf(pane: ManagedPaneInternal): { synchronizedOutput: boolean } { + return ( + pane.terminal as unknown as { + _core: { coreService: { decPrivateModes: { synchronizedOutput: boolean } } } + } + )._core.coreService.decPrivateModes +} + +beforeEach(() => { + vi.useFakeTimers() + // Two rAFs, then the settled-frame callback runs. + vi.stubGlobal('requestAnimationFrame', (cb: () => void) => { + setTimeout(cb, 0) + return 1 + }) +}) + +afterEach(() => { + vi.unstubAllGlobals() + vi.useRealTimers() +}) + +describe('reveal repaint releases an abandoned synchronized-output latch', () => { + it('clears the latch on the reveal path so the repaint is not swallowed', async () => { + const pane = createPane(true) + schedulePaneRevealRepaint(() => [pane]) + await vi.runAllTimersAsync() + expect(modesOf(pane).synchronizedOutput).toBe(false) + }) + + it('leaves an unlatched pane untouched on the reveal path', async () => { + const pane = createPane(false) + schedulePaneRevealRepaint(() => [pane]) + await vi.runAllTimersAsync() + expect(modesOf(pane).synchronizedOutput).toBe(false) + }) + + /** + * The defect in the reverted #10907: it also released on the plain-refocus + * present path. A refocus never hid its panes, so a latch there is a LIVE + * frame mid-flight and clearing it presents a half-drawn frame. Agent TUIs + * write these brackets many times a second, so that path would tear a frame + * on ordinary window refocus. + */ + it('does NOT clear the latch on the plain-refocus present path', async () => { + const pane = createPane(true) + schedulePaneRevealPresent(() => [pane]) + await vi.runAllTimersAsync() + expect(modesOf(pane).synchronizedOutput).toBe(true) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.test.ts b/src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.test.ts new file mode 100644 index 00000000000..e04efbd4d1a --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it, vi } from 'vitest' +import { releaseAbandonedSynchronizedOutput } from './terminal-synchronized-output-release' + +function createTerminal(options: { + synchronizedOutput?: boolean | undefined + withoutCore?: boolean + withoutModes?: boolean + flush?: () => unknown +}): unknown { + if (options.withoutCore) { + return {} + } + return { + _core: { + coreService: options.withoutModes + ? {} + : { decPrivateModes: { synchronizedOutput: options.synchronizedOutput } }, + _renderService: { _syncOutputHandler: { flush: options.flush } } + } + } +} + +describe('releaseAbandonedSynchronizedOutput', () => { + it('clears a latched synchronized-output frame and flushes its buffered rows', () => { + const flush = vi.fn() + const terminal = createTerminal({ synchronizedOutput: true, flush }) + + expect(releaseAbandonedSynchronizedOutput(terminal)).toBe(true) + expect( + (terminal as { _core: { coreService: { decPrivateModes: { synchronizedOutput: boolean } } } }) + ._core.coreService.decPrivateModes.synchronizedOutput + ).toBe(false) + // The buffered range dies with the latch; the caller's own full refresh + // repaints those rows, and leaving it armed keeps xterm's 1s timer alive. + expect(flush).toHaveBeenCalledTimes(1) + }) + + it('leaves an unlatched terminal untouched', () => { + const flush = vi.fn() + expect( + releaseAbandonedSynchronizedOutput(createTerminal({ synchronizedOutput: false, flush })) + ).toBe(false) + expect( + releaseAbandonedSynchronizedOutput(createTerminal({ synchronizedOutput: undefined, flush })) + ).toBe(false) + expect(flush).not.toHaveBeenCalled() + }) + + it('degrades to a no-op when xterm internals are unavailable', () => { + expect(releaseAbandonedSynchronizedOutput(createTerminal({ withoutCore: true }))).toBe(false) + expect(releaseAbandonedSynchronizedOutput(createTerminal({ withoutModes: true }))).toBe(false) + expect(releaseAbandonedSynchronizedOutput(null)).toBe(false) + expect(releaseAbandonedSynchronizedOutput(undefined)).toBe(false) + }) + + it('still clears the latch when the buffered-row flush throws', () => { + const terminal = createTerminal({ + synchronizedOutput: true, + flush: () => { + throw new Error('disposed mid-frame') + } + }) + + expect(releaseAbandonedSynchronizedOutput(terminal)).toBe(true) + expect( + (terminal as { _core: { coreService: { decPrivateModes: { synchronizedOutput: boolean } } } }) + ._core.coreService.decPrivateModes.synchronizedOutput + ).toBe(false) + }) + + it('unblocks the repaint that xterm would otherwise swallow', () => { + // Mirrors RenderService.refreshRows' gate order: the synchronizedOutput + // check precedes rendering, so a latched frame turns every reveal repaint + // into a no-op — the STA-2694 failure. + const modes = { synchronizedOutput: true } + let rendered = 0 + const refreshRows = (): void => { + if (modes.synchronizedOutput) { + return + } + rendered += 1 + } + const terminal = { + _core: { coreService: { decPrivateModes: modes }, _renderService: {} } + } + + refreshRows() + expect(rendered, 'a latched frame swallows the repaint').toBe(0) + + releaseAbandonedSynchronizedOutput(terminal) + refreshRows() + expect(rendered, 'releasing the latch lets the repaint through').toBe(1) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.ts b/src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.ts new file mode 100644 index 00000000000..08141cf9fcc --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.ts @@ -0,0 +1,83 @@ +/** + * Releases a DEC 2026 (synchronized output) frame that a TUI left open across a + * hide, so reveal repaints are not silently swallowed. + * + * Why: alt-screen agent TUIs (OpenCode/OpenTUI, Codex, grok) bracket every + * repaint in `?2026h … ?2026l`. If the pane is hidden mid-bracket — the exact + * moment a worktree switch or cold-park lands, since the fixture writes these + * brackets many times a second — xterm keeps `decPrivateModes.synchronizedOutput` + * latched. RenderService.refreshRows then buffers instead of rendering, and it + * checks that BEFORE the sync flag, so on reveal every repaint Orca owns is a + * no-op: the forced render-pause repaint, the plain `refresh()` fallback, and + * the shared glyph-atlas rebuild all render zero rows. The canvas keeps + * compositing pre-hide pixels while xterm's buffer is perfectly correct. + * + * xterm's 1s safety timeout does NOT bound this: it is armed only inside + * `bufferRows`, and `refreshRows` returns at its `_isPaused` check first — so + * while a pane is occluded nothing reaches `bufferRows` and no timer is ever + * pending. A pane hidden mid-frame holds the latch with no watchdog behind it, + * indefinitely. (terminal-reveal-draw-command-probe.spec.ts asserts exactly + * this: latch set, `_timeout === undefined`, and a repaint drawing 0 instances.) + * Resizing the window "fixes" it because SIGWINCH makes the TUI repaint, and + * that repaint's closing `?2026l` clears the latch — exactly the workaround + * users report (STA-2694). + * + * Reveal is a safe place to drop the latch: a frame opened before the pane was + * hidden can no longer be completed atomically, and its content is already in + * the buffer. Clearing it costs at most one extra repaint of a frame the TUI + * was mid-way through, versus a terminal that stays visibly corrupted. + * + * CALL-SITE RULE — only where the pane was demonstrably not presenting: + * the reveal repaint (the pane was hidden) and the post-replay restore boundary + * (live bytes are still deferred there, so a latch can only have come from the + * replay). Never on the plain-refocus present path: a refocus never hid its + * panes, so a latch there is a LIVE frame mid-flight, and releasing it presents + * a half-drawn frame. Agent TUIs write these brackets many times a second, so + * that path would tear a frame on ordinary window refocus. + * + * All access is behind typeof guards: an xterm upgrade that renames these + * internals degrades to a no-op rather than throwing into a render frame. + */ + +type MaybeSynchronizedOutputModes = { + synchronizedOutput?: boolean +} + +type TerminalWithSynchronizedOutput = { + _core?: { + coreService?: { decPrivateModes?: MaybeSynchronizedOutputModes } + _renderService?: { _syncOutputHandler?: { flush?: () => unknown } } + } +} + +function getDecPrivateModes(terminal: unknown): MaybeSynchronizedOutputModes | null { + const modes = (terminal as TerminalWithSynchronizedOutput | null)?._core?.coreService + ?.decPrivateModes + return modes && typeof modes === 'object' ? modes : null +} + +/** + * Clears a synchronized-output latch left open across a hide. + * + * Returns true when a latch was actually released, so callers can treat it as + * "this pane needs a repaint now" — the buffered rows are dropped with the + * latch, and the caller's own full refresh is what repaints them. + */ +export function releaseAbandonedSynchronizedOutput(terminal: unknown): boolean { + const modes = getDecPrivateModes(terminal) + if (modes?.synchronizedOutput !== true) { + return false + } + modes.synchronizedOutput = false + try { + // Why: drop the handler's buffered row range with the latch. Leaving it + // armed would let a later flush widen an unrelated refresh to rows this + // pane is about to repaint in full anyway, and keeps its 1s timer alive. + const handler = (terminal as TerminalWithSynchronizedOutput)._core?._renderService + ?._syncOutputHandler + handler?.flush?.() + } catch { + // The latch is already cleared; a failed flush must not block the repaint. + } + return true +}