diff --git a/src/main/ssh/ssh-connection-utils.ts b/src/main/ssh/ssh-connection-utils.ts index aea238ef2..cc8269c1b 100644 --- a/src/main/ssh/ssh-connection-utils.ts +++ b/src/main/ssh/ssh-connection-utils.ts @@ -100,6 +100,13 @@ export function wrapRemoteCommandForPosixShell(command: string): string { export type SshExecOptions = { wrapCommand?: boolean + signal?: AbortSignal +} + +export function createSshOperationAbortError(): Error & { name: string } { + const error = new Error('SSH operation was cancelled') as Error & { name: string } + error.name = 'AbortError' + return error } function cmdEscape(s: string): string { diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index ade239d56..f40d27077 100644 --- a/src/main/ssh/ssh-connection.test.ts +++ b/src/main/ssh/ssh-connection.test.ts @@ -302,6 +302,14 @@ describe('SshConnection', () => { expect(clientInstances[0].setNoDelay).toHaveBeenCalledWith(true) }) + it('allows concurrent exec commands for ssh2 transport', async () => { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + + expect(conn.usesSystemSshTransport()).toBe(false) + expect(conn.canRunConcurrentExecCommands()).toBe(true) + }) + it('removes startup listeners after ssh2 connect succeeds', async () => { const conn = new SshConnection(createTarget(), createCallbacks()) @@ -762,8 +770,36 @@ describe('SshConnection', () => { ) }) + it('allows concurrent exec commands for system SSH with an Orca ControlMaster socket', async () => { + getOrcaControlSocketPathMock.mockReturnValue('/tmp/orca-ssh-501/live-socket') + vi.mocked(resolveWithSshG).mockResolvedValueOnce(createResolvedConfig()) + const conn = new SshConnection(createTarget({ configHost: 'fdpass-host' }), createCallbacks()) + + await conn.connect() + + expect(conn.usesSystemSshTransport()).toBe(true) + expect(conn.canRunConcurrentExecCommands()).toBe(true) + }) + + it('keeps concurrent exec commands disabled for system SSH without a reusable socket', async () => { + getOrcaControlSocketPathMock.mockReturnValue(null) + vi.mocked(resolveWithSshG).mockResolvedValueOnce(createResolvedConfig()) + const conn = new SshConnection( + createTarget({ configHost: 'fdpass-host', systemSshConnectionReuse: false }), + createCallbacks() + ) + + await conn.connect() + + expect(conn.usesSystemSshTransport()).toBe(true) + expect(conn.canRunConcurrentExecCommands()).toBe(false) + }) + it('retries a failed system SSH probe without ControlMaster and disables mux for the session', async () => { - getOrcaControlSocketPathMock.mockReturnValue('/tmp/orca-ssh-501/stale-socket') + getOrcaControlSocketPathMock.mockImplementation( + (_target: SshTarget, options?: { disableControlMaster?: boolean }) => + options?.disableControlMaster ? null : '/tmp/orca-ssh-501/stale-socket' + ) spawnSystemSshCommandMock .mockImplementationOnce(() => createFailingSystemCommandChannel(255, 'mux client failed')) .mockImplementation(() => createSystemCommandChannel()) @@ -812,6 +848,7 @@ describe('SshConnection', () => { resolvedConfig: expect.objectContaining({ proxyUseFdpass: true }) }) ) + expect(conn.canRunConcurrentExecCommands()).toBe(false) }) it('uses system SSH transport for ProxyCommand targets before ssh2 auth', async () => { @@ -952,7 +989,10 @@ describe('SshConnection', () => { }) it('retries direct system SSH connections without ControlMaster after mux startup failure', async () => { - getOrcaControlSocketPathMock.mockReturnValue('/tmp/orca-ssh-501/stale-socket') + getOrcaControlSocketPathMock.mockImplementation( + (_target: SshTarget, options?: { disableControlMaster?: boolean }) => + options?.disableControlMaster ? null : '/tmp/orca-ssh-501/stale-socket' + ) spawnSystemSshMock .mockReturnValueOnce(createFailingSystemSshProcess(255)) .mockImplementation(() => createSystemSshProcess()) @@ -977,6 +1017,7 @@ describe('SshConnection', () => { resolvedConfig: expect.objectContaining({ proxyUseFdpass: true }) } ) + expect(conn.canRunConcurrentExecCommands()).toBe(false) }) it('kills delayed direct system SSH startup on disconnect and ignores late stdout', async () => { diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index bdc7543c5..528f03c4c 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -30,6 +30,7 @@ import { resolveEffectiveProxy, spawnProxyCommand, wrapRemoteCommandForPosixShell, + createSshOperationAbortError, type SshExecOptions, type SshConnectionCallbacks } from './ssh-connection-utils' @@ -85,6 +86,16 @@ export class SshConnection { usesSystemSshTransport(): boolean { return this.useSystemSshTransport } + canRunConcurrentExecCommands(): boolean { + if (!this.useSystemSshTransport) { + return true + } + return ( + getOrcaControlSocketPath(this.target, { + ...this.getSystemSshBuildArgsOptions() + }) !== null + ) + } getTarget(): SshTarget { return { ...this.target } } @@ -107,6 +118,9 @@ export class SshConnection { } async exec(cmd: string, options?: SshExecOptions): Promise { + if (options?.signal?.aborted) { + throw createSshOperationAbortError() + } if (this.useSystemSshTransport) { if (this.disposed || this.state.status !== 'connected') { throw new Error('Not connected') @@ -121,7 +135,8 @@ export class SshConnection { return this.waitForSshCallback( 'SSH exec channel timed out', (callback) => client.exec(remoteCommand, callback), - (channel) => channel.close() + (channel) => channel.close(), + options?.signal ) } @@ -143,12 +158,26 @@ export class SshConnection { private waitForSshCallback( timeoutMessage: string, register: (callback: (error: Error | undefined, value: T) => void) => void, - cleanupLateValue?: (value: T) => void + cleanupLateValue?: (value: T) => void, + signal?: AbortSignal ): Promise { return new Promise((resolve, reject) => { let settled = false + const cleanup = (): void => { + clearTimeout(timer) + signal?.removeEventListener('abort', onAbort) + } + const onAbort = (): void => { + if (settled) { + return + } + settled = true + cleanup() + reject(createSshOperationAbortError()) + } const timer = setTimeout(() => { settled = true + signal?.removeEventListener('abort', onAbort) reject(new Error(timeoutMessage)) }, CONNECT_TIMEOUT_MS) const finish = (error: Error | undefined, value?: T): void => { @@ -166,13 +195,18 @@ export class SshConnection { return } settled = true - clearTimeout(timer) + cleanup() if (error) { reject(error) return } resolve(value as T) } + if (signal?.aborted) { + onAbort() + return + } + signal?.addEventListener('abort', onAbort, { once: true }) try { // Why: higher-level channel timers start only after ssh2 invokes its @@ -686,6 +720,9 @@ export class SshConnection { } private spawnTrackedSystemSshCommand(command: string, options?: SshExecOptions): ClientChannel { + if (options?.signal?.aborted) { + throw createSshOperationAbortError() + } const buildArgsOptions = this.getSystemSshBuildArgsOptions() const commandOptions = options === undefined && Object.keys(buildArgsOptions).length === 0 @@ -696,9 +733,14 @@ export class SshConnection { ? spawnSystemSshCommand(this.target, command) : spawnSystemSshCommand(this.target, command, commandOptions) this.systemCommandChannels.add(channel) + const onAbort = (): void => { + channel.close() + } const cleanup = (): void => { + options?.signal?.removeEventListener('abort', onAbort) this.systemCommandChannels.delete(channel) } + options?.signal?.addEventListener('abort', onAbort, { once: true }) channel.once('close', cleanup) channel.once('error', cleanup) return channel diff --git a/src/main/ssh/ssh-relay-cross-version-isolation.test.ts b/src/main/ssh/ssh-relay-cross-version-isolation.test.ts index e1f93d8ec..c426e616f 100644 --- a/src/main/ssh/ssh-relay-cross-version-isolation.test.ts +++ b/src/main/ssh/ssh-relay-cross-version-isolation.test.ts @@ -50,6 +50,7 @@ import type { SshConnection } from './ssh-connection' function makeMockConnection(): SshConnection { return { + canRunConcurrentExecCommands: vi.fn().mockReturnValue(false), exec: vi.fn().mockResolvedValue({ on: vi.fn(), stderr: { on: vi.fn() }, diff --git a/src/main/ssh/ssh-relay-deploy-helpers.test.ts b/src/main/ssh/ssh-relay-deploy-helpers.test.ts index 4b4a3783c..8bd40901d 100644 --- a/src/main/ssh/ssh-relay-deploy-helpers.test.ts +++ b/src/main/ssh/ssh-relay-deploy-helpers.test.ts @@ -217,6 +217,55 @@ describe('execCommand', () => { } }) + it('closes and rejects with AbortError when a command is aborted', async () => { + const channel = createMockChannel() + const controller = new AbortController() + const conn = { + exec: vi.fn().mockResolvedValue(channel) + } + const commandPromise = execCommand(conn as never, 'sleep 60', { + signal: controller.signal + }) + + await Promise.resolve() + controller.abort() + + await expect(commandPromise).rejects.toMatchObject({ name: 'AbortError' }) + expect(channel.close).toHaveBeenCalledOnce() + expect(channel.listenerCount('error')).toBe(0) + expect(channel.listenerCount('data')).toBe(0) + expect(channel.listenerCount('close')).toBe(0) + expect(channel.stderr.listenerCount('error')).toBe(0) + expect(channel.stderr.listenerCount('data')).toBe(0) + }) + + it('handles aborts that happen while the SSH exec channel is still opening', async () => { + const channel = createMockChannel() + const controller = new AbortController() + let resolveExec: (channel: ClientChannel) => void = () => {} + const conn = { + exec: vi.fn().mockReturnValue( + new Promise((resolve) => { + resolveExec = resolve + }) + ) + } + + const commandPromise = execCommand(conn as never, 'sleep 60', { + signal: controller.signal + }) + controller.abort() + resolveExec(channel) + + await expect(commandPromise).rejects.toMatchObject({ name: 'AbortError' }) + expect(channel.close).toHaveBeenCalledOnce() + expect(channel.listenerCount('error')).toBe(0) + expect(channel.listenerCount('data')).toBe(0) + expect(channel.listenerCount('close')).toBe(0) + expect(channel.stderr.listenerCount('error')).toBe(0) + expect(channel.stderr.listenerCount('data')).toBe(0) + }) + it('uses custom command timeouts without forwarding them to SSH exec', async () => { vi.useFakeTimers() try { diff --git a/src/main/ssh/ssh-relay-deploy-helpers.ts b/src/main/ssh/ssh-relay-deploy-helpers.ts index ecc7f2767..1473e3233 100644 --- a/src/main/ssh/ssh-relay-deploy-helpers.ts +++ b/src/main/ssh/ssh-relay-deploy-helpers.ts @@ -1,6 +1,6 @@ import type { ClientChannel } from 'ssh2' import type { SshConnection } from './ssh-connection' -import type { SshExecOptions } from './ssh-connection-utils' +import { createSshOperationAbortError, type SshExecOptions } from './ssh-connection-utils' import { RELAY_SENTINEL, RELAY_SENTINEL_TIMEOUT_MS } from './relay-protocol' import type { MultiplexerTransport } from './ssh-channel-multiplexer' import { buildRelayVersionMismatchError } from './ssh-relay-handshake-mismatch' @@ -247,6 +247,10 @@ export async function execCommand( options?: ExecCommandOptions ): Promise { const { timeoutMs = EXEC_TIMEOUT_MS, ...execOptions } = options ?? {} + const signal = options?.signal + if (signal?.aborted) { + throw createSshOperationAbortError() + } const channel = await conn.exec(command, execOptions) return new Promise((resolve, reject) => { let stdout = '' @@ -255,6 +259,7 @@ export async function execCommand( const cleanup = (): void => { clearTimeout(timeout) + signal?.removeEventListener('abort', onAbort) channel.off('error', fail) channel.stderr.off('error', fail) channel.off('data', onStdoutData) @@ -272,6 +277,10 @@ export async function execCommand( const fail = (err: Error): void => { settle(reject, err) } + const onAbort = (): void => { + channel.close() + settle(reject, createSshOperationAbortError()) + } const onStdoutData = (data: Buffer): void => { stdout += data.toString('utf-8') } @@ -293,10 +302,14 @@ export async function execCommand( // Why: remote reboot tears down exec channels with stream errors. Without // scoped listeners, Node treats those as uncaught exceptions. + signal?.addEventListener('abort', onAbort, { once: true }) channel.on('error', fail) channel.stderr.on('error', fail) channel.on('data', onStdoutData) channel.stderr.on('data', onStderrData) channel.on('close', onClose) + if (signal?.aborted) { + onAbort() + } }) } diff --git a/src/main/ssh/ssh-relay-deploy.test.ts b/src/main/ssh/ssh-relay-deploy.test.ts index eddb0e9f0..ec825cdc4 100644 --- a/src/main/ssh/ssh-relay-deploy.test.ts +++ b/src/main/ssh/ssh-relay-deploy.test.ts @@ -69,7 +69,9 @@ vi.mock('./ssh-connection-utils', () => ({ import { deployAndLaunchRelay } from './ssh-relay-deploy' import { execCommand, waitForSentinel } from './ssh-relay-deploy-helpers' import { resolveRemoteNodePath } from './ssh-remote-node-resolution' +import { isRelayAlreadyInstalled } from './ssh-relay-versioned-install' import type { SshConnection } from './ssh-connection' +import type * as SshRemoteNodeResolution from './ssh-remote-node-resolution' import { DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS, MAX_SSH_RELAY_GRACE_PERIOD_SECONDS @@ -90,6 +92,7 @@ function extractWindowsMarkerPath(script: string): string { function makeMockConnection(): SshConnection { return { + canRunConcurrentExecCommands: vi.fn().mockReturnValue(true), exec: vi.fn().mockResolvedValue({ on: vi.fn(), stderr: { on: vi.fn() }, @@ -161,6 +164,259 @@ describe('deployAndLaunchRelay', () => { expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1) }) + it('resolves node concurrently with remote home, not after the install-state chain', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + + let markNodeResolutionStarted: () => void = () => {} + const nodeResolutionStarted = new Promise((resolve) => { + markNodeResolutionStarted = resolve + }) + vi.mocked(resolveRemoteNodePath).mockImplementationOnce(() => { + markNodeResolutionStarted() + return Promise.resolve('/usr/bin/node') + }) + + // Hold the first install-state step open. The optimization starts the node + // branch before the remote-home -> install-check chain finishes. + let releaseRemoteHome: (home: string) => void = () => {} + mockExecCommand.mockReturnValueOnce( + new Promise((resolve) => { + releaseRemoteHome = resolve + }) + ) + + const deployPromise = deployAndLaunchRelay(conn) + let assertionError: unknown + let deployError: unknown + try { + await nodeResolutionStarted + + expect(isRelayAlreadyInstalled).not.toHaveBeenCalled() + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1) + } catch (err) { + assertionError = err + } finally { + // Drain the rest of the happy path so a failed assertion does not leave + // the deploy promise pending until its 300s timeout. + mockExecCommand.mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') // native deps probe + mockExecCommand.mockResolvedValueOnce('DEAD') // socket probe + mockExecCommand.mockResolvedValueOnce('READY') // socket poll + releaseRemoteHome('/home/user') + deployError = await deployPromise.then( + () => undefined, + (err: unknown) => err + ) + } + if (assertionError) { + throw assertionError + } + if (deployError) { + throw deployError + } + }) + + it('keeps bootstrap sequential when the connection cannot run concurrent exec commands', async () => { + const conn = makeMockConnection() + vi.mocked(conn.canRunConcurrentExecCommands).mockReturnValue(false) + const mockExecCommand = vi.mocked(execCommand) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + let releaseRemoteHome: (home: string) => void = () => {} + let remoteHomeProbeStarted: () => void = () => {} + const remoteHomeProbeStartedPromise = new Promise((resolve) => { + remoteHomeProbeStarted = resolve + }) + mockExecCommand.mockReturnValueOnce( + new Promise((resolve) => { + remoteHomeProbeStarted() + releaseRemoteHome = resolve + }) + ) + + const deployPromise = deployAndLaunchRelay(conn) + await remoteHomeProbeStartedPromise + expect(resolveRemoteNodePath).not.toHaveBeenCalled() + + mockExecCommand.mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') // native deps probe + mockExecCommand.mockResolvedValueOnce('DEAD') // socket probe + mockExecCommand.mockResolvedValueOnce('READY') // socket poll + releaseRemoteHome('/home/user') + await deployPromise + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1) + }) + + it('falls back to sequential bootstrap when concurrent SSH sessions are refused', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + const { resolveRemoteNodePath: resolveRemoteNodePathActual } = await vi.importActual< + typeof SshRemoteNodeResolution + >('./ssh-remote-node-resolution') + let fallbackInstallStateCompleted = false + vi.mocked(resolveRemoteNodePath) + .mockImplementationOnce(resolveRemoteNodePathActual) + .mockImplementationOnce(() => { + if (!fallbackInstallStateCompleted) { + throw new Error('Sequential fallback resolved node before install state finished') + } + return Promise.resolve('/usr/bin/node') + }) + vi.mocked(isRelayAlreadyInstalled) + .mockImplementationOnce(async (_conn, _dir, _host, options) => { + expect(options?.rethrowSessionLimitErrors).toBe(true) + return true + }) + .mockImplementationOnce(async (_conn, _dir, _host, options) => { + expect(options?.rethrowSessionLimitErrors).toBeUndefined() + fallbackInstallStateCompleted = true + return true + }) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + mockExecCommand.mockResolvedValueOnce('/home/user') // concurrent install-state $HOME + mockExecCommand.mockRejectedValueOnce(sessionLimitError) // concurrent node path probe + mockExecCommand.mockResolvedValueOnce('/home/user') // sequential fallback $HOME + mockExecCommand.mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') // native deps probe + mockExecCommand.mockResolvedValueOnce('DEAD') // socket probe + mockExecCommand.mockResolvedValueOnce('READY') // socket poll + + await deployAndLaunchRelay(conn) + + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(2) + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(2) + }) + + it('falls back to sequential bootstrap when the install-state probe hits a session limit', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + vi.mocked(isRelayAlreadyInstalled) + .mockImplementationOnce(async (_conn, _dir, _host, options) => { + if (!options?.rethrowSessionLimitErrors) { + return true + } + throw sessionLimitError + }) + .mockResolvedValueOnce(true) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + mockExecCommand.mockResolvedValueOnce('/home/user') // concurrent install-state $HOME + mockExecCommand.mockResolvedValueOnce('/home/user') // sequential fallback $HOME + mockExecCommand.mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') // native deps probe + mockExecCommand.mockResolvedValueOnce('DEAD') // socket probe + mockExecCommand.mockResolvedValueOnce('READY') // socket poll + + await deployAndLaunchRelay(conn) + + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(2) + expect(vi.mocked(isRelayAlreadyInstalled).mock.calls[0]?.[3]).toMatchObject({ + rethrowSessionLimitErrors: true + }) + expect(vi.mocked(isRelayAlreadyInstalled).mock.calls[1]?.[3]).toBeUndefined() + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(2) + }) + + it('does not retry bootstrap for non-session failures', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + const nodeError = new Error('Node.js not found on remote host') + vi.mocked(resolveRemoteNodePath).mockRejectedValueOnce(nodeError) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + mockExecCommand.mockResolvedValueOnce('/home/user') // concurrent install-state $HOME + + await expect(deployAndLaunchRelay(conn)).rejects.toBe(nodeError) + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(1) + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1) + }) + + it('aborts a pending sibling probe and preserves a non-session install-state failure', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + let nodeProbeAborted = false + vi.mocked(resolveRemoteNodePath).mockImplementationOnce((_conn, _host, options) => { + return new Promise((_resolve, reject) => { + options?.signal?.addEventListener('abort', () => { + nodeProbeAborted = true + const abortError = new Error('aborted') + abortError.name = 'AbortError' + reject(abortError) + }) + }) + }) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + mockExecCommand.mockResolvedValueOnce('relative-home') // invalid install-state $HOME + + const timedDeploy = Promise.race([ + deployAndLaunchRelay(conn), + new Promise((_resolve, reject) => { + setTimeout(() => reject(new Error('deploy did not fail promptly')), 100) + }) + ]) + + await expect(timedDeploy).rejects.toThrow(/Remote home is not a valid path/) + expect(nodeProbeAborted).toBe(true) + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1) + }) + + it('does not retry when a session-limit failure races with a real install-state failure', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + const installError = new Error('permission denied while checking relay install') + vi.mocked(resolveRemoteNodePath).mockRejectedValueOnce(sessionLimitError) + vi.mocked(isRelayAlreadyInstalled).mockRejectedValueOnce(installError) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + mockExecCommand.mockResolvedValueOnce('/home/user') // concurrent install-state $HOME + + await expect(deployAndLaunchRelay(conn)).rejects.toBe(installError) + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(1) + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1) + }) + + it('does not retry until the surviving first-attempt probe settles', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + let releaseRemoteHome: (home: string) => void = () => {} + let remoteHomeSettled = false + mockExecCommand.mockReturnValueOnce( + new Promise((resolve) => { + releaseRemoteHome = (home: string) => { + remoteHomeSettled = true + resolve(home) + } + }) + ) + vi.mocked(resolveRemoteNodePath).mockImplementationOnce(() => Promise.reject(sessionLimitError)) + vi.mocked(resolveRemoteNodePath).mockImplementationOnce(() => { + if (!remoteHomeSettled) { + throw new Error('Sequential fallback started before first install-state probe settled') + } + return Promise.resolve('/usr/bin/node') + }) + + const deployPromise = deployAndLaunchRelay(conn) + await vi.waitFor(() => expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1)) + await new Promise((resolve) => setImmediate(resolve)) + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(1) + + mockExecCommand.mockResolvedValueOnce('/home/user') // sequential fallback $HOME + mockExecCommand.mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') // native deps probe + mockExecCommand.mockResolvedValueOnce('DEAD') // socket probe + mockExecCommand.mockResolvedValueOnce('READY') // socket poll + releaseRemoteHome('/home/user') + await deployPromise + expect(resolveRemoteNodePath).toHaveBeenCalledTimes(2) + }) + it('defaults fresh relays to keep-alive-until-reset', async () => { const conn = makeMockConnection() const mockExecCommand = vi.mocked(execCommand) diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index bee9f63de..78e7dec44 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -42,6 +42,7 @@ import { import { detectRemoteHostPlatform } from './ssh-remote-platform-detection' import { powerShellCommand, powerShellLiteral, powerShellNativeArg } from './ssh-remote-powershell' import { relaySocketNameForInstanceId } from './ssh-relay-instance-id' +import { isSshSessionLimitError } from './ssh-session-limit-error' import { isWindowsRelayPipePath, relayEndpointForHost, @@ -80,11 +81,12 @@ function execHostCommand( conn: SshConnection, hostPlatform: RemoteHostPlatform, command: string, - options?: { timeoutMs?: number } + options?: { timeoutMs?: number; signal?: AbortSignal } ): Promise { return execCommand(conn, command, { wrapCommand: !isWindowsRemoteHost(hostPlatform), - timeoutMs: options?.timeoutMs + timeoutMs: options?.timeoutMs, + signal: options?.signal }) } @@ -122,6 +124,119 @@ export async function deployAndLaunchRelay( } } +/** + * Resolve the remote home, derive the versioned relay directory, and check + * whether the relay is already installed there. + * + * Why: extracted so the deploy can run this chain concurrently with node-path + * resolution (the two are independent). Home and install-check stay sequential + * here because the install-check needs the resolved directory. + */ +async function resolveRemoteInstallState( + conn: SshConnection, + hostPlatform: RemoteHostPlatform, + fullVersion: string, + options?: { rethrowSessionLimitErrors?: boolean; signal?: AbortSignal } +): Promise<{ remoteHome: string; remoteRelayDir: string; alreadyInstalled: boolean }> { + // Why: SFTP does not expand `~`, so we must resolve the remote home + // explicitly with the host's native shell and normalize it before use. + const remoteHome = normalizeRemoteHome( + await execHostCommand(conn, hostPlatform, readRemoteHomeCommand(hostPlatform), { + signal: options?.signal + }), + hostPlatform + ) + // Why: we only interpolate $HOME into single-quoted shell strings later, so + // this validation only needs to reject obviously unsafe control characters. + // Allow spaces and non-ASCII so valid home directories are not rejected. + if (!validateRemoteHome(remoteHome, hostPlatform)) { + throw new Error(`Remote home is not a valid path: ${remoteHome.slice(0, 100)}`) + } + const remoteRelayDir = computeRemoteRelayDir(remoteHome, fullVersion, hostPlatform.pathFlavor) + const probeOptions = + options?.rethrowSessionLimitErrors || options?.signal + ? { + rethrowSessionLimitErrors: options.rethrowSessionLimitErrors, + signal: options.signal + } + : undefined + const alreadyInstalled = await isRelayAlreadyInstalled( + conn, + remoteRelayDir, + hostPlatform, + probeOptions + ) + return { remoteHome, remoteRelayDir, alreadyInstalled } +} + +type RelayBootstrapState = { + remoteHome: string + remoteRelayDir: string + alreadyInstalled: boolean + nodePath: string +} + +async function resolveRelayBootstrapStateSequentially( + conn: SshConnection, + hostPlatform: RemoteHostPlatform, + fullVersion: string +): Promise { + const installState = await resolveRemoteInstallState(conn, hostPlatform, fullVersion) + const nodePath = await resolveRemoteNodePath(conn, hostPlatform) + return { ...installState, nodePath } +} + +async function resolveRelayBootstrapState( + conn: SshConnection, + hostPlatform: RemoteHostPlatform, + fullVersion: string +): Promise { + if (!conn.canRunConcurrentExecCommands()) { + return resolveRelayBootstrapStateSequentially(conn, hostPlatform, fullVersion) + } + const abortController = new AbortController() + const installStatePromise = resolveRemoteInstallState(conn, hostPlatform, fullVersion, { + rethrowSessionLimitErrors: true, + signal: abortController.signal + }) + const nodePathPromise = resolveRemoteNodePath(conn, hostPlatform, { + rethrowSessionLimitErrors: true, + signal: abortController.signal + }) + try { + const [installState, nodePath] = await Promise.all([installStatePromise, nodePathPromise]) + return { ...installState, nodePath } + } catch (err) { + abortController.abort() + const settled = await Promise.allSettled([installStatePromise, nodePathPromise]) + if (!isSshSessionLimitError(err)) { + throw err + } + const nonSessionFailure = settled.find( + (result) => + result.status === 'rejected' && + !isSshSessionLimitError(result.reason) && + !isAbortError(result.reason) + ) + if (nonSessionFailure?.status === 'rejected') { + throw nonSessionFailure.reason + } + console.warn( + '[ssh-relay] Concurrent bootstrap probes hit the remote SSH session limit; retrying sequentially.' + ) + return resolveRelayBootstrapStateSequentially(conn, hostPlatform, fullVersion) + } +} + +function isAbortError(err: unknown): boolean { + return err instanceof Error && err.name === 'AbortError' +} + +/** + * Detect the remote platform, resolve install state and node path, install the + * relay if it is not already present, then launch it and return the transport. + * Inner implementation wrapped by `deployAndLaunchRelay` with an overall timeout. + */ async function deployAndLaunchRelayInner( conn: SshConnection, onProgress?: (status: string) => void, @@ -152,25 +267,17 @@ async function deployAndLaunchRelayInner( // docs/ssh-relay-versioned-install-dirs.md "Data Flow: Upstream Error". const fullVersion = readLocalFullVersion(localRelayDir) - // Why: SFTP does not expand `~`, so we must resolve the remote home - // explicitly with the host's native shell and normalize it before use. - const remoteHome = normalizeRemoteHome( - await execHostCommand(conn, hostPlatform, readRemoteHomeCommand(hostPlatform)), - hostPlatform - ) - // Why: we only interpolate $HOME into single-quoted shell strings later, so - // this validation only needs to reject obviously unsafe control characters. - // Allow spaces and non-ASCII so valid home directories are not rejected. - if (!validateRemoteHome(remoteHome, hostPlatform)) { - throw new Error(`Remote home is not a valid path: ${remoteHome.slice(0, 100)}`) - } - const remoteRelayDir = computeRemoteRelayDir(remoteHome, fullVersion, hostPlatform.pathFlavor) - console.log(`[ssh-relay] Remote dir: ${remoteRelayDir}`) - onProgress?.('Checking existing relay...') - const alreadyInstalled = await isRelayAlreadyInstalled(conn, remoteRelayDir, hostPlatform) + // Why: the remote-home -> install-check chain and node resolution are + // independent (both only need hostPlatform, not each other's results), yet + // each is a separate SSH exec round trip. Run them concurrently so the deploy + // pays one round trip instead of two for this phase. Most failures stay + // fail-fast; remotes that reject overlapping session channels retry the old + // sequential order so restrictive SSH servers keep working. + const { remoteHome, remoteRelayDir, alreadyInstalled, nodePath } = + await resolveRelayBootstrapState(conn, hostPlatform, fullVersion) + console.log(`[ssh-relay] Remote dir: ${remoteRelayDir}`) console.log(`[ssh-relay] Already installed at ${fullVersion}: ${alreadyInstalled}`) - const nodePath = await resolveRemoteNodePath(conn, hostPlatform) if (alreadyInstalled) { await repairInstalledNativeDeps(conn, remoteRelayDir, platform, hostPlatform, nodePath) diff --git a/src/main/ssh/ssh-relay-native-deps-install.test.ts b/src/main/ssh/ssh-relay-native-deps-install.test.ts index b8bfcfeb7..0cedf1aff 100644 --- a/src/main/ssh/ssh-relay-native-deps-install.test.ts +++ b/src/main/ssh/ssh-relay-native-deps-install.test.ts @@ -109,6 +109,7 @@ function makeMockConnection(capture: SftpWriteCapture): SshConnection { end: vi.fn() }) return { + canRunConcurrentExecCommands: vi.fn().mockReturnValue(false), exec: vi.fn().mockResolvedValue({ on: vi.fn(), stderr: { on: vi.fn() }, diff --git a/src/main/ssh/ssh-relay-versioned-install.test.ts b/src/main/ssh/ssh-relay-versioned-install.test.ts index 29ea274c3..8cc46e8b1 100644 --- a/src/main/ssh/ssh-relay-versioned-install.test.ts +++ b/src/main/ssh/ssh-relay-versioned-install.test.ts @@ -88,6 +88,26 @@ describe('isRelayAlreadyInstalled', () => { expect(await isRelayAlreadyInstalled(conn, '/r')).toBe(false) }) + it('keeps default probe failures as not installed for SSH session-limit-shaped errors', async () => { + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + mockExec.mockRejectedValueOnce(sessionLimitError) + + await expect(isRelayAlreadyInstalled(conn, '/r')).resolves.toBe(false) + }) + + it('rethrows SSH session-limit errors in strict mode', async () => { + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + mockExec.mockRejectedValueOnce(sessionLimitError) + + await expect( + isRelayAlreadyInstalled(conn, '/r', undefined, { rethrowSessionLimitErrors: true }) + ).rejects.toBe(sessionLimitError) + }) + it('checks for relay.js AND .install-complete in addition to the dir', async () => { mockExec.mockResolvedValueOnce('OK') await isRelayAlreadyInstalled(conn, '/r') diff --git a/src/main/ssh/ssh-relay-versioned-install.ts b/src/main/ssh/ssh-relay-versioned-install.ts index ab3c88fb6..4074e31f6 100644 --- a/src/main/ssh/ssh-relay-versioned-install.ts +++ b/src/main/ssh/ssh-relay-versioned-install.ts @@ -36,6 +36,7 @@ import { type RemotePathFlavor } from './ssh-remote-platform' import { windowsRelayPipePathsForSocketName } from './ssh-relay-endpoints' +import { isSshSessionLimitError } from './ssh-session-limit-error' // Why: the GC pass and the version-dir parser must agree on what counts as a // relay install dir. Single source of truth for both. The pattern matches the @@ -64,12 +65,21 @@ const INSTALL_LOCK_TIMEOUT_MS = 120_000 const INSTALL_LOCK_STALE_MS = 120_000 const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') +type RelayInstalledProbeOptions = { + rethrowSessionLimitErrors?: boolean + signal?: AbortSignal +} + function execHostCommand( conn: SshConnection, host: RemoteHostPlatform, - command: string + command: string, + options?: { signal?: AbortSignal } ): Promise { - return execCommand(conn, command, { wrapCommand: host.commandDialect !== 'powershell' }) + return execCommand(conn, command, { + wrapCommand: host.commandDialect !== 'powershell', + signal: options?.signal + }) } /** @@ -124,16 +134,21 @@ export function computeRemoteRelayDir( export async function isRelayAlreadyInstalled( conn: SshConnection, remoteRelayDir: string, - host: RemoteHostPlatform = DEFAULT_REMOTE_HOST + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + options?: RelayInstalledProbeOptions ): Promise { try { const probe = await execHostCommand( conn, host, - probeRelayInstalledCommand(host, remoteRelayDir) + probeRelayInstalledCommand(host, remoteRelayDir), + { signal: options?.signal } ) return probe.trim() === 'OK' - } catch { + } catch (err) { + if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { + throw err + } return false } } diff --git a/src/main/ssh/ssh-remote-node-resolution.test.ts b/src/main/ssh/ssh-remote-node-resolution.test.ts index b007c8fd1..23ea90975 100644 --- a/src/main/ssh/ssh-remote-node-resolution.test.ts +++ b/src/main/ssh/ssh-remote-node-resolution.test.ts @@ -4,6 +4,7 @@ import os from 'node:os' import path from 'node:path' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { SshConnection } from './ssh-connection' +import { getRemoteHostPlatform } from './ssh-remote-platform' const execCommandMock = vi.hoisted(() => vi.fn()) @@ -270,6 +271,66 @@ describe('resolveRemoteNodePath', () => { await expect(resolveRemoteNodePath(conn)).rejects.toThrow(/Node\.js not found/) }) + it('keeps the default path-probe fallback for SSH session-limit-shaped errors', async () => { + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + execCommandMock + .mockRejectedValueOnce(sessionLimitError) + .mockResolvedValueOnce('/bin/bash') + .mockResolvedValueOnce('\n') + + await expect(resolveRemoteNodePath(conn)).rejects.toThrow(/Node\.js not found/) + }) + + it('rethrows SSH session-limit errors from the path probe in strict mode', async () => { + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + execCommandMock.mockRejectedValueOnce(sessionLimitError) + + await expect( + resolveRemoteNodePath(conn, undefined, { rethrowSessionLimitErrors: true }) + ).rejects.toBe(sessionLimitError) + }) + + it('rethrows SSH session-limit errors from candidate version checks in strict mode', async () => { + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + execCommandMock + .mockResolvedValueOnce('/usr/local/bin/node\n') + .mockRejectedValueOnce(sessionLimitError) + + await expect( + resolveRemoteNodePath(conn, undefined, { rethrowSessionLimitErrors: true }) + ).rejects.toBe(sessionLimitError) + }) + + it('rethrows SSH session-limit errors from login-shell resolution in strict mode', async () => { + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + execCommandMock.mockResolvedValueOnce('\n').mockRejectedValueOnce(sessionLimitError) + + await expect( + resolveRemoteNodePath(conn, undefined, { rethrowSessionLimitErrors: true }) + ).rejects.toBe(sessionLimitError) + }) + + it('rethrows SSH session-limit errors from Windows node resolution in strict mode', async () => { + const sessionLimitError = Object.assign(new Error('(SSH) Channel open failure: open failed'), { + reason: 4 + }) + execCommandMock.mockRejectedValueOnce(sessionLimitError) + + await expect( + resolveRemoteNodePath(conn, getRemoteHostPlatform('win32-x64'), { + rethrowSessionLimitErrors: true + }) + ).rejects.toBe(sessionLimitError) + }) + it('throws when every candidate across both strategies is below the minimum', async () => { execCommandMock .mockResolvedValueOnce('/old/node\n') // path probe diff --git a/src/main/ssh/ssh-remote-node-resolution.ts b/src/main/ssh/ssh-remote-node-resolution.ts index a5ec9b9fd..4117ddf97 100644 --- a/src/main/ssh/ssh-remote-node-resolution.ts +++ b/src/main/ssh/ssh-remote-node-resolution.ts @@ -4,6 +4,7 @@ import type { RemoteHostPlatform } from './ssh-remote-platform' import { isWindowsRemoteHost, normalizeWindowsRemotePath } from './ssh-remote-platform' import { powerShellCommand } from './ssh-remote-powershell' import { execCommand } from './ssh-relay-deploy-helpers' +import { isSshSessionLimitError } from './ssh-session-limit-error' // Why: the relay requires Node.js 18+. Version managers like nvm keep every // installed version on disk, so a naive "highest version" glob can hand back @@ -15,12 +16,18 @@ const MIN_NODE_MAJOR = 18 // hang a login shell, so keep this short. const LOGIN_SHELL_PROBE_TIMEOUT_MS = 8_000 +type RemoteNodeResolutionOptions = { + rethrowSessionLimitErrors?: boolean + signal?: AbortSignal +} + export async function resolveRemoteNodePath( conn: SshConnection, - host?: RemoteHostPlatform + host?: RemoteHostPlatform, + options?: RemoteNodeResolutionOptions ): Promise { if (host && isWindowsRemoteHost(host)) { - return resolveRemoteWindowsNodePath(conn) + return resolveRemoteWindowsNodePath(conn, options) } // Strategy 1: probe well-known install directories for every common Node @@ -28,14 +35,14 @@ export async function resolveRemoteNodePath( // This doesn't depend on shell startup-file semantics — bash -lc skips // .bashrc and zsh -lc skips .zshrc, but those are exactly the files where // nvm/mise/asdf hooks live. Probing directories directly is deterministic. - const probedPath = await tryResolveViaKnownPaths(conn) + const probedPath = await tryResolveViaKnownPaths(conn, options) if (probedPath) { return probedPath } // Strategy 2 (fallback): ask the user's login shell. Catches custom PATH // setups in ~/.profile / ~/.bash_profile that the probes don't cover. - const loginShellPath = await tryResolveViaLoginShell(conn) + const loginShellPath = await tryResolveViaLoginShell(conn, options) if (loginShellPath) { return loginShellPath } @@ -47,7 +54,10 @@ export async function resolveRemoteNodePath( // plus system package-manager locations. Every probe runs unconditionally so // a missing directory prints nothing rather than short-circuiting later // probes. Returns the first candidate that meets the minimum version. -async function tryResolveViaKnownPaths(conn: SshConnection): Promise { +async function tryResolveViaKnownPaths( + conn: SshConnection, + options?: RemoteNodeResolutionOptions +): Promise { const script = ` command -v node 2>/dev/null nvm_dirs=\${NVM_DIR:-"$HOME/.nvm"} @@ -94,7 +104,7 @@ true ` try { - const result = await execCommand(conn, script) + const result = await execCommand(conn, script, { signal: options?.signal }) const seen = new Set() for (const line of result.split('\n')) { const candidate = line.trim() @@ -102,12 +112,15 @@ true continue } seen.add(candidate) - if (await nodeMeetsVersionRequirement(conn, candidate)) { + if (await nodeMeetsVersionRequirement(conn, candidate, options)) { console.log(`[ssh-relay] Found node via path probe: ${candidate}`) return candidate } } - } catch { + } catch (err) { + if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { + throw err + } // Fall through to login shell. } return null @@ -116,14 +129,18 @@ true // Run `command -v node` under the user's login shell, then verify the result // meets the minimum version. Returns null on any failure (shell missing, no // node found, version too old, timeout) so callers fall through to the error. -async function tryResolveViaLoginShell(conn: SshConnection): Promise { +async function tryResolveViaLoginShell( + conn: SshConnection, + options?: RemoteNodeResolutionOptions +): Promise { try { // Why: $SHELL is the user's configured login shell (set by chsh / passwd). // Using it — rather than hardcoding bash — means zsh/fish users whose // custom PATH hooks live in profile files get coverage too. We fall back // to sh if $SHELL is unset (rare, e.g. restricted accounts). const shellResult = await execCommand(conn, 'echo "${SHELL:-/bin/sh}"', { - timeoutMs: LOGIN_SHELL_PROBE_TIMEOUT_MS + timeoutMs: LOGIN_SHELL_PROBE_TIMEOUT_MS, + signal: options?.signal }) const shell = shellResult.trim().split('\n')[0] if (!shell) { @@ -132,18 +149,22 @@ async function tryResolveViaLoginShell(conn: SshConnection): Promise { try { const versionOutput = await execCommand(conn, `${shellEscape(nodePath)} --version`, { - wrapCommand: false + wrapCommand: false, + signal: options?.signal }) const match = versionOutput.trim().match(/^v?(\d+)/) if (!match) { @@ -174,13 +197,19 @@ async function nodeMeetsVersionRequirement( } const major = Number.parseInt(match[1]!, 10) return major >= MIN_NODE_MAJOR - } catch { + } catch (err) { + if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { + throw err + } // Binary missing or fails to run — not usable. return false } } -async function resolveRemoteWindowsNodePath(conn: SshConnection): Promise { +async function resolveRemoteWindowsNodePath( + conn: SshConnection, + options?: RemoteNodeResolutionOptions +): Promise { const script = [ '$paths = @()', '$cmd = Get-Command node.exe -ErrorAction SilentlyContinue', @@ -199,14 +228,20 @@ async function resolveRemoteWindowsNodePath(conn: SshConnection): Promise { + it('matches ssh2 open failures with the resource-shortage reason code', () => { + expect( + isSshSessionLimitError( + Object.assign(new Error('(SSH) Channel open failure: open failed'), { reason: 4 }) + ) + ).toBe(true) + }) + + it('matches OpenSSH mux and MaxSessions failures', () => { + expect( + isSshSessionLimitError( + new Error( + 'mux_client_request_session: session request failed: Session open refused by peer' + ) + ) + ).toBe(true) + expect( + isSshSessionLimitError(new Error('open failed: MaxSessions limit reached on remote host')) + ).toBe(true) + }) + + it('does not match generic channel-open failures without a session-limit reason', () => { + expect( + isSshSessionLimitError( + Object.assign(new Error('(SSH) Channel open failure: open failed'), { reason: 1 }) + ) + ).toBe(false) + expect( + isSshSessionLimitError( + Object.assign(new Error('(SSH) Channel open failure: open failed'), { reason: 2 }) + ) + ).toBe(false) + expect( + isSshSessionLimitError( + Object.assign(new Error('(SSH) Channel open failure: open failed'), { reason: 3 }) + ) + ).toBe(false) + }) + + it('does not match unrelated command failures', () => { + expect( + isSshSessionLimitError(new Error('Command "node" failed (exit 1): Node.js not found')) + ).toBe(false) + expect(isSshSessionLimitError(new Error('channel open failure while parsing output'))).toBe( + false + ) + expect( + isSshSessionLimitError( + new Error('open failed: administratively prohibited: forwarding disabled') + ) + ).toBe(false) + }) +}) diff --git a/src/main/ssh/ssh-session-limit-error.ts b/src/main/ssh/ssh-session-limit-error.ts new file mode 100644 index 000000000..aed848525 --- /dev/null +++ b/src/main/ssh/ssh-session-limit-error.ts @@ -0,0 +1,19 @@ +export function isSshSessionLimitError(err: unknown): boolean { + if (!(err instanceof Error)) { + return false + } + const reason = (err as { reason?: unknown }).reason + const message = err.message.toLowerCase() + if ( + reason === 4 && + (message.includes('channel open failure') || message.includes('open failed')) + ) { + return true + } + return ( + message.includes('no free channels available') || + message.includes('maxsessions') || + message.includes('session open refused') || + (message.includes('mux_client_request_session') && message.includes('session request failed')) + ) +}