This commit is contained in:
YOMXXX 2026-05-31 01:35:38 +00:00 committed by GitHub
commit 188760151d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 874 additions and 14 deletions

View file

@ -10,7 +10,7 @@ export {
inspectAgentExecutableResolution,
resolveAgentExecutable,
} from './runtimes/executables.js';
export { applyAgentLaunchEnv, resolveAgentLaunch } from './runtimes/launch.js';
export { applyAgentLaunchEnv, resolveAgentLaunch, resolveAgentLaunchWithMinVersion } from './runtimes/launch.js';
export { resolveAgentBin } from './runtimes/resolution.js';
export { spawnEnvForAgent } from './runtimes/env.js';
export { buildLiveArtifactsMcpServersForAgent } from './runtimes/mcp.js';

View file

@ -27,6 +27,7 @@ import {
applyAgentLaunchEnv,
getAgentDef,
resolveAgentLaunch,
resolveAgentLaunchWithMinVersion,
spawnEnvForAgent,
} from './agents.js';
import {
@ -1664,7 +1665,13 @@ async function testAgentConnectionInternal(
validateAgentCliEnv(input.agentCliEnv),
input.agentId,
);
const executableResolution = resolveAgentLaunch(def, configuredAgentEnv);
// Async variant so any def with `minVersion` (currently Gemini,
// #978) warms the version-aware resolver cache before the
// connection test spawn picks a binary — otherwise the test could
// succeed against the version-checked pick while the actual chat
// run still launches the stale first-PATH match (or vice versa),
// making the test result misleading.
const executableResolution = await resolveAgentLaunchWithMinVersion(def, configuredAgentEnv);
const resolvedBin = executableResolution.selectedPath;
if (!resolvedBin || !executableResolution.launchPath) {
return {
@ -2189,6 +2196,11 @@ export async function testAgentConnection(
const configuredCodexBin = validatedPrefs?.codex?.CODEX_BIN?.trim() || '';
const configuredAgentEnv = agentCliEnvForAgent(validatedPrefs, input.agentId);
const def = getAgentDef(input.agentId);
// testAgentConnectionInternal above already warmed the chooser
// cache for this agent via resolveAgentLaunchWithMinVersion, so
// this synchronous resolveAgentLaunch reads the same
// version-checked pick that the spawn just used (the diagnostics
// summary must match what actually ran).
const executableResolution = def
? resolveAgentLaunch(def, configuredAgentEnv)
: {

View file

@ -66,6 +66,7 @@ import {
applyAgentLaunchEnv,
getAgentDef,
resolveAgentLaunch,
resolveAgentLaunchWithMinVersion,
spawnEnvForAgent,
} from './agents.js';
import { agentCliEnvForAgent, readAppConfig } from './app-config.js';
@ -811,7 +812,13 @@ async function callLocalCli(provider, system, user, options) {
configuredAgentEnv = {};
}
const launch = resolveAgentLaunch(def, configuredAgentEnv);
// Async variant so any def with `minVersion` (currently Gemini,
// #978) warms the version-aware resolver cache before the
// memory-llm spawn picks a binary. Without this an auto-memory
// extract on a fresh daemon (no /api/agents call yet) could pick
// up a stale `/usr/local/bin/gemini` even when a modern Homebrew
// install sits later on PATH.
const launch = await resolveAgentLaunchWithMinVersion(def, configuredAgentEnv);
if (!launch?.launchPath) {
throw new Error(`${def.name} CLI is not installed or not on PATH`);
}

View file

@ -35,4 +35,14 @@ export const geminiAgentDef = {
promptViaStdin: true,
streamFormat: 'json-event-stream',
eventParser: 'gemini',
// The `--output-format stream-json` flag did not exist in early
// Gemini CLI builds; an old `npm i -g @google/gemini-cli` left in
// `/usr/local/bin` shadows the modern Homebrew/nvm install on
// macOS GUI launches and the spawn fails with yargs
// `Unknown arguments: output-format, outputFormat` (#978). Pin a
// version floor so the resolver picks the first candidate on
// PATH+toolchain that actually supports the flag we send.
// 0.30.0 is the cited baseline from #978 where the flag shipped
// in stable form.
minVersion: '0.30.0',
} satisfies RuntimeAgentDef;

View file

@ -1,3 +1,4 @@
import { chooseExecutableByMinVersion } from './executables.js';
import { execAgentFile } from './invocation.js';
import { AGENT_DEFS } from './registry.js';
import { DEFAULT_MODEL_OPTION, rememberLiveModels } from './models.js';
@ -131,6 +132,20 @@ async function probe(
def: RuntimeAgentDef,
configuredEnv: Record<string, string> = {},
): Promise<DetectedAgent> {
// For agents that declare a minimum CLI version, warm the
// version-aware resolver cache first so the version-checked binary
// is cached before `resolveAgentLaunch` reads it (#978). Without
// this pre-step, `inspectAgentExecutableResolution` would still
// return first-found and we would probe a stale binary even though
// a modern one sits later on the PATH/toolchain. The two-step
// shape (await chooser, then sync resolveAgentLaunch) — instead of
// the `resolveAgentLaunchWithMinVersion` helper that the spawn
// paths use — keeps the existing `detection-resilience` mock
// intact, which exercises module-level `resolveAgentLaunch` to
// simulate a single-agent throw.
if (def.minVersion) {
await chooseExecutableByMinVersion(def, configuredEnv);
}
// Detection must probe the exact path the runtime will spawn, not just the
// PATH-visible shim. This is load-bearing for Codex under nvm/fnm/mise:
// the discovered `codex` entry is often a `#!/usr/bin/env node` wrapper

View file

@ -5,6 +5,7 @@ import { homedir } from 'node:os';
import { fileURLToPath } from 'node:url';
import { wellKnownUserToolchainBins } from '@open-design/platform';
import { resolveSandboxRuntimeConfigFromEnv } from '../sandbox-mode.js';
import { execAgentFile } from './invocation.js';
import { expandHomePath } from './paths.js';
import type { RuntimeAgentDef } from './types.js';
@ -105,6 +106,31 @@ export function resolveOnPath(bin: string): string | null {
return null;
}
// Same search shape as `resolveOnPath`, but returns *every* directory
// the bin resolves in (PATH order, then toolchain dirs). Used by the
// version-aware chooser to evaluate every candidate rather than stop
// at the first match (#978).
export function enumerateOnPath(bin: string): string[] {
const exts =
process.platform === 'win32'
? (process.env.PATHEXT || '.EXE;.CMD;.BAT').split(';')
: [''];
const dirs = resolvePathDirs();
const found: string[] = [];
const seen = new Set<string>();
for (const dir of dirs) {
for (const ext of exts) {
const full = path.join(dir, bin + ext);
if (!full || seen.has(full)) continue;
if (existsSync(full)) {
seen.add(full);
found.push(full);
}
}
}
return found;
}
function looksExecutableOnWindows(filePath: string): boolean {
const ext = path.extname(filePath).trim().toUpperCase();
if (!ext) return false;
@ -253,16 +279,29 @@ export function inspectAgentExecutableResolution(
};
}
const configuredOverridePath = configuredExecutableOverride(def, configuredEnv);
const candidates = [
def.bin,
...(Array.isArray(def.fallbackBins) ? def.fallbackBins : []),
];
// Version-aware cache lookup runs only when there is no explicit
// override and the def opts in via `minVersion`. The cache is
// populated by `chooseExecutableByMinVersion` during detection so
// chat-time spawn sees the same binary detection picked instead of
// falling back to first-match (#978).
let pathResolvedPath: string | null = null;
for (const bin of candidates) {
const resolved = resolveOnPath(bin);
if (resolved) {
pathResolvedPath = resolved;
break;
if (!configuredOverridePath && def.minVersion) {
const cached = versionAwareCache.get(def.id);
if (cached && existsSync(cached)) {
pathResolvedPath = cached;
}
}
if (!pathResolvedPath) {
const candidates = [
def.bin,
...(Array.isArray(def.fallbackBins) ? def.fallbackBins : []),
];
for (const bin of candidates) {
const resolved = resolveOnPath(bin);
if (resolved) {
pathResolvedPath = resolved;
break;
}
}
}
const builtInPath = packagedBuiltInExecutable(def, configuredEnv);
@ -272,3 +311,170 @@ export function inspectAgentExecutableResolution(
selectedPath: configuredOverridePath || builtInPath || pathResolvedPath,
};
}
// ---- Version-aware resolution (#978) -------------------------------------
//
// Gemini-style "stale binary shadows the modern one" problem: macOS GUI
// launches inherit `/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin`, and an
// ancient `/usr/local/bin/gemini=0.1.12` left by an old `npm i -g
// @google/gemini-cli` shadows the modern Homebrew/nvm install. The old
// binary lacks `--output-format`, so the daemon spawn lands on yargs
// `Unknown arguments`. The fix runs `--version` against every candidate
// and picks the first that meets the floor pinned on the def.
const VERSION_PROBE_TIMEOUT_MS = 1_500;
// agent.id → resolved path that passed the version gate. Populated by
// `chooseExecutableByMinVersion`; consulted by
// `inspectAgentExecutableResolution` so the sync chat-spawn path sees
// the same pick detection landed on. Only writes for the auto-pick path
// — an explicit `<AGENT>_BIN` override is intentionally NOT cached so
// clearing the env reliably falls back to auto-pick (#1007 round-2 P2).
const versionAwareCache = new Map<string, string>();
export function clearVersionAwareResolutionCache(agentId?: string): void {
if (agentId === undefined) {
versionAwareCache.clear();
} else {
versionAwareCache.delete(agentId);
}
}
// Strict, anchored semver parse. Accepts a leading `v` and tolerates
// trailing pre-release (`-rc.1`) / build metadata (`+build.5`) but
// only major.minor.patch participates in comparison. Returns `null`
// for unparseable input so the chooser explicitly rejects the
// candidate instead of letting it pass the gate (#1007 round-1 P1).
const SEMVER_RE = /^v?(\d+)\.(\d+)\.(\d+)(?:[-+][0-9A-Za-z.-]+)?$/;
export function compareSemver(a: string, b: string): number | null {
const left = parseSemver(a);
const right = parseSemver(b);
if (!left || !right) return null;
const [la, lb, lc] = left;
const [ra, rb, rc] = right;
if (la !== ra) return la - ra;
if (lb !== rb) return lb - rb;
if (lc !== rc) return lc - rc;
return 0;
}
function parseSemver(value: string): [number, number, number] | null {
if (typeof value !== 'string') return null;
const m = SEMVER_RE.exec(value.trim());
if (!m) return null;
return [Number(m[1]), Number(m[2]), Number(m[3])];
}
export interface ChooseExecutableByMinVersionOptions {
// Test seam: inject a fake `runVersion(path)` so unit tests do not
// need real binaries that print versions. Production passes the
// default that spawns `<path> <def.versionArgs>` with a timeout.
runVersion?: (resolvedPath: string) => Promise<string>;
}
export async function chooseExecutableByMinVersion(
def: RuntimeAgentDef,
configuredEnv: Record<string, string> = {},
options: ChooseExecutableByMinVersionOptions = {},
): Promise<string | null> {
if (!def?.bin || !def.minVersion) return resolveAgentExecutable(def, configuredEnv);
// Explicit user override always wins; do not probe and do not pollute
// the cache (a later run with the override cleared must rediscover
// a fresh auto-pick).
const override = configuredExecutableOverride(def, configuredEnv);
if (override) return override;
// Cached fast path: reuse the resolution from a prior call without
// re-probing every PATH candidate. `probeVersionWithTimeout` has a
// ~1.5 s timeout per candidate and the enumeration below waits on
// `Promise.all`, so a stale `.cmd` shim or a hung wrapper would add
// that delay to every chat-run / connection-test / memory-extract
// launch instead of only the first cold lookup (#2797 mrcfps review
// on `launch.ts:30`). The cache is cleared on negative resolution
// (line ~390 below) and via `clearVersionAwareResolutionCache()`, so
// a missing or relocated cached pick still gets re-probed at the
// next launch.
const cached = versionAwareCache.get(def.id);
if (cached && existsSync(cached)) {
return cached;
}
// Enumerate every match for def.bin and (if declared) any fallback
// bins, in the same order resolveOnPath would walk.
const candidates: string[] = [];
const seen = new Set<string>();
const bins = [def.bin, ...(Array.isArray(def.fallbackBins) ? def.fallbackBins : [])];
for (const bin of bins) {
for (const hit of enumerateOnPath(bin)) {
if (seen.has(hit)) continue;
seen.add(hit);
candidates.push(hit);
}
}
if (candidates.length === 0) {
versionAwareCache.delete(def.id);
return null;
}
const runVersion = options.runVersion ?? ((p) => probeVersionWithTimeout(p, def));
const probes = await Promise.all(
candidates.map(async (p) => {
try {
const out = await runVersion(p);
return { path: p, version: typeof out === 'string' ? out.trim().split('\n')[0] ?? '' : '' };
} catch {
return { path: p, version: '' };
}
}),
);
for (const probe of probes) {
const cmp = compareSemver(probe.version, def.minVersion);
if (cmp !== null && cmp >= 0) {
versionAwareCache.set(def.id, probe.path);
return probe.path;
}
}
// Regression-safe fallback: keep the previous behavior (first-found)
// so the existing "agent exited with code 1" surface still fires
// when nothing meets the floor, instead of the agent silently
// disappearing from the picker. Drop any stale cache entry so a
// later install of a modern binary is not occluded.
versionAwareCache.delete(def.id);
return candidates[0] ?? null;
}
async function probeVersionWithTimeout(
resolvedPath: string,
def: RuntimeAgentDef,
): Promise<string> {
// Mirror the core of `applyAgentLaunchEnv`: prepend the daemon's
// own Node binary directory and the candidate's directory to PATH so
// Node-wrapper CLIs (`#!/usr/bin/env node`, npm `.cmd` shims on
// Windows) can resolve `node` even when the daemon was GUI-launched
// with a stripped PATH. We do NOT import `applyAgentLaunchEnv`
// directly: launch.ts depends on executables.ts and the import
// cycle would break.
const env: NodeJS.ProcessEnv = { ...process.env, ...(def.env || {}) };
const pathKey = Object.keys(env).find((k) => k.toLowerCase() === 'path') ?? 'PATH';
const existing = typeof env[pathKey] === 'string' ? (env[pathKey] as string) : '';
const prepend = [path.dirname(process.execPath), path.dirname(resolvedPath)].filter(Boolean);
const merged = [...prepend, ...existing.split(delimiter).filter(Boolean)];
const seen = new Set<string>();
const deduped: string[] = [];
for (const dir of merged) {
if (!seen.has(dir)) {
seen.add(dir);
deduped.push(dir);
}
}
env[pathKey] = deduped.join(delimiter);
const { stdout } = await execAgentFile(resolvedPath, def.versionArgs, {
env,
timeout: VERSION_PROBE_TIMEOUT_MS,
});
return String(stdout);
}

View file

@ -1,6 +1,6 @@
import { accessSync, constants, readdirSync, readFileSync, realpathSync, statSync } from 'node:fs';
import path, { delimiter } from 'node:path';
import { inspectAgentExecutableResolution } from './executables.js';
import { chooseExecutableByMinVersion, inspectAgentExecutableResolution } from './executables.js';
import type { RuntimeAgentDef } from './types.js';
export type AgentLaunchKind = 'selected' | 'codex-native';
@ -12,6 +12,26 @@ export type AgentLaunchResolution = ReturnType<typeof inspectAgentExecutableReso
diagnostic: string | null;
};
// Async wrapper around `resolveAgentLaunch` that first warms the
// version-aware resolver cache for any def that opts in via
// `minVersion` (#978). Spawn paths must use this — `resolveAgentLaunch`
// itself stays sync to keep its existing callers, but on its own it
// reads the cache the chooser populates, which means a chat-spawn or
// connection-test launched before detection ran would still land on
// the stale first-PATH match. Calling this helper guarantees the
// cache is populated before the launch decision happens, regardless
// of whether `/api/agents` has been hit first (#978 round-2 review
// on PR #2797).
export async function resolveAgentLaunchWithMinVersion(
def: RuntimeAgentDef,
configuredEnv: Record<string, string> = {},
): Promise<AgentLaunchResolution> {
if (def.minVersion) {
await chooseExecutableByMinVersion(def, configuredEnv);
}
return resolveAgentLaunch(def, configuredEnv);
}
export function resolveAgentLaunch(
def: RuntimeAgentDef,
configuredEnv: Record<string, string> = {},

View file

@ -162,6 +162,7 @@ export type RuntimeAgentDef = {
// present in the daemon's `process.env`; Settings-UI per-agent env
// values only reach the spawned child and are NOT consulted here.
defaultModelEnvVar?: string;
minVersion?: string;
};
export type DetectedAgent = Omit<

View file

@ -41,6 +41,7 @@ import {
isKnownModel,
applyAgentLaunchEnv,
resolveAgentLaunch,
resolveAgentLaunchWithMinVersion,
sanitizeCustomModel,
spawnEnvForAgent,
} from './agents.js';
@ -11286,7 +11287,12 @@ export async function startServer({
configuredAgentEnv = {};
}
const agentLaunch = resolveAgentLaunch(def, configuredAgentEnv);
// Use the async variant so any def with `minVersion` set
// (currently Gemini, #978) warms the version-aware resolver cache
// before this spawn picks a binary. Without this the chat-run
// launch could land on the stale first-PATH match even when
// detection had not run yet.
const agentLaunch = await resolveAgentLaunchWithMinVersion(def, configuredAgentEnv);
const resolvedBin = agentLaunch.selectedPath;
// Hoisted above the AMR catalog preflight: the empty-catalog branch

View file

@ -0,0 +1,583 @@
// Version-aware executable resolution for Gemini (#978).
//
// macOS GUI launchers strip PATH to `/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin`,
// then `userToolchainDirs()` appends Homebrew / nvm / npm-global / mise paths
// at the end. The previous `resolveOnPath()` was first-match, so a stale
// `/usr/local/bin/gemini=0.1.12` left over from an old global npm install
// would shadow the modern `/opt/homebrew/bin/gemini=0.40.1+` and the daemon
// spawn landed on yargs `Unknown arguments: output-format, outputFormat`
// (PR #1007 design, reviewer-approved).
//
// The fix is opt-in per agent via `RuntimeAgentDef.minVersion`. When set,
// the resolver enumerates every candidate path, probes `--version` on each
// in parallel, and returns the first whose version meets `minVersion`. Other
// agents (claude / codex / opencode / ...) skip the version walk and keep
// first-match resolution unchanged.
import { afterEach, describe, expect, it } from 'vitest';
import {
chmodSync,
mkdirSync,
mkdtempSync,
rmSync,
writeFileSync,
} from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { delimiter } from 'node:path';
import {
chooseExecutableByMinVersion,
clearVersionAwareResolutionCache,
compareSemver,
enumerateOnPath,
inspectAgentExecutableResolution,
resolveAgentExecutable,
} from '../../src/runtimes/executables.js';
import { resolveAgentLaunchWithMinVersion } from '../../src/runtimes/launch.js';
import { gemini, minimalAgentDef } from './helpers/test-helpers.js';
const fsTest = process.platform === 'win32' ? it.skip : it;
afterEach(() => {
clearVersionAwareResolutionCache();
});
describe('compareSemver (#978 reviewer round 1: unparseable input must not pass through)', () => {
it('returns 0 for equal versions', () => {
expect(compareSemver('1.2.3', '1.2.3')).toBe(0);
});
it('returns positive when a > b on major / minor / patch', () => {
expect(compareSemver('2.0.0', '1.99.99')).toBeGreaterThan(0);
expect(compareSemver('1.3.0', '1.2.99')).toBeGreaterThan(0);
expect(compareSemver('1.2.4', '1.2.3')).toBeGreaterThan(0);
});
it('returns negative when a < b', () => {
expect(compareSemver('0.1.12', '0.30.0')).toBeLessThan(0);
expect(compareSemver('0.29.9', '0.30.0')).toBeLessThan(0);
});
it('tolerates a leading "v" and trailing pre-release / metadata', () => {
// Real-world Gemini CLI prints like "0.40.1", but other CLIs print
// "v1.2.3", "1.2.3-rc.1", or "1.2.3+build.5". Major.minor.patch is
// the only thing the version gate cares about.
expect(compareSemver('v0.40.1', '0.30.0')).toBeGreaterThan(0);
expect(compareSemver('0.40.1-rc.1', '0.30.0')).toBeGreaterThan(0);
expect(compareSemver('0.40.1+build.5', '0.30.0')).toBeGreaterThan(0);
});
it('returns null for unparseable input — prose like CLI help text must not be silently accepted as ≥ minVersion', () => {
// The previous draft of compareSemver let unparseable input return 0,
// which meant a stale binary that printed `Usage: gemini ...` on
// --version would silently pass the version gate. The reviewer
// P1 fix forces unparseable inputs to null so the chooser explicitly
// rejects them.
expect(compareSemver('not-a-version', '0.30.0')).toBeNull();
expect(compareSemver('Usage: gemini [options]', '0.30.0')).toBeNull();
expect(compareSemver('', '0.30.0')).toBeNull();
expect(compareSemver('0.30.0', 'minVersion-broken')).toBeNull();
});
it('is anchored — refuses prose-wrapped versions to prevent a partial regex match', () => {
// "Help: see version 1.2.3 for details" must not parse as 1.2.3.
expect(compareSemver('Help: see version 1.2.3 for details', '0.30.0')).toBeNull();
});
});
describe('enumerateOnPath (#978: list every match across PATH + toolchain, not just the first)', () => {
fsTest('returns one entry per PATH directory that contains the binary, in PATH order', () => {
const a = mkdtempSync(join(tmpdir(), 'od-enum-a-'));
const b = mkdtempSync(join(tmpdir(), 'od-enum-b-'));
try {
writeFileSync(join(a, 'gemini'), '#!/bin/sh\necho 0.1.12\n');
writeFileSync(join(b, 'gemini'), '#!/bin/sh\necho 0.40.1\n');
chmodSync(join(a, 'gemini'), 0o755);
chmodSync(join(b, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = a; // suppresses toolchain leakage
process.env.PATH = `${a}${delimiter}${b}`;
const found = enumerateOnPath('gemini');
expect(found).toEqual([join(a, 'gemini'), join(b, 'gemini')]);
} finally {
rmSync(a, { recursive: true, force: true });
rmSync(b, { recursive: true, force: true });
}
});
fsTest('returns an empty array when the binary is not on PATH', () => {
const dir = mkdtempSync(join(tmpdir(), 'od-enum-empty-'));
try {
process.env.OD_AGENT_HOME = dir;
process.env.PATH = dir;
expect(enumerateOnPath('gemini')).toEqual([]);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
fsTest('deduplicates identical paths that appear twice in PATH', () => {
const dir = mkdtempSync(join(tmpdir(), 'od-enum-dedup-'));
try {
writeFileSync(join(dir, 'gemini'), '#!/bin/sh\necho 0.40.1\n');
chmodSync(join(dir, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = dir;
process.env.PATH = `${dir}${delimiter}${dir}`;
expect(enumerateOnPath('gemini')).toEqual([join(dir, 'gemini')]);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});
describe('chooseExecutableByMinVersion (#978: skip stale binaries that fail the floor)', () => {
// The chooser is pure on top of an injectable `runVersion` — we pass a
// fake probe so the tests do not depend on a real `gemini` install.
fsTest('picks the second-on-PATH when the first is too old (the headline #978 scenario)', async () => {
const usrLocal = mkdtempSync(join(tmpdir(), 'od-mv-usrlocal-'));
const homebrew = mkdtempSync(join(tmpdir(), 'od-mv-homebrew-'));
try {
writeFileSync(join(usrLocal, 'gemini'), '');
writeFileSync(join(homebrew, 'gemini'), '');
chmodSync(join(usrLocal, 'gemini'), 0o755);
chmodSync(join(homebrew, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = usrLocal;
process.env.PATH = `${usrLocal}${delimiter}${homebrew}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const versions = new Map<string, string>([
[join(usrLocal, 'gemini'), '0.1.12'],
[join(homebrew, 'gemini'), '0.40.1'],
]);
const chosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async (p) => versions.get(p) ?? '',
});
expect(chosen).toBe(join(homebrew, 'gemini'));
} finally {
rmSync(usrLocal, { recursive: true, force: true });
rmSync(homebrew, { recursive: true, force: true });
}
});
fsTest('returns the first candidate when no candidate meets minVersion (regression-safe fallback)', async () => {
// When every installed gemini is too old, fall through to the
// first-found path. The existing "agent exited with code 1" error
// surfaces — same as before — instead of the chooser silently
// hiding the agent.
const a = mkdtempSync(join(tmpdir(), 'od-mv-allold-a-'));
const b = mkdtempSync(join(tmpdir(), 'od-mv-allold-b-'));
try {
writeFileSync(join(a, 'gemini'), '');
writeFileSync(join(b, 'gemini'), '');
chmodSync(join(a, 'gemini'), 0o755);
chmodSync(join(b, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = a;
process.env.PATH = `${a}${delimiter}${b}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const chosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async () => '0.1.12',
});
expect(chosen).toBe(join(a, 'gemini'));
} finally {
rmSync(a, { recursive: true, force: true });
rmSync(b, { recursive: true, force: true });
}
});
fsTest('returns null when the binary is on no PATH directory at all', async () => {
const dir = mkdtempSync(join(tmpdir(), 'od-mv-none-'));
try {
process.env.OD_AGENT_HOME = dir;
process.env.PATH = dir;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const chosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async () => '0.40.1',
});
expect(chosen).toBeNull();
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
fsTest('rejects unparseable version output and continues probing the next candidate', async () => {
// A stale binary that prints help text to `--version` (typical of
// pre-1.0 CLIs) used to silently pass when compareSemver returned 0
// for unparseable input. Now it does not parse, the chooser treats
// the candidate as failed, and proceeds to the next one.
const broken = mkdtempSync(join(tmpdir(), 'od-mv-broken-'));
const ok = mkdtempSync(join(tmpdir(), 'od-mv-ok-'));
try {
writeFileSync(join(broken, 'gemini'), '');
writeFileSync(join(ok, 'gemini'), '');
chmodSync(join(broken, 'gemini'), 0o755);
chmodSync(join(ok, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = broken;
process.env.PATH = `${broken}${delimiter}${ok}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const versions = new Map<string, string>([
[join(broken, 'gemini'), 'Usage: gemini [options]'],
[join(ok, 'gemini'), '0.40.1'],
]);
const chosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async (p) => versions.get(p) ?? '',
});
expect(chosen).toBe(join(ok, 'gemini'));
} finally {
rmSync(broken, { recursive: true, force: true });
rmSync(ok, { recursive: true, force: true });
}
});
fsTest('honors the configured `<AGENT>_BIN` override without running any version probes', async () => {
// Explicit env override means the user has opted out of auto-pick.
// The chooser must return the override verbatim and must not run
// --version on anything (probing the override would be both
// pointless and a noisy startup-time side effect).
const override = mkdtempSync(join(tmpdir(), 'od-mv-override-'));
const onPath = mkdtempSync(join(tmpdir(), 'od-mv-onpath-'));
try {
writeFileSync(join(override, 'gemini'), '');
writeFileSync(join(onPath, 'gemini'), '');
chmodSync(join(override, 'gemini'), 0o755);
chmodSync(join(onPath, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = onPath;
process.env.PATH = onPath;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
let probeCalls = 0;
const chosen = await chooseExecutableByMinVersion(
def,
{ GEMINI_BIN: join(override, 'gemini') },
{
runVersion: async () => {
probeCalls++;
return '0.40.1';
},
},
);
expect(chosen).toBe(join(override, 'gemini'));
expect(probeCalls).toBe(0);
} finally {
rmSync(override, { recursive: true, force: true });
rmSync(onPath, { recursive: true, force: true });
}
});
fsTest('falls through to first-found when a version probe throws (e.g. binary times out or segfaults)', async () => {
// We do not want a single misbehaving binary to break detection
// for every Gemini install on the box. A throw should be treated
// the same as "could not parse" — try the next candidate; if
// nothing meets the floor, regression-safe fallback fires.
const flaky = mkdtempSync(join(tmpdir(), 'od-mv-flaky-'));
const ok = mkdtempSync(join(tmpdir(), 'od-mv-flaky-ok-'));
try {
writeFileSync(join(flaky, 'gemini'), '');
writeFileSync(join(ok, 'gemini'), '');
chmodSync(join(flaky, 'gemini'), 0o755);
chmodSync(join(ok, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = flaky;
process.env.PATH = `${flaky}${delimiter}${ok}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const chosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async (p) => {
if (p === join(flaky, 'gemini')) throw new Error('boom');
return '0.40.1';
},
});
expect(chosen).toBe(join(ok, 'gemini'));
} finally {
rmSync(flaky, { recursive: true, force: true });
rmSync(ok, { recursive: true, force: true });
}
});
fsTest('second call reuses the cached pick without re-probing every PATH candidate (mrcfps review on PR #2797)', async () => {
// Background: chooseExecutableByMinVersion enumerates every PATH
// candidate and runs `--version` on each in parallel. Without a
// cached fast path, a stale `.cmd` shim that hangs to the probe
// timeout (1.5 s) would add that delay to every chat-run /
// connection-test / memory-extract launch instead of only the
// first cold lookup. The fix consults the version-aware cache
// before re-probing, and the regression below proves a second
// call uses zero `runVersion` invocations.
const flaky = mkdtempSync(join(tmpdir(), 'od-cache-flaky-'));
const ok = mkdtempSync(join(tmpdir(), 'od-cache-ok-'));
try {
writeFileSync(join(flaky, 'gemini'), '');
writeFileSync(join(ok, 'gemini'), '');
chmodSync(join(flaky, 'gemini'), 0o755);
chmodSync(join(ok, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = flaky;
process.env.PATH = `${flaky}${delimiter}${ok}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
let probes = 0;
const runVersion = async (p: string) => {
probes += 1;
if (p === join(flaky, 'gemini')) return '0.1.12';
return '0.40.1';
};
// First call: cold cache — probes every candidate (>= 2 here).
const firstChosen = await chooseExecutableByMinVersion(def, {}, { runVersion });
expect(firstChosen).toBe(join(ok, 'gemini'));
const probesAfterCold = probes;
expect(probesAfterCold).toBeGreaterThanOrEqual(2);
// Second call: warm cache — must not invoke runVersion at all.
const secondChosen = await chooseExecutableByMinVersion(def, {}, { runVersion });
expect(secondChosen).toBe(join(ok, 'gemini'));
expect(probes).toBe(probesAfterCold);
} finally {
rmSync(flaky, { recursive: true, force: true });
rmSync(ok, { recursive: true, force: true });
}
});
fsTest('cached pick that no longer exists on disk is re-probed instead of returned stale', async () => {
// existsSync gate on the cached value: if the user removed or
// moved the binary between launches, the cache must NOT serve a
// dangling path. Re-probe and re-cache instead.
const oldDir = mkdtempSync(join(tmpdir(), 'od-cache-old-'));
const newDir = mkdtempSync(join(tmpdir(), 'od-cache-new-'));
try {
writeFileSync(join(oldDir, 'gemini'), '');
chmodSync(join(oldDir, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = oldDir;
process.env.PATH = oldDir;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const firstChosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async () => '0.40.1',
});
expect(firstChosen).toBe(join(oldDir, 'gemini'));
// User moves the binary: cached path no longer exists.
rmSync(join(oldDir, 'gemini'));
writeFileSync(join(newDir, 'gemini'), '');
chmodSync(join(newDir, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = newDir;
process.env.PATH = newDir;
const secondChosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async () => '0.40.1',
});
expect(secondChosen).toBe(join(newDir, 'gemini'));
} finally {
rmSync(oldDir, { recursive: true, force: true });
rmSync(newDir, { recursive: true, force: true });
}
});
});
describe('inspectAgentExecutableResolution + minVersion cache wiring (#978)', () => {
// The async chooser populates a module-scoped cache; the sync resolver
// used at chat-spawn time consults that cache so detection-time and
// spawn-time always land on the same binary.
fsTest('after a successful async chooser populates the cache, the sync resolver returns the version-checked pick', async () => {
const usrLocal = mkdtempSync(join(tmpdir(), 'od-cache-usrlocal-'));
const homebrew = mkdtempSync(join(tmpdir(), 'od-cache-homebrew-'));
try {
writeFileSync(join(usrLocal, 'gemini'), '');
writeFileSync(join(homebrew, 'gemini'), '');
chmodSync(join(usrLocal, 'gemini'), 0o755);
chmodSync(join(homebrew, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = usrLocal;
process.env.PATH = `${usrLocal}${delimiter}${homebrew}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const versions = new Map<string, string>([
[join(usrLocal, 'gemini'), '0.1.12'],
[join(homebrew, 'gemini'), '0.40.1'],
]);
const chosen = await chooseExecutableByMinVersion(def, {}, {
runVersion: async (p) => versions.get(p) ?? '',
});
expect(chosen).toBe(join(homebrew, 'gemini'));
// Without the cache, this sync call would still return usrLocal
// (first-found). With it, it returns the version-checked pick.
expect(resolveAgentExecutable(def)).toBe(join(homebrew, 'gemini'));
expect(inspectAgentExecutableResolution(def).selectedPath).toBe(join(homebrew, 'gemini'));
} finally {
rmSync(usrLocal, { recursive: true, force: true });
rmSync(homebrew, { recursive: true, force: true });
}
});
fsTest('configured override beats the cache so the user escape hatch always wins', async () => {
const cached = mkdtempSync(join(tmpdir(), 'od-cache-cached-'));
const override = mkdtempSync(join(tmpdir(), 'od-cache-override-'));
try {
writeFileSync(join(cached, 'gemini'), '');
writeFileSync(join(override, 'gemini'), '');
chmodSync(join(cached, 'gemini'), 0o755);
chmodSync(join(override, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = cached;
process.env.PATH = cached;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
await chooseExecutableByMinVersion(def, {}, {
runVersion: async () => '0.40.1',
});
// Cache is populated for the no-override case.
expect(resolveAgentExecutable(def)).toBe(join(cached, 'gemini'));
// Now the user pins GEMINI_BIN. The configured override must win
// regardless of cache contents.
expect(resolveAgentExecutable(def, { GEMINI_BIN: join(override, 'gemini') }))
.toBe(join(override, 'gemini'));
} finally {
rmSync(cached, { recursive: true, force: true });
rmSync(override, { recursive: true, force: true });
}
});
fsTest('clearing GEMINI_BIN after a previous override-with-cache run still falls back to the auto-pick (override does not pollute cache, #1007 round-2 P2)', async () => {
// Reviewer P2 fix on PR #1007: if `chooseExecutableByMinVersion`
// were to write the override path into the cache, clearing the env
// would leave the daemon pinned to a stale binary even though the
// user has opted back into auto-pick. The override path must skip
// the cache write entirely.
const usrLocal = mkdtempSync(join(tmpdir(), 'od-cache-clear-usr-'));
const homebrew = mkdtempSync(join(tmpdir(), 'od-cache-clear-brew-'));
const override = mkdtempSync(join(tmpdir(), 'od-cache-clear-override-'));
try {
writeFileSync(join(usrLocal, 'gemini'), '');
writeFileSync(join(homebrew, 'gemini'), '');
writeFileSync(join(override, 'gemini'), '');
chmodSync(join(usrLocal, 'gemini'), 0o755);
chmodSync(join(homebrew, 'gemini'), 0o755);
chmodSync(join(override, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = usrLocal;
process.env.PATH = `${usrLocal}${delimiter}${homebrew}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const versions = new Map<string, string>([
[join(usrLocal, 'gemini'), '0.1.12'],
[join(homebrew, 'gemini'), '0.40.1'],
]);
// First run with the override active — must not pollute the cache.
await chooseExecutableByMinVersion(
def,
{ GEMINI_BIN: join(override, 'gemini') },
{ runVersion: async (p) => versions.get(p) ?? '' },
);
// Next call with no override: cache lookup must MISS for the
// override path and instead trigger a fresh auto-pick.
const after = await chooseExecutableByMinVersion(def, {}, {
runVersion: async (p) => versions.get(p) ?? '',
});
expect(after).toBe(join(homebrew, 'gemini'));
} finally {
rmSync(usrLocal, { recursive: true, force: true });
rmSync(homebrew, { recursive: true, force: true });
rmSync(override, { recursive: true, force: true });
}
});
});
describe('resolveAgentLaunchWithMinVersion (#978 round-2 review on PR #2797: every spawn path must warm the cache, not just /api/agents)', () => {
// The fix in PR #2797 round 1 only warmed the cache inside
// `probe()` (the detection path that feeds /api/agents). Actual
// spawn paths in server.ts, connectionTest.ts, and memory-llm.ts
// called the sync `resolveAgentLaunch` directly, so a chat-run /
// connection-test / memory-extract on a fresh daemon (no
// /api/agents call yet) still landed on the stale first-PATH
// match. The async wrapper guarantees the cache is populated
// before the launch decision happens, no matter which path
// triggered the spawn.
fsTest('on a cold cache, returns the version-checked binary instead of the stale first-found one', async () => {
// Use real fixture binaries so the helper's default `runVersion`
// actually spawns them. Two shell scripts print fixed semvers;
// we trust the daemon's own Node binary is on PATH to resolve
// `#!/bin/sh` (which is always the case on macOS/Linux).
const usrLocal = mkdtempSync(join(tmpdir(), 'od-launch-usr-'));
const homebrew = mkdtempSync(join(tmpdir(), 'od-launch-brew-'));
try {
writeFileSync(join(usrLocal, 'gemini'), '#!/bin/sh\necho 0.1.12\n');
writeFileSync(join(homebrew, 'gemini'), '#!/bin/sh\necho 0.40.1\n');
chmodSync(join(usrLocal, 'gemini'), 0o755);
chmodSync(join(homebrew, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = usrLocal;
process.env.PATH = `${usrLocal}${delimiter}${homebrew}`;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const launch = await resolveAgentLaunchWithMinVersion(def);
expect(launch.selectedPath).toBe(join(homebrew, 'gemini'));
} finally {
rmSync(usrLocal, { recursive: true, force: true });
rmSync(homebrew, { recursive: true, force: true });
}
});
fsTest('skips the chooser when def.minVersion is undefined (no behavior change for other agents)', async () => {
// Pin the no-op shape: agents without `minVersion` (claude, codex,
// opencode, ...) must keep first-match resolution. The wrapper
// never calls the chooser for them, so even a stale binary on PATH
// wins (mirroring the pre-#978 status quo for those agents).
const a = mkdtempSync(join(tmpdir(), 'od-launch-noop-a-'));
const b = mkdtempSync(join(tmpdir(), 'od-launch-noop-b-'));
try {
writeFileSync(join(a, 'opencode'), '');
writeFileSync(join(b, 'opencode'), '');
chmodSync(join(a, 'opencode'), 0o755);
chmodSync(join(b, 'opencode'), 0o755);
process.env.OD_AGENT_HOME = a;
process.env.PATH = `${a}${delimiter}${b}`;
// No minVersion on this def.
const def = minimalAgentDef({ id: 'opencode-test', bin: 'opencode' });
const launch = await resolveAgentLaunchWithMinVersion(def);
expect(launch.selectedPath).toBe(join(a, 'opencode'));
} finally {
rmSync(a, { recursive: true, force: true });
rmSync(b, { recursive: true, force: true });
}
});
fsTest('honors a configured `<AGENT>_BIN` override without probing', async () => {
// Reuses the override-skips-probes invariant the chooser already
// owns; assert it at the wrapper level so spawn callers can rely
// on it without knowing the internal layering.
const override = mkdtempSync(join(tmpdir(), 'od-launch-override-'));
const onPath = mkdtempSync(join(tmpdir(), 'od-launch-onpath-'));
try {
writeFileSync(join(override, 'gemini'), '');
writeFileSync(join(onPath, 'gemini'), '#!/bin/sh\necho 0.40.1\n');
chmodSync(join(override, 'gemini'), 0o755);
chmodSync(join(onPath, 'gemini'), 0o755);
process.env.OD_AGENT_HOME = onPath;
process.env.PATH = onPath;
const def = minimalAgentDef({ id: 'gemini', bin: 'gemini', minVersion: '0.30.0' });
const launch = await resolveAgentLaunchWithMinVersion(def, {
GEMINI_BIN: join(override, 'gemini'),
});
expect(launch.selectedPath).toBe(join(override, 'gemini'));
} finally {
rmSync(override, { recursive: true, force: true });
rmSync(onPath, { recursive: true, force: true });
}
});
});
describe('Gemini def carries the minVersion floor (#978)', () => {
it('ships with minVersion set to the first stable --output-format release', () => {
// Hard-pin the value so a future tweak that drops or changes the
// floor is intentional, not a refactor accident.
expect((gemini as { minVersion?: string }).minVersion).toBe('0.30.0');
});
});