From b4af509890fd05bff02cdb0a13f3ecdeff6dd0d4 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 14:24:21 +0900 Subject: [PATCH 1/3] fix(codex): discover Codex App runtimes and defer the prompt probe's version check The prompt probe carried a private four-path POSIX resolver. The Windows Codex App installs codex.exe under %LOCALAPPDATA%\OpenAI\Codex\bin\, which that list can never match, so the probe reported an absent candidate on machines where Codex was plainly installed (issue 4458). - runtime.ts gains an "installed" source that enumerates the Windows Codex App bin root newest-first (deterministic name tie-break) and keeps the four POSIX paths the probe used to hardcode. It ranks after PATH, so PATH stays authoritative. - deps.probeVersion === false selects a spawnable candidate without running `codex --version`. A request-path probe needs something it can spawn, not a version, and ~1s of blocking exec per candidate is what made the probe give up. - That deferred selection gets its own memo and never publishes into runtime authority, because peekCodexRuntimeProcessCache feeds convergence and the bundled catalog, which would read a null version as "unknown version". - ENOENT from the version probe is now program-not-found rather than a generic --version failure, so a missing program stops looking like a broken one. - prompt-text-probe.ts asks the shared resolver, spawns through codexExecInvocation so a Windows .cmd launches correctly, and reports a stable failure kind. Process stderr is read only to classify and never returned. Co-authored-by: Clive Rosfield <64878945+S0RYUASUKA@users.noreply.github.com> --- src/codex/prompt-text-probe.ts | 295 +++++++++++++++--- src/codex/runtime.ts | 149 ++++++++- .../codex-prompt-text-probe.test.ts | 75 +++++ tests/codex-integration/codex-runtime.test.ts | 160 ++++++++++ 4 files changed, 633 insertions(+), 46 deletions(-) diff --git a/src/codex/prompt-text-probe.ts b/src/codex/prompt-text-probe.ts index d0926a061e..246abc509a 100644 --- a/src/codex/prompt-text-probe.ts +++ b/src/codex/prompt-text-probe.ts @@ -19,8 +19,14 @@ import { spawn } from "node:child_process"; import { existsSync, statSync } from "node:fs"; import { join } from "node:path"; -import { homedir } from "node:os"; +import { codexExecInvocation } from "./exec-invocation"; import { resolveCodexHomeDir } from "./home"; +import { + CODEX_PROGRAM_NOT_FOUND_REASON, + resolveCodexRuntime, + type CodexRuntimeSource, + type ResolveCodexRuntimeResult, +} from "./runtime"; /** * Layer id -> the tag Codex actually renders it under. @@ -79,27 +85,49 @@ export interface LayerText { sourcePath?: string; } +/** + * Why a probe failed, as a stable token the caller can branch on. The prose + * `detail` string is kept for display, but matching on it was never a contract: + * a caller that needs "is Codex installed at all" versus "Codex rejected the + * command" cannot get that from a sentence. + */ +export type PromptProbeFailureKind = + | "program-not-found" + | "command-unsupported" + | "execution-failed" + | "output-invalid"; + +export interface PromptProbeFailure { + kind: PromptProbeFailureKind; + /** The command line that was attempted or resolved, for display. */ + command: string; + /** + * A short fixed phrase plus the command - never captured process output. + * Codex stderr can carry the user's config path, model name, or environment + * details, and this response is served over the management API, so raw + * process output does not belong in it. + */ + detail: string; +} + export interface PromptTextProbe { ok: boolean; /** The Codex home the probe reported on. */ codexHome: string; layers: Record; + /** The runtime the probe resolved and tried, when resolution produced one. */ + runtime?: { command: string; source: CodexRuntimeSource }; + /** Stable failure classification; `detail` remains the display string. */ + failure?: PromptProbeFailure; detail?: string; } -function resolveCodexBinary(): string | null { - const candidates = [ - join(homedir(), ".codex/packages/standalone/current/bin/codex"), - join(homedir(), ".local/bin/codex"), - "/usr/local/bin/codex", - "/opt/homebrew/bin/codex", - ]; - return candidates.find(path => existsSync(path)) ?? null; -} - /** 8 MiB is far above any real prompt and far below anything that hurts the server. */ const MAX_PROBE_OUTPUT_BYTES = 8 * 1024 * 1024; +/** stderr is captured only to classify the failure, never to echo back. */ +const MAX_PROBE_STDERR_BYTES = 64 * 1024; + interface ProbeCommand { binary: string; args: string[]; @@ -111,7 +139,7 @@ interface ProbeCommand { interface PromptProbeFlight { key: string; controller: AbortController; - result: Promise; + result: Promise; closed: Promise; waiters: number; joinable: boolean; @@ -120,17 +148,27 @@ interface PromptProbeFlight { } interface PromptProbeExecution { - result: Promise; + result: Promise; closed: Promise; } +/** + * The process outcome travels with its classification so a shared flight hands + * every joined caller the same `failure`, not just the same null. + */ +interface PromptProbeExecutionResult { + raw: string | null; + failure: PromptProbeFailure | null; +} + type SharedPromptProbeOutcome = | { kind: "output"; raw: string } - | { kind: "failed" } + | { kind: "failed"; failure?: PromptProbeFailure } | { kind: "busy" }; let activePromptProbe: PromptProbeFlight | null = null; let probeCommandForTests: { binary: string; args: string[] } | null = null; +let probeRuntimeForTests: { command: string; source: CodexRuntimeSource } | null | undefined; let probeSpawnAttemptsForTests = 0; let probeCloseBarrierForTests: Promise | null = null; @@ -144,24 +182,73 @@ function commandKey(command: ProbeCommand): string { ]); } -function completedExecution(value: string | null): PromptProbeExecution { +function completedExecution(value: PromptProbeExecutionResult | null): PromptProbeExecution { return { result: Promise.resolve(value), closed: Promise.resolve() }; } +function commandDescription(command: ProbeCommand): string { + return [command.binary, ...command.args].join(" "); +} + +function probeFailure( + command: ProbeCommand, + kind: PromptProbeFailureKind, + detail: string, +): PromptProbeFailure { + return { kind, command: commandDescription(command), detail }; +} + +/** + * A non-zero exit whose stderr reports an unknown subcommand means the resolved + * binary is a Codex too old (or too new) for `debug prompt-input` - a different + * remedy than "the process died". The stderr text itself is used only for this + * check; it never enters the response. + */ +function classifyProcessFailure(command: ProbeCommand, code: number | null, stderr: string): PromptProbeFailure { + const lower = stderr.toLowerCase(); + const unsupported = + (/unrecognized|unknown|unexpected|invalid/.test(lower) && /subcommand|command|argument|option/.test(lower)) + || /usage:/.test(lower); + const kind: PromptProbeFailureKind = unsupported ? "command-unsupported" : "execution-failed"; + const phrase = unsupported + ? "codex does not support this probe command" + : `codex probe exited with code ${code ?? "unknown"}`; + return probeFailure(command, kind, `${phrase}: ${commandDescription(command)}`); +} + +/** + * The resolver reports why each candidate lost. A candidate that is simply not + * there (issue 4458's repeated "path does not exist" on Windows) is a + * program-not-found; a candidate that exists but could not be probed is an + * execution problem on an installed program. + */ +function classifyRuntimeFailure(result: ResolveCodexRuntimeResult): PromptProbeFailure { + const isNotFound = (reason: string) => + reason === CODEX_PROGRAM_NOT_FOUND_REASON || /does not exist|not found|ENOENT/i.test(reason); + const representative = result.failures.find(item => !isNotFound(item.reason)) + ?? result.failures[0]; + const kind: PromptProbeFailureKind = !representative || isNotFound(representative.reason) + ? "program-not-found" + : "execution-failed"; + const command = representative?.command ?? result.runtime.command; + const phrase = kind === "program-not-found" ? "codex program not found" : "codex runtime could not be probed"; + return { kind, command, detail: `${phrase}: ${command}` }; +} + function runProbe( command: ProbeCommand, signal: AbortSignal, onStopping: () => void, ): PromptProbeExecution { - if (signal.aborted) return completedExecution(null); - let resolveResult!: (value: string | null) => void; + if (signal.aborted) return completedExecution({ raw: null, failure: null }); + let resolveResult!: (value: PromptProbeExecutionResult | null) => void; let resolveClosed!: () => void; - const result = new Promise(resolve => { resolveResult = resolve; }); + const result = new Promise(resolve => { resolveResult = resolve; }); const closed = new Promise(resolve => { resolveClosed = resolve; }); let resultSettled = false; let closeSettled = false; - const finishResult = (value: string | null) => { + const finishResult = (value: PromptProbeExecutionResult | null) => { if (resultSettled) return; resultSettled = true; resolveResult(value); @@ -179,22 +266,39 @@ function runProbe( let child: ReturnType; try { if (probeCommandForTests) probeSpawnAttemptsForTests += 1; - child = spawn(command.binary, command.args, { + // Route through the shared invocation helper: on Windows a resolved + // `codex.cmd` cannot be spawned directly and must go through cmd.exe, + // which `commandInvocation` does with correct metacharacter escaping. + const invocation = codexExecInvocation(command.binary, command.args, process.platform); + child = spawn(invocation.file, invocation.args, { cwd: command.cwd, - stdio: ["ignore", "pipe", "ignore"], + stdio: ["ignore", "pipe", "pipe"], + windowsHide: true, + ...invocation.options, + }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + finishResult({ + raw: null, + failure: probeFailure( + command, + /ENOENT|not found/i.test(message) ? "program-not-found" : "execution-failed", + `codex probe could not start: ${commandDescription(command)}`, + ), }); - } catch { - finishResult(null); finishClosed(); return { result, closed }; } const chunks: Buffer[] = []; + const errorChunks: Buffer[] = []; let size = 0; + let errorSize = 0; let settled = false; let stopping = false; + let stoppingFailure: PromptProbeFailure | null = null; let timer: ReturnType | undefined; - const finish = (value: string | null) => { + const finish = (value: PromptProbeExecutionResult) => { if (settled) return; settled = true; if (timer) clearTimeout(timer); @@ -205,9 +309,16 @@ function runProbe( // Keep the flight admitted until `close`: kill() only requests termination // and does not prove the exact child has released its process and stdio. - const terminate = () => { + const terminate = ( + failure = probeFailure( + command, + "execution-failed", + `codex probe was terminated: ${commandDescription(command)}`, + ), + ) => { if (settled || stopping) return; stopping = true; + stoppingFailure = failure; onStopping(); if (timer) clearTimeout(timer); signal.removeEventListener("abort", onAbort); @@ -215,7 +326,7 @@ function runProbe( // The caller is bounded even if OS termination later fails. Admission is // retained separately by `closed`, and later probes fail soft while this // exact child remains unproven terminal. - finishResult(null); + finishResult({ raw: null, failure }); if (child.exitCode !== null || child.signalCode !== null) return; try { child.kill("SIGKILL"); @@ -233,18 +344,50 @@ function runProbe( if (size > MAX_PROBE_OUTPUT_BYTES) { terminate(); return; } chunks.push(chunk); }); - child.on("error", () => { + child.stderr?.on("data", (chunk: Buffer) => { + errorSize += chunk.length; + if (errorSize <= MAX_PROBE_STDERR_BYTES) errorChunks.push(chunk); + }); + child.on("error", error => { // No PID means spawn itself failed, so there is no live child to drain. if (child.pid === undefined) { - finish(null); + const message = error instanceof Error ? error.message : String(error); + finish({ + raw: null, + failure: probeFailure( + command, + /ENOENT|not found/i.test(message) ? "program-not-found" : "execution-failed", + `codex probe could not start: ${commandDescription(command)}`, + ), + }); } - else terminate(); + else terminate(probeFailure( + command, + "execution-failed", + `codex probe process error: ${commandDescription(command)}`, + )); }); child.on("close", code => { // Decode once, at the end: `String(chunk)` per chunk corrupts any UTF-8 // character that straddles a chunk boundary. const recordClose = () => { - finish(!stopping && code === 0 ? Buffer.concat(chunks).toString("utf8") : null); + if (stopping) { + finish({ + raw: null, + failure: stoppingFailure ?? probeFailure( + command, + "execution-failed", + `codex probe was terminated: ${commandDescription(command)}`, + ), + }); + } else if (code === 0) { + finish({ raw: Buffer.concat(chunks).toString("utf8"), failure: null }); + } else { + finish({ + raw: null, + failure: classifyProcessFailure(command, code, Buffer.concat(errorChunks).toString("utf8")), + }); + } }; const barrier = probeCloseBarrierForTests; if (barrier) void barrier.then(recordClose, recordClose); @@ -253,7 +396,14 @@ function runProbe( // Close the race between the pre-spawn check and listener registration. if (signal.aborted) terminate(); } catch { - finishResult(null); + finishResult({ + raw: null, + failure: probeFailure( + command, + "execution-failed", + `codex probe failed: ${commandDescription(command)}`, + ), + }); finishClosed(); } return { result, closed }; @@ -296,19 +446,26 @@ async function runSharedPromptProbe( if (signal?.aborted) return { kind: "failed" }; const active = activePromptProbe; if (!active) { - const raw = await waitForPromptProbeFlight(startPromptProbeFlight(command), signal); - return raw === null ? { kind: "failed" } : { kind: "output", raw }; + const result = await waitForPromptProbeFlight(startPromptProbeFlight(command), signal); + if (!result) return { kind: "failed" }; + if (result.failure) return { kind: "failed", failure: result.failure }; + return result.raw === null ? { kind: "failed" } : { kind: "output", raw: result.raw }; } if (active.key === key && active.joinable && !active.controller.signal.aborted) { - const raw = await waitForPromptProbeFlight(active, signal); - return raw === null ? { kind: "failed" } : { kind: "output", raw }; + const result = await waitForPromptProbeFlight(active, signal); + if (!result) return { kind: "failed" }; + if (result.failure) return { kind: "failed", failure: result.failure }; + return result.raw === null ? { kind: "failed" } : { kind: "output", raw: result.raw }; } // A different or terminating flight still owns the sole process slot. Never // wait unboundedly for an unproven close and never launch beside it. return { kind: "busy" }; } -async function waitForPromptProbeFlight(flight: PromptProbeFlight, signal?: AbortSignal): Promise { +async function waitForPromptProbeFlight( + flight: PromptProbeFlight, + signal?: AbortSignal, +): Promise { if (signal?.aborted) { if (flight.waiters === 0 && !flight.settled) flight.controller.abort(); return null; @@ -317,7 +474,7 @@ async function waitForPromptProbeFlight(flight: PromptProbeFlight, signal?: Abor let onAbort: (() => void) | undefined; try { if (!signal) return await flight.result; - const aborted = new Promise(resolve => { + const aborted = new Promise(resolve => { onAbort = () => resolve(null); signal.addEventListener("abort", onAbort, { once: true }); if (signal.aborted) onAbort(); @@ -390,9 +547,44 @@ export async function probePromptText( if (signal?.aborted) { return { ok: false, codexHome, layers: {}, detail: "prompt probe cancelled" }; } - const binary = probeCommandForTests?.binary ?? resolveCodexBinary(); + // Resolve through the shared runtime resolver, not a private path list: the + // old four-path POSIX check could never match the Codex App's Windows install + // under %LOCALAPPDATA%\OpenAI\Codex\bin\, so the probe reported + // "not found" on machines where Codex was plainly installed (issue 4458). + // + // Both flags are deliberate. This runs on a request path: discoverAlternatives + // would walk the whole PATH just to fill a newerAvailable diagnostic the probe + // never shows, and probeVersion would pay a blocking `--version` exec per + // candidate. The probe needs a command it can spawn; the version is irrelevant. + const resolved = probeCommandForTests || probeRuntimeForTests !== undefined + ? null + : resolveCodexRuntime({ discoverAlternatives: false, probeVersion: false }); + const runtime: { command: string; source: CodexRuntimeSource } | undefined = + probeRuntimeForTests !== undefined + ? probeRuntimeForTests ?? undefined + : resolved && resolved.runtime.source !== "fallback" + ? { command: resolved.runtime.command, source: resolved.runtime.source } + : undefined; + // A `fallback` result is the resolver saying "nothing concrete was found" - + // its command is the bare word "codex", not a located binary. Reporting it as + // resolved would just relabel the same not-found as a spawn failure. + const binary = probeCommandForTests?.binary ?? runtime?.command ?? null; if (!binary) { - return { ok: false, codexHome, layers: {}, detail: "codex binary not found" }; + const failure = resolved + ? classifyRuntimeFailure(resolved) + : probeFailure( + { binary: "codex", args: ["debug", "prompt-input"], cwd: codexHome, timeoutMs, promptStateFingerprint }, + "program-not-found", + "codex program not found: codex debug prompt-input", + ); + return { + ok: false, + codexHome, + layers: {}, + ...(runtime ? { runtime } : {}), + failure, + detail: "codex binary not found", + }; } const command: ProbeCommand = { binary, @@ -407,6 +599,8 @@ export async function probePromptText( ok: false, codexHome, layers: {}, + ...(runtime ? { runtime } : {}), + ...(outcome.kind === "failed" && outcome.failure ? { failure: outcome.failure } : {}), detail: signal?.aborted ? "prompt probe cancelled" : outcome.kind === "busy" @@ -419,7 +613,18 @@ export async function probePromptText( if (sections.size === 0) { // Zero sections from a zero-exit probe means the output did not parse, which // is a failed read - not fifteen layers that each chose to send nothing. - return { ok: false, codexHome, layers: {}, detail: "prompt output could not be parsed" }; + return { + ok: false, + codexHome, + layers: {}, + ...(runtime ? { runtime } : {}), + failure: probeFailure( + command, + "output-invalid", + `codex prompt output could not be parsed: ${commandDescription(command)}`, + ), + detail: "prompt output could not be parsed", + }; } const layers: Record = {}; for (const [layerId, tag] of Object.entries(LAYER_SECTION_TAGS)) { @@ -453,7 +658,7 @@ export async function probePromptText( for (const id of UNMAPPED_LAYER_IDS) { layers[id] ??= { text: null, reason: "not-exposed", bytes: 0 }; } - return { ok: true, codexHome, layers }; + return { ok: true, codexHome, layers, ...(runtime ? { runtime } : {}) }; } /** Test-only command seam; production always resolves the installed Codex binary. */ @@ -461,6 +666,13 @@ export function setPromptTextProbeCommandForTests(command: { binary: string; arg probeCommandForTests = command ? { binary: command.binary, args: [...command.args] } : null; } +/** Test-only runtime seam: stands in for the shared resolver's answer. */ +export function setPromptTextProbeRuntimeForTests( + runtime: { command: string; source: CodexRuntimeSource } | null | undefined, +): void { + probeRuntimeForTests = runtime; +} + /** Test-only process-start counter for proving admission without timing guesses. */ export function promptTextProbeSpawnAttemptsForTests(): number { return probeSpawnAttemptsForTests; @@ -484,6 +696,7 @@ export async function resetPromptTextProbeForTests(): Promise { } if (activePromptProbe === active) activePromptProbe = null; probeCommandForTests = null; + probeRuntimeForTests = undefined; probeSpawnAttemptsForTests = 0; probeCloseBarrierForTests = null; } diff --git a/src/codex/runtime.ts b/src/codex/runtime.ts index 576f454c9c..7e70d7ff47 100644 --- a/src/codex/runtime.ts +++ b/src/codex/runtime.ts @@ -1,6 +1,6 @@ import { execFileSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, unlinkSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, unlinkSync } from "node:fs"; +import { homedir, tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { atomicWriteFile, getConfigDir } from "../config"; import { codexExecInvocation, isSpawnableCodexCandidate } from "./exec-invocation"; @@ -10,6 +10,7 @@ export type CodexRuntimeSource = | "environment" | "configured" | "shim" + | "installed" | "path" | "fallback"; @@ -74,6 +75,27 @@ export interface ResolveCodexRuntimeDeps { * newerAvailable discovery). Use for hot UI/status paths. */ discoverAlternatives?: boolean; + /** + * When false, select a spawnable candidate without running `codex --version`. + * The prompt probe needs a command it can spawn, not a version, and paying + * ~1s of blocking exec per candidate on a UI path is what made it report an + * absent candidate instead of the Windows Codex App install (issue 4458). + */ + probeVersion?: boolean; + /** + * Directory listing used by Windows App-root discovery. Injected so tests can + * exercise the LOCALAPPDATA OpenAI/Codex/bin layout without a real Windows + * filesystem. Must be listed in resolveCacheKey's injection guard: an injected + * listing that leaked into the process memo would pin every later test in this + * file to a fake install. + */ + readdirSync?: (path: string) => string[]; + /** + * Stat used to order Windows App version directories by mtime. Same injection + * contract as readdirSync: a test-supplied impl must not populate the process + * memo. + */ + statSync?: (path: string) => { mtimeMs: number; isDirectory(): boolean }; } export interface PersistedCodexRuntimeState { @@ -89,6 +111,16 @@ const CLAMP_PERSIST_FILE = "codex-runtime-clamp.json"; /** Probe rejection for an absolute candidate whose file is gone. Matched when retiring a dead pin (#4035). */ const PATH_MISSING_REASON = "path does not exist"; +/** + * Probe rejection when the selected command cannot even be spawned. Distinct + * from PATH_MISSING_REASON (the absolute path was gone before spawn) and from + * the generic `failed --version (...)` string (the binary ran and failed). + * Exported because the prompt probe classifies this as program-not-found, so a + * PATH fallback that is simply not installed must not look like an execution + * failure (issue 4458). + */ +export const CODEX_PROGRAM_NOT_FOUND_REASON = "program not found (ENOENT)"; + function cloneAndDeepFreeze(value: T): DeepReadonly { const clone = (current: unknown): unknown => { if (Array.isArray(current)) return current.map(clone); @@ -111,6 +143,7 @@ function isCodexRuntimeSource(value: unknown): value is CodexRuntimeSource { return value === "environment" || value === "configured" || value === "shim" + || value === "installed" || value === "path" || value === "fallback"; } @@ -313,7 +346,7 @@ export function clearPersistedCodexRuntime(deps: ResolveCodexRuntimeDeps = {}): function probeVersion( command: string, deps: ResolveCodexRuntimeDeps, -): { ok: true; version: string } | { ok: false; reason: string } { +): { ok: true; version: string | null } | { ok: false; reason: string } { const platform = deps.platform ?? process.platform; if (command.includes("/") || command.includes("\\") || /^[A-Za-z]:/.test(command)) { const exists = deps.existsSync ?? existsSync; @@ -322,6 +355,11 @@ function probeVersion( return { ok: false, reason: "not a spawnable Codex launcher on this platform" }; } } + // The prompt probe needs a spawnable candidate, not a version. Running + // `codex --version` here is ~1s of blocking exec per candidate; on the + // dashboard probe that cost made Windows report Codex as missing even when + // the App install was sitting under LOCALAPPDATA/OpenAI/Codex/bin (issue 4458). + if (deps.probeVersion === false) return { ok: true, version: null }; const execFile = deps.execFileSync ?? (execFileSync as unknown as RuntimeExecFile); // Sandbox the probe's CODEX_HOME: a real Codex CLI creates state (tmp/, logs) under // CODEX_HOME even for `--version`, and the probe inherits the caller's env — so a @@ -350,6 +388,9 @@ function probeVersion( return { ok: true, version }; } catch (error) { if (!probeHome) return { ok: false, reason: "probe sandbox unavailable" }; + if ((error as NodeJS.ErrnoException)?.code === "ENOENT") { + return { ok: false, reason: CODEX_PROGRAM_NOT_FOUND_REASON }; + } const message = error instanceof Error ? error.message : String(error); const redacted = redactUserPath(redactSecretString(message)).slice(0, 160); return { ok: false, reason: `failed --version (${redacted})` }; @@ -401,6 +442,53 @@ function pathCandidates(deps: ResolveCodexRuntimeDeps): string[] { return [...new Set(out)]; } +/** + * Codex installs that PATH does not necessarily expose. + * + * The Windows Codex App writes codex.exe under + * LOCALAPPDATA/OpenAI/Codex/bin//, which never appears on + * the service process PATH. The prompt probe used to hardcode four POSIX + * paths and miss that layout, then report an absent candidate (issue 4458). + * POSIX keeps those four paths so an install that resolved before this source + * existed still resolves. + */ +function installedCodexCandidates(deps: ResolveCodexRuntimeDeps): string[] { + const platform = deps.platform ?? process.platform; + const env = deps.env ?? process.env; + if (platform === "win32") { + const localAppData = env.LOCALAPPDATA?.trim(); + if (!localAppData) return []; + const root = join(localAppData, "OpenAI", "Codex", "bin"); + const readDir = deps.readdirSync ?? ((path: string) => readdirSync(path)); + const stat = deps.statSync ?? ((path: string) => statSync(path)); + try { + const names = readDir(root); + const dirs: Array<{ name: string; directory: string; mtimeMs: number }> = []; + for (const name of names) { + const directory = join(root, name); + try { + const st = stat(directory); + if (!st.isDirectory()) continue; + dirs.push({ name, directory, mtimeMs: st.mtimeMs }); + } catch { + continue; + } + } + dirs.sort((a, b) => b.mtimeMs - a.mtimeMs || a.name.localeCompare(b.name)); + return dirs.map(entry => join(entry.directory, "codex.exe")); + } catch { + return []; + } + } + const home = env.HOME?.trim() || env.USERPROFILE?.trim() || homedir(); + return [ + join(home, ".codex", "packages", "standalone", "current", "bin", "codex"), + join(home, ".local", "bin", "codex"), + "/usr/local/bin/codex", + "/opt/homebrew/bin/codex", + ]; +} + interface RankedCandidate { command: string; source: CodexRuntimeSource; @@ -496,6 +584,20 @@ export type CodexRuntimeProcessCachePeek = let resolveCacheEpoch = 0; let resolveCache: ResolveCacheMemo | null = null; +/** + * Memo for probeVersion === false resolves. Kept separate from resolveCache + * because peekCodexRuntimeProcessCache is read by convergence and the bundled + * catalog as "what runtime are we on". Publishing a null version there would + * be read as "unknown version" and become process authority (issue 4458). + */ +interface DeferredResolveCacheMemo { + readonly key: string; + readonly at: number; + readonly value: DeepReadonly; +} + +let deferredResolveCache: DeferredResolveCacheMemo | null = null; + /** * Bumped whenever persisted runtime state is replaced or process authority is cleared. * @@ -522,6 +624,7 @@ function publishResolveCache(key: string, at: number, value: ResolveCodexRuntime function clearResolveCache(): void { resolveCacheEpoch += 1; resolveCache = null; + deferredResolveCache = null; } /** Clear process-local runtime authority without resolving a replacement. */ @@ -558,7 +661,15 @@ function persistedRuntimeCacheStamp(deps: ResolveCodexRuntimeDeps): string { function resolveCacheKey(deps: ResolveCodexRuntimeDeps): string | null { // Only memoize uninjected process-env resolves (settings/status hot paths). - if (deps.execFileSync || deps.existsSync || deps.readFileSync || deps.configDir || deps.now) { + if ( + deps.execFileSync + || deps.existsSync + || deps.readFileSync + || deps.readdirSync + || deps.statSync + || deps.configDir + || deps.now + ) { return null; } const env = deps.env ?? process.env; @@ -567,6 +678,10 @@ function resolveCacheKey(deps: ResolveCodexRuntimeDeps): string | null { path: env.PATH ?? "", platform: deps.platform ?? process.platform, discover: deps.discoverAlternatives !== false, + probeVersion: deps.probeVersion !== false, + localAppData: env.LOCALAPPDATA?.trim() ?? "", + homeDir: env.HOME?.trim() ?? "", + userProfile: env.USERPROFILE?.trim() ?? "", home: process.env.OPENCODEX_HOME ?? "", persisted: persistedRuntimeCacheStamp(deps), }); @@ -577,6 +692,27 @@ function resolveCacheKey(deps: ResolveCodexRuntimeDeps): string | null { */ export function resolveCodexRuntime(deps: ResolveCodexRuntimeDeps = {}): ResolveCodexRuntimeResult { const cacheKey = resolveCacheKey(deps); + // A deferred selection has no validated version and must not publish into + // runtime authority. peekCodexRuntimeProcessCache would otherwise report + // "available" with version null, which catalog/convergence read as unknown. + if (deps.probeVersion === false) { + if (cacheKey + && deferredResolveCache + && deferredResolveCache.key === cacheKey + && Date.now() - deferredResolveCache.at < RESOLVE_CACHE_MS) { + return cloneAndDeepFreeze(deferredResolveCache.value); + } + + const deferred = resolveCodexRuntimeUncached(deps); + if (!cacheKey) return cloneAndDeepFreeze(deferred); + deferredResolveCache = { + key: cacheKey, + at: Date.now(), + value: cloneAndDeepFreeze(deferred), + }; + return cloneAndDeepFreeze(deferredResolveCache.value); + } + if (cacheKey && resolveCache && resolveCache.key === cacheKey && Date.now() - resolveCache.at < RESOLVE_CACHE_MS) { return cloneAndDeepFreeze(resolveCache.value); } @@ -623,6 +759,9 @@ function resolveCodexRuntimeUncached(deps: ResolveCodexRuntimeDeps = {}): Resolv for (const command of pathCandidates(deps)) { ordered.push({ command, source: "path" }); } + for (const command of installedCodexCandidates(deps)) { + ordered.push({ command, source: "installed" }); + } ordered.push({ command: "codex", source: "fallback" }); const seen = new Set(); @@ -644,7 +783,7 @@ function resolveCodexRuntimeUncached(deps: ResolveCodexRuntimeDeps = {}): Resolv }; } - // Prefer first valid in priority order (environment → configured → shim → path → fallback). + // Prefer first valid in priority order (environment → configured → shim → path → installed → fallback). let selected = valid[0]!; let replacedConfigured: ResolveCodexRuntimeResult["replacedConfigured"]; diff --git a/tests/codex-integration/codex-prompt-text-probe.test.ts b/tests/codex-integration/codex-prompt-text-probe.test.ts index 32b01eac3c..4d64015af7 100644 --- a/tests/codex-integration/codex-prompt-text-probe.test.ts +++ b/tests/codex-integration/codex-prompt-text-probe.test.ts @@ -17,6 +17,7 @@ import { resetPromptTextProbeForTests, setPromptTextProbeCloseBarrierForTests, setPromptTextProbeCommandForTests, + setPromptTextProbeRuntimeForTests, } from "../../src/codex/prompt-text-probe"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { INTERNAL_DEADLINE_MS } from "../helpers/test-budget"; @@ -319,3 +320,77 @@ describe("prompt probe process lifecycle", () => { expect(promptTextProbeSpawnAttemptsForTests()).toBe(1); }); }); + +describe("runtime resolution and failure classification", () => { + test("a runtime the shared resolver finds is spawned, not reported missing", async () => { + // Issue 4458: the old four-path POSIX check reported "codex binary not + // found" on a Windows machine where the Codex App had installed codex.exe + // under %LOCALAPPDATA%. The resolver's answer must reach the spawn. + const started = join(root(), "resolved-runtime.txt"); + setPromptTextProbeRuntimeForTests({ command: process.execPath, source: "installed" }); + setPromptTextProbeCommandForTests({ + binary: process.execPath, + args: ["-e", `require("node:fs").writeFileSync(${JSON.stringify(started)}, "1"); process.stdout.write(${JSON.stringify(VALID_PROBE_OUTPUT)})`], + }); + + const result = await probePromptText(2_000); + + expect(result.ok).toBe(true); + expect(result.detail).not.toBe("codex binary not found"); + expect(result.runtime).toEqual({ command: process.execPath, source: "installed" }); + expect(existsSync(started)).toBe(true); + }); + + test("a resolver that finds nothing yields failure.kind program-not-found", async () => { + setPromptTextProbeRuntimeForTests(null); + + const result = await probePromptText(2_000); + + expect(result.ok).toBe(false); + expect(result.detail).toBe("codex binary not found"); + expect(result.failure?.kind).toBe("program-not-found"); + expect(promptTextProbeSpawnAttemptsForTests()).toBe(0); + }); + + test("unparseable output from a zero-exit run yields output-invalid", async () => { + setPromptTextProbeCommandForTests({ + binary: process.execPath, + args: ["-e", "process.stdout.write(\"this is not probe json\")"], + }); + + const result = await probePromptText(2_000); + + expect(result.ok).toBe(false); + expect(result.detail).toBe("prompt output could not be parsed"); + expect(result.failure?.kind).toBe("output-invalid"); + }); + + test("an unknown-subcommand exit yields command-unsupported without echoing stderr", async () => { + const marker = `stderr-marker-${process.pid}-do-not-echo`; + const source = `process.stderr.write(${JSON.stringify("error: unrecognized subcommand 'prompt-input' ")} + ${JSON.stringify(marker)}); process.exit(2);`; + setPromptTextProbeCommandForTests({ binary: process.execPath, args: ["-e", source] }); + + const result = await probePromptText(2_000); + + expect(result.ok).toBe(false); + expect(result.failure?.kind).toBe("command-unsupported"); + // stderr classifies the failure; it must never be served back in detail. + expect(result.failure?.detail).not.toContain(marker); + expect(result.failure?.detail).not.toContain("unrecognized subcommand"); + }); + + test("an ordinary non-zero exit yields execution-failed without echoing stderr", async () => { + const marker = `stderr-marker-${process.pid}-do-not-echo`; + setPromptTextProbeCommandForTests({ + binary: process.execPath, + args: ["-e", `process.stderr.write(${JSON.stringify("boom ")} + ${JSON.stringify(marker)}); process.exit(1);`], + }); + + const result = await probePromptText(2_000); + + expect(result.ok).toBe(false); + expect(result.detail).toBe("codex debug prompt-input failed"); + expect(result.failure?.kind).toBe("execution-failed"); + expect(result.failure?.detail).not.toContain(marker); + }); +}); diff --git a/tests/codex-integration/codex-runtime.test.ts b/tests/codex-integration/codex-runtime.test.ts index e413994ce6..a97939fb63 100644 --- a/tests/codex-integration/codex-runtime.test.ts +++ b/tests/codex-integration/codex-runtime.test.ts @@ -21,6 +21,7 @@ import { join, dirname } from "node:path"; import { clearCodexRuntimeResolveCache, compareCodexVersions, + CODEX_PROGRAM_NOT_FOUND_REASON, displayCodexRuntimePath, effortClampAppliesToRuntime, liveRemovedEfforts, @@ -1163,3 +1164,162 @@ describe("dead configured pin recovery (#4035)", () => { }); }); + +describe("installed Codex discovery and deferred version probes", () => { + test("discovers the newest Windows Codex App install from an injected listing", () => { + const localAppData = "C:\\Users\\test\\AppData\\Local"; + const root = join(localAppData, "OpenAI", "Codex", "bin"); + const older = join(root, "older", "codex.exe"); + const newer = join(root, "newer", "codex.exe"); + const result = resolveCodexRuntime({ + configDir: tempConfigDir(), + env: { LOCALAPPDATA: localAppData, PATH: NO_CODEX_PATH }, + platform: "win32", + existsSync: path => path === older || path === newer, + readdirSync: path => path === root ? ["older", "newer"] : [], + statSync: path => { + if (path === join(root, "older")) return { mtimeMs: 1_000, isDirectory: () => true }; + if (path === join(root, "newer")) return { mtimeMs: 2_000, isDirectory: () => true }; + return { mtimeMs: 0, isDirectory: () => false }; + }, + execFileSync: file => { + expect(String(file)).toBe(newer); + return "codex-cli 0.154.0-alpha.6.2"; + }, + discoverAlternatives: false, + }); + expect(result.runtime.command).toBe(newer); + expect(result.runtime.source).toBe("installed"); + expect(result.runtime.version).toBe("0.154.0-alpha.6.2"); + }); + + test("orders equal-mtime Windows App directories by name", () => { + const localAppData = "C:\\Users\\test\\AppData\\Local"; + const root = join(localAppData, "OpenAI", "Codex", "bin"); + const alpha = join(root, "alpha", "codex.exe"); + const zeta = join(root, "zeta", "codex.exe"); + const probed: string[] = []; + const result = resolveCodexRuntime({ + configDir: tempConfigDir(), + env: { LOCALAPPDATA: localAppData, PATH: NO_CODEX_PATH }, + platform: "win32", + existsSync: path => path === alpha || path === zeta, + readdirSync: path => path === root ? ["zeta", "alpha"] : [], + statSync: path => { + if (path === join(root, "alpha") || path === join(root, "zeta")) { + return { mtimeMs: 1_000, isDirectory: () => true }; + } + return { mtimeMs: 0, isDirectory: () => false }; + }, + execFileSync: file => { + probed.push(String(file)); + return "codex-cli 0.154.0-alpha.6.2"; + }, + }); + expect(result.runtime.command).toBe(alpha); + expect(result.runtime.source).toBe("installed"); + expect(probed.slice(0, 2)).toEqual([alpha, zeta]); + }); + + test("can select a runtime without synchronously probing its version", () => { + let probeCalls = 0; + const result = resolveCodexRuntime({ + configDir: tempConfigDir(), + env: { CODEX_CLI_PATH: "C:\\codex\\codex.exe", PATH: "" }, + platform: "win32", + existsSync: () => true, + execFileSync: () => { + probeCalls += 1; + return "codex-cli 0.154.0"; + }, + probeVersion: false, + }); + expect(result.runtime.command).toBe("C:\\codex\\codex.exe"); + expect(result.runtime.version).toBeNull(); + expect(probeCalls).toBe(0); + }); + + test("a deferred resolve does not publish a null version into process authority", () => { + const deps = { env: { PATH: "" }, discoverAlternatives: false as const }; + resetCodexRuntimeResolveCacheForTests(); + try { + setCodexRuntimeResolveCacheForTests({ + runtime: { command: "validated-codex", version: "0.154.0", source: "path" }, + failures: [], + }, deps); + const before = peekCodexRuntimeProcessCache(); + expect(before.kind).toBe("available"); + + const selected = resolveCodexRuntime({ ...deps, probeVersion: false }); + expect(selected.runtime.version).toBeNull(); + expect(peekCodexRuntimeProcessCache()).toEqual(before); + + resetCodexRuntimeResolveCacheForTests(); + resolveCodexRuntime({ ...deps, probeVersion: false }); + const peeked = peekCodexRuntimeProcessCache(); + expect(peeked.kind === "available" && peeked.value.runtime.version === null).toBe(false); + } finally { + resetCodexRuntimeResolveCacheForTests(); + } + }); + + test("classifies a missing-program ENOENT distinctly from a generic version-probe failure", () => { + const error = Object.assign(new Error("spawn ENOENT"), { code: "ENOENT" }); + const result = resolveCodexRuntime({ + configDir: tempConfigDir(), + env: { CODEX_CLI_PATH: "C:\\missing-bin\\codex.exe", PATH: "" }, + platform: "win32", + existsSync: () => true, + execFileSync: () => { + throw error; + }, + }); + expect(result.failures.some(item => item.reason === CODEX_PROGRAM_NOT_FOUND_REASON)).toBe(true); + expect(result.failures.some(item => item.reason.includes("failed --version"))).toBe(false); + }); + + test("PATH still outranks an installed candidate when both are valid", () => { + const localAppData = "C:\\Users\\test\\AppData\\Local"; + const root = join(localAppData, "OpenAI", "Codex", "bin"); + const installed = join(root, "app", "codex.exe"); + const pathDir = "C:\\on-path"; + const pathCommand = join(pathDir, "codex.exe"); + const result = resolveCodexRuntime({ + configDir: tempConfigDir(), + env: { LOCALAPPDATA: localAppData, PATH: pathDir }, + platform: "win32", + existsSync: path => path === pathCommand || path === installed, + readdirSync: path => path === root ? ["app"] : [], + statSync: path => path === join(root, "app") + ? { mtimeMs: 2_000, isDirectory: () => true } + : { mtimeMs: 0, isDirectory: () => false }, + execFileSync: file => { + const text = String(file); + if (text === pathCommand || text === installed) return "codex-cli 0.154.0"; + throw new Error(`unexpected probe: ${text}`); + }, + discoverAlternatives: false, + }); + expect(result.runtime.command).toBe(pathCommand); + expect(result.runtime.source).toBe("path"); + }); + + test("restores the established Unix Codex install locations", () => { + const home = "/home/test"; + const installed = join(home, ".codex", "packages", "standalone", "current", "bin", "codex"); + const result = resolveCodexRuntime({ + configDir: tempConfigDir(), + env: { HOME: home, PATH: NO_CODEX_PATH }, + platform: "linux", + existsSync: path => String(path) === installed, + execFileSync: file => { + expect(String(file)).toBe(installed); + return "codex-cli 0.154.0-alpha.6.2"; + }, + discoverAlternatives: false, + }); + expect(result.runtime.command).toBe(installed); + expect(result.runtime.source).toBe("installed"); + expect(result.runtime.version).toBe("0.154.0-alpha.6.2"); + }); +}); From 6817fa467d5c6de4612ba7a85d6d7b2c8bb006af Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 14:40:26 +0900 Subject: [PATCH 2/3] fix(codex): stop a stale auto-discovered runtime from overriding a newer one codex-runtime.json recorded command, source and version, but not how the record got there. resolveAndPersistCodexRuntime writes every automatically discovered selection, so a "configured" entry proved nothing about operator intent - and resolution then stuck to it even when a newer runtime was present. On the reporting machine a still-runnable codex-cli 0.135.0 kept winning over the 0.153.4 the Codex App was actually running, and the catalog derived its reasoning ladder from the older binary (issue 4204). - PersistedCodexRuntimeState gains origin: "pinned" | "discovered". resolveAndPersistCodexRuntime writes "discovered"; a direct persistCodexRuntime call, which is how doctor --fix selects, stays "pinned". A record with no origin reads as discovered, because auto-discovery is what wrote every one of them, and reading it as a pin would leave the bug unfixed on exactly the installs that have it. - An unpinned record hands over only to a strictly newer valid candidate. A pin is never touched, equal versions stick, and an unknown version on either side is not evidence of an upgrade. The handover is reported as supersededDiscovered, kept separate from replacedConfigured so "superseded" never reads as "gone". - The no-discovery fast path still probes the bounded Codex App roots when the persisted record is unpinned. That path is what the catalog's bundled loader uses, so without it the comparison could never run where the bug actually bites. PATH-wide discovery stays off. - The prompt probe's reported runtime and failure detail now go through displayCodexRuntimePath: the response is served over the management API and a Windows Codex App path contains the account name. No config-schema field was added; codex-runtime.json is runtime-owned state. --- src/codex/prompt-text-probe.ts | 32 ++- src/codex/runtime.ts | 107 ++++++- .../codex-prompt-text-probe.test.ts | 19 +- tests/codex-integration/codex-runtime.test.ts | 271 ++++++++++++++++++ 4 files changed, 416 insertions(+), 13 deletions(-) diff --git a/src/codex/prompt-text-probe.ts b/src/codex/prompt-text-probe.ts index 246abc509a..30a0d142d0 100644 --- a/src/codex/prompt-text-probe.ts +++ b/src/codex/prompt-text-probe.ts @@ -23,6 +23,7 @@ import { codexExecInvocation } from "./exec-invocation"; import { resolveCodexHomeDir } from "./home"; import { CODEX_PROGRAM_NOT_FOUND_REASON, + displayCodexRuntimePath, resolveCodexRuntime, type CodexRuntimeSource, type ResolveCodexRuntimeResult, @@ -186,8 +187,18 @@ function completedExecution(value: PromptProbeExecutionResult | null): PromptPro return { result: Promise.resolve(value), closed: Promise.resolve() }; } +/** + * The command line as it may be shown to a caller. + * + * Redacted, because this response is served over the management API and a + * resolved Codex path is a user path: the Windows Codex App lives under the + * profile directory, so echoing the raw command would put the account name in + * a diagnostic. `displayCodexRuntimePath` is the same helper the runtime log + * line and doctor output already use, so the probe reports a path in the form + * the rest of the product reports it. + */ function commandDescription(command: ProbeCommand): string { - return [command.binary, ...command.args].join(" "); + return [displayCodexRuntimePath(command.binary), ...command.args].join(" "); } function probeFailure( @@ -230,7 +241,9 @@ function classifyRuntimeFailure(result: ResolveCodexRuntimeResult): PromptProbeF const kind: PromptProbeFailureKind = !representative || isNotFound(representative.reason) ? "program-not-found" : "execution-failed"; - const command = representative?.command ?? result.runtime.command; + // Same redaction obligation as commandDescription: a rejected candidate is a + // real filesystem path, and every one of them is reported to the caller. + const command = displayCodexRuntimePath(representative?.command ?? result.runtime.command); const phrase = kind === "program-not-found" ? "codex program not found" : "codex runtime could not be probed"; return { kind, command, detail: `${phrase}: ${command}` }; } @@ -569,6 +582,13 @@ export async function probePromptText( // its command is the bare word "codex", not a located binary. Reporting it as // resolved would just relabel the same not-found as a spawn failure. const binary = probeCommandForTests?.binary ?? runtime?.command ?? null; + // The response travels over the management API, so the reported runtime is + // redacted while `binary` keeps the real path the spawn needs. On Windows the + // Codex App install sits under the user's profile directory, so the raw + // command carries the account name. + const reportedRuntime = runtime + ? { command: displayCodexRuntimePath(runtime.command), source: runtime.source } + : undefined; if (!binary) { const failure = resolved ? classifyRuntimeFailure(resolved) @@ -581,7 +601,7 @@ export async function probePromptText( ok: false, codexHome, layers: {}, - ...(runtime ? { runtime } : {}), + ...(reportedRuntime ? { runtime: reportedRuntime } : {}), failure, detail: "codex binary not found", }; @@ -599,7 +619,7 @@ export async function probePromptText( ok: false, codexHome, layers: {}, - ...(runtime ? { runtime } : {}), + ...(reportedRuntime ? { runtime: reportedRuntime } : {}), ...(outcome.kind === "failed" && outcome.failure ? { failure: outcome.failure } : {}), detail: signal?.aborted ? "prompt probe cancelled" @@ -617,7 +637,7 @@ export async function probePromptText( ok: false, codexHome, layers: {}, - ...(runtime ? { runtime } : {}), + ...(reportedRuntime ? { runtime: reportedRuntime } : {}), failure: probeFailure( command, "output-invalid", @@ -658,7 +678,7 @@ export async function probePromptText( for (const id of UNMAPPED_LAYER_IDS) { layers[id] ??= { text: null, reason: "not-exposed", bytes: 0 }; } - return { ok: true, codexHome, layers, ...(runtime ? { runtime } : {}) }; + return { ok: true, codexHome, layers, ...(reportedRuntime ? { runtime: reportedRuntime } : {}) }; } /** Test-only command seam; production always resolves the installed Codex binary. */ diff --git a/src/codex/runtime.ts b/src/codex/runtime.ts index 7e70d7ff47..aa9a57563a 100644 --- a/src/codex/runtime.ts +++ b/src/codex/runtime.ts @@ -43,6 +43,13 @@ export interface ResolveCodexRuntimeResult { readonly runtime: ResolvedCodexRuntime; readonly failures: readonly RuntimeProbeFailure[]; readonly replacedConfigured?: Readonly<{ from: ResolvedCodexRuntime; reason: string }>; + /** + * Set when an unpinned, still-runnable persisted discovery is handed over to a + * strictly newer valid candidate. Distinct from replacedConfigured, which means + * the configured runtime became invalid; conflating "gone" with "superseded" + * would make the doctor output lie. + */ + readonly supersededDiscovered?: Readonly<{ from: ResolvedCodexRuntime; to: ResolvedCodexRuntime; reason: string }>; readonly newerAvailable?: ResolvedCodexRuntime; /** Set when the selected runtime could not be written to codex-runtime.json. */ readonly persistError?: string; @@ -98,12 +105,24 @@ export interface ResolveCodexRuntimeDeps { statSync?: (path: string) => { mtimeMs: number; isDirectory(): boolean }; } +/** + * How a `codex-runtime.json` record got onto disk. + * + * "pinned" is an intentional operator selection (doctor --fix). "discovered" is + * automatic resolve-and-persist. Absent is the pre-field shape and is treated + * as discovered, not pinned: every such file was written by + * resolveAndPersistCodexRuntime, so reading it as a pin would leave issue 4204 + * unfixed on exactly the installs that have it. + */ +export type CodexRuntimePinOrigin = "pinned" | "discovered"; + export interface PersistedCodexRuntimeState { readonly version: 1; readonly command: string; readonly source: CodexRuntimeSource; readonly selectedVersion?: string | null; readonly updatedAt: string; + readonly origin?: CodexRuntimePinOrigin; } const PERSIST_FILE = "codex-runtime.json"; @@ -148,6 +167,10 @@ function isCodexRuntimeSource(value: unknown): value is CodexRuntimeSource { || value === "fallback"; } +function isCodexRuntimePinOrigin(value: unknown): value is CodexRuntimePinOrigin { + return value === "pinned" || value === "discovered"; +} + export function codexRuntimeStatePath(configDir: string = getConfigDir()): string { return join(configDir, PERSIST_FILE); } @@ -282,6 +305,9 @@ export function parsePersistedCodexRuntime( if (raw.selectedVersion !== undefined && raw.selectedVersion !== null && typeof raw.selectedVersion !== "string") return null; + // Absent origin is legal (pre-field files). A present value that is neither + // literal makes the whole record invalid, same as every other field. + if (raw.origin !== undefined && !isCodexRuntimePinOrigin(raw.origin)) return null; return cloneAndDeepFreeze(raw as PersistedCodexRuntimeState); } catch { return null; @@ -300,9 +326,35 @@ export function loadPersistedCodexRuntime( } } +/** + * True only when the operator intentionally pinned this runtime. + * + * A record with no origin is NOT pinned: every such file predates this field + * and was written by resolveAndPersistCodexRuntime, which is auto-discovery. + * Reading a missing origin as an intentional pin would leave issue 4204 + * unfixed on exactly the installs that have it — the still-runnable 0.135.0 + * CLI that kept winning over a 0.153.4 Desktop runtime sitting right there. + */ +export function persistedCodexRuntimeIsPinned( + state: DeepReadonly | null | undefined, +): boolean { + return state?.origin === "pinned"; +} + +/** + * Persist the selected Codex runtime. + * + * `origin` defaults to "pinned" ON PURPOSE: a direct call is a deliberate + * selection. src/cli/doctor.ts calls this from `doctor --fix`. The automatic + * discovery path is resolveAndPersistCodexRuntime, which passes "discovered" + * explicitly. Flipping the default would make doctor --fix look like an + * accident, and a later resolve would silently replace the operator's choice + * (issue 4204). + */ export function persistCodexRuntime( runtime: ResolvedCodexRuntime, deps: ResolveCodexRuntimeDeps = {}, + origin: CodexRuntimePinOrigin = "pinned", ): void { const configDir = deps.configDir ?? getConfigDir(); mkdirSync(configDir, { recursive: true, mode: 0o700 }); @@ -312,6 +364,7 @@ export function persistCodexRuntime( source: runtime.source, selectedVersion: runtime.version, updatedAt: new Date((deps.now ?? Date.now)()).toISOString(), + origin, }; // Invalidate process authority before the persisted replacement is visible. clearCodexRuntimeResolveCache(); @@ -766,14 +819,37 @@ function resolveCodexRuntimeUncached(deps: ResolveCodexRuntimeDeps = {}): Resolv const seen = new Set(); const valid: ResolvedCodexRuntime[] = []; + // A caller that declined PATH-wide discovery normally gets the first valid + // candidate and nothing else, which is right for a hot path and wrong for + // exactly one arrangement: an unpinned persisted selection sitting in front of + // a Codex App runtime that PATH never exposes. + // + // That arrangement is issue 4204. The catalog's bundled loader passes + // discoverAlternatives: false, so it stopped at a still-runnable codex-cli + // 0.135.0 and derived the reasoning ladder from it while the Desktop app was + // running 0.153.4 out of LOCALAPPDATA. Nothing downstream could notice, + // because the newer runtime was never probed. + // + // So the early stop keeps skipping PATH — which is the expensive part, 100+ + // launcher probes on a dev machine — but still probes the `installed` roots, + // a bounded set with one entry per Codex App version directory. A pinned + // record skips even that: the operator's choice is not up for revision, and + // there is then nothing to compare it against. + const persistedIsUnpinned = Boolean(persisted?.command) && !persistedCodexRuntimeIsPinned(persisted); for (const candidate of ordered) { const key = candidate.command.toLowerCase(); if (seen.has(key)) continue; seen.add(key); + if ( + deps.discoverAlternatives === false + && valid.length > 0 + && !(persistedIsUnpinned && candidate.source === "installed") + ) { + continue; + } const resolved = tryCandidate(candidate, failures, deps); if (!resolved) continue; valid.push(resolved); - if (deps.discoverAlternatives === false) break; } if (valid.length === 0) { @@ -786,6 +862,7 @@ function resolveCodexRuntimeUncached(deps: ResolveCodexRuntimeDeps = {}): Resolv // Prefer first valid in priority order (environment → configured → shim → path → installed → fallback). let selected = valid[0]!; let replacedConfigured: ResolveCodexRuntimeResult["replacedConfigured"]; + let supersededDiscovered: ResolveCodexRuntimeResult["supersededDiscovered"]; const envValid = envPath ? valid.find(item => sameRuntimeCommand(item.command, envPath) && item.source === "environment") @@ -811,6 +888,31 @@ function resolveCodexRuntimeUncached(deps: ResolveCodexRuntimeDeps = {}): Resolv } else if (!envValid && configuredStillValid) { // Stick to configured even when a later PATH entry is also valid. selected = valid.find(item => sameRuntimeCommand(item.command, persisted.command)) ?? selected; + // An explicit pin is the user's decision and this change must never + // silently replace it — issue 4204 says so in as many words. Stick. + // An unpinned record (missing origin, or origin "discovered") may hand + // over to a strictly newer valid candidate. Unknown (null) versions on + // either side are not evidence of an upgrade: compareCodexVersions treats + // null as less-than, which would otherwise make any known alternative + // look newer than a deferred probe. probeVersion === false yields null + // everywhere, so the comparison cannot fire there; equal versions stick. + if (!persistedCodexRuntimeIsPinned(persisted)) { + const newerDiscovered = valid + .filter(item => + !sameRuntimeCommand(item.command, selected.command) + && typeof item.version === "string" + && typeof selected.version === "string" + && compareCodexVersions(item.version, selected.version) > 0) + .sort((a, b) => compareCodexVersions(b.version, a.version))[0]; + if (newerDiscovered) { + supersededDiscovered = { + from: selected, + to: newerDiscovered, + reason: `discovered runtime ${selected.version} superseded by newer runtime ${newerDiscovered.version}`, + }; + selected = newerDiscovered; + } + } } } @@ -826,6 +928,7 @@ function resolveCodexRuntimeUncached(deps: ResolveCodexRuntimeDeps = {}): Resolv runtime: selected, failures, replacedConfigured, + supersededDiscovered, newerAvailable: newer, }; } @@ -846,7 +949,7 @@ export function resolveAndPersistCodexRuntime( && (persistedRuntime.selectedVersion ?? null) === (result.runtime.version ?? null); if (result.runtime.command && result.runtime.source !== "fallback" && !selectionUnchanged) { try { - persistCodexRuntime(result.runtime, deps); + persistCodexRuntime(result.runtime, deps, "discovered"); } catch (error) { const message = error instanceof Error ? error.message : String(error); const persistError = redactUserPath(redactSecretString(message)).slice(0, 200); diff --git a/tests/codex-integration/codex-prompt-text-probe.test.ts b/tests/codex-integration/codex-prompt-text-probe.test.ts index 4d64015af7..48c53b65b5 100644 --- a/tests/codex-integration/codex-prompt-text-probe.test.ts +++ b/tests/codex-integration/codex-prompt-text-probe.test.ts @@ -20,6 +20,7 @@ import { setPromptTextProbeRuntimeForTests, } from "../../src/codex/prompt-text-probe"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { displayCodexRuntimePath } from "../../src/codex/runtime"; import { INTERNAL_DEADLINE_MS } from "../helpers/test-budget"; const lifecycleRoots: string[] = []; @@ -337,7 +338,12 @@ describe("runtime resolution and failure classification", () => { expect(result.ok).toBe(true); expect(result.detail).not.toBe("codex binary not found"); - expect(result.runtime).toEqual({ command: process.execPath, source: "installed" }); + // The reported command is redacted the same way every other runtime path in + // the product is, because this response is served over the management API. + expect(result.runtime).toEqual({ + command: displayCodexRuntimePath(process.execPath), + source: "installed", + }); expect(existsSync(started)).toBe(true); }); @@ -366,8 +372,11 @@ describe("runtime resolution and failure classification", () => { }); test("an unknown-subcommand exit yields command-unsupported without echoing stderr", async () => { - const marker = `stderr-marker-${process.pid}-do-not-echo`; - const source = `process.stderr.write(${JSON.stringify("error: unrecognized subcommand 'prompt-input' ")} + ${JSON.stringify(marker)}); process.exit(2);`; + // The sentinels are concatenated inside the child so they exist only on + // stderr: failure.detail legitimately echoes the attempted command line, so + // a marker written literally into argv would make these assertions vacuous. + const marker = "stderr-marker-do-not-echo"; + const source = `process.stderr.write("error: " + "unrecognized" + " subcommand 'prompt-input' " + "stderr-marker-" + "do-not-echo"); process.exit(2);`; setPromptTextProbeCommandForTests({ binary: process.execPath, args: ["-e", source] }); const result = await probePromptText(2_000); @@ -380,10 +389,10 @@ describe("runtime resolution and failure classification", () => { }); test("an ordinary non-zero exit yields execution-failed without echoing stderr", async () => { - const marker = `stderr-marker-${process.pid}-do-not-echo`; + const marker = "stderr-marker-do-not-echo"; setPromptTextProbeCommandForTests({ binary: process.execPath, - args: ["-e", `process.stderr.write(${JSON.stringify("boom ")} + ${JSON.stringify(marker)}); process.exit(1);`], + args: ["-e", `process.stderr.write("boom " + "stderr-marker-" + "do-not-echo"); process.exit(1);`], }); const result = await probePromptText(2_000); diff --git a/tests/codex-integration/codex-runtime.test.ts b/tests/codex-integration/codex-runtime.test.ts index a97939fb63..c03a3414b8 100644 --- a/tests/codex-integration/codex-runtime.test.ts +++ b/tests/codex-integration/codex-runtime.test.ts @@ -28,6 +28,8 @@ import { loadLastEffortClamp, loadPersistedCodexRuntime, parseCodexVersionOutput, + parsePersistedCodexRuntime, + persistedCodexRuntimeIsPinned, peekCodexRuntimeProcessCache, persistCodexRuntime, persistEffortClamp, @@ -1323,3 +1325,272 @@ describe("installed Codex discovery and deferred version probes", () => { expect(result.runtime.version).toBe("0.154.0-alpha.6.2"); }); }); + +describe("unpinned discovered runtime handover (issue 4204)", () => { + function writeLegacyPersisted( + configDir: string, + command: string, + selectedVersion: string, + origin?: "pinned" | "discovered", + ): void { + const payload: Record = { + version: 1, + command, + source: "configured", + selectedVersion, + updatedAt: "2026-01-01T00:00:00.000Z", + }; + if (origin !== undefined) payload.origin = origin; + writeFileSync(join(configDir, "codex-runtime.json"), JSON.stringify(payload)); + } + + test("a still-runnable persisted 0.135.0 with no origin yields to 0.153.4 and reports supersededDiscovered", () => { + // Issue 4204: resolveAndPersistCodexRuntime wrote every automatic selection + // without an origin, so a still-runnable 0.135.0 CLI kept winning over a + // 0.153.4 Desktop runtime sitting on PATH. The catalog clamp then observed + // the old ladder and stripped max/ultra. + const configDir = tempConfigDir(); + writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0"); + expect(persistedCodexRuntimeIsPinned(loadPersistedCodexRuntime({ configDir }))).toBe(false); + const execFileSync: RuntimeExecFile = (file) => { + const text = String(file); + if (text.includes("old")) return "codex-cli 0.135.0"; + if (text.includes("new")) return "codex-cli 0.153.4"; + return "codex-cli 0.120.0"; + }; + const result = resolveCodexRuntime({ + configDir, + env: { PATH: "C:\\new" }, + platform: "win32", + existsSync: () => true, + execFileSync, + }); + expect(result.runtime.command).toContain("new"); + expect(result.runtime.version).toBe("0.153.4"); + expect(result.supersededDiscovered?.from).toEqual({ + command: "C:\\old\\codex.exe", + version: "0.135.0", + source: "configured", + }); + expect(result.supersededDiscovered?.to.command).toContain("new"); + expect(result.supersededDiscovered?.to.version).toBe("0.153.4"); + expect(result.supersededDiscovered?.reason).toBe( + "discovered runtime 0.135.0 superseded by newer runtime 0.153.4", + ); + expect(result.replacedConfigured).toBeUndefined(); + }); + + test("origin pinned still resolves to 0.135.0 and reports no handover", () => { + const configDir = tempConfigDir(); + writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0", "pinned"); + expect(persistedCodexRuntimeIsPinned(loadPersistedCodexRuntime({ configDir }))).toBe(true); + const execFileSync: RuntimeExecFile = (file) => { + const text = String(file); + if (text.includes("old")) return "codex-cli 0.135.0"; + if (text.includes("new")) return "codex-cli 0.153.4"; + return "codex-cli 0.120.0"; + }; + const result = resolveCodexRuntime({ + configDir, + env: { PATH: "C:\\new" }, + platform: "win32", + existsSync: () => true, + execFileSync, + }); + expect(result.runtime.command).toBe("C:\\old\\codex.exe"); + expect(result.runtime.version).toBe("0.135.0"); + expect(result.supersededDiscovered).toBeUndefined(); + expect(result.newerAvailable?.command).toContain("new"); + expect(result.newerAvailable?.version).toBe("0.153.4"); + }); + + test("origin discovered still yields to a strictly newer runtime", () => { + const configDir = tempConfigDir(); + writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0", "discovered"); + const execFileSync: RuntimeExecFile = (file) => { + const text = String(file); + if (text.includes("old")) return "codex-cli 0.135.0"; + if (text.includes("new")) return "codex-cli 0.153.4"; + return "codex-cli 0.120.0"; + }; + const result = resolveCodexRuntime({ + configDir, + env: { PATH: "C:\\new" }, + platform: "win32", + existsSync: () => true, + execFileSync, + }); + expect(result.runtime.version).toBe("0.153.4"); + expect(result.supersededDiscovered?.reason).toBe( + "discovered runtime 0.135.0 superseded by newer runtime 0.153.4", + ); + }); + + test("an unpinned persisted record with an equal-version alternative sticks", () => { + const configDir = tempConfigDir(); + writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0"); + const execFileSync: RuntimeExecFile = (file) => { + const text = String(file); + if (text.includes("old")) return "codex-cli 0.135.0"; + if (text.includes("new")) return "codex-cli 0.135.0"; + return "codex-cli 0.120.0"; + }; + const result = resolveCodexRuntime({ + configDir, + env: { PATH: "C:\\new" }, + platform: "win32", + existsSync: () => true, + execFileSync, + }); + expect(result.runtime.command).toBe("C:\\old\\codex.exe"); + expect(result.supersededDiscovered).toBeUndefined(); + }); + + test("an unpinned persisted record whose alternative has an unknown version sticks", () => { + // probeVersion === false yields null versions everywhere, so the strictly- + // newer comparison cannot fire. Absence of a version is not evidence of an + // upgrade — the same conservative rule as the in-place clamp diagnostic. + const configDir = tempConfigDir(); + writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0"); + const result = resolveCodexRuntime({ + configDir, + env: { PATH: "C:\\new" }, + platform: "win32", + existsSync: () => true, + execFileSync: () => "codex-cli 0.153.4", + probeVersion: false, + }); + expect(result.runtime.command).toBe("C:\\old\\codex.exe"); + expect(result.runtime.version).toBeNull(); + expect(result.supersededDiscovered).toBeUndefined(); + }); + + test("resolveAndPersistCodexRuntime writes origin discovered; persistCodexRuntime writes pinned", () => { + const discoveredDir = tempConfigDir(); + resolveAndPersistCodexRuntime({ + configDir: discoveredDir, + env: { CODEX_CLI_PATH: "C:\\keep\\codex.exe", PATH: "" }, + platform: "win32", + existsSync: () => true, + execFileSync: () => "codex-cli 0.153.4", + }); + const discovered = loadPersistedCodexRuntime({ configDir: discoveredDir }); + expect(discovered?.origin).toBe("discovered"); + expect(persistedCodexRuntimeIsPinned(discovered)).toBe(false); + + const pinnedDir = tempConfigDir(); + persistCodexRuntime({ + command: "C:\\keep\\codex.exe", + version: "0.153.4", + source: "configured", + }, { configDir: pinnedDir }); + const pinned = loadPersistedCodexRuntime({ configDir: pinnedDir }); + expect(pinned?.origin).toBe("pinned"); + expect(persistedCodexRuntimeIsPinned(pinned)).toBe(true); + }); + + test("parsePersistedCodexRuntime accepts a missing origin and rejects a junk origin", () => { + const base = { + version: 1 as const, + command: "C:\\old\\codex.exe", + source: "configured", + selectedVersion: "0.135.0", + updatedAt: "2026-01-01T00:00:00.000Z", + }; + const withoutOrigin = parsePersistedCodexRuntime(JSON.stringify(base)); + expect(withoutOrigin?.command).toBe("C:\\old\\codex.exe"); + expect(withoutOrigin?.origin).toBeUndefined(); + expect(persistedCodexRuntimeIsPinned(withoutOrigin)).toBe(false); + + expect(parsePersistedCodexRuntime(JSON.stringify({ ...base, origin: "pinned" }))?.origin).toBe("pinned"); + expect(parsePersistedCodexRuntime(JSON.stringify({ ...base, origin: "discovered" }))?.origin).toBe("discovered"); + expect(parsePersistedCodexRuntime(JSON.stringify({ ...base, origin: "accidental" }))).toBeNull(); + }); +}); + +describe("Codex App handover without PATH-wide discovery (issue 4204)", () => { + const LOCAL_APP_DATA = "C:\\Users\\test\\AppData\\Local"; + const APP_ROOT = join(LOCAL_APP_DATA, "OpenAI", "Codex", "bin"); + const APP_EXE = join(APP_ROOT, "0.153.4", "codex.exe"); + const STALE = "C:\\Users\\test\\AppData\\Local\\Programs\\OpenAI\\Codex\\bin\\codex.exe"; + + function appDeps(configDir: string) { + return { + configDir, + env: { LOCALAPPDATA: LOCAL_APP_DATA, PATH: NO_CODEX_PATH }, + platform: "win32" as const, + existsSync: (path: string) => path === STALE || path === APP_EXE, + readdirSync: (path: string) => path === APP_ROOT ? ["0.153.4"] : [], + statSync: (path: string) => path === join(APP_ROOT, "0.153.4") + ? { mtimeMs: 2_000, isDirectory: () => true } + : { mtimeMs: 0, isDirectory: () => false }, + execFileSync: ((file: string) => { + if (String(file) === STALE) return "codex-cli 0.135.0"; + if (String(file) === APP_EXE) return "codex-cli 0.153.4"; + throw Object.assign(new Error("spawn ENOENT"), { code: "ENOENT" }); + }) as RuntimeExecFile, + discoverAlternatives: false as const, + }; + } + + function writePersisted(configDir: string, origin?: "pinned" | "discovered"): void { + const payload: Record = { + version: 1, + command: STALE, + source: "configured", + selectedVersion: "0.135.0", + updatedAt: "2026-01-01T00:00:00.000Z", + }; + if (origin !== undefined) payload.origin = origin; + writeFileSync(join(configDir, "codex-runtime.json"), JSON.stringify(payload)); + } + + test("an unpinned stale pin still yields to the Codex App runtime PATH never exposes", () => { + // This is the arrangement issue 4204 actually reports. The catalog's bundled + // loader passes discoverAlternatives: false, so before this the resolve + // stopped at the still-runnable 0.135.0 under Programs\OpenAI and never + // probed the 0.153.4 the Desktop app was running out of LOCALAPPDATA. + const configDir = tempConfigDir(); + writePersisted(configDir); + const result = resolveCodexRuntime(appDeps(configDir)); + expect(result.runtime.command).toBe(APP_EXE); + expect(result.runtime.version).toBe("0.153.4"); + expect(result.runtime.source).toBe("installed"); + expect(result.supersededDiscovered?.from.version).toBe("0.135.0"); + expect(result.supersededDiscovered?.to.version).toBe("0.153.4"); + }); + + test("a pinned stale selection is left alone even though the App runtime is newer", () => { + const configDir = tempConfigDir(); + writePersisted(configDir, "pinned"); + const result = resolveCodexRuntime(appDeps(configDir)); + expect(result.runtime.command).toBe(STALE); + expect(result.runtime.version).toBe("0.135.0"); + expect(result.supersededDiscovered).toBeUndefined(); + }); + + test("with no persisted record the early stop still skips the installed roots", () => { + // Nothing to supersede means nothing to compare against, so the hot path + // keeps its original cost: first valid candidate wins and the scan ends. + const configDir = tempConfigDir(); + const probed: string[] = []; + const deps = appDeps(configDir); + // A colon-free PATH entry: pathCandidates splits on node's path delimiter, + // which is ":" on the POSIX runners this suite also runs on, so a drive + // letter here would split into two directories that match nothing. + const pathDir = "/opt/on-path"; + const pathExe = join(pathDir, "codex.exe"); + const result = resolveCodexRuntime({ + ...deps, + env: { LOCALAPPDATA: LOCAL_APP_DATA, PATH: pathDir }, + existsSync: (path: string) => path === pathExe || path === APP_EXE, + execFileSync: ((file: string) => { + probed.push(String(file)); + return "codex-cli 0.140.0"; + }) as RuntimeExecFile, + }); + expect(result.runtime.source).toBe("path"); + expect(result.runtime.command).toBe(pathExe); + expect(probed).not.toContain(APP_EXE); + }); +}); From e584fb270221840cf4144b8bd044d337be3ea7da Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 14:51:54 +0900 Subject: [PATCH 3/3] test(codex): keep the PATH-outranks-installed case on a colon-free PATH entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pathCandidates splits PATH on node's delimiter, which is ":" on the POSIX runners this suite also runs on. The Windows-style "C:\on-path" therefore split into "C" and "\on-path", neither of which produced a candidate the fixture's existsSync recognised, so every PATH candidate failed and the installed Codex App runtime won — the exact opposite of what the test asserts. It failed on macOS 1/2 and test 1/4 and passed nowhere the split occurs. The entry is now colon-free, so the directory survives the split on every platform and the test proves the ranking it was written for. --- tests/codex-integration/codex-runtime.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/codex-integration/codex-runtime.test.ts b/tests/codex-integration/codex-runtime.test.ts index c03a3414b8..5991c05f7c 100644 --- a/tests/codex-integration/codex-runtime.test.ts +++ b/tests/codex-integration/codex-runtime.test.ts @@ -1284,7 +1284,12 @@ describe("installed Codex discovery and deferred version probes", () => { const localAppData = "C:\\Users\\test\\AppData\\Local"; const root = join(localAppData, "OpenAI", "Codex", "bin"); const installed = join(root, "app", "codex.exe"); - const pathDir = "C:\\on-path"; + // A colon-free PATH entry. pathCandidates splits PATH on node's delimiter, + // which is ":" on the POSIX runners this suite also runs on, so a drive + // letter here splits into two directories that match no candidate at all — + // every PATH candidate then fails and the installed runtime wins, which is + // the opposite of what this test is for. + const pathDir = "/opt/on-path"; const pathCommand = join(pathDir, "codex.exe"); const result = resolveCodexRuntime({ configDir: tempConfigDir(),