mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-09-02 17:49:20 +00:00
feat(llm): Add OpenAI Codex (ChatGPT subscription) as LLM provider (#1461)
* feat(llm): add OpenAI Codex backend config and OAuth session manager Add OpenAiCodex as a new LLM backend variant with config for auth endpoint, API base URL, client ID, and session persistence path. The session manager implements OpenAI's device code auth flow (headless-friendly, no browser required on the server) with automatic token refresh, following the same persistence pattern as the existing NEAR AI session manager. Closes #742 Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(llm): add Responses API client and token-refreshing decorator Native Responses API client for chatgpt.com/backend-api/codex/responses, the endpoint that works with ChatGPT subscription tokens. Handles SSE streaming, text completions, and tool call round-trips. Token-refreshing decorator wraps the provider to pre-emptively refresh OAuth tokens before API calls and retry once on auth failures. Reports zero cost since billing is through subscription. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(llm): wire OpenAI Codex into provider factory, CLI, and setup wizard Connect the new provider to the LLM factory, add openai_codex to the CLI --backend flag, and add it as an option in the onboarding wizard. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(llm): address PR #744 review feedback (20 items) Review fixes for the OpenAI Codex provider PR: - Remove dead `generate_pkce()` code (device flow gets PKCE from server) - Fix `refresh_tokens()` to use `.form()` instead of `.json()` per OAuth spec - Inline codex dispatch into `build_provider_chain()` (single async function, no separate `assemble_provider_chain()` helper — matches main's pattern) - Remove Clone from `OpenAiCodexSession`, restrict fields to `pub(crate)` - Propagate HTTP client builder error instead of silent fallback - Redact device code response body from debug log - Change `set_model()` in TokenRefreshingProvider to delegate to inner - Replace hardcoded `/tmp/` test path with `tempfile::tempdir()` - Accept `request_timeout_secs` from config instead of hardcoded 300s - Parse `Retry-After` header on 429 responses (matches nearai_chat.rs pattern) - Reuse `normalize_schema_strict()` for Codex tool definitions - Add warning log for dropped image attachments - Add doc comments on `list_models()` and `include` field - Add `OPENAI_CODEX_API_URL` to `.env.example` - Fix codex error message in `create_llm_provider()` for clarity - Revert unrelated `.worktrees` addition to `.gitignore` - Update `src/llm/CLAUDE.md` with Codex provider docs [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review feedback and harden OpenAI Codex provider (takeover #744) Security: - Add SSRF validation (validate_base_url) on OPENAI_CODEX_AUTH_URL and OPENAI_CODEX_API_URL, matching the pattern used by all other base URL configs (regression test for #1103 included) Correctness: - Add missing cache_write_multiplier() and cache_read_discount() trait delegation in TokenRefreshingProvider - Cap device-code polling backoff at 60s to prevent unbounded interval growth on repeated 429 responses - Default expires_in to 3600s when server returns 0, preventing immediately-expired sessions - Fix pre-existing SseEvent::JobResult missing fallback_deliverable field in job_monitor.rs tests Cleanup: - Extract duplicated make_test_jwt() and test_codex_config() into shared codex_test_helpers module Co-Authored-By: Sanjeev-S <[email protected]> Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address PR review feedback on OpenAI Codex provider (#1461) - Login command now resolves OPENAI_CODEX_* env overrides even when LLM_BACKEND isn't set to openai_codex (Copilot review) - Setup wizard "Keep current provider?" for codex no longer re-triggers device code login — mirrors Bedrock's keep-and-return pattern (Copilot) - Revert provider init log from info back to debug (Copilot) - Add warning log when token expires_in=0, before defaulting to 3600s (Gemini review) Co-Authored-By: Sanjeev-S <[email protected]> Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Sanjeev Suresh <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Sanjeev Suresh
Claude Opus 4.6
parent
cba1bc3799
commit
3da9810e87
+198
-3
@@ -37,6 +37,7 @@ impl LlmConfig {
|
||||
},
|
||||
provider: None,
|
||||
bedrock: None,
|
||||
openai_codex: None,
|
||||
request_timeout_secs: 120,
|
||||
cheap_model: None,
|
||||
smart_routing_cascade: false,
|
||||
@@ -72,8 +73,12 @@ impl LlmConfig {
|
||||
backend_lower == "nearai" || backend_lower == "near_ai" || backend_lower == "near";
|
||||
let is_bedrock =
|
||||
backend_lower == "bedrock" || backend_lower == "aws_bedrock" || backend_lower == "aws";
|
||||
let is_openai_codex = backend_lower == "openai_codex"
|
||||
|| backend_lower == "openai-codex"
|
||||
|| backend_lower == "codex";
|
||||
|
||||
if !is_nearai && !is_bedrock && registry.find(&backend_lower).is_none() {
|
||||
if !is_nearai && !is_bedrock && !is_openai_codex && registry.find(&backend_lower).is_none()
|
||||
{
|
||||
tracing::warn!(
|
||||
"Unknown LLM backend '{}'. Will attempt as openai_compatible fallback.",
|
||||
backend
|
||||
@@ -126,8 +131,8 @@ impl LlmConfig {
|
||||
smart_routing_cascade: parse_optional_env("SMART_ROUTING_CASCADE", true)?,
|
||||
};
|
||||
|
||||
// Resolve registry provider config (for non-NearAI, non-Bedrock backends)
|
||||
let provider = if is_nearai || is_bedrock {
|
||||
// Resolve registry provider config (for non-NearAI, non-Bedrock, non-Codex backends)
|
||||
let provider = if is_nearai || is_bedrock || is_openai_codex {
|
||||
None
|
||||
} else {
|
||||
Some(Self::resolve_registry_provider(
|
||||
@@ -174,6 +179,38 @@ impl LlmConfig {
|
||||
None
|
||||
};
|
||||
|
||||
// Resolve OpenAI Codex config
|
||||
let openai_codex = if is_openai_codex {
|
||||
// Model: OPENAI_CODEX_MODEL > OPENAI_MODEL > settings.selected_model > default
|
||||
let model = optional_env("OPENAI_CODEX_MODEL")?
|
||||
.or(optional_env("OPENAI_MODEL")?)
|
||||
.or_else(|| settings.selected_model.clone())
|
||||
.unwrap_or_else(|| "gpt-5.3-codex".to_string());
|
||||
let auth_endpoint = optional_env("OPENAI_CODEX_AUTH_URL")?
|
||||
.unwrap_or_else(|| "https://auth.openai.com".to_string());
|
||||
validate_base_url(&auth_endpoint, "OPENAI_CODEX_AUTH_URL")?;
|
||||
let api_base_url = optional_env("OPENAI_CODEX_API_URL")?
|
||||
.unwrap_or_else(|| "https://chatgpt.com/backend-api/codex".to_string());
|
||||
validate_base_url(&api_base_url, "OPENAI_CODEX_API_URL")?;
|
||||
let client_id = optional_env("OPENAI_CODEX_CLIENT_ID")?
|
||||
.unwrap_or_else(|| "app_EMoamEEZ73f0CkXaXp7hrann".to_string());
|
||||
let session_path = optional_env("OPENAI_CODEX_SESSION_PATH")?
|
||||
.map(PathBuf::from)
|
||||
.unwrap_or_else(|| ironclaw_base_dir().join("openai_codex_session.json"));
|
||||
let token_refresh_margin_secs =
|
||||
parse_optional_env("OPENAI_CODEX_REFRESH_MARGIN_SECS", 300)?;
|
||||
Some(OpenAiCodexConfig {
|
||||
model,
|
||||
auth_endpoint,
|
||||
api_base_url,
|
||||
client_id,
|
||||
session_path,
|
||||
token_refresh_margin_secs,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let request_timeout_secs = parse_optional_env("LLM_REQUEST_TIMEOUT_SECS", 120)?;
|
||||
|
||||
// Generic cheap model (works with any backend).
|
||||
@@ -189,6 +226,8 @@ impl LlmConfig {
|
||||
"nearai".to_string()
|
||||
} else if is_bedrock {
|
||||
"bedrock".to_string()
|
||||
} else if is_openai_codex {
|
||||
"openai_codex".to_string()
|
||||
} else if let Some(ref p) = provider {
|
||||
p.provider_id.clone()
|
||||
} else {
|
||||
@@ -198,6 +237,7 @@ impl LlmConfig {
|
||||
nearai,
|
||||
provider,
|
||||
bedrock,
|
||||
openai_codex,
|
||||
request_timeout_secs,
|
||||
cheap_model,
|
||||
smart_routing_cascade,
|
||||
@@ -1069,4 +1109,159 @@ mod tests {
|
||||
std::env::remove_var("LLM_REQUEST_TIMEOUT_SECS");
|
||||
}
|
||||
}
|
||||
|
||||
// ── OpenAI Codex tests ──────────────────────────────────────────
|
||||
|
||||
/// Clear all openai-codex-related env vars.
|
||||
fn clear_openai_codex_env() {
|
||||
// SAFETY: Only called under ENV_MUTEX in tests.
|
||||
unsafe {
|
||||
std::env::remove_var("LLM_BACKEND");
|
||||
std::env::remove_var("OPENAI_CODEX_MODEL");
|
||||
std::env::remove_var("OPENAI_MODEL");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn openai_codex_resolves_config() {
|
||||
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
|
||||
clear_openai_codex_env();
|
||||
|
||||
let settings = Settings {
|
||||
llm_backend: Some("openai_codex".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed");
|
||||
assert_eq!(cfg.backend, "openai_codex");
|
||||
let codex = cfg.openai_codex.expect("codex config should be present");
|
||||
assert_eq!(codex.model, "gpt-5.3-codex"); // default
|
||||
assert!(
|
||||
cfg.provider.is_none(),
|
||||
"codex should not use registry provider"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn openai_codex_model_env_resolution() {
|
||||
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
|
||||
clear_openai_codex_env();
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::set_var("OPENAI_CODEX_MODEL", "o3-pro");
|
||||
}
|
||||
|
||||
let settings = Settings {
|
||||
llm_backend: Some("openai_codex".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed");
|
||||
let codex = cfg.openai_codex.expect("codex config should be present");
|
||||
assert_eq!(codex.model, "o3-pro");
|
||||
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::remove_var("OPENAI_CODEX_MODEL");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn openai_codex_falls_back_to_openai_model() {
|
||||
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
|
||||
clear_openai_codex_env();
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::set_var("OPENAI_MODEL", "gpt-4o");
|
||||
}
|
||||
|
||||
let settings = Settings {
|
||||
llm_backend: Some("openai_codex".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed");
|
||||
let codex = cfg.openai_codex.expect("codex config should be present");
|
||||
assert_eq!(codex.model, "gpt-4o");
|
||||
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::remove_var("OPENAI_MODEL");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn openai_codex_falls_back_to_selected_model() {
|
||||
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
|
||||
clear_openai_codex_env();
|
||||
|
||||
let settings = Settings {
|
||||
llm_backend: Some("openai_codex".to_string()),
|
||||
selected_model: Some("gpt-4o-mini".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed");
|
||||
let codex = cfg.openai_codex.expect("codex config should be present");
|
||||
assert_eq!(codex.model, "gpt-4o-mini");
|
||||
}
|
||||
|
||||
/// Regression: SSRF validation on OPENAI_CODEX_API_URL (#1103).
|
||||
#[test]
|
||||
fn openai_codex_rejects_ssrf_api_url() {
|
||||
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
|
||||
clear_openai_codex_env();
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::set_var(
|
||||
"OPENAI_CODEX_API_URL",
|
||||
"http://169.254.169.254/latest/meta-data",
|
||||
);
|
||||
}
|
||||
|
||||
let settings = Settings {
|
||||
llm_backend: Some("openai_codex".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let err = LlmConfig::resolve(&settings).unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("OPENAI_CODEX_API_URL"),
|
||||
"error should reference the field name: {msg}"
|
||||
);
|
||||
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::remove_var("OPENAI_CODEX_API_URL");
|
||||
}
|
||||
}
|
||||
|
||||
/// Regression: SSRF validation on OPENAI_CODEX_AUTH_URL (#1103).
|
||||
#[test]
|
||||
fn openai_codex_rejects_ssrf_auth_url() {
|
||||
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
|
||||
clear_openai_codex_env();
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::set_var("OPENAI_CODEX_AUTH_URL", "http://10.0.0.1");
|
||||
}
|
||||
|
||||
let settings = Settings {
|
||||
llm_backend: Some("openai_codex".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let err = LlmConfig::resolve(&settings).unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("OPENAI_CODEX_AUTH_URL"),
|
||||
"error should reference the field name: {msg}"
|
||||
);
|
||||
|
||||
// SAFETY: Under ENV_MUTEX.
|
||||
unsafe {
|
||||
std::env::remove_var("OPENAI_CODEX_AUTH_URL");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -54,7 +54,7 @@ pub use self::transcription::TranscriptionConfig;
|
||||
pub use self::tunnel::TunnelConfig;
|
||||
pub use self::wasm::WasmConfig;
|
||||
pub use crate::llm::config::{
|
||||
BedrockConfig, CacheRetention, LlmConfig, NearAiConfig, OAUTH_PLACEHOLDER,
|
||||
BedrockConfig, CacheRetention, LlmConfig, NearAiConfig, OAUTH_PLACEHOLDER, OpenAiCodexConfig,
|
||||
RegistryProviderConfig,
|
||||
};
|
||||
pub use crate::llm::session::SessionConfig;
|
||||
@@ -377,7 +377,7 @@ pub(crate) fn resolve_owner_id(settings: &Settings) -> Result<String, ConfigErro
|
||||
/// are read by `optional_env()` before falling back to `std::env::var()`,
|
||||
/// so explicit env vars always win.
|
||||
///
|
||||
/// Also loads tokens from OS credential stores (macOS Keychain, Linux
|
||||
/// Also loads tokens from OS credential stores (macOS Keychain / Linux
|
||||
/// credentials files) which don't require the secrets DB.
|
||||
pub async fn inject_llm_keys_from_secrets(
|
||||
secrets: &dyn crate::secrets::SecretsStore,
|
||||
|
||||
Reference in New Issue
Block a user