diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 1c781a23..d6d85c5d 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -75,11 +75,17 @@ import { SAFE_PATH_PATTERN, findClaudeDir, getClaudeCliVersion, + getClaudeNotFoundMessage, resolveOpenCodeDir, + getOpenCodeNotFoundMessage, resolveCodexDir, + getCodexNotFoundMessage, resolveGeminiDir, + getGeminiNotFoundMessage, resolveAntigravityDir, + getAntigravityNotFoundMessage, resolvePiDir, + getPiNotFoundMessage, resolveLocalShell, loginShellArgs, } from './utils/index.js'; @@ -1853,29 +1859,28 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { return session; } - // Resolve CLI binary directory based on mode + // Resolve CLI binary directory based on mode. The not-found messages come + // from the resolvers (formatCliNotFoundMessage) so the error names WHERE it + // looked — server PATH, login shell, checked directories — instead of just + // asserting the CLI is missing (the classic systemd/launchd PATH trap). const { pathExport, dir: cliDir } = this.buildPathExport(mode); if (mode === 'claude' && !cliDir) { - throw new Error('Claude CLI not found. Install it with: curl -fsSL https://claude.ai/install.sh | bash'); + throw new Error(getClaudeNotFoundMessage()); } if (mode === 'opencode' && !cliDir) { - throw new Error('OpenCode CLI not found. Install with: curl -fsSL https://opencode.ai/install | bash'); + throw new Error(getOpenCodeNotFoundMessage()); } if (mode === 'codex' && !cliDir) { - throw new Error('Codex CLI not found. Install with: npm install -g @openai/codex'); + throw new Error(getCodexNotFoundMessage()); } if (mode === 'gemini' && !cliDir) { - throw new Error('Gemini CLI not found. Install with: npm install -g @google/gemini-cli'); + throw new Error(getGeminiNotFoundMessage()); } if (mode === 'antigravity' && !cliDir) { - throw new Error( - 'Antigravity CLI not found. Install with: curl -fsSL https://antigravity.google/cli/install.sh | bash' - ); + throw new Error(getAntigravityNotFoundMessage()); } if (mode === 'pi' && !cliDir) { - throw new Error( - 'Pi CLI not found. Install with: npm install -g --ignore-scripts @earendil-works/pi-coding-agent' - ); + throw new Error(getPiNotFoundMessage()); } const envExportsStr = this.buildEnvExports(sessionId, muxName, mode).join(' && '); diff --git a/src/utils/antigravity-cli-resolver.ts b/src/utils/antigravity-cli-resolver.ts index d85adc0e..85d25dd6 100644 --- a/src/utils/antigravity-cli-resolver.ts +++ b/src/utils/antigravity-cli-resolver.ts @@ -28,13 +28,13 @@ const ANTIGRAVITY_SEARCH_DIRS = [ const ANTIGRAVITY_NOT_FOUND = 'Antigravity CLI not found. Install with: curl -fsSL https://antigravity.google/cli/install.sh | bash'; -function createAntigravityResolver(host?: CliResolverHost) { - return createCliExecutableResolver({ binary: 'agy', searchDirs: ANTIGRAVITY_SEARCH_DIRS }, host); +function createAntigravityResolver(host?: CliResolverHost, now?: () => number) { + return createCliExecutableResolver({ binary: 'agy', searchDirs: ANTIGRAVITY_SEARCH_DIRS, now }, host); } -/** Creates an isolated Antigravity wrapper around an injected resolver host. */ -export function createAntigravityResolverForTest(host: CliResolverHost) { - return createAntigravityResolver(host); +/** Creates an isolated Antigravity wrapper around an injected resolver host and clock. */ +export function createAntigravityResolverForTest(host: CliResolverHost, now?: () => number) { + return createAntigravityResolver(host, now); } const antigravityResolver = createAntigravityResolver(); diff --git a/src/utils/claude-cli-resolver.ts b/src/utils/claude-cli-resolver.ts index 27781c34..04a1b15f 100644 --- a/src/utils/claude-cli-resolver.ts +++ b/src/utils/claude-cli-resolver.ts @@ -177,6 +177,9 @@ function probeClaudeCliVersion(): string | null { encoding: 'utf-8', timeout: EXEC_TIMEOUT_MS, env: { ...process.env, PATH: getAugmentedPath() }, + // execFileSync's timeout only SENDS the signal and then keeps waiting; a + // child that ignores SIGTERM would block the server thread permanently. + killSignal: 'SIGKILL', }); const match = out.match(/(\d+\.\d+\.\d+)/); return match ? match[1] : null; diff --git a/src/utils/cli-executable-resolver.ts b/src/utils/cli-executable-resolver.ts index cf1ec7a9..1c670e0e 100644 --- a/src/utils/cli-executable-resolver.ts +++ b/src/utils/cli-executable-resolver.ts @@ -1,3 +1,35 @@ +/** + * @fileoverview Shared CLI executable resolution for the per-CLI resolvers. + * + * One lookup chain behind all six *-cli-resolver modules (claude, opencode, + * codex, gemini, antigravity, pi): the server process PATH first, then the + * CLI's common install directories in order, then — last, because it is the + * only step that spawns anything — an interactive login shell, which is what + * finds nvm/Homebrew/user-npm installs when Codeman runs as a systemd/launchd + * service with a minimal PATH (launchd hands a job `/usr/bin:/bin:/usr/sbin:/sbin`). + * + * Caching is asymmetric, same shape as `resolveClaudeCliVersion` in + * claude-cli-resolver.ts: a successful resolution is cached for the process + * lifetime, a MISS is negative-cached and retried only after a doubling backoff + * (`cliResolveRetryDelayMs`). The callers are request-facing (the per-CLI + * status endpoints in system-routes.ts, the availability gates in + * session-routes.ts, and tmux-manager's spawn path), and the login-shell probe + * is a SYNCHRONOUS spawn bounded by `EXEC_TIMEOUT_MS` — without the negative + * cache, a missing CLI re-ran the whole chain and stalled the event loop for up + * to 5s on every request, forever. + * + * Test hermeticity: under vitest (`process.env.VITEST`) the production host + * short-circuits — IO primitives that were not injected become inert stubs, so + * a suite can never scan the machine's PATH or spawn login shells (the same + * rule as `IS_TEST_MODE` in tmux-manager and the VITEST gate in + * `getClaudeCliVersion`). Tests opt back in through the injection hooks + * (`runCommand`/`isExecutableFile` fakes do no real IO by construction) or, for + * fixtures that need the real filesystem predicate against their own temp + * files, via `allowRealIoUnderVitest`. + * + * @module utils/cli-executable-resolver + */ + import { execFileSync } from 'node:child_process'; import { accessSync, constants, statSync } from 'node:fs'; import { basename, delimiter, dirname, isAbsolute, join } from 'node:path'; @@ -10,6 +42,26 @@ const LOGIN_SHELL_END_MARKER = '__CODEMAN_CLI_RESOLVE_END__'; /** Maximum rendered length of each bounded diagnostic field, excluding its label. */ const DIAGNOSTIC_FIELD_MAX_LENGTH = 1024; +/** First retry window after a full-chain resolution miss. */ +const RESOLVE_RETRY_BASE_MS = 60_000; +/** + * Ceiling for the doubling backoff. Deliberately shorter than the 15min cap on + * the claude version probe: that one is cosmetic, while this gates the Run + * flow, and "installing a CLI while the server is running is picked up without + * a restart" should stay true within minutes. + */ +const RESOLVE_RETRY_MAX_MS = 5 * 60_000; + +/** + * How long to wait before re-running the resolution chain after `failures` + * consecutive misses: 1min, 2min, 4min… capped at 5min. Mirrors + * `claudeVersionRetryDelayMs` in claude-cli-resolver.ts. Exported for tests. + */ +export function cliResolveRetryDelayMs(failures: number): number { + if (failures <= 0) return 0; + return Math.min(RESOLVE_RETRY_BASE_MS * 2 ** (failures - 1), RESOLVE_RETRY_MAX_MS); +} + export type CliResolutionSource = 'process-path' | 'common-directory' | 'login-shell'; export interface CliResolutionDiagnostics { @@ -50,6 +102,7 @@ export interface CliResolverCommandOptions { encoding: 'utf8'; timeout: number; stdio: ['ignore', 'pipe', 'ignore']; + killSignal: 'SIGKILL'; } export type CliResolverCommandRunner = (file: string, args: string[], options: CliResolverCommandOptions) => string; @@ -60,6 +113,13 @@ export interface ProductionCliResolverHostOptions { shellArgs?: string[]; runCommand?: CliResolverCommandRunner; isExecutableFile?: (path: string) => boolean; + /** + * Test-only escape hatch: keep the REAL IO primitives even under vitest. + * For tests that exercise `isExecutableRegularFile` against their own temp + * fixtures. Such a test must still inject `runCommand` if it can reach the + * login-shell step, or it would spawn a real interactive shell. + */ + allowRealIoUnderVitest?: boolean; } function isExecutableRegularFile(path: string): boolean { @@ -97,15 +157,26 @@ export function createProductionCliResolverHost(options: ProductionCliResolverHo const shellPath = options.shellPath ?? resolveLocalShell(); const shellArgs = options.shellArgs ?? loginShellArgs(shellPath).trim().split(/\s+/).filter(Boolean); const processPath = options.processPath ?? process.env.PATH ?? ''; - const isExecutableFile = options.isExecutableFile ?? isExecutableRegularFile; + // Hermeticity gate (see @fileoverview): under vitest, any IO primitive the + // caller did not inject is replaced by an inert stub. The suites must never + // depend on — or execute — whatever happens to be installed on the machine + // running them, and route tests hitting the per-CLI status endpoints would + // otherwise scan the real PATH and spawn real login shells on CI. + const inert = Boolean(process.env.VITEST) && options.allowRealIoUnderVitest !== true; + const isExecutableFile = options.isExecutableFile ?? (inert ? () => false : isExecutableRegularFile); const runCommand: CliResolverCommandRunner = - options.runCommand ?? ((file, args, commandOptions) => execFileSync(file, args, commandOptions)); + options.runCommand ?? (inert ? () => '' : (file, args, commandOptions) => execFileSync(file, args, commandOptions)); const run = (file: string, args: string[]): string => { try { return runCommand(file, args, { encoding: 'utf8', timeout: EXEC_TIMEOUT_MS, stdio: ['ignore', 'pipe', 'ignore'], + // SIGKILL is load-bearing: execFileSync's `timeout` only SENDS the kill + // signal and then keeps waiting for the child to exit. Interactive bash + // ignores SIGTERM (the default), so a login shell stuck in a blocking + // .bash_profile would survive the timeout and block the server forever. + killSignal: 'SIGKILL', }); } catch { return ''; @@ -138,6 +209,8 @@ export function createCliExecutableResolver( binary: string; searchDirs: string[]; validateCandidate?: (path: string) => CandidateValidation; + /** Clock injection for tests driving the failure backoff. Defaults to `Date.now`. */ + now?: () => number; }, host: CliResolverHost = createProductionCliResolverHost() ): CliExecutableResolver { @@ -145,7 +218,13 @@ export function createCliExecutableResolver( throw new Error(`Unsafe CLI binary name: ${options.binary}`); } + const now = options.now ?? Date.now; + /** Successful resolution, cached for the process lifetime. */ let cached: CliResolution | null = null; + /** Consecutive full-chain misses (drives the retry backoff). */ + let failures = 0; + /** Timestamp of the most recent miss. */ + let lastFailureAt = 0; const accept = (path: string | null, source: CliResolutionSource): CliResolution | null => { if (!path || !isAbsolute(path) || !host.exists(path)) return null; const validation = options.validateCandidate?.(path) ?? ({ accepted: true } as CandidateValidation); @@ -161,17 +240,31 @@ export function createCliExecutableResolver( return { resolve() { if (cached) return cached; + // Negative cache: a miss is remembered and the chain — whose login-shell + // tail is a synchronous 5s-bounded spawn — is not re-run until the + // backoff elapses. Without this, every status poll and Run click against + // a missing CLI froze the event loop for the full probe, forever. + if (failures > 0 && now() - lastFailureAt < cliResolveRetryDelayMs(failures)) return null; cached = accept(host.findOnProcessPath(options.binary), 'process-path'); - if (cached) return cached; - - for (const dir of options.searchDirs) { - cached = accept(join(dir, options.binary), 'common-directory'); - if (cached) return cached; + if (!cached) { + for (const dir of options.searchDirs) { + cached = accept(join(dir, options.binary), 'common-directory'); + if (cached) break; + } + } + if (!cached) { + cached = accept(host.findInLoginShell(options.binary), 'login-shell'); } - cached = accept(host.findInLoginShell(options.binary), 'login-shell'); - return cached; + if (cached) { + failures = 0; + lastFailureAt = 0; + return cached; + } + failures += 1; + lastFailureAt = now(); + return null; }, diagnostics: () => ({ binary: options.binary, diff --git a/src/utils/index.ts b/src/utils/index.ts index 4533d901..2df372fb 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -28,12 +28,22 @@ export { stringSimilarity, fuzzyPhraseMatch, todoContentHash } from './string-si export { assertNever } from './type-safety.js'; export { wrapWithNice } from './nice-wrapper.js'; export { resolveLocalShell, loginShellArgs } from './shell-resolver.js'; -export { findClaudeDir, getAugmentedPath, getClaudeCliVersion, getClaudeBinaryPath } from './claude-cli-resolver.js'; +export { + findClaudeDir, + getAugmentedPath, + getClaudeCliVersion, + getClaudeBinaryPath, + getClaudeNotFoundMessage, +} from './claude-cli-resolver.js'; export { spawnPtyWithHelperRepair } from './node-pty-repair.js'; -export { resolveOpenCodeDir } from './opencode-cli-resolver.js'; -export { resolveCodexDir, isCodexAvailable } from './codex-cli-resolver.js'; -export { resolveGeminiDir, isGeminiAvailable } from './gemini-cli-resolver.js'; -export { resolveAntigravityDir, isAntigravityAvailable } from './antigravity-cli-resolver.js'; -export { resolvePiDir, isPiAvailable, getPiCliVersion } from './pi-cli-resolver.js'; +export { resolveOpenCodeDir, getOpenCodeNotFoundMessage } from './opencode-cli-resolver.js'; +export { resolveCodexDir, isCodexAvailable, getCodexNotFoundMessage } from './codex-cli-resolver.js'; +export { resolveGeminiDir, isGeminiAvailable, getGeminiNotFoundMessage } from './gemini-cli-resolver.js'; +export { + resolveAntigravityDir, + isAntigravityAvailable, + getAntigravityNotFoundMessage, +} from './antigravity-cli-resolver.js'; +export { resolvePiDir, isPiAvailable, getPiCliVersion, getPiNotFoundMessage } from './pi-cli-resolver.js'; export { compileFileQuery, matchFileQuery } from './file-query.js'; export type { FileQueryMatcher } from './file-query.js'; diff --git a/src/utils/pi-cli-resolver.ts b/src/utils/pi-cli-resolver.ts index 845faa89..4cfa4011 100644 --- a/src/utils/pi-cli-resolver.ts +++ b/src/utils/pi-cli-resolver.ts @@ -56,13 +56,25 @@ const PI_NOT_FOUND = 'Pi CLI not found. Install with: npm install -g --ignore-sc * 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 — and since `pi` is a short + * GENERIC name, this probe would EXECUTE whatever binary of that name the + * machine carries. The shared resolver host is already inert under vitest, so + * this gate is defense in depth for any opted-in host that still carries the + * default probe; tests drive resolution via `createPiResolverForTest`, whose + * injected probe bypasses it. Pinned by test/pi-cli-resolver.test.ts. */ function probePiVersion(binPath: string): string | null { + if (process.env.VITEST) return null; try { const out = execFileSync(binPath, ['--version'], { encoding: 'utf-8', timeout: EXEC_TIMEOUT_MS, stdio: ['ignore', 'pipe', 'ignore'], + // A stuck or hostile `pi` that ignores SIGTERM would survive the timeout + // and block the server (execFileSync keeps waiting after the signal). + killSignal: 'SIGKILL', }).trim(); // Upstream prints a bare version today; tolerate a `pi 0.84.1` style prefix too. const candidate = PI_VERSION_REGEX.exec(out)?.[1]; @@ -76,7 +88,7 @@ function probePiVersion(binPath: string): string | null { type PiVersionProbe = (binPath: string) => string | null; -function createPiResolver(host?: CliResolverHost, versionProbe: PiVersionProbe = probePiVersion) { +function createPiResolver(host?: CliResolverHost, versionProbe: PiVersionProbe = probePiVersion, now?: () => number) { return createCliExecutableResolver( { binary: 'pi', @@ -85,14 +97,19 @@ function createPiResolver(host?: CliResolverHost, versionProbe: PiVersionProbe = const version = versionProbe(binPath); return version ? { accepted: true, metadata: version } : { accepted: false }; }, + now, }, host ); } -/** Creates an isolated Pi wrapper around an injected host and version probe. */ -export function createPiResolverForTest(host: CliResolverHost, versionProbe: PiVersionProbe) { - return createPiResolver(host, versionProbe); +/** + * Creates an isolated Pi wrapper around an injected host, version probe and + * clock. Omitting `versionProbe` keeps the ambient (VITEST-gated) probe, which + * is exactly what the hermeticity test exercises. + */ +export function createPiResolverForTest(host: CliResolverHost, versionProbe?: PiVersionProbe, now?: () => number) { + return createPiResolver(host, versionProbe ?? probePiVersion, now); } const piResolver = createPiResolver(); diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index f5f838a1..00c08f55 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -802,54 +802,42 @@ export function registerSessionRoutes( } } - // Check OpenCode availability if requested + // Check OpenCode availability if requested. The error text comes from the + // resolver (formatCliNotFoundMessage) so it names where resolution looked — + // server PATH, login shell, common directories — same for the modes below. if (body.mode === 'opencode') { - const { isOpenCodeAvailable } = await import('../../utils/opencode-cli-resolver.js'); + const { isOpenCodeAvailable, getOpenCodeNotFoundMessage } = await import('../../utils/opencode-cli-resolver.js'); if (!isOpenCodeAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'OpenCode CLI not found. Install with: curl -fsSL https://opencode.ai/install | bash' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getOpenCodeNotFoundMessage()); } } // Check Codex availability if requested if (body.mode === 'codex') { - const { isCodexAvailable } = await import('../../utils/codex-cli-resolver.js'); + const { isCodexAvailable, getCodexNotFoundMessage } = await import('../../utils/codex-cli-resolver.js'); if (!isCodexAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Codex CLI not found. Install with: npm install -g @openai/codex' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getCodexNotFoundMessage()); } } // Check Gemini availability if requested if (body.mode === 'gemini') { - const { isGeminiAvailable } = await import('../../utils/gemini-cli-resolver.js'); + const { isGeminiAvailable, getGeminiNotFoundMessage } = await import('../../utils/gemini-cli-resolver.js'); if (!isGeminiAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Gemini CLI not found. Install with: npm install -g @google/gemini-cli' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getGeminiNotFoundMessage()); } } if (body.mode === 'antigravity') { - const { isAntigravityAvailable } = await import('../../utils/antigravity-cli-resolver.js'); + const { isAntigravityAvailable, getAntigravityNotFoundMessage } = + await import('../../utils/antigravity-cli-resolver.js'); if (!isAntigravityAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Antigravity CLI not found. Install with: curl -fsSL https://antigravity.google/cli/install.sh | bash' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getAntigravityNotFoundMessage()); } } if (body.mode === 'pi') { - const { isPiAvailable } = await import('../../utils/pi-cli-resolver.js'); + const { isPiAvailable, getPiNotFoundMessage } = await import('../../utils/pi-cli-resolver.js'); if (!isPiAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Pi CLI not found. Install with: npm install -g --ignore-scripts @earendil-works/pi-coding-agent' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getPiNotFoundMessage()); } } @@ -2827,58 +2815,46 @@ export function registerSessionRoutes( dockerResumeId = dockerCase.lastClaudeSessionId; } } else { - // Check OpenCode availability if requested + // Check OpenCode availability if requested. Error text comes from the + // resolver so it carries the resolution diagnostics; same for the modes below. if (mode === 'opencode') { - const { isOpenCodeAvailable } = await import('../../utils/opencode-cli-resolver.js'); + const { isOpenCodeAvailable, getOpenCodeNotFoundMessage } = + await import('../../utils/opencode-cli-resolver.js'); if (!isOpenCodeAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'OpenCode CLI not found. Install with: curl -fsSL https://opencode.ai/install | bash' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getOpenCodeNotFoundMessage()); } } // Check Codex availability if requested if (mode === 'codex') { - const { isCodexAvailable } = await import('../../utils/codex-cli-resolver.js'); + const { isCodexAvailable, getCodexNotFoundMessage } = await import('../../utils/codex-cli-resolver.js'); if (!isCodexAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Codex CLI not found. Install with: npm install -g @openai/codex' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getCodexNotFoundMessage()); } } // Check Gemini availability if requested if (mode === 'gemini') { - const { isGeminiAvailable } = await import('../../utils/gemini-cli-resolver.js'); + const { isGeminiAvailable, getGeminiNotFoundMessage } = await import('../../utils/gemini-cli-resolver.js'); if (!isGeminiAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Gemini CLI not found. Install with: npm install -g @google/gemini-cli' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getGeminiNotFoundMessage()); } } // Check Antigravity availability if requested if (mode === 'antigravity') { - const { isAntigravityAvailable } = await import('../../utils/antigravity-cli-resolver.js'); + const { isAntigravityAvailable, getAntigravityNotFoundMessage } = + await import('../../utils/antigravity-cli-resolver.js'); if (!isAntigravityAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Antigravity CLI not found. Install with: curl -fsSL https://antigravity.google/cli/install.sh | bash' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getAntigravityNotFoundMessage()); } } // Check Pi availability if requested if (mode === 'pi') { - const { isPiAvailable } = await import('../../utils/pi-cli-resolver.js'); + const { isPiAvailable, getPiNotFoundMessage } = await import('../../utils/pi-cli-resolver.js'); if (!isPiAvailable()) { - return createErrorResponse( - ApiErrorCode.OPERATION_FAILED, - 'Pi CLI not found. Install with: npm install -g --ignore-scripts @earendil-works/pi-coding-agent' - ); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getPiNotFoundMessage()); } } diff --git a/test/antigravity-cli-resolver.test.ts b/test/antigravity-cli-resolver.test.ts index ee9e97dd..482361fd 100644 --- a/test/antigravity-cli-resolver.test.ts +++ b/test/antigravity-cli-resolver.test.ts @@ -5,7 +5,11 @@ import { homedir } from 'node:os'; import { join } from 'node:path'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createAntigravityResolverForTest, isAntigravityAvailable } from '../src/utils/antigravity-cli-resolver.js'; -import type { CliResolution, CliResolverHost } from '../src/utils/cli-executable-resolver.js'; +import { + cliResolveRetryDelayMs, + type CliResolution, + type CliResolverHost, +} from '../src/utils/cli-executable-resolver.js'; const availabilityResolution = vi.hoisted(() => ({ current: null as CliResolution | null })); @@ -84,13 +88,19 @@ describe('Antigravity CLI resolver', () => { expect(resolver.resolve()).toBeNull(); }); - it('retries a failed lookup and caches the first successful login-shell discovery', () => { + it('retries a failed lookup after the backoff and caches the first successful login-shell discovery', () => { const binaryPath = '/late-login-shell/bin/agy'; + let now = 0; const resolver = createAntigravityResolverForTest( - createHost({ loginShellResults: [null, binaryPath], existingPaths: [binaryPath] }) + createHost({ loginShellResults: [null, binaryPath], existingPaths: [binaryPath] }), + () => now ); expect(resolver.resolve()).toBeNull(); + // A miss is negative-cached: within the backoff window nothing re-runs the + // chain (its login-shell tail is a synchronous bounded spawn in production). + expect(resolver.resolve()).toBeNull(); + now = cliResolveRetryDelayMs(1); expect(resolver.resolve()?.binaryPath).toBe(binaryPath); expect(resolver.resolve()?.binaryPath).toBe(binaryPath); }); diff --git a/test/cli-executable-resolver.test.ts b/test/cli-executable-resolver.test.ts index b1fd56cd..36800a8b 100644 --- a/test/cli-executable-resolver.test.ts +++ b/test/cli-executable-resolver.test.ts @@ -1,15 +1,25 @@ -import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { EXEC_TIMEOUT_MS } from '../src/config/exec-timeout.js'; import { + cliResolveRetryDelayMs, createCliExecutableResolver, createProductionCliResolverHost, formatCliNotFoundMessage, type CliResolverHost, } from '../src/utils/cli-executable-resolver.js'; +// Pass-through spy on execFileSync so the vitest-hermeticity test below can +// PROVE the un-injected production host never spawns anything. +const { execFileSyncSpy } = vi.hoisted(() => ({ execFileSyncSpy: vi.fn() })); +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + execFileSyncSpy.mockImplementation(actual.execFileSync as (...args: unknown[]) => unknown); + return { ...actual, execFileSync: execFileSyncSpy }; +}); + const BEGIN_MARKER = '__CODEMAN_CLI_RESOLVE_BEGIN__'; const END_MARKER = '__CODEMAN_CLI_RESOLVE_END__'; @@ -97,17 +107,56 @@ describe('createCliExecutableResolver', () => { }); }); - it('caches success but retries failure', () => { + it('caches success, and retries a miss only after the backoff elapses', () => { + let now = 0; const findInLoginShell = vi.fn<() => string | null>().mockReturnValueOnce(null).mockReturnValue('/new/bin/codex'); const h = host({ findInLoginShell, exists: vi.fn((path) => path === '/new/bin/codex') }); - const resolver = createCliExecutableResolver({ binary: 'codex', searchDirs: [] }, h); + const resolver = createCliExecutableResolver({ binary: 'codex', searchDirs: [], now: () => now }, h); expect(resolver.resolve()).toBeNull(); + // Within the backoff window the miss is answered from the negative cache: + // the chain — whose login-shell tail is a synchronous 5s-bounded spawn — + // must NOT re-run per call, or a missing CLI stalls every status request. + now = cliResolveRetryDelayMs(1) - 1; + expect(resolver.resolve()).toBeNull(); + expect(findInLoginShell).toHaveBeenCalledTimes(1); + + // Once the backoff elapses the retry runs, so installing a CLI while the + // server is up is still picked up without a restart. + now = cliResolveRetryDelayMs(1); expect(resolver.resolve()?.binaryPath).toBe('/new/bin/codex'); expect(resolver.resolve()?.binaryPath).toBe('/new/bin/codex'); expect(findInLoginShell).toHaveBeenCalledTimes(2); }); + it('doubles the retry delay per consecutive miss and caps it at five minutes', () => { + let now = 0; + const findInLoginShell = vi.fn(() => null); + const resolver = createCliExecutableResolver( + { binary: 'codex', searchDirs: [], now: () => now }, + host({ findInLoginShell }) + ); + + expect(cliResolveRetryDelayMs(0)).toBe(0); + expect(cliResolveRetryDelayMs(1)).toBe(60_000); + expect(cliResolveRetryDelayMs(2)).toBe(120_000); + expect(cliResolveRetryDelayMs(3)).toBe(240_000); + expect(cliResolveRetryDelayMs(4)).toBe(300_000); + expect(cliResolveRetryDelayMs(60)).toBe(300_000); + + // Consecutive misses stack: after the second miss the SECOND delay applies. + expect(resolver.resolve()).toBeNull(); + now += cliResolveRetryDelayMs(1); + expect(resolver.resolve()).toBeNull(); + expect(findInLoginShell).toHaveBeenCalledTimes(2); + now += cliResolveRetryDelayMs(2) - 1; + expect(resolver.resolve()).toBeNull(); + expect(findInLoginShell).toHaveBeenCalledTimes(2); + now += 1; + expect(resolver.resolve()).toBeNull(); + expect(findInLoginShell).toHaveBeenCalledTimes(3); + }); + it('rejects unsafe binary names', () => { const h = host(); @@ -120,14 +169,16 @@ describe('createCliExecutableResolver', () => { }); it('rejects relative and nonexistent candidates', () => { + let now = 0; const findInLoginShell = vi .fn<() => string | null>() .mockReturnValueOnce('relative/codex') .mockReturnValue('/missing/codex'); const h = host({ findInLoginShell, exists: vi.fn(() => false) }); - const resolver = createCliExecutableResolver({ binary: 'codex', searchDirs: [] }, h); + const resolver = createCliExecutableResolver({ binary: 'codex', searchDirs: [], now: () => now }, h); expect(resolver.resolve()).toBeNull(); + now = cliResolveRetryDelayMs(1); expect(resolver.resolve()).toBeNull(); expect(h.exists).toHaveBeenCalledTimes(1); expect(h.exists).toHaveBeenCalledWith('/missing/codex'); @@ -189,36 +240,58 @@ describe('formatCliNotFoundMessage', () => { }); describe('createProductionCliResolverHost', () => { - it('contains no ambient VITEST branch in the production resolver source', () => { - const source = readFileSync(new URL('../src/utils/cli-executable-resolver.ts', import.meta.url), 'utf8'); + // Hermeticity gate (the guards PR #329 deleted, restored shared): under + // vitest an un-injected host must neither scan the machine nor spawn a login + // shell — route tests hitting the per-CLI status endpoints would otherwise + // walk the real PATH and execute real binaries on whatever box runs the suite. + it('never scans the machine or spawns a login shell under vitest without injected IO', () => { + const root = mkdtempSync(join(tmpdir(), 'codeman-cli-vitest-gate-')); + temporaryDirectories.push(root); + writeFileSync(join(root, 'codex'), '#!/bin/sh\n'); + chmodSync(join(root, 'codex'), 0o755); + execFileSyncSpy.mockClear(); - expect(source).not.toContain('process.env.VITEST'); + const gatedHost = createProductionCliResolverHost({ + processPath: root, + shellPath: '/bin/bash', + shellArgs: ['-i', '-l'], + }); + + // The real, executable candidate is invisible: the filesystem predicate is inert. + expect(gatedHost.findOnProcessPath('codex')).toBeNull(); + expect(gatedHost.exists(join(root, 'codex'))).toBe(false); + // The login-shell step yields nothing and never reaches execFileSync. + expect(gatedHost.findInLoginShell('codex')).toBeNull(); + expect(execFileSyncSpy).not.toHaveBeenCalled(); + + // The same fixture through the test-only real-IO opt-in IS found, proving + // the nulls above come from the vitest gate rather than from the fixture. + const optedInHost = createProductionCliResolverHost({ + processPath: root, + shellPath: '/bin/bash', + shellArgs: ['-i', '-l'], + runCommand: () => '', + allowRealIoUnderVitest: true, + }); + expect(optedInHost.findOnProcessPath('codex')).toBe(join(root, 'codex')); }); - it("resolves through the injected login-shell runner when VITEST is 'false'", () => { - const hadVitest = Object.hasOwn(process.env, 'VITEST'); - const previousVitest = process.env.VITEST; - process.env.VITEST = 'false'; - try { - const runCommand = vi.fn(() => `${BEGIN_MARKER}\n/home/u/.nvm/bin/codex\n${END_MARKER}`); - const productionHost = createProductionCliResolverHost({ - processPath: '', - shellPath: '/bin/bash', - shellArgs: ['-i', '-l'], - runCommand, - isExecutableFile: () => true, - }); - const resolver = createCliExecutableResolver({ binary: 'codex', searchDirs: [] }, productionHost); + it('resolves through injected IO hooks under vitest (injection is the opt-in)', () => { + const runCommand = vi.fn(() => `${BEGIN_MARKER}\n/home/u/.nvm/bin/codex\n${END_MARKER}`); + const productionHost = createProductionCliResolverHost({ + processPath: '', + shellPath: '/bin/bash', + shellArgs: ['-i', '-l'], + runCommand, + isExecutableFile: () => true, + }); + const resolver = createCliExecutableResolver({ binary: 'codex', searchDirs: [] }, productionHost); - expect(resolver.resolve()).toMatchObject({ - binaryPath: '/home/u/.nvm/bin/codex', - source: 'login-shell', - }); - expect(runCommand).toHaveBeenCalledTimes(1); - } finally { - if (hadVitest) process.env.VITEST = previousVitest; - else delete process.env.VITEST; - } + expect(resolver.resolve()).toMatchObject({ + binaryPath: '/home/u/.nvm/bin/codex', + source: 'login-shell', + }); + expect(runCommand).toHaveBeenCalledTimes(1); }); it('searches the captured process PATH directly in directory order without running a command', () => { @@ -255,6 +328,11 @@ describe('createProductionCliResolverHost', () => { processPath: [directoryCandidate, plainDirectory, executableDirectory].join(':'), shellPath: '/bin/bash', shellArgs: ['-i', '-l'], + // This test exists to exercise the REAL executable-regular-file predicate + // against its own temp fixtures, so it opts out of the vitest inert-IO + // gate; the stubbed runCommand keeps the login-shell path inert anyway. + runCommand: () => '', + allowRealIoUnderVitest: true, }); expect(productionHost.findOnProcessPath('codex')).toBe(join(executableDirectory, 'codex')); @@ -282,6 +360,9 @@ describe('createProductionCliResolverHost', () => { encoding: 'utf8', timeout: EXEC_TIMEOUT_MS, stdio: ['ignore', 'pipe', 'ignore'], + // SIGKILL is load-bearing: interactive bash ignores SIGTERM, and + // execFileSync's timeout only sends the signal, then keeps waiting. + killSignal: 'SIGKILL', } ); }); diff --git a/test/pi-cli-resolver.test.ts b/test/pi-cli-resolver.test.ts new file mode 100644 index 00000000..de5946fd --- /dev/null +++ b/test/pi-cli-resolver.test.ts @@ -0,0 +1,132 @@ +/** + * @fileoverview Tests for the Pi CLI resolver wrapper. + * + * Pi is the resolver with a version probe: `pi` is a short, generic binary + * name, so a resolved path is only accepted once `pi --version` prints a + * semver-shaped string. The probe EXECUTES the candidate, which is exactly why + * it must never run under vitest — the hermeticity test below pins that gate + * with a real executable fixture that would make the test fail loudly if the + * gate were deleted again (as PR #329 once did). + */ +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createPiResolverForTest } from '../src/utils/pi-cli-resolver.js'; +import { + cliResolveRetryDelayMs, + createProductionCliResolverHost, + type CliResolverHost, +} from '../src/utils/cli-executable-resolver.js'; + +const temporaryDirectories: string[] = []; + +afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +function createHost( + options: { + processPathResult?: string | null; + loginShellResults?: Array; + existingPaths?: string[]; + } = {} +): CliResolverHost { + const loginShellResults = [...(options.loginShellResults ?? [])]; + const existingPaths = new Set(options.existingPaths ?? []); + return { + processPath: '/service/bin', + shellPath: '/bin/zsh', + shellArgs: ['-l'], + findOnProcessPath: () => options.processPathResult ?? null, + findInLoginShell: () => loginShellResults.shift() ?? null, + exists: (path) => existingPaths.has(path), + }; +} + +describe('Pi CLI resolver', () => { + it('accepts a candidate the version probe verifies and carries the version as metadata', () => { + const binaryPath = '/service/bin/pi'; + const probe = vi.fn(() => '0.84.1'); + const resolver = createPiResolverForTest( + createHost({ processPathResult: binaryPath, existingPaths: [binaryPath] }), + probe + ); + + expect(resolver.resolve()).toMatchObject({ + binaryPath, + directory: '/service/bin', + source: 'process-path', + metadata: '0.84.1', + }); + expect(probe).toHaveBeenCalledWith(binaryPath); + }); + + it('rejects a candidate the probe refuses and falls through to a later one', () => { + // An unrelated `pi` on the service PATH (probe returns null) must not mask + // the real coding agent found by the login shell. + const impostor = '/service/bin/pi'; + const genuine = '/login-shell/bin/pi'; + const probe = vi.fn((binPath: string) => (binPath === genuine ? '0.84.1' : null)); + const resolver = createPiResolverForTest( + createHost({ + processPathResult: impostor, + loginShellResults: [genuine], + existingPaths: [impostor, genuine], + }), + probe + ); + + expect(resolver.resolve()).toMatchObject({ binaryPath: genuine, source: 'login-shell', metadata: '0.84.1' }); + }); + + it('negative-caches a miss and retries only after the backoff elapses', () => { + const binaryPath = '/late/bin/pi'; + let now = 0; + const probe = vi.fn(() => '0.84.1'); + const resolver = createPiResolverForTest( + createHost({ loginShellResults: [null, binaryPath], existingPaths: [binaryPath] }), + probe, + () => now + ); + + expect(resolver.resolve()).toBeNull(); + expect(resolver.resolve()).toBeNull(); // within the backoff: no re-run + expect(probe).not.toHaveBeenCalled(); + now = cliResolveRetryDelayMs(1); + expect(resolver.resolve()?.metadata).toBe('0.84.1'); + expect(resolver.resolve()?.binaryPath).toBe(binaryPath); + }); + + it('never executes a pi candidate under vitest (the ambient probe is VITEST-gated)', () => { + // A REAL executable fixture that prints a valid version. If the guard in + // probePiVersion is ever removed again, the probe runs this script, the + // resolution SUCCEEDS, and this test fails — pinning hermeticity by + // behavior rather than by source text. (The suites must never execute + // whatever `pi` binary the machine running them happens to carry.) + const root = mkdtempSync(join(tmpdir(), 'codeman-pi-vitest-gate-')); + temporaryDirectories.push(root); + const binaryPath = join(root, 'pi'); + writeFileSync(binaryPath, '#!/bin/sh\necho 0.99.0\n'); + chmodSync(binaryPath, 0o755); + const hostOptions = { + processPath: root, + shellPath: '/bin/bash', + shellArgs: ['-i', '-l'] as string[], + runCommand: () => '', + isExecutableFile: (path: string) => path === binaryPath, + }; + + // Default (ambient) probe: the candidate is found but never executed, so + // the VITEST gate reports it unusable and resolution misses. + const gated = createPiResolverForTest(createProductionCliResolverHost(hostOptions)); + expect(gated.resolve()).toBeNull(); + + // Control: identical setup with an injected probe resolves, proving the + // null above comes from the gate, not from the fixture or the host. + const control = createPiResolverForTest(createProductionCliResolverHost(hostOptions), () => '0.99.0'); + expect(control.resolve()).toMatchObject({ binaryPath, metadata: '0.99.0' }); + }); +});