fix(spawn): spawn the detected absolute CLI path, not the bare name (#104)

src/daemon/agents/kimi.ts

@@ -28,7 +28,7 @@ import { quoteValue, quotePath, validateValue } from './quote.js';
 import { spawnHeadless } from '../headless.js';
 import { parseOpencode, parseOpencodeExit, parseKimi } from './parsers/index.js';
 import { atomicWriteJsonSync } from '../../lib/atomic-write.js';
-import { detectAllClis } from '../../lib/cli-detect.js';
+import { detectAllClis, resolveCliBinaryPath } from '../../lib/cli-detect.js';
 import { wrapWithPty } from './opencode.js';
 import { assertSandboxSupported, sandboxFailClosed } from './sandbox-guard.js';
 
@@ -312,7 +312,10 @@ export const kimiShim: AgentShim = {
     // failure mode the opencode PTY fix was written to prevent. Confirmed
     // launch-eve by both deepseek and gemini reviewing the shims; the bug
     // was a copy-paste oversight when the PTY fix landed in opencode.ts.
-    const { command, args } = wrapWithPty('opencode', opencodeArgs);
+    // #104: resolve opencode to its detected absolute path before PTY-wrapping
+    // — the wrapper embeds the binary in argv, so the spawn core only sees
+    // `script` and can't resolve it. (Same rationale as opencodeShim.)
+    const { command, args } = wrapWithPty(resolveCliBinaryPath('opencode'), opencodeArgs);
     const run = spawnHeadless({
       command,
       args,

src/daemon/agents/opencode.ts

@@ -14,6 +14,7 @@ import type {
 } from './types.js';
 import { quotePath, validateValue } from './quote.js';
 import { spawnHeadless } from '../headless.js';
+import { resolveCliBinaryPath } from '../../lib/cli-detect.js';
 import { parseOpencode, parseOpencodeExit } from './parsers/index.js';
 import { assertSandboxSupported, sandboxFailClosed } from './sandbox-guard.js';
 import * as fs from 'fs';
@@ -197,7 +198,10 @@ export const opencodeShim: AgentShim = {
     // forwards the child exit code so cli_failed detection still works.
     // Header/footer are absent under -q, so parseOpencode (which already
     // tryJson's every line and discards non-JSON) needs no change.
-    const { command, args } = wrapWithPty('opencode', opencodeArgs);
+    // #104: resolve opencode to its detected absolute path HERE. The PTY
+    // wrapper embeds the binary inside argv (Linux: `script -qfec '<bin>…'`),
+    // so the spawn core only ever sees `script` and can't resolve it for us.
+    const { command, args } = wrapWithPty(resolveCliBinaryPath('opencode'), opencodeArgs);
 
     const run = spawnHeadless({
       command,

src/daemon/headless.ts

@@ -28,38 +28,51 @@ import * as os from 'os';
 import * as path from 'path';
 import type { AgentEvent } from './agents/types.js';
 import { cliPaths } from '../lib/cli-paths.js';
+import { resolveCliBinaryPath } from '../lib/cli-detect.js';
 
 // Cache binary→path resolutions. `where` shells out — don't repeat per spawn.
 const binaryPathCache = new Map<string, string>();
 
 /**
- * Resolve a binary name to a full path with extension. Critical on Windows
- * where Node's spawn won't resolve `.cmd` shims (npm globals like
- * claude.cmd, codex.cmd, gemini.cmd) without shell:true (DEP0190 in
- * Node 22+). On Unix returns the name unchanged — spawn handles PATH
- * resolution natively for ELF/script files with shebangs.
+ * Resolve a binary name to a full path with extension. Two stages:
+ *
+ *   1. #104 — prefer the absolute path detection already resolved AND verified
+ *      for this CLI, so the daemon spawns exactly the binary detection chose.
+ *      This closes two gaps between detection and execution: ENOENT (a CLI
+ *      found only via the fallback-dir scan whose dir isn't on the spawn PATH)
+ *      and shadowing (two same-named binaries, e.g. ~/.kimi/bin/kimi vs
+ *      ~/.kimi-code/bin/kimi, where bare-name spawn could pick the other one).
+ *      When detection has no path, `resolveCliBinaryPath` returns the bare name
+ *      unchanged and the existing resolution below applies — no behaviour
+ *      change for the unresolved case.
+ *   2. Windows `where` fallback — critical where Node's spawn won't resolve
+ *      `.cmd` shims (npm globals like claude.cmd) without shell:true (DEP0190
+ *      in Node 22+). Skipped once stage 1 yields an absolute path. On Unix a
+ *      bare name is passed through — spawn handles PATH resolution natively
+ *      for ELF/script files with shebangs.
  */
 function resolveBinaryPath(command: string): string {
-  if (process.platform !== 'win32') return command;
+  const detected = resolveCliBinaryPath(command);
+  if (process.platform !== 'win32') return detected;
   // Use the Windows-specific isAbsolute (`path.win32`) so absolute
   // detection works the same on a real Windows host AND on a Linux CI
   // run where the test stubs `process.platform = 'win32'` but the
   // top-level `path` module is still POSIX.
-  if (path.win32.isAbsolute(command)) return command;
-  const cached = binaryPathCache.get(command);
+  if (path.win32.isAbsolute(detected)) return detected;
+  const cached = binaryPathCache.get(detected);
   if (cached) return cached;
-  const r = spawnSync('where', [command], { encoding: 'utf-8', timeout: 3000 });
+  const r = spawnSync('where', [detected], { encoding: 'utf-8', timeout: 3000 });
   if (r.status !== 0 || !r.stdout) {
-    binaryPathCache.set(command, command);
-    return command; // fallback — daemon will surface ENOENT cleanly.
+    binaryPathCache.set(detected, detected);
+    return detected; // fallback — daemon will surface ENOENT cleanly.
   }
   // npm globals on Windows ship two siblings: `claude` (Bash shim, not
   // executable by Node spawn) and `claude.cmd` (Windows shim). `where`
   // returns both; we must pick the .cmd/.bat/.exe variant for Node.
   const lines = r.stdout.split(/\r?\n/).map((s) => s.trim()).filter(Boolean);
   const preferred = lines.find((l) => /\.(cmd|bat|exe)$/i.test(l));
-  const resolved = preferred ?? lines[0] ?? command;
-  binaryPathCache.set(command, resolved);
+  const resolved = preferred ?? lines[0] ?? detected;
+  binaryPathCache.set(detected, resolved);
   return resolved;
 }
 

src/lib/cli-detect.ts

@@ -447,6 +447,54 @@ export function clearDetectionCache(): void {
   detectCache = null;
 }
 
+/**
+ * Reverse of BINARY_NAME: bare binary name → DetectableCli id. BINARY_NAME's
+ * values are unique, so the inversion is lossless. Lets a spawn site that only
+ * knows the bare command (e.g. `kimi`) recover the detection id without
+ * threading it through every shim.
+ */
+const CLI_ID_BY_BINARY: Record<string, DetectableCli> = Object.fromEntries(
+  (Object.entries(BINARY_NAME) as [DetectableCli, string][]).map(
+    ([id, bin]) => [bin, id],
+  ),
+) as Record<string, DetectableCli>;
+
+/**
+ * Resolve a bare CLI binary name to the absolute path detection already
+ * verified for it, so the daemon spawns the SAME binary it detected (#104).
+ *
+ * Detection resolves a concrete path (PATH lookup → fallback-dir scan → manual
+ * override); spawning by bare name re-resolves against the daemon's PATH and
+ * can disagree two ways:
+ *   - ENOENT — a CLI found only via the fallback-dir scan (its install dir is
+ *     not on the spawn PATH) can't be launched by bare name at all.
+ *   - shadowing — when two same-named binaries coexist (e.g. ~/.kimi/bin/kimi
+ *     and ~/.kimi-code/bin/kimi), bare-name spawn may run a different build
+ *     than detection resolved, so the transport decision (#101) and the binary
+ *     that actually runs end up describing different binaries.
+ *
+ * Returns `command` UNCHANGED when:
+ *   - it already carries a path separator (the caller passed a concrete path —
+ *     e.g. a manual override, or the `script` PTY wrapper's resolved target);
+ *   - it isn't one of our known CLI binaries (e.g. `script`, `sh`);
+ *   - detection currently can't find that CLI — falling through to the
+ *     caller's existing bare-name resolution (Unix PATH / Windows `where`),
+ *     preserving pre-#104 behaviour.
+ *
+ * Reads the cached detection result (30s TTL) — the steady-state cost is a map
+ * lookup; a cold cache pays one detect sweep, the same one the kimi transport
+ * probe already triggers.
+ */
+export function resolveCliBinaryPath(command: string): string {
+  // Only ever rewrite a bare name. Anything with a separator is a concrete
+  // path the caller has already committed to.
+  if (command.includes('/') || command.includes('\\')) return command;
+  const id = CLI_ID_BY_BINARY[command];
+  if (!id) return command;
+  const hit = detectAllClis().find((c) => c.id === id && c.found && c.path);
+  return hit?.path ?? command;
+}
+
 /**
  * Validate a user-supplied path for a given CLI. Used by the
  * "Set path manually" fallback when auto-detect misses.

tests/cli-detect-resolve.test.ts

@@ -0,0 +1,121 @@
+/**
+ * resolveCliBinaryPath — spawn the binary detection verified (issue #104).
+ *
+ * Detection (`detectAllClis`) resolves a concrete absolute path: PATH lookup
+ * (`which`/`where`) → fallback-dir scan → manual override. Headless spawns,
+ * however, used the BARE name and re-resolved against the daemon's PATH. The
+ * two could disagree:
+ *   - ENOENT: a CLI found only via the fallback-dir scan (its dir not on the
+ *     spawn PATH) can't be spawned by bare name.
+ *   - shadowing: two same-named binaries (e.g. ~/.kimi/bin/kimi and
+ *     ~/.kimi-code/bin/kimi) — bare-name spawn may run a different build than
+ *     detection resolved.
+ *
+ * resolveCliBinaryPath closes that gap by mapping a bare CLI name back to the
+ * absolute path detection already chose. These tests drive REAL detection
+ * (staged executables on a prepended PATH) so they fail if the reverse map or
+ * the found-predicate regresses.
+ */
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { randomUUID } from 'node:crypto';
+
+import {
+  resolveCliBinaryPath,
+  detectAllClis,
+  clearDetectionCache,
+} from '../src/lib/cli-detect.js';
+
+// Mirrors BINARY_NAME in cli-detect.ts. Duplicated intentionally: a rename of
+// a detection binary is a deliberate change that should make this test fail.
+const BINARY_NAME: Record<string, string> = {
+  'claude-code': 'claude',
+  'codex-cli': 'codex',
+  'gemini-cli': 'gemini',
+  'opencode-cli': 'opencode',
+  'kimi-cli': 'kimi',
+  'grok-cli': 'grok',
+  'antigravity-cli': 'agy',
+};
+
+let tmpRoot: string;
+let savedPath: string | undefined;
+
+beforeEach(() => {
+  tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'chorus-resolve-'));
+  savedPath = process.env.PATH;
+  clearDetectionCache();
+});
+
+afterEach(() => {
+  if (savedPath === undefined) delete process.env.PATH;
+  else process.env.PATH = savedPath;
+  clearDetectionCache();
+  fs.rmSync(tmpRoot, { recursive: true, force: true });
+});
+
+/**
+ * Stage an executable shell stub named `name` in a fresh dir, prepend that dir
+ * to PATH so `which`/`where` resolves it FIRST (ahead of any host install),
+ * and return its absolute path. The stub echoes `versionOutput` so detection's
+ * `<bin> --version` signature gate passes.
+ */
+function stageOnPath(name: string, versionOutput: string): string {
+  const dir = path.join(tmpRoot, randomUUID());
+  fs.mkdirSync(dir, { recursive: true });
+  const full = path.join(dir, name);
+  fs.writeFileSync(full, `#!/bin/sh\necho "${versionOutput}"\n`, { mode: 0o755 });
+  process.env.PATH = `${dir}${path.delimiter}${process.env.PATH ?? ''}`;
+  return full;
+}
+
+describe('resolveCliBinaryPath (issue #104)', () => {
+  it('resolves a bare known CLI name to the absolute path detection found', () => {
+    const staged = stageOnPath('kimi', 'kimi, version 1.46.0');
+    detectAllClis(true); // prime the cache against the staged PATH
+    expect(resolveCliBinaryPath('kimi')).toBe(staged);
+  });
+
+  it('leaves an already-absolute path unchanged (caller already decided)', () => {
+    // No detection lookup — a path-bearing command is spawned verbatim.
+    expect(resolveCliBinaryPath('/opt/custom/kimi')).toBe('/opt/custom/kimi');
+  });
+
+  it('leaves a relative path (with separator) unchanged', () => {
+    expect(resolveCliBinaryPath('./bin/kimi')).toBe('./bin/kimi');
+  });
+
+  it('leaves an unknown binary name unchanged (e.g. the `script` PTY wrapper)', () => {
+    // `script` is not one of our CLIs, so it must never be rewritten — the
+    // opencode PTY path spawns it with opencode embedded in argv.
+    const staged = stageOnPath('kimi', 'kimi, version 1.46.0');
+    detectAllClis(true);
+    expect(staged).toContain('kimi'); // sanity: detection primed
+    expect(resolveCliBinaryPath('script')).toBe('script');
+    expect(resolveCliBinaryPath('sh')).toBe('sh');
+  });
+
+  it('returns the bare name for a known CLI that detection cannot find', () => {
+    // Pick whichever supported CLI the host genuinely lacks so the assertion
+    // is deterministic regardless of what's installed. (At least grok or
+    // antigravity is absent on any normal dev/CI box.) Skip in the unlikely
+    // event every CLI is installed.
+    process.env.PATH = path.join(tmpRoot, 'empty');
+    fs.mkdirSync(process.env.PATH, { recursive: true });
+    const missing = detectAllClis(true).find((d) => !d.found);
+    if (!missing) return;
+    const bare = BINARY_NAME[missing.id];
+    expect(resolveCliBinaryPath(bare)).toBe(bare);
+  });
+
+  it('mirrors detection for every CLI: detected path when found, bare name otherwise', () => {
+    const results = detectAllClis(true);
+    for (const d of results) {
+      const bare = BINARY_NAME[d.id];
+      const expected = d.found && d.path ? d.path : bare;
+      expect(resolveCliBinaryPath(bare)).toBe(expected);
+    }
+  });
+});