fix(codex): reject conflicting retained provider table

src/codex/inject/remove.ts

@@ -133,7 +133,15 @@ export function extractOcxProviderTableBlock(content: string): string | null {
  * only do if the append is a transform rather than a second file operation.
  */
 export function appendOcxProviderTableBlock(content: string, block: string): string {
-  if (hasOcxProviderTable(content)) return content;
+  if (hasOcxProviderTable(content)) {
+    const existing = extractOcxProviderTableBlock(content);
+    if (existing !== block.replace(/\n+$/, "") + "\n") {
+      throw new Error(
+        "Codex restore refused: the native config already defines a different [model_providers.opencodex] table.",
+      );
+    }
+    return content;
+  }
   return `${content.replace(/\n+$/, "")}\n\n${block.replace(/\n+$/, "")}\n`;
 }
 

structure/codex-home.md

@@ -323,7 +323,7 @@ On apply, that reason retires the relabel unit and the config/profile/journal wr
 
 `history_paginated_openai_requires_native_writer` is the one apply-side reason that is neither of those. It means a provider-table transition found an `openai`-tagged row already paginated, and the transition as planned would take the root `openai_base_url` out from under it. `applyPaginatedOpenaiCompat` in `src/codex/inject/paginated-openai-compat.ts` resolves it in the same window as the provider-table retention above, before the witness: it keeps the marker-owned root override beside the table and downgrades the reason to the stand-down constant, so the relabel unit never starts and the paginated row is neither read nor written. The retained line is journaled as OpenCodex's own, which is what lets restore remove it later; a line the user owns is left in place and journaled as theirs. Only an admission-token form still refuses, because Codex's built-in `openai` entry cannot carry `x-opencodex-api-key`, and that refusal names the configuration that resolves it rather than telling the operator not to retry (#5321).
 
-On restore and removal, that same reason no longer refuses the config half. It selects a degraded restore: every OpenCodex root routing key comes out, `[model_providers.opencodex]` is retained verbatim including its ownership marker, and the history relabel is skipped rather than attempted. The retained table is captured from the pre-transform bytes and re-appended into the same buffer, so the write is one atomic transformation — a config carrying root `model_provider = "opencodex"` without a matching table fails the whole Codex config load, not one thread, which makes that intermediate state strictly worse than the routing it replaces. `resolveRestoreHistoryDisposition` in `src/codex/inject/restore.ts` is the single place that reads the preflight reason and answers the separate question of whether routing may come out. Every other reason keeps the hard refusal and compensates on every artifact, because retiring a provider definition its thread rows still name would orphan them. A failed config restore stops catalog/history work; coordinated restore rolls back its published remove transition. Legacy first-line provider patches are bound to the validated file identity before and after writing. These compensating checks do not provide a native-writer lock or authorize external ordinal allocation.
+On restore and removal, that same reason no longer refuses the config half. It selects a degraded restore: every OpenCodex root routing key comes out, `[model_providers.opencodex]` is retained verbatim including its ownership marker, and the history relabel is skipped rather than attempted. The retained table is captured from the pre-transform bytes and re-appended into the same buffer, so the write is one atomic transformation — a config carrying root `model_provider = "opencodex"` without a matching table fails the whole Codex config load, not one thread, which makes that intermediate state strictly worse than the routing it replaces. If an exact journal restore brings back a same-named provider table, retention accepts it only when the extracted table block matches the captured one — the extractor collapses blank-line runs and trims the block tail, so this is a normalized comparison, not a raw-byte one; a different table fails the restore and compensation reinstates the pre-restore files rather than rebinding tagged threads to another destination. `resolveRestoreHistoryDisposition` in `src/codex/inject/restore.ts` is the single place that reads the preflight reason and answers the separate question of whether routing may come out. Every other reason keeps the hard refusal and compensates on every artifact, because retiring a provider definition its thread rows still name would orphan them. A failed config restore stops catalog/history work; coordinated restore rolls back its published remove transition. Legacy first-line provider patches are bound to the validated file identity before and after writing. These compensating checks do not provide a native-writer lock or authorize external ordinal allocation.
 
 The legacy external writer is now refused for affected rows in any store whose schema includes history_mode, even while their row mode is still legacy. This deliberately sacrifices automatic relabeling on migration-capable stores rather than racing native conversion. It no longer costs the home its ability to be uninstalled: synchronous and asynchronous restore, inline journal restore, and direct config removal all take routing down on that reason while keeping the provider table, so an already-paginated home can be stopped and uninstalled and plain `codex` returns to the built-in provider. Rows naming `opencodex` still resolve through the retained table; their requests reach a proxy that is gone and fail with an ordinary connection error, which is a per-conversation failure rather than a broken config. `ocx restore --remove-codex-provider-table` removes the table for a user who accepts that those conversations stop opening; nothing selects it implicitly.
 

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

@@ -162,6 +162,75 @@ describe("injectCodexConfig integration (Design B)", () => {
     expect(result.historyPreserved).toBe(true);
   });
 
+  test("a journaled same-named foreign table aborts restore and reinstates the pre-restore config", () => {
+    // The user's pre-injection config carries its own [model_providers.opencodex] table.
+    // Injection strips it (it is not ours) but journals it as the user's baseline, so the
+    // journal restore replays it verbatim. Retention then sees a table that differs from
+    // the captured block, refuses to rebind tagged threads, and the failed config artifact
+    // makes the caller roll every file back to its pre-restore bytes.
+    const configPath = join(codexHome, "config.toml");
+    const journalPath = join(codexHome, "opencodex-journal.json");
+    writeFileSync(configPath, [
+      'model="test"',
+      "",
+      "[model_providers.opencodex]",
+      'name="Unrelated Provider"',
+      'base_url="https://unrelated.invalid/v1"',
+      "",
+    ].join("\n"));
+    const seed = runInject(codexHome, ocxHome, JSON.stringify({ codexClientCompaction: true }));
+    expect(seed.status).toBe(0);
+    expect(JSON.parse(seed.stdout).success).toBe(true);
+    const configBefore = readFileSync(configPath, "utf8");
+    expect(configBefore).not.toContain("unrelated.invalid");
+    const script = `
+      const fs = require("node:fs");
+      const { join } = require("node:path");
+      const { Database } = require("bun:sqlite");
+      const { restoreNativeCodex } = require("./src/codex/inject");
+      const { syncCodexHistoryProvider, historyBackupPathFor } = require("./src/codex/history-provider");
+      const dbPath = require("./src/codex/paths").resolveCodexStateDbPath();
+      const rollout = join(process.env.CODEX_HOME, "manifest-fixture.jsonl");
+      fs.writeFileSync(rollout, JSON.stringify({type:"session_meta",payload:{id:"fixture",model_provider:"openai",source:"cli"}})+String.fromCharCode(10));
+      const db = new Database(dbPath);
+      db.run("CREATE TABLE threads (id TEXT PRIMARY KEY, rollout_path TEXT, model_provider TEXT, source TEXT, first_user_message TEXT, has_user_event INTEGER)");
+      db.run("INSERT INTO threads VALUES ('fixture', ?, 'openai', 'cli', 'hello', 1)", rollout);
+      const routed = syncCodexHistoryProvider("opencodex", dbPath);
+      if (routed.failed || routed.rows !== 1) throw new Error("fixture history route failed");
+      db.run("ALTER TABLE threads ADD COLUMN history_mode TEXT DEFAULT 'legacy'");
+      db.close();
+      const historyPaths = [historyBackupPathFor(dbPath), rollout];
+      const beforeHistory = historyPaths.map(p=>fs.readFileSync(p,"utf8"));
+      const result = restoreNativeCodex();
+      const restoredDb = new Database(dbPath, { readonly: true });
+      const provider = restoredDb.query("SELECT model_provider FROM threads WHERE id='fixture'").get().model_provider;
+      restoredDb.close();
+      const configAfter = fs.readFileSync(join(process.env.CODEX_HOME,"config.toml"),"utf8");
+      console.log(JSON.stringify({
+        result,
+        provider,
+        configAfter,
+        journalExists: fs.existsSync(join(process.env.CODEX_HOME,"opencodex-journal.json")),
+        historyPreserved: historyPaths.every((p,i)=>fs.readFileSync(p,"utf8")===beforeHistory[i]),
+      }));
+    `;
+    const child = spawnSync(process.execPath, ["--eval", script], {
+      cwd: repoRoot, env: { ...process.env, CODEX_HOME: codexHome, CODEX_SQLITE_HOME: "", OPENCODEX_HOME: ocxHome },
+      encoding: "utf8", timeout: SPAWN_BUDGET_MS - 5_000,
+    });
+    expect(child.status, child.stderr).toBe(0);
+    const result = JSON.parse(child.stdout);
+    expect(result.result.success).toBe(false);
+    expect(result.result.artifacts.config).toMatchObject({ state: "failed", action: "failed" });
+    expect(result.result.artifacts.config.message).toContain(
+      "native config already defines a different [model_providers.opencodex] table",
+    );
+    expect(result.configAfter).toBe(configBefore);
+    expect(result.journalExists).toBe(true);
+    expect(result.provider).toBe("opencodex");
+    expect(result.historyPreserved).toBe(true);
+  });
+
   // The denial has to be a real filesystem permission. `inject-coordination.ts`
   // binds `readFileSync` as an ESM named import, so `spyOn(fs, "readFileSync")`
   // on the child's `require("node:fs")` handle never reached it: the mock

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

@@ -21,7 +21,10 @@ import {
   buildProviderTableBlockForTarget,
   resolveCodexProviderDisplayName,
 } from "../../src/codex/inject/config-toml";
-import { extractOcxProviderTableBlock } from "../../src/codex/inject/remove";
+import {
+  appendOcxProviderTableBlock,
+  extractOcxProviderTableBlock,
+} from "../../src/codex/inject/remove";
 import { OCX_ROUTING_MARKER_LINE, OCX_SECTION_MARKER, stripJournaledOpenaiBaseUrl } from "../../src/codex/injected-marker";
 import {
   MANAGED_AGENTS_TABLE_MARKER,
@@ -711,6 +714,52 @@ describe("Design B openai_base_url injection", () => {
     ].join("\n"));
   });
 
+  test("provider-table retention refuses to rebind tagged threads to a different table", () => {
+    const captured = [
+      "# Auto-injected by opencodex",
+      "[model_providers.opencodex]",
+      'name = "OpenCodex Proxy"',
+      'base_url = "http://127.0.0.1:10100/v1"',
+      "",
+    ].join("\n");
+    const restored = [
+      "[model_providers.opencodex]",
+      'name = "Unrelated Provider"',
+      'base_url = "https://unrelated.invalid/v1"',
+      "",
+    ].join("\n");
+
+    expect(() => appendOcxProviderTableBlock(restored, captured)).toThrow(
+      "native config already defines a different [model_providers.opencodex] table",
+    );
+    expect(appendOcxProviderTableBlock(captured, captured)).toBe(captured);
+  });
+
+  test("provider-table retention accepts a table that differs only in blank-line count", () => {
+    // The comparison is intentionally on the extracted table string, not raw bytes:
+    // extractOcxProviderTableBlock collapses blank-line runs and trims the block tail,
+    // so a cosmetic blank-line edit keeps the user's own bytes instead of refusing.
+    const captured = [
+      "# Auto-injected by opencodex",
+      "[model_providers.opencodex]",
+      'name = "OpenCodex Proxy"',
+      'base_url = "http://127.0.0.1:10100/v1"',
+      "",
+    ].join("\n");
+    const current = [
+      "# Auto-injected by opencodex",
+      "[model_providers.opencodex]",
+      'name = "OpenCodex Proxy"',
+      'base_url = "http://127.0.0.1:10100/v1"',
+      "",
+      "",
+      "",
+    ].join("\n");
+
+    expect(extractOcxProviderTableBlock(current)).toBe(captured);
+    expect(appendOcxProviderTableBlock(current, captured)).toBe(current);
+  });
+
   test("legacy marker directly before the provider table survives the root strip order (removeOcxSection keeps its anchor)", () => {
     // No Design B form present — stripInjectedOpenaiBaseUrl must not eat the legacy EOF marker
     // in a way that leaves the [model_providers.opencodex] table behind.