Skip to content

Commit cab43d1

Browse files
BunsDevclaude
andcommitted
feat(api): harden Codex provider and unbreak Codex /fast
Two correctness fixes for the OAuth-only Codex provider. 1. Replace 5x `self.tokens.lock().unwrap()` in providers/codex.rs with parking_lot::Mutex, which has no poison semantics. The std Mutex variants would panic-cascade if any holder panicked mid-refresh (e.g. on a malformed token response), permanently bricking the provider for the rest of the session. parking_lot guarantees the lock keeps working through panics. 2. Teach ModelRegistry that Codex is OAuth-only and absent from the bundled models.dev snapshot. `best_model_for_provider("codex")` and `best_small_model_for_provider("codex")` previously returned None, which broke the `/model` default resolution and the `/fast` toggle for any Codex session. They now fall back to claurst_core::codex_oauth::DEFAULT_CODEX_MODEL and the first `*-mini` entry in CODEX_MODELS, respectively. Same path is taken for the `openai-codex` alias. cargo test claurst-api: 4/4 codex tests pass (1 new). Full workspace: 1520 passing, 0 failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 031db4c commit cab43d1

2 files changed

Lines changed: 69 additions & 8 deletions

File tree

src-rust/crates/api/src/model_registry.rs

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,11 @@ impl ModelRegistry {
739739
models = self.list_by_provider(provider_id);
740740
}
741741
if models.is_empty() {
742-
return None;
742+
// Codex models are not on models.dev (OAuth-only), so the bundled
743+
// snapshot has no entries for them. Fall back to the canonical
744+
// constant in claurst_core::codex_oauth so `/model` and the
745+
// effective-model resolver still pick a real id for Codex users.
746+
return codex_models_fallback(provider_id, /* small = */ false);
743747
}
744748

745749
let priority_patterns = flagship_patterns_for(provider_id);
@@ -785,7 +789,8 @@ impl ModelRegistry {
785789
models = self.list_by_provider(provider_id);
786790
}
787791
if models.is_empty() {
788-
return None;
792+
// See best_model_for_provider for the Codex rationale.
793+
return codex_models_fallback(provider_id, /* small = */ true);
789794
}
790795

791796
let small_priority = small_patterns_for(provider_id);
@@ -1039,6 +1044,28 @@ fn flagship_patterns_for(provider_id: &str) -> &'static [&'static str] {
10391044
}
10401045

10411046
/// Substring patterns marking a model as the lightweight/cheap default.
1047+
/// Codex is OAuth-only and absent from the models.dev bundled snapshot, so
1048+
/// `list_by_provider("codex")` always returns empty. This fallback returns
1049+
/// the canonical default / small ids from
1050+
/// `claurst_core::codex_oauth::CODEX_MODELS` so `/model` and `/fast` keep
1051+
/// working for Codex users.
1052+
fn codex_models_fallback(provider_id: &str, small: bool) -> Option<String> {
1053+
if !matches!(provider_id, "codex" | "openai-codex") {
1054+
return None;
1055+
}
1056+
if small {
1057+
// Prefer the documented "mini" entry; fall through to the default
1058+
// model id if the constant is ever pruned.
1059+
claurst_core::codex_oauth::CODEX_MODELS
1060+
.iter()
1061+
.find(|(id, _)| id.contains("mini"))
1062+
.map(|(id, _)| (*id).to_string())
1063+
.or_else(|| Some(claurst_core::codex_oauth::DEFAULT_CODEX_MODEL.to_string()))
1064+
} else {
1065+
Some(claurst_core::codex_oauth::DEFAULT_CODEX_MODEL.to_string())
1066+
}
1067+
}
1068+
10421069
fn small_patterns_for(provider_id: &str) -> &'static [&'static str] {
10431070
match provider_id {
10441071
"anthropic" | "amazon-bedrock" | "github-copilot" | "azure" => {
@@ -1188,6 +1215,39 @@ mod tests {
11881215
assert!(anthropic.env.iter().any(|e| e == "ANTHROPIC_API_KEY"));
11891216
}
11901217

1218+
#[test]
1219+
fn codex_best_models_return_real_ids_without_snapshot() {
1220+
// Codex is OAuth-only and intentionally absent from the bundled
1221+
// models.dev snapshot. Before the fallback in codex_models_fallback,
1222+
// best_model_for_provider("codex") returned None, which broke
1223+
// `/model` defaults and the `/fast` toggle for Codex users.
1224+
let reg = ModelRegistry::new();
1225+
// Sanity: snapshot really has no codex entries.
1226+
assert!(reg.list_by_provider("codex").is_empty());
1227+
1228+
let best = reg
1229+
.best_model_for_provider("codex")
1230+
.expect("codex best model must fall back to CODEX_MODELS default");
1231+
assert!(
1232+
best.contains("codex"),
1233+
"best codex model should look like a codex id, got {best:?}"
1234+
);
1235+
let small = reg
1236+
.best_small_model_for_provider("codex")
1237+
.expect("codex small model must fall back to a mini/default id");
1238+
assert!(
1239+
small.contains("codex"),
1240+
"small codex model should look like a codex id, got {small:?}"
1241+
);
1242+
// The alias `openai-codex` must take the same path.
1243+
assert!(reg.best_model_for_provider("openai-codex").is_some());
1244+
assert!(reg.best_small_model_for_provider("openai-codex").is_some());
1245+
// Unrelated providers continue to return None when absent.
1246+
assert!(reg
1247+
.best_model_for_provider("definitely-not-a-provider")
1248+
.is_none());
1249+
}
1250+
11911251
#[test]
11921252
fn opencode_remapped_to_opencode_zen() {
11931253
// models.dev uses "opencode" as the top-level key; we must normalize it

src-rust/crates/api/src/providers/codex.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
// Model list: static — the Codex endpoint does not expose a /models route,
1313
// so we use the `CODEX_MODELS` constant from `claurst-core`.
1414

15+
use parking_lot::Mutex;
1516
use std::pin::Pin;
16-
use std::sync::{Arc, Mutex};
17+
use std::sync::Arc;
1718
use std::time::{SystemTime, UNIX_EPOCH};
1819

1920
use async_stream::stream;
@@ -93,7 +94,7 @@ impl CodexProvider {
9394
async fn access_token(&self) -> Result<String, ProviderError> {
9495
// Check expiry under the lock; clone what we need; release.
9596
let (token, needs_refresh, refresh_token) = {
96-
let guard = self.tokens.lock().unwrap();
97+
let guard = self.tokens.lock();
9798
let expired = Self::is_expired(&guard);
9899
(
99100
guard.access_token.clone(),
@@ -193,7 +194,7 @@ impl CodexProvider {
193194

194195
// Persist and cache the refreshed tokens.
195196
let mut updated = {
196-
let guard = self.tokens.lock().unwrap();
197+
let guard = self.tokens.lock();
197198
guard.clone()
198199
};
199200
updated.access_token = new_access.clone();
@@ -207,7 +208,7 @@ impl CodexProvider {
207208
}
208209

209210
{
210-
let mut guard = self.tokens.lock().unwrap();
211+
let mut guard = self.tokens.lock();
211212
*guard = updated;
212213
}
213214

@@ -237,7 +238,7 @@ impl CodexProvider {
237238
}
238239

239240
fn account_id(&self) -> Option<String> {
240-
self.tokens.lock().unwrap().account_id.clone()
241+
self.tokens.lock().account_id.clone()
241242
}
242243

243244
fn system_prompt_to_text(request: &ProviderRequest) -> String {
@@ -913,7 +914,7 @@ impl LlmProvider for CodexProvider {
913914

914915
async fn health_check(&self) -> Result<ProviderStatus, ProviderError> {
915916
// Validate that a non-expired token exists without making a network call.
916-
let guard = self.tokens.lock().unwrap();
917+
let guard = self.tokens.lock();
917918
if guard.access_token.is_empty() {
918919
return Ok(ProviderStatus::Unavailable {
919920
reason: "no Codex access token — run /connect to authenticate".to_string(),

0 commit comments

Comments
 (0)