From bd129343625722f5a817ddc56c2ae8d7e2b970cb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:03:07 -0700 Subject: [PATCH] fix(terminal): repaint a recovered remote pane from the host's retained buffer (#14095) * fix(terminal): repaint a recovered remote pane from the host's retained buffer A remote-runtime pane that loses its stream re-subscribes, but the recovery subscribe only carries new bytes. When the host's push snapshot is empty -- an idle pane, or one whose PTY has exited and is preserved with its buffer -- the transport dropped it and nothing re-armed the restore, so the pane stayed blank until a visibility flip issued the tagged snapshot request. That is why switching worktrees once "fixed" it. Emit onStreamRecovered from the recovery subscribe path only, and mark the hidden-output restore needed so the pane pulls the buffer the host still holds. The initial subscribe is untouched: it already carries the host snapshot, and re-arming there would cost every pane a redundant restore request on open. * perf(terminal): avoid duplicate recovery snapshot replay --- .../terminal-pane/pty-connection.ts | 5 ++ .../terminal-pane/pty-transport-types.ts | 3 + ...ote-hidden-output-restore-outcomes.test.ts | 23 +++++ .../remote-runtime-pty-transport.test.ts | 85 +++++++++++++++++++ .../remote-runtime-pty-transport.ts | 8 ++ 5 files changed, 124 insertions(+) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 907a8af9696..16b2bc34346 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -5828,6 +5828,11 @@ export function connectPanePty( reportRemoteRendererSerializerReady() } }, + onStreamRecovered: (): void => { + if (isCurrent()) { + markHiddenOutputRestoreNeeded() + } + }, onData: (data: string, meta?: PtyDataMeta): void => { if (isCurrent()) { dataCallback(data, meta, generation) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index fadc7bd0feb..9befbf45a00 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -104,6 +104,9 @@ type PtyCallbacks = { /** Called before an adopted PTY can publish buffered/live bytes. */ onReattachDetermined?: () => void onConnect?: () => void + /** A stream re-established after loss carries only new bytes, so the pane must + * re-pull the host's retained buffer or an idle/exited pane paints nothing. */ + onStreamRecovered?: () => void onDisconnect?: () => void onData?: (data: string, meta?: PtyDataMeta) => void onReplayData?: (data: string, meta?: PtyReplayDataMeta) => void diff --git a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts index 0c9b286703c..0e98202aab3 100644 --- a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts @@ -32,6 +32,7 @@ function leafIdForPane(paneId: number): string { type ConnectCallbacks = { onReattachDetermined?: () => void onConnect?: () => void + onStreamRecovered?: () => void onData?: ( data: string, meta?: { seq?: number; rawLength?: number; background?: boolean; droppedOutput?: boolean } @@ -379,6 +380,7 @@ type RemotePaneDrive = { disposable: { dispose: () => void } deliver: (data: string, seq: number) => void setOutputPaused: (paused: boolean) => void + recoverStream: () => void writtenChunks: () => string[] } @@ -396,9 +398,11 @@ async function connectHiddenRemoteAgentPane( const capturedOutputPauseCallback: { current: ((paused: boolean, supported: boolean) => void) | null } = { current: null } + const capturedStreamRecoveredCallback: { current: (() => void) | null } = { current: null } transport.connect.mockImplementation(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => { capturedDataCallback.current = callbacks?.onData ?? null capturedOutputPauseCallback.current = callbacks?.onOutputPauseChanged ?? null + capturedStreamRecoveredCallback.current = callbacks?.onStreamRecovered ?? null return REMOTE_PTY_ID }) transportFactoryQueue.push(transport) @@ -415,6 +419,7 @@ async function connectHiddenRemoteAgentPane( disposable, deliver: (data, seq) => capturedDataCallback.current?.(data, { seq, rawLength: data.length }), setOutputPaused: (paused) => capturedOutputPauseCallback.current?.(paused, true), + recoverStream: () => capturedStreamRecoveredCallback.current?.(), writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data)) } } @@ -618,6 +623,24 @@ describe('remote hidden-output restore outcomes', () => { resetAgentStartupDelayedDeliveryForTests() }) + it('[modern] repaints a recovered visible pane from the retained host buffer', async () => { + const serializeBuffer = vi.fn() + const serializeBufferOutcome = vi.fn().mockResolvedValue({ + availability: { kind: 'snapshot' }, + snapshot: HOST_SNAPSHOT + }) + const drive = await connectHiddenRemoteAgentPane(serializeBuffer, serializeBufferOutcome) + ;(drive.deps.isVisibleRef as { current: boolean }).current = true + + drive.recoverStream() + await flushAsyncTicks(20) + + expect(serializeBufferOutcome).toHaveBeenCalledTimes(1) + expect(drive.writtenChunks().join('')).toContain(HOST_SNAPSHOT_MARKER) + expect(serializeBuffer).not.toHaveBeenCalled() + drive.disposable.dispose() + }) + it('[modern] accepts an empty snapshot as successful recovery without a loss banner', async () => { const serializeBuffer = vi.fn() const serializeBufferOutcome = vi.fn().mockResolvedValue({ diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index 0292d0b1749..4087cf59a15 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -5054,6 +5054,91 @@ describe('createRemoteRuntimePtyTransport', () => { transport.destroy?.() }) + it('re-arms the retained-buffer restore when a recovery subscribe replays no snapshot', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onReplayData = vi.fn() + const onStreamRecovered = vi.fn() + const onConnect = vi.fn() + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + await transport.connect({ + url: '', + callbacks: { onReplayData, onStreamRecovered, onConnect } + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + const firstStreamId = latestSubscribePayload().streamId + emitSnapshot(firstStreamId, 'INITIAL_SNAPSHOT') + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'subscribed', + streamId: firstStreamId, + capabilities: { outputPause: 1 } + } + }) + await vi.waitFor(() => expect(onConnect).toHaveBeenCalledTimes(1)) + // The first subscribe already carries the host snapshot; re-arming there would cost + // every pane a redundant restore request on open. + expect(onStreamRecovered).not.toHaveBeenCalled() + + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(2)) + await vi.waitFor(() => + expect( + subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + ).toHaveLength(2) + ) + const reconnectStreamId = latestSubscribePayload().streamId + // An exited-but-preserved pane has nothing to push and will never emit live bytes, + // so without the re-arm the pane stays blank until a visibility flip. + emitSnapshot(reconnectStreamId, '') + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'subscribed', + streamId: reconnectStreamId, + capabilities: { outputPause: 1 } + } + }) + + await vi.waitFor(() => expect(onStreamRecovered).toHaveBeenCalledTimes(1)) + expect(onReplayData.mock.calls.map((call) => call[0])).toEqual(['INITIAL_SNAPSHOT']) + + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(3)) + await vi.waitFor(() => + expect( + subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + ).toHaveLength(3) + ) + const populatedReconnectStreamId = latestSubscribePayload().streamId + emitSnapshot(populatedReconnectStreamId, 'RECOVERY_SNAPSHOT') + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'subscribed', + streamId: populatedReconnectStreamId, + capabilities: { outputPause: 1 } + } + }) + + await vi.waitFor(() => expect(onConnect).toHaveBeenCalledTimes(3)) + expect(onStreamRecovered).toHaveBeenCalledTimes(1) + expect(onReplayData.mock.calls.map((call) => call[0])).toEqual([ + 'INITIAL_SNAPSHOT', + 'RECOVERY_SNAPSHOT' + ]) + transport.destroy?.() + }) + it('backs off before retrying a capacity-rejected terminal stream', async () => { vi.useFakeTimers() try { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 8908b39c63c..6eff59d84ce 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -1735,6 +1735,7 @@ export function createRemoteRuntimePtyTransport( setAttachmentReady(false) let transportClosed = false let subscriptionAttached = false + let subscriptionSnapshotHadContent = false // Why: viewport handed to subscribe; a resize during the round-trip falls back to the refresh-only one-shot RPC, replayed through the stream below once current. const subscribedViewport = desiredViewport const isCurrentSubscription = (): boolean => @@ -1760,6 +1761,7 @@ export function createRemoteRuntimePtyTransport( onSnapshot: (data, meta) => { // Why: an empty snapshot can still carry a pending mid-escape tail that must replay so the next live chunk completes it. if ((data || meta?.pendingEscapeTailAnsi) && isCurrentSubscription()) { + subscriptionSnapshotHadContent = true if (subscribedPtyId && bufferPtyShutdownReplayData(subscribedPtyId, data)) { return } @@ -1806,6 +1808,12 @@ export function createRemoteRuntimePtyTransport( markRecoveryHealthy() emitRecoveryState() storedCallbacks.onConnect?.() + // Why: a recovery subscribe replays nothing when the host's push snapshot is + // empty (idle or exited pane), so ask for the retained buffer instead of + // waiting for bytes that an exited process will never send. + if (expectedRecoveryEpoch !== undefined && !subscriptionSnapshotHadContent) { + storedCallbacks.onStreamRecovered?.() + } storedCallbacks.onStatus?.('shell') }, onEnd: () => {