Merge remote-tracking branch 'origin/fix/native-deps-markerless-probe' into adhoc/ssh-sweep-combined

src/main/ssh/ssh-relay-deploy-helpers.test.ts

@@ -550,6 +550,27 @@ describe('execCommand', () => {
     expect(channel.stderr.listenerCount('data')).toBe(0)
   })
 
+  it('hands a zero-exit command stderr to onStderr instead of dropping it', async () => {
+    // Why: probes fenced with `|| echo MISSING` always exit 0, so the resolve path used to be the
+    // one place the failure reason was discarded.
+    const channel = createMockChannel()
+    const conn = { exec: vi.fn().mockResolvedValue(channel) }
+    const captured: string[] = []
+    const commandPromise = execCommand(conn as never, "(node -e 'x' || echo MISSING)", {
+      onStderr: (stderr) => captured.push(stderr)
+    })
+
+    await Promise.resolve()
+    channel.stderr.emit('data', Buffer.from('node: --bogus is not allowed in NODE_OPTIONS\n'))
+    channel.emit('data', Buffer.from('MISSING\n'))
+    channel.emit('close', 0)
+
+    await expect(commandPromise).resolves.toBe('MISSING\n')
+    expect(captured).toEqual(['node: --bogus is not allowed in NODE_OPTIONS\n'])
+    // onStderr must not leak into the SSH exec options.
+    expect(conn.exec).toHaveBeenCalledWith("(node -e 'x' || echo MISSING)", {})
+  })
+
   it('uses custom command timeouts without forwarding them to SSH exec', async () => {
     vi.useFakeTimers()
     try {

src/main/ssh/ssh-relay-deploy.ts

@@ -118,12 +118,13 @@ function execHostCommand(
   conn: SshConnection,
   hostPlatform: RemoteHostPlatform,
   command: string,
-  options?: { timeoutMs?: number; signal?: AbortSignal }
+  options?: { timeoutMs?: number; signal?: AbortSignal; onStderr?: (stderr: string) => void }
 ): Promise<string> {
   return execCommand(conn, command, {
     wrapCommand: !isWindowsRemoteHost(hostPlatform),
     timeoutMs: options?.timeoutMs,
-    signal: options?.signal
+    signal: options?.signal,
+    onStderr: options?.onStderr
   })
 }
 
@@ -735,24 +736,34 @@ function nativeDepsProbeJs(successToken: string): string {
   return `(()=>{const missing=[];try{${loadNodePty}}catch{missing.push("node-pty")}try{require("@parcel/watcher")}catch{missing.push("@parcel/watcher")}if(missing.length){console.log("${NATIVE_DEPS_MISSING_PREFIX}"+missing.join(","));process.exitCode=1}else{console.log(${JSON.stringify(successToken)})}})()`
 }
 
-function missingNativeDepsFromProbe(output: string): RelayNativeDepName[] {
+/**
+ * Which deps the probe *named* as unloadable, or `undefined` when the answer names none.
+ *
+ * Only the probe's own marker line is evidence about the deps. An answer without one (node never
+ * ran, was killed, exited before the script) says nothing, so it must not be read as "all of them" —
+ * that inference deleted both native modules on every reconnect of an affected host.
+ */
+function missingNativeDepsFromProbe(output: string): RelayNativeDepName[] | undefined {
   const marker = output
     .split(/\r?\n/)
     .find((line) => line.trim().startsWith(NATIVE_DEPS_MISSING_PREFIX))
   if (!marker) {
-    return [...RELAY_NATIVE_DEP_NAMES]
+    return undefined
   }
   const reported = marker.trim().slice(NATIVE_DEPS_MISSING_PREFIX.length).split(',')
-  return RELAY_NATIVE_DEP_NAMES.filter((name) => reported.includes(name))
+  const named = RELAY_NATIVE_DEP_NAMES.filter((name) => reported.includes(name))
+  return named.length > 0 ? named : undefined
 }
 
 /**
- * `ok` — the probe answered and both deps loaded. `blocked` — the probe answered and named deps
- * that failed to load. `unverifiable` — the probe never answered, which is evidence about the
- * transport, not about the deps.
+ * `ok` — the probe answered and both deps loaded. `blocked` — the probe answered with a marker
+ * naming deps that failed to load. `unverifiable` — the probe never answered, or answered nothing
+ * that names a dep; both are evidence about the probe, not about the deps.
  *
  * Why `unverifiable` is not `blocked`: repairing on it does `rm -rf node_modules/node-pty` and a
- * node-gyp source build (no Linux prebuild) against a relay that was never shown to be broken.
+ * node-gyp source build (no Linux prebuild) against a relay that was never shown to be broken. An
+ * unparseable answer is the worse half of that — it is deterministic and per-host, so a node that
+ * cannot start (bad NODE_OPTIONS, OOM, exit 127) deleted both modules on every reconnect forever.
  * Same verdict discipline as `src/main/orcad/node-pty-precondition.ts` and
  * docs/reference/ssh-execution-boundary.md — loss of contact is not evidence.
  */
@@ -767,6 +778,7 @@ async function probeRequiredNativeDeps(
 ): Promise<{ status: RelayNativeDepsProbeStatus; missing: RelayNativeDepName[] }> {
   const escapedNode = shellEscape(nodePath)
   const probeJs = nativeDepsProbeJs('ORCA-NATIVE-DEPS-OK')
+  let probeStderr = ''
   try {
     const command = isWindowsRemoteHost(hostPlatform)
       ? commandWithNodePath(
@@ -775,16 +787,32 @@ async function probeRequiredNativeDeps(
           remoteDir,
           `try { & ${powerShellLiteral(nodePath)} -e ${powerShellNativeArg(probeJs)} } catch { 'MISSING' }`
         )
-      : commandWithNodePath(
+      : // Why: no `2>/dev/null` — it discarded the only line that says why node never reached the
+        // script. stderr stays its own stream so it can't be mistaken for the verdict, mirroring
+        // src/main/orcad/node-pty-precondition.ts.
+        commandWithNodePath(
           hostPlatform,
           nodePath,
           remoteDir,
-          `(${escapedNode} -e ${shellEscape(probeJs)} 2>/dev/null || echo MISSING)`
+          `(${escapedNode} -e ${shellEscape(probeJs)} || echo MISSING)`
         )
-    const probe = await execHostCommand(conn, hostPlatform, command, { signal })
-    return probe.includes('ORCA-NATIVE-DEPS-OK')
-      ? { status: 'ok', missing: [] }
-      : { status: 'blocked', missing: missingNativeDepsFromProbe(probe) }
+    const probe = await execHostCommand(conn, hostPlatform, command, {
+      signal,
+      onStderr: (text) => {
+        probeStderr = text
+      }
+    })
+    if (probe.includes('ORCA-NATIVE-DEPS-OK')) {
+      return { status: 'ok', missing: [] }
+    }
+    const missing = missingNativeDepsFromProbe(probe)
+    if (!missing) {
+      console.warn(
+        `[ssh-relay][NATIVE-DEPS-PROBE-UNPARSEABLE] Probe at ${remoteDir} answered without naming a dep; launching as-is. stdout=${probe.trim().slice(-200)} stderr=${probeStderr.trim().slice(-500)}`
+      )
+      return { status: 'unverifiable', missing: [] }
+    }
+    return { status: 'blocked', missing }
   } catch {
     signal?.throwIfAborted()
     // Why: an unanswered probe says nothing about the deps; reporting MISSING here reset and
@@ -1350,7 +1378,8 @@ async function probeInstalledNativeDeps(
   }
   return {
     available: probeOutput.includes(PROBE_OK),
-    missing: probeOutput.includes(PROBE_OK) ? [] : missingNativeDepsFromProbe(probeOutput),
+    // A markerless answer names no dep, so it reports none; `available` already carries the failure.
+    missing: probeOutput.includes(PROBE_OK) ? [] : (missingNativeDepsFromProbe(probeOutput) ?? []),
     output: probeOutput,
     stderr: remoteStderr
   }

src/main/ssh/ssh-relay-exec-command.ts

@@ -13,6 +13,11 @@ const MAX_EXEC_OUTPUT_CHARS = 1024 * 1024
 
 type ExecCommandOptions = SshExecOptions & {
   timeoutMs?: number
+  // Why: a zero-exit command resolves with stdout alone, so the reason a wrapped-in-`|| echo`
+  // probe failed is discarded. Callers that need that diagnostic opt in here rather than
+  // folding stderr into stdout, where it would match the probe's own token strings.
+  // On the system-ssh transport this stream also carries local OpenSSH noise; log-only.
+  onStderr?: (stderr: string) => void
 }
 
 type SshCommandTerminationError = Error & {
@@ -33,7 +38,7 @@ export async function execCommand(
   command: string,
   options?: ExecCommandOptions
 ): Promise<string> {
-  const { timeoutMs = EXEC_TIMEOUT_MS, ...execOptions } = options ?? {}
+  const { timeoutMs = EXEC_TIMEOUT_MS, onStderr, ...execOptions } = options ?? {}
   const signal = options?.signal
   if (signal?.aborted) {
     throw createSshOperationAbortError()
@@ -151,6 +156,9 @@ export async function execCommand(
           )
         )
       } else {
+        if (stderr && onStderr) {
+          onStderr(redactRelayInstallMarkerTokens(stderr))
+        }
         settle(resolve, stdout)
       }
     }

src/main/ssh/ssh-relay-native-deps-install-fixture.ts

@@ -54,6 +54,11 @@ export function makeMockConnection(capture: SftpWriteCapture): SshConnection {
 
 export type ExecResponse = string | { reject: string }
 
+// The answer a genuinely broken pair produces: a marker line naming both deps. A bare `MISSING`
+// names none, so it is unverifiable and must never stand in for this.
+export const BOTH_NATIVE_DEPS_MISSING_PROBE =
+  'ORCA-NATIVE-DEPS-MISSING:node-pty,@parcel/watcher\nMISSING'
+
 const STAGE_OWNER = '.sftp-namespace-00000000000000000000000000000000'
 
 export function makeStagedFirstInstallExecPrefix(): ExecResponse[] {
@@ -71,12 +76,11 @@ export function makeStagedFirstInstallExecPrefix(): ExecResponse[] {
 // Repair reconnect (isRelayAlreadyInstalled → true) where BOTH native deps are broken and the host
 // cannot compile node-pty, so the caller's resets must survive into the node-pty-less reinstall.
 export function makeRepairToolchainSkipExecResponses(): ExecResponse[] {
-  const bothMissing = 'ORCA-NATIVE-DEPS-MISSING:node-pty,@parcel/watcher\nMISSING'
   return [
     '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
     '/home/u',
-    bothMissing, // health probe before lock
-    bothMissing, // re-probe under the repair lock
+    BOTH_NATIVE_DEPS_MISSING_PROBE, // health probe before lock
+    BOTH_NATIVE_DEPS_MISSING_PROBE, // re-probe under the repair lock
     '', // SFTP-namespace install-owner marker (repair)
     { reject: 'gyp ERR! stack Error: not found: make' },
     'PKG apk', // toolchain probe: no HAVE lines
@@ -139,7 +143,7 @@ export function makeExecResponses(opts: {
       '', // rm -rf node-pty + reinstall without it
       // node-pty is always reported missing here; the probe never resolves OK, so cat + rm both run.
       opts.nodePtySkipWatcher === 'missing'
-        ? 'ORCA-NATIVE-DEPS-MISSING:node-pty,@parcel/watcher\nMISSING\n'
+        ? `${BOTH_NATIVE_DEPS_MISSING_PROBE}\n`
         : 'ORCA-NATIVE-DEPS-MISSING:node-pty\nMISSING\n',
       '', // cat probe stderr
       '', // rm -f probe stderr

src/main/ssh/ssh-relay-native-deps-install.test.ts

@@ -83,6 +83,7 @@ import {
 import { acquireInstallLock } from './ssh-relay-install-lock'
 import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock'
 import {
+  BOTH_NATIVE_DEPS_MISSING_PROBE,
   decodePowerShellCommand,
   makeExecResponses,
   makeMockConnection,
@@ -677,8 +678,8 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => {
     feed([
       '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
       '/home/u',
-      'MISSING', // health probe: require() fails
-      'MISSING', // re-probe after lock
+      BOTH_NATIVE_DEPS_MISSING_PROBE, // health probe: require() names both deps
+      BOTH_NATIVE_DEPS_MISSING_PROBE, // re-probe after lock
       '', // SFTP-namespace install-owner marker (repair)
       { reject: 'npm ERR! network ETIMEDOUT' }, // npm install fails (offline)
       'DEAD',
@@ -701,8 +702,8 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => {
     vi.mocked(execCommand)
       .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64')
       .mockResolvedValueOnce('/home/u')
-      .mockResolvedValueOnce('MISSING')
-      .mockResolvedValueOnce('MISSING')
+      .mockResolvedValueOnce(BOTH_NATIVE_DEPS_MISSING_PROBE)
+      .mockResolvedValueOnce(BOTH_NATIVE_DEPS_MISSING_PROBE)
       .mockResolvedValueOnce('') // SFTP-namespace install-owner marker (repair)
       .mockRejectedValueOnce(
         Object.assign(new Error('npm termination was not confirmed'), {
@@ -843,7 +844,7 @@ describe('installNativeDeps (via deployAndLaunchRelay)', () => {
     feed([
       '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
       '/home/u',
-      'MISSING',
+      BOTH_NATIVE_DEPS_MISSING_PROBE,
       'DEAD',
       '', // remote credential generation without a namespace marker
       'READY'

src/main/ssh/ssh-relay-native-deps-probe-verdict.test.ts

@@ -75,8 +75,11 @@ vi.mock('./ssh-connection-utils', () => ({
 import { deployAndLaunchRelay } from './ssh-relay-deploy'
 import { execCommand, uploadDirectory } from './ssh-relay-deploy-helpers'
 import { parseUnameToRelayPlatform } from './relay-protocol'
+import { resolveRemoteNodePath } from './ssh-remote-node-resolution'
 import { finalizeInstall, isRelayAlreadyInstalled } from './ssh-relay-versioned-install'
 import {
+  BOTH_NATIVE_DEPS_MISSING_PROBE,
+  decodePowerShellCommand,
   makeMockConnection,
   type ExecResponse,
   type SftpWriteCapture
@@ -156,13 +159,67 @@ describe('native-deps repair probe verdicts', () => {
     expect(outcome, 'lost contact must not abort the connection').not.toBeInstanceOf(Error)
   })
 
-  it('still resets and repairs when the probe answers without the OK marker', async () => {
+  it('leaves node_modules intact when the probe answers without naming a dep', async () => {
+    // The bare `MISSING` a `|| echo MISSING` subshell emits when node never reached the script
+    // (bad NODE_OPTIONS, OOM kill, exit 127). The shell answered; the answer is not about the deps.
     const conn = makeMockConnection(sftpCapture)
     feed([
       '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
       '/home/u',
-      'MISSING', // answered, no marker line: both deps are genuinely broken
-      'MISSING', // re-probe under the repair lock
+      'MISSING', // answered, no marker line: nothing here names a dep
+      '', // launch namespace marker
+      'DEAD',
+      '', // publish the per-launch credential
+      'READY'
+    ])
+
+    const outcome = await deployAndLaunchRelay(conn).then(
+      (result) => result,
+      (err: Error) => err
+    )
+
+    const commands = execCommands()
+    expect(warnings().some((message) => message.includes('Repairing missing native deps'))).toBe(
+      false
+    )
+    expect(commands.some((command) => command.includes(NODE_PTY_RESET))).toBe(false)
+    expect(commands.some((command) => command.includes(WATCHER_RESET))).toBe(false)
+    expect(commands.some((command) => command.includes('npm install'))).toBe(false)
+    // One probe only: an unverifiable answer must not fall through to the locked re-probe.
+    expect(commands.filter((command) => command.includes('ORCA-NATIVE-DEPS-OK'))).toHaveLength(1)
+    expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled()
+    expect(outcome, 'an unparseable answer must not abort the connection').not.toBeInstanceOf(Error)
+    expect(warnings().some((message) => message.includes('NATIVE-DEPS-PROBE-UNPARSEABLE'))).toBe(
+      true
+    )
+  })
+
+  it('carries the probe stderr into the unparseable-answer warning', async () => {
+    const conn = makeMockConnection(sftpCapture)
+    vi.mocked(execCommand)
+      .mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Linux x86_64')
+      .mockResolvedValueOnce('/home/u')
+      .mockImplementationOnce((_conn, _command, options) => {
+        options?.onStderr?.('node: --inspect-brk is not allowed in NODE_OPTIONS')
+        return Promise.resolve('MISSING')
+      })
+    feed(['', 'DEAD', '', 'READY'])
+
+    await expect(deployAndLaunchRelay(conn)).resolves.toBeDefined()
+
+    // Why: `2>/dev/null` used to drop the one line that says which host config broke node.
+    expect(
+      warnings().find((message) => message.includes('NATIVE-DEPS-PROBE-UNPARSEABLE'))
+    ).toContain('not allowed in NODE_OPTIONS')
+  })
+
+  it('still resets both deps when the probe names both', async () => {
+    const conn = makeMockConnection(sftpCapture)
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
+      '/home/u',
+      BOTH_NATIVE_DEPS_MISSING_PROBE, // answered: both deps are genuinely broken
+      BOTH_NATIVE_DEPS_MISSING_PROBE, // re-probe under the repair lock
       '', // SFTP-namespace install-owner marker (repair)
       '', // npm install native deps
       '', // chmod prebuilds
@@ -181,6 +238,62 @@ describe('native-deps repair probe verdicts', () => {
     expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1)
   })
 
+  it('leaves a Windows relay intact when its probe answers without naming a dep', async () => {
+    // The PowerShell branch has the same hole: `try { & node -e ... } catch { 'MISSING' }` prints
+    // nothing when node exits non-zero without reaching the script.
+    vi.mocked(parseUnameToRelayPlatform).mockReturnValueOnce('win32-x64')
+    vi.mocked(resolveRemoteNodePath).mockResolvedValueOnce('C:/Program Files/nodejs/node.exe')
+    const conn = makeMockConnection(sftpCapture)
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Windows AMD64',
+      'C:\\Users\\u',
+      '', // health probe: PowerShell swallowed the native failure, so nothing names a dep
+      '', // no persisted active pipe marker
+      'WAITING', // initial pipe probe
+      '', // publish the per-launch credential
+      '', // WMI relay launch
+      'READY', // readiness poll
+      '' // persist active pipe marker
+    ])
+
+    await expect(deployAndLaunchRelay(conn)).resolves.toBeDefined()
+
+    const scripts = execCommands().map((command) => decodePowerShellCommand(command) ?? command)
+    expect(scripts.some((script) => script.includes('node_modules/node-pty'))).toBe(false)
+    expect(scripts.some((script) => script.includes('node_modules/@parcel/watcher'))).toBe(false)
+    expect(scripts.some((script) => script.includes('npm install'))).toBe(false)
+    expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled()
+    expect(warnings().some((message) => message.includes('NATIVE-DEPS-PROBE-UNPARSEABLE'))).toBe(
+      true
+    )
+  })
+
+  it('resets only the dep the probe names', async () => {
+    const conn = makeMockConnection(sftpCapture)
+    const watcherMissing = 'ORCA-NATIVE-DEPS-MISSING:@parcel/watcher\nMISSING'
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
+      '/home/u',
+      watcherMissing,
+      watcherMissing, // re-probe under the repair lock
+      '', // SFTP-namespace install-owner marker (repair)
+      '', // npm install native deps
+      '', // chmod prebuilds
+      'ORCA-NPTY-PROBE-OK\n',
+      '', // rm probe stderr
+      'DEAD',
+      '', // publish the per-launch credential
+      'READY'
+    ])
+
+    await expect(deployAndLaunchRelay(conn)).resolves.toBeDefined()
+
+    const install = execCommands().find((command) => command.includes('npm install')) ?? ''
+    expect(install).toContain(WATCHER_RESET)
+    expect(install).not.toContain(NODE_PTY_RESET)
+    expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1)
+  })
+
   it('skips repair entirely when the probe answers OK', async () => {
     const conn = makeMockConnection(sftpCapture)
     feed([

src/main/ssh/ssh-relay-sftp-namespace-install.test.ts

@@ -91,6 +91,7 @@ import {
   isRelayAlreadyInstalled
 } from './ssh-relay-versioned-install'
 import { tryAcquireRelayRepairLock } from './ssh-relay-repair-lock'
+import { BOTH_NATIVE_DEPS_MISSING_PROBE } from './ssh-relay-native-deps-install-fixture'
 import type { SshConnection } from './ssh-connection'
 import type { SftpNamespacePathMapping } from './sftp-namespace-resolution'
 
@@ -281,8 +282,8 @@ const POSIX_SYSTEM_SSH_FIRST_INSTALL = [
 const POSIX_REPAIR = [
   '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
   SHELL_HOME,
-  'MISSING', // probe before the repair lock
-  'MISSING', // re-probe under the lock
+  BOTH_NATIVE_DEPS_MISSING_PROBE, // probe before the repair lock: the marker names both deps
+  BOTH_NATIVE_DEPS_MISSING_PROBE, // re-probe under the lock
   '', // install-owner marker
   '', // npm install native deps
   '', // chmod prebuilds
@@ -692,8 +693,8 @@ describe('relay repair writes on a split SFTP namespace', () => {
     feed([
       '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
       SHELL_HOME,
-      'MISSING',
-      'MISSING',
+      BOTH_NATIVE_DEPS_MISSING_PROBE,
+      BOTH_NATIVE_DEPS_MISSING_PROBE,
       '', // npm install native deps
       '', // chmod prebuilds
       'ORCA-NPTY-PROBE-OK\n',
@@ -716,7 +717,12 @@ describe('relay repair writes on a split SFTP namespace', () => {
 
   it('degrades to shell paths when marker creation fails outright', async () => {
     const conn = makeConnection(capture)
-    feed(['__ORCA_REMOTE_PLATFORM__ Linux x86_64', SHELL_HOME, 'MISSING', 'MISSING'])
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
+      SHELL_HOME,
+      BOTH_NATIVE_DEPS_MISSING_PROBE,
+      BOTH_NATIVE_DEPS_MISSING_PROBE
+    ])
     vi.mocked(execCommand).mockRejectedValueOnce(new Error('read-only file system'))
     feed([
       '', // npm install native deps
@@ -742,7 +748,12 @@ describe('relay repair writes on a split SFTP namespace', () => {
 
   it('keeps the repair lock when marker creation has unconfirmed termination', async () => {
     const conn = makeConnection(capture)
-    feed(['__ORCA_REMOTE_PLATFORM__ Linux x86_64', SHELL_HOME, 'MISSING', 'MISSING'])
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
+      SHELL_HOME,
+      BOTH_NATIVE_DEPS_MISSING_PROBE,
+      BOTH_NATIVE_DEPS_MISSING_PROBE
+    ])
     vi.mocked(execCommand).mockRejectedValueOnce(
       Object.assign(new Error('marker teardown unconfirmed'), { sshChannelCloseConfirmed: false })
     )