From a5213cc6eba5afe5007a08dd55408b208d6cac81 Mon Sep 17 00:00:00 2001 From: Glen Maddern Date: Tue, 14 Jul 2026 06:36:02 +0000 Subject: [PATCH 1/5] fix(ssh): repair unbuilt relay native deps --- src/main/ssh/ssh-relay-deploy.ts | 135 +++++++++++++----- .../ssh/ssh-relay-native-deps-install.test.ts | 70 ++++++++- 2 files changed, 171 insertions(+), 34 deletions(-) diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index 26501a8371b..95cbf26be5d 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -438,6 +438,8 @@ const RELAY_NATIVE_DEPS = { '@parcel/watcher': '2.5.6' } as const +const LOAD_RELAY_NATIVE_DEPS = 'require("node-pty"); require("@parcel/watcher")' + async function hasRequiredNativeDeps( conn: SshConnection, remoteDir: string, @@ -451,13 +453,13 @@ async function hasRequiredNativeDeps( 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(`${LOAD_RELAY_NATIVE_DEPS}; console.log("ORCA-NATIVE-DEPS-OK")`)} } 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 '${LOAD_RELAY_NATIVE_DEPS}; console.log("ORCA-NATIVE-DEPS-OK")' 2>/dev/null || echo MISSING)` ) const probe = await execHostCommand(conn, hostPlatform, command) return probe.includes('ORCA-NATIVE-DEPS-OK') @@ -509,12 +511,10 @@ async function installNativeDeps( hostPlatform: RemoteHostPlatform, nodePath: string ): 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 @@ -542,7 +542,9 @@ async function installNativeDeps( hostPlatform, nodePath, remoteDir, - `npm install --omit=dev --no-audit --no-fund ${Object.entries(RELAY_NATIVE_DEPS) + `npm install --ignore-scripts=false --omit=dev --no-audit --no-fund ${Object.entries( + RELAY_NATIVE_DEPS + ) .map(([dep, version]) => powerShellLiteral(`${dep}@${version}`)) .join(' ')}` ) @@ -550,7 +552,7 @@ async function installNativeDeps( hostPlatform, nodePath, remoteDir, - `npm install --omit=dev --no-audit --no-fund ${installArgs} 2>&1` + `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 @@ -576,54 +578,121 @@ 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) + + let probe = await probeInstalledNativeDeps(conn, remoteDir, hostPlatform, nodePath) + 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) + rebuilt = true + } catch (err) { + console.warn( + `[ssh-relay][NATIVE-DEPS-REBUILD-FAIL] npm rebuild native deps failed at ${remoteDir} (${platform}): ${(err as Error).message}` + ) + } + if (rebuilt) { + await makeNodePtySpawnHelperExecutable(conn, remoteDir, hostPlatform) + probe = await probeInstalledNativeDeps(conn, remoteDir, hostPlatform, nodePath) + } } - // 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)}` + ) + } +} + +async function rebuildNativeDeps( + conn: SshConnection, + remoteDir: string, + hostPlatform: RemoteHostPlatform, + nodePath: string +): 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(' ')}` ) : 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` + ) + await execHostCommand(conn, hostPlatform, command, { + timeoutMs: NATIVE_DEPS_INSTALL_TIMEOUT_MS + }) +} + +async function makeNodePtySpawnHelperExecutable( + conn: SshConnection, + remoteDir: string, + hostPlatform: RemoteHostPlatform +): 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, + `find ${shellEscape(joinRemotePath(hostPlatform, remoteDir, 'node_modules/node-pty/prebuilds'))} -name spawn-helper -exec chmod +x {} + 2>/dev/null; true` + ) +} + +async function probeInstalledNativeDeps( + conn: SshConnection, + remoteDir: string, + hostPlatform: RemoteHostPlatform, + nodePath: string +): Promise<{ available: boolean; 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 = `${LOAD_RELAY_NATIVE_DEPS}; console.log(process.argv[1])` + 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 '${probeJs}' ${shellEscape(PROBE_OK)} 2>${escapedStderr} || echo MISSING)` ) const probeOutput = await execHostCommand(conn, hostPlatform, probeCommand) - if (!probeOutput.includes(PROBE_OK)) { - const remoteStderr = isWindowsRemoteHost(hostPlatform) + const remoteStderr = + probeOutput.includes(PROBE_OK) || 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, removeRemoteFileCommand(hostPlatform, stderrFile) ).catch(() => {}) + return { + available: probeOutput.includes(PROBE_OK), + output: probeOutput, + stderr: remoteStderr + } } function getLocalRelayPath(platform: RelayPlatform): string | null { 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 4d2f92eb947..bd1335a1a81 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. @@ -129,6 +129,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 +144,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 +191,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 @@ -262,6 +276,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { (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 @@ -379,6 +394,27 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { expect(vi.mocked(abandonInstall)).not.toHaveBeenCalled() }) + 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('lets a probe SSH-channel failure bubble up rather than silently mapping to MISSING', async () => { const conn = makeMockConnection(sftpCapture) feed( @@ -536,6 +572,9 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { '', // 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 + '', // remove second probe stderr file '', // no persisted active pipe marker 'WAITING', '', // WMI relay launch @@ -555,6 +594,13 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { expect(probeScript).toContain('$LASTEXITCODE -ne 0') expect(probeScript).toContain("'MISSING'") + 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) + 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) @@ -629,6 +675,28 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { ).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('require("@parcel/watcher")') + expect(healthProbe).not.toContain('require.resolve') + }) + it('does not mutate an existing relay dir when required native deps are present', async () => { vi.mocked(isRelayAlreadyInstalled).mockResolvedValue(true) const conn = makeMockConnection(sftpCapture) From 3556f04df8606f4000d959d6ad90f073e8a90e28 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 15 Jul 2026 23:31:50 -0700 Subject: [PATCH 2/5] fix(ssh): keep relay native-deps repair non-fatal + bound deploy timeout Hardening on top of the ignore-scripts native-deps repair so it can never leave a user worse off than before the change: - repairInstalledNativeDeps is now best-effort. An already-installed relay can always launch in degraded mode (fs/git/preflight still work), so a repair failure must not block the connection. Neither lock contention nor an offline/unbuildable repair aborts the deploy now: on lock-acquire failure we degrade without releasing a lock we do not hold; on repair-body failure we release our lock and fall through to a degraded launch. Pre-repair the health check used require.resolve (which passed for a present-but-unloadable binding), so a fatal repair here was a straight regression for offline / no-toolchain hosts. The fresh-install path keeps its fatal semantics. - RELAY_DEPLOY_TIMEOUT_MS now exceeds a first install AND its follow-up rebuild (2 x NATIVE_DEPS_INSTALL_TIMEOUT_MS + headroom) so a legitimate install-then-rebuild is not falsely timed out mid-repair. - shellEscape the two POSIX probe scripts instead of raw single-quoting so a future edit adding a quote can't silently break the node -e probe. Adds regression coverage for degraded-launch-on-repair-throw, degraded-launch when the repair lock can't be acquired, and the widened deploy-timeout bound. Co-authored-by: Orca --- src/main/ssh/ssh-relay-deploy.test.ts | 13 ++- src/main/ssh/ssh-relay-deploy.ts | 51 +++++++++--- .../ssh/ssh-relay-native-deps-install.test.ts | 81 +++++++++++++++++++ 3 files changed, 130 insertions(+), 15 deletions(-) diff --git a/src/main/ssh/ssh-relay-deploy.test.ts b/src/main/ssh/ssh-relay-deploy.test.ts index 33467819035..aa68f984aa6 100644 --- a/src/main/ssh/ssh-relay-deploy.test.ts +++ b/src/main/ssh/ssh-relay-deploy.test.ts @@ -202,7 +202,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 @@ -499,7 +499,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,11 +514,15 @@ 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(240_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 540s') vi.useRealTimers() }) diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index 95cbf26be5d..681f24b98ff 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -66,17 +66,22 @@ 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. +// default 30s exec timeout. The same bound covers the post-install `npm rebuild` +// retry that repairs a native binding skipped by an ignore-scripts host. const NATIVE_DEPS_INSTALL_TIMEOUT_MS = 240_000 +// Why: individual exec commands have their own timeouts, but the full deploy +// pipeline (detect platform → check existing → upload → npm install → rebuild → +// launch) has no overall bound, so a hanging step or slow SFTP upload could +// block the connection indefinitely. This is a backstop, sized so the common +// native-deps case — a first install AND its follow-up rebuild, each up to +// NATIVE_DEPS_INSTALL_TIMEOUT_MS — plus headroom is not falsely timed out. +// (Adding heavy install-lock contention on top could still trip it; that stays +// a rare, self-healing-on-retry edge rather than a reason to loosen the +// backstop further.) +const RELAY_DEPLOY_TIMEOUT_MS = 2 * NATIVE_DEPS_INSTALL_TIMEOUT_MS + 60_000 + function execHostCommand( conn: SshConnection, hostPlatform: RemoteHostPlatform, @@ -459,7 +464,7 @@ async function hasRequiredNativeDeps( hostPlatform, nodePath, remoteDir, - `(${escapedNode} -e '${LOAD_RELAY_NATIVE_DEPS}; console.log("ORCA-NATIVE-DEPS-OK")' 2>/dev/null || echo MISSING)` + `(${escapedNode} -e ${shellEscape(`${LOAD_RELAY_NATIVE_DEPS}; console.log("ORCA-NATIVE-DEPS-OK")`)} 2>/dev/null || echo MISSING)` ) const probe = await execHostCommand(conn, hostPlatform, command) return probe.includes('ORCA-NATIVE-DEPS-OK') @@ -479,8 +484,24 @@ async function repairInstalledNativeDeps( return } + // Why: an already-installed relay can ALWAYS launch in degraded mode (fs/git/ + // preflight still work — only native-backed ops fail), so native-deps repair + // is best-effort and must never block the connection: not on lock contention, + // and not on an offline/unbuildable repair. 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. launchRelay still surfaces + // a genuinely dead transport on its own. console.warn(`[ssh-relay] Repairing missing native deps at ${remoteDir}`) - await acquireInstallLock(conn, remoteDir, hostPlatform) + try { + await acquireInstallLock(conn, remoteDir, hostPlatform) + } catch (err) { + console.warn( + `[ssh-relay] Could not lock native-deps repair at ${remoteDir}; launching degraded: ${ + err instanceof Error ? err.message : String(err) + }` + ) + return + } 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. @@ -491,8 +512,14 @@ async function repairInstalledNativeDeps( await abandonInstall(conn, remoteDir, hostPlatform) } } catch (err) { + // Release the lock and fall through to a degraded launch. The next reconnect + // retries, so a transient failure (offline registry) self-heals. await abandonInstall(conn, remoteDir, hostPlatform) - throw err + console.warn( + `[ssh-relay] Native deps repair failed at ${remoteDir}; launching degraded: ${ + err instanceof Error ? err.message : String(err) + }` + ) } } @@ -674,7 +701,7 @@ async function probeInstalledNativeDeps( hostPlatform, nodePath, remoteDir, - `(${shellEscape(nodePath)} -e '${probeJs}' ${shellEscape(PROBE_OK)} 2>${escapedStderr} || echo MISSING)` + `(${shellEscape(nodePath)} -e ${shellEscape(probeJs)} ${shellEscape(PROBE_OK)} 2>${escapedStderr} || echo MISSING)` ) const probeOutput = await execHostCommand(conn, hostPlatform, probeCommand) const remoteStderr = 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 bd1335a1a81..d15d68b1640 100644 --- a/src/main/ssh/ssh-relay-native-deps-install.test.ts +++ b/src/main/ssh/ssh-relay-native-deps-install.test.ts @@ -415,6 +415,35 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { 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('lets a probe SSH-channel failure bubble up rather than silently mapping to MISSING', async () => { const conn = makeMockConnection(sftpCapture) feed( @@ -675,6 +704,58 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { ).toBe(true) }) + 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('launches degraded when the repair install lock cannot be acquired', async () => { + // 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(acquireInstallLock).mockRejectedValueOnce( + new Error('Could not acquire relay install lock at /x; another install is in progress') + ) + const conn = makeMockConnection(sftpCapture) + feed([ + '__ORCA_REMOTE_PLATFORM__ Linux x86_64', + '/home/u', + 'MISSING', // health probe: require() fails + '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('Could not lock native-deps repair'))).toBe(true) + }) + it('loads native bindings when checking whether a completed relay needs repair', async () => { vi.mocked(isRelayAlreadyInstalled).mockResolvedValue(true) const conn = makeMockConnection(sftpCapture) From 3eeec1c100babfc2519f45c4145c37d5ef39033b Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 16 Jul 2026 09:12:47 -0400 Subject: [PATCH 3/5] fix(ssh): harden relay native repair recovery Co-authored-by: Orca --- src/main/ssh/ssh-relay-deploy-timing.ts | 13 ++ src/main/ssh/ssh-relay-deploy.test.ts | 4 + src/main/ssh/ssh-relay-deploy.ts | 53 ++---- .../ssh/ssh-relay-install-lock-commands.ts | 146 +++++++++++++++ .../ssh/ssh-relay-native-deps-install.test.ts | 18 +- src/main/ssh/ssh-relay-repair-lock.ts | 61 +++++++ .../ssh/ssh-relay-versioned-install.test.ts | 96 +++++++--- src/main/ssh/ssh-relay-versioned-install.ts | 78 ++++---- src/main/ssh/ssh-remote-commands.test.ts | 169 +++++++++++++++++- src/main/ssh/ssh-remote-commands.ts | 30 ---- src/relay/pty-handler.ts | 9 +- src/relay/subprocess.test.ts | 39 +++- 12 files changed, 580 insertions(+), 136 deletions(-) create mode 100644 src/main/ssh/ssh-relay-deploy-timing.ts create mode 100644 src/main/ssh/ssh-relay-install-lock-commands.ts create mode 100644 src/main/ssh/ssh-relay-repair-lock.ts 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 00000000000..0bd2ca95a17 --- /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. +export const NATIVE_DEPS_REPAIR_BUDGET_MS = 2 * NATIVE_DEPS_COMMAND_TIMEOUT_MS + +// Why: individual exec commands have their own timeouts, but the full deploy +// pipeline has no overall bound. Leave headroom after the native-deps budget +// for platform detection, upload, probes, and launch. +export const RELAY_DEPLOY_TIMEOUT_MS = NATIVE_DEPS_REPAIR_BUDGET_MS + 60_000 diff --git a/src/main/ssh/ssh-relay-deploy.test.ts b/src/main/ssh/ssh-relay-deploy.test.ts index aa68f984aa6..4eb5bd57364 100644 --- a/src/main/ssh/ssh-relay-deploy.test.ts +++ b/src/main/ssh/ssh-relay-deploy.test.ts @@ -62,6 +62,10 @@ vi.mock('./ssh-relay-versioned-install', () => ({ gcOldRelayVersions: vi.fn().mockResolvedValue(undefined) })) +vi.mock('./ssh-relay-repair-lock', () => ({ + tryAcquireRelayRepairLock: vi.fn().mockResolvedValue(true) +})) + vi.mock('./ssh-connection-utils', () => ({ shellEscape: (s: string) => `'${s}'` })) diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index 681f24b98ff..fb0bad68c8f 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -19,6 +19,8 @@ import { abandonInstall, gcOldRelayVersions } from './ssh-relay-versioned-install' +import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' +import { NATIVE_DEPS_COMMAND_TIMEOUT_MS, RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' import { shellEscape } from './ssh-connection-utils' import { probeBuildToolchain, @@ -66,22 +68,6 @@ export type RelayDeployResult = { sockPath?: string } -// npm install on a cold Windows cache plus antivirus scanning can exceed the -// default 30s exec timeout. The same bound covers the post-install `npm rebuild` -// retry that repairs a native binding skipped by an ignore-scripts host. -const NATIVE_DEPS_INSTALL_TIMEOUT_MS = 240_000 - -// Why: individual exec commands have their own timeouts, but the full deploy -// pipeline (detect platform → check existing → upload → npm install → rebuild → -// launch) has no overall bound, so a hanging step or slow SFTP upload could -// block the connection indefinitely. This is a backstop, sized so the common -// native-deps case — a first install AND its follow-up rebuild, each up to -// NATIVE_DEPS_INSTALL_TIMEOUT_MS — plus headroom is not falsely timed out. -// (Adding heavy install-lock contention on top could still trip it; that stays -// a rare, self-healing-on-retry edge rather than a reason to loosen the -// backstop further.) -const RELAY_DEPLOY_TIMEOUT_MS = 2 * NATIVE_DEPS_INSTALL_TIMEOUT_MS + 60_000 - function execHostCommand( conn: SshConnection, hostPlatform: RemoteHostPlatform, @@ -443,6 +429,12 @@ const RELAY_NATIVE_DEPS = { '@parcel/watcher': '2.5.6' } as const +// 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]) +) + const LOAD_RELAY_NATIVE_DEPS = 'require("node-pty"); require("@parcel/watcher")' async function hasRequiredNativeDeps( @@ -484,22 +476,14 @@ async function repairInstalledNativeDeps( return } - // Why: an already-installed relay can ALWAYS launch in degraded mode (fs/git/ - // preflight still work — only native-backed ops fail), so native-deps repair - // is best-effort and must never block the connection: not on lock contention, - // and not on an offline/unbuildable repair. 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. launchRelay still surfaces - // a genuinely dead transport on its own. + // 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}`) - try { - await acquireInstallLock(conn, remoteDir, hostPlatform) - } catch (err) { - console.warn( - `[ssh-relay] Could not lock native-deps repair at ${remoteDir}; launching degraded: ${ - err instanceof Error ? err.message : String(err) - }` - ) + if (!(await tryAcquireRelayRepairLock(conn, remoteDir, hostPlatform))) { + console.warn(`[ssh-relay] Native-deps repair lock is busy at ${remoteDir}; launching degraded`) return } try { @@ -551,7 +535,8 @@ 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, @@ -582,7 +567,7 @@ async function installNativeDeps( `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 }) } catch (err) { // Don't write .install-complete on hard fail; reconnect retries on a @@ -657,7 +642,7 @@ async function rebuildNativeDeps( `npm rebuild --ignore-scripts=false ${depNames.map(shellEscape).join(' ')} 2>&1` ) await execHostCommand(conn, hostPlatform, command, { - timeoutMs: NATIVE_DEPS_INSTALL_TIMEOUT_MS + timeoutMs: NATIVE_DEPS_COMMAND_TIMEOUT_MS }) } 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 00000000000..dd341161ecc --- /dev/null +++ b/src/main/ssh/ssh-relay-install-lock-commands.ts @@ -0,0 +1,146 @@ +import { shellEscape } from './ssh-connection-utils' +import { powerShellCommand, powerShellLiteral } from './ssh-remote-powershell' +import { isWindowsRemoteHost, type RemoteHostPlatform } from './ssh-remote-platform' + +const STALE_STEAL_LOCK_SECONDS = 120 + +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` + } + // 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 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=${escapedStealLockPrefix}.$mtime;`, + 'owns_steal=0;', + 'owned_steal=;', + 'lock_tombstone=;', + 'attempts=0;', + `while [ "$attempts" -lt 8 ] && [ "$owns_steal" != 1 ]; do`, + 'if mkdir "$steal" 2>/dev/null; then owns_steal=1; owned_steal=$steal; break; fi;', + `steal_key=$(stat -c %Y:%i "$steal" 2>/dev/null || stat -f %m:%i "$steal" 2>/dev/null) && steal_mtime=\${steal_key%%:*} && steal_now=$(date +%s) && steal_age=$((steal_now - steal_mtime)) || break;`, + `if [ "\${steal_age:-0}" -le ${STALE_STEAL_LOCK_SECONDS} ] 2>/dev/null; then break; fi;`, + `steal_token=$(printf '%s' "$steal_key" | tr ':/' '__');`, + `steal="$steal.next.$steal_token";`, + 'attempts=$((attempts + 1));', + 'done;', + 'if [ "$owns_steal" = 1 ]; then', + `trap 'rmdir "$owned_steal" 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 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 {`, + '$steal = "$lock.steal.$mtime"', + '$ownsSteal = $false', + '$ownedSteal = $null', + '$lockTombstone = $null', + 'try {', + 'for ($attempt = 0; ($attempt -lt 8) -and (-not $ownsSteal); $attempt++) {', + 'try {', + '$null = New-Item -ItemType Directory -Path $steal -ErrorAction Stop', + '$ownsSteal = $true', + '$ownedSteal = $steal', + 'break', + '} catch {', + 'try {', + '$stealItem = Get-Item -LiteralPath $steal -ErrorAction Stop', + '$stealMtime = ([DateTimeOffset]$stealItem.LastWriteTimeUtc).ToUnixTimeSeconds()', + '$stealAge = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds() - $stealMtime', + `if ($stealAge -le ${STALE_STEAL_LOCK_SECONDS}) { break }`, + '$stealIdentity = "$stealMtime-$($stealItem.CreationTimeUtc.Ticks)"', + '$steal = "$steal.next.$stealIdentity"', + '} 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', + "try { $null = New-Item -ItemType Directory -Path $lock -ErrorAction Stop; 'OK' } catch { 'BUSY' }", + "} else { 'BUSY' }", + '}', + "} catch { 'BUSY' } finally { if ($null -ne $ownedSteal) { Remove-Item -LiteralPath $ownedSteal -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-native-deps-install.test.ts b/src/main/ssh/ssh-relay-native-deps-install.test.ts index d15d68b1640..5dd1b4905ca 100644 --- a/src/main/ssh/ssh-relay-native-deps-install.test.ts +++ b/src/main/ssh/ssh-relay-native-deps-install.test.ts @@ -47,6 +47,10 @@ vi.mock('./ssh-relay-versioned-install', () => ({ gcOldRelayVersions: vi.fn().mockResolvedValue(undefined) })) +vi.mock('./ssh-relay-repair-lock', () => ({ + tryAcquireRelayRepairLock: vi.fn().mockResolvedValue(true) +})) + vi.mock('./ssh-connection-utils', () => ({ shellEscape: (s: string) => `'${s}'` })) @@ -61,6 +65,7 @@ import { finalizeInstall, isRelayAlreadyInstalled } from './ssh-relay-versioned-install' +import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' import type { SshConnection } from './ssh-connection' type SftpWriteCapture = { @@ -270,6 +275,10 @@ 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( @@ -694,7 +703,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { await deployAndLaunchRelay(conn) - expect(vi.mocked(acquireInstallLock)).toHaveBeenCalledTimes(1) + expect(vi.mocked(tryAcquireRelayRepairLock)).toHaveBeenCalledTimes(1) expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1) const execCalls = vi.mocked(execCommand).mock.calls.map(([, c]) => c) expect( @@ -734,9 +743,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { // 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(acquireInstallLock).mockRejectedValueOnce( - new Error('Could not acquire relay install lock at /x; another install is in progress') - ) + vi.mocked(tryAcquireRelayRepairLock).mockResolvedValueOnce(false) const conn = makeMockConnection(sftpCapture) feed([ '__ORCA_REMOTE_PLATFORM__ Linux x86_64', @@ -753,7 +760,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { 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('Could not lock native-deps repair'))).toBe(true) + expect(warnMessages.some((m) => m.includes('repair lock is busy'))).toBe(true) }) it('loads native bindings when checking whether a completed relay needs repair', async () => { @@ -792,6 +799,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { await deployAndLaunchRelay(conn) expect(vi.mocked(acquireInstallLock)).not.toHaveBeenCalled() + expect(vi.mocked(tryAcquireRelayRepairLock)).not.toHaveBeenCalled() expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled() 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 00000000000..ebcac8c09ab --- /dev/null +++ b/src/main/ssh/ssh-relay-repair-lock.ts @@ -0,0 +1,61 @@ +import type { SshConnection } from './ssh-connection' +import { execCommand } from './ssh-relay-deploy-helpers' +import { + acquireInstallLockParentCommand, + tryCreateInstallLockCommand, + tryStealInstallLockCommand +} from './ssh-relay-install-lock-commands' +import { + getRemoteHostPlatform, + isWindowsRemoteHost, + joinRemotePath, + type RemoteHostPlatform +} from './ssh-remote-platform' +import { INSTALL_LOCK_STALE_SECONDS, RELAY_INSTALL_LOCK_NAME } from './ssh-relay-versioned-install' + +const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') + +function execHostCommand( + conn: SshConnection, + host: RemoteHostPlatform, + command: string +): Promise { + return execCommand(conn, command, { wrapCommand: !isWindowsRemoteHost(host) }) +} + +/** + * 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 +): Promise { + const lockDir = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) + try { + await execHostCommand(conn, host, acquireInstallLockParentCommand(host, remoteRelayDir)) + const firstAttempt = await execHostCommand( + conn, + host, + tryCreateInstallLockCommand(host, lockDir) + ) + if (firstAttempt.trim().endsWith('OK')) { + return true + } + const steal = await execHostCommand( + conn, + host, + tryStealInstallLockCommand(host, lockDir, INSTALL_LOCK_STALE_SECONDS) + ) + if (steal.trim().endsWith('OK')) { + console.warn(`[ssh-relay] Stealing stale install lock at ${lockDir}`) + return true + } + return false + } catch { + return false + } +} diff --git a/src/main/ssh/ssh-relay-versioned-install.test.ts b/src/main/ssh/ssh-relay-versioned-install.test.ts index 244ae80a48c..e7e96ad7c2f 100644 --- a/src/main/ssh/ssh-relay-versioned-install.test.ts +++ b/src/main/ssh/ssh-relay-versioned-install.test.ts @@ -23,6 +23,7 @@ import { abandonInstall, gcOldRelayVersions } from './ssh-relay-versioned-install' +import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' import { execCommand } from './ssh-relay-deploy-helpers' import { getRemoteHostPlatform } from './ssh-remote-platform' import type { SshConnection } from './ssh-connection' @@ -131,6 +132,38 @@ describe('acquireInstallLock', () => { expect(mockExec).toHaveBeenCalledTimes(2) }) + it('returns immediately when a best-effort repair lock is busy', async () => { + mockExec.mockResolvedValueOnce('').mockResolvedValueOnce('BUSY').mockResolvedValueOnce('BUSY') + + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe(false) + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands.some((command) => command.includes('.install-lock.steal'))).toBe(true) + expect(commands.filter((command) => command.startsWith('rm -rf'))).toHaveLength(0) + }) + + it('does not steal a live repair lock that exceeds the deploy backstop', 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('').mockResolvedValueOnce('BUSY').mockResolvedValueOnce('BUSY') + + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe(false) + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands.some((command) => command.includes('.install-lock.steal'))).toBe(true) + expect(commands.filter((command) => command.startsWith('rm -rf'))).toHaveLength(0) + }) + + it('recovers a stale best-effort repair lock without polling', async () => { + mockExec.mockResolvedValueOnce('').mockResolvedValueOnce('BUSY').mockResolvedValueOnce('OK') + + await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe(true) + + const commands = mockExec.mock.calls.map(([, command]) => command) + expect(commands.some((command) => command.includes('.install-lock.steal'))).toBe(true) + expect(commands.filter((command) => command.includes('.install-lock.steal'))).toHaveLength(1) + }) + it('polls until the lock becomes available (concurrent installer wins, then we acquire)', async () => { vi.useFakeTimers() try { @@ -159,36 +192,29 @@ describe('acquireInstallLock', () => { } }) - it('steals a stale lock and retries with a reset timeout window', async () => { + it('steals a stale lock with a serialized remote steal command', async () => { vi.useFakeTimers({ now: 1_700_000_000_000 }) try { - let mkdirCalls = 0 mockExec.mockImplementation(async (_conn: unknown, cmd: string) => { if (cmd.startsWith('mkdir -p')) { return '' } + if (cmd.includes('.install-lock.steal')) { + return 'OK' + } 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) + // Drive through the full deploy bound so stale recovery fires. + await vi.advanceTimersByTimeAsync(545_000) await promise const cmds = mockExec.mock.calls.map(([, c]) => c) - expect(cmds.some((c) => c.includes('rm -rf') && c.includes('.install-lock'))).toBe(true) + expect(cmds.some((c) => c.includes('.install-lock.steal'))).toBe(true) } finally { vi.useRealTimers() } @@ -201,23 +227,50 @@ describe('acquireInstallLock', () => { if (cmd.startsWith('mkdir -p')) { return '' } + if (cmd.includes('.install-lock.steal')) { + 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) + await vi.advanceTimersByTimeAsync(545_000) await rejection } 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.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.some(([, cmd]) => cmd.includes('.install-lock.steal'))).toBe(false) + } finally { + vi.useRealTimers() + } + }) + it('finalizeInstall writes .install-complete then removes the lock', async () => { mockExec.mockResolvedValueOnce('').mockResolvedValueOnce('') await finalizeInstall(conn, '/r') @@ -273,8 +326,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,9 +336,8 @@ 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 diff --git a/src/main/ssh/ssh-relay-versioned-install.ts b/src/main/ssh/ssh-relay-versioned-install.ts index a669f0b7875..4028bf4233b 100644 --- a/src/main/ssh/ssh-relay-versioned-install.ts +++ b/src/main/ssh/ssh-relay-versioned-install.ts @@ -15,16 +15,20 @@ 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 { RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' import { acquireInstallLockParentCommand, + lockAgeSecondsCommand, + tryCreateInstallLockCommand, + tryStealInstallLockCommand +} from './ssh-relay-install-lock-commands' +import { listRelayBaseDirsCommand, - lockMtimeEpochCommand, probeDirectoryExistsCommand, probeFileExistsCommand, probeRelayInstalledCommand, relayLivenessProbeCommand, removeRemoteTreeCommand, - tryCreateInstallLockCommand, writeRemoteEmptyFileCommand } from './ssh-remote-commands' import { @@ -51,18 +55,21 @@ 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' +export const RELAY_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: 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: 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 +// the lock. Native-deps repair can keep running after the deploy backstop wins +// Promise.race, so leave enough room for its bounded under-lock exec sequence. +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') type RelayInstalledProbeOptions = { @@ -166,13 +173,12 @@ export async function acquireInstallLock( remoteRelayDir: string, host: RemoteHostPlatform = DEFAULT_REMOTE_HOST ): Promise { - const lockDir = joinRemotePath(host, remoteRelayDir, INSTALL_LOCK_NAME) + const lockDir = joinRemotePath(host, remoteRelayDir, RELAY_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)) @@ -183,24 +189,16 @@ export async function acquireInstallLock( /* 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) { + // Stale-lock recovery is one remote command so two observers cannot both + // delete a lock that the other one just recreated. + const steal = await execHostCommand( + conn, + host, + tryStealInstallLockCommand(host, lockDir, INSTALL_LOCK_STALE_SECONDS) + ).catch(() => 'BUSY') + if (steal.trim().endsWith('OK')) { console.warn(`[ssh-relay] Stealing stale install lock at ${lockDir}`) - await execHostCommand(conn, host, removeRemoteTreeCommand(host, lockDir)).catch(() => {}) - recoveredOnce = true - start = Date.now() - continue + return } throw new Error( `Could not acquire relay install lock at ${lockDir} after ${ @@ -212,22 +210,20 @@ export async function acquireInstallLock( } } -async function isLockStale( +export async function isRelayInstallLockStale( 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)) { + // Why: compute age on the remote host so clock skew between two Orca + // clients cannot make a live repair lock look older than it is. + const out = await execHostCommand(conn, host, lockAgeSecondsCommand(host, lockDir)) + const ageSec = Number.parseInt(out.trim(), 10) + if (!Number.isFinite(ageSec) || ageSec < 0) { return false } - const ageMs = Date.now() - mtimeSec * 1000 - return ageMs > INSTALL_LOCK_STALE_MS + return ageSec * 1000 > INSTALL_LOCK_STALE_MS } catch { return false } @@ -245,7 +241,7 @@ export async function finalizeInstall( host: RemoteHostPlatform = DEFAULT_REMOTE_HOST ): Promise { const sentinel = joinRemotePath(host, remoteRelayDir, INSTALL_COMPLETE_NAME) - const lock = joinRemotePath(host, remoteRelayDir, INSTALL_LOCK_NAME) + const lock = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) await execHostCommand(conn, host, writeRemoteEmptyFileCommand(host, sentinel)) await execHostCommand(conn, host, removeRemoteTreeCommand(host, lock)).catch(() => {}) } @@ -260,7 +256,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(() => {}) } @@ -346,7 +342,7 @@ async function isCandidateSafeToRemove( ): Promise { const isLegacy = LEGACY_RELAY_DIR_REGEX.test(name) - const lockDir = joinRemotePath(host, dir, INSTALL_LOCK_NAME) + const lockDir = joinRemotePath(host, dir, RELAY_INSTALL_LOCK_NAME) const lockProbe = await execHostCommand( conn, host, @@ -356,13 +352,13 @@ async function isCandidateSafeToRemove( 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`) diff --git a/src/main/ssh/ssh-remote-commands.test.ts b/src/main/ssh/ssh-remote-commands.test.ts index 952daf9edde..ae62a46de7c 100644 --- a/src/main/ssh/ssh-remote-commands.test.ts +++ b/src/main/ssh/ssh-remote-commands.test.ts @@ -1,4 +1,21 @@ +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, @@ -6,19 +23,52 @@ import { makeRemoteDirectoryCommand, 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 +}) 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}`)) + }) + }) +} + describe('ssh remote command builders', () => { it('keeps POSIX deploy commands POSIX-native', () => { expect(readRemoteHomeCommand(posix)).toBe('echo $HOME') @@ -101,6 +151,121 @@ describe('ssh remote command builders', () => { 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 install-lock recovery under a sibling steal mutex', () => { + 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.steal') + expect(posixCommand).toContain('$mtime') + expect(posixCommand).toContain('-gt 1200') + expect(posixCommand).toContain('current_mtime') + expect(posixCommand).toContain('steal_age') + expect(posixCommand).toContain('steal="$steal.next.$steal_token"') + expect(posixCommand).toContain('trap') + expect(windowsScript).toContain('$lock.steal.$mtime') + expect(windowsScript).toContain('-gt 1200') + expect(windowsScript).toContain('$currentIdentity -eq $lockIdentity') + expect(windowsScript).toContain('$stealAge -le 120') + expect(windowsScript).toContain('$steal = "$steal.next.$stealIdentity"') + expect(windowsScript).toContain('New-Item -ItemType Directory -Path $steal') + 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') + }) + + it.runIf(process.platform !== 'win32')( + 'recovers a stale POSIX steal mutex left by a crashed stealer', + () => { + 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) + const stealDir = `${lockDir}.steal.${lockMtimeSeconds}` + mkdirSync(stealDir) + utimesSync(stealDir, 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(existsSync(stealDir)).toBe(true) + expect(readdirSync(root).some((name) => name.includes('.next.'))).toBe(false) + 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 stealer take over a preexisting stale steal mutex', + 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 lockMtimeSeconds = Math.floor(statSync(lockDir).mtimeMs / 1000) + const stealDir = `${lockDir}.steal.${lockMtimeSeconds}` + mkdirSync(stealDir) + utimesSync(stealDir, 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(existsSync(stealDir)).toBe(true) + expect(readdirSync(root).some((name) => name.includes('.next.'))).toBe(false) + 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 339f4aef594..751c91017da 100644 --- a/src/main/ssh/ssh-remote-commands.ts +++ b/src/main/ssh/ssh-remote-commands.ts @@ -81,36 +81,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` diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index c1de6809c15..74fd755e6fd 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -51,7 +51,14 @@ async function loadPty(): Promise { ptyModule = await import('node-pty') return ptyModule } catch { - return null + try { + // Why: Node caches rejected dynamic imports, while a failed CommonJS + // load remains retryable after deploy repairs node_modules in place. + ptyModule = require('node-pty') as typeof NodePty + return ptyModule + } catch { + return null + } } } diff --git a/src/relay/subprocess.test.ts b/src/relay/subprocess.test.ts index ee20461dd68..ea017bb8316 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' @@ -103,6 +111,35 @@ 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 = spawnRelay(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') + + const nodePtyDir = path.join(tmpDir, 'node_modules', 'node-pty') + mkdirSync(nodePtyDir, { recursive: true }) + writeFileSync( + path.join(nodePtyDir, 'index.js'), + `module.exports = { spawn() { return { + pid: process.pid, + process: 'mock-shell', + onData() {}, onExit() {}, write() {}, resize() {}, kill() {}, clear() {} + } } }\n` + ) + + 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('responds to fs.stat over stdin/stdout', async () => { tmpDir = mkdtempSync(path.join(tmpdir(), 'relay-sub-')) writeFileSync(path.join(tmpDir, 'test.txt'), 'hello') From 04371c3d828044bda8a1ff63139be5f07799c2ff Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 16 Jul 2026 16:08:06 -0400 Subject: [PATCH 4/5] fix(ssh): harden relay repair cancellation and locking Co-authored-by: Orca --- 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 | 3 + 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 | 10 +- src/main/ssh/ssh-relay-deploy.test.ts | 187 +++++++- src/main/ssh/ssh-relay-deploy.ts | 420 ++++++++++++------ src/main/ssh/ssh-relay-exec-command.ts | 156 +++++++ .../ssh/ssh-relay-install-lock-commands.ts | 48 +- src/main/ssh/ssh-relay-install-lock.ts | 116 +++++ .../ssh/ssh-relay-native-deps-install.test.ts | 219 ++++++++- src/main/ssh/ssh-relay-repair-lock.ts | 24 +- .../ssh/ssh-relay-versioned-install.test.ts | 107 +++-- src/main/ssh/ssh-relay-versioned-install.ts | 89 +--- src/main/ssh/ssh-remote-commands.test.ts | 39 +- 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 | 9 + src/main/ssh/system-ssh-command.ts | 3 + .../ssh/system-ssh-operation-lifecycle.ts | 8 +- 24 files changed, 1793 insertions(+), 535 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-exec-command.ts create mode 100644 src/main/ssh/ssh-relay-install-lock.ts diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index e8c5d71234c..3274d3f5daa 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 2856d33b2f0..b8bd0cbd57f 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 00000000000..49abe0a1839 --- /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 00000000000..a7b6cce2e49 --- /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 cdbdb676441..208deceb6e0 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 a29801c3632..da132775545 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() })) diff --git a/src/main/ssh/ssh-relay-deploy-helpers.test.ts b/src/main/ssh/ssh-relay-deploy-helpers.test.ts index cb5477091d6..4df9c8ad28a 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 1b1cdd27074..80d2b78ced2 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 index 0bd2ca95a17..5dc24f937f8 100644 --- a/src/main/ssh/ssh-relay-deploy-timing.ts +++ b/src/main/ssh/ssh-relay-deploy-timing.ts @@ -5,9 +5,9 @@ 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. -export const NATIVE_DEPS_REPAIR_BUDGET_MS = 2 * NATIVE_DEPS_COMMAND_TIMEOUT_MS +const NATIVE_DEPS_REPAIR_BUDGET_MS = 2 * NATIVE_DEPS_COMMAND_TIMEOUT_MS -// Why: individual exec commands have their own timeouts, but the full deploy -// pipeline has no overall bound. Leave headroom after the native-deps budget -// for platform detection, upload, probes, and launch. -export const RELAY_DEPLOY_TIMEOUT_MS = NATIVE_DEPS_REPAIR_BUDGET_MS + 60_000 +// 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 4eb5bd57364..70c253934b0 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,31 +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(true) })) 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 { @@ -137,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) }) ) }) @@ -157,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) @@ -322,7 +353,10 @@ describe('deployAndLaunchRelay', () => { 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(resolveRemoteNodePath).toHaveBeenCalledTimes(2) }) @@ -522,15 +556,136 @@ describe('deployAndLaunchRelay', () => { await vi.advanceTimersByTimeAsync(301_000) expect(await Promise.race([promise, Promise.resolve('pending')])).toBe('pending') - await vi.advanceTimersByTimeAsync(240_000) + await vi.advanceTimersByTimeAsync(600_000) const result = await promise expect(result).toBeInstanceOf(Error) - expect((result as Error).message).toBe('Relay deployment timed out after 540s') + 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() @@ -607,6 +762,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 fb0bad68c8f..35161c15ccc 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -8,20 +8,25 @@ 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 { acquireInstallLock } from './ssh-relay-install-lock' import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock' import { NATIVE_DEPS_COMMAND_TIMEOUT_MS, RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' -import { shellEscape } from './ssh-connection-utils' +import { createSshOperationAbortError, shellEscape } from './ssh-connection-utils' import { probeBuildToolchain, formatMissingToolchainError, @@ -99,15 +104,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 { @@ -170,22 +185,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 @@ -196,10 +218,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 } @@ -215,7 +239,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) } } @@ -232,11 +258,12 @@ async function deployAndLaunchRelayInner( conn: SshConnection, onProgress?: (status: string) => void, graceTimeSeconds?: number, - relayInstanceId?: string + 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.' @@ -266,47 +293,70 @@ 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}`) if (alreadyInstalled) { - await repairInstalledNativeDeps(conn, remoteRelayDir, platform, hostPlatform, nodePath) + await repairInstalledNativeDeps( + conn, + remoteRelayDir, + platform, + hostPlatform, + nodePath, + deploySignal + ) + 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 // re-checks `alreadyInstalled` (now true) or steals a stale lock. - await acquireInstallLock(conn, remoteRelayDir, hostPlatform) + await acquireInstallLock(conn, remoteRelayDir, hostPlatform, { signal: deploySignal }) 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) + await finalizeInstall(conn, remoteRelayDir, hostPlatform, { signal: deploySignal }) } else { await abandonInstall(conn, remoteRelayDir, hostPlatform) } } 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) + } throw err } } + deploySignal?.throwIfAborted() onProgress?.('Starting relay...') console.log('[ssh-relay] Launching relay...') const launched = await launchRelay( @@ -315,7 +365,8 @@ async function deployAndLaunchRelayInner( hostPlatform, nodePath, graceTimeSeconds, - relayInstanceId + relayInstanceId, + deploySignal ) console.log('[ssh-relay] Relay started successfully') @@ -343,7 +394,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)) { @@ -354,16 +406,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 } ) } @@ -374,7 +429,8 @@ async function uploadRelay( conn, hostPlatform, joinRemotePath(hostPlatform, remoteDir, '.version'), - fullVersion + fullVersion, + signal ) } @@ -382,10 +438,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 } @@ -401,10 +458,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 } @@ -441,7 +499,8 @@ async function hasRequiredNativeDeps( conn: SshConnection, remoteDir: string, hostPlatform: RemoteHostPlatform, - nodePath: string + nodePath: string, + signal?: AbortSignal ): Promise { const escapedNode = shellEscape(nodePath) try { @@ -458,9 +517,10 @@ async function hasRequiredNativeDeps( remoteDir, `(${escapedNode} -e ${shellEscape(`${LOAD_RELAY_NATIVE_DEPS}; console.log("ORCA-NATIVE-DEPS-OK")`)} 2>/dev/null || echo MISSING)` ) - const probe = await execHostCommand(conn, hostPlatform, command) + const probe = await execHostCommand(conn, hostPlatform, command, { signal }) return probe.includes('ORCA-NATIVE-DEPS-OK') } catch { + signal?.throwIfAborted() return false } } @@ -470,9 +530,10 @@ async function repairInstalledNativeDeps( remoteDir: string, platform: RelayPlatform, hostPlatform: RemoteHostPlatform, - nodePath: string + nodePath: string, + signal?: AbortSignal ): Promise { - if (await hasRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath)) { + if (await hasRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal)) { return } @@ -482,23 +543,25 @@ async function repairInstalledNativeDeps( // 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}`) - if (!(await tryAcquireRelayRepairLock(conn, remoteDir, hostPlatform))) { + if (!(await tryAcquireRelayRepairLock(conn, remoteDir, hostPlatform, { signal }))) { console.warn(`[ssh-relay] Native-deps repair lock is busy at ${remoteDir}; launching degraded`) return } 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) + if (!(await hasRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal))) { + await installNativeDeps(conn, remoteDir, platform, hostPlatform, nodePath, signal) + await finalizeInstall(conn, remoteDir, hostPlatform, { signal }) } else { await abandonInstall(conn, remoteDir, hostPlatform) } } catch (err) { // Release the lock and fall through to a degraded launch. The next reconnect // retries, so a transient failure (offline registry) self-heals. - await abandonInstall(conn, remoteDir, hostPlatform) + if (!isUnconfirmedSshCommandTermination(err)) { + await abandonInstall(conn, remoteDir, hostPlatform) + } console.warn( `[ssh-relay] Native deps repair failed at ${remoteDir}; launching degraded: ${ err instanceof Error ? err.message : String(err) @@ -520,7 +583,8 @@ async function installNativeDeps( remoteDir: string, platform: RelayPlatform, hostPlatform: RemoteHostPlatform, - nodePath: string + nodePath: string, + signal?: AbortSignal ): Promise { // 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`) @@ -542,7 +606,8 @@ async function installNativeDeps( conn, hostPlatform, joinRemotePath(hostPlatform, remoteDir, 'package.json'), - pkgJson + pkgJson, + signal ) try { @@ -558,7 +623,7 @@ async function installNativeDeps( RELAY_NATIVE_DEPS ) .map(([dep, version]) => powerShellLiteral(`${dep}@${version}`)) - .join(' ')}` + .join(' ')}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }` ) : commandWithNodePath( hostPlatform, @@ -567,9 +632,14 @@ async function installNativeDeps( `npm install --ignore-scripts=false --omit=dev --no-audit --no-fund ${installArgs} 2>&1` ) await execHostCommand(conn, hostPlatform, command, { - timeoutMs: NATIVE_DEPS_COMMAND_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. @@ -582,7 +652,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)) } @@ -590,25 +660,30 @@ async function installNativeDeps( throw err } - await makeNodePtySpawnHelperExecutable(conn, remoteDir, hostPlatform) + await makeNodePtySpawnHelperExecutable(conn, remoteDir, hostPlatform, signal) - let probe = await probeInstalledNativeDeps(conn, remoteDir, hostPlatform, nodePath) + 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) + 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) - probe = await probeInstalledNativeDeps(conn, remoteDir, hostPlatform, nodePath) + await makeNodePtySpawnHelperExecutable(conn, remoteDir, hostPlatform, signal) + probe = await probeInstalledNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal) } } @@ -625,7 +700,8 @@ async function rebuildNativeDeps( conn: SshConnection, remoteDir: string, hostPlatform: RemoteHostPlatform, - nodePath: string + nodePath: string, + signal?: AbortSignal ): Promise { const depNames = Object.keys(RELAY_NATIVE_DEPS) const command = isWindowsRemoteHost(hostPlatform) @@ -633,7 +709,7 @@ async function rebuildNativeDeps( hostPlatform, nodePath, remoteDir, - `npm rebuild --ignore-scripts=false ${depNames.map(powerShellLiteral).join(' ')}` + `npm rebuild --ignore-scripts=false ${depNames.map(powerShellLiteral).join(' ')}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }` ) : commandWithNodePath( hostPlatform, @@ -642,14 +718,16 @@ async function rebuildNativeDeps( `npm rebuild --ignore-scripts=false ${depNames.map(shellEscape).join(' ')} 2>&1` ) await execHostCommand(conn, hostPlatform, command, { - timeoutMs: NATIVE_DEPS_COMMAND_TIMEOUT_MS + timeoutMs: NATIVE_DEPS_COMMAND_TIMEOUT_MS, + signal }) } async function makeNodePtySpawnHelperExecutable( conn: SshConnection, remoteDir: string, - hostPlatform: RemoteHostPlatform + hostPlatform: RemoteHostPlatform, + signal?: AbortSignal ): Promise { if (isWindowsRemoteHost(hostPlatform)) { return @@ -659,7 +737,8 @@ async function makeNodePtySpawnHelperExecutable( await execHostCommand( conn, hostPlatform, - `find ${shellEscape(joinRemotePath(hostPlatform, remoteDir, 'node_modules/node-pty/prebuilds'))} -name spawn-helper -exec chmod +x {} + 2>/dev/null; true` + `find ${shellEscape(joinRemotePath(hostPlatform, remoteDir, 'node_modules/node-pty/prebuilds'))} -name spawn-helper -exec chmod +x {} + 2>/dev/null; true`, + { signal } ) } @@ -667,7 +746,8 @@ async function probeInstalledNativeDeps( conn: SshConnection, remoteDir: string, hostPlatform: RemoteHostPlatform, - nodePath: string + nodePath: string, + signal?: AbortSignal ): Promise<{ available: boolean; 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. @@ -688,18 +768,21 @@ async function probeInstalledNativeDeps( remoteDir, `(${shellEscape(nodePath)} -e ${shellEscape(probeJs)} ${shellEscape(PROBE_OK)} 2>${escapedStderr} || echo MISSING)` ) - const probeOutput = await execHostCommand(conn, hostPlatform, probeCommand) + 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`).catch( - () => '' - ) - await execHostCommand( - conn, - hostPlatform, - removeRemoteFileCommand(hostPlatform, stderrFile) - ).catch(() => {}) + : 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), output: probeOutput, @@ -744,7 +827,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 @@ -776,21 +860,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 @@ -800,29 +890,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 } @@ -843,7 +947,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', () => {}) @@ -865,36 +969,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}`) } @@ -903,9 +1010,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( @@ -925,15 +1051,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 @@ -948,15 +1079,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( @@ -982,17 +1116,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, @@ -1000,6 +1136,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) @@ -1017,15 +1154,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, @@ -1033,6 +1171,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) @@ -1054,20 +1193,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, @@ -1079,8 +1227,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}`) } @@ -1091,13 +1243,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( @@ -1164,12 +1317,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' } @@ -1183,7 +1338,8 @@ async function waitForWindowsRelayPipe( sockPath: string }, timeoutMs: number, - intervalMs: number + intervalMs: number, + signal?: AbortSignal ): Promise { try { const result = await execHostCommand( @@ -1192,10 +1348,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 00000000000..e4bfd8bc042 --- /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-install-lock-commands.ts b/src/main/ssh/ssh-relay-install-lock-commands.ts index dd341161ecc..20ca1b7ec53 100644 --- a/src/main/ssh/ssh-relay-install-lock-commands.ts +++ b/src/main/ssh/ssh-relay-install-lock-commands.ts @@ -2,8 +2,6 @@ import { shellEscape } from './ssh-connection-utils' import { powerShellCommand, powerShellLiteral } from './ssh-remote-powershell' import { isWindowsRemoteHost, type RemoteHostPlatform } from './ssh-remote-platform' -const STALE_STEAL_LOCK_SECONDS = 120 - export function acquireInstallLockParentCommand( host: RemoteHostPlatform, remoteRelayDir: string @@ -57,25 +55,24 @@ function posixStealInstallLockCommand(lockDir: string, staleAfterSeconds: number 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=${escapedStealLockPrefix}.$mtime;`, + `steal_root=${escapedStealLockPrefix};`, + 'steal_generation=0;', + 'steal="$steal_root.$steal_generation";', 'owns_steal=0;', - 'owned_steal=;', 'lock_tombstone=;', - 'attempts=0;', - `while [ "$attempts" -lt 8 ] && [ "$owns_steal" != 1 ]; do`, - 'if mkdir "$steal" 2>/dev/null; then owns_steal=1; owned_steal=$steal; break; fi;', - `steal_key=$(stat -c %Y:%i "$steal" 2>/dev/null || stat -f %m:%i "$steal" 2>/dev/null) && steal_mtime=\${steal_key%%:*} && steal_now=$(date +%s) && steal_age=$((steal_now - steal_mtime)) || break;`, - `if [ "\${steal_age:-0}" -le ${STALE_STEAL_LOCK_SECONDS} ] 2>/dev/null; then break; fi;`, - `steal_token=$(printf '%s' "$steal_key" | tr ':/' '__');`, - `steal="$steal.next.$steal_token";`, - 'attempts=$((attempts + 1));', + '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 'rmdir "$owned_steal" 2>/dev/null || true; rm -rf "$lock_tombstone" 2>/dev/null || true' EXIT;`, + `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 mv ${escapedLockDir} "$lock_tombstone" 2>/dev/null; then mkdir ${escapedLockDir} 2>&1 && echo OK || echo BUSY; else echo BUSY; fi;`, + `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(' ') @@ -91,25 +88,25 @@ function windowsStealInstallLockCommand(lockDir: string, staleAfterSeconds: numb '$lockIdentity = "${mtime}:$($item.CreationTimeUtc.Ticks)"', '$now = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds()', `if (($now - $mtime) -le ${staleAfterSeconds}) { 'BUSY' } else {`, - '$steal = "$lock.steal.$mtime"', + '$stealRoot = "$lock.steal"', + '$stealGeneration = 0', + '$steal = "$stealRoot.$stealGeneration"', '$ownsSteal = $false', - '$ownedSteal = $null', '$lockTombstone = $null', 'try {', - 'for ($attempt = 0; ($attempt -lt 8) -and (-not $ownsSteal); $attempt++) {', + 'while (-not $ownsSteal) {', 'try {', '$null = New-Item -ItemType Directory -Path $steal -ErrorAction Stop', '$ownsSteal = $true', - '$ownedSteal = $steal', 'break', '} catch {', 'try {', '$stealItem = Get-Item -LiteralPath $steal -ErrorAction Stop', '$stealMtime = ([DateTimeOffset]$stealItem.LastWriteTimeUtc).ToUnixTimeSeconds()', '$stealAge = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds() - $stealMtime', - `if ($stealAge -le ${STALE_STEAL_LOCK_SECONDS}) { break }`, - '$stealIdentity = "$stealMtime-$($stealItem.CreationTimeUtc.Ticks)"', - '$steal = "$steal.next.$stealIdentity"', + 'if ($stealAge -le 120) { break }', + '$stealGeneration++', + '$steal = "$stealRoot.$stealGeneration"', '} catch { break }', '}', '}', @@ -124,7 +121,14 @@ function windowsStealInstallLockCommand(lockDir: string, staleAfterSeconds: numb "try { $null = New-Item -ItemType Directory -Path $lock -ErrorAction Stop; 'OK' } catch { 'BUSY' }", "} else { 'BUSY' }", '}', - "} catch { 'BUSY' } finally { if ($null -ne $ownedSteal) { Remove-Item -LiteralPath $ownedSteal -Recurse -Force -ErrorAction SilentlyContinue }; if ($null -ne $lockTombstone) { Remove-Item -LiteralPath $lockTombstone -Recurse -Force -ErrorAction SilentlyContinue } }", + "} 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('; ') 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 00000000000..47dab9d00e1 --- /dev/null +++ b/src/main/ssh/ssh-relay-install-lock.ts @@ -0,0 +1,116 @@ +import type { SshConnection } from './ssh-connection' +import { execCommand } from './ssh-relay-deploy-helpers' +import { RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' +import { + acquireInstallLockParentCommand, + tryCreateInstallLockCommand, + tryStealInstallLockCommand +} from './ssh-relay-install-lock-commands' +import { + getRemoteHostPlatform, + joinRemotePath, + type RemoteHostPlatform +} from './ssh-remote-platform' + +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 + }) +} + +/** + * Acquire the per-version install lock via atomic `mkdir`. + * Why: `mkdir` gives every supported remote shell one winner without polling state locally. + */ +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) + await execHostCommand(conn, host, acquireInstallLockParentCommand(host, remoteRelayDir), { + signal: options?.signal + }) + + const start = Date.now() + let lastStaleCheckAt = Number.NEGATIVE_INFINITY + while (true) { + try { + const result = await execHostCommand(conn, host, tryCreateInstallLockCommand(host, lockDir), { + signal: options?.signal + }) + if (result.trim().endsWith('OK')) { + return + } + } 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}`) + return + } + } + 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 5dd1b4905ca..49211526f6b 100644 --- a/src/main/ssh/ssh-relay-native-deps-install.test.ts +++ b/src/main/ssh/ssh-relay-native-deps-install.test.ts @@ -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,12 +44,15 @@ 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(true) })) @@ -56,15 +62,16 @@ vi.mock('./ssh-connection-utils', () => ({ })) 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' @@ -226,6 +233,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] @@ -453,6 +461,45 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { 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 () => { const conn = makeMockConnection(sftpCapture) feed( @@ -609,10 +656,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 - '', // remove second probe stderr file '', // no persisted active pipe marker 'WAITING', '', // WMI relay launch @@ -638,6 +683,16 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { .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) @@ -704,6 +759,9 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { await deployAndLaunchRelay(conn) 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( @@ -739,6 +797,157 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { 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('launches degraded when the repair install lock cannot be acquired', async () => { // 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. diff --git a/src/main/ssh/ssh-relay-repair-lock.ts b/src/main/ssh/ssh-relay-repair-lock.ts index ebcac8c09ab..f7cd3e1e8b8 100644 --- a/src/main/ssh/ssh-relay-repair-lock.ts +++ b/src/main/ssh/ssh-relay-repair-lock.ts @@ -11,16 +11,17 @@ import { joinRemotePath, type RemoteHostPlatform } from './ssh-remote-platform' -import { INSTALL_LOCK_STALE_SECONDS, RELAY_INSTALL_LOCK_NAME } from './ssh-relay-versioned-install' +import { INSTALL_LOCK_STALE_SECONDS, RELAY_INSTALL_LOCK_NAME } from './ssh-relay-install-lock' const DEFAULT_REMOTE_HOST = getRemoteHostPlatform('linux-x64') function execHostCommand( conn: SshConnection, host: RemoteHostPlatform, - command: string + command: string, + signal?: AbortSignal ): Promise { - return execCommand(conn, command, { wrapCommand: !isWindowsRemoteHost(host) }) + return execCommand(conn, command, { wrapCommand: !isWindowsRemoteHost(host), signal }) } /** @@ -32,15 +33,22 @@ function execHostCommand( export async function tryAcquireRelayRepairLock( conn: SshConnection, remoteRelayDir: string, - host: RemoteHostPlatform = DEFAULT_REMOTE_HOST + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + options?: { signal?: AbortSignal } ): Promise { const lockDir = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) try { - await execHostCommand(conn, host, acquireInstallLockParentCommand(host, remoteRelayDir)) + await execHostCommand( + conn, + host, + acquireInstallLockParentCommand(host, remoteRelayDir), + options?.signal + ) const firstAttempt = await execHostCommand( conn, host, - tryCreateInstallLockCommand(host, lockDir) + tryCreateInstallLockCommand(host, lockDir), + options?.signal ) if (firstAttempt.trim().endsWith('OK')) { return true @@ -48,7 +56,8 @@ export async function tryAcquireRelayRepairLock( const steal = await execHostCommand( conn, host, - tryStealInstallLockCommand(host, lockDir, INSTALL_LOCK_STALE_SECONDS) + tryStealInstallLockCommand(host, lockDir, INSTALL_LOCK_STALE_SECONDS), + options?.signal ) if (steal.trim().endsWith('OK')) { console.warn(`[ssh-relay] Stealing stale install lock at ${lockDir}`) @@ -56,6 +65,7 @@ export async function tryAcquireRelayRepairLock( } return false } catch { + options?.signal?.throwIfAborted() return false } } diff --git a/src/main/ssh/ssh-relay-versioned-install.test.ts b/src/main/ssh/ssh-relay-versioned-install.test.ts index e7e96ad7c2f..2453b5a8cd7 100644 --- a/src/main/ssh/ssh-relay-versioned-install.test.ts +++ b/src/main/ssh/ssh-relay-versioned-install.test.ts @@ -18,11 +18,11 @@ 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 { execCommand } from './ssh-relay-deploy-helpers' import { getRemoteHostPlatform } from './ssh-remote-platform' @@ -89,6 +89,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 @@ -132,17 +144,7 @@ describe('acquireInstallLock', () => { expect(mockExec).toHaveBeenCalledTimes(2) }) - it('returns immediately when a best-effort repair lock is busy', async () => { - mockExec.mockResolvedValueOnce('').mockResolvedValueOnce('BUSY').mockResolvedValueOnce('BUSY') - - await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe(false) - - const commands = mockExec.mock.calls.map(([, command]) => command) - expect(commands.some((command) => command.includes('.install-lock.steal'))).toBe(true) - expect(commands.filter((command) => command.startsWith('rm -rf'))).toHaveLength(0) - }) - - it('does not steal a live repair lock that exceeds the deploy backstop', async () => { + 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('').mockResolvedValueOnce('BUSY').mockResolvedValueOnce('BUSY') @@ -150,7 +152,7 @@ describe('acquireInstallLock', () => { await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe(false) const commands = mockExec.mock.calls.map(([, command]) => command) - expect(commands.some((command) => command.includes('.install-lock.steal'))).toBe(true) + expect(commands.some((command) => command.includes('lock_tombstone'))).toBe(true) expect(commands.filter((command) => command.startsWith('rm -rf'))).toHaveLength(0) }) @@ -160,8 +162,22 @@ describe('acquireInstallLock', () => { await expect(tryAcquireRelayRepairLock(conn, '/r')).resolves.toBe(true) const commands = mockExec.mock.calls.map(([, command]) => command) - expect(commands.some((command) => command.includes('.install-lock.steal'))).toBe(true) - expect(commands.filter((command) => command.includes('.install-lock.steal'))).toHaveLength(1) + expect(commands.some((command) => command.includes('lock_tombstone'))).toBe(true) + expect(commands.filter((command) => command.includes('lock_tombstone'))).toHaveLength(1) + }) + + 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 () => { @@ -176,6 +192,7 @@ describe('acquireInstallLock', () => { .mockResolvedValueOnce('') .mockResolvedValueOnce('BUSY') .mockResolvedValueOnce('BUSY') + .mockResolvedValueOnce('BUSY') .mockResolvedValueOnce('OK') const promise = acquireInstallLock(conn, '/r') @@ -192,15 +209,26 @@ describe('acquireInstallLock', () => { } }) - it('steals a stale lock with a serialized remote steal command', async () => { + it('immediately tries to steal an already-stale lock', async () => { + mockExec.mockResolvedValueOnce('').mockResolvedValueOnce('BUSY').mockResolvedValueOnce('OK') + + await expect(acquireInstallLock(conn, '/r')).resolves.toBeUndefined() + + const cmds = mockExec.mock.calls.map(([, c]) => c) + expect(cmds).toHaveLength(3) + expect(cmds[2]).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 { + const recoverableAt = Date.now() + 60_000 mockExec.mockImplementation(async (_conn: unknown, cmd: string) => { if (cmd.startsWith('mkdir -p')) { return '' } - if (cmd.includes('.install-lock.steal')) { - return 'OK' + if (cmd.includes('lock_tombstone')) { + return Date.now() >= recoverableAt ? 'OK' : 'BUSY' } if (cmd.includes('mkdir') && cmd.includes('.install-lock')) { return 'BUSY' @@ -209,12 +237,12 @@ describe('acquireInstallLock', () => { }) const promise = acquireInstallLock(conn, '/r') - // Drive through the full deploy bound so stale recovery fires. - await vi.advanceTimersByTimeAsync(545_000) - await promise + await vi.advanceTimersByTimeAsync(61_000) - const cmds = mockExec.mock.calls.map(([, c]) => c) - expect(cmds.some((c) => c.includes('.install-lock.steal'))).toBe(true) + await expect(promise).resolves.toBeUndefined() + expect(mockExec.mock.calls.filter(([, cmd]) => cmd.includes('lock_tombstone'))).toHaveLength( + 2 + ) } finally { vi.useRealTimers() } @@ -227,7 +255,7 @@ describe('acquireInstallLock', () => { if (cmd.startsWith('mkdir -p')) { return '' } - if (cmd.includes('.install-lock.steal')) { + if (cmd.includes('lock_tombstone')) { return 'BUSY' } if (cmd.includes('mkdir') && cmd.includes('.install-lock')) { @@ -239,14 +267,37 @@ describe('acquireInstallLock', () => { return '' }) - const rejection = expect(acquireInstallLock(conn, '/r')).rejects.toThrow(/not yet stale/i) - await vi.advanceTimersByTimeAsync(545_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.mockResolvedValueOnce('').mockResolvedValue('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 { @@ -265,7 +316,9 @@ describe('acquireInstallLock', () => { await vi.advanceTimersByTimeAsync(501_000) await expect(promise).resolves.toBeUndefined() - expect(mockExec.mock.calls.some(([, cmd]) => cmd.includes('.install-lock.steal'))).toBe(false) + expect( + mockExec.mock.calls.filter(([, cmd]) => cmd.includes('lock_tombstone')).length + ).toBeGreaterThan(1) } finally { vi.useRealTimers() } diff --git a/src/main/ssh/ssh-relay-versioned-install.ts b/src/main/ssh/ssh-relay-versioned-install.ts index 4028bf4233b..88df794cd02 100644 --- a/src/main/ssh/ssh-relay-versioned-install.ts +++ b/src/main/ssh/ssh-relay-versioned-install.ts @@ -15,13 +15,8 @@ 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 { RELAY_DEPLOY_TIMEOUT_MS } from './ssh-relay-deploy-timing' -import { - acquireInstallLockParentCommand, - lockAgeSecondsCommand, - tryCreateInstallLockCommand, - tryStealInstallLockCommand -} from './ssh-relay-install-lock-commands' +import { lockAgeSecondsCommand } from './ssh-relay-install-lock-commands' +import { INSTALL_LOCK_STALE_MS, RELAY_INSTALL_LOCK_NAME } from './ssh-relay-install-lock' import { listRelayBaseDirsCommand, probeDirectoryExistsCommand, @@ -55,21 +50,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+$/ -export const RELAY_INSTALL_LOCK_NAME = '.install-lock' const INSTALL_COMPLETE_NAME = '.install-complete' - -const INSTALL_LOCK_POLL_MS = 1_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: 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. Native-deps repair can keep running after the deploy backstop wins -// Promise.race, so leave enough room for its bounded under-lock exec sequence. -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') type RelayInstalledProbeOptions = { @@ -152,6 +133,7 @@ export async function isRelayAlreadyInstalled( ) return probe.trim() === 'OK' } catch (err) { + options?.signal?.throwIfAborted() if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err } @@ -159,58 +141,7 @@ 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, RELAY_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() - 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) { - // Stale-lock recovery is one remote command so two observers cannot both - // delete a lock that the other one just recreated. - const steal = await execHostCommand( - conn, - host, - tryStealInstallLockCommand(host, lockDir, INSTALL_LOCK_STALE_SECONDS) - ).catch(() => 'BUSY') - if (steal.trim().endsWith('OK')) { - console.warn(`[ssh-relay] Stealing stale install lock at ${lockDir}`) - return - } - 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)) - } -} - -export async function isRelayInstallLockStale( +async function isRelayInstallLockStale( conn: SshConnection, lockDir: string, host: RemoteHostPlatform = DEFAULT_REMOTE_HOST @@ -238,12 +169,18 @@ export async function isRelayInstallLockStale( export async function finalizeInstall( conn: SshConnection, remoteRelayDir: string, - host: RemoteHostPlatform = DEFAULT_REMOTE_HOST + host: RemoteHostPlatform = DEFAULT_REMOTE_HOST, + options?: { signal?: AbortSignal } ): Promise { const sentinel = joinRemotePath(host, remoteRelayDir, INSTALL_COMPLETE_NAME) const lock = joinRemotePath(host, remoteRelayDir, RELAY_INSTALL_LOCK_NAME) - await execHostCommand(conn, host, writeRemoteEmptyFileCommand(host, sentinel)) - await execHostCommand(conn, host, removeRemoteTreeCommand(host, lock)).catch(() => {}) + await execHostCommand(conn, host, writeRemoteEmptyFileCommand(host, sentinel), { + signal: options?.signal + }) + await execHostCommand(conn, host, removeRemoteTreeCommand(host, lock), { + signal: options?.signal + }).catch(() => {}) + options?.signal?.throwIfAborted() } /** diff --git a/src/main/ssh/ssh-remote-commands.test.ts b/src/main/ssh/ssh-remote-commands.test.ts index ae62a46de7c..460fb4651f6 100644 --- a/src/main/ssh/ssh-remote-commands.test.ts +++ b/src/main/ssh/ssh-remote-commands.test.ts @@ -163,7 +163,7 @@ describe('ssh remote command builders', () => { expect(windowsScript).toContain('Write-Output ($now - $mtime)') }) - it('serializes stale install-lock recovery under a sibling steal mutex', () => { + it('serializes stale recovery with unbounded numbered sibling claims', () => { const posixCommand = tryStealInstallLockCommand( posix, '/home/me/.orca-remote/relay/.install-lock', @@ -173,19 +173,20 @@ describe('ssh remote command builders', () => { 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('$mtime') + 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_age') - expect(posixCommand).toContain('steal="$steal.next.$steal_token"') + expect(posixCommand).toContain('steal_generation + 1') + expect(posixCommand).not.toContain('.next.') expect(posixCommand).toContain('trap') - expect(windowsScript).toContain('$lock.steal.$mtime') + expect(windowsScript).toContain('$lock.steal') + expect(windowsScript).toContain('$stealGeneration++') expect(windowsScript).toContain('-gt 1200') expect(windowsScript).toContain('$currentIdentity -eq $lockIdentity') - expect(windowsScript).toContain('$stealAge -le 120') - expect(windowsScript).toContain('$steal = "$steal.next.$stealIdentity"') - expect(windowsScript).toContain('New-Item -ItemType Directory -Path $steal') + expect(windowsScript).not.toContain('.next.') expect(windowsScript).toContain('finally') }) @@ -208,7 +209,7 @@ describe('ssh remote command builders', () => { }) it.runIf(process.platform !== 'win32')( - 'recovers a stale POSIX steal mutex left by a crashed stealer', + 'recovers and cleans more than eight orphaned numbered steal claims', () => { const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-')) try { @@ -217,17 +218,18 @@ describe('ssh remote command builders', () => { const staleDate = new Date(Date.now() - 60 * 60_000) utimesSync(lockDir, staleDate, staleDate) const lockMtimeSeconds = Math.floor(statSync(lockDir).mtimeMs / 1000) - const stealDir = `${lockDir}.steal.${lockMtimeSeconds}` - mkdirSync(stealDir) - utimesSync(stealDir, staleDate, staleDate) + 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(existsSync(stealDir)).toBe(true) - expect(readdirSync(root).some((name) => name.includes('.next.'))).toBe(false) + 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 }) @@ -236,7 +238,7 @@ describe('ssh remote command builders', () => { ) it.runIf(process.platform !== 'win32')( - 'lets only one POSIX stealer take over a preexisting stale steal mutex', + 'lets only one POSIX caller move and recreate a stale install lock', async () => { const root = mkdtempSync(join(tmpdir(), 'orca-install-lock-race-')) try { @@ -244,11 +246,6 @@ describe('ssh remote command builders', () => { mkdirSync(lockDir) const staleDate = new Date(Date.now() - 60 * 60_000) utimesSync(lockDir, staleDate, staleDate) - const lockMtimeSeconds = Math.floor(statSync(lockDir).mtimeMs / 1000) - const stealDir = `${lockDir}.steal.${lockMtimeSeconds}` - mkdirSync(stealDir) - utimesSync(stealDir, staleDate, staleDate) - const command = tryStealInstallLockCommand(posix, lockDir, 20 * 60) const outputs = await Promise.all( Array.from({ length: 64 }, () => runShellCommand(command)) @@ -257,8 +254,6 @@ describe('ssh remote command builders', () => { expect(okCount).toBe(1) expect(existsSync(lockDir)).toBe(true) - expect(existsSync(stealDir)).toBe(true) - expect(readdirSync(root).some((name) => name.includes('.next.'))).toBe(false) expect(readdirSync(root).some((name) => name.includes('.tombstone'))).toBe(false) } finally { rmSync(root, { recursive: true, force: true }) diff --git a/src/main/ssh/ssh-remote-node-resolution.ts b/src/main/ssh/ssh-remote-node-resolution.ts index 01d6a0c7d48..a5041ed158d 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 f72b5384335..44c31e617e4 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 { 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 1f0ce5ed6df..eab96245ba5 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 76a2ae3fb12..c9c4424325b 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) { From 94687d9ad6b023eb54436c1751a872a6daba6c20 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:14:56 -0400 Subject: [PATCH 5/5] Fix Windows relay native repair and locking Probe node-pty's actual Windows binding, reset broken package trees before repair, and keep degraded relays retryable after self-heal. Use atomic FileMode.CreateNew locks under Windows PowerShell 5.1 and cover lock races and platform-specific system SSH behavior. --- src/main/ssh/ssh-relay-deploy.ts | 113 ++++++++++++++--- .../ssh/ssh-relay-install-lock-commands.ts | 27 +++- src/main/ssh/ssh-relay-install-lock.ts | 5 +- .../ssh/ssh-relay-native-deps-install.test.ts | 10 +- src/main/ssh/ssh-relay-versioned-install.ts | 8 +- src/main/ssh/ssh-remote-commands.test.ts | 118 +++++++++++++++++- src/main/ssh/ssh-system-fallback.test.ts | 35 +++--- src/relay/pty-handler.test.ts | 23 ++++ src/relay/pty-handler.ts | 103 +++++++++------ 9 files changed, 360 insertions(+), 82 deletions(-) diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index 35161c15ccc..a68b3207478 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -309,7 +309,7 @@ async function deployAndLaunchRelayInner( 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, { signal: deploySignal }) try { @@ -487,41 +487,64 @@ const RELAY_NATIVE_DEPS = { '@parcel/watcher': '2.5.6' } as const +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]) ) -const LOAD_RELAY_NATIVE_DEPS = 'require("node-pty"); require("@parcel/watcher")' +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)})}})()` +} -async function hasRequiredNativeDeps( +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, signal?: AbortSignal -): Promise { +): 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(`${LOAD_RELAY_NATIVE_DEPS}; console.log("ORCA-NATIVE-DEPS-OK")`)} } catch { 'MISSING' }` + `try { & ${powerShellLiteral(nodePath)} -e ${powerShellNativeArg(probeJs)} } catch { 'MISSING' }` ) : commandWithNodePath( hostPlatform, nodePath, remoteDir, - `(${escapedNode} -e ${shellEscape(`${LOAD_RELAY_NATIVE_DEPS}; 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, { signal }) - return probe.includes('ORCA-NATIVE-DEPS-OK') + const available = probe.includes('ORCA-NATIVE-DEPS-OK') + return { available, missing: available ? [] : missingNativeDepsFromProbe(probe) } } catch { signal?.throwIfAborted() - return false + return { available: false, missing: [...RELAY_NATIVE_DEP_NAMES] } } } @@ -533,7 +556,7 @@ async function repairInstalledNativeDeps( nodePath: string, signal?: AbortSignal ): Promise { - if (await hasRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal)) { + if ((await probeRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal)).available) { return } @@ -550,8 +573,17 @@ async function repairInstalledNativeDeps( 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, signal))) { - await installNativeDeps(conn, remoteDir, platform, hostPlatform, nodePath, signal) + 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 }) } else { await abandonInstall(conn, remoteDir, hostPlatform) @@ -584,7 +616,8 @@ async function installNativeDeps( platform: RelayPlatform, hostPlatform: RemoteHostPlatform, nodePath: string, - signal?: AbortSignal + signal?: AbortSignal, + resetDeps: RelayNativeDepName[] = [] ): Promise { // 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`) @@ -614,12 +647,16 @@ async function installNativeDeps( 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 --ignore-scripts=false --omit=dev --no-audit --no-fund ${Object.entries( + `${resetPrefix}npm install --ignore-scripts=false --omit=dev --no-audit --no-fund ${Object.entries( RELAY_NATIVE_DEPS ) .map(([dep, version]) => powerShellLiteral(`${dep}@${version}`)) @@ -629,7 +666,7 @@ async function installNativeDeps( hostPlatform, nodePath, remoteDir, - `npm install --ignore-scripts=false --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_COMMAND_TIMEOUT_MS, @@ -696,6 +733,44 @@ async function installNativeDeps( } } +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, @@ -748,13 +823,18 @@ async function probeInstalledNativeDeps( hostPlatform: RemoteHostPlatform, nodePath: string, signal?: AbortSignal -): Promise<{ available: boolean; output: string; stderr: string }> { +): 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 = `${LOAD_RELAY_NATIVE_DEPS}; console.log(process.argv[1])` + const probeJs = nativeDepsProbeJs(PROBE_OK) const probeCommand = isWindowsRemoteHost(hostPlatform) ? commandWithNodePath( hostPlatform, @@ -785,6 +865,7 @@ async function probeInstalledNativeDeps( } return { available: probeOutput.includes(PROBE_OK), + missing: probeOutput.includes(PROBE_OK) ? [] : missingNativeDepsFromProbe(probeOutput), output: probeOutput, stderr: remoteStderr } diff --git a/src/main/ssh/ssh-relay-install-lock-commands.ts b/src/main/ssh/ssh-relay-install-lock-commands.ts index 20ca1b7ec53..78878b7a2ee 100644 --- a/src/main/ssh/ssh-relay-install-lock-commands.ts +++ b/src/main/ssh/ssh-relay-install-lock-commands.ts @@ -18,9 +18,26 @@ export function tryCreateInstallLockCommand(host: RemoteHostPlatform, lockDir: s 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. + // Why: concurrent New-Item directory calls can both report success in Windows + // PowerShell 5.1. FileMode.CreateNew maps to an exclusive Win32 create. return powerShellCommand( - `$ErrorActionPreference = "Stop"; try { $null = New-Item -ItemType Directory -Path ${powerShellLiteral(lockDir)}; 'OK' } catch { 'BUSY' }` + [ + '$stream = $null', + 'try {', + `$stream = [System.IO.File]::Open(${powerShellLiteral(lockDir)}, [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` + } + // Windows locks are files now, but old Orca versions may leave directories. + return powerShellCommand( + `if (Test-Path -LiteralPath ${powerShellLiteral(lockPath)}) { 'LOCKED' } else { 'OPEN' }` ) } @@ -96,7 +113,8 @@ function windowsStealInstallLockCommand(lockDir: string, staleAfterSeconds: numb 'try {', 'while (-not $ownsSteal) {', 'try {', - '$null = New-Item -ItemType Directory -Path $steal -ErrorAction Stop', + '$stealStream = [System.IO.File]::Open($steal, [System.IO.FileMode]::CreateNew, [System.IO.FileAccess]::Write, [System.IO.FileShare]::None)', + '$stealStream.Dispose()', '$ownsSteal = $true', 'break', '} catch {', @@ -118,7 +136,8 @@ function windowsStealInstallLockCommand(lockDir: string, staleAfterSeconds: numb `if (($currentIdentity -eq $lockIdentity) -and (($currentNow - $currentMtime) -gt ${staleAfterSeconds})) {`, '$lockTombstone = "$lock.tombstone.$PID.$([DateTimeOffset]::UtcNow.ToUnixTimeMilliseconds())"', 'Move-Item -LiteralPath $lock -Destination $lockTombstone -ErrorAction Stop', - "try { $null = New-Item -ItemType Directory -Path $lock -ErrorAction Stop; 'OK' } catch { 'BUSY' }", + '$successorStream = $null', + "try { $successorStream = [System.IO.File]::Open($lock, [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 {", diff --git a/src/main/ssh/ssh-relay-install-lock.ts b/src/main/ssh/ssh-relay-install-lock.ts index 47dab9d00e1..d31fd0dae9d 100644 --- a/src/main/ssh/ssh-relay-install-lock.ts +++ b/src/main/ssh/ssh-relay-install-lock.ts @@ -41,8 +41,9 @@ function execHostCommand( } /** - * Acquire the per-version install lock via atomic `mkdir`. - * Why: `mkdir` gives every supported remote shell one winner without polling state locally. + * Acquire the per-version install lock via a host-native exclusive create. + * Why: POSIX mkdir and Windows FileMode.CreateNew each give one winner without + * polling state locally. */ export async function acquireInstallLock( conn: SshConnection, 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 49211526f6b..e759e89fa3a 100644 --- a/src/main/ssh/ssh-relay-native-deps-install.test.ts +++ b/src/main/ssh/ssh-relay-native-deps-install.test.ts @@ -676,6 +676,7 @@ 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) @@ -746,8 +747,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', @@ -769,6 +770,10 @@ 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 () => { @@ -990,6 +995,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => { .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') }) diff --git a/src/main/ssh/ssh-relay-versioned-install.ts b/src/main/ssh/ssh-relay-versioned-install.ts index 88df794cd02..d96fc2db439 100644 --- a/src/main/ssh/ssh-relay-versioned-install.ts +++ b/src/main/ssh/ssh-relay-versioned-install.ts @@ -15,11 +15,13 @@ 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 { lockAgeSecondsCommand } from './ssh-relay-install-lock-commands' +import { + lockAgeSecondsCommand, + probeInstallLockExistsCommand +} from './ssh-relay-install-lock-commands' import { INSTALL_LOCK_STALE_MS, RELAY_INSTALL_LOCK_NAME } from './ssh-relay-install-lock' import { listRelayBaseDirsCommand, - probeDirectoryExistsCommand, probeFileExistsCommand, probeRelayInstalledCommand, relayLivenessProbeCommand, @@ -283,7 +285,7 @@ async function isCandidateSafeToRemove( const lockProbe = await execHostCommand( conn, host, - probeDirectoryExistsCommand(host, lockDir) + probeInstallLockExistsCommand(host, lockDir) ).catch(() => 'OPEN') const locked = lockProbe.trim() === 'LOCKED' diff --git a/src/main/ssh/ssh-remote-commands.test.ts b/src/main/ssh/ssh-remote-commands.test.ts index 460fb4651f6..4ba506780a0 100644 --- a/src/main/ssh/ssh-remote-commands.test.ts +++ b/src/main/ssh/ssh-remote-commands.test.ts @@ -37,6 +37,13 @@ const powerShellExecutable = ( }) 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+/=]+)/) @@ -69,6 +76,32 @@ function runShellCommand(command: string): Promise { }) } +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') @@ -84,7 +117,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 -Path for Windows directory creation and an exclusive file lock', () => { const mkdirScript = decodePowerShellCommand( makeRemoteDirectoryCommand(windows, 'C:/Users/me/.orca-remote') ) @@ -93,9 +126,10 @@ 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(mkdirScript).not.toContain('New-Item -ItemType Directory -Force -LiteralPath') - expect(lockScript).not.toContain('New-Item -ItemType Directory -LiteralPath') + expect(lockScript).not.toContain('New-Item -ItemType Directory') }) it('uses named pipe try-connect liveness for Windows GC', () => { @@ -146,7 +180,7 @@ 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') }) @@ -186,6 +220,7 @@ describe('ssh remote command builders', () => { 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') }) @@ -208,6 +243,81 @@ describe('ssh remote command builders', () => { expect(result.stdout.trim()).toBe('BUSY') }) + 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).isFile()).toBe(true) + } finally { + rmSync(root, { recursive: true, force: true }) + } + } + ) + + 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).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 }) + } + } + ) + + 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).isFile()).toBe(true) + expect(readdirSync(root).filter((name) => name.includes('.tombstone.'))).toHaveLength(0) + } finally { + rmSync(root, { recursive: true, force: true }) + } + } + ) + it.runIf(process.platform !== 'win32')( 'recovers and cleans more than eight orphaned numbered steal claims', () => { diff --git a/src/main/ssh/ssh-system-fallback.test.ts b/src/main/ssh/ssh-system-fallback.test.ts index 46602619e23..71f94204d58 100644 --- a/src/main/ssh/ssh-system-fallback.test.ts +++ b/src/main/ssh/ssh-system-fallback.test.ts @@ -83,6 +83,16 @@ function expectNoOrcaControlMasterArgs(args: string[]): void { expect(args).not.toContain('ControlPersist=300') } +function expectOrcaControlMasterArgs(args: string[]): void { + if (process.platform === 'win32') { + expectNoOrcaControlMasterArgs(args) + return + } + expect(args).toContain('ControlMaster=auto') + expect(args.some((arg) => 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', () => { diff --git a/src/relay/pty-handler.test.ts b/src/relay/pty-handler.test.ts index efab6bce51f..8de7ec8051b 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 74fd755e6fd..1e0e3d65ba1 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,28 +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 { - try { - // Why: Node caches rejected dynamic imports, while a failed CommonJS - // load remains retryable after deploy repairs node_modules in place. - ptyModule = require('node-pty') as typeof NodePty - 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 = { @@ -303,6 +288,8 @@ 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 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 @@ -323,6 +310,41 @@ export class PtyHandler { this.registerHandlers() } + private async loadPty(): Promise { + if (this.ptyModule) { + return this.ptyModule + } + if (!this.reloadPtyModuleFromDisk) { + try { + this.ptyModule = await import('node-pty') + return this.ptyModule + } catch { + this.reloadPtyModuleFromDisk = true + } + } + const moduleEntry = join(process.cwd(), '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(process.cwd(), '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)) } @@ -748,7 +770,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') } @@ -821,17 +843,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 @@ -1222,7 +1255,7 @@ export class PtyHandler { } private async reviveEntry(entry: SerializedPtyEntry): Promise { - const ptyMod = await loadPty() + const ptyMod = await this.loadPty() if (!ptyMod) { return }