From 844fb035b5df099f89161cde657b38a29928d543 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 25 Jul 2026 14:21:26 -0700 Subject: [PATCH] fix(terminal): paint SSH reveal from main's model even when the relay replay is empty (C1 review #1) A relay restart empties the replay buffer; the reveal previously painted nothing even when main's headless model held the session. The reattach now prefetches the model snapshot when no structural replay exists (SSH-shaped ptys only) and paints it inside the coordinator; emptiness is judged on the composed payload (scrollbackAnsi + data + pendingEscapeTailAnsi) so an alt-screen snapshot with an empty screen frame still paints. Co-authored-by: Orca --- .../terminal-pane/pty-connection.ts | 26 ++++++++++++++----- .../ssh-reattach-model-restore.test.ts | 16 +++++++++++- .../ssh-reattach-model-restore.ts | 13 ++++++++-- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index f264e645819..ca8d24f4f0d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -7617,7 +7617,6 @@ export function connectPanePty( // Why: mobile streaming needs xterm's exact screen state; install the serializer + lastTitle source for main-process hydration parity. registerPaneSerializerFor(ptyId) - let reattachPayloadApplied = !hasStructuralReplay // Why (C1 SSH parking): main's headless model holds ~5k rows for SSH ptys // while the relay replay is a 100KiB raw-byte tail; prefer the model on // reveal. Only a non-empty 'headless'-sourced snapshot qualifies — the @@ -7644,6 +7643,18 @@ export function connectPanePty( } return snapshot } + // Why: a relay restart empties the replay buffer, but main's model may + // still hold the session — an SSH reattach probes it even with no replay + // so the reveal is never blank when main has content. Prefetched (before + // the payload task) so the coordinator route covers the paint. + let prefetchedSshModelSnapshot: PtyBufferSnapshot | null = null + if (!hasStructuralReplay && connectResult?.isReattach) { + prefetchedSshModelSnapshot = await fetchSshMainModelReattachSnapshot() + if (!isCurrentReattachPayload()) { + return false + } + } + let reattachPayloadApplied = !hasStructuralReplay && prefetchedSshModelSnapshot === null const applyReattachPayload = async (): Promise => { if (!isCurrentReattachPayload()) { return @@ -7688,8 +7699,9 @@ export function connectPanePty( window.api.pty.ackColdRestore(ptyId) } } - } else if (connectResult?.replay) { - const modelSnapshot = await fetchSshMainModelReattachSnapshot() + } else if (connectResult?.replay || prefetchedSshModelSnapshot) { + const modelSnapshot = + prefetchedSshModelSnapshot ?? (await fetchSshMainModelReattachSnapshot()) if (!isCurrentReattachPayload()) { return } @@ -7726,10 +7738,10 @@ export function connectPanePty( setRestoredSnapshotBaseline(ptyId, modelSnapshot) recordRendererOrderedSeq(modelSnapshot) sendFocusedReattachFocusInAfterReplay(ptyId, attemptGeneration) - if (connectResult.coldRestore && !isRemoteRuntimePtyId(ptyId)) { + if (connectResult?.coldRestore && !isRemoteRuntimePtyId(ptyId)) { window.api.pty.ackColdRestore(ptyId) } - } else { + } else if (connectResult?.replay) { rememberReattachPayloadAgentSignal(connectResult.replay, { fullScreenReplay: true }) // Relay replay may overlap xterm's pre-disconnect content; clear first to avoid duplication. writeReplayData('\x1b[2J\x1b[3J\x1b[H') @@ -7813,7 +7825,7 @@ export function connectPanePty( schedulePendingStartupCommandDelivery() } } - if (hasStructuralReplay) { + if (hasStructuralReplay || prefetchedSshModelSnapshot) { await waitForTerminalReplayWritesParsed(pane.terminal) if (!isCurrentReattachPayload()) { return @@ -7867,7 +7879,7 @@ export function connectPanePty( window.api.pty.signal(reattachPtyId, 'SIGWINCH') } } - if (hasStructuralReplay) { + if (hasStructuralReplay || prefetchedSshModelSnapshot) { await structuralReplayCoordinator.run(applyReattachPayload, { shouldRestore: isCurrentReattachPayload, afterRestore: fitAfterReattachRestore 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 021b7781e16..e57a321222c 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 @@ -39,7 +39,8 @@ describe('decideSshReattachPaintSource', () => { null, { data: 'screen', source: 'renderer' as const }, { data: 'screen' }, - { data: '', source: 'headless' as const } + { data: '', source: 'headless' as const }, + { data: '', scrollbackAnsi: '', pendingEscapeTailAnsi: '', source: 'headless' as const } ]) { expect( decideSshReattachPaintSource({ ptyId: SSH_PTY_ID, sshParkingEnabled: true, snapshot }) @@ -47,6 +48,19 @@ describe('decideSshReattachPaintSource', () => { } }) + it('judges emptiness on the composed payload, not the screen frame alone', () => { + // Why: an alt-screen model snapshot can hold all content in scrollbackAnsi + // with an empty screen; a dangling escape tail alone must also paint. + for (const snapshot of [ + { data: '', scrollbackAnsi: 'history', source: 'headless' as const }, + { data: '', pendingEscapeTailAnsi: '\x1b[', source: 'headless' as const } + ]) { + expect( + decideSshReattachPaintSource({ ptyId: SSH_PTY_ID, sshParkingEnabled: true, snapshot }) + ).toBe('main-model-snapshot') + } + }) + it('never upgrades when the kill switch is off or the pty is not SSH', () => { expect( decideSshReattachPaintSource({ 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 4ba28068b24..c6eb1ecab7b 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 @@ -5,6 +5,8 @@ export type SshReattachPaintSource = 'main-model-snapshot' | 'relay-replay' export type SshReattachModelSnapshot = { data: string source?: 'headless' | 'renderer' + scrollbackAnsi?: string + pendingEscapeTailAnsi?: string } /** @@ -12,6 +14,9 @@ export type SshReattachModelSnapshot = { * 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 paint payload (scrollback + screen + + * escape tail): an alt-screen snapshot can carry all content in scrollbackAnsi + * with an empty screen frame. */ export function decideSshReattachPaintSource(args: { ptyId: string @@ -21,10 +26,14 @@ export function decideSshReattachPaintSource(args: { if (!args.sshParkingEnabled || parseAppSshPtyId(args.ptyId) === null) { return 'relay-replay' } - if (!args.snapshot || args.snapshot.source !== 'headless' || args.snapshot.data.length === 0) { + if (!args.snapshot || args.snapshot.source !== 'headless') { return 'relay-replay' } - return 'main-model-snapshot' + const composedLength = + (args.snapshot.scrollbackAnsi?.length ?? 0) + + args.snapshot.data.length + + (args.snapshot.pendingEscapeTailAnsi?.length ?? 0) + return composedLength === 0 ? 'relay-replay' : 'main-model-snapshot' } /** Skip the snapshot fetch entirely when the paint could never use it. */