diff --git a/src/__tests__/local-agent-path-resolution.test.ts b/src/__tests__/local-agent-path-resolution.test.ts new file mode 100644 index 0000000..74b34cb --- /dev/null +++ b/src/__tests__/local-agent-path-resolution.test.ts @@ -0,0 +1,213 @@ +/** + * REGRESSION (issue #78 — macOS/POSIX): the local-agent detector resolved the CLI binary + * against the SERVER process PATH. When T3MP3ST is launched from Finder, the desktop app, or + * any non-interactive shell, launchd hands it a minimal PATH (/usr/bin:/bin:/usr/sbin:/sbin) — + * missing ~/.local/bin (Claude Code's native install), Homebrew, npm-global, and version-manager + * shims (mise/nvm/fnm). `execFile('claude', …)` then throws ENOENT and an installed, working CLI + * is reported `installed:false` (the Settings checkboxes go dead — reads like a UI bug). + * + * The fix is `resolveBin()`: pure-fs resolution over PATH + a curated set of well-known install + * dirs under the real agent home, with an executable-file check, fail-open to the bare name. + * These tests pin the resolver in isolation, the detector wiring, AND the two spawn call-sites + * (runLocalAgent / localAgentChat) the fix rewired — all through the public API. + * + * POSIX-only by design: `resolveBin` short-circuits on win32 (Windows resolution — where.exe / + * .cmd shims — is PR #18's domain, deliberately not duplicated here). The win32 case is asserted + * via a platform stub so the boundary is documented and locked. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { resolveBin, detectLocalAgents, runLocalAgent, localAgentChat } from '../agent/local-agents.js'; + +const FAKE_CLI = '#!/bin/sh\necho "1.2.3"\n'; +// For the chat path, which writes the prompt to the child's stdin: drain it before replying so the +// parent's write()/end() completes cleanly (no EPIPE races), then emit a clean, non-error reply. +const FAKE_CLI_STDIN = '#!/bin/sh\ncat >/dev/null 2>&1\necho "1.2.3"\n'; +const tmpDirs: string[] = []; +const savedEnv = { PATH: process.env.PATH, AGENT_HOME: process.env.T3MP3ST_AGENT_HOME }; + +/** A throwaway dir; tracked for afterEach cleanup. */ +function scratch(): string { + const d = mkdtempSync(join(tmpdir(), 't3mp3st-pathres-')); + tmpDirs.push(d); + return d; +} + +/** Drop an executable (0o755) fake binary at `dir/name`; returns its absolute path. */ +function putExe(dir: string, name: string, content: string = FAKE_CLI): string { + mkdirSync(dir, { recursive: true }); + const p = join(dir, name); + writeFileSync(p, content, { mode: 0o755 }); + return p; +} + +afterEach(() => { + process.env.PATH = savedEnv.PATH; + if (savedEnv.AGENT_HOME === undefined) delete process.env.T3MP3ST_AGENT_HOME; else process.env.T3MP3ST_AGENT_HOME = savedEnv.AGENT_HOME; + for (const d of tmpDirs.splice(0)) { try { rmSync(d, { recursive: true, force: true }); } catch { /* noop */ } } +}); + +describe('resolveBin — macOS/POSIX well-known-dir resolution (issue #78)', () => { + it('finds a CLI in ~/.local/bin even when PATH omits it (the reported bug)', () => { + const home = scratch(); + const bin = putExe(join(home, '.local', 'bin'), 'faketool'); + process.env.T3MP3ST_AGENT_HOME = home; // real agent home for this scan + process.env.PATH = '/usr/bin:/bin'; // minimal launchd-style PATH — omits ~/.local/bin + + expect(resolveBin('faketool')).toBe(bin); + }); + + // A representative spread of the curated dirs (not just index-0 ~/.local/bin) so a typo or + // reorder in any entry is caught. Paths are relative to the agent home. + it.each([ + ['.asdf/shims'], + ['.bun/bin'], + ['.local/share/pnpm'], + ])('resolves a CLI installed under the well-known dir %s', (rel) => { + const home = scratch(); + const bin = putExe(join(home, ...rel.split('/')), 'faketool'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + expect(resolveBin('faketool')).toBe(bin); + }); + + it('finds a CLI under an nvm per-version bin dir (dynamic enumeration)', () => { + const home = scratch(); + const bin = putExe(join(home, '.nvm', 'versions', 'node', 'v20.11.0', 'bin'), 'faketool'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + expect(resolveBin('faketool')).toBe(bin); + }); + + it('finds a CLI under an fnm per-version bin dir (dynamic enumeration)', () => { + const home = scratch(); + const bin = putExe(join(home, '.local', 'share', 'fnm', 'node-versions', 'v20.11.0', 'installation', 'bin'), 'faketool'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + expect(resolveBin('faketool')).toBe(bin); + }); + + it('resolves via PATH when the CLI is on it (rich-shell launch still works)', () => { + const dir = scratch(); + const bin = putExe(dir, 'faketool'); + process.env.T3MP3ST_AGENT_HOME = scratch(); // empty home — force the hit to come from PATH + process.env.PATH = `/usr/bin:${dir}`; + + expect(resolveBin('faketool')).toBe(bin); + }); + + it('prefers a PATH match over a well-known-dir match when both exist (shell precedence)', () => { + const home = scratch(); + const wellKnown = putExe(join(home, '.local', 'bin'), 'faketool'); + const pathDir = scratch(); + const onPath = putExe(pathDir, 'faketool'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = `/usr/bin:${pathDir}`; // PATH scanned before well-known dirs + + expect(resolveBin('faketool')).toBe(onPath); + expect(resolveBin('faketool')).not.toBe(wellKnown); + }); + + it('follows a symlink to an executable (asdf/nvm shims & Claude Code\'s own install are symlinks)', () => { + const home = scratch(); + const real = putExe(join(home, 'tools'), 'realtool'); + const linkDir = join(home, '.local', 'bin'); + mkdirSync(linkDir, { recursive: true }); + const link = join(linkDir, 'faketool'); + symlinkSync(real, link); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + expect(resolveBin('faketool')).toBe(link); // the resolved candidate path, symlink followed for the file-check + }); + + it('skips a non-executable match (parity with execFile X_OK) and fails open to the bare name', () => { + const home = scratch(); + const p = join(home, '.local', 'bin', 'faketool'); + mkdirSync(join(home, '.local', 'bin'), { recursive: true }); + writeFileSync(p, 'not executable', { mode: 0o644 }); // present but not +x + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + expect(resolveBin('faketool')).toBe('faketool'); // not the non-exec path + }); + + it('does NOT match a directory named like the CLI (dirs carry the search bit; execvp would skip it)', () => { + // Regression guard: a bare `accessSync(X_OK)` passes for a directory, so resolveBin would return + // the dir → execFile fails EACCES (not ENOENT) → a false installed:true. Require a regular file. + const home = scratch(); + mkdirSync(join(home, '.local', 'bin', 'faketool'), { recursive: true }); // a DIRECTORY, not a binary + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + expect(resolveBin('faketool')).toBe('faketool'); // bare name, not the dir path + }); + + it('returns an already-qualified path unchanged (the bin.includes("/") guard — no re-resolution)', () => { + expect(resolveBin('/opt/custom/claude')).toBe('/opt/custom/claude'); + expect(resolveBin('./rel/claude')).toBe('./rel/claude'); + }); + + it('fails open to the bare name when nothing matches anywhere', () => { + process.env.T3MP3ST_AGENT_HOME = scratch(); + process.env.PATH = '/usr/bin:/bin'; + expect(resolveBin('definitely-not-a-real-cli-xyz')).toBe('definitely-not-a-real-cli-xyz'); + }); + + it('does NOT scan on win32 — returns the bare name (Windows resolution is PR #18\'s domain)', () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'faketool'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + const orig = Object.getOwnPropertyDescriptor(process, 'platform') || { value: process.platform, configurable: true }; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + try { + expect(resolveBin('faketool')).toBe('faketool'); + } finally { + Object.defineProperty(process, 'platform', orig); + } + }); +}); + +describe('detectLocalAgents — wiring: a well-known-dir CLI is reported installed (issue #78)', () => { + it('reports Claude Code installed + versioned when `claude` lives in ~/.local/bin and PATH omits it', async () => { + const home = scratch(); + const claudePath = putExe(join(home, '.local', 'bin'), 'claude'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; // pre-fix: execFile('claude') → ENOENT → installed:false + delete process.env.T3MP3ST_DISABLE_LOCAL_AGENTS; // insurance: a leaked disable flag would short-circuit to [] + + const agents = await detectLocalAgents(); + const claude = agents.find((a) => a.id === 'claude'); + expect(claude?.installed).toBe(true); + expect(claude?.path).toBe(claudePath); + expect(claude?.version).toBe('1.2.3'); // parsed from the resolved binary's --version output + // codex/hermes are absent from this temp home → still not installed (no false positives). + expect(agents.find((a) => a.id === 'codex')?.installed).toBe(false); + }); +}); + +describe('spawn call-sites use the resolved path (issue #78 — detected-but-unspawnable would be worse)', () => { + // Behavioral, not a spawn spy: if the rewiring regressed to the bare `spec.bin`, spawn('claude') + // under PATH=/usr/bin:/bin throws ENOENT → ok:false / reject. Success proves the resolved path ran. + it('runLocalAgent launches the well-known-dir CLI (not the bare name)', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + const res = await runLocalAgent('claude', 'ping', { timeoutMs: 4000 }); + expect(res.ok).toBe(true); + expect(res.output).toContain('1.2.3'); + }); + + it('localAgentChat launches the well-known-dir CLI (not the bare name)', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', FAKE_CLI_STDIN); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + await expect(localAgentChat('claude', 'ping', { timeoutMs: 4000 })).resolves.toContain('1.2.3'); + }); +}); diff --git a/src/agent/local-agents.ts b/src/agent/local-agents.ts index fed5c4c..cf6b5ed 100644 --- a/src/agent/local-agents.ts +++ b/src/agent/local-agents.ts @@ -13,7 +13,7 @@ // ============================================================================= import { execFile, execFileSync, spawn } from 'child_process'; -import { existsSync, mkdtempSync, readFileSync, rmSync } from 'fs'; +import { accessSync, constants, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync } from 'fs'; import { homedir, tmpdir, userInfo } from 'os'; import { join } from 'path'; @@ -182,12 +182,70 @@ export interface AgentDetection { ready: boolean; // installed && authed } -function resolvePath(bin: string): string | undefined { - try { - return execFileSync('command', ['-v', bin], { shell: '/bin/bash', encoding: 'utf8' }).trim() || undefined; - } catch { - try { return execFileSync('which', [bin], { encoding: 'utf8' }).trim() || undefined; } catch { return undefined; } +/** + * Curated POSIX install dirs an agent CLI commonly lands in, under the REAL agent home (issue #78). + * + * The bug: a T3MP3ST server launched from Finder / the desktop app / any non-interactive shell + * inherits launchd's minimal PATH (/usr/bin:/bin:/usr/sbin:/sbin). That omits every place a CLI + * actually installs — so `execFile('claude', …)` throws ENOENT and a working, authed CLI reads as + * `installed:false`. We recover it by ALSO scanning these dirs (they are where Claude Code's native + * installer, Homebrew, npm-global, and the version managers put their bins). Anchored to agentHome() + * — NOT os.homedir() — so a HOME redirect can't hide them (same rationale as `agentHome`). + */ +function wellKnownBinDirs(home: string): string[] { + const dirs = [ + join(home, '.local', 'bin'), // Claude Code native installer, pipx, mise + join(home, '.local', 'share', 'mise', 'shims'), // mise + join(home, '.asdf', 'shims'), // asdf + '/opt/homebrew/bin', // Homebrew (Apple Silicon) + '/usr/local/bin', // Homebrew (Intel) / manual installs + join(home, '.bun', 'bin'), // bun + join(home, '.deno', 'bin'), // deno + join(home, '.volta', 'bin'), // volta + join(home, '.npm-global', 'bin'), // npm prefix override + join(home, '.yarn', 'bin'), // yarn global + join(home, 'Library', 'pnpm'), // pnpm (macOS default) + join(home, '.local', 'share', 'pnpm'), // pnpm (XDG) + '/opt/local/bin', // MacPorts + ]; + // Version managers keep a per-version bin dir — enumerate the installed versions so an + // npm-global CLI (e.g. codex) under the active node still resolves under a minimal PATH. + for (const nodeVersions of [join(home, '.nvm', 'versions', 'node')]) { + try { for (const v of readdirSync(nodeVersions)) dirs.push(join(nodeVersions, v, 'bin')); } catch { /* not present */ } } + for (const fnmDir of [join(home, '.local', 'share', 'fnm', 'node-versions'), + join(home, 'Library', 'Application Support', 'fnm', 'node-versions')]) { + try { for (const v of readdirSync(fnmDir)) dirs.push(join(fnmDir, v, 'installation', 'bin')); } catch { /* not present */ } + } + return dirs; +} + +/** + * Absolute path to `bin`, resolved WITHOUT a shell against PATH plus the well-known install dirs + * (issue #78). Fail-open: an unresolved name is returned unchanged so `execFile`/`spawn` still throw + * the same ENOENT (→ `installed:false`) they did before — no behavior change when a CLI is truly + * absent. Pure-fs (no shell) so it can't hang or be injected, and it uses an X_OK access check to + * match `execFile`'s own executable-file semantics (a non-executable name-collision is skipped, not + * returned). win32 is intentionally NOT handled here (where.exe / .cmd-shim resolution is PR #18's + * domain) — the bare name is returned so Node's own %PATH%/PATHEXT lookup stays in effect. + */ +export function resolveBin(bin: string): string { + if (process.platform === 'win32' || bin.includes('/')) return bin; + const path = process.env.PATH || ''; + for (const dir of [...path.split(':'), ...wellKnownBinDirs(agentHome())]) { + // Absolute dirs only: skips blank PATH segments (whose POSIX "= cwd" meaning is a known + // CWD-execution vector) and relative segments (which would leak a non-absolute `path`). + if (!dir.startsWith('/')) continue; + const cand = join(dir, bin); + try { + // X_OK alone also passes for a DIRECTORY named `bin` (dirs carry the search bit); the OS's own + // execvp skips such a directory and searches on, so require a regular file to match that and + // avoid returning a dir that execFile would then reject with EACCES (a false installed:true). + accessSync(cand, constants.X_OK); + if (statSync(cand).isFile()) return cand; + } catch { /* missing, not executable, or not a regular file — keep scanning */ } + } + return bin; } function authState(spec: AgentSpec): { authed: boolean; method?: string } { @@ -196,7 +254,10 @@ function authState(spec: AgentSpec): { authed: boolean; method?: string } { } if (spec.keychainService) { try { - execFileSync('security', ['find-generic-password', '-s', spec.keychainService], { stdio: 'ignore' }); + // timeout: a headless/SSH/locked-keychain session (the same non-interactive launch context + // as #78) can make `security` block on an ACL prompt; bound it so detection can't hang the + // event loop. A timeout throws → treated as "not in keychain" (fail-open, same as absence). + execFileSync('security', ['find-generic-password', '-s', spec.keychainService], { stdio: 'ignore', timeout: 2000 }); return { authed: true, method: 'keychain' }; } catch { /* not in keychain */ } } @@ -208,8 +269,12 @@ function detectOne(spec: AgentSpec): Promise { id: spec.id, label: spec.label, vendor: spec.vendor, bin: spec.bin, blurb: spec.blurb, invokeHint: spec.invokeHint, }; + // Resolve against PATH + well-known install dirs so a Finder/desktop launch's minimal PATH + // doesn't hide an installed CLI (issue #78). Absolute when found; the bare name otherwise + // (→ execFile throws the same ENOENT → installed:false, preserving the true-absence path). + const exe = resolveBin(spec.bin); return new Promise((resolve) => { - execFile(spec.bin, spec.versionArgs, { timeout: 8000 }, (err, stdout) => { + execFile(exe, spec.versionArgs, { timeout: 8000 }, (err, stdout) => { if (err && (err as NodeJS.ErrnoException).code === 'ENOENT') { resolve({ ...base, installed: false, authed: false, ready: false }); return; @@ -220,7 +285,7 @@ function detectOne(spec: AgentSpec): Promise { resolve({ ...base, installed: true, - path: resolvePath(spec.bin), + path: exe !== spec.bin ? exe : undefined, version, authed: auth.authed, authMethod: auth.method, @@ -277,7 +342,9 @@ export function runLocalAgent( const env = childEnv(); return new Promise((resolve) => { // stdin:'ignore' so the agent doesn't stall waiting on piped input (e.g. `claude -p`'s 3s stdin wait). - const child = spawn(spec.bin, args, { env, stdio: ['ignore', 'pipe', 'pipe'] }); + // resolveBin: same PATH-robust resolution the detector uses, so a CLI detected under a minimal + // launchd PATH also SPAWNS (detected-but-unspawnable would be worse than undetected) — issue #78. + const child = spawn(resolveBin(spec.bin), args, { env, stdio: ['ignore', 'pipe', 'pipe'] }); let out = ''; let errOut = ''; let done = false; @@ -340,7 +407,8 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin const cleanup = () => { if (workDir) { try { rmSync(workDir, { recursive: true, force: true }); } catch { /* noop */ } } }; return new Promise((resolve, reject) => { - const child = spawn(spec.bin, args, { env, stdio: [viaStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'] }); + // resolveBin: PATH-robust resolution (issue #78) — see runLocalAgent. + const child = spawn(resolveBin(spec.bin), args, { env, stdio: [viaStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'] }); let out = ''; let errOut = ''; let done = false;