From 1310dd20b9cf1c16f42be184a0c543f2e1731283 Mon Sep 17 00:00:00 2001 From: mihneanob Date: Tue, 28 Jul 2026 11:56:25 +0300 Subject: [PATCH 1/2] fix(catalog): stop respawning the codex --version probe on every catalog read Loading the bundled Codex catalog spawned `codex --version` on every call, even on a warm 60s cache hit. On Windows that pushed /api/claude-code and /v1/models to 5-8s, past the 3s budget cmdClaude allows, so `ocx claude` printed both the context-window and gateway-cache warnings on almost every launch: [1m] marking was skipped and ~/.claude/cache/gateway-models.json was never refreshed, leaving a stale model picker (mine was 6 days stale). Two independent causes: 1. loadBundledCodexCatalog forwarded its own already-defaulted execFileSync into resolveAndPersistCodexRuntime. resolveCacheKey() deliberately refuses to memoize injected-dependency resolves, so the real-runtime path never hit the memo. Forward an injected execFileSync only. 2. resolveAndPersistCodexRuntime persisted unconditionally on any non-fallback selection. persistCodexRuntime() clears the in-process memo and persistedRuntimeCacheStamp() folds updatedAt into the cache key, so each write both cleared the memo and rekeyed it -- even when the resolved runtime was byte-identical. Only write when the selection actually changed. Also default the catalog path to discoverAlternatives: false. Catalog loading consumes only resolved.runtime.command, never newerAvailable, but full PATH discovery probed 102 candidate launchers (~1.2s) per cold read on this machine. Priority selection is identical either way; callers that want discovery diagnostics still opt in explicitly. Measured on Windows 11, /api/claude-code over HTTP: cold (fresh proxy) 5000-8300ms -> 165ms warm 6300ms -> ~25ms after 65s idle (cache expired) 6325ms -> 1440ms Tests: two regression tests in tests/codex-runtime.test.ts. The first counts real --version invocations across repeated catalog reads and asserts the count stops growing; the second asserts an unchanged selection keeps its persisted updatedAt. Both fail on dev and pass with this change. Verified: bun run typecheck clean, bun run privacy:scan passed, and 144 tests across tests/codex-runtime.test.ts plus the four codex-catalog suites pass. The full bun run test sweep could not complete locally -- Bun 1.3.14 on Windows panics with an internal assertion failure at ~2.9GB RSS, the same runtime memory issue ocx doctor warns about, and it reproduces independently of this change. Please treat CI as authoritative for the full suite. Refs #606 --- src/codex/catalog/bundled.ts | 11 +++- src/codex/runtime.ts | 11 +++- tests/codex-runtime.test.ts | 120 ++++++++++++++++++++++++++++++++++- 3 files changed, 138 insertions(+), 4 deletions(-) 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..572c87784 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; + 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..e8068971d 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,124 @@ 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=$(dirname "$0")`, + `if [ "$1" = "--version" ]; then`, + ` echo probe >> "$d/probes.log"`, + ` echo "codex-cli 0.145.0-alpha.30"`, + " exit 0", + "fi", + `cat "$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(); + 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, + }; + + 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("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"); From 056aa2d6e2ffc2a5328e93e68f890583d0509b15 Mon Sep 17 00:00:00 2001 From: mihneanob Date: Tue, 28 Jul 2026 12:16:18 +0300 Subject: [PATCH 2/2] fix(test): address runtime cache review feedback --- src/codex/runtime.ts | 2 +- tests/codex-runtime.test.ts | 44 +++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/codex/runtime.ts b/src/codex/runtime.ts index 572c87784..6978da569 100644 --- a/src/codex/runtime.ts +++ b/src/codex/runtime.ts @@ -510,7 +510,7 @@ export function resolveAndPersistCodexRuntime( const selectionUnchanged = persistedRuntime !== null && persistedRuntime.command === result.runtime.command && persistedRuntime.source === result.runtime.source - && (persistedRuntime.selectedVersion ?? null) === result.runtime.version; + && (persistedRuntime.selectedVersion ?? null) === (result.runtime.version ?? null); if (result.runtime.command && result.runtime.source !== "fallback" && !selectionUnchanged) { try { persistCodexRuntime(result.runtime, deps); diff --git a/tests/codex-runtime.test.ts b/tests/codex-runtime.test.ts index e8068971d..bf48f1977 100644 --- a/tests/codex-runtime.test.ts +++ b/tests/codex-runtime.test.ts @@ -324,13 +324,15 @@ describe("resolveCodexRuntime", () => { } else { writeFileSync(bin, [ "#!/bin/sh", - `d=$(dirname "$0")`, + "d=${0%/*}", `if [ "$1" = "--version" ]; then`, - ` echo probe >> "$d/probes.log"`, - ` echo "codex-cli 0.145.0-alpha.30"`, + ` printf "%s\\n" probe >> "$d/probes.log"`, + ` printf "%s\\n" "codex-cli 0.145.0-alpha.30"`, " exit 0", "fi", - `cat "$d/catalog.json"`, + `while IFS= read -r line || [ -n "$line" ]; do`, + ` printf "%s\\n" "$line"`, + `done < "$d/catalog.json"`, "", ].join("\n"), "utf8"); chmodSync(bin, 0o755); @@ -384,12 +386,14 @@ describe("resolveCodexRuntime", () => { 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); @@ -405,6 +409,38 @@ describe("resolveCodexRuntime", () => { 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");