From dc6f6bf685eca9008efd34e3113ebc2b7953346d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 14 Aug 2026 02:10:18 -0700 Subject: [PATCH] =?UTF-8?q?test(terminal):=20pin=20that=20panes=20are=20PA?= =?UTF-8?q?INTED,=20not=20merely=20bound=20=E2=80=94=20and=20fix=20a=20bro?= =?UTF-8?q?ken=20commit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems, both mine. 1) 0103a80b484 swept in an untracked fixture and left the branch failing typecheck (unused Terminal import in painted-pane-fixture.ts). Its canvas stub also threw 'clearRect is not a function' on every refresh. Fixed here. 2) direct-ssh-reconnect-repaint.test.ts, which I wrote to guard the reconnect repaint, is VACUOUS: it re-implements finalizeHydratedTerminalPanes inside the test and mocks the registry, so deleting the real fix from useIpcEvents leaves it green. direct-ssh-reconnect-repaint-wiring.test.ts replaces that guarantee by capturing the real callback the hook hands the coordinator and running it against live panes — deleting the two scheduling lines now reddens it. The gap this closes: content survival was already well covered at the BYTE layer (snapshot roundtrip, hide/reveal stitching, cold-restore scrollback), but every pane test stubbed terminal as {cols, rows, refresh: vi.fn()}, so 'repainted' only ever meant 'a spy fired'. No test ran a real xterm through a real PaneManager. pane-content-survival.test.ts does, reading .xterm-rows — what the user actually sees — across reconnect, restart-shaped restore, tab reveal, window show, split and unsplit, for plain shells and alt-screen TUIs. The alt-screen distinction is now pinned explicitly: forcing a resize inside fitAllPanes reddens only the TUI test, because a plain shell reflows and survives while a TUI frame does not. That asymmetry is why the reported bug looked like a plain-shell problem. 11 tests, each mutation-proven to redden only its own. 760 pane-manager tests green; the 2 failures here are the known environmental IME baseline. Flagged, not fixed: the unsplit path reparents the DOM without the dispose/ reattach that splitManagedPane does explicitly because 'DOM reparenting can silently invalidate a WebGL context without firing contextlost', and follows it with a safeFit that no-ops when the box is unchanged. Same shape as the reconnect bug. happy-dom has no WebGL, so only a real-GPU E2E can confirm it. --- ...irect-ssh-reconnect-repaint-wiring.test.ts | 102 +++++++ .../lib/pane-manager/painted-pane-fixture.ts | 25 +- .../pane-content-survival.test.ts | 272 ++++++++++++++++++ 3 files changed, 392 insertions(+), 7 deletions(-) create mode 100644 src/renderer/src/hooks/direct-ssh-reconnect-repaint-wiring.test.ts create mode 100644 src/renderer/src/lib/pane-manager/pane-content-survival.test.ts diff --git a/src/renderer/src/hooks/direct-ssh-reconnect-repaint-wiring.test.ts b/src/renderer/src/hooks/direct-ssh-reconnect-repaint-wiring.test.ts new file mode 100644 index 00000000000..6d8cfb5b9ef --- /dev/null +++ b/src/renderer/src/hooks/direct-ssh-reconnect-repaint-wiring.test.ts @@ -0,0 +1,102 @@ +// @vitest-environment happy-dom +/** + * The reconnect repaint is wired to the real registry, and it really repaints. + * + * direct-ssh-reconnect-repaint.test.ts asserts the same behavior against a hand-written copy + * of the finalize hook and a mocked registry: delete the scheduling from useIpcEvents and it + * stays green. This test takes the callback useIpcEvents actually hands the reconnect + * coordinator, runs it against a live PaneManager holding real xterm panes, and asserts the + * panes come back PAINTED — the reported symptom was blank terminals after a hosts-popup + * disconnect/reconnect, with the buffers intact the whole time. + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { DirectSshReconnectCoordinatorDeps } from './direct-ssh-reconnect-coordinator-types' +import { createHarnessStoreState, loadIpcEventsHarness } from './ipc-events-test-harness' +// Type-only: the fixture is imported dynamically below, after the module registry reset. +import type * as PaintedPaneFixtureModule from '@/lib/pane-manager/painted-pane-fixture' + +type PaintedPaneFixture = typeof PaintedPaneFixtureModule + +const capturedDeps: DirectSshReconnectCoordinatorDeps[] = [] + +vi.mock('./direct-ssh-reconnect-coordinator', () => ({ + createDirectSshReconnectCoordinator: (deps: DirectSshReconnectCoordinatorDeps) => { + capturedDeps.push(deps) + // Every coordinator method is a no-op here: this test exercises the callback useIpcEvents + // supplied, not the reconnect state machine (covered by the coordinator's own tests). + return new Proxy({} as Record, { get: () => vi.fn() }) + } +})) + +const AUTHORITY = { targetId: 'target-1', host: 'example.test', user: 'orca', port: 22 } + +let fixture: PaintedPaneFixture | null = null + +afterEach(() => { + fixture?.destroyPaneTabs() + fixture = null + capturedDeps.length = 0 + vi.unstubAllGlobals() + vi.restoreAllMocks() + document.body.replaceChildren() +}) + +/** + * Runs useIpcEvents against the shared preload stub and returns both the finalize callback it + * installed and the pane fixture. The fixture is imported AFTER the harness resets the module + * registry so its PaneManagers register into the same pane-manager-registry instance the hook + * just imported — two copies of that module-global registry would make this test pass on an + * empty set of managers. + */ +async function loadReconnectFinalize(): Promise<{ + finalize: DirectSshReconnectCoordinatorDeps['finalizeHydratedTerminalPanes'] + panes: PaintedPaneFixture +}> { + const harness = await loadIpcEventsHarness(createHarnessStoreState({ tabsByWorktree: {} })) + // The hook schedules its settled pass through window.setTimeout; the harness's window stub + // has no timers of its own. + const stubbedWindow = globalThis.window as unknown as { setTimeout?: typeof setTimeout } + stubbedWindow.setTimeout = ((handler: () => void, ms?: number) => + setTimeout(handler, ms)) as unknown as typeof setTimeout + harness.useIpcEvents() + const deps = capturedDeps.at(-1) + if (!deps) { + throw new Error('useIpcEvents did not create the direct SSH reconnect coordinator') + } + fixture = await import('@/lib/pane-manager/painted-pane-fixture') + fixture.stubTerminalTextMeasurement() + return { finalize: deps.finalizeHydratedTerminalPanes, panes: fixture } +} + +describe('the reconnect finalize hook useIpcEvents installs', () => { + // Deleting this test lets the two scheduling lines in useIpcEvents' finalizeHydratedTerminalPanes + // be removed with every other test still green — which is the state the branch was in when the + // blank-panes report came back: reattach restores the buffer, nothing repaints the frame. + it('repaints live panes in every tab, including a split and an alt-screen TUI', async () => { + const { finalize, panes } = await loadReconnectFinalize() + + const foreground = panes.createPaneTab() + const background = panes.createPaneTab() + const shell = foreground.manager.createInitialPane({ focus: false }) + const tui = foreground.manager.splitPane(shell.id, 'vertical') + const otherTab = background.manager.createInitialPane({ focus: false }) + if (!tui) { + throw new Error('expected the split to create a second pane') + } + await panes.writeToPane(shell, '$ ssh host output\r\n') + await panes.writeAlternateScreenFrame(tui, ['agent frame']) + await panes.writeToPane(otherTab, '$ second tab output\r\n') + await panes.settleFrames(30) + for (const pane of [shell, tui, otherTab]) { + panes.blankPaintedLayer(pane) + } + expect(panes.paintedText(shell)).toBe('') + + finalize(AUTHORITY as never) + await panes.settleFrames() + + expect(panes.paintedText(shell)).toContain('ssh host output') + expect(panes.paintedText(tui)).toContain('agent frame') + expect(panes.paintedText(otherTab)).toContain('second tab output') + }) +}) diff --git a/src/renderer/src/lib/pane-manager/painted-pane-fixture.ts b/src/renderer/src/lib/pane-manager/painted-pane-fixture.ts index 39413e13081..88d306d6383 100644 --- a/src/renderer/src/lib/pane-manager/painted-pane-fixture.ts +++ b/src/renderer/src/lib/pane-manager/painted-pane-fixture.ts @@ -10,16 +10,22 @@ * compositing nothing (stale/blank pixels) while the buffer is intact, which is exactly the * reported reconnect/reveal symptom; production repaint paths must bring it back. */ -import { Terminal } from '@xterm/xterm' import { vi } from 'vitest' import { PaneManager } from './pane-manager' import type { ManagedPane } from './pane-manager-types' -/** xterm's WidthCache needs a 2D context to open a terminal at all. */ +/** xterm's WidthCache needs a 2D context to open a terminal at all, and its decoration + * layer paints into another one on every refresh; happy-dom supplies neither. */ export function stubTerminalTextMeasurement(): void { - vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({ - measureText: () => ({ width: 10 }) - } as unknown as CanvasRenderingContext2D) + const context = new Proxy( + { measureText: () => ({ width: 10 }) } as Record, + { + get: (target, prop: string) => (prop in target ? target[prop] : () => undefined) + } + ) + vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue( + context as unknown as CanvasRenderingContext2D + ) } export type PaneTab = { @@ -66,9 +72,14 @@ function rowsElement(pane: ManagedPane): Element | null { return pane.terminal.element?.querySelector('.xterm-rows') ?? null } -/** What the user sees: the DOM renderer's presented rows. */ +/** What the user sees: the DOM renderer's presented rows. A pane detached from the document + * shows nothing no matter what its rows still hold, so report that as blank. */ export function paintedText(pane: ManagedPane): string { - return (rowsElement(pane)?.textContent ?? '').trim() + const rows = rowsElement(pane) + if (!rows?.isConnected) { + return '' + } + return (rows.textContent ?? '').trim() } /** Models a renderer presenting nothing while the buffer is untouched — the diff --git a/src/renderer/src/lib/pane-manager/pane-content-survival.test.ts b/src/renderer/src/lib/pane-manager/pane-content-survival.test.ts new file mode 100644 index 00000000000..ade5a83bb3b --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-content-survival.test.ts @@ -0,0 +1,272 @@ +// @vitest-environment happy-dom +/** + * Does the pane still SHOW its content after X? + * + * Every other pane test asserts a pane is bound to a pty, or that a repaint spy was called. + * A blank-but-attached pane passes all of them — and that is the bug users keep reporting: + * after an SSH reconnect, a restore, or a tab reveal the terminal comes back empty until a + * resize or a sidebar toggle forces a relayout. These tests run the real PaneManager over + * real xterm terminals and assert against the PAINTED rows plus the buffer, so "repainted" + * means the content is back on screen and unchanged, not that a mock was invoked. + * + * Axes covered here: reconnect/bulk-restore refit, tab reveal, window show, split, unsplit; + * single pane, split panes, more than one tab; plain shell scrollback and an agent TUI's + * alternate-screen frame. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + blankPaintedLayer, + bufferText, + createPaneTab, + destroyPaneTabs, + paintedText, + settleFrames, + stubTerminalTextMeasurement, + writeAlternateScreenFrame, + writeToPane +} from './painted-pane-fixture' +import { refitAndRefreshAllTerminalPanes } from './pane-manager-registry' + +const SHELL_SCROLLBACK = ['$ git status', 'nothing to commit', '$ '].join('\r\n') +const TUI_FRAME = ['╭─ claude ─────╮', '│ waiting… │', '╰──────────────╯'] + +beforeEach(() => { + stubTerminalTextMeasurement() +}) + +afterEach(() => { + destroyPaneTabs() + vi.restoreAllMocks() + document.body.replaceChildren() +}) + +describe('content survives an SSH reconnect / bulk restore refit', () => { + // Deleting this test lets PaneManager.refreshAllPanes stop repainting (or repaint a + // narrower row range) with no test noticing: the reported symptom is a plain shell whose + // scrollback is in the buffer but not on screen after reconnect. + it('repaints a single plain-shell pane from its buffer without touching the scrollback', async () => { + const { manager } = createPaneTab() + const pane = manager.createInitialPane({ focus: false }) + await writeToPane(pane, SHELL_SCROLLBACK) + await settleFrames(30) + const content = bufferText(pane) + expect(paintedText(pane)).toContain('nothing to commit') + + blankPaintedLayer(pane) + expect(paintedText(pane), 'the pane must actually be blank before the repaint').toBe('') + + refitAndRefreshAllTerminalPanes() + await settleFrames(30) + + expect(paintedText(pane)).toContain('nothing to commit') + expect(paintedText(pane)).toContain('$ git status') + expect(bufferText(pane), 'a repaint must not scroll, clear, or reflow the buffer').toBe(content) + }) + + // Deleting this test lets the repaint stop after the first pane of a split (the reported + // case had splits open); one blank half of a split is the same bug, half-visible. + it('repaints every pane of a split, not just the active one', async () => { + const { manager } = createPaneTab() + const first = manager.createInitialPane({ focus: false }) + const second = manager.splitPane(first.id, 'vertical') + if (!second) { + throw new Error('expected the split to create a second pane') + } + await writeToPane(first, 'left pane output') + await writeToPane(second, 'right pane output') + await settleFrames(30) + + blankPaintedLayer(first) + blankPaintedLayer(second) + refitAndRefreshAllTerminalPanes() + await settleFrames(30) + + expect(paintedText(first)).toContain('left pane output') + expect(paintedText(second)).toContain('right pane output') + }) + + // Deleting this test lets someone bound the reconnect repaint to visible managers the way + // resetAndRefreshAllTerminalWebglAtlases already is. A reconnect rebinds every tab's panes, + // so a background tab would then stay blank until it is revealed AND resized. + it('repaints background tabs too, not only the foreground one', async () => { + const foreground = createPaneTab() + const background = createPaneTab({ background: true }) + const visible = foreground.manager.createInitialPane({ focus: false }) + const hidden = background.manager.createInitialPane({ focus: false }) + await writeToPane(visible, 'foreground tab output') + await writeToPane(hidden, 'background tab output') + await settleFrames(30) + expect(background.manager.isVisibleForAtlasRecovery()).toBe(false) + + blankPaintedLayer(visible) + blankPaintedLayer(hidden) + refitAndRefreshAllTerminalPanes() + await settleFrames(30) + + expect(paintedText(visible)).toContain('foreground tab output') + expect(paintedText(hidden)).toContain('background tab output') + }) + + // Deleting this test removes the only coverage that the reconnect repaint restores an + // alternate-screen frame. A TUI repaints itself on resize and a plain shell does not, which + // is why plain shells showed the bug more visibly — but a reconnect resizes nothing, and a + // TUI whose far side is idle will never redraw itself. The repaint must not depend on a + // resize, and it must not lose the frame (alt-screen content has no scrollback to recover). + it('restores an agent TUI alternate-screen frame without resizing the terminal', async () => { + const { manager } = createPaneTab() + const pane = manager.createInitialPane({ focus: false }) + await writeToPane(pane, SHELL_SCROLLBACK) + await writeAlternateScreenFrame(pane, TUI_FRAME) + await settleFrames(30) + const grid = { cols: pane.terminal.cols, rows: pane.terminal.rows } + expect(pane.terminal.buffer.active.type).toBe('alternate') + + blankPaintedLayer(pane) + refitAndRefreshAllTerminalPanes() + await settleFrames(30) + + expect(paintedText(pane)).toContain('waiting…') + expect({ cols: pane.terminal.cols, rows: pane.terminal.rows }).toEqual(grid) + expect(pane.terminal.buffer.active.type, 'the TUI must stay on its own screen').toBe( + 'alternate' + ) + }) + + // Deleting this test drops the Orca-restart shape: panes are rehydrated from a snapshot + // while their tab is still background/rendering-suspended, and the restore's own writes are + // the only paint they ever get. If the settled refit skips them the relaunched app opens on + // a blank terminal. + it('paints a background tab hydrated by a restore while its rendering was suspended', async () => { + const tab = createPaneTab({ background: true }) + const pane = tab.manager.createInitialPane({ focus: false }) + tab.manager.suspendRendering() + await writeToPane(pane, '$ restored from snapshot\r\n') + // Why settle first: xterm's own render is rAF-debounced, so blanking before it lands + // would let the restore's pending paint — not the code under test — repaint the pane. + await settleFrames(30) + blankPaintedLayer(pane) + + tab.manager.resumeRendering() + tab.manager.setAtlasRecoveryVisible(true) + refitAndRefreshAllTerminalPanes() + await settleFrames(30) + + expect(paintedText(pane)).toContain('restored from snapshot') + }) +}) + +describe('content survives a tab reveal', () => { + // Deleting this test lets PaneManager.scheduleRevealRepaint stop reaching the revealed + // tab's panes: the background-tab-revealed blank, with both content kinds in one split. + it('repaints both a plain pane and a TUI pane of the revealed tab', async () => { + const tab = createPaneTab({ background: true }) + const shell = tab.manager.createInitialPane({ focus: false }) + const tui = tab.manager.splitPane(shell.id, 'horizontal') + if (!tui) { + throw new Error('expected the split to create a second pane') + } + await writeToPane(shell, SHELL_SCROLLBACK) + await writeAlternateScreenFrame(tui, TUI_FRAME) + tab.manager.suspendRendering() + await settleFrames(30) + blankPaintedLayer(shell) + blankPaintedLayer(tui) + + // The reveal order TerminalPane uses: resume rendering and mark visible, then repaint. + tab.manager.resumeRendering() + tab.manager.setAtlasRecoveryVisible(true) + tab.manager.scheduleRevealRepaint() + await settleFrames() + + expect(paintedText(shell)).toContain('nothing to commit') + expect(paintedText(tui)).toContain('waiting…') + }) + + // Deleting this test drops the ordering constraint that made reveals blank in the first + // place: the settled repaint only reaches managers that are already marked visible, so a + // reveal that schedules the repaint before marking the tab visible paints nothing. + it('does not reach a tab that has not been marked visible yet', async () => { + const tab = createPaneTab({ background: true }) + const pane = tab.manager.createInitialPane({ focus: false }) + await writeToPane(pane, 'still hidden') + await settleFrames(30) + blankPaintedLayer(pane) + + tab.manager.scheduleRevealRepaint() + await settleFrames() + + expect(paintedText(pane), 'an unrevealed tab is skipped — reveal must mark visible first').toBe( + '' + ) + }) +}) + +describe('content survives a window hide/show', () => { + // Deleting this test lets the plain-refocus present path stop presenting. A window that was + // occluded never hid its panes, so nothing re-writes them; without the present they keep + // showing whatever the compositor dropped. + it('presents already-visible panes of every open tab', async () => { + const first = createPaneTab() + const second = createPaneTab() + const firstPane = first.manager.createInitialPane({ focus: false }) + const secondPane = second.manager.createInitialPane({ focus: false }) + await writeToPane(firstPane, 'tab one output') + await writeToPane(secondPane, 'tab two output') + await settleFrames(30) + blankPaintedLayer(firstPane) + blankPaintedLayer(secondPane) + + first.manager.scheduleRevealPresent() + second.manager.scheduleRevealPresent() + await settleFrames() + + expect(paintedText(firstPane)).toContain('tab one output') + expect(paintedText(secondPane)).toContain('tab two output') + }) +}) + +describe('content survives a split and an unsplit', () => { + // Deleting this test lets the split path start over on the source pane's terminal. The + // split reparents that pane's live DOM subtree; its scrollback and painted frame must come + // through the move, and the new pane must not inherit them. + it('keeps the source pane painted when a split reparents it', async () => { + const { manager } = createPaneTab() + const source = manager.createInitialPane({ focus: false }) + await writeToPane(source, SHELL_SCROLLBACK) + await settleFrames(30) + const content = bufferText(source) + + const created = manager.splitPane(source.id, 'vertical') + if (!created) { + throw new Error('expected the split to create a second pane') + } + await settleFrames() + + expect(bufferText(source)).toBe(content) + expect(paintedText(source)).toContain('nothing to commit') + expect(paintedText(created), 'a fresh split pane starts empty').toBe('') + }) + + // Deleting this test lets the unsplit promote a survivor that is blank. closePane + // reparents the surviving pane with replaceChild — the same DOM move the split path + // explicitly compensates for — and only refits it; if the refit is a no-op because the + // pane's box did not change, nothing else repaints it. + it('keeps the surviving pane painted after the other half closes', async () => { + const { manager } = createPaneTab() + const survivor = manager.createInitialPane({ focus: false }) + const doomed = manager.splitPane(survivor.id, 'vertical') + if (!doomed) { + throw new Error('expected the split to create a second pane') + } + await writeToPane(survivor, SHELL_SCROLLBACK) + await settleFrames(30) + const content = bufferText(survivor) + + manager.closePane(doomed.id) + await settleFrames() + + expect(manager.getPanes()).toHaveLength(1) + expect(bufferText(survivor)).toBe(content) + expect(paintedText(survivor)).toContain('nothing to commit') + }) +})