fix(cli): Codex own-key routing, credential isolation and error reporting - #682
Merged
Harry19081 merged 9 commits intoAug 6, 2026
Merged
Conversation
Codex only speaks the Responses wire API — the installed codex-cli rejects `wire_api = "chat"` outright. The compatible-provider list nevertheless advertised every generic OpenAI-compatible provider, so selecting one produced a session that failed at request time instead of an upfront, actionable rejection in the picker. Narrow the list to providers whose `/responses` route is supported or has been verified end to end: `openai_api` and `zenmux_api`. Pre-commit hook ran. Total eslint: 0, total circular: 0
Own-key and hosted Codex runs both pointed `CODEX_HOME` at shared state, so two concurrent sessions raced on one `config.toml` and the last writer decided everyone's routing. Give hosted sessions their own profile root alongside the existing per-account one, and generate the hosted profile from a single writer instead of merging into the user's global config. Housekeeping, storage reporting and native-transcript indexing all learn about the new root so hosted rollouts are still swept, sized and imported. Also expose `write_cli_profile_file_atomic` so callers that own credential files can replace them crash-safely without inheriting the warn-on-chmod-failure behaviour of the sensitive-file helper. Pre-commit hook ran. Total eslint: 0, total circular: 0
Codex app-server task failures carried their message in the task payload only, so a failed turn was replayed as a plain completion: no error chunk, `task_completed` lifecycle, and nothing for the UI to show. Emit the error chunk, mark the turn failed, and keep the same shape in the native transcript importer. The reverse problem existed too. `Model metadata for ... Defaulting to fallback metadata` is a non-fatal notice — Codex falls back and keeps running — but only the structured parser dropped it. Hoist the rule into one shared predicate so the stderr fallback cannot re-promote it into the persisted failure message of an unrelated error. On the frontend, error recognition no longer requires the `system`/`failed`/`message` triple: normalized CLI chunks whose action, function or display variant is `error` now route to the error card, and the message is taken from `error` / `error_message` / `observation` / display text rather than `observation` alone. Pre-commit hook ran. Total eslint: 0, total circular: 0
`refresh_codex_oauth_key` / `refresh_claude_code_oauth_key` returned a `ModelKey` no matter what they were handed, so a cross-provider or API-key credential came back looking like a successful token rotation. Return an explicit `OAuthRefreshOutcome` (`Refreshed` / `AlreadyRotated` / `NotApplicable`) and make every caller — provider clients, quota validation, OAuth health bookkeeping, the CLI retry path — decide what to do with a non-OAuth key instead of guessing. OAuth health now errors on a key that is not a refreshable native OAuth account rather than recording failures against it. CLI OAuth retry eligibility keys off the selected credential rather than the agent alone, `auth.json` is built from that credential (API key vs token bundle) instead of scraped from the environment, and it is written atomically: a crash mid-write must not leave a truncated file that silently downgrades the next launch to unauthenticated. Direct `openai_api` keys now keep Codex's built-in `openai` provider — official endpoint, Responses, native OpenAI auth, WebSockets and Codex's own retry defaults — instead of being routed through the synthetic `orgii_compatible` table, which downgrades all four for no benefit. A custom `base_url` still gets the table. Any ORGII-written `config.toml` left by an earlier run is cleared first, so clearing a base URL override actually takes effect; profiles Codex wrote itself are left alone. Terminal transport errors are captured as they happen and take precedence over the stderr summary, so a failed session reports the CLI's own error rather than whichever line of launch noise matched a keyword. Pre-commit hook ran. Total eslint: 0, total circular: 0
`clippy::items_after_test_module` fires on the inline test module added in the previous commit: everything declared after a `#[cfg(test)] mod` is easy to miss. Move the module to the end of the file; no logic change. Pre-commit hook ran. Total eslint: 0, total circular: 0
`summarize_cli_stderr` filtered the fallback-metadata notice out of the keyword pass but not out of the last-line fallback, so a session whose stderr held only that notice still reported it as the failure reason — the exact behaviour the filter exists to prevent, surviving only because some other line usually matches first. The test asserted that outcome, which pinned the bug rather than the rule. Apply the same suppression in the fallback: a real line behind the notice is still reachable, and a session that logged nothing else now has no stderr-derived message at all, which beats blaming a notice Codex recovered from. Pre-commit hook ran. Total eslint: 0, total circular: 0
The atomic writer created its temp file with `File::create`, so with the common 0002 umask the payload sat at 0664 for the whole write and the destination stayed 0664 until the caller's `chmod` landed after the rename. For `auth.json` that payload is an access/refresh token pair, and the profile directories are 0775, so other local accounts could read it in that window — or indefinitely, from a staging file left behind by a crash, since cleanup only runs on the error path. This was narrowly a regression: the previous non-atomic write reused the existing 0600 file, so only first creation was exposed; routing every refresh through a fresh temp file reopened it on every write. Set the mode at creation for credential writes instead of correcting it afterwards. `set_sensitive_file_permissions` stays where it is — it still pins the destination and covers Windows ACLs — but it is no longer what stands between a token and a group-readable file. Pre-commit hook ran. Total eslint: 0, total circular: 0
A `Reconnecting...` / `willRetry` notice is dropped on sight so a recovered retry never renders as an error. But when the terminal event reports failure with no error body, that notice was the only description of what went wrong, and the turn ended as a bare "Turn failed". Keep it as a last-resort body: used only after the terminal error and the pending fatal error, cleared on every success, gated on a non-zero exit in `on_exit`, and never lent to an interrupted turn. Pre-commit hook ran. Total eslint: 0, total circular: 0
The stderr reader was a detached task and every consumer — the OAuth retry probe in both transports, and the finalizer's error summary — read the buffer straight after `child.wait()`. A child exiting only closes the write end of the pipe; it says nothing about whether the reader has picked up what is still in it. A CLI that dumps its diagnostics in one write loses the tail, which is where the error line is: a session that failed loudly on stderr reports no reason at all. Wrap the buffer and its JoinHandle in `CliStderrCollector` and drain before reading. The drain is idempotent, so both transports and the run loop can each ask for it, and bounded at 3s so a grandchild holding the pipe open cannot hang the turn. Pre-commit hook ran. Total eslint: 0, total circular: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #681
Third-party OpenAI-compatible providers on the Codex CLI failed in ways that
were hard to act on: the picker offered providers Codex cannot talk to, two
sessions overwrote each other's
config.toml, and when a turn did fail the UIusually showed nothing — or showed a line of launch noise instead of the error.
This branch fixes the routing, isolates the credentials, and makes a failed
turn actually report why it failed.
Routing and credentials
speaks the Responses wire API and the installed
codex-clirejectswire_api = "chat"outright, but the compatible-provider list advertisedevery generic OpenAI-compatible provider. Narrowed to
openai_apiandzenmux_api(supported or verified end to end).pointed
CODEX_HOMEat shared state, so concurrent sessions raced on oneconfig.tomland the last writer decided everyone's routing. Hosted sessionsget their own profile root, generated by a single writer instead of merged
into the user's global config. Housekeeping, storage reporting and
native-transcript indexing all learn about the new root.
openai_apikeys keep Codex's built-inopenaiprovider —official endpoint, Responses, native OpenAI auth, WebSockets and Codex's own
retry defaults — instead of being routed through the synthetic
orgii_compatibletable, which downgrades all four for no benefit. A custombase_urlstill gets the table, and any ORGII-writtenconfig.tomlfrom anearlier run is cleared first so clearing a base-URL override takes effect.
Profiles Codex wrote itself are left alone.
refresh_codex_oauth_key/refresh_claude_code_oauth_keyused to return aModelKeyno matter whatthey were handed, so a cross-provider or API-key credential came back looking
like a successful rotation. They now return
OAuthRefreshOutcome(Refreshed/AlreadyRotated/NotApplicable) andevery caller decides. CLI retry eligibility keys off the selected credential
rather than the agent, and
auth.jsonis built from that credential insteadof scraped from the environment.
The staging file used by the atomic replace was created with the default
umask and chmod'd afterwards, leaving a token group/world-readable for that
window — or indefinitely if a crash left the staging file behind.
Error reporting
message in the task payload only, so a failed turn replayed as a plain
completion: no error chunk,
task_completedlifecycle, nothing to show. Theerror chunk is emitted, the turn is marked failed, and the native transcript
importer keeps the same shape.
task and every consumer read the buffer straight after
child.wait(). Achild exiting only closes the write end of the pipe — it says nothing about
whether the reader picked up what is still in it, so a CLI that dumps its
diagnostics in one write loses the tail, which is where the error line is.
Drain is idempotent (both transports and the run loop can each ask) and
bounded at 3s; on timeout the reader is aborted rather than detached, so a
grandchild holding the pipe open cannot leak an fd + task per attempt.
Model metadata for ... Defaulting to fallback metadatais a notice Codex recovers from. It wasfiltered in the structured parser and in the stderr keyword pass, but not in
the stderr last-line fallback — so a session whose stderr held only that
notice still reported it as the cause. One shared predicate now covers all
three paths.
Reconnecting.../willRetryis dropped on sight so a recovered retry never renders as anerror — but when the terminal event reported failure with no error body, that
notice was the only description of what went wrong and the turn ended as a
bare "Turn failed". It is now used only after the terminal and pending fatal
errors, cleared on every success, gated on a non-zero exit, and never lent to
an interrupted turn.
chunks whose action, function or display variant is
errorroute to theerror card, and the message is taken from
error/error_message/observation/ display text rather thanobservationalone.Behaviour change worth calling out
Codex's compatible-provider list goes from twelve entries to two, so ten
providers move from selectable but failing at request time to not selectable
on Codex:
atlascloud_api,openrouter_api,azure_openai_api,deepseek_api,groq_api,xai_api,dashscope_api,moonshot_api,longcat_api,vllm_api. They remain available on the other agents. Anupfront, actionable rejection in the picker beats a session that dies
mid-request, but this is a visible capability reduction for anyone who had one
of them selected.
The list is verified, not possible. Azure OpenAI and OpenRouter plausibly do
speak Responses — they are out because no one has run a Codex session against
them end to end, not because they are known incompatible. Adding one back is a
one-line registry change plus a passing session; that is the path I'd prefer
over re-widening the list, and it's why the narrowing is deliberately
aggressive rather than best-guess.
Testing
cargo test -p org2 --libcargo test -p agent_cli -p key_vault -p orgtrack_core -p app_paths -p agent_corecargo clippy --all-targets -- -D warnings(changed crates)pnpm typecheckpnpm lintpnpm run testNew coverage includes the stderr drain (a 200-line single-write burst that
fails without the drain) and a grandchild-holds-the-pipe test asserting the
reader is aborted, not detached, after the deadline.
Pre-existing failures, not from this PR
orgtrack::history_commands::tests::reduced_prewarm_never_displaces_full_projection_and_is_invisible_to_full_readers— a genuine bug in
ImportedTurnProjectionCache::get, which drops asignature-mismatched entry instead of putting it back. This branch does not
touch that file.
dateLocalDisplay.test.ts > preserves browser-locale month labels when locale is explicitly undefined— fails only on day-first locales becauseformatLocalMonthDayhardcodes month-first order; green on CI'smacos-latestdefault locale. This branch does not touch that file either.
Both are filed / to be filed separately.