fix(ssh): name missing build tools when the relay has no node-pty (#22670)

On an SSH host without a C/C++ compiler, the relay installs without node-pty, and opening a terminal used to say "could not establish why … reconnect to retry", which never helped. The relay now treats a missing node-pty folder ("not found" only) as not installed, runs its existing build-tools check, and names the missing tools with the install command for the host's package manager. It diagnoses the node-pty install the relay's import actually resolved, and keeps any other error as "can't tell".

Part of #20386. Removing the need for a compiler on the host is #1693.

src/relay/node-pty-binding-survey.test.ts

@@ -1,4 +1,12 @@
-import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import {
+  chmodSync,
+  mkdirSync,
+  mkdtempSync,
+  realpathSync,
+  rmSync,
+  symlinkSync,
+  writeFileSync
+} from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import process from 'node:process'
@@ -7,6 +15,7 @@ import { isFlattenedNodePtyLoaderMessage } from '../main/orcad/node-pty-loader-d
 import {
   collectNodePtyUnavailableDiagnosis,
   readNodeGypBuildRecord,
+  resolveNodePtyInstallDir,
   surveyNodePtyBinding
 } from './node-pty-binding-survey'
 import { formatNodePtyUnavailableMessage } from './node-pty-unavailable-diagnosis'
@@ -73,6 +82,26 @@ describe('readNodeGypBuildRecord', () => {
   })
 })
 
+describe('resolveNodePtyInstallDir', () => {
+  it('finds the install an ancestor node_modules supplies, as the bare import does', () => {
+    // realpath: the resolver answers the real path, and macOS tmpdir is a link.
+    const root = realpathSync(mkdtempSync(join(tmpdir(), 'orca-node-pty-')))
+    roots.push(root)
+    const installDir = join(root, 'node_modules', 'node-pty')
+    mkdirSync(installDir, { recursive: true })
+    writeFileSync(join(installDir, 'package.json'), '{}\n')
+
+    expect(resolveNodePtyInstallDir(join(root, 'relay', 'abc123'))).toBe(installDir)
+  })
+
+  it('answers nothing when no node_modules up the tree holds node-pty', () => {
+    const root = mkdtempSync(join(tmpdir(), 'orca-node-pty-'))
+    roots.push(root)
+
+    expect(resolveNodePtyInstallDir(root)).toBeNull()
+  })
+})
+
 describe('collectNodePtyUnavailableDiagnosis', () => {
   it("recovers the dynamic loader's own words that node-pty threw away", async () => {
     // The whole defect in one assertion: what reaches the relay is FLATTENED, which names
@@ -107,17 +136,61 @@ describe('collectNodePtyUnavailableDiagnosis', () => {
     }
   }, 20_000)
 
-  it('reports an unlocatable install as unverifiable, not as a diagnosis', async () => {
+  it('diagnoses a node-pty directory that does not exist instead of calling it unverifiable (#20386)', async () => {
+    // What a Linux host without a compiler gets: the deploy reinstalls with node-pty removed.
+    const root = mkdtempSync(join(tmpdir(), 'orca-node-pty-'))
+    roots.push(root)
+    const missingDir = join(root, 'node-pty')
     const diagnosis = await collectNodePtyUnavailableDiagnosis({
-      nodePtyDir: null,
-      error: new Error(FLATTENED)
+      nodePtyDir: missingDir,
+      error: new Error(`no node-pty at ${join(missingDir, 'lib', 'index.js')}`)
     })
-    expect(diagnosis.status).toBe('unverifiable')
-    const text = formatNodePtyUnavailableMessage(diagnosis)
-    expect(text).toContain('could not establish why')
-    // It still has to be reportable: the raw error is the only thing an issue can quote.
-    expect(text).toContain(FLATTENED)
-  })
+    expect(diagnosis.status).toBe('blocked')
+    expect(['toolchain_missing', 'dependency_missing']).toContain(diagnosis.reason)
+    expect(diagnosis.survey).toMatchObject({ installed: false, bindingPath: null })
+    expect(diagnosis.toolchain === null).toBe(process.platform !== 'linux')
+    expect(formatNodePtyUnavailableMessage(diagnosis)).toContain(
+      `node-pty is not installed at ${missingDir}`
+    )
+  }, 20_000)
+
+  it.skipIf(process.platform === 'win32')(
+    'treats a node-pty link whose target is gone as not installed',
+    async () => {
+      const root = mkdtempSync(join(tmpdir(), 'orca-node-pty-'))
+      roots.push(root)
+      const linked = join(root, 'node-pty')
+      symlinkSync(join(root, 'gone'), linked)
+      const diagnosis = await collectNodePtyUnavailableDiagnosis({ nodePtyDir: linked })
+      expect(diagnosis.survey).toMatchObject({ installed: false, bindingPath: null })
+    },
+    20_000
+  )
+
+  it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)(
+    'keeps a directory it was refused as unverifiable rather than absent',
+    async () => {
+      const root = mkdtempSync(join(tmpdir(), 'orca-node-pty-'))
+      roots.push(root)
+      const locked = join(root, 'locked')
+      mkdirSync(join(locked, 'node-pty'), { recursive: true })
+      chmodSync(locked, 0o000)
+      try {
+        const diagnosis = await collectNodePtyUnavailableDiagnosis({
+          nodePtyDir: join(locked, 'node-pty'),
+          error: new Error(FLATTENED)
+        })
+        expect(diagnosis.status).toBe('unverifiable')
+        expect(diagnosis.detail).toContain('EACCES')
+        const text = formatNodePtyUnavailableMessage(diagnosis)
+        expect(text).toContain('could not establish why')
+        // It still has to be reportable: the raw error is the only thing an issue can quote.
+        expect(text).toContain(FLATTENED)
+      } finally {
+        chmodSync(locked, 0o755)
+      }
+    }
+  )
 
   it('probes the host toolchain only when nothing was compiled', async () => {
     const diagnosis = await collectNodePtyUnavailableDiagnosis({

src/relay/node-pty-binding-survey.ts

@@ -19,8 +19,9 @@
  * Every step is best-effort and failure-tolerant: whatever cannot be established is
  * reported as unestablished rather than guessed (docs/reference/ssh-execution-boundary.md).
  */
-import { existsSync, readFileSync } from 'node:fs'
-import { join } from 'node:path'
+import { existsSync, readFileSync, statSync } from 'node:fs'
+import { createRequire } from 'node:module'
+import { dirname, join } from 'node:path'
 import { release } from 'node:os'
 import process from 'node:process'
 import { runProcess } from '../shared/child-process/run-process'
@@ -85,6 +86,7 @@ export function surveyNodePtyBinding(
   const built = readNodeGypBuildRecord(nodePtyDir)
   return {
     moduleDir: nodePtyDir,
+    installed: true,
     bindingPath,
     searched,
     builtNodeAbi: built.nodeAbi,
@@ -181,6 +183,34 @@ async function probeRelayBuildToolchain(
   }
 }
 
+/** The install a bare `node-pty` import from `fromDir` loads — Node walks up, so it can be an ancestor's. */
+export function resolveNodePtyInstallDir(fromDir: string): string | null {
+  try {
+    return dirname(createRequire(join(fromDir, 'relay.js')).resolve('node-pty/package.json'))
+  } catch {
+    return null
+  }
+}
+
+// Only ENOENT is absence — the relay observing its own host. `stat` judges the directory the
+// loader reads, so a dangling link is absent; `existsSync` would also answer false for EACCES.
+function readNodePtyDirPresence(
+  nodePtyDir: string
+): 'present' | 'absent' | { unverifiable: string } {
+  try {
+    statSync(nodePtyDir)
+    return 'present'
+  } catch (error) {
+    const code = error instanceof Error && 'code' in error ? String(error.code) : null
+    if (code === 'ENOENT') {
+      return 'absent'
+    }
+    return {
+      unverifiable: `the relay could not read its node-pty install directory (${code ?? readErrorMessage(error) ?? 'unknown error'})`
+    }
+  }
+}
+
 function readErrorMessage(error: unknown): string | null {
   if (error instanceof Error) {
     return error.message
@@ -193,21 +223,32 @@ function readErrorMessage(error: unknown): string | null {
  * one's answer. Called only on the failure path, so a spawn that works pays nothing.
  */
 export async function collectNodePtyUnavailableDiagnosis(options: {
-  nodePtyDir: string | null
+  nodePtyDir: string
   error?: unknown
 }): Promise<NodePtyUnavailableDiagnosis> {
   const abi = detectNativeHostAbi()
   const host: NodePtyUnavailableHost = { ...abi, nodeVersion: process.version }
   const requireError = readErrorMessage(options.error)
-  if (!options.nodePtyDir) {
+  const presence = readNodePtyDirPresence(options.nodePtyDir)
+  if (typeof presence === 'object') {
     return diagnoseNodePtyUnavailable({
       host,
       survey: null,
       requireError,
-      unverifiableBecause: 'the relay could not locate its node-pty install directory'
+      unverifiableBecause: presence.unverifiable
     })
   }
-  const survey = surveyNodePtyBinding(options.nodePtyDir, host)
+  const survey: NodePtyBindingSurvey | null =
+    presence === 'absent'
+      ? {
+          moduleDir: options.nodePtyDir,
+          installed: false,
+          bindingPath: null,
+          searched: [],
+          builtNodeAbi: null,
+          builtArch: null
+        }
+      : surveyNodePtyBinding(options.nodePtyDir, host)
   const probed = survey?.bindingPath ? await probeNodePtyLoader(options.nodePtyDir) : {}
   const toolchain =
     survey && !survey.bindingPath ? await probeRelayBuildToolchain(host.platform) : null

src/relay/node-pty-unavailable-diagnosis.test.ts

@@ -27,6 +27,7 @@ const SEARCHED = ['build/Release', 'build/Debug', 'prebuilds/linux-x64']
 
 const INSTALLED: NodePtyBindingSurvey = {
   moduleDir: MODULE_DIR,
+  installed: true,
   bindingPath: `${MODULE_DIR}/build/Release/pty.node`,
   searched: SEARCHED,
   builtNodeAbi: null,
@@ -140,6 +141,53 @@ describe('diagnoseNodePtyUnavailable', () => {
     })
     expect(present.reason).toBe('dependency_missing')
     expect(formatNodePtyUnavailableMessage(present)).not.toContain('apt-get')
+    expect(formatNodePtyUnavailableMessage(present)).toContain(
+      'build tools needed to compile it are present'
+    )
+  })
+
+  it('never claims the build tools are present when no toolchain probe answered', () => {
+    // docs/reference/ssh-execution-boundary.md: a probe that timed out established nothing.
+    const linuxUnchecked = message({ survey: NOTHING_INSTALLED, toolchain: null })
+    expect(linuxUnchecked).not.toContain('are present')
+    expect(linuxUnchecked).toContain('could not be checked')
+    expect(linuxUnchecked).toContain('Reconnect to reinstall')
+
+    // Off Linux the probe never runs: node-pty ships prebuilds, so tools are beside the point.
+    const macos = message({
+      host: { ...UBUNTU_2004, platform: 'darwin', libc: 'none', glibcVersion: null },
+      survey: NOTHING_INSTALLED,
+      toolchain: null
+    })
+    expect(macos).not.toContain('build tools')
+    expect(macos).toContain('Reconnect to reinstall')
+  })
+
+  it('names an absent node-pty directory as not installed and still offers the build tools (#20386)', () => {
+    // The no-toolchain deploy removes node-pty outright, so there is no directory to search.
+    const text = message({
+      survey: { ...NOTHING_INSTALLED, installed: false, searched: [] },
+      toolchain: toolchain(['python3'])
+    })
+    expect(text).toContain(`node-pty is not installed at ${MODULE_DIR}`)
+    expect(text).toContain('sudo apt-get install -y build-essential python3')
+    expect(text).not.toContain('reconnect to retry')
+  })
+
+  it('does not blame the build tools for an absent directory on a host that has them', () => {
+    // The node-pty-less reinstall also leaves no directory when it fails for a non-toolchain
+    // reason (ENOSPC, registry unreachable). Naming tools the host already has is the
+    // confidently-wrong diagnosis #20386's fix must not introduce.
+    const verdict = diagnose({
+      survey: { ...NOTHING_INSTALLED, installed: false, searched: [] },
+      toolchain: toolchain(['make', 'g++', 'python3'])
+    })
+    expect(verdict.reason).toBe('dependency_missing')
+    const text = formatNodePtyUnavailableMessage(verdict)
+    expect(text).toContain(`node-pty is not installed at ${MODULE_DIR}`)
+    expect(text).toContain('build tools needed to compile it are present')
+    expect(text).not.toContain('apt-get')
+    expect(text).not.toContain('not installed.')
   })
 
   it('reports a binding that killed the probe as a crash rather than a miss', () => {

src/relay/node-pty-unavailable-diagnosis.ts

@@ -33,6 +33,8 @@ import type { TerminalUnavailableCause } from '../shared/terminal-unavailable-ca
 export type NodePtyBindingSurvey = {
   /** The node-pty install the relay would load from. */
   moduleDir: string
+  /** False when `moduleDir` itself is absent — the no-toolchain deploy skips node-pty entirely. */
+  installed: boolean
   /** The compiled binding the loader would open, or null when no directory holds one. */
   bindingPath: string | null
   /** Directories checked, so "nothing is installed" is a statement with evidence. */
@@ -312,8 +314,7 @@ function remedyFor(diagnosis: NodePtyUnavailableDiagnosis): string {
     case 'dependency_missing':
       return (
         `node-pty has no compiled binary on this host (${searchedPhrase(survey)}). ` +
-        `The C/C++ build tools needed to compile it are present, so reconnect to reinstall ` +
-        `the relay's native modules.`
+        `${toolchainPresenceSentence(host, toolchain)}Reconnect to reinstall the relay's native modules.`
       )
     case 'abi_mismatch':
       return (
@@ -355,7 +356,25 @@ function remedyFor(diagnosis: NodePtyUnavailableDiagnosis): string {
   }
 }
 
+// Claims "present" only off a probe that answered; the probe runs on Linux only, since
+// node-pty ships prebuilds elsewhere and a reinstall there needs no compiler.
+function toolchainPresenceSentence(
+  host: NodePtyUnavailableHost,
+  toolchain: BuildToolchainStatus | null
+): string {
+  if (toolchain) {
+    return 'The C/C++ build tools needed to compile it are present. '
+  }
+  if (host.platform === 'linux') {
+    return 'Whether the C/C++ build tools needed to compile it are installed could not be checked. '
+  }
+  return ''
+}
+
 function searchedPhrase(survey: NodePtyBindingSurvey | null): string {
+  if (survey && !survey.installed) {
+    return `node-pty is not installed at ${survey.moduleDir}`
+  }
   return survey && survey.searched.length > 0
     ? `checked ${survey.searched.join(', ')} under ${survey.moduleDir}`
     : 'nothing was found where node-pty looks'

src/relay/pty-handler-spawn-admission.test.ts

@@ -1,11 +1,13 @@
 import './mock-descendant-sweep'
 import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'
-import { mkdtempSync, rmSync } from 'node:fs'
+import { existsSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'
 import { tmpdir } from 'node:os'
-import { join } from 'node:path'
+import { dirname, join } from 'node:path'
 import * as ptyChildProcessInspection from './pty-child-process-inspection'
 import * as ptyShellUtils from './pty-shell-utils'
 import * as processTableSnapshotReader from '../shared/process-table-snapshot-reader'
+import * as runProcessModule from '../shared/child-process/run-process'
+import * as nodePtyBindingSurvey from './node-pty-binding-survey'
 
 const { mockPtySpawn, mockPtyInstance, mockCreateShellPromptReadinessProbe } = vi.hoisted(() => ({
   mockPtySpawn: vi.fn(),
@@ -325,30 +327,78 @@ describe('PtyHandler', () => {
     expect(handler.activePtyCount).toBe(0)
   })
 
-  it('keeps the load error it was handed instead of replacing it with guesses', async () => {
-    // #17830: the user got three remedies for four possible faults and could verify none.
-    // The relay must carry what it was actually told, and must not prescribe a toolchain
-    // install it never probed for.
-    const thrown =
-      'Failed to load native module: conpty.node, checked: build/Release, prebuilds/win32-x64'
-    mockPtySpawn.mockImplementationOnce(() => {
-      throw new Error(thrown)
-    })
+  const THROWN_LOAD_ERROR =
+    'Failed to load native module: pty.node, checked: build/Release, prebuilds/linux-x64'
 
-    const message = await dispatcher.callRequest('pty.spawn', {}).then(
-      () => '',
-      (error: Error) => error.message
+  function failSpawnWithToolchainProbe(
+    toolchainProbeStdout: string
+  ): Promise<(Error & { data?: unknown }) | null> {
+    const realRunProcess = runProcessModule.runProcess
+    vi.spyOn(runProcessModule, 'runProcess').mockImplementation((spec) =>
+      spec.program === '/bin/sh'
+        ? Promise.resolve({
+            stdout: toolchainProbeStdout,
+            stderr: '',
+            code: 0,
+            signal: null,
+            timedOut: false
+          })
+        : realRunProcess(spec)
     )
+    mockPtySpawn.mockImplementationOnce(() => {
+      throw new Error(THROWN_LOAD_ERROR)
+    })
+    return dispatcher.callRequest('pty.spawn', {}).then(
+      () => null,
+      (error: Error & { data?: unknown }) => error
+    )
+  }
 
-    expect(message).toContain(thrown)
-    expect(message).not.toContain('install make, a C++ compiler, and python3')
-    // Nothing here established a cause — the relay's node-pty directory is not on disk in
-    // this harness — so per docs/reference/ssh-execution-boundary.md it must say so rather
-    // than pick a diagnosis. Every message still names the host, for the bug report.
-    expect(message).toContain('could not establish why')
+  it('diagnoses a relay installed without node-pty instead of asking for a reconnect (#20386)', async () => {
+    // Relies on no node-pty beside the relay source — what the no-toolchain deploy leaves. Absence
+    // is observed on the owning host, so it is a diagnosis (docs/reference/ssh-execution-boundary.md).
+    expect(typeof process.resourcesPath, 'packaged node-pty lookup must be off').not.toBe('string')
+    expect(
+      existsSync(join(__dirname, 'node_modules', 'node-pty')),
+      'a node-pty beside src/relay would turn this into the installed-but-unbuilt case'
+    ).toBe(false)
+    // The checkout's own node_modules holds a node-pty an SSH host's relay dir never has.
+    vi.spyOn(nodePtyBindingSurvey, 'resolveNodePtyInstallDir').mockReturnValue(null)
+
+    const rejection = await failSpawnWithToolchainProbe('HAVE python3\nPKG apt-get\n')
+    const message = rejection?.message ?? ''
+
+    // #17830: the load error the relay was handed still travels with the rejection.
+    // No compiler means no rebuild, so the client must not auto-reconnect on this one.
+    expect(rejection?.data).toMatchObject({
+      reason: 'toolchain_missing',
+      repairable: false,
+      rawError: THROWN_LOAD_ERROR
+    })
+    expect(message).not.toContain('could not establish why')
+    expect(message).toContain('node-pty is not installed at')
+    expect(message).toContain('sudo apt-get install -y build-essential python3')
+    // Every message still names the host, for the bug report.
     expect(message).toMatch(/Host: linux\/\w+, .*Node v[\d.]+ \(ABI \d+\)/)
   })
 
+  it('diagnoses the node-pty an ancestor node_modules supplied, not the absent one beside the relay', async () => {
+    const ancestorInstall = join(mkdtempSync(join(tmpdir(), 'orca-node-pty-')), 'node-pty')
+    mkdirSync(ancestorInstall)
+    vi.spyOn(nodePtyBindingSurvey, 'resolveNodePtyInstallDir').mockReturnValue(ancestorInstall)
+
+    try {
+      const message =
+        (await failSpawnWithToolchainProbe('HAVE make\nHAVE g++\nHAVE python3\nPKG apt-get\n'))
+          ?.message ?? ''
+
+      expect(message).not.toContain('node-pty is not installed at')
+      expect(message).toContain(`under ${ancestorInstall}`)
+    } finally {
+      rmSync(dirname(ancestorInstall), { recursive: true, force: true })
+    }
+  })
+
   it('preserves unrelated node-pty spawn failures', async () => {
     mockPtySpawn.mockImplementationOnce(() => {
       throw new Error('File not found: missing-shell.exe')

src/relay/pty-handler.ts

@@ -128,7 +128,10 @@ import {
   injectRelayHistoryEnv
 } from './terminal-history'
 import { isFlattenedNodePtyLoaderMessage } from '../main/orcad/node-pty-loader-diagnosis'
-import { collectNodePtyUnavailableDiagnosis } from './node-pty-binding-survey'
+import {
+  collectNodePtyUnavailableDiagnosis,
+  resolveNodePtyInstallDir
+} from './node-pty-binding-survey'
 import { describeRelayRuntime } from './relay-runtime-identity'
 import { relayConptyDllSpawnOptions } from './relay-windows-conpty'
 import {
@@ -670,9 +673,10 @@ export class PtyHandler {
    * healthy relay never pays for them.
    */
   private async nodePtyUnavailableError(spawnError?: unknown): Promise<Error> {
-    const nodePtyDir = this.relayNodePtyDir()
+    // Why: diagnose the install the bare import loaded; the bundle's own dir is only the fallback.
+    const nodePtyDir = resolveNodePtyInstallDir(__dirname) ?? this.relayNodePtyDir()
     const diagnosis = await collectNodePtyUnavailableDiagnosis({
-      nodePtyDir: existsSync(nodePtyDir) ? nodePtyDir : null,
+      nodePtyDir,
       error: spawnError ?? this.lastPtyLoadError
     })
     return Object.assign(new Error(formatNodePtyUnavailableMessage(diagnosis)), {

src/renderer/src/components/terminal-pane/ipc-pty-connect.ts

@@ -181,6 +181,7 @@ function handleConnectError(
   context: IpcPtyConnectContext
 ): PtyConnectResult | undefined {
   const { connectionId } = context.transportOptions
+  // Unclamped: host diagnoses put the remedy on later lines, and the pane toast renders them all.
   const message =
     readIpcErrorDetail(error) ?? (error instanceof Error ? error.message : String(error))
   if (connectionId && options.sessionId && isSshSessionGoneError(message)) {

src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts

@@ -260,4 +260,23 @@ describe('createIpcPtyTransport', () => {
 
     expect(onError).toHaveBeenCalledWith(createTerminalSessionStateSaveFailureMessage())
   })
+
+  it('keeps every line of a multi-line spawn failure for the pane toast (#20386)', async () => {
+    const { createIpcPtyTransport } = await import('./pty-transport')
+    const diagnosis =
+      'Remote terminals are unavailable: make and a C++ compiler are not installed. Install them on the remote host, then reconnect:\n' +
+      '  sudo apt-get install -y build-essential python3\n' +
+      'Host: linux/x64, glibc 2.36, Node v22.12.0 (ABI 127), prebuild slot linux-x64-glibc.'
+    vi.mocked(window.api.pty.spawn).mockRejectedValueOnce(
+      new Error(`Error invoking remote method 'pty:spawn': Error: ${diagnosis}`)
+    )
+
+    const onError = vi.fn()
+    await createIpcPtyTransport({ connectionId: 'ssh-1' }).connect({
+      url: '',
+      callbacks: { onError }
+    })
+
+    expect(onError).toHaveBeenCalledWith(diagnosis)
+  })
 })