diff --git a/src/codex/catalog/bundled.ts b/src/codex/catalog/bundled.ts index 25ae3cdc4..7003099a7 100644 --- a/src/codex/catalog/bundled.ts +++ b/src/codex/catalog/bundled.ts @@ -151,14 +151,21 @@ export function loadBundledCodexCatalog(deps: BundledCatalogDeps = {}): RawCatal let cacheKey: string | null = null; const candidates = deps.commandCandidates?.() ?? (() => { const resolved = resolveAndPersistCodexRuntime({ - execFileSync: execFile, + // Forward an INJECTED execFileSync only. Passing the real one unconditionally made + // resolveCacheKey() bail out (it refuses to memoize injected-dep resolves), so every + // catalog read re-ran the ~1s `codex --version` probe even on a warm cache hit. + ...(deps.execFileSync ? { execFileSync: deps.execFileSync } : {}), configDir: deps.configDir, env: deps.env, platform: deps.platform, existsSync: deps.existsSync, readFileSync: deps.readFileSync, now: deps.now, - discoverAlternatives: deps.discoverAlternatives, + // Catalog loading only consumes `resolved.runtime.command`, never `newerAvailable`. + // Full PATH discovery probes every candidate launcher (100+ on a dev machine, ~1.2s), + // which alone can exceed the 3s budget `ocx claude` allows /api/claude-code. Priority + // selection is identical either way; callers wanting discovery diagnostics opt in. + discoverAlternatives: deps.discoverAlternatives ?? false, }); if (useCache) { cacheKey = [ diff --git a/src/codex/runtime.ts b/src/codex/runtime.ts index 2a2c587c6..6978da569 100644 --- a/src/codex/runtime.ts +++ b/src/codex/runtime.ts @@ -502,7 +502,16 @@ export function resolveAndPersistCodexRuntime( deps: ResolveCodexRuntimeDeps = {}, ): ResolveCodexRuntimeResult { const result = resolveCodexRuntime(deps); - if (result.runtime.command && result.runtime.source !== "fallback") { + // Only WRITE when the selection actually changed. persistCodexRuntime() clears the + // in-process resolve memo and persistedRuntimeCacheStamp() folds `updatedAt` into the + // cache key, so an unconditional rewrite made every caller re-run the ~1s + // `codex --version` probe even when the resolved runtime was byte-identical. + const persistedRuntime = loadPersistedCodexRuntime(deps); + const selectionUnchanged = persistedRuntime !== null + && persistedRuntime.command === result.runtime.command + && persistedRuntime.source === result.runtime.source + && (persistedRuntime.selectedVersion ?? null) === (result.runtime.version ?? null); + if (result.runtime.command && result.runtime.source !== "fallback" && !selectionUnchanged) { try { persistCodexRuntime(result.runtime, deps); } catch (error) { diff --git a/tests/codex-runtime.test.ts b/tests/codex-runtime.test.ts index 4e5b494c7..bf48f1977 100644 --- a/tests/codex-runtime.test.ts +++ b/tests/codex-runtime.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, dirname } from "node:path"; import { @@ -287,6 +287,160 @@ describe("resolveCodexRuntime", () => { expect(failed.persistError!.length).toBeGreaterThan(0); }); + test("repeated catalog reads reuse the runtime probe instead of respawning it", async () => { + const { chmodSync, mkdirSync } = await import("node:fs"); + const { + loadBundledCodexCatalog, + resetBundledCatalogCacheForTests, + } = await import("../src/codex/catalog/bundled"); + + const home = tempConfigDir(); + const binDir = join(home, "bin"); + mkdirSync(binDir, { recursive: true }); + const bin = process.platform === "win32" ? join(binDir, "codex.cmd") : join(binDir, "codex"); + + // Count --version probes by having the launcher append a line per invocation. + const probeLog = join(binDir, "probes.log"); + const catalog = JSON.stringify({ + models: [{ + slug: "gpt-5.5", + base_instructions: "x", + supported_reasoning_levels: [{ effort: "medium", description: "medium" }], + default_reasoning_level: "medium", + }], + }); + writeFileSync(join(binDir, "catalog.json"), `${catalog}\n`, "utf8"); + if (process.platform === "win32") { + writeFileSync(bin, [ + "@echo off", + `if "%~1"=="--version" (`, + ` echo probe>>"%~dp0probes.log"`, + " echo codex-cli 0.145.0-alpha.30", + " exit /b 0", + ")", + `type "%~dp0catalog.json"`, + "", + ].join("\r\n"), "utf8"); + } else { + writeFileSync(bin, [ + "#!/bin/sh", + "d=${0%/*}", + `if [ "$1" = "--version" ]; then`, + ` printf "%s\\n" probe >> "$d/probes.log"`, + ` printf "%s\\n" "codex-cli 0.145.0-alpha.30"`, + " exit 0", + "fi", + `while IFS= read -r line || [ -n "$line" ]; do`, + ` printf "%s\\n" "$line"`, + `done < "$d/catalog.json"`, + "", + ].join("\n"), "utf8"); + chmodSync(bin, 0o755); + } + + const countProbes = (): number => { + if (!existsSync(probeLog)) return 0; + return readFileSync(probeLog, "utf8").split("\n").filter(line => line.trim().length > 0).length; + }; + + const previousHome = process.env.OPENCODEX_HOME; + const previousCli = process.env.CODEX_CLI_PATH; + const previousPath = process.env.PATH; + process.env.OPENCODEX_HOME = home; + process.env.CODEX_CLI_PATH = bin; + process.env.PATH = ""; + resetCodexRuntimeResolveCacheForTests(); + resetBundledCatalogCacheForTests(); + + try { + expect(loadBundledCodexCatalog()?.models?.[0]?.slug).toBe("gpt-5.5"); + expect(countProbes()).toBeGreaterThan(0); + + // The first read has no persisted selection yet, so it legitimately writes one and the + // write clears the resolve memo — the second read re-probes once and then persists + // nothing. From there the count must STOP GROWING. + expect(loadBundledCodexCatalog()?.models?.[0]?.slug).toBe("gpt-5.5"); + const warm = countProbes(); + + // Warm reads must not respawn the probe. Two regressions broke this and made the count + // grow once per read: (1) loadBundledCodexCatalog forwarding its own already-defaulted + // execFileSync, which opts resolveCacheKey() out of memoizing, and (2) an unconditional + // persist whose `updatedAt` both clears the memo and rekeys it. Either one made every + // catalog read spawn `codex --version` (~1s), pushing /api/claude-code past the 3s + // budget ocx claude allows and silently skipping the gateway-model cache refresh. + for (let i = 0; i < 4; i++) { + expect(loadBundledCodexCatalog()?.models?.[0]?.slug).toBe("gpt-5.5"); + } + expect(countProbes()).toBe(warm); + } finally { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (previousCli === undefined) delete process.env.CODEX_CLI_PATH; + else process.env.CODEX_CLI_PATH = previousCli; + if (previousPath === undefined) delete process.env.PATH; + else process.env.PATH = previousPath; + resetCodexRuntimeResolveCacheForTests(); + resetBundledCatalogCacheForTests(); + } + }); + + test("repeat resolveAndPersistCodexRuntime keeps an unchanged selection's persisted stamp", () => { + const configDir = tempConfigDir(); + let now = 0; + const deps = { + configDir, + env: { CODEX_CLI_PATH: "C:\\keep\\codex.exe", PATH: "" }, + platform: "win32" as const, + existsSync: () => true, + execFileSync: (() => "codex-cli 0.145.0-alpha.30") as RuntimeExecFile, + now: () => ++now, + }; + + const first = resolveAndPersistCodexRuntime(deps); + expect(first.persistError).toBeUndefined(); + const firstStamp = loadPersistedCodexRuntime({ configDir })?.updatedAt; + expect(typeof firstStamp).toBe("string"); + + // An identical selection must not rewrite the file: the write clears the resolve memo + // and its `updatedAt` feeds the memo cache key, so rewriting defeats caching entirely. + const second = resolveAndPersistCodexRuntime(deps); + expect(second.runtime.command).toBe(first.runtime.command); + expect(second.runtime.version).toBe(first.runtime.version); + expect(loadPersistedCodexRuntime({ configDir })?.updatedAt).toBe(firstStamp); + }); + + test("treats missing persisted and resolved versions as the same selection", () => { + const configDir = tempConfigDir(); + const statePath = join(configDir, "codex-runtime.json"); + writeFileSync(statePath, JSON.stringify({ + version: 1, + command: "codex", + source: "environment", + updatedAt: "2026-01-01T00:00:00.000Z", + })); + + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = configDir; + resetCodexRuntimeResolveCacheForTests(); + const deps = { env: { PATH: "" }, discoverAlternatives: false }; + + try { + const cached = resolveCodexRuntime(deps); + expect(cached.runtime.source).toBe("fallback"); + cached.runtime.source = "environment"; + (cached.runtime as { version?: string | null }).version = undefined; + const before = readFileSync(statePath, "utf8"); + + resolveAndPersistCodexRuntime(deps); + + expect(readFileSync(statePath, "utf8")).toBe(before); + } finally { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + resetCodexRuntimeResolveCacheForTests(); + } + }); + test("catalog clamp clears diagnostics inside deps.configDir when probe fails", async () => { const { clampCatalogModelsToCodexSupport } = await import("../src/codex/catalog/effort"); const nested = join(tempConfigDir(), "nested", "opencodex-home");