feat(provider-openai-codex): Codex (ChatGPT subscription) Responses provider - #382
Conversation
…rovider
Add a new llm-router provider that generates against a ChatGPT/Codex
subscription via OpenAI's Responses API at the Codex backend, instead of a
pay-per-token API key.
- OAuth-only, dumb token consumer: credentials resolve from the auth-credentials
vault (auth::get_token); the vault owns refresh (oauth::openai-codex::refresh).
When no vault is present, falls back to a read-only local ~/.codex/auth.json
(local dev only).
- Responses request builder + SSE decoder mapped onto the shared
AssistantMessageEvent model: text, reasoning, and tool-call streaming, with
usage (incl. reasoning tokens). Unknown SSE event types are ignored
(forward-compat).
- Static, namespaced catalog (codex/*) so model ids never collide with
provider-openai's dynamic gpt-5.* catalog.
- Registers provider::openai-codex::{stream,refresh_models,on_router_ready};
identity binds via a persisted registration token.
Local/personal dev only; see README for the terms-of-service caveat and the
follow-up to run the auth-credentials + oauth-openai-codex workers for refresh.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Rust worker crate, Changesprovider-openai-codex worker implementation
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Provider as provider-openai-codex
participant State as iii-state
participant Router as llm-router
participant Vault as auth-credentials vault
participant OpenAI as OpenAI Responses API
CLI->>Provider: start binary or --manifest
Provider->>State: load_token()
Provider->>Router: register(declaration)
Router-->>Provider: registration_token
Provider->>Vault: get_token / refresh / import_codex_home_if_absent()
Provider->>Router: reconcile(static_models)
Provider->>OpenAI: POST /v1/responses (stream)
OpenAI-->>Provider: SSE frames
Provider-->>Router: forward AssistantMessageEvent stream
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 30 skipped (no docs/).
Four for four. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
provider-openai-codex/src/curated.rs (1)
12-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a named struct instead of an anonymous 6-tuple.
(&str, &str, &str, u64, u64, bool)requires readers to track field order/meaning across theCATALOGdefinition,upstream_model_id, andstatic_models. A small struct (ModelSpec { router_id, upstream_id, display, context_window, max_output_tokens, xhigh }) would self-document and avoid positional-destructuring mistakes if the schema grows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@provider-openai-codex/src/curated.rs` around lines 12 - 45, Replace the anonymous 6-tuple used by CATALOG in curated.rs with a named struct such as ModelSpec so the fields are self-documenting. Update the CATALOG entries to use that struct, and adjust the code in upstream_model_id and static_models to read named fields instead of positional tuple indexes/destructuring.provider-openai-codex/src/register.rs (1)
56-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo hand-rolled backoff loops with slightly different shapes.
persist_registration_token(bounded, 5 attempts, cap 2s) anddeclare_with_backoff(unbounded, cap 10s) duplicate the same exponential-backoff-with-cap pattern. Not a correctness problem, but a sharedretry_with_backoffhelper (parameterized by max attempts/cap) would remove the duplication and theunreachable!()escape hatch in the bounded version.Also applies to: 75-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@provider-openai-codex/src/register.rs` around lines 56 - 72, The retry logic in persist_registration_token duplicates the same exponential-backoff pattern already used by declare_with_backoff, so refactor both to share a retry_with_backoff helper that accepts the attempt limit and cap duration. Move the common sleep-and-double-delay behavior into that helper, and have persist_registration_token and declare_with_backoff call it with their respective bounds so the bounded version can return directly without the unreachable!() fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@provider-openai-codex/src/auth.rs`:
- Around line 151-159: Remove the redundant base64::Engine import in the tests
module, since the trait is already in scope through use super::* and the parent
module import. Update the tests block in auth.rs around the tests module and
jwt_with helper so only the necessary import remains, avoiding the
unused_imports clippy failure.
In `@provider-openai-codex/src/lib.rs`:
- Around line 1-2: Update the crate-level doc comment in the module containing
the provider entrypoint to match provider-openai-codex instead of
provider-openai. The header should describe this crate as the OpenAI
Codex/Responses API provider behind llm-router, and should align with
PROVIDER_ID and the README so the stale copy-pasted wording is removed.
In `@provider-openai-codex/src/main.rs`:
- Line 1: Update the crate-level doc comment in main.rs so it names
provider-openai-codex instead of provider-openai; this is a copy/paste leftover
from the sibling crate and should match the binary entry point represented by
the current crate. Keep the fix scoped to the top-level documentation comment at
the start of the file.
In `@provider-openai-codex/src/manifest.rs`:
- Around line 1-2: The doc comment in manifest.rs uses the wrong binary name and
should match provider-openai-codex instead of provider-openai. Update the module
header comment near build_manifest() so it accurately describes the manifest as
emitted by the provider-openai-codex binary, keeping the rest of the wording
consistent with the package’s actual entrypoint and the related lib.rs doc
header.
In `@provider-openai-codex/src/register.rs`:
- Around line 94-101: `declare_and_refresh` can run concurrently from the boot
path and `router::ready`, causing overlapping `declare_once`/token persistence
work to race on the registration token. Serialize the body of
`declare_and_refresh` with a shared mutual exclusion guard, such as a
`tokio::sync::Mutex<()>` or equivalent single-flight mechanism, so only one
invocation can execute `declare_with_backoff`,
`auth::import_codex_home_if_absent`, and `refresh_models` at a time. Use the
`declare_and_refresh` function as the main place to add the guard, and ensure
all callers that spawn it rely on the same shared lock.
In `@provider-openai-codex/src/request.rs`:
- Around line 46-55: The clippy failure is coming from the non-inlined
formatting in build_headers, specifically the authorization header construction.
Update build_headers in request.rs to use an inlined format argument style for
cfg.access_token, matching the other format! calls in this area, so
clippy::uninlined_format_args no longer triggers under the strict CI settings.
In `@provider-openai-codex/src/sse.rs`:
- Around line 303-315: The function-call delta handling in sse.rs is incorrectly
appending every `response.function_call_arguments.delta` to
`state.tool_calls.last_mut()`, which can mix arguments across concurrent tool
calls. Update the `n if n.contains("function_call_arguments.delta")` branch to
resolve the target call by the delta’s `item_id`/`output_index` or the `call_id`
stored when `output_item.added` creates the `ContentBlock::FunctionCall`, then
append only to that matched entry. If no matching call is found, return an error
or otherwise fail the event instead of silently attaching the delta to the last
call.
In `@provider-openai-codex/src/stream_fn.rs`:
- Around line 90-105: The credential lookup in stream_fn should not treat every
router/vault error as “no vault credential” because .ok().flatten() silently
falls through to auth::read_codex_home_credential(). Update the logic around
router_client::get_token and router_client::refresh so only an explicit
not-configured/no-vault condition, or an intentional local-dev opt-in, triggers
the Codex home fallback. Keep vault precedence intact and avoid returning local
credentials for transient router/vault failures.
- Around line 49-52: The helper send_event currently returns Result<(), ()>,
which triggers the denied clippy::result_unit_err lint and blocks CI. Update
send_event in stream_fn.rs to return a non-unit error shape, such as bool or a
small dedicated error type, and adjust the sink.send error mapping accordingly
while keeping the same behavior for serializing and forwarding
AssistantMessageEvent frames.
In `@provider-openai-codex/src/upstream.rs`:
- Around line 129-142: The `[DONE]` and EOF completion paths in
`provider-openai-codex/src/upstream.rs` should use the same finalization
sequence as the normal `response.completed` path. Update the `data == "[DONE]"`
branch and the EOF handling near `build_final` so they both close any open
block, emit `AssistantMessageEvent::Stop` before `AssistantMessageEvent::Done`,
and reuse the same finalization helper/logic instead of diverging behavior.
- Around line 123-126: The SSE parsing in upstream.rs is decoding each network
chunk too early in the block around the chunk buffer handling, which can corrupt
split UTF-8 and miss CRLF-delimited frames. Update the streaming logic that uses
buf, data_line, and the frame loop to buffer raw bytes first, detect complete
SSE frames using both "\n\n" and "\r\n\r\n" separators, and only then decode the
full frame before JSON parsing.
In `@provider-openai-codex/src/wire/mod.rs`:
- Line 1: The module-level doc comment in mod.rs is stale and still refers to
OpenAI Chat Completions, but this wire module implements Responses API shapes.
Update the comment to describe the Responses API contract consistently with the
submodules, and keep the terminology aligned with messages.rs and tools.rs so
readers can locate the correct wire format implementation.
---
Nitpick comments:
In `@provider-openai-codex/src/curated.rs`:
- Around line 12-45: Replace the anonymous 6-tuple used by CATALOG in curated.rs
with a named struct such as ModelSpec so the fields are self-documenting. Update
the CATALOG entries to use that struct, and adjust the code in upstream_model_id
and static_models to read named fields instead of positional tuple
indexes/destructuring.
In `@provider-openai-codex/src/register.rs`:
- Around line 56-72: The retry logic in persist_registration_token duplicates
the same exponential-backoff pattern already used by declare_with_backoff, so
refactor both to share a retry_with_backoff helper that accepts the attempt
limit and cap duration. Move the common sleep-and-double-delay behavior into
that helper, and have persist_registration_token and declare_with_backoff call
it with their respective bounds so the bounded version can return directly
without the unreachable!() fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1a45d68-c23e-4d66-b852-25e2fc4e8bdb
⛔ Files ignored due to path filters (1)
provider-openai-codex/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
provider-openai-codex/.gitignoreprovider-openai-codex/Cargo.tomlprovider-openai-codex/README.mdprovider-openai-codex/build.rsprovider-openai-codex/config.yamlprovider-openai-codex/iii-permissions.yamlprovider-openai-codex/iii.worker.yamlprovider-openai-codex/src/auth.rsprovider-openai-codex/src/config.rsprovider-openai-codex/src/curated.rsprovider-openai-codex/src/discovery.rsprovider-openai-codex/src/errors.rsprovider-openai-codex/src/lib.rsprovider-openai-codex/src/main.rsprovider-openai-codex/src/manifest.rsprovider-openai-codex/src/reasoning.rsprovider-openai-codex/src/register.rsprovider-openai-codex/src/request.rsprovider-openai-codex/src/router_client.rsprovider-openai-codex/src/sse.rsprovider-openai-codex/src/state.rsprovider-openai-codex/src/stream_fn.rsprovider-openai-codex/src/surface.rsprovider-openai-codex/src/upstream.rsprovider-openai-codex/src/wire/messages.rsprovider-openai-codex/src/wire/mod.rsprovider-openai-codex/src/wire/names.rsprovider-openai-codex/src/wire/tools.rsprovider-openai-codex/tests/golden/schemas/provider.openai-codex.on_router_ready.jsonprovider-openai-codex/tests/golden/schemas/provider.openai-codex.refresh_models.jsonprovider-openai-codex/tests/golden/schemas/provider.openai-codex.stream.jsonprovider-openai-codex/tests/schemas.rsprovider-openai-codex/tests/support/mod.rs
| //! provider-openai: OpenAI Chat Completions provider behind llm-router. | ||
| //! Spec: tech-specs/2026-06-agentic/llm-router.md § The provider protocol. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Stale crate doc header copy-pasted from provider-openai.
The module doc says provider-openai: OpenAI Chat Completions provider, but this crate is provider-openai-codex using the Responses API (per README and PROVIDER_ID = "openai-codex" below). This looks like leftover text copied from provider-openai/src/lib.rs.
✏️ Proposed fix
-//! provider-openai: OpenAI Chat Completions provider behind llm-router.
+//! provider-openai-codex: OpenAI Codex (ChatGPT subscription) Responses API provider behind llm-router.
//! Spec: tech-specs/2026-06-agentic/llm-router.md § The provider protocol.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! provider-openai: OpenAI Chat Completions provider behind llm-router. | |
| //! Spec: tech-specs/2026-06-agentic/llm-router.md § The provider protocol. | |
| //! provider-openai-codex: OpenAI Codex (ChatGPT subscription) Responses API provider behind llm-router. | |
| //! Spec: tech-specs/2026-06-agentic/llm-router.md § The provider protocol. |
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/lib.rs` around lines 1 - 2, Update the crate-level
doc comment in the module containing the provider entrypoint to match
provider-openai-codex instead of provider-openai. The header should describe
this crate as the OpenAI Codex/Responses API provider behind llm-router, and
should align with PROVIDER_ID and the README so the stale copy-pasted wording is
removed.
| @@ -0,0 +1,114 @@ | |||
| //! `provider-openai` binary entry. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Doc comment names the wrong crate.
//! \provider-openai` binary entry.— this isprovider-openai-codex's binary, not provider-openai's. Looks like a copy/paste leftover from the sibling provider-openai` crate; confusing for anyone reading this entry point.
✏️ Fix
-//! `provider-openai` binary entry.
+//! `provider-openai-codex` binary entry.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! `provider-openai` binary entry. | |
| //! `provider-openai-codex` binary entry. |
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/main.rs` at line 1, Update the crate-level doc
comment in main.rs so it names provider-openai-codex instead of provider-openai;
this is a copy/paste leftover from the sibling crate and should match the binary
entry point represented by the current crate. Keep the fix scoped to the
top-level documentation comment at the start of the file.
| //! Registry-publish manifest emitted by `provider-openai --manifest` | ||
| //! (binary-worker.md § manifest; same shape as provider-anthropic/src/manifest.rs). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Doc comment references the wrong binary name.
Says "emitted by provider-openai --manifest", but this is provider-openai-codex's manifest module (confirmed by main.rs calling provider_openai_codex::manifest::build_manifest()). Same copy-paste pattern as the lib.rs doc header.
✏️ Proposed fix
-//! Registry-publish manifest emitted by `provider-openai --manifest`
+//! Registry-publish manifest emitted by `provider-openai-codex --manifest`
//! (binary-worker.md § manifest; same shape as provider-anthropic/src/manifest.rs).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! Registry-publish manifest emitted by `provider-openai --manifest` | |
| //! (binary-worker.md § manifest; same shape as provider-anthropic/src/manifest.rs). | |
| //! Registry-publish manifest emitted by `provider-openai-codex --manifest` | |
| //! (binary-worker.md § manifest; same shape as provider-anthropic/src/manifest.rs). |
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/manifest.rs` around lines 1 - 2, The doc comment in
manifest.rs uses the wrong binary name and should match provider-openai-codex
instead of provider-openai. Update the module header comment near
build_manifest() so it accurately describes the manifest as emitted by the
provider-openai-codex binary, keeping the rest of the wording consistent with
the package’s actual entrypoint and the related lib.rs doc header.
| pub async fn declare_and_refresh(iii: IIIClient, http: reqwest::Client) { | ||
| declare_with_backoff(iii.clone()).await; | ||
| auth::import_codex_home_if_absent(&iii).await; | ||
| match refresh_models(&iii, &http).await { | ||
| Ok(count) => println!("[provider-openai-codex] catalog reconciled: {count} models"), | ||
| Err(e) => eprintln!("[provider-openai-codex] post-register reconcile failed ({e})"), | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Concurrent declare_and_refresh runs can race on the persisted registration token.
declare_and_refresh is spawned twice with no mutual exclusion: unconditionally at boot (line 145) and again every time router::ready fires (line 131). If a router::ready event arrives while the initial boot task's declare_with_backoff call is still in flight (a real possibility, since the event literally signals "the router is now up" — the exact moment the boot task's retry loop is waiting on), two declare_once calls can run concurrently. Each independently does read-then-write on the persisted token (state::load_token → router_client::register → persist_registration_token), so the two writes can interleave and the last one to finish wins, potentially persisting a token that doesn't match what the router considers current for this instance.
Consider serializing declare_and_refresh invocations, e.g. guard the body with a shared tokio::sync::Mutex<()> so overlapping triggers queue instead of racing:
🔒 Suggested guard against concurrent declare_and_refresh runs
+use std::sync::Arc;
+use tokio::sync::Mutex;
+
+/// Serializes declare_and_refresh so the boot-time call and any
+/// router::ready-triggered call never race on the persisted token.
+static DECLARE_LOCK: once_cell::sync::Lazy<Arc<Mutex<()>>> =
+ once_cell::sync::Lazy::new(|| Arc::new(Mutex::new(())));
+
pub async fn declare_and_refresh(iii: IIIClient, http: reqwest::Client) {
+ let _guard = DECLARE_LOCK.lock().await;
declare_with_backoff(iii.clone()).await;
auth::import_codex_home_if_absent(&iii).await;
match refresh_models(&iii, &http).await {(Any equivalent single-flight/serialization mechanism works — the important part is that only one declare_once/persist_registration_token sequence runs at a time.)
Also applies to: 126-136, 145-146
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/register.rs` around lines 94 - 101,
`declare_and_refresh` can run concurrently from the boot path and
`router::ready`, causing overlapping `declare_once`/token persistence work to
race on the registration token. Serialize the body of `declare_and_refresh` with
a shared mutual exclusion guard, such as a `tokio::sync::Mutex<()>` or
equivalent single-flight mechanism, so only one invocation can execute
`declare_with_backoff`, `auth::import_codex_home_if_absent`, and
`refresh_models` at a time. Use the `declare_and_refresh` function as the main
place to add the guard, and ensure all callers that spawn it rely on the same
shared lock.
| fn send_event(sink: &dyn FrameSink, ev: &AssistantMessageEvent) -> Result<(), ()> { | ||
| let frame = serde_json::to_string(ev).expect("serializable event"); | ||
| sink.send(&frame).map_err(|_| ()) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no unit-error Results remain in this provider.
rg -nP 'Result\s*<\s*\(\)\s*,\s*\(\)\s*>|map_err\(\|_\|\s*\(\)\)' provider-openai-codex/srcRepository: iii-hq/workers
Length of output: 358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' provider-openai-codex/src/stream_fn.rs
printf '\n--- call sites ---\n'
rg -n 'send_event\(' provider-openai-codex/srcRepository: iii-hq/workers
Length of output: 6904
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '190,230p' provider-openai-codex/src/stream_fn.rsRepository: iii-hq/workers
Length of output: 1315
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- root Cargo.toml ---'
sed -n '1,220p' Cargo.toml 2>/dev/null || true
printf '\n%s\n' '--- clippy config files ---'
fd -a 'clippy.toml|.clippy.toml'
printf '\n%s\n' '--- deny attributes mentioning clippy or warnings ---'
rg -n '#!\[deny\(|#!\[forbid\(|clippy::|warnings' -g '!target' .Repository: iii-hq/workers
Length of output: 14425
Avoid Result<(), ()> in send_event. clippy::result_unit_err is denied here, so this blocks CI. Return bool or a small error type instead.
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/stream_fn.rs` around lines 49 - 52, The helper
send_event currently returns Result<(), ()>, which triggers the denied
clippy::result_unit_err lint and blocks CI. Update send_event in stream_fn.rs to
return a non-unit error shape, such as bool or a small dedicated error type, and
adjust the sink.send error mapping accordingly while keeping the same behavior
for serializing and forwarding AssistantMessageEvent frames.
Source: Pipeline failures
| if let Some(cred) = router_client::get_token(iii, PROVIDER_ID) | ||
| .await | ||
| .ok() | ||
| .flatten() | ||
| { | ||
| if near_expiry(&cred) { | ||
| let _ = router_client::refresh(iii, PROVIDER_ID).await; // vault-owned | ||
| return router_client::get_token(iii, PROVIDER_ID) | ||
| .await | ||
| .ok() | ||
| .flatten() | ||
| .or(Some(cred)); | ||
| } | ||
| return Some(cred); | ||
| } | ||
| if let Some(cred) = auth::read_codex_home_credential() { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not fall back to local Codex credentials on every vault error.
.ok().flatten() treats transient vault/router failures the same as “no vault credential”. That can silently switch a configured environment to ~/.codex/auth.json, violating vault precedence and potentially using the wrong user account. Fall back only for an explicit not-configured/no-vault condition or behind a local-dev opt-in.
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/stream_fn.rs` around lines 90 - 105, The credential
lookup in stream_fn should not treat every router/vault error as “no vault
credential” because .ok().flatten() silently falls through to
auth::read_codex_home_credential(). Update the logic around
router_client::get_token and router_client::refresh so only an explicit
not-configured/no-vault condition, or an intentional local-dev opt-in, triggers
the Codex home fallback. Keep vault precedence intact and avoid returning local
credentials for transient router/vault failures.
| buf.push_str(&String::from_utf8_lossy(&chunk)); | ||
| while let Some(idx) = buf.find("\n\n") { | ||
| let block: String = buf.drain(..idx + 2).collect(); | ||
| let Some(data) = data_line(&block) else { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,240p' provider-openai-codex/src/upstream.rsRepository: iii-hq/workers
Length of output: 8690
🏁 Script executed:
python3 - <<'PY'
# Demonstrate the two claims against the current parsing strategy.
import codecs
# 1) Split UTF-8 decoded per chunk with replacement loses the original char.
s = "Hello 🌍"
b = s.encode("utf-8")
a, c = b[:8], b[8:] # split inside the 4-byte emoji
print("chunk1 lossys:", a.decode("utf-8", "replace"))
print("chunk2 lossys:", c.decode("utf-8", "replace"))
print("combined:", a.decode("utf-8", "replace") + c.decode("utf-8", "replace"))
# 2) Searching only for "\n\n" misses CRLF CRLF framing.
buf = "data: x\r\n\r\n"
print('has "\\n\\n":', "\n\n" in buf)
print('has "\\r\\n\\r\\n":', "\r\n\r\n" in buf)
PYRepository: iii-hq/workers
Length of output: 252
Parse SSE frames before decoding chunks. String::from_utf8_lossy on each network chunk can corrupt split multibyte characters before JSON parsing, and find("\n\n") misses valid \r\n\r\n SSE delimiters. Buffer bytes, split complete frames on either separator, then decode the full frame.
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/upstream.rs` around lines 123 - 126, The SSE
parsing in upstream.rs is decoding each network chunk too early in the block
around the chunk buffer handling, which can corrupt split UTF-8 and miss
CRLF-delimited frames. Update the streaming logic that uses buf, data_line, and
the frame loop to buffer raw bytes first, detect complete SSE frames using both
"\n\n" and "\r\n\r\n" separators, and only then decode the full frame before
JSON parsing.
| if data == "[DONE]" { | ||
| let _ = tx | ||
| .send(AssistantMessageEvent::Stop { | ||
| stop_reason: state.stop_reason(), | ||
| error_message: None, | ||
| error_kind: None, | ||
| }) | ||
| .await; | ||
| let _ = tx | ||
| .send(AssistantMessageEvent::Done { | ||
| message: build_final(&state, &args.model), | ||
| }) | ||
| .await; | ||
| return; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use the same finalization path for [DONE] and EOF.
The normal response.completed path closes the open block and emits Stop before Done. The [DONE] branch skips block-end events, and the EOF branch emits Done without Stop, so consumers see different contracts for successful completion paths.
Also applies to: 158-163
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/upstream.rs` around lines 129 - 142, The `[DONE]`
and EOF completion paths in `provider-openai-codex/src/upstream.rs` should use
the same finalization sequence as the normal `response.completed` path. Update
the `data == "[DONE]"` branch and the EOF handling near `build_final` so they
both close any open block, emit `AssistantMessageEvent::Stop` before
`AssistantMessageEvent::Done`, and reuse the same finalization helper/logic
instead of diverging behavior.
| @@ -0,0 +1,4 @@ | |||
| //! AgentMessage/AgentFunction → OpenAI Chat Completions wire shapes. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale doc comment: says "Chat Completions", but this module builds Responses API shapes.
Sibling files explicitly document Responses API semantics (messages.rs: "AgentMessage[] → OpenAI Responses API input items"; tools.rs: contrasts with Chat Completions' nested function shape). This module-level comment appears to be leftover from the Chat Completions provider and will mislead future readers about the wire contract these submodules implement.
📝 Proposed fix
-//! AgentMessage/AgentFunction → OpenAI Chat Completions wire shapes.
+//! AgentMessage/AgentFunction → OpenAI Responses API wire shapes.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! AgentMessage/AgentFunction → OpenAI Chat Completions wire shapes. | |
| //! AgentMessage/AgentFunction → OpenAI Responses API wire shapes. |
🧰 Tools
🪛 GitHub Actions: CI / provider-openai-codex rust lint + test
[error] Command failed: cargo clippy --all-targets --all-features -- -D warnings (exit code 101) due to 1 previous error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider-openai-codex/src/wire/mod.rs` at line 1, The module-level doc
comment in mod.rs is stale and still refers to OpenAI Chat Completions, but this
wire module implements Responses API shapes. Update the comment to describe the
Responses API contract consistently with the submodules, and keep the
terminology aligned with messages.rs and tools.rs so readers can locate the
correct wire format implementation.
…ests The test module's `use super::*` already brings `base64::Engine as _` into scope, so the explicit re-import tripped `-D warnings` (unused import) and failed the rust lint + test CI job.
Add it to Create Tag's worker dropdown and Release's push-tag patterns so the standard release flow works for it (it shipped in #382 but was never wired per the new-worker onboarding SOP).
2 participants
What
A new
llm-routerprovider,provider-openai-codex, that generates againsta ChatGPT/Codex subscription (billed to the plan) via OpenAI's Responses
API at the Codex backend — instead of a pay-per-token API key.
Why
Lets the stack (harness, console, any
router::chatconsumer) use a ChatGPTsubscription for generation. Uses the existing credential-vault pattern rather
than putting auth in the provider.
How it works
auth-credentialsvault (auth::get_token); the vault owns refresh(
oauth::openai-codex::refresh) — the provider never calls the OAuthendpoints. API keys are rejected (those stay on
provider-openai).${CODEX_HOME:-$HOME/.codex}/auth.jsonread-only (thecodexCLI owns thatfile's refresh). Gated so a real vault credential always wins.
AssistantMessageEventmodel: text, reasoning, and tool-call streaming, withusage (incl. reasoning tokens). Unknown SSE events are ignored
(forward-compat). Headers:
Authorization: Bearer,chatgpt-account-id,openai-beta: responses=experimental,originator: codex_cli_rs.codex/*model ids so they can't collide withprovider-openai's dynamicgpt-5.*catalog (which would make a modelunroutable via
AmbiguousModel).provider::openai-codex::{stream,refresh_models,on_router_ready};identity binds via a persisted registration token.
Scope / caveat
Local/personal dev only. It drives a personal ChatGPT subscription through
the undocumented
chatgpt.com/backend-api/codexbackend, which is plausiblyagainst OpenAI's terms and is unversioned. For team/CI/production use official
API-key billing (
provider-openai). Documented in the README.Verification
cargo buildclean;cargo testgreen (49 tests — config/auth/wire/sse/curated units,
upstreamTCP stubs, wire-schema goldens).openai-codex, publishes the 4codex/*models, routescodex/gpt-5.5→openai-codex, and returns a real streamed completion with usage(
input/output/reasoning). One live wire fix landed: the Codex backendrejects
max_output_tokens, so it is no longer sent.Follow-ups (not in this PR)
auth-credentials+oauth-openai-codexon iii-sdk 0.20 for the fullvault-owned refresh + console "Sign in with ChatGPT" (the dev fallback covers
local use meanwhile).
via the vault + a clean
auth_expiredon 401).integration test was provider-specific and was not carried over).
Test plan
cargo buildandcargo testpass inprovider-openai-codex/.llm-routerand listscodex/*models.~/.codex/auth.json(or the vault), acodex/*chat streamsa real completion; with neither, it returns a clear "not configured" frame.
codex/*ids do not collide withprovider-openai's catalog.Summary by CodeRabbit