fix(ssh): stop treating an unanswered native-deps probe as broken deps (#17979)

probeRequiredNativeDeps mapped any thrown error to available:false, which both triggered the repair and fed resetDeps — so one dropped exec channel rm -rf'd node_modules/node-pty on a healthy relay and forced a node-gyp source build. Verdicts are now ok / blocked / unverifiable; only an answered probe may repair, and only an answered probe may name reset deps. (cherry picked from commit 02ffd40edc)
2026-09-30 16:02:56 +00:00 · 2026-09-01 19:18:08 -04:00
parent 085e467ffd
commit ea76d669f2
2 changed files with 251 additions and 7 deletions
@@ -733,13 +733,25 @@ function missingNativeDepsFromProbe(output: string): RelayNativeDepName[] {
  return RELAY_NATIVE_DEP_NAMES.filter((name) => reported.includes(name))
 }

+/**
+ * `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.
+ *
+ * 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.
+ * Same verdict discipline as `src/main/orcad/node-pty-precondition.ts` and
+ * docs/reference/ssh-execution-boundary.md — loss of contact is not evidence.
+ */
+type RelayNativeDepsProbeStatus = 'ok' | 'blocked' | 'unverifiable'
+
 async function probeRequiredNativeDeps(
  conn: SshConnection,
  remoteDir: string,
  hostPlatform: RemoteHostPlatform,
  nodePath: string,
  signal?: AbortSignal
-): Promise<{ available: boolean; missing: RelayNativeDepName[] }> {
+): Promise<{ status: RelayNativeDepsProbeStatus; missing: RelayNativeDepName[] }> {
  const escapedNode = shellEscape(nodePath)
  const probeJs = nativeDepsProbeJs('ORCA-NATIVE-DEPS-OK')
  try {
@@ -757,11 +769,14 @@ async function probeRequiredNativeDeps(
          `(${escapedNode} -e ${shellEscape(probeJs)} 2>/dev/null || echo MISSING)`
        )
    const probe = await execHostCommand(conn, hostPlatform, command, { signal })
-    const available = probe.includes('ORCA-NATIVE-DEPS-OK')
-    return { available, missing: available ? [] : missingNativeDepsFromProbe(probe) }
+    return probe.includes('ORCA-NATIVE-DEPS-OK')
+      ? { status: 'ok', missing: [] }
+      : { status: 'blocked', missing: missingNativeDepsFromProbe(probe) }
  } catch {
    signal?.throwIfAborted()
-    return { available: false, missing: [...RELAY_NATIVE_DEP_NAMES] }
+    // Why: an unanswered probe says nothing about the deps; reporting MISSING here reset and
+    // recompiled healthy relays, turning one dropped exec channel into a multi-minute reconnect.
+    return { status: 'unverifiable', missing: [] }
  }
 }

@@ -810,7 +825,8 @@ async function repairInstalledNativeDeps(
    lockResult === 'busy' || lockResult === 'error'
      ? await acquireRelayLaunchGcFence(conn, remoteDir, hostPlatform, signal)
      : undefined
-  if (initialProbe.available) {
+  // Why: only a probe that answered may trigger repair; an unverifiable one launches as-is and the next reconnect re-probes.
+  if (initialProbe.status !== 'blocked') {
    // Why: even a healthy reconnect stays fenced until launch liveness is observable, or cross-version GC can rename after this probe.
    if (lockResult !== 'acquired') {
      return { ownsInstallLock: false, gcClaimToken }
@@ -847,7 +863,9 @@ async function repairInstalledNativeDeps(
    // Why: older complete relay dirs predate @parcel/watcher; re-probe under the lock so only one reconnect mutates the dir.
    const probe = await probeRequiredNativeDeps(conn, remoteDir, hostPlatform, nodePath, signal)
    let repairNamespace: RelayInstallNamespace | undefined
-    if (!probe.available) {
+    if (probe.status !== 'ok') {
+      // Why: the locked re-probe can only narrow the repair; when it can't answer, the initial probe's answered evidence still stands.
+      const resetDeps = probe.status === 'unverifiable' ? initialProbe.missing : probe.missing
      // Why: only stamp ownership once the locked recheck proves this connection is the one about to write.
      repairNamespace = await createRelayLaunchNamespace(
        conn,
@@ -863,7 +881,7 @@ async function repairInstalledNativeDeps(
        hostPlatform,
        nodePath,
        signal,
-        probe.missing,
+        resetDeps,
        repairNamespace
      )
      await finalizeInstall(conn, remoteDir, hostPlatform, { signal, releaseLock: false })
@@ -0,0 +1,226 @@
+// Why: the repair path used to map ANY probe failure to "all deps missing", so one dropped exec
+// channel rm -rf'd node-pty on a healthy relay and forced a node-gyp rebuild. Verdicts are
+// ok / blocked / unverifiable — see docs/reference/ssh-execution-boundary.md.
+
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import type * as RelayInstallMarkerModule from './ssh-relay-install-marker'
+
+vi.mock('electron', () => ({
+  app: { getAppPath: () => '/mock/app' }
+}))
+
+vi.mock('fs', () => ({
+  existsSync: vi.fn().mockReturnValue(true),
+  readFileSync: vi.fn().mockReturnValue('0.1.0+testhash')
+}))
+
+vi.mock('./relay-protocol', () => ({
+  RELAY_VERSION: '0.1.0',
+  RELAY_REMOTE_DIR: '.orca-remote',
+  parseUnameToRelayPlatform: vi.fn().mockReturnValue('linux-x64'),
+  RELAY_SENTINEL: 'ORCA-RELAY v0.1.0 READY\n',
+  RELAY_SENTINEL_TIMEOUT_MS: 10_000
+}))
+
+vi.mock('./ssh-relay-deploy-helpers', () => ({
+  uploadDirectory: vi.fn().mockResolvedValue(undefined),
+  waitForSentinel: vi.fn().mockResolvedValue({
+    write: vi.fn(),
+    onData: vi.fn(),
+    onClose: vi.fn()
+  }),
+  isUnconfirmedSshCommandTermination: (error: unknown) =>
+    error instanceof Error &&
+    (error as Error & { sshChannelCloseConfirmed?: boolean }).sshChannelCloseConfirmed === false,
+  execCommand: vi.fn()
+}))
+
+vi.mock('./ssh-remote-node-resolution', () => ({
+  resolveRemoteNodePath: vi.fn().mockResolvedValue('/usr/bin/node')
+}))
+
+vi.mock('./ssh-relay-install-marker', async (importOriginal) => ({
+  ...(await importOriginal<typeof RelayInstallMarkerModule>()),
+  createRelayInstallMarkerFileName: () => '.sftp-namespace-00000000000000000000000000000000'
+}))
+
+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(true),
+  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),
+  RELAY_INSTALL_LOCK_NAME: '.install-lock'
+}))
+
+vi.mock('./ssh-relay-repair-lock', () => ({
+  tryAcquireRelayRepairLock: vi.fn().mockResolvedValue('acquired')
+}))
+
+vi.mock('./ssh-relay-gc-claim', () => ({
+  releaseRelayGcClaimWithRetry: vi.fn().mockResolvedValue('released'),
+  tryAcquireRelayGcClaim: vi.fn().mockResolvedValue('launch-token'),
+  waitForRelayGcClaimRelease: vi.fn().mockResolvedValue(undefined)
+}))
+
+vi.mock('./ssh-connection-utils', () => ({
+  shellEscape: (s: string) => `'${s}'`
+}))
+
+import { deployAndLaunchRelay } from './ssh-relay-deploy'
+import { execCommand, uploadDirectory } from './ssh-relay-deploy-helpers'
+import { parseUnameToRelayPlatform } from './relay-protocol'
+import { finalizeInstall, isRelayAlreadyInstalled } from './ssh-relay-versioned-install'
+import {
+  makeMockConnection,
+  type ExecResponse,
+  type SftpWriteCapture
+} from './ssh-relay-native-deps-install-fixture'
+
+const NODE_PTY_RESET = "rm -rf 'node_modules/node-pty'"
+const WATCHER_RESET = "rm -rf 'node_modules/@parcel/watcher'"
+
+describe('native-deps repair probe verdicts', () => {
+  const sftpCapture: SftpWriteCapture = { paths: [], contents: {}, execCallCountAtWrite: {} }
+  let warnSpy: ReturnType<typeof vi.spyOn>
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    vi.mocked(execCommand).mockReset().mockResolvedValue('')
+    vi.mocked(uploadDirectory).mockResolvedValue(undefined)
+    sftpCapture.paths.length = 0
+    for (const key of Object.keys(sftpCapture.contents)) {
+      delete sftpCapture.contents[key]
+    }
+    for (const key of Object.keys(sftpCapture.execCallCountAtWrite)) {
+      delete sftpCapture.execCallCountAtWrite[key]
+    }
+    vi.mocked(parseUnameToRelayPlatform).mockReturnValue('linux-x64')
+    vi.mocked(isRelayAlreadyInstalled).mockResolvedValue(true)
+    warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+  })
+
+  function feed(execResponses: ExecResponse[]): void {
+    const mockExec = vi.mocked(execCommand)
+    for (const response of execResponses) {
+      if (typeof response === 'string') {
+        mockExec.mockResolvedValueOnce(response)
+      } else {
+        mockExec.mockRejectedValueOnce(new Error(response.reject))
+      }
+    }
+  }
+
+  function execCommands(): string[] {
+    return vi.mocked(execCommand).mock.calls.map(([, command]) => command)
+  }
+
+  function warnings(): string[] {
+    return warnSpy.mock.calls.map((args) => String(args[0] ?? ''))
+  }
+
+  it('launches an intact relay when the health probe never answers', async () => {
+    const conn = makeMockConnection(sftpCapture)
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
+      '/home/u',
+      { reject: 'SSH channel closed unexpectedly' }, // health probe: unverifiable, not MISSING
+      '', // launch namespace marker
+      'DEAD',
+      '', // publish the per-launch credential
+      'READY'
+    ])
+
+    // Assert the repair-avoidance facts before the launch outcome so a regression names the defect
+    // rather than the fixture drift that follows from an unexpected repair.
+    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)
+    // Exactly one probe: 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, 'lost contact must not abort the connection').not.toBeInstanceOf(Error)
+  })
+
+  it('still resets and repairs when the probe answers without the OK marker', async () => {
+    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
+      '', // 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(NODE_PTY_RESET)
+    expect(install).toContain(WATCHER_RESET)
+    expect(vi.mocked(finalizeInstall)).toHaveBeenCalledTimes(1)
+  })
+
+  it('skips repair entirely when the probe answers OK', async () => {
+    const conn = makeMockConnection(sftpCapture)
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
+      '/home/u',
+      'ORCA-NATIVE-DEPS-OK',
+      '', // launch namespace marker
+      'DEAD',
+      '', // publish the per-launch credential
+      'READY'
+    ])
+
+    await expect(deployAndLaunchRelay(conn)).resolves.toBeDefined()
+
+    expect(execCommands().some((command) => command.includes('npm install'))).toBe(false)
+    expect(vi.mocked(finalizeInstall)).not.toHaveBeenCalled()
+  })
+
+  it('keeps the answered reset scope when the locked re-probe cannot answer', async () => {
+    const conn = makeMockConnection(sftpCapture)
+    feed([
+      '__ORCA_REMOTE_PLATFORM__ Linux x86_64',
+      '/home/u',
+      'ORCA-NATIVE-DEPS-MISSING:@parcel/watcher\nMISSING', // answered: only the watcher is broken
+      { reject: 'SSH channel closed unexpectedly' }, // re-probe under the lock: unverifiable
+      '', // 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)
+    // The unanswered re-probe must not widen the reset to a dep no probe ever reported broken.
+    expect(install).not.toContain(NODE_PTY_RESET)
+  })
+})