fix(setup): layered binary resolution + manual override — kill the A1 first-launch lockout (#495)

src/main/ipc/setup.ts

@@ -1,9 +1,15 @@
 import { ipcMain } from 'electron'
 
-import type { SetupInstallTarget, SetupToolId } from '@shared/types/setup.js'
+import type {
+  SetupInstallTarget,
+  SetupSetToolPathResult,
+  SetupToolId,
+} from '@shared/types/setup.js'
+import { classifyExecutable } from '@main/setup/binaryResolver.js'
 import { installWithHomebrew } from '@main/setup/homebrewInstaller.js'
 import { checkPrerequisites } from '@main/setup/prerequisites.js'
-import { markOptionalSkipped } from '@main/setup/setupState.js'
+import { markOptionalSkipped, setManualToolPath } from '@main/setup/setupState.js'
+import { refreshToolchainFromState } from '@main/setup/toolchain.js'
 
 export function registerSetupIpc(): void {
   ipcMain.handle('setup:check', async () => {
@@ -31,4 +37,50 @@ export function registerSetupIpc(): void {
     await markOptionalSkipped(tool, true)
     return await checkPrerequisites()
   })
+
+  // Escape hatch for #495 A1: automatic resolution is a probe, and a
+  // probe false-negative must never be the *sole* gate on the whole
+  // product. The user pastes an absolute path; we accept it iff it is an
+  // executable *regular file*, persist it via setManualToolPath() — which
+  // records durable user intent in manualToolPaths AND updates the same
+  // toolPaths cache the automatic probe writes, so PATH augmentation
+  // treats it identically to a probed path — then re-run the check so the
+  // gate can unlock in one round-trip.
+  //
+  // A valid override outranks every automatic layer from here on:
+  // checkPrerequisites and revalidateToolchain both consult
+  // manualToolPaths before probing (precedence comment in
+  // prerequisites.ts), so neither the recheck below nor the next app
+  // launch can silently replace or delete what the user chose.
+  ipcMain.handle(
+    'setup:set-tool-path',
+    async (_evt, tool: SetupToolId, rawPath: string): Promise<SetupSetToolPathResult> => {
+      const candidate = rawPath.trim()
+      if (!candidate.startsWith('/')) {
+        return { ok: false, reason: 'Enter an absolute path (starting with /).' }
+      }
+      // classifyExecutable, not a bare X_OK check: POSIX access(dir, X_OK)
+      // succeeds for any searchable directory, so `/usr/local/bin` used to
+      // validate here and the provider later tried to spawn a directory.
+      // The verdict lets us tell the user *how* the path is wrong — the
+      // directory case gets its own message because it is the natural typo
+      // (pasting the containing dir instead of the binary).
+      switch (await classifyExecutable(candidate)) {
+        case 'ok':
+          break
+        case 'directory':
+          return {
+            ok: false,
+            reason: `That path is a directory — enter the path to the binary itself (e.g. ${candidate.replace(/\/+$/, '')}/${tool}).`,
+          }
+        case 'missing':
+          return { ok: false, reason: 'No file exists at that path.' }
+        default:
+          return { ok: false, reason: 'Not an executable file at that path.' }
+      }
+      await setManualToolPath(tool, candidate)
+      await refreshToolchainFromState()
+      return { ok: true, check: await checkPrerequisites() }
+    },
+  )
 }

src/main/setup/binaryResolver.ts

@@ -1,5 +1,7 @@
-import { access } from 'fs/promises'
+import { access, stat } from 'fs/promises'
 import { constants as fsConstants } from 'fs'
+import { homedir } from 'os'
+import { join } from 'path'
 
 import type { SetupToolId } from '@shared/types/setup.js'
 import { runLoginShell } from '@main/setup/shell.js'
@@ -18,26 +20,103 @@ const TOOL_COMMAND: Record<SetupToolId, string> = {
   ),
 } as Record<SetupToolId, string>
 
-export async function isExecutable(path: string): Promise<boolean> {
+// WHY a classifier and not a plain boolean: on POSIX, X_OK against a
+// DIRECTORY means "searchable", so `access('/usr/local/bin', X_OK)`
+// succeeds. The old boolean-only check therefore accepted directories
+// as "executables" — a user could paste `/usr/local/bin` into the
+// manual override, the SetupGate would happily unlock, and the failure
+// (spawn EACCES/EISDIR) surfaced minutes later when a provider tried
+// to exec the directory, with no hint that setup was the culprit.
+// The classifier lets callers that talk to humans (setup:set-tool-path)
+// say *which* way the path is wrong, while machine callers collapse it
+// through isExecutable() below.
+//
+// stat() (not lstat) on purpose: it follows symlinks, and a symlink to
+// a real binary (brew's `/opt/homebrew/bin/*`, volta/asdf shims) must
+// classify by its target — rejecting symlinks would break the most
+// common install layouts on macOS.
+export type ExecutableVerdict =
+  | 'ok'
+  | 'missing' // nothing stat-able at the path (or unreadable parent)
+  | 'directory' // exists but is a directory — the /usr/local/bin trap
+  | 'not-a-file' // socket/FIFO/device — spawnable by nothing we do
+  | 'not-executable' // regular file without +x for this user
+
+export async function classifyExecutable(path: string): Promise<ExecutableVerdict> {
+  let info
+  try {
+    info = await stat(path)
+  } catch {
+    return 'missing'
+  }
+  if (info.isDirectory()) return 'directory'
+  if (!info.isFile()) return 'not-a-file'
   try {
     await access(path, fsConstants.X_OK)
-    return true
   } catch {
-    return false
+    return 'not-executable'
+  }
+  return 'ok'
+}
+
+export async function isExecutable(path: string): Promise<boolean> {
+  return (await classifyExecutable(path)) === 'ok'
+}
+
+// Layer 2 of resolution (#495 A1): a direct executable scan that cannot be
+// poisoned by the user's shell. Covers (a) non-POSIX $SHELL where the
+// login-shell probe ran through a substitute shell that never sources the
+// user's real rc files, and (b) tools whose dir is added by a non-login rc
+// block the `-l` probe never sources.
+//
+// WHY these extra dirs: they are the standard install targets of the tools
+// we resolve (npm-global/volta/asdf/bun shims for the provider CLIs, brew
+// for git+mitmdump) and each costs one stat. process.env.PATH comes first —
+// when the app is launched from a terminal it is the user's real PATH; from
+// Finder it is launchd's minimal one, which is exactly when the well-known
+// dirs earn their keep.
+const WELL_KNOWN_BIN_DIRS = [
+  join(homedir(), '.local', 'bin'),
+  join(homedir(), '.volta', 'bin'),
+  join(homedir(), '.asdf', 'shims'),
+  join(homedir(), '.bun', 'bin'),
+  join(homedir(), '.cargo', 'bin'),
+  '/opt/homebrew/bin',
+  '/usr/local/bin',
+  '/usr/bin',
+  '/bin',
+]
+
+async function scanForTool(command: string): Promise<string | null> {
+  const pathDirs = (process.env.PATH ?? '').split(':').filter(Boolean)
+  const seen = new Set<string>()
+  for (const dir of [...pathDirs, ...WELL_KNOWN_BIN_DIRS]) {
+    if (seen.has(dir)) continue
+    seen.add(dir)
+    const candidate = join(dir, command)
+    if (await isExecutable(candidate)) return candidate
   }
+  return null
 }
 
 export async function resolveToolPath(tool: SetupToolId): Promise<string | null> {
   const command = TOOL_COMMAND[tool]
+  // Layer 1: the login-shell probe. It stays first because its answer
+  // reflects the user's curated environment — a version manager's shim
+  // dir early in the login PATH must beat a stale system copy that the
+  // raw scan below would otherwise surface.
   try {
     const result = await runLoginShell(`command -v ${command}`, {
       timeoutMs: 10_000,
       maxBuffer: 64 * 1024,
     })
     const path = result.stdout.trim().split('\n')[0]?.trim()
-    return path && await isExecutable(path) ? path : null
+    if (path && await isExecutable(path)) return path
   } catch {
-    return null
+    // Probe failure is NOT evidence of absence (#495 A1) — a broken rc
+    // file, a slow shell init past the timeout, or an exotic $SHELL all
+    // land here. Fall through to the direct scan.
   }
+  return scanForTool(command)
 }
 

src/main/setup/prerequisites.ts

@@ -3,7 +3,7 @@ import type {
   SetupToolId,
   SetupToolStatus,
 } from '@shared/types/setup.js'
-import { resolveToolPath } from '@main/setup/binaryResolver.js'
+import { isExecutable, resolveToolPath } from '@main/setup/binaryResolver.js'
 import {
   isBundledArchiveAvailable,
   type BundledToolId,
@@ -89,7 +89,42 @@ export async function checkPrerequisites(): Promise<SetupCheckResult> {
   const brewPath = await resolveToolPath('brew')
   const entries = await Promise.all(
     CHECK_ORDER.map(async tool => {
-      const systemPath = tool === 'brew' ? brewPath : await resolveToolPath(tool)
+      // Resolution precedence (#495 A1 + codex review of #504), highest
+      // first — each layer only runs when every layer above it failed:
+      //
+      //   0. Valid manual override (manualToolPaths + file still an
+      //      executable regular file). The user's explicit word beats
+      //      every probe: an earlier revision consulted the probes FIRST,
+      //      so a user who deliberately pointed us at
+      //      ~/bin/claude-wrapper had it silently replaced by PATH's
+      //      /usr/local/bin/claude on the very next check, and the
+      //      write-back below then erased the override from toolPaths.
+      //      Override-first also makes the write-back safe by
+      //      construction: when the override wins, the value written back
+      //      IS the override. An override whose file vanished or lost +x
+      //      falls through to auto layers (the gate must not stay
+      //      "found" on a dead path) but stays recorded in
+      //      manualToolPaths — intent is durable, and it resumes winning
+      //      if the file comes back.
+      //   1. Login-shell probe (`command -v` through a POSIX shell) —
+      //      reflects the user's curated environment.
+      //   2. Direct PATH/well-known-dir scan inside resolveToolPath —
+      //      catches exotic-$SHELL / broken-rc false negatives.
+      //   3. Persisted auto-path fallback: a previously-probed toolPaths
+      //      entry whose file still execs, so a transient probe failure
+      //      (slow rc file, shell hiccup) can't wipe a good cached path
+      //      via the null → delete write-back. Re-checks the file, not
+      //      the bookkeeping, so genuinely-removed tools still clear.
+      const manual = state.manualToolPaths[tool]
+      let systemPath =
+        manual && (await isExecutable(manual)) ? manual : null
+      if (!systemPath) {
+        systemPath = tool === 'brew' ? brewPath : await resolveToolPath(tool)
+      }
+      if (!systemPath) {
+        const persisted = state.toolPaths[tool]
+        if (persisted && (await isExecutable(persisted))) systemPath = persisted
+      }
       // WHY bundled detection takes precedence over PATH:
       //   When we ship a tool, that is the version we tested with and
       //   the version proxy diagnostics + behaviour assume. A user's

src/main/setup/setupState.ts

@@ -9,13 +9,31 @@ const SETUP_STATE_FILE = join(STATE_DIR, 'setup.json')
 export type PersistedSetupState = {
   version: 1
   toolPaths: Partial<Record<SetupToolId, string>>
+  // WHY a second map instead of a flag on toolPaths entries: toolPaths is
+  // the *effective* resolution cache — checkPrerequisites and
+  // revalidateToolchain overwrite it freely with whatever the probes find,
+  // and that churn is correct for auto-resolved paths (a new volta shim
+  // SHOULD replace a stale /usr/local copy). A manual override from
+  // setup:set-tool-path is different in kind: it is the user's explicit
+  // word, and codex review of #504 caught that storing it only in the
+  // shared map let the very next auto-probe silently erase it (user's
+  // ~/bin/claude-wrapper replaced by PATH's /usr/local/bin/claude).
+  // Keeping user intent in its own map means the auto writers can stay
+  // dumb — they never have to know which toolPaths entries are sacred —
+  // while the two auto-resolution entry points (checkPrerequisites,
+  // revalidateToolchain) consult this map first and skip the auto layers
+  // for a still-valid override. Absent from setup.json files written
+  // before this field existed; loadSetupState defaults it to {} so no
+  // version bump / migration is needed.
+  manualToolPaths: Partial<Record<SetupToolId, string>>
   skippedOptionalTools: Partial<Record<SetupToolId, boolean>>
   updatedAt: number
 }
 
 const DEFAULT_SETUP_STATE: PersistedSetupState = {
   version: 1,
   toolPaths: {},
+  manualToolPaths: {},
   skippedOptionalTools: {},
   updatedAt: 0,
 }
@@ -31,6 +49,7 @@ export async function loadSetupState(): Promise<PersistedSetupState> {
     cache = {
       version: 1,
       toolPaths: parsed.toolPaths ?? {},
+      manualToolPaths: parsed.manualToolPaths ?? {},
       skippedOptionalTools: parsed.skippedOptionalTools ?? {},
       updatedAt: typeof parsed.updatedAt === 'number' ? parsed.updatedAt : 0,
     }
@@ -85,6 +104,26 @@ export async function updateToolPaths(
   return await saveSetupState({ ...state, toolPaths })
 }
 
+// Records a user-supplied override from setup:set-tool-path. Writes BOTH
+// maps: manualToolPaths is the durable record of user intent (what makes
+// the override win over future auto-probes — see the type comment above),
+// and toolPaths is the effective cache that refreshToolchainFromState /
+// applyToolEnv consume immediately, so the override takes effect in the
+// same round-trip instead of waiting for the next checkPrerequisites
+// write-back. Callers are expected to have validated the path (executable
+// regular file) BEFORE calling — this module stays pure bookkeeping.
+export async function setManualToolPath(
+  tool: SetupToolId,
+  path: string,
+): Promise<PersistedSetupState> {
+  const state = await loadSetupState()
+  return await saveSetupState({
+    ...state,
+    manualToolPaths: { ...state.manualToolPaths, [tool]: path },
+    toolPaths: { ...state.toolPaths, [tool]: path },
+  })
+}
+
 export async function markOptionalSkipped(
   tool: SetupToolId,
   skipped: boolean,

src/main/setup/shell.ts

@@ -1,9 +1,45 @@
 import { execFile } from 'child_process'
 import { promisify } from 'util'
+import { accessSync, constants as fsConstants } from 'fs'
+import { basename } from 'path'
 
 const execFileAsync = promisify(execFile)
 
-const LOGIN_SHELL = process.env.SHELL || '/bin/zsh'
+// WHY a POSIX allowlist instead of just trusting $SHELL (#495 A1):
+// runLoginShell exists to see the PATH a user's *login* environment builds
+// (rc files add ~/.local/bin, `brew shellenv`, volta/asdf shims, …). That
+// only works if the shell accepts the POSIX `-lc <script>` contract and
+// gives `command -v` POSIX semantics. fish, nushell, xonsh, elvish and
+// pwsh don't reliably do either — with one of those as $SHELL the probe
+// used to fail, the caller treated "probe broke" as "tool missing", and
+// SetupGate hard-locked the entire app on first launch with no Continue
+// button and no override. So: use $SHELL only when its basename is a known
+// POSIX shell; otherwise probe through the first of /bin/zsh → /bin/bash →
+// /bin/sh that exists. A fish user's fish-only PATH additions are covered
+// by the direct PATH/well-known-dir scan in binaryResolver (layer 2 of
+// resolution), NOT by contorting this probe per-shell — teaching this file
+// fish/nu dialects would couple us to every shell's flag grammar forever.
+const POSIX_SHELLS = new Set(['bash', 'zsh', 'sh', 'dash', 'ksh'])
+
+// Computed per call, not at module load: two stat syscalls are nothing
+// next to the fork+exec that follows, and a module-level const would bake
+// in whatever $SHELL looked like at first import for the process lifetime.
+function pickProbeShell(): string {
+  const userShell = process.env.SHELL
+  if (userShell && POSIX_SHELLS.has(basename(userShell))) return userShell
+  for (const candidate of ['/bin/zsh', '/bin/bash', '/bin/sh']) {
+    try {
+      accessSync(candidate, fsConstants.X_OK)
+      return candidate
+    } catch {
+      // keep looking
+    }
+  }
+  // Nothing probeable exists — return /bin/sh anyway so the spawn error
+  // surfaces to the caller (which maps it to "not found" and falls through
+  // to the direct PATH scan) instead of throwing from here synchronously.
+  return '/bin/sh'
+}
 
 export type LoginShellResult = {
   stdout: string
@@ -15,7 +51,7 @@ export async function runLoginShell(
   options: { timeoutMs?: number; maxBuffer?: number } = {},
 ): Promise<LoginShellResult> {
   const { stdout, stderr } = await execFileAsync(
-    LOGIN_SHELL,
+    pickProbeShell(),
     ['-lc', script],
     {
       timeout: options.timeoutMs ?? 30_000,
@@ -28,4 +64,3 @@ export async function runLoginShell(
     stderr: String(stderr ?? ''),
   }
 }
-