From e22a8b836ea82bdddd871982b24c9ee3d7d31eb0 Mon Sep 17 00:00:00 2001 From: Liu Zhangjian Date: Thu, 4 Jun 2026 20:28:45 +0800 Subject: [PATCH 1/5] fix(codex): ensure [model_providers.] TOML section exists in live config.toml Codex CLI requires the `[model_providers.]` section to route requests to a custom (non-reserved) provider. Without it, Codex falls back to `model_provider: openai` and all API calls go directly to api.openai.com. The section can be lost when: - Codex Desktop App rewrites config.toml, stripping unknown tables - CC-Switch imports a pre-existing flat-format config - A backup/restore cycle drops the section Add a defensive normalization step (`ensure_codex_model_provider_section`) that creates an empty `[model_providers.]` table whenever `model_provider` is set to a custom provider ID but the corresponding table is missing. This runs in both `write_codex_live_for_provider` and `prepare_codex_provider_live_config`, covering all code paths that write config.toml to disk. Fixes #3449 Signed-off-by: Liu Zhangjian --- src-tauri/src/codex_config.rs | 194 +++++++++++++++++++++++++++++++++- 1 file changed, 189 insertions(+), 5 deletions(-) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index 5f1a7ddb6d..f65e4f689d 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1037,6 +1037,63 @@ pub fn read_codex_live_settings() -> Result { Ok(json!({ "auth": auth, "config": cfg_text })) } +/// Ensure the `[model_providers.]` TOML table exists when +/// `model_provider` is set to a non-reserved (custom) provider ID. +/// +/// Codex CLI requires this section to route requests to the custom provider; +/// without it, Codex falls back to `model_provider: openai` and all API calls go +/// directly to `api.openai.com`. The section can be lost when: +/// +/// - Codex Desktop App rewrites `config.toml`, stripping unknown tables +/// - CC-Switch imports a pre-existing flat-format config +/// - A backup/restore cycle drops the section +/// +/// This function is a defensive normalization: if the top-level key is present +/// but the corresponding table is missing, it creates an empty table so that +/// subsequent `update_codex_toml_field` calls have a valid target. +pub fn ensure_codex_model_provider_section(config_text: &str) -> String { + if config_text.trim().is_empty() { + return config_text.to_string(); + } + + let Ok(mut doc) = config_text.parse::() else { + return config_text.to_string(); + }; + + let Some(provider_id) = active_codex_model_provider_id(&doc) else { + return config_text.to_string(); + }; + + if !is_custom_codex_model_provider_id(&provider_id) { + return config_text.to_string(); + } + + // Already has the section — nothing to do. + if doc + .get("model_providers") + .and_then(|mp| mp.as_table()) + .is_some_and(|t| t.contains_key(&provider_id)) + { + return config_text.to_string(); + } + + // Create [model_providers.] table. + if doc.get("model_providers").is_none() { + doc["model_providers"] = toml_edit::table(); + } + + if let Some(mp) = doc.get_mut("model_providers").and_then(|item| item.as_table_mut()) + { + if !mp.contains_key(&provider_id) { + let mut table = toml_edit::Table::new(); + table.set_implicit(true); + mp.insert(&provider_id, toml_edit::Item::Table(table)); + } + } + + doc.to_string() +} + /// Route a Codex live write between full auth+config or config-only. /// /// Official providers with usable login material own `auth.json`. Third-party @@ -1047,14 +1104,18 @@ pub fn write_codex_live_for_provider( auth: &Value, config_text: Option<&str>, ) -> Result<(), AppError> { + let normalized = config_text + .map(|t| ensure_codex_model_provider_section(t)) + .unwrap_or_default(); + 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 should_write_auth { - write_codex_live_atomic(auth, config_text) + write_codex_live_atomic(auth, Some(&normalized)) } else { - let live_config = prepare_codex_provider_live_config(auth, config_text.unwrap_or(""))?; + let live_config = prepare_codex_provider_live_config(auth, &normalized)?; write_codex_live_config_atomic(Some(&live_config)) } } @@ -1065,16 +1126,21 @@ pub fn write_codex_live_for_provider( /// requests can use a provider-scoped `experimental_bearer_token`, so switching /// providers only needs to update `config.toml`; `auth.json` stays as the user's /// long-lived ChatGPT login cache. +/// +/// As a defensive measure, this also ensures the `[model_providers.]` TOML +/// table exists when `model_provider` points at a custom provider — see +/// `ensure_codex_model_provider_section` for rationale. pub fn prepare_codex_provider_live_config( auth: &Value, config_text: &str, ) -> Result { + let normalized = ensure_codex_model_provider_section(config_text); let token = extract_codex_auth_api_key(auth) - .or_else(|| extract_codex_experimental_bearer_token(config_text)); + .or_else(|| extract_codex_experimental_bearer_token(&normalized)); Ok(match token { - Some(token) => set_codex_experimental_bearer_token(config_text, &token)?, - None => config_text.to_string(), + Some(token) => set_codex_experimental_bearer_token(&normalized, &token)?, + None => normalized, }) } @@ -2129,4 +2195,122 @@ model_catalog_json = "cc-switch-model-catalog.json" "None arm should remove relative cc-switch-owned field" ); } + + #[test] + fn ensure_model_provider_section_creates_missing_section() { + // Simulates a config that Codex Desktop App rewrote to a flat format, + // stripping the [model_providers.custom] table. Codex CLI needs that + // table to route requests to the custom provider. + let input = r#"model_provider = "custom" +model = "deepseek-v4-flash" +model_reasoning_effort = "high" +disable_response_storage = true +"#; + let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + assert_eq!( + parsed + .get("model_provider") + .and_then(|v| v.as_str()), + Some("custom"), + "model_provider top-level key must be preserved" + ); + let section = parsed + .get("model_providers") + .and_then(|v| v.get("custom")) + .expect("[model_providers.custom] table must be created"); + assert!( + section.is_table(), + "[model_providers.custom] must be a TOML table" + ); + } + + #[test] + fn ensure_model_provider_section_preserves_existing_section() { + let input = r#"model_provider = "vendor_x" +model = "gpt-5" + +[model_providers.vendor_x] +name = "Vendor X" +base_url = "https://vendor.example/v1" +wire_api = "responses" +"#; + let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + // Original table must be left untouched. + let section = parsed + .get("model_providers") + .and_then(|v| v.get("vendor_x")) + .expect("[model_providers.vendor_x] must be preserved"); + assert_eq!( + section.get("base_url").and_then(|v| v.as_str()), + Some("https://vendor.example/v1"), + "existing [model_providers.vendor_x] fields must be preserved" + ); + } + + #[test] + fn ensure_model_provider_section_ignores_reserved_providers() { + // Reserved providers (openai, ollama, etc.) don't need a custom table. + let input = r#"model_provider = "openai" +model = "gpt-4" +"#; + let result = ensure_codex_model_provider_section(input); + assert_eq!( + result.lines().find(|l| l.trim() == "[model_providers.openai]"), + None, + "reserved provider openai must not get a [model_providers.openai] table" + ); + } + + #[test] + fn ensure_model_provider_section_noops_on_empty_config() { + assert_eq!(ensure_codex_model_provider_section(""), ""); + assert_eq!(ensure_codex_model_provider_section(" "), " "); + } + + #[test] + fn ensure_model_provider_section_noops_without_model_provider() { + // No model_provider key at all — nothing to do. + let input = r#"model = "gpt-4" +wire_api = "responses" +"#; + let result = ensure_codex_model_provider_section(input); + assert!( + result + .lines() + .find(|l| l.trim() == "[model_providers.") + .is_none(), + "no [model_providers.*] section should be created without model_provider" + ); + } + + #[test] + fn prepare_live_config_injects_section_when_missing() { + // This is the core fix: when prepare_codex_provider_live_config receives + // a flat config, it must normalize it so the written TOML has the table. + let input = r#"model_provider = "deepseek" +model = "deepseek-v4-pro" +"#; + let auth = json!({ "OPENAI_API_KEY": "sk-deepseek" }); + let result = prepare_codex_provider_live_config(&auth, input).unwrap(); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + assert!( + parsed + .get("model_providers") + .and_then(|v| v.get("deepseek")) + .is_some(), + "prepare_codex_provider_live_config must add [model_providers.deepseek] table" + ); + assert_eq!( + parsed + .get("experimental_bearer_token") + .and_then(|v| v.as_str()), + Some("sk-deepseek"), + "API key must be moved to experimental_bearer_token" + ); + } } From 1761570241021fc619d5116520f1395df0a2859c Mon Sep 17 00:00:00 2001 From: Liu Zhangjian Date: Thu, 4 Jun 2026 21:53:19 +0800 Subject: [PATCH 2/5] fix(codex): render non-implicit TOML table for [model_providers.] set_implicit(true) on the synthesized table prevented toml_edit from emitting the [model_providers.] section header, causing the empty table to be invisible to subsequent DocumentMut parsing. This made set_codex_experimental_bearer_token fall through to the top-level fallback instead of scoping the token inside the provider table. Also update two test assertions to check the bearer token at its scoped location inside [model_providers.] rather than the top level. --- src-tauri/src/codex_config.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index f65e4f689d..b057884408 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1085,9 +1085,9 @@ pub fn ensure_codex_model_provider_section(config_text: &str) -> String { if let Some(mp) = doc.get_mut("model_providers").and_then(|item| item.as_table_mut()) { if !mp.contains_key(&provider_id) { - let mut table = toml_edit::Table::new(); - table.set_implicit(true); - mp.insert(&provider_id, toml_edit::Item::Table(table)); + let mut t = toml_edit::Table::new(); + t.set_implicit(false); + mp.insert(&provider_id, toml_edit::Item::Table(t)); } } @@ -1397,7 +1397,7 @@ experimental_bearer_token = "stale-table-key" } #[test] - fn prepare_provider_live_config_does_not_create_incomplete_provider_table() { + fn prepare_provider_live_config_synthesizes_missing_provider_table() { let input = r#"model_provider = "vendor_x" model = "gpt-5" "#; @@ -1407,15 +1407,24 @@ model = "gpt-5" .expect("prepare live config"); let parsed: toml::Value = toml::from_str(&output).expect("parse output"); + // The bearer token is written into the scoped provider table, not at top level. assert_eq!( parsed - .get("experimental_bearer_token") + .get("model_providers") + .and_then(|v| v.get("vendor_x")) + .and_then(|v| v.get("experimental_bearer_token")) .and_then(|v| v.as_str()), - Some("sk-test") + Some("sk-test"), + "API key must be written to [model_providers.vendor_x].experimental_bearer_token" ); + // The defensive normalization now ensures [model_providers.] + // exists so Codex CLI can route to the custom provider. assert!( - parsed.get("model_providers").is_none(), - "missing provider tables should not be synthesized without endpoint fields" + parsed + .get("model_providers") + .and_then(|v| v.get("vendor_x")) + .is_some(), + "[model_providers.vendor_x] table must be synthesized even without endpoint fields" ); } @@ -2307,10 +2316,12 @@ model = "deepseek-v4-pro" ); assert_eq!( parsed - .get("experimental_bearer_token") + .get("model_providers") + .and_then(|v| v.get("deepseek")) + .and_then(|v| v.get("experimental_bearer_token")) .and_then(|v| v.as_str()), Some("sk-deepseek"), - "API key must be moved to experimental_bearer_token" + "API key must be written to [model_providers.deepseek].experimental_bearer_token" ); } } From 6e29aa48cb3ee4b752e2a31575c4d3def9c7a7df Mon Sep 17 00:00:00 2001 From: Kakueeen Date: Fri, 5 Jun 2026 09:34:45 +0800 Subject: [PATCH 3/5] fix(codex): migrate flat-format base_url/wire_api into synthesized provider table Address AI review P2 feedback: when ensure_codex_model_provider_section() creates a new [model_providers.] table, it now copies/migrates the legacy top-level base_url and wire_api fields into the synthesized table and removes the top-level copies. This ensures that configs in the flat layout (model_provider + top-level endpoint fields) that Codex Desktop App or CC-Switch import can leave behind are normalized so that Codex CLI finds all required routing fields inside [model_providers.]. Also fixes cargo fmt issues that caused CI to fail. Closes #3719 --- src-tauri/src/codex_config.rs | 115 ++++++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index b057884408..5289e880cd 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1077,17 +1077,45 @@ pub fn ensure_codex_model_provider_section(config_text: &str) -> String { return config_text.to_string(); } - // Create [model_providers.] table. + // Create [model_providers.] table, migrating legacy flat-format endpoint + // fields so the provider table has everything Codex CLI needs to route requests. if doc.get("model_providers").is_none() { doc["model_providers"] = toml_edit::table(); } - if let Some(mp) = doc.get_mut("model_providers").and_then(|item| item.as_table_mut()) + // Extract top-level endpoint fields before taking a mutable borrow of the doc. + // We collect them into a Vec so we can clone into the new table without + // borrowing `doc` while `mp` holds a mutable reference. + let flat_fields: Vec<(String, toml_edit::Item)> = ["base_url", "wire_api"] + .iter() + .filter_map(|field| { + doc.as_table() + .get(field) + .map(|v| (field.to_string(), v.clone())) + }) + .collect(); + + if let Some(mp) = doc + .get_mut("model_providers") + .and_then(|item| item.as_table_mut()) { if !mp.contains_key(&provider_id) { let mut t = toml_edit::Table::new(); t.set_implicit(false); + + // Migrate legacy top-level endpoint fields into the synthesized table so + // Codex CLI finds everything it needs when it reads [model_providers.]. + for (field, val) in &flat_fields { + t.insert(field, val.clone()); + } + mp.insert(&provider_id, toml_edit::Item::Table(t)); + + // Clean up the top-level copies so the written config is clean and + // consistent (Codex CLI reads from the provider table, not top level). + for (field, _) in &flat_fields { + doc.remove(field.as_str()); + } } } @@ -2219,9 +2247,7 @@ disable_response_storage = true let parsed: toml::Value = toml::from_str(&result).unwrap(); assert_eq!( - parsed - .get("model_provider") - .and_then(|v| v.as_str()), + parsed.get("model_provider").and_then(|v| v.as_str()), Some("custom"), "model_provider top-level key must be preserved" ); @@ -2268,7 +2294,9 @@ model = "gpt-4" "#; let result = ensure_codex_model_provider_section(input); assert_eq!( - result.lines().find(|l| l.trim() == "[model_providers.openai]"), + result + .lines() + .find(|l| l.trim() == "[model_providers.openai]"), None, "reserved provider openai must not get a [model_providers.openai] table" ); @@ -2324,4 +2352,79 @@ model = "deepseek-v4-pro" "API key must be written to [model_providers.deepseek].experimental_bearer_token" ); } + + #[test] + fn ensure_model_provider_section_migrates_flat_endpoint_fields() { + // When a config has model_provider set with top-level base_url / wire_api + // (the "flat" layout that Codex Desktop App can leave behind), the + // synthesized [model_providers.] table must inherit those fields so + // that Codex CLI can resolve the third-party endpoint. + let input = r#"model_provider = "vendor" +model = "gpt-5" +base_url = "https://vendor.example/v1" +wire_api = "responses" +"#; + let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + // Endpoint fields must now live inside the provider table, not at top level. + assert_eq!( + parsed + .get("model_providers") + .and_then(|v| v.get("vendor")) + .and_then(|v| v.get("base_url")) + .and_then(|v| v.as_str()), + Some("https://vendor.example/v1"), + "base_url must be migrated into [model_providers.vendor]" + ); + assert_eq!( + parsed + .get("model_providers") + .and_then(|v| v.get("vendor")) + .and_then(|v| v.get("wire_api")) + .and_then(|v| v.as_str()), + Some("responses"), + "wire_api must be migrated into [model_providers.vendor]" + ); + // Top-level copies must be removed to avoid ambiguity. + assert!( + parsed.get("base_url").is_none(), + "top-level base_url must be removed after migration" + ); + assert!( + parsed.get("wire_api").is_none(), + "top-level wire_api must be removed after migration" + ); + } + + #[test] + fn ensure_model_provider_section_skips_migration_when_table_exists() { + // If the provider table already exists (even partially), migration must + // NOT overwrite existing scoped fields with stale top-level copies. + let input = r#"model_provider = "vendor" +base_url = "https://stale.example/v1" + +[model_providers.vendor] +base_url = "https://correct.example/v1" +"#; + let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + // Existing scoped value must be preserved; top-level stale value ignored. + assert_eq!( + parsed + .get("model_providers") + .and_then(|v| v.get("vendor")) + .and_then(|v| v.get("base_url")) + .and_then(|v| v.as_str()), + Some("https://correct.example/v1"), + "existing scoped base_url must not be overwritten by top-level copy" + ); + // Top-level value must remain untouched when the table already existed. + assert_eq!( + parsed.get("base_url").and_then(|v| v.as_str()), + Some("https://stale.example/v1"), + "top-level base_url must remain when provider table already existed" + ); + } } From a8d1bedd2af224454d91ddf4be0671e37b0bb6c2 Mon Sep 17 00:00:00 2001 From: Kakueeen Date: Fri, 5 Jun 2026 11:10:14 +0800 Subject: [PATCH 4/5] fix(codex): address maintainer review feedback P1 (Clippy): Use method reference instead of closure. P1/P2: Handle flat config without model_provider (issue #3449 format). When top-level base_url exists but model_provider is absent, synthesize model_provider = "custom" + [model_providers.custom] table with migrated endpoint fields. P2: Fill incomplete existing provider tables. When [model_providers.] exists but is missing base_url/wire_api while top-level copies remain, fill the gap so Codex CLI can route. Refactored ensure_codex_model_provider_section() with clearer documentation covering all three normalization scenarios. Closes #3719 --- src-tauri/src/codex_config.rs | 257 +++++++++++++++++++++++++++------- 1 file changed, 203 insertions(+), 54 deletions(-) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index 5289e880cd..8d2995c416 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1037,20 +1037,26 @@ pub fn read_codex_live_settings() -> Result { Ok(json!({ "auth": auth, "config": cfg_text })) } -/// Ensure the `[model_providers.]` TOML table exists when -/// `model_provider` is set to a non-reserved (custom) provider ID. +/// Normalize a Codex `config.toml` so Codex CLI can route to the configured +/// third-party provider. /// -/// Codex CLI requires this section to route requests to the custom provider; -/// without it, Codex falls back to `model_provider: openai` and all API calls go -/// directly to `api.openai.com`. The section can be lost when: +/// Codex CLI requires `[model_providers.]` to exist with at minimum a +/// `base_url` so it can route requests. This function handles three scenarios +/// that can leave the config in a broken state: /// -/// - Codex Desktop App rewrites `config.toml`, stripping unknown tables -/// - CC-Switch imports a pre-existing flat-format config -/// - A backup/restore cycle drops the section +/// - **Flat config without `model_provider`** (P1/P2): Codex Desktop App or +/// other tools can produce a config with top-level `base_url`/`wire_api` but +/// no `model_provider`. We detect this by checking for top-level endpoint +/// fields and synthesize `model_provider = "custom"` + `[model_providers.custom]`. +/// - **Missing provider table** (the original fix): `model_provider` is set to +/// a custom provider but the table is absent — we create it, migrating any +/// top-level endpoint fields. +/// - **Incomplete existing table** (P2): The provider table exists but is missing +/// `base_url`/`wire_api` while top-level copies remain — we fill the gap so +/// the table is complete for Codex CLI routing. /// -/// This function is a defensive normalization: if the top-level key is present -/// but the corresponding table is missing, it creates an empty table so that -/// subsequent `update_codex_toml_field` calls have a valid target. +/// Returns the normalized config text, or the original text unchanged on parse +/// errors. Reserved provider IDs ("openai", "ollama", etc.) are left untouched. pub fn ensure_codex_model_provider_section(config_text: &str) -> String { if config_text.trim().is_empty() { return config_text.to_string(); @@ -1060,7 +1066,18 @@ pub fn ensure_codex_model_provider_section(config_text: &str) -> String { return config_text.to_string(); }; - let Some(provider_id) = active_codex_model_provider_id(&doc) else { + let active_provider = active_codex_model_provider_id(&doc); + let top_has_endpoint = doc.as_table().get("base_url").is_some(); + + // P1/P2: Flat config with top-level endpoint fields but no model_provider. + // Synthesize model_provider = "custom" so Codex CLI routes correctly. + let provider_id = if let Some(id) = active_provider { + id + } else if top_has_endpoint { + doc["model_provider"] = toml_edit::value("custom"); + "custom".to_string() + } else { + // No active provider and no top-level endpoint — nothing to normalize. return config_text.to_string(); }; @@ -1068,24 +1085,7 @@ pub fn ensure_codex_model_provider_section(config_text: &str) -> String { return config_text.to_string(); } - // Already has the section — nothing to do. - if doc - .get("model_providers") - .and_then(|mp| mp.as_table()) - .is_some_and(|t| t.contains_key(&provider_id)) - { - return config_text.to_string(); - } - - // Create [model_providers.] table, migrating legacy flat-format endpoint - // fields so the provider table has everything Codex CLI needs to route requests. - if doc.get("model_providers").is_none() { - doc["model_providers"] = toml_edit::table(); - } - // Extract top-level endpoint fields before taking a mutable borrow of the doc. - // We collect them into a Vec so we can clone into the new table without - // borrowing `doc` while `mp` holds a mutable reference. let flat_fields: Vec<(String, toml_edit::Item)> = ["base_url", "wire_api"] .iter() .filter_map(|field| { @@ -1095,26 +1095,59 @@ pub fn ensure_codex_model_provider_section(config_text: &str) -> String { }) .collect(); - if let Some(mp) = doc + // Ensure the [model_providers] parent table exists. + if doc.get("model_providers").is_none() { + doc["model_providers"] = toml_edit::table(); + } + + let mp = doc .get_mut("model_providers") - .and_then(|item| item.as_table_mut()) - { - if !mp.contains_key(&provider_id) { - let mut t = toml_edit::Table::new(); - t.set_implicit(false); - - // Migrate legacy top-level endpoint fields into the synthesized table so - // Codex CLI finds everything it needs when it reads [model_providers.]. - for (field, val) in &flat_fields { - t.insert(field, val.clone()); - } + .and_then(|item| item.as_table_mut()); + + let Some(mp) = mp else { + return config_text.to_string(); + }; + + let table_exists = mp.contains_key(&provider_id); - mp.insert(&provider_id, toml_edit::Item::Table(t)); + if !table_exists { + // Create a new provider table, migrating top-level endpoint fields. + let mut t = toml_edit::Table::new(); + t.set_implicit(false); + for (field, val) in &flat_fields { + t.insert(field, val.clone()); + } + mp.insert(&provider_id, toml_edit::Item::Table(t)); + + // Clean up top-level copies after migration. + for (field, _) in &flat_fields { + doc.remove(field); + } + } else { + // P2: Provider table exists but may be missing endpoint fields. + // Fill in missing fields from top level if available. + let fields_to_insert: Vec<(String, toml_edit::Item)> = flat_fields + .iter() + .filter(|(field, _)| { + mp.get(&provider_id) + .and_then(|item| item.as_table()) + .is_none_or(|t| !t.contains_key(field)) + }) + .cloned() + .collect(); - // Clean up the top-level copies so the written config is clean and - // consistent (Codex CLI reads from the provider table, not top level). - for (field, _) in &flat_fields { - doc.remove(field.as_str()); + if !fields_to_insert.is_empty() { + if let Some(provider_table) = mp + .get_mut(&provider_id) + .and_then(|item| item.as_table_mut()) + { + for (field, val) in &fields_to_insert { + provider_table.insert(field, val.clone()); + } + } + // Clean up top-level copies after filling table (after provider_table borrow ends). + for (field, _) in &fields_to_insert { + doc.remove(field); } } } @@ -1133,7 +1166,7 @@ pub fn write_codex_live_for_provider( config_text: Option<&str>, ) -> Result<(), AppError> { let normalized = config_text - .map(|t| ensure_codex_model_provider_section(t)) + .map(ensure_codex_model_provider_section) .unwrap_or_default(); let should_write_auth = (category == Some("official") && codex_auth_has_login_material(auth)) @@ -2309,18 +2342,134 @@ model = "gpt-4" } #[test] - fn ensure_model_provider_section_noops_without_model_provider() { - // No model_provider key at all — nothing to do. + fn ensure_model_provider_section_synthesizes_from_flat_config_without_model_provider() { + // P1/P2: Flat config with top-level endpoint fields but no model_provider. + // This is the exact format #3449 reported as broken: Codex Desktop App or + // other tools can produce a config with base_url/wire_api but no model_provider. + // We must detect this and synthesize model_provider = "custom" + table. + let input = r#"base_url = "https://astron.top/v1" +wire_api = "responses" +model = "astron-code-latest" +experimental_bearer_token = "PROXY_MANAGED" +model_catalog_json = "cc-switch-model-catalog.json" +"#; + let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + // model_provider must be synthesized as "custom". + assert_eq!( + parsed.get("model_provider").and_then(|v| v.as_str()), + Some("custom"), + "model_provider = \"custom\" must be synthesized from flat config" + ); + // Provider table must be created with migrated endpoint fields. + let section = parsed + .get("model_providers") + .and_then(|v| v.get("custom")) + .expect("[model_providers.custom] table must be created from flat config"); + assert!( + section.is_table(), + "[model_providers.custom] must be a TOML table" + ); + assert_eq!( + section.get("base_url").and_then(|v| v.as_str()), + Some("https://astron.top/v1"), + "base_url must be migrated from top level" + ); + assert_eq!( + section.get("wire_api").and_then(|v| v.as_str()), + Some("responses"), + "wire_api must be migrated from top level" + ); + // Top-level copies must be removed after migration. + assert!( + parsed.get("base_url").is_none(), + "top-level base_url must be removed after migration" + ); + } + + #[test] + fn ensure_model_provider_section_noops_without_model_provider_and_no_endpoint() { + // No model_provider key AND no top-level endpoint fields — nothing to normalize. let input = r#"model = "gpt-4" +disable_response_storage = true +"#; + let result = ensure_codex_model_provider_section(input); + assert_eq!( + result, input, + "config without model_provider and without endpoint fields must pass through unchanged" + ); + } + + #[test] + fn ensure_model_provider_section_fills_incomplete_existing_table() { + // P2: Provider table exists but is missing base_url/wire_api while + // top-level copies remain. We should fill in the missing fields. + let input = r#"model_provider = "vendor" +model = "gpt-5" +base_url = "https://vendor.example/v1" wire_api = "responses" + +[model_providers.vendor] +name = "Vendor" "#; let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + let section = parsed + .get("model_providers") + .and_then(|v| v.get("vendor")) + .expect("[model_providers.vendor] must exist"); + // Missing fields must be filled from top level. + assert_eq!( + section.get("base_url").and_then(|v| v.as_str()), + Some("https://vendor.example/v1"), + "base_url must be filled into incomplete provider table" + ); + assert_eq!( + section.get("wire_api").and_then(|v| v.as_str()), + Some("responses"), + "wire_api must be filled into incomplete provider table" + ); + // Original fields must be preserved. + assert_eq!( + section.get("name").and_then(|v| v.as_str()), + Some("Vendor"), + "existing fields must be preserved" + ); + // Top-level copies must be cleaned up. assert!( - result - .lines() - .find(|l| l.trim() == "[model_providers.") - .is_none(), - "no [model_providers.*] section should be created without model_provider" + parsed.get("base_url").is_none(), + "top-level base_url must be removed after filling table" + ); + } + + #[test] + fn ensure_model_provider_section_does_not_overwrite_existing_endpoint_in_table() { + // When provider table already has base_url, top-level copy must NOT overwrite it. + let input = r#"model_provider = "vendor" +base_url = "https://stale.example/v1" + +[model_providers.vendor] +base_url = "https://correct.example/v1" +"#; + let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + let section = parsed + .get("model_providers") + .and_then(|v| v.get("vendor")) + .expect("[model_providers.vendor] must exist"); + assert_eq!( + section.get("base_url").and_then(|v| v.as_str()), + Some("https://correct.example/v1"), + "existing scoped base_url must not be overwritten by top-level copy" + ); + // Top-level stale value must remain untouched (table existed before). + assert_eq!( + parsed.get("base_url").and_then(|v| v.as_str()), + Some("https://stale.example/v1"), + "top-level base_url must remain when provider table already existed" ); } From 2314748dd5177de021acca5b0449345a497d0890 Mon Sep 17 00:00:00 2001 From: Kakueeen Date: Mon, 8 Jun 2026 09:50:37 +0800 Subject: [PATCH 5/5] fix(codex): preserve flat fields when provider entry is inline table When the provider entry uses TOML inline table syntax (e.g. `[model_providers] vendor = { name = "Vendor" }`), as_table_mut() returns None so we cannot insert endpoint fields into it. The code previously still executed doc.remove() for top-level base_url/wire_api, causing data loss. Now the cleanup only happens after a successful insert into a genuine table. Added test: ensure_model_provider_section_preserves_flat_fields_for_inline_table Closes #3719 --- src-tauri/src/codex_config.rs | 44 +++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index 8d2995c416..2282134ed1 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1137,6 +1137,10 @@ pub fn ensure_codex_model_provider_section(config_text: &str) -> String { .collect(); if !fields_to_insert.is_empty() { + // Only proceed with cleanup if we can actually write into the + // provider entry. Inline tables (e.g. `vendor = { name = "V" }`) + // are values, not tables — as_table_mut() returns None for them, + // so we must not remove top-level fields we couldn't migrate. if let Some(provider_table) = mp .get_mut(&provider_id) .and_then(|item| item.as_table_mut()) @@ -1144,10 +1148,11 @@ pub fn ensure_codex_model_provider_section(config_text: &str) -> String { for (field, val) in &fields_to_insert { provider_table.insert(field, val.clone()); } - } - // Clean up top-level copies after filling table (after provider_table borrow ends). - for (field, _) in &fields_to_insert { - doc.remove(field); + // Clean up top-level copies only after successful insertion + // (borrow on provider_table has ended by this point). + for (field, _) in &fields_to_insert { + doc.remove(field); + } } } } @@ -2473,6 +2478,37 @@ base_url = "https://correct.example/v1" ); } + #[test] + fn ensure_model_provider_section_preserves_flat_fields_for_inline_table() { + // When the provider entry is an inline table (value, not a standard table), + // as_table_mut() returns None so we cannot insert into it. Top-level + // endpoint fields must NOT be removed in this case — doing so would + // silently drop the routing information Codex CLI needs. + let input = r#"model_provider = "vendor" +model = "gpt-5" +base_url = "https://vendor.example/v1" +wire_api = "responses" + +[model_providers] +vendor = { name = "Vendor" } +"#; + let result = ensure_codex_model_provider_section(input); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + // Top-level endpoint fields must be preserved — we couldn't migrate them + // into the inline table, so removing them would be data loss. + assert_eq!( + parsed.get("base_url").and_then(|v| v.as_str()), + Some("https://vendor.example/v1"), + "top-level base_url must be preserved when provider is an inline table" + ); + assert_eq!( + parsed.get("wire_api").and_then(|v| v.as_str()), + Some("responses"), + "top-level wire_api must be preserved when provider is an inline table" + ); + } + #[test] fn prepare_live_config_injects_section_when_missing() { // This is the core fix: when prepare_codex_provider_live_config receives