fix: spawn agents via resolved absolute path on Windows (closes #10) (#13)

* fix: spawn agents via resolved absolute path on Windows (#10)

Detection in `/api/agents` resolves each agent's full executable path,
but `/api/chat` was spawning the bare `def.bin` ("claude"). On Windows
the child process's PATH often doesn't include the user's npm-global
shim directory, so spawn() failed with ENOENT despite the picker
showing the agent as available.

Use the resolved path at spawn time, and pass `shell: true` when the
resolved bin is a `.cmd`/`.bat` shim so Node ≥21 doesn't refuse to
execute it (CVE-2024-27980).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address PR #13 review — friendlier ENOENT, document shell:true caveats

- When `resolveAgentBin` returns null we now emit a friendly SSE `error`
  pointing at /api/agents and end the stream, instead of silently falling
  back to spawn(def.bin) — which would re-introduce the exact issue #10
  symptom this PR is meant to prevent.
- Strengthen the comment around `shell: true` on Windows: call out that
  the only thing keeping user-controlled prompt text safe today is Node's
  CVE-2024-27980 escaper, that the proper fix is to route the composed
  prompt through child stdin (not a new `-p $prompt`-style flag), and
  that cmd.exe's ~8191-char command-line cap reintroduces an
  ENAMETOOLONG-class failure for long prompts under shell:true.
- Document why the `.cmd`/`.bat` regex is the right gate: those are the
  only PATHEXT extensions that strictly need cmd.exe; `.exe`/`.com`
  launch directly and `.ps1`/`.vbs` would need a different host that
  shell:true (=cmd.exe) wouldn't help with.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

daemon/agents.js

@@ -339,6 +339,16 @@ export function getAgentDef(id) {
   return AGENT_DEFS.find((a) => a.id === id) || null;
 }
 
+// Resolve the absolute path of an agent's binary on the current PATH.
+// Used by the chat handler so spawn() gets the same executable that
+// detection reported as available — fixes Windows ENOENT when the bare
+// bin name isn't on the child process's PATH (issue #10).
+export function resolveAgentBin(id) {
+  const def = getAgentDef(id);
+  if (!def?.bin) return null;
+  return resolveOnPath(def.bin);
+}
+
 // Daemon's /api/chat needs to validate the user's model pick against the
 // list we last surfaced to the UI. We keep a per-agent cache of the most
 // recent live list (refreshed every detectAgents() call) and additionally

daemon/server.js

@@ -10,6 +10,7 @@ import {
   detectAgents,
   getAgentDef,
   isKnownModel,
+  resolveAgentBin,
   sanitizeCustomModel,
 } from './agents.js';
 import { listSkills } from './skills.js';
@@ -815,9 +816,54 @@ export async function startServer({ port = 7456 } = {}) {
       res.write(`data: ${JSON.stringify(data)}\n\n`);
     };
 
+    // Resolve the agent's bin to its absolute path. Detection (`/api/agents`)
+    // already locates the executable via PATH, but spawning the bare name here
+    // fails on Windows (ENOENT) when the child process's PATH doesn't contain
+    // the user's npm-global / shim directory — see issue #10.
+    //
+    // If detection can't find the binary, surface a friendly SSE error
+    // pointing at /api/agents instead of silently falling back to
+    // spawn(def.bin) — that fallback re-introduces the exact ENOENT symptom
+    // from issue #10 the rest of this block is meant to prevent.
+    const resolvedBin = resolveAgentBin(agentId);
+    if (!resolvedBin) {
+      send('error', {
+        message:
+          `Agent "${def.name}" (\`${def.bin}\`) is not installed or not on PATH. ` +
+          'Install it and refresh the agent list (GET /api/agents) before retrying.',
+      });
+      return res.end();
+    }
+    // npm shims on Windows are .cmd/.bat files; Node ≥21 refuses to spawn
+    // those without `shell: true` (CVE-2024-27980). When `shell: true` is set
+    // on Windows, Node escapes argv items for the cmd.exe shell — that
+    // escape is what currently keeps user-controlled prompt text in `args`
+    // (composed via `def.buildArgs(prompt, ...)` above) from being
+    // interpreted as shell metacharacters. Two caveats this leaves on the
+    // table for a future contributor to be aware of:
+    //   1. Defensibility relies on Node's escaper staying correct. The
+    //      stronger fix is to keep user text out of argv entirely by piping
+    //      the composed prompt through child stdin instead of passing it
+    //      as a `-p $prompt`-style flag. Do NOT add a new prompt-bearing
+    //      flag in `buildArgs` thinking shell:true makes it safe — route
+    //      it through stdin instead.
+    //   2. cmd.exe caps the full command line at ~8191 chars (well below
+    //      Node's direct-spawn argv cap), so long prompts can fail with an
+    //      ENAMETOOLONG-class error here. Same mitigation: stdin.
+    //
+    // We only flip shell:true for `.cmd`/`.bat` because those are the only
+    // PATHEXT entries that strictly require cmd.exe to launch. `.exe`/`.com`
+    // launch directly (no shell needed); `.ps1`/`.vbs` etc. would need a
+    // different host (powershell / wscript) — `shell: true` (which uses
+    // cmd.exe) wouldn't actually help those, so we don't pretend it would.
+    // In practice npm-installed CLIs ship as `.cmd` shims, which is the
+    // case this branch covers.
+    const useShell =
+      process.platform === 'win32' && /\.(cmd|bat)$/i.test(resolvedBin);
+
     send('start', {
       agentId,
-      bin: def.bin,
+      bin: resolvedBin,
       streamFormat: def.streamFormat ?? 'plain',
       projectId: typeof projectId === 'string' ? projectId : null,
       cwd,
@@ -827,10 +873,11 @@ export async function startServer({ port = 7456 } = {}) {
 
     let child;
     try {
-      child = spawn(def.bin, args, {
+      child = spawn(resolvedBin, args, {
         env: { ...process.env },
         stdio: ['ignore', 'pipe', 'pipe'],
         cwd: cwd || undefined,
+        shell: useShell,
       });
     } catch (err) {
       send('error', { message: `spawn failed: ${err.message}` });