From d8b37cc250954060acc21486a271a6c41b10f7df Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Thu, 20 Aug 2026 23:02:20 +0900 Subject: [PATCH 1/2] fix(codex): refresh journaled injection ownership --- src/codex/inject.ts | 10 ++++++- src/codex/journal.ts | 24 +++++++++++----- tests/codex-journal.test.ts | 35 ++++++++++++++++++++++- tests/codex-restore-app-rewrite.test.ts | 38 +++++++++++++++++++++++-- 4 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/codex/inject.ts b/src/codex/inject.ts index e137d1567b..e7e123c6e3 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -894,7 +894,15 @@ export async function injectCodexConfig( }); atomicWriteFile(CODEX_CONFIG_PATH, content); atomicWriteFile(CODEX_PROFILE_PATH, profileContent); - markJournalInjectedState(content, profileContent); + markJournalInjectedState(content, profileContent, { + // A root override is ours only in loopback Design B when no user-owned value won. + injectedOpenaiBaseUrl: legacyMode || keptUserBaseUrl + ? null + : rootTomlString(content, "openai_base_url"), + // This is the catalog artifact selected for this injection, even when config.toml + // already points at that path and therefore needs no textual rewrite. + injectedCatalogPath: catalogPath, + }); }; /* diff --git a/src/codex/journal.ts b/src/codex/journal.ts index a1a34806c9..f6b6e29b42 100644 --- a/src/codex/journal.ts +++ b/src/codex/journal.ts @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { existsSync, readFileSync, unlinkSync } from "node:fs"; import { join } from "node:path"; import { atomicWriteFile } from "../config"; -import { hasInjectedCodexRouting, rootTomlString } from "./injected-marker"; +import { hasInjectedCodexRouting } from "./injected-marker"; import { CODEX_HOME, CODEX_CONFIG_PATH, CODEX_PROFILE_PATH } from "./paths"; /** @@ -108,16 +108,26 @@ export function writeJournal(options: WriteJournalOptions = {}): void { atomicWriteFile(JOURNAL_PATH, JSON.stringify(journal)); } -export function markJournalInjectedState(config: string, profile: string | null): void { +export interface InjectedJournalOwnership { + injectedOpenaiBaseUrl: string | null; + injectedCatalogPath: string | null; +} + +export function markJournalInjectedState( + config: string, + profile: string | null, + ownership: InjectedJournalOwnership, +): void { const journal = readJournal(); if (!journal) return; - if (journal.injectedConfigHash) return; + // Keep the first native snapshot, but refresh the state written by every successful + // reinjection. Otherwise restore compares against stale bytes and stale ownership values. journal.injectedConfigHash = sha256(config) ?? undefined; journal.injectedProfileHash = sha256(profile); - // Read from the bytes we are about to install, not from the file: another writer may - // already have rewritten it, and then the recorded value would describe their config. - journal.injectedOpenaiBaseUrl = rootTomlString(config, "openai_base_url"); - journal.injectedCatalogPath = rootTomlString(config, "model_catalog_json"); + // Only the caller knows which values it actually owns. Deriving these from the final TOML + // would mistake a preserved user override for injected routing. + journal.injectedOpenaiBaseUrl = ownership.injectedOpenaiBaseUrl; + journal.injectedCatalogPath = ownership.injectedCatalogPath; atomicWriteFile(JOURNAL_PATH, JSON.stringify(journal)); } diff --git a/tests/codex-journal.test.ts b/tests/codex-journal.test.ts index 827ddd29d4..b09c31d4aa 100644 --- a/tests/codex-journal.test.ts +++ b/tests/codex-journal.test.ts @@ -553,7 +553,10 @@ describe("codex-journal", () => { const r = runScript(testDir, ` const { markJournalInjectedState } = require("./src/codex/journal"); - markJournalInjectedState("# injected\\n", null); + markJournalInjectedState("# injected\\n", null, { + injectedOpenaiBaseUrl: null, + injectedCatalogPath: null, + }); console.log(String(process.pid)); `); expect(r.status).toBe(0); @@ -564,6 +567,36 @@ describe("codex-journal", () => { expect(typeof second.injectedConfigHash).toBe("string"); // marked by the second }); + test("reinjection refreshes injected hashes and ownership without replacing the native snapshot", () => { + const r = runScript(testDir, ` + const fs = require("fs"); + const path = require("path"); + const { writeJournal, markJournalInjectedState } = require("./src/codex/journal"); + const journalPath = path.join(process.env.CODEX_HOME, "opencodex-journal.json"); + writeJournal(); + markJournalInjectedState("# first injection\\n", null, { + injectedOpenaiBaseUrl: "http://127.0.0.1:10100/v1", + injectedCatalogPath: "first-catalog.json", + }); + const first = JSON.parse(fs.readFileSync(journalPath, "utf8")); + markJournalInjectedState("# second injection\\n", "# second profile\\n", { + injectedOpenaiBaseUrl: "http://127.0.0.1:10200/v1", + injectedCatalogPath: "second-catalog.json", + }); + const second = JSON.parse(fs.readFileSync(journalPath, "utf8")); + console.log(JSON.stringify({ firstHash: first.injectedConfigHash, secondHash: second.injectedConfigHash })); + `); + expect(r.status).toBe(0); + const hashes = JSON.parse(r.stdout) as { firstHash: string; secondHash: string }; + expect(hashes.secondHash).not.toBe(hashes.firstHash); + + const journal = JSON.parse(readFileSync(join(testDir, "opencodex-journal.json"), "utf8")); + expect(Buffer.from(journal.originalConfig, "base64").toString("utf8")).toContain("# original config"); + expect(journal.injectedOpenaiBaseUrl).toBe("http://127.0.0.1:10200/v1"); + expect(journal.injectedCatalogPath).toBe("second-catalog.json"); + expect(typeof journal.injectedProfileHash).toBe("string"); + }); + test("writeJournal() with no options still snapshots a native config", () => { const r = runScript(testDir, `require("./src/codex/journal").writeJournal(); console.log("written");`); expect(r.status).toBe(0); diff --git a/tests/codex-restore-app-rewrite.test.ts b/tests/codex-restore-app-rewrite.test.ts index 21cea8dce4..2212385ce4 100644 --- a/tests/codex-restore-app-rewrite.test.ts +++ b/tests/codex-restore-app-rewrite.test.ts @@ -39,7 +39,7 @@ const INJECT_REWRITE_RESTORE = [ " const rewritten = injected", " .split(String.fromCharCode(10))", ' .filter(line => !line.trim().startsWith("#"))', - " .join(String.fromCharCode(10));", + " .join(String.fromCharCode(10)) + String.fromCharCode(10) + '# app rewrite';", ' fs.writeFileSync(configPath, rewritten, "utf8");', " const result = restoreNativeCodex();", " console.log(JSON.stringify({ success: result.success, message: result.message }));", @@ -74,6 +74,28 @@ const CATALOG_REWRITE_RESTORE = [ " console.log(JSON.stringify({ success: result.success, catalog: result.artifacts.catalog.path }));", "})();", ].join(String.fromCharCode(10)); + +/** Reinject with a new route and catalog, then expose the durable ownership record. */ +const REINJECT_AND_READ_JOURNAL = [ + 'const fs = require("fs");', + 'const path = require("path");', + 'const { injectCodexConfig } = require("./src/codex/inject");', + "(async () => {", + ' const firstCatalog = path.join(process.env.CODEX_HOME, "first-catalog.json");', + ' const secondCatalog = path.join(process.env.CODEX_HOME, "second-catalog.json");', + " const config = {", + " port: 10100,", + " providers: {},", + ' defaultProvider: "openai",', + ' injectionModel: "gpt-5.6-sol",', + ' injectionEffort: "high",', + " };", + " await injectCodexConfig(10100, config, { catalogPath: firstCatalog });", + " await injectCodexConfig(10200, { ...config, port: 10200 }, { catalogPath: secondCatalog });", + ' const journal = JSON.parse(fs.readFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), "utf8"));', + " console.log(JSON.stringify({ url: journal.injectedOpenaiBaseUrl, catalog: journal.injectedCatalogPath }));", + "})();", +].join(String.fromCharCode(10)); function runScript(codexHome: string, script: string): { stdout: string; stderr: string; status: number } { const result = spawnSync(process.execPath, ["--eval", script], { cwd: repoRoot, @@ -109,8 +131,9 @@ describe("#1798 restore after the Codex app rewrites the config", () => { }, 15_000); test("a user's own openai_base_url written before injection is preserved", () => { + // Force a byte mismatch so exact journal restore cannot hide a fallback ownership bug. // The mirror-image risk of the fix: stripping ANY unmarked openai_base_url would - // delete a URL we never wrote. The journaled baseline is the arbiter, not the key name. + // delete a URL we never wrote. The journaled ownership evidence is the arbiter. writeFileSync( join(testDir, "config.toml"), 'openai_base_url = "https://my-own-gateway.example/v1"\nmodel = "gpt-5.5"\n', @@ -125,6 +148,17 @@ describe("#1798 restore after the Codex app rewrites the config", () => { expect(restored).not.toContain("127.0.0.1:10100"); }, 15_000); + test("reinjection refreshes the owned route and catalog recorded for restore", () => { + writeFileSync(join(testDir, "config.toml"), 'model = "gpt-5.5"\n', "utf8"); + + const r = runScript(testDir, REINJECT_AND_READ_JOURNAL); + if (r.status !== 0) throw new Error(r.stderr || r.stdout); + + const recorded = JSON.parse(r.stdout) as { url: string; catalog: string }; + expect(recorded.url).toBe("http://127.0.0.1:10200/v1"); + expect(recorded.catalog).toBe(join(testDir, "second-catalog.json")); + }, 20_000); + test("the routed catalog we wrote is restored even when the rewrite dropped model_catalog_json", () => { // The catalog half of #1798. Restore used to re-resolve its target from the CURRENT // config, so a rewrite that removed `model_catalog_json` sent it to the default catalog From 33d9a3bee31cce69bc6ff6942c9a16fc854ee408 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Fri, 21 Aug 2026 00:54:16 +0900 Subject: [PATCH 2/2] fix(codex): preserve edits across reinjection restore --- src/codex/journal.ts | 10 +++-- tests/codex-journal.test.ts | 7 +++- tests/codex-restore-app-rewrite.test.ts | 56 +++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/src/codex/journal.ts b/src/codex/journal.ts index f6b6e29b42..b8923a6daa 100644 --- a/src/codex/journal.ts +++ b/src/codex/journal.ts @@ -120,9 +120,13 @@ export function markJournalInjectedState( ): void { const journal = readJournal(); if (!journal) return; - // Keep the first native snapshot, but refresh the state written by every successful - // reinjection. Otherwise restore compares against stale bytes and stale ownership values. - journal.injectedConfigHash = sha256(config) ?? undefined; + // The first exact injected config is the only safe whole-snapshot restore boundary for + // the first native snapshot. A later reinjection may preserve user edits made while routed; + // hashing those newer bytes and restoring the first snapshot would delete those edits. + // Keep the first hash so changed/reinjected configs take the owned-field fallback path. + journal.injectedConfigHash ??= sha256(config) ?? undefined; + // The profile file is wholly generated by OpenCodex, so its latest exact hash remains safe + // to refresh and lets restore remove the latest generated profile after a port change. journal.injectedProfileHash = sha256(profile); // Only the caller knows which values it actually owns. Deriving these from the final TOML // would mistake a preserved user override for injected routing. diff --git a/tests/codex-journal.test.ts b/tests/codex-journal.test.ts index b09c31d4aa..b1e5e1fbce 100644 --- a/tests/codex-journal.test.ts +++ b/tests/codex-journal.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test, beforeEach, afterEach, setDefaultTimeout } from "bun:test"; +import { createHash } from "node:crypto"; import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync } from "node:fs"; import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; @@ -567,7 +568,7 @@ describe("codex-journal", () => { expect(typeof second.injectedConfigHash).toBe("string"); // marked by the second }); - test("reinjection refreshes injected hashes and ownership without replacing the native snapshot", () => { + test("reinjection keeps the first config hash while refreshing owned route and catalog", () => { const r = runScript(testDir, ` const fs = require("fs"); const path = require("path"); @@ -588,7 +589,9 @@ describe("codex-journal", () => { `); expect(r.status).toBe(0); const hashes = JSON.parse(r.stdout) as { firstHash: string; secondHash: string }; - expect(hashes.secondHash).not.toBe(hashes.firstHash); + expect(typeof hashes.firstHash).toBe("string"); + expect(hashes.firstHash).toBe(createHash("sha256").update("# first injection\n").digest("hex")); + expect(hashes.secondHash).toBe(hashes.firstHash); const journal = JSON.parse(readFileSync(join(testDir, "opencodex-journal.json"), "utf8")); expect(Buffer.from(journal.originalConfig, "base64").toString("utf8")).toContain("# original config"); diff --git a/tests/codex-restore-app-rewrite.test.ts b/tests/codex-restore-app-rewrite.test.ts index 2212385ce4..2e37309e18 100644 --- a/tests/codex-restore-app-rewrite.test.ts +++ b/tests/codex-restore-app-rewrite.test.ts @@ -96,6 +96,38 @@ const REINJECT_AND_READ_JOURNAL = [ " console.log(JSON.stringify({ url: journal.injectedOpenaiBaseUrl, catalog: journal.injectedCatalogPath }));", "})();", ].join(String.fromCharCode(10)); + +/** Preserve a user edit made after the first injection across reinjection and restore. */ +const REINJECT_AFTER_USER_EDIT_RESTORE = [ + 'const fs = require("fs");', + 'const path = require("path");', + 'const { injectCodexConfig, restoreNativeCodex } = require("./src/codex/inject");', + "(async () => {", + " const config = {", + " port: 10100,", + " providers: {},", + ' defaultProvider: "openai",', + ' injectionModel: "gpt-5.6-sol",', + ' injectionEffort: "high",', + " };", + " await injectCodexConfig(10100, config, { catalogPath: null });", + ' const configPath = path.join(process.env.CODEX_HOME, "config.toml");', + ' fs.appendFileSync(configPath, String.fromCharCode(10) + \'approval_policy = "never"\' + String.fromCharCode(10), "utf8");', + " await injectCodexConfig(10200, { ...config, port: 10200 }, { catalogPath: null });", + ' const beforeRestore = fs.readFileSync(configPath, "utf8");', + " const result = restoreNativeCodex({ skipHistory: true });", + ' const afterRestore = fs.readFileSync(configPath, "utf8");', + ' const profileExistsAfterRestore = fs.existsSync(path.join(process.env.CODEX_HOME, "opencodex.config.toml"));', + " console.log(JSON.stringify({", + " success: result.success,", + " action: result.artifacts.config.action,", + " beforeRestore,", + " afterRestore,", + " profileExistsAfterRestore,", + " }));", + "})();", +].join(String.fromCharCode(10)); + function runScript(codexHome: string, script: string): { stdout: string; stderr: string; status: number } { const result = spawnSync(process.execPath, ["--eval", script], { cwd: repoRoot, @@ -159,6 +191,30 @@ describe("#1798 restore after the Codex app rewrites the config", () => { expect(recorded.catalog).toBe(join(testDir, "second-catalog.json")); }, 20_000); + test("a user setting added after first injection survives reinjection and restore", () => { + writeFileSync(join(testDir, "config.toml"), 'model = "gpt-5.5"\n', "utf8"); + + const r = runScript(testDir, REINJECT_AFTER_USER_EDIT_RESTORE); + if (r.status !== 0) throw new Error(r.stderr || r.stdout); + + const result = JSON.parse(r.stdout) as { + success: boolean; + action: string; + beforeRestore: string; + afterRestore: string; + profileExistsAfterRestore: boolean; + }; + expect(result.success).toBe(true); + expect(result.action).toBe("owned-fields-stripped"); + expect(result.beforeRestore).toContain('approval_policy = "never"'); + expect(result.beforeRestore).toContain("127.0.0.1:10200"); + expect(result.afterRestore).toContain('approval_policy = "never"'); + expect(result.afterRestore).toContain('model = "gpt-5.5"'); + expect(result.afterRestore).not.toContain("openai_base_url"); + expect(result.afterRestore).not.toContain("127.0.0.1:10200"); + expect(result.profileExistsAfterRestore).toBe(false); + }, 20_000); + test("the routed catalog we wrote is restored even when the rewrite dropped model_catalog_json", () => { // The catalog half of #1798. Restore used to re-resolve its target from the CURRENT // config, so a rewrite that removed `model_catalog_json` sent it to the default catalog