diff --git a/src-tauri/src/cli/tui/app/tests.rs b/src-tauri/src/cli/tui/app/tests.rs index 1050e5b0..0d3a363e 100644 --- a/src-tauri/src/cli/tui/app/tests.rs +++ b/src-tauri/src/cli/tui/app/tests.rs @@ -6143,6 +6143,7 @@ mod tests { ); } + #[cfg(unix)] #[test] #[serial(home_settings)] fn openclaw_workspace_open_failure_is_localized() { @@ -6266,6 +6267,7 @@ mod tests { assert_eq!(editor.text(), "late content"); } + #[cfg(unix)] #[test] #[serial(home_settings)] fn openclaw_daily_memory_save_failure_is_localized() { diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index b3e349e6..463f0daa 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1196,9 +1196,9 @@ pub fn extract_codex_experimental_bearer_token(config_text: &str) -> Option doc .get("model_providers") - .and_then(|item| item.as_table()) + .and_then(|item| item.as_table_like()) .and_then(|table| table.get(id)) - .and_then(|item| item.as_table()) + .and_then(|item| item.as_table_like()) .and_then(|table| table.get("experimental_bearer_token")) .and_then(|item| item.as_str()) .or_else(top_level_token), @@ -1239,13 +1239,21 @@ fn set_codex_experimental_bearer_token(config_text: &str, token: &str) -> Result if let Some(model_providers) = doc .get_mut("model_providers") - .and_then(|item| item.as_table_mut()) + .and_then(|item| item.as_table_like_mut()) { if let Some(provider_table) = model_providers .get_mut(provider_id.as_str()) - .and_then(|item| item.as_table_mut()) + .and_then(|item| item.as_table_like_mut()) { - provider_table["experimental_bearer_token"] = toml_edit::value(token); + // Provider-specific auth sources take precedence over a bearer + // token. Do not create conflicting credentials in Codex config. + if provider_table.get("env_key").is_some() + || provider_table.get("auth").is_some() + || provider_table.get("aws").is_some() + { + return Ok(doc.to_string()); + } + provider_table.insert("experimental_bearer_token", toml_edit::value(token)); return Ok(doc.to_string()); } } @@ -1269,9 +1277,9 @@ pub fn remove_codex_experimental_bearer_token_if( if let Some(provider_id) = active_codex_model_provider_id(&doc) { if let Some(provider_table) = doc .get_mut("model_providers") - .and_then(|item| item.as_table_mut()) + .and_then(|item| item.as_table_like_mut()) .and_then(|table| table.get_mut(provider_id.as_str())) - .and_then(|item| item.as_table_mut()) + .and_then(|item| item.as_table_like_mut()) { let should_remove = provider_table .get("experimental_bearer_token") @@ -1545,32 +1553,24 @@ pub fn write_codex_live_for_provider( }; let config_text = unified_official_config.as_deref().or(config_text); - // A third-party provider must authenticate with its API key, never with a - // stray ChatGPT OAuth login that leaked into auth.json (e.g. from running - // `codex login` while it was active). Strip OAuth material for non-official - // providers, recovering the key from a config bearer token when auth.json - // only carried OAuth (issue #328). Official providers own auth.json. - let sanitized_auth = if category == Some("official") { - None - } else { - Some(sanitize_codex_third_party_auth( - Some(auth), - config_text, - None, - None, - )) - }; - let auth = sanitized_auth.as_ref().unwrap_or(auth); - - let should_write_auth = (category == Some("official") && codex_auth_has_login_material(auth)) - || (category != Some("official") - && !crate::settings::preserve_codex_official_auth_on_switch()); + if category == Some("official") { + return if codex_auth_has_login_material(auth) { + write_codex_live_atomic(auth, config_text) + } else { + write_codex_live_config_atomic(config_text) + }; + } - if should_write_auth { - write_codex_live_atomic(auth, config_text) - } else { - let live_config = prepare_codex_provider_live_config(auth, config_text.unwrap_or(""))?; + let preserve_official_login = crate::settings::preserve_codex_official_auth_on_switch(); + let live_config = prepare_codex_third_party_live_config( + auth, + config_text.unwrap_or(""), + preserve_official_login, + )?; + if preserve_official_login { write_codex_live_config_atomic(Some(&live_config)) + } else { + write_codex_live_atomic_optional_auth(None, Some(&live_config)) } } @@ -1593,6 +1593,98 @@ pub fn prepare_codex_provider_live_config( }) } +/// Prepare a third-party config for Codex 0.149+, where custom providers no +/// longer inherit their API key from `auth.json`. +pub fn prepare_codex_third_party_live_config( + auth: &Value, + config_text: &str, + preserve_official_login: bool, +) -> Result { + if extract_codex_api_key(Some(auth), Some(config_text)).is_none() { + let doc = config_text + .parse::() + .map_err(|e| AppError::Message(format!("Invalid Codex config.toml: {e}")))?; + let uses_official_auth_fallback = match active_codex_model_provider_id(&doc).as_deref() { + Some(id) if is_custom_codex_model_provider_id(id) => doc + .get("model_providers") + .and_then(|item| item.as_table_like()) + .and_then(|providers| providers.get(id)) + .and_then(|item| item.as_table_like()) + .is_some_and(|table| { + table + .get("requires_openai_auth") + .and_then(|item| item.as_bool()) + .unwrap_or(false) + && table.get("env_key").is_none() + && table.get("auth").is_none() + && table.get("aws").is_none() + }), + Some(id) if id.eq_ignore_ascii_case("openai") => doc + .get("openai_base_url") + .and_then(|item| item.as_str()) + .is_some_and(|url| !url.trim().is_empty()), + None => doc + .get("openai_base_url") + .and_then(|item| item.as_str()) + .is_some_and(|url| !url.trim().is_empty()), + _ => false, + }; + if uses_official_auth_fallback { + return Err(AppError::localized( + "provider.codex.config.official_auth_fallback", + "该 Codex 配置没有 API 密钥,却会回退使用 auth.json 中的登录凭据访问第三方地址", + "This Codex config has no API key and would fall back to the auth.json login for a third-party endpoint", + )); + } + } + + let mut live_config = prepare_codex_provider_live_config(auth, config_text)?; + if extract_codex_experimental_bearer_token(&live_config).is_none() { + return Ok(live_config); + } + let mut doc = live_config + .parse::() + .map_err(|e| AppError::Message(format!("Invalid Codex config.toml: {e}")))?; + let Some(provider_id) = active_codex_model_provider_id(&doc) else { + return Err(AppError::localized( + "provider.codex.config.no_custom_provider", + "Codex 第三方配置必须包含自定义 model_providers 条目以承载 API 密钥", + "A Codex third-party config must define a custom model_providers entry to carry the API key", + )); + }; + if !is_custom_codex_model_provider_id(&provider_id) { + return Err(AppError::localized( + "provider.codex.config.no_custom_provider", + "Codex 第三方供应商必须使用自定义 model_provider 条目以承载 API 密钥", + "A Codex third-party provider must use a custom model_provider entry to carry the API key", + )); + } + let Some(provider_table) = doc + .get_mut("model_providers") + .and_then(|item| item.as_table_like_mut()) + .and_then(|providers| providers.get_mut(provider_id.as_str())) + .and_then(|item| item.as_table_like_mut()) + else { + return Err(AppError::localized( + "provider.codex.config.no_custom_provider", + "Codex 第三方配置缺少活动 model_provider 的配置表,无法承载 API 密钥", + "The active Codex model_provider table is missing, so it cannot carry the API key", + )); + }; + let has_provider_token = provider_table + .get("experimental_bearer_token") + .and_then(|item| item.as_str()) + .is_some_and(|value| !value.trim().is_empty()); + if has_provider_token { + provider_table.insert( + "requires_openai_auth", + toml_edit::value(preserve_official_login), + ); + } + live_config = doc.to_string(); + Ok(live_config) +} + /// During DB backfill, lift a live `experimental_bearer_token` back into /// `auth.OPENAI_API_KEY` so the stored provider keeps its canonical shape /// and generated live tokens don't leak into stored provider TOML. diff --git a/src-tauri/src/services/provider/codex.rs b/src-tauri/src/services/provider/codex.rs index 13e9a0de..16fbd60f 100644 --- a/src-tauri/src/services/provider/codex.rs +++ b/src-tauri/src/services/provider/codex.rs @@ -529,6 +529,109 @@ impl ProviderService { Ok(()) } + /// Capture the active Codex provider's live config back to the database snapshot. + /// + /// This ensures the TUI displays current state for the active provider, not stale + /// data from before activation. Called after successfully writing live config. + pub(super) fn capture_codex_active_snapshot( + config: &mut MultiAppConfig, + provider_id: &str, + ) -> Result<(), AppError> { + let provider = config + .get_manager(&AppType::Codex) + .and_then(|manager| manager.providers.get(provider_id)) + .cloned(); + let Some(provider) = provider else { + log::debug!("Skip capture for nonexistent provider '{provider_id}'"); + return Ok(()); + }; + + let auth_path = get_codex_auth_path(); + let config_path = get_codex_config_path(); + + // Read live auth; if absent, keep the existing snapshot auth + let auth = if auth_path.exists() { + match read_json_file::(&auth_path) { + Ok(auth) => Some(auth), + Err(err) => { + log::warn!("Failed to read live auth.json for active snapshot: {err}"); + provider.settings_config.get("auth").cloned() + } + } + } else { + provider.settings_config.get("auth").cloned() + }; + + // Read live config.toml; if absent or fails, keep existing snapshot + let config_text = if config_path.exists() { + match std::fs::read_to_string(&config_path) { + Ok(text) => Some(text), + Err(err) => { + log::warn!("Failed to read live config.toml for active snapshot: {err}"); + None + } + } + } else { + None + }; + + let Some(text) = config_text else { + return Ok(()); + }; + + let is_official = Self::codex_live_write_category(&provider) == Some("official"); + let stored_auth = provider.settings_config.get("auth"); + let stored_config = provider + .settings_config + .get("config") + .and_then(Value::as_str); + + let capture_auth = if is_official { + auth.clone() + } else { + Some(crate::codex_config::sanitize_codex_third_party_auth( + auth.as_ref(), + Some(text.as_str()), + stored_auth, + stored_config, + )) + }; + + let mut raw_settings = serde_json::Map::new(); + if let Some(auth) = capture_auth { + raw_settings.insert("auth".to_string(), auth); + } + raw_settings.insert("config".to_string(), Value::String(text)); + let mut settings_for_storage = Value::Object(raw_settings); + crate::codex_config::strip_codex_mcp_servers_from_settings(&mut settings_for_storage)?; + if is_official { + crate::codex_config::strip_codex_unified_session_bucket_from_settings( + &mut settings_for_storage, + )?; + } + + let mut snapshot_provider = provider.clone(); + snapshot_provider.settings_config = settings_for_storage; + snapshot_provider = Self::migrate_provider_snapshot_for_storage( + &AppType::Codex, + &snapshot_provider, + config.common_config_snippets.codex.as_deref(), + )?; + + Self::preserve_codex_model_catalog_for_backfill( + &provider, + &mut snapshot_provider.settings_config, + ); + + if let Some(manager) = config.get_manager_mut(&AppType::Codex) { + if let Some(target) = manager.providers.get_mut(provider_id) { + *target = snapshot_provider; + } + } + + Ok(()) + } + /// Write Codex live configuration. /// /// Aligned with upstream: the stored `settings_config.config` is the full config.toml text. @@ -604,15 +707,6 @@ impl ProviderService { clean_config_text }; - // `force_sync` only bypasses the live-sync policy above. Authentication - // placement must remain identical to the upstream provider write: an - // official snapshot writes auth.json only when it contains login - // material, while a third-party provider writes unless preservation is - // enabled. - let should_write_auth = (is_official - && crate::codex_config::codex_auth_has_login_material(auth)) - || (!is_official && !crate::settings::preserve_codex_official_auth_on_switch()); - // A third-party provider must authenticate with its API key, never with a // stray ChatGPT OAuth login that leaked into auth.json (e.g. from running // `codex login` while it was active). Strip OAuth material for non-official @@ -629,31 +723,26 @@ impl ProviderService { ) }; - // config.toml is a clean OVERWRITE with the provider's effective config. - // When auth.json is preserved (third-party + preserve flag) the API key - // is injected into config.toml as an experimental_bearer_token instead. - let config_text = if should_write_auth { - live_config_text - } else { - crate::codex_config::prepare_codex_provider_live_config(&write_auth, &live_config_text)? - }; - - // auth.json follows Preserve/Write/Delete (no merge): a switch always - // prefers the incoming provider's auth, but never clobbers a preserved - // ChatGPT OAuth cache when auth is preserved. An empty/null incoming auth - // removes the stale live auth.json rather than writing an empty file. - let auth = if should_write_auth { - if write_auth.is_null() - || write_auth - .as_object() - .is_some_and(serde_json::Map::is_empty) - { - PreparedCodexAuthWrite::Delete + let preserve_official_login = crate::settings::preserve_codex_official_auth_on_switch(); + let (config_text, auth) = if is_official { + let auth_write = if crate::codex_config::codex_auth_has_login_material(auth) { + PreparedCodexAuthWrite::Write(auth.clone()) } else { - PreparedCodexAuthWrite::Write(write_auth) - } + PreparedCodexAuthWrite::Preserve + }; + (live_config_text, auth_write) } else { - PreparedCodexAuthWrite::Preserve + let config_text = crate::codex_config::prepare_codex_third_party_live_config( + &write_auth, + &live_config_text, + preserve_official_login, + )?; + let auth_write = if preserve_official_login { + PreparedCodexAuthWrite::Preserve + } else { + PreparedCodexAuthWrite::Delete + }; + (config_text, auth_write) }; Ok(PreparedLiveWrite::Codex { diff --git a/src-tauri/src/services/provider/codex_openai_auth_tests.rs b/src-tauri/src/services/provider/codex_openai_auth_tests.rs index 46f605e6..53c17283 100644 --- a/src-tauri/src/services/provider/codex_openai_auth_tests.rs +++ b/src-tauri/src/services/provider/codex_openai_auth_tests.rs @@ -30,18 +30,17 @@ fn switch_codex_provider_writes_stored_config_directly() { let manager = config .get_manager_mut(&AppType::Codex) .expect("codex manager"); - manager.providers.insert( + let mut official = Provider::with_id( "p1".to_string(), - Provider::with_id( - "p1".to_string(), - "OpenAI".to_string(), - json!({ - "auth": { "OPENAI_API_KEY": "sk-test" }, - "config": "model_provider = \"openai\"\nmodel = \"gpt-4o\"\n\n[model_providers.openai]\nbase_url = \"https://api.openai.com/v1\"\nwire_api = \"chat\"\nrequires_openai_auth = true\n" - }), - None, - ), + "OpenAI".to_string(), + json!({ + "auth": { "OPENAI_API_KEY": "sk-test" }, + "config": "model_provider = \"openai\"\nmodel = \"gpt-4o\"\n\n[model_providers.openai]\nbase_url = \"https://api.openai.com/v1\"\nwire_api = \"chat\"\nrequires_openai_auth = true\n" + }), + None, ); + official.category = Some("official".to_string()); + manager.providers.insert("p1".to_string(), official); } let state = state_from_config(config); @@ -209,7 +208,7 @@ fn switch_codex_overwrites_config_toml_respecting_auth_mode() { #[test] #[serial] -fn force_sync_codex_third_party_refreshes_auth_when_preserve_is_disabled() { +fn force_sync_codex_third_party_uses_provider_bearer_when_preserve_is_disabled() { let temp_home = TempDir::new().expect("create temp home"); let _env = TestEnvGuard::isolated(temp_home.path()); std::fs::create_dir_all(crate::codex_config::get_codex_config_dir()) @@ -227,19 +226,16 @@ fn force_sync_codex_third_party_refreshes_auth_when_preserve_is_disabled() { ProviderService::write_codex_live_force(&provider, None, false) .expect("force sync should succeed"); - let auth: Value = - crate::config::read_json_file(&get_codex_auth_path()).expect("read auth.json"); - assert_eq!( - auth.get("OPENAI_API_KEY").and_then(Value::as_str), - Some("sk-current-provider"), - "force sync must replace a stale third-party API key" + assert!( + !get_codex_auth_path().exists(), + "third-party sync must clear auth.json when official login preservation is disabled" ); let config_text = std::fs::read_to_string(get_codex_config_path()).expect("read config.toml"); assert_eq!( crate::codex_config::extract_codex_experimental_bearer_token(&config_text), - None, - "preserve disabled must keep the provider API key in auth.json" + Some("sk-current-provider".to_string()), + "the provider API key must be written to config.toml for Codex 0.149+" ); } @@ -276,6 +272,10 @@ fn force_sync_codex_third_party_preserves_oauth_when_preserve_is_enabled() { ); let config_text = std::fs::read_to_string(get_codex_config_path()).expect("read config.toml"); + assert!( + config_text.contains("requires_openai_auth = true"), + "preserved official login should remain available to Codex" + ); assert_eq!( crate::codex_config::extract_codex_experimental_bearer_token(&config_text).as_deref(), Some("sk-current-provider"), @@ -371,22 +371,24 @@ fn switch_codex_third_party_discards_stray_chatgpt_oauth_after_login() { ProviderService::switch(&state, AppType::Codex, "thirdparty") .expect("switch back to thirdparty"); - let auth_final: Value = - crate::config::read_json_file(&get_codex_auth_path()).expect("auth.json final"); let cfg_final = std::fs::read_to_string(get_codex_config_path()).expect("config.toml final"); assert!( - auth_final.pointer("/tokens/access_token").is_none(), - "live auth.json must not retain ChatGPT OAuth tokens after switching to third-party: {auth_final}" + !get_codex_auth_path().exists(), + "switching to a third-party provider must clear auth.json when preservation is disabled" + ); + assert!( + cfg_final.contains("base_url = \"http://localhost:8317/v1\""), + "config.toml should point at the third-party endpoint: {cfg_final}" ); assert_eq!( - auth_final.get("OPENAI_API_KEY").and_then(Value::as_str), + crate::codex_config::extract_codex_experimental_bearer_token(&cfg_final).as_deref(), Some("sk-thirdparty"), - "live auth.json must carry the third-party API key: {auth_final}" + "the active provider table must carry its API key" ); assert!( - cfg_final.contains("base_url = \"http://localhost:8317/v1\""), - "config.toml should point at the third-party endpoint: {cfg_final}" + cfg_final.contains("requires_openai_auth = false"), + "without preserved official login the custom provider should not require Codex login" ); } diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index 67a25f74..e49768a6 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -3123,6 +3123,17 @@ impl ProviderService { Ok(((), Some(action))) })?; + // Capture the newly-activated provider's snapshot back to the database + // so the TUI displays current state, not stale data from before activation. + if app_type == AppType::Codex { + let mut guard = state.config.write().map_err(AppError::from)?; + if let Err(err) = Self::capture_codex_active_snapshot(&mut guard, provider_id) { + log::warn!("Failed to capture active Codex provider snapshot: {err}"); + } + drop(guard); + let _ = state.save(); + } + if !app_type.is_additive_mode() { crate::settings::set_current_provider(&app_type, Some(provider_id))?; } diff --git a/src-tauri/tests/provider_service.rs b/src-tauri/tests/provider_service.rs index dee26abe..a2749190 100644 --- a/src-tauri/tests/provider_service.rs +++ b/src-tauri/tests/provider_service.rs @@ -4183,6 +4183,127 @@ fn provider_service_switch_codex_preserves_missing_wire_api_for_openai_official( } #[test] +#[test] +#[serial] +fn switch_codex_updates_both_active_and_inactive_snapshots() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + + std::fs::create_dir_all(home.join(".codex")).expect("create codex dir (initialized)"); + + let mut config = MultiAppConfig::default(); + { + let manager = config + .get_manager_mut(&AppType::Codex) + .expect("codex manager"); + + manager.providers.insert( + "provider-a".to_string(), + codex_provider( + "provider-a", + "Provider A", + "sk-key-a", + "vendor_a", + "https://a.example/v1", + ), + ); + manager.providers.insert( + "provider-b".to_string(), + codex_provider( + "provider-b", + "Provider B", + "sk-key-b", + "vendor_b", + "https://b.example/v1", + ), + ); + manager.current = "provider-a".to_string(); + } + + let state = state_from_config(config); + + // Switch A → B + ProviderService::switch(&state, AppType::Codex, "provider-b").expect("switch to provider-b"); + + { + let guard = state.config.read().expect("read config"); + let manager = guard.get_manager(&AppType::Codex).expect("codex manager"); + + let provider_a = manager + .providers + .get("provider-a") + .expect("provider-a exists"); + let provider_b = manager + .providers + .get("provider-b") + .expect("provider-b exists"); + + // Provider A (now inactive) should have experimental_bearer_token from backfill + let a_config = provider_a + .settings_config + .get("config") + .and_then(|v| v.as_str()) + .expect("provider-a has config"); + assert!( + a_config.contains("experimental_bearer_token"), + "Inactive provider A should have experimental_bearer_token in snapshot after backfill" + ); + + // Provider B (now active) should also have experimental_bearer_token from capture + let b_config = provider_b + .settings_config + .get("config") + .and_then(|v| v.as_str()) + .expect("provider-b has config"); + assert!( + b_config.contains("experimental_bearer_token"), + "Active provider B should have experimental_bearer_token in snapshot after capture" + ); + } + + // Switch B → A + ProviderService::switch(&state, AppType::Codex, "provider-a") + .expect("switch back to provider-a"); + + { + let guard = state.config.read().expect("read config"); + let manager = guard.get_manager(&AppType::Codex).expect("codex manager"); + + let provider_a = manager + .providers + .get("provider-a") + .expect("provider-a exists"); + let provider_b = manager + .providers + .get("provider-b") + .expect("provider-b exists"); + + // Both should still have experimental_bearer_token + let a_config = provider_a + .settings_config + .get("config") + .and_then(|v| v.as_str()) + .expect("provider-a has config"); + assert!( + a_config.contains("experimental_bearer_token"), + "Provider A should have experimental_bearer_token after second switch" + ); + + let b_config = provider_b + .settings_config + .get("config") + .and_then(|v| v.as_str()) + .expect("provider-b has config"); + assert!( + b_config.contains("experimental_bearer_token"), + "Provider B should have experimental_bearer_token after being backfilled again" + ); + } +} + +#[test] +#[serial] fn provider_service_switch_codex_preserves_missing_requires_openai_auth_for_openai_official() { let _guard = lock_test_mutex(); reset_test_fs();