refactor(terminal): drop the scrollback flag; the wipe was never justified here

The previous commit added a host flag so the client could tell whether a
recovery payload carried scrollback. It cannot: every payload reaching that
branch is screen-only by construction. The ack-recovery emit serializes with
scrollbackRows: 0 literally, and the untagged resync request carries no
scrollback either. A payload that does carry scrollback arrives tagged, and the
request branch never clears.

So the flag was always false where it mattered. Delete it -- the wire field, the
host helper, the parse, and the conditional -- and simply stop emitting the
scrollback erase on this path. No wire change at all now.
This commit is contained in:
Brennan Benson
2026-08-14 16:14:06 -07:00
parent a88cba81e6
commit 3f1fd2cf8a
4 changed files with 9 additions and 50 deletions
-16
View File
@@ -101,17 +101,6 @@ type SnapshotFrameOptions = {
pendingEscapeTailAnsi?: string
/** Effective kitty flags proven at this frame's own `seq`. */
kittyKeyboardFlags?: number
/**
* Whether `data` carries scrollback as well as the visible screen. Desktop
* snapshots are serialized with `scrollbackRows: 0`, so a client that erases
* its own scrollback to apply one destroys history the frame cannot replace.
*/
scrollbackIncluded?: boolean
}
/** Whether a serialized snapshot's payload carries scrollback, not just the visible screen. */
function snapshotCarriesScrollback(snapshot: SerializedSnapshot): boolean {
return Boolean(snapshot && (snapshot.scrollbackRows > 0 || snapshot.scrollbackAnsi))
}
type SerializedSnapshot = {
@@ -671,9 +660,6 @@ function sendSnapshotFrames(
...(typeof options.seq === 'number' && options.kittyKeyboardFlags !== undefined
? { kittyKeyboardFlags: options.kittyKeyboardFlags }
: {}),
// Why additive on the same rule: absence means "unknown", and a client
// that cannot read it keeps its existing behavior.
...(options.scrollbackIncluded === true ? { scrollbackIncluded: true } : {}),
truncated: options.truncated === true,
truncatedByByteBudget: options.truncatedByByteBudget === true
})
@@ -1868,7 +1854,6 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [
source: serialized.source,
kittyKeyboardFlags: serialized.kittyKeyboardFlags,
truncatedByByteBudget: serialized.truncatedByByteBudget,
scrollbackIncluded: snapshotCarriesScrollback(serialized),
data: serialized.data
}
)
@@ -2362,7 +2347,6 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [
truncatedByByteBudget: serialized?.truncatedByByteBudget,
// Why: no serializer answered, which is not proof the pane is empty — say so instead of passing off '' as the buffer.
unavailable: serialized ? undefined : 'no-serializable-buffer',
scrollbackIncluded: snapshotCarriesScrollback(serialized),
data: serialized?.data ?? ''
})
} catch (error) {
@@ -16,9 +16,7 @@ import { replaceRuntimeEnvironmentRevisions } from './runtime-environment-revisi
// Why screen-only: a resync reply is serialized with `scrollbackRows: 0`, so it
// carries the visible screen and nothing else. Erasing the client's scrollback
// (`\x1b[3J`) to apply it would destroy history the frame cannot put back. The
// host sets `scrollbackIncluded` when a payload does carry scrollback, and only
// then is the full clear correct.
// (`\x1b[3J`) to apply it would destroy history the frame cannot put back.
const RECOVERY_SCREEN_CLEAR = '\x1b[2J\x1b[H'
// Why: reproduces the silent frame-drop corruption. The server multiplex path
@@ -217,10 +217,6 @@ type RemoteRuntimeSnapshotInfo = {
// must write it AFTER the replay reset so the next live chunk completes it
// instead of rendering literally (#7329).
pendingEscapeTailAnsi?: string
// Why: whether the payload carries scrollback as well as the screen. Absent
// from older hosts, which is why the recovery path treats false as "do not
// erase scrollback" rather than "there is none".
scrollbackIncluded?: boolean
}
type RemoteRuntimeSnapshotRequest = {
@@ -908,19 +904,19 @@ class RemoteRuntimeTerminalMultiplexer {
kittyKeyboardFlags: info?.kittyKeyboardFlags
})
} else if (target === 'recovery') {
// Why: a recovery repaint must never destroy more than it replaces.
// `\x1b[3J` erases xterm's scrollback, but desktop snapshots are
// serialized with `scrollbackRows: 0`, so the frame cannot put that
// history back — clear the screen alone unless the host says this
// payload carries scrollback. An absent flag means an older host,
// which never sends scrollback on this path either.
const clearAnsi = info?.scrollbackIncluded ? '\x1b[2J\x1b[3J\x1b[H' : '\x1b[2J\x1b[H'
// Why the screen only, never `\x1b[3J`: that erases xterm's scrollback,
// and every payload reaching this branch is screen-only by
// construction — the ack-recovery emit serializes with
// `scrollbackRows: 0`, and the untagged resync request carries no
// scrollback either. Erasing history the frame cannot put back is
// silent data loss. A payload that DOES carry scrollback arrives
// tagged, and the request branch above never clears.
// Why not an empty repaint: with no payload there is nothing to put
// on screen, so applying one only blanks a pane the host could not
// describe. A mid-escape tail still has to replay so the next live
// chunk completes it instead of rendering literally (#7329).
if (data) {
stream.callbacks.onSnapshot(`${clearAnsi}${data}`, {
stream.callbacks.onSnapshot(`\x1b[2J\x1b[H${data}`, {
pendingEscapeTailAnsi: info?.pendingEscapeTailAnsi,
seq: info?.seq,
kittyKeyboardFlags: info?.kittyKeyboardFlags
@@ -1565,7 +1561,6 @@ function decodeSnapshotInfo(
requestId?: unknown
truncated?: unknown
unavailable?: unknown
scrollbackIncluded?: unknown
pendingEscapeTailAnsi?: unknown
kittyKeyboardFlags?: unknown
}>(payload)
@@ -1582,7 +1577,6 @@ function decodeSnapshotInfo(
requestId: typeof raw.requestId === 'number' ? raw.requestId : undefined,
truncated: raw.truncated === true,
unavailable: parseTerminalSnapshotUnavailableReason(raw.unavailable),
scrollbackIncluded: raw.scrollbackIncluded === true,
pendingEscapeTailAnsi:
typeof raw.pendingEscapeTailAnsi === 'string' ? raw.pendingEscapeTailAnsi : undefined
}
@@ -595,23 +595,6 @@ describe('remote runtime terminal multiplex ACK gate', () => {
})
expect(onSubscribed).toHaveBeenCalledTimes(1)
// Why scrollback may be erased here: the host states the payload carries
// it, so the frame replaces the history it removes.
injectSnapshot(
{
kind: 'scrollback',
cols: 120,
rows: 40,
reason: 'ack-pending-overflow',
truncated: false,
scrollbackIncluded: true
},
'restored with history'
)
expect(onSnapshot).toHaveBeenCalledWith(`\x1b[2J\x1b[3J\x1b[H${'restored with history'}`, {
pendingEscapeTailAnsi: undefined
})
// Why nothing at all: this replaced the previous rule, which read an empty
// payload as an authoritative blank and cleared on it. An empty payload is
// the host failing to describe the pane, not proof the pane is empty, and