diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index a60d6f85..af02c1b3 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -178,6 +178,29 @@ fn stable_codex_model_provider_id_from_config(config_text: &str) -> Option Result, AppError> { + if config_text.trim().is_empty() { + return Ok(None); + } + + let doc = config_text + .parse::() + .map_err(|e| AppError::Message(format!("Invalid Codex config.toml: {e}")))?; + let Some(provider_id) = active_codex_model_provider_id(&doc) else { + return Ok(None); + }; + + let has_provider_table = doc + .get("model_providers") + .and_then(|item| item.as_table()) + .and_then(|table| table.get(provider_id.as_str())) + .is_some(); + + Ok(has_provider_table.then_some(provider_id)) +} + fn normalize_codex_live_config_model_provider_with_anchors<'a>( config_text: &str, anchor_config_texts: impl IntoIterator, @@ -295,6 +318,77 @@ pub fn normalize_codex_settings_config_model_provider( Ok(()) } +fn restore_codex_backfill_model_provider_id( + config_text: &str, + template_config_text: &str, +) -> Result { + let Some(template_provider_id) = + codex_model_provider_id_with_table_from_config(template_config_text)? + else { + return Ok(config_text.to_string()); + }; + + if config_text.trim().is_empty() { + return Ok(config_text.to_string()); + } + + let mut doc = config_text + .parse::() + .map_err(|e| AppError::Message(format!("Invalid Codex config.toml: {e}")))?; + let Some(live_provider_id) = active_codex_model_provider_id(&doc) else { + return Ok(config_text.to_string()); + }; + + if live_provider_id == template_provider_id { + return Ok(config_text.to_string()); + } + + if let Some(model_providers) = doc + .get_mut("model_providers") + .and_then(|item| item.as_table_mut()) + { + let Some(provider_table) = model_providers.remove(live_provider_id.as_str()) else { + return Ok(config_text.to_string()); + }; + model_providers[template_provider_id.as_str()] = provider_table; + } else { + return Ok(config_text.to_string()); + } + + rewrite_codex_profile_model_provider_refs(&mut doc, &live_provider_id, &template_provider_id); + doc["model_provider"] = toml_edit::value(template_provider_id.as_str()); + + Ok(doc.to_string()) +} + +/// Convert a Codex live config that was normalized for history stability back +/// to the provider-specific id used by the stored provider template. +pub fn restore_codex_settings_config_model_provider_for_backfill( + settings: &mut Value, + template_settings: &Value, +) -> Result<(), AppError> { + let Some(config_text) = settings + .get("config") + .and_then(|value| value.as_str()) + .map(str::to_string) + else { + return Ok(()); + }; + let Some(template_config_text) = template_settings + .get("config") + .and_then(|value| value.as_str()) + else { + return Ok(()); + }; + + let restored = restore_codex_backfill_model_provider_id(&config_text, template_config_text)?; + if let Some(obj) = settings.as_object_mut() { + obj.insert("config".to_string(), Value::String(restored)); + } + + Ok(()) +} + /// Atomically write Codex live config after normalizing provider-specific ids. /// /// Use this for provider-driven live writes. Keep `write_codex_live_atomic` available diff --git a/src-tauri/src/services/provider/live.rs b/src-tauri/src/services/provider/live.rs index e8b66049..488e88fb 100644 --- a/src-tauri/src/services/provider/live.rs +++ b/src-tauri/src/services/provider/live.rs @@ -530,29 +530,55 @@ pub(crate) fn strip_common_config_from_live_settings( app_type.as_str(), provider.id ); - return live_settings; + return restore_live_settings_for_provider_backfill(app_type, provider, live_settings); } }; - if !provider_uses_common_config(app_type, provider, snippet.as_deref()) { - return live_settings; - } - - let Some(snippet_text) = snippet.as_deref() else { - return live_settings; + let backfill_settings = if provider_uses_common_config(app_type, provider, snippet.as_deref()) { + match snippet.as_deref() { + Some(snippet_text) => { + match remove_common_config_from_settings(app_type, &live_settings, snippet_text) { + Ok(settings) => settings, + Err(err) => { + log::warn!( + "Failed to strip common config for {} provider '{}': {err}", + app_type.as_str(), + provider.id + ); + live_settings + } + } + } + None => live_settings, + } + } else { + live_settings }; - match remove_common_config_from_settings(app_type, &live_settings, snippet_text) { - Ok(settings) => settings, - Err(err) => { - log::warn!( - "Failed to strip common config for {} provider '{}': {err}", - app_type.as_str(), - provider.id - ); - live_settings - } + restore_live_settings_for_provider_backfill(app_type, provider, backfill_settings) +} + +fn restore_live_settings_for_provider_backfill( + app_type: &AppType, + provider: &Provider, + live_settings: Value, +) -> Value { + if !matches!(app_type, AppType::Codex) { + return live_settings; } + + let mut settings = live_settings; + if let Err(err) = crate::codex_config::restore_codex_settings_config_model_provider_for_backfill( + &mut settings, + &provider.settings_config, + ) { + log::warn!( + "Failed to restore Codex provider id while backfilling '{}': {err}", + provider.id + ); + } + + settings } pub(crate) fn normalize_provider_common_config_for_storage( diff --git a/src-tauri/tests/provider_service.rs b/src-tauri/tests/provider_service.rs index 281d7d7d..bb963be6 100644 --- a/src-tauri/tests/provider_service.rs +++ b/src-tauri/tests/provider_service.rs @@ -347,6 +347,133 @@ requires_openai_auth = true ); } +#[test] +fn provider_service_switch_codex_backfill_keeps_provider_specific_model_provider_id() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let _home = ensure_test_home(); + + let legacy_auth = json!({ "OPENAI_API_KEY": "rightcode-key" }); + let provider_a_config = r#"model_provider = "rightcode" +model = "gpt-5.4" + +[model_providers.rightcode] +name = "RightCode" +base_url = "https://rightcode.example/v1" +wire_api = "responses" +requires_openai_auth = true +"#; + write_codex_live_atomic(&legacy_auth, Some(provider_a_config)) + .expect("seed existing codex live config"); + + let mut initial_config = MultiAppConfig::default(); + { + let manager = initial_config + .get_manager_mut(&AppType::Codex) + .expect("codex manager"); + manager.current = "provider-a".to_string(); + manager.providers.insert( + "provider-a".to_string(), + Provider::with_id( + "provider-a".to_string(), + "RightCode".to_string(), + json!({ + "auth": {"OPENAI_API_KEY": "rightcode-key"}, + "config": provider_a_config + }), + None, + ), + ); + manager.providers.insert( + "provider-b".to_string(), + Provider::with_id( + "provider-b".to_string(), + "AiHubMix".to_string(), + json!({ + "auth": {"OPENAI_API_KEY": "aihubmix-key"}, + "config": r#"model_provider = "aihubmix" +model = "gpt-5.4" +profile = "work" + +[model_providers.aihubmix] +name = "AiHubMix" +base_url = "https://aihubmix.example/v1" +wire_api = "responses" +requires_openai_auth = true + +[profiles.work] +model_provider = "aihubmix" +model = "gpt-5.4" +"# + }), + None, + ), + ); + manager.providers.insert( + "provider-c".to_string(), + Provider::with_id( + "provider-c".to_string(), + "Vendor C".to_string(), + json!({ + "auth": {"OPENAI_API_KEY": "vendor-c-key"}, + "config": r#"model_provider = "vendor_c" +model = "gpt-5.4" + +[model_providers.vendor_c] +name = "Vendor C" +base_url = "https://vendor-c.example/v1" +wire_api = "responses" +requires_openai_auth = true +"# + }), + None, + ), + ); + } + + let state = create_test_state_with_config(&initial_config).expect("create test state"); + + ProviderService::switch(&state, AppType::Codex, "provider-b") + .expect("switch to provider b should succeed"); + ProviderService::switch(&state, AppType::Codex, "provider-c") + .expect("switch to provider c should succeed"); + + let providers = state + .db + .get_all_providers(AppType::Codex.as_str()) + .expect("read providers after switches"); + let provider_b_config = providers + .get("provider-b") + .expect("provider b exists") + .settings_config + .get("config") + .and_then(|v| v.as_str()) + .expect("provider b config"); + let parsed: toml::Value = toml::from_str(provider_b_config).expect("parse provider b config"); + + assert_eq!( + parsed.get("model_provider").and_then(|v| v.as_str()), + Some("aihubmix"), + "backfill should restore provider b's storage-specific model_provider id" + ); + assert!( + parsed + .get("model_providers") + .and_then(|v| v.get("aihubmix")) + .is_some(), + "provider b should keep its own model_providers table after backfill" + ); + assert_eq!( + parsed + .get("profiles") + .and_then(|v| v.get("work")) + .and_then(|v| v.get("model_provider")) + .and_then(|v| v.as_str()), + Some("aihubmix"), + "profile overrides should be restored to provider b's storage-specific id" + ); +} + #[test] fn sync_current_provider_for_app_keeps_live_takeover_and_updates_restore_backup() { let _guard = test_mutex().lock().expect("acquire test mutex");