diff --git a/README.md b/README.md index 8243f3dc..e8d4d61f 100644 --- a/README.md +++ b/README.md @@ -323,6 +323,8 @@ Manage API configurations for **Claude Code**, **Codex**, **Gemini**, **OpenCode Pi provider management follows Pi's native additive model: membership comes from `models.json.providers`. CC-Switch does not modify Pi login credentials or its global default provider/model. The Pi TUI keeps the same table/form/shortcut conventions as the other apps and exposes Presets, System Prompts, and Prompt Templates as separate pages. +The Codex provider form includes **Review Model**. Enter a model ID to override `review_model` for that provider when switching or launching, taking precedence over common config. Leave it empty to preserve existing configuration behavior. The selection is stored as `meta.codexReviewModel` and does not require model mapping. Use a model supported by the provider and restart Codex after changing it. + **Features:** One-click switching, standalone Claude settings export, multi-endpoint support, API key management, remote model discovery, and per-app diagnostics such as speed testing or stream health checks where supported. ```bash diff --git a/README_ZH.md b/README_ZH.md index e111c5e1..3ffd283d 100644 --- a/README_ZH.md +++ b/README_ZH.md @@ -326,6 +326,8 @@ copy target\release\cc-switch.exe C:\Windows\System32\ Pi 供应商遵循原生的增量管理模型:是否启用完全取决于 `models.json.providers` 中的成员关系。CC-Switch 不会修改 Pi 的登录凭据或全局默认供应商/模型。 Pi TUI 延续其他应用的表格、表单与快捷键交互,并将预设、系统提示词和 Prompt Templates 分为独立页面。 +Codex 供应商表单提供「审查模型」:填写模型 ID 后,该供应商切换或启动时会使用独立的 `review_model`,优先于通用配置;留空保留原有配置行为。此设置以 `meta.codexReviewModel` 保存在供应商中,不依赖模型映射。模型必须受该供应商支持,修改后请重新启动 Codex。 + **功能:** 一键切换、Claude 独立 settings 导出、多端点支持、API 密钥管理、远端模型发现,以及按应用提供的速度测试、流式健康检查等诊断能力。 ```bash diff --git a/src-tauri/src/cli/codex_shared_launch.rs b/src-tauri/src/cli/codex_shared_launch.rs index bdd7049d..e199b3e5 100644 --- a/src-tauri/src/cli/codex_shared_launch.rs +++ b/src-tauri/src/cli/codex_shared_launch.rs @@ -107,6 +107,7 @@ fn shared_config(provider: &Provider, sqlite_home: &Path) -> Result() .map_err(|err| AppError::Config(err.to_string()))?; @@ -329,6 +330,20 @@ mod tests { ) } + #[test] + fn codex_review_model_is_projected_for_launch_without_mutating_template() { + let original = "model = 'main'\nreview_model = 'legacy'\n"; + let mut provider = provider("review", original); + provider.meta = Some(crate::provider::ProviderMeta { + codex_review_model: Some("vendor-review".into()), + ..Default::default() + }); + let config = shared_config(&provider, Path::new("/shared/sqlite")).unwrap(); + let doc = config.parse::().unwrap(); + assert_eq!(doc["review_model"].as_str(), Some("vendor-review")); + assert_eq!(provider.settings_config["config"], original); + } + #[test] fn shared_config_preserves_selected_endpoint_and_original_settings() { let original = "model_provider = 'relay'\nmodel = 'demo'\n[model_providers.relay]\nname = 'Relay'\nbase_url = 'https://relay.invalid/v1'\nexperimental_bearer_token = 'private-token'\nwire_api = 'responses'\n"; diff --git a/src-tauri/src/cli/codex_temp_launch.rs b/src-tauri/src/cli/codex_temp_launch.rs index 139e85d8..38e4be6e 100644 --- a/src-tauri/src/cli/codex_temp_launch.rs +++ b/src-tauri/src/cli/codex_temp_launch.rs @@ -172,6 +172,12 @@ where let config_path = codex_home.join("config.toml"); write_secret_file(&config_path, launch_settings.config_text.as_bytes())?; + if provider.codex_review_model().is_some() { + write_secret_file( + &codex_home.join(crate::codex_config::CODEX_REVIEW_MODEL_MARKER), + b"1", + )?; + } if let Some(auth) = launch_settings.auth { let auth_path = codex_home.join("auth.json"); @@ -198,12 +204,12 @@ where } } -struct CodexLaunchSettings<'a> { - config_text: &'a str, +struct CodexLaunchSettings { + config_text: String, auth: Option, } -fn parse_launch_settings(provider: &Provider) -> Result, AppError> { +fn parse_launch_settings(provider: &Provider) -> Result { let settings = provider.settings_config.as_object().ok_or_else(|| { AppError::localized( "codex.temp_launch_settings_not_object", @@ -240,6 +246,8 @@ fn parse_launch_settings(provider: &Provider) -> Result, } }; + let config_text = + crate::codex_config::apply_codex_review_model(config_text, provider.codex_review_model())?; Ok(CodexLaunchSettings { config_text, auth }) } @@ -382,6 +390,25 @@ mod tests { } #[cfg(unix)] + #[test] + fn codex_review_model_is_projected_for_launch_without_mutating_template() { + let original = "model = 'main'\nreview_model = 'legacy'\n"; + let mut provider = provider_with(original, Some(serde_json::json!({}))); + provider.meta = Some(crate::provider::ProviderMeta { + codex_review_model: Some("vendor-review".into()), + ..Default::default() + }); + let temp = TempDir::new().unwrap(); + let path = write_temp_codex_home(temp.path(), &provider).unwrap(); + assert!(path + .join(crate::codex_config::CODEX_REVIEW_MODEL_MARKER) + .exists()); + let config = std::fs::read_to_string(path.join("config.toml")).unwrap(); + let doc = config.parse::().unwrap(); + assert_eq!(doc["review_model"].as_str(), Some("vendor-review")); + assert_eq!(provider.settings_config["config"], original); + } + #[test] fn unix_handoff_command_exports_codex_home_and_cleans_up_temp_dir() { let prepared = PreparedCodexLaunch { diff --git a/src-tauri/src/cli/i18n.rs b/src-tauri/src/cli/i18n.rs index 54c1eedc..514e77c9 100644 --- a/src-tauri/src/cli/i18n.rs +++ b/src-tauri/src/cli/i18n.rs @@ -2300,6 +2300,14 @@ pub mod texts { } } + pub fn tui_label_codex_review_model() -> &'static str { + if is_chinese() { + "审查模型" + } else { + "Review Model" + } + } + pub fn tui_codex_reasoning_levels_header() -> &'static str { if is_chinese() { "档位" diff --git a/src-tauri/src/cli/tui/form.rs b/src-tauri/src/cli/tui/form.rs index 37f2e4e3..c86f6161 100644 --- a/src-tauri/src/cli/tui/form.rs +++ b/src-tauri/src/cli/tui/form.rs @@ -281,6 +281,7 @@ pub enum ProviderAddField { // (loaded from config, used as the serialization fallback) are kept. #[allow(dead_code)] CodexModel, + CodexReviewModel, CodexAdvancedDivider, CodexPromptCacheRouting, CodexLocalRouting, @@ -606,6 +607,7 @@ pub struct ProviderAddFormState { pub codex_base_url: TextInput, pub codex_model: TextInput, + pub codex_review_model: TextInput, pub codex_wire_api: CodexWireApi, pub codex_requires_openai_auth: bool, pub codex_env_key: TextInput, diff --git a/src-tauri/src/cli/tui/form/provider_json.rs b/src-tauri/src/cli/tui/form/provider_json.rs index 8c7d2288..d2a90eb9 100644 --- a/src-tauri/src/cli/tui/form/provider_json.rs +++ b/src-tauri/src/cli/tui/form/provider_json.rs @@ -249,7 +249,7 @@ impl ProviderAddFormState { pub(crate) fn effective_codex_config_text(&self) -> String { if self.is_codex_official_provider() { - return self.effective_official_codex_config_text(); + return self.preview_codex_review_model(self.effective_official_codex_config_text()); } let fallback_model = if self.codex_model.is_blank() { @@ -266,7 +266,12 @@ impl ProviderAddFormState { } else { fallback_model }; - self.effective_custom_codex_config_text(model) + self.preview_codex_review_model(self.effective_custom_codex_config_text(model)) + } + + fn preview_codex_review_model(&self, config: String) -> String { + crate::codex_config::apply_codex_review_model(&config, Some(&self.codex_review_model.value)) + .unwrap_or(config) } pub(crate) fn effective_codex_config_text_with_common_config( @@ -286,11 +291,13 @@ impl ProviderAddFormState { ) .map_err(|err| err.to_string())?; - Ok(effective - .get("config") - .and_then(Value::as_str) - .unwrap_or_default() - .to_string()) + Ok(self.preview_codex_review_model( + effective + .get("config") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(), + )) } fn codex_config_and_model_catalog_for_save(&self) -> (String, Vec) { @@ -975,6 +982,7 @@ impl ProviderAddFormState { && !self.has_usage_script_meta() && !self.usage_query_touched && !should_write_full_url + && self.codex_review_model.value.trim().is_empty() && !provider_obj.get("meta").is_some_and(Value::is_object) { return; @@ -1103,6 +1111,10 @@ impl ProviderAddFormState { } } + if matches!(self.app_type, AppType::Codex) { + upsert_optional_trimmed(meta_obj, "codexReviewModel", &self.codex_review_model.value); + } + if matches!(self.app_type, AppType::Claude | AppType::Codex) { if should_write_full_url { meta_obj.insert("isFullUrl".to_string(), json!(true)); @@ -1185,7 +1197,12 @@ impl ProviderAddFormState { self.update_usage_script_meta(meta_obj); - if meta_obj.is_empty() { + // An omitted meta means "preserve existing" to ProviderService::update. + // Keep an explicit empty object when clearing the last review override. + let clearing_review_model = matches!(self.app_type, AppType::Codex) + && self.codex_review_model.is_blank() + && self.extra.pointer("/meta/codexReviewModel").is_some(); + if meta_obj.is_empty() && !clearing_review_model { provider_obj.remove("meta"); } } diff --git a/src-tauri/src/cli/tui/form/provider_state.rs b/src-tauri/src/cli/tui/form/provider_state.rs index ceb4923c..b4380bd1 100644 --- a/src-tauri/src/cli/tui/form/provider_state.rs +++ b/src-tauri/src/cli/tui/form/provider_state.rs @@ -205,6 +205,7 @@ impl ProviderAddFormState { codex_max_output_tokens: TextInput::new(""), codex_base_url: TextInput::new(codex_defaults.0), codex_model: TextInput::new(codex_defaults.1), + codex_review_model: TextInput::new(""), codex_wire_api: codex_defaults.2, codex_requires_openai_auth: codex_defaults.3, codex_env_key: TextInput::new("OPENAI_API_KEY"), @@ -519,6 +520,7 @@ impl ProviderAddFormState { } } AppType::Codex => { + fields.push(ProviderAddField::CodexReviewModel); if !self.is_codex_official_provider() { fields.push(ProviderAddField::CodexBaseUrl); fields.push(ProviderAddField::CodexApiKey); @@ -727,6 +729,7 @@ impl ProviderAddFormState { ProviderAddField::CodexBaseUrl => Some(&self.codex_base_url), ProviderAddField::CodexMaxOutputTokens => Some(&self.codex_max_output_tokens), ProviderAddField::CodexModel => Some(&self.codex_model), + ProviderAddField::CodexReviewModel => Some(&self.codex_review_model), ProviderAddField::CodexEnvKey => Some(&self.codex_env_key), ProviderAddField::CodexApiKey => Some(&self.codex_api_key), ProviderAddField::GeminiApiKey => Some(&self.gemini_api_key), @@ -793,6 +796,7 @@ impl ProviderAddFormState { ProviderAddField::CodexBaseUrl => Some(&mut self.codex_base_url), ProviderAddField::CodexMaxOutputTokens => Some(&mut self.codex_max_output_tokens), ProviderAddField::CodexModel => Some(&mut self.codex_model), + ProviderAddField::CodexReviewModel => Some(&mut self.codex_review_model), ProviderAddField::CodexEnvKey => Some(&mut self.codex_env_key), ProviderAddField::CodexApiKey => Some(&mut self.codex_api_key), ProviderAddField::GeminiApiKey => Some(&mut self.gemini_api_key), diff --git a/src-tauri/src/cli/tui/form/provider_state_loading.rs b/src-tauri/src/cli/tui/form/provider_state_loading.rs index d1706cc4..69647ac4 100644 --- a/src-tauri/src/cli/tui/form/provider_state_loading.rs +++ b/src-tauri/src/cli/tui/form/provider_state_loading.rs @@ -229,6 +229,8 @@ fn populate_claude_form(form: &mut ProviderAddFormState, provider: &Provider) { } fn populate_codex_form(form: &mut ProviderAddFormState, provider: &Provider) { + form.codex_review_model + .set(provider.codex_review_model().unwrap_or("")); if let Some(config) = provider .settings_config .get("config") diff --git a/src-tauri/src/cli/tui/form/provider_templates.rs b/src-tauri/src/cli/tui/form/provider_templates.rs index 7945baae..37e17d30 100644 --- a/src-tauri/src/cli/tui/form/provider_templates.rs +++ b/src-tauri/src/cli/tui/form/provider_templates.rs @@ -324,6 +324,7 @@ impl ProviderAddFormState { self.codex_api_key.set(""); self.codex_base_url.set(""); self.codex_model.set(CODEX_DEFAULT_MODEL); + self.codex_review_model.set(""); self.codex_wire_api = CodexWireApi::Responses; self.codex_requires_openai_auth = true; self.codex_env_key.set("OPENAI_API_KEY"); @@ -561,6 +562,7 @@ impl ProviderAddFormState { self.codex_api_key = defaults.codex_api_key; self.codex_chat_reasoning = defaults.codex_chat_reasoning; self.codex_prompt_cache_routing = defaults.codex_prompt_cache_routing; + self.codex_review_model = defaults.codex_review_model; self.codex_model_catalog = defaults.codex_model_catalog; self.codex_local_routing_enabled = defaults.codex_local_routing_enabled; self.codex_goal_mode = defaults.codex_goal_mode; diff --git a/src-tauri/src/cli/tui/form/tests.rs b/src-tauri/src/cli/tui/form/tests.rs index 568f33b3..edbb3bed 100644 --- a/src-tauri/src/cli/tui/form/tests.rs +++ b/src-tauri/src/cli/tui/form/tests.rs @@ -8094,3 +8094,74 @@ fn provider_add_form_pi_preserves_raw_native_settings_and_names_a_copy() { expected_copy ); } + +#[test] +fn codex_review_model_form_roundtrip_overrides_common_and_clears_to_legacy() { + let mut provider = Provider::with_id( + "review".into(), + "Review".into(), + json!({"auth": {}, "config": "model = \"main\"\nreview_model = \"legacy\"\n"}), + None, + ); + provider.meta = Some(crate::provider::ProviderMeta { + apply_common_config: Some(true), + ..Default::default() + }); + let common = "review_model = \"shared\"\n"; + for official in [false, true] { + provider.category = official.then(|| "official".into()); + let mut form = ProviderAddFormState::from_provider_with_common_snippet( + AppType::Codex, + &provider, + common, + ); + assert!(form.fields().contains(&ProviderAddField::CodexReviewModel)); + assert!(form.codex_review_model.is_blank()); + form.codex_review_model.set(" vendor-review "); + let saved: Provider = serde_json::from_value(form.to_provider_json_value()).unwrap(); + assert_eq!(saved.codex_review_model(), Some("vendor-review")); + let mut reopened = + ProviderAddFormState::from_provider_with_common_snippet(AppType::Codex, &saved, common); + let preview: toml::Value = toml::from_str( + &reopened + .effective_codex_config_text_with_common_config(common) + .unwrap(), + ) + .unwrap(); + assert_eq!(preview["review_model"].as_str(), Some("vendor-review")); + reopened.codex_review_model.set(""); + let cleared: Provider = serde_json::from_value(reopened.to_provider_json_value()).unwrap(); + assert_eq!(cleared.codex_review_model(), None); + let preview: toml::Value = toml::from_str( + &reopened + .effective_codex_config_text_with_common_config(common) + .unwrap(), + ) + .unwrap(); + assert_eq!(preview["review_model"].as_str(), Some("shared")); + } +} + +#[test] +fn codex_review_model_creates_metadata_for_existing_official_provider() { + let mut provider = Provider::with_id( + "official".into(), + "Official".into(), + json!({"auth": {}, "config": "model = 'main'\n"}), + None, + ); + provider.category = Some("official".into()); + assert!(provider.meta.is_none()); + let mut form = ProviderAddFormState::from_provider(AppType::Codex, &provider); + form.codex_review_model.set("review-model"); + let saved: Provider = serde_json::from_value(form.to_provider_json_value()).unwrap(); + assert_eq!(saved.codex_review_model(), Some("review-model")); + let mut reopened = ProviderAddFormState::from_provider(AppType::Codex, &saved); + reopened.codex_review_model.set(""); + let cleared: Provider = serde_json::from_value(reopened.to_provider_json_value()).unwrap(); + assert!( + cleared.meta.is_some(), + "explicit empty meta must clear the stored override" + ); + assert_eq!(cleared.codex_review_model(), None); +} diff --git a/src-tauri/src/cli/tui/help.rs b/src-tauri/src/cli/tui/help.rs index bb4b2cb5..129762c0 100644 --- a/src-tauri/src/cli/tui/help.rs +++ b/src-tauri/src/cli/tui/help.rs @@ -679,6 +679,13 @@ fn provider_field_help(app_type: AppType, field: ProviderAddField) -> HelpConten "Provider API key. After saving, it is written using this app's config rules. The UI shows the current value in plaintext.", ), ), + ProviderAddField::CodexReviewModel => HelpContent::new( + texts::tui_label_codex_review_model(), + help_lines( + "仅此供应商的审查模型,优先于通用配置中的 review_model。留空保留原有配置行为。模型必须受此供应商支持。保存后重新启动 Codex 生效。", + "Review model for this provider, overriding review_model in common config. Leave empty to preserve the existing configuration behavior. The model must be supported by this provider. Restart Codex after saving.", + ), + ), ProviderAddField::CodexModel => HelpContent::new( texts::model_label(), help_lines( diff --git a/src-tauri/src/cli/tui/ui/forms/provider.rs b/src-tauri/src/cli/tui/ui/forms/provider.rs index 7fd4c138..7026911e 100644 --- a/src-tauri/src/cli/tui/ui/forms/provider.rs +++ b/src-tauri/src/cli/tui/ui/forms/provider.rs @@ -1966,6 +1966,7 @@ pub(crate) fn provider_field_label_and_value( texts::tui_label_codex_max_output_tokens().to_string() } ProviderAddField::CodexModel => texts::model_label().to_string(), + ProviderAddField::CodexReviewModel => texts::tui_label_codex_review_model().to_string(), ProviderAddField::CodexPromptCacheRouting => { texts::tui_label_codex_prompt_cache_routing().to_string() } diff --git a/src-tauri/src/cli/tui/ui/tests.rs b/src-tauri/src/cli/tui/ui/tests.rs index e6938451..3dcbe847 100644 --- a/src-tauri/src/cli/tui/ui/tests.rs +++ b/src-tauri/src/cli/tui/ui/tests.rs @@ -14975,3 +14975,39 @@ fn home_usage_card_rail_falls_back_to_the_glyph_when_the_label_will_not_fit() { let wide_rail = line_at(&wide, wide_row); assert!(wide_rail.contains("⠸ Refreshing"), "{wide_rail}"); } + +#[test] +fn codex_review_model_field_renders_in_both_languages() { + let _lock = lock_env(); + for language in [Language::English, Language::Chinese] { + let _lang = use_test_language(language); + let p = Provider::with_id( + "p".into(), + "Provider".into(), + json!({"auth": {}, "config": ""}), + None, + ); + let mut form = + crate::cli::tui::form::ProviderAddFormState::from_provider(AppType::Codex, &p); + form.codex_review_model.set("vendor-review"); + form.field_idx = form + .fields() + .iter() + .position(|f| *f == ProviderAddField::CodexReviewModel) + .unwrap(); + let mut app = App::new(Some(AppType::Codex)); + app.route = Route::Providers; + app.focus = Focus::Content; + app.form = Some(FormState::ProviderAdd(form)); + let data = minimal_data(&AppType::Codex); + for width in [80, 120] { + let text = all_text(&render_with_size(&app, &data, width, 30)); + let compact = text.replace(' ', ""); + assert!( + compact.contains(&texts::tui_label_codex_review_model().replace(' ', "")), + "{text}" + ); + assert!(compact.contains("vendor-review"), "{text}"); + } + } +} diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index 06a1d71d..75f35bff 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -3389,3 +3389,80 @@ experimental_bearer_token = "sk-live" ); } } + +/// Project a cc-switch provider override into Codex's top-level config. +/// Keeping the override outside TOML prevents common-config deduplication from +/// erasing an explicit selection that happens to equal the shared value. +pub fn apply_codex_review_model(config: &str, model: Option<&str>) -> Result { + let Some(model) = model.map(str::trim).filter(|model| !model.is_empty()) else { + return Ok(config.to_string()); + }; + let mut doc = config + .parse::() + .map_err(|err| AppError::Config(format!("Invalid Codex config.toml: {err}")))?; + doc["review_model"] = toml_edit::value(model); + Ok(doc.to_string()) +} + +pub(crate) const CODEX_REVIEW_MODEL_MARKER: &str = ".cc-switch-review-model"; + +/// Undo the projection before importing live settings back into this provider. +/// An explicit override is managed by cc-switch; preserve the underlying legacy +/// value so clearing the override restores the previous behavior. +pub(crate) fn restore_codex_review_model( + settings: &mut Value, + provider: &crate::provider::Provider, +) { + if provider.codex_review_model().is_some() { + restore_codex_review_model_from_template(settings, provider); + } +} + +pub(crate) fn restore_codex_review_model_from_template( + settings: &mut Value, + provider: &crate::provider::Provider, +) { + let Some(text) = settings.get("config").and_then(Value::as_str) else { + return; + }; + let Ok(mut doc) = text.parse::() else { + return; + }; + let original = provider + .settings_config + .get("config") + .and_then(Value::as_str) + .unwrap_or(""); + let Ok(original) = original.parse::() else { + return; + }; + if let Some(value) = original.get("review_model") { + doc["review_model"] = value.clone(); + } else { + doc.as_table_mut().remove("review_model"); + } + settings["config"] = Value::String(doc.to_string()); +} + +#[cfg(test)] +mod review_model_tests { + use super::*; + + #[test] + fn review_model_override_is_top_level_escaped_and_optional() { + let original = "# keep\nmodel = \"main\"\n[model_providers.custom]\nname = \"Custom\"\n"; + assert_eq!(apply_codex_review_model(original, None).unwrap(), original); + assert_eq!( + apply_codex_review_model(original, Some(" ")).unwrap(), + original + ); + let result = apply_codex_review_model(original, Some("review\"model")).unwrap(); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + assert_eq!(parsed["review_model"].as_str(), Some("review\"model")); + assert!(parsed["model_providers"]["custom"] + .get("review_model") + .is_none()); + assert!(result.contains("# keep")); + assert!(apply_codex_review_model("[broken", Some("review")).is_err()); + } +} diff --git a/src-tauri/src/provider.rs b/src-tauri/src/provider.rs index dcfb496f..88b54e26 100644 --- a/src-tauri/src/provider.rs +++ b/src-tauri/src/provider.rs @@ -239,6 +239,15 @@ impl Provider { .unwrap_or(false) } + pub fn codex_review_model(&self) -> Option<&str> { + self.meta + .as_ref()? + .codex_review_model + .as_deref() + .map(str::trim) + .filter(|model| !model.is_empty()) + } + pub fn codex_fast_mode_enabled(&self) -> bool { self.meta .as_ref() @@ -510,6 +519,9 @@ pub struct ProviderMeta { /// Codex 官方供应商标记(官方无需填写 API Key,使用 codex login 凭证) #[serde(rename = "codexOfficial", skip_serializing_if = "Option::is_none")] pub codex_official: Option, + /// Provider-local review model override. None preserves the legacy config. + #[serde(rename = "codexReviewModel", skip_serializing_if = "Option::is_none")] + pub codex_review_model: Option, /// 自定义端点列表(按 URL 去重存储) #[serde(default, skip_serializing_if = "HashMap::is_empty")] pub custom_endpoints: HashMap, diff --git a/src-tauri/src/proxy/providers/codex.rs b/src-tauri/src/proxy/providers/codex.rs index 25e21b04..024da4b7 100644 --- a/src-tauri/src/proxy/providers/codex.rs +++ b/src-tauri/src/proxy/providers/codex.rs @@ -247,7 +247,9 @@ pub fn apply_codex_upstream_model(provider: &Provider, body: &mut JsonValue) -> .map(str::trim) .filter(|model| !model.is_empty()) { - if catalog_model_ids.contains(request_model) { + if catalog_model_ids.contains(request_model) + || provider.codex_review_model() == Some(request_model) + { return Some(request_model.to_string()); } } @@ -1040,6 +1042,25 @@ wire_api = "chat" assert_eq!(body["model"], "deepseek-reasoner"); } + #[test] + fn test_apply_codex_model_preserves_provider_review_model_without_catalog() { + let mut provider = create_provider(json!({ + "base_url": "https://api.example.com/v1", + "api_format": "openai_chat", + "model": "main-model" + })); + provider.meta = Some(crate::provider::ProviderMeta { + codex_review_model: Some("review-model".into()), + ..Default::default() + }); + let mut body = json!({"model": "review-model", "input": "hello"}); + assert_eq!( + apply_codex_chat_upstream_model(&provider, &mut body).as_deref(), + Some("review-model") + ); + assert_eq!(body["model"], "review-model"); + } + #[test] fn test_resolve_codex_chat_reasoning_infers_deepseek_effort_support() { let provider = create_provider(json!({ diff --git a/src-tauri/src/services/config.rs b/src-tauri/src/services/config.rs index ece91590..62636da0 100644 --- a/src-tauri/src/services/config.rs +++ b/src-tauri/src/services/config.rs @@ -352,6 +352,7 @@ impl ConfigService { "auth": auth_after, "config": cfg_text_after, }); + crate::codex_config::restore_codex_review_model(&mut restored, provider); let restore_provider_token = crate::codex_config::should_restore_codex_provider_token_for_backfill( ProviderService::codex_live_write_category(provider), diff --git a/src-tauri/src/services/provider/codex.rs b/src-tauri/src/services/provider/codex.rs index d55634d1..ef155345 100644 --- a/src-tauri/src/services/provider/codex.rs +++ b/src-tauri/src/services/provider/codex.rs @@ -76,6 +76,17 @@ impl ProviderService { raw_settings.insert("auth".to_string(), auth); raw_settings.insert("config".to_string(), Value::String(cfg_text_for_storage)); let mut settings_to_store = Value::Object(raw_settings); + // The override may have been cleared while this launch was running. + // Its projected value must never become a legacy config on exit. + if codex_home + .join(crate::codex_config::CODEX_REVIEW_MODEL_MARKER) + .exists() + { + crate::codex_config::restore_codex_review_model_from_template( + &mut settings_to_store, + &provider, + ); + } if Self::codex_live_write_category(&provider) == Some("official") { crate::codex_config::strip_codex_unified_session_bucket_from_settings( &mut settings_to_store, @@ -453,7 +464,15 @@ impl ProviderService { if config_path.exists() { let text = std::fs::read_to_string(&config_path).map_err(|e| AppError::io(&config_path, e))?; - Self::maybe_update_codex_common_config_snippet(config, &text)?; + let mut extraction_settings = serde_json::json!({"config": &text}); + crate::codex_config::restore_codex_review_model( + &mut extraction_settings, + ¤t_provider, + ); + Self::maybe_update_codex_common_config_snippet( + config, + extraction_settings["config"].as_str().unwrap_or(&text), + )?; let capture_auth = if is_official { auth.clone() @@ -478,6 +497,10 @@ impl ProviderService { &mut settings_for_storage, )?; } + crate::codex_config::restore_codex_review_model( + &mut settings_for_storage, + ¤t_provider, + ); snapshot_provider.settings_config = settings_for_storage; snapshot_provider = Self::migrate_provider_snapshot_for_storage( &AppType::Codex, diff --git a/src-tauri/src/services/provider/common_config.rs b/src-tauri/src/services/provider/common_config.rs index d8b3733a..806da668 100644 --- a/src-tauri/src/services/provider/common_config.rs +++ b/src-tauri/src/services/provider/common_config.rs @@ -766,6 +766,17 @@ pub(super) fn build_effective_settings_with_common_config( apply_codex_oauth_claude_context_defaults(&mut effective_settings, provider); } + if matches!(app_type, AppType::Codex) { + let config = effective_settings + .get("config") + .and_then(Value::as_str) + .unwrap_or(""); + let config = + crate::codex_config::apply_codex_review_model(config, provider.codex_review_model())?; + if let Some(settings) = effective_settings.as_object_mut() { + settings.insert("config".into(), Value::String(config)); + } + } Ok(effective_settings) } diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index e1180832..8f577140 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -1168,8 +1168,6 @@ impl ProviderService { AppType::Codex => { let auth_path = get_codex_auth_path(); let cfg_text = crate::codex_config::read_and_validate_codex_config_text()?; - let common_snippet_extracted = - Self::extract_codex_common_config_from_config_toml(&cfg_text)?; let cfg_text_for_storage = Self::strip_codex_mcp_servers_from_snapshot_config(&cfg_text)?; @@ -1199,6 +1197,14 @@ impl ProviderService { provider.settings_config.get("auth").cloned() }; + let mut extraction_settings = serde_json::json!({"config": &cfg_text}); + crate::codex_config::restore_codex_review_model( + &mut extraction_settings, + &provider, + ); + let common_snippet_extracted = Self::extract_codex_common_config_from_config_toml( + extraction_settings["config"].as_str().unwrap_or(&cfg_text), + )?; let effective_common_snippet = if common_snippet_for_strip .as_deref() .unwrap_or_default() @@ -1258,6 +1264,10 @@ impl ProviderService { obj.insert("auth".to_string(), sanitized); } } + crate::codex_config::restore_codex_review_model( + &mut settings_for_storage, + &provider, + ); let mut snapshot_provider = provider.clone(); snapshot_provider.settings_config = settings_for_storage; @@ -1667,6 +1677,10 @@ impl ProviderService { settings_config: Value, common_config_snippet: Option<&str>, ) -> Result { + let mut settings_config = settings_config; + if matches!(app_type, AppType::Codex) { + crate::codex_config::restore_codex_review_model(&mut settings_config, provider); + } let mut snapshot_provider = provider.clone(); snapshot_provider.settings_config = settings_config; Self::normalize_provider_for_storage( diff --git a/src-tauri/src/services/provider/tests.rs b/src-tauri/src/services/provider/tests.rs index a6f3fbd7..a149e7fb 100644 --- a/src-tauri/src/services/provider/tests.rs +++ b/src-tauri/src/services/provider/tests.rs @@ -7660,3 +7660,113 @@ fn delete_rejects_last_failover_queue_provider_while_active() { .expect("read queued provider") .is_some()); } + +#[test] +fn codex_review_model_backfill_preserves_legacy_and_common_extraction() { + let provider = Provider::with_id( + "review".into(), + "Review".into(), + json!({"auth": {}, "config": "review_model = \"legacy\"\n"}), + None, + ); + let mut provider = provider; + provider.meta = Some(crate::provider::ProviderMeta { + codex_review_model: Some("override".into()), + ..Default::default() + }); + let restored = ProviderService::normalize_settings_config_for_storage( + &AppType::Codex, + &provider, + json!({"auth": {}, "config": "review_model = \"override\"\nmodel = \"main\"\n"}), + None, + ) + .unwrap(); + let parsed: toml::Value = toml::from_str(restored["config"].as_str().unwrap()).unwrap(); + assert_eq!(parsed["review_model"].as_str(), Some("legacy")); + assert_eq!(parsed["model"].as_str(), Some("main")); + let common = ProviderService::extract_codex_common_config_from_config_toml( + "review_model = \"local\"\nmodel_reasoning_effort = \"high\"\n", + ) + .unwrap(); + let parsed: toml::Value = toml::from_str(&common).unwrap(); + assert_eq!(parsed["review_model"].as_str(), Some("local")); + assert_eq!(parsed["model_reasoning_effort"].as_str(), Some("high")); +} + +#[test] +fn codex_review_model_cleared_during_temp_launch_is_not_restored_on_exit() { + let home = TempDir::new().unwrap(); + let _env = EnvGuard::isolated(home.path()); + for baseline in ["", "review_model = 'legacy'\n"] { + let mut config = MultiAppConfig::default(); + let provider = Provider::with_id( + "review".into(), + "Review".into(), + codex_settings(baseline), + None, + ); + config + .get_manager_mut(&AppType::Codex) + .unwrap() + .providers + .insert("review".into(), provider); + let state = state_from_config(config); + // Simulate a launch created with an override before it was cleared. + let launch = TempDir::new().unwrap(); + std::fs::write( + launch + .path() + .join(crate::codex_config::CODEX_REVIEW_MODEL_MARKER), + "1", + ) + .unwrap(); + std::fs::write( + launch.path().join("config.toml"), + "review_model = 'old-override'\nmodel = 'new-main'\n", + ) + .unwrap(); + ProviderService::capture_codex_temp_launch_snapshot(&state, "review", launch.path()) + .unwrap(); + let providers = ProviderService::list(&state, AppType::Codex).unwrap(); + let saved = &providers["review"]; + assert!(saved.codex_review_model().is_none()); + let actual: toml::Value = + toml::from_str(saved.settings_config["config"].as_str().unwrap()).unwrap(); + let expected: toml::Value = toml::from_str(baseline).unwrap(); + assert_eq!(actual.get("review_model"), expected.get("review_model")); + assert_eq!(actual["model"].as_str(), Some("new-main")); + } +} + +#[cfg(feature = "cli")] +#[test] +fn codex_review_model_official_form_clear_persists_through_service_update() { + let home = TempDir::new().unwrap(); + let _env = EnvGuard::isolated(home.path()); + let mut config = MultiAppConfig::default(); + let mut provider = Provider::with_id( + "official".into(), + "Official".into(), + codex_settings(""), + None, + ); + provider.category = Some("official".into()); + config + .get_manager_mut(&AppType::Codex) + .unwrap() + .providers + .insert("official".into(), provider.clone()); + let state = state_from_config(config); + for review in ["review-model", ""] { + let mut form = + crate::cli::tui::ProviderAddFormState::from_provider(AppType::Codex, &provider); + form.codex_review_model.set(review); + let edited: Provider = serde_json::from_value(form.to_provider_json_value()).unwrap(); + ProviderService::update(&state, AppType::Codex, edited).unwrap(); + provider = state.db.get_all_providers("codex").unwrap()["official"].clone(); + assert_eq!( + provider.codex_review_model(), + (!review.is_empty()).then_some(review) + ); + } +} diff --git a/src-tauri/tests/provider_service.rs b/src-tauri/tests/provider_service.rs index dee26abe..4cc2a3bb 100644 --- a/src-tauri/tests/provider_service.rs +++ b/src-tauri/tests/provider_service.rs @@ -5134,3 +5134,101 @@ fn provider_service_sync_current_to_live_keeps_existing_prompt_file_without_acti "unmanaged prompt" ); } + +#[test] +fn codex_review_model_is_independent_across_switches_and_clearing() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let _home = ensure_test_home(); + write_codex_live_atomic(&json!({}), Some("")).unwrap(); + let mut config = MultiAppConfig::default(); + config.common_config_snippets.codex = Some("review_model = \"shared-review\"\n".into()); + for (id, review) in [ + ("a", Some("a-review")), + ("b", Some("shared-review")), + ("c", None), + ] { + let mut provider = codex_provider(id, id, "test-key", "custom", "https://example.test/v1"); + provider.meta = Some(ProviderMeta { + apply_common_config: Some(true), + codex_review_model: review.map(str::to_string), + ..Default::default() + }); + config + .get_manager_mut(&AppType::Codex) + .unwrap() + .providers + .insert(id.into(), provider); + } + let state = state_from_config(config); + for (id, expected) in [ + ("a", "a-review"), + ("b", "shared-review"), + ("a", "a-review"), + ("c", "shared-review"), + ("b", "shared-review"), + ] { + ProviderService::switch(&state, AppType::Codex, id).unwrap(); + let live: toml::Value = toml::from_str( + &std::fs::read_to_string(cc_switch_lib::get_codex_config_path()).unwrap(), + ) + .unwrap(); + assert_eq!(live["review_model"].as_str(), Some(expected)); + } + // Equal shared/explicit values must survive storage deduplication. + assert_eq!( + state.db.get_all_providers("codex").unwrap()["b"].codex_review_model(), + Some("shared-review") + ); + let mut a = state.db.get_all_providers("codex").unwrap()["a"].clone(); + a.meta.as_mut().unwrap().codex_review_model = None; + ProviderService::update(&state, AppType::Codex, a).unwrap(); + ProviderService::switch(&state, AppType::Codex, "a").unwrap(); + let live: toml::Value = + toml::from_str(&std::fs::read_to_string(cc_switch_lib::get_codex_config_path()).unwrap()) + .unwrap(); + assert_eq!(live["review_model"].as_str(), Some("shared-review")); + // Neither live projections nor metadata may leak to an unconfigured provider. + let c = state.db.get_all_providers("codex").unwrap()["c"].clone(); + assert!(c.codex_review_model().is_none()); + assert!(!c.settings_config["config"] + .as_str() + .unwrap() + .contains("a-review")); +} + +#[test] +fn codex_review_model_clears_after_backfill_without_common_config() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let _home = ensure_test_home(); + write_codex_live_atomic(&json!({}), Some("")).unwrap(); + let mut config = MultiAppConfig::default(); + for id in ["a", "b"] { + let mut p = codex_provider(id, id, "test-key", "custom", "https://example.test/v1"); + p.meta = Some(ProviderMeta { + codex_review_model: Some(format!("{id}-review")), + ..Default::default() + }); + config + .get_manager_mut(&AppType::Codex) + .unwrap() + .providers + .insert(id.into(), p); + } + let state = state_from_config(config); + for id in ["a", "b", "a", "b"] { + ProviderService::switch(&state, AppType::Codex, id).unwrap(); + } + let mut a = state.db.get_all_providers("codex").unwrap()["a"].clone(); + let stored: toml::Value = + toml::from_str(a.settings_config["config"].as_str().unwrap()).unwrap(); + assert!(stored.get("review_model").is_none()); + a.meta.as_mut().unwrap().codex_review_model = None; + ProviderService::update(&state, AppType::Codex, a).unwrap(); + ProviderService::switch(&state, AppType::Codex, "a").unwrap(); + let live: toml::Value = + toml::from_str(&std::fs::read_to_string(cc_switch_lib::get_codex_config_path()).unwrap()) + .unwrap(); + assert!(live.get("review_model").is_none()); +}