diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 3a950b90894..6324955b1ba 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -271,7 +271,8 @@ import { decideSshReattachPaintSource, memoizeSshReattachModelSnapshotProbe, resolveSshReattachModelSnapshotWithTimeout, - shouldFetchSshReattachModelSnapshot + shouldFetchSshReattachModelSnapshot, + sshReconnectPaintsFromModel } from './ssh-reattach-model-restore' import { readInFlightCommandCodeTurn } from './parked-terminal-command-status' import { @@ -8221,10 +8222,14 @@ export function connectPanePty( mountFollowsTerminalPark && (connectResult?.isReattach === true || isRemoteRuntimePtyId(ptyId)) // An SSH reconnect remounts the pane (tab.generation is its React key), so it also paints into - // a fresh xterm — but unlike a park it may only use the model for a FULL-SCREEN app. See the - // alternate-screen gate below for why. Consumed with the flag above: directSshRetryAttempt - // stays true for the whole tab generation, not just this mount, so reading it directly would - // re-arm on every later remount. + // a fresh xterm — but unlike a park it may only use the model for a FULL-SCREEN app. See + // sshReconnectPaintsFromModel for why. + // + // NOT consume-once, unlike mountFollowsTerminalPark: followsDirectSshReconnect is captured per + // connectPanePty and never cleared, so this re-arms if one connect reaches handleReattachResult + // twice. Bounded by connectStarted and by the emptiness/alt-screen gates rather than by the + // read itself. It still reads the PENDING retry rather than directSshRetryAttempt, which also + // accepts the live binding and so stays truthy for every later remount of the generation. const reconnectMayUseModel = followsDirectSshReconnect && !revealFollowsTerminalPark mountFollowsTerminalPark = false // Why: ordinary parking destroys xterm. Rebuild from the authoritative @@ -8327,14 +8332,11 @@ export function connectPanePty( // model still holds, but an in-place reattach (network reconnect, wake, // reload) already has that replay in hand, so probing would only delay its // paint by the timeout. Memoized, so this is never a second probe. - // Why an SSH reconnect may only use the model on the ALTERNATE SCREEN: the reconnect replay - // reaches the renderer without passing through main's model (forwardReattachReplay and the - // inline attach replay both bypass onPtyData), so at this moment the model is stale by - // exactly the outage. For a full-screen app that is the right trade — a byte tail cannot - // rebuild a frame it no longer contains, while a grid can, and the SIGWINCH the restore - // already sends makes the app repaint the delta itself. For a scrolling shell it is the - // wrong trade: the tail holds output the model never saw, and preferring the grid would - // drop it for good. A park has no such hole, so it keeps using the model either way. + // An SSH reconnect may use the model only under sshReconnectPaintsFromModel: the reconnect + // replay reaches the renderer without passing through main's model (forwardReattachReplay + // and the inline attach replay both bypass onPtyData), so the model is stale by exactly + // the outage and the replay is the only witness to it. A park has no such hole, so it + // keeps using the model either way. const revealSnapshot = revealFollowsTerminalPark ? (prefetchedParkModelSnapshot ?? (isRemoteRuntimePtyId(ptyId) ? null : await fetchSshMainModelReattachSnapshot())) @@ -8343,8 +8345,16 @@ export function connectPanePty( !revealSnapshot && reconnectMayUseModel ? await fetchSshMainModelReattachSnapshot() : null + const paintsReconnectFromModel = sshReconnectPaintsFromModel({ + snapshot: reconnectSnapshot, + replay: connectResult?.replay, + altFrameWouldBeSkipped: shouldSkipAltFrameForWidthMismatch( + reconnectSnapshot?.cols, + readProposedTerminalCols(pane) + ) + }) const modelSnapshot = - revealSnapshot ?? (reconnectSnapshot?.alternateScreen ? reconnectSnapshot : null) + revealSnapshot ?? (paintsReconnectFromModel ? reconnectSnapshot : null) if (!isCurrentReattachPayload()) { return } @@ -8373,6 +8383,13 @@ export function connectPanePty( kittyKeyboardFlags: modelSnapshot.kittyKeyboardFlags, snapshotSeq: modelSnapshot.seq }) + if (paintsReconnectFromModel && connectResult?.replay) { + // Why after the snapshot: painting the model discards the replay, but the app's kitty + // pushes during the outage exist ONLY there. Snapshot first for the pre-outage + // baseline, then the replay layers the outage on top — otherwise Option/Alt chords + // encode against a stale flag stack. + kittyKeyboardModes.scanReplay(connectResult.replay) + } // Why shared: park+reveal of an alt-screen TUI needs the same // ?1049l/?1049h rebuild as applyMainBufferSnapshot (main strips // the ?1049h marker when splitting scrollbackAnsi) — inlined here 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..09f262fd763 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 @@ -61,6 +61,59 @@ export function decideSshReattachPaintSource(args: { return contentLength === 0 ? 'relay-replay' : 'main-model-snapshot' } +/** + * The last alternate-screen transition in a relay replay, or null if it holds none. + * + * A reconnect replay is the ONLY record of what happened while the client was gone — main's model + * never saw those bytes — so it is the authority on which screen the app is on now, however stale + * the rest of it is. 47 and 1047 count alongside 1049: older apps still use them, and any of the + * three leaving reset means the frame is gone. + */ +export function lastAlternateScreenTransition( + replay: string | undefined +): 'entered' | 'exited' | null { + if (!replay) { + return null + } + let transition: 'entered' | 'exited' | null = null + // A tail can begin mid-escape, so match forward rather than assuming a well-formed start. + for (const match of replay.matchAll(/\x1b\[\?([0-9;]*)([hl])/g)) { + if (match[1].split(';').some((mode) => mode === '47' || mode === '1047' || mode === '1049')) { + transition = match[2] === 'h' ? 'entered' : 'exited' + } + } + return transition +} + +/** + * Whether an SSH RECONNECT paints main's model instead of the relay tail. + * + * Only for a full-screen app, and only when the replay does not contradict the model. A tail cannot + * rebuild a frame whose start it no longer contains, while a grid can; a scrolling shell is the + * opposite, since the tail holds outage output the model never saw and the grid would drop it. + * + * The `exited` veto is the case this gate exists to refuse: if the app left the alternate screen + * while the client was gone, the model still reports alternateScreen because it never consumed + * those bytes. Painting it would freeze a frame of an application that no longer exists AND discard + * the replay carrying the shell's real output — strictly worse than the fragments this fix targets. + */ +export function sshReconnectPaintsFromModel(args: { + snapshot: { alternateScreen?: boolean } | null + replay: string | undefined + altFrameWouldBeSkipped: boolean +}): boolean { + if (!args.snapshot?.alternateScreen) { + return false + } + if (!args.replay) { + // Nothing to degrade to, so the vetoes below would only trade a stale frame for a blank one. + return true + } + // The alt frame is dropped for a width mismatch, leaving a cleared screen the app must repaint. + // A park could afford that with no tail to lose; here it would mean discarding a usable one. + return !args.altFrameWouldBeSkipped && lastAlternateScreenTransition(args.replay) !== 'exited' +} + /** Skip the snapshot fetch entirely when the paint could never use it. */ export function shouldFetchSshReattachModelSnapshot(args: { ptyId: string diff --git a/src/renderer/src/components/terminal-pane/ssh-reconnect-model-paint-gate.test.ts b/src/renderer/src/components/terminal-pane/ssh-reconnect-model-paint-gate.test.ts new file mode 100644 index 00000000000..838ea03e789 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/ssh-reconnect-model-paint-gate.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from 'vitest' + +import { + lastAlternateScreenTransition, + sshReconnectPaintsFromModel +} from './ssh-reattach-model-restore' + +/** + * The gate deciding whether an SSH RECONNECT repaints from main's grid or the relay byte tail. + * + * Worth testing directly rather than through a reattach: the inputs are a stale belief (the model's + * alternate-screen flag, which predates the outage) and the only witness to the outage (the replay). + * Every interesting case is a disagreement between those two, which is awkward to stage end-to-end + * and trivial to state here. + */ +const ALT_ON = '\x1b[?1049h' +const ALT_OFF = '\x1b[?1049l' + +describe('lastAlternateScreenTransition', () => { + it('reports nothing for a replay with no screen change', () => { + expect(lastAlternateScreenTransition('plain output\r\n$ ')).toBeNull() + expect(lastAlternateScreenTransition('')).toBeNull() + expect(lastAlternateScreenTransition(undefined)).toBeNull() + }) + + it('reports the LAST transition, not the first', () => { + expect(lastAlternateScreenTransition(`${ALT_ON}frame${ALT_OFF}$ `)).toBe('exited') + expect(lastAlternateScreenTransition(`${ALT_OFF}$ vim\r\n${ALT_ON}frame`)).toBe('entered') + }) + + it('counts the legacy 47 and 1047 modes', () => { + // Older apps still use these, and any of the three leaving means the frame is gone. + expect(lastAlternateScreenTransition('\x1b[?47l')).toBe('exited') + expect(lastAlternateScreenTransition('\x1b[?1047h')).toBe('entered') + }) + + it('finds the mode inside a multi-parameter DECSET', () => { + expect(lastAlternateScreenTransition('\x1b[?1049;1006;2004h')).toBe('entered') + }) + + it('ignores unrelated private modes', () => { + // Bracketed paste and cursor visibility must not be read as screen changes. + expect(lastAlternateScreenTransition('\x1b[?2004h\x1b[?25l')).toBeNull() + // 10491 merely CONTAINS 1049 as a substring. + expect(lastAlternateScreenTransition('\x1b[?10491h')).toBeNull() + }) + + it('still scans a tail that begins mid-escape', () => { + // The relay tail is cut at a byte offset, so its first bytes are routinely a partial sequence. + expect(lastAlternateScreenTransition(`49;1006h garbage ${ALT_OFF}`)).toBe('exited') + }) +}) + +describe('sshReconnectPaintsFromModel', () => { + const args = (overrides: Partial[0]> = {}) => ({ + snapshot: { alternateScreen: true }, + replay: 'some output', + altFrameWouldBeSkipped: false, + ...overrides + }) + + it('paints a full-screen app from the grid', () => { + // The reported bug: a tail cannot rebuild a frame whose start it no longer contains. + expect(sshReconnectPaintsFromModel(args())).toBe(true) + }) + + it('leaves a scrolling shell on the tail', () => { + // Inverted trade: the tail holds outage output the model never saw, and the grid would drop it. + expect(sshReconnectPaintsFromModel(args({ snapshot: { alternateScreen: false } }))).toBe(false) + expect(sshReconnectPaintsFromModel(args({ snapshot: {} }))).toBe(false) + expect(sshReconnectPaintsFromModel(args({ snapshot: null }))).toBe(false) + }) + + it('refuses the grid when the app LEFT the alternate screen during the outage', () => { + // The model never consumed the outage, so it still claims alternateScreen. Painting it would + // freeze a frame of an application that no longer exists and discard the shell's real output. + expect( + sshReconnectPaintsFromModel(args({ replay: `${ALT_OFF}\r\n$ echo done\r\ndone\r\n$ ` })) + ).toBe(false) + }) + + it('keeps the grid when the app re-entered the alternate screen after leaving', () => { + expect(sshReconnectPaintsFromModel(args({ replay: `${ALT_OFF}$ vim\r\n${ALT_ON}frame` }))).toBe( + true + ) + }) + + it('keeps the grid when the replay says nothing about the screen', () => { + // The common case: an idle TUI whose tail carries only redraws. + expect(sshReconnectPaintsFromModel(args({ replay: 'redraw redraw' }))).toBe(true) + }) + + it('degrades to the tail when the alt frame would be dropped for a width mismatch', () => { + // That guard leaves only a cleared screen for the app to repaint. A park can afford it with no + // tail to lose; here it would mean having discarded a usable one for a blank pane. + expect(sshReconnectPaintsFromModel(args({ altFrameWouldBeSkipped: true }))).toBe(false) + }) + + it('still paints the grid when there is no tail to degrade to', () => { + // Both vetoes exist to protect a usable replay. With none, they would only trade a stale frame + // for a blank one. + expect( + sshReconnectPaintsFromModel(args({ replay: undefined, altFrameWouldBeSkipped: true })) + ).toBe(true) + expect(sshReconnectPaintsFromModel(args({ replay: '', altFrameWouldBeSkipped: true }))).toBe( + true + ) + }) + + it('refuses a normal-buffer model even with no tail', () => { + expect( + sshReconnectPaintsFromModel(args({ snapshot: { alternateScreen: false }, replay: '' })) + ).toBe(false) + }) +}) diff --git a/tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts b/tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts index 2b8bf71aab5..273a149fd4a 100644 --- a/tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts +++ b/tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts @@ -113,10 +113,16 @@ test.describe('SSH reconnect pane restore', () => { const freshContent = await getTerminalContent(orcaPage, 8000) expect(freshContent).not.toContain(marker) - // A FULL-SCREEN app is the third case, and the one a byte tail cannot serve: a tail can begin - // mid-escape and misses the alt-screen enter and absolute positioning that built the frame, so - // replaying it paints fragments. `top` is used because it is present on any Linux image and - // repaints on a fixed interval, so a coherent frame after the reconnect is unambiguous. + // A FULL-SCREEN app is the third case: a reconnect must leave a TUI pane alive and drawing, + // not blank or frozen. + // + // SCOPE, because it is easy to over-read: this does NOT prove which payload painted the pane. + // top redraws itself every few seconds, so these assertions pass whichever way the paint-source + // gate decided — including with it reverted. What discriminates model-vs-tail is unit-level, in + // ssh-reconnect-model-paint-gate.test.ts, because the interesting cases are disagreements + // between main's pre-outage alt-screen belief and a replay produced during the outage, which + // is not something this fixture can stage. Kept anyway: it is the only coverage that a + // reconnected TUI pane recovers at all. await execInTerminal(orcaPage, freshPtyId, 'top -b -n 1 > /dev/null; top') await waitForTerminalOutput(orcaPage, 'load average', 30_000, 8000) @@ -124,8 +130,6 @@ test.describe('SSH reconnect pane restore', () => { await waitForActiveTerminalManager(orcaPage, 60_000) await waitForActivePanePtyId(orcaPage, 60_000) - // The header only survives as a whole if a grid was repainted; a tail that lost the frame - // start shows rows without it. await waitForTerminalOutput(orcaPage, 'load average', 60_000, 8000) const tuiContent = await getTerminalContent(orcaPage, 8000) expect(tuiContent).toContain('PID')