From 8a20f55bc8abd50f7c6ece01de845c8155b9dc9b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 14 Aug 2026 18:42:39 -0700 Subject: [PATCH] fix(terminal): bound recovery snapshot retries --- ...runtime-terminal-frame-drop-resync.test.ts | 59 ++++++++++++++++++- .../remote-runtime-terminal-multiplexer.ts | 8 ++- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/runtime/remote-runtime-terminal-frame-drop-resync.test.ts b/src/renderer/src/runtime/remote-runtime-terminal-frame-drop-resync.test.ts index 777dfc71e0c..3a11456fb5a 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-frame-drop-resync.test.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-frame-drop-resync.test.ts @@ -49,8 +49,10 @@ class FakeMultiplexServer { holdNextRecoverySnapshot = false emptyNextRecoverySnapshot = false unavailableNextRecoverySnapshot = false + unavailableRecoverySnapshots = false pendingEscapeTailAnsiNextRecoverySnapshot: string | undefined snapshotRequests: (number | undefined)[] = [] + unsubscribeRequests = 0 private heldManualRequestId: number | null = null private snapshotData = 'INITIAL' @@ -90,7 +92,10 @@ class FakeMultiplexServer { this.emptyNextRecoverySnapshot = false this.snapshotData = '' } - if (typeof payload?.requestId !== 'number' && this.unavailableNextRecoverySnapshot) { + if ( + typeof payload?.requestId !== 'number' && + (this.unavailableNextRecoverySnapshot || this.unavailableRecoverySnapshots) + ) { this.unavailableNextRecoverySnapshot = false this.snapshotData = '' recoveryOptions.unavailable = 'no-serializable-buffer' @@ -118,6 +123,10 @@ class FakeMultiplexServer { return } this.sendSnapshot(payload?.requestId, recoveryOptions) + return + } + if (frame.opcode === TerminalStreamOpcode.Unsubscribe) { + this.unsubscribeRequests += 1 } } @@ -234,7 +243,9 @@ describe('remote terminal frame-drop resync', () => { vi.unstubAllGlobals() }) - async function subscribeClient(): Promise<{ + async function subscribeClient( + onTransportClose?: (event: { recoverable: boolean }) => void + ): Promise<{ data: string[] metas: { seq?: number; rawLength?: number; transformed?: boolean }[] snapshots: string[] @@ -257,7 +268,8 @@ describe('remote terminal frame-drop resync', () => { onSnapshot: (chunk, meta) => { snapshots.push(chunk) snapshotMetas.push(meta ?? {}) - } + }, + onTransportClose } }) // Let the initial snapshot round-trip settle. @@ -347,6 +359,47 @@ describe('remote terminal frame-drop resync', () => { } }) + it('replaces the stream after five unavailable recovery attempts', async () => { + vi.useFakeTimers() + try { + const onTransportClose = vi.fn() + const first = await subscribeClient(onTransportClose) + const unavailableServer = server + unavailableServer.unavailableRecoverySnapshots = true + + unavailableServer.output('aaa') + unavailableServer.dropNextOutput = true + unavailableServer.output('bbb') + unavailableServer.output('ccc') + unavailableServer.output('corrupt-live-tail') + + expect(unavailableServer.snapshotRequests).toEqual([undefined]) + expect(first.snapshots).toEqual(['INITIAL']) + expect(first.data).toEqual(['aaa']) + + await vi.advanceTimersByTimeAsync(500 + 1_000 + 2_000 + 4_000) + + expect(unavailableServer.snapshotRequests).toEqual(Array(5).fill(undefined)) + expect(unavailableServer.unsubscribeRequests).toBe(1) + expect(onTransportClose).toHaveBeenCalledOnce() + expect(onTransportClose).toHaveBeenCalledWith({ recoverable: true }) + expect(first.snapshots).toEqual(['INITIAL']) + expect(first.data).toEqual(['aaa']) + expect(vi.getTimerCount()).toBe(0) + + await vi.advanceTimersByTimeAsync(60_000) + expect(unavailableServer.snapshotRequests).toHaveLength(5) + + const replacement = await subscribeClient() + server.output('later-live-output') + expect(subscribe).toHaveBeenCalledTimes(2) + expect(replacement.snapshots).toEqual(['INITIAL']) + expect(replacement.data).toEqual(['later-live-output']) + } finally { + vi.useRealTimers() + } + }) + it('replays a pending escape tail after clearing an empty recovered screen', async () => { const { snapshots, snapshotMetas } = await subscribeClient() server.emptyNextRecoverySnapshot = true diff --git a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts index 933ade20e3f..3fdba8b48eb 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts @@ -234,6 +234,7 @@ const REMOTE_TERMINAL_RESYNC_TIMEOUT_MS = 10_000 // retrying once per incoming chunk would stampede it, so back off instead. const REMOTE_TERMINAL_RESYNC_RETRY_BASE_MS = 500 const REMOTE_TERMINAL_RESYNC_RETRY_MAX_MS = 5_000 +const REMOTE_TERMINAL_RESYNC_MAX_ATTEMPTS = 5 // Why: exported so the transport can classify it as benign — the snapshot was // skipped but live output continues, so it must not surface a fatal red banner. export const REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE = @@ -1021,6 +1022,7 @@ class RemoteRuntimeTerminalMultiplexer { private sendResyncSnapshot(stream: RemoteRuntimeMultiplexedTerminalState): void { stream.resyncPendingSend = false this.startResyncTimer(stream) + stream.resyncAttempts += 1 const sent = this.sendFrame( stream.streamId, TerminalStreamOpcode.SnapshotRequest, @@ -1030,13 +1032,17 @@ class RemoteRuntimeTerminalMultiplexer { // Transport is down; the reconnect path re-subscribes from scratch. clearResyncTimer(stream) stream.resyncInFlight = false + stream.resyncAttempts -= 1 } } // Why: keep the gate shut across the backoff — the post-gap tail is corrupt // either way — and heal even if the flood ends with no further output. private scheduleResyncRetry(stream: RemoteRuntimeMultiplexedTerminalState): void { - stream.resyncAttempts += 1 + if (stream.resyncAttempts >= REMOTE_TERMINAL_RESYNC_MAX_ATTEMPTS) { + this.recoverStalledStream(stream) + return + } const delay = Math.min( REMOTE_TERMINAL_RESYNC_RETRY_MAX_MS, REMOTE_TERMINAL_RESYNC_RETRY_BASE_MS * 2 ** Math.min(stream.resyncAttempts - 1, 4)