diff --git a/src/main/ssh/ssh-channel-multiplexer.test.ts b/src/main/ssh/ssh-channel-multiplexer.test.ts index f58e41727..eeca0ce51 100644 --- a/src/main/ssh/ssh-channel-multiplexer.test.ts +++ b/src/main/ssh/ssh-channel-multiplexer.test.ts @@ -502,6 +502,50 @@ describe('SshChannelMultiplexer', () => { await expect(mux.request('pty.spawn')).rejects.toThrow('Multiplexer disposed') }) + it('tags a request after a shutdown dispose with DISPOSED', async () => { + mux.dispose() + + const error = (await mux.request('pty.spawn').catch((e: unknown) => e)) as Error & { + code?: string + } + expect(error.code).toBe('DISPOSED') + }) + + it('reports a request after a lost connection as transient', async () => { + transport.closeCallbacks[0]() + + const error = (await mux + .request('fs.readDir', { path: '/home/me' }) + .catch((e: unknown) => e)) as Error & { code?: string } + expect(error.message).toBe('SSH connection lost, reconnecting...') + expect(error.code).toBe('CONNECTION_LOST') + }) + + it('reports a settled notify after a lost connection as transient', () => { + transport.closeCallbacks[0]() + + const settled = vi.fn() + mux.notifyWithSettlement('pty.data', { id: 'pty-1', data: 'x' }, settled) + + expect(settled).toHaveBeenCalledWith({ + ok: false, + error: expect.objectContaining({ + message: 'SSH connection lost, reconnecting...', + code: 'CONNECTION_LOST' + }) + }) + }) + + it('fires a dispose handler registered after dispose with the recorded reason', () => { + mux.dispose('connection_lost') + + const disposeHandler = vi.fn() + mux.onDispose(disposeHandler) + + expect(disposeHandler).toHaveBeenCalledWith('connection_lost') + expect(disposeHandler).toHaveBeenCalledTimes(1) + }) + it('ignores notify after dispose', () => { mux.dispose() mux.notify('pty.data', { id: 'pty-1', data: 'x' }) diff --git a/src/main/ssh/ssh-channel-multiplexer.ts b/src/main/ssh/ssh-channel-multiplexer.ts index 206d008c1..0000f2f35 100644 --- a/src/main/ssh/ssh-channel-multiplexer.ts +++ b/src/main/ssh/ssh-channel-multiplexer.ts @@ -41,6 +41,21 @@ export type NotificationHandler = (method: string, params: Record) => void export type RequestHandler = (params: Record) => Promise | unknown +export type MultiplexerDisposeReason = 'shutdown' | 'connection_lost' + +// Why: the renderer uses the message/code to distinguish temporary disconnects +// (show reconnection overlay) from permanent shutdown (show error toast), so +// every producer of a disposal rejection must mint it here — a divergent copy +// silently downgrades the relay-lost UI to a bug-report toast. +export function createSshDisposalError(reason: MultiplexerDisposeReason): Error & { code: string } { + const lost = reason === 'connection_lost' + const err = new Error( + lost ? 'SSH connection lost, reconnecting...' : 'Multiplexer disposed' + ) as Error & { code: string } + err.code = lost ? 'CONNECTION_LOST' : 'DISPOSED' + return err +} + const REQUEST_TIMEOUT_MS = 30_000 const MAX_ORDINARY_UNACKED_TIMESTAMPS = 4095 const MAX_UNACKED_TIMESTAMPS = MAX_ORDINARY_UNACKED_TIMESTAMPS + 1 @@ -85,6 +100,7 @@ export class SshChannelMultiplexer { private disposeHandlers: ((reason: 'shutdown' | 'connection_lost') => void)[] = [] private connectionHealthTimer: ReturnType | null = null private disposed = false + private disposeReason: 'shutdown' | 'connection_lost' | null = null private decoderReadPaused = false private writerSaturated = false @@ -181,6 +197,12 @@ export class SshChannelMultiplexer { // never fires the reconnect logic. onDispose(handler: (reason: 'shutdown' | 'connection_lost') => void): () => void { if (this.disposed) { + // Why: a late subscriber must still learn the channel died; retaining it would leak the closure (#11953). + try { + handler(this.disposeReason ?? 'shutdown') + } catch { + // Don't let a handler error escape into the subscriber's registration path + } return () => {} } this.disposeHandlers.push(handler) @@ -201,7 +223,7 @@ export class SshChannelMultiplexer { options?: SshMultiplexerRequestOptions ): Promise { if (this.disposed) { - throw new Error('Multiplexer disposed') + throw this.disposedError() } if (options?.signal?.aborted) { const error = new Error(`Request "${method}" was cancelled`) as Error & { name: string } @@ -289,7 +311,7 @@ export class SshChannelMultiplexer { onSettled: (result: { ok: true } | { ok: false; error: Error }) => void ): void { if (this.disposed) { - onSettled({ ok: false, error: new Error('Multiplexer disposed') }) + onSettled({ ok: false, error: this.disposedError() }) return } this.sendMessage( @@ -338,33 +360,24 @@ export class SshChannelMultiplexer { ) } this.disposed = true + this.disposeReason = reason if (this.connectionHealthTimer) { clearInterval(this.connectionHealthTimer) this.connectionHealthTimer = null } - // Why: the renderer uses the error code to distinguish temporary disconnects - // (show reconnection overlay) from permanent shutdown (show error toast). - const errorMessage = - reason === 'connection_lost' ? 'SSH connection lost, reconnecting...' : 'Multiplexer disposed' - const errorCode = reason === 'connection_lost' ? 'CONNECTION_LOST' : 'DISPOSED' - for (const waiter of this.livenessProbeWaiters.splice(0)) { waiter.fail() } for (const [id, pending] of this.pendingRequests) { pending.cleanup() - const err = new Error(errorMessage) as Error & { code: string } - err.code = errorCode - pending.reject(err) + pending.reject(this.disposedError()) this.pendingRequests.delete(id) } - const writerError = new Error(errorMessage) as Error & { code: string } - writerError.code = errorCode - this.writer.dispose(writerError) + this.writer.dispose(this.disposedError()) this.unackedTimestamps.clear() // Why: relay teardown can race with late provider registration; disposed // muxes must not retain provider/session closures through subscribers. @@ -389,6 +402,10 @@ export class SshChannelMultiplexer { // ── Private ─────────────────────────────────────────────────────── + private disposedError(): Error & { code: string } { + return createSshDisposalError(this.disposeReason ?? 'shutdown') + } + private sendMessage( msg: JsonRpcMessage, onSettled?: (result: MultiplexerWriteSettlement) => void diff --git a/src/main/ssh/ssh-git-response-stream-reader.test.ts b/src/main/ssh/ssh-git-response-stream-reader.test.ts new file mode 100644 index 000000000..8b08701a1 --- /dev/null +++ b/src/main/ssh/ssh-git-response-stream-reader.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it, vi } from 'vitest' +import { requestGitStreamable } from './ssh-git-response-stream-reader' +import { SshChannelMultiplexer, type MultiplexerTransport } from './ssh-channel-multiplexer' + +function createMockTransport(): MultiplexerTransport { + return { + write: () => {}, + onData: () => {}, + onClose: () => {} + } +} + +describe('requestGitStreamable on an already-dead multiplexer', () => { + it('rejects as a transient relay loss and leaves no listener on the caller signal', async () => { + const mux = new SshChannelMultiplexer(createMockTransport()) + mux.dispose('connection_lost') + const controller = new AbortController() + const addListener = vi.spyOn(controller.signal, 'addEventListener') + const removeListener = vi.spyOn(controller.signal, 'removeEventListener') + + await expect( + requestGitStreamable(mux, 'git.status', { cwd: '/repo' }, { signal: controller.signal }) + ).rejects.toThrow('SSH connection lost, reconnecting...') + + // #11953: a disposed mux fails synchronously inside onDispose, so the abort + // listener must already be registered when that cleanup runs — otherwise it + // outlives the request for the lifetime of the caller's signal. + expect(removeListener).toHaveBeenCalledTimes(addListener.mock.calls.length) + }) +}) diff --git a/src/main/ssh/ssh-git-response-stream-reader.ts b/src/main/ssh/ssh-git-response-stream-reader.ts index ef63c7a6d..6a50dc28a 100644 --- a/src/main/ssh/ssh-git-response-stream-reader.ts +++ b/src/main/ssh/ssh-git-response-stream-reader.ts @@ -1,4 +1,5 @@ import type { SshChannelMultiplexer } from './ssh-channel-multiplexer' +import { createSshDisposalError } from './ssh-channel-multiplexer' import { RelayErrorCode, isGitResponseStreamMarker } from './relay-protocol' const SENTINEL_STREAM_ID = -1 @@ -245,18 +246,6 @@ export function requestGitStreamable( handleStreamError(p) }) ) - unsubscribers.push( - mux.onDispose((reason) => { - const err = new Error( - reason === 'connection_lost' - ? 'SSH connection lost, reconnecting...' - : 'Multiplexer disposed' - ) as Error & { code: string } - err.code = reason === 'connection_lost' ? 'CONNECTION_LOST' : 'DISPOSED' - fail(err) - }) - ) - if (options?.signal) { const signal = options.signal if (signal.aborted) { @@ -274,6 +263,10 @@ export function requestGitStreamable( unsubscribers.push(() => signal.removeEventListener('abort', onAbort)) } + // Why: registered last because an already-disposed mux fails synchronously here, + // and that cleanup must be able to drop the abort listener above (#11953). + unsubscribers.push(mux.onDispose((reason) => fail(createSshDisposalError(reason)))) + // Why: forward only the mux-request options (signal/timeoutMs) and omit them // entirely when absent, so callers that previously issued a 2-arg // mux.request keep the same call shape (and their tests). inactivityTimeoutMs diff --git a/src/main/ssh/ssh-relay-session-relay-loss.test.ts b/src/main/ssh/ssh-relay-session-relay-loss.test.ts new file mode 100644 index 000000000..9e70466ab --- /dev/null +++ b/src/main/ssh/ssh-relay-session-relay-loss.test.ts @@ -0,0 +1,259 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { SshRelaySession } from './ssh-relay-session' +import type { SshConnection } from './ssh-connection' +import { SSH_RELAY_CONFIGURE_GRACE_TIME_METHOD } from '../../shared/ssh-types' +import { createMockDeps, mockDeploySuccess } from './ssh-relay-session-test-fixtures' + +// #11953: the grace-time notify is the last thing establish()/reconnect() do +// before latching 'ready', and it can dispose the mux synchronously (writer +// admission cap / throwing transport). Latching 'ready' there left the status +// bar on "connected" with fs/pty/git providers bound to a dead mux and no +// relay-loss watcher, so nothing ever scheduled a reconnect. + +const { muxRequestMock, openConsumerSessionMock, registeredPtyProvider } = vi.hoisted(() => ({ + muxRequestMock: vi.fn(), + openConsumerSessionMock: vi.fn(), + registeredPtyProvider: { dispose: vi.fn(), attachForReconnect: vi.fn() } +})) + +vi.mock('./ssh-relay-deploy', () => ({ deployAndLaunchRelay: vi.fn() })) +vi.mock('./ssh-pty-consumer-session', () => ({ + openSshPtyConsumerSession: openConsumerSessionMock +})) +vi.mock('../ipc/ssh-pty-output-intake-registry', () => ({ + acceptSshPtyOutputData: vi.fn().mockResolvedValue(undefined), + acceptSshPtyOutputExit: vi.fn().mockResolvedValue(undefined), + allocateSshPtyProviderGeneration: vi.fn(() => 41), + beginSshPtyOutputGenerationMigration: vi.fn(() => ({ + byPty: new Map(), + completion: Promise.resolve() + })), + closeSshPtyOutputGeneration: vi.fn(), + getSshPtyAcceptedSourceCheckpoints: vi.fn(() => []), + applySshPtySourceCancellationProof: vi.fn(() => true), + applySshPtySourceRecoveryCancellationProof: vi.fn(() => true), + installSshPtySourceAckPublisher: vi.fn(() => () => {}), + installSshPtySourceCancellationPublisher: vi.fn(() => () => {}) +})) +vi.mock('./ssh-relay-deploy-helpers', () => ({ + execCommand: vi.fn().mockResolvedValue('') +})) + +// Mirrors the real multiplexer contract: dispose() latches, and a failed write +// during notify disposes the mux synchronously via handleProtocolError. +vi.mock('./ssh-channel-multiplexer', () => ({ + SshChannelMultiplexer: class MockSshChannelMultiplexer { + private disposed = false + private disposeReason: string | null = null + private disposeHandlers: ((reason: string) => void)[] = [] + /** Set by the test to kill the channel from inside notify(). */ + failNotifyMethod: string | null = null + notify = vi.fn((method: string) => { + if (method === this.failNotifyMethod) { + this.dispose('connection_lost') + } + }) + notifyWithSettlement = vi.fn() + request = muxRequestMock + onNotification = vi.fn().mockReturnValue(() => {}) + onNotificationByMethod = vi.fn().mockReturnValue(() => {}) + onRequest = vi.fn().mockReturnValue(() => {}) + onDispose = vi.fn((handler: (reason: string) => void) => { + if (this.disposed) { + handler(this.disposeReason ?? 'shutdown') + return () => {} + } + this.disposeHandlers.push(handler) + return () => { + const idx = this.disposeHandlers.indexOf(handler) + if (idx !== -1) { + this.disposeHandlers.splice(idx, 1) + } + } + }) + dispose = vi.fn((reason = 'shutdown') => { + if (this.disposed) { + return + } + this.disposed = true + this.disposeReason = reason + for (const handler of this.disposeHandlers.splice(0)) { + handler(reason) + } + }) + isDisposed = vi.fn(() => this.disposed) + } +})) + +vi.mock('../providers/ssh-pty-provider', () => ({ + isSshPtyNotFoundError: () => false, + isSshPtyIdentityMismatchError: () => false, + SshPtyProvider: class MockSshPtyProvider { + onData = vi.fn().mockReturnValue(() => {}) + onReplay = vi.fn().mockReturnValue(() => {}) + onExit = vi.fn().mockReturnValue(() => {}) + attach = vi.fn().mockResolvedValue(undefined) + attachForReconnect = vi.fn().mockResolvedValue({}) + setPtyDeliveryPauseAdapter = vi.fn() + dispose = vi.fn() + } +})) +vi.mock('../providers/ssh-filesystem-provider', () => ({ + SshFilesystemProvider: class MockSshFilesystemProvider { + dispose = vi.fn() + } +})) +vi.mock('../providers/ssh-git-provider', () => ({ + SshGitProvider: class MockSshGitProvider {} +})) +vi.mock('../ipc/pty', () => ({ + registerSshPtyProvider: vi.fn(), + unregisterSshPtyProvider: vi.fn(), + getSshPtyProvider: vi.fn().mockReturnValue(registeredPtyProvider), + getPtyIdsForConnection: vi.fn().mockReturnValue([]), + clearPtyOwnershipForConnection: vi.fn(), + clearProviderPtyState: vi.fn(), + deletePtyOwnership: vi.fn(), + setPtyOwnership: vi.fn(), + restorePtyIncarnation: vi.fn(), + isCurrentPtyExit: vi.fn(() => true) +})) +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + registerSshFilesystemProvider: vi.fn(), + unregisterSshFilesystemProvider: vi.fn(), + getSshFilesystemProvider: vi.fn().mockReturnValue({ dispose: vi.fn() }) +})) +vi.mock('../providers/ssh-git-dispatch', () => ({ + registerSshGitProvider: vi.fn(), + unregisterSshGitProvider: vi.fn() +})) + +const { registerSshFilesystemProvider, unregisterSshFilesystemProvider } = + await import('../providers/ssh-filesystem-dispatch') +const { getPtyIdsForConnection } = await import('../ipc/pty') + +describe('SshRelaySession relay loss during setup', () => { + /** Armed by a test so the *next* mux dies inside its grace-time notify. */ + let armGraceTimeFailure = false + let armProviderRegistrationFailure = false + + beforeEach(() => { + vi.clearAllMocks() + armGraceTimeFailure = false + armProviderRegistrationFailure = false + muxRequestMock.mockReset() + vi.mocked(getPtyIdsForConnection).mockReturnValue([]) + registeredPtyProvider.attachForReconnect.mockReset().mockResolvedValue({}) + openConsumerSessionMock.mockImplementation(async (_mux, options) => ({ + mode: 'legacy-fallback', + clientInstanceId: options.clientInstanceId, + serverBuildId: 'test-relay-build' + })) + mockDeploySuccess() + }) + + function createSession(): { + session: SshRelaySession + onRelayLost: ReturnType + onReady: ReturnType + mockStore: ReturnType['mockStore'] + } { + const { mockStore, mockPortForward, getMainWindow } = createMockDeps() + const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) + const onRelayLost = vi.fn() + const onReady = vi.fn() + session.setOnRelayLost(onRelayLost) + session.setOnReady(onReady) + // Arm once the mux exists; every attempt issues requests before the notify. + muxRequestMock.mockImplementation(async (method: string) => { + const mux = session.getMux() as unknown as { + failNotifyMethod: string | null + dispose: (reason: string) => void + } | null + if (mux && armGraceTimeFailure) { + mux.failNotifyMethod = SSH_RELAY_CONFIGURE_GRACE_TIME_METHOD + } + if (mux && armProviderRegistrationFailure && method === 'git.listWorktrees') { + mux.dispose('connection_lost') + throw new Error('SSH connection lost, reconnecting...') + } + return [] + }) + return { session, onRelayLost, onReady, mockStore } + } + + it('fails establish instead of reporting ready on a dead channel', async () => { + const { session, onReady } = createSession() + armGraceTimeFailure = true + + await expect(session.establish({} as SshConnection)).rejects.toThrow( + 'Relay connection lost during establish' + ) + + expect(registerSshFilesystemProvider).toHaveBeenCalledWith('target-1', expect.anything()) + expect(session.getState()).not.toBe('ready') + expect(onReady).not.toHaveBeenCalled() + expect(unregisterSshFilesystemProvider).toHaveBeenCalledWith('target-1') + }) + + it('routes a dead channel during reconnect into relay-loss recovery', async () => { + const { session, onRelayLost, onReady } = createSession() + + await session.establish({} as SshConnection) + expect(session.getState()).toBe('ready') + onReady.mockClear() + + armGraceTimeFailure = true + await session.reconnect({} as SshConnection) + + expect(session.getState()).not.toBe('ready') + expect(onReady).not.toHaveBeenCalled() + expect(onRelayLost).toHaveBeenCalledTimes(1) + expect(unregisterSshFilesystemProvider).toHaveBeenCalledWith('target-1') + }) + + it('routes a mux that dies during provider registration into relay-loss recovery', async () => { + const { session, onRelayLost, onReady, mockStore } = createSession() + + await session.establish({} as SshConnection) + expect(session.getState()).toBe('ready') + onReady.mockClear() + + vi.mocked(mockStore.getRepos).mockReturnValue([ + { connectionId: 'target-1', path: '/repo' } as ReturnType[number] + ]) + armProviderRegistrationFailure = true + + await session.reconnect({} as SshConnection) + + expect(session.getState()).not.toBe('ready') + expect(onReady).not.toHaveBeenCalled() + expect(onRelayLost).toHaveBeenCalledTimes(1) + expect(unregisterSshFilesystemProvider).toHaveBeenCalledWith('target-1') + }) + + // #11953: reattachKnownPtys swallows every per-PTY failure, so a mux killed by the + // reattach burst itself never reaches the catch — the post-reattach gate has to notice. + it('routes a mux that dies during PTY reattach into relay-loss recovery', async () => { + const { session, onRelayLost, onReady } = createSession() + + await session.establish({} as SshConnection) + expect(session.getState()).toBe('ready') + onReady.mockClear() + + vi.mocked(getPtyIdsForConnection).mockReturnValue(['pty-1']) + registeredPtyProvider.attachForReconnect.mockImplementation(async () => { + const mux = session.getMux() as unknown as { dispose: (reason: string) => void } | null + mux?.dispose('connection_lost') + throw new Error('SSH connection lost, reconnecting...') + }) + + await session.reconnect({} as SshConnection) + + expect(registeredPtyProvider.attachForReconnect).toHaveBeenCalled() + expect(session.getState()).not.toBe('ready') + expect(onReady).not.toHaveBeenCalled() + expect(onRelayLost).toHaveBeenCalledTimes(1) + expect(unregisterSshFilesystemProvider).toHaveBeenCalledWith('target-1') + }) +}) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 34bdf4988..5f97828b6 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -126,6 +126,22 @@ const SSH_PTY_REATTACH_ATTEMPT_TIMEOUT_MS = 10_000 const SSH_PTY_REATTACH_RETRY_MIN_DELAY_MS = 50 const SSH_PTY_REATTACH_RETRY_JITTER_MS = 200 const SSH_SOURCE_RECOVERY_CANCELLATION_FAILED = 'ssh_source_recovery_cancellation_failed' + +// Why: superseded attempts stop quietly; a dead mux still owned by this attempt must enter recovery. +function verifyRelayAttempt( + mux: SshChannelMultiplexer, + isAttemptCurrent: () => boolean, + phase: string +): boolean { + if (!isAttemptCurrent()) { + return false + } + if (mux.isDisposed()) { + throw new Error(`Relay connection lost during ${phase}`) + } + return true +} + type PendingPtyReattach = { mux: SshChannelMultiplexer providerGeneration: number @@ -468,14 +484,15 @@ export class SshRelaySession { const mux = new SshChannelMultiplexer(transport) this.mux = mux - const ownsAttempt = (): boolean => this.mux === mux && !mux.isDisposed() && !this.isDisposed() + const isAttemptCurrent = (): boolean => this.mux === mux && !this.isDisposed() + const shouldContinue = (): boolean => isAttemptCurrent() && !mux.isDisposed() const ptyConsumerSessionState = await this.openPtyConsumerSession( mux, serverBuildId, - ownsAttempt + shouldContinue ) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'consumer session setup')) { if (!mux.isDisposed()) { mux.dispose() } @@ -483,7 +500,7 @@ export class SshRelaySession { } this.ptyConsumerSessionState = ptyConsumerSessionState await this.rememberPtyConsumerRecovery(serverBuildId) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'consumer recovery persistence')) { if (!mux.isDisposed()) { mux.dispose() } @@ -491,7 +508,7 @@ export class SshRelaySession { } await mux.request('session.resolveHome', { path: '~' }) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'home resolution')) { if (!mux.isDisposed()) { mux.dispose() } @@ -499,33 +516,35 @@ export class SshRelaySession { } const connectionIncarnation = randomUUID() - const registered = await this.registerProviders(mux, ownsAttempt, connectionIncarnation) + const registered = await this.registerProviders(mux, shouldContinue, connectionIncarnation) if (!registered) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'provider registration')) { + if (!mux.isDisposed()) { + mux.dispose() + } + throw new Error('Session disposed during establish') + } + throw new Error('Relay provider registration stopped unexpectedly') + } + + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'provider registration')) { if (!mux.isDisposed()) { mux.dispose() } throw new Error('Session disposed during establish') } - // Why: registerProviders swallows mux errors, so an isDisposed check catches a transport that closed mid-registration before we reach 'ready'. - if (mux.isDisposed()) { - throw new Error('Relay connection lost during provider registration') - } - - if (this.isDisposed()) { - this.teardownProviders('connection_lost') - throw new Error('Session disposed during establish') - } - // Why: explicit disconnect keeps PTY ownership, so a later manual connect must reattach those remote PTYs. - await this.reattachKnownPtys(mux, ownsAttempt) + await this.reattachKnownPtys(mux, shouldContinue) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'PTY reattach')) { throw new Error('Session disposed during establish') } this.configureRelayGraceTime(mux, graceTimeSeconds) + verifyRelayAttempt(mux, isAttemptCurrent, 'establish') this.watchMuxForRelayLoss(mux) + verifyRelayAttempt(mux, isAttemptCurrent, 'establish') this._state = 'ready' this.startPortScanning() this._onReady?.(this.targetId) @@ -609,19 +628,19 @@ export class SshRelaySession { const mux = new SshChannelMultiplexer(transport) this.mux = mux - const ownsAttempt = (): boolean => + const isAttemptCurrent = (): boolean => this.mux === mux && - !mux.isDisposed() && this.abortController === abortController && !abortController.signal.aborted && !this.isDisposed() + const shouldContinue = (): boolean => isAttemptCurrent() && !mux.isDisposed() const ptyConsumerSessionState = await this.openPtyConsumerSession( mux, serverBuildId, - ownsAttempt + shouldContinue ) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'consumer session setup')) { if (!mux.isDisposed()) { mux.dispose() } @@ -629,7 +648,7 @@ export class SshRelaySession { } this.ptyConsumerSessionState = ptyConsumerSessionState await this.rememberPtyConsumerRecovery(serverBuildId) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'consumer recovery persistence')) { if (!mux.isDisposed()) { mux.dispose() } @@ -637,7 +656,7 @@ export class SshRelaySession { } await mux.request('session.resolveHome', { path: '~' }) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'home resolution')) { if (!mux.isDisposed()) { mux.dispose() } @@ -645,20 +664,21 @@ export class SshRelaySession { } const connectionIncarnation = randomUUID() - const registered = await this.registerProviders(mux, ownsAttempt, connectionIncarnation) + const registered = await this.registerProviders(mux, shouldContinue, connectionIncarnation) if (!registered) { - if (!mux.isDisposed()) { - mux.dispose() + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'provider registration')) { + if (this.mux === mux) { + this.teardownProviders('shutdown') + } else if (!mux.isDisposed()) { + mux.dispose() + } + return } - return - } - - if (mux.isDisposed()) { - throw new Error('Relay connection lost during provider registration') + throw new Error('Relay provider registration stopped unexpectedly') } // Why: dispose() during registration/attach already cleaned up, but this.mux was reassigned above — clean up the new mux so it doesn't leak. - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'provider registration')) { if (this.mux === mux) { this.teardownProviders('shutdown') } else if (!mux.isDisposed()) { @@ -667,14 +687,16 @@ export class SshRelaySession { return } - await this.reattachKnownPtys(mux, ownsAttempt) + await this.reattachKnownPtys(mux, shouldContinue) - if (!ownsAttempt()) { + if (!verifyRelayAttempt(mux, isAttemptCurrent, 'PTY reattach')) { return } this.configureRelayGraceTime(mux, graceTimeSeconds) + verifyRelayAttempt(mux, isAttemptCurrent, 'reconnect') this.watchMuxForRelayLoss(mux) + verifyRelayAttempt(mux, isAttemptCurrent, 'reconnect') this._state = 'ready' this.startPortScanning() this._onReady?.(this.targetId) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts index 03bb896eb..56e7ffd0a 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts @@ -7,6 +7,9 @@ import { const SSH_FAILURE = "SSH connection failed: Error invoking remote method 'ssh:connect': Error: Relay package for linux-x64 not found locally." +// Relay loss reaches reportError already IPC-wrapped, so the marker is mid-string. +const RELAY_LOST = + "Error invoking remote method 'pty:attach': Error: SSH connection lost, reconnecting..." describe('isSshReconnectOwnedTerminalError', () => { it('matches raw ssh:connect failures and inactive-host messages', () => { @@ -22,6 +25,11 @@ describe('isSshReconnectOwnedTerminalError', () => { ).toBe(true) }) + it('matches an IPC-wrapped relay-loss message', () => { + expect(isSshReconnectOwnedTerminalError(RELAY_LOST)).toBe(true) + expect(isSshReconnectOwnedTerminalError('SSH connection lost, reconnecting...')).toBe(true) + }) + it('leaves unrelated terminal errors for the toast', () => { expect(isSshReconnectOwnedTerminalError('Paste failed.')).toBe(false) expect(isSshReconnectOwnedTerminalError('node-pty: open_slave failed: EMFILE')).toBe(false) @@ -49,6 +57,11 @@ describe('stripSshReconnectOwnedErrorLines', () => { ).toBe('Paste failed.') }) + it('drops an IPC-wrapped relay-loss line and keeps the rest', () => { + expect(stripSshReconnectOwnedErrorLines(RELAY_LOST)).toBeNull() + expect(stripSshReconnectOwnedErrorLines(`Paste failed.\n${RELAY_LOST}`)).toBe('Paste failed.') + }) + it('leaves an error with no SSH text untouched', () => { expect(stripSshReconnectOwnedErrorLines('Paste failed.')).toBe('Paste failed.') }) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index 7289dba79..7f38733fa 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -2,6 +2,8 @@ import { translate } from '@/i18n/i18n' const SSH_PREFIX = 'SSH connection is not active' // Produced by pty-connection.ts reportError() when a PTY reattach can't reach its SSH host. const SSH_CONNECT_FAILURE_PREFIX = 'SSH connection failed' +// Matched with includes(): this arrives IPC-wrapped ("Error invoking remote method 'pty:…': Error: …"). +const SSH_RELAY_LOST_MARKER = 'SSH connection lost, reconnecting' const STALE_NODE_PTY_DAEMON_MARKERS = [ "Daemon's node-pty install is gone", 'node-pty: posix_spawn failed: ENOENT' @@ -12,12 +14,16 @@ const STALE_DAEMON_CWD_MARKERS = [ ] function isSshError(error: string): boolean { - return error.startsWith(SSH_PREFIX) + return error.startsWith(SSH_PREFIX) || error.includes(SSH_RELAY_LOST_MARKER) } /** A single error line the SSH reconnect banner already covers — hide instead of stacking under/over it. */ export function isSshReconnectOwnedTerminalError(error: string): boolean { - return error.startsWith(SSH_CONNECT_FAILURE_PREFIX) || error.startsWith(SSH_PREFIX) + return ( + error.startsWith(SSH_CONNECT_FAILURE_PREFIX) || + error.startsWith(SSH_PREFIX) || + error.includes(SSH_RELAY_LOST_MARKER) + ) } // Why: onPtyError aggregates errors into one newline-joined string, so classify per line —