Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions scripts/test-layout/layout.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"version": 1,
"root": "tests",
"explicit": {
"codex-config-preservation.test.ts": "codex-integration",
"codex-credits.test.ts": "codex-integration",
"codex-credits-settings.test.ts": "codex-integration",
"codex-credits-probes.test.ts": "codex-integration",
Expand Down
427 changes: 179 additions & 248 deletions src/codex/inject/config-toml.ts

Large diffs are not rendered by default.

7 changes: 5 additions & 2 deletions src/codex/inject/provider-table.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown> | null {
Expand Down Expand Up @@ -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
Expand Down
31 changes: 23 additions & 8 deletions src/codex/inject/remove.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -94,18 +104,23 @@ 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<number>();
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);
const managedDefaults = transformManagedSubagentDefaults(out, null);
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,
};
}
Expand All @@ -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)
);
}
Expand Down
51 changes: 28 additions & 23 deletions src/codex/injected-marker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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";
}

/**
Expand All @@ -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 {
Expand All @@ -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<string, unknown>)[key];
if (typeof value === "string") return value.trim();
} catch { /* Incomplete multiline assignments are not single-line ownership evidence. */ }
}
return null;
}
Expand Down Expand Up @@ -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<number>();
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.
Expand All @@ -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;
}
Expand Down
94 changes: 93 additions & 1 deletion src/codex/toml-source-lines.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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<string, unknown>;
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;

/**
Expand Down
Loading
Loading