diff --git a/src/cli/connect.ts b/src/cli/connect.ts index b3986f7df24..9c40994b3c7 100644 --- a/src/cli/connect.ts +++ b/src/cli/connect.ts @@ -1,7 +1,7 @@ import { existsSync, lstatSync, readFileSync } from "node:fs"; import { DEFAULT_CATALOG_PATH } from "../codex/paths"; import { codexSupportedReasoningEfforts } from "../codex/catalog/effort"; -import { resolveCodexRuntime } from "../codex/runtime"; +import { resolveCodexRuntime, type ResolvedCodexRuntime } from "../codex/runtime"; import { inspectClientCatalogReadiness, type CatalogCompatibilityDeps, @@ -61,14 +61,14 @@ export interface ClientCatalogProbeDeps extends CatalogCompatibilityDeps { /** Injected in tests; defaults to reading the materialized client catalog off disk. */ readCatalogBody?: () => string | null; /** - * A Codex command the caller already resolved, handed over so readiness skips resolving it + * The Codex runtime the caller already resolved, handed over so readiness skips resolving it * again. General `ocx status` resolves the full runtime for its diagnostics block; the resolver * memo is keyed by discovery scope and holds one entry, so a priority-only readiness resolve * and the full one miss each other and re-probe the same command with `--version` — up to eight - * seconds apiece. Passing the command across adds no cache state, and it cannot disagree with + * seconds apiece. Passing the selection across adds no cache state, and it cannot disagree with * what status prints because it is the selection status is printing. */ - selectedCodexCommand?: string; + selectedCodexRuntime?: ResolvedCodexRuntime; } export const CONNECT_USAGE = `Usage: @@ -139,12 +139,14 @@ function readInstalledCatalogBody(): string | null { * to the catalog probe: readiness does not consume alternative-runtime diagnostics. The resolver * keeps this priority-only cache separate from the full discovery used by `ocx status`. * - * A caller that has already resolved passes its selection in through `selectedCodexCommand` rather + * A caller that has already resolved passes its selection in through `selectedCodexRuntime` rather * than paying for a second `--version` probe of the command it just resolved. */ -function observeLocalCodexEffortLadder(selected?: string): ReadonlySet | null { - const command = selected ?? resolveCodexRuntime({ discoverAlternatives: false }).runtime.command; - return codexSupportedReasoningEfforts({ commandCandidates: () => [command] }); +function observeLocalCodexEffortLadder(selected?: ResolvedCodexRuntime): ReadonlySet | null { + const runtime = selected ?? resolveCodexRuntime({ discoverAlternatives: false }).runtime; + // The selection travels with its command so the probe can tell an unresolved sentinel, which may + // fall back to the ChatGPT.app CLI on macOS (#8360), from a bare command that actually ran. + return codexSupportedReasoningEfforts({ commandCandidates: () => [runtime.command], selectedRuntime: runtime }); } /** @@ -154,8 +156,8 @@ function observeLocalCodexEffortLadder(selected?: string): ReadonlySet | * would run its own probe, which is how the two checks could disagree about the ladder within * a single `ocx connect`. */ -function catalogObserver(deps: ClientCatalogProbeDeps | undefined): CatalogCompatibilityDeps { - const selected = deps?.selectedCodexCommand; +export function catalogObserver(deps: ClientCatalogProbeDeps | undefined): CatalogCompatibilityDeps { + const selected = deps?.selectedCodexRuntime; return { supportedEfforts: deps?.supportedEfforts ?? (() => observeLocalCodexEffortLadder(selected)) }; } diff --git a/src/cli/status.ts b/src/cli/status.ts index 7d1f00c2965..bc4e51522bb 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -912,7 +912,7 @@ export async function collectStatus(options: { mainAccountPolicy?: boolean } = { } })(); const clientConnection = collectClientConnectionStatus(Date.now(), undefined, { - selectedCodexCommand: resolvedRuntime.runtime.command, + selectedCodexRuntime: resolvedRuntime.runtime, }); // Asked before the local probes below so a connected client's report is hub-sourced from its // first line. Bounded and failure-tolerant: an offline hub degrades the remoteHub block, it diff --git a/src/codex/catalog/bundled.ts b/src/codex/catalog/bundled.ts index 09178baa28e..04355fb1877 100644 --- a/src/codex/catalog/bundled.ts +++ b/src/codex/catalog/bundled.ts @@ -48,6 +48,7 @@ import { codexRuntimeSelectionIdentity, codexRuntimeStateEpoch, execCodexFileAsync, + codexRuntimeIsUnresolved, } from "../runtime"; import type { DeepReadonly, @@ -190,6 +191,12 @@ export interface BundledCatalogDeps { /** Already-observed source for server clamping; null never triggers a probe. */ observedCatalog?: ReadonlyRawCatalog | null; commandCandidates?: () => string[]; + /** + * The resolver selection `commandCandidates` came from, when the caller resolved first. Only an + * unresolved selection lets the macOS probe add the ChatGPT.app CLI (#8360); without it, injected + * candidates are probed as given. + */ + selectedRuntime?: Pick; execFileSync?: ExecFile; execFile?: (...args: Parameters) => Promise; onEffortClamp?: (diagnostic: EffortClampDiagnostic) => void; @@ -358,9 +365,11 @@ function bundledCatalogCandidates( // An explicit empty list asks for no probe at all. if (candidates.length === 0) return candidates; if ((deps.platform ?? process.platform) !== "darwin") return candidates; - // Only the unresolved sentinel is a bare name; a PATH or installed selection is absolute, and - // that selection is the runtime the operator is already running, not a gap to fill. - if (candidates.some(command => command.includes("/") || command.includes("\\"))) return candidates; + // Decide from the resolver's own result, not from the command's shape: a bare command that ran + // (a PATH fallback or a bare CODEX_CLI_PATH) is the operator's runtime, and borrowing the app's + // catalog for it would report a ladder that runtime may reject. + const selection = resolved?.runtime ?? deps.selectedRuntime; + if (!selection || !codexRuntimeIsUnresolved(selection)) return candidates; const exists = deps.existsSync ?? existsSync; return [...candidates, ...macCodexAppCommands(deps).filter(command => exists(command))]; } diff --git a/src/codex/runtime.ts b/src/codex/runtime.ts index 8de8bed5e17..9c6d6fccffd 100644 --- a/src/codex/runtime.ts +++ b/src/codex/runtime.ts @@ -55,6 +55,18 @@ export interface EffortClampDiagnostic { readonly affectedModels: readonly string[]; } +/** + * True for the selection the resolver returns when no candidate ran: the bare `codex` fallback + * with no version. A probed candidate always carries the version it parsed, so a bare command + * that actually ran (a bare CODEX_CLI_PATH or shim, or the bare fallback itself) is not this + * sentinel. Deferred resolution (`probeVersion: false`) gives every candidate a null version and + * so cannot be told apart here; the bundled-catalog probe, `ocx connect` and `ocx status` all + * resolve with probing. + */ +export function codexRuntimeIsUnresolved(runtime: Pick): boolean { + return runtime.source === "fallback" && runtime.version === null; +} + export interface ResolveCodexRuntimeResult { readonly runtime: ResolvedCodexRuntime; readonly failures: readonly RuntimeProbeFailure[]; diff --git a/tests/cli/cli-connect-readiness.test.ts b/tests/cli/cli-connect-readiness.test.ts index c46913c9576..c4833ff7bf7 100644 --- a/tests/cli/cli-connect-readiness.test.ts +++ b/tests/cli/cli-connect-readiness.test.ts @@ -831,3 +831,28 @@ describe("#4451 the connected-sync refresh shares the connect terminal boundary" expect(thrown.name).toBe("ClientCatalogIncompatibleError"); }); }); + +/** + * #8360 follow-up: readiness may borrow the ChatGPT.app catalog only for an unresolved selection, + * which the bundled loader can tell only if connect forwards the selection with its command. + */ +describe("connect readiness forwards the selected runtime to the catalog probe", () => { + test.each([ + { command: "codex", version: null, source: "fallback" }, + { command: "codex", version: "0.140.0", source: "fallback" }, + { command: "/usr/local/bin/codex", version: "0.150.0", source: "path" }, + ] as const)("selection %o", async selected => { + const effort = await import("../../src/codex/catalog/effort"); + const { catalogObserver } = await import("../../src/cli/connect"); + const seen: { candidates?: string[]; selectedRuntime?: unknown }[] = []; + const spy = spyOn(effort, "codexSupportedReasoningEfforts").mockImplementation(deps => { + seen.push({ candidates: deps?.commandCandidates?.(), selectedRuntime: deps?.selectedRuntime }); + return new Set(OLD_CLI); + }); + try { + expect(catalogObserver({ selectedCodexRuntime: selected }).supportedEfforts?.()).toEqual(new Set(OLD_CLI)); + } finally { spy.mockRestore(); } + expect(seen).toEqual([{ candidates: [selected.command], selectedRuntime: selected }]); + }); +}); + diff --git a/tests/codex-integration/codex-runtime-macos-desktop.test.ts b/tests/codex-integration/codex-runtime-macos-desktop.test.ts index a1ebf35ee8c..4624dd0d235 100644 --- a/tests/codex-integration/codex-runtime-macos-desktop.test.ts +++ b/tests/codex-integration/codex-runtime-macos-desktop.test.ts @@ -327,7 +327,8 @@ describe("bundled app catalog consumer", () => { * selection on the bare `codex` sentinel. The readiness probe then asked a command that does * not exist while a real, trusted Codex sat inside the app. */ -describe("app-bundled CLI fallback for connect readiness (#8360)", () => { +// The virtual app paths below are POSIX; on Windows the exec boundary treats them as relative. +describe.skipIf(process.platform === "win32")("app-bundled CLI fallback for connect readiness (#8360)", () => { /** A catalog with a distinctive ladder, so a warm fixture value cannot pass by accident. */ const bundledCatalog = JSON.stringify({ models: [{ slug: "gpt-5.6-sol", @@ -357,6 +358,7 @@ describe("app-bundled CLI fallback for connect readiness (#8360)", () => { const efforts = codexSupportedReasoningEfforts({ ...f.deps, execFileSync: exec, commandCandidates: () => [selected.runtime.command], + selectedRuntime: selected.runtime, }); expect(efforts).toEqual(new Set(["low", "high"])); expect(inspectClientCatalogReadiness("present", bundledCatalog, { supportedEfforts: () => efforts })) @@ -371,6 +373,7 @@ describe("app-bundled CLI fallback for connect readiness (#8360)", () => { const exec = bundledOnlyExec(f.current, spawned); const efforts = codexSupportedReasoningEfforts({ ...f.deps, execFileSync: exec, commandCandidates: () => ["codex"], + selectedRuntime: { command: "codex", version: null, source: "fallback" }, }); expect(efforts).toBeNull(); // No app binary exists, so none is offered; the bare sentinel cannot resolve at the exec @@ -390,10 +393,34 @@ describe("app-bundled CLI fallback for connect readiness (#8360)", () => { }; expect(codexSupportedReasoningEfforts({ ...f.deps, execFileSync: exec, commandCandidates: () => [standalone], + selectedRuntime: { command: standalone, version: "0.167.0", source: "installed" }, })).toEqual(new Set(["low", "high"])); expect(spawned).toEqual([standalone]); }); + test("a selected runnable bare command keeps its own answer instead of borrowing the app's ladder", async () => { + const f = fixture(); + f.versions.set(f.current, "0.166.0-alpha.6"); + const spawned: string[] = []; + // Only the app answers. The selected bare command was runnable when it was resolved (it has a + // version), so it is the operator's runtime rather than the unresolved sentinel: its failure to + // report a catalog must leave the ladder unverified, not substitute the app's. + const exec = bundledOnlyExec(f.current, spawned); + for (const selectedRuntime of [ + { command: "codex", version: "0.140.0", source: "fallback" }, + { command: "codex-beta", version: "0.141.0", source: "environment" }, + ] as const) { + expect(codexSupportedReasoningEfforts({ + ...f.deps, execFileSync: exec, commandCandidates: () => [selectedRuntime.command], selectedRuntime, + })).toBeNull(); + expect(await loadBundledCodexCatalogAsync({ + ...f.deps, commandCandidates: () => [selectedRuntime.command], selectedRuntime, + execFileSync: exec as never, execFile: async (file, args) => exec(file, args), + })).toBeNull(); + } + expect(spawned).not.toContain(f.current); + }); + test("the async bundled-catalog driver widens a bare sentinel the same way", async () => { const f = fixture(); f.versions.set(f.current, "0.166.0-alpha.6"); @@ -402,6 +429,7 @@ describe("app-bundled CLI fallback for connect readiness (#8360)", () => { const catalog = await loadBundledCodexCatalogAsync({ ...f.deps, commandCandidates: () => ["codex"], + selectedRuntime: { command: "codex", version: null, source: "fallback" }, execFileSync: exec as never, execFile: async (file, args) => exec(file, args), });