From b43b6cd108f425cdfd79bbe1384f0f6aa64ad634 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:34:10 -0700 Subject: [PATCH] fix(terminal): restore retained tail for empty snapshots --- src/main/runtime/rpc/methods/terminal.ts | 60 ++++--- ...ultiplex-initial-snapshot-fallback.test.ts | 155 ++++++++++++++++++ ...-requested-snapshot-unavailability.test.ts | 18 +- 3 files changed, 206 insertions(+), 27 deletions(-) create mode 100644 src/main/runtime/rpc/terminal-multiplex-initial-snapshot-fallback.test.ts diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 69f4eb72190..b252eb2dd0c 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -118,6 +118,12 @@ type SerializedSnapshot = { kittyKeyboardFlags?: number } | null +function hasSerializedSnapshotContent( + snapshot: SerializedSnapshot +): snapshot is Exclude { + return Boolean(snapshot && (snapshot.data.length > 0 || snapshot.scrollbackAnsi)) +} + type TerminalViewportClient = { id: string type?: 'mobile' | 'desktop' @@ -2330,24 +2336,25 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ return } } - sentSnapshotOutputSeq = serialized?.seq + const serializedContent = hasSerializedSnapshotContent(serialized) ? serialized : null + sentSnapshotOutputSeq = serializedContent?.seq sendSnapshotFrames((opcode, payload) => sendFrame(stream.streamId, opcode, payload), { kind: 'scrollback', cols: serialized?.cols ?? size?.cols ?? 80, rows: serialized?.rows ?? size?.rows ?? 24, requestId, displayMode, - seq: serialized?.seq, - cwd: serialized?.cwd, - source: serialized?.source, - kittyKeyboardFlags: serialized?.kittyKeyboardFlags, - oscLinks: serialized?.oscLinks, - pendingEscapeTailAnsi: serialized?.pendingEscapeTailAnsi, + seq: serializedContent?.seq, + cwd: serializedContent?.cwd, + source: serializedContent?.source, + kittyKeyboardFlags: serializedContent?.kittyKeyboardFlags, + oscLinks: serializedContent?.oscLinks, + pendingEscapeTailAnsi: serializedContent?.pendingEscapeTailAnsi, truncated: false, 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', - data: serialized?.data ?? '' + // Why: an absent or empty serializer answer is not proof the pane is empty. + unavailable: serializedContent ? undefined : 'no-serializable-buffer', + data: serializedContent?.data ?? '' }) } catch (error) { sendStreamError( @@ -2637,8 +2644,9 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ const size = runtime.getTerminalSize(ptyId) const displayMode = runtime.getMobileDisplayMode(ptyId) const layoutSeq = runtime.getLayout(ptyId)?.seq + const serializedContent = hasSerializedSnapshotContent(serialized) ? serialized : null // Why: layout versions and output offsets are different sequence domains. - const snapshotOutputSeq = serialized?.seq + const snapshotOutputSeq = serializedContent?.seq emit({ type: 'subscribed', streamId: request.streamId, @@ -2659,18 +2667,19 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ }) stream.sourceRangeReplacement = stream.ackOutputSourceRanges && - serialized?.source !== undefined && - typeof serialized.seq === 'number' + serializedContent?.source !== undefined && + typeof serializedContent.seq === 'number' ? runtime.reserveRemoteTerminalSourceRangeReplacement( { ptyId, consumerId: stream.remoteDesktopSubscriptionKey, streamGeneration: stream.streamGeneration }, - serialized.seq, + serializedContent.seq, 'initial-snapshot' ) : null + const retainedTail = read.tail.length > 0 ? `${read.tail.join('\r\n')}\r\n` : '' const snapshotPublication = sendSnapshotFrames( (opcode, payload) => sendFrame(request.streamId, opcode, payload), { @@ -2679,15 +2688,16 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ rows: serialized?.rows ?? size?.rows ?? 24, displayMode, seq: snapshotOutputSeq, - cwd: serialized?.cwd, + cwd: serializedContent?.cwd, truncated: initialOutputOverflowed, truncatedByByteBudget: serialized?.truncatedByByteBudget, - source: serialized?.source, - kittyKeyboardFlags: serialized?.kittyKeyboardFlags, - oscLinks: serialized?.oscLinks, - pendingEscapeTailAnsi: serialized?.pendingEscapeTailAnsi, - data: - serialized?.data ?? (read.tail.length > 0 ? `${read.tail.join('\r\n')}\r\n` : '') + source: serializedContent?.source, + kittyKeyboardFlags: serializedContent?.kittyKeyboardFlags, + oscLinks: serializedContent?.oscLinks, + pendingEscapeTailAnsi: serializedContent?.pendingEscapeTailAnsi, + unavailable: + serializedContent || read.tail.length > 0 ? undefined : 'no-serializable-buffer', + data: serializedContent?.data ?? retainedTail } ) const replacement = stream.sourceRangeReplacement @@ -2695,11 +2705,11 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (replacement) { const committed = snapshotPublication.published && - serialized?.source !== undefined && - typeof serialized.seq === 'number' && + serializedContent?.source !== undefined && + typeof serializedContent.seq === 'number' && runtime.commitRemoteTerminalSourceRangeReplacement(replacement, { - source: serialized.source, - seq: serialized.seq + source: serializedContent.source, + seq: serializedContent.seq }) if (!committed) { runtime.rollbackRemoteTerminalSourceRangeReplacement( diff --git a/src/main/runtime/rpc/terminal-multiplex-initial-snapshot-fallback.test.ts b/src/main/runtime/rpc/terminal-multiplex-initial-snapshot-fallback.test.ts new file mode 100644 index 00000000000..7894a2e59a0 --- /dev/null +++ b/src/main/runtime/rpc/terminal-multiplex-initial-snapshot-fallback.test.ts @@ -0,0 +1,155 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RuntimeTerminalWait } from '../../../shared/runtime-types' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + decodeTerminalStreamText, + encodeTerminalStreamFrame, + encodeTerminalStreamJson +} from '../../../shared/terminal-stream-protocol' +import type { OrcaRuntimeService } from '../orca-runtime' +import type { RpcRequest } from './core' +import { RpcDispatcher } from './dispatcher' +import { TERMINAL_METHODS } from './methods/terminal' + +type SerializedBuffer = { + data: string + scrollbackAnsi?: string + cols: number + rows: number + seq?: number + source?: 'headless' | 'renderer' +} | null + +function makeRequest(method: string, params?: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +async function subscribeSnapshot(options: { + connectionId: string + serialized: SerializedBuffer + tail: string[] +}): Promise<{ start: Record; data: string }> { + const messages: string[] = [] + const binaryFrames: Uint8Array[] = [] + const handlers = new Map< + number, + (frame: NonNullable>) => void + >() + const cleanups = new Map void>() + const runtime = { + getRuntimeId: () => 'test-runtime', + registerRemoteTerminalViewSubscriber: () => () => {}, + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + requestRendererTerminalTabMount: vi.fn().mockReturnValue(true), + updateRemoteDesktopViewer: vi.fn().mockResolvedValue(true), + unregisterRemoteDesktopViewer: vi.fn().mockResolvedValue(true), + unregisterRemoteDesktopViewers: vi.fn().mockResolvedValue(true), + isPtyResizeDrivenRemotely: vi.fn().mockReturnValue(false), + getRemoteDesktopFitHold: vi.fn().mockReturnValue({ mode: 'desktop-fit', cols: 120, rows: 40 }), + isRemoteDesktopViewerOwner: vi.fn().mockReturnValue(false), + getPtyOutputSequence: vi.fn().mockReturnValue(0), + readTerminal: vi.fn().mockResolvedValue({ tail: options.tail, truncated: false }), + serializeTerminalBuffer: vi.fn().mockResolvedValue(options.serialized), + getTerminalSize: vi.fn().mockReturnValue({ cols: 120, rows: 40 }), + getMobileDisplayMode: vi.fn().mockReturnValue('auto'), + getLayout: vi.fn().mockReturnValue({ seq: 1 }), + subscribeToTerminalData: vi.fn().mockReturnValue(vi.fn()), + subscribeToTerminalResize: vi.fn().mockReturnValue(vi.fn()), + subscribeToFitOverrideChanges: vi.fn().mockReturnValue(vi.fn()), + subscribeToDriverChanges: vi.fn().mockReturnValue(vi.fn()), + getTerminalFitOverride: vi.fn().mockReturnValue(null), + getDriver: vi.fn().mockReturnValue({ kind: 'idle' }), + registerSubscriptionCleanup: vi.fn((id: string, cleanup: () => void) => { + cleanups.set(id, cleanup) + }), + cleanupSubscription: vi.fn((id: string) => cleanups.get(id)?.()), + waitForTerminal: vi.fn(() => new Promise(() => {})) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + const dispatchPromise = dispatcher.dispatchStreaming( + makeRequest('terminal.multiplex', {}), + (message) => messages.push(message), + { + connectionId: options.connectionId, + sendBinary: (bytes) => { + binaryFrames.push(bytes) + }, + registerBinaryStreamHandler: (streamId, handler) => { + handlers.set(streamId, handler) + return () => handlers.delete(streamId) + } + } + ) + + await vi.waitFor(() => + expect(messages.some((message) => JSON.parse(message).result?.type === 'ready')).toBe(true) + ) + handlers.get(0)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Subscribe, + streamId: 0, + seq: 1, + payload: encodeTerminalStreamJson({ + streamId: 7, + terminal: 'terminal-1', + client: { id: 'desktop-1', type: 'desktop' } + }) + }) + )! + ) + await vi.waitFor(() => + expect(messages.some((message) => JSON.parse(message).result?.type === 'subscribed')).toBe(true) + ) + + const frames = binaryFrames + .map((bytes) => decodeTerminalStreamFrame(bytes)) + .filter((frame) => frame?.streamId === 7) + const start = frames.find((frame) => frame?.opcode === TerminalStreamOpcode.SnapshotStart)! + const data = frames + .filter((frame) => frame?.opcode === TerminalStreamOpcode.SnapshotChunk) + .map((frame) => (frame ? decodeTerminalStreamText(frame.payload) : '')) + .join('') + cleanups.get(`terminal-multiplex:${options.connectionId}`)?.() + await dispatchPromise + return { start: decodeTerminalStreamJson>(start.payload)!, data } +} + +describe('terminal multiplex initial snapshot fallback', () => { + it('serves the retained tail when serialization returns an empty snapshot object', async () => { + const snapshot = await subscribeSnapshot({ + connectionId: 'conn-empty-snapshot-tail', + serialized: { data: '', cols: 120, rows: 40, seq: 42, source: 'renderer' }, + tail: ['retained line 1', 'retained line 2'] + }) + + expect(snapshot.data).toBe('retained line 1\r\nretained line 2\r\n') + expect(snapshot.start.unavailable).toBeUndefined() + expect(snapshot.start.seq).toBeUndefined() + expect(snapshot.start.source).toBeUndefined() + }) + + it('keeps the retained-tail fallback when serialization returns null', async () => { + const snapshot = await subscribeSnapshot({ + connectionId: 'conn-null-snapshot-tail', + serialized: null, + tail: ['retained line'] + }) + + expect(snapshot.data).toBe('retained line\r\n') + expect(snapshot.start.unavailable).toBeUndefined() + }) + + it('reports unavailable when neither serialized data nor a retained tail exists', async () => { + const snapshot = await subscribeSnapshot({ + connectionId: 'conn-null-snapshot-no-tail', + serialized: null, + tail: [] + }) + + expect(snapshot.data).toBe('') + expect(snapshot.start.unavailable).toBe('no-serializable-buffer') + }) +}) 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 c5f05cb0456..c970a439449 100644 --- a/src/main/runtime/rpc/terminal-requested-snapshot-unavailability.test.ts +++ b/src/main/runtime/rpc/terminal-requested-snapshot-unavailability.test.ts @@ -13,7 +13,7 @@ import { encodeTerminalStreamJson } from '../../../shared/terminal-stream-protocol' -type SerializedBuffer = { data: string; cols: number; rows: number } | null +type SerializedBuffer = { data: string; scrollbackAnsi?: string; cols: number; rows: number } | null type SnapshotStartPayload = Record // Why: 256 KiB is the pending-output budget, so this many 1 KiB chunks always trips the overflow guard. @@ -168,13 +168,27 @@ describe('requested terminal snapshot unavailability reasons', () => { expect(start.unavailable).toBeUndefined() }) - it('omits a reason for a proven-empty buffer so absence stays distinguishable from failure', async () => { + it('reports no-serializable-buffer for an empty snapshot object', 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') + }) + + it('accepts scrollback-only serialized content', async () => { + const { start, chunks } = await requestSnapshotReply({ + connectionId: 'conn-reason-scrollback', + serializeRequested: async () => ({ + data: '', + scrollbackAnsi: 'restored scrollback', + cols: 120, + rows: 40 + }) + }) + expect(chunks).toBe('restored scrollback') expect(start.unavailable).toBeUndefined() })