fix(runtimes): reach a working agent CLI past a broken shim

apps/daemon/src/runtimes/detection.ts

@@ -11,7 +11,11 @@ import { spawnEnvForAgent } from './env.js';
 import { probeAgentAuthStatus } from './auth.js';
 import { agentCapabilities } from './capabilities.js';
 import { installMetaForAgent } from './metadata.js';
-import { resolveAmrOpenCodeExecutable } from './executables.js';
+import {
+  forgetDetectedExecutable,
+  rememberDetectedExecutable,
+  resolveAmrOpenCodeExecutable,
+} from './executables.js';
 import { resolveAmrProfile } from '../integrations/vela.js';
 import {
   buildAuthDiagnostic,
@@ -46,6 +50,12 @@ export interface DetectedRuntimeVersions {
 // exact executable family without spawning another process on every turn.
 const detectedRuntimeVersions = new Map<string, DetectedRuntimeVersions>();
 
+// How many unusable binaries detection will walk past before giving up on an
+// agent. Each attempt costs one bounded `--version` spawn, and the healthy
+// case stops at the first candidate, so this only bounds the pathological
+// shape: the same CLI name shadowed in many search directories at once.
+const MAX_EXECUTABLE_ATTEMPTS = 8;
+
 export function getDetectedRuntimeVersions(
   agentId: string | null | undefined,
 ): DetectedRuntimeVersions | null {
@@ -248,6 +258,9 @@ async function probe(
   configuredEnv: Record<string, string> = {},
 ): Promise<DetectedAgent> {
   detectedRuntimeVersions.delete(def.id);
+  // Drop the previous winner before re-probing: a rescan after the user fixes
+  // or removes a CLI must not resolve against a stale one.
+  forgetDetectedExecutable(def.id);
   // Detection must probe the exact path the runtime will spawn, not just the
   // PATH-visible shim. This is load-bearing for Codex under nvm/fnm/mise:
   // the discovered `codex` entry is often a `#!/usr/bin/env node` wrapper
@@ -256,11 +269,23 @@ async function probe(
   // If detection probes the shim but chat/run spawns the native binary, the
   // UI incorrectly reports "not installed" until the user pins CODEX_BIN by
   // hand even though the real launch path is healthy.
-  const launch = resolveAgentLaunch(def, configuredEnv);
-  if (!launch.selectedPath || !launch.launchPath) {
+  const initialLaunch = resolveAgentLaunch(def, configuredEnv);
+  if (!initialLaunch.selectedPath || !initialLaunch.launchPath) {
     return unavailableAgent(def, [buildExecutableDiagnostic(def, configuredEnv)]);
   }
-  const probeEnv = applyAgentLaunchEnv(
+  // Carry the narrowed pair explicitly: the candidate walk below reassigns
+  // this binding, which would otherwise discard the null-check above and
+  // force every downstream reader to re-prove the paths are present.
+  type ProbedLaunch = ReturnType<typeof resolveAgentLaunch> & {
+    selectedPath: string;
+    launchPath: string;
+  };
+  let launch: ProbedLaunch = {
+    ...initialLaunch,
+    selectedPath: initialLaunch.selectedPath,
+    launchPath: initialLaunch.launchPath,
+  };
+  let probeEnv = applyAgentLaunchEnv(
     spawnEnvForAgent(
       def.id,
       {
@@ -273,14 +298,76 @@ async function probe(
     ),
     launch,
   );
-  const outcome = await probeVersionAtPath(def, launch.launchPath, probeEnv);
+  let outcome = await probeVersionAtPath(def, launch.launchPath, probeEnv);
+  // Resolving a name on PATH only proves a file exists there, never that it
+  // runs. A directory that ranks earlier in the search order can hold a
+  // wrapper orphaned by a half-finished `npm i -g` — the shim survives, the
+  // package it points at does not — and stopping at that first hit hides a
+  // perfectly good CLI of the same name further down the list. Walk past
+  // every candidate that cannot be executed before declaring the agent
+  // unusable. Only spawn-level failures advance the walk: a version that
+  // parses badly, or a binary that runs and exits non-zero, is a real
+  // answer from the right binary and must not fall through to another one.
+  const attemptedPaths: string[] = [];
+  while (
+    outcome.kind === 'not-invocable' &&
+    attemptedPaths.length < MAX_EXECUTABLE_ATTEMPTS
+  ) {
+    const failedPath = launch.selectedPath;
+    if (!failedPath) break;
+    attemptedPaths.push(failedPath);
+    const next = resolveAgentLaunch(def, configuredEnv, {
+      skipPathCandidates: attemptedPaths,
+    });
+    // No candidate left, or the resolver handed back something already
+    // proven broken (an explicit override or packaged built-in, which are
+    // deliberately not skippable) — either way there is nothing new to try.
+    if (!next.selectedPath || !next.launchPath) break;
+    if (attemptedPaths.includes(next.selectedPath)) break;
+    launch = {
+      ...next,
+      selectedPath: next.selectedPath,
+      launchPath: next.launchPath,
+    };
+    probeEnv = applyAgentLaunchEnv(
+      spawnEnvForAgent(
+        def.id,
+        {
+          ...process.env,
+          ...(def.env || {}),
+        },
+        configuredEnv,
+        undefined,
+        { resolvedBin: next.selectedPath },
+      ),
+      next,
+    );
+    outcome = await probeVersionAtPath(def, next.launchPath, probeEnv);
+  }
+  // Publish the executable this pass proved invocable, so chat, the connection
+  // test, and every other spawn site resolve to the same binary instead of
+  // redoing the naive first-hit-on-PATH walk. This restores the invariant
+  // stated above — detection probes the exact path the runtime will spawn —
+  // which the candidate walk would otherwise have broken.
+  if (outcome.kind !== 'not-invocable') {
+    rememberDetectedExecutable(def.id, launch.selectedPath);
+  }
   if (outcome.kind === 'not-invocable') {
-    return unavailableAgent(def, [
-      buildNotInvocableDiagnostic(def, launch, outcome.cause),
-    ]);
+    // Report the path that was actually tried. The agent picker only renders
+    // an unavailable agent when it carries a path (that is what makes the
+    // row actionable), so dropping it here erases the agent from the UI and
+    // leaves the user with no way to see or fix what went wrong.
+    return unavailableAgent(
+      def,
+      [buildNotInvocableDiagnostic(def, launch, outcome.cause)],
+      { path: launch.selectedPath },
+    );
   }
   if (def.versionPolicy?.requireVersion && !outcome.version) {
-    return unavailableAgent(def, [buildVersionDiagnostic(def, outcome.version)]);
+    return unavailableAgent(def, [buildVersionDiagnostic(def, outcome.version)], {
+      path: launch.selectedPath,
+      version: outcome.version,
+    });
   }
   let runtimeCompanionVersion: string | undefined;
   if (def.compatibilityProbe) {

apps/daemon/src/runtimes/executables.ts

@@ -128,19 +128,36 @@ export function agentBinEnvKey(agentId: string | undefined): string | null {
   return AGENT_BIN_ENV_KEYS.get(agentId) ?? null;
 }
 
-export function resolveOnPath(bin: string): string | null {
+// Every file named `bin` that exists on the search path, in resolution
+// order. Detection needs the whole list, not just the winner: a directory
+// that ranks earlier can hold a wrapper left behind by a half-finished
+// install, and executing it fails even though a working CLI of the same
+// name sits in a later directory. Resolution alone cannot tell the two
+// apart — only spawning can — so the caller walks candidates until one
+// actually runs.
+export function resolveAllOnPath(bin: string): string[] {
   const exts =
     process.platform === 'win32'
       ? (process.env.PATHEXT || '.EXE;.CMD;.BAT').split(';')
       : [''];
   const dirs = resolvePathDirs();
+  const found: string[] = [];
+  const seen = new Set<string>();
   for (const dir of dirs) {
     for (const ext of exts) {
       const full = path.join(dir, bin + ext);
-      if (full && existsSync(full)) return full;
+      if (!full || seen.has(full)) continue;
+      if (existsSync(full)) {
+        seen.add(full);
+        found.push(full);
+      }
     }
   }
-  return null;
+  return found;
+}
+
+export function resolveOnPath(bin: string): string | null {
+  return resolveAllOnPath(bin)[0] ?? null;
 }
 
 function looksExecutableOnWindows(filePath: string): boolean {
@@ -336,9 +353,55 @@ export function resolveAgentExecutable(
   return inspectAgentExecutableResolution(def, configuredEnv).selectedPath;
 }
 
+// The executable a completed detection pass settled on, per agent id.
+//
+// Detection is the only stage that learns which candidate can actually be
+// executed — it is the only one that spawns anything. Without recording that
+// answer, every later resolution (chat, connection test, memory summariser,
+// companion install) would redo the naive "first hit on PATH" walk and land
+// back on the very shim detection just rejected: Settings would advertise the
+// agent as installed while each turn exec'd a broken wrapper. Publishing the
+// winner here keeps detection and launch pointed at the same binary.
+const detectedExecutables = new Map<string, string>();
+
+/** Record the executable a detection pass proved invocable. */
+export function rememberDetectedExecutable(agentId: string, resolvedPath: string): void {
+  detectedExecutables.set(agentId, resolvedPath);
+}
+
+/**
+ * Drop an agent's remembered executable. Detection clears it before each pass
+ * so a re-scan after the user repairs or removes a CLI never resolves against
+ * a stale winner.
+ */
+export function forgetDetectedExecutable(agentId: string): void {
+  detectedExecutables.delete(agentId);
+}
+
+/**
+ * Pick the remembered winner, but only when the live search already offers it.
+ *
+ * The memory reorders candidates; it must never introduce one. Resolution has
+ * to stay a pure function of the current environment — an emptied PATH, a
+ * sandboxed `OD_AGENT_HOME`, or an uninstalled CLI all have to keep meaning
+ * "not found", and a winner remembered from a richer environment must not
+ * resurrect a binary the caller can no longer see.
+ */
+function preferRememberedExecutable(agentId: string, candidates: string[]): string | null {
+  const remembered = detectedExecutables.get(agentId);
+  if (!remembered) return null;
+  return candidates.includes(remembered) ? remembered : null;
+}
+
 export function inspectAgentExecutableResolution(
   def: RuntimeAgentDef,
   configuredEnv: Record<string, string> = {},
+  // Paths already proven unusable by a spawn attempt. Only PATH-derived
+  // candidates are skippable: an explicit `*_BIN` override, a packaged
+  // built-in, and the Codex app bundle are deliberate selections, so a
+  // broken one must surface as an error rather than silently resolving to
+  // some other binary the user never pointed at.
+  options: { skipPathCandidates?: readonly string[] } = {},
 ): {
   configuredOverridePath: string | null;
   pathResolvedPath: string | null;
@@ -356,14 +419,22 @@ export function inspectAgentExecutableResolution(
     def.bin,
     ...(Array.isArray(def.fallbackBins) ? def.fallbackBins : []),
   ];
-  let pathResolvedPath: string | null = null;
+  const skip = new Set(options.skipPathCandidates ?? []);
+  const pathCandidates: string[] = [];
   for (const bin of candidates) {
-    const resolved = resolveOnPath(bin);
-    if (resolved) {
-      pathResolvedPath = resolved;
-      break;
+    for (const resolved of resolveAllOnPath(bin)) {
+      if (skip.has(resolved) || pathCandidates.includes(resolved)) continue;
+      pathCandidates.push(resolved);
     }
   }
+  // Among what the current environment offers, prefer the candidate detection
+  // proved invocable; otherwise take the first hit as before. Plain order is
+  // not enough on its own — the first file that merely *exists* is exactly the
+  // broken shim detection walked past. An explicit `*_BIN` override and a
+  // packaged built-in still outrank both (see the selectedPath order below),
+  // so this only settles guesswork, never a deliberate choice.
+  const pathResolvedPath: string | null =
+    preferRememberedExecutable(def.id, pathCandidates) ?? pathCandidates[0] ?? null;
   const builtInPath = packagedBuiltInExecutable(def, configuredEnv);
   const appBundlePath = codexAppBundleExecutable(def);
   return {

apps/daemon/src/runtimes/launch.ts

@@ -15,8 +15,9 @@ export type AgentLaunchResolution = ReturnType<typeof inspectAgentExecutableReso
 export function resolveAgentLaunch(
   def: RuntimeAgentDef,
   configuredEnv: Record<string, string> = {},
+  options: { skipPathCandidates?: readonly string[] } = {},
 ): AgentLaunchResolution {
-  const resolution = inspectAgentExecutableResolution(def, configuredEnv);
+  const resolution = inspectAgentExecutableResolution(def, configuredEnv, options);
   if (!resolution.selectedPath) {
     return { ...resolution, launchPath: null, launchKind: 'selected', childPathPrepend: [], diagnostic: null };
   }