mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 14:39:42 +02:00
fix(cli-resolvers): find CLIs installed via nvm/Homebrew when running as a service
A CLI installed by nvm, Homebrew or a user-level npm prefix lives on a PATH that only a login shell sets up. Codeman running under systemd or launchd does not get that PATH — launchd hands a job `/usr/bin:/bin:/usr/sbin:/sbin` — so every resolver reported the CLI as unavailable on installs where it is plainly there and works from a terminal. Each of the six resolvers had its own hand-rolled copy of the same PATH walk, so the fix is factored into one shared `createCliExecutableResolver()` with an explicit lookup order: the server process PATH, then common install directories in order, then an interactive login shell as the last resort. Only the last step spawns anything, and only when the cheap lookups have already missed. Also adds `formatCliNotFoundMessage()`, so a failure explains where it looked instead of just asserting the CLI is missing. Its diagnostics are bounded and control characters are flattened, so a not-found message cannot dump arbitrary environment data. Success is cached and failure is retried, so installing a CLI while the server is running is picked up without a restart. Net -103 lines across the six resolvers. Behaviour is unchanged wherever the CLI was already on the process PATH: that remains the first thing checked. Tests: 20 cases in test/cli-executable-resolver.test.ts covering the precedence order, login-shell-only resolution, the caching rule, unsafe-name rejection, and the bounded diagnostics.
This commit is contained in:
@@ -15,11 +15,15 @@
|
||||
* @module utils/pi-cli-resolver
|
||||
*/
|
||||
|
||||
import { execFileSync, execSync } from 'node:child_process';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { join } from 'node:path';
|
||||
import { homedir } from 'node:os';
|
||||
import { EXEC_TIMEOUT_MS } from '../config/exec-timeout.js';
|
||||
import {
|
||||
createCliExecutableResolver,
|
||||
formatCliNotFoundMessage,
|
||||
type CliResolverHost,
|
||||
} from './cli-executable-resolver.js';
|
||||
|
||||
/** Common directories where the Pi CLI binary may be installed */
|
||||
const PI_SEARCH_DIRS = [
|
||||
@@ -45,22 +49,15 @@ const PI_SEARCH_DIRS = [
|
||||
*/
|
||||
export const PI_VERSION_REGEX = /(?:^|\s)(\d+\.\d+\.\d+)/;
|
||||
|
||||
/** Cached directory containing the pi binary (empty string = searched but not found) */
|
||||
let _piDir: string | null = null;
|
||||
/** Cached version string reported by the resolved binary (empty string = probed, unusable) */
|
||||
let _piVersion: string | null = null;
|
||||
const PI_NOT_FOUND = 'Pi CLI not found. Install with: npm install -g --ignore-scripts @earendil-works/pi-coding-agent';
|
||||
|
||||
/**
|
||||
* Run `pi --version` on a candidate path and return the trimmed version when it
|
||||
* looks like the coding agent. Returns null for anything else — a missing
|
||||
* binary, a non-zero exit, a hang (timeout), or output that is not semver-shaped
|
||||
* (which is how an unrelated `pi` on PATH gets rejected).
|
||||
*
|
||||
* Never runs under vitest: the suites must stay hermetic and must not depend on
|
||||
* whether the dev box happens to have pi installed.
|
||||
*/
|
||||
function probePiVersion(binPath: string): string | null {
|
||||
if (process.env.VITEST) return null;
|
||||
try {
|
||||
const out = execFileSync(binPath, ['--version'], {
|
||||
encoding: 'utf-8',
|
||||
@@ -77,6 +74,29 @@ function probePiVersion(binPath: string): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
type PiVersionProbe = (binPath: string) => string | null;
|
||||
|
||||
function createPiResolver(host?: CliResolverHost, versionProbe: PiVersionProbe = probePiVersion) {
|
||||
return createCliExecutableResolver<string>(
|
||||
{
|
||||
binary: 'pi',
|
||||
searchDirs: PI_SEARCH_DIRS,
|
||||
validateCandidate: (binPath) => {
|
||||
const version = versionProbe(binPath);
|
||||
return version ? { accepted: true, metadata: version } : { accepted: false };
|
||||
},
|
||||
},
|
||||
host
|
||||
);
|
||||
}
|
||||
|
||||
/** Creates an isolated Pi wrapper around an injected host and version probe. */
|
||||
export function createPiResolverForTest(host: CliResolverHost, versionProbe: PiVersionProbe) {
|
||||
return createPiResolver(host, versionProbe);
|
||||
}
|
||||
|
||||
const piResolver = createPiResolver();
|
||||
|
||||
/**
|
||||
* Finds the directory containing a verified `pi` binary.
|
||||
* Checks `which pi` first, then falls back to common install locations. Every
|
||||
@@ -86,46 +106,7 @@ function probePiVersion(binPath: string): string | null {
|
||||
* @returns Directory path, or null if not found
|
||||
*/
|
||||
export function resolvePiDir(): string | null {
|
||||
if (_piDir !== null) return _piDir || null;
|
||||
|
||||
const accept = (binPath: string): string | null => {
|
||||
// Under vitest the probe never runs, so existence alone decides (keeps the
|
||||
// suites hermetic and matches how the sibling resolvers behave there).
|
||||
if (process.env.VITEST) {
|
||||
_piDir = dirname(binPath);
|
||||
_piVersion = '';
|
||||
return _piDir;
|
||||
}
|
||||
const version = probePiVersion(binPath);
|
||||
if (!version) return null;
|
||||
_piDir = dirname(binPath);
|
||||
_piVersion = version;
|
||||
return _piDir;
|
||||
};
|
||||
|
||||
try {
|
||||
const result = execSync('which pi', {
|
||||
encoding: 'utf-8',
|
||||
timeout: EXEC_TIMEOUT_MS,
|
||||
}).trim();
|
||||
if (result && existsSync(result)) {
|
||||
const dir = accept(result);
|
||||
if (dir) return dir;
|
||||
}
|
||||
} catch {
|
||||
// pi not in PATH, will check common locations
|
||||
}
|
||||
|
||||
for (const dir of PI_SEARCH_DIRS) {
|
||||
const binPath = join(dir, 'pi');
|
||||
if (!existsSync(binPath)) continue;
|
||||
const accepted = accept(binPath);
|
||||
if (accepted) return accepted;
|
||||
}
|
||||
|
||||
_piDir = '';
|
||||
_piVersion = '';
|
||||
return null;
|
||||
return piResolver.resolve()?.directory ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -135,12 +116,14 @@ export function isPiAvailable(): boolean {
|
||||
return resolvePiDir() !== null;
|
||||
}
|
||||
|
||||
export function getPiNotFoundMessage(): string {
|
||||
return formatCliNotFoundMessage(PI_NOT_FOUND, piResolver.diagnostics());
|
||||
}
|
||||
|
||||
/**
|
||||
* Version reported by the resolved `pi` binary, or null when pi is unavailable
|
||||
* (or when the probe was skipped, i.e. under vitest). Surfaced through
|
||||
* `GET /api/pi/status` so a misresolution is diagnosable from the UI.
|
||||
* Version reported by the resolved `pi` binary, or null when pi is unavailable.
|
||||
* Surfaced through `GET /api/pi/status` so a misresolution is diagnosable from the UI.
|
||||
*/
|
||||
export function getPiCliVersion(): string | null {
|
||||
resolvePiDir();
|
||||
return _piVersion || null;
|
||||
return piResolver.resolve()?.metadata ?? null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user