From bbf349aee2ba32b59cbe25d70373dcd105968734 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Wed, 30 Sep 2026 17:09:02 +0900 Subject: [PATCH] fix(codex): preserve TOML values and routing marker ownership --- .../troubleshooting/codex-cannot-sign-in.md | 17 + scripts/test-layout/layout.json | 1 + src/codex/inject/config-toml.ts | 427 ++++++++---------- src/codex/inject/provider-table.ts | 7 +- src/codex/inject/remove.ts | 31 +- src/codex/injected-marker.ts | 51 ++- src/codex/toml-source-lines.ts | 94 +++- structure/codex-home.md | 2 +- .../codex-config-preservation.test.ts | 292 ++++++++++++ .../codex-provider-table-retention.test.ts | 128 ++++++ tests/fixtures/test-layout-expected.json | 1 + 11 files changed, 768 insertions(+), 283 deletions(-) create mode 100644 tests/codex-integration/codex-config-preservation.test.ts diff --git a/docs-site/src/content/docs/troubleshooting/codex-cannot-sign-in.md b/docs-site/src/content/docs/troubleshooting/codex-cannot-sign-in.md index a5971443b34..9bff3814fdb 100644 --- a/docs-site/src/content/docs/troubleshooting/codex-cannot-sign-in.md +++ b/docs-site/src/content/docs/troubleshooting/codex-cannot-sign-in.md @@ -75,6 +75,23 @@ comment above other keys it manages, such as an injected `developer_instructions`, and deleting those will not help you sign in while costing you configuration you may want back. +Repeated standalone ownership comments do not change TOML values or routing. +Provider-table reinjection and removal consume the exact bare marker or its +`(undo: ocx restore)` form immediately above `[model_providers.opencodex]`. +Marker text inside another user comment or a multiline string is preserved; +existing comments separated from the table are not claimed by that transform. +Root routing ownership uses an exact adjacent marker at a structural root +assignment. A matching injected value recorded in the journal also proves +ownership after a Codex app rewrite drops comments. Quoting the marker in a +user comment or a multiline string does not prove ownership. Managed +`web_search` replacements record the complete original assignment, including +multiline values, so switching it back on or fallback restore can put it back. +Removal and capture validate complete assignments without converting a valid +64-bit integer into a JavaScript number. Value-based ownership still requires +successful value decoding; invalid or incomplete assignments are refused. +Fallback removal compacts document +blank lines outside multiline values, preserving blank lines within those values. + Delete the `model_catalog_json` line **with** the routing, not on its own. A `model_catalog_json` that names a file which no longer exists makes Codex fail to load its configuration at all, which looks like the same lockout for a diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 2e8719fa6d9..c99437285ee 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -2,6 +2,7 @@ "version": 1, "root": "tests", "explicit": { + "codex-config-preservation.test.ts": "codex-integration", "service-desktop-startup-health.test.ts": "service", "service-desktop-startup.test.ts": "service", "startup-health-packaged-probe.test.ts": "server", diff --git a/src/codex/inject/config-toml.ts b/src/codex/inject/config-toml.ts index 99d8d0437c4..36c31719f6f 100644 --- a/src/codex/inject/config-toml.ts +++ b/src/codex/inject/config-toml.ts @@ -3,23 +3,22 @@ import { existsSync, readFileSync } from "node:fs"; import { contextCompatibleBaseLine } from "../context-compat"; import { resolveEffectiveProjectModelProvider } from "../project-config-warnings"; import { - OCX_SECTION_MARKER, OCX_ROUTING_MARKER_LINE, REALTIME_WS_BASE_URL_KEY, + isOcxRoutingMarkerLine, isRootOpenaiBaseUrlLine, isRootRealtimeWsBaseUrlLine, providerTableStart, providerTableString, rootTomlString, - tomlStringPattern, } from "../injected-marker"; import { CODEX_CONFIG_PATH, DEFAULT_CATALOG_PATH, - parseTomlString, resolveCodexConfigPath, tomlString, } from "../paths"; +import { normalizeStructuralWhitespace, rootAssignmentKey, rootSourceLines, sourceAssignment, sourceAssignmentSpan, sourceText, type SourceLine } from "../toml-source-lines"; import { readBoundedCodexConfig } from "./bounded-config-reader"; import { type CodexRoutingTarget, @@ -67,10 +66,9 @@ export function observedExternalCodexModelProvider(): string | null { } /** - * Detect the file's dominant line ending. Every transform in this module is LF-pure - * (split("\n") + hard "\n" joins), so CRLF configs (Windows-edited config.toml) are - * normalized to LF at the pipeline boundary and converted back on write — otherwise a - * single inject would leave a mixed-EOL file. + * Detect the file's dominant line ending. Structural edits retain physical line endings; + * the full injection/removal pipeline still normalizes to LF at its boundary and converts + * back on write so generated blocks do not leave a mixed-EOL file. */ export function dominantEol(content: string): "\r\n" | "\n" { const crlf = (content.match(/\r\n/g) ?? []).length; @@ -234,66 +232,43 @@ export function setRootOpenaiBaseUrl( if (typeof portOrTarget !== "number") { return setRootOpenaiBaseUrlForTarget(content, validateCodexRoutingTarget(portOrTarget)); } - const lines = content.split("\n"); - const firstTable = lines.findIndex((l) => /^\s*\[/.test(l)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; - const key = contextCompatibleBaseLine(content, buildOpenaiBaseUrlLine(portOrTarget, hostname)); + return setRootUrl(content, contextCompatibleBaseLine(content, buildOpenaiBaseUrlLine(portOrTarget, hostname))); +} - for (let i = 0; i < rootEnd; i++) { - if (!isRootOpenaiBaseUrlLine(lines[i])) continue; - const markerOwned = i > 0 && lines[i - 1].includes(OCX_SECTION_MARKER); - if (!markerOwned) return { content, keptUserBaseUrl: true }; - // Refresh the marker too, so a config injected by a build that predates the recovery - // hint gains it on the next `ocx start` instead of keeping a bare marker forever. - lines[i - 1] = OCX_ROUTING_MARKER_LINE; - lines[i] = key; - return { content: lines.join("\n"), keptUserBaseUrl: false }; - } +function ownedRootUrl(lines: SourceLine[], index: number, key: string): boolean { + return lines[index]!.structural && rootTomlString(lines[index]!.text, key) !== null + && index > 0 && lines[index - 1]!.structural && isOcxRoutingMarkerLine(lines[index - 1]!.text); +} - if (firstTable === -1) { - return { - content: - content.replace(/\n+$/, "") + - "\n" + - OCX_ROUTING_MARKER_LINE + - "\n" + - key + - "\n", - keptUserBaseUrl: false, - }; +function insertRootSourceLines(content: string, added: string[]): string { + const { bom, lines, rootEnd } = rootSourceLines(content); + const eol = dominantEol(content); + if (rootEnd === lines.length) { + return content.replace(/(?:\r?\n)+$/, "") + eol + added.join(eol) + eol; + } + let at = rootEnd; + while (at > 0 && lines[at - 1]!.structural && lines[at - 1]!.text.trim() === "") at -= 1; + lines.splice(at, 0, ...added.map(text => ({ text, eol, structural: true }))); + return bom + sourceText(lines); +} + +function setRootUrl(content: string, key: string): { content: string; keptUserBaseUrl: boolean } { + const { bom, lines, rootEnd } = rootSourceLines(content); + for (let index = 0; index < rootEnd; index++) { + if (!lines[index]!.structural || !isRootOpenaiBaseUrlLine(lines[index]!.text)) continue; + if (!ownedRootUrl(lines, index, "openai_base_url")) return { content, keptUserBaseUrl: true }; + lines[index - 1]!.text = OCX_ROUTING_MARKER_LINE; + lines[index]!.text = key; + return { content: bom + sourceText(lines), keptUserBaseUrl: false }; } - let insertAt = firstTable; - while (insertAt > 0 && lines[insertAt - 1].trim() === "") insertAt--; - lines.splice(insertAt, 0, OCX_ROUTING_MARKER_LINE, key); - return { content: lines.join("\n"), keptUserBaseUrl: false }; + return { content: insertRootSourceLines(content, [OCX_ROUTING_MARKER_LINE, key]), keptUserBaseUrl: false }; } export function setRootOpenaiBaseUrlForTarget( content: string, target: CodexRoutingTarget, ): { content: string; keptUserBaseUrl: boolean } { - const lines = content.split("\n"); - const firstTable = lines.findIndex((line) => /^\s*\[/.test(line)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; - const key = contextCompatibleBaseLine(content, buildOpenaiBaseUrlLineForTarget(target)); - for (let index = 0; index < rootEnd; index += 1) { - if (!isRootOpenaiBaseUrlLine(lines[index])) continue; - const markerOwned = index > 0 && lines[index - 1].includes(OCX_SECTION_MARKER); - if (!markerOwned) return { content, keptUserBaseUrl: true }; - lines[index - 1] = OCX_ROUTING_MARKER_LINE; - lines[index] = key; - return { content: lines.join("\n"), keptUserBaseUrl: false }; - } - if (firstTable === -1) { - return { - content: `${content.replace(/\n+$/, "")}\n${OCX_ROUTING_MARKER_LINE}\n${key}\n`, - keptUserBaseUrl: false, - }; - } - let insertAt = firstTable; - while (insertAt > 0 && lines[insertAt - 1].trim() === "") insertAt -= 1; - lines.splice(insertAt, 0, OCX_ROUTING_MARKER_LINE, key); - return { content: lines.join("\n"), keptUserBaseUrl: false }; + return setRootUrl(content, contextCompatibleBaseLine(content, buildOpenaiBaseUrlLineForTarget(target))); } /** @@ -310,25 +285,22 @@ export function setRootRealtimeWsBaseUrl( content: string, target: CodexRoutingTarget, ): { content: string; keptUserRealtimeWsBaseUrl: boolean } { - const lines = content.split("\n"); - const firstTable = lines.findIndex((line) => /^\s*\[/.test(line)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; + const { bom, lines, rootEnd } = rootSourceLines(content); const key = buildRealtimeWsBaseUrlLine(validateCodexRoutingTarget(target)); - for (let index = 0; index < rootEnd; index += 1) { - if (!isRootRealtimeWsBaseUrlLine(lines[index])) continue; - const markerOwned = index > 0 && lines[index - 1].includes(OCX_SECTION_MARKER); - if (!markerOwned) return { content, keptUserRealtimeWsBaseUrl: true }; - lines[index - 1] = OCX_ROUTING_MARKER_LINE; - lines[index] = key; - return { content: lines.join("\n"), keptUserRealtimeWsBaseUrl: false }; + for (let index = 0; index < rootEnd; index++) { + if (!lines[index]!.structural || !isRootRealtimeWsBaseUrlLine(lines[index]!.text)) continue; + if (!ownedRootUrl(lines, index, REALTIME_WS_BASE_URL_KEY)) return { content, keptUserRealtimeWsBaseUrl: true }; + lines[index - 1]!.text = OCX_ROUTING_MARKER_LINE; + lines[index]!.text = key; + return { content: bom + sourceText(lines), keptUserRealtimeWsBaseUrl: false }; } - for (let index = 0; index < rootEnd; index += 1) { - if (!isRootOpenaiBaseUrlLine(lines[index])) continue; - if (!(index > 0 && lines[index - 1].includes(OCX_SECTION_MARKER))) continue; - lines.splice(index + 1, 0, OCX_ROUTING_MARKER_LINE, key); - return { content: lines.join("\n"), keptUserRealtimeWsBaseUrl: false }; + for (let index = 0; index < rootEnd; index++) { + if (!isRootOpenaiBaseUrlLine(lines[index]!.text) || !ownedRootUrl(lines, index, "openai_base_url")) continue; + const eol = dominantEol(content); + lines[index]!.eol ||= eol; + lines.splice(index + 1, 0, ...[OCX_ROUTING_MARKER_LINE, key].map(text => ({ text, eol, structural: true }))); + return { content: bom + sourceText(lines), keptUserRealtimeWsBaseUrl: false }; } - // No marker-owned routing override to attach to: the override has no owner, so inject nothing. return { content, keptUserRealtimeWsBaseUrl: false }; } @@ -345,40 +317,30 @@ export const ROOT_WEB_SEARCH_DISABLED_LINE = 'web_search = "disabled"'; export const ROOT_WEB_SEARCH_DISABLED_VALUE = "disabled"; export function isRootWebSearchLine(line: string): boolean { - // The quoted spellings are the same key to TOML, and `tomlStringPattern` -- which the value - // evidence below goes through -- already reads them. A predicate that matched only the bare + // Quoted spellings (including escaped basic keys) name the same key to TOML. + // A predicate that matched only the bare // spelling would leave `"web_search" = "live"` in place while inserting our own line, and two // root keys of the same name stop Codex from loading the file at all. - return /^\s*(?:"web_search"|'web_search'|web_search)\s*=/.test(line); + return rootAssignmentKey(line) === ROOT_WEB_SEARCH_KEY; } /** What an earlier injection recorded about this key, read back from the journal. */ export interface RootWebSearchJournal { /** The value that injection wrote, or null when it wrote none. */ injectedValue?: string | null; - /** The user-owned line that injection had to remove, or null when there was none. */ + /** The complete user-owned assignment removed (possibly multiline), or null. */ replacedUserLine?: string | null; } /** What one pass of {@link ensureRootWebSearchDisabled} did, for the journal to record. */ export interface RootWebSearchOutcome { content: string; - /** The user-owned root line this pass removed, or null when there was none. */ + /** The complete user-owned root assignment removed (possibly multiline), or null. */ replacedUserLine: string | null; /** The value this pass wrote, or null when it wrote none. */ wroteValue: string | null; } -/** Insert root-level lines ahead of the first table; TOML root keys may not follow one. */ -function insertRootLines(lines: string[], text: string): string { - const firstTable = lines.findIndex((line) => /^\s*\[/.test(line)); - if (firstTable === -1) return `${lines.join("\n").replace(/\n+$/, "")}\n${text}\n`; - let at = firstTable; - while (at > 0 && lines[at - 1].trim() === "") at -= 1; - lines.splice(at, 0, ...text.split("\n")); - return lines.join("\n"); -} - /** * Ensure the root `web_search` key follows the web-search sidecar's master switch. * @@ -406,14 +368,13 @@ export function ensureRootWebSearchDisabled( disabled: boolean, journal: RootWebSearchJournal = {}, ): RootWebSearchOutcome { - const lines = stripInjectedRootWebSearch(content, journal.injectedValue).split("\n"); - const firstTable = lines.findIndex((line) => /^\s*\[/.test(line)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; + const stripped = stripInjectedRootWebSearch(content, journal.injectedValue); + const { bom, lines, rootEnd } = rootSourceLines(stripped); if (!disabled) { - const restore = journal.replacedUserLine?.trim(); - const owned = lines.slice(0, rootEnd).some(isRootWebSearchLine); + const restore = journal.replacedUserLine; + const owned = lines.slice(0, rootEnd).some(line => line.structural && isRootWebSearchLine(line.text)); return { - content: restore && !owned ? insertRootLines(lines, restore) : lines.join("\n"), + content: restore?.trim() && !owned ? insertRootSourceLines(stripped, [applyEol(restore, dominantEol(stripped))]) : stripped, replacedUserLine: null, wroteValue: null, }; @@ -424,14 +385,21 @@ export function ensureRootWebSearchDisabled( // line keeps the one an earlier off pass recorded — the ordinary way to reach that state is a // second injection while the switch is still off (a model change), and the operator's mode must // not evaporate because the line it came from is already gone. - const replacedUserLine = lines - .slice(0, rootEnd) - .find((line) => isRootWebSearchLine(line)) - ?.replace(/\r$/, "") ?? journal.replacedUserLine?.trim() ?? null; + const drop = new Set(); + let replacedUserLine = journal.replacedUserLine ?? null; + let foundUserAssignment = false; + for (let index = 0; index < rootEnd; index++) { + if (!lines[index]!.structural || !isRootWebSearchLine(lines[index]!.text)) continue; + const assignment = sourceAssignmentSpan(lines, index, rootEnd); + if (!assignment) throw new Error("Cannot safely rewrite an incomplete root web_search assignment."); + if (!foundUserAssignment) replacedUserLine = assignment.text; + foundUserAssignment = true; + for (let at = index; at < assignment.end; at++) drop.add(at); + } return { - content: insertRootLines( - lines.filter((line, index) => index >= rootEnd || !isRootWebSearchLine(line)), - `${OCX_ROUTING_MARKER_LINE}\n${ROOT_WEB_SEARCH_DISABLED_LINE}`, + content: insertRootSourceLines( + bom + sourceText(lines.filter((_, index) => !drop.has(index))), + [OCX_ROUTING_MARKER_LINE, ROOT_WEB_SEARCH_DISABLED_LINE], ), replacedUserLine, wroteValue: ROOT_WEB_SEARCH_DISABLED_VALUE, @@ -446,26 +414,20 @@ export function ensureRootWebSearchDisabled( * comment a Codex app reserialize dropped is still recognized as ours. */ export function stripInjectedRootWebSearch(content: string, injectedValue?: string | null): string { - const lines = content.split("\n"); - const firstTable = lines.findIndex((line) => /^\s*\[/.test(line)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; + const { bom, lines, rootEnd } = rootSourceLines(content); const drop = new Set(); - for (let i = 0; i + 1 < rootEnd; i += 1) { - if (lines[i].includes(OCX_SECTION_MARKER) && isRootWebSearchLine(lines[i + 1])) { - drop.add(i); - drop.add(i + 1); - } + for (let index = 0; index < rootEnd; index++) { + const line = lines[index]!; + if (!line.structural || !isRootWebSearchLine(line.text)) continue; + const assignment = sourceAssignment(lines, index, rootEnd); + if (!assignment || typeof assignment.value !== "string") continue; + const value = assignment.value; + const markerOwned = index > 0 && lines[index - 1]!.structural && isOcxRoutingMarkerLine(lines[index - 1]!.text); + if (!markerOwned && (!injectedValue || value !== injectedValue)) continue; + for (let at = index; at < assignment.end; at++) drop.add(at); + if (markerOwned) drop.add(index - 1); } - if (injectedValue) { - for (let i = 0; i < rootEnd; i += 1) { - if (!isRootWebSearchLine(lines[i])) continue; - if (rootTomlString(lines[i], ROOT_WEB_SEARCH_KEY) !== injectedValue) continue; - drop.add(i); - if (i > 0 && lines[i - 1].includes(OCX_SECTION_MARKER)) drop.add(i - 1); - } - } - if (drop.size === 0) return content; - return lines.filter((_, index) => !drop.has(index)).join("\n"); + return drop.size ? bom + sourceText(lines.filter((_, index) => !drop.has(index))) : content; } /** @@ -475,21 +437,23 @@ export function stripInjectedRootWebSearch(content: string, injectedValue?: stri * A marker-owned `experimental_realtime_ws_base_url` pair is removed by the same rule. */ export function stripInjectedOpenaiBaseUrl(content: string): string { - const lines = content.split("\n"); - const firstTable = lines.findIndex((l) => /^\s*\[/.test(l)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; + const { bom, lines, rootEnd } = rootSourceLines(content); const drop = new Set(); - for (let i = 0; i < rootEnd; i++) { - if (!lines[i].includes(OCX_SECTION_MARKER)) continue; - if (i + 1 < rootEnd && (isRootOpenaiBaseUrlLine(lines[i + 1]) || isRootRealtimeWsBaseUrlLine(lines[i + 1]))) { - drop.add(i); - drop.add(i + 1); - } else if (i + 1 >= rootEnd || lines[i + 1].trim() === "") { - drop.add(i); // orphaned marker at root + for (let index = 0; index < rootEnd; index++) { + const line = lines[index]!; + if (!line.structural || !isOcxRoutingMarkerLine(line.text)) continue; + const next = lines[index + 1]; + if (index + 1 < rootEnd && next?.structural + && (isRootOpenaiBaseUrlLine(next.text) || isRootRealtimeWsBaseUrlLine(next.text))) { + const key = rootAssignmentKey(next.text)!; + if (rootTomlString(next.text, key) === null) continue; + drop.add(index); + drop.add(index + 1); + } else if (index + 1 >= rootEnd || (next?.structural && next.text.trim() === "")) { + drop.add(index); } } - if (drop.size === 0) return content; - return lines.filter((_, i) => !drop.has(i)).join("\n"); + return drop.size ? bom + sourceText(lines.filter((_, index) => !drop.has(index))) : content; } /** @@ -500,18 +464,19 @@ export function stripInjectedOpenaiBaseUrl(content: string): string { * untouched. */ export function stripExistingModelProvider(content: string): string { - const lines = content.split("\n"); - const firstTable = lines.findIndex((l) => /^\s*\[/.test(l)); - const out: string[] = []; - lines.forEach((line, i) => { - if (/^\s*model_provider\s*=/.test(line)) { - const isOurs = /^\s*model_provider\s*=\s*"opencodex"\s*$/.test(line); - const isRoot = firstTable === -1 || i < firstTable; - if (isOurs || isRoot) return; // drop it + const { bom, lines, rootEnd } = rootSourceLines(content); + const drop = new Set(); + for (let index = 0; index < lines.length; index++) { + if (!lines[index]!.structural || rootAssignmentKey(lines[index]!.text) !== "model_provider") continue; + const assignment = sourceAssignmentSpan(lines, index, index < rootEnd ? rootEnd : lines.length); + if (!assignment) throw new Error("Cannot safely rewrite an incomplete model_provider assignment."); + if (index >= rootEnd) { + const decoded = sourceAssignment(lines, index); + if (typeof decoded?.value !== "string" || decoded.value.trim() !== "opencodex") continue; } - out.push(line); - }); - return out.join("\n"); + for (let at = index; at < assignment.end; at++) drop.add(at); + } + return bom + sourceText(lines.filter((_, index) => !drop.has(index))); } /** @@ -521,29 +486,21 @@ export function stripExistingModelProvider(content: string): string { * compaction limits do not alter the advertised context window and must survive reinjection. */ export function stripRootContextWindowOverrides(content: string): string { - const lines = content.split("\n"); - const firstTable = lines.findIndex((l) => /^\s*\[/.test(l)); - return lines - .filter((line, i) => { - const isRoot = firstTable === -1 || i < firstTable; - return !isRoot || !/^\s*model_context_window\s*=/.test(line); - }) - .join("\n"); + const { bom, lines, rootEnd } = rootSourceLines(content); + const drop = new Set(); + for (let index = 0; index < rootEnd; index++) { + if (!lines[index]!.structural || rootAssignmentKey(lines[index]!.text) !== "model_context_window") continue; + const assignment = sourceAssignmentSpan(lines, index, rootEnd); + if (!assignment) throw new Error("Cannot safely rewrite an incomplete model_context_window assignment."); + for (let at = index; at < assignment.end; at++) drop.add(at); + } + return bom + sourceText(lines.filter((_, index) => !drop.has(index))); } export function stripRootRoutedModel(content: string): string { - const lines = content.split("\n"); - const firstTable = lines.findIndex((l) => /^\s*\[/.test(l)); - return lines - .filter((line, i) => { - const isRoot = firstTable === -1 || i < firstTable; - if (!isRoot) return true; - const m = line.match(/^\s*model\s*=\s*("(?:\\.|[^"\\])*"|'[^']*')\s*$/); - if (!m) return true; - const model = parseTomlString(m[1]); - return !model?.includes("/"); - }) - .join("\n"); + const { bom, lines, rootEnd } = rootSourceLines(content); + return bom + sourceText(lines.filter((line, index) => index >= rootEnd || !line.structural + || !rootTomlString(line.text, "model")?.includes("/"))); } /** @@ -551,28 +508,16 @@ export function stripRootRoutedModel(content: string): string { * header (TOML root keys must precede all tables). If there are no tables, append it to the root body. */ export function setRootModelProvider(content: string): string { - const lines = content.split("\n"); - const firstTable = lines.findIndex((l) => /^\s*\[/.test(l)); - const key = 'model_provider = "opencodex"'; - if (firstTable === -1) { - return content.replace(/\n+$/, "") + "\n" + key + "\n"; - } - let insertAt = firstTable; - while (insertAt > 0 && lines[insertAt - 1].trim() === "") insertAt--; - lines.splice(insertAt, 0, key); - return lines.join("\n"); + return insertRootSourceLines(content, ['model_provider = "opencodex"']); } function readRootModelCatalogPath(content: string): string | null { - const lines = content.split("\n"); - const firstTable = lines.findIndex((line) => /^\s*\[/.test(line)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; - const modelCatalogAssignment = tomlStringPattern("model_catalog_json"); + const { lines, rootEnd } = rootSourceLines(content); let ownedCatalogPath: string | null = null; for (let index = 0; index < rootEnd; index += 1) { - const match = modelCatalogAssignment.exec(lines[index]); - if (!match) continue; - const catalogPath = parseTomlString(match[1]); + if (!lines[index].structural) continue; + const catalogPath = rootTomlString(lines[index].text, "model_catalog_json"); + if (catalogPath === null) continue; if (!isOpencodexCatalogPath(catalogPath)) return catalogPath; ownedCatalogPath ??= catalogPath; } @@ -580,18 +525,14 @@ function readRootModelCatalogPath(content: string): string | null { } export function setRootModelCatalogPath(content: string, catalogPath: string): string { - const lines = content.split("\n"); - const firstTable = lines.findIndex((l) => /^\s*\[/.test(l)); + const { bom, lines, rootEnd } = rootSourceLines(content); const key = `model_catalog_json = ${tomlString(catalogPath)}`; - const modelCatalogAssignment = tomlStringPattern("model_catalog_json"); - const rootEnd = firstTable === -1 ? lines.length : firstTable; const ownedAssignments: number[] = []; let hasUserAssignment = false; for (let i = 0; i < rootEnd; i++) { - const m = modelCatalogAssignment.exec(lines[i]); - if (!m) continue; - const existing = parseTomlString(m[1]); - if (isOpencodexCatalogPath(existing)) { + if (!lines[i].structural || rootAssignmentKey(lines[i].text) !== "model_catalog_json") continue; + const existing = rootTomlString(lines[i].text, "model_catalog_json"); + if (existing !== null && isOpencodexCatalogPath(existing)) { ownedAssignments.push(i); } else { hasUserAssignment = true; @@ -599,53 +540,38 @@ export function setRootModelCatalogPath(content: string, catalogPath: string): s } if (hasUserAssignment) { const owned = new Set(ownedAssignments); - return lines.filter((_, index) => !owned.has(index)).join("\n"); + return bom + sourceText(lines.filter((_, index) => !owned.has(index))); } if (ownedAssignments.length > 0) { - lines[ownedAssignments[0]] = key; + lines[ownedAssignments[0]].text = key; const duplicates = new Set(ownedAssignments.slice(1)); - return lines.filter((_, index) => !duplicates.has(index)).join("\n"); + return bom + sourceText(lines.filter((_, index) => !duplicates.has(index))); } - if (firstTable === -1) { - return content.replace(/\n+$/, "") + "\n" + key + "\n"; - } - let insertAt = firstTable; - while (insertAt > 0 && lines[insertAt - 1].trim() === "") insertAt--; - lines.splice(insertAt, 0, key); - return lines.join("\n"); + return insertRootSourceLines(content, [key]); } export function removeProfileSection(content: string): string { - const lines = content.split("\n"); - const filtered: string[] = []; + const { bom, lines } = rootSourceLines(content); let inProfile = false; - for (const line of lines) { - if (line.trim() === "[profiles.opencodex]") { - inProfile = true; - continue; - } - if (inProfile) { - if (/^\s*\[/.test(line) && line.trim() !== "[profiles.opencodex]") { - inProfile = false; - filtered.push(line); - } - continue; + const kept = lines.filter(line => { + if (line.structural && /^\s*\[/.test(line.text)) { + try { + const parsed = Bun.TOML.parse(line.text) as Record; + inProfile = typeof parsed.profiles === "object" && parsed.profiles !== null + && Object.hasOwn(parsed.profiles, "opencodex"); + } catch { inProfile = false; } } - filtered.push(line); - } - return ( - filtered - .join("\n") - .replace(/\n{3,}/g, "\n\n") - .trimEnd() + "\n" - ); + return !inProfile; + }); + return normalizeStructuralWhitespace(bom + sourceText(kept)); } export function normalizeServiceTier(content: string): string { - return content.replace( - /^(\s*service_tier\s*=\s*)["']priority["']\s*$/gm, - '$1"fast"', - ); + const { bom, lines, rootEnd } = rootSourceLines(content); + for (const line of lines.slice(0, rootEnd)) { + if (line.structural) line.text = line.text.replace(/^(\s*service_tier\s*=\s*)["']priority["']\s*$/, '$1"fast"'); + } + return bom + sourceText(lines); } export function ensureFastModeFeature(content: string, fastMode?: boolean): string { @@ -654,32 +580,41 @@ export function ensureFastModeFeature(content: string, fastMode?: boolean): stri // untouched (no [features] table is added and an existing fast_mode line is // preserved as-is). Table and key matching accept the valid TOML spellings // `[features] # comment`, `["features"]` / `['features']`, and quoted keys. - const lines = content.split("\n"); - const featuresHeader = /^\s*\[(["']?)\s*features\s*\1\]\s*(?:#.*)?$/; - const fastModeKey = /^\s*(?:"fast_mode"|'fast_mode'|fast_mode)\s*=/; - const featuresStart = lines.findIndex(line => featuresHeader.test(line)); + if (fastMode === undefined) return content; + const { bom, lines } = rootSourceLines(content); + const eol = lines.find(line => line.eol)?.eol || "\n"; + const featuresStart = lines.findIndex(line => { + if (!line.structural || !/^\s*\[/.test(line.text)) return false; + try { + const parsed = Bun.TOML.parse(line.text) as Record; + return Object.keys(parsed).length === 1 && typeof parsed.features === "object" + && parsed.features !== null && !Array.isArray(parsed.features) + && Object.keys(parsed.features).length === 0; + } catch { return false; } + }); if (featuresStart === -1) { - if (fastMode === undefined) return content; - return content.trimEnd() + "\n\n[features]\nfast_mode = " + (fastMode ? "true" : "false") + "\n"; + return content.replace(/[\r\n]+$/, "") + eol + eol + "[features]" + eol + + "fast_mode = " + (fastMode ? "true" : "false") + eol; } const nextTable = lines.findIndex( - (line, index) => index > featuresStart && /^\s*\[/.test(line), + (line, index) => index > featuresStart && line.structural && /^\s*\[/.test(line.text), ); const featuresEnd = nextTable === -1 ? lines.length : nextTable; for (let i = featuresStart + 1; i < featuresEnd; i++) { - if (fastModeKey.test(lines[i])) { - if (fastMode === undefined) return lines.join("\n"); - lines[i] = lines[i].replace(/^(\s*)(?:"fast_mode"|'fast_mode'|fast_mode)\s*=.*$/, `$1fast_mode = ${fastMode ? "true" : "false"}`); - return lines.join("\n"); + if (lines[i].structural && rootAssignmentKey(lines[i].text) === "fast_mode") { + // Refuse to rewrite an incomplete physical view of a collection/string value. + try { Bun.TOML.parse(lines[i].text); } catch { return content; } + lines[i].text = `${/^\s*/.exec(lines[i].text)![0]}fast_mode = ${fastMode ? "true" : "false"}`; + return bom + sourceText(lines); } } - if (fastMode === undefined) return lines.join("\n"); let insertAt = featuresEnd; - while (insertAt > featuresStart + 1 && lines[insertAt - 1].trim() === "") insertAt--; - lines.splice(insertAt, 0, `fast_mode = ${fastMode ? "true" : "false"}`); - return lines.join("\n"); + while (insertAt > featuresStart + 1 && lines[insertAt - 1].structural && lines[insertAt - 1].text.trim() === "") insertAt--; + if (insertAt > 0 && !lines[insertAt - 1].eol) lines[insertAt - 1].eol = eol; + lines.splice(insertAt, 0, { text: `fast_mode = ${fastMode ? "true" : "false"}`, eol, structural: true }); + return bom + sourceText(lines); } function isOpencodexCatalogPath(path: string): boolean { @@ -687,17 +622,13 @@ function isOpencodexCatalogPath(path: string): boolean { } export function stripOpencodexCatalogPath(content: string): string { - const modelCatalogAssignment = tomlStringPattern("model_catalog_json"); - const lines = content.split("\n"); - const firstTable = lines.findIndex((line) => /^\s*\[/.test(line)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; - return lines + const { bom, lines, rootEnd } = rootSourceLines(content); + return bom + sourceText(lines .filter((line, index) => { - if (index >= rootEnd) return true; - const m = modelCatalogAssignment.exec(line); - return !m || !isOpencodexCatalogPath(parseTomlString(m[1])); - }) - .join("\n"); + if (index >= rootEnd || !line.structural) return true; + const path = rootTomlString(line.text, "model_catalog_json"); + return path === null || !isOpencodexCatalogPath(path); + })); } export function buildProfileFile(port: number, catalogPath?: string | null, supportsWebsockets?: boolean, includeApiAuthHeader?: boolean, hostname?: string, fastMode?: boolean): string; diff --git a/src/codex/inject/provider-table.ts b/src/codex/inject/provider-table.ts index 8d86b7151ea..b7c2a3d4a9f 100644 --- a/src/codex/inject/provider-table.ts +++ b/src/codex/inject/provider-table.ts @@ -1,5 +1,5 @@ import { isDeepStrictEqual } from "node:util"; -import { OCX_SECTION_MARKER } from "../injected-marker"; +import { OCX_ROUTING_MARKER_LINE, OCX_SECTION_MARKER } from "../injected-marker"; import { splitSourceLines, type SourceLine } from "../toml-source-lines"; function record(value: unknown): Record | null { @@ -61,7 +61,10 @@ function providerLines(content: string): { bom: string; lines: SourceLine[]; own section = headerKind(text); if (section === "provider") { const previous = lines[index - 1]; - if (previous?.structural && previous.text.trim() === OCX_SECTION_MARKER) owned.add(index - 1); + const marker = previous?.text.trim(); + if (previous?.structural && (marker === OCX_SECTION_MARKER || marker === OCX_ROUTING_MARKER_LINE)) { + owned.add(index - 1); + } } } else if (line.structural && unsupportedProviderAssignment(text, section)) { // These forms have no independently removable table span. Do not append a diff --git a/src/codex/inject/remove.ts b/src/codex/inject/remove.ts index fac225b3f0e..d41e2dbb09f 100644 --- a/src/codex/inject/remove.ts +++ b/src/codex/inject/remove.ts @@ -1,4 +1,5 @@ import { existsSync, readFileSync, unlinkSync } from "node:fs"; +import { normalizeStructuralWhitespace, rootAssignmentKey, rootSourceLines, sourceAssignment, sourceText } from "../toml-source-lines"; import { atomicWriteFile } from "../../config"; import { REALTIME_WS_BASE_URL_KEY, @@ -13,7 +14,7 @@ import { journaledInjectedRootWebSearch, journaledReplacedRootWebSearch, } from "../journal"; -import { CODEX_CONFIG_PATH, CODEX_PROFILE_PATH, readRootTomlString } from "../paths"; +import { CODEX_CONFIG_PATH, CODEX_PROFILE_PATH } from "../paths"; import { transformManagedSubagentDefaults } from "../subagent-defaults"; import { applyEol, @@ -59,6 +60,15 @@ interface StripOpencodexConfigResult { managedDefaultsError: string | null; } +function hasRootOcxProvider(content: string): boolean { + const { lines, rootEnd } = rootSourceLines(content); + return lines.slice(0, rootEnd).some((line, index) => { + if (!line.structural || rootAssignmentKey(line.text) !== "model_provider") return false; + const value = sourceAssignment(lines, index, rootEnd)?.value; + return typeof value === "string" && value.trim() === "opencodex"; + }); +} + /** * Detailed form used by the on-disk restore path. A damaged ownership marker is * ambiguous: keep the associated value, but return the transform error so the @@ -73,7 +83,7 @@ function stripOpencodexConfigResult( ): StripOpencodexConfigResult { let out = content; const hadRootOcxProvider = - readRootTomlString(out, "model_provider") === "opencodex"; + hasRootOcxProvider(out); // #1798: marker adjacency is FORMATTING evidence, and a Codex app rewrite keeps values // while dropping comments. Fall back to VALUE evidence -- the exact URL we recorded // writing -- so an app-rewritten config is still recognized as ours. @@ -94,10 +104,15 @@ function stripOpencodexConfigResult( out = removeProfileSection(out); // Regex (not exact-string) removal so compact `model_provider="opencodex"` is stripped too — // must match the detection regex above, or a detected line could survive un-removed. - out = out - .split("\n") - .filter((l) => !/^\s*model_provider\s*=\s*"opencodex"\s*$/.test(l)) - .join("\n"); + const routingSource = rootSourceLines(out); + const providerLines = new Set(); + routingSource.lines.forEach((line, index) => { + if (!line.structural || rootAssignmentKey(line.text) !== "model_provider") return; + const assignment = sourceAssignment(routingSource.lines, index); + if (!assignment || typeof assignment.value !== "string" || assignment.value.trim() !== "opencodex") return; + for (let at = index; at < assignment.end; at++) providerLines.add(at); + }); + out = routingSource.bom + sourceText(routingSource.lines.filter((_, index) => !providerLines.has(index))); // Routed root model ids (`model = "provider/slug"`) only make sense while the proxy serves // them — strip on both the legacy re-tag form and the Design B injected-base-url form. if (hadRootOcxProvider || hadInjectedBaseUrl) out = stripRootRoutedModel(out); @@ -105,7 +120,7 @@ function stripOpencodexConfigResult( if (managedDefaults.ok) out = managedDefaults.content; out = stripOpencodexCatalogPath(out); return { - content: out.replace(/\n{3,}/g, "\n\n").trimEnd() + "\n", + content: normalizeStructuralWhitespace(out), managedDefaultsError: !managedDefaults.ok ? managedDefaults.error : null, }; } @@ -118,7 +133,7 @@ export function stripOpencodexConfig(content: string): string { function hasOpencodexRouting(content: string): boolean { return ( hasOcxProviderTable(content) || - /^\s*model_provider\s*=\s*"opencodex"/m.test(content) || + hasRootOcxProvider(content) || hasInjectedOpenaiBaseUrl(content) ); } diff --git a/src/codex/injected-marker.ts b/src/codex/injected-marker.ts index bf27f8185aa..e1bb40a1440 100644 --- a/src/codex/injected-marker.ts +++ b/src/codex/injected-marker.ts @@ -8,6 +8,7 @@ * two public predicates, so external callers see no change. */ import { parseTomlString } from "./paths"; +import { rootAssignmentKey, rootSourceLines, sourceText } from "./toml-source-lines"; export const OCX_SECTION_MARKER = "# Auto-injected by opencodex"; @@ -20,17 +21,22 @@ export const OCX_SECTION_MARKER = "# Auto-injected by opencodex"; * and it said nothing but "Auto-injected by opencodex" — so the recovery they found was to * hand-delete lines and the catalog file, which is strictly worse than `ocx restore`. * - * Every ownership predicate below matches on {@link OCX_SECTION_MARKER} as a SUBSTRING, never by - * equality, so this longer line is recognized by them and by any opencodex old enough to predate - * it. That is the whole reason the hint is appended to the marker instead of occupying a line of - * its own: a separate comment line would survive removal as orphaned text. + * Root routing predicates and lossless provider table capture/removal accept either exact marker + * line directly above structural routing syntax; they do not claim user comments or opaque value + * lines that merely contain the marker. Keeping the hint on the same + * line lets those transforms remove it together with the routing they own. * * Scope is routing only. Prompt layers keep the bare marker: `ocx restore` is not their undo. */ export const OCX_ROUTING_MARKER_LINE = `${OCX_SECTION_MARKER} (undo: ocx restore)`; +export function isOcxRoutingMarkerLine(line: string): boolean { + const text = line.trim(); + return text === OCX_SECTION_MARKER || text === OCX_ROUTING_MARKER_LINE; +} + export function isRootOpenaiBaseUrlLine(line: string): boolean { - return /^\s*openai_base_url\s*=/.test(line); + return rootAssignmentKey(line) === "openai_base_url"; } /** @@ -44,7 +50,7 @@ export function isRootOpenaiBaseUrlLine(line: string): boolean { export const REALTIME_WS_BASE_URL_KEY = "experimental_realtime_ws_base_url"; export function isRootRealtimeWsBaseUrlLine(line: string): boolean { - return /^\s*experimental_realtime_ws_base_url\s*=/.test(line); + return rootAssignmentKey(line) === REALTIME_WS_BASE_URL_KEY; } export function tomlStringPattern(key: string): RegExp { @@ -58,13 +64,13 @@ export function tomlStringPattern(key: string): RegExp { } export function rootTomlString(content: string, key: string): string | null { - const lines = content.split("\n"); - const firstTable = lines.findIndex(line => /^\s*\[/.test(line)); - const rootLines = lines.slice(0, firstTable === -1 ? lines.length : firstTable); - const pattern = tomlStringPattern(key); - for (const line of rootLines) { - const match = pattern.exec(line); - if (match?.[1]) return parseTomlString(match[1]).trim(); + const { lines, rootEnd } = rootSourceLines(content); + for (const line of lines.slice(0, rootEnd)) { + if (!line.structural || rootAssignmentKey(line.text) !== key) continue; + try { + const value = (Bun.TOML.parse(line.text) as Record)[key]; + if (typeof value === "string") return value.trim(); + } catch { /* Incomplete multiline assignments are not single-line ownership evidence. */ } } return null; } @@ -103,12 +109,11 @@ export function stripJournaledOpenaiBaseUrl( injectedRealtimeWsUrl: string | null = null, ): string { if (!injectedUrl && !injectedRealtimeWsUrl) return content; - const lines = content.split(String.fromCharCode(10)); - const firstTable = lines.findIndex(l => /^\s*\[/.test(l)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; + const { bom, lines, rootEnd } = rootSourceLines(content); const drop = new Set(); for (let i = 0; i < rootEnd; i++) { - const line = lines[i]!; + if (!lines[i]!.structural) continue; + const line = lines[i]!.text; // Each key is matched against ITS OWN recorded value. The realtime override is // journaled separately so a user-owned override that happens to equal the proxy // URL is never mistaken for ours. @@ -122,18 +127,18 @@ export function stripJournaledOpenaiBaseUrl( drop.add(i); // Take an ownership marker directly above it too, so repeated cycles cannot // accumulate orphaned comments. - if (i > 0 && lines[i - 1]!.includes(OCX_SECTION_MARKER)) drop.add(i - 1); + if (i > 0 && lines[i - 1]!.structural && isOcxRoutingMarkerLine(lines[i - 1]!.text)) drop.add(i - 1); } if (drop.size === 0) return content; - return lines.filter((_, i) => !drop.has(i)).join(String.fromCharCode(10)); + return bom + sourceText(lines.filter((_, i) => !drop.has(i))); } export function hasInjectedOpenaiBaseUrl(content: string): boolean { - const lines = content.split("\n"); - const firstTable = lines.findIndex(l => /^\s*\[/.test(l)); - const rootEnd = firstTable === -1 ? lines.length : firstTable; + const { lines, rootEnd } = rootSourceLines(content); for (let i = 1; i < rootEnd; i++) { - if (isRootOpenaiBaseUrlLine(lines[i]!) && lines[i - 1]!.includes(OCX_SECTION_MARKER)) return true; + if (lines[i]!.structural && lines[i - 1]!.structural + && isRootOpenaiBaseUrlLine(lines[i]!.text) && rootTomlString(lines[i]!.text, "openai_base_url") !== null + && isOcxRoutingMarkerLine(lines[i - 1]!.text)) return true; } return false; } diff --git a/src/codex/toml-source-lines.ts b/src/codex/toml-source-lines.ts index e85dd8b187d..1085bd1e3be 100644 --- a/src/codex/toml-source-lines.ts +++ b/src/codex/toml-source-lines.ts @@ -2,7 +2,7 @@ export interface SourceLine { text: string; eol: "\r\n" | "\n" | ""; - /** False when this physical line began inside a TOML multiline string. */ + /** False when this physical line began inside a multiline string or collection. */ structural: boolean; } @@ -28,6 +28,98 @@ export function splitSourceLines(content: string): SourceLine[] { return lines; } +/** Root boundaries exclude headers and assignments that are merely multiline/array data. */ +export function rootSourceLines(content: string): { bom: string; lines: SourceLine[]; rootEnd: number } { + const bom = content.startsWith("\uFEFF") ? "\uFEFF" : ""; + const lines = splitSourceLines(content.slice(bom.length)); + const firstTable = lines.findIndex(line => line.structural && /^\s*\[/.test(line.text)); + return { bom, lines, rootEnd: firstTable === -1 ? lines.length : firstTable }; +} + +export function sourceText(lines: readonly SourceLine[]): string { + return lines.map(line => line.text + line.eol).join(""); +} + +const assignmentKeyToken = /^[ \t]*([A-Za-z0-9_-]+|"(?:[^"\\]|\\.)*"|'[^']*')[ \t]*=/; + +/** Decode only a single key token, without parsing unrelated values or dotted paths. */ +export function rootAssignmentKey(text: string): string | null { + const match = assignmentKeyToken.exec(text); + if (!match) return null; + try { + return Object.keys(Bun.TOML.parse(`${match[1]} = true\n`))[0] ?? null; + } catch { + return null; + } +} + +interface AssignmentSpan { key: string; start: number; end: number; text: string } + +function lexicalAssignmentSpan(lines: readonly SourceLine[], start: number, limit: number): AssignmentSpan | null { + const line = lines[start]; + if (!line?.structural) return null; + const key = rootAssignmentKey(line.text); + if (key === null) return null; + let end = start + 1; + while (end < limit && !lines[end]!.structural) end += 1; + // Store the original assignment bytes, excluding only its final document separator. + const text = sourceText(lines.slice(start, end)).replace(/\r?\n$/, ""); + return { key, start, end, text }; +} + +/** A complete assignment whose value can be decoded without losing precision. */ +export function sourceAssignment(lines: readonly SourceLine[], start: number, limit = lines.length): + (AssignmentSpan & { value: unknown }) | null { + const span = lexicalAssignmentSpan(lines, start, limit); + if (!span) return null; + try { + const parsed = Bun.TOML.parse(span.text) as Record; + return Object.keys(parsed).length === 1 && Object.hasOwn(parsed, span.key) + ? { ...span, value: parsed[span.key] } : null; + } catch { return null; } +} + +/** Validate complete syntax for removal/capture without requiring a JavaScript value. */ +export function sourceAssignmentSpan(lines: readonly SourceLine[], start: number, limit = lines.length): AssignmentSpan | null { + const decoded = sourceAssignment(lines, start, limit); + if (decoded) return decoded; + const span = lexicalAssignmentSpan(lines, start, limit); + // Bun rejects valid TOML integers outside the JS safe-integer range. Only a complete + // scalar integer gets this exception: collections, strings and other parse failures do not. + if (!span || span.end !== start + 1) return null; + const keyToken = assignmentKeyToken.exec(span.text)!; + const scalar = /^([ \t]*)([+-]?(?:0|[1-9](?:_?[0-9])*)|0x[0-9A-Fa-f](?:_?[0-9A-Fa-f])*|0o[0-7](?:_?[0-7])*|0b[01](?:_?[01])*)([ \t]*(?:#[^\r\n]*)?)$/.exec(span.text.slice(keyToken[0].length)); + if (!scalar) return null; + const integer = BigInt(scalar[2]!.replaceAll("_", "")); + if (integer < -(1n << 63n) || integer >= (1n << 63n)) return null; + // Keep the parser responsible for the key, separator and comment grammar. Replacing + // this already validated integer for validation never edits or decodes the source value. + try { + const parsed = Bun.TOML.parse(`${keyToken[0]}${scalar[1]}0${scalar[3]}`); + return Object.keys(parsed).length === 1 && Object.hasOwn(parsed, span.key) ? span : null; + } catch { return null; } +} + +/** Cosmetic document whitespace only; opaque value lines retain every byte. */ +export function normalizeStructuralWhitespace(content: string): string { + const { bom, lines } = rootSourceLines(content); + const out: SourceLine[] = []; + let blankRun = 0; + for (const line of lines) { + if (line.structural && line.text === "") { + blankRun += 1; + if (blankRun > 1) continue; + } else { + blankRun = 0; + } + out.push(line); + } + while (out.at(-1)?.structural && out.at(-1)!.text.trim() === "") out.pop(); + const text = bom + sourceText(out); + const eol = lines.find(line => line.eol)?.eol || "\n"; + return text.endsWith("\n") ? text : text + eol; +} + type MultilineStringKind = "basic" | "literal" | null; /** diff --git a/structure/codex-home.md b/structure/codex-home.md index 90ff9cf3912..26a215b59fc 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -430,7 +430,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. If an exact journal restore brings back a same-named provider table, retention accepts it only when the parsed provider values match. `src/codex/inject/provider-table.ts` captures lossless table spans using the shared lexical lines in `src/codex/toml-source-lines.ts`, also consumed by the native defaults editor. Header-shaped text and blank lines inside multiline values stay intact; only the isolated provider block is parsed so unrelated large integers do not prevent retention. Cosmetic spacing and comments do not change equality, but multiline string contents do. Invalid, duplicate, array-root or unsupported inline provider definitions refuse; retained bytes are never regenerated from parsed values. 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. +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 parsed provider values match. `src/codex/inject/provider-table.ts` captures lossless table spans using the shared lexical lines in `src/codex/toml-source-lines.ts`, also consumed by the native defaults editor. Header-shaped text and blank lines inside multiline values stay intact; only the isolated provider block is parsed so unrelated large integers do not prevent retention. Capture and removal own only an adjacent structural comment whose trimmed text is exactly the bare ownership marker or its routing undo-hint form; marker substrings in user comments and multiline values remain untouched. Root URL and web-search ownership likewise requires an exact adjacent structural marker and a complete root assignment, with quoted keys decoded as TOML keys. Journal-recorded injected values are separate ownership evidence for marker-less app rewrites. Managed root assignment edits consume their complete validated lexical spans; value-free removal and capture accept scalar TOML integers throughout the signed 64-bit range without JavaScript number decoding, while value-based ownership still requires decoded values and malformed assignments refuse; the web-search journal field retains the whole replaced assignment, including multiline values, for re-enabling or fallback restore. Catalog and feature edits exclude header/key-shaped multiline data. Fallback blank-line normalization applies only to structural document lines, preserving opaque multiline values. Cosmetic spacing and comments do not change equality, but multiline string contents do. Invalid, duplicate, array-root or unsupported inline provider definitions refuse; retained bytes are never regenerated from parsed values. 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. diff --git a/tests/codex-integration/codex-config-preservation.test.ts b/tests/codex-integration/codex-config-preservation.test.ts new file mode 100644 index 00000000000..cc465baab33 --- /dev/null +++ b/tests/codex-integration/codex-config-preservation.test.ts @@ -0,0 +1,292 @@ +import { describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { repoRoot } from "../helpers/repo-root"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; +import { + setRootOpenaiBaseUrlForTarget, + setRootRealtimeWsBaseUrl, + stripInjectedOpenaiBaseUrl, + stripInjectedRootWebSearch, + removeProfileSection, + ensureRootWebSearchDisabled, ensureFastModeFeature, stripOpencodexCatalogPath, + setRootModelCatalogPath, + stripExistingModelProvider, stripRootContextWindowOverrides, +} from "../../src/codex/inject/config-toml"; +import { stripOpencodexConfig } from "../../src/codex/inject/remove"; +import { deriveCodexInjectionPlan } from "../../src/codex/inject/plan"; +import { rootSourceLines, sourceAssignment, sourceAssignmentSpan } from "../../src/codex/toml-source-lines"; +import { + hasInjectedOpenaiBaseUrl, OCX_ROUTING_MARKER_LINE, OCX_SECTION_MARKER, + stripJournaledOpenaiBaseUrl, +} from "../../src/codex/injected-marker"; + +const target = { baseUrl: "http://127.0.0.1:10100/v1", requiresAdmissionToken: false, tokenEnv: "OPENCODEX_API_AUTH_TOKEN" as const }; +const userUrl = "https://fixture.invalid/v1"; +const parse = (text: string) => Bun.TOML.parse(text.replace(/^\uFEFF/, "")); + +describe("structural root ownership and multiline preservation", () => { + test.each(["openai_base_url", '"openai_base_url"', "'openai_base_url'", '"openai_base_\\u0075rl"'])("user marker quotations never own the root URL (%s)", key => { + const content = `# user quote: ${OCX_ROUTING_MARKER_LINE}\n${key} = "${userUrl}"\nmodel = "user/model"\n`; + expect(hasInjectedOpenaiBaseUrl(content)).toBe(false); + expect(stripInjectedOpenaiBaseUrl(content)).toBe(content); + expect(parse(stripOpencodexConfig(content))).toEqual(parse(content)); + expect(setRootOpenaiBaseUrlForTarget(content, target)).toEqual({ content, keptUserBaseUrl: true }); + }); + + test.each([OCX_SECTION_MARKER, OCX_ROUTING_MARKER_LINE])("exact adjacent markers own quoted root keys (%s)", marker => { + const content = `${marker}\n"openai_base_url" = "${target.baseUrl}"\n${marker}\n'experimental_realtime_ws_base_url' = "${target.baseUrl}"\nmodel = "fixture-model"\n`; + expect(hasInjectedOpenaiBaseUrl(content)).toBe(true); + expect(stripInjectedOpenaiBaseUrl(content)).toBe('model = "fixture-model"\n'); + }); + + test("journal value evidence removes its own URL but preserves a user marker quotation", () => { + const comment = `# user quote: ${OCX_ROUTING_MARKER_LINE}\n`; + const content = `${comment}"openai_base_url" = "${target.baseUrl}"\nmodel = "fixture-model"\n`; + expect(stripJournaledOpenaiBaseUrl(content, target.baseUrl)).toBe(comment + 'model = "fixture-model"\n'); + expect(stripJournaledOpenaiBaseUrl(content, userUrl)).toBe(content); + }); + + test("an exact marker above a different root setting remains user data", () => { + const content = `${OCX_ROUTING_MARKER_LINE}\nmodel = "fixture-model"\nopenai_base_url = "${userUrl}"\n`; + expect(stripInjectedOpenaiBaseUrl(content)).toBe(content); + expect(hasInjectedOpenaiBaseUrl(content)).toBe(false); + }); + + const variants = ["\n", "\r\n"].flatMap(eol => ["", "\uFEFF"].flatMap(bom => + ['"""', "'''"].map(delimiter => ({ eol, bom, delimiter })))); + test.each(variants)("actual multiline web-search assignments survive an Off/On rollback (%j)", ({ eol, bom, delimiter }) => { + const assignment = ` 'web_search' = ${delimiter}\nlive${delimiter} # user mode`.replaceAll("\n", eol); + const content = bom + assignment + eol + `model = "fixture-model"${eol}[user]${eol}value = "keep"${eol}`; + const off = ensureRootWebSearchDisabled(content, true); + expect(parse(off.content).web_search).toBe("disabled"); + expect(off.replacedUserLine).toBe(assignment); + const journal = { injectedValue: off.wroteValue, replacedUserLine: off.replacedUserLine }; + expect(ensureRootWebSearchDisabled(off.content, true, journal).replacedUserLine).toBe(assignment); + const on = ensureRootWebSearchDisabled(off.content, false, journal).content; + expect(parse(on)).toEqual(parse(content)); + expect(on).toContain(assignment); + expect(on.startsWith(bom)).toBe(true); + }); + + test.each(variants)("actual multiline provider assignments are removed as complete spans (%j)", ({ eol, bom, delimiter }) => { + const content = bom + `"model_provider" = ${delimiter}\nopenai${delimiter}\nmodel = "fixture-model"\n[user]\nvalue = "keep"\n`.replaceAll("\n", eol); + expect(parse(stripExistingModelProvider(content))).toEqual({ model: "fixture-model", user: { value: "keep" } }); + for (const requiresAdmissionToken of [false, true]) { + const plan = deriveCodexInjectionPlan(content, { config: undefined, routingTarget: { ...target, requiresAdmissionToken }, catalogPathOption: null, journalReadOnly: true }); + expect(plan.kind).toBe("ok"); + if (plan.kind !== "ok") throw new Error(plan.message); + expect(parse(plan.content).user).toEqual({ value: "keep" }); + expect(parse(plan.baselineContent)).toEqual(parse(content)); + } + }); + + test.each(['"""', "'''"])("journal value evidence strips a complete marker-less multiline value (%s)", delimiter => { + const content = `web_search = ${delimiter}\ndisabled${delimiter}\nmodel = "fixture-model"\n`; + expect(stripInjectedRootWebSearch(content)).toBe(content); + expect(stripInjectedRootWebSearch(content, "disabled")).toBe('model = "fixture-model"\n'); + expect(stripInjectedRootWebSearch(content, "live")).toBe(content); + const spaced = content.replace("disabled", " disabled "); + expect(stripInjectedRootWebSearch(spaced, "disabled")).toBe(spaced); + const child = `[user]\nmodel_provider = ${delimiter}\nexternal${delimiter}\n`; + expect(stripExistingModelProvider(child)).toBe(child); + const owned = child.replace("external", "opencodex"); + expect(parse(stripOpencodexConfig(owned))).toEqual({ user: {} }); + }); + + test("context override removal consumes the complete collection without touching its neighbor", () => { + const content = 'model_context_window = [\n1,\n2\n]\nmodel = "fixture-model"\n'; + expect(parse(stripRootContextWindowOverrides(content))).toEqual({ model: "fixture-model" }); + }); + + test.each([ + "9007199254740992", "+9007199254740992", "-9007199254740992", + "9223372036854775807", "-9223372036854775808", "9_223_372_036_854_775_807", + "-9_223_372_036_854_775_808", "0x7fff_ffff_ffff_ffff", "0o777777777777777777777", + "0b111111111111111111111111111111111111111111111111111111111111111", + ])("value-free removal accepts a complete signed 64-bit integer (%s)", numeric => { + for (const eol of ["\n", "\r\n"]) for (const bom of ["", "\uFEFF"]) { + const neighbor = `model = "fixture-model"${eol}# user comment${eol}[child]${eol}value = "keep"${eol}`; + const key = numeric === "9007199254740992" || numeric === "9223372036854775807" + ? "model_context_window" : "'model_context_window'"; + const assignment = ` ${key} = ${numeric} # fixture integer`; + const content = bom + assignment + eol + neighbor; + const source = rootSourceLines(content); + expect(sourceAssignment(source.lines, 0, source.rootEnd)).toBeNull(); + expect(sourceAssignmentSpan(source.lines, 0, source.rootEnd)?.text).toBe(assignment); + expect(stripRootContextWindowOverrides(content)).toBe(bom + neighbor); + for (const requiresAdmissionToken of [false, true]) { + const plan = deriveCodexInjectionPlan(content, { config: undefined, routingTarget: { ...target, requiresAdmissionToken }, catalogPathOption: null, journalReadOnly: true }); + expect(plan.kind).toBe("ok"); + if (plan.kind !== "ok") throw new Error(plan.message); + expect(parse(plan.content).model).toBe("fixture-model"); + expect(plan.baselineContent).toBe(content); + } + expect(stripExistingModelProvider(content.replace("model_context_window", "model_provider"))).toBe(bom + neighbor); + const webSearch = content.replace("model_context_window", "web_search"); + const off = ensureRootWebSearchDisabled(webSearch, true); + expect(off.replacedUserLine).toBe(assignment.replace("model_context_window", "web_search")); + const journal = { injectedValue: off.wroteValue, replacedUserLine: off.replacedUserLine }; + expect(ensureRootWebSearchDisabled(off.content, false, journal).content).toContain(off.replacedUserLine!); + const child = bom + neighbor + `model_provider = ${numeric}${eol}`; + expect(stripExistingModelProvider(child)).toBe(child); + } + }); + + test.each([ + "9223372036854775808", "-9223372036854775809", "0x8000000000000000", + "0o1000000000000000000000", "+0x7fffffffffffffff", "-0x7fffffffffffffff", + "09007199254740992", "9__007199254740992", "9007199254740992_", "_9007199254740992", + "9007199254740992 garbage", "9007199254740992 1", "9007199254740992 # invalid\u0001comment", + "[9007199254740992", '"""unclosed', "'''unclosed", "", "0x", "0b2", + ])("value-free removal rejects invalid or incomplete integer assignments (%s)", value => { + const content = `model_context_window = ${value}\nmodel = "fixture-model"\n`; + expect(() => stripRootContextWindowOverrides(content)).toThrow(); + expect(() => stripExistingModelProvider(content.replace("model_context_window", "model_provider"))).toThrow(); + expect(() => ensureRootWebSearchDisabled(content.replace("model_context_window", "web_search"), true)).toThrow(); + }); + test.each(variants)("restore and injection preserve opaque multiline bytes (%j)", ({ eol, bom, delimiter }) => { + const value = `a\n${OCX_ROUTING_MARKER_LINE}\n\n\nb\n# user quote: ${OCX_SECTION_MARKER}\nopenai_base_url = "https://data.invalid/v1"\n[profiles.opencodex]\n[not_a_table]\nmodel_provider = "opencodex"\nservice_tier = "priority"\n${OCX_ROUTING_MARKER_LINE}\nweb_search = "disabled"\nz`; + const content = bom + `instructions = ${delimiter}${value}${delimiter}\n"openai_base_url" = "${userUrl}"\n'web_search' = "live"\n[child]\nnote = ${delimiter}first\n\n\nlast${delimiter}\n[child.headers]\n"x-fixture" = "keep"\n`.replaceAll("\n", eol); + expect(stripInjectedOpenaiBaseUrl(content)).toBe(content); + expect(stripInjectedRootWebSearch(content)).toBe(content); + expect(removeProfileSection(content)).toBe(content); + expect(hasInjectedOpenaiBaseUrl(content)).toBe(false); + expect(setRootOpenaiBaseUrlForTarget(content, target)).toEqual({ content, keptUserBaseUrl: true }); + expect(setRootRealtimeWsBaseUrl(content, target).content).toBe(content); + expect(parse(stripOpencodexConfig(content))).toEqual(parse(content)); + const plan = deriveCodexInjectionPlan(content, { config: undefined, routingTarget: target, catalogPathOption: null, journalReadOnly: true }); + expect(plan.kind).toBe("ok"); + if (plan.kind !== "ok") throw new Error(plan.message); + const next = parse(plan.content); + const original = parse(content); + expect(next.instructions).toBe(original.instructions); + expect(next.child).toEqual(original.child); + expect(next.openai_base_url).toBe(userUrl); + expect(next.web_search).toBe("live"); + expect(plan.content.startsWith(bom)).toBe(true); + }); + + test.each(["\n", "\r\n"])("structural whitespace cleanup leaves multiline blank lines and child tables intact (%j)", eol => { + const content = `# user comment\n\n\n\nmodel = "fixture-model"\n[child]\nbasic = """a\n\n\nb"""\nliteral = '''c\n\n\nd'''\n[child.headers]\n"x-fixture" = "keep"\n\n\n`.replaceAll("\n", eol); + const result = stripOpencodexConfig(content); + expect(parse(result)).toEqual(parse(content)); + expect(result).toContain(`a${eol}${eol}${eol}b`); + expect(result).toContain(`c${eol}${eol}${eol}d`); + expect(result).toContain('[child.headers]'); + }); + + test.each([ + { name: "strip catalog", transform: stripOpencodexCatalogPath }, + { name: "set catalog", transform: (content: string) => setRootModelCatalogPath(content, "fixture-catalog.json") }, + { name: "feature toggle", transform: (content: string) => ensureFastModeFeature(content, true) }, + { name: "web-search toggle", transform: (content: string) => ensureRootWebSearchDisabled(content, true).content }, + ])("key-shaped string data remains opaque (%j)", ({ transform }) => { + for (const { eol, bom, delimiter } of variants) { + const content = bom + `instructions = ${delimiter}a\nmodel_catalog_json = "opencodex-catalog.json"\n[features]\nfast_mode = false\nweb_search = "live"\nz${delimiter}\n[child]\nvalue = "keep"\n`.replaceAll("\n", eol); + const result = transform(content); + expect(parse(result).instructions).toBe(parse(content).instructions); + expect(parse(result).child).toEqual(parse(content).child); + expect(result.startsWith(bom)).toBe(true); + if (eol === "\r\n") expect(result.replaceAll("\r\n", "")).not.toContain("\n"); + } + }); + + test("provider-shaped string data cannot own a user's routed root model", () => { + const content = 'instructions = """a\nmodel_provider = "opencodex"\nz"""\nmodel = "user/model"\n'; + expect(parse(stripOpencodexConfig(content))).toEqual(parse(content)); + }); + + test.each(['["features"]', "['features']", '["f\\u0065atures"]'])("real quoted feature keys are updated outside opaque values (%s)", header => { + const content = `\uFEFF${header}\r\nnote = '''a\r\nfast_mode = false\r\n\r\n\r\nb'''\r\n'fast_mode' = false\r\n[features.child]\r\nfast_mode = false\r\n`; + const result = ensureFastModeFeature(content, true); + const original = parse(content); + const next = parse(result); + expect((next.features as Record).fast_mode).toBe(true); + expect((next.features as Record).note).toBe((original.features as Record).note); + expect((next.features as Record).child).toEqual((original.features as Record).child); + expect(result.startsWith("\uFEFF")).toBe(true); + expect(result.replaceAll("\r\n", "")).not.toContain("\n"); + }); + + test("owned marker adjacency does not overwrite an incomplete single-line view of a multiline URL", () => { + const content = `${OCX_ROUTING_MARKER_LINE}\nopenai_base_url = """https://fixture.invalid/v1"""\ninstructions = """a\\\n \n b"""\n`; + const multiline = content.replace('"""https://fixture.invalid/v1"""', '"""\nhttps://fixture.invalid/v1\n"""'); + expect(stripInjectedOpenaiBaseUrl(multiline)).toBe(multiline); + expect(setRootOpenaiBaseUrlForTarget(multiline, target)).toEqual({ content: multiline, keptUserBaseUrl: true }); + expect(parse(stripOpencodexConfig(multiline))).toEqual(parse(multiline)); + }); + + test.each(['"""', "'''"])("artifact failure rollback and journal fallback preserve actual multiline settings (%s)", delimiter => { + const fixture = mkdtempSync(join(tmpdir(), "ocx-managed-span-")); + const codexHome = join(fixture, "codex"); + const ocxHome = join(fixture, "opencodex"); + mkdirSync(codexHome); + mkdirSync(ocxHome); + const assignment = ` 'web_search' = ${delimiter}\nlive${delimiter} # user mode`; + const native = `${assignment}\nmodel_provider = ${delimiter}\nopenai${delimiter}\nmodel = "fixture-model"\n[user]\nvalue = "keep"\n`; + writeFileSync(join(codexHome, "config.toml"), native); + const script = ` + import assert from "node:assert/strict"; + import { readFileSync, appendFileSync, existsSync } from "node:fs"; + import { createHash } from "node:crypto"; + import { join } from "node:path"; + import { injectCodexConfig, restoreNativeCodex, setHistoryArtifactStageForTests } from "./src/codex/inject"; + const configPath = join(process.env.CODEX_HOME, "config.toml"); + const profilePath = join(process.env.CODEX_HOME, "opencodex.config.toml"); + const journalPath = join(process.env.CODEX_HOME, "opencodex-journal.json"); + const original = readFileSync(configPath, "utf8"); + const hash = text => createHash("sha256").update(text).digest("hex"); + let hit = false; + setHistoryArtifactStageForTests(stage => { + if (stage === "after-config") { hit = true; throw new Error("synthetic artifact failure"); } + }); + let failed; + try { failed = await injectCodexConfig(10100, { webSearchSidecar: { enabled: false } }); } + catch (error) { assert.match(String(error), /synthetic artifact failure/); } + assert.equal(hit, true); + assert.notEqual(failed?.success, true); + assert.equal(readFileSync(configPath, "utf8"), original); + assert.equal(existsSync(profilePath), false); + assert.equal(existsSync(journalPath), false); + setHistoryArtifactStageForTests(undefined); + const injected = await injectCodexConfig(10100, { webSearchSidecar: { enabled: false } }); + assert.equal(injected.success, true); + const firstJournal = JSON.parse(readFileSync(journalPath, "utf8")); + assert.equal(firstJournal.replacedRootWebSearch, process.env.TEST_ORIGINAL_WEB_SEARCH); + assert.equal(firstJournal.injectedConfigHash, hash(readFileSync(configPath, "utf8"))); + const exact = restoreNativeCodex({ skipHistory: true, removeProviderTable: true }); + assert.equal(exact.success, true); + assert.equal(exact.artifacts.config.action, "journal-restored"); + assert.equal(readFileSync(configPath, "utf8"), original); + const reinjected = await injectCodexConfig(10100, { webSearchSidecar: { enabled: false } }); + assert.equal(reinjected.success, true); + const journal = JSON.parse(readFileSync(journalPath, "utf8")); + assert.equal(journal.replacedRootWebSearch, process.env.TEST_ORIGINAL_WEB_SEARCH); + assert.equal(journal.injectedConfigHash, hash(readFileSync(configPath, "utf8"))); + appendFileSync(configPath, '\\n[user_edit]\\nvalue = "keep edit"\\n'); + assert.notEqual(journal.injectedConfigHash, hash(readFileSync(configPath, "utf8"))); + const fallback = restoreNativeCodex({ skipHistory: true, removeProviderTable: true }); + assert.equal(fallback.success, true); + assert.equal(fallback.artifacts.config.action, "owned-fields-stripped"); + const after = readFileSync(configPath, "utf8"); + const parsed = Bun.TOML.parse(after); + assert.equal(parsed.web_search, "live"); + assert.deepEqual(parsed.user, { value: "keep" }); + assert.deepEqual(parsed.user_edit, { value: "keep edit" }); + assert.ok(after.includes(process.env.TEST_ORIGINAL_WEB_SEARCH)); + console.log(JSON.stringify({ rollback: true, exact: true, fallback: true })); + `; + try { + const child = spawnSync(process.execPath, ["--no-env-file", "--eval", script], { + cwd: repoRoot(), env: { ...process.env, CODEX_HOME: codexHome, CODEX_SQLITE_HOME: "", OPENCODEX_HOME: ocxHome, TEST_ORIGINAL_WEB_SEARCH: assignment }, + encoding: "utf8", timeout: SPAWN_BUDGET_MS, + }); + expect(child.status, child.error?.message ?? child.stderr).toBe(0); + expect(JSON.parse(child.stdout.trim())).toEqual({ rollback: true, exact: true, fallback: true }); + } finally { removeTreeWithRetry(fixture); } + }, SPAWN_BUDGET_MS); +}); diff --git a/tests/codex-integration/codex-provider-table-retention.test.ts b/tests/codex-integration/codex-provider-table-retention.test.ts index 1a42a8e12f0..b39067bd3e1 100644 --- a/tests/codex-integration/codex-provider-table-retention.test.ts +++ b/tests/codex-integration/codex-provider-table-retention.test.ts @@ -1,4 +1,13 @@ import { describe, expect, test } from "bun:test"; +import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { deriveCodexInjectionPlan } from "../../src/codex/inject/plan"; +import { OCX_ROUTING_MARKER_LINE, OCX_SECTION_MARKER } from "../../src/codex/injected-marker"; +import { repoRoot } from "../helpers/repo-root"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; import { appendOcxProviderTableBlock, extractOcxProviderTableBlock, @@ -104,3 +113,122 @@ describe("lossless retained provider tables", () => { expect(() => appendOcxProviderTableBlock(content, header + 'name = "retained"\n')).toThrow(); }); }); + +describe("provider table ownership markers", () => { + const variants = ["\n", "\r\n"].flatMap(eol => ["", "\uFEFF"].flatMap(bom => + [OCX_SECTION_MARKER, OCX_ROUTING_MARKER_LINE].map(marker => ({ eol, bom, marker })))); + + test.each(variants)("capture and removal own only the adjacent exact marker (%j)", ({ eol, bom, marker }) => { + const prefix = '# user comment\nmodel = "fixture-model"\n'.replaceAll("\n", eol); + const block = `${marker}\n${header}name = "fixture"\n`.replaceAll("\n", eol); + const foreign = '[user]\nnote = "keep"\n'.replaceAll("\n", eol); + expect(extractOcxProviderTableBlock(bom + prefix + block + foreign)).toBe(block); + expect(removeOcxSection(bom + prefix + block + foreign)).toBe(bom + prefix + foreign); + expect(appendOcxProviderTableBlock(bom + prefix + block + foreign, block)).toBe(bom + prefix + block + foreign); + }); + + test.each(["\n", "\r\n"])("real reinjection plans are byte idempotent (%j)", eol => { + for (const bom of ["", "\uFEFF"]) { + const ctx = { + config: undefined, + routingTarget: { baseUrl: "http://127.0.0.1:10100/v1", requiresAdmissionToken: true, tokenEnv: "OPENCODEX_API_AUTH_TOKEN" as const }, + catalogPathOption: null, + journalReadOnly: true, + }; + // A foreign table before the provider prevents root-marker cleanup from masking this bug. + const native = bom + '# user comment\nmodel = "fixture-model"\n[user]\nnote = "fixture"\n'.replaceAll("\n", eol); + const first = deriveCodexInjectionPlan(native, ctx); + expect(first.kind).toBe("ok"); + if (first.kind !== "ok") throw new Error(first.message); + let current = first.content; + for (let cycle = 0; cycle < 5; cycle++) { + const next = deriveCodexInjectionPlan(current, ctx); + expect(next.kind).toBe("ok"); + if (next.kind !== "ok") throw new Error(next.message); + expect(next.content).toBe(first.content); + expect(next.content.split(eol).filter(line => line.trim() === OCX_ROUTING_MARKER_LINE)).toHaveLength(1); + current = next.content; + } + expect(current.startsWith(bom + "# user comment")).toBe(true); + } + }); + + test.each([ + `${OCX_SECTION_MARKER} user note`, + `${OCX_ROUTING_MARKER_LINE} user note`, + `# user quoted ${OCX_ROUTING_MARKER_LINE}`, + `${OCX_ROUTING_MARKER_LINE}\n`, + ])("user comments and nonadjacent orphan markers remain untouched (%s)", comment => { + const prefix = `${comment}\n`; + const block = header + 'name = "fixture"\n'; + expect(extractOcxProviderTableBlock(prefix + block)).toBe(block); + expect(removeOcxSection(prefix + block)).toBe(prefix); + }); + + test.each(['"""', "'''"])("multiline marker data and separated child tables stay lossless (%s)", delimiter => { + const foreign = `[user]\nnote = ${delimiter}start\n${OCX_ROUTING_MARKER_LINE}\n[model_providers.opencodex]\nend${delimiter}\n`; + const root = `${OCX_ROUTING_MARKER_LINE}\n${header}name = "fixture"\n`; + const child = `${OCX_SECTION_MARKER}\n[model_providers.opencodex.http_headers]\n"x-fixture" = "value"\n`; + expect(extractOcxProviderTableBlock(root + foreign + child)).toBe(root + child); + expect(removeOcxSection(root + foreign + child)).toBe(foreign); + expect(removeOcxSection(foreign)).toBe(foreign); + }); + + test("provider ownership leaves root routing marker pairs intact", () => { + const root = `${OCX_ROUTING_MARKER_LINE}\nopenai_base_url = "https://fixture.invalid/v1"\n`; + const block = `${OCX_ROUTING_MARKER_LINE}\n${header}name = "fixture"\n`; + expect(removeOcxSection(root + block)).toBe(root); + expect(extractOcxProviderTableBlock(root + block)).toBe(block); + }); + + test("a journal byte mismatch takes fallback restore without orphaning the provider marker", () => { + const fixture = mkdtempSync(join(tmpdir(), "ocx-provider-marker-")); + const codexHome = join(fixture, "codex"); + const ocxHome = join(fixture, "opencodex"); + mkdirSync(codexHome); + mkdirSync(ocxHome); + writeFileSync(join(codexHome, "config.toml"), '# user comment\nmodel = "fixture-model"\n[user]\nnote = "fixture"\n'); + const script = ` + import { readFileSync, appendFileSync } from "node:fs"; + import { createHash } from "node:crypto"; + import { join } from "node:path"; + import { injectCodexConfig, restoreNativeCodex } from "./src/codex/inject"; + const configPath = join(process.env.CODEX_HOME, "config.toml"); + await injectCodexConfig(10100, { + port: 10100, providers: {}, defaultProvider: "openai", + codexDesktopAuthless: true, + }, { catalogPath: null }); + const before = readFileSync(configPath, "utf8"); + const journal = JSON.parse(readFileSync(join(process.env.CODEX_HOME, "opencodex-journal.json"), "utf8")); + const hash = content => createHash("sha256").update(content).digest("hex"); + if (!journal.injectedConfigHash || journal.injectedConfigHash !== hash(before)) { + throw new Error("fixture must have a journal matching the injected bytes"); + } + appendFileSync(configPath, '\\n[user_edit]\\nvalue = "keep"\\n'); + if (journal.injectedConfigHash === hash(readFileSync(configPath, "utf8"))) { + throw new Error("fixture must force a journal hash mismatch"); + } + const result = restoreNativeCodex({ skipHistory: true, removeProviderTable: true }); + console.log(JSON.stringify({ before, after: readFileSync(configPath, "utf8"), success: result.success, action: result.artifacts.config.action })); + `; + try { + const child = spawnSync(process.execPath, ["--no-env-file", "--eval", script], { + cwd: repoRoot(), + env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: ocxHome }, + encoding: "utf8", timeout: SPAWN_BUDGET_MS, + }); + if (child.error || child.status !== 0) throw new Error(`fixture child failed: ${child.error?.message ?? child.stderr}`); + const result = JSON.parse(child.stdout.trim()); + expect(result.before).toContain(OCX_ROUTING_MARKER_LINE); + expect(result.before).toContain("[model_providers.opencodex]"); + expect(result.success).toBe(true); + expect(result.action).toBe("owned-fields-stripped"); + expect(result.after).not.toContain(OCX_ROUTING_MARKER_LINE); + expect(result.after).not.toContain("[model_providers.opencodex]"); + expect(result.after).toContain("# user comment"); + expect(Bun.TOML.parse(result.after)).toEqual({ model: "fixture-model", user: { note: "fixture" }, user_edit: { value: "keep" } }); + } finally { + removeTreeWithRetry(fixture); + } + }, 2 * SPAWN_BUDGET_MS); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index f021ccb19b0..f6ccc922422 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,4 +1,5 @@ { + "codex-config-preservation.test.ts": "codex-integration", "service-desktop-startup-health.test.ts": "service", "service-desktop-startup.test.ts": "service", "startup-health-packaged-probe.test.ts": "server",