From dfa68048671413cee34faa454b35a0811bfe1de3 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 27 Jul 2026 03:13:24 -0700 Subject: [PATCH] docs(terminal): rewrite the STA-2694 handoff after the refutation Records that the garble window is unbounded (the 1s watchdog never arms for an occluded pane), that the diff-based-staleness hypothesis was refuted by measurement and reverted, why pixel oracles are structurally blind here, and the two leads now closed by measurement (dimension staleness, lazy atlas bindings). Refs STA-2694. Co-authored-by: Orca --- ...694-alt-screen-reveal-artifacts-handoff.md | 322 ++++++++---------- 1 file changed, 135 insertions(+), 187 deletions(-) diff --git a/docs/reference/plans/2026-07-27-sta-2694-alt-screen-reveal-artifacts-handoff.md b/docs/reference/plans/2026-07-27-sta-2694-alt-screen-reveal-artifacts-handoff.md index 0621ae1a444..5b68ea02916 100644 --- a/docs/reference/plans/2026-07-27-sta-2694-alt-screen-reveal-artifacts-handoff.md +++ b/docs/reference/plans/2026-07-27-sta-2694-alt-screen-reveal-artifacts-handoff.md @@ -1,8 +1,7 @@ # STA-2694 — garbled terminal after switching away from an AI workspace Handoff snapshot: 2026-07-27, branch -`neil/sta-2694-fix-ui-rendering-artifacts-when-switching-away-from-ai`, -2 commits on top of `a1a78da878`. +`neil/sta-2694-fix-ui-rendering-artifacts-when-switching-away-from-ai`. ## The report @@ -15,251 +14,200 @@ with Claude Code and probably grok. ## Status -**Two real defects found and fixed. Neither is proven to be the whole report.** +**One defect, root-caused and fixed, with a mechanism-level test that fails +without the fix.** The garble window is *unbounded*, not the ~1s I originally +claimed — see [the correction](#correction-the-window-is-unbounded). -Both fixes are mechanism-level: each is a place where Orca's reveal repaint -provably renders zero cells, verified by reading xterm's source and pinned by -unit tests. Neither is confirmed against the field symptom, because **no -automated oracle in this repo can observe a stale canvas** (see -[Why the e2e suite can't close this](#why-the-e2e-suite-cant-close-this)). +A second hypothesis was implemented, then **refuted by measurement and +reverted**. That refutation is the other half of the value here: it rules out a +whole class of "just force a fuller repaint" fixes, and it is pinned by a test so +it cannot be reintroduced. | | | |---|---| -| Fixed + committed | `ed1eaf55f1`, `0f7ec4458d` | -| Unit tests | 12 across 3 files, all passing | -| e2e | 10 headless passing (4.8m) + 1 headful desktop-hide passing (22.6s) | -| Typecheck / lint | `typecheck:web` exit 0; oxlint clean on changed files | -| Confirmed against the field symptom | **No** — needs the in-app sentinel on real hardware | +| Fix | `ed1eaf55f1` — release an abandoned synchronized-output frame on reveal | +| Refuted + reverted | `0f7ec4458d`, reverted in `8d5eacecb4` | +| Oracle + scope correction | `3ccffc17ec` | +| Unit tests | 8 passing across 2 files | +| e2e | 5 draw-command oracle + 10 headless convergence + 1 headful | +| Teeth-verified | Yes — removing the fix fails the stranded-latch test | -## Defect 1 — a latched synchronized-output frame swallows every repaint - -Commit `ed1eaf55f1`, `src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.ts`. +## The defect Alt-screen agent TUIs (OpenCode/OpenTUI, Codex, grok) bracket every repaint in -DEC 2026 synchronized output — `\x1b[?2026h … \x1b[?2026l` — many times a -second. Hiding a pane mid-bracket (a worktree switch or cold park lands there -routinely) leaves xterm's `decPrivateModes.synchronizedOutput` latched `true`. +DEC 2026 synchronized output — `\x1b[?2026h … \x1b[?2026l` — many times a second. +Hide a pane mid-bracket (a worktree switch, a cold park, or an OS-level occlusion +lands there routinely) and xterm keeps +`decPrivateModes.synchronizedOutput` latched `true`. `RenderService.refreshRows` checks that latch **before** rendering -(`node_modules/@xterm/xterm/src/browser/services/RenderService.ts:156`), so -while it holds, every repaint Orca owns is a no-op: +(`RenderService.ts:162`), so while it holds, 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 against a perfectly correct buffer. -- the forced render-pause repaint (`forceRepaintThroughRenderPause`), -- the plain `terminal.refresh()` fallback, -- the shared glyph-atlas rebuild. +The fix (`terminal-synchronized-output-release.ts`) clears the latch and flushes +the handler's buffered row range at both reveal repaint entry points, before the +repaint. Both reveal paths reach it: -All three return having rendered zero rows, while the xterm buffer is perfectly -correct — which is exactly the shape of the report. +- `schedulePaneRevealPresent` (plain refocus / desktop return) +- `resetWebglTextureAtlas` (worktree switch, cold park, tab reveal — also reached + from `resetAndRefreshAllTerminalWebglAtlases` via the light tab-resume path) -The fix clears the latch and flushes `SynchronizedOutputHandler`'s buffered rows -at both reveal repaint entry points, before the repaint. +### Correction: the window is unbounded -**Scope limit:** xterm arms a 1s watchdog that clears this latch on its own, and -that watchdog only re-arms on the next `bufferRows`. So this closes a bounded -window, not the indefinite garble in the report. +`ed1eaf55f1`'s commit message says this "closes a bounded window rather than the +whole STA-2694 report", because xterm arms a 1s watchdog that clears the latch. +**That was wrong.** The watchdog is armed only inside +`SynchronizedOutputHandler.bufferRows`, and `refreshRows` returns at its +`_isPaused` check *first*: -## Defect 2 — the plain-refocus path did a diff-based refresh with a populated model +```ts +public refreshRows(start, end, sync = false, isRedrawOnly = false): void { + if (this._isPaused) { this._needsFullRefresh = true; return } // ← occluded pane stops here + if (this._coreService.decPrivateModes.synchronizedOutput) { + this._syncOutputHandler.bufferRows(start, end) // ← only place the watchdog arms + return + } + ... +``` -Commit `0f7ec4458d`, `src/renderer/src/lib/pane-manager/terminal-render-model-clear.ts`. +While a pane is occluded its `IntersectionObserver` sets `_isPaused`, so nothing +ever reaches `bufferRows` and **no timer is ever pending**. A pane hidden +mid-frame holds the latch with no watchdog behind it, indefinitely — which is the +indefinite garble the report describes. `terminal-reveal-draw-command-probe.spec.ts` +asserts exactly this: latch set, `_timeout === undefined`, and a repaint drawing +0 instances. -`schedulePaneRevealPresent` — the atlas-preserving path — only called -`terminal.refresh()`. xterm's renderers are **diff-based**: -`WebglRenderer._updateModel` early-continues on any cell whose code/fg/bg/ext -still match the cached model ("Nothing has changed, no updates needed"). For a -pane whose buffer never changed while hidden, a refresh therefore repaints -**nothing**. +This also explains the specific workaround: resizing the window makes the TUI +repaint via SIGWINCH, and that repaint's closing `?2026l` clears the latch. -When an occluded window loses its canvas contents while that model stays -populated, the refresh skips exactly the cells that went stale, and the pane -keeps compositing pre-hide pixels — until a window resize reallocates the model. -That is the repair the reporter found by hand. +## The refuted hypothesis (do not reimplement) -The fix clears the model first: `RenderService.clear()` → renderer `clear()` → -`_clearModel(true)`. That drops cached cells and glyph vertices but **not** the -texture atlas — a deliberate distinction, since the atlas is a module-global -shared by every same-config terminal (`CharAtlasCache.ts`) and wiping it -mid-stream re-arms xterm's page-merge garble race (xterm.js #4480). That race is -the whole reason this path is atlas-preserving. +The idea: xterm's renderers are diff-based, so a reveal `refresh()` skips cells +whose model entry still matches the buffer, leaving an occluded canvas showing +pre-hide pixels until a resize rebuilds the model. It is a *plausible* reading — +`WebglRenderer._updateModel` really does early-continue per unchanged cell. -Also covers the DOM-renderer fallback in `resetWebglTextureAtlas`: -`clearTextureAtlas()` was what invalidated the model on the WebGL path, so a -pane without an addon had nothing invalidate it and hit the identical skip. +It is wrong, measured on a live pane: -### Why this path is the one the reporter hits - -The literal action is "switch away to the desktop" — OS-level occlusion, not -in-app navigation. Trigger mapping in -`use-terminal-window-wake-recovery.ts`: - -| Trigger | `clearGlyphAtlases` | Reveal path | +| Refresh | `updateCell` calls | instances drawn | |---|---|---| -| `window 'focus'` | `false` | `scheduleRevealPresent()` — **defect 2** | -| `document 'visibilitychange'` (visible) | `true` | `scheduleRevealRepaint()` | -| `window.api.ui.onSystemResumed` | `true` | `scheduleRevealRepaint()` | +| diff-skipped (model matches buffer) | 0 | 562 | +| model cleared first | 561 | 562 | -And `App.tsx:1437-1453` (STA-2383) records that **on macOS occlusion-uncover -fires only `focus`** — `visibilitychange` never comes, which is why that code -relays a synthetic reveal to main. So the desktop-switch case lands squarely on -`scheduleRevealPresent()`, the path that until this commit did a no-op refresh. +`GlyphRenderer.render` copies vertices for **every** row up to `lineLengths[y]` +into the active buffer and issues **one** full-viewport `drawElementsInstanced`. +The diff only decides how much *vertex data* is rewritten, never how much is +*drawn*. The DOM renderer likewise `replaceChildren()`s every row +unconditionally. So clearing the model cannot change what reaches the screen. -## Call graph (for orientation) +Worse, it has a cost: `_clearModel(true)` zeroes every glyph vertex, and +`RenderService.clear()` fires no repaint of its own — so any paint landing between +the clear and the repopulating refresh draws an **empty** viewport (0 instances, +also asserted in the spec). On a reveal that refresh is debounced through a RAF +and can itself be swallowed, which would turn a merely-stale pane into a blank +one. Reverted in `8d5eacecb4`. -``` -window 'focus' / 'visibilitychange' / onSystemResumed - └─ use-terminal-window-wake-recovery.ts coalesces: 1 immediate + 1 settled RAF pass - └─ recoverVisibleTerminalWindowWake() terminal-visibility-resume.ts - ├─ clearGlyphAtlases → resetAndRefreshAllTerminalWebglAtlases() + scheduleRevealRepaint() - └─ !clearGlyphAtlases → scheduleRevealPresent() - └─ PaneManager.scheduleRevealPresent() pane-manager.ts:344-355 - └─ schedulePaneRevealPresent() pane-reveal-repaint.ts - ├─ reattachWebglIfNeeded() - ├─ releaseAbandonedSynchronizedOutput() ← fix 1 - ├─ clearTerminalRenderModel() ← fix 2 - └─ terminal.refresh(0, rows-1) -``` +The first test in the oracle spec pins the refutation so this cannot come back as +a "fix". -Tab/worktree reveal enters the same file via `resumeTerminalVisibility`: -`shouldUseLightTabResume` → `scheduleTabRevealWebglAtlasRecovery()` + -`scheduleRevealRepaint()`; the heavy path returns a post-paint `run()` that -drains backlog, resets all atlases, then `scheduleRevealRepaint()`. +## Why pixel oracles failed, and what replaced them -Both schedules run their work on a **double-RAF settled frame**, because the -WebGL renderer silently drops redraw requests until the pane is attached and -measured. +The field defect is "the buffer is correct but the compositor shows pre-hide +pixels". Two pixel oracles were built and **both were proven blind** by injecting +that exact defect (freeze `RenderService.refreshRows`, then write new content): -## Why the e2e suite can't close this - -`tests/e2e/terminal-opencode-altscreen-reveal-artifacts.spec.ts` (777 lines, 5 -tests) plus `tests/e2e/fixtures/opencode-altscreen-live-fixture.cjs` (an -OpenTUI-shaped alt-screen TUI with `?2026h`-bracketed frames and -`ORCA_FREEZE_NOW` / `ORCA_FREEZE_MID_FRAME` stdin sentinels). - -**These tests do not detect a stale/garbled canvas.** Two oracles were built for -it and both were *proven blind* by injecting the exact defect — freeze -`RenderService.refreshRows`, then write new content, so the buffer advances while -the canvas cannot: - -1. **Canvas-vs-buffer ink sampling** (the sentinel's method). `drawImage` on a - non-`preserveDrawingBuffer` WebGL canvas hands back a *re-rendered* copy, so - it reported 0 missing cells against 5263 cells of text the canvas had never - drawn. Removed. +1. **Canvas-vs-buffer ink sampling.** `drawImage` on a + non-`preserveDrawingBuffer` WebGL canvas hands back a *re-rendered* copy — it + reported 0 missing cells against 5263 cells of text the canvas had never + drawn. 2. **Screenshot vs. a forced repaint.** Playwright's screenshot drives a fresh compositor frame, which *heals* the stale paint before capture; and the - "repair" calls the same repaint code the reveal already ran, so a defect - shared by both shots cancels out. Documented as a known limitation in the - spec header. + "repair" ran the same code the reveal already ran, so a shared defect cancels + out. -An earlier resize-referenced oracle was also unsound and was discarded: -resizing shifts alt-screen rows, so its 4.4% pixel diff was measuring legitimate -reflow, not the defect. +An earlier resize-referenced oracle was also unsound: resizing shifts alt-screen +rows, so its 4.4% pixel diff measured legitimate reflow. -So green here means "no buffer/geometry regression", not "the canvas painted -correctly". Headless macOS with `hasWebgl: true` never reproduced the field -failure at all. What the suite *does* guard: the buffer converges to the live -frame, the pane stays on the alternate buffer, and xterm's grid, the fit -proposal, and the PTY-applied size all agree without a resize — across worktree -switch, cold park, idle agent, and (headful) desktop hide. +**Pixels are the wrong layer** — anything that reads them can trigger the repaint +that hides the bug. `tests/e2e/terminal-reveal-draw-command-probe.spec.ts` +instead wraps `GlyphRenderer.updateCell` and `gl.drawElementsInstanced` on the +live pane and counts draw commands. A draw command cannot be healed after the +fact, so "did the reveal repaint?" is directly observable — and falsifiable, +which is how the second hypothesis got refuted. -Test list: +Use that spec for **paint** questions; +`terminal-opencode-altscreen-reveal-artifacts.spec.ts` for **buffer, geometry and +PTY-size convergence** (worktree switch, cold park, idle agent, headful desktop +hide). The idle-agent case matters: every earlier test kept the TUI streaming +across the reveal, so live frames repainted whatever the reveal got wrong and the +defect healed itself before any assertion ran. -- `worktree switch away and back paints correctly without a manual resize` -- `parked tab reveal paints correctly without a manual resize` -- `parked reveal of an IDLE alt-screen agent paints without a manual repair` -- `@headful desktop switch away and back paints an idle agent without a manual repair` -- `reveal repaints a pane hidden inside an unclosed synchronized-output frame` +## Leads closed by measurement -The idle-agent case matters: every earlier test kept the TUI streaming across the -reveal, so live frames repainted whatever the reveal got wrong and the defect -healed itself before any assertion ran. A real OpenCode session sits waiting for -input. +Both were live suspects in the previous handoff; both are now ruled out. -## How to actually confirm a canvas-level fix +- **Dimension staleness.** Every dimension-rebuilding path in `WebglRenderer` is + behind an unchanged/zero-sized early exit that a hidden pane trips, and + `performSafeFit` early-returns at unchanged geometry — so nothing on the reveal + path re-runs `handleResize()`. Measured across a real `window.hide()` / + `show()`: canvas backing store `1272×1104` matched the renderer's computed + dimensions exactly, char metrics stayed `8.65×16`, `_isAttached` stayed true, + the screen element stayed connected. No staleness. +- **Lagging atlas page bindings.** Right after a reveal the bound texture version + *does* lag the atlas page (e.g. 1297 vs 1366) — but bindings are lazy and + `GlyphRenderer.render` rebinds any page whose version moved, so one repaint + brings them level (1366/1366). Normal, not a defect. Judge bindings only + *after* a draw. -The in-app render-desync sentinel -(`src/renderer/src/components/terminal-pane/terminal-render-desync-sentinel.ts`) -samples the compositor-presented canvas *before* any forced redraw can heal it, -and persists the corrupt pixels + buffer to disk. It must run on real hardware — -headless has never tripped it. +## Residual risk + +The oracle proves the repaint is *issued*; it does not photograph the user's +screen. If a report survives this fix, the remaining suspects are below the draw +call — GPU-process/compositor-level content loss — where the in-app sentinel on +real hardware is the tool: ```js localStorage.setItem('orca:render-desync-sentinel', '1') // then reload ``` -Then reproduce (OpenCode in a worktree, switch to the desktop, come back) and -**⌘-click** (Ctrl-click off Mac) inside the garbled pane. That starts a 10s -sampling burst at 250ms intervals; a trip requires the same cells missing across -2 consecutive samples, ≥200 text cells, ≥8% missing. On a confirmed trip it -writes `corrupt.png` + `corrupt.json` to -`/terminal-render-desync-evidence//`, then runs the -shared-atlas recovery and captures the healed frame for comparison. - -Armed automatically at import when the flag is set -(`terminal-freeze-breadcrumbs.ts:45`). - -## Open lead — dimension staleness (unverified) - -This is the only mechanism found that explains why **a resize specifically** is -the repair. It is a code-reading hypothesis; nothing has been written or tested -for it. - -Every dimension-rebuilding path in `WebglRenderer` is guarded by an -"unchanged / zero-sized" early exit that a hidden pane trips: - -| Guard | Location | Behavior when hidden | -|---|---|---| -| `performSafeFit` unchanged-geometry return | `pane-fit.ts:131-136` | reveal fit does **no repaint at all** | -| `handleDevicePixelRatioChange` | `WebglRenderer.ts:183` | no-ops unless `dpr` actually changed | -| `_updateDimensions` | `WebglRenderer.ts:649` | early-returns when char width/height is 0 | -| `_setCanvasDevicePixelDimensions` | `WebglRenderer.ts:701` | early-returns when dimensions unchanged | -| `observeDevicePixelDimensions` | `DevicePixelObserver.ts` | drops 0×0 entries ("canvas is likely hidden") | -| `CharSizeService._validateAndSet` | `CharSizeService.ts` | retains stale values when the element measures 0 | -| `WebglRenderer.renderRows` | `WebglRenderer.ts:354` | returns early when `!_isAttached` / screen element disconnected | - -Net effect: on return from occlusion, **nothing re-runs -`WebglRenderer.handleResize()`** — the only routine that resets -`_canvas.width/height`, re-runs `_refreshCharAtlas()`, calls `_clearModel(false)`, -and fires a *synchronous* full redraw. A manual window resize *does* change -geometry, so it reaches `fitAddon.fit()` → `terminal.resize()` → -`handleResize()`, and heals it. - -Note that fix 2 already covers the model half of this (`_clearModel`) while -deliberately not touching the atlas. What it does *not* cover is a canvas whose -**backing store** went stale while hidden — if that is the real mechanism, the -fix would be re-driving the canvas dimensions (or `handleCharSizeChanged()`, -which routes to `handleResize`) on reveal, which needs care: `handleResize` -touches the shared atlas and fires a sync redraw. - -**Do not implement this without confirming it first** — preferably via the -sentinel, whose `rendererState` payload records the atlas/model versions and -canvas dimensions at the moment of the trip. +Reproduce, then **⌘-click** (Ctrl-click off Mac) in the garbled pane. That starts +a 10s burst at 250ms intervals; a trip needs the same cells missing across 2 +consecutive samples, ≥200 text cells, ≥8% missing. It writes `corrupt.png` + +`corrupt.json` to `/terminal-render-desync-evidence//`, then +runs atlas recovery and captures the healed frame. Armed at import when the flag +is set (`terminal-freeze-breadcrumbs.ts:45`). ## Verification commands ```bash npx vitest run --config config/vitest.config.ts src/renderer/src/lib/pane-manager/ npx tsc --noEmit -p config/tsconfig.tc.web.json -npx playwright test tests/e2e/terminal-opencode-altscreen-reveal-artifacts.spec.ts \ +npx playwright test tests/e2e/terminal-reveal-draw-command-probe.spec.ts \ + tests/e2e/terminal-opencode-altscreen-reveal-artifacts.spec.ts \ tests/e2e/terminal-inline-tui-reveal-convergence.spec.ts \ --config tests/playwright.config.ts --project=electron-headless --workers=1 SKIP_BUILD=1 npx playwright test tests/e2e/terminal-opencode-altscreen-reveal-artifacts.spec.ts \ --config tests/playwright.config.ts --project=electron-headful --workers=1 ``` -Last run: 58 pane-manager unit files / 610 tests passing (1 skipped); -`typecheck:web` exit 0; 10 headless e2e passing in 4.8m; 1 headful in 22.6s. +To confirm the fix still has teeth, delete the +`releaseAbandonedSynchronizedOutput(pane.terminal)` line from +`schedulePaneRevealPresent` and re-run the oracle: the stranded-latch test fails +with "the reveal repaint did not release the stranded latch, so the pane stays +frozen". ## Files Production: -- `src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.ts` (new, fix 1) -- `src/renderer/src/lib/pane-manager/terminal-render-model-clear.ts` (new, fix 2) -- `src/renderer/src/lib/pane-manager/pane-reveal-repaint.ts` (both fixes wired into `schedulePaneRevealPresent`) -- `src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts` (`resetWebglTextureAtlas`: fix 1 + DOM-renderer fallback) +- `src/renderer/src/lib/pane-manager/terminal-synchronized-output-release.ts` (the fix) +- `src/renderer/src/lib/pane-manager/pane-reveal-repaint.ts` (`schedulePaneRevealPresent`) +- `src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts` (`resetWebglTextureAtlas`) Tests: -- `terminal-synchronized-output-release.test.ts`, `reveal-repaint-synchronized-output.test.ts`, - `terminal-render-model-clear.test.ts` (12 tests) +- `terminal-synchronized-output-release.test.ts`, `reveal-repaint-synchronized-output.test.ts` +- `tests/e2e/terminal-reveal-draw-command-probe.spec.ts` (the oracle) - `tests/e2e/terminal-opencode-altscreen-reveal-artifacts.spec.ts`, `tests/e2e/fixtures/opencode-altscreen-live-fixture.cjs`