From fa85536f3a952430375b8448880d32cd82397005 Mon Sep 17 00:00:00 2001 From: Glen Maddern Date: Fri, 17 Jul 2026 14:32:13 +1000 Subject: [PATCH] fix(ssh): repair unbuilt relay native deps (#8686) Co-authored-by: Orca Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Jinwoo-H Co-authored-by: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> --- src/main/ssh/ssh-connection.test.ts | 151 +++- src/main/ssh/ssh-connection.ts | 255 ++++-- src/main/ssh/ssh-file-transfer-abort.test.ts | 55 ++ src/main/ssh/ssh-file-transfer-abort.ts | 76 ++ src/main/ssh/ssh-relay-build-toolchain.ts | 9 +- .../ssh-relay-cross-version-isolation.test.ts | 7 +- src/main/ssh/ssh-relay-deploy-helpers.test.ts | 147 +++- src/main/ssh/ssh-relay-deploy-helpers.ts | 158 +--- src/main/ssh/ssh-relay-deploy-timing.ts | 13 + src/main/ssh/ssh-relay-deploy.test.ts | 210 ++++- src/main/ssh/ssh-relay-deploy.ts | 818 ++++++++++++++---- src/main/ssh/ssh-relay-exec-command.ts | 156 ++++ src/main/ssh/ssh-relay-gc-claim.ts | 236 +++++ src/main/ssh/ssh-relay-gc-retry.test.ts | 275 ++++++ src/main/ssh/ssh-relay-gc-tombstone.ts | 33 + .../ssh/ssh-relay-install-lock-commands.ts | 176 ++++ src/main/ssh/ssh-relay-install-lock.ts | 162 ++++ .../ssh/ssh-relay-native-deps-install.test.ts | 399 ++++++++- src/main/ssh/ssh-relay-repair-lock.ts | 153 ++++ .../ssh/ssh-relay-versioned-install.test.ts | 632 +++++++++++++- src/main/ssh/ssh-relay-versioned-install.ts | 209 ++--- src/main/ssh/ssh-remote-commands.test.ts | 335 ++++++- src/main/ssh/ssh-remote-commands.ts | 54 +- src/main/ssh/ssh-remote-node-resolution.ts | 5 + src/main/ssh/ssh-remote-platform-detection.ts | 34 +- src/main/ssh/ssh-system-fallback.test.ts | 44 +- src/main/ssh/system-ssh-command.ts | 3 + .../ssh/system-ssh-operation-lifecycle.ts | 8 +- src/relay/pty-handler.test.ts | 23 + src/relay/pty-handler.ts | 111 ++- src/relay/subprocess-test-utils.ts | 2 +- src/relay/subprocess.test.ts | 83 +- 32 files changed, 4339 insertions(+), 693 deletions(-) create mode 100644 src/main/ssh/ssh-file-transfer-abort.test.ts create mode 100644 src/main/ssh/ssh-file-transfer-abort.ts create mode 100644 src/main/ssh/ssh-relay-deploy-timing.ts create mode 100644 src/main/ssh/ssh-relay-exec-command.ts create mode 100644 src/main/ssh/ssh-relay-gc-claim.ts create mode 100644 src/main/ssh/ssh-relay-gc-retry.test.ts create mode 100644 src/main/ssh/ssh-relay-gc-tombstone.ts create mode 100644 src/main/ssh/ssh-relay-install-lock-commands.ts create mode 100644 src/main/ssh/ssh-relay-install-lock.ts create mode 100644 src/main/ssh/ssh-relay-repair-lock.ts diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index e8c5d7123..3274d3f5d 100644 --- a/src/main/ssh/ssh-connection.test.ts +++ b/src/main/ssh/ssh-connection.test.ts @@ -23,6 +23,7 @@ type MockSshClient = { lastExecCommand?: string lastConnectConfig?: unknown exec: (cmd: string, cb: (err: Error | undefined, channel: unknown) => void) => void + sftp: (cb: (err: Error | undefined, channel: unknown) => void) => void } let clientInstances: MockSshClient[] = [] @@ -690,15 +691,15 @@ describe('SshConnection', () => { vi.useFakeTimers() try { - const outcomePromise = conn - .exec('printf ready') - .then(() => 'opened') - .catch((error: Error) => error.message) + const outcomePromise = conn.exec('printf ready').catch((error: Error) => error) await vi.advanceTimersByTimeAsync(30_000) const outcome = await Promise.race([outcomePromise, Promise.resolve('pending')]) - expect(outcome).toBe('SSH exec channel timed out') + expect(outcome).toMatchObject({ + message: 'SSH exec channel timed out', + sshChannelCloseConfirmed: false + }) } finally { vi.useRealTimers() } @@ -784,15 +785,54 @@ describe('SshConnection', () => { try { const outcomePromise = conn .exec('printf ready', { signal: controller.signal }) - .then(() => 'opened') - .catch((error: Error) => error.name) + .catch((error: Error) => error) controller.abort() // Why: a hung socket must not pin the aborted caller for the full 30s // connect timeout — the abort settles at the 5s grace bound instead. await vi.advanceTimersByTimeAsync(5_000) - await expect(outcomePromise).resolves.toBe('AbortError') + await expect(outcomePromise).resolves.toMatchObject({ + name: 'AbortError', + sshChannelCloseConfirmed: false + }) + } finally { + vi.useRealTimers() + } + }) + + it('drains an exec channel that opens after the abort grace has settled', async () => { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + execBehavior = 'pending' + const controller = new AbortController() + const lateChannel = Object.assign(new EventEmitter(), { + close: vi.fn(), + resume: vi.fn(), + stderr: { resume: vi.fn() } + }) + + vi.useFakeTimers() + try { + const outcomePromise = conn + .exec('printf ready', { signal: controller.signal }) + .catch((error: Error) => error) + + controller.abort() + await vi.advanceTimersByTimeAsync(5_000) + const outcome = await outcomePromise + expect(outcome).toMatchObject({ + name: 'AbortError', + sshChannelCloseConfirmed: false + }) + + pendingExecCallback?.(undefined, lateChannel) + + expect(lateChannel.resume).toHaveBeenCalledTimes(1) + expect(lateChannel.stderr.resume).toHaveBeenCalledTimes(1) + expect(lateChannel.close).toHaveBeenCalledTimes(1) + lateChannel.emit('close') + expect(outcome).toMatchObject({ sshChannelCloseConfirmed: true }) } finally { vi.useRealTimers() } @@ -837,7 +877,7 @@ describe('SshConnection', () => { const lateChannel = Object.assign(new EventEmitter(), { close: vi.fn(), resume: vi.fn(), - stderr: { resume: vi.fn() } + stderr: Object.assign(new EventEmitter(), { resume: vi.fn() }) }) const outcomePromise = conn @@ -855,11 +895,41 @@ describe('SshConnection', () => { expect(early).toBe('pending') expect(lateChannel.close).toHaveBeenCalledTimes(1) expect(lateChannel.resume).toHaveBeenCalled() + expect(() => lateChannel.emit('error', new Error('late channel teardown'))).not.toThrow() + expect(() => lateChannel.stderr.emit('error', new Error('late stderr teardown'))).not.toThrow() lateChannel.emit('close') await expect(outcomePromise).resolves.toBe('AbortError') }) + it('removes the late-channel close listener when abort grace expires', async () => { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + vi.useFakeTimers() + try { + sftpBehavior = 'pending' + const controller = new AbortController() + const lateSftp = Object.assign(new EventEmitter(), { end: vi.fn() }) + + const outcomePromise = conn + .sftp(controller.signal) + .then(() => 'opened') + .catch((error: Error) => error.name) + await vi.advanceTimersByTimeAsync(0) + controller.abort() + pendingSftpCallback?.(undefined, lateSftp) + expect(lateSftp.listenerCount('close')).toBe(1) + expect(() => lateSftp.emit('error', new Error('late SFTP teardown'))).not.toThrow() + + await vi.advanceTimersByTimeAsync(5_000) + + await expect(outcomePromise).resolves.toBe('AbortError') + expect(lateSftp.listenerCount('close')).toBe(0) + } finally { + vi.useRealTimers() + } + }) + it('times out when ssh2 never opens an SFTP channel', async () => { const conn = new SshConnection(createTarget(), createCallbacks()) await conn.connect() @@ -867,15 +937,13 @@ describe('SshConnection', () => { vi.useFakeTimers() try { - const outcomePromise = conn - .sftp() - .then(() => 'opened') - .catch((error: Error) => error.message) + const outcomePromise = conn.sftp().catch((error: Error) => error) await vi.advanceTimersByTimeAsync(30_000) const outcome = await Promise.race([outcomePromise, Promise.resolve('pending')]) - expect(outcome).toBe('SSH SFTP channel timed out') + expect(outcome).toMatchObject({ message: 'SSH SFTP channel timed out' }) + expect(outcome).not.toHaveProperty('sshChannelCloseConfirmed') } finally { vi.useRealTimers() } @@ -1368,6 +1436,61 @@ describe('SshConnection', () => { ) }) + it('composes a caller abort into system SSH relay uploads', async () => { + vi.mocked(resolveWithSshG).mockResolvedValueOnce(createResolvedConfig()) + const conn = new SshConnection(createTarget({ configHost: 'fdpass-host' }), createCallbacks()) + const controller = new AbortController() + let transferSignal: AbortSignal | undefined + vi.mocked(uploadDirectoryViaSystemSsh).mockImplementationOnce( + (_target, _localDir, _remoteDir, options) => { + transferSignal = options?.signal + return new Promise((_resolve, reject) => { + transferSignal?.addEventListener('abort', () => reject(transferSignal?.reason), { + once: true + }) + }) + } + ) + + await conn.connect() + const upload = conn.uploadDirectory('/tmp/local-relay', '/remote/relay', { + signal: controller.signal + }) + await vi.waitFor(() => expect(transferSignal).toBeDefined()) + controller.abort() + + await expect(upload).rejects.toMatchObject({ name: 'AbortError' }) + expect(transferSignal?.aborted).toBe(true) + }) + + it('keeps connection disconnect cancellation linked to caller-scoped relay writes', async () => { + vi.mocked(resolveWithSshG).mockResolvedValueOnce(createResolvedConfig()) + const conn = new SshConnection(createTarget({ configHost: 'fdpass-host' }), createCallbacks()) + const controller = new AbortController() + let transferSignal: AbortSignal | undefined + vi.mocked(writeFileViaSystemSsh).mockImplementationOnce( + (_target, _remotePath, _contents, options) => { + transferSignal = options?.signal + return new Promise((_resolve, reject) => { + transferSignal?.addEventListener('abort', () => reject(transferSignal?.reason), { + once: true + }) + }) + } + ) + + await conn.connect() + const write = conn.writeFile('/remote/relay/.version', '0.1.0', { + signal: controller.signal + }) + await vi.waitFor(() => expect(transferSignal).toBeDefined()) + await conn.disconnect() + + await expect(write).rejects.toMatchObject({ name: 'AbortError' }) + expect(controller.signal.aborted).toBe(false) + expect(transferSignal?.aborted).toBe(true) + }) + it('keeps an upload session cancelled after the connection disconnects', async () => { const conn = new SshConnection( createTarget({ proxyCommand: 'ssh -W %h:%p bastion.example.com' }), diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index 2856d33b2..b8bd0cbd5 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -41,6 +41,10 @@ import { import type { RemoteHostPlatform } from './ssh-remote-platform' import type { FileUploadSession } from '../providers/types' import { isSshSessionLimitError } from './ssh-session-limit-error' +import { + createLinkedSshFileTransferSignal, + raceSftpFileTransferWithAbort +} from './ssh-file-transfer-abort' export type { SshConnectionCallbacks } from './ssh-connection-utils' type SshRemoteFileOptions = { @@ -157,13 +161,14 @@ export class SshConnection { 'SSH exec channel timed out', (callback) => client.exec(remoteCommand, callback), (channel) => channel.close(), - options?.signal + options?.signal, + true ), options?.signal ) } - async sftp(): Promise { + async sftp(signal?: AbortSignal): Promise { if (this.useSystemSshTransport) { throw new Error('SFTP is not available when using system SSH transport') } @@ -171,12 +176,15 @@ export class SshConnection { throw new Error('Not connected') } const client = this.client - return this.openSessionChannelWithRetry(() => - this.waitForSshCallback( - 'SSH SFTP channel timed out', - (callback) => client.sftp(callback), - (sftp) => sftp.end() - ) + return this.openSessionChannelWithRetry( + () => + this.waitForSshCallback( + 'SSH SFTP channel timed out', + (callback) => client.sftp(callback), + (sftp) => sftp.end(), + signal + ), + signal ) } @@ -219,10 +227,20 @@ export class SshConnection { timeoutMessage: string, register: (callback: (error: Error | undefined, value: T) => void) => void, cleanupLateValue?: (value: T) => void, - signal?: AbortSignal + signal?: AbortSignal, + trackRemoteCommandTermination = false ): Promise { return new Promise((resolve, reject) => { + type ChannelOpenTerminationError = Error & { sshChannelCloseConfirmed: boolean } let settled = false + let unconfirmedOpenError: ChannelOpenTerminationError | null = null + const markOpenUnconfirmed = (error: Error): Error => { + if (!trackRemoteCommandTermination) { + return error + } + unconfirmedOpenError = Object.assign(error, { sshChannelCloseConfirmed: false }) + return unconfirmedOpenError + } // Why: rejecting the instant the signal aborts lets the caller proceed // while the in-flight channel open completes in the background and holds // a server-side session slot (MaxSessions). Mark the abort and settle @@ -241,16 +259,41 @@ export class SshConnection { abortDeadlineTimer = setTimeout(() => { settled = true cleanup() - reject(createSshOperationAbortError()) + reject(markOpenUnconfirmed(createSshOperationAbortError())) }, ABORTED_CHANNEL_CLOSE_GRACE_MS) } const timer = setTimeout(() => { settled = true cleanup() - reject(abortRequested ? createSshOperationAbortError() : new Error(timeoutMessage)) + reject( + markOpenUnconfirmed( + abortRequested ? createSshOperationAbortError() : new Error(timeoutMessage) + ) + ) }, CONNECT_TIMEOUT_MS) + const discardLateValue = (value: T, onClose?: () => void): void => { + const emitter = value as Partial & { + resume?: () => void + stderr?: Partial & { resume?: () => void } + } + const swallowLateError = (): void => {} + emitter.on?.('error', swallowLateError) + emitter.stderr?.on?.('error', swallowLateError) + if (onClose) { + emitter.once?.('close', onClose) + } + // Why: ssh2 can withhold CHANNEL_CLOSE while discarded exec streams + // remain unread, and teardown errors have no other owner. + emitter.resume?.() + emitter.stderr?.resume?.() + try { + cleanupLateValue?.(value) + } catch { + /* best effort */ + } + } const rejectAfterClose = (value: T): void => { - const abortError = createSshOperationAbortError() + const abortError = markOpenUnconfirmed(createSshOperationAbortError()) const emitter = value as Partial & { resume?: () => void stderr?: { resume?: () => void } @@ -262,23 +305,24 @@ export class SshConnection { } finished = true clearTimeout(closeGraceTimer) + emitter.removeListener?.('close', confirmAndDone) reject(abortError) } + const confirmAndDone = (): void => { + if (unconfirmedOpenError === abortError) { + unconfirmedOpenError.sshChannelCloseConfirmed = true + } + done() + } // Why: bounded — a remote that never confirms the close must not hang // the aborted operation forever. const closeGraceTimer = setTimeout(done, ABORTED_CHANNEL_CLOSE_GRACE_MS) if (typeof emitter.once === 'function') { - emitter.once('close', done) + emitter.once('close', confirmAndDone) } // Why: ssh2 withholds the 'close' event until the channel's streams // are drained; nobody else will ever read this discarded channel. - emitter.resume?.() - emitter.stderr?.resume?.() - try { - cleanupLateValue?.(value) - } catch { - /* best effort */ - } + discardLateValue(value) if (typeof emitter.once !== 'function') { done() } @@ -289,11 +333,11 @@ export class SshConnection { // rejected. Close that late resource so the remote channel is not // left open with no owner. if (!error && value !== undefined) { - try { - cleanupLateValue?.(value) - } catch { - /* best effort */ - } + discardLateValue(value, () => { + if (unconfirmedOpenError) { + unconfirmedOpenError.sshChannelCloseConfirmed = true + } + }) } return } @@ -334,23 +378,51 @@ export class SshConnection { async uploadDirectory( localDir: string, remoteDir: string, - options?: SshRemoteFileOptions + options?: SshRemoteFileOptions & { signal?: AbortSignal } ): Promise { - if (!this.useSystemSshTransport) { - const sftp = await this.sftp() - try { - const { uploadDirectory } = await import('./ssh-relay-deploy-helpers') - await uploadDirectory(sftp, localDir, remoteDir) - } finally { - sftp.end() + // Why: relay deployment timeout and connection teardown are independent; + // either owner must stop a transfer that can otherwise outlive its lock. + const linkedSignal = createLinkedSshFileTransferSignal( + [this.systemOperationAbortController.signal, options?.signal].filter( + (signal): signal is AbortSignal => signal !== undefined + ) + ) + try { + if (!this.useSystemSshTransport) { + const sftp = await this.sftp(linkedSignal.signal) + const swallowLateSftpError = (): void => {} + let sftpEndRequested = false + const endSftp = (): void => { + if (!sftpEndRequested) { + sftpEndRequested = true + sftp.end() + } + } + sftp.on('error', swallowLateSftpError) + sftp.once('close', () => sftp.removeListener('error', swallowLateSftpError)) + try { + const { uploadDirectory } = await import('./ssh-relay-deploy-helpers') + await raceSftpFileTransferWithAbort( + uploadDirectory(sftp, localDir, remoteDir), + linkedSignal.signal, + (onClose) => { + sftp.once('close', onClose) + endSftp() + } + ) + } finally { + endSftp() + } + return } - return + await uploadDirectoryViaSystemSsh(this.target, localDir, remoteDir, { + signal: linkedSignal.signal, + hostPlatform: options?.hostPlatform, + ...this.getSystemSshBuildArgsOptions() + }) + } finally { + linkedSignal.dispose() } - await uploadDirectoryViaSystemSsh(this.target, localDir, remoteDir, { - signal: this.systemOperationAbortController.signal, - hostPlatform: options?.hostPlatform, - ...this.getSystemSshBuildArgsOptions() - }) } async downloadFile( @@ -404,55 +476,74 @@ export class SshConnection { async writeFile( remotePath: string, contents: string, - options?: SshRemoteFileOptions + options?: SshRemoteFileOptions & { signal?: AbortSignal } ): Promise { - if (!this.useSystemSshTransport) { - const sftp = await this.sftp() - const swallowLateSftpError = (): void => {} - sftp.on('error', swallowLateSftpError) - try { - await new Promise((resolve, reject) => { - const ws = sftp.createWriteStream(remotePath) - let settled = false - const cleanup = (): void => { - ws.removeListener('close', onClose) - ws.removeListener('error', onError) + // Keep package/version writes under the same dual cancellation contract as uploads. + const linkedSignal = createLinkedSshFileTransferSignal( + [this.systemOperationAbortController.signal, options?.signal].filter( + (signal): signal is AbortSignal => signal !== undefined + ) + ) + try { + if (!this.useSystemSshTransport) { + const sftp = await this.sftp(linkedSignal.signal) + const swallowLateSftpError = (): void => {} + let sftpEndRequested = false + const endSftp = (): void => { + if (!sftpEndRequested) { + sftpEndRequested = true + sftp.end() } - const onClose = (): void => { - if (settled) { - return + } + sftp.on('error', swallowLateSftpError) + sftp.once('close', () => sftp.removeListener('error', swallowLateSftpError)) + try { + const write = new Promise((resolve, reject) => { + const ws = sftp.createWriteStream(remotePath) + let settled = false + const cleanup = (): void => { + sftp.removeListener('error', onError) + ws.removeListener('close', onClose) + ws.removeListener('error', onError) } - settled = true - cleanup() - resolve() - } - const onError = (err: Error): void => { - sftp.removeListener('error', onError) - if (settled) { - return + const onClose = (): void => { + if (settled) { + return + } + settled = true + cleanup() + resolve() } - settled = true - cleanup() - reject(err) - } - sftp.prependOnceListener('error', onError) - ws.once('close', onClose) - ws.once('error', onError) - ws.end(contents) - }) - } finally { - sftp.end() - setImmediate(() => { - sftp.removeListener('error', swallowLateSftpError) - }) + const onError = (err: Error): void => { + if (settled) { + return + } + settled = true + cleanup() + reject(err) + } + sftp.prependOnceListener('error', onError) + ws.once('close', onClose) + ws.once('error', onError) + ws.end(contents) + }) + await raceSftpFileTransferWithAbort(write, linkedSignal.signal, (onClose) => { + sftp.once('close', onClose) + endSftp() + }) + } finally { + endSftp() + } + return } - return + await writeFileViaSystemSsh(this.target, remotePath, contents, { + signal: linkedSignal.signal, + hostPlatform: options?.hostPlatform, + ...this.getSystemSshBuildArgsOptions() + }) + } finally { + linkedSignal.dispose() } - await writeFileViaSystemSsh(this.target, remotePath, contents, { - signal: this.systemOperationAbortController.signal, - hostPlatform: options?.hostPlatform, - ...this.getSystemSshBuildArgsOptions() - }) } async writeBuffer( diff --git a/src/main/ssh/ssh-file-transfer-abort.test.ts b/src/main/ssh/ssh-file-transfer-abort.test.ts new file mode 100644 index 000000000..49abe0a18 --- /dev/null +++ b/src/main/ssh/ssh-file-transfer-abort.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it, vi } from 'vitest' +import { raceSftpFileTransferWithAbort } from './ssh-file-transfer-abort' + +describe('raceSftpFileTransferWithAbort', () => { + it('waits for confirmed SFTP close before rejecting an abort', async () => { + const controller = new AbortController() + let confirmClose: () => void = () => {} + const promise = raceSftpFileTransferWithAbort( + new Promise(() => {}), + controller.signal, + (onClose) => { + confirmClose = onClose + } + ) + + controller.abort() + const pending = await Promise.race([ + promise.then( + () => 'settled', + () => 'settled' + ), + Promise.resolve('pending') + ]) + expect(pending).toBe('pending') + + confirmClose() + await expect(promise).rejects.toMatchObject({ + name: 'AbortError', + sshChannelCloseConfirmed: true + }) + }) + + it('marks teardown unconfirmed when SFTP never closes', async () => { + vi.useFakeTimers() + try { + const controller = new AbortController() + const promise = raceSftpFileTransferWithAbort( + new Promise(() => {}), + controller.signal, + () => {} + ) + const outcome = promise.catch((error: Error) => error) + + controller.abort() + await vi.advanceTimersByTimeAsync(5_000) + + await expect(outcome).resolves.toMatchObject({ + name: 'AbortError', + sshChannelCloseConfirmed: false + }) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/main/ssh/ssh-file-transfer-abort.ts b/src/main/ssh/ssh-file-transfer-abort.ts new file mode 100644 index 000000000..a7b6cce2e --- /dev/null +++ b/src/main/ssh/ssh-file-transfer-abort.ts @@ -0,0 +1,76 @@ +import { createSshOperationAbortError } from './ssh-connection-utils' + +export function createLinkedSshFileTransferSignal(signals: readonly AbortSignal[]): { + signal: AbortSignal + dispose: () => void +} { + const controller = new AbortController() + const listeners = signals.map((signal) => { + const listener = (): void => controller.abort(signal.reason) + if (signal.aborted) { + listener() + } else { + signal.addEventListener('abort', listener, { once: true }) + } + return { signal, listener } + }) + return { + signal: controller.signal, + dispose: () => { + for (const { signal, listener } of listeners) { + signal.removeEventListener('abort', listener) + } + } + } +} + +export function raceSftpFileTransferWithAbort( + operation: Promise, + signal: AbortSignal, + closeSftp: (onClose: () => void) => void +): Promise { + return new Promise((resolve, reject) => { + let settled = false + let abortError: (Error & { sshChannelCloseConfirmed: boolean }) | null = null + let closeGraceTimer: ReturnType | null = null + const settle = (fn: typeof resolve | typeof reject, value: T | Error): void => { + if (settled) { + return + } + settled = true + if (closeGraceTimer) { + clearTimeout(closeGraceTimer) + } + signal.removeEventListener('abort', onAbort) + fn(value as never) + } + const onAbort = (): void => { + // Why: rejecting the caller is insufficient; ending SFTP stops the + // abandoned transfer from mutating a successor relay install. + abortError = Object.assign(createSshOperationAbortError(), { + sshChannelCloseConfirmed: false + }) + closeGraceTimer = setTimeout(() => settle(reject, abortError!), 5_000) + closeSftp(() => { + abortError!.sshChannelCloseConfirmed = true + settle(reject, abortError!) + }) + } + signal.addEventListener('abort', onAbort, { once: true }) + void operation.then( + (value) => { + if (!abortError) { + settle(resolve, value) + } + }, + (error: unknown) => { + if (!abortError) { + settle(reject, error instanceof Error ? error : new Error(String(error))) + } + } + ) + if (signal.aborted) { + onAbort() + } + }) +} diff --git a/src/main/ssh/ssh-relay-build-toolchain.ts b/src/main/ssh/ssh-relay-build-toolchain.ts index cdbdb6764..208deceb6 100644 --- a/src/main/ssh/ssh-relay-build-toolchain.ts +++ b/src/main/ssh/ssh-relay-build-toolchain.ts @@ -141,15 +141,20 @@ export function formatMissingToolchainError( // callers fall back to the original install error in those cases. export async function probeBuildToolchain( conn: SshConnection, - hostPlatform: RemoteHostPlatform + hostPlatform: RemoteHostPlatform, + signal?: AbortSignal ): Promise { if (isWindowsRemoteHost(hostPlatform)) { return null } try { - const output = await execCommand(conn, buildToolchainProbeCommand(), { wrapCommand: true }) + const output = await execCommand(conn, buildToolchainProbeCommand(), { + wrapCommand: true, + signal + }) return parseBuildToolchainProbe(output) } catch { + signal?.throwIfAborted() return null } } 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 a29801c36..14f063e3b 100644 --- a/src/main/ssh/ssh-relay-cross-version-isolation.test.ts +++ b/src/main/ssh/ssh-relay-cross-version-isolation.test.ts @@ -33,6 +33,9 @@ vi.mock('./ssh-relay-deploy-helpers', () => ({ onData: vi.fn(), onClose: vi.fn() }), + isUnconfirmedSshCommandTermination: (error: unknown) => + error instanceof Error && + (error as Error & { sshChannelCloseConfirmed?: boolean }).sshChannelCloseConfirmed === false, execCommand: vi.fn() })) @@ -101,8 +104,10 @@ describe('cross-version isolation', () => { '__ORCA_REMOTE_PLATFORM__ Linux x86_64', // tagged POSIX platform probe '/home/u', // echo $HOME 'MISSING', // isRelayAlreadyInstalled (v2 dir doesn't exist) + 'OPEN', // no sibling GC claim '', // mkdir -p remoteRelayDir (v2) 'OK', // mkdir lock OK + 'OPEN', // GC did not claim while the install lock was acquired 'MISSING', // re-probe after lock → still missing → proceed with install '', // mkdir remoteDir (uploadRelay) '', // chmod +x node @@ -111,9 +116,9 @@ describe('cross-version isolation', () => { 'ORCA-NPTY-PROBE-OK\n', // node -e require() load-test (post-install verify) '', // rm -f probe-stderr (best-effort cleanup after probe resolved) '', // touch .install-complete (finalizeInstall) - '', // rm -rf .install-lock 'DEAD', // launch socket probe 'READY', // socket poll + '', // release .install-lock after relay liveness is observable // GC scan begins here 'relay-0.1.0+v1hash\nrelay-0.1.0+v2hash\n', // ls listing 'OPEN', // v1 lock probe (siblings only — current dir is v2) diff --git a/src/main/ssh/ssh-relay-deploy-helpers.test.ts b/src/main/ssh/ssh-relay-deploy-helpers.test.ts index cb5477091..4df9c8ad2 100644 --- a/src/main/ssh/ssh-relay-deploy-helpers.test.ts +++ b/src/main/ssh/ssh-relay-deploy-helpers.test.ts @@ -11,9 +11,10 @@ import { function createMockChannel(): ClientChannel { return Object.assign(new EventEmitter(), { - stderr: new EventEmitter(), + stderr: Object.assign(new EventEmitter(), { resume: vi.fn() }), stdin: { write: vi.fn() }, - close: vi.fn() + close: vi.fn(), + resume: vi.fn() }) as unknown as ClientChannel } @@ -29,6 +30,44 @@ async function execCommandRejection(promise: Promise): Promise { } describe('waitForSentinel', () => { + it('closes and rejects with AbortError while clearing startup resources on abort', async () => { + vi.useFakeTimers() + try { + const channel = createMockChannel() + const controller = new AbortController() + const removeListener = vi.spyOn(controller.signal, 'removeEventListener') + const transportPromise = waitForSentinel(channel, controller.signal) + + expect(vi.getTimerCount()).toBe(1) + controller.abort() + + await expect(transportPromise).rejects.toMatchObject({ name: 'AbortError' }) + expect(channel.close).toHaveBeenCalledTimes(1) + expect(removeListener).toHaveBeenCalledWith('abort', expect.any(Function)) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + }) + + it('removes the abort listener and timers when startup fails before the sentinel', async () => { + vi.useFakeTimers() + try { + const channel = createMockChannel() + const controller = new AbortController() + const removeListener = vi.spyOn(controller.signal, 'removeEventListener') + const transportPromise = waitForSentinel(channel, controller.signal) + + channel.emit('error', new Error('remote host rebooted')) + + await expect(transportPromise).rejects.toThrow('remote host rebooted') + expect(removeListener).toHaveBeenCalledWith('abort', expect.any(Function)) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + }) + it('buffers post-sentinel chunks until the transport subscribes', async () => { const channel = createMockChannel() const transportPromise = waitForSentinel(channel) @@ -168,6 +207,32 @@ describe('waitForSentinel', () => { }) describe('execCommand', () => { + it('waits for channel close before rejecting a timed-out remote command', async () => { + vi.useFakeTimers() + try { + const channel = createMockChannel() + const conn = { exec: vi.fn().mockResolvedValue(channel) } + const commandPromise = execCommand(conn as never, 'npm rebuild', { timeoutMs: 1_000 }) + + await vi.advanceTimersByTimeAsync(1_000) + expect(channel.close).toHaveBeenCalledTimes(1) + expect( + await Promise.race([ + commandPromise.then( + () => 'settled', + () => 'settled' + ), + Promise.resolve('pending') + ]) + ).toBe('pending') + + channel.emit('close', 0) + await expect(commandPromise).rejects.toThrow('timed out after 1s') + } finally { + vi.useRealTimers() + } + }) + it('rejects on command channel errors instead of emitting uncaught errors', async () => { const channel = createMockChannel() const conn = { @@ -177,6 +242,8 @@ describe('execCommand', () => { await Promise.resolve() expect(() => channel.emit('error', new Error('remote host rebooted'))).not.toThrow() + expect(channel.close).toHaveBeenCalledOnce() + channel.emit('close', 1) await expect(commandPromise).rejects.toThrow('remote host rebooted') expect(channel.listenerCount('error')).toBe(0) expect(channel.listenerCount('data')).toBe(0) @@ -224,6 +291,22 @@ describe('execCommand', () => { expect(error.message).toContain('gnu++20') }) + it('bounds command output while preserving the actionable tail', async () => { + const channel = createMockChannel() + const conn = { exec: vi.fn().mockResolvedValue(channel) } + const commandPromise = execCommand(conn as never, 'npm rebuild') + + await Promise.resolve() + channel.emit('data', Buffer.from(`old-prefix-${'x'.repeat(1024 * 1024)}`)) + channel.emit('data', Buffer.from('gyp ERR! tail diagnosis')) + channel.emit('close', 1) + + const error = await execCommandRejection(commandPromise) + expect(error.message).not.toContain('old-prefix') + expect(error.message).toContain('gyp ERR! tail diagnosis') + expect(error.message.length).toBeLessThan(1024 * 1024 + 200) + }) + it('keeps the merged error message greppable by the build-toolchain probe', async () => { const channel = createMockChannel() const conn = { @@ -260,14 +343,26 @@ describe('execCommand', () => { await Promise.resolve() const rejection = expect(commandPromise).rejects.toThrow('timed out') await vi.advanceTimersByTimeAsync(30_000) + expect(channel.close).toHaveBeenCalledOnce() + await vi.advanceTimersByTimeAsync(5_000) await rejection expect(channel.close).toHaveBeenCalledOnce() - expect(channel.listenerCount('error')).toBe(0) + expect(channel.resume).toHaveBeenCalledOnce() + expect(channel.stderr.resume).toHaveBeenCalledOnce() + expect(() => channel.emit('error', new Error('late channel error'))).not.toThrow() + expect(() => channel.stderr.emit('error', new Error('late stderr error'))).not.toThrow() + channel.emit('data', Buffer.from('late stdout')) + channel.stderr.emit('data', Buffer.from('late stderr')) + expect(channel.listenerCount('error')).toBe(1) expect(channel.listenerCount('data')).toBe(0) + expect(channel.listenerCount('close')).toBe(1) + expect(channel.stderr.listenerCount('error')).toBe(1) + expect(channel.stderr.listenerCount('data')).toBe(0) + channel.emit('close', 0) + expect(channel.listenerCount('error')).toBe(0) expect(channel.listenerCount('close')).toBe(0) expect(channel.stderr.listenerCount('error')).toBe(0) - expect(channel.stderr.listenerCount('data')).toBe(0) } finally { vi.useRealTimers() } @@ -309,6 +404,48 @@ describe('execCommand', () => { expect(channel.stderr.listenerCount('data')).toBe(0) }) + it('keeps system-SSH command termination unconfirmed after the local child closes', async () => { + const channel = createMockChannel() + const controller = new AbortController() + const conn = { + exec: vi.fn().mockResolvedValue(channel), + usesSystemSshTransport: vi.fn().mockReturnValueOnce(true).mockReturnValue(false) + } + const commandPromise = execCommand(conn as never, 'npm install', { + signal: controller.signal + }) + + await Promise.resolve() + controller.abort() + channel.emit('close', 0) + + await expect(commandPromise).rejects.toMatchObject({ + name: 'AbortError', + sshChannelCloseConfirmed: false + }) + }) + + it('tags an externally closed system-SSH command as unconfirmed', async () => { + const channel = createMockChannel() as ClientChannel & { _closeRequested?: boolean } + channel.close = vi.fn(() => { + channel._closeRequested = true + }) + const conn = { + exec: vi.fn().mockResolvedValue(channel), + usesSystemSshTransport: vi.fn().mockReturnValue(true) + } + const commandPromise = execCommand(conn as never, 'npm install') + + await Promise.resolve() + channel.close() + channel.emit('close', 1) + + await expect(commandPromise).rejects.toMatchObject({ + name: 'AbortError', + sshChannelCloseConfirmed: false + }) + }) + it('handles aborts that happen while the SSH exec channel is still opening', async () => { const channel = createMockChannel() const controller = new AbortController() @@ -357,6 +494,8 @@ describe('execCommand', () => { expect(conn.exec).toHaveBeenCalledWith('npm install', { wrapCommand: false }) const rejection = expect(commandPromise).rejects.toThrow('timed out after 240s') await vi.advanceTimersByTimeAsync(240_000) + expect(channel.close).toHaveBeenCalledOnce() + await vi.advanceTimersByTimeAsync(5_000) await rejection expect(channel.close).toHaveBeenCalledOnce() diff --git a/src/main/ssh/ssh-relay-deploy-helpers.ts b/src/main/ssh/ssh-relay-deploy-helpers.ts index 1b1cdd270..80d2b78ce 100644 --- a/src/main/ssh/ssh-relay-deploy-helpers.ts +++ b/src/main/ssh/ssh-relay-deploy-helpers.ts @@ -1,18 +1,21 @@ import type { ClientChannel } from 'ssh2' -import type { SshConnection } from './ssh-connection' -import { createSshOperationAbortError, type SshExecOptions } from './ssh-connection-utils' +import { createSshOperationAbortError } 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' export { uploadFile, uploadDirectory, mkdirSftp } from './sftp-upload' +export { execCommand, isUnconfirmedSshCommandTermination } from './ssh-relay-exec-command' // ── Sentinel detection ──────────────────────────────────────────────── const MAX_RELAY_STARTUP_BUFFER_BYTES = 64 * 1024 const RELAY_SENTINEL_BUFFER = Buffer.from(RELAY_SENTINEL, 'utf-8') -export function waitForSentinel(channel: ClientChannel): Promise { +export function waitForSentinel( + channel: ClientChannel, + signal?: AbortSignal +): Promise { return new Promise((resolve, reject) => { let sentinelReceived = false let settled = false @@ -39,17 +42,14 @@ export function waitForSentinel(channel: ClientChannel): Promise { timeoutFired = true - channel.close() timeoutGraceTimer = setTimeout(() => { - if (!settled) { - settled = true - reject( - new Error( - `Relay failed to start within ${RELAY_SENTINEL_TIMEOUT_MS / 1000}s.${stderrOutput ? ` stderr: ${stderrOutput.trim()}` : ''}` - ) + rejectStartup( + new Error( + `Relay failed to start within ${RELAY_SENTINEL_TIMEOUT_MS / 1000}s.${stderrOutput ? ` stderr: ${stderrOutput.trim()}` : ''}` ) - } + ) }, TIMEOUT_GRACE_MS) + channel.close() }, RELAY_SENTINEL_TIMEOUT_MS) const cancelTimers = (): void => { @@ -59,6 +59,31 @@ export function waitForSentinel(channel: ClientChannel): Promise { + cancelTimers() + signal?.removeEventListener('abort', onAbort) + } + const rejectStartup = (err: Error): void => { + if (settled) { + return + } + settled = true + cleanupStartup() + reject(err) + } + const onAbort = (): void => { + if (settled) { + return + } + settled = true + cleanupStartup() + channel.close() + reject(createSshOperationAbortError()) + } + signal?.addEventListener('abort', onAbort, { once: true }) + if (signal?.aborted) { + onAbort() + } channel.on('exit', (code: number | null) => { if (typeof code === 'number') { @@ -87,12 +112,8 @@ export function waitForSentinel(channel: ClientChannel): Promise { - cancelTimers() if (!sentinelReceived) { - if (!settled) { - settled = true - reject(err) - } + rejectStartup(err) return } notifyClosed() @@ -113,9 +134,7 @@ export function waitForSentinel(channel: ClientChannel): Promise { if (!sentinelReceived) { - cancelTimers() if (!settled) { - settled = true // Why: a wire-handshake mismatch on the daemon side closes the // socket; --connect prints the mismatch detail to stderr and exits // with code 42 BEFORE writing the sentinel. Translate that into a @@ -126,13 +145,13 @@ export function waitForSentinel(channel: ClientChannel): Promise 0) { pendingAfterSentinel = afterSentinel } - settled = true - const transport: MultiplexerTransport = { write: (buf: Buffer) => channel.stdin.write(buf), onData: (cb) => { @@ -233,97 +251,3 @@ export function waitForSentinel(channel: ClientChannel): 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 = '' - let stderr = '' - let settled = false - - const cleanup = (): void => { - clearTimeout(timeout) - signal?.removeEventListener('abort', onAbort) - channel.off('error', fail) - channel.stderr.off('error', fail) - channel.off('data', onStdoutData) - channel.stderr.off('data', onStderrData) - channel.off('close', onClose) - } - const settle = (fn: typeof resolve | typeof reject, val: string | Error): void => { - if (settled) { - return - } - settled = true - cleanup() - fn(val as never) - } - const fail = (err: Error): void => { - settle(reject, abortRequested ? createSshOperationAbortError() : err) - } - // Why: sshd counts the session against MaxSessions until CHANNEL_CLOSE - // completes. Settling on abort before the channel actually closes lets the - // concurrent-bootstrap sequential fallback reissue an exec while the slot - // is still held, so it gets refused again. Close and settle from onClose. - let abortRequested = false - const onAbort = (): void => { - abortRequested = true - channel.close() - } - const onStdoutData = (data: Buffer): void => { - stdout += data.toString('utf-8') - } - const onStderrData = (data: Buffer): void => { - stderr += data.toString('utf-8') - } - const onClose = (code: number): void => { - if (abortRequested) { - settle(reject, createSshOperationAbortError()) - } else if (code !== 0) { - // Why: on the system-ssh transport channel.stderr carries local OpenSSH - // client noise; preferring it masks the real failure in stdout (2>&1). - const output = [stderr.trim(), stdout.trim()].filter(Boolean).join('\n') - settle(reject, new Error(`Command "${command}" failed (exit ${code}): ${output}`)) - } else { - settle(resolve, stdout) - } - } - const timeout = setTimeout(() => { - channel.close() - settle( - reject, - abortRequested - ? createSshOperationAbortError() - : new Error(`Command "${command}" timed out after ${timeoutMs / 1000}s`) - ) - }, timeoutMs) - - // 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-timing.ts b/src/main/ssh/ssh-relay-deploy-timing.ts new file mode 100644 index 000000000..5dc24f937 --- /dev/null +++ b/src/main/ssh/ssh-relay-deploy-timing.ts @@ -0,0 +1,13 @@ +// npm install on a cold Windows cache plus antivirus scanning can exceed the +// default 30s exec timeout. +export const NATIVE_DEPS_COMMAND_TIMEOUT_MS = 240_000 + +// Why: a missing binding can require both install and rebuild while the same +// install lock is held. Concurrent first installs must wait for that valid +// holder instead of failing halfway through its bounded work. +const NATIVE_DEPS_REPAIR_BUDGET_MS = 2 * NATIVE_DEPS_COMMAND_TIMEOUT_MS + +// Why: native repair also runs separately bounded chmod/probe/diagnostic/ +// finalize commands around install + rebuild. Keep the outer bound above that +// valid worst path while remaining below the 20-minute stale-lock threshold. +export const RELAY_DEPLOY_TIMEOUT_MS = NATIVE_DEPS_REPAIR_BUDGET_MS + 7 * 60_000 diff --git a/src/main/ssh/ssh-relay-deploy.test.ts b/src/main/ssh/ssh-relay-deploy.test.ts index 334678190..28fe2a3e4 100644 --- a/src/main/ssh/ssh-relay-deploy.test.ts +++ b/src/main/ssh/ssh-relay-deploy.test.ts @@ -42,6 +42,9 @@ vi.mock('./ssh-relay-deploy-helpers', () => ({ onData: vi.fn(), onClose: vi.fn() }), + isUnconfirmedSshCommandTermination: (error: unknown) => + error instanceof Error && + (error as Error & { sshChannelCloseConfirmed?: boolean }).sshChannelCloseConfirmed === false, execCommand: vi.fn().mockResolvedValue('__ORCA_REMOTE_PLATFORM__ Linux x86_64') })) @@ -49,27 +52,38 @@ vi.mock('./ssh-remote-node-resolution', () => ({ resolveRemoteNodePath: vi.fn().mockResolvedValue('/usr/bin/node') })) -// Why: the versioned-install module shells out to the remote for install -// state, lock acquisition, and GC. Tests stub these to no-ops so the deploy -// happy-path is exercised without a real SSH connection. +// Why: the versioned-install modules shell out for install state, locking, +// and GC. Stub them so deploy tests need no real SSH connection. vi.mock('./ssh-relay-versioned-install', () => ({ readLocalFullVersion: vi.fn().mockReturnValue('0.1.0+abcdef012345'), computeRemoteRelayDir: (home: string, v: string) => `${home}/.orca-remote/relay-${v}`, isRelayAlreadyInstalled: vi.fn().mockResolvedValue(true), - acquireInstallLock: vi.fn().mockResolvedValue(undefined), finalizeInstall: vi.fn().mockResolvedValue(undefined), abandonInstall: vi.fn().mockResolvedValue(undefined), gcOldRelayVersions: vi.fn().mockResolvedValue(undefined) })) +vi.mock('./ssh-relay-install-lock', () => ({ + acquireInstallLock: vi.fn().mockResolvedValue(undefined) +})) + +vi.mock('./ssh-relay-repair-lock', () => ({ + tryAcquireRelayRepairLock: vi.fn().mockResolvedValue('acquired') +})) + vi.mock('./ssh-connection-utils', () => ({ - shellEscape: (s: string) => `'${s}'` + shellEscape: (s: string) => `'${s}'`, + createSshOperationAbortError: () => + Object.assign(new Error('SSH operation was cancelled'), { + name: 'AbortError' + }) })) 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 { acquireInstallLock } from './ssh-relay-install-lock' import type { SshConnection } from './ssh-connection' import type * as SshRemoteNodeResolution from './ssh-remote-node-resolution' import { @@ -133,7 +147,8 @@ describe('deployAndLaunchRelay', () => { expect(mockExecCommand).toHaveBeenCalledWith( conn, - "printf '\\n%s ' '__ORCA_REMOTE_PLATFORM__'; uname -sm" + "printf '\\n%s ' '__ORCA_REMOTE_PLATFORM__'; uname -sm", + expect.objectContaining({ signal: expect.any(AbortSignal) }) ) }) @@ -153,6 +168,26 @@ describe('deployAndLaunchRelay', () => { expect(progress).toContain('Starting relay...') }) + it('does not launch fresh after unconfirmed stale-socket cleanup', async () => { + const conn = makeMockConnection() + const unconfirmedCleanup = Object.assign(new Error('socket cleanup still running'), { + sshChannelCloseConfirmed: false + }) + vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('stale relay reconnect failed')) + vi.mocked(execCommand) + .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64') + .mockResolvedValueOnce('/home/user') + .mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') + .mockResolvedValueOnce('ALIVE') + .mockRejectedValueOnce(unconfirmedCleanup) + + await expect(deployAndLaunchRelay(conn)).rejects.toBe(unconfirmedCleanup) + + const commands = vi.mocked(conn.exec).mock.calls.map(([command]) => command) + expect(commands).toHaveLength(1) + expect(commands.some((command) => command.includes('--detached'))).toBe(false) + }) + it('resolves the remote node path once per deploy', async () => { const conn = makeMockConnection() const mockExecCommand = vi.mocked(execCommand) @@ -202,7 +237,7 @@ describe('deployAndLaunchRelay', () => { 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. + // the deploy promise pending until the overall deploy timeout. mockExecCommand.mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') // native deps probe mockExecCommand.mockResolvedValueOnce('DEAD') // socket probe mockExecCommand.mockResolvedValueOnce('READY') // socket poll @@ -287,7 +322,10 @@ describe('deployAndLaunchRelay', () => { await deployAndLaunchRelay(conn) - expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(2) + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(3) + expect(vi.mocked(isRelayAlreadyInstalled).mock.calls[2]?.[3]).toMatchObject({ + rethrowSessionLimitErrors: true + }) expect(resolveRemoteNodePath).toHaveBeenCalledTimes(2) }) @@ -314,11 +352,17 @@ describe('deployAndLaunchRelay', () => { await deployAndLaunchRelay(conn) - expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(2) + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(3) expect(vi.mocked(isRelayAlreadyInstalled).mock.calls[0]?.[3]).toMatchObject({ rethrowSessionLimitErrors: true }) - expect(vi.mocked(isRelayAlreadyInstalled).mock.calls[1]?.[3]).toBeUndefined() + expect(vi.mocked(isRelayAlreadyInstalled).mock.calls[1]?.[3]).toMatchObject({ + rethrowSessionLimitErrors: undefined, + signal: expect.any(AbortSignal) + }) + expect(vi.mocked(isRelayAlreadyInstalled).mock.calls[2]?.[3]).toMatchObject({ + rethrowSessionLimitErrors: true + }) expect(resolveRemoteNodePath).toHaveBeenCalledTimes(2) }) @@ -499,7 +543,10 @@ describe('deployAndLaunchRelay', () => { expect(sawLegacyDir).toBe(false) }) - it('has a 300-second overall timeout', async () => { + it('bounds the overall deploy so install + rebuild both fit under the timeout', async () => { + // Why: the outer bound must exceed the worst-case sequential native-deps + // work — a first install (240s) AND a follow-up rebuild (240s) — so a + // legitimate install-then-rebuild is not falsely timed out mid-repair. const conn = makeMockConnection() const mockExecCommand = vi.mocked(execCommand) @@ -511,15 +558,140 @@ describe('deployAndLaunchRelay', () => { // Catch the rejection immediately to avoid unhandled rejection warning const promise = deployAndLaunchRelay(conn).catch((err: Error) => err) + // Not timed out yet at the old 300s bound (install + rebuild need more). await vi.advanceTimersByTimeAsync(301_000) + expect(await Promise.race([promise, Promise.resolve('pending')])).toBe('pending') + + await vi.advanceTimersByTimeAsync(600_000) const result = await promise expect(result).toBeInstanceOf(Error) - expect((result as Error).message).toBe('Relay deployment timed out after 300s') + expect((result as Error).message).toBe('Relay deployment timed out after 900s') vi.useRealTimers() }) + it('aborts a contended install-lock wait at the overall deploy timeout', async () => { + vi.useFakeTimers() + try { + const conn = makeMockConnection() + vi.mocked(execCommand) + .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64') + .mockResolvedValueOnce('/home/user') + vi.mocked(isRelayAlreadyInstalled).mockResolvedValueOnce(false) + let lockSignal: AbortSignal | undefined + vi.mocked(acquireInstallLock).mockImplementationOnce((_conn, _dir, _host, options) => { + lockSignal = options?.signal + return new Promise((_resolve, reject) => { + lockSignal?.addEventListener('abort', () => reject(lockSignal?.reason), { once: true }) + }) + }) + + const promise = deployAndLaunchRelay(conn).catch((err: Error) => err) + await vi.advanceTimersByTimeAsync(0) + expect(acquireInstallLock).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(900_000) + + const result = await promise + expect(result).toBeInstanceOf(Error) + expect((result as Error).message).toBe('Relay deployment timed out after 900s') + expect(lockSignal?.aborted).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + it('aborts an in-progress relay upload at the overall deploy timeout', async () => { + vi.useFakeTimers() + try { + const conn = makeMockConnection() + vi.mocked(execCommand) + .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64') + .mockResolvedValueOnce('/home/user') + .mockResolvedValueOnce('') // mkdir remote relay dir + vi.mocked(isRelayAlreadyInstalled).mockResolvedValueOnce(false).mockResolvedValueOnce(false) + let uploadSignal: AbortSignal | undefined + conn.uploadDirectory = vi.fn((_localDir, _remoteDir, options) => { + uploadSignal = options?.signal + return new Promise((_resolve, reject) => { + uploadSignal?.addEventListener('abort', () => reject(uploadSignal?.reason), { + once: true + }) + }) + }) + + const promise = deployAndLaunchRelay(conn).catch((err: Error) => err) + await vi.advanceTimersByTimeAsync(0) + expect(conn.uploadDirectory).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(900_000) + + const result = await promise + expect(result).toBeInstanceOf(Error) + expect((result as Error).message).toBe('Relay deployment timed out after 900s') + expect(uploadSignal?.aborted).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + it('aborts a launch started near the deploy deadline and closes its channel once', async () => { + vi.useFakeTimers() + try { + const launchChannel = { + on: vi.fn(), + stderr: { on: vi.fn() }, + stdin: {}, + stdout: { on: vi.fn() }, + close: vi.fn() + } + const conn = makeMockConnection() + vi.mocked(conn.exec).mockResolvedValue(launchChannel as never) + const mockExecCommand = vi.mocked(execCommand) + mockExecCommand + .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64') + .mockResolvedValueOnce('/home/user') + .mockImplementationOnce( + () => + new Promise((resolve) => + setTimeout(() => resolve('ORCA-NATIVE-DEPS-OK'), 899_900) + ) + ) + .mockResolvedValueOnce('DEAD') + .mockImplementationOnce((_conn, _command, options) => { + return new Promise((_resolve, reject) => { + options?.signal?.addEventListener( + 'abort', + () => { + const error = new Error('SSH operation was cancelled') + error.name = 'AbortError' + reject(error) + }, + { once: true } + ) + }) + }) + + const promise = deployAndLaunchRelay(conn).catch((err: Error) => err) + await vi.advanceTimersByTimeAsync(899_900) + expect(conn.exec).toHaveBeenCalledTimes(1) + expect(launchChannel.close).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(100) + + const result = await promise + expect(result).toBeInstanceOf(Error) + expect((result as Error).message).toBe('Relay deployment timed out after 900s') + expect(launchChannel.close).toHaveBeenCalledTimes(1) + expect(mockExecCommand).toHaveBeenCalledTimes(5) + await vi.advanceTimersByTimeAsync(10_000) + expect(mockExecCommand).toHaveBeenCalledTimes(5) + } finally { + vi.useRealTimers() + } + }) + it('uses distinct target-specific relay socket paths', async () => { const connA = makeMockConnection() const connB = makeMockConnection() @@ -596,6 +768,20 @@ describe('deployAndLaunchRelay', () => { expect(launchScript).not.toContain('\\\\.\\pipe\\agent-hooks') const waitScript = decodedScripts.find((script) => script.includes('deadline=Date.now()')) ?? '' expect(waitScript).toContain('setTimeout(attempt,intervalMs)') + const windowsLaunchCalls = mockExecCommand.mock.calls.filter(([, command]) => { + const script = decodePowerShellCommand(command) + return ( + script?.includes('.windows-active-pipe') || + script?.includes('Invoke-CimMethod') || + script?.includes('deadline=Date.now()') + ) + }) + expect(windowsLaunchCalls.length).toBeGreaterThan(0) + expect( + windowsLaunchCalls.every(([, , options]) => options?.signal instanceof AbortSignal) + ).toBe(true) + expect(vi.mocked(conn.exec).mock.calls[0]?.[1]?.signal).toBeInstanceOf(AbortSignal) + expect(vi.mocked(waitForSentinel).mock.calls[0]?.[1]).toBeInstanceOf(AbortSignal) }) it('relaunches Windows remotes on a fallback pipe when reconnecting the occupied pipe fails', async () => { diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index 26501a837..56779b87f 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -8,18 +8,30 @@ import { app } from 'electron' import type { SshConnection } from './ssh-connection' import type { RelayPlatform } from './relay-protocol' import type { MultiplexerTransport } from './ssh-channel-multiplexer' -import { uploadDirectory, waitForSentinel, execCommand } from './ssh-relay-deploy-helpers' +import { + uploadDirectory, + waitForSentinel, + execCommand, + isUnconfirmedSshCommandTermination +} from './ssh-relay-deploy-helpers' import { resolveRemoteNodePath } from './ssh-remote-node-resolution' import { readLocalFullVersion, computeRemoteRelayDir, isRelayAlreadyInstalled, - acquireInstallLock, finalizeInstall, abandonInstall, gcOldRelayVersions } from './ssh-relay-versioned-install' -import { shellEscape } from './ssh-connection-utils' +import { acquireInstallLock } from './ssh-relay-install-lock' +import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' +import { + releaseRelayGcClaimWithRetry, + tryAcquireRelayGcClaim, + waitForRelayGcClaimRelease +} from './ssh-relay-gc-claim' +import { NATIVE_DEPS_COMMAND_TIMEOUT_MS, RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' +import { createSshOperationAbortError, shellEscape } from './ssh-connection-utils' import { probeBuildToolchain, formatMissingToolchainError, @@ -66,16 +78,14 @@ export type RelayDeployResult = { sockPath?: string } -// Why: individual exec commands have 30s timeouts, but the full deploy -// pipeline (detect platform → check existing → upload → npm install → -// launch) has no overall bound. A hanging `npm install` or slow SFTP -// upload could block the connection indefinitely. First-time installs need -// room for the longer native dependency install bound below. -const RELAY_DEPLOY_TIMEOUT_MS = 300_000 - -// npm install on a cold Windows cache plus antivirus scanning can exceed the -// default 30s exec timeout. -const NATIVE_DEPS_INSTALL_TIMEOUT_MS = 240_000 +class RelayDirectoryGcConflictError extends Error { + constructor( + readonly remoteRelayDir: string, + readonly hostPlatform: RemoteHostPlatform + ) { + super(`Relay directory GC is in progress at ${remoteRelayDir}`) + } +} function execHostCommand( conn: SshConnection, @@ -108,15 +118,25 @@ export async function deployAndLaunchRelay( relayInstanceId?: string ): Promise { let timeoutHandle: ReturnType + // Why: Promise.race does not cancel its loser. Stop a contended install-lock + // waiter so it cannot acquire and mutate the relay after this call times out. + const deployAbortController = new AbortController() const timeoutPromise = new Promise((_resolve, reject) => { timeoutHandle = setTimeout(() => { + deployAbortController.abort() reject(new Error(`Relay deployment timed out after ${RELAY_DEPLOY_TIMEOUT_MS / 1000}s`)) }, RELAY_DEPLOY_TIMEOUT_MS) }) try { return await Promise.race([ - deployAndLaunchRelayInner(conn, onProgress, graceTimeSeconds, relayInstanceId), + deployAndLaunchRelayInner( + conn, + onProgress, + graceTimeSeconds, + relayInstanceId, + deployAbortController.signal + ), timeoutPromise ]) } finally { @@ -179,22 +199,29 @@ type RelayBootstrapState = { async function resolveRelayBootstrapStateSequentially( conn: SshConnection, hostPlatform: RemoteHostPlatform, - fullVersion: string + fullVersion: string, + signal?: AbortSignal ): Promise { - const installState = await resolveRemoteInstallState(conn, hostPlatform, fullVersion) - const nodePath = await resolveRemoteNodePath(conn, hostPlatform) + const installState = await resolveRemoteInstallState(conn, hostPlatform, fullVersion, { signal }) + const nodePath = await resolveRemoteNodePath(conn, hostPlatform, { signal }) return { ...installState, nodePath } } async function resolveRelayBootstrapState( conn: SshConnection, hostPlatform: RemoteHostPlatform, - fullVersion: string + fullVersion: string, + signal?: AbortSignal ): Promise { if (!conn.canRunConcurrentExecCommands()) { - return resolveRelayBootstrapStateSequentially(conn, hostPlatform, fullVersion) + return resolveRelayBootstrapStateSequentially(conn, hostPlatform, fullVersion, signal) } const abortController = new AbortController() + const abortForDeploy = (): void => abortController.abort() + signal?.addEventListener('abort', abortForDeploy, { once: true }) + if (signal?.aborted) { + abortForDeploy() + } const installStatePromise = resolveRemoteInstallState(conn, hostPlatform, fullVersion, { rethrowSessionLimitErrors: true, signal: abortController.signal @@ -205,10 +232,12 @@ async function resolveRelayBootstrapState( }) try { const [installState, nodePath] = await Promise.all([installStatePromise, nodePathPromise]) + signal?.throwIfAborted() return { ...installState, nodePath } } catch (err) { abortController.abort() const settled = await Promise.allSettled([installStatePromise, nodePathPromise]) + signal?.throwIfAborted() if (!isSshSessionLimitError(err)) { throw err } @@ -224,7 +253,9 @@ async function resolveRelayBootstrapState( console.warn( '[ssh-relay] Concurrent bootstrap probes hit the remote SSH session limit; retrying sequentially.' ) - return resolveRelayBootstrapStateSequentially(conn, hostPlatform, fullVersion) + return resolveRelayBootstrapStateSequentially(conn, hostPlatform, fullVersion, signal) + } finally { + signal?.removeEventListener('abort', abortForDeploy) } } @@ -241,11 +272,40 @@ async function deployAndLaunchRelayInner( conn: SshConnection, onProgress?: (status: string) => void, graceTimeSeconds?: number, - relayInstanceId?: string + relayInstanceId?: string, + deploySignal?: AbortSignal +): Promise { + while (true) { + deploySignal?.throwIfAborted() + try { + return await deployAndLaunchRelayAttempt( + conn, + onProgress, + graceTimeSeconds, + relayInstanceId, + deploySignal + ) + } catch (err) { + if (!(err instanceof RelayDirectoryGcConflictError)) { + throw err + } + // Why: GC atomically moves the old install aside. Wait for its stable + // sibling claim to clear, then recompute install state from scratch. + await waitForRelayGcClaimRelease(conn, err.remoteRelayDir, err.hostPlatform, deploySignal) + } + } +} + +async function deployAndLaunchRelayAttempt( + conn: SshConnection, + onProgress?: (status: string) => void, + graceTimeSeconds?: number, + relayInstanceId?: string, + deploySignal?: AbortSignal ): Promise { onProgress?.('Detecting remote platform...') console.log('[ssh-relay] Detecting remote platform...') - const hostPlatform = await detectRemoteHostPlatform(conn) + const hostPlatform = await detectRemoteHostPlatform(conn, { signal: deploySignal }) if (!hostPlatform) { throw new Error( 'Unsupported remote platform. Orca relay supports: linux-x64, linux-arm64, darwin-x64, darwin-arm64, win32-x64, win32-arm64.' @@ -275,57 +335,104 @@ async function deployAndLaunchRelayInner( // 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) + await resolveRelayBootstrapState(conn, hostPlatform, fullVersion, deploySignal) console.log(`[ssh-relay] Remote dir: ${remoteRelayDir}`) console.log(`[ssh-relay] Already installed at ${fullVersion}: ${alreadyInstalled}`) + let ownsInstallLock = false + let launchGcClaimToken: string | undefined if (alreadyInstalled) { - await repairInstalledNativeDeps(conn, remoteRelayDir, platform, hostPlatform, nodePath) + const launchFence = await repairInstalledNativeDeps( + conn, + remoteRelayDir, + platform, + hostPlatform, + nodePath, + deploySignal + ) + ownsInstallLock = launchFence.ownsInstallLock + launchGcClaimToken = launchFence.gcClaimToken + deploySignal?.throwIfAborted() } else { // Why: serialize concurrent first-installs of the same version against - // each other via an atomic mkdir lock. The losing caller polls and either + // each other via a host-native exclusive lock. The losing caller polls and either // re-checks `alreadyInstalled` (now true) or steals a stale lock. - await acquireInstallLock(conn, remoteRelayDir, hostPlatform) + await acquireInstallLock(conn, remoteRelayDir, hostPlatform, { signal: deploySignal }) + ownsInstallLock = true try { // Re-probe after acquiring the lock — a sibling installer may have // finished while we were waiting. - if (!(await isRelayAlreadyInstalled(conn, remoteRelayDir, hostPlatform))) { + if ( + !(await isRelayAlreadyInstalled(conn, remoteRelayDir, hostPlatform, { + signal: deploySignal + })) + ) { onProgress?.('Uploading relay...') console.log('[ssh-relay] Uploading relay...') - await uploadRelay(conn, platform, remoteRelayDir, fullVersion, hostPlatform) + await uploadRelay(conn, platform, remoteRelayDir, fullVersion, hostPlatform, deploySignal) console.log('[ssh-relay] Upload complete') onProgress?.('Installing native dependencies...') console.log('[ssh-relay] Installing native dependencies...') - await installNativeDeps(conn, remoteRelayDir, platform, hostPlatform, nodePath) + await installNativeDeps( + conn, + remoteRelayDir, + platform, + hostPlatform, + nodePath, + deploySignal + ) console.log('[ssh-relay] Native deps installed') - // Why: write `.install-complete` BEFORE releasing the lock so a - // sibling never observes the dir as "complete but locked", which - // would lead GC to skip a recoverable dir indefinitely. - await finalizeInstall(conn, remoteRelayDir, hostPlatform) - } else { - await abandonInstall(conn, remoteRelayDir, hostPlatform) + // Why: mark complete but retain the lock until launch makes daemon + // liveness observable to cross-version GC. + await finalizeInstall(conn, remoteRelayDir, hostPlatform, { + signal: deploySignal, + releaseLock: false + }) } } catch (err) { // Why: leave a partial install dir in place (no `.install-complete`) // so the next deploy detects the partial and re-runs upload + install. - // Just release the lock so a concurrent caller can retry. - await abandonInstall(conn, remoteRelayDir, hostPlatform) + // Keep the lock if remote command termination was not confirmed; stale + // recovery is safer than overlapping a still-running npm process. + if (!isUnconfirmedSshCommandTermination(err)) { + await abandonInstall(conn, remoteRelayDir, hostPlatform) + ownsInstallLock = false + } throw err } } - onProgress?.('Starting relay...') - console.log('[ssh-relay] Launching relay...') - const launched = await launchRelay( - conn, - remoteRelayDir, - hostPlatform, - nodePath, - graceTimeSeconds, - relayInstanceId - ) + let launched: Awaited> + let launchLivenessObserved = false + try { + deploySignal?.throwIfAborted() + onProgress?.('Starting relay...') + console.log('[ssh-relay] Launching relay...') + launched = await launchRelay( + conn, + remoteRelayDir, + hostPlatform, + nodePath, + graceTimeSeconds, + relayInstanceId, + deploySignal + ) + launchLivenessObserved = true + } finally { + // Why: older clients understand only the install lock. If launch never + // becomes live, retain it for stale recovery so their GC cannot race a + // concurrent caller that was waiting behind this owner. + if (ownsInstallLock && launchLivenessObserved) { + await abandonInstall(conn, remoteRelayDir, hostPlatform) + } + // The detached start may outlive a timed-out SSH command. Preserve either + // fence on failed launch until stale recovery can prove the handoff ended. + if (launchGcClaimToken && launchLivenessObserved) { + await releaseRelayGcClaimWithRetry(conn, remoteRelayDir, launchGcClaimToken, hostPlatform) + } + } console.log('[ssh-relay] Relay started successfully') // Why: best-effort cleanup of unreferenced sibling version dirs. Errors @@ -352,7 +459,8 @@ async function uploadRelay( platform: RelayPlatform, remoteDir: string, fullVersion: string, - hostPlatform: RemoteHostPlatform + hostPlatform: RemoteHostPlatform, + signal?: AbortSignal ): Promise { const localRelayDir = getLocalRelayPath(platform) if (!localRelayDir || !existsSync(localRelayDir)) { @@ -363,16 +471,19 @@ async function uploadRelay( } // Create remote directory - await execHostCommand(conn, hostPlatform, makeRemoteDirectoryCommand(hostPlatform, remoteDir)) + await execHostCommand(conn, hostPlatform, makeRemoteDirectoryCommand(hostPlatform, remoteDir), { + signal + }) - await uploadDirectoryForConnection(conn, localRelayDir, remoteDir, hostPlatform) + await uploadDirectoryForConnection(conn, localRelayDir, remoteDir, hostPlatform, signal) // Make the node binary executable if (!isWindowsRemoteHost(hostPlatform)) { await execHostCommand( conn, hostPlatform, - makeRemoteExecutableCommand(hostPlatform, joinRemotePath(hostPlatform, remoteDir, 'node')) + makeRemoteExecutableCommand(hostPlatform, joinRemotePath(hostPlatform, remoteDir, 'node')), + { signal } ) } @@ -383,7 +494,8 @@ async function uploadRelay( conn, hostPlatform, joinRemotePath(hostPlatform, remoteDir, '.version'), - fullVersion + fullVersion, + signal ) } @@ -391,10 +503,11 @@ async function uploadDirectoryForConnection( conn: SshConnection, localRelayDir: string, remoteDir: string, - hostPlatform: RemoteHostPlatform + hostPlatform: RemoteHostPlatform, + signal?: AbortSignal ): Promise { if (typeof conn.uploadDirectory === 'function') { - await conn.uploadDirectory(localRelayDir, remoteDir, { hostPlatform }) + await conn.uploadDirectory(localRelayDir, remoteDir, { hostPlatform, signal }) return } @@ -410,10 +523,11 @@ async function writeRemoteFile( conn: SshConnection, hostPlatform: RemoteHostPlatform, remotePath: string, - contents: string + contents: string, + signal?: AbortSignal ): Promise { if (typeof conn.writeFile === 'function') { - await conn.writeFile(remotePath, contents, { hostPlatform }) + await conn.writeFile(remotePath, contents, { hostPlatform, signal }) return } @@ -438,31 +552,64 @@ const RELAY_NATIVE_DEPS = { '@parcel/watcher': '2.5.6' } as const -async function hasRequiredNativeDeps( +type RelayNativeDepName = keyof typeof RELAY_NATIVE_DEPS +const RELAY_NATIVE_DEP_NAMES = Object.keys(RELAY_NATIVE_DEPS) as RelayNativeDepName[] +const NATIVE_DEPS_MISSING_PREFIX = 'ORCA-NATIVE-DEPS-MISSING:' + +// Why: npm 12 blocks dependency lifecycle scripts unless each exact package +// version is approved, even when ignore-scripts is explicitly disabled. +const RELAY_NATIVE_DEP_SCRIPT_ALLOWLIST = Object.fromEntries( + Object.entries(RELAY_NATIVE_DEPS).map(([name, version]) => [`${name}@${version}`, true]) +) + +function nativeDepsProbeJs(successToken: string): string { + // Why: node-pty's Windows wrapper defers loading conpty.node until first + // spawn, so require("node-pty") alone cannot prove the binding is healthy. + const loadNodePty = + 'require("node-pty"); require("node-pty/lib/utils").loadNativeModule(process.platform==="win32"&&Number(require("os").release().split(".")[2])>=18309?"conpty":"pty")' + return `(()=>{const missing=[];try{${loadNodePty}}catch{missing.push("node-pty")}try{require("@parcel/watcher")}catch{missing.push("@parcel/watcher")}if(missing.length){console.log("${NATIVE_DEPS_MISSING_PREFIX}"+missing.join(","));process.exitCode=1}else{console.log(${JSON.stringify(successToken)})}})()` +} + +function missingNativeDepsFromProbe(output: string): RelayNativeDepName[] { + const marker = output + .split(/\r?\n/) + .find((line) => line.trim().startsWith(NATIVE_DEPS_MISSING_PREFIX)) + if (!marker) { + return [...RELAY_NATIVE_DEP_NAMES] + } + const reported = marker.trim().slice(NATIVE_DEPS_MISSING_PREFIX.length).split(',') + return RELAY_NATIVE_DEP_NAMES.filter((name) => reported.includes(name)) +} + +async function probeRequiredNativeDeps( conn: SshConnection, remoteDir: string, hostPlatform: RemoteHostPlatform, - nodePath: string -): Promise { + nodePath: string, + signal?: AbortSignal +): Promise<{ available: boolean; missing: RelayNativeDepName[] }> { const escapedNode = shellEscape(nodePath) + const probeJs = nativeDepsProbeJs('ORCA-NATIVE-DEPS-OK') try { const command = isWindowsRemoteHost(hostPlatform) ? commandWithNodePath( hostPlatform, nodePath, remoteDir, - `try { & ${powerShellLiteral(nodePath)} -e ${powerShellNativeArg('require.resolve("node-pty"); require.resolve("@parcel/watcher"); console.log("ORCA-NATIVE-DEPS-OK")')} } catch { 'MISSING' }` + `try { & ${powerShellLiteral(nodePath)} -e ${powerShellNativeArg(probeJs)} } catch { 'MISSING' }` ) : commandWithNodePath( hostPlatform, nodePath, remoteDir, - `(${escapedNode} -e 'require.resolve("node-pty"); require.resolve("@parcel/watcher"); console.log("ORCA-NATIVE-DEPS-OK")' 2>/dev/null || echo MISSING)` + `(${escapedNode} -e ${shellEscape(probeJs)} 2>/dev/null || echo MISSING)` ) - const probe = await execHostCommand(conn, hostPlatform, command) - return probe.includes('ORCA-NATIVE-DEPS-OK') + const probe = await execHostCommand(conn, hostPlatform, command, { signal }) + const available = probe.includes('ORCA-NATIVE-DEPS-OK') + return { available, missing: available ? [] : missingNativeDepsFromProbe(probe) } } catch { - return false + signal?.throwIfAborted() + return { available: false, missing: [...RELAY_NATIVE_DEP_NAMES] } } } @@ -471,25 +618,116 @@ async function repairInstalledNativeDeps( remoteDir: string, platform: RelayPlatform, hostPlatform: RemoteHostPlatform, - nodePath: string -): Promise { - if (await hasRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath)) { - return + nodePath: string, + signal?: AbortSignal +): Promise<{ ownsInstallLock: boolean; gcClaimToken?: string }> { + const initialProbe = await probeRequiredNativeDeps( + conn, + remoteDir, + hostPlatform, + nodePath, + signal + ) + const lockResult = await tryAcquireRelayRepairLock(conn, remoteDir, hostPlatform, { signal }) + if (lockResult === 'gc') { + throw new RelayDirectoryGcConflictError(remoteDir, hostPlatform) + } + if (lockResult === 'acquired') { + let stillInstalled: boolean + try { + stillInstalled = await isRelayAlreadyInstalled(conn, remoteDir, hostPlatform, { + rethrowSessionLimitErrors: true, + signal + }) + } catch (err) { + await abandonInstall(conn, remoteDir, hostPlatform) + throw err + } + if (!stillInstalled) { + // Why: GC may finish its rename before our lock attempt recreates the + // original path. Never trust probes made before this locked recheck. + await abandonInstall(conn, remoteDir, hostPlatform) + throw new RelayDirectoryGcConflictError(remoteDir, hostPlatform) + } + } + const gcClaimToken = + lockResult === 'busy' || lockResult === 'error' + ? await acquireRelayLaunchGcFence(conn, remoteDir, hostPlatform, signal) + : undefined + if (initialProbe.available) { + // Why: even a healthy reconnect must stay fenced until launch liveness is + // observable; otherwise cross-version GC can rename after this probe. + return { ownsInstallLock: lockResult === 'acquired', gcClaimToken } } + // Why: an already-installed relay can launch in degraded mode (fs/git/preflight + // still work; native-backed ops fail), so native-deps repair is best-effort: + // lock contention and repair failures must not abort the connection. Pre-repair + // this path used require.resolve (which passed for a present-but-unloadable + // binding), so a fatal repair here would be a straight regression. console.warn(`[ssh-relay] Repairing missing native deps at ${remoteDir}`) - await acquireInstallLock(conn, remoteDir, hostPlatform) + if (lockResult === 'busy' || lockResult === 'error') { + console.warn( + `[ssh-relay] Native-deps repair lock is ${lockResult} at ${remoteDir}; launching degraded` + ) + return { ownsInstallLock: false, gcClaimToken } + } try { // Why: older complete relay dirs were created before @parcel/watcher was // installed. Re-probe under the lock so only one reconnect mutates the dir. - if (!(await hasRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath))) { - await installNativeDeps(conn, remoteDir, platform, hostPlatform, nodePath) - await finalizeInstall(conn, remoteDir, hostPlatform) - } else { - await abandonInstall(conn, remoteDir, hostPlatform) + const probe = await probeRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal) + if (!probe.available) { + await installNativeDeps( + conn, + remoteDir, + platform, + hostPlatform, + nodePath, + signal, + probe.missing + ) + await finalizeInstall(conn, remoteDir, hostPlatform, { signal, releaseLock: false }) } + return { ownsInstallLock: true } } catch (err) { - await abandonInstall(conn, remoteDir, hostPlatform) + const terminationUnconfirmed = isUnconfirmedSshCommandTermination(err) + // Why: keep a confirmed-failure lock through degraded launch so GC cannot + // move the relay before liveness is visible. Unconfirmed remote mutation + // keeps its stale-recoverable lock beyond this connection. + console.warn( + `[ssh-relay] Native deps repair failed at ${remoteDir}; launching degraded: ${ + err instanceof Error ? err.message : String(err) + }` + ) + return { ownsInstallLock: !terminationUnconfirmed } + } +} + +async function acquireRelayLaunchGcFence( + conn: SshConnection, + remoteDir: string, + hostPlatform: RemoteHostPlatform, + signal?: AbortSignal +): Promise { + const token = await tryAcquireRelayGcClaim(conn, remoteDir, hostPlatform, signal) + if (!token) { + signal?.throwIfAborted() + throw new RelayDirectoryGcConflictError(remoteDir, hostPlatform) + } + try { + signal?.throwIfAborted() + const stillInstalled = await isRelayAlreadyInstalled(conn, remoteDir, hostPlatform, { + rethrowSessionLimitErrors: true, + signal + }) + if (!stillInstalled) { + throw new RelayDirectoryGcConflictError(remoteDir, hostPlatform) + } + // Why: a caller that cannot own the install lock still needs its own + // durable fence; never borrow another connection's lock through launch. + return token + } catch (err) { + await releaseRelayGcClaimWithRetry(conn, remoteDir, token, hostPlatform) throw err } } @@ -507,14 +745,14 @@ async function installNativeDeps( remoteDir: string, platform: RelayPlatform, hostPlatform: RemoteHostPlatform, - nodePath: string + nodePath: string, + signal?: AbortSignal, + resetDeps: RelayNativeDepName[] = [] ): Promise { - // Why: node's bin directory must be in PATH for npm's child processes. + // Why: commandWithNodePath puts node's bin directory in PATH for npm's child processes. // npm install runs node-pty's prebuild script (`node scripts/prebuild.js`) // which spawns `node` as a child — if node isn't in PATH, that child // fails with exit 127 even though we invoked npm via its full path. - const escapedNode = shellEscape(nodePath) - // npm init -y rejects '+' in derived package names (content-hashed dir // names like relay-0.1.0+abc123). Bypass it with a fixed minimal // package.json. type:commonjs pins module resolution against Node default @@ -524,38 +762,51 @@ async function installNativeDeps( version: '1.0.0', private: true, type: 'commonjs', - dependencies: RELAY_NATIVE_DEPS + dependencies: RELAY_NATIVE_DEPS, + allowScripts: RELAY_NATIVE_DEP_SCRIPT_ALLOWLIST })}\n` await writeRemoteFile( conn, hostPlatform, joinRemotePath(hostPlatform, remoteDir, 'package.json'), - pkgJson + pkgJson, + signal ) try { const installArgs = Object.entries(RELAY_NATIVE_DEPS) .map(([dep, version]) => shellEscape(`${dep}@${version}`)) .join(' ') + // Why: npm reports a present package as up to date even when one packaged + // native file was deleted. Reset only dependencies the probe found broken. + const resetCommand = resetNativeDepsCommand(hostPlatform, resetDeps) + const resetPrefix = resetCommand ? `${resetCommand}; ` : '' const command = isWindowsRemoteHost(hostPlatform) ? commandWithNodePath( hostPlatform, nodePath, remoteDir, - `npm install --omit=dev --no-audit --no-fund ${Object.entries(RELAY_NATIVE_DEPS) + `${resetPrefix}npm install --ignore-scripts=false --omit=dev --no-audit --no-fund ${Object.entries( + RELAY_NATIVE_DEPS + ) .map(([dep, version]) => powerShellLiteral(`${dep}@${version}`)) - .join(' ')}` + .join(' ')}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }` ) : commandWithNodePath( hostPlatform, nodePath, remoteDir, - `npm install --omit=dev --no-audit --no-fund ${installArgs} 2>&1` + `${resetPrefix}npm install --ignore-scripts=false --omit=dev --no-audit --no-fund ${installArgs} 2>&1` ) await execHostCommand(conn, hostPlatform, command, { - timeoutMs: NATIVE_DEPS_INSTALL_TIMEOUT_MS + timeoutMs: NATIVE_DEPS_COMMAND_TIMEOUT_MS, + signal }) } catch (err) { + if (isUnconfirmedSshCommandTermination(err)) { + throw err + } + signal?.throwIfAborted() // Don't write .install-complete on hard fail; reconnect retries on a // partial install. Greppable token so user bug reports paste something // searchable. @@ -568,7 +819,7 @@ async function installNativeDeps( // remote and replace node-gyp's opaque `not found: make` with an actionable // install hint instead of leaking the raw npm output to the user. if (platform.startsWith('linux') && shouldProbeBuildToolchainAfterNativeDepsFailure(msg)) { - const toolchain = await probeBuildToolchain(conn, hostPlatform) + const toolchain = await probeBuildToolchain(conn, hostPlatform, signal) if (toolchain?.toolchainMissing) { throw new Error(formatMissingToolchainError(toolchain, msg)) } @@ -576,54 +827,178 @@ async function installNativeDeps( throw err } - // SFTP doesn't preserve execute bits; node-pty's spawn-helper prebuild - // must be +x for posix_spawnp. - if (!isWindowsRemoteHost(hostPlatform)) { - await execHostCommand( - conn, - hostPlatform, - `find ${shellEscape(joinRemotePath(hostPlatform, remoteDir, 'node_modules/node-pty/prebuilds'))} -name spawn-helper -exec chmod +x {} + 2>/dev/null; true` - ) + await makeNodePtySpawnHelperExecutable(conn, remoteDir, hostPlatform, signal) + + let probe = await probeInstalledNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal) + if (!probe.available) { + // Why: npm treats an already-present package as up to date, so enabling + // lifecycle scripts on install cannot repair a binding skipped earlier. + console.warn(`[ssh-relay] Rebuilding unloadable native deps at ${remoteDir}`) + let rebuilt = false + try { + await rebuildNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal) + rebuilt = true + } catch (err) { + if (isUnconfirmedSshCommandTermination(err)) { + throw err + } + signal?.throwIfAborted() + console.warn( + `[ssh-relay][NATIVE-DEPS-REBUILD-FAIL] npm rebuild native deps failed at ${remoteDir} (${platform}): ${(err as Error).message}` + ) + } + signal?.throwIfAborted() + if (rebuilt) { + await makeNodePtySpawnHelperExecutable(conn, remoteDir, hostPlatform, signal) + probe = await probeInstalledNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal) + } } - // node -e require() catches unloadable installs (wrong arch, missing - // prebuild, broken native binding) that test -d cannot. Stderr → file - // so .bashrc noise can't pollute the sentinel match; preserved for the - // [NPTY-MISSING] breadcrumb. MISSING is non-fatal by design — see - // docs/ssh-relay-versioned-install-dirs.md (relay still serves - // fs/git/preflight; only pty.spawn fails at runtime). - const PROBE_OK = 'ORCA-NPTY-PROBE-OK' - const stderrFile = joinRemotePath(hostPlatform, remoteDir, '.npty-probe.stderr') - const escapedStderr = shellEscape(stderrFile) - const probeCommand = isWindowsRemoteHost(hostPlatform) + // MISSING is non-fatal by design: the relay still serves fs/git/preflight; + // only native-backed operations fail on hosts that cannot build the addons. + if (!probe.available) { + console.warn( + `[ssh-relay][NPTY-MISSING] native deps installed but require() failed at ${remoteDir} (${platform}). stdout=${probe.output.trim().slice(-200)} stderr=${probe.stderr.trim().slice(-500)}` + ) + } +} + +function resetNativeDepsCommand( + hostPlatform: RemoteHostPlatform, + resetDeps: RelayNativeDepName[] +): string { + if (resetDeps.length === 0) { + return '' + } + const resetNodePty = resetDeps.includes('node-pty') + const resetWatcher = resetDeps.includes('@parcel/watcher') + if (isWindowsRemoteHost(hostPlatform)) { + const commands: string[] = [] + if (resetNodePty) { + commands.push( + `Remove-Item -LiteralPath ${powerShellLiteral('node_modules/node-pty')} -Recurse -Force -ErrorAction SilentlyContinue` + ) + } + if (resetWatcher) { + commands.push( + `Remove-Item -LiteralPath ${powerShellLiteral('node_modules/@parcel/watcher')} -Recurse -Force -ErrorAction SilentlyContinue`, + `$parcelScope = ${powerShellLiteral('node_modules/@parcel')}`, + `Get-ChildItem -LiteralPath $parcelScope -Force -ErrorAction SilentlyContinue | Where-Object { $_.Name.StartsWith('watcher-', [StringComparison]::Ordinal) } | Remove-Item -Recurse -Force -ErrorAction SilentlyContinue` + ) + } + return commands.join('; ') + } + const commands: string[] = [] + if (resetNodePty) { + commands.push(`rm -rf ${shellEscape('node_modules/node-pty')}`) + } + if (resetWatcher) { + commands.push( + `rm -rf ${shellEscape('node_modules/@parcel/watcher')}`, + `find ${shellEscape('node_modules/@parcel')} -maxdepth 1 -name 'watcher-*' -exec rm -rf {} + 2>/dev/null || true` + ) + } + return commands.join('; ') +} + +async function rebuildNativeDeps( + conn: SshConnection, + remoteDir: string, + hostPlatform: RemoteHostPlatform, + nodePath: string, + signal?: AbortSignal +): Promise { + const depNames = Object.keys(RELAY_NATIVE_DEPS) + const command = isWindowsRemoteHost(hostPlatform) ? commandWithNodePath( hostPlatform, nodePath, remoteDir, - `try { & ${powerShellLiteral(nodePath)} -e ${powerShellNativeArg('require("node-pty"); console.log(process.argv[1])')} ${powerShellLiteral(PROBE_OK)}; if ($LASTEXITCODE -ne 0) { 'MISSING' } } catch { 'MISSING' }` + `npm rebuild --ignore-scripts=false ${depNames.map(powerShellLiteral).join(' ')}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }` ) : commandWithNodePath( hostPlatform, nodePath, remoteDir, - `(${escapedNode} -e 'require("node-pty"); console.log(process.argv[1])' ${shellEscape(PROBE_OK)} 2>${escapedStderr} || echo MISSING)` + `npm rebuild --ignore-scripts=false ${depNames.map(shellEscape).join(' ')} 2>&1` ) - const probeOutput = await execHostCommand(conn, hostPlatform, probeCommand) - if (!probeOutput.includes(PROBE_OK)) { - const remoteStderr = isWindowsRemoteHost(hostPlatform) - ? '' - : await execHostCommand(conn, hostPlatform, `cat ${escapedStderr} 2>/dev/null; true`).catch( - () => '' - ) - console.warn( - `[ssh-relay][NPTY-MISSING] node-pty installed but require() failed at ${remoteDir} (${platform}). stdout=${probeOutput.trim().slice(-200)} stderr=${remoteStderr.trim().slice(-500)}` - ) + await execHostCommand(conn, hostPlatform, command, { + timeoutMs: NATIVE_DEPS_COMMAND_TIMEOUT_MS, + signal + }) +} + +async function makeNodePtySpawnHelperExecutable( + conn: SshConnection, + remoteDir: string, + hostPlatform: RemoteHostPlatform, + signal?: AbortSignal +): Promise { + if (isWindowsRemoteHost(hostPlatform)) { + return } + // SFTP doesn't preserve execute bits; node-pty's spawn-helper prebuild + // must be +x for posix_spawnp. await execHostCommand( conn, hostPlatform, - removeRemoteFileCommand(hostPlatform, stderrFile) - ).catch(() => {}) + `find ${shellEscape(joinRemotePath(hostPlatform, remoteDir, 'node_modules/node-pty/prebuilds'))} -name spawn-helper -exec chmod +x {} + 2>/dev/null; true`, + { signal } + ) +} + +async function probeInstalledNativeDeps( + conn: SshConnection, + remoteDir: string, + hostPlatform: RemoteHostPlatform, + nodePath: string, + signal?: AbortSignal +): Promise<{ + available: boolean + missing: RelayNativeDepName[] + output: string + stderr: string +}> { + // require() catches unloadable installs (wrong arch, missing prebuild, or a + // skipped lifecycle script) that require.resolve() and test -d both miss. + const PROBE_OK = 'ORCA-NPTY-PROBE-OK' + const stderrFile = joinRemotePath(hostPlatform, remoteDir, '.npty-probe.stderr') + const escapedStderr = shellEscape(stderrFile) + const probeJs = nativeDepsProbeJs(PROBE_OK) + const probeCommand = isWindowsRemoteHost(hostPlatform) + ? commandWithNodePath( + hostPlatform, + nodePath, + remoteDir, + `try { & ${powerShellLiteral(nodePath)} -e ${powerShellNativeArg(probeJs)} ${powerShellLiteral(PROBE_OK)}; if ($LASTEXITCODE -ne 0) { 'MISSING' } } catch { 'MISSING' }` + ) + : commandWithNodePath( + hostPlatform, + nodePath, + remoteDir, + `(${shellEscape(nodePath)} -e ${shellEscape(probeJs)} ${shellEscape(PROBE_OK)} 2>${escapedStderr} || echo MISSING)` + ) + const probeOutput = await execHostCommand(conn, hostPlatform, probeCommand, { signal }) + const remoteStderr = + probeOutput.includes(PROBE_OK) || isWindowsRemoteHost(hostPlatform) + ? '' + : await execHostCommand(conn, hostPlatform, `cat ${escapedStderr} 2>/dev/null; true`, { + signal + }).catch(() => '') + signal?.throwIfAborted() + if (!isWindowsRemoteHost(hostPlatform)) { + // The POSIX probe redirects stderr to this file; the Windows probe does not. + await execHostCommand(conn, hostPlatform, removeRemoteFileCommand(hostPlatform, stderrFile), { + signal + }).catch(() => {}) + signal?.throwIfAborted() + } + return { + available: probeOutput.includes(PROBE_OK), + missing: probeOutput.includes(PROBE_OK) ? [] : missingNativeDepsFromProbe(probeOutput), + output: probeOutput, + stderr: remoteStderr + } } function getLocalRelayPath(platform: RelayPlatform): string | null { @@ -663,7 +1038,8 @@ async function launchRelay( hostPlatform: RemoteHostPlatform, nodePath: string, graceTimeSeconds?: number, - relayInstanceId?: string + relayInstanceId?: string, + signal?: AbortSignal ): Promise<{ transport: MultiplexerTransport; nodePath: string; sockPath: string }> { // Why: Phase 1 of the plan requires Node.js on the remote. We use the // system `node` rather than bundling a node binary, keeping the relay @@ -695,21 +1071,27 @@ async function launchRelay( conn, hostPlatform, remoteDir, - activePipeMarkerPath + activePipeMarkerPath, + signal )) ?? { sockPath: sockFile, endpointDir } const fallbackEndpoint = buildWindowsRelayFallbackEndpoint(hostPlatform, remoteDir, sockName) - return launchWindowsRelay(conn, hostPlatform, { - remoteDir, - nodePath, - sockPath: activeEndpoint.sockPath, - endpointDir: activeEndpoint.endpointDir, - graceTime, - activePipeMarkerPath, - reconnectFallback: fallbackEndpoint - }) + return launchWindowsRelay( + conn, + hostPlatform, + { + remoteDir, + nodePath, + sockPath: activeEndpoint.sockPath, + endpointDir: activeEndpoint.endpointDir, + graceTime, + activePipeMarkerPath, + reconnectFallback: fallbackEndpoint + }, + signal + ) } // Why: after an app restart a relay may still be running in its grace @@ -719,29 +1101,43 @@ async function launchRelay( try { const probeOutput = await execCommand( conn, - `test -S ${shellEscape(sockFile)} && echo ALIVE || echo DEAD` + `test -S ${shellEscape(sockFile)} && echo ALIVE || echo DEAD`, + { signal } ) console.warn(`[ssh-relay] Socket probe result: "${probeOutput.trim()}"`) if (probeOutput.trim() === 'ALIVE') { console.log('[ssh-relay] Existing relay socket found, attempting reconnect...') try { const channel = await conn.exec( - `cd ${escapedDir} && ${escapedNode} relay.js --connect --sock-path ${shellEscape(sockFile)}` + `cd ${escapedDir} && ${escapedNode} relay.js --connect --sock-path ${shellEscape(sockFile)}`, + { signal } ) - const transport = await waitForSentinel(channel) + const transport = await waitForSentinel(channel, signal) console.log('[ssh-relay] Reconnected to existing relay via socket') return { transport, nodePath, sockPath: sockFile } } catch (err) { + signal?.throwIfAborted() console.warn( '[ssh-relay] Socket reconnect failed, launching fresh relay:', err instanceof Error ? err.message : String(err) ) // Why: stale socket from a crashed relay — remove it so the // fresh launch can bind a new socket at the same path. - await execCommand(conn, `rm -f ${shellEscape(sockFile)}`).catch(() => {}) + await execCommand(conn, `rm -f ${shellEscape(sockFile)}`, { signal }).catch( + (cleanupErr) => { + if (isUnconfirmedSshCommandTermination(cleanupErr)) { + throw cleanupErr + } + } + ) + signal?.throwIfAborted() } } - } catch { + } catch (err) { + if (isUnconfirmedSshCommandTermination(err)) { + throw err + } + signal?.throwIfAborted() // Probe failed — fall through to fresh launch } @@ -762,7 +1158,7 @@ async function launchRelay( // subsequent logging (it wraps process.stderr/stdout), so the current log // stays at relay.log for the `tail relay.log` diagnostics workflow. const launchCmd = `cd ${escapedDir} && nohup ${escapedNode} relay.js --detached --grace-time ${graceTime} --sock-path ${shellEscape(sockFile)} --log-file ${shellEscape(logFile)} > ${shellEscape(logFile)} 2>&1 {}) launchChannel.on('error', () => {}) launchChannel.stderr.on('data', () => {}) @@ -784,36 +1180,39 @@ async function launchRelay( const POLL_TIMEOUT_MS = 10_000 const pollStart = Date.now() let socketReady = false - while (Date.now() - pollStart < POLL_TIMEOUT_MS) { - try { - // Why: node is guaranteed to exist on the remote (we just deployed - // the relay with it). Using it to probe the socket is more portable - // than python3/socat/perl which may not be installed. The socket - // path is passed as argv[1] to avoid shell quoting issues with -e. - const result = await execCommand( - conn, - `${escapedNode} -e 'var s=require("net").connect(process.argv[1]);s.on("connect",function(){s.destroy();process.stdout.write("READY")});s.on("error",function(){process.stdout.write("WAITING")})' ${shellEscape(sockFile)} 2>/dev/null || (test -S ${shellEscape(sockFile)} && echo READY || echo WAITING)` - ) - if (result.trim() === 'READY') { - socketReady = true - break + try { + while (Date.now() - pollStart < POLL_TIMEOUT_MS) { + try { + // Why: node is guaranteed to exist on the remote (we just deployed + // the relay with it). Using it to probe the socket is more portable + // than python3/socat/perl which may not be installed. The socket + // path is passed as argv[1] to avoid shell quoting issues with -e. + const result = await execCommand( + conn, + `${escapedNode} -e 'var s=require("net").connect(process.argv[1]);s.on("connect",function(){s.destroy();process.stdout.write("READY")});s.on("error",function(){process.stdout.write("WAITING")})' ${shellEscape(sockFile)} 2>/dev/null || (test -S ${shellEscape(sockFile)} && echo READY || echo WAITING)`, + { signal } + ) + if (result.trim() === 'READY') { + socketReady = true + break + } + } catch { + signal?.throwIfAborted() + /* exec failed, retry */ } - } catch { - /* exec failed, retry */ + await waitForRelayPoll(POLL_INTERVAL_MS, signal) } - await new Promise((r) => setTimeout(r, POLL_INTERVAL_MS)) + } finally { + launchChannel.close() } - // Why: close the fire-and-forget launch channel now that the relay's - // socket is either ready or the poll timed out. Leaving it open leaks - // an SSH channel per relay restart. - launchChannel.close() - if (!socketReady) { const logOutput = await execCommand( conn, - `tail -20 ${shellEscape(logFile)} 2>/dev/null || echo "(no log)"` + `tail -20 ${shellEscape(logFile)} 2>/dev/null || echo "(no log)"`, + { signal } ).catch(() => '(could not read log)') + signal?.throwIfAborted() throw new Error(`Relay failed to start within ${POLL_TIMEOUT_MS / 1000}s. Log:\n${logOutput}`) } @@ -822,9 +1221,28 @@ async function launchRelay( // stdin/stdout to the relay's Unix socket — same path used for reconnect // after app restart. const channel = await conn.exec( - `cd ${escapedDir} && ${escapedNode} relay.js --connect --sock-path ${shellEscape(sockFile)}` + `cd ${escapedDir} && ${escapedNode} relay.js --connect --sock-path ${shellEscape(sockFile)}`, + { signal } ) - return { transport: await waitForSentinel(channel), nodePath, sockPath: sockFile } + return { transport: await waitForSentinel(channel, signal), nodePath, sockPath: sockFile } +} + +function waitForRelayPoll(delayMs: number, signal?: AbortSignal): Promise { + return new Promise((resolve, reject) => { + const onAbort = (): void => { + clearTimeout(timeout) + signal?.removeEventListener('abort', onAbort) + reject(createSshOperationAbortError()) + } + const timeout = setTimeout(() => { + signal?.removeEventListener('abort', onAbort) + resolve() + }, delayMs) + signal?.addEventListener('abort', onAbort, { once: true }) + if (signal?.aborted) { + onAbort() + } + }) } function buildWindowsRelayFallbackEndpoint( @@ -844,15 +1262,20 @@ async function readWindowsActiveRelayEndpoint( conn: SshConnection, hostPlatform: RemoteHostPlatform, remoteDir: string, - markerPath: string + markerPath: string, + signal?: AbortSignal ): Promise { const output = await execHostCommand( conn, hostPlatform, powerShellCommand( `if (Test-Path -LiteralPath ${powerShellLiteral(markerPath)} -PathType Leaf) { Get-Content -LiteralPath ${powerShellLiteral(markerPath)} -Raw -ErrorAction SilentlyContinue }` - ) - ).catch(() => '') + ), + { signal } + ).catch(() => { + signal?.throwIfAborted() + return '' + }) const sockPath = output.trim() if (!isWindowsRelayPipePath(sockPath)) { return null @@ -867,15 +1290,18 @@ async function rememberWindowsActiveRelayEndpoint( conn: SshConnection, hostPlatform: RemoteHostPlatform, markerPath: string, - sockPath: string + sockPath: string, + signal?: AbortSignal ): Promise { await execHostCommand( conn, hostPlatform, powerShellCommand( `Set-Content -LiteralPath ${powerShellLiteral(markerPath)} -Value ${powerShellLiteral(sockPath)} -NoNewline` - ) + ), + { signal } ).catch((err) => { + signal?.throwIfAborted() // Why: fallback pipe names are deterministic, so losing this marker does // not force the next deploy to orphan an undiscoverable relay. console.warn( @@ -901,17 +1327,19 @@ type WindowsRelayLaunchOptions = { async function launchWindowsRelay( conn: SshConnection, hostPlatform: RemoteHostPlatform, - opts: WindowsRelayLaunchOptions + opts: WindowsRelayLaunchOptions, + signal?: AbortSignal ): Promise<{ transport: MultiplexerTransport; nodePath: string; sockPath: string }> { let launchOpts = opts - if ((await probeWindowsRelayPipe(conn, hostPlatform, opts)) === 'READY') { + if ((await probeWindowsRelayPipe(conn, hostPlatform, opts, signal)) === 'READY') { try { - const transport = await connectWindowsRelay(conn, hostPlatform, opts) + const transport = await connectWindowsRelay(conn, hostPlatform, opts, signal) await rememberWindowsActiveRelayEndpoint( conn, hostPlatform, opts.activePipeMarkerPath, - opts.sockPath + opts.sockPath, + signal ) return { transport, @@ -919,6 +1347,7 @@ async function launchWindowsRelay( sockPath: opts.sockPath } } catch (err) { + signal?.throwIfAborted() console.warn( '[ssh-relay] Windows named pipe reconnect failed, launching fresh relay:', err instanceof Error ? err.message : String(err) @@ -936,15 +1365,16 @@ async function launchWindowsRelay( if ( launchOpts !== opts && - (await probeWindowsRelayPipe(conn, hostPlatform, launchOpts)) === 'READY' + (await probeWindowsRelayPipe(conn, hostPlatform, launchOpts, signal)) === 'READY' ) { try { - const transport = await connectWindowsRelay(conn, hostPlatform, launchOpts) + const transport = await connectWindowsRelay(conn, hostPlatform, launchOpts, signal) await rememberWindowsActiveRelayEndpoint( conn, hostPlatform, launchOpts.activePipeMarkerPath, - launchOpts.sockPath + launchOpts.sockPath, + signal ) return { transport, @@ -952,6 +1382,7 @@ async function launchWindowsRelay( sockPath: launchOpts.sockPath } } catch (err) { + signal?.throwIfAborted() console.warn( '[ssh-relay] Windows fallback pipe reconnect failed, relaunching relay:', err instanceof Error ? err.message : String(err) @@ -973,20 +1404,29 @@ async function launchWindowsRelay( launchOpts.graceTime, logFile, errFile - ) + ), + { signal } ) const POLL_INTERVAL_MS = 200 const POLL_TIMEOUT_MS = 10_000 if ( - await waitForWindowsRelayPipe(conn, hostPlatform, launchOpts, POLL_TIMEOUT_MS, POLL_INTERVAL_MS) + await waitForWindowsRelayPipe( + conn, + hostPlatform, + launchOpts, + POLL_TIMEOUT_MS, + POLL_INTERVAL_MS, + signal + ) ) { - const transport = await connectWindowsRelay(conn, hostPlatform, launchOpts) + const transport = await connectWindowsRelay(conn, hostPlatform, launchOpts, signal) await rememberWindowsActiveRelayEndpoint( conn, hostPlatform, launchOpts.activePipeMarkerPath, - launchOpts.sockPath + launchOpts.sockPath, + signal ) return { transport, @@ -998,8 +1438,12 @@ async function launchWindowsRelay( const logOutput = await execHostCommand( conn, hostPlatform, - windowsRelayTailLogCommand(logFile, errFile) - ).catch(() => '(could not read log)') + windowsRelayTailLogCommand(logFile, errFile), + { signal } + ).catch(() => { + signal?.throwIfAborted() + return '(could not read log)' + }) throw new Error(`Relay failed to start within ${POLL_TIMEOUT_MS / 1000}s. Log:\n${logOutput}`) } @@ -1010,13 +1454,14 @@ async function connectWindowsRelay( remoteDir: string nodePath: string sockPath: string - } + }, + signal?: AbortSignal ): Promise { const channel = await conn.exec( windowsRelayConnectCommand(hostPlatform, opts.nodePath, opts.remoteDir, opts.sockPath), - { wrapCommand: false } + { wrapCommand: false, signal } ) - return waitForSentinel(channel) + return waitForSentinel(channel, signal) } function windowsRelayConnectCommand( @@ -1083,12 +1528,14 @@ async function probeWindowsRelayPipe( remoteDir: string nodePath: string sockPath: string - } + }, + signal?: AbortSignal ): Promise<'READY' | 'WAITING'> { const result = await execHostCommand( conn, hostPlatform, - windowsRelayProbeCommand(hostPlatform, opts.nodePath, opts.remoteDir, opts.sockPath) + windowsRelayProbeCommand(hostPlatform, opts.nodePath, opts.remoteDir, opts.sockPath), + { signal } ) return result.trim() === 'READY' ? 'READY' : 'WAITING' } @@ -1102,7 +1549,8 @@ async function waitForWindowsRelayPipe( sockPath: string }, timeoutMs: number, - intervalMs: number + intervalMs: number, + signal?: AbortSignal ): Promise { try { const result = await execHostCommand( @@ -1111,10 +1559,12 @@ async function waitForWindowsRelayPipe( windowsRelayWaitCommand(hostPlatform, opts.nodePath, opts.remoteDir, opts.sockPath, { timeoutMs, intervalMs - }) + }), + { signal } ) return result.trim() === 'READY' } catch { + signal?.throwIfAborted() return false } } diff --git a/src/main/ssh/ssh-relay-exec-command.ts b/src/main/ssh/ssh-relay-exec-command.ts new file mode 100644 index 000000000..e4bfd8bc0 --- /dev/null +++ b/src/main/ssh/ssh-relay-exec-command.ts @@ -0,0 +1,156 @@ +import type { SshConnection } from './ssh-connection' +import { createSshOperationAbortError, type SshExecOptions } from './ssh-connection-utils' +import type { SystemSshCommandChannel } from './system-ssh-command' + +const EXEC_TIMEOUT_MS = 30_000 +const COMMAND_CLOSE_GRACE_MS = 5_000 +const MAX_EXEC_OUTPUT_CHARS = 1024 * 1024 + +type ExecCommandOptions = SshExecOptions & { + timeoutMs?: number +} + +type SshCommandTerminationError = Error & { + sshChannelCloseConfirmed: boolean +} + +export function isUnconfirmedSshCommandTermination( + error: unknown +): error is SshCommandTerminationError { + return ( + error instanceof Error && + (error as Partial).sshChannelCloseConfirmed === false + ) +} + +export async function execCommand( + conn: SshConnection, + command: string, + options?: ExecCommandOptions +): Promise { + const { timeoutMs = EXEC_TIMEOUT_MS, ...execOptions } = options ?? {} + const signal = options?.signal + if (signal?.aborted) { + throw createSshOperationAbortError() + } + // Why: reconnect/disconnect can flip the connection back to ssh2 before a + // killed local OpenSSH child emits close; the channel's transport is immutable. + const openedWithSystemSsh = conn.usesSystemSshTransport?.() === true + const channel = await conn.exec(command, execOptions) + return new Promise((resolve, reject) => { + let stdout = '' + let stderr = '' + let settled = false + let terminationError: SshCommandTerminationError | null = null + let closeGraceTimer: ReturnType | null = null + + const cleanup = (): void => { + clearTimeout(timeout) + if (closeGraceTimer) { + clearTimeout(closeGraceTimer) + } + signal?.removeEventListener('abort', onAbort) + channel.off('error', fail) + channel.stderr.off('error', fail) + channel.off('data', onStdoutData) + channel.stderr.off('data', onStderrData) + channel.off('close', onClose) + } + const settle = (fn: typeof resolve | typeof reject, val: string | Error): void => { + if (settled) { + return + } + settled = true + cleanup() + fn(val as never) + } + const guardUnconfirmedTeardown = (): void => { + const swallowLateError = (): void => {} + const cleanupGuards = (): void => { + channel.off('error', swallowLateError) + channel.stderr.off('error', swallowLateError) + channel.off('close', cleanupGuards) + } + channel.on('error', swallowLateError) + channel.stderr.on('error', swallowLateError) + channel.once('close', cleanupGuards) + // Why: an unconfirmed close can arrive after the caller's bounded wait; + // keep draining discarded streams so ssh2 can finish CHANNEL_CLOSE. + channel.resume() + channel.stderr.resume() + } + // Why: sshd counts the session against MaxSessions until CHANNEL_CLOSE + // completes. Settling on abort before the channel actually closes lets the + // concurrent-bootstrap sequential fallback reissue an exec while the slot + // is still held, so it gets refused again. Close and settle from onClose. + const requestTermination = (error: Error): void => { + if (terminationError) { + return + } + terminationError = Object.assign(error, { sshChannelCloseConfirmed: false }) + clearTimeout(timeout) + // Why: callers must not release an install lock while its remote npm + // process can still mutate node_modules. Prefer confirmed channel close, + // but bound a broken transport's teardown wait. + closeGraceTimer = setTimeout(() => { + guardUnconfirmedTeardown() + settle(reject, error) + }, COMMAND_CLOSE_GRACE_MS) + channel.close() + } + const fail = (err: Error): void => requestTermination(err) + const onAbort = (): void => requestTermination(createSshOperationAbortError()) + const onStdoutData = (data: Buffer): void => { + stdout = appendExecOutputTail(stdout, data.toString('utf-8')) + } + const onStderrData = (data: Buffer): void => { + stderr = appendExecOutputTail(stderr, data.toString('utf-8')) + } + const onClose = (code: number): void => { + if ( + !terminationError && + openedWithSystemSsh && + (channel as SystemSshCommandChannel)._closeRequested + ) { + terminationError = Object.assign(createSshOperationAbortError(), { + sshChannelCloseConfirmed: false + }) + } + if (terminationError) { + // Why: a system-SSH channel closes when the local OpenSSH child exits; + // that does not prove the remote command stopped, especially with a ControlMaster. + if (!openedWithSystemSsh) { + terminationError.sshChannelCloseConfirmed = true + } + settle(reject, terminationError) + } else if (code !== 0) { + // Why: on the system-ssh transport channel.stderr carries local OpenSSH + // client noise; preferring it masks the real failure in stdout (2>&1). + const output = [stderr.trim(), stdout.trim()].filter(Boolean).join('\n') + settle(reject, new Error(`Command "${command}" failed (exit ${code}): ${output}`)) + } else { + settle(resolve, stdout) + } + } + const timeout = setTimeout(() => { + requestTermination(new Error(`Command "${command}" timed out after ${timeoutMs / 1000}s`)) + }, timeoutMs) + + // 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() + } + }) +} + +function appendExecOutputTail(existing: string, chunk: string): string { + const combined = existing + chunk + return combined.length > MAX_EXEC_OUTPUT_CHARS ? combined.slice(-MAX_EXEC_OUTPUT_CHARS) : combined +} diff --git a/src/main/ssh/ssh-relay-gc-claim.ts b/src/main/ssh/ssh-relay-gc-claim.ts new file mode 100644 index 000000000..d0c8cc122 --- /dev/null +++ b/src/main/ssh/ssh-relay-gc-claim.ts @@ -0,0 +1,236 @@ +import { randomUUID } from 'node:crypto' +import type { SshConnection } from './ssh-connection' +import { shellEscape } from './ssh-connection-utils' +import { execCommand } from './ssh-relay-deploy-helpers' +import { + probeInstallLockExistsCommand, + tryCreateInstallLockCommand, + tryStealInstallLockCommand +} from './ssh-relay-install-lock-commands' +import { removeRemoteTreeCommand } from './ssh-remote-commands' +import { powerShellCommand, powerShellLiteral } from './ssh-remote-powershell' +import { + getRemoteHostPlatform, + isWindowsRemoteHost, + joinRemotePath, + type RemoteHostPlatform +} from './ssh-remote-platform' + +const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') +const RELAY_GC_CLAIM_SUFFIX = '.gc-claim' +const RELAY_GC_OWNER_NAME = '.gc-owner' +// Why: the claim guards only a bounded sibling rename or launch handoff, not +// npm or deletion. Ten minutes bounds crashes while exceeding either sequence. +const RELAY_GC_CLAIM_STALE_SECONDS = 10 * 60 + +function execHostCommand( + conn: SshConnection, + host: RemoteHostPlatform, + command: string, + signal?: AbortSignal +): Promise { + return execCommand(conn, command, { + wrapCommand: !isWindowsRemoteHost(host), + signal + }) +} + +export function relayGcClaimPath(remoteRelayDir: string): string { + return `${remoteRelayDir}${RELAY_GC_CLAIM_SUFFIX}` +} + +export async function isRelayGcClaimed( + conn: SshConnection, + remoteRelayDir: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + signal?: AbortSignal +): Promise { + const claimPath = relayGcClaimPath(remoteRelayDir) + const output = await execHostCommand( + conn, + host, + probeInstallLockExistsCommand(host, claimPath), + signal + ) + const markers = new Set( + output + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line === 'LOCKED' || line === 'OPEN') + ) + if (markers.size !== 1) { + throw new Error(`Inconclusive relay GC claim probe at ${claimPath}`) + } + return markers.has('LOCKED') +} + +export async function tryAcquireRelayGcClaim( + conn: SshConnection, + remoteRelayDir: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + signal?: AbortSignal +): Promise { + const claimPath = relayGcClaimPath(remoteRelayDir) + try { + const created = await execHostCommand( + conn, + host, + tryCreateInstallLockCommand(host, claimPath), + signal + ) + if (created.trim().endsWith('OK')) { + return writeRelayGcClaimOwner(conn, remoteRelayDir, host, signal) + } + const stolen = await execHostCommand( + conn, + host, + tryStealInstallLockCommand(host, claimPath, RELAY_GC_CLAIM_STALE_SECONDS), + signal + ) + if (!stolen.trim().endsWith('OK')) { + return null + } + return writeRelayGcClaimOwner(conn, remoteRelayDir, host, signal) + } catch { + signal?.throwIfAborted() + return null + } +} + +async function writeRelayGcClaimOwner( + conn: SshConnection, + remoteRelayDir: string, + host: RemoteHostPlatform, + signal?: AbortSignal +): Promise { + const token = `${process.pid}-${Date.now()}-${randomUUID()}` + const claimPath = relayGcClaimPath(remoteRelayDir) + const ownerPath = joinRemotePath(host, claimPath, RELAY_GC_OWNER_NAME) + const command = isWindowsRemoteHost(host) + ? powerShellCommand( + `Set-Content -LiteralPath ${powerShellLiteral(ownerPath)} -Value ${powerShellLiteral(token)} -NoNewline -ErrorAction Stop` + ) + : `printf %s ${shellEscape(token)} > ${shellEscape(ownerPath)}` + try { + await execHostCommand(conn, host, command, signal) + return token + } catch { + // Why: the write may have succeeded remotely before SSH lost its reply. + // A conditional release removes only the claim generation with our token. + await releaseRelayGcClaim(conn, remoteRelayDir, token, host) + return null + } +} + +export async function isRelayGcClaimOwned( + conn: SshConnection, + remoteRelayDir: string, + token: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST +): Promise { + const ownerPath = joinRemotePath(host, relayGcClaimPath(remoteRelayDir), RELAY_GC_OWNER_NAME) + const command = isWindowsRemoteHost(host) + ? powerShellCommand( + `if ((Get-Content -LiteralPath ${powerShellLiteral(ownerPath)} -Raw -ErrorAction SilentlyContinue) -ceq ${powerShellLiteral(token)}) { 'OWNED' } else { 'LOST' }` + ) + : `test "$(cat ${shellEscape(ownerPath)} 2>/dev/null)" = ${shellEscape(token)} && echo OWNED || echo LOST` + const output = await execHostCommand(conn, host, command).catch(() => 'LOST') + return output.trim() === 'OWNED' +} + +export type RelayGcClaimReleaseResult = 'released' | 'lost' | 'unknown' + +export async function releaseRelayGcClaim( + conn: SshConnection, + remoteRelayDir: string, + token: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST +): Promise { + const claimPath = relayGcClaimPath(remoteRelayDir) + const ownerPath = joinRemotePath(host, claimPath, RELAY_GC_OWNER_NAME) + const command = isWindowsRemoteHost(host) + ? powerShellCommand( + `$claim = ${powerShellLiteral(claimPath)}; ` + + `if (-not (Test-Path -LiteralPath $claim)) { 'RELEASED' } ` + + `elseif ((Get-Content -LiteralPath ${powerShellLiteral(ownerPath)} -Raw -ErrorAction SilentlyContinue) -cne ${powerShellLiteral(token)}) { 'LOST' } ` + + 'else { try { Remove-Item -LiteralPath $claim -Recurse -Force -ErrorAction Stop } catch {}; ' + + "if (Test-Path -LiteralPath $claim) { 'UNKNOWN' } else { 'RELEASED' } }" + ) + : [ + `if ! test -e ${shellEscape(claimPath)}; then echo RELEASED;`, + `elif test "$(cat ${shellEscape(ownerPath)} 2>/dev/null)" != ${shellEscape(token)}; then echo LOST;`, + `else ${removeRemoteTreeCommand(host, claimPath)} 2>/dev/null;`, + `if test -e ${shellEscape(claimPath)}; then echo UNKNOWN; else echo RELEASED; fi; fi` + ].join(' ') + const output = await execHostCommand(conn, host, command).catch(() => 'UNKNOWN') + switch (output.trim()) { + case 'RELEASED': + return 'released' + case 'LOST': + return 'lost' + default: + return 'unknown' + } +} + +export async function releaseRelayGcClaimWithRetry( + conn: SshConnection, + remoteRelayDir: string, + token: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST +): Promise { + let result: RelayGcClaimReleaseResult = 'unknown' + for (let attempt = 0; attempt < 3 && result === 'unknown'; attempt++) { + result = await releaseRelayGcClaim(conn, remoteRelayDir, token, host) + } + return result +} + +export async function waitForRelayGcClaimRelease( + conn: SshConnection, + remoteRelayDir: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + signal?: AbortSignal +): Promise { + while (true) { + const claimed = await isRelayGcClaimed(conn, remoteRelayDir, host, signal).catch(() => true) + signal?.throwIfAborted() + if (!claimed) { + return + } + const claimPath = relayGcClaimPath(remoteRelayDir) + const recovered = await execHostCommand( + conn, + host, + tryStealInstallLockCommand(host, claimPath, RELAY_GC_CLAIM_STALE_SECONDS), + signal + ).catch(() => 'BUSY') + signal?.throwIfAborted() + if (recovered.trim().endsWith('OK')) { + const token = await writeRelayGcClaimOwner(conn, remoteRelayDir, host, signal) + if (token) { + const release = await releaseRelayGcClaim(conn, remoteRelayDir, token, host) + if (release === 'released') { + return + } + } + // A lost or uncertain release can mean a successor owns the stable path. + // Probe again instead of treating recovery as complete. + continue + } + await new Promise((resolve, reject) => { + const onAbort = (): void => { + clearTimeout(timer) + reject(signal?.reason) + } + const timer = setTimeout(() => { + signal?.removeEventListener('abort', onAbort) + resolve() + }, 1_000) + signal?.addEventListener('abort', onAbort, { once: true }) + if (signal?.aborted) { + onAbort() + } + }) + } +} diff --git a/src/main/ssh/ssh-relay-gc-retry.test.ts b/src/main/ssh/ssh-relay-gc-retry.test.ts new file mode 100644 index 000000000..fee4946d3 --- /dev/null +++ b/src/main/ssh/ssh-relay-gc-retry.test.ts @@ -0,0 +1,275 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ app: { getAppPath: () => '/mock/app' } })) +vi.mock('fs', () => ({ + existsSync: vi.fn().mockReturnValue(true), + readFileSync: vi.fn().mockReturnValue('0.1.0+gc-retry') +})) +vi.mock('./relay-protocol', () => ({ + RELAY_VERSION: '0.1.0', + RELAY_REMOTE_DIR: '.orca-remote', + parseUnameToRelayPlatform: vi.fn().mockReturnValue('linux-x64'), + RELAY_SENTINEL: 'ORCA-RELAY v0.1.0 READY\n', + RELAY_SENTINEL_TIMEOUT_MS: 10_000 +})) +vi.mock('./ssh-relay-deploy-helpers', () => ({ + uploadDirectory: vi.fn(), + waitForSentinel: vi.fn().mockResolvedValue({ + write: vi.fn(), + onData: vi.fn(), + onClose: vi.fn() + }), + isUnconfirmedSshCommandTermination: () => false, + execCommand: vi.fn() +})) +vi.mock('./ssh-remote-node-resolution', () => ({ + resolveRemoteNodePath: vi.fn().mockResolvedValue('/usr/bin/node') +})) +vi.mock('./ssh-relay-versioned-install', () => ({ + readLocalFullVersion: vi.fn().mockReturnValue('0.1.0+gc-retry'), + computeRemoteRelayDir: (home: string, version: string) => `${home}/.orca-remote/relay-${version}`, + isRelayAlreadyInstalled: vi.fn().mockResolvedValue(true), + finalizeInstall: vi.fn(), + abandonInstall: vi.fn(), + gcOldRelayVersions: vi.fn().mockResolvedValue(undefined) +})) +vi.mock('./ssh-relay-install-lock', () => ({ acquireInstallLock: vi.fn() })) +vi.mock('./ssh-relay-repair-lock', () => ({ + tryAcquireRelayRepairLock: vi.fn().mockResolvedValue('acquired') +})) +vi.mock('./ssh-relay-gc-claim', () => ({ + releaseRelayGcClaimWithRetry: vi.fn().mockResolvedValue('released'), + tryAcquireRelayGcClaim: vi.fn().mockResolvedValue('launch-token'), + waitForRelayGcClaimRelease: vi.fn().mockResolvedValue(undefined) +})) +vi.mock('./ssh-connection-utils', () => ({ + shellEscape: (value: string) => `'${value}'`, + createSshOperationAbortError: () => Object.assign(new Error('cancelled'), { name: 'AbortError' }) +})) + +import { deployAndLaunchRelay } from './ssh-relay-deploy' +import { execCommand, waitForSentinel } from './ssh-relay-deploy-helpers' +import { + releaseRelayGcClaimWithRetry, + tryAcquireRelayGcClaim, + waitForRelayGcClaimRelease +} from './ssh-relay-gc-claim' +import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' +import { abandonInstall, isRelayAlreadyInstalled } from './ssh-relay-versioned-install' +import type { SshConnection } from './ssh-connection' + +function makeConnection(): SshConnection { + const channel = { + on: vi.fn(), + stderr: { on: vi.fn() }, + close: vi.fn() + } + return { + canRunConcurrentExecCommands: vi.fn().mockReturnValue(true), + exec: vi.fn().mockResolvedValue(channel) + } as unknown as SshConnection +} + +describe('relay GC deploy retry', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('recomputes install state when GC wins before a healthy relay launch', async () => { + const conn = makeConnection() + vi.mocked(tryAcquireRelayRepairLock).mockResolvedValueOnce('gc') + vi.mocked(execCommand).mockImplementation(async (_conn, command) => { + if (command.includes('__ORCA_REMOTE_PLATFORM__')) { + return '__ORCA_REMOTE_PLATFORM__ Linux x86_64' + } + if (command === 'echo $HOME') { + return '/home/user' + } + if (command.includes('node-pty')) { + return 'ORCA-NATIVE-DEPS-OK' + } + if (command.includes('var s=require("net").connect')) { + return 'READY' + } + if (command.includes('test -S')) { + return 'DEAD' + } + return '' + }) + + await deployAndLaunchRelay(conn) + + expect(waitForRelayGcClaimRelease).toHaveBeenCalledTimes(1) + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(3) + expect(conn.exec).toHaveBeenCalledTimes(2) + }) + + it('recomputes state when GC finishes between the install probe and lock acquisition', async () => { + const conn = makeConnection() + vi.mocked(isRelayAlreadyInstalled) + .mockResolvedValueOnce(true) + .mockResolvedValueOnce(false) + .mockResolvedValue(true) + vi.mocked(execCommand).mockImplementation(async (_conn, command) => { + if (command.includes('__ORCA_REMOTE_PLATFORM__')) { + return '__ORCA_REMOTE_PLATFORM__ Linux x86_64' + } + if (command === 'echo $HOME') { + return '/home/user' + } + if (command.includes('node-pty')) { + return 'ORCA-NATIVE-DEPS-OK' + } + if (command.includes('var s=require("net").connect')) { + return 'READY' + } + if (command.includes('test -S')) { + return 'DEAD' + } + return '' + }) + + await deployAndLaunchRelay(conn) + + expect(abandonInstall).toHaveBeenCalledTimes(2) + expect(waitForRelayGcClaimRelease).toHaveBeenCalledTimes(1) + expect(isRelayAlreadyInstalled).toHaveBeenCalledTimes(4) + expect(tryAcquireRelayRepairLock).toHaveBeenCalledTimes(2) + }) + + it('owns a GC claim through launch when the install-lock state is indeterminate', async () => { + const conn = makeConnection() + vi.mocked(tryAcquireRelayRepairLock).mockResolvedValueOnce('error') + let resolveReady: ((value: Awaited>) => void) | undefined + vi.mocked(waitForSentinel).mockImplementationOnce( + async () => + new Promise>>((resolve) => { + resolveReady = resolve + }) + ) + vi.mocked(execCommand).mockImplementation(async (_conn, command) => { + if (command.includes('__ORCA_REMOTE_PLATFORM__')) { + return '__ORCA_REMOTE_PLATFORM__ Linux x86_64' + } + if (command === 'echo $HOME') { + return '/home/user' + } + if (command.includes('node-pty')) { + return 'ORCA-NATIVE-DEPS-OK' + } + if (command.includes('var s=require("net").connect')) { + return 'READY' + } + if (command.includes('test -S')) { + return 'DEAD' + } + return '' + }) + + const deploy = deployAndLaunchRelay(conn) + await vi.waitFor(() => expect(resolveReady).toBeDefined()) + expect(releaseRelayGcClaimWithRetry).not.toHaveBeenCalled() + resolveReady?.({ + write: vi.fn(), + onData: vi.fn(), + onClose: vi.fn() + }) + await deploy + + expect(waitForRelayGcClaimRelease).not.toHaveBeenCalled() + expect(tryAcquireRelayRepairLock).toHaveBeenCalledTimes(1) + expect(tryAcquireRelayGcClaim).toHaveBeenCalledTimes(1) + expect(releaseRelayGcClaimWithRetry).toHaveBeenCalledTimes(1) + expect(conn.exec).toHaveBeenCalledTimes(2) + }) + + it('retains an owned install lock when launch never becomes live', async () => { + const conn = makeConnection() + vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('launch failed')) + vi.mocked(execCommand).mockImplementation(async (_conn, command) => { + if (command.includes('__ORCA_REMOTE_PLATFORM__')) { + return '__ORCA_REMOTE_PLATFORM__ Linux x86_64' + } + if (command === 'echo $HOME') { + return '/home/user' + } + if (command.includes('node-pty')) { + return 'ORCA-NATIVE-DEPS-OK' + } + if (command.includes('var s=require("net").connect')) { + return 'READY' + } + if (command.includes('test -S')) { + return 'DEAD' + } + return '' + }) + + await expect(deployAndLaunchRelay(conn)).rejects.toThrow('launch failed') + + expect(abandonInstall).not.toHaveBeenCalled() + expect(releaseRelayGcClaimWithRetry).not.toHaveBeenCalled() + }) + + it('retains an owned launch claim when launch never becomes live', async () => { + const conn = makeConnection() + vi.mocked(tryAcquireRelayRepairLock).mockResolvedValueOnce('busy') + vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('launch failed')) + vi.mocked(execCommand).mockImplementation(async (_conn, command) => { + if (command.includes('__ORCA_REMOTE_PLATFORM__')) { + return '__ORCA_REMOTE_PLATFORM__ Linux x86_64' + } + if (command === 'echo $HOME') { + return '/home/user' + } + if (command.includes('node-pty')) { + return 'ORCA-NATIVE-DEPS-OK' + } + if (command.includes('var s=require("net").connect')) { + return 'READY' + } + if (command.includes('test -S')) { + return 'DEAD' + } + return '' + }) + + await expect(deployAndLaunchRelay(conn)).rejects.toThrow('launch failed') + + expect(abandonInstall).not.toHaveBeenCalled() + expect(releaseRelayGcClaimWithRetry).not.toHaveBeenCalled() + }) + + it('keeps retrying repeated launch-claim contention within the deploy bound', async () => { + const conn = makeConnection() + vi.mocked(tryAcquireRelayRepairLock).mockResolvedValue('busy') + vi.mocked(tryAcquireRelayGcClaim) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValue('launch-token') + vi.mocked(execCommand).mockImplementation(async (_conn, command) => { + if (command.includes('__ORCA_REMOTE_PLATFORM__')) { + return '__ORCA_REMOTE_PLATFORM__ Linux x86_64' + } + if (command === 'echo $HOME') { + return '/home/user' + } + if (command.includes('node-pty')) { + return 'ORCA-NATIVE-DEPS-OK' + } + if (command.includes('var s=require("net").connect')) { + return 'READY' + } + if (command.includes('test -S')) { + return 'DEAD' + } + return '' + }) + + await deployAndLaunchRelay(conn) + + expect(waitForRelayGcClaimRelease).toHaveBeenCalledTimes(2) + expect(tryAcquireRelayGcClaim).toHaveBeenCalledTimes(3) + expect(releaseRelayGcClaimWithRetry).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/ssh/ssh-relay-gc-tombstone.ts b/src/main/ssh/ssh-relay-gc-tombstone.ts new file mode 100644 index 000000000..95684d4d2 --- /dev/null +++ b/src/main/ssh/ssh-relay-gc-tombstone.ts @@ -0,0 +1,33 @@ +import type { SshConnection } from './ssh-connection' +import { execCommand } from './ssh-relay-deploy-helpers' +import { removeRemoteTreeCommand } from './ssh-remote-commands' +import { + getRemoteHostPlatform, + isWindowsRemoteHost, + joinRemotePath, + type RemoteHostPlatform +} from './ssh-remote-platform' + +const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') +const RELAY_GC_TOMBSTONE_REGEX = + /^relay-(?:v?\d+\.\d+\.\d+(?:\+[0-9a-f]+)?)\.gc-tombstone\.\d+\.\d+$/ + +export async function cleanupRelayGcTombstones( + conn: SshConnection, + baseDir: string, + entries: string[], + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST +): Promise { + // Why: a confirmed rename isolates these paths from any recreated install. + // Strictly named leftovers are safe to retry after an interrupted GC pass. + for (const name of entries.filter((entry) => RELAY_GC_TOMBSTONE_REGEX.test(entry))) { + const tombstone = joinRemotePath(host, baseDir, name) + await execCommand(conn, removeRemoteTreeCommand(host, tombstone), { + wrapCommand: !isWindowsRemoteHost(host) + }).catch((err) => { + console.warn( + `[ssh-relay] GC failed to remove tombstone ${tombstone}: ${err instanceof Error ? err.message : String(err)}` + ) + }) + } +} diff --git a/src/main/ssh/ssh-relay-install-lock-commands.ts b/src/main/ssh/ssh-relay-install-lock-commands.ts new file mode 100644 index 000000000..d6eab561a --- /dev/null +++ b/src/main/ssh/ssh-relay-install-lock-commands.ts @@ -0,0 +1,176 @@ +import { shellEscape } from './ssh-connection-utils' +import { powerShellCommand, powerShellLiteral } from './ssh-remote-powershell' +import { isWindowsRemoteHost, type RemoteHostPlatform } from './ssh-remote-platform' + +export function acquireInstallLockParentCommand( + host: RemoteHostPlatform, + remoteRelayDir: string +): string { + if (!isWindowsRemoteHost(host)) { + return `mkdir -p ${shellEscape(remoteRelayDir)}` + } + return powerShellCommand( + `$null = New-Item -ItemType Directory -Force -Path ${powerShellLiteral(remoteRelayDir)}` + ) +} + +export function tryCreateInstallLockCommand(host: RemoteHostPlatform, lockDir: string): string { + if (!isWindowsRemoteHost(host)) { + return `mkdir ${shellEscape(lockDir)} 2>&1 && echo OK || echo BUSY` + } + // Why: old Orca clients recognize only a directory at `.install-lock`, while + // concurrent New-Item calls can both report success in PowerShell 5.1. Keep + // that directory marker and arbitrate ownership with an atomic child file. + return powerShellCommand( + [ + `$lock = ${powerShellLiteral(lockDir)}`, + '$stream = $null', + 'try {', + "if (Test-Path -LiteralPath $lock) { 'BUSY' } else {", + '$null = New-Item -ItemType Directory -Path $lock -ErrorAction Stop', + "$owner = Join-Path $lock '.owner'", + '$stream = [System.IO.File]::Open($owner, [System.IO.FileMode]::CreateNew, [System.IO.FileAccess]::Write, [System.IO.FileShare]::None)', + "'OK'", + '}', + `} catch { 'BUSY' } finally { if ($null -ne $stream) { $stream.Dispose() } }` + ].join('; ') + ) +} + +export function probeInstallLockExistsCommand(host: RemoteHostPlatform, lockPath: string): string { + if (!isWindowsRemoteHost(host)) { + return `test -e ${shellEscape(lockPath)} && echo LOCKED || echo OPEN` + } + // Why: one prerelease briefly wrote file locks; accept both shapes so those + // hosts remain recoverable after upgrading to directory-plus-owner locks. + return powerShellCommand( + `if (Test-Path -LiteralPath ${powerShellLiteral(lockPath)}) { 'LOCKED' } else { 'OPEN' }` + ) +} + +export function lockAgeSecondsCommand(host: RemoteHostPlatform, lockDir: string): string { + if (!isWindowsRemoteHost(host)) { + return `${posixLockAgeSecondsAssignment(lockDir)} && echo "$age" || echo` + } + return powerShellCommand( + [ + `$item = Get-Item -LiteralPath ${powerShellLiteral(lockDir)} -ErrorAction Stop`, + '$mtime = ([DateTimeOffset]$item.LastWriteTimeUtc).ToUnixTimeSeconds()', + '$now = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds()', + 'Write-Output ($now - $mtime)' + ].join('; ') + ) +} + +export function tryStealInstallLockCommand( + host: RemoteHostPlatform, + lockDir: string, + staleAfterSeconds: number +): string { + if (!isWindowsRemoteHost(host)) { + return posixStealInstallLockCommand(lockDir, staleAfterSeconds) + } + return windowsStealInstallLockCommand(lockDir, staleAfterSeconds) +} + +function posixStealInstallLockCommand(lockDir: string, staleAfterSeconds: number): string { + const escapedLockDir = shellEscape(lockDir) + const escapedStealLockPrefix = shellEscape(`${lockDir}.steal`) + return [ + `${posixLockIdentityAssignment(lockDir, 'lock_key')} && mtime=\${lock_key%%:*} && now=$(date +%s) && age=$((now - mtime)) || age=0;`, + `if [ "\${age:-0}" -le ${staleAfterSeconds} ] 2>/dev/null; then echo BUSY; else`, + `steal_root=${escapedStealLockPrefix};`, + 'steal_generation=0;', + 'steal="$steal_root.$steal_generation";', + 'owns_steal=0;', + 'lock_tombstone=;', + 'while [ "$owns_steal" != 1 ]; do', + 'if mkdir "$steal" 2>/dev/null; then owns_steal=1; break; fi;', + `steal_mtime=$(stat -c %Y "$steal" 2>/dev/null || stat -f %m "$steal" 2>/dev/null) && steal_now=$(date +%s) && steal_age=$((steal_now - steal_mtime)) || break;`, + 'if [ "${steal_age:-0}" -le 120 ] 2>/dev/null; then break; fi;', + 'steal_generation=$((steal_generation + 1));', + 'steal="$steal_root.$steal_generation";', + 'done;', + 'if [ "$owns_steal" = 1 ]; then', + `trap 'rm -rf "$steal_root".* 2>/dev/null || true; rm -rf "$lock_tombstone" 2>/dev/null || true' EXIT;`, + `${posixLockIdentityAssignment(lockDir, 'current_key')} && current_mtime=\${current_key%%:*} && current_now=$(date +%s) && current_age=$((current_now - current_mtime)) || current_age=0;`, + `if [ "$current_key" = "$lock_key" ] && [ "\${current_age:-0}" -gt ${staleAfterSeconds} ] 2>/dev/null; then`, + `lock_tombstone=${escapedLockDir}.tombstone.$$.$(date +%s);`, + `if [ ! -e "$lock_tombstone" ] && mv ${escapedLockDir} "$lock_tombstone" 2>/dev/null; then mkdir ${escapedLockDir} 2>&1 && echo OK || echo BUSY; else echo BUSY; fi;`, + 'else echo BUSY; fi;', + 'else echo BUSY; fi; fi' + ].join(' ') +} + +function windowsStealInstallLockCommand(lockDir: string, staleAfterSeconds: number): string { + return powerShellCommand( + [ + `$lock = ${powerShellLiteral(lockDir)}`, + 'try {', + '$item = Get-Item -LiteralPath $lock -ErrorAction Stop', + '$mtime = ([DateTimeOffset]$item.LastWriteTimeUtc).ToUnixTimeSeconds()', + '$lockIdentity = "${mtime}:$($item.CreationTimeUtc.Ticks)"', + '$now = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds()', + `if (($now - $mtime) -le ${staleAfterSeconds}) { 'BUSY' } else {`, + '$stealRoot = "$lock.steal"', + '$stealGeneration = 0', + '$steal = "$stealRoot.$stealGeneration"', + '$ownsSteal = $false', + '$lockTombstone = $null', + 'try {', + 'while (-not $ownsSteal) {', + 'try {', + '$stealStream = [System.IO.File]::Open($steal, [System.IO.FileMode]::CreateNew, [System.IO.FileAccess]::Write, [System.IO.FileShare]::None)', + '$stealStream.Dispose()', + '$ownsSteal = $true', + 'break', + '} catch {', + 'try {', + '$stealItem = Get-Item -LiteralPath $steal -ErrorAction Stop', + '$stealMtime = ([DateTimeOffset]$stealItem.LastWriteTimeUtc).ToUnixTimeSeconds()', + '$stealAge = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds() - $stealMtime', + 'if ($stealAge -le 120) { break }', + '$stealGeneration++', + '$steal = "$stealRoot.$stealGeneration"', + '} catch { break }', + '}', + '}', + "if (-not $ownsSteal) { 'BUSY' } else {", + '$current = Get-Item -LiteralPath $lock -ErrorAction Stop', + '$currentMtime = ([DateTimeOffset]$current.LastWriteTimeUtc).ToUnixTimeSeconds()', + '$currentIdentity = "${currentMtime}:$($current.CreationTimeUtc.Ticks)"', + '$currentNow = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds()', + `if (($currentIdentity -eq $lockIdentity) -and (($currentNow - $currentMtime) -gt ${staleAfterSeconds})) {`, + '$lockTombstone = "$lock.tombstone.$PID.$([DateTimeOffset]::UtcNow.ToUnixTimeMilliseconds())"', + 'Move-Item -LiteralPath $lock -Destination $lockTombstone -ErrorAction Stop', + '$successorStream = $null', + "try { $null = New-Item -ItemType Directory -Path $lock -ErrorAction Stop; $successorOwner = Join-Path $lock '.owner'; $successorStream = [System.IO.File]::Open($successorOwner, [System.IO.FileMode]::CreateNew, [System.IO.FileAccess]::Write, [System.IO.FileShare]::None); 'OK' } catch { 'BUSY' } finally { if ($null -ne $successorStream) { $successorStream.Dispose() } }", + "} else { 'BUSY' }", + '}', + "} catch { 'BUSY' } finally {", + 'if ($ownsSteal) {', + '$stealParent = Split-Path -Parent $stealRoot', + '$stealLeaf = Split-Path -Leaf $stealRoot', + 'Get-ChildItem -LiteralPath $stealParent -Force -ErrorAction SilentlyContinue | Where-Object { $_.Name.StartsWith($stealLeaf + ".", [StringComparison]::Ordinal) } | Remove-Item -Recurse -Force -ErrorAction SilentlyContinue', + '}', + 'if ($null -ne $lockTombstone) { Remove-Item -LiteralPath $lockTombstone -Recurse -Force -ErrorAction SilentlyContinue }', + '}', + '}', + "} catch { 'BUSY' }" + ].join('; ') + ) +} + +function posixLockAgeSecondsAssignment(lockDir: string): string { + return `${posixLockMtimeSecondsAssignment(lockDir, 'mtime')} && now=$(date +%s) && age=$((now - mtime))` +} + +function posixLockIdentityAssignment(lockDir: string, variableName: string): string { + const escapedLockDir = shellEscape(lockDir) + return `${variableName}=$(stat -c %Y:%i ${escapedLockDir} 2>/dev/null || stat -f %m:%i ${escapedLockDir} 2>/dev/null)` +} + +function posixLockMtimeSecondsAssignment(lockDir: string, variableName: string): string { + const escapedLockDir = shellEscape(lockDir) + return `${variableName}=$(stat -c %Y ${escapedLockDir} 2>/dev/null || stat -f %m ${escapedLockDir} 2>/dev/null)` +} diff --git a/src/main/ssh/ssh-relay-install-lock.ts b/src/main/ssh/ssh-relay-install-lock.ts new file mode 100644 index 000000000..7cfd561b1 --- /dev/null +++ b/src/main/ssh/ssh-relay-install-lock.ts @@ -0,0 +1,162 @@ +import type { SshConnection } from './ssh-connection' +import { execCommand } from './ssh-relay-deploy-helpers' +import { RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' +import { isRelayGcClaimed, waitForRelayGcClaimRelease } from './ssh-relay-gc-claim' +import { + acquireInstallLockParentCommand, + lockAgeSecondsCommand, + tryCreateInstallLockCommand, + tryStealInstallLockCommand +} from './ssh-relay-install-lock-commands' +import { + getRemoteHostPlatform, + joinRemotePath, + type RemoteHostPlatform +} from './ssh-remote-platform' +import { removeRemoteTreeCommand } from './ssh-remote-commands' + +export const RELAY_INSTALL_LOCK_NAME = '.install-lock' + +const INSTALL_LOCK_POLL_MS = 1_000 +// Why: a fresh lock can cross the stale threshold during our bounded wait. +// Recheck infrequently so it becomes recoverable without adding an exec per poll. +const INSTALL_LOCK_STALE_RECHECK_MS = 60_000 +// Why: the lock holder can legitimately use the full deploy bound for upload, +// native install/rebuild, probes, and finalization. A concurrent first install +// must not fail earlier; completed-relay repair uses a separate one-shot path. +const INSTALL_LOCK_TIMEOUT_MS = RELAY_DEPLOY_TIMEOUT_MS +// Why: native-deps repair can keep running after the deploy backstop wins its +// Promise.race, so stale takeover must leave room for that bounded work. +export const INSTALL_LOCK_STALE_MS = 20 * 60_000 +export const INSTALL_LOCK_STALE_SECONDS = INSTALL_LOCK_STALE_MS / 1000 +const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') + +function execHostCommand( + conn: SshConnection, + host: RemoteHostPlatform, + command: string, + options?: { signal?: AbortSignal } +): Promise { + return execCommand(conn, command, { + wrapCommand: host.commandDialect !== 'powershell', + signal: options?.signal + }) +} + +export async function isRelayInstallLockStale( + conn: SshConnection, + lockDir: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST +): Promise { + try { + // Why: remote time avoids clock skew between Orca clients making a live + // repair lock look old enough for GC or another installer to recover. + const out = await execHostCommand(conn, host, lockAgeSecondsCommand(host, lockDir)) + const ageSec = Number.parseInt(out.trim(), 10) + return Number.isFinite(ageSec) && ageSec >= 0 && ageSec * 1000 > INSTALL_LOCK_STALE_MS + } catch { + return false + } +} + +/** + * Acquire the per-version install lock via a host-native exclusive create. + * Why: POSIX mkdir and a Windows directory plus atomic owner file each give + * one winner while keeping the marker visible to older Windows Orca clients. + */ +export async function acquireInstallLock( + conn: SshConnection, + remoteRelayDir: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + options?: { signal?: AbortSignal } +): Promise { + const lockDir = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) + + const start = Date.now() + let lastStaleCheckAt = Number.NEGATIVE_INFINITY + while (true) { + // Why: a crashed GC can leave the stable sibling claim behind. The shared + // waiter recovers stale claims instead of polling that orphan forever. + await waitForRelayGcClaimRelease(conn, remoteRelayDir, host, options?.signal) + options?.signal?.throwIfAborted() + await execHostCommand(conn, host, acquireInstallLockParentCommand(host, remoteRelayDir), { + signal: options?.signal + }) + try { + const result = await execHostCommand(conn, host, tryCreateInstallLockCommand(host, lockDir), { + signal: options?.signal + }) + if (result.trim().endsWith('OK')) { + // Why: GC may claim the sibling path between our first probe and lock + // creation. Recheck while holding the in-tree lock; one side backs off. + const claimedAfterAcquire = await isRelayGcClaimed( + conn, + remoteRelayDir, + host, + options?.signal + ).catch(() => true) + if (!claimedAfterAcquire && !options?.signal?.aborted) { + return + } + await execHostCommand(conn, host, removeRemoteTreeCommand(host, lockDir)).catch(() => {}) + options?.signal?.throwIfAborted() + } + } catch { + options?.signal?.throwIfAborted() + // A failed mkdir is lock contention; keep the connection-specific error + // out of the user path until the bounded wait expires. + } + if (Date.now() - lastStaleCheckAt >= INSTALL_LOCK_STALE_RECHECK_MS) { + lastStaleCheckAt = Date.now() + // Why: recover an already-stale lock immediately, then keep checking in + // case a fresh holder crosses the stale threshold while we are waiting. + const steal = await execHostCommand( + conn, + host, + tryStealInstallLockCommand(host, lockDir, INSTALL_LOCK_STALE_SECONDS), + { signal: options?.signal } + ).catch(() => 'BUSY') + options?.signal?.throwIfAborted() + if (steal.trim().endsWith('OK')) { + console.warn(`[ssh-relay] Stealing stale install lock at ${lockDir}`) + const claimedAfterSteal = await isRelayGcClaimed( + conn, + remoteRelayDir, + host, + options?.signal + ).catch(() => true) + if (!claimedAfterSteal && !options?.signal?.aborted) { + return + } + await execHostCommand(conn, host, removeRemoteTreeCommand(host, lockDir)).catch(() => {}) + options?.signal?.throwIfAborted() + } + } + if (Date.now() - start >= INSTALL_LOCK_TIMEOUT_MS) { + throw new Error( + `Could not acquire relay install lock at ${lockDir} after ${ + INSTALL_LOCK_TIMEOUT_MS / 1000 + }s; another install is still in progress.` + ) + } + await waitForInstallLockPoll(options?.signal) + } +} + +function waitForInstallLockPoll(signal?: AbortSignal): Promise { + return new Promise((resolve, reject) => { + const handleAbort = (): void => { + clearTimeout(timeout) + signal?.removeEventListener('abort', handleAbort) + reject(signal?.reason) + } + const timeout = setTimeout(() => { + signal?.removeEventListener('abort', handleAbort) + resolve() + }, INSTALL_LOCK_POLL_MS) + signal?.addEventListener('abort', handleAbort, { once: true }) + if (signal?.aborted) { + handleAbort() + } + }) +} 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 4d2f92eb9..f38a2b106 100644 --- a/src/main/ssh/ssh-relay-native-deps-install.test.ts +++ b/src/main/ssh/ssh-relay-native-deps-install.test.ts @@ -1,6 +1,6 @@ // Why: regression coverage for the install-probe contract. The original // "node-pty is not available" bug shipped because every layer that should -// have caught it (chained shell, swallowing catch, dir-only probe) was +// have caught it (chained shell, swallowing catch, resolve-only probe) was // silent. Tests below pin the parts that, individually, would have caught // it. @@ -30,6 +30,9 @@ vi.mock('./ssh-relay-deploy-helpers', () => ({ onData: vi.fn(), onClose: vi.fn() }), + isUnconfirmedSshCommandTermination: (error: unknown) => + error instanceof Error && + (error as Error & { sshChannelCloseConfirmed?: boolean }).sshChannelCloseConfirmed === false, execCommand: vi.fn() })) @@ -41,26 +44,41 @@ vi.mock('./ssh-relay-versioned-install', () => ({ readLocalFullVersion: vi.fn().mockReturnValue('0.1.0+testhash'), computeRemoteRelayDir: (home: string, v: string) => `${home}/.orca-remote/relay-${v}`, isRelayAlreadyInstalled: vi.fn().mockResolvedValue(false), - acquireInstallLock: vi.fn().mockResolvedValue(undefined), finalizeInstall: vi.fn().mockResolvedValue(undefined), abandonInstall: vi.fn().mockResolvedValue(undefined), gcOldRelayVersions: vi.fn().mockResolvedValue(undefined) })) +vi.mock('./ssh-relay-install-lock', () => ({ + acquireInstallLock: vi.fn().mockResolvedValue(undefined) +})) + +vi.mock('./ssh-relay-repair-lock', () => ({ + tryAcquireRelayRepairLock: vi.fn().mockResolvedValue('acquired') +})) + +vi.mock('./ssh-relay-gc-claim', () => ({ + releaseRelayGcClaimWithRetry: vi.fn().mockResolvedValue('released'), + tryAcquireRelayGcClaim: vi.fn().mockResolvedValue('launch-token'), + waitForRelayGcClaimRelease: vi.fn().mockResolvedValue(undefined) +})) + vi.mock('./ssh-connection-utils', () => ({ shellEscape: (s: string) => `'${s}'` })) import { deployAndLaunchRelay } from './ssh-relay-deploy' -import { execCommand } from './ssh-relay-deploy-helpers' +import { execCommand, uploadDirectory } from './ssh-relay-deploy-helpers' +import { RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' import { parseUnameToRelayPlatform } from './relay-protocol' import { resolveRemoteNodePath } from './ssh-remote-node-resolution' import { - acquireInstallLock, abandonInstall, finalizeInstall, isRelayAlreadyInstalled } from './ssh-relay-versioned-install' +import { acquireInstallLock } from './ssh-relay-install-lock' +import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' import type { SshConnection } from './ssh-connection' type SftpWriteCapture = { @@ -129,6 +147,7 @@ function decodePowerShellCommand(command: string): string | null { // 7: probe (cd && node -e require) // [8: cat stderr — only when probe stdout is MISSING (graceful path)] // 8 or 9: rm probe-stderr (best-effort cleanup; runs whenever probe resolved) +// [next: npm rebuild → chmod → second probe when the first probe is MISSING] // next: socket DEAD next: socket READY // // When the probe rejects (SSH channel close or cd-failure when the install @@ -143,6 +162,9 @@ function makeExecResponses(opts: { // Override probe stdout for shell-noise pressure tests. If set, replaces // the load-test stdout entirely (useful for testing pollution prefixes). probeStdoutOverride?: string + // Result after the automatic rebuild. Defaults to missing so legacy tests + // continue to exercise the final degraded-mode warning. + repairProbe?: 'ok' | 'missing' // Raw stdout for the build-toolchain probe that runs in installNativeDeps' // catch when `npm install` rejects on Linux. Defaults to a fully-present // toolchain so the original npm error propagates unchanged. @@ -187,6 +209,16 @@ function makeExecResponses(opts: { slots.push('') // cat stderr (graceful failure path captures detail) } slots.push('') // rm -f stderr (best-effort cleanup) + if (!probeOk) { + slots.push('') // npm rebuild with lifecycle scripts explicitly enabled + slots.push('') // chmod prebuilds after rebuild + const repairProbe = opts.repairProbe === 'ok' ? 'ORCA-NPTY-PROBE-OK\n' : 'MISSING\n' + slots.push(repairProbe) + if (!repairProbe.includes('ORCA-NPTY-PROBE-OK')) { + slots.push('') // cat stderr after unsuccessful rebuild + } + slots.push('') // rm -f stderr after rebuild probe + } } slots.push('DEAD', 'READY') return slots @@ -207,6 +239,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { // a leaked response. clearAllMocks doesn't drop the queue (it only clears // .mock.calls), so we explicitly mockReset. vi.mocked(execCommand).mockReset() + vi.mocked(uploadDirectory).mockResolvedValue(undefined) sftpCapture.paths.length = 0 for (const k of Object.keys(sftpCapture.contents)) { delete sftpCapture.contents[k] @@ -256,12 +289,17 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { // break `require('node-pty')`. expect(parsed.type).toBe('commonjs') expect(parsed.dependencies).toEqual({ '@parcel/watcher': '2.5.6', 'node-pty': '1.1.0' }) + expect(parsed.allowScripts).toEqual({ + '@parcel/watcher@2.5.6': true, + 'node-pty@1.1.0': true + }) const execCalls = vi.mocked(execCommand).mock.calls.map(([, c]) => c) const npmInstallIdx = execCalls.findIndex( (c) => c.includes('npm install') && c.includes('node-pty') && c.includes('@parcel/watcher') ) expect(npmInstallIdx).toBeGreaterThanOrEqual(0) + expect(execCalls[npmInstallIdx]).toContain('--ignore-scripts=false') // Pin actual ordering: number of execCommand calls observed at the moment // ws.end() ran for package.json must be < the index of `npm install`. // Catches a future refactor that fires SFTP-write and npm install via @@ -376,7 +414,96 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { expect(warnMessages.some((m) => m.includes('[ssh-relay][NPTY-MISSING]'))).toBe(true) expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1) - expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) + }) + + it('rebuilds unloadable native deps and recovers before first relay launch', async () => { + const conn = makeMockConnection(sftpCapture) + feed(makeExecResponses({ npmInstall: 'ok', probe: 'missing', repairProbe: 'ok' })) + + await deployAndLaunchRelay(conn) + + const execCalls = vi.mocked(execCommand).mock.calls.map(([, c]) => c) + const failedProbeIdx = execCalls.findIndex((c) => c.includes('require("node-pty")')) + const rebuildIdx = execCalls.findIndex((c) => c.includes('npm rebuild')) + const repairedProbeIdx = execCalls.findIndex( + (c, index) => index > rebuildIdx && c.includes('require("node-pty")') + ) + expect(rebuildIdx).toBeGreaterThan(failedProbeIdx) + expect(execCalls[rebuildIdx]).toContain('--ignore-scripts=false') + expect(repairedProbeIdx).toBeGreaterThan(rebuildIdx) + + const warnMessages = warnSpy.mock.calls.map((args) => String(args[0] ?? '')) + expect(warnMessages.some((m) => m.includes('[ssh-relay][NPTY-MISSING]'))).toBe(false) + expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1) + }) + + it('propagates an SSH-channel failure from the post-rebuild re-probe', async () => { + // Why: the rebuild itself degrades gracefully, but the verification probe + // after it must still surface transport death — conflating a dead channel + // with "native deps missing" would finalize a half-repaired install. + const conn = makeMockConnection(sftpCapture) + feed([ + '__ORCA_REMOTE_PLATFORM__ Linux x86_64', // uname + '/home/u', // $HOME + '', // mkdir remoteDir (uploadRelay) + '', // chmod +x node + '', // npm install native deps + '', // chmod prebuilds + 'MISSING\n', // first probe: require() fails + '', // cat probe stderr + '', // rm probe stderr + '', // npm rebuild native deps + '', // chmod prebuilds after rebuild + { reject: 'SSH channel closed during native deps re-probe' } // re-probe rejects + ]) + + await expect(deployAndLaunchRelay(conn)).rejects.toThrow(/SSH channel closed/) + + // Rebuild failure is swallowed; a re-probe transport failure must not be. + const warnMessages = warnSpy.mock.calls.map((args) => String(args[0] ?? '')) + expect(warnMessages.some((m) => m.includes('[ssh-relay][NPTY-MISSING]'))).toBe(false) + expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) + }) + + it('aborts an in-progress native install and releases its lock at deploy timeout', async () => { + vi.useFakeTimers() + try { + const conn = makeMockConnection(sftpCapture) + feed([ + '__ORCA_REMOTE_PLATFORM__ Linux x86_64', + '/home/u', + '', // mkdir remoteDir + '' // chmod +x node + ]) + let installSignal: AbortSignal | undefined + vi.mocked(execCommand).mockImplementationOnce((_conn, command, options) => { + expect(command).toContain('npm install') + installSignal = options?.signal + return new Promise((_resolve, reject) => { + installSignal?.addEventListener('abort', () => reject(installSignal?.reason), { + once: true + }) + }) + }) + + const promise = deployAndLaunchRelay(conn).catch((err: Error) => err) + await vi.waitFor(() => expect(installSignal).toBeDefined()) + + await vi.advanceTimersByTimeAsync(RELAY_DEPLOY_TIMEOUT_MS) + + const result = await promise + expect(result).toBeInstanceOf(Error) + expect((result as Error).message).toBe( + `Relay deployment timed out after ${RELAY_DEPLOY_TIMEOUT_MS / 1000}s` + ) + expect(installSignal?.aborted).toBe(true) + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) + expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } }) it('lets a probe SSH-channel failure bubble up rather than silently mapping to MISSING', async () => { @@ -479,9 +606,9 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { expect(chmodPrebuildsIdx).toBeGreaterThan(npmIdx) expect(probeIdx).toBeGreaterThan(chmodPrebuildsIdx) - // Happy path: finalize exactly once, abandon never. + // Hold the install lock through launch, then release it exactly once. expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1) - expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) }) it('matches the sentinel even with bashrc/MOTD noise prefixed to probe stdout', async () => { @@ -522,7 +649,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { const warnMessages = warnSpy.mock.calls.map((args) => String(args[0] ?? '')) expect(warnMessages.some((m) => m.includes('[ssh-relay][NPTY-MISSING]'))).toBe(true) expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1) - expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) }) it('keeps Windows node-pty probe failures non-fatal by checking LASTEXITCODE', async () => { @@ -535,7 +662,8 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { '', // mkdir remoteDir '', // npm install native deps 'MISSING\n', // native process exit normalized by PowerShell command - '', // remove probe stderr file + '', // npm rebuild native deps + 'MISSING\n', // rebuilt native process still cannot load '', // no persisted active pipe marker 'WAITING', '', // WMI relay launch @@ -554,11 +682,29 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { const probeScript = decodePowerShellCommand(probeCommand) ?? '' expect(probeScript).toContain('$LASTEXITCODE -ne 0') expect(probeScript).toContain("'MISSING'") + expect(probeScript).toContain('loadNativeModule') + + const npmScripts = vi + .mocked(execCommand) + .mock.calls.map(([, command]) => decodePowerShellCommand(command) ?? '') + .filter((script) => script.includes('npm install') || script.includes('npm rebuild')) + expect(npmScripts).toHaveLength(2) + expect(npmScripts.every((script) => script.includes('--ignore-scripts=false'))).toBe(true) + expect( + npmScripts.every((script) => + script.includes('if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }') + ) + ).toBe(true) + expect( + vi + .mocked(execCommand) + .mock.calls.some(([, command]) => command.includes('.npty-probe.stderr')) + ).toBe(false) const warnMessages = warnSpy.mock.calls.map((args) => String(args[0] ?? '')) expect(warnMessages.some((m) => m.includes('[ssh-relay][NPTY-MISSING]'))).toBe(true) expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1) - expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) }) it('includes the platform tuple in NPTY-MISSING and native install failure logs', async () => { @@ -607,8 +753,8 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { feed([ '__ORCA_REMOTE_PLATFORM__ Linux x86_64', '/home/u', - 'MISSING', // first native-deps probe before lock - 'MISSING', // re-probe after lock + 'ORCA-NATIVE-DEPS-MISSING:@parcel/watcher\nMISSING', // first probe before lock + 'ORCA-NATIVE-DEPS-MISSING:@parcel/watcher\nMISSING', // re-probe after lock '', // npm install native deps '', // chmod prebuilds 'ORCA-NPTY-PROBE-OK\n', @@ -619,7 +765,10 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { await deployAndLaunchRelay(conn) - expect(vi.mocked(acquireInstallLock)).toHaveBeenCalledTimes(1) + expect(vi.mocked(tryAcquireRelayRepairLock)).toHaveBeenCalledTimes(1) + expect(vi.mocked(tryAcquireRelayRepairLock).mock.calls[0]?.[3]?.signal).toBeInstanceOf( + AbortSignal + ) expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1) const execCalls = vi.mocked(execCommand).mock.calls.map(([, c]) => c) expect( @@ -627,6 +776,228 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { (c) => c.includes('npm install') && c.includes('node-pty') && c.includes('@parcel/watcher') ) ).toBe(true) + const installCommand = execCalls.find((c) => c.includes('npm install')) ?? '' + expect(installCommand).toContain('node_modules/@parcel/watcher') + expect(installCommand).toContain("-name 'watcher-*'") + expect(installCommand).not.toContain("rm -rf 'node_modules/node-pty'") + }) + + it('launches an already-installed relay in degraded mode when repair throws', async () => { + // Why: a repair failure on a completed dir (e.g. offline/proxy-locked npm) + // must not block the connection — the relay still serves fs/git/preflight. + // Pre-fix this rethrew and aborted the whole deploy. The next reconnect + // retries, so a transient failure self-heals. + vi.mocked(isRelayAlreadyInstalled).mockResolvedValue(true) + const conn = makeMockConnection(sftpCapture) + feed([ + '__ORCA_REMOTE_PLATFORM__ Linux x86_64', + '/home/u', + 'MISSING', // health probe: require() fails + 'MISSING', // re-probe after lock + { reject: 'npm ERR! network ETIMEDOUT' }, // npm install fails (offline) + 'DEAD', + 'READY' + ]) + + // Deploy must resolve (degraded), not reject. + await deployAndLaunchRelay(conn) + + expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) + const warnMessages = warnSpy.mock.calls.map((args) => String(args[0] ?? '')) + expect(warnMessages.some((m) => m.includes('launching degraded'))).toBe(true) + }) + + it('retains the repair lock when remote command termination is unconfirmed', async () => { + vi.mocked(isRelayAlreadyInstalled).mockResolvedValue(true) + const conn = makeMockConnection(sftpCapture) + vi.mocked(execCommand) + .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64') + .mockResolvedValueOnce('/home/u') + .mockResolvedValueOnce('MISSING') + .mockResolvedValueOnce('MISSING') + .mockRejectedValueOnce( + Object.assign(new Error('npm termination was not confirmed'), { + sshChannelCloseConfirmed: false + }) + ) + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('READY') + + await deployAndLaunchRelay(conn) + + expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + const warnMessages = warnSpy.mock.calls.map((args) => String(args[0] ?? '')) + expect(warnMessages.some((message) => message.includes('launching degraded'))).toBe(true) + }) + + it('retains the first-install lock when an aborted npm install has unconfirmed teardown', async () => { + vi.useFakeTimers() + try { + const conn = makeMockConnection(sftpCapture) + feed([ + '__ORCA_REMOTE_PLATFORM__ Linux x86_64', + '/home/u', + '', // mkdir remoteDir + '' // chmod +x node + ]) + let installSignal: AbortSignal | undefined + vi.mocked(execCommand).mockImplementationOnce((_conn, command, options) => { + expect(command).toContain('npm install') + installSignal = options?.signal + return new Promise((_resolve, reject) => { + installSignal?.addEventListener( + 'abort', + () => { + // Mirrors execCommand's bounded close grace when ssh2 never + // confirms that the remote npm process stopped. + setTimeout( + () => + reject( + Object.assign(new Error('npm teardown remained unconfirmed'), { + sshChannelCloseConfirmed: false + }) + ), + 5_000 + ) + }, + { once: true } + ) + }) + }) + + const deploy = deployAndLaunchRelay(conn).catch((err: Error) => err) + await vi.waitFor(() => expect(installSignal).toBeDefined()) + await vi.advanceTimersByTimeAsync(RELAY_DEPLOY_TIMEOUT_MS) + const result = await deploy + expect(result).toBeInstanceOf(Error) + expect((result as Error).message).toContain('Relay deployment timed out') + await vi.advanceTimersByTimeAsync(5_000) + + expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it('does not finalize or release a first-install lock after unconfirmed rebuild teardown', async () => { + const conn = makeMockConnection(sftpCapture) + vi.mocked(execCommand) + .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64') + .mockResolvedValueOnce('/home/u') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('MISSING') + .mockResolvedValueOnce('rebuild diagnostics') + .mockResolvedValueOnce('') + .mockRejectedValueOnce( + Object.assign(new Error('rebuild termination was not confirmed'), { + sshChannelCloseConfirmed: false + }) + ) + + await expect(deployAndLaunchRelay(conn)).rejects.toThrow( + 'rebuild termination was not confirmed' + ) + expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + }) + + it('retains the first-install lock when an aborted rebuild has unconfirmed teardown', async () => { + vi.useFakeTimers() + try { + const conn = makeMockConnection(sftpCapture) + feed([ + '__ORCA_REMOTE_PLATFORM__ Linux x86_64', + '/home/u', + '', // mkdir remoteDir + '', // chmod +x node + '', // npm install + '', // chmod prebuilds + 'MISSING', + 'rebuild diagnostics', + '' // remove probe diagnostics + ]) + let rebuildSignal: AbortSignal | undefined + vi.mocked(execCommand).mockImplementationOnce((_conn, command, options) => { + expect(command).toContain('npm rebuild') + rebuildSignal = options?.signal + return new Promise((_resolve, reject) => { + rebuildSignal?.addEventListener( + 'abort', + () => { + setTimeout( + () => + reject( + Object.assign(new Error('rebuild teardown remained unconfirmed'), { + sshChannelCloseConfirmed: false + }) + ), + 5_000 + ) + }, + { once: true } + ) + }) + }) + + const deploy = deployAndLaunchRelay(conn).catch((err: Error) => err) + await vi.waitFor(() => expect(rebuildSignal).toBeDefined()) + await vi.advanceTimersByTimeAsync(RELAY_DEPLOY_TIMEOUT_MS) + const result = await deploy + expect(result).toBeInstanceOf(Error) + expect((result as Error).message).toContain('Relay deployment timed out') + await vi.advanceTimersByTimeAsync(5_000) + + expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it.each(['busy', 'error'] as const)('launches degraded when lock is %s', async (lockResult) => { + // Why: lock contention/wedge must not block a completed relay from launching + // in degraded mode — repair is best-effort and we hold no lock to release. + vi.mocked(isRelayAlreadyInstalled).mockResolvedValue(true) + vi.mocked(tryAcquireRelayRepairLock).mockResolvedValueOnce(lockResult) + const conn = makeMockConnection(sftpCapture) + feed(['__ORCA_REMOTE_PLATFORM__ Linux x86_64', '/home/u', 'MISSING', 'DEAD', 'READY']) + + await deployAndLaunchRelay(conn) + + expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() + const execCalls = vi.mocked(execCommand).mock.calls.map(([, c]) => c) + expect(execCalls.some((c) => c.includes('npm install'))).toBe(false) + const warnMessages = warnSpy.mock.calls.map((args) => String(args[0] ?? '')) + expect(warnMessages.some((m) => m.includes(`repair lock is ${lockResult}`))).toBe(true) + }) + + it('loads native bindings when checking whether a completed relay needs repair', async () => { + vi.mocked(isRelayAlreadyInstalled).mockResolvedValue(true) + const conn = makeMockConnection(sftpCapture) + feed([ + '__ORCA_REMOTE_PLATFORM__ Linux x86_64', + '/home/u', + 'ORCA-NATIVE-DEPS-OK', + 'DEAD', + 'READY' + ]) + + await deployAndLaunchRelay(conn) + + const healthProbe = vi + .mocked(execCommand) + .mock.calls.map(([, c]) => c) + .find((c) => c.includes('ORCA-NATIVE-DEPS-OK')) + expect(healthProbe).toContain('require("node-pty")') + expect(healthProbe).toContain('loadNativeModule') + expect(healthProbe).toContain('require("@parcel/watcher")') + expect(healthProbe).not.toContain('require.resolve') }) it('does not mutate an existing relay dir when required native deps are present', async () => { @@ -643,7 +1014,9 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { await deployAndLaunchRelay(conn) expect(vi.mocked(acquireInstallLock)).not.toHaveBeenCalled() + expect(vi.mocked(tryAcquireRelayRepairLock)).toHaveBeenCalledTimes(1) expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() + expect(vi.mocked(abandonInstall)).toHaveBeenCalledTimes(1) const execCalls = vi.mocked(execCommand).mock.calls.map(([, c]) => c) expect(execCalls.some((c) => c.includes('npm install'))).toBe(false) }) diff --git a/src/main/ssh/ssh-relay-repair-lock.ts b/src/main/ssh/ssh-relay-repair-lock.ts new file mode 100644 index 000000000..01d58e390 --- /dev/null +++ b/src/main/ssh/ssh-relay-repair-lock.ts @@ -0,0 +1,153 @@ +import type { SshConnection } from './ssh-connection' +import { execCommand } from './ssh-relay-deploy-helpers' +import { + acquireInstallLockParentCommand, + lockAgeSecondsCommand, + probeInstallLockExistsCommand, + tryCreateInstallLockCommand, + tryStealInstallLockCommand +} from './ssh-relay-install-lock-commands' +import { isRelayGcClaimed } from './ssh-relay-gc-claim' +import { + getRemoteHostPlatform, + isWindowsRemoteHost, + joinRemotePath, + type RemoteHostPlatform +} from './ssh-remote-platform' +import { INSTALL_LOCK_STALE_SECONDS, RELAY_INSTALL_LOCK_NAME } from './ssh-relay-install-lock' +import { removeRemoteTreeCommand } from './ssh-remote-commands' + +const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') + +export type RelayRepairLockResult = 'acquired' | 'busy' | 'gc' | 'error' + +function execHostCommand( + conn: SshConnection, + host: RemoteHostPlatform, + command: string, + signal?: AbortSignal +): Promise { + return execCommand(conn, command, { wrapCommand: !isWindowsRemoteHost(host), signal }) +} + +/** + * Try once to acquire the install lock for best-effort repair work. + * + * Why: a completed relay can launch in degraded mode, so repair must not wait + * behind another installer. An already-stale lock is still recovered now. + */ +export async function tryAcquireRelayRepairLock( + conn: SshConnection, + remoteRelayDir: string, + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + options?: { signal?: AbortSignal } +): Promise { + const lockDir = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) + try { + const gcClaimedBeforeAcquire = await isRelayGcClaimed( + conn, + remoteRelayDir, + host, + options?.signal + ).catch(() => undefined) + options?.signal?.throwIfAborted() + if (gcClaimedBeforeAcquire === true) { + return 'gc' + } + if (gcClaimedBeforeAcquire !== false) { + return 'error' + } + await execHostCommand( + conn, + host, + acquireInstallLockParentCommand(host, remoteRelayDir), + options?.signal + ) + const firstAttempt = await execHostCommand( + conn, + host, + tryCreateInstallLockCommand(host, lockDir), + options?.signal + ) + if (firstAttempt.trim().endsWith('OK')) { + return finishRepairLockAcquire(conn, remoteRelayDir, lockDir, host, options?.signal) + } + const steal = await execHostCommand( + conn, + host, + tryStealInstallLockCommand(host, lockDir, INSTALL_LOCK_STALE_SECONDS), + options?.signal + ) + if (steal.trim().endsWith('OK')) { + console.warn(`[ssh-relay] Stealing stale install lock at ${lockDir}`) + return finishRepairLockAcquire(conn, remoteRelayDir, lockDir, host, options?.signal) + } + return classifyRepairLockContention(conn, remoteRelayDir, lockDir, host, options?.signal) + } catch { + options?.signal?.throwIfAborted() + return classifyRepairLockContention(conn, remoteRelayDir, lockDir, host, options?.signal) + } +} + +async function classifyRepairLockContention( + conn: SshConnection, + remoteRelayDir: string, + lockDir: string, + host: RemoteHostPlatform, + signal?: AbortSignal +): Promise { + const gcClaimed = await isRelayGcClaimed(conn, remoteRelayDir, host, signal).catch( + () => undefined + ) + signal?.throwIfAborted() + if (gcClaimed === true) { + return 'gc' + } + if (gcClaimed !== false) { + return 'error' + } + + const lockProbe = await execHostCommand( + conn, + host, + probeInstallLockExistsCommand(host, lockDir), + signal + ).catch(() => '') + signal?.throwIfAborted() + if (lockProbe.trim() !== 'LOCKED') { + return 'error' + } + const ageOutput = await execHostCommand( + conn, + host, + lockAgeSecondsCommand(host, lockDir), + signal + ).catch(() => '') + signal?.throwIfAborted() + const ageSeconds = Number.parseInt(ageOutput.trim(), 10) + // Why: GC may remove stale locks, so only a positively observed fresh lock + // proves that another launch/repair owner is fencing this directory. + return Number.isFinite(ageSeconds) && ageSeconds >= 0 && ageSeconds <= INSTALL_LOCK_STALE_SECONDS + ? 'busy' + : 'error' +} + +async function finishRepairLockAcquire( + conn: SshConnection, + remoteRelayDir: string, + lockDir: string, + host: RemoteHostPlatform, + signal?: AbortSignal +): Promise { + const gcClaimed = await isRelayGcClaimed(conn, remoteRelayDir, host, signal).catch( + () => undefined + ) + if (gcClaimed === false && !signal?.aborted) { + return 'acquired' + } + // Why: GC may win its stable sibling claim while this command creates the + // in-tree lock. Back out before npm can mutate a directory being renamed. + await execHostCommand(conn, host, removeRemoteTreeCommand(host, lockDir)).catch(() => {}) + signal?.throwIfAborted() + return gcClaimed ? 'gc' : 'error' +} diff --git a/src/main/ssh/ssh-relay-versioned-install.test.ts b/src/main/ssh/ssh-relay-versioned-install.test.ts index 244ae80a4..9bd3f73c8 100644 --- a/src/main/ssh/ssh-relay-versioned-install.test.ts +++ b/src/main/ssh/ssh-relay-versioned-install.test.ts @@ -18,11 +18,19 @@ import { readLocalFullVersion, computeRemoteRelayDir, isRelayAlreadyInstalled, - acquireInstallLock, finalizeInstall, abandonInstall, gcOldRelayVersions } from './ssh-relay-versioned-install' +import { acquireInstallLock } from './ssh-relay-install-lock' +import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' +import { + isRelayGcClaimed, + relayGcClaimPath, + releaseRelayGcClaim, + tryAcquireRelayGcClaim, + waitForRelayGcClaimRelease +} from './ssh-relay-gc-claim' import { execCommand } from './ssh-relay-deploy-helpers' import { getRemoteHostPlatform } from './ssh-remote-platform' import type { SshConnection } from './ssh-connection' @@ -88,6 +96,18 @@ describe('isRelayAlreadyInstalled', () => { expect(await isRelayAlreadyInstalled(conn, '/r')).toBe(false) }) + it('does not convert an aborted install probe into a missing install', async () => { + const controller = new AbortController() + mockExec.mockImplementationOnce(async () => { + controller.abort() + throw Object.assign(new Error('cancelled'), { name: 'AbortError' }) + }) + + await expect( + isRelayAlreadyInstalled(conn, '/r', undefined, { signal: controller.signal }) + ).rejects.toMatchObject({ name: 'AbortError' }) + }) + 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 @@ -124,26 +144,130 @@ describe('acquireInstallLock', () => { }) it('returns when mkdir reports OK', async () => { - // 1st call: mkdir -p remoteRelayDir - // 2nd call: mkdir lockDir → OK - mockExec.mockResolvedValueOnce('').mockResolvedValueOnce('OK') + mockExec + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('OPEN') await acquireInstallLock(conn, '/r') - expect(mockExec).toHaveBeenCalledTimes(2) + expect(mockExec).toHaveBeenCalledTimes(4) + }) + + it('recovers a stale sibling GC claim before first-install lock acquisition', async () => { + mockExec + .mockResolvedValueOnce('LOCKED') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('RELEASED') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('OPEN') + + await expect(acquireInstallLock(conn, '/r')).resolves.toBeUndefined() + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands[1]).toContain('lock_tombstone') + expect(commands[4]).toBe("mkdir -p '/r'") + }) + + it('returns immediately without deleting a live repair lock', async () => { + // Why: repair can run npm install plus rebuild under the same lock; a + // second reconnect must launch degraded rather than corrupt node_modules. + mockExec + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('BUSY') + .mockResolvedValueOnce('BUSY') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('LOCKED') + .mockResolvedValueOnce('0') + + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe('busy') + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands.some((command) => command.includes('lock_tombstone'))).toBe(true) + expect(commands.filter((command) => command.startsWith('rm -rf'))).toHaveLength(0) + }) + + it('does not report stale or indeterminate contention as a launch fence', async () => { + mockExec + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('BUSY') + .mockResolvedValueOnce('BUSY') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('LOCKED') + .mockResolvedValueOnce(`${21 * 60}`) + + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe('error') + + mockExec.mockReset().mockRejectedValueOnce(new Error('claim probe failed')) + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe('error') + }) + + it('recovers a stale best-effort repair lock without polling', async () => { + mockExec + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('BUSY') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('OPEN') + + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe('acquired') + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands.some((command) => command.includes('lock_tombstone'))).toBe(true) + expect(commands.filter((command) => command.includes('lock_tombstone'))).toHaveLength(1) + }) + + it('backs out when GC claims the sibling path during repair lock acquisition', async () => { + mockExec + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('LOCKED') + .mockResolvedValueOnce('') + + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe('gc') + + const lastCommand = mockExec.mock.calls.at(-1)?.[1] ?? '' + expect(lastCommand).toBe("rm -rf '/r/.install-lock'") + }) + + it('propagates cancellation through best-effort repair lock commands', async () => { + const abortController = new AbortController() + const abortError = Object.assign(new Error('cancelled'), { name: 'AbortError' }) + mockExec.mockRejectedValueOnce(abortError) + + const promise = tryAcquireRelayRepairLock(conn, '/r', undefined, { + signal: abortController.signal + }) + abortController.abort() + + await expect(promise).rejects.toMatchObject({ name: 'AbortError' }) + expect(mockExec.mock.calls[0]?.[2]?.signal).toBe(abortController.signal) }) it('polls until the lock becomes available (concurrent installer wins, then we acquire)', async () => { vi.useFakeTimers() try { - // Sequence: - // 1. mkdir -p (parent dir prep) - // 2. mkdir lockDir → BUSY (someone else holds it) - // 3. mkdir lockDir → BUSY again - // 4. mkdir lockDir → OK (concurrent installer released) - mockExec - .mockResolvedValueOnce('') - .mockResolvedValueOnce('BUSY') - .mockResolvedValueOnce('BUSY') - .mockResolvedValueOnce('OK') + let createAttempts = 0 + mockExec.mockImplementation(async (_conn: unknown, command: string) => { + if (command.includes('.gc-claim')) { + return 'OPEN' + } + if (command.startsWith('mkdir -p')) { + return '' + } + if (command.includes('lock_tombstone')) { + return 'BUSY' + } + if (command.includes('.install-lock')) { + createAttempts++ + return createAttempts >= 3 ? 'OK' : 'BUSY' + } + return '' + }) const promise = acquireInstallLock(conn, '/r') // Drive the polling loop: each iteration awaits a 1s timer. @@ -159,36 +283,48 @@ describe('acquireInstallLock', () => { } }) - it('steals a stale lock and retries with a reset timeout window', async () => { + it('immediately tries to steal an already-stale lock', async () => { + mockExec + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('BUSY') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('OPEN') + + await expect(acquireInstallLock(conn, '/r')).resolves.toBeUndefined() + + const cmds = mockExec.mock.calls.map(([, c]) => c) + expect(cmds).toHaveLength(5) + expect(cmds[3]).toContain('lock_tombstone') + }) + + it('retries stale takeover when a fresh lock ages out during the wait', async () => { vi.useFakeTimers({ now: 1_700_000_000_000 }) try { - let mkdirCalls = 0 + const recoverableAt = Date.now() + 60_000 mockExec.mockImplementation(async (_conn: unknown, cmd: string) => { + if (cmd.includes('.gc-claim')) { + return 'OPEN' + } if (cmd.startsWith('mkdir -p')) { return '' } + if (cmd.includes('lock_tombstone')) { + return Date.now() >= recoverableAt ? 'OK' : 'BUSY' + } if (cmd.includes('mkdir') && cmd.includes('.install-lock')) { - mkdirCalls++ - return mkdirCalls > 200 ? 'OK' : 'BUSY' - } - if (cmd.includes('stat')) { - return `${Math.floor((Date.now() - 10 * 60 * 1000) / 1000)}\n` - } - if (cmd.startsWith('rm -rf')) { - mkdirCalls = 1000 - return '' + return 'BUSY' } return '' }) const promise = acquireInstallLock(conn, '/r') - // Drive through the full timeout (120s) so the stale-recovery branch - // fires, then drive a few more seconds for the post-recovery retry. - await vi.advanceTimersByTimeAsync(125_000) - await promise + await vi.advanceTimersByTimeAsync(61_000) - const cmds = mockExec.mock.calls.map(([, c]) => c) - expect(cmds.some((c) => c.includes('rm -rf') && c.includes('.install-lock'))).toBe(true) + await expect(promise).resolves.toBeUndefined() + expect(mockExec.mock.calls.filter(([, cmd]) => cmd.includes('lock_tombstone'))).toHaveLength( + 2 + ) } finally { vi.useRealTimers() } @@ -198,26 +334,86 @@ describe('acquireInstallLock', () => { vi.useFakeTimers({ now: 1_700_000_000_000 }) try { mockExec.mockImplementation(async (_conn: unknown, cmd: string) => { + if (cmd.includes('.gc-claim')) { + return 'OPEN' + } if (cmd.startsWith('mkdir -p')) { return '' } + if (cmd.includes('lock_tombstone')) { + return 'BUSY' + } if (cmd.includes('mkdir') && cmd.includes('.install-lock')) { return 'BUSY' } if (cmd.includes('stat')) { - return `${Math.floor(Date.now() / 1000)}\n` + return '0\n' } return '' }) - const rejection = expect(acquireInstallLock(conn, '/r')).rejects.toThrow(/not yet stale/i) - await vi.advanceTimersByTimeAsync(125_000) + const rejection = expect(acquireInstallLock(conn, '/r')).rejects.toThrow( + /another install is still in progress/i + ) + await vi.advanceTimersByTimeAsync(905_000) await rejection } finally { vi.useRealTimers() } }) + it('stops polling when the caller aborts the lock wait', async () => { + vi.useFakeTimers() + try { + const abortController = new AbortController() + mockExec.mockImplementation(async (_conn: unknown, cmd: string) => + cmd.includes('.gc-claim') ? 'OPEN' : 'BUSY' + ) + + const promise = acquireInstallLock(conn, '/r', undefined, { + signal: abortController.signal + }) + await vi.advanceTimersByTimeAsync(1_000) + abortController.abort() + await expect(promise).rejects.toMatchObject({ name: 'AbortError' }) + + const callCountAfterAbort = mockExec.mock.calls.length + await vi.advanceTimersByTimeAsync(10_000) + expect(mockExec).toHaveBeenCalledTimes(callCountAfterAbort) + } finally { + vi.useRealTimers() + } + }) + + it('keeps waiting while a slow first installer is within the deploy bound', async () => { + vi.useFakeTimers({ now: 1_700_000_000_000 }) + try { + const availableAt = Date.now() + 500_000 + mockExec.mockImplementation(async (_conn: unknown, cmd: string) => { + if (cmd.includes('.gc-claim')) { + return 'OPEN' + } + if (cmd.startsWith('mkdir -p')) { + return '' + } + if (cmd.includes('mkdir') && cmd.includes('.install-lock')) { + return Date.now() >= availableAt ? 'OK' : 'BUSY' + } + return '' + }) + + const promise = acquireInstallLock(conn, '/r') + await vi.advanceTimersByTimeAsync(501_000) + + await expect(promise).resolves.toBeUndefined() + expect( + mockExec.mock.calls.filter(([, cmd]) => cmd.includes('lock_tombstone')).length + ).toBeGreaterThan(1) + } finally { + vi.useRealTimers() + } + }) + it('finalizeInstall writes .install-complete then removes the lock', async () => { mockExec.mockResolvedValueOnce('').mockResolvedValueOnce('') await finalizeInstall(conn, '/r') @@ -238,26 +434,220 @@ describe('acquireInstallLock', () => { }) }) +describe('relay GC claim', () => { + beforeEach(() => { + vi.clearAllMocks() + mockExec.mockReset() + }) + + it('parses explicit claim markers through remote startup noise', async () => { + mockExec.mockResolvedValueOnce('Welcome to host\nLOCKED\n') + await expect(isRelayGcClaimed(conn, '/relay/version')).resolves.toBe(true) + + mockExec.mockResolvedValueOnce('Last login: today\nOPEN\n') + await expect(isRelayGcClaimed(conn, '/relay/version')).resolves.toBe(false) + }) + + it('rejects missing or conflicting claim markers', async () => { + mockExec.mockResolvedValueOnce('Welcome to host\n') + await expect(isRelayGcClaimed(conn, '/relay/version')).rejects.toThrow('Inconclusive') + + mockExec.mockResolvedValueOnce('LOCKED\nOPEN\n') + await expect(isRelayGcClaimed(conn, '/relay/version')).rejects.toThrow('Inconclusive') + }) + + it('uses a stable sibling path outside the recursively deleted install', async () => { + mockExec.mockResolvedValueOnce('OK').mockResolvedValueOnce('') + + await expect(tryAcquireRelayGcClaim(conn, '/relay/version')).resolves.toEqual( + expect.any(String) + ) + + expect(relayGcClaimPath('/relay/version')).toBe('/relay/version.gc-claim') + expect(mockExec.mock.calls[0]?.[1]).toContain("mkdir '/relay/version.gc-claim'") + }) + + it('recovers and removes a stale sibling claim before retrying deploy', async () => { + mockExec + .mockResolvedValueOnce('LOCKED') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('RELEASED') + + await waitForRelayGcClaimRelease(conn, '/relay/version') + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands[1]).toContain('lock_tombstone') + expect(commands[3]).toContain("rm -rf '/relay/version.gc-claim'") + }) + + it('keeps waiting after losing a recovered claim until the claim is actually gone', async () => { + mockExec + .mockResolvedValueOnce('LOCKED') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('LOST') + .mockResolvedValueOnce('OPEN') + + await waitForRelayGcClaimRelease(conn, '/relay/version') + + expect(mockExec).toHaveBeenCalledTimes(5) + }) + + it('writes and conditionally releases the Windows sibling claim owner token', async () => { + const windows = getRemoteHostPlatform('win32-x64') + mockExec.mockResolvedValueOnce('OK').mockResolvedValueOnce('').mockResolvedValueOnce('RELEASED') + + const token = await tryAcquireRelayGcClaim(conn, 'C:/relay/version', windows) + expect(token).toEqual(expect.any(String)) + await expect(releaseRelayGcClaim(conn, 'C:/relay/version', token!, windows)).resolves.toBe( + 'released' + ) + + const ownerScript = decodePowerShellCommand(mockExec.mock.calls[1]?.[1] ?? '') + const releaseScript = decodePowerShellCommand(mockExec.mock.calls[2]?.[1] ?? '') + expect(ownerScript).toContain('Set-Content -LiteralPath') + expect(ownerScript).toContain('.gc-claim/.gc-owner') + expect(releaseScript).toContain('Get-Content -LiteralPath') + expect(releaseScript).toContain('-cne') + expect(releaseScript).toContain('Remove-Item -LiteralPath') + expect(releaseScript).toContain("'RELEASED'") + expect(releaseScript).toContain("'LOST'") + expect(releaseScript).toContain("'UNKNOWN'") + expect(releaseScript).not.toContain('}; elseif') + expect(releaseScript).not.toContain('{;') + }) + + it('conditionally releases a claim when the owner write reply is lost', async () => { + mockExec + .mockResolvedValueOnce('OK') + .mockRejectedValueOnce(new Error('owner write reply lost')) + .mockResolvedValueOnce('RELEASED') + + await expect(tryAcquireRelayGcClaim(conn, '/relay/version')).resolves.toBeNull() + + const ownerCommand = mockExec.mock.calls[1]?.[1] ?? '' + const releaseCommand = mockExec.mock.calls[2]?.[1] ?? '' + const token = ownerCommand.match(/printf %s '([^']+)'/)?.[1] + expect(token).toBeTruthy() + expect(releaseCommand).toContain(`!= '${token}'`) + }) +}) + describe('gcOldRelayVersions', () => { beforeEach(() => { vi.clearAllMocks() + mockExec.mockReset() }) it('removes a sibling that is complete, unlocked, and has no live socket', async () => { // ls listing mockExec.mockResolvedValueOnce('relay-0.1.0+aaa\nrelay-0.1.0+bbb\n') - // For sibling "aaa": LOCKED probe → OPEN, COMPLETE probe → COMPLETE, sock probe → empty (no ALIVE), then rm -rf + // For sibling "aaa": safety probes pass, then GC claims the install lock before removal. mockExec .mockResolvedValueOnce('OPEN') .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OK') .mockResolvedValueOnce('') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OWNED') + .mockResolvedValueOnce('MOVED') + .mockResolvedValueOnce('RELEASED') .mockResolvedValueOnce('') await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') const lastCmd = mockExec.mock.calls.at(-1)?.[1] ?? '' expect(lastCmd).toContain('rm -rf') - expect(lastCmd).toContain('relay-0.1.0+aaa') + expect(lastCmd).toContain('relay-0.1.0+aaa.gc-tombstone') + const commands = mockExec.mock.calls.map(([, command]) => command) + expect( + commands.some((command) => command === "rm -rf '/home/u/.orca-remote/relay-0.1.0+aaa'") + ).toBe(false) + }) + + it('cleans only strict POSIX orphan tombstones even with no relay candidates', async () => { + mockExec + .mockResolvedValueOnce( + [ + 'relay-0.1.0+abc.gc-tombstone.123.456', + 'relay-0.1.0+abc.gc-tombstone.bad.456', + 'xrelay-0.1.0+abc.gc-tombstone.123.456', + 'relay-0.1.0+abc.gc-tombstone.123.456.extra', + 'relay-0.1.0+abc.gc-tombstone.123.456/child', + 'logs' + ].join('\n') + ) + .mockResolvedValueOnce('') + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + const removeCommands = mockExec.mock.calls + .map(([, command]) => command) + .filter((command) => command.startsWith('rm -rf')) + expect(removeCommands).toEqual([ + "rm -rf '/home/u/.orca-remote/relay-0.1.0+abc.gc-tombstone.123.456'" + ]) + }) + + it('cleans only strict Windows orphan tombstones even with no relay candidates', async () => { + const windows = getRemoteHostPlatform('win32-x64') + mockExec + .mockResolvedValueOnce( + 'relay-v0.1.0.gc-tombstone.123.456\nrelay-v0.1.0.gc-tombstone.latest.456\n' + ) + .mockResolvedValueOnce('') + + await gcOldRelayVersions(conn, 'C:/Users/u', 'C:/Users/u/.orca-remote/relay-0.1.0+bbb', windows) + + const removeScript = decodePowerShellCommand(mockExec.mock.calls[1]?.[1] ?? '') + expect(removeScript).toContain('relay-v0.1.0.gc-tombstone.123.456') + expect(removeScript).not.toContain('relay-v0.1.0.gc-tombstone.latest.456') + }) + + it('retries orphan tombstone cleanup on a later GC pass', async () => { + const tombstone = 'relay-0.1.0+abc.gc-tombstone.123.456' + mockExec + .mockResolvedValueOnce(tombstone) + .mockRejectedValueOnce(new Error('remove failed')) + .mockResolvedValueOnce(tombstone) + .mockResolvedValueOnce('') + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + const removeCommands = mockExec.mock.calls + .map(([, command]) => command) + .filter((command) => command.startsWith('rm -rf')) + expect(removeCommands).toHaveLength(2) + }) + + it('retries only an unknown claim release and stops after observing a lost generation', async () => { + mockExec + .mockResolvedValueOnce('relay-0.1.0+aaa\n') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OWNED') + .mockResolvedValueOnce('MOVED') + .mockResolvedValueOnce('UNKNOWN') + .mockResolvedValueOnce('LOST') + .mockResolvedValueOnce('') + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + const releaseCommands = mockExec.mock.calls + .map(([, command]) => command) + .filter((command) => command.includes('.gc-owner') && command.includes('echo RELEASED')) + expect(releaseCommands).toHaveLength(2) }) it('skips siblings that are missing .install-complete (mid-install or partial)', async () => { @@ -273,8 +663,8 @@ describe('gcOldRelayVersions', () => { it('skips siblings whose .install-lock is held and fresh', async () => { mockExec.mockResolvedValueOnce('relay-0.1.0+aaa\n') mockExec.mockResolvedValueOnce('LOCKED') - // isLockStale: mtime ~now → not stale. - mockExec.mockResolvedValueOnce(`${Math.floor(Date.now() / 1000)}\n`) + // isLockStale: age ~now → not stale. + mockExec.mockResolvedValueOnce('0\n') await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') const cmds = mockExec.mock.calls.map(([, c]) => c) expect(cmds.some((c) => c.includes('rm -rf'))).toBe(false) @@ -283,12 +673,20 @@ describe('gcOldRelayVersions', () => { it('removes a sibling with a stale lock + .install-complete (rm-lock failed mid-finalize)', async () => { mockExec.mockResolvedValueOnce('relay-0.1.0+aaa\n') mockExec.mockResolvedValueOnce('LOCKED') - // isLockStale: mtime well in the past → stale. - const staleSec = Math.floor((Date.now() - 10 * 60 * 1000) / 1000) - mockExec.mockResolvedValueOnce(`${staleSec}\n`) + // isLockStale: age well above the stale window → stale. + mockExec.mockResolvedValueOnce(`${21 * 60}\n`) mockExec.mockResolvedValueOnce('COMPLETE') // .install-complete present - mockExec.mockResolvedValueOnce('') // socket probe → no ALIVE - mockExec.mockResolvedValueOnce('') // rm -rf + mockExec.mockResolvedValueOnce('DEAD') // socket probe + mockExec.mockResolvedValueOnce('OK') // stable sibling GC claim + mockExec.mockResolvedValueOnce('') // write claim ownership token + mockExec.mockResolvedValueOnce('LOCKED') + mockExec.mockResolvedValueOnce(`${21 * 60}\n`) + mockExec.mockResolvedValueOnce('COMPLETE') + mockExec.mockResolvedValueOnce('DEAD') + mockExec.mockResolvedValueOnce('OWNED') + mockExec.mockResolvedValueOnce('MOVED') + mockExec.mockResolvedValueOnce('RELEASED') // release sibling claim + mockExec.mockResolvedValueOnce('') // remove tombstone await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') const lastCmd = mockExec.mock.calls.at(-1)?.[1] ?? '' expect(lastCmd).toContain('rm -rf') @@ -298,8 +696,15 @@ describe('gcOldRelayVersions', () => { it('GCs a legacy relay-v0.1.0 dir whose daemon is dead (no .install-complete required)', async () => { mockExec.mockResolvedValueOnce('relay-v0.1.0\n') mockExec.mockResolvedValueOnce('OPEN') // not locked - mockExec.mockResolvedValueOnce('') // socket probe → no ALIVE (no completeProbe — legacy) - mockExec.mockResolvedValueOnce('') // rm -rf + mockExec.mockResolvedValueOnce('DEAD') // socket probe (no completeProbe — legacy) + mockExec.mockResolvedValueOnce('OK') // GC claims candidate + mockExec.mockResolvedValueOnce('') + mockExec.mockResolvedValueOnce('OPEN') + mockExec.mockResolvedValueOnce('DEAD') + mockExec.mockResolvedValueOnce('OWNED') + mockExec.mockResolvedValueOnce('MOVED') + mockExec.mockResolvedValueOnce('RELEASED') + mockExec.mockResolvedValueOnce('') await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') const cmds = mockExec.mock.calls.map(([, c]) => c) expect(cmds.some((c) => c.includes('rm -rf') && c.includes('relay-v0.1.0'))).toBe(true) @@ -327,6 +732,32 @@ describe('gcOldRelayVersions', () => { expect(cmds.some((c) => c.includes('rm -rf'))).toBe(false) }) + it('keeps a sibling when the install-lock probe fails or returns unexpected output', async () => { + mockExec + .mockResolvedValueOnce('relay-0.1.0+aaa\nrelay-0.1.0+ccc\n') + .mockRejectedValueOnce(new Error('lock probe failed')) + .mockResolvedValueOnce('INCONCLUSIVE') + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + expect(mockExec).toHaveBeenCalledTimes(3) + }) + + it('keeps a sibling when the relay-liveness probe fails or returns unexpected output', async () => { + mockExec + .mockResolvedValueOnce('relay-0.1.0+aaa\nrelay-0.1.0+ccc\n') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockRejectedValueOnce(new Error('liveness probe failed')) + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('INCONCLUSIVE') + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + expect(mockExec).toHaveBeenCalledTimes(7) + }) + it('probes Windows GC liveness by connecting to named pipes, not process command lines', async () => { const windows = getRemoteHostPlatform('win32-x64') mockExec.mockResolvedValueOnce('relay-0.1.0+aaa\n') @@ -334,6 +765,14 @@ describe('gcOldRelayVersions', () => { .mockResolvedValueOnce('OPEN') .mockResolvedValueOnce('COMPLETE') .mockResolvedValueOnce('WAITING') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('WAITING') + .mockResolvedValueOnce('OWNED') + .mockResolvedValueOnce('MOVED') + .mockResolvedValueOnce('RELEASED') .mockResolvedValueOnce('') await gcOldRelayVersions( @@ -366,14 +805,113 @@ describe('gcOldRelayVersions', () => { mockExec .mockResolvedValueOnce('OPEN') .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OK') .mockResolvedValueOnce('') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OWNED') + .mockResolvedValueOnce('MOVED') + .mockResolvedValueOnce('RELEASED') .mockResolvedValueOnce('') await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') const cmds = mockExec.mock.calls.map(([, c]) => c) - const rmCmds = cmds.filter((c) => c.includes('rm -rf')) + const rmCmds = cmds.filter((c) => c.startsWith('rm') && c.includes('gc-tombstone')) expect(rmCmds).toHaveLength(1) expect(rmCmds[0]).toContain('relay-0.1.0+aaa') expect(rmCmds[0]).not.toContain('logs') expect(rmCmds[0]).not.toContain('backup') }) + + it('does not remove a candidate claimed by repair after the initial lock probe', async () => { + mockExec + .mockResolvedValueOnce('relay-0.1.0+aaa\n') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') // socket probe + .mockResolvedValueOnce('OK') // GC sibling claim + .mockResolvedValueOnce('') // write claim ownership token + .mockResolvedValueOnce('LOCKED') // repair won before the safety recheck + .mockResolvedValueOnce('0') + .mockResolvedValueOnce('RELEASED') // release GC sibling claim + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect( + commands.some( + (command) => + command.startsWith("rm -rf '/home/u/.orca-remote/relay-0.1.0+aaa'") && + !command.includes('.install-lock') + ) + ).toBe(false) + }) + + it('does not move a candidate after losing the sibling claim generation', async () => { + mockExec + .mockResolvedValueOnce('relay-0.1.0+aaa\n') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('LOST') + .mockResolvedValueOnce('LOST') + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands.some((command) => command.startsWith('mv '))).toBe(false) + }) + + it('releases the GC claim after a confirmed move failure', async () => { + mockExec + .mockResolvedValueOnce('relay-0.1.0+aaa\n') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OWNED') + .mockRejectedValueOnce(new Error('move failed')) + .mockResolvedValueOnce('RELEASED') + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + const lastCommand = mockExec.mock.calls.at(-1)?.[1] ?? '' + expect(lastCommand).toContain('rm -rf') + expect(lastCommand).toContain('relay-0.1.0+aaa.gc-claim') + }) + + it('keeps the GC claim when remote move termination is unconfirmed', async () => { + const unconfirmed = Object.assign(new Error('move timed out'), { + sshChannelCloseConfirmed: false + }) + mockExec + .mockResolvedValueOnce('relay-0.1.0+aaa\n') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OK') + .mockResolvedValueOnce('') + .mockResolvedValueOnce('OPEN') + .mockResolvedValueOnce('COMPLETE') + .mockResolvedValueOnce('DEAD') + .mockResolvedValueOnce('OWNED') + .mockRejectedValueOnce(unconfirmed) + + await gcOldRelayVersions(conn, '/home/u', '/home/u/.orca-remote/relay-0.1.0+bbb') + + const releaseCommands = mockExec.mock.calls + .map(([, command]) => command) + .filter((command) => command.includes('relay-0.1.0+aaa.gc-claim') && command.startsWith('rm')) + expect(releaseCommands).toHaveLength(0) + }) }) diff --git a/src/main/ssh/ssh-relay-versioned-install.ts b/src/main/ssh/ssh-relay-versioned-install.ts index a669f0b78..b211fec4d 100644 --- a/src/main/ssh/ssh-relay-versioned-install.ts +++ b/src/main/ssh/ssh-relay-versioned-install.ts @@ -15,16 +15,21 @@ import { existsSync, readFileSync } from 'node:fs' import type { SshConnection } from './ssh-connection' import { RELAY_REMOTE_DIR } from './relay-protocol' import { execCommand } from './ssh-relay-deploy-helpers' +import { probeInstallLockExistsCommand } from './ssh-relay-install-lock-commands' +import { isRelayInstallLockStale, RELAY_INSTALL_LOCK_NAME } from './ssh-relay-install-lock' +import { + isRelayGcClaimOwned, + releaseRelayGcClaimWithRetry, + tryAcquireRelayGcClaim +} from './ssh-relay-gc-claim' +import { cleanupRelayGcTombstones } from './ssh-relay-gc-tombstone' import { - acquireInstallLockParentCommand, listRelayBaseDirsCommand, - lockMtimeEpochCommand, - probeDirectoryExistsCommand, + moveRemoteTreeCommand, probeFileExistsCommand, probeRelayInstalledCommand, relayLivenessProbeCommand, removeRemoteTreeCommand, - tryCreateInstallLockCommand, writeRemoteEmptyFileCommand } from './ssh-remote-commands' import { @@ -36,6 +41,7 @@ import { type RemotePathFlavor } from './ssh-remote-platform' import { windowsRelayPipePathsForSocketName } from './ssh-relay-endpoints' +import { isUnconfirmedSshCommandTermination } from './ssh-relay-exec-command' import { isSshSessionLimitError } from './ssh-session-limit-error' // Why: the GC pass and the version-dir parser must agree on what counts as a @@ -51,18 +57,7 @@ const RELAY_VERSION_DIR_REGEX = /^relay-(v?\d+\.\d+\.\d+(\+[0-9a-f]+)?)$/ // living on remote disks forever. const LEGACY_RELAY_DIR_REGEX = /^relay-v\d+\.\d+\.\d+$/ -const INSTALL_LOCK_NAME = '.install-lock' const INSTALL_COMPLETE_NAME = '.install-complete' - -const INSTALL_LOCK_POLL_MS = 1_000 -const INSTALL_LOCK_TIMEOUT_MS = 120_000 -// Why: a stale lock dir from a crashed installer must be recoverable without -// user intervention. After the timeout we check the lock's mtime; if it's -// older than this window the previous installer is assumed dead and we steal -// the lock. 2 minutes is well above a normal `npm install node-pty` runtime -// (10–60s on slow hosts) so a slow concurrent installer is not falsely -// declared dead. -const INSTALL_LOCK_STALE_MS = 120_000 const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') type RelayInstalledProbeOptions = { @@ -145,6 +140,7 @@ export async function isRelayAlreadyInstalled( ) return probe.trim() === 'OK' } catch (err) { + options?.signal?.throwIfAborted() if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err } @@ -153,101 +149,27 @@ export async function isRelayAlreadyInstalled( } /** - * Acquire the per-version install lock via atomic `mkdir`. Returns when the - * caller owns the lock; throws if the lock could not be acquired within - * INSTALL_LOCK_TIMEOUT_MS even after one stale-lock recovery attempt. - * - * Why mkdir: POSIX `mkdir` is atomic and fails with EEXIST if the dir already - * exists, giving us a free mutex. A second concurrent caller polls and - * eventually either acquires the lock or steals it after the stale window. - */ -export async function acquireInstallLock( - conn: SshConnection, - remoteRelayDir: string, - host: RemoteHostPlatform = DEFAULT_REMOTE_HOST -): Promise { - const lockDir = joinRemotePath(host, remoteRelayDir, INSTALL_LOCK_NAME) - // Why: the parent dir may not exist yet on a first install. mkdir -p is - // safe to run multiple times — it's a no-op if the dir already exists. - await execHostCommand(conn, host, acquireInstallLockParentCommand(host, remoteRelayDir)) - - let start = Date.now() - let recoveredOnce = false - while (true) { - try { - const result = await execHostCommand(conn, host, tryCreateInstallLockCommand(host, lockDir)) - if (result.trim().endsWith('OK')) { - return - } - } catch { - /* mkdir failed with non-zero — fall through to BUSY treatment */ - } - if (Date.now() - start >= INSTALL_LOCK_TIMEOUT_MS) { - if (recoveredOnce) { - throw new Error( - `Could not acquire relay install lock at ${lockDir} after ${ - INSTALL_LOCK_TIMEOUT_MS / 1000 - }s; another install is in progress or the lock is wedged.` - ) - } - // Stale-lock recovery: if the lock dir's mtime is older than the stale - // window, the previous installer crashed. Steal it and retry once, - // resetting the timeout window so a single post-recovery race doesn't - // immediately exhaust the budget. - const ageOk = await isLockStale(conn, lockDir, host) - if (ageOk) { - console.warn(`[ssh-relay] Stealing stale install lock at ${lockDir}`) - await execHostCommand(conn, host, removeRemoteTreeCommand(host, lockDir)).catch(() => {}) - recoveredOnce = true - start = Date.now() - continue - } - throw new Error( - `Could not acquire relay install lock at ${lockDir} after ${ - INSTALL_LOCK_TIMEOUT_MS / 1000 - }s and the lock is not yet stale.` - ) - } - await new Promise((r) => setTimeout(r, INSTALL_LOCK_POLL_MS)) - } -} - -async function isLockStale( - conn: SshConnection, - lockDir: string, - host: RemoteHostPlatform = DEFAULT_REMOTE_HOST -): Promise { - try { - // Why: `stat` flags differ between GNU coreutils (Linux) and BSD (macOS). - // We try GNU first, then BSD; both produce a Unix epoch in seconds on - // stdout. If both fail we conservatively treat the lock as not stale. - const out = await execHostCommand(conn, host, lockMtimeEpochCommand(host, lockDir)) - const mtimeSec = Number.parseInt(out.trim(), 10) - if (!Number.isFinite(mtimeSec)) { - return false - } - const ageMs = Date.now() - mtimeSec * 1000 - return ageMs > INSTALL_LOCK_STALE_MS - } catch { - return false - } -} - -/** - * Mark the install as complete and release the lock. Sentinel ordering is: - * write `.install-complete` FIRST, then remove `.install-lock`. This ensures - * a sibling dir is never observed by GC as "complete but locked", which - * would lead GC to skip a recoverable dir indefinitely. + * Mark the install as complete, then normally release the lock. Deploy keeps + * the lock through first launch so cross-version GC cannot move the directory + * between finalization and daemon liveness becoming observable. */ export async function finalizeInstall( conn: SshConnection, remoteRelayDir: string, - host: RemoteHostPlatform = DEFAULT_REMOTE_HOST + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + options?: { signal?: AbortSignal; releaseLock?: boolean } ): Promise { const sentinel = joinRemotePath(host, remoteRelayDir, INSTALL_COMPLETE_NAME) - const lock = joinRemotePath(host, remoteRelayDir, INSTALL_LOCK_NAME) - await execHostCommand(conn, host, writeRemoteEmptyFileCommand(host, sentinel)) - await execHostCommand(conn, host, removeRemoteTreeCommand(host, lock)).catch(() => {}) + const lock = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) + await execHostCommand(conn, host, writeRemoteEmptyFileCommand(host, sentinel), { + signal: options?.signal + }) + if (options?.releaseLock !== false) { + await execHostCommand(conn, host, removeRemoteTreeCommand(host, lock), { + signal: options?.signal + }).catch(() => {}) + } + options?.signal?.throwIfAborted() } /** @@ -260,7 +182,7 @@ export async function abandonInstall( remoteRelayDir: string, host: RemoteHostPlatform = DEFAULT_REMOTE_HOST ): Promise { - const lock = joinRemotePath(host, remoteRelayDir, INSTALL_LOCK_NAME) + const lock = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) await execHostCommand(conn, host, removeRemoteTreeCommand(host, lock)).catch(() => {}) } @@ -295,10 +217,14 @@ export async function gcOldRelayVersions( } catch { return } - const candidates = listing + const entries = listing .split('\n') .map((s) => s.trim()) .filter(Boolean) + + await cleanupRelayGcTombstones(conn, baseDir, entries, host) + + const candidates = entries .filter((name) => RELAY_VERSION_DIR_REGEX.test(name)) .filter((name) => name !== currentDirName) @@ -316,7 +242,47 @@ export async function gcOldRelayVersions( kept.push(name) continue } - await execHostCommand(conn, host, removeRemoteTreeCommand(host, dir)) + // Why: the claim is a sibling, so it survives moving/deleting the + // candidate and lets installers back out before mutating the old path. + const gcClaimToken = await tryAcquireRelayGcClaim(conn, dir, host) + if (!gcClaimToken) { + kept.push(name) + continue + } + let preserveGcClaim = false + let gcClaimReleaseNeeded = true + try { + // Recheck under the stable claim. New installers probe the claim both + // before and after creating their in-tree lock, closing both orders. + if (!(await isCandidateSafeToRemove(conn, dir, name, host, options))) { + kept.push(name) + continue + } + if (!(await isRelayGcClaimOwned(conn, dir, gcClaimToken, host))) { + kept.push(name) + continue + } + const tombstone = `${dir}.gc-tombstone.${process.pid}.${Date.now()}` + const moved = await execHostCommand(conn, host, moveRemoteTreeCommand(host, dir, tombstone)) + if (moved.trim() !== 'MOVED') { + kept.push(name) + continue + } + // Once renamed, a fresh install at the original path is isolated from + // deletion of the tombstone, so the sibling claim can be released. + const release = await releaseRelayGcClaimWithRetry(conn, dir, gcClaimToken, host) + gcClaimReleaseNeeded = release === 'unknown' + await execHostCommand(conn, host, removeRemoteTreeCommand(host, tombstone)) + } catch (err) { + if (isUnconfirmedSshCommandTermination(err)) { + preserveGcClaim = true + } + throw err + } finally { + if (!preserveGcClaim && gcClaimReleaseNeeded) { + await releaseRelayGcClaimWithRetry(conn, dir, gcClaimToken, host) + } + } removed.push(name) } catch (err) { console.warn( @@ -346,23 +312,28 @@ async function isCandidateSafeToRemove( ): Promise { const isLegacy = LEGACY_RELAY_DIR_REGEX.test(name) - const lockDir = joinRemotePath(host, dir, INSTALL_LOCK_NAME) - const lockProbe = await execHostCommand( - conn, - host, - probeDirectoryExistsCommand(host, lockDir) - ).catch(() => 'OPEN') - const locked = lockProbe.trim() === 'LOCKED' + const lockDir = joinRemotePath(host, dir, RELAY_INSTALL_LOCK_NAME) + let lockProbe: string + try { + lockProbe = await execHostCommand(conn, host, probeInstallLockExistsCommand(host, lockDir)) + } catch { + return false + } + const lockState = lockProbe.trim() + if (lockState !== 'OPEN' && lockState !== 'LOCKED') { + return false + } + const locked = lockState === 'LOCKED' if (locked) { // Why: a locked dir is normally unsafe to remove — but a STALE lock - // (mtime older than INSTALL_LOCK_STALE_MS) means the previous installer + // (remote age older than INSTALL_LOCK_STALE_MS) means the previous installer // crashed and is never coming back. If the dir also has the // .install-complete sentinel (touch succeeded but the rm-lock at the // end of finalizeInstall failed), removing the dir is safe — no // installer is racing us, and the daemon (if any) keeps running off // its already-loaded code regardless of disk state. - if (!(await isLockStale(conn, lockDir, host))) { + if (!(await isRelayInstallLockStale(conn, lockDir, host))) { return false } process.stderr.write?.(`[ssh-relay] GC: lock at ${lockDir} is stale; treating as recoverable\n`) @@ -419,8 +390,10 @@ async function hasLiveRelaySocket( host, relayLivenessProbeCommand(host, dir, windowsOptions) ) - return out.includes('ALIVE') + const state = out.trim() + return state !== 'DEAD' && state !== 'WAITING' } catch { - return false + // Why: an inconclusive liveness probe must never authorize deletion. + return true } } diff --git a/src/main/ssh/ssh-remote-commands.test.ts b/src/main/ssh/ssh-remote-commands.test.ts index 952daf9ed..96cc89255 100644 --- a/src/main/ssh/ssh-remote-commands.test.ts +++ b/src/main/ssh/ssh-remote-commands.test.ts @@ -1,24 +1,109 @@ +import { execFileSync, spawn, spawnSync } from 'node:child_process' +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + rmSync, + statSync, + utimesSync +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { describe, expect, it } from 'vitest' +import { + lockAgeSecondsCommand, + tryCreateInstallLockCommand, + tryStealInstallLockCommand +} from './ssh-relay-install-lock-commands' import { commandInRemoteDirectory, commandWithNodePath, listRelayBaseDirsCommand, makeRemoteDirectoryCommand, + moveRemoteTreeCommand, + probeDirectoryExistsCommand, probeRelayInstalledCommand, readRemoteHomeCommand, - relayLivenessProbeCommand, - tryCreateInstallLockCommand + relayLivenessProbeCommand } from './ssh-remote-commands' import { getRemoteHostPlatform } from './ssh-remote-platform' const posix = getRemoteHostPlatform('linux-x64') const windows = getRemoteHostPlatform('win32-x64') +const powerShellExecutable = ( + process.platform === 'win32' ? ['pwsh.exe', 'powershell.exe'] : ['pwsh'] +).find((candidate) => { + const result = spawnSync(candidate, ['-NoProfile', '-NonInteractive', '-Command', 'exit 0'], { + stdio: 'ignore' + }) + return result.status === 0 +}) +const powerShell51Executable = + process.platform === 'win32' && + spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', 'exit 0'], { + stdio: 'ignore' + }).status === 0 + ? 'powershell.exe' + : undefined function decodePowerShellCommand(command: string): string { const match = command.match(/-EncodedCommand\s+([A-Za-z0-9+/=]+)/) return match ? Buffer.from(match[1], 'base64').toString('utf16le') : '' } +function runShellCommand(command: string): Promise { + return new Promise((resolve, reject) => { + const child = spawn('/bin/sh', ['-c', command], { + stdio: ['ignore', 'pipe', 'pipe'] + }) + let stdout = '' + let stderr = '' + child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') + child.stdout.on('data', (chunk) => { + stdout += chunk + }) + child.stderr.on('data', (chunk) => { + stderr += chunk + }) + child.on('error', reject) + child.on('close', (code) => { + if (code === 0) { + resolve(stdout) + return + } + reject(new Error(`shell exited ${code}: ${stderr}`)) + }) + }) +} + +function runPowerShellCommand(executable: string, script: string): Promise { + return new Promise((resolve, reject) => { + const child = spawn(executable, ['-NoProfile', '-NonInteractive', '-Command', script], { + stdio: ['ignore', 'pipe', 'pipe'] + }) + let stdout = '' + let stderr = '' + child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') + child.stdout.on('data', (chunk) => { + stdout += chunk + }) + child.stderr.on('data', (chunk) => { + stderr += chunk + }) + child.on('error', reject) + child.on('close', (code) => { + if (code === 0) { + resolve(stdout) + return + } + reject(new Error(`PowerShell exited ${code}: ${stderr}`)) + }) + }) +} + describe('ssh remote command builders', () => { it('keeps POSIX deploy commands POSIX-native', () => { expect(readRemoteHomeCommand(posix)).toBe('echo $HOME') @@ -34,7 +119,7 @@ describe('ssh remote command builders', () => { expect(probeRelayInstalledCommand(windows, 'C:/Users/me/relay')).toContain('-EncodedCommand') }) - it('uses -Path for Windows New-Item commands', () => { + it('uses a legacy-visible Windows lock directory with an exclusive owner file', () => { const mkdirScript = decodePowerShellCommand( makeRemoteDirectoryCommand(windows, 'C:/Users/me/.orca-remote') ) @@ -43,9 +128,31 @@ describe('ssh remote command builders', () => { ) expect(mkdirScript).toContain('New-Item -ItemType Directory -Force -Path') - expect(lockScript).toContain('New-Item -ItemType Directory -Path') + expect(lockScript).toContain('[System.IO.FileMode]::CreateNew') + expect(lockScript).toContain('[System.IO.FileShare]::None') + expect(lockScript).toContain('New-Item -ItemType Directory -Path $lock') + expect(lockScript).toContain("Join-Path $lock '.owner'") expect(mkdirScript).not.toContain('New-Item -ItemType Directory -Force -LiteralPath') - expect(lockScript).not.toContain('New-Item -ItemType Directory -LiteralPath') + }) + + it('moves GC candidates to a sibling tombstone with host-native commands', () => { + expect(moveRemoteTreeCommand(posix, '/relay/old', '/relay/old.gc-tombstone')).toContain( + "mv '/relay/old' '/relay/old.gc-tombstone'" + ) + const windowsScript = decodePowerShellCommand( + moveRemoteTreeCommand(windows, 'C:/Users/me/relay/old', 'C:/Users/me/relay/old.gc-tombstone') + ) + expect(windowsScript).toContain('Move-Item -LiteralPath') + expect(windowsScript).toContain("-Destination 'C:/Users/me/relay/old.gc-tombstone'") + expect(windowsScript).toContain("'MOVED'") + }) + + it('emits an explicit POSIX liveness result so GC can fail closed', () => { + const command = relayLivenessProbeCommand(posix, '/home/u/.orca-remote/relay-0.1.0') + + expect(command).toContain('state=DEAD') + expect(command).toContain('[ -S "$f" ] && state=ALIVE') + expect(command).toContain('echo "$state"') }) it('uses named pipe try-connect liveness for Windows GC', () => { @@ -96,11 +203,227 @@ describe('ssh remote command builders', () => { tryCreateInstallLockCommand(windows, 'C:/Users/me/.orca-remote/relay/.install-lock') ) - expect(script).toContain('$ErrorActionPreference = "Stop"; try {') + expect(script).toContain('$stream = $null; try {') expect(script).toContain("} catch { 'BUSY' }") expect(script).not.toContain('}; catch') }) + it('computes install-lock age on the remote host clock', () => { + const posixCommand = lockAgeSecondsCommand(posix, '/home/me/.orca-remote/relay/.install-lock') + const windowsScript = decodePowerShellCommand( + lockAgeSecondsCommand(windows, 'C:/Users/me/.orca-remote/relay/.install-lock') + ) + + expect(posixCommand).toContain('date +%s') + expect(posixCommand).toContain('echo "$age"') + expect(windowsScript).toContain('[DateTimeOffset]::UtcNow.ToUnixTimeSeconds()') + expect(windowsScript).toContain('Write-Output ($now - $mtime)') + }) + + it('serializes stale recovery with unbounded numbered sibling claims', () => { + const posixCommand = tryStealInstallLockCommand( + posix, + '/home/me/.orca-remote/relay/.install-lock', + 20 * 60 + ) + const windowsScript = decodePowerShellCommand( + tryStealInstallLockCommand(windows, 'C:/Users/me/.orca-remote/relay/.install-lock', 20 * 60) + ) + + expect(posixCommand).toContain('.install-lock') + expect(posixCommand).toContain('.install-lock.steal') + expect(posixCommand).toContain('steal_generation') + expect(posixCommand).toContain('mtime=${lock_key%%:*}') + expect(posixCommand).toContain('-gt 1200') + expect(posixCommand).toContain('current_mtime') + expect(posixCommand).toContain('steal_generation + 1') + expect(posixCommand).not.toContain('.next.') + expect(posixCommand).toContain('trap') + expect(windowsScript).toContain('$lock.steal') + expect(windowsScript).toContain('$stealGeneration++') + expect(windowsScript).toContain('-gt 1200') + expect(windowsScript).toContain('$currentIdentity -eq $lockIdentity') + expect(windowsScript).toContain('[System.IO.FileMode]::CreateNew') + expect(windowsScript).not.toContain('.next.') + expect(windowsScript).toContain('finally') + }) + + it.runIf(powerShellExecutable)( + 'emits a parseable Windows stale-lock recovery command', + () => { + const script = decodePowerShellCommand( + tryStealInstallLockCommand( + windows, + 'C:/Users/orca-missing/.orca-remote/relay/.install-lock', + 20 * 60 + ) + ) + const result = spawnSync( + powerShellExecutable!, + ['-NoProfile', '-NonInteractive', '-Command', script], + { encoding: 'utf8' } + ) + + expect(result.status, result.stderr).toBe(0) + expect(result.stdout.trim()).toBe('BUSY') + }, + 15_000 + ) + + it.runIf(powerShell51Executable)( + 'lets only one Windows PowerShell 5.1 caller acquire an install lock', + async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-windows-race-')) + try { + const lockPath = join(root, '.install-lock') + const script = decodePowerShellCommand(tryCreateInstallLockCommand(windows, lockPath)) + const outputs = await Promise.all( + Array.from({ length: 16 }, () => runPowerShellCommand(powerShell51Executable!, script)) + ) + + expect(outputs.filter((output) => output.trim().endsWith('OK'))).toHaveLength(1) + expect(statSync(lockPath).isDirectory()).toBe(true) + expect(statSync(join(lockPath, '.owner')).isFile()).toBe(true) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }, + 30_000 + ) + + it.runIf(powerShell51Executable)( + 'recovers a stale Windows lock past abandoned steal generations', + async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-windows-stale-')) + try { + const lockPath = join(root, '.install-lock') + mkdirSync(lockPath) + const staleDate = new Date(Date.now() - 60 * 60_000) + utimesSync(lockPath, staleDate, staleDate) + for (let i = 0; i < 12; i++) { + const orphan = `${lockPath}.steal.${i}` + mkdirSync(orphan) + utimesSync(orphan, staleDate, staleDate) + } + const script = decodePowerShellCommand( + tryStealInstallLockCommand(windows, lockPath, 20 * 60) + ) + + const output = await runPowerShellCommand(powerShell51Executable!, script) + + expect(output.trim()).toBe('OK') + expect(statSync(lockPath).isDirectory()).toBe(true) + expect(statSync(join(lockPath, '.owner')).isFile()).toBe(true) + expect(readdirSync(root).filter((name) => name.includes('.steal.'))).toHaveLength(0) + expect(readdirSync(root).filter((name) => name.includes('.tombstone.'))).toHaveLength(0) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }, + 15_000 + ) + + it.runIf(powerShell51Executable)( + 'lets only one Windows PowerShell 5.1 caller replace a stale lock', + async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-windows-steal-race-')) + try { + const lockPath = join(root, '.install-lock') + mkdirSync(lockPath) + const staleDate = new Date(Date.now() - 60 * 60_000) + utimesSync(lockPath, staleDate, staleDate) + const script = decodePowerShellCommand( + tryStealInstallLockCommand(windows, lockPath, 20 * 60) + ) + + const outputs = await Promise.all( + Array.from({ length: 16 }, () => runPowerShellCommand(powerShell51Executable!, script)) + ) + + expect(outputs.filter((output) => output.trim().endsWith('OK'))).toHaveLength(1) + expect(statSync(lockPath).isDirectory()).toBe(true) + expect(statSync(join(lockPath, '.owner')).isFile()).toBe(true) + expect(readdirSync(root).filter((name) => name.includes('.tombstone.'))).toHaveLength(0) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }, + 30_000 + ) + + it.runIf(powerShellExecutable)( + 'keeps a new Windows lock visible to the previous directory-only GC probe', + async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-windows-compat-')) + try { + const lockPath = join(root, '.install-lock') + const acquire = decodePowerShellCommand(tryCreateInstallLockCommand(windows, lockPath)) + const legacyProbe = decodePowerShellCommand(probeDirectoryExistsCommand(windows, lockPath)) + + await expect(runPowerShellCommand(powerShellExecutable!, acquire)).resolves.toMatch(/OK/) + await expect(runPowerShellCommand(powerShellExecutable!, legacyProbe)).resolves.toMatch( + /LOCKED/ + ) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }, + 15_000 + ) + + it.runIf(process.platform !== 'win32')( + 'recovers and cleans more than eight orphaned numbered steal claims', + () => { + const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-')) + try { + const lockDir = join(root, '.install-lock') + mkdirSync(lockDir) + const staleDate = new Date(Date.now() - 60 * 60_000) + utimesSync(lockDir, staleDate, staleDate) + const lockMtimeSeconds = Math.floor(statSync(lockDir).mtimeMs / 1000) + for (let i = 0; i < 12; i++) { + const orphan = `${lockDir}.steal.${i}` + mkdirSync(orphan) + utimesSync(orphan, staleDate, staleDate) + } + + const command = tryStealInstallLockCommand(posix, lockDir, 20 * 60) + const output = execFileSync('/bin/sh', ['-c', command], { encoding: 'utf8' }) + + expect(output.trim()).toBe('OK') + expect(existsSync(lockDir)).toBe(true) + expect(readdirSync(root).filter((name) => name.includes('.steal.'))).toHaveLength(0) + expect(Math.floor(statSync(lockDir).mtimeMs / 1000)).toBeGreaterThan(lockMtimeSeconds) + } finally { + rmSync(root, { recursive: true, force: true }) + } + } + ) + + it.runIf(process.platform !== 'win32')( + 'lets only one POSIX caller move and recreate a stale install lock', + async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-race-')) + try { + const lockDir = join(root, '.install-lock') + mkdirSync(lockDir) + const staleDate = new Date(Date.now() - 60 * 60_000) + utimesSync(lockDir, staleDate, staleDate) + const command = tryStealInstallLockCommand(posix, lockDir, 20 * 60) + const outputs = await Promise.all( + Array.from({ length: 64 }, () => runShellCommand(command)) + ) + const okCount = outputs.filter((output) => output.trim().endsWith('OK')).length + + expect(okCount).toBe(1) + expect(existsSync(lockDir)).toBe(true) + expect(readdirSync(root).some((name) => name.includes('.tombstone'))).toBe(false) + } finally { + rmSync(root, { recursive: true, force: true }) + } + } + ) + it('makes Windows remote directory changes fail before running scoped commands', () => { const scopedCommand = decodePowerShellCommand( commandInRemoteDirectory(windows, 'C:/Users/me/.orca-remote/relay-0.1.0', "'READY'") diff --git a/src/main/ssh/ssh-remote-commands.ts b/src/main/ssh/ssh-remote-commands.ts index 339f4aef5..b70761f8d 100644 --- a/src/main/ssh/ssh-remote-commands.ts +++ b/src/main/ssh/ssh-remote-commands.ts @@ -45,6 +45,24 @@ export function removeRemoteTreeCommand(host: RemoteHostPlatform, remotePath: st ) } +export function moveRemoteTreeCommand( + host: RemoteHostPlatform, + sourcePath: string, + destinationPath: string +): string { + if (!isWindowsRemoteHost(host)) { + return `mv ${shellEscape(sourcePath)} ${shellEscape(destinationPath)} 2>&1 && echo MOVED || echo BUSY` + } + return powerShellCommand( + [ + 'try {', + `Move-Item -LiteralPath ${powerShellLiteral(sourcePath)} -Destination ${powerShellLiteral(destinationPath)} -ErrorAction Stop`, + "'MOVED'", + `} catch { 'BUSY' }` + ].join('; ') + ) +} + export function writeRemoteEmptyFileCommand(host: RemoteHostPlatform, remotePath: string): string { if (!isWindowsRemoteHost(host)) { return `touch ${shellEscape(remotePath)}` @@ -81,36 +99,6 @@ export function probeRelayInstalledCommand( ) } -export function acquireInstallLockParentCommand( - host: RemoteHostPlatform, - remoteRelayDir: string -): string { - return makeRemoteDirectoryCommand(host, remoteRelayDir) -} - -export function tryCreateInstallLockCommand(host: RemoteHostPlatform, lockDir: string): string { - if (!isWindowsRemoteHost(host)) { - return `mkdir ${shellEscape(lockDir)} 2>&1 && echo OK || echo BUSY` - } - // New-Item has no -LiteralPath parameter; using it breaks stock Windows PowerShell. - return powerShellCommand( - `$ErrorActionPreference = "Stop"; try { $null = New-Item -ItemType Directory -Path ${powerShellLiteral(lockDir)}; 'OK' } catch { 'BUSY' }` - ) -} - -export function lockMtimeEpochCommand(host: RemoteHostPlatform, lockDir: string): string { - if (!isWindowsRemoteHost(host)) { - return `stat -c %Y ${shellEscape(lockDir)} 2>/dev/null || stat -f %m ${shellEscape(lockDir)} 2>/dev/null || echo` - } - return powerShellCommand( - [ - `$item = Get-Item -LiteralPath ${powerShellLiteral(lockDir)} -ErrorAction Stop`, - '$dto = [DateTimeOffset]$item.LastWriteTimeUtc', - 'Write-Output $dto.ToUnixTimeSeconds()' - ].join('; ') - ) -} - export function listRelayBaseDirsCommand(host: RemoteHostPlatform, baseDir: string): string { if (!isWindowsRemoteHost(host)) { return `ls -1 ${shellEscape(baseDir)} 2>/dev/null || true` @@ -155,9 +143,9 @@ export function relayLivenessProbeCommand( ): string { if (!isWindowsRemoteHost(host)) { return ( - `for f in ${shellEscape(dir)}/relay-*.sock ${shellEscape(dir)}/relay.sock; do ` + - `[ -S "$f" ] && echo ALIVE && break; ` + - 'done; true' + `state=DEAD; for f in ${shellEscape(dir)}/relay-*.sock ${shellEscape(dir)}/relay.sock; do ` + + `[ -S "$f" ] && state=ALIVE && break; ` + + 'done; echo "$state"' ) } if (!windowsOptions) { diff --git a/src/main/ssh/ssh-remote-node-resolution.ts b/src/main/ssh/ssh-remote-node-resolution.ts index 01d6a0c7d..a5041ed15 100644 --- a/src/main/ssh/ssh-remote-node-resolution.ts +++ b/src/main/ssh/ssh-remote-node-resolution.ts @@ -122,6 +122,7 @@ true if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err } + throwIfAborted(options) // Fall through to login shell. } return null @@ -167,6 +168,7 @@ async function tryResolveViaLoginShell( if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err } + throwIfAborted(options) // Fall through. } return null @@ -191,6 +193,7 @@ async function nodeMeetsVersionRequirement( if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err } + throwIfAborted(options) // Binary missing or fails to run — not usable. return false } @@ -240,6 +243,7 @@ async function resolveRemoteWindowsNodePath( if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err } + throwIfAborted(options) // Fall through to the shared error below. } @@ -262,6 +266,7 @@ async function windowsNodeMeetsVersionRequirement( if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err } + throwIfAborted(options) return false } } diff --git a/src/main/ssh/ssh-remote-platform-detection.ts b/src/main/ssh/ssh-remote-platform-detection.ts index f72b53843..44c31e617 100644 --- a/src/main/ssh/ssh-remote-platform-detection.ts +++ b/src/main/ssh/ssh-remote-platform-detection.ts @@ -11,30 +11,38 @@ import { powerShellCommand } from './ssh-remote-powershell' const PLATFORM_PROBE_MARKER = '__ORCA_REMOTE_PLATFORM__' export async function detectRemoteHostPlatform( - conn: SshConnection + conn: SshConnection, + options?: { signal?: AbortSignal } ): Promise { - const unamePlatform = await detectUnamePlatform(conn) + const unamePlatform = await detectUnamePlatform(conn, options?.signal) if (unamePlatform) { return getRemoteHostPlatform(unamePlatform) } - const windowsPlatform = await detectWindowsPlatform(conn) + const windowsPlatform = await detectWindowsPlatform(conn, options?.signal) return windowsPlatform ? getRemoteHostPlatform(windowsPlatform) : null } -async function detectUnamePlatform(conn: SshConnection): Promise { +async function detectUnamePlatform( + conn: SshConnection, + signal?: AbortSignal +): Promise { try { - const output = await execCommand( - conn, - // Why: Remote startup output may omit its trailing newline and must not absorb the marker. - `printf '\\n%s ' '${PLATFORM_PROBE_MARKER}'; uname -sm` - ) + // Why: Remote startup output may omit its trailing newline and must not absorb the marker. + const command = `printf '\\n%s ' '${PLATFORM_PROBE_MARKER}'; uname -sm` + const output = signal + ? await execCommand(conn, command, { signal }) + : await execCommand(conn, command) return parseRemotePlatformOutput(output) } catch { + signal?.throwIfAborted() return null } } -async function detectWindowsPlatform(conn: SshConnection): Promise { +async function detectWindowsPlatform( + conn: SshConnection, + signal?: AbortSignal +): Promise { try { const script = [ '$arch = $env:PROCESSOR_ARCHITECTURE', @@ -43,9 +51,13 @@ async function detectWindowsPlatform(conn: SshConnection): Promise arg.startsWith('ControlPath='))).toBe(true) + expect(args).toContain('ControlPersist=300') +} + type EventedProcess = EventEmitter & { stdin: EventEmitter & { write: ReturnType @@ -360,9 +370,7 @@ describe('spawnSystemSsh', () => { }) }) - expect(args).toContain('ControlMaster=auto') - expect(args.some((arg) => arg.startsWith('ControlPath='))).toBe(true) - expect(args).toContain('ControlPersist=300') + expectOrcaControlMasterArgs(args) expect(args).not.toContain('-S') }) @@ -374,9 +382,7 @@ describe('spawnSystemSsh', () => { }) }) - expect(args).toContain('ControlMaster=auto') - expect(args.some((arg) => arg.startsWith('ControlPath='))).toBe(true) - expect(args).toContain('ControlPersist=300') + expectOrcaControlMasterArgs(args) expect(args).not.toContain('-S') }) @@ -387,9 +393,7 @@ describe('spawnSystemSsh', () => { }) }) - expect(args).toContain('ControlMaster=auto') - expect(args.some((arg) => arg.startsWith('ControlPath='))).toBe(true) - expect(args).toContain('ControlPersist=300') + expectOrcaControlMasterArgs(args) expect(args).not.toContain('-S') }) @@ -414,9 +418,7 @@ describe('spawnSystemSsh', () => { resolvedConfig: createResolvedConfig() }) - expect(args).toContain('ControlMaster=auto') - expect(args.some((arg) => arg.startsWith('ControlPath='))).toBe(true) - expect(args).toContain('ControlPersist=300') + expectOrcaControlMasterArgs(args) expect(args).not.toContain('-S') }) @@ -432,10 +434,11 @@ describe('spawnSystemSsh', () => { it('adds keepalive options to Orca-owned ControlMaster connections', () => { const args = buildSshArgs(createTarget(), { resolvedConfig: createResolvedConfig() }) - expect(args).toContain('ControlMaster=auto') - expect(args).toContain('ControlPersist=300') - expect(args).toContain('ServerAliveInterval=15') - expect(args).toContain('ServerAliveCountMax=3') + expectOrcaControlMasterArgs(args) + if (process.platform !== 'win32') { + expect(args).toContain('ServerAliveInterval=15') + expect(args).toContain('ServerAliveCountMax=3') + } }) it('spawns a remote command through the system ssh target', () => { @@ -507,6 +510,15 @@ describe('spawnSystemSsh', () => { expect(mockProc.stdin.end).toHaveBeenCalledWith('contents') }) + it('marks a system command channel when local teardown is requested', () => { + const channel = spawnSystemSshCommand(createTarget(), 'npm install') + + channel.close() + + expect(channel._closeRequested).toBe(true) + expect(mockProc.kill).toHaveBeenCalledWith('SIGTERM') + }) + it('removes wrapped process listeners after command close', () => { const proc = createEventedProcess() spawnMock.mockReturnValue(proc) diff --git a/src/main/ssh/system-ssh-command.ts b/src/main/ssh/system-ssh-command.ts index 1f0ce5ed6..eab96245b 100644 --- a/src/main/ssh/system-ssh-command.ts +++ b/src/main/ssh/system-ssh-command.ts @@ -17,6 +17,7 @@ export type SystemSshProcess = { export type SystemSshCommandChannel = ClientChannel & { _process?: ChildProcess + _closeRequested?: boolean } type SystemSshCommandOptions = SshExecOptions & SystemSshBuildArgsOptions @@ -103,12 +104,14 @@ function wrapCommandProcess(proc: ChildProcess): SystemSshCommandChannel { stdin: NodeJS.WritableStream stderr: NodeJS.ReadableStream _process?: ChildProcess + _closeRequested?: boolean close: () => void } mutableChannel.stdin = proc.stdin! mutableChannel.stderr = proc.stderr! mutableChannel._process = proc mutableChannel.close = () => { + mutableChannel._closeRequested = true try { proc.kill('SIGTERM') } catch { diff --git a/src/main/ssh/system-ssh-operation-lifecycle.ts b/src/main/ssh/system-ssh-operation-lifecycle.ts index 76a2ae3fb..c9c442432 100644 --- a/src/main/ssh/system-ssh-operation-lifecycle.ts +++ b/src/main/ssh/system-ssh-operation-lifecycle.ts @@ -100,7 +100,13 @@ export async function awaitWithSystemSshAbort( // children to emit close after we've already signaled them. abortChildren() suppressLateOperationError = true - abortReject?.(createAbortError()) + abortReject?.( + Object.assign(createAbortError(), { + // The child was signaled, but this fast abort path intentionally does + // not wait for process exit; callers holding remote locks must retain them. + sshChannelCloseConfirmed: false + }) + ) } signal.addEventListener('abort', abort, { once: true }) if (signal.aborted) { diff --git a/src/relay/pty-handler.test.ts b/src/relay/pty-handler.test.ts index efab6bce5..8de7ec805 100644 --- a/src/relay/pty-handler.test.ts +++ b/src/relay/pty-handler.test.ts @@ -181,6 +181,29 @@ describe('PtyHandler', () => { expect(handler.activePtyCount).toBe(1) }) + it('normalizes a missing native binding as degraded node-pty availability', async () => { + mockPtySpawn.mockImplementationOnce(() => { + throw new Error( + 'Failed to load native module: conpty.node, checked: build/Release, prebuilds/win32-x64' + ) + }) + + await expect(dispatcher.callRequest('pty.spawn', {})).rejects.toThrow( + 'node-pty is not available on this remote host' + ) + expect(handler.activePtyCount).toBe(0) + }) + + it('preserves unrelated node-pty spawn failures', async () => { + mockPtySpawn.mockImplementationOnce(() => { + throw new Error('File not found: missing-shell.exe') + }) + + await expect(dispatcher.callRequest('pty.spawn', {})).rejects.toThrow( + 'File not found: missing-shell.exe' + ) + }) + it('atomically caps concurrent PTY spawn admission', async () => { const results = await Promise.allSettled( Array.from({ length: MAX_RELAY_PTY_SESSIONS + 1 }, () => diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index c1de6809c..fb0d38bf5 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -1,6 +1,8 @@ /* oxlint-disable max-lines */ import type { IPty } from 'node-pty' import type * as NodePty from 'node-pty' +import { existsSync } from 'node:fs' +import { join } from 'node:path' import { resolveWindowsGitBashShellPath } from '../main/git-bash' import { WINDOWS_GIT_BASH_SHELL } from '../shared/windows-terminal-shell' import type { RelayDispatcher, RequestContext } from './dispatcher' @@ -38,21 +40,11 @@ import { import { isTuiAgent } from '../shared/tui-agent-config' import { forceKillPosixPtyProcessGroups } from '../main/pty/posix-pty-process-groups' -// Why: node-pty is a native addon that may not be installed on the remote. -// Dynamic import keeps the require() lazy so loadPty() returns null gracefully -// when the native module is unavailable. The static type import lets vitest -// intercept it in tests. -let ptyModule: typeof NodePty | null = null -async function loadPty(): Promise { - if (ptyModule) { - return ptyModule - } - try { - ptyModule = await import('node-pty') - return ptyModule - } catch { - return null - } +function isMissingNodePtyNativeBinding(error: unknown): boolean { + return ( + error instanceof Error && + /Failed to load native module: (?:conpty|pty)\.node(?:,|$)/.test(error.message) + ) } type ManagedPty = { @@ -296,6 +288,9 @@ export class PtyHandler { private pendingCreationDrainResolvers = new Set<() => void>() private worktreeRemovalCoordinator: RelayPtyWorktreeRemovalCoordinator | null = null private disposePromise: Promise | null = null + private ptyModule: typeof NodePty | null = null + private ptyModuleLoadPromise: Promise | null = null + private reloadPtyModuleFromDisk = false // Why: external observers need to drop per-pane state when a PTY exits. // Today the relay composes multiple consumers (hook-server cache eviction // and plugin-overlay dir cleanup) into a single callback at the call site @@ -316,6 +311,55 @@ export class PtyHandler { this.registerHandlers() } + private async loadPty(): Promise { + if (this.ptyModule) { + return this.ptyModule + } + if (this.ptyModuleLoadPromise) { + return this.ptyModuleLoadPromise + } + this.ptyModuleLoadPromise = this.loadPtyUncached() + try { + return await this.ptyModuleLoadPromise + } finally { + this.ptyModuleLoadPromise = null + } + } + + private async loadPtyUncached(): Promise { + if (!this.reloadPtyModuleFromDisk) { + try { + this.ptyModule = await import('node-pty') + return this.ptyModule + } catch { + this.reloadPtyModuleFromDisk = true + } + } + // Why: the relay is launched from its install dir today, but module + // resolution must remain tied to the deployed bundle rather than cwd. + const moduleEntry = join(__dirname, 'node_modules', 'node-pty', 'lib', 'index.js') + if (!existsSync(moduleEntry)) { + return null + } + try { + this.ptyModule = require(moduleEntry) as typeof NodePty + return this.ptyModule + } catch { + return null + } + } + + private invalidatePtyModuleAfterBindingFailure(): void { + this.ptyModule = null + this.reloadPtyModuleFromDisk = true + const moduleRoot = join(__dirname, 'node_modules', 'node-pty') + for (const cachedPath of Object.keys(require.cache)) { + if (isPathInsideOrEqual(moduleRoot, cachedPath)) { + delete require.cache[cachedPath] + } + } + } + setGraceTimeMs(graceTimeMs: number): void { this.graceTimeMs = Math.max(0, Math.floor(graceTimeMs)) } @@ -741,7 +785,7 @@ export class PtyHandler { params: Record, context?: RequestContext ): Promise<{ id: string }> { - const pty = await loadPty() + const pty = await this.loadPty() if (!pty) { throw new Error('node-pty is not available on this remote host') } @@ -814,17 +858,28 @@ export class PtyHandler { // includes Homebrew, nvm, and user-installed CLIs (claude, codex, gh). // When overlays are injected, the launch wrapper keeps those paths after // user startup files re-export their defaults. - const term = pty.spawn(shell, shellLaunch.args, { - // Why: node-pty overwrites env.TERM with `name`; keep caller-selected - // terminal identities instead of losing them at the final spawn boundary. - name: spawnEnv.TERM ?? 'xterm-256color', - cols, - rows, - cwd, - // Why: relay shells inherit process.env; never let an ambient Orca marker - // enable shell-ready behavior unless this spawn explicitly requested it. - env: { ...spawnEnv, ORCA_SHELL_READY_MARKER: '0', ...shellLaunch.env } - }) + let term: IPty + try { + term = pty.spawn(shell, shellLaunch.args, { + // Why: node-pty overwrites env.TERM with `name`; keep caller-selected + // terminal identities instead of losing them at the final spawn boundary. + name: spawnEnv.TERM ?? 'xterm-256color', + cols, + rows, + cwd, + // Why: relay shells inherit process.env; never let an ambient Orca marker + // enable shell-ready behavior unless this spawn explicitly requested it. + env: { ...spawnEnv, ORCA_SHELL_READY_MARKER: '0', ...shellLaunch.env } + }) + } catch (error) { + // Why: Windows node-pty loads conpty.node only on first spawn, after the + // wrapper import succeeded. Keep that late failure on the degraded path. + if (isMissingNodePtyNativeBinding(error)) { + this.invalidatePtyModuleAfterBindingFailure() + throw new Error('node-pty is not available on this remote host') + } + throw error + } // Why: capture the renderer-supplied paneKey on the managed entry so the // exit listener can evict per-pane caches without the relay needing a @@ -1215,7 +1270,7 @@ export class PtyHandler { } private async reviveEntry(entry: SerializedPtyEntry): Promise { - const ptyMod = await loadPty() + const ptyMod = await this.loadPty() if (!ptyMod) { return } diff --git a/src/relay/subprocess-test-utils.ts b/src/relay/subprocess-test-utils.ts index 69a6e607a..b721f1796 100644 --- a/src/relay/subprocess-test-utils.ts +++ b/src/relay/subprocess-test-utils.ts @@ -25,7 +25,7 @@ export type RelayProcess = { export function spawnRelay( entryPath: string, args: string[] = [], - options: Pick = {} + options: Pick = {} ): RelayProcess { const proc = spawn('node', [entryPath, ...args], { stdio: ['pipe', 'pipe', 'pipe'], diff --git a/src/relay/subprocess.test.ts b/src/relay/subprocess.test.ts index ee20461dd..c772a8e51 100644 --- a/src/relay/subprocess.test.ts +++ b/src/relay/subprocess.test.ts @@ -1,5 +1,13 @@ import { afterAll, beforeAll, describe, expect, it, afterEach } from 'vitest' -import { existsSync, mkdtempSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' +import { + copyFileSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + unlinkSync, + writeFileSync +} from 'node:fs' import { rm } from 'node:fs/promises' import * as path from 'node:path' import { tmpdir } from 'node:os' @@ -46,9 +54,14 @@ afterAll(async () => { } }) -function spawn(args: string[] = [], env?: NodeJS.ProcessEnv): RelayProcess { +function spawnRelayEntry( + entryPath: string, + args: string[] = [], + env?: NodeJS.ProcessEnv +): RelayProcess { let relayArgs = args if (!args.includes('--sock-path')) { + // Why: Windows relays require a named pipe; filesystem socket paths fail with EACCES. const socketDir = mkdtempSync(path.join(tmpdir(), 'relay-sock-')) spawnedSocketDirs.push(socketDir) relayArgs = [ @@ -59,7 +72,11 @@ function spawn(args: string[] = [], env?: NodeJS.ProcessEnv): RelayProcess { path.join(socketDir, 'agent-hooks') ] } - return spawnRelay(relayEntry, relayArgs, env ? { env } : undefined) + return spawnRelay(entryPath, relayArgs, env ? { env } : undefined) +} + +function spawn(args: string[] = [], env?: NodeJS.ProcessEnv): RelayProcess { + return spawnRelayEntry(relayEntry, args, env) } function waitForChildExit( @@ -75,6 +92,22 @@ function waitForChildExit( }) } +function writeMockNodePty(root: string, source: string, withPackageEntry = false): void { + const nodePtyDir = path.join(root, 'node_modules', 'node-pty') + const libDir = path.join(nodePtyDir, 'lib') + mkdirSync(libDir, { recursive: true }) + if (withPackageEntry) { + writeFileSync(path.join(nodePtyDir, 'package.json'), '{"main":"lib/index.js"}\n') + } + writeFileSync(path.join(libDir, 'index.js'), source) +} + +const WORKING_NODE_PTY_MODULE = `module.exports = { spawn() { return { + pid: process.pid, + process: 'mock-shell', + onData() {}, onExit() {}, write() {}, resize() {}, kill() {}, clear() {} +} } }\n` + describe('Subprocess: Relay entry point', () => { let relay: RelayProcess | null = null let tmpDir: string @@ -103,6 +136,50 @@ describe('Subprocess: Relay entry point', () => { expect(readFileSync(relayEntry, 'utf8')).not.toContain('.toReversed(') }) + it('loads node-pty after an in-place dependency repair without restarting', async () => { + tmpDir = mkdtempSync(path.join(tmpdir(), 'relay-native-repair-')) + const repairedRelayEntry = path.join(tmpDir, 'relay.js') + copyFileSync(relayEntry, repairedRelayEntry) + + relay = spawnRelayEntry(repairedRelayEntry) + await relay.sentinelReceived + + const failedId = relay.send('pty.spawn', { cols: 80, rows: 24 }) + const failed = await relay.waitForResponse(failedId) + expect(failed.error?.message).toContain('node-pty is not available') + + writeMockNodePty(tmpDir, WORKING_NODE_PTY_MODULE) + + const repairedId = relay.send('pty.spawn', { cols: 80, rows: 24 }) + const repaired = await relay.waitForResponse(repairedId) + expect(repaired.error).toBeUndefined() + expect(repaired.result).toMatchObject({ id: 'pty-1' }) + }, 10_000) + + it('reloads node-pty after a late native binding failure without restarting', async () => { + tmpDir = mkdtempSync(path.join(tmpdir(), 'relay-native-late-repair-')) + const repairedRelayEntry = path.join(tmpDir, 'relay.js') + copyFileSync(relayEntry, repairedRelayEntry) + writeMockNodePty( + tmpDir, + `module.exports = { spawn() { throw new Error('Failed to load native module: conpty.node, checked: prebuilds/win32-x64') } }\n`, + true + ) + + relay = spawnRelayEntry(repairedRelayEntry) + await relay.sentinelReceived + + const failedId = relay.send('pty.spawn', { cols: 80, rows: 24 }) + const failed = await relay.waitForResponse(failedId) + expect(failed.error?.message).toContain('node-pty is not available') + + writeMockNodePty(tmpDir, WORKING_NODE_PTY_MODULE, true) + const repairedId = relay.send('pty.spawn', { cols: 80, rows: 24 }) + const repaired = await relay.waitForResponse(repairedId) + expect(repaired.error).toBeUndefined() + expect(repaired.result).toMatchObject({ id: 'pty-2' }) + }, 10_000) + it('responds to fs.stat over stdin/stdout', async () => { tmpDir = mkdtempSync(path.join(tmpdir(), 'relay-sub-')) writeFileSync(path.join(tmpDir, 'test.txt'), 'hello')