fix(codex): remove injected routing and restore the routed catalog after a Codex app rewrite #1862
+259
−9
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 { CODEX_HOME, CODEX_CONFIG_PATH, CODEX_PROFILE_PATH } from "./paths"; | ||
|
|
||
| /** | ||
|
|
@@ -22,6 +22,24 @@ interface Journal { | |
| originalProfile: string | null; | ||
| injectedConfigHash?: string; | ||
| injectedProfileHash?: string | null; | ||
| /** | ||
| * The exact root `openai_base_url` this injection wrote, when it wrote one. | ||
| * | ||
| * #1798: ownership used to be inferred from a marker COMMENT on the preceding line, | ||
| * which a reserializing Codex app deletes while keeping the value. Recording the value | ||
| * we actually wrote makes ownership provable from evidence rather than from formatting, | ||
| * and it is what lets restore tell OUR loopback URL apart from a gateway the user set. | ||
| */ | ||
| injectedOpenaiBaseUrl?: string | null; | ||
| /** | ||
| * The catalog path this injection actually wrote to. | ||
| * | ||
| * #1798: restore re-resolves the catalog from the CURRENT config, so a Codex app rewrite | ||
| * that dropped `model_catalog_json` sends restore to the default catalog while the | ||
| * proxy-written one is left routed. The injected path is the only durable record of which | ||
| * file we actually touched. | ||
| */ | ||
| injectedCatalogPath?: string | null; | ||
| pid: number; | ||
| timestamp: string; | ||
| } | ||
|
|
@@ -96,9 +114,30 @@ export function markJournalInjectedState(config: string, profile: string | null) | |
| if (journal.injectedConfigHash) return; | ||
| 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"); | ||
|
There was a problem hiding this comment.
When routing is reinjected while the existing journal remains—for example, after changing the proxy port or hostname—the config receives the new URL, but the AGENTS.md reference: src/AGENTS.md:L7-L10 Useful? React with 👍 / 👎. |
||
| journal.injectedCatalogPath = rootTomlString(config, "model_catalog_json"); | ||
| atomicWriteFile(JOURNAL_PATH, JSON.stringify(journal)); | ||
| } | ||
|
|
||
| /** | ||
| * The root `openai_base_url` the last injection wrote, or null when it wrote none. | ||
| * | ||
| * #1798: the fallback strip recognizes an injected URL by the marker COMMENT above it, | ||
| * and a Codex app rewrite keeps values while dropping comments. This is the evidence that | ||
| * survives such a rewrite, so restore can still prove the URL is ours -- and, just as | ||
| * importantly, prove that a DIFFERENT URL is not. | ||
| */ | ||
| export function journaledInjectedOpenaiBaseUrl(): string | null { | ||
| return readJournal()?.injectedOpenaiBaseUrl ?? null; | ||
| } | ||
|
|
||
| /** The catalog path the last injection wrote to, or null when none was recorded. */ | ||
| export function journaledInjectedCatalogPath(): string | null { | ||
| return readJournal()?.injectedCatalogPath ?? null; | ||
| } | ||
|
|
||
| export function removeJournal(): void { | ||
| try { unlinkSync(JOURNAL_PATH); } catch { /* ignore */ } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| import { describe, expect, test, beforeEach, afterEach } from "bun:test"; | ||
| import { mkdtempSync, rmSync, writeFileSync, readFileSync } from "node:fs"; | ||
| import { spawnSync } from "node:child_process"; | ||
| import { tmpdir } from "node:os"; | ||
| import { dirname, join } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| /** | ||
| * #1798: the Codex app rewrites config.toml AFTER injection, so the journal's | ||
| * exact-bytes restore no longer fires and the fallback strip is the only thing left. | ||
| * That fallback recognizes an injected `openai_base_url` ONLY by the marker comment | ||
| * on the line above it. An app rewrite reserializes the file and drops the comment, so | ||
| * the proxy URL stops being recognized as ours and survives `ocx stop` / `ocx restore` | ||
| * while the command reports success -- leaving plain Codex pointed at a dead port. | ||
| * | ||
| * These tests reproduce that state literally: inject, drop every comment the way a TOML | ||
| * reserializer would (values kept, comments gone), then restore. | ||
| */ | ||
|
|
||
| const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); | ||
|
|
||
| /** Inject, simulate the app's comment-dropping rewrite, then restore. */ | ||
| const INJECT_REWRITE_RESTORE = [ | ||
| 'const fs = require("fs");', | ||
| 'const path = require("path");', | ||
| 'const { injectCodexConfig, restoreNativeCodex } = require("./src/codex/inject");', | ||
| "(async () => {", | ||
| " await injectCodexConfig(10100, {", | ||
| " port: 10100,", | ||
| " providers: {},", | ||
| ' defaultProvider: "openai",', | ||
| ' injectionModel: "gpt-5.6-sol",', | ||
| ' injectionEffort: "high",', | ||
| " }, { catalogPath: null });", | ||
| ' const configPath = path.join(process.env.CODEX_HOME, "config.toml");', | ||
| ' const injected = fs.readFileSync(configPath, "utf8");', | ||
| " // Exactly what a reserializing app writer produces: every VALUE survives,", | ||
| " // every COMMENT -- including our ownership marker -- is gone.", | ||
| " const rewritten = injected", | ||
| " .split(String.fromCharCode(10))", | ||
| ' .filter(line => !line.trim().startsWith("#"))', | ||
| " .join(String.fromCharCode(10));", | ||
| ' fs.writeFileSync(configPath, rewritten, "utf8");', | ||
| " const result = restoreNativeCodex();", | ||
| " console.log(JSON.stringify({ success: result.success, message: result.message }));", | ||
| "})();", | ||
| ].join("\n"); | ||
|
|
||
| /** Inject with an explicit catalog path, drop `model_catalog_json` the way a rewrite does, then restore. */ | ||
| const CATALOG_REWRITE_RESTORE = [ | ||
| 'const fs = require("fs");', | ||
| 'const path = require("path");', | ||
| 'const { injectCodexConfig, restoreNativeCodex } = require("./src/codex/inject");', | ||
| "(async () => {", | ||
| ' const cachePath = path.join(process.env.CODEX_HOME, "models_cache.json");', | ||
| " // The catalog file itself is written by catalog sync, which needs network state this", | ||
| " // test has no business standing up. Seed it directly: what is under test is WHICH file", | ||
| " // restore targets, not how sync populates it.", | ||
| ' fs.writeFileSync(cachePath, JSON.stringify({ models: [{ slug: "gpt-5.5" }, { slug: "opencode-go/deepseek-v4-flash" }] }), "utf8");', | ||
| " await injectCodexConfig(10100, {", | ||
| " port: 10100,", | ||
| " providers: {},", | ||
| ' defaultProvider: "openai",', | ||
| ' injectionModel: "gpt-5.6-sol",', | ||
| ' injectionEffort: "high",', | ||
| " }, { catalogPath: cachePath });", | ||
| ' const configPath = path.join(process.env.CODEX_HOME, "config.toml");', | ||
| ' const rewritten = fs.readFileSync(configPath, "utf8")', | ||
| " .split(String.fromCharCode(10))", | ||
| ' .filter(line => !line.trim().startsWith("#") && !line.includes("model_catalog_json"))', | ||
| " .join(String.fromCharCode(10));", | ||
| ' fs.writeFileSync(configPath, rewritten, "utf8");', | ||
| " const result = restoreNativeCodex();", | ||
| " console.log(JSON.stringify({ success: result.success, catalog: result.artifacts.catalog.path }));", | ||
| "})();", | ||
| ].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, | ||
| env: { ...process.env, CODEX_HOME: codexHome }, | ||
| encoding: "utf8", | ||
| }); | ||
| return { stdout: result.stdout?.trim() ?? "", stderr: result.stderr?.trim() ?? "", status: result.status ?? 1 }; | ||
| } | ||
|
|
||
| describe("#1798 restore after the Codex app rewrites the config", () => { | ||
| let testDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| testDir = mkdtempSync(join(tmpdir(), "ocx-1798-")); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| rmSync(testDir, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| test("an unmarked injected openai_base_url is still removed", () => { | ||
| writeFileSync(join(testDir, "config.toml"), '# original config\nmodel = "gpt-5.5"\n', "utf8"); | ||
|
|
||
| const r = runScript(testDir, INJECT_REWRITE_RESTORE); | ||
| if (r.status !== 0) throw new Error(r.stderr || r.stdout); | ||
|
|
||
| const restored = readFileSync(join(testDir, "config.toml"), "utf8"); | ||
| // The reported defect: the proxy URL survives, so plain Codex talks to a dead port. | ||
| expect(restored).not.toContain("openai_base_url"); | ||
| expect(restored).not.toContain("127.0.0.1:10100"); | ||
| // The user's own pre-injection content is still theirs. | ||
| expect(restored).toContain("gpt-5.5"); | ||
| }); | ||
|
|
||
| test("a user's own openai_base_url written before injection is preserved", () => { | ||
| // 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. | ||
| writeFileSync( | ||
| join(testDir, "config.toml"), | ||
| 'openai_base_url = "https://my-own-gateway.example/v1"\nmodel = "gpt-5.5"\n', | ||
| "utf8", | ||
| ); | ||
|
|
||
| const r = runScript(testDir, INJECT_REWRITE_RESTORE); | ||
| if (r.status !== 0) throw new Error(r.stderr || r.stdout); | ||
|
|
||
| const restored = readFileSync(join(testDir, "config.toml"), "utf8"); | ||
| expect(restored).toContain("https://my-own-gateway.example/v1"); | ||
| expect(restored).not.toContain("127.0.0.1:10100"); | ||
| }); | ||
|
|
||
| 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 | ||
| // while the proxy-written models_cache.json kept every routed entry. | ||
| writeFileSync(join(testDir, "config.toml"), 'model = "gpt-5.5"' + String.fromCharCode(10), "utf8"); | ||
|
|
||
| const r = runScript(testDir, CATALOG_REWRITE_RESTORE); | ||
| if (r.status !== 0) throw new Error(r.stderr || r.stdout); | ||
|
|
||
| const cachePath = join(testDir, "models_cache.json"); | ||
| const cache = JSON.parse(readFileSync(cachePath, "utf8")); | ||
| const routed = (cache.models ?? []).filter((m: { slug?: string }) => typeof m.slug === "string" && m.slug.includes("/")); | ||
| expect(routed).toEqual([]); | ||
| expect(JSON.parse(r.stdout).catalog).toBe(cachePath); | ||
| }); | ||
| }); | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
config.tomlalready contains an unmarked rootopenai_base_url,setRootOpenaiBaseUrldeliberately preserves it, but this assignment nevertheless records that user-owned value as injected. If Codex later reserializes the file differently or the user edits any other setting, the journal hash no longer matches andstripJournaledOpenaiBaseUrldeletes the user's gateway because its value matches this record. Recordnullunless the transform actually inserted or owned the URL, and cover the fallback path with a pre-existing URL plus an unrelated edit.AGENTS.md reference: src/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.