From eef12baec3380f07b9262ab1c0b5975e3ee20168 Mon Sep 17 00:00:00 2001 From: kingbo Date: Sat, 25 Apr 2026 09:54:07 +0800 Subject: [PATCH] fix: preserve unmanaged Claude settings on switch --- src-tauri/src/services/provider/claude.rs | 127 +++++++- src-tauri/src/services/provider/common.rs | 28 -- src-tauri/src/services/provider/mod.rs | 37 ++- src-tauri/src/services/provider/tests.rs | 374 ++++++++++++++++++++++ 4 files changed, 529 insertions(+), 37 deletions(-) diff --git a/src-tauri/src/services/provider/claude.rs b/src-tauri/src/services/provider/claude.rs index b2eb7875..28f83b69 100644 --- a/src-tauri/src/services/provider/claude.rs +++ b/src-tauri/src/services/provider/claude.rs @@ -1,6 +1,19 @@ +use super::common::merge_json_values; use super::*; impl ProviderService { + const CLAUDE_PROVIDER_ENV_KEYS: &'static [&'static str] = &[ + "ANTHROPIC_AUTH_TOKEN", + "ANTHROPIC_API_KEY", + "ANTHROPIC_BASE_URL", + "ANTHROPIC_MODEL", + "ANTHROPIC_REASONING_MODEL", + "ANTHROPIC_SMALL_FAST_MODEL", + "ANTHROPIC_DEFAULT_HAIKU_MODEL", + "ANTHROPIC_DEFAULT_SONNET_MODEL", + "ANTHROPIC_DEFAULT_OPUS_MODEL", + ]; + pub(super) fn parse_common_claude_config_snippet(snippet: &str) -> Result { let value: Value = serde_json::from_str(snippet).map_err(|e| { AppError::localized( @@ -119,6 +132,104 @@ impl ProviderService { ) } + fn strip_claude_provider_owned_live_keys(settings: &mut Value) { + let Some(env) = settings.get_mut("env").and_then(Value::as_object_mut) else { + return; + }; + + for key in Self::CLAUDE_PROVIDER_ENV_KEYS { + env.remove(*key); + } + } + + pub(super) fn merge_claude_provider_owned_values_from_live(snapshot: &mut Value, live: &Value) { + let Some(live_env) = live.get("env").and_then(Value::as_object) else { + return; + }; + + if !snapshot.is_object() { + *snapshot = json!({}); + } + + let snapshot_obj = snapshot + .as_object_mut() + .expect("snapshot should be object after normalization"); + let env_value = snapshot_obj + .entry("env".to_string()) + .or_insert_with(|| json!({})); + if !env_value.is_object() { + *env_value = json!({}); + } + let snapshot_env = env_value + .as_object_mut() + .expect("env should be object after normalization"); + + for key in Self::CLAUDE_PROVIDER_ENV_KEYS { + snapshot_env.remove(*key); + if let Some(value) = live_env.get(*key) { + snapshot_env.insert((*key).to_string(), value.clone()); + } + } + } + + pub(super) fn build_claude_live_snapshot_for_write( + provider: &Provider, + common_config_snippet: Option<&str>, + previous_common_config_snippet: Option<&str>, + apply_common_config: bool, + existing_live_settings: Option, + ) -> Result { + let apply_common_config = Self::resolve_live_apply_common_config( + &AppType::Claude, + provider, + common_config_snippet, + apply_common_config, + ); + + let mut live_base = existing_live_settings.unwrap_or_else(|| json!({})); + if !live_base.is_object() { + live_base = json!({}); + } + + Self::strip_claude_provider_owned_live_keys(&mut live_base); + + if let Some(snippet) = previous_common_config_snippet.map(str::trim) { + if !snippet.is_empty() { + match common_config::remove_common_config_from_settings( + &AppType::Claude, + &live_base, + snippet, + ) { + Ok(settings) => live_base = settings, + Err(err) + if Self::should_skip_common_config_migration_error( + &AppType::Claude, + &err, + ) => + { + log::warn!( + "skip stripping invalid stored Claude common config snippet from live base: {err}" + ); + live_base = json!({}); + } + Err(err) => return Err(err), + } + } + } + + let mut provider_content = common_config::build_effective_settings_with_common_config( + &AppType::Claude, + provider, + common_config_snippet, + apply_common_config, + )?; + let _ = Self::normalize_claude_models_in_value(&mut provider_content); + merge_json_values(&mut live_base, &provider_content); + let _ = Self::normalize_claude_models_in_value(&mut live_base); + + Ok(live_base) + } + pub(super) fn prepare_switch_claude( config: &mut MultiAppConfig, provider_id: &str, @@ -180,7 +291,10 @@ impl ProviderService { ); if let Some(manager) = config.get_manager_mut(&AppType::Claude) { if let Some(current) = manager.providers.get_mut(current_id) { - current.settings_config = live; + Self::merge_claude_provider_owned_values_from_live( + &mut current.settings_config, + &live, + ); } } @@ -214,6 +328,7 @@ impl ProviderService { pub(super) fn write_claude_live( provider: &Provider, common_config_snippet: Option<&str>, + previous_common_config_snippet: Option<&str>, apply_common_config: bool, ) -> Result<(), AppError> { if !crate::sync_policy::should_sync_live(&AppType::Claude) { @@ -221,11 +336,17 @@ impl ProviderService { } let settings_path = get_claude_settings_path(); - let content_to_write = Self::build_effective_live_snapshot( - &AppType::Claude, + let existing_live_settings = if settings_path.exists() { + Some(read_json_file::(&settings_path)?) + } else { + None + }; + let content_to_write = Self::build_claude_live_snapshot_for_write( provider, common_config_snippet, + previous_common_config_snippet, apply_common_config, + existing_live_settings, )?; write_json_file(&settings_path, &content_to_write)?; diff --git a/src-tauri/src/services/provider/common.rs b/src-tauri/src/services/provider/common.rs index c26b894a..fbcad7b3 100644 --- a/src-tauri/src/services/provider/common.rs +++ b/src-tauri/src/services/provider/common.rs @@ -160,31 +160,3 @@ pub(super) fn merge_json_values(base: &mut Value, overlay: &Value) { } } } - -pub(super) fn strip_common_values(target: &mut Value, common: &Value) { - match (target, common) { - (Value::Object(target_map), Value::Object(common_map)) => { - for (key, common_value) in common_map { - let should_remove = match target_map.get_mut(key) { - Some(target_value) => match target_value { - Value::Object(_) if matches!(common_value, Value::Object(_)) => { - strip_common_values(target_value, common_value); - target_value.as_object().is_some_and(|m| m.is_empty()) - } - _ => target_value == common_value, - }, - None => false, - }; - - if should_remove { - target_map.remove(key); - } - } - } - (target_value, common_value) => { - if target_value == common_value { - *target_value = Value::Null; - } - } - } -} diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index 963eb4c5..b3feed43 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -69,6 +69,7 @@ struct PostCommitAction { sync_mcp: bool, refresh_snapshot: bool, common_config_snippet: Option, + previous_common_config_snippet: Option, takeover_active: bool, } @@ -135,7 +136,13 @@ impl ProviderService { }; for provider in &providers { - Self::write_live_snapshot(&AppType::OpenClaw, provider, snippet.as_deref(), true)?; + Self::write_live_snapshot( + &AppType::OpenClaw, + provider, + snippet.as_deref(), + None, + true, + )?; } Ok(()) @@ -389,6 +396,7 @@ impl ProviderService { &action.app_type, &action.provider, action.common_config_snippet.as_deref(), + action.previous_common_config_snippet.as_deref(), apply_common_config, )?; } @@ -456,7 +464,10 @@ impl ProviderService { let mut guard = state.config.write().map_err(AppError::from)?; if let Some(manager) = guard.get_manager_mut(app_type) { if let Some(target) = manager.providers.get_mut(provider_id) { - target.settings_config = live_after; + Self::merge_claude_provider_owned_values_from_live( + &mut target.settings_config, + &live_after, + ); } } } @@ -729,6 +740,7 @@ impl ProviderService { config: &MultiAppConfig, app_type: &AppType, current_provider_id: Option<&str>, + previous_common_config_snippet: Option, takeover_active: bool, ) -> Result, AppError> { if app_type.is_additive_mode() { @@ -743,6 +755,7 @@ impl ProviderService { config, app_type, ¤t_provider_id, + previous_common_config_snippet, takeover_active, ) } @@ -751,6 +764,7 @@ impl ProviderService { config: &MultiAppConfig, app_type: &AppType, current_provider_id: &str, + previous_common_config_snippet: Option, takeover_active: bool, ) -> Result, AppError> { let provider = config @@ -768,6 +782,7 @@ impl ProviderService { sync_mcp: matches!(app_type, AppType::Codex) && !takeover_active, refresh_snapshot: false, common_config_snippet: config.common_config_snippets.get(app_type).cloned(), + previous_common_config_snippet, takeover_active, })) } @@ -1026,6 +1041,7 @@ impl ProviderService { config, &app_type_clone, effective_current_provider.as_deref(), + old_snippet, takeover_active, )?; Ok(((), action)) @@ -1134,6 +1150,7 @@ impl ProviderService { sync_mcp: matches!(&app_type_clone, AppType::Codex), refresh_snapshot: false, common_config_snippet, + previous_common_config_snippet: None, takeover_active: false, }) } else { @@ -1247,6 +1264,7 @@ impl ProviderService { sync_mcp: matches!(&app_type_clone, AppType::Codex), refresh_snapshot: false, common_config_snippet, + previous_common_config_snippet: None, takeover_active: false, }) } else { @@ -1672,7 +1690,8 @@ impl ProviderService { continue; } - if let Err(e) = Self::write_live_snapshot(app_type, provider, snippet.as_deref(), true) + if let Err(e) = + Self::write_live_snapshot(app_type, provider, snippet.as_deref(), None, true) { log::warn!("sync_current_to_live: 写入 {app_type} live 配置失败: {e}"); } @@ -1774,6 +1793,7 @@ impl ProviderService { .common_config_snippets .get(&app_type_clone) .cloned(), + previous_common_config_snippet: None, takeover_active: false, }; @@ -1808,6 +1828,7 @@ impl ProviderService { sync_mcp: true, // v3.7.0: 所有应用切换时都同步 MCP，防止配置丢失 refresh_snapshot: true, common_config_snippet: config.common_config_snippets.get(&app_type_clone).cloned(), + previous_common_config_snippet: None, takeover_active: false, }; @@ -1825,6 +1846,7 @@ impl ProviderService { app_type: &AppType, provider: &Provider, common_config_snippet: Option<&str>, + previous_common_config_snippet: Option<&str>, apply_common_config: bool, ) -> Result<(), AppError> { let apply_common_config = Self::resolve_live_apply_common_config( @@ -1838,9 +1860,12 @@ impl ProviderService { AppType::Codex => { Self::write_codex_live(provider, common_config_snippet, apply_common_config) } - AppType::Claude => { - Self::write_claude_live(provider, common_config_snippet, apply_common_config) - } + AppType::Claude => Self::write_claude_live( + provider, + common_config_snippet, + previous_common_config_snippet, + apply_common_config, + ), AppType::Gemini => Self::write_gemini_live( provider, if apply_common_config { diff --git a/src-tauri/src/services/provider/tests.rs b/src-tauri/src/services/provider/tests.rs index 88857d5a..d3a94865 100644 --- a/src-tauri/src/services/provider/tests.rs +++ b/src-tauri/src/services/provider/tests.rs @@ -2222,6 +2222,380 @@ fn common_config_snippet_is_not_persisted_into_provider_snapshot_on_switch() { ); } +#[test] +#[serial] +fn clear_common_config_removes_old_values_and_preserves_unmanaged_live_settings() { + let temp_home = TempDir::new().expect("create temp home"); + let _env = EnvGuard::set_home(temp_home.path()); + std::fs::create_dir_all(crate::config::get_claude_config_dir()) + .expect("create ~/.claude (initialized)"); + + let mut config = MultiAppConfig::default(); + config.ensure_app(&AppType::Claude); + config.common_config_snippets.claude = Some( + r#"{"env":{"CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC":1},"includeCoAuthoredBy":false}"# + .to_string(), + ); + { + let manager = config + .get_manager_mut(&AppType::Claude) + .expect("claude manager"); + manager.current = "p1".to_string(); + manager.providers.insert( + "p1".to_string(), + Provider::with_id( + "p1".to_string(), + "First".to_string(), + json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1", + "ANTHROPIC_BASE_URL": "https://claude.one" + } + }), + None, + ), + ); + } + + write_json_file( + &get_claude_settings_path(), + &json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1", + "ANTHROPIC_BASE_URL": "https://claude.one", + "CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC": 1 + }, + "includeCoAuthoredBy": false, + "statusLine": { + "type": "command", + "command": "~/.claude/statusline.sh" + } + }), + ) + .expect("seed live settings"); + + let state = state_from_config(config); + ProviderService::clear_common_config_snippet(&state, AppType::Claude) + .expect("clear common snippet"); + + let live: Value = read_json_file(&get_claude_settings_path()).expect("read live settings"); + let env = live + .get("env") + .and_then(Value::as_object) + .expect("live env should be object"); + assert_eq!( + env.get("ANTHROPIC_AUTH_TOKEN").and_then(Value::as_str), + Some("token1") + ); + assert_eq!( + env.get("ANTHROPIC_BASE_URL").and_then(Value::as_str), + Some("https://claude.one") + ); + assert!( + !env.contains_key("CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC"), + "clearing common config should remove old common env values" + ); + assert!( + live.get("includeCoAuthoredBy").is_none(), + "clearing common config should remove old common top-level values" + ); + assert_eq!( + live["statusLine"]["command"], + json!("~/.claude/statusline.sh"), + "unmanaged live settings should remain after clearing common config" + ); +} + +#[test] +#[serial] +fn switch_claude_preserves_unmanaged_live_settings() { + let temp_home = TempDir::new().expect("create temp home"); + let _env = EnvGuard::set_home(temp_home.path()); + std::fs::create_dir_all(crate::config::get_claude_config_dir()) + .expect("create ~/.claude (initialized)"); + + let mut config = MultiAppConfig::default(); + config.ensure_app(&AppType::Claude); + { + let manager = config + .get_manager_mut(&AppType::Claude) + .expect("claude manager"); + manager.current = "p1".to_string(); + manager.providers.insert( + "p1".to_string(), + Provider::with_id( + "p1".to_string(), + "First".to_string(), + json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1", + "ANTHROPIC_BASE_URL": "https://claude.one" + } + }), + None, + ), + ); + manager.providers.insert( + "p2".to_string(), + Provider::with_id( + "p2".to_string(), + "Second".to_string(), + json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token2", + "ANTHROPIC_BASE_URL": "https://claude.two" + } + }), + None, + ), + ); + } + + write_json_file( + &get_claude_settings_path(), + &json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1", + "ANTHROPIC_BASE_URL": "https://claude.one" + }, + "statusLine": { + "type": "command", + "command": "~/.claude/statusline.sh" + }, + "permissions": { + "allow": ["Bash(ls)"] + } + }), + ) + .expect("seed live settings"); + + let state = state_from_config(config); + ProviderService::switch(&state, AppType::Claude, "p2").expect("switch to p2"); + + let live: Value = read_json_file(&get_claude_settings_path()).expect("read live settings"); + assert_eq!( + live["env"]["ANTHROPIC_AUTH_TOKEN"], + json!("token2"), + "selected provider token should win" + ); + assert_eq!( + live["env"]["ANTHROPIC_BASE_URL"], + json!("https://claude.two"), + "selected provider base URL should win" + ); + assert_eq!( + live["statusLine"]["command"], + json!("~/.claude/statusline.sh"), + "unmanaged top-level settings should be preserved" + ); + assert_eq!( + live["permissions"]["allow"], + json!(["Bash(ls)"]), + "unmanaged nested settings should be preserved" + ); + + let cfg = state.config.read().expect("read config"); + let manager = cfg.get_manager(&AppType::Claude).expect("claude manager"); + let p2_after = manager.providers.get("p2").expect("p2 exists"); + assert!( + p2_after.settings_config.get("statusLine").is_none(), + "refreshing the selected provider snapshot should not store unmanaged top-level live settings" + ); + assert!( + p2_after.settings_config.get("permissions").is_none(), + "refreshing the selected provider snapshot should not store unmanaged nested live settings" + ); +} + +#[test] +#[serial] +fn switch_claude_removes_stale_provider_owned_live_keys() { + let temp_home = TempDir::new().expect("create temp home"); + let _env = EnvGuard::set_home(temp_home.path()); + std::fs::create_dir_all(crate::config::get_claude_config_dir()) + .expect("create ~/.claude (initialized)"); + + let mut config = MultiAppConfig::default(); + config.ensure_app(&AppType::Claude); + { + let manager = config + .get_manager_mut(&AppType::Claude) + .expect("claude manager"); + manager.current = "p1".to_string(); + manager.providers.insert( + "p1".to_string(), + Provider::with_id( + "p1".to_string(), + "First".to_string(), + json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1", + "ANTHROPIC_BASE_URL": "https://claude.one", + "ANTHROPIC_API_KEY": "stale-api-key", + "ANTHROPIC_SMALL_FAST_MODEL": "legacy-small" + } + }), + None, + ), + ); + manager.providers.insert( + "p2".to_string(), + Provider::with_id( + "p2".to_string(), + "Second".to_string(), + json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token2", + "ANTHROPIC_BASE_URL": "https://claude.two" + } + }), + None, + ), + ); + } + + write_json_file( + &get_claude_settings_path(), + &json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1", + "ANTHROPIC_BASE_URL": "https://claude.one", + "ANTHROPIC_API_KEY": "stale-api-key", + "ANTHROPIC_SMALL_FAST_MODEL": "legacy-small" + }, + "statusLine": { + "type": "command", + "command": "~/.claude/statusline.sh" + } + }), + ) + .expect("seed live settings"); + + let state = state_from_config(config); + ProviderService::switch(&state, AppType::Claude, "p2").expect("switch to p2"); + + let live: Value = read_json_file(&get_claude_settings_path()).expect("read live settings"); + let env = live + .get("env") + .and_then(Value::as_object) + .expect("live env should be object"); + assert_eq!( + env.get("ANTHROPIC_AUTH_TOKEN").and_then(Value::as_str), + Some("token2") + ); + assert_eq!( + env.get("ANTHROPIC_BASE_URL").and_then(Value::as_str), + Some("https://claude.two") + ); + assert!( + !env.contains_key("ANTHROPIC_API_KEY"), + "stale provider-owned live keys should not be preserved" + ); + assert!( + !env.contains_key("ANTHROPIC_SMALL_FAST_MODEL"), + "legacy provider-owned model keys should not be preserved" + ); + assert_eq!( + live["statusLine"]["command"], + json!("~/.claude/statusline.sh"), + "unmanaged settings should still be preserved" + ); +} + +#[test] +#[serial] +fn backfill_claude_does_not_store_unmanaged_live_settings() { + let temp_home = TempDir::new().expect("create temp home"); + let _env = EnvGuard::set_home(temp_home.path()); + std::fs::create_dir_all(crate::config::get_claude_config_dir()) + .expect("create ~/.claude (initialized)"); + + let mut config = MultiAppConfig::default(); + config.ensure_app(&AppType::Claude); + { + let manager = config + .get_manager_mut(&AppType::Claude) + .expect("claude manager"); + manager.current = "p1".to_string(); + manager.providers.insert( + "p1".to_string(), + Provider::with_id( + "p1".to_string(), + "First".to_string(), + json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1", + "ANTHROPIC_BASE_URL": "https://claude.one" + } + }), + None, + ), + ); + manager.providers.insert( + "p2".to_string(), + Provider::with_id( + "p2".to_string(), + "Second".to_string(), + json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token2", + "ANTHROPIC_BASE_URL": "https://claude.two" + } + }), + None, + ), + ); + } + + write_json_file( + &get_claude_settings_path(), + &json!({ + "env": { + "ANTHROPIC_AUTH_TOKEN": "token1-updated", + "ANTHROPIC_BASE_URL": "https://claude.one.updated" + }, + "statusLine": { + "type": "command", + "command": "~/.claude/statusline.sh" + }, + "permissions": { + "allow": ["Bash(ls)"] + } + }), + ) + .expect("seed live settings"); + + let state = state_from_config(config); + ProviderService::switch(&state, AppType::Claude, "p2").expect("switch to p2"); + + let cfg = state.config.read().expect("read config"); + let manager = cfg.get_manager(&AppType::Claude).expect("claude manager"); + let p1_after = manager.providers.get("p1").expect("p1 exists"); + let env = p1_after + .settings_config + .get("env") + .and_then(Value::as_object) + .expect("provider env should be object"); + assert_eq!( + env.get("ANTHROPIC_AUTH_TOKEN").and_then(Value::as_str), + Some("token1-updated"), + "backfill should still update provider-owned values" + ); + assert_eq!( + env.get("ANTHROPIC_BASE_URL").and_then(Value::as_str), + Some("https://claude.one.updated"), + "backfill should still update provider-owned values" + ); + assert!( + p1_after.settings_config.get("statusLine").is_none(), + "backfill should not store unmanaged top-level live settings" + ); + assert!( + p1_after.settings_config.get("permissions").is_none(), + "backfill should not store unmanaged nested live settings" + ); +} + #[test] #[serial] fn updating_common_snippet_removes_stale_fields_from_other_claude_provider_snapshots() {