diff --git a/.github/workflows/linux-wayland-gpu-sandbox.yml b/.github/workflows/linux-wayland-gpu-sandbox.yml index be4f4ecc4..fc297326d 100644 --- a/.github/workflows/linux-wayland-gpu-sandbox.yml +++ b/.github/workflows/linux-wayland-gpu-sandbox.yml @@ -95,6 +95,12 @@ jobs: run: | set -euo pipefail git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.sha }}" + # Why: once the PR base includes this workaround, the partial + # checkout cannot reconstruct an unfixed Wayland negative control. + if git show "${{ github.event.pull_request.base.sha }}:src/main/startup/configure-process.ts" | grep -Eq "appendSwitch\\(['\"]disable-gpu-sandbox['\"]\\)"; then + echo "The base build already contains the Linux Wayland GPU sandbox workaround; skipping unfixed-path reproduction." + exit 0 + fi # Why: keep the new verifier scripts, but run them against the # unfixed production terminal/GPU path instead of a hybrid checkout. git checkout "${{ github.event.pull_request.base.sha }}" -- \ diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index add52d563..b5dffb1a2 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -305,9 +305,18 @@ describe('registerPtyHandlers', () => { mainWindow.webContents.on.mockReset() mainWindow.webContents.send.mockReset() + // Why: mirror real Electron — ipcMain.handle throws on a duplicate channel + // unless removeHandler cleared it first. This catches a re-registration + // (macOS re-activate / new window) that forgets to remove a handle channel. handleMock.mockImplementation((channel: string, handler: (...a: unknown[]) => unknown) => { + if (handlers.has(channel)) { + throw new Error(`Attempted to register a second handler for '${channel}'`) + } handlers.set(channel, handler) }) + removeHandlerMock.mockImplementation((channel: string) => { + handlers.delete(channel) + }) getPathMock.mockReturnValue('/tmp/orca-user-data') existsSyncMock.mockReturnValue(true) statSyncMock.mockReturnValue({ isDirectory: () => true, mode: 0o755 }) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 09fb8f690..a64371fb3 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -1185,6 +1185,7 @@ export function registerPtyHandlers( ipcMain.removeHandler('pty:hasChildProcesses') ipcMain.removeHandler('pty:getForegroundProcess') ipcMain.removeHandler('pty:getCwd') + ipcMain.removeHandler('pty:getSize') ipcMain.removeHandler('pty:declarePendingPaneSerializer') ipcMain.removeHandler('pty:settlePaneSerializer') ipcMain.removeHandler('pty:clearPendingPaneSerializer') @@ -3238,6 +3239,21 @@ export function registerPtyHandlers( } }) + // Why: the renderer forwards resizes fire-and-forget and otherwise has no way + // to learn the PTY's actual size. A resize dropped main-side (suppression + // window, mobile-driver gate, or a provider no-op) leaves the renderer + // believing it synced when it did not, so a later same-cols layout never + // re-forwards and the TUI stays garbled. Exposing the last APPLIED size lets + // the renderer detect true drift on resume and re-assert. ptySizes is only + // written when a resize actually lands (and at spawn), so it is the + // authoritative "what the PTY believes it is". + ipcMain.handle( + 'pty:getSize', + (_event, args: { id: string }): { cols: number; rows: number } | null => { + return ptySizes.get(args.id) ?? null + } + ) + // Why: pre-signal handshake handlers. See // docs/mobile-prefer-renderer-scrollback.md and the rationale on // `pendingByPaneKey` above. The IPC contract is: renderer awaits declare diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 0f2ac202f..655aef88d 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1140,6 +1140,7 @@ export type PreloadApi = { hasChildProcesses: (id: string) => Promise getForegroundProcess: (id: string) => Promise getCwd: (id: string) => Promise + getSize: (id: string) => Promise<{ cols: number; rows: number } | null> listSessions: () => Promise<{ id: string; cwd: string; title: string }[]> getMainBufferSnapshot: ( id: string, diff --git a/src/preload/index.ts b/src/preload/index.ts index db4dfad6b..ec69e1a0e 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -861,6 +861,12 @@ const api = { * Returns `''` when the id is unknown or the platform cannot resolve one. */ getCwd: (id: string): Promise => ipcRenderer.invoke('pty:getCwd', { id }), + /** The PTY's last APPLIED size (its real winsize), or null if unknown. + * Lets the renderer detect drift after a resize was dropped main-side and + * re-assert, instead of trusting the size it last fired blind. */ + getSize: (id: string): Promise<{ cols: number; rows: number } | null> => + ipcRenderer.invoke('pty:getSize', { id }), + onData: ( callback: (data: { id: string; data: string; seq?: number; rawLength?: number }) => void ): (() => void) => { 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 efca37633..8dd2bc348 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -606,6 +606,7 @@ describe('connectPanePty', () => { pty: { signal: vi.fn(), listSessions: vi.fn().mockResolvedValue([]), + getSize: vi.fn().mockResolvedValue(null), getMainBufferSnapshot: vi.fn().mockResolvedValue(null), getForegroundProcess: vi.fn().mockResolvedValue(null), hasChildProcesses: vi.fn().mockResolvedValue(false), @@ -10750,4 +10751,153 @@ describe('connectPanePty', () => { expect(listSessions).not.toHaveBeenCalled() }) }) + + describe('PTY size re-assert on visibility resume', () => { + // Why: a resize dropped while the pane was hidden (suppression window, + // mobile-driver gate, provider no-op) leaves xterm and the PTY silently + // diverged. The renderer dedupes on what it *thinks* it sent, so a later + // same-cols layout never re-forwards — "resizing sometimes doesn't fix it". + // On resume the binding reads the PTY's ACTUAL size and re-asserts only on + // real drift. The mock pane's fitAddon has no proposeDimensions, so safeFit + // is a no-op and pane.terminal stays at its createPane() default (120x40). + async function connectResumablePane(depsOverrides: Record = {}): Promise<{ + binding: { noteVisibilityResume: () => void } + transport: MockTransport + deps: ReturnType + }> { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-pane-2') + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) }, + ...depsOverrides + }) + const pane = createPane(2) + const binding = connectPanePty(pane as never, manager as never, deps as never) as unknown as { + noteVisibilityResume: () => void + } + return { binding, transport, deps } + } + + it('re-asserts the current size when the PTY drifted from xterm', async () => { + vi.mocked(window.api.pty.getSize).mockResolvedValue({ cols: 80, rows: 24 }) + const { binding, transport } = await connectResumablePane() + transport.resize.mockClear() + + binding.noteVisibilityResume() + await flushAsyncTicks() + + // xterm is 120x40 (createPane default), PTY reports 80x24 → re-assert. + expect(transport.resize).toHaveBeenCalledWith(120, 40) + }) + + it('does NOT re-assert when the PTY already matches xterm (no spurious SIGWINCH)', async () => { + vi.mocked(window.api.pty.getSize).mockResolvedValue({ cols: 120, rows: 40 }) + const { binding, transport } = await connectResumablePane() + transport.resize.mockClear() + + binding.noteVisibilityResume() + await flushAsyncTicks() + + expect(transport.resize).not.toHaveBeenCalled() + }) + + it('re-asserts when the PTY size is unknown (cannot confirm synced)', async () => { + vi.mocked(window.api.pty.getSize).mockResolvedValue(null) + const { binding, transport } = await connectResumablePane() + transport.resize.mockClear() + + binding.noteVisibilityResume() + await flushAsyncTicks() + + expect(transport.resize).toHaveBeenCalledWith(120, 40) + }) + + it('skips remote-runtime PTYs (their size lives outside the local ptySizes map)', async () => { + const getSize = vi.mocked(window.api.pty.getSize) + getSize.mockClear() + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('remote:env-1@@terminal-2') + transport.getConnectionId.mockReturnValue(null) + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + const pane = createPane(2) + const binding = connectPanePty(pane as never, manager as never, deps as never) as unknown as { + noteVisibilityResume: () => void + } + transport.resize.mockClear() + + binding.noteVisibilityResume() + await flushAsyncTicks() + + // Never even queries size for a remote pane, and never re-asserts. + expect(getSize).not.toHaveBeenCalled() + expect(transport.resize).not.toHaveBeenCalled() + }) + + it('does NOT re-assert while a mobile-fit override parks the PTY at phone dims', async () => { + const { setFitOverride } = await import('@/lib/pane-manager/mobile-fit-overrides') + vi.mocked(window.api.pty.getSize).mockResolvedValue({ cols: 80, rows: 24 }) + const { binding, transport } = await connectResumablePane() + // Park the PTY at phone dims — desktop re-assert must be suppressed. + setFitOverride('pty-pane-2', 'mobile-fit', 40, 30) + transport.resize.mockClear() + + binding.noteVisibilityResume() + await flushAsyncTicks() + + expect(transport.resize).not.toHaveBeenCalled() + setFitOverride('pty-pane-2', 'desktop-fit', 0, 0) + }) + + it('does NOT forward when the pane is hidden again before getSize resolves (stale hop)', async () => { + // The load-bearing safety property: a getSize promise resolving AFTER the + // pane was re-hidden must not emit a hidden-tab SIGWINCH (which can reset + // alt-screen TUIs). Suppression is the send-time visibility re-check. + let resolveSize: (v: { cols: number; rows: number } | null) => void = () => {} + vi.mocked(window.api.pty.getSize).mockImplementation( + () => + new Promise((resolve) => { + resolveSize = resolve + }) + ) + const { binding, transport, deps } = await connectResumablePane() + transport.resize.mockClear() + + binding.noteVisibilityResume() + // Pane is hidden again while the size query is still in flight. + deps.isVisibleRef.current = false + resolveSize({ cols: 80, rows: 24 }) // drift — would re-assert if visible + await flushAsyncTicks() + + expect(transport.resize).not.toHaveBeenCalled() + }) + + it('coalesces overlapping resumes into a single size query (re-entrancy guard)', async () => { + const getSize = vi.mocked(window.api.pty.getSize) + getSize.mockClear() + let resolveSize: (v: { cols: number; rows: number } | null) => void = () => {} + getSize.mockImplementation( + () => + new Promise((resolve) => { + resolveSize = resolve + }) + ) + const { binding } = await connectResumablePane() + getSize.mockClear() + + // Two rapid resumes before the first query resolves → only one query. + binding.noteVisibilityResume() + binding.noteVisibilityResume() + expect(getSize).toHaveBeenCalledTimes(1) + resolveSize({ cols: 120, rows: 40 }) + await flushAsyncTicks() + }) + }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index de7a9e611..7ec8ac3df 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -26,6 +26,7 @@ import type { PtyConnectionDeps } from './pty-connection-types' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' import { getFitOverrideForPty, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' import { isPtyLocked } from '@/lib/pane-manager/mobile-driver-state' +import { reconcilePtySizeAcrossFrames, type PtySizeReconcileHandle } from './pty-size-reconcile' import { isPaneReplaying, replayIntoTerminal, replayIntoTerminalAsync } from './replay-guard' import { nativeWindowsRewriteNeedsFollowupRenderRefresh, @@ -2250,52 +2251,91 @@ export function connectPanePty( // Why: the deferred-rAF fit can spawn the PTY at a stale width when the pane's // real (e.g. split/narrower) layout has not settled by the first frame — the // PTY is born at the wide window width while xterm later reflows to the pane - // width. The corrective onResize is then dropped (cols already matched at fit - // time, or isRendererPtyResizeAuthoritative() was false mid-mount), pinning - // process.stdout.columns forever and garbling TUIs. Re-fit once layout has - // settled and force the PTY to xterm's dimensions; the initial spawn-time sync - // is authoritative by definition, so it bypasses the visibility gate (but not - // the mobile-fit override, which legitimately parks the PTY at phone dims). + // width. The corrective onResize is then dropped (isRendererPtyResizeAuthoritative() + // is false mid-mount), pinning process.stdout.columns forever and garbling + // TUIs. The reconcile re-fits across frames until the grid settles and forces + // the PTY to xterm's dimensions; the spawn-time sync is authoritative by + // definition so it bypasses the visibility gate (but not the mobile-fit + // override, which legitimately parks the PTY at phone dims). See + // pty-size-reconcile.ts for the convergence loop. + let ptySizeReconcileHandle: PtySizeReconcileHandle | null = null const reconcilePtySizeAfterSpawn = ( ptyId: string, spawnCols: number, spawnRows: number ): void => { - // Why: a single post-spawn frame is not enough — the pane's real layout can - // keep changing for several frames (split equalize, sidebar/title reflow), - // so a one-shot re-fit can still measure a stale width and leave the PTY - // pinned. Poll across frames and forward the settled size to the PTY - // whenever it differs from what the PTY was last told, mirroring the - // ResizeObserver stability loop. Each resize is gated on an actual change, - // so a TUI sees at most a couple of SIGWINCH during startup, not a loop. - const MAX_RECONCILE_FRAMES = 12 - let frame = 0 - let lastSentCols = spawnCols - let lastSentRows = spawnRows - const tick = (): void => { - if (disposed || transport.getPtyId() !== ptyId) { - return - } - // Mobile legitimately parks the PTY at phone dims; a transient guard - // should only skip this frame, not cancel the reconcile window. - if (!getFitOverrideForPty(ptyId) && !isPtyLocked(ptyId)) { + ptySizeReconcileHandle?.cancel() + ptySizeReconcileHandle = reconcilePtySizeAcrossFrames({ + spawnCols, + spawnRows, + isAlive: () => !disposed && transport.getPtyId() === ptyId, + // Mobile legitimately parks the PTY at phone dims; skip those frames + // (neither fit nor forward) instead of cancelling the reconcile window. + isParked: () => Boolean(getFitOverrideForPty(ptyId)) || isPtyLocked(ptyId), + // Once the renderer resize is authoritative (pane visible), the live + // onResize owns future corrections, so the reconcile can hand off after + // the grid stabilizes. While hidden it keeps watching for a late settle. + isAuthoritative: () => isRendererPtyResizeAuthoritative(), + measure: () => { safeFit(pane) const cols = pane.terminal.cols const rows = pane.terminal.rows - if (cols > 0 && rows > 0 && (cols !== lastSentCols || rows !== lastSentRows)) { - // Initial spawn-time sync is authoritative, so it bypasses the - // visibility gate that onResize honors (but not the mobile guards above). - transport.resize(cols, rows) - lastSentCols = cols - lastSentRows = rows + return cols > 0 && rows > 0 ? { cols, rows } : null + }, + resize: (cols, rows) => transport.resize(cols, rows), + requestFrame: (callback) => requestAnimationFrame(callback), + cancelFrame: (handle) => { + if (typeof cancelAnimationFrame === 'function') { + cancelAnimationFrame(handle) } } - frame += 1 - if (frame < MAX_RECONCILE_FRAMES) { - requestAnimationFrame(tick) - } + }) + } + + // Why: the renderer forwards resizes fire-and-forget and dedupes on the size + // it *last sent*, so a resize dropped main-side (it was hidden, a suppression + // window, or a provider no-op) leaves xterm and the PTY silently diverged — + // and a later same-cols layout fires no onResize, so it never self-corrects + // ("resizing sometimes doesn't fix it"). On becoming visible, re-fit and + // compare xterm against the PTY's ACTUAL size (not what we think we sent); if + // they truly differ, re-assert. Gated on real drift so we emit no spurious + // SIGWINCH (which would jar alt-screen TUIs) on an already-synced resume. + let reassertingPtySizeOnResume = false + const reassertPtySizeOnResume = (): void => { + const ptyId = transport.getPtyId() + // forwardPtyResize re-checks the visibility/mobile gates at send time, so + // here we only need the cheap pre-hop early-outs. Skip remote-runtime PTYs: + // their resize goes through a separate viewport channel (not pty:resize), so + // the local ptySizes map getSize reads is never populated for them. + if (disposed || reassertingPtySizeOnResume || !ptyId || isRemoteRuntimePtyId(ptyId)) { + return } - requestAnimationFrame(tick) + reassertingPtySizeOnResume = true + void window.api.pty + .getSize(ptyId) + .then((actual) => { + // The pane may have been disposed or rebound to a different PTY during + // the async hop; bail if this reconcile no longer owns it. + if (disposed || transport.getPtyId() !== ptyId) { + return + } + safeFit(pane) + const cols = pane.terminal.cols + const rows = pane.terminal.rows + if (cols <= 0 || rows <= 0) { + return + } + // Re-assert only on genuine divergence from the PTY's applied size (a + // null read = unknown id, treated as "cannot confirm synced" so we + // forward once). forwardPtyResize owns the authoritative/mobile gating. + if (!actual || actual.cols !== cols || actual.rows !== rows) { + forwardPtyResize(cols, rows) + } + }) + .catch(() => {}) + .finally(() => { + reassertingPtySizeOnResume = false + }) } // Defer PTY spawn/attach to next frame so FitAddon has time to calculate @@ -4562,10 +4602,18 @@ export function connectPanePty( // keeps the typing hot path off the listSessions IPC between resumes. noteVisibilityResume() { livenessRecheckFiredSinceResume = false + // Why: re-assert the PTY size on resume so a resize that was dropped while + // this pane was hidden self-heals on show, instead of waiting for a manual + // resize that may never change xterm's column count. + reassertPtySizeOnResume() }, reconcileIfSessionDead, dispose() { disposed = true + // Why: the post-spawn reconcile polls across frames; cancel its pending + // rAF so a torn-down pane cannot keep fitting/resizing after disposal. + ptySizeReconcileHandle?.cancel() + ptySizeReconcileHandle = null if (terminalKeyTargetSupportsEvents) { terminalKeyTarget.removeEventListener('keydown', onTerminalKeyDown, { capture: true }) } diff --git a/src/renderer/src/components/terminal-pane/pty-size-reconcile.test.ts b/src/renderer/src/components/terminal-pane/pty-size-reconcile.test.ts new file mode 100644 index 000000000..9f02527d5 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-size-reconcile.test.ts @@ -0,0 +1,315 @@ +import { describe, expect, it, vi } from 'vitest' +import { + reconcilePtySizeAcrossFrames, + type PtySizeReconcileDimensions, + type PtySizeReconcileOptions +} from './pty-size-reconcile' + +/** + * Reproduction harness for the terminal column-desync bug (Slack: Claude Code + * renders garbled when a new worktree is opened with the side split panel on). + * + * The PTY is spawned at the wide window width, then the split/sidebar layout + * narrows the pane some frames LATER. The fix must converge the PTY to the + * settled narrow width regardless of WHEN the layout lands — including a settle + * that arrives well after any fixed frame budget (the prior fix used a fixed + * 12-frame budget that expired before the split equalized). The reconcile keeps + * polling while the pane is not yet authoritative (the hidden mount window where + * the live onResize is dropped) and hands off once authoritative + stable. + */ + +/** A deterministic frame scheduler: callbacks queue, then run() drains them. */ +function createFrameScheduler() { + const queue = new Map void>() + let nextHandle = 1 + return { + requestFrame: (callback: () => void): number => { + const handle = nextHandle++ + queue.set(handle, callback) + return handle + }, + cancelFrame: (handle: number): void => { + queue.delete(handle) + }, + /** Run up to `maxFrames` queued frames, one per tick. Returns frames run. */ + run(maxFrames = 1000): number { + let ran = 0 + while (queue.size > 0 && ran < maxFrames) { + const [handle, callback] = queue.entries().next().value as [number, () => void] + queue.delete(handle) + callback() + ran += 1 + } + return ran + }, + pending: () => queue.size + } +} + +/** + * A pane whose measured grid follows a timeline keyed by frame index: it starts + * unmeasurable (null) or wide, then narrows at some frame. `measure()` is called + * once per reconcile frame, so the call count tracks frames elapsed. + */ +function createTimelinePane(timeline: (frame: number) => PtySizeReconcileDimensions | null) { + let frame = 0 + return { + measure: vi.fn((): PtySizeReconcileDimensions | null => { + const dims = timeline(frame) + frame += 1 + return dims + }) + } +} + +function runReconcile( + overrides: Partial & Pick, + maxFrames = 1000 +): { resize: ReturnType; framesRun: number } { + const scheduler = createFrameScheduler() + const resize = vi.fn() + reconcilePtySizeAcrossFrames({ + spawnCols: 203, + spawnRows: 50, + isAlive: () => true, + isParked: () => false, + // Default: pane is visible (the common case). Specific tests override this + // to model the hidden mount window where the live onResize is dropped. + isAuthoritative: () => true, + resize, + requestFrame: scheduler.requestFrame, + cancelFrame: scheduler.cancelFrame, + ...overrides + }) + const framesRun = scheduler.run(maxFrames) + return { resize, framesRun } +} + +describe('reconcilePtySizeAcrossFrames', () => { + it('forwards a narrow settle that lands AFTER a fixed 12-frame budget — while hidden', () => { + // Golden repro: the pane mounts hidden (onResize dropped — the desync + // window), spawned wide (203), still measuring wide for the first 15 frames + // (split equalize / sidebar reflow in flight), then settles to 79. A + // 12-frame-budget reconcile would have stopped at frame 12 — still wide, + // leaving the PTY pinned. The convergent loop keeps watching while hidden. + const NARROW_AT = 15 + const pane = createTimelinePane((frame) => + frame < NARROW_AT ? { cols: 203, rows: 50 } : { cols: 79, rows: 50 } + ) + const { resize } = runReconcile({ measure: pane.measure, isAuthoritative: () => false }) + + expect(resize).toHaveBeenCalled() + expect(resize).toHaveBeenLastCalledWith(79, 50) + }) + + it('forwards a narrow settle that lands LATE while hidden — no fixed frame floor', () => { + // The bug the adversarial review caught: a fixed MIN-frames floor (e.g. 24) + // would treat the still-wide spawn measurement as "settled" and stop before + // the real narrowing lands. A split that equalizes at frame 40 while the + // pane is still hidden must STILL be forwarded — the loop watches until it + // becomes authoritative (where onResize takes over) or the hard cap. + const NARROW_AT = 40 + const pane = createTimelinePane((frame) => + frame < NARROW_AT ? { cols: 203, rows: 50 } : { cols: 79, rows: 50 } + ) + const { resize } = runReconcile({ measure: pane.measure, isAuthoritative: () => false }) + + expect(resize).toHaveBeenCalled() + expect(resize).toHaveBeenLastCalledWith(79, 50) + }) + + it('forwards a late settle that lands just before the pane becomes authoritative', () => { + // Realistic handoff: hidden through the settle, narrows at frame 40, then + // the pane becomes visible at frame 45. The reconcile must have already + // forwarded the narrow width during the hidden window (its resize bypasses + // the visibility gate); the later authoritative+stable state lets it stop. + const NARROW_AT = 40 + const AUTHORITATIVE_AT = 45 + let frameSeen = 0 + const pane = createTimelinePane((frame) => { + frameSeen = frame + return frame < NARROW_AT ? { cols: 203, rows: 50 } : { cols: 79, rows: 50 } + }) + const { resize } = runReconcile({ + measure: pane.measure, + isAuthoritative: () => frameSeen >= AUTHORITATIVE_AT + }) + + expect(resize).toHaveBeenCalled() + expect(resize).toHaveBeenLastCalledWith(79, 50) + }) + + it('hands off after the pane is visible+stable, leaving later reflows to the live onResize', () => { + // Handoff boundary (verified design): once the pane is authoritative AND the + // grid has been stable for the settle window, the live onResize / + // ResizeObserver path owns any FURTHER reflow (it fires on every + // grid-changing fit() and is not suppressed for a visible desktop pane). So + // the reconcile is allowed to stop here — a split that equalizes much later, + // after this handoff, is caught by that backstop, not by the reconcile. This + // pins that the reconcile terminates promptly in the steady visible state + // rather than polling to the hard cap. The narrow-while-hidden window (where + // onResize is dropped) is covered by the dedicated tests above. + const pane = createTimelinePane(() => ({ cols: 79, rows: 50 })) + const { resize, framesRun } = runReconcile({ measure: pane.measure }) + expect(resize).toHaveBeenCalledTimes(1) + expect(resize).toHaveBeenLastCalledWith(79, 50) + // Frame 1 forwards the single change (resets the window); frames 2..9 are + // authoritative + unchanged, so the loop settles exactly at SETTLE_FRAMES(8) + // observed-stable frames — i.e. 9 frames total, far short of the 180 cap. + expect(framesRun).toBe(9) + }) + + it('keeps polling through unmeasurable frames (pane has no layout yet)', () => { + // A fresh split mount can be unmeasurable for many frames before the real + // grid lands. Unmeasurable frames must NOT count as "settled". + const NARROW_AT = 20 + const pane = createTimelinePane((frame) => (frame < NARROW_AT ? null : { cols: 80, rows: 24 })) + const { resize } = runReconcile({ measure: pane.measure }) + + expect(resize).toHaveBeenCalledTimes(1) + expect(resize).toHaveBeenLastCalledWith(80, 24) + }) + + it('hands off (stops) once authoritative and the grid has been stable', () => { + // Once visible and stable, the live onResize owns future corrections; the + // reconcile should stop rather than poll to the hard cap forever. + const pane = createTimelinePane(() => ({ cols: 79, rows: 50 })) + const { framesRun } = runReconcile({ measure: pane.measure }) + // Should settle a few frames after the single resize, well short of the cap. + expect(framesRun).toBeGreaterThan(0) + expect(framesRun).toBeLessThan(180) + }) + + it('does NOT hand off while hidden — keeps watching until the hard cap', () => { + // While never authoritative, a stable grid is not a safe stopping point + // (onResize cannot back us up), so the loop runs to the hard cap. + const pane = createTimelinePane(() => ({ cols: 79, rows: 50 })) + const { framesRun } = runReconcile({ + measure: pane.measure, + isAuthoritative: () => false + }) + expect(framesRun).toBe(180) + }) + + it('forwards no resize when the settled size never changes from spawn dims', () => { + // If xterm already matches the spawn width the whole time, no SIGWINCH at all. + const pane = createTimelinePane(() => ({ cols: 203, rows: 50 })) + const { resize } = runReconcile({ measure: pane.measure }) + expect(resize).not.toHaveBeenCalled() + }) + + it('does not loop forever — terminates within the hard frame cap', () => { + // Pane that never stabilizes (oscillates) must still hit the hard cap. + const pane = createTimelinePane((frame) => + frame % 2 === 0 ? { cols: 100, rows: 30 } : { cols: 101, rows: 30 } + ) + const { framesRun } = runReconcile({ measure: pane.measure }, 10_000) + expect(framesRun).toBe(180) + }) + + it('issues only a couple of SIGWINCH for a monotonic narrow settle (not one per frame)', () => { + // 203 → 120 → 79 over the first frames, then stable. The TUI should see the + // size change a small, bounded number of times during its own startup. + const pane = createTimelinePane((frame) => { + if (frame < 5) { + return { cols: 203, rows: 50 } + } + if (frame < 10) { + return { cols: 120, rows: 50 } + } + return { cols: 79, rows: 50 } + }) + const { resize } = runReconcile({ measure: pane.measure }) + expect(resize.mock.calls.length).toBeLessThanOrEqual(3) + expect(resize).toHaveBeenLastCalledWith(79, 50) + }) + + it('skips parked (mobile-fit) frames without forwarding a desktop resize', () => { + const pane = createTimelinePane(() => ({ cols: 79, rows: 50 })) + const { resize, framesRun } = runReconcile({ + measure: pane.measure, + isParked: () => true + }) + expect(resize).not.toHaveBeenCalled() + expect(pane.measure).not.toHaveBeenCalled() + // Parked frames still count toward the cap so a parked PTY can't loop forever. + expect(framesRun).toBe(180) + }) + + it('resumes and converges after a transient park (mobile take-back during mount)', () => { + // Parked frames are SKIPPED, not cancelled: if mobile transiently drives the + // PTY during the mount window and then hands control back, the reconcile must + // resume and forward the settled desktop width — not abort permanently. + const PARKED_UNTIL = 10 + let frameSeen = 0 + const pane = createTimelinePane(() => ({ cols: 79, rows: 50 })) + const scheduler = createFrameScheduler() + const resize = vi.fn() + reconcilePtySizeAcrossFrames({ + spawnCols: 203, + spawnRows: 50, + isAlive: () => true, + isParked: () => frameSeen++ < PARKED_UNTIL, + isAuthoritative: () => true, + measure: pane.measure, + resize, + requestFrame: scheduler.requestFrame, + cancelFrame: scheduler.cancelFrame + }) + scheduler.run() + // While parked, measure()/resize() are skipped; after take-back the desktop + // width is measured and forwarded exactly once. + expect(resize).toHaveBeenCalledTimes(1) + expect(resize).toHaveBeenLastCalledWith(79, 50) + }) + + it('stops promptly once cancelled (pane disposed mid-reconcile)', () => { + const scheduler = createFrameScheduler() + const resize = vi.fn() + const pane = createTimelinePane((frame) => + frame < 30 ? { cols: 203, rows: 50 } : { cols: 79, rows: 50 } + ) + const handle = reconcilePtySizeAcrossFrames({ + spawnCols: 203, + spawnRows: 50, + isAlive: () => true, + isParked: () => false, + isAuthoritative: () => true, + measure: pane.measure, + resize, + requestFrame: scheduler.requestFrame, + cancelFrame: scheduler.cancelFrame + }) + // Run a few frames, then cancel — no further frames should be scheduled. + scheduler.run(3) + handle.cancel() + expect(scheduler.pending()).toBe(0) + const measuredBefore = pane.measure.mock.calls.length + scheduler.run(100) + expect(pane.measure.mock.calls.length).toBe(measuredBefore) + }) + + it('stops when the PTY is no longer alive (rebound / disposed)', () => { + const scheduler = createFrameScheduler() + const resize = vi.fn() + let alive = true + const pane = createTimelinePane(() => ({ cols: 79, rows: 50 })) + reconcilePtySizeAcrossFrames({ + spawnCols: 203, + spawnRows: 50, + isAlive: () => alive, + isParked: () => false, + isAuthoritative: () => true, + measure: pane.measure, + resize, + requestFrame: scheduler.requestFrame, + cancelFrame: scheduler.cancelFrame + }) + scheduler.run(2) + const callsBefore = resize.mock.calls.length + alive = false + scheduler.run(100) + expect(resize.mock.calls.length).toBe(callsBefore) + expect(scheduler.pending()).toBe(0) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-size-reconcile.ts b/src/renderer/src/components/terminal-pane/pty-size-reconcile.ts new file mode 100644 index 000000000..d8c290d69 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-size-reconcile.ts @@ -0,0 +1,144 @@ +// Why: the deferred-rAF fit can spawn the PTY at a stale (wide) width when the +// pane's real layout has not settled by the first frame — e.g. a tab that +// MOUNTS with a split layout already present (a new worktree opened with the +// side panel on). The PTY is born at the wide window width while xterm later +// reflows to the narrower split/pane width. The corrective xterm onResize is +// dropped during the mount window because it honors the visibility gate, which +// is not yet authoritative (deps.isVisibleRef flips true after mount). So +// process.stdout.columns stays pinned wide forever and interactive TUIs render +// garbled (output sized for the wide width wrapping ~1 char per line into the +// narrow pane). Only a later manual resize re-syncs it. +// +// This post-spawn reconcile bridges exactly that gap: it polls across frames, +// forwarding xterm's measured grid to the PTY on every actual change. Its +// resize is authoritative by definition, so it bypasses the visibility gate. +// It keeps polling while the pane is NOT yet authoritative (the hidden mount +// window where onResize is dropped) so a late-settling split/sidebar layout is +// still forwarded; once the pane is authoritative and the grid has been stable, +// it stops and hands off to the live onResize path, which catches any later +// layout change. A hard frame cap guarantees termination. +// +// This loop tracks what it last SENT, not what the PTY actually applied, so a +// forward dropped main-side (e.g. a mobile take-back resize-suppression window) +// can still leave the PTY stale here. The visibility-resume re-assert in +// pty-connection.ts is the backstop: on show it reads the PTY's real size +// (pty:getSize) and re-forwards on true drift, healing a pane that later +// hides/shows. + +export type PtySizeReconcileDimensions = { cols: number; rows: number } + +export type PtySizeReconcileOptions = { + /** Dimensions the PTY was spawned at — the size it currently believes it is. */ + spawnCols: number + spawnRows: number + /** True while this reconcile still owns a live PTY (not disposed / not rebound). */ + isAlive: () => boolean + /** + * True while the PTY is legitimately parked at non-pane dims (mobile-fit + * override / mobile driving). Such frames are skipped — neither fit nor + * forwarded — but still count toward the hard cap so a permanently-parked + * PTY cannot loop forever. + */ + isParked: () => boolean + /** + * True once the live onResize path will forward future PTY resizes itself + * (i.e. the pane is visible / renderer resize is authoritative). The + * reconcile only needs to run while this is false (the mount window where + * onResize is dropped); once true and the grid is stable it hands off. + */ + isAuthoritative: () => boolean + /** + * Fit the pane and return its current measured grid, or null when the pane is + * not yet measurable. A measured grid that differs from what the PTY was last + * told is forwarded; a matching grid counts toward the stability window. + */ + measure: () => PtySizeReconcileDimensions | null + /** Forward the settled size to the PTY (authoritative — bypasses visibility). */ + resize: (cols: number, rows: number) => void + /** Schedule the next frame; mirrors requestAnimationFrame's id contract. */ + requestFrame: (callback: () => void) => number + cancelFrame: (handle: number) => void +} + +export type PtySizeReconcileHandle = { cancel: () => void } + +// Hand off (stop) once the grid has held steady for SETTLE_FRAMES of frames +// observed *while authoritative*. Rationale for the two-part gate: +// - While hidden, the live onResize is dropped, so the reconcile is the SOLE +// corrector: it must keep polling and forwarding every change (a narrow that +// settles during the hidden window is the real shipped bug). Hidden frames +// therefore never count toward the settle window — the loop cannot hand off. +// - Once authoritative (pane visible), the live onResize/ResizeObserver path +// is the authoritative detector of any further reflow, so after a short +// stable window under authority the reconcile hands off to it. The settle +// window also gives a ~SETTLE-frame grace for a narrow landing right at the +// visibility transition before the handoff. +// MAX_FRAMES (~3s at 60fps) guarantees termination for a pane that never +// becomes authoritative (a permanently-hidden background spawn) or never +// stabilizes; such panes re-fit via the resume-time path when shown. +const POST_SPAWN_RECONCILE_SETTLE_FRAMES = 8 +const POST_SPAWN_RECONCILE_MAX_FRAMES = 180 + +export function reconcilePtySizeAcrossFrames( + options: PtySizeReconcileOptions +): PtySizeReconcileHandle { + let frame = 0 + // Counts consecutive unchanged frames observed *while authoritative*. Frames + // measured while hidden never advance it, so a long hidden-wide mount window + // cannot make the loop hand off the instant the pane becomes visible (before + // the split has equalized) on a width it never confirmed under authority. + let authoritativeStableFrames = 0 + let lastSentCols = options.spawnCols + let lastSentRows = options.spawnRows + let pendingFrame: number | null = null + let cancelled = false + + const tick = (): void => { + pendingFrame = null + if (cancelled || !options.isAlive()) { + return + } + frame += 1 + if (!options.isParked()) { + const measured = options.measure() + if (measured && measured.cols > 0 && measured.rows > 0) { + if (measured.cols !== lastSentCols || measured.rows !== lastSentRows) { + // Authoritative spawn-time correction: bypasses the visibility gate + // the live onResize honors (but the caller still skips parked frames). + // A real change resets the stability window so we wait for it to hold. + options.resize(measured.cols, measured.rows) + lastSentCols = measured.cols + lastSentRows = measured.rows + authoritativeStableFrames = 0 + } else if (options.isAuthoritative()) { + // Only stability seen *under authority* counts toward handoff — a grid + // that merely held steady while hidden is not a safe stopping point. + authoritativeStableFrames += 1 + } + } + // A null/zero measurement makes no stability progress: layout isn't ready. + } + // Why authoritative-gated rather than a fixed frame floor: while the pane is + // hidden the live onResize cannot forward, so the reconcile must keep + // watching (and forwarding, since its resize bypasses the gate) for a late + // layout settle. Once the pane has been visible AND its grid has held steady + // for the settle window, the live onResize/ResizeObserver path owns any + // further change, so we hand off. The hard cap guarantees termination. + const settled = authoritativeStableFrames >= POST_SPAWN_RECONCILE_SETTLE_FRAMES + if (!settled && frame < POST_SPAWN_RECONCILE_MAX_FRAMES) { + pendingFrame = options.requestFrame(tick) + } + } + + pendingFrame = options.requestFrame(tick) + + return { + cancel: () => { + cancelled = true + if (pendingFrame !== null) { + options.cancelFrame(pendingFrame) + pendingFrame = null + } + } + } +} diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index e7d0c976b..812e7607a 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -2542,6 +2542,7 @@ function createPtyApi(): NonNullable['pty']> { hasChildProcesses: () => Promise.resolve(false), getForegroundProcess: () => Promise.resolve(null), getCwd: () => Promise.resolve('~'), + getSize: () => Promise.resolve(null), listSessions: () => Promise.resolve([]), getMainBufferSnapshot: () => Promise.resolve(null), getRendererDeliveryDebugSnapshot: () => diff --git a/tests/e2e/terminal-column-desync-repro.spec.ts b/tests/e2e/terminal-column-desync-repro.spec.ts index f4c06de93..54fc3be17 100644 --- a/tests/e2e/terminal-column-desync-repro.spec.ts +++ b/tests/e2e/terminal-column-desync-repro.spec.ts @@ -276,6 +276,81 @@ test.describe('Terminal column desync repro', () => { } }) + // Why: the user-reported case — "start a new worktree with the side split + // panel on". A tab that MOUNTS with a split layout already present (two panes + // side by side from frame 0) spawns each PTY at the wide window width, then + // the split equalize narrows each pane AFTER the post-spawn reconcile window + // has closed. The corrective onResize is dropped by the visibility gate during + // the mount window, so the PTY stays pinned wide while xterm shows the narrow + // split width — only a later manual resize re-syncs it ("resizing fixed it"). + // We reproduce the fresh split mount by splitting then reloading: the split + // layout persists across reload, so the tab remounts with two panes already + // present, re-running the first-mount spawn for each. + test('both panes stay PTY-synced when a tab MOUNTS with a split layout present', async ({ + orcaPage + }) => { + test.setTimeout(240_000) + + // A single mount only trips the race intermittently, so reload-loop the + // restored-split first-mount and assert none of the attempts desynced. + const MOUNT_ATTEMPTS = 6 + const desyncs: { attempt: number; ptyId: string; ptyCols: number; xtermCols: number }[] = [] + + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await closeRightSidebarAndFeatureTips(orcaPage) + await ensureTerminalVisible(orcaPage) + await orcaPage.setViewportSize({ width: 1440, height: 900 }) + await orcaPage.waitForTimeout(300) + await settleTerminal(orcaPage) + + // Establish the persisted split layout once; reloads below rebuild it. + await splitActiveTerminalPane(orcaPage, 'vertical') + await waitForPaneIdentitySnapshot(orcaPage, 2) + + for (let attempt = 0; attempt < MOUNT_ATTEMPTS; attempt += 1) { + // Re-run the split first-mount path: a wide window, reload so the tab + // remounts and re-spawns both PTYs at the wide width from the restored + // split layout, then resize down while the panes are still mounting. + await orcaPage.setViewportSize({ width: 1440, height: 900 }) + await orcaPage.reload() + await orcaPage.waitForFunction(() => Boolean(window.__store), null, { timeout: 30_000 }) + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await closeRightSidebarAndFeatureTips(orcaPage) + await ensureTerminalVisible(orcaPage) + + // Resize narrower while the split panes are mounting / their PTYs spawn. + await orcaPage.setViewportSize({ width: 1180, height: 800 }) + await orcaPage.waitForTimeout(300) + + const snapshot = await waitForPaneIdentitySnapshot(orcaPage, 2) + // Let layout equalize and the (current) reconcile window run to completion. + await orcaPage.waitForTimeout(900) + + for (const pane of snapshot.panes) { + const ptyId = pane.ptyId + expect(ptyId, 'restored split pane should be bound to a PTY').toBeTruthy() + if (!ptyId) { + continue + } + const ptyCols = await readPtyCols(orcaPage, ptyId) + const xtermCols = await readRenderedColsForPty(orcaPage, ptyId) + if (ptyCols !== xtermCols) { + desyncs.push({ attempt, ptyId, ptyCols, xtermCols }) + } + } + } + + expect( + desyncs, + `PTY columns desynced from xterm on a restored-split mount (${desyncs.length} pane(s) ` + + `across ${MOUNT_ATTEMPTS} attempts). A PTY pinned at the wide startup width while xterm ` + + `reflowed to the narrower split width is the column-desync bug that garbles interactive ` + + `TUIs: ${JSON.stringify(desyncs)}` + ).toEqual([]) + }) + // Why: this is the tightest isolation of the real bug. A viewport resize that // lands in the terminal's initial mount window — after xterm exists but // before the PTY binding/visibility settle — reflows xterm to the new width,