From e6541fe9b8702b427033b2cb421f983dd49ba0d7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:55:38 -0700 Subject: [PATCH] fix(ssh): repaint a reconnected pane from the grid, not a byte tail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reconnect restored plain shells correctly but was reported to bring full-screen apps back as fragments of a frame — Claude Code showed a few rules and its cost line until a resize forced it to repaint. The two payloads are not interchangeable. Relay replay is a byte TAIL: it can begin mid-escape, and it misses the alt-screen enter, the clears and the absolute cursor positioning that built the frame, so replaying it into a fresh terminal paints whatever fragments survive. The model snapshot is a serialized GRID — which is what tmux repaints on attach, and the only payload that reliably restores a TUI. Orca already had the grid path and already preferred it; it was gated to PARKING. A reconnect needs it for the same underlying reason a park does: the pane paints into a terminal holding nothing, because a reconnect bumps tab.generation, which is the pane's React key, so TerminalPane remounts and the old xterm is disposed with its buffer. So the gate now admits both, and prepaintParkedSshSnapshot is prepaintSshModelSnapshot since parking is no longer the only caller. Deliberately NOT inheriting the parking kill switch: main keeps its headless model regardless of terminalSshViewParking, so a user who turns view parking off would otherwise be stranded on the tail. Every safety gate below eligibility is untouched, and pinned that way: null, renderer-sourced, sourceless, empty, and escape-tail-only snapshots all still degrade to relay replay, so widening WHY the model is trusted cannot widen WHAT is trusted and cannot regress to a blank pane. Reverting either half of the gate fails three of the new tests. HONESTY ABOUT WHAT THIS IS VERIFIED TO DO. I could not reproduce the corruption it targets. Two attempts against a live host, both on a build WITHOUT this change, both restored correctly: a freshly started Claude Code and Codex side by side, and an alt-screen `less` scrolled 4000 lines so its original full paint had aged out of the relay's 100KB tail. The reporter's case also involved pulling wifi — an abrupt drop rather than a clean disconnect — which is the one variable I cannot simulate here. So this is verified to be correct-by-construction and non-regressing: with it applied, the same scenarios still restore correctly (top live, less at its scrolled offset in alt-screen, both agent TUIs coherent). It is NOT verified to fix the reported symptom, because the symptom did not reproduce. Treat the symptom as open until someone confirms it on an abrupt drop. Also: top was a poor proxy for a TUI in my earlier verification precisely because it repaints every second and therefore self-heals within a tick. --- .../terminal-pane/pty-connection.ts | 36 +++++-- .../ssh-reattach-model-restore.test.ts | 96 +++++++++++++++++++ .../ssh-reattach-model-restore.ts | 43 +++++++-- 3 files changed, 158 insertions(+), 17 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 14a514cfee5..97c072c2d6a 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -7968,6 +7968,13 @@ export function connectPanePty( return true } + // A reconnect remounts the pane (tab.generation is its React key), so like a park it always + // paints into a terminal that holds nothing. directSshRetryAttempt is exactly that mount: + // it is only set when a pending or live SSH pane retry matches this connection AND this tab + // generation. + const isSshReconnectRemount = Boolean(directSshRetryAttempt) + const paintsIntoEmptyTerminal = mountFollowsTerminalPark || isSshReconnectRemount + let parkedSshSnapshotPrefetch: { ptyId: string fetch: () => Promise @@ -7978,7 +7985,13 @@ export function connectPanePty( ): (() => Promise) => memoizeSshReattachModelSnapshotProbe(async (): Promise => { const sshParkingEnabled = useAppStore.getState().settings?.terminalSshViewParking !== false - if (!shouldFetchSshReattachModelSnapshot({ ptyId, sshParkingEnabled })) { + if ( + !shouldFetchSshReattachModelSnapshot({ + ptyId, + sshParkingEnabled, + isSshReconnectRemount + }) + ) { return null } const snapshot = await resolveSshReattachModelSnapshotWithTimeout( @@ -7987,8 +8000,12 @@ export function connectPanePty( }) ) return snapshot && - decideSshReattachPaintSource({ ptyId, sshParkingEnabled, snapshot }) === - 'main-model-snapshot' + decideSshReattachPaintSource({ + ptyId, + sshParkingEnabled, + isSshReconnectRemount, + snapshot + }) === 'main-model-snapshot' ? snapshot : null }) @@ -8002,11 +8019,14 @@ export function connectPanePty( return parkedSshSnapshotPrefetch.fetch } - const prepaintParkedSshSnapshot = (ptyId: string | null): void => { + // Renamed from prepaintParkedSshSnapshot: parking is no longer the only mount that needs it. + // A reconnect remount arrives with an empty terminal for the same reason a park does, and only + // a grid snapshot restores a full-screen application — see decideSshReattachPaintSource. + const prepaintSshModelSnapshot = (ptyId: string | null): void => { const parsedPtyId = ptyId ? parseAppSshPtyId(ptyId) : null if ( !ptyId || - !mountFollowsTerminalPark || + !paintsIntoEmptyTerminal || parsedPtyId?.connectionId !== connectionId || !capturedDirectSshRetryLeaseMatches() ) { @@ -8015,7 +8035,7 @@ export function connectPanePty( const capturedGeneration = authoritativeReattachGeneration const isCurrent = (): boolean => !disposed && - mountFollowsTerminalPark && + paintsIntoEmptyTerminal && authoritativeReattachGeneration === capturedGeneration && capturedDirectSshRetryLeaseMatches() const fetchSnapshot = getSshMainModelSnapshotProbe(ptyId) @@ -8584,7 +8604,7 @@ export function connectPanePty( const legacyWorkerOwnsPane = isLegacyWorkerAutomaticResumeBlocked() if (gate.enterDeferredFlow && (!legacyWorkerOwnsPane || !gate.sshConnected)) { // Paint main's parked model while SSH recovery continues off the render path. - prepaintParkedSshSnapshot(pendingSessionId) + prepaintSshModelSnapshot(pendingSessionId) void (async () => { // Why: for a passphrase target with no cached credential, don't auto-fire ssh.connect — a prompt popping just from focusing a tab / Cmd+J would surprise the user. // Wait for a user-initiated connect first; no-passphrase targets return false here and auto-connect as before. @@ -8947,7 +8967,7 @@ export function connectPanePty( if (deferredReattachSessionId) { allowInitialIdleCacheSeed = true recordPtyConnectDiagnostic(`pane=${pane.id} -> REATTACH ${deferredReattachSessionId}`) - prepaintParkedSshSnapshot(deferredReattachSessionId) + prepaintSshModelSnapshot(deferredReattachSessionId) // Why: pre-signal (declare) before the reattach connect so the cooperation gate suppresses the daemon seed for this paneKey; Electron preserves IPC order. // See docs/mobile-prefer-renderer-scrollback.md (Renderer-side prerequisite requirement #4). diff --git a/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.test.ts b/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.test.ts index faedd69c971..a4b824630c3 100644 --- a/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.test.ts +++ b/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.test.ts @@ -48,6 +48,40 @@ describe('shouldFetchSshReattachModelSnapshot', () => { shouldFetchSshReattachModelSnapshot({ ptyId: LOCAL_PTY_ID, sshParkingEnabled: true }) ).toBe(false) }) + + // A reconnect remount paints into an empty terminal exactly as a park does, and main keeps its + // model regardless of the parking setting — so turning view parking off must not strand a + // reconnect on the relay byte tail, which cannot rebuild a full-screen app. + it('fetches for a reconnect remount even with SSH parking disabled', () => { + expect( + shouldFetchSshReattachModelSnapshot({ + ptyId: SSH_PTY_ID, + sshParkingEnabled: false, + isSshReconnectRemount: true + }) + ).toBe(true) + }) + + // The pty gate is independent of the reason: a local pty has no relay and no SSH model. + it('still refuses a non-SSH pty on a reconnect remount', () => { + expect( + shouldFetchSshReattachModelSnapshot({ + ptyId: LOCAL_PTY_ID, + sshParkingEnabled: false, + isSshReconnectRemount: true + }) + ).toBe(false) + }) + + it('is unchanged for an ordinary mount that is neither parked nor reconnecting', () => { + expect( + shouldFetchSshReattachModelSnapshot({ + ptyId: SSH_PTY_ID, + sshParkingEnabled: false, + isSshReconnectRemount: false + }) + ).toBe(false) + }) }) describe('memoizeSshReattachModelSnapshotProbe', () => { @@ -154,4 +188,66 @@ describe('decideSshReattachPaintSource', () => { }) ).toBe('relay-replay') }) + + describe('a reconnect remount', () => { + // The defect this closes: a reconnect disposes the pane's xterm (tab.generation is its React + // key), so the pane repaints from the relay byte tail — which cannot rebuild an alt-screen app. + // Claude Code came back as fragments of a frame until the user resized it. + it('paints from the main model even with SSH parking disabled', () => { + expect( + decideSshReattachPaintSource({ + ptyId: SSH_PTY_ID, + sshParkingEnabled: false, + isSshReconnectRemount: true, + snapshot: headless + }) + ).toBe('main-model-snapshot') + }) + + // Every safety gate below the eligibility check still applies — widening WHY the model is + // trusted must not widen WHAT is trusted, or a reconnect paints a blank pane. + it.each([ + ['a null snapshot', null], + ['a renderer-sourced snapshot', { data: 'screen', source: 'renderer' as const }], + ['a sourceless snapshot', { data: 'screen' }], + ['an empty headless snapshot', { data: '', source: 'headless' as const }], + [ + 'a headless snapshot holding only a dangling escape', + { data: '', source: 'headless' as const, pendingEscapeTailAnsi: '\u001b[' } + ] + ])('still degrades to relay replay for %s', (_label, snapshot) => { + expect( + decideSshReattachPaintSource({ + ptyId: SSH_PTY_ID, + sshParkingEnabled: false, + isSshReconnectRemount: true, + snapshot + }) + ).toBe('relay-replay') + }) + + // An alt-screen app can carry all its content in scrollback with an empty screen frame; that is + // the shape a TUI reconnect actually produces, so it must not read as empty. + it('accepts a snapshot whose content is entirely scrollback', () => { + expect( + decideSshReattachPaintSource({ + ptyId: SSH_PTY_ID, + sshParkingEnabled: false, + isSshReconnectRemount: true, + snapshot: { data: '', source: 'headless', scrollbackAnsi: 'history' } + }) + ).toBe('main-model-snapshot') + }) + + it('still refuses a non-SSH pty', () => { + expect( + decideSshReattachPaintSource({ + ptyId: LOCAL_PTY_ID, + sshParkingEnabled: false, + isSshReconnectRemount: true, + snapshot: headless + }) + ).toBe('relay-replay') + }) + }) }) diff --git a/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts b/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts index c641332758c..1bdf4681212 100644 --- a/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts +++ b/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts @@ -33,20 +33,41 @@ export async function resolveSshReattachModelSnapshotWithTimeout( } /** - * Which payload paints an SSH reattach (C1 SSH-parking design gate). Only - * main's headless model is trusted: a 'renderer'-sourced snapshot serializes a - * mounted xterm, which no longer exists once the pane parked — anything but a - * non-empty headless snapshot degrades to the relay replay, never a blank paint. - * Emptiness is judged on the composed CONTENT (scrollback + screen): an - * alt-screen snapshot can carry all content in scrollbackAnsi with an empty - * screen frame. + * Which payload paints an SSH reattach. Only main's headless model is trusted: a + * 'renderer'-sourced snapshot serializes a mounted xterm, which no longer exists + * once the pane parked — anything but a non-empty headless snapshot degrades to + * the relay replay, never a blank paint. Emptiness is judged on the composed + * CONTENT (scrollback + screen): an alt-screen snapshot can carry all content in + * scrollbackAnsi with an empty screen frame. + * + * Two situations trust the model, for the same underlying reason — the pane is + * painting into a terminal that holds nothing: + * + * - PARKING (the original C1 gate), where the pane was unmounted on purpose. + * - AN SSH RECONNECT, where a reconnect bumps tab.generation, which is the + * pane's React key, so TerminalPane remounts and the old xterm is disposed + * with its buffer. + * + * The reconnect case is not optional polish. Relay replay is a byte TAIL, and a + * tail cannot reconstruct a full-screen application: it can begin mid-escape and + * it misses the alt-screen enter, the clears and the absolute positioning that + * built the frame. Replaying one paints fragments of a frame — which is what a + * reconnected Claude Code showed until the user resized it. The model snapshot + * is a serialized GRID, which is what tmux repaints on attach and the only + * payload that restores a TUI correctly. Main keeps that model regardless of the + * parking setting, so the reconnect case does not inherit its gate. */ export function decideSshReattachPaintSource(args: { ptyId: string sshParkingEnabled: boolean + /** This mount replaces a terminal disposed by an SSH reconnect remount. */ + isSshReconnectRemount?: boolean snapshot: SshReattachModelSnapshot | null }): SshReattachPaintSource { - if (!args.sshParkingEnabled || parseAppSshPtyId(args.ptyId) === null) { + if ( + (!args.sshParkingEnabled && !args.isSshReconnectRemount) || + parseAppSshPtyId(args.ptyId) === null + ) { return 'relay-replay' } if (!args.snapshot || args.snapshot.source !== 'headless') { @@ -65,8 +86,12 @@ export function decideSshReattachPaintSource(args: { export function shouldFetchSshReattachModelSnapshot(args: { ptyId: string sshParkingEnabled: boolean + isSshReconnectRemount?: boolean }): boolean { - return args.sshParkingEnabled && parseAppSshPtyId(args.ptyId) !== null + return ( + (args.sshParkingEnabled || args.isSshReconnectRemount === true) && + parseAppSshPtyId(args.ptyId) !== null + ) } /**