From 6eb7a2f00a1e65a009a4fd70ee3c26293dfed7a2 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:03:40 -0700 Subject: [PATCH] fix(terminal): report unavailable only when no serializer answered The previous commit reported `unavailable: no-serializable-buffer` whenever the serializer's answer was empty. Clients map that reason to retry-worthy and banner after the seventh such answer, so a genuinely empty pane -- a fresh one that has produced no output -- would retry and then show a loss banner. That is the case `accepts an empty snapshot as successful recovery without a loss banner` exists to prevent; it kept passing only because it stubs the transport and never sees what the host now sends. #14095 makes the client request a tagged restore on every recovery whose pushed snapshot was empty, so this path is routine rather than rare. `unavailable` means nobody answered. A serializer that answered with an empty buffer has answered. Restrict the reason to a null serializer on both paths, and keep the empty-content check for the payload and for seq/source metadata, which is the part that must not be published from an empty snapshot. --- src/main/runtime/rpc/methods/terminal.ts | 12 +++++++++--- ...l-requested-snapshot-unavailability.test.ts | 18 ++++++++++++++++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index b252eb2dd0c..667b64deb50 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -2352,8 +2352,11 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ pendingEscapeTailAnsi: serializedContent?.pendingEscapeTailAnsi, truncated: false, truncatedByByteBudget: serialized?.truncatedByByteBudget, - // Why: an absent or empty serializer answer is not proof the pane is empty. - unavailable: serializedContent ? undefined : 'no-serializable-buffer', + // Why: `unavailable` means nobody answered, so the client retries and can + // eventually banner. A serializer that DID answer with an empty buffer has + // answered — reporting that as unavailable banners genuinely empty panes. + // Empty content still suppresses the seq/source metadata below. + unavailable: serialized ? undefined : 'no-serializable-buffer', data: serializedContent?.data ?? '' }) } catch (error) { @@ -2695,8 +2698,11 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ kittyKeyboardFlags: serializedContent?.kittyKeyboardFlags, oscLinks: serializedContent?.oscLinks, pendingEscapeTailAnsi: serializedContent?.pendingEscapeTailAnsi, + // Why: same rule as the tagged path — only an absent serializer answer is + // "unavailable". A serializer that answered empty, with an empty retained + // tail, is a genuinely empty pane and must not be reported as a failure. unavailable: - serializedContent || read.tail.length > 0 ? undefined : 'no-serializable-buffer', + serialized || read.tail.length > 0 ? undefined : 'no-serializable-buffer', data: serializedContent?.data ?? retainedTail } ) diff --git a/src/main/runtime/rpc/terminal-requested-snapshot-unavailability.test.ts b/src/main/runtime/rpc/terminal-requested-snapshot-unavailability.test.ts index c970a439449..f86be1586d5 100644 --- a/src/main/runtime/rpc/terminal-requested-snapshot-unavailability.test.ts +++ b/src/main/runtime/rpc/terminal-requested-snapshot-unavailability.test.ts @@ -168,14 +168,28 @@ describe('requested terminal snapshot unavailability reasons', () => { expect(start.unavailable).toBeUndefined() }) - it('reports no-serializable-buffer for an empty snapshot object', async () => { + // Why: `unavailable` drives client retries and eventually a loss banner. A serializer + // that answered with an empty buffer HAS answered; a genuinely empty pane must not be + // reported as a failure or every fresh pane banners. Only an absent answer is a failure. + it('does not report a reason when the serializer answered with an empty buffer', async () => { const { start, chunks } = await requestSnapshotReply({ connectionId: 'conn-reason-empty', serializeRequested: async () => ({ data: '', cols: 120, rows: 40 }) }) expect(chunks).toBe('') expect(start).toMatchObject({ requestId: 77, truncated: false }) - expect(start.unavailable).toBe('no-serializable-buffer') + expect(start.unavailable).toBeUndefined() + }) + + // Why: an empty answer must still not publish seq/source metadata, or the client + // anchors its stream to a snapshot that carries no content. + it('omits seq and source metadata for an empty serialized answer', async () => { + const { start } = await requestSnapshotReply({ + connectionId: 'conn-reason-empty-metadata', + serializeRequested: async () => ({ data: '', cols: 120, rows: 40 }) + }) + expect(start.seq).toBeUndefined() + expect(start.source).toBeUndefined() }) it('accepts scrollback-only serialized content', async () => {