fix(codex): preserve ambiguous legacy runtime pins

src/codex/runtime.ts

@@ -109,10 +109,9 @@ export interface ResolveCodexRuntimeDeps {
  * 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.
+ * automatic resolve-and-persist. Absent is the pre-field shape and is
+ * ambiguous — both paths wrote it — so it is read conservatively as a possible
+ * operator pin rather than surrendered to the next automatic resolution.
  */
 export type CodexRuntimePinOrigin = "pinned" | "discovered";
 
@@ -327,18 +326,16 @@ export function loadPersistedCodexRuntime(
 }
 
 /**
- * True only when the operator intentionally pinned this runtime.
+ * True unless the record explicitly identifies an automatically discovered 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.
+ * Records without an origin predate provenance tracking and are ambiguous:
+ * both automatic discovery and `doctor --fix-codex-runtime` wrote that shape.
+ * Preserve the operator's possible explicit choice rather than replacing it.
  */
 export function persistedCodexRuntimeIsPinned(
   state: DeepReadonly<PersistedCodexRuntimeState> | null | undefined,
 ): boolean {
-  return state?.origin === "pinned";
+  return state != null && state.origin !== "discovered";
 }
 
 /**
@@ -890,7 +887,7 @@ function resolveCodexRuntimeUncached(deps: ResolveCodexRuntimeDeps = {}): Resolv
       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
+      // An explicitly discovered record 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
@@ -943,13 +940,27 @@ export function resolveAndPersistCodexRuntime(
   // 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
+  const selectionMatches = persistedRuntime !== null
     && persistedRuntime.command === result.runtime.command
     && persistedRuntime.source === result.runtime.source
     && (persistedRuntime.selectedVersion ?? null) === (result.runtime.version ?? null);
+  // Origin-less records may have been written by the legacy doctor fix path.
+  // Backfill an unchanged record as pinned so its conservative interpretation
+  // is durable instead of leaving the provenance ambiguity in place.
+  const selectionUnchanged = selectionMatches && persistedRuntime.origin !== undefined;
   if (result.runtime.command && result.runtime.source !== "fallback" && !selectionUnchanged) {
     try {
-      persistCodexRuntime(result.runtime, deps, "discovered");
+      // Provenance follows the command, not the observed metadata. The binary
+      // at a pinned path can be upgraded in place (a version mismatch), and a
+      // persisted command is re-probed as the `configured` candidate whatever
+      // source the record stored, so neither mismatch demotes a pin. While the
+      // persisted command keeps the seat, pinned and ambiguous origin-less
+      // records stay pinned; only a different selected command, or a record
+      // already marked discovered, writes "discovered".
+      const retainedPin = persistedRuntime !== null
+        && persistedRuntime.command === result.runtime.command
+        && persistedCodexRuntimeIsPinned(persistedRuntime);
+      persistCodexRuntime(result.runtime, deps, retainedPin ? "pinned" : "discovered");
     } catch (error) {
       const message = error instanceof Error ? error.message : String(error);
       const persistError = redactUserPath(redactSecretString(message)).slice(0, 200);

structure/runtime.md

@@ -74,6 +74,13 @@ The prefilter is only an optimization, not final process-membership authority.
 
 ## Explicit Codex CLI installation observation
 
+Codex runtime selection state records whether a choice was explicitly pinned or
+automatically discovered. Legacy records lack that provenance — both discovery and
+`doctor --fix-codex-runtime` wrote them — so resolution retains them as pins, backfills
+`origin: "pinned"`, and keeps the persisted command selected: in-place binary upgrades
+and source normalization to `configured` never demote a pin. Only an explicit
+`origin: "discovered"` record may hand over to a strictly newer discovered runtime.
+
 `src/cli/codex-cli-update.ts` dispatches the opt-in Windows x64 `attest` operation to
 `src/codex/cli-installation-identity.ts`. With no options, `src/codex/cli-installation-targets.ts`
 derives the four inputs from the proof-bound launcher snapshot: the configured candidate or

tests/codex-integration/codex-runtime.test.ts

@@ -846,7 +846,7 @@ describe("resolveCodexRuntime", () => {
     expect(loadPersistedCodexRuntime({ configDir })?.updatedAt).toBe(firstStamp);
   });
 
-  test("treats missing persisted and resolved versions as the same selection", () => {
+  test("backfills an origin-less matching selection as pinned when both versions are missing", () => {
     const configDir = tempConfigDir();
     const statePath = join(configDir, "codex-runtime.json");
     writeFileSync(statePath, JSON.stringify({
@@ -868,11 +868,12 @@ describe("resolveCodexRuntime", () => {
         runtime: { command: "codex", version: null, source: "environment" },
         failures: cached.failures,
       }, deps);
-      const before = readFileSync(statePath, "utf8");
-
       resolveAndPersistCodexRuntime(deps);
 
-      expect(readFileSync(statePath, "utf8")).toBe(before);
+      const migrated = loadPersistedCodexRuntime({ configDir });
+      expect(migrated?.command).toBe("codex");
+      expect(migrated?.selectedVersion).toBeNull();
+      expect(migrated?.origin).toBe("pinned");
     } finally {
       if (previousHome === undefined) delete process.env.OPENCODEX_HOME;
       else process.env.OPENCODEX_HOME = previousHome;
@@ -1337,52 +1338,147 @@ describe("unpinned discovered runtime handover (issue 4204)", () => {
     command: string,
     selectedVersion: string,
     origin?: "pinned" | "discovered",
+    source: string = "configured",
   ): void {
     const payload: Record<string, unknown> = {
       version: 1,
       command,
-      source: "configured",
+      source,
       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.
+  test("an origin-less legacy runtime is conservatively retained as a possible operator pin", () => {
+    // Before provenance tracking, both automatic resolution and the deliberate
+    // doctor fix path wrote this shape. The resolver cannot safely distinguish
+    // them, so it must preserve the possible operator selection.
     const configDir = tempConfigDir();
     writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0");
-    expect(persistedCodexRuntimeIsPinned(loadPersistedCodexRuntime({ configDir }))).toBe(false);
+    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({
+    const result = resolveAndPersistCodexRuntime({
       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.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");
     expect(result.replacedConfigured).toBeUndefined();
+    expect(loadPersistedCodexRuntime({ configDir })?.origin).toBe("pinned");
+  });
+
+  test("an in-place binary upgrade keeps an explicit pin pinned", () => {
+    // The file at a pinned path can be replaced by a newer build without the
+    // path changing. The version mismatch must not demote the record to
+    // discovered, or a later resolve could hand the pin to a newer candidate.
+    const configDir = tempConfigDir();
+    writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0", "pinned");
+    const execFileSync: RuntimeExecFile = (file) => {
+      const text = String(file);
+      if (text.includes("old")) return "codex-cli 0.154.0";
+      if (text.includes("new")) return "codex-cli 0.155.0";
+      return "codex-cli 0.120.0";
+    };
+    const deps = {
+      configDir,
+      env: { PATH: "C:\\new" },
+      platform: "win32" as const,
+      existsSync: () => true,
+      execFileSync,
+    };
+    const result = resolveAndPersistCodexRuntime(deps);
+    expect(result.runtime.command).toBe("C:\\old\\codex.exe");
+    expect(result.runtime.version).toBe("0.154.0");
+    const persisted = loadPersistedCodexRuntime({ configDir });
+    expect(persisted?.selectedVersion).toBe("0.154.0");
+    expect(persisted?.origin).toBe("pinned");
+
+    // The surviving pin must still refuse handover on the next resolution.
+    const next = resolveCodexRuntime(deps);
+    expect(next.runtime.command).toBe("C:\\old\\codex.exe");
+    expect(next.supersededDiscovered).toBeUndefined();
+  });
+
+  test("an in-place binary upgrade backfills an origin-less record as pinned", () => {
+    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.154.0";
+      if (text.includes("new")) return "codex-cli 0.155.0";
+      return "codex-cli 0.120.0";
+    };
+    const deps = {
+      configDir,
+      env: { PATH: "C:\\new" },
+      platform: "win32" as const,
+      existsSync: () => true,
+      execFileSync,
+    };
+    const result = resolveAndPersistCodexRuntime(deps);
+    expect(result.runtime.command).toBe("C:\\old\\codex.exe");
+    const persisted = loadPersistedCodexRuntime({ configDir });
+    expect(persisted?.selectedVersion).toBe("0.154.0");
+    expect(persisted?.origin).toBe("pinned");
+
+    const next = resolveCodexRuntime(deps);
+    expect(next.runtime.command).toBe("C:\\old\\codex.exe");
+    expect(next.supersededDiscovered).toBeUndefined();
+  });
+
+  test("an origin-less record stored under a discovery source stays pinned", () => {
+    // Pre-provenance files store the source the original resolve observed.
+    // The persisted command is re-probed as the `configured` candidate, so the
+    // stored source normalizes on the next write — that mismatch must not
+    // demote the ambiguous record either.
+    const configDir = tempConfigDir();
+    writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0", undefined, "path");
+    const result = resolveAndPersistCodexRuntime({
+      configDir,
+      env: { PATH: "C:\\new" },
+      platform: "win32",
+      existsSync: () => true,
+      execFileSync: (file) =>
+        String(file).includes("old") ? "codex-cli 0.135.0" : "codex-cli 0.153.4",
+    });
+    expect(result.runtime.command).toBe("C:\\old\\codex.exe");
+    const persisted = loadPersistedCodexRuntime({ configDir });
+    expect(persisted?.origin).toBe("pinned");
+    expect(persisted?.source).toBe("configured");
+  });
+
+  test("an in-place binary upgrade keeps a discovered record discovered", () => {
+    const configDir = tempConfigDir();
+    writeLegacyPersisted(configDir, "C:\\old\\codex.exe", "0.135.0", "discovered");
+    const result = resolveAndPersistCodexRuntime({
+      configDir,
+      env: { PATH: "C:\\new" },
+      platform: "win32",
+      existsSync: () => true,
+      execFileSync: (file) => {
+        const text = String(file);
+        if (text.includes("old")) return "codex-cli 0.154.0";
+        if (text.includes("new")) return "codex-cli 0.153.4";
+        return "codex-cli 0.120.0";
+      },
+    });
+    expect(result.runtime.command).toBe("C:\\old\\codex.exe");
+    const persisted = loadPersistedCodexRuntime({ configDir });
+    expect(persisted?.selectedVersion).toBe("0.154.0");
+    expect(persisted?.origin).toBe("discovered");
   });
 
   test("origin pinned still resolves to 0.135.0 and reports no handover", () => {
@@ -1505,7 +1601,7 @@ describe("unpinned discovered runtime handover (issue 4204)", () => {
     const withoutOrigin = parsePersistedCodexRuntime(JSON.stringify(base));
     expect(withoutOrigin?.command).toBe("C:\\old\\codex.exe");
     expect(withoutOrigin?.origin).toBeUndefined();
-    expect(persistedCodexRuntimeIsPinned(withoutOrigin)).toBe(false);
+    expect(persistedCodexRuntimeIsPinned(withoutOrigin)).toBe(true);
 
     expect(parsePersistedCodexRuntime(JSON.stringify({ ...base, origin: "pinned" }))?.origin).toBe("pinned");
     expect(parsePersistedCodexRuntime(JSON.stringify({ ...base, origin: "discovered" }))?.origin).toBe("discovered");
@@ -1556,7 +1652,7 @@ describe("Codex App handover without PATH-wide discovery (issue 4204)", () => {
     // 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);
+    writePersisted(configDir, "discovered");
     const result = resolveCodexRuntime(appDeps(configDir));
     expect(result.runtime.command).toBe(APP_EXE);
     expect(result.runtime.version).toBe("0.153.4");