From 1667b77f0baebf74b51700d6421cf6f0a3397f90 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Tue, 4 Aug 2026 23:21:31 -0700 Subject: [PATCH] fix(remote-runtime): keep a remote outage from flooding the error surface and dead-ending the pane (#12650) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a remote runtime went unreachable (laptop sleep, Tailscale drop), the UI filled with dozens of repeated timeout errors until it was nearly unusable, and the affected terminal then accepted no input after connectivity returned — leaving "close the session and resume it in a new one" as the only escape. Four causes, three of which were still live: - Errors accumulated into one ever-growing surface with no de-duplication or cap. - Queue-overload rejections lose their structured error code crossing the IPC boundary, so they were never classified as recoverable and surfaced raw. - A transient failure misclassified as fatal called `recovery.cancel()`, setting the pane to an idle phase — which unmounts the Reconnect banner and makes manual retry, online and resume triggers all no-ops. A true dead end, and the reason recreating the session was the only way out. - Dismissing an error cleared the surface but not the dedup memory, so an identical fatal error recurring in the same outage was suppressed forever while the pane looked healthy; dedup also compared single lines, so multi-line errors never matched and stacked without bound. The ordinary reconnect loop was already fixed in v1.4.150/160 — bounded backoff, a Reconnect banner and auto-recovery already ship. This fixes what remained. Note the fix routes fatal resubscribe failures back through the shared terminal error handler: bypassing it had silently dropped stale-handle re-resolution, terminal-gone retirement, SSH-expired recovery and oversized-snapshot suppression — a stuck-pane regression inside the stuck-pane fix, caught in review and covered by 6 dedicated tests. Verified: reproductions red on main before the fix; after rebasing onto #11542, reverting the dead-end fix still turns its test red. Follow-up STA-3456 tracks preserving typed error codes across the IPC boundary so classification stops matching message text. --- .../components/terminal-pane/TerminalPane.tsx | 12 +- .../terminal-pane/pty-transport-types.ts | 2 + ...te-runtime-error-surface-dismissal.test.ts | 78 ++++ ...ge-toast-flood-and-stuck-reconnect.test.ts | 438 ++++++++++++++++++ .../remote-runtime-pty-transport.test.ts | 3 +- .../remote-runtime-pty-transport.ts | 58 ++- ...subscribe-failure-recovery-routing.test.ts | 354 ++++++++++++++ .../terminal-error-accumulation.test.ts | 70 +++ .../terminal-error-accumulation.ts | 22 + ...untime-client-error-classification.test.ts | 20 +- ...ote-runtime-client-error-classification.ts | 12 + 11 files changed, 1046 insertions(+), 23 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/remote-runtime-error-surface-dismissal.test.ts create mode 100644 src/renderer/src/components/terminal-pane/remote-runtime-outage-toast-flood-and-stuck-reconnect.test.ts create mode 100644 src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-error-accumulation.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 3d3b0d072..057f9e1f7 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -207,6 +207,7 @@ import { type TerminalPasteSource, type TerminalPasteTextOptions } from './terminal-paste-coordinator' +import { appendTerminalErrorMessage } from './terminal-error-accumulation' import { formatTerminalPasteExecutionError } from './terminal-paste-errors' import { resolveTerminalPasteRuntime } from './terminal-paste-runtime' import { getTerminalPasteSshRemotePlatform } from './terminal-paste-ssh-platform' @@ -473,8 +474,15 @@ function TerminalPane( setSessionStateSaveFailureOpen(true) return } - setTerminalError((prev) => (prev ? `${prev}\n${message}` : message)) + setTerminalError((prev) => appendTerminalErrorMessage(prev, message)) }) + /** Dismissal is the only signal that the user has seen the surface, so it must also release the transports' repeat-suppression memory. */ + const dismissTerminalError = useCallback(() => { + setTerminalError(null) + for (const transport of paneTransportsRef.current.values()) { + transport.notifyErrorSurfaceDismissed?.() + } + }, []) const onPtyRecoveryStateRef = useRef( (paneId: number, state: PtyTransportRecoveryState | null) => { setPtyRecoveryStatesByPaneId((previous) => @@ -2892,7 +2900,7 @@ function TerminalPane( {terminalError && isActive && !showSshReconnectOverlay ? ( setTerminalError(null)} + onDismiss={dismissTerminalError} onRestartDaemon={() => daemonActions.setPending('restart')} /> ) : null} 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 b0dd95941..c64b93934 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -161,6 +161,8 @@ export type PtyTransport = { getRecoveryState?: () => PtyTransportRecoveryState /** Starts a fresh connection epoch while preserving the authoritative remote PTY identity. */ retryRecovery?: () => boolean + /** The user dismissed the error surface; the next occurrence of the same message must surface again. */ + notifyErrorSurfaceDismissed?: () => void getPtyId: () => string | null getConnectionId?: () => string | null | undefined /** The runtime captured by this transport; legacy remote PTY ids do not diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-error-surface-dismissal.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-error-surface-dismissal.test.ts new file mode 100644 index 000000000..1df9a355a --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-error-surface-dismissal.test.ts @@ -0,0 +1,78 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +// Why: the transport suppresses a repeat of the message it last surfaced so one outage cannot spam the +// pane. Dismissal is the only evidence the user consumed that surface, so it must re-arm the memory — +// otherwise an identical fatal error recurring during the same outage leaves a dead pane looking fine. +describe('remote runtime error surface dismissal', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const subscriptionSendBinary = vi.fn() + const FATAL_ERROR = 'Remote terminal rejected the write: permission denied.' + + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + subscriptionSendBinary.mockReset() + runtimeCall.mockImplementation(async () => ({ ok: true, result: {} })) + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: { onResponse: (response: unknown) => void }) => { + queueMicrotask(() => callbacks.onResponse({ ok: true, result: { type: 'ready' } })) + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall, + subscribe: runtimeSubscribe + } + } + }) + }) + + async function attachTransportWithFatalSends(onError: (message: string) => void) { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { worktreeId: 'wt-1' }) + transport.attach({ existingPtyId: 'remote:terminal-1', callbacks: { onError } }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'terminal.send') { + throw new Error(FATAL_ERROR) + } + return { ok: true, result: {} } + }) + return transport + } + + it('re-surfaces an identical fatal error after the user dismisses the error surface', async () => { + const onError = vi.fn() + const transport = await attachTransportWithFatalSends(onError) + + expect(await transport.sendInputAccepted?.('a')).toBe(false) + expect(await transport.sendInputAccepted?.('b')).toBe(false) + // Contract preserved: one continuous outage still surfaces the repeated error once. + expect(onError.mock.calls).toEqual([[FATAL_ERROR]]) + + transport.notifyErrorSurfaceDismissed?.() + + expect(await transport.sendInputAccepted?.('c')).toBe(false) + expect(onError.mock.calls).toEqual([[FATAL_ERROR], [FATAL_ERROR]]) + + // The memory re-arms: repeats after the re-surfaced error are suppressed again until the next dismissal. + expect(await transport.sendInputAccepted?.('d')).toBe(false) + expect(onError).toHaveBeenCalledTimes(2) + transport.destroy?.() + }) + + it('leaves an undismissed surface deduped', async () => { + const onError = vi.fn() + const transport = await attachTransportWithFatalSends(onError) + + expect(await transport.sendInputAccepted?.('a')).toBe(false) + expect(await transport.sendInputAccepted?.('b')).toBe(false) + expect(await transport.sendInputAccepted?.('c')).toBe(false) + + expect(onError).toHaveBeenCalledTimes(1) + transport.destroy?.() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-outage-toast-flood-and-stuck-reconnect.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-outage-toast-flood-and-stuck-reconnect.test.ts new file mode 100644 index 000000000..8cd8600da --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-outage-toast-flood-and-stuck-reconnect.test.ts @@ -0,0 +1,438 @@ +/** + * Deterministic reproduction for issue3-error-flood-stuck-terminal + * (remote-runtime outage floods the UI with repeated raw error toasts; + * terminal panes stay stuck after connectivity is restored). + * + * Three tests assert the adjudicated fix; the STA-3002 case covers reconnect + * activation, which PR #11542 landed separately. + * + * Fault-injection point: window.api.runtimeEnvironments.call — the exact + * Electron IPC boundary the remote PTY transport uses. Rejections are shaped + * like real Electron IPC rejections: a plain Error whose message carries the + * "Error invoking remote method 'runtimeEnvironments:call': : " + * prefix and NO `code` property (Electron structured-clone keeps only + * name/message/stack), which forces the renderer's message-fragment + * classification path. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + encodeTerminalStreamFrame, + encodeTerminalStreamJson, + encodeTerminalStreamText +} from '../../../../shared/terminal-stream-protocol' +import { RuntimeRpcCallQueueOverloadError } from '../../../../shared/runtime-rpc-call-queue' +import { withRemoteRuntimeTailscaleHint } from '../../../../shared/remote-runtime-tailscale-hint' +import type { PtyTransportRecoveryState } from './pty-transport-types' + +const ELECTRON_IPC_PREFIX = "Error invoking remote method 'runtimeEnvironments:call': " + +/** A rejection exactly as the renderer sees it after Electron IPC strips custom props. */ +function electronIpcShapedRejection(errorName: string, message: string): Error { + return new Error(`${ELECTRON_IPC_PREFIX}${errorName}: ${message}`) +} + +const QUEUE_OVERLOAD_RAW = new RuntimeRpcCallQueueOverloadError('selector').message + +// The exact user-reported toast text family (timeout + Tailscale funnel hint). +const TIMEOUT_WITH_TAILSCALE_HINT = withRemoteRuntimeTailscaleHint( + 'Timed out waiting for the remote Orca runtime to respond.', + 'https://orca-server.tail1234.ts.net' +) + +describe('remote runtime outage: toast flood and stuck reconnect (issue3)', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const refreshSessionTabsSnapshot = vi.fn(async () => {}) + const subscriptionSendBinary = vi.fn() + let subscriptionCallbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { code: string; message: string }) => void + onClose?: () => void + } | null = null + + function emitMultiplexReady(): void { + subscriptionCallbacks?.onResponse({ ok: true, result: { type: 'ready' } }) + } + + function latestSubscribePayload(): { streamId: number; terminal: string } { + const frames = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + const frame = frames.at(-1) + if (!frame) { + throw new Error('missing terminal subscribe frame') + } + const payload = decodeTerminalStreamJson<{ streamId: number; terminal: string }>(frame.payload) + if (!payload) { + throw new Error('invalid terminal subscribe payload') + } + return payload + } + + function subscribedTerminalHandles(): string[] { + return subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .flatMap((frame) => { + if (frame?.opcode !== TerminalStreamOpcode.Subscribe) { + return [] + } + const payload = decodeTerminalStreamJson<{ terminal: string }>(frame.payload) + return payload ? [payload.terminal] : [] + }) + } + + function emitSnapshot(streamId: number, data: string): void { + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotStart, + streamId, + seq: 1, + payload: encodeTerminalStreamJson({ kind: 'scrollback' }) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotChunk, + streamId, + seq: 2, + payload: encodeTerminalStreamText(data) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotEnd, + streamId, + seq: 3, + payload: new Uint8Array() + }) + ) + } + + function installHealthyRuntimeCallMock(): void { + runtimeCall.mockImplementation(async (request: { method: string; params?: unknown }) => { + if (request.method === 'session.tabs.activate') { + const params = request.params as { tabId: string; leafId?: string } + const resolvedLeafId = params.leafId ?? 'pane:1' + return { + ok: true, + result: { + worktree: 'id:wt-1', + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: 'group-1', + activeTabId: `${params.tabId}::${resolvedLeafId}`, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `${params.tabId}::${resolvedLeafId}`, + parentTabId: params.tabId, + leafId: resolvedLeafId, + title: 'Terminal', + isActive: true, + status: 'ready', + terminal: 'terminal-1' + } + ] + } + } + } + if (request.method === 'terminal.resolvePane') { + const params = request.params as { paneKey: string; worktreeId: string } + const separator = params.paneKey.indexOf(':') + return { + ok: true, + result: { + terminal: { + handle: 'terminal-1', + tabId: params.paneKey.slice(0, separator), + leafId: params.paneKey.slice(separator + 1), + worktreeId: params.worktreeId + } + } + } + } + return { ok: true, result: { terminal: { handle: 'terminal-1' } } } + }) + } + + beforeEach(() => { + vi.resetModules() + vi.doUnmock('../../runtime/remote-runtime-terminal-multiplexer') + vi.doMock('@/runtime/web-runtime-session', () => ({ + refreshWebRuntimeSessionTabsSnapshot: refreshSessionTabsSnapshot + })) + vi.clearAllMocks() + subscriptionCallbacks = null + subscriptionSendBinary.mockReset() + installHealthyRuntimeCallMock() + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + subscriptionCallbacks = callbacks + queueMicrotask(emitMultiplexReady) + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall, + subscribe: runtimeSubscribe + } + } + }) + }) + + it('sanity: the exact reported timeout+funnel toast text classifies as recoverable, so flood text must come from unclassified surfaces', async () => { + const { isRecoverableRemoteRuntimeConnectionError, toRemoteRuntimeClientErrorLike } = + await import('../../../../shared/remote-runtime-client-error-classification') + const rendererSide = toRemoteRuntimeClientErrorLike( + electronIpcShapedRejection('RemoteRuntimeClientError', TIMEOUT_WITH_TAILSCALE_HINT) + ) + // Electron IPC stripped the code; the fragment list still catches this one. + expect(rendererSide.code).toBeUndefined() + expect(isRecoverableRemoteRuntimeConnectionError(rendererSide)).toBe(true) + // …but the queue-overload rejection produced by the same outage (main's + // per-selector RPC queue saturated by 15s-timeout calls) is classified + // fatal even though its own code says "retry later". + const overload = toRemoteRuntimeClientErrorLike( + electronIpcShapedRejection('RuntimeRpcCallQueueOverloadError', QUEUE_OVERLOAD_RAW) + ) + expect(overload.code).toBeUndefined() + // DESIRED: transient capacity pressure during an outage is recoverable, + // not a fatal red-toast error. RED on main: not in codes or fragments. + expect(isRecoverableRemoteRuntimeConnectionError(overload)).toBe(true) + }) + + it('FLOOD: repeated identical outage-shaped send failures surface at most one red toast per pane', async () => { + // Outage onset: the multiplexed stream subscription round-trip hangs + // (socket black-holed), so keystrokes fall back to one-shot terminal.send + // RPCs; main's saturated per-selector queue rejects each one instantly. + runtimeSubscribe.mockImplementation(async () => new Promise(() => {})) + let sendRejections = 0 + installHealthyRuntimeCallMock() + const healthyImpl = runtimeCall.getMockImplementation()! + runtimeCall.mockImplementation(async (request: { method: string; params?: unknown }) => { + if (request.method === 'terminal.send') { + sendRejections += 1 + throw electronIpcShapedRejection('RuntimeRpcCallQueueOverloadError', QUEUE_OVERLOAD_RAW) + } + return healthyImpl(request) + }) + + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onError = vi.fn() + const recoveryPhases: string[] = [] + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: { + onError, + onRecoveryStateChange: (state: PtyTransportRecoveryState) => + recoveryPhases.push(state.phase) + } + }) + await vi.waitFor(() => expect(transport.getPtyId()).toBe('remote:env-1@@terminal-1')) + + // Three keystroke bursts during the same outage. + expect(transport.sendInputImmediate?.('k1')).toBe(true) + await vi.waitFor(() => expect(sendRejections).toBe(1)) + expect(transport.sendInputImmediate?.('k2')).toBe(true) + await vi.waitFor(() => expect(sendRejections).toBe(2)) + expect(transport.sendInputImmediate?.('k3')).toBe(true) + await vi.waitFor(() => expect(sendRejections).toBe(3)) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(onError).not.toHaveBeenCalled() + expect(recoveryPhases).toContain('backoff') + transport.destroy?.() + }) + + it('STUCK (cancel dead-end): a fatal resubscribe error leaves no recovery path after connectivity returns', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { retryAllRemoteRuntimePtyRecoveriesNow } = + await import('./remote-runtime-pty-recovery-state') + const { updateTerminalRemoteRuntimeRecoveryUiState } = + await import('./terminal-remote-runtime-recovery-ui-state') + const onError = vi.fn() + let bannerUiState: Parameters[0] = {} + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: { + onError, + onRecoveryStateChange: (state: PtyTransportRecoveryState) => { + bannerUiState = updateTerminalRemoteRuntimeRecoveryUiState(bannerUiState, 1, state) + } + } + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + emitSnapshot(latestSubscribePayload().streamId, 'live before outage') + expect(transport.isConnected()).toBe(true) + + // The dedicated stream dies, then a fatal retry response must leave the + // terminal disconnected but manually revivable. + const fatalMessage = 'Remote runtime pairing credentials expired.' + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'terminal.resolvePane') { + throw Object.assign(new Error(fatalMessage), { code: 'unauthorized' }) + } + throw electronIpcShapedRejection('RemoteRuntimeClientError', TIMEOUT_WITH_TAILSCALE_HINT) + }) + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(onError).toHaveBeenCalled()) + expect(onError).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledWith(fatalMessage) + + // Connectivity fully restored. + installHealthyRuntimeCallMock() + const subscribeCallsBeforeTriggers = runtimeSubscribe.mock.calls.length + // Fire every built-in revival trigger the app has: + const revivedByOnlineOrResume = retryAllRemoteRuntimePtyRecoveriesNow() + const manualRetryAccepted = transport.retryRecovery?.() ?? false + const reconnectBannerVisible = 1 in bannerUiState + + // DESIRED INVARIANT (RED on main): after the fault clears, at least one + // recovery affordance must exist — the online/resume trigger revives a + // parked retry, or the Reconnect banner is visible and its retry is + // accepted. On main: cancel() latched phase 'idle', pendingRetry is gone, + // the banner is unmounted, and retryRecovery() returns false — keystrokes + // silently vanish and no output ever renders again. + expect( + { + revivedByOnlineOrResume, + manualRetryAccepted, + reconnectBannerVisible + }, + 'pane must remain revivable after a fatal resubscribe error' + ).not.toEqual({ + revivedByOnlineOrResume: 0, + manualRetryAccepted: false, + reconnectBannerVisible: false + }) + + // Full recovery: a fresh subscribe attempt must reach the runtime. + await vi.waitFor(() => + expect(runtimeSubscribe.mock.calls.length).toBeGreaterThan(subscribeCallsBeforeTriggers) + ) + transport.destroy?.() + }) + + // PR #11542 owns reconnect activation for STA-3002; it has landed, so this must stay green. + it('STUCK (STA-3002 shape): reconnect never re-materializes a host surface demoted to pending-handle, even via online trigger and Reconnect', async () => { + vi.useFakeTimers() + try { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { retryAllRemoteRuntimePtyRecoveriesNow } = + await import('./remote-runtime-pty-recovery-state') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'web-terminal-host-tab-1', + leafId: 'leaf-1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: {} + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + emitSnapshot(latestSubscribePayload().streamId, 'live before host restart') + expect(transport.isConnected()).toBe(true) + + // Host restarted during the outage: it republishes this pane as + // status 'pending-handle' with no terminal. A real host only mints the + // PTY handle when someone calls session.tabs.activate. + let hostActivated = false + let activateCallsAfterOutage = 0 + let listCallsAfterOutage = 0 + const hostSnapshot = () => ({ + ok: true, + result: { + worktree: 'wt-1', + publicationEpoch: 'epoch-2', + snapshotVersion: 2 + listCallsAfterOutage, + activeGroupId: null, + activeTabId: 'host-tab-1::leaf-1', + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: 'host-tab-1::leaf-1', + parentTabId: 'host-tab-1', + leafId: 'leaf-1', + title: 'Terminal', + isActive: true, + ...(hostActivated + ? { status: 'ready', terminal: 'terminal-2' } + : { status: 'pending-handle', terminal: null }) + } + ] + } + }) + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'session.tabs.list') { + listCallsAfterOutage += 1 + return hostSnapshot() + } + if (request.method === 'session.tabs.activate') { + activateCallsAfterOutage += 1 + hostActivated = true + return hostSnapshot() + } + return { ok: true, result: {} } + }) + + // Stream lost → reconnect. The resubscribe path looks for a status:'ready' + // handle and finds only the pending surface. + subscriptionCallbacks?.onClose?.() + await vi.advanceTimersByTimeAsync(16_000) + + // Auto-recovery deadline latches the pane 'disconnected'. + await vi.advanceTimersByTimeAsync(60_000) + expect(transport.getRecoveryState?.().phase).toBe('disconnected') + + // Connectivity restored; 'online'/system-resume trigger fires. + retryAllRemoteRuntimePtyRecoveriesNow() + await vi.advanceTimersByTimeAsync(16_000) + + // Latch again, then the user clicks the Reconnect banner. + await vi.advanceTimersByTimeAsync(60_000) + transport.retryRecovery?.() + await vi.advanceTimersByTimeAsync(16_000) + + // A reconnect against a host that publishes this pane as pending-handle + // must call session.tabs.activate (the only RPC that materializes the PTY) + // and attach to the minted handle. Before #11542 only the initial-connect + // path activated, so every reconnect polled session.tabs.list forever and + // the pane stayed stuck until the user resumed the session in a new one. + expect( + activateCallsAfterOutage, + `reconnect ran ${listCallsAfterOutage} list-only inventory polls across online trigger + Reconnect click without ever activating the pending surface` + ).toBeGreaterThan(0) + await vi.waitFor(() => expect(subscribedTerminalHandles()).toContain('terminal-2')) + emitSnapshot(latestSubscribePayload().streamId, 'rematerialized') + expect(transport.isConnected()).toBe(true) + expect(transport.getPtyId()).toBe('remote:env-1@@terminal-2') + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) +}) 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 6126c039e..198c24818 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 @@ -535,7 +535,8 @@ describe('createRemoteRuntimePtyTransport', () => { expect( runtimeCall.mock.calls.filter(([args]) => args.method === 'terminal.create') ).toHaveLength(1) - expect(onError).toHaveBeenCalledTimes(1) + expect(onError).not.toHaveBeenCalled() + expect(transport.getRecoveryState?.().phase).toBe('disconnected') transport.destroy?.() }) 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 42f7df894..a74fed287 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 @@ -2,6 +2,7 @@ import type { RuntimeRpcResponse } from '../../../../shared/runtime-rpc-envelope' import { isRecoverableRemoteRuntimeConnectionError, + isRuntimeRpcQueueOverloadError, toRemoteRuntimeClientErrorLike } from '../../../../shared/remote-runtime-client-error-classification' import type { @@ -181,6 +182,7 @@ export function createRemoteRuntimePtyTransport( let desiredOutputPaused = false let desiredViewport: { cols: number; rows: number } | null = null let storedCallbacks: Parameters[0]['callbacks'] = {} + let lastSurfacedErrorMessage: string | null = null let resubscribeEpoch: number | null = null let resubscribeRequestedHandle: string | null = null let resubscribeRequestedReplacementPolicy: HostHandleReplacementPolicy = 'reuse' @@ -435,6 +437,19 @@ export function createRemoteRuntimePtyTransport( storedCallbacks.onRecoveryStateChange?.(state) } + function surfaceErrorMessage(message: string): void { + if (message === lastSurfacedErrorMessage) { + return + } + lastSurfacedErrorMessage = message + storedCallbacks.onError?.(message) + } + + function markRecoveryHealthy(): void { + lastSurfacedErrorMessage = null + recovery.markHealthy() + } + function hostSnapshotOwnsLaunch( result: RemoteAgentSessionLaunchResult, environmentId: string @@ -761,12 +776,12 @@ export function createRemoteRuntimePtyTransport( return undefined } if (hostHandle === null) { - storedCallbacks.onError?.('Remote terminal was closed.') + surfaceErrorMessage('Remote terminal was closed.') return undefined } if (!hostHandle || !isCurrent()) { if (isCurrent()) { - storedCallbacks.onError?.('Remote terminal was closed.') + surfaceErrorMessage('Remote terminal was closed.') } return undefined } @@ -1144,7 +1159,7 @@ export function createRemoteRuntimePtyTransport( }) .catch((error) => { if (!destroyed && handle === expiredHandle) { - storedCallbacks.onError?.(runtimeTerminalErrorMessage(error)) + surfaceErrorMessage(runtimeTerminalErrorMessage(error)) } }) .finally(() => { @@ -1438,14 +1453,19 @@ export function createRemoteRuntimePtyTransport( recoverExpiredHostPane() return } - if (isRecoverableRemoteRuntimeConnectionError(toRemoteRuntimeClientErrorLike(error))) { + const clientError = toRemoteRuntimeClientErrorLike(error) + if (isRuntimeRpcQueueOverloadError(clientError)) { + scheduleCapacityPressureRetry() + return + } + if (isRecoverableRemoteRuntimeConnectionError(clientError)) { // Why: a partition is attachment state, not a terminal failure; keep the red error surface for actionable fatal errors. scheduleResubscribeAfterTransportClose() return } connecting = false emitRecoveryState() - storedCallbacks.onError?.(message) + surfaceErrorMessage(message) } function recoverAfterSubscribeFailure( @@ -1627,7 +1647,9 @@ export function createRemoteRuntimePtyTransport( scheduleResubscribeAfterTransportClose(currentReplacementPolicy, nextEpoch) }) } else { - recovery.cancel() + recovery.markDisconnected() + // Why: stale/gone/SSH-expired handling lives in handleRemoteTerminalError; its + // fallthrough surfaces the message, so routing here keeps those recoveries alive. handleRemoteTerminalError(error) } } @@ -1743,7 +1765,7 @@ export function createRemoteRuntimePtyTransport( setAttachmentReady(true) connecting = false resetRecoveryReplacementPolicy() - recovery.markHealthy() + markRecoveryHealthy() emitRecoveryState() storedCallbacks.onConnect?.() storedCallbacks.onStatus?.('shell') @@ -1844,7 +1866,7 @@ export function createRemoteRuntimePtyTransport( setAttachmentReady(subscriptionAttached) if (subscriptionAttached) { resetRecoveryReplacementPolicy() - recovery.markHealthy() + markRecoveryHealthy() } // Why: a viewport change during the subscribe round-trip hit the no-op one-shot fallback; replay the latest viewport so the PTY isn't stuck at subscribe-time size. if (pendingViewportClaim && desiredViewport) { @@ -1875,6 +1897,7 @@ export function createRemoteRuntimePtyTransport( const createEnvironmentId = currentRuntimeEnvironmentId lastConnectOptions = options lastAttachOptions = null + lastSurfacedErrorMessage = null storedCallbacks = options.callbacks resetRecoveryReplacementPolicy() resetSameHandleEndReuse() @@ -2082,13 +2105,18 @@ export function createRemoteRuntimePtyTransport( } catch (error) { if (!destroyed && lifecycleEpoch === connectLifecycleEpoch) { connecting = false - recovery.cancel() const message = runtimeTerminalErrorMessage(error) if (isRemoteTerminalGoneMessage(message)) { + recovery.cancel() handleRemoteTerminalError(error) + } else if ( + isRecoverableRemoteRuntimeConnectionError(toRemoteRuntimeClientErrorLike(error)) + ) { + recovery.markDisconnected() } else { + recovery.cancel() emitRecoveryState() - storedCallbacks.onError?.(message) + surfaceErrorMessage(message) } } return undefined @@ -2104,6 +2132,7 @@ export function createRemoteRuntimePtyTransport( resetSameHandleEndReuse() clearPublishedHandleWait() lastAttachOptions = options + lastSurfacedErrorMessage = null storedCallbacks = options.callbacks terminalEnded = false connecting = true @@ -2128,7 +2157,7 @@ export function createRemoteRuntimePtyTransport( handle = null connecting = false emitRecoveryState() - storedCallbacks.onError?.('Remote runtime terminal id is invalid.') + surfaceErrorMessage('Remote runtime terminal id is invalid.') return } const persistedHandle = nextHandle @@ -2176,7 +2205,7 @@ export function createRemoteRuntimePtyTransport( return } if (!resolved) { - storedCallbacks.onError?.('Remote terminal was closed.') + surfaceErrorMessage('Remote terminal was closed.') return } await adoptResolvedHostPane(resolved, options, false, generation) @@ -2352,6 +2381,11 @@ export function createRemoteRuntimePtyTransport( getRecoveryState, + // Why: dedup exists to stop one outage spamming the surface; once the user dismisses it, the next occurrence is new information again. + notifyErrorSurfaceDismissed() { + lastSurfacedErrorMessage = null + }, + retryRecovery() { if ( !destroyed && diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts new file mode 100644 index 000000000..7353e491e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts @@ -0,0 +1,354 @@ +/** + * Pins the routing of a NON-RECOVERABLE resubscribe failure in + * remote-runtime-pty-transport. + * + * `scheduleResubscribeAfterTransportClose` catches the rejection of + * `resubscribeAfterTransportClose`. When the error is not a recoverable + * connection error it must hand the error to `handleRemoteTerminalError`. + * Surfacing the message directly instead looks harmless — the pane turns red + * either way — but it skips four lifecycle routes, because none of these + * messages appear in RECOVERABLE_MESSAGE_FRAGMENTS and so all of them land in + * exactly this branch: + * + * terminal_handle_stale -> require-replacement resubscribe (or retire) + * terminal_gone / terminal_exited -> retire the pane + * SSH_SESSION_EXPIRED -> terminal.recoverPane on the hub + * snapshot-too-large -> informational, no red error at all + * + * Fault-injection point: window.api.runtimeEnvironments.call, the Electron IPC + * boundary the transport really uses. Each pane is first driven to a genuinely + * connected state, then the multiplexed stream is closed so recovery starts + * with a live epoch, and only the resubscribe attempt fails. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + encodeTerminalStreamFrame, + encodeTerminalStreamJson, + encodeTerminalStreamText +} from '../../../../shared/terminal-stream-protocol' +import type { PtyTransport } from './pty-transport-types' + +type ResolvePaneOutcome = { handle: string } | { error: Error } + +const PANE_TAB_ID = 'tab-1' +const PANE_LEAF_ID = 'pane:1' +const PANE_WORKTREE_ID = 'wt-1' +const FIRST_HANDLE = 'terminal-1' +const FIRST_PTY_ID = 'remote:env-1@@terminal-1' + +describe('remote runtime resubscribe failure: recovery routing', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const refreshSessionTabsSnapshot = vi.fn(async () => {}) + const subscriptionSendBinary = vi.fn() + let subscriptionCallbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { code: string; message: string }) => void + onClose?: () => void + } | null = null + /** Consumed in order by terminal.resolvePane; the last entry repeats. */ + let resolvePaneOutcomes: ResolvePaneOutcome[] = [{ handle: FIRST_HANDLE }] + let recoverPaneOutcome: ResolvePaneOutcome = { handle: FIRST_HANDLE } + let methodLog: string[] = [] + let subscribeOutcomes: (Error | null)[] = [] + + function emitMultiplexReady(): void { + subscriptionCallbacks?.onResponse({ ok: true, result: { type: 'ready' } }) + } + + function latestSubscribePayload(): { streamId: number; terminal: string } { + const frame = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .findLast((candidate) => candidate?.opcode === TerminalStreamOpcode.Subscribe) + if (!frame) { + throw new Error('missing terminal subscribe frame') + } + const payload = decodeTerminalStreamJson<{ streamId: number; terminal: string }>(frame.payload) + if (!payload) { + throw new Error('invalid terminal subscribe payload') + } + return payload + } + + function subscribedTerminalHandles(): string[] { + return subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .flatMap((frame) => { + if (frame?.opcode !== TerminalStreamOpcode.Subscribe) { + return [] + } + const payload = decodeTerminalStreamJson<{ terminal: string }>(frame.payload) + return payload ? [payload.terminal] : [] + }) + } + + function emitSnapshot(streamId: number, data: string): void { + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotStart, + streamId, + seq: 1, + payload: encodeTerminalStreamJson({ kind: 'scrollback' }) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotChunk, + streamId, + seq: 2, + payload: encodeTerminalStreamText(data) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotEnd, + streamId, + seq: 3, + payload: new Uint8Array() + }) + ) + } + + function nextOutcome(outcomes: ResolvePaneOutcome[]): ResolvePaneOutcome { + return outcomes.length > 1 ? (outcomes.shift() as ResolvePaneOutcome) : outcomes[0] + } + + function paneResult(handle: string, paneKey: string, worktreeId: string): unknown { + const separator = paneKey.indexOf(':') + return { + ok: true, + result: { + terminal: { + handle, + tabId: paneKey.slice(0, separator), + leafId: paneKey.slice(separator + 1), + worktreeId + } + } + } + } + + /** Fails the pane's next resubscribe with `error`, then serves `thenHandle`. */ + function failNextResubscribeWith(error: Error, thenHandle = FIRST_HANDLE): void { + resolvePaneOutcomes = [{ error }, { handle: thenHandle }] + methodLog = [] + } + + async function attachLivePane( + overrides: Partial<{ tabId: string; leafId: string }> & { + onError?: (message: string) => void + onPtyExit?: (ptyId: string) => void + onPtyRebind?: (nextPtyId: string, previousPtyId: string) => void + } + ): Promise { + const { onError, onPtyExit, onPtyRebind, ...ids } = overrides + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: PANE_WORKTREE_ID, + ...('tabId' in ids ? { tabId: ids.tabId } : { tabId: PANE_TAB_ID }), + ...('leafId' in ids ? { leafId: ids.leafId } : { leafId: PANE_LEAF_ID }), + onPtyExit, + onPtyRebind + }) + transport.attach({ + existingPtyId: FIRST_PTY_ID, + cols: 80, + rows: 24, + callbacks: { onError } + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + emitSnapshot(latestSubscribePayload().streamId, 'live before the fault') + expect(transport.isConnected()).toBe(true) + return transport + } + + /** Kills the multiplexed stream so the transport enters recovery with a live epoch. */ + function dropMultiplexedStream(): void { + subscriptionCallbacks?.onClose?.() + } + + beforeEach(() => { + vi.resetModules() + vi.doUnmock('../../runtime/remote-runtime-terminal-multiplexer') + vi.doMock('@/runtime/web-runtime-session', () => ({ + refreshWebRuntimeSessionTabsSnapshot: refreshSessionTabsSnapshot + })) + vi.clearAllMocks() + subscriptionCallbacks = null + subscriptionSendBinary.mockReset() + resolvePaneOutcomes = [{ handle: FIRST_HANDLE }] + recoverPaneOutcome = { handle: FIRST_HANDLE } + subscribeOutcomes = [] + methodLog = [] + runtimeCall.mockImplementation(async (request: { method: string; params?: unknown }) => { + methodLog.push(request.method) + if (request.method === 'terminal.resolvePane') { + const params = request.params as { paneKey: string; worktreeId: string } + const outcome = nextOutcome(resolvePaneOutcomes) + if ('error' in outcome) { + throw outcome.error + } + return paneResult(outcome.handle, params.paneKey, params.worktreeId) + } + if (request.method === 'terminal.recoverPane') { + const params = request.params as { paneKey: string; worktreeId: string } + if ('error' in recoverPaneOutcome) { + throw recoverPaneOutcome.error + } + return paneResult(recoverPaneOutcome.handle, params.paneKey, params.worktreeId) + } + return { ok: true, result: { terminal: { handle: FIRST_HANDLE } } } + }) + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + const outcome = subscribeOutcomes.shift() + if (outcome) { + throw outcome + } + subscriptionCallbacks = callbacks + queueMicrotask(emitMultiplexReady) + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall, + subscribe: runtimeSubscribe + } + } + }) + }) + + it('re-resolves and adopts the replacement handle when a resubscribe fails stale', async () => { + const onError = vi.fn() + const onPtyRebind = vi.fn() + const transport = await attachLivePane({ onError, onPtyRebind }) + + // The host fenced this handle during the outage and has already minted its + // successor; only a require-replacement retry can reach it. + failNextResubscribeWith(new Error('terminal_handle_stale'), 'terminal-2') + dropMultiplexedStream() + + await vi.waitFor(() => expect(subscribedTerminalHandles()).toContain('terminal-2')) + // Two resolvePane round-trips: the stale one, then the require-replacement retry. + expect(methodLog.filter((method) => method === 'terminal.resolvePane')).toHaveLength(2) + expect(onPtyRebind).toHaveBeenCalledWith('remote:env-1@@terminal-2', FIRST_PTY_ID) + expect(transport.getPtyId()).toBe('remote:env-1@@terminal-2') + + emitSnapshot(latestSubscribePayload().streamId, 'after replacement') + expect(transport.isConnected()).toBe(true) + expect(transport.getRecoveryState?.().phase).toBe('connected') + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('retires the pane when the stale resubscribe finds only the fenced handle', async () => { + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ onError, onPtyExit }) + + // Host still advertises the fenced handle; require-replacement forbids + // reattaching to it, so the pane must retire rather than mirror a dead PTY. + failNextResubscribeWith(new Error('terminal_handle_stale'), FIRST_HANDLE) + dropMultiplexedStream() + + await vi.waitFor(() => expect(onPtyExit).toHaveBeenCalledWith(FIRST_PTY_ID)) + expect(methodLog.filter((method) => method === 'terminal.resolvePane')).toHaveLength(2) + expect(subscribedTerminalHandles().filter((handle) => handle === FIRST_HANDLE)).toHaveLength(1) + expect(transport.getRecoveryState?.().phase).toBe('ended') + expect(transport.getPtyId()).toBeNull() + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('retires a pane whose stale resubscribe has no tab/leaf ids to re-resolve', async () => { + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ + tabId: undefined, + leafId: undefined, + onError, + onPtyExit + }) + + // Without tab/leaf/worktree coordinates the resubscribe goes straight back + // to the multiplexer, and a stale rejection there has nothing to re-resolve. + subscribeOutcomes = [new Error('terminal_handle_stale')] + dropMultiplexedStream() + + await vi.waitFor(() => expect(onPtyExit).toHaveBeenCalledWith(FIRST_PTY_ID)) + expect(transport.getRecoveryState?.().phase).toBe('ended') + expect(transport.getPtyId()).toBeNull() + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('retires the pane when a resubscribe fails with terminal-gone', async () => { + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ onError, onPtyExit }) + + failNextResubscribeWith(new Error('terminal_gone')) + dropMultiplexedStream() + + await vi.waitFor(() => expect(onPtyExit).toHaveBeenCalledWith(FIRST_PTY_ID)) + // Lifecycle evidence, not a replaceable handle: no second re-resolve. + expect(methodLog.filter((method) => method === 'terminal.resolvePane')).toHaveLength(1) + expect(transport.getRecoveryState?.().phase).toBe('ended') + expect(transport.getPtyId()).toBeNull() + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('recovers the host pane when a resubscribe fails with an expired SSH session', async () => { + const onError = vi.fn() + const onPtyRebind = vi.fn() + const transport = await attachLivePane({ onError, onPtyRebind }) + + recoverPaneOutcome = { handle: 'terminal-3' } + failNextResubscribeWith(new Error('SSH_SESSION_EXPIRED')) + dropMultiplexedStream() + + await vi.waitFor(() => expect(methodLog).toContain('terminal.recoverPane')) + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'terminal.recoverPane', + params: { + paneKey: `${PANE_TAB_ID}:${PANE_LEAF_ID}`, + worktreeId: PANE_WORKTREE_ID, + expectedTerminal: FIRST_HANDLE + } + }) + ) + await vi.waitFor(() => expect(subscribedTerminalHandles()).toContain('terminal-3')) + expect(onPtyRebind).toHaveBeenCalledWith('remote:env-1@@terminal-3', FIRST_PTY_ID) + + emitSnapshot(latestSubscribePayload().streamId, 'after ssh pane recovery') + expect(transport.isConnected()).toBe(true) + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('keeps an oversized-snapshot resubscribe failure informational', async () => { + const { REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE } = + await import('../../runtime/remote-runtime-terminal-multiplexer') + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ onError, onPtyExit }) + + failNextResubscribeWith(new Error(REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE)) + dropMultiplexedStream() + + await vi.waitFor(() => expect(transport.getRecoveryState?.().phase).toBe('disconnected')) + // The snapshot was skipped, not the terminal: no red banner, no retirement. + expect(onError).not.toHaveBeenCalled() + expect(onPtyExit).not.toHaveBeenCalled() + expect(transport.getPtyId()).toBe(FIRST_PTY_ID) + transport.destroy?.() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-error-accumulation.test.ts b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.test.ts new file mode 100644 index 000000000..3de52452c --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from 'vitest' +import { appendTerminalErrorMessage } from './terminal-error-accumulation' +import { stripSshReconnectOwnedErrorLines } from './TerminalErrorToast' + +const MULTILINE_ERROR = 'Remote terminal write failed.\nThe remote runtime rejected the request.' + +describe('appendTerminalErrorMessage', () => { + it('starts the surface with the first message', () => { + expect(appendTerminalErrorMessage(null, 'Paste failed.')).toBe('Paste failed.') + }) + + it('appends distinct messages as newline-joined entries', () => { + const accumulated = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, 'Paste failed.'), + 'Remote terminal was closed.' + ) + expect(accumulated).toBe('Paste failed.\nRemote terminal was closed.') + }) + + it('keeps the first occurrence of a repeated single-line message', () => { + const accumulated = appendTerminalErrorMessage(null, 'Paste failed.') + expect(appendTerminalErrorMessage(accumulated, 'Paste failed.')).toBe(accumulated) + }) + + it('does not re-append a repeated multi-line message', () => { + let accumulated = appendTerminalErrorMessage(null, MULTILINE_ERROR) + accumulated = appendTerminalErrorMessage(accumulated, MULTILINE_ERROR) + accumulated = appendTerminalErrorMessage(accumulated, MULTILINE_ERROR) + expect(accumulated).toBe(MULTILINE_ERROR) + }) + + it('detects a repeated multi-line message in any position of the surface', () => { + const leading = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, MULTILINE_ERROR), + 'Paste failed.' + ) + expect(appendTerminalErrorMessage(leading, MULTILINE_ERROR)).toBe(leading) + + const trailing = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, 'Paste failed.'), + MULTILINE_ERROR + ) + expect(appendTerminalErrorMessage(trailing, MULTILINE_ERROR)).toBe(trailing) + + const middle = appendTerminalErrorMessage(trailing, 'Remote terminal was closed.') + expect(appendTerminalErrorMessage(middle, MULTILINE_ERROR)).toBe(middle) + }) + + it('keeps per-line dedup for a single-line message already present as a line', () => { + const accumulated = appendTerminalErrorMessage(null, MULTILINE_ERROR) + expect(appendTerminalErrorMessage(accumulated, 'Remote terminal write failed.')).toBe( + accumulated + ) + }) + + it('appends a message that is only a substring of an existing line', () => { + const accumulated = appendTerminalErrorMessage(null, MULTILINE_ERROR) + expect(appendTerminalErrorMessage(accumulated, 'terminal write failed.')).toBe( + `${MULTILINE_ERROR}\nterminal write failed.` + ) + }) + + it('stays a newline-joined string the toast can still filter per line', () => { + const accumulated = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, 'SSH connection failed: host unreachable'), + MULTILINE_ERROR + ) + expect(stripSshReconnectOwnedErrorLines(accumulated)).toBe(MULTILINE_ERROR) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts new file mode 100644 index 000000000..719d02250 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts @@ -0,0 +1,22 @@ +// Why: the error surface aggregates every pane error into ONE newline-joined +// string so TerminalErrorToast's per-line filters (isSshReconnectOwnedTerminalError, +// stripSshReconnectOwnedErrorLines) keep working. That join makes line-based +// dedup wrong for messages that themselves contain newlines: a multi-line +// message is never one line of the accumulated value, so it would re-append on +// every recurrence and grow without bound. +function containsWholeLineRun(accumulated: string, message: string): boolean { + return ( + accumulated === message || + accumulated.startsWith(`${message}\n`) || + accumulated.endsWith(`\n${message}`) || + accumulated.includes(`\n${message}\n`) + ) +} + +/** Appends an error to the aggregated surface, keeping the first occurrence of an already-present message. */ +export function appendTerminalErrorMessage(accumulated: string | null, message: string): string { + if (!accumulated) { + return message + } + return containsWholeLineRun(accumulated, message) ? accumulated : `${accumulated}\n${message}` +} diff --git a/src/shared/remote-runtime-client-error-classification.test.ts b/src/shared/remote-runtime-client-error-classification.test.ts index b819f6ba4..786b3287a 100644 --- a/src/shared/remote-runtime-client-error-classification.test.ts +++ b/src/shared/remote-runtime-client-error-classification.test.ts @@ -5,14 +5,17 @@ import { } from './remote-runtime-client-error-classification' describe('remote runtime client error classification', () => { - it.each(['remote_runtime_unavailable', 'runtime_timeout', 'runtime_unavailable', 'reconnecting'])( - 'treats %s as recoverable', - (code) => { - expect(isRecoverableRemoteRuntimeConnectionError({ code, message: 'transport failed' })).toBe( - true - ) - } - ) + it.each([ + 'remote_runtime_unavailable', + 'runtime_rpc_queue_overloaded', + 'runtime_timeout', + 'runtime_unavailable', + 'reconnecting' + ])('treats %s as recoverable', (code) => { + expect(isRecoverableRemoteRuntimeConnectionError({ code, message: 'transport failed' })).toBe( + true + ) + }) it('does not retry authentication or protocol failures', () => { expect( @@ -31,6 +34,7 @@ describe('remote runtime client error classification', () => { 'Remote Orca runtime closed the connection.', 'Remote Orca runtime connection closed.', 'Remote Orca runtime is not connected.', + "Error invoking remote method 'runtimeEnvironments:call': RuntimeRpcCallQueueOverloadError: Remote runtime call queue is full; retry after current calls finish.", 'Remote runtime subscription closed before it started.' ])('normalizes unstructured connection failure: %s', (message) => { const error = toRemoteRuntimeClientErrorLike(new Error(message)) diff --git a/src/shared/remote-runtime-client-error-classification.ts b/src/shared/remote-runtime-client-error-classification.ts index af5a531d3..1732a454f 100644 --- a/src/shared/remote-runtime-client-error-classification.ts +++ b/src/shared/remote-runtime-client-error-classification.ts @@ -1,7 +1,11 @@ export type RemoteRuntimeClientErrorLike = { code?: string; message: string } +export const RUNTIME_RPC_QUEUE_OVERLOAD_CODE = 'runtime_rpc_queue_overloaded' +export const RUNTIME_RPC_QUEUE_OVERLOAD_MESSAGE_FRAGMENT = 'remote runtime call queue is full' + const RECOVERABLE_CODES = new Set([ 'remote_runtime_unavailable', + RUNTIME_RPC_QUEUE_OVERLOAD_CODE, 'runtime_timeout', 'runtime_unavailable', 'reconnecting', @@ -13,12 +17,20 @@ const RECOVERABLE_MESSAGE_FRAGMENTS = [ 'remote orca runtime closed the connection', 'remote orca runtime connection closed', 'remote orca runtime is not connected', + RUNTIME_RPC_QUEUE_OVERLOAD_MESSAGE_FRAGMENT, 'remote runtime connection closed', 'remote runtime subscription closed before it started', 'remote terminal stream is not connected', 'timed out waiting for the remote orca runtime' ] +export function isRuntimeRpcQueueOverloadError(error: RemoteRuntimeClientErrorLike): boolean { + return ( + error.code === RUNTIME_RPC_QUEUE_OVERLOAD_CODE || + error.message.toLowerCase().includes(RUNTIME_RPC_QUEUE_OVERLOAD_MESSAGE_FRAGMENT) + ) +} + export function isRecoverableRemoteRuntimeConnectionError( error: RemoteRuntimeClientErrorLike ): boolean {