From d5328e52907852e4d3c636e588aafa3e4b922267 Mon Sep 17 00:00:00 2001 From: Jason Date: Sun, 31 May 2026 12:11:21 +0800 Subject: [PATCH] Fix Codex model catalog lost on proxy takeover-off restore Turning proxy takeover off restores Live from a stored backup via write_codex_live_verbatim. That path mishandled the Codex model catalog for two backup shapes that need opposite treatment: - Snapshot backup (read_codex_live_settings -> {auth, config}): no inline modelCatalog, but the config.toml text already carries the live model_catalog_json pointer. The old code ran catalog projection, which saw no specs and stripped the pointer. - Provider-rebuilt backup (update_live_backup_from_provider): inline modelCatalog (DB SSOT) with a pointer-less config text. A pure verbatim write ignored the inline catalog and never regenerated the pointer. The projection decision is orthogonal to auth: a provider-rebuilt backup can pair an inline modelCatalog with empty auth.json ({}) when the API key lives in the config's experimental_bearer_token. The empty-auth branch raw-wrote config and skipped projection entirely, so that shape lost its mapping too. Decide the config.toml text once, before splitting on auth, via the new prepare_codex_live_config_text_with_optional_catalog helper: project only when an inline modelCatalog is present, else keep the text raw (preserving any existing pointer). Every config-writing branch (write-auth, delete-auth, no-auth) now applies it consistently. Add regression tests covering all three shapes. --- src-tauri/src/codex_config.rs | 43 +++++-- src-tauri/src/services/proxy.rs | 221 +++++++++++++++++++++++++++++++- 2 files changed, 251 insertions(+), 13 deletions(-) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index 5b83daf22..eb58160fb 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -846,18 +846,39 @@ fn build_simplified_catalog_from_texts(config_text: &str, catalog_text: &str) -> Some(json!({ "models": entries })) } -/// Unified helper: write Codex live config with model catalog preparation. -/// Replaces scattered `prepare_codex_config_text_with_model_catalog` calls. -pub fn write_codex_live_with_catalog( +/// Decide the `config.toml` text to write during a takeover-off restore, +/// projecting the model catalog **only when `settings` carries an inline +/// `modelCatalog`**. +/// +/// Restore feeds back a stored backup, and Codex backups come in two shapes that +/// need opposite handling: +/// +/// - **Snapshot backup** (`read_codex_live_settings`): `{ auth, config }` with no +/// inline `modelCatalog`. Its `config.toml` text already carries whatever +/// `model_catalog_json` pointer existed at backup time, and the generated +/// catalog file on disk is untouched. Here we must keep the config **raw** — +/// running catalog projection would see "no specs" and strip the live pointer. +/// - **Provider-rebuilt backup** (`update_live_backup_from_provider`): the DB +/// provider's settings, i.e. `{ auth, config (no pointer), modelCatalog +/// (inline DB SSOT) }`. Here the pointer/catalog file must be (re)generated +/// from the inline `modelCatalog`, or the mapping is lost on restore. +/// +/// Gating on the presence of the inline `modelCatalog` key routes each shape +/// correctly; an empty inline catalog still projects (and so correctly drops a +/// now-stale pointer), while an absent key leaves the text untouched. This is +/// **orthogonal to auth** — a provider-rebuilt backup can pair an inline +/// `modelCatalog` with empty `auth.json` (the API key living in the config's +/// `experimental_bearer_token`), so the caller must decide config projection +/// independently of whether it writes or deletes `auth.json`. +pub fn prepare_codex_live_config_text_with_optional_catalog( settings: &Value, - auth: &Value, - config_text: Option<&str>, -) -> Result<(), AppError> { - let prepared_config = config_text - .map(|text| prepare_codex_config_text_with_model_catalog(settings, text)) - .transpose()?; - - write_codex_live_atomic(auth, prepared_config.as_deref()) + config_text: &str, +) -> Result { + if settings.get("modelCatalog").is_some() { + prepare_codex_config_text_with_model_catalog(settings, config_text) + } else { + Ok(config_text.to_string()) + } } pub fn write_codex_provider_live_with_catalog( diff --git a/src-tauri/src/services/proxy.rs b/src-tauri/src/services/proxy.rs index 99115a719..ad9bf7083 100644 --- a/src-tauri/src/services/proxy.rs +++ b/src-tauri/src/services/proxy.rs @@ -2131,7 +2131,29 @@ impl ProxyService { let auth = config.get("auth"); let config_str = config.get("config").and_then(|v| v.as_str()); - match (auth, config_str) { + // Decide the config.toml text ONCE, before splitting on auth. A stored + // Codex backup comes in two shapes needing opposite handling: + // - snapshot backup (`read_codex_live_settings`): no inline `modelCatalog`; + // the config text already carries the live `model_catalog_json` pointer + // → keep raw, or projection would strip it. + // - provider-rebuilt backup (`update_live_backup_from_provider`): inline + // `modelCatalog` (DB SSOT) with a pointer-less config text → project, + // or the mapping is lost on restore. + // The projection decision is orthogonal to auth: a provider-rebuilt backup + // can pair an inline `modelCatalog` with empty/absent `auth.json` (the key + // living in the config's `experimental_bearer_token`). Computing it up here + // keeps every config-writing branch — write-auth, delete-auth, no-auth — + // consistent instead of letting the empty-auth path skip projection. + let prepared_cfg = config_str + .map(|cfg| { + crate::codex_config::prepare_codex_live_config_text_with_optional_catalog( + config, cfg, + ) + }) + .transpose() + .map_err(|e| format!("写入 Codex 配置失败: {e}"))?; + + match (auth, prepared_cfg.as_deref()) { (Some(auth), Some(cfg)) => { let auth_path = get_codex_auth_path(); if auth.as_object().is_some_and(|obj| obj.is_empty()) { @@ -2140,7 +2162,7 @@ impl ProxyService { crate::config::write_text_file(&config_path, cfg) .map_err(|e| format!("写入 Codex config 失败: {e}"))?; } else { - crate::codex_config::write_codex_live_with_catalog(config, auth, Some(cfg)) + crate::codex_config::write_codex_live_atomic(auth, Some(cfg)) .map_err(|e| format!("写入 Codex 配置失败: {e}"))?; } } @@ -4471,4 +4493,199 @@ requires_openai_auth = true "switch should surface catalog write failure, got: {message}" ); } + + /// Regression: turning proxy takeover off restores Live from the backup. The + /// backup snapshot is `read_codex_live_settings()` output (`{auth, config}`, + /// never an inline `modelCatalog`). The restore must NOT route the config + /// through catalog projection, which would see no specs and strip the + /// `model_catalog_json` pointer — silently dropping the user's Codex model + /// mapping from Live even though the DB SSOT still holds it. + #[tokio::test] + #[serial] + async fn codex_restore_from_backup_preserves_model_catalog_pointer() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + + let db = Arc::new(Database::memory().expect("init db")); + let service = ProxyService::new(db.clone()); + + // Pre-takeover Live state: config.toml points at the cc-switch generated + // catalog file, and that file exists on disk (takeover never touches it). + let catalog_path = crate::codex_config::get_codex_model_catalog_path(); + if let Some(parent) = catalog_path.parent() { + std::fs::create_dir_all(parent).expect("create codex dir"); + } + std::fs::write( + &catalog_path, + r#"{"models":[{"slug":"deepseek-v4-flash"}]}"#, + ) + .expect("seed generated catalog file"); + + let pointer = catalog_path.to_string_lossy().to_string(); + let backup_config = format!( + "model_provider = \"custom\"\n\ + model = \"deepseek-v4-flash\"\n\ + model_catalog_json = \"{pointer}\"\n\n\ + [model_providers.custom]\n\ + name = \"DeepSeek\"\n\ + base_url = \"https://api.deepseek.example/v1\"\n\ + wire_api = \"responses\"\n" + ); + let backup_json = serde_json::to_string(&json!({ + "auth": { "OPENAI_API_KEY": "deepseek-key" }, + "config": backup_config, + })) + .expect("serialize backup"); + db.save_live_backup("codex", &backup_json) + .await + .expect("seed live backup"); + + // Turning takeover off restores Live from this backup. + service + .restore_live_config_for_app_with_fallback(&AppType::Codex) + .await + .expect("restore codex live from backup"); + + let restored = std::fs::read_to_string(crate::codex_config::get_codex_config_path()) + .expect("read restored config.toml"); + assert!( + restored.contains("model_catalog_json"), + "restore must preserve the model_catalog_json pointer, got:\n{restored}" + ); + assert!( + restored.contains(pointer.as_str()), + "restored pointer must still reference the cc-switch generated catalog file" + ); + } + + /// Regression: a hot-switch during takeover rebuilds the backup from the DB + /// provider (`update_live_backup_from_provider`), so the backup carries an + /// inline `modelCatalog` (DB SSOT) but a `config.toml` text WITHOUT a + /// `model_catalog_json` pointer. Restoring that backup must project the + /// inline catalog — (re)generating both the catalog file and the pointer — + /// or the Codex model mapping vanishes from Live after takeover-off. + #[tokio::test] + #[serial] + async fn codex_restore_from_backup_projects_inline_model_catalog() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + + let db = Arc::new(Database::memory().expect("init db")); + let service = ProxyService::new(db.clone()); + + // Catalog projection needs a model template; seed `models_cache.json` + // with the template slug so we don't depend on the `codex` CLI. + let codex_dir = crate::codex_config::get_codex_config_dir(); + std::fs::create_dir_all(&codex_dir).expect("create codex dir"); + std::fs::write( + codex_dir.join("models_cache.json"), + r#"{"models":[{"slug":"gpt-5.5"}]}"#, + ) + .expect("seed models_cache template"); + + // Provider-rebuilt backup shape: inline modelCatalog, pointer-less config. + let backup_json = serde_json::to_string(&json!({ + "auth": { "OPENAI_API_KEY": "deepseek-key" }, + "config": "model_provider = \"custom\"\nmodel = \"deepseek-v4-flash\"\n\n[model_providers.custom]\nname = \"DeepSeek\"\nbase_url = \"https://api.deepseek.example/v1\"\nwire_api = \"responses\"\n", + "modelCatalog": { + "models": [ + { "model": "deepseek-v4-flash", "displayName": "DeepSeek V4 Flash", "contextWindow": 1_000_000 } + ] + } + })) + .expect("serialize backup"); + db.save_live_backup("codex", &backup_json) + .await + .expect("seed live backup"); + + service + .restore_live_config_for_app_with_fallback(&AppType::Codex) + .await + .expect("restore codex live from backup"); + + let restored = std::fs::read_to_string(crate::codex_config::get_codex_config_path()) + .expect("read restored config.toml"); + let catalog_path = crate::codex_config::get_codex_model_catalog_path(); + assert!( + restored.contains("model_catalog_json"), + "restore must (re)generate the model_catalog_json pointer from inline catalog, got:\n{restored}" + ); + assert!( + catalog_path.exists(), + "restore must generate the cc-switch catalog file on disk" + ); + let catalog: Value = serde_json::from_str( + &std::fs::read_to_string(&catalog_path).expect("read generated catalog"), + ) + .expect("parse generated catalog"); + let slugs: Vec<&str> = catalog + .get("models") + .and_then(|m| m.as_array()) + .expect("catalog models") + .iter() + .filter_map(|m| m.get("slug").and_then(|s| s.as_str())) + .collect(); + assert!( + slugs.contains(&"deepseek-v4-flash"), + "generated catalog must contain the inline model, got slugs: {slugs:?}" + ); + } + + /// Regression: a provider-rebuilt backup can pair an inline `modelCatalog` + /// with EMPTY `auth.json` (`{}`) — the bearer-token / Mobile-compat shape + /// where the API key lives in the config's `experimental_bearer_token`. The + /// empty-auth restore branch deletes `auth.json` and writes config raw; it + /// must still project the inline catalog (decision is orthogonal to auth), or + /// the model mapping vanishes on takeover-off for this provider shape. + #[tokio::test] + #[serial] + async fn codex_restore_empty_auth_backup_still_projects_inline_catalog() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + + let db = Arc::new(Database::memory().expect("init db")); + let service = ProxyService::new(db.clone()); + + let codex_dir = crate::codex_config::get_codex_config_dir(); + std::fs::create_dir_all(&codex_dir).expect("create codex dir"); + std::fs::write( + codex_dir.join("models_cache.json"), + r#"{"models":[{"slug":"gpt-5.5"}]}"#, + ) + .expect("seed models_cache template"); + + // Empty auth.json + key carried in config.toml's experimental_bearer_token, + // plus the inline modelCatalog (DB SSOT). + let backup_json = serde_json::to_string(&json!({ + "auth": {}, + "config": "model_provider = \"custom\"\nmodel = \"deepseek-v4-flash\"\n\n[model_providers.custom]\nname = \"DeepSeek\"\nbase_url = \"https://api.deepseek.example/v1\"\nwire_api = \"responses\"\nexperimental_bearer_token = \"sk-deepseek\"\n", + "modelCatalog": { + "models": [ { "model": "deepseek-v4-flash", "displayName": "DeepSeek V4 Flash" } ] + } + })) + .expect("serialize backup"); + db.save_live_backup("codex", &backup_json) + .await + .expect("seed live backup"); + + service + .restore_live_config_for_app_with_fallback(&AppType::Codex) + .await + .expect("restore codex live from backup"); + + let restored = std::fs::read_to_string(crate::codex_config::get_codex_config_path()) + .expect("read restored config.toml"); + assert!( + restored.contains("model_catalog_json"), + "empty-auth restore must still project the inline catalog pointer, got:\n{restored}" + ); + assert!( + crate::codex_config::get_codex_model_catalog_path().exists(), + "empty-auth restore must generate the cc-switch catalog file" + ); + assert!( + !crate::codex_config::get_codex_auth_path().exists(), + "empty-auth restore must delete auth.json rather than write an empty one" + ); + } }