Repository navigation
fix: codex_cli picks its Codex model slug from a custom model_catalog_json - #185
Conversation
When config_overrides sets model_catalog_json, Codex reads that file instead of its release catalog, but alignment still chose the --model slug from the release (or bundled) catalog. The slug could then be one the custom catalog lacks, or miss an entry only the custom catalog has. Alignment now reads the custom catalog from the sandbox and chooses from it. When it cannot be read (a relative path, a missing file, or not a JSON object), alignment keeps the release catalog and a warning names the path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
36953dc to
3b439f0
Compare
Run codex_cli with a custom model_catalog_json whose only entry carries marked instructions, and check the first request Codex sends: a model only the custom catalog has gets that entry, and a model only the release catalog has is aliased to it rather than to Codex's generic fallback. The filter answers every request, so no API key is needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dragonstyle
left a comment
There was a problem hiding this comment.
🤖 Automated review (CASE) at 40177e08ca
Reading the custom catalog and aligning against it works for the documented case, and the tests cover it. Two input-handling gaps let alignment drift from the catalog Codex actually loads, which is the mismatch this PR fixes. Details inline.
Codex trims -c keys and accepts dotted subkeys; the code-mode check already matched keys that way but alignment looked the key up exactly, so a padded key loaded the custom catalog in Codex while alignment used the release one. Both now share one key matcher. A catalog object without a models list is now unreadable (warning, release fallback) instead of silently aligning to nothing; an empty models list still passes through, as Codex would see it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reader strips quotes but does not decode TOML escapes, so a quoted path with a backslash named a different file than the one Codex opens. Decoding would need a TOML parser on Python 3.10 (a new runtime dependency) for a shape nothing uses, so such a path is not read: alignment warns and uses the release catalog, as it does for other paths it cannot read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex parses the value as TOML, so a quoted string keeps any other quote character inside it, and only a value that is not TOML has every quote trimmed from its ends. Stripping quotes from both ends in all cases made alignment read a different file from Codex's for a path ending in a quote. The docs now say an escaped path is not read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pair Three review rounds found TOML forms the quote handling misread: escapes, then inner quotes, now multi-line strings and text after the closing quote, which made alignment read a different file from Codex's or none. Rather than grow a partial TOML decoder, a value that starts with a quote is read only when it is one pair of matching quotes with no copy of that quote and no escape inside; every other quoted value is refused with the warning. Values that do not start with a quote keep Codex's quote trimming. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… slug A model below gpt-5.4 whose catalog entry binds tool_search is sent to Codex's generic prompt through the slug inspect-generic. A caller's catalog can hold an entry that slug matches by prefix (inspect-generic, inspect), which would bind that entry's tools again, so resolution now raises before launch unless model_config is set. A quoted model_catalog_json value with a control character TOML rejects (other than tab) is also left unread: Codex falls back to trimming its quotes, so alignment would read a different file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed in 56adf0c:
Generated with Claude ( |
Ransom chose to parse the value as Codex does (2026-10-15) once the Python 3.11 floor landed (#194), so tomllib costs no dependency. Codex parses the trimmed value as a TOML value and, when that fails, trims quotes from the raw text; alignment now does the same, so escaped, literal and multi-line strings name the file Codex reads. This replaces the restricted-format reader and its refusal list. A value that parses to something other than a string names no file. Codex parses TOML 1.1, whose \e and \xHH escapes tomllib reads only from Python 3.15, so a double-quoted value with a backslash that tomllib rejects is not read rather than guessed at. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review 9 B1: the guard refused every double-quoted value with a
backslash that tomllib rejects, which skipped Codex's raw fallback for
plain invalid escapes ("/tmp/c\d.json" names /tmp/c\d.json in Codex).
Now only a \xHH or \e escape not preceded by an escaped backslash is
refused, since Codex's TOML 1.1 parser decodes those and tomllib does
so only from Python 3.15; every other parse failure takes the raw
fallback. Ransom chose this documented, warned fallback over decoding
the escapes (2026-10-20). The docs, docstring and warning now state the
exception and name the escape (review 9 O1).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…se failure Review 10 B1: the guard refused any rejected value that held a \xHH or \e escape, even when the value is invalid in TOML 1.1 too (another bad escape, no closing quote, text after the string). Codex takes its raw quote-trim fallback for those, so alignment read nothing and chose a release slug. Now the escapes are swapped for a TOML 1.0 escape only to check whether the rest parses; the value is refused only if it does, and is never decoded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Matching Codex's value parser (Rust trim, TOML 1.1 parse, raw quote-trim fallback) kept turning up a new edge case each review round, all for values nobody writes. Read the value only when it is an absolute path, bare or in one pair of quotes, with no quote, backslash or control character; Codex names the same file for every such value. Anything else warns and uses the release catalog without reading a file. This drops the tomllib parse, the raw fallback and the TOML 1.1 escape check, so Python 3.15 no longer behaves differently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dragonstyle
left a comment
There was a problem hiding this comment.
🤖 Automated review (CASE) at 4312f4bec1
The model_catalog_json reading now matches how Codex reads the path (plain paths, matched quotes, refused TOML 1.1 escapes and control characters), the generic-slug capture guard is right, and the callers are unaffected. One regression, inline.
The -c pair passes the value to Codex with f"{key}={value}", so a Path
launched fine before alignment read the file, but alignment called
.strip() on it and crashed the launch. Convert the value with str() the
same way, so alignment reads the file Codex reads.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed in 9f8629f:
Generated with Claude ( |
When
config_overridessetsmodel_catalog_json, Codex reads that file instead of its release catalog.codex_clistill chose the--modelslug from the release catalog (or the bundled snapshot), so the slug could be one the custom catalog lacks, or miss an entry only the custom catalog has. This was found in the review of #184 and left out of it as pre-existing (decision: Ransom, 2026-10-07).Changes:
model_configis not set, the launch reads themodel_catalog_jsonfile from the sandbox and alignment chooses from it. Without one it uses the release catalog, else the bundled snapshot, as before.-cpair winning. A dotted key such asmodel_catalog_json.xnames no file.str(), as the-cpair passed to Codex does, so aPathnames its file. Only a plain path is read: after trimming surrounding whitespace, a bare path or one in a matching pair of"or', with no backslash, quote character or control character in it, and no other whitespace at the ends of a bare path. Codex 0.160.1 (codex-rs/utils/cli/src/config_override.rs) parses the value as TOML, else uses the raw text with quotes trimmed; for every such value both readings name the same file. Any other value (TOML escapes, embedded or unmatched quotes, multi-line strings, a trailing comment, control characters) is not read: a warning names it and says only a plain absolute path, optionally quoted, is read, and alignment uses the release catalog. Matching Codex's parser for those values turned up a new edge case every review round, for paths nobody writes (decision: Ransom, 2026-10-21).modelslist), alignment uses the release catalog and a warning names the path. Codex itself fails to start on a missing or invalid absolute path, so this mainly covers a relative path, which Codex resolves itself.tool_searchis sent to Codex's generic prompt through the sluginspect-generic. If the custom catalog has an entry that slug matches (inspect-generic,inspect), the launch now fails with an error naming the entry, unlessmodel_configis set.model_catalog_jsonis set; it now shares its key matcher with alignment.model_configoption says where the slug comes from, thatmodel_catalog_jsonmust be a plain absolute path in the sandbox, and what happens otherwise.Tests in
tests/test_codex_code_mode.pycover alignment against a custom catalog (a slug only it has, and one only the release catalog has), the release fallback without one, no read whenmodel_configis set, padded keys, aPathvalue, and non-string TOML values. Bare, double-quoted, single-quoted and space-padded paths read the right file, with decoy catalogs at the other candidate names. Refused values (escapes, a backslash, embedded or mismatched quotes, triple-quoted strings, a trailing comment, control characters including U+001C, trailing non-ASCII whitespace) warn and read no file, with catalogs at every name they could mean. The fallback and warning when the file is unreadable are covered too, and an emptymodelslist passes through, since Codex rejects it itself. Slow Docker tests intests/test_codex_align.pycheck the first request Codex sends: a model only the custom catalog has gets that entry's instructions, and a model only the release catalog has is aliased to the custom entry, not to Codex's generic prompt. Another passes a bare path and checks that Codex and alignment both use the custom entry. Two more cover gpt-5.2: it gets the generic prompt withouttool_search, and the launch fails when aninspectentry would capture that slug.🤖 Generated with Claude Code
Agent review
Regression analysis (aggregator, pass 13)
tests/test_codex_code_mode.py:334; explicit plain-path restriction, decision: Ransom, 2026-10-21. This replaces the earlier TOML/version exception.tests/test_codex_code_mode.py:338; explicitly refused by Ransom's 2026-10-21 rule.tests/test_codex_code_mode.py:342; explicit refused family, decision: Ransom, 2026-10-21.tests/test_codex_code_mode.py:344; independent U+001C probe selects release slug, zero reads and one warning. Decision: Ransom, 2026-10-21.tests/test_codex_code_mode.py:341,349; refuses reinterpretation instead of guessing a filename, per the explicit 2026-10-21 restriction.tests/test_codex_code_mode.py:289; decision: Ransom, 2026-10-13.model_catalog.py:253; bounded, explicitly requested read is negligible.model_catalog.py:253; no provider/request behavior change.Risk areas
Not traced