fix: prevent common config loss during proxy takeover and stabilize snippet lifecycle

- Make sync_current_provider_for_app takeover-aware: update restore
  backup instead of overwriting live config when proxy is active
- Introduce explicit "cleared" flag for common config snippets to
  prevent auto-extraction from resurrecting user-cleared snippets
- Reorder startup: extract snippets from clean live files before
  restoring proxy takeover state
- Add one-time migration flag to skip legacy commonConfigEnabled
  migration on subsequent startups
- Add regression tests for takeover backup preservation, explicit
  clear semantics, and migration flag roundtrip
This commit is contained in:
Jason
2026-03-12 17:16:22 +08:00
Unverified
parent e561084f62
commit 7ca33ff901
6 changed files with 376 additions and 86 deletions
+13 -10
View File
@@ -212,20 +212,22 @@ pub async fn set_claude_common_config_snippet(
snippet: String,
state: tauri::State<'_, crate::store::AppState>,
) -> Result<(), String> {
let is_cleared = snippet.trim().is_empty();
if !snippet.trim().is_empty() {
serde_json::from_str::<serde_json::Value>(&snippet).map_err(invalid_json_format_error)?;
}
let value = if snippet.trim().is_empty() {
None
} else {
Some(snippet)
};
let value = if is_cleared { None } else { Some(snippet) };
state
.db
.set_config_snippet("claude", value)
.map_err(|e| e.to_string())?;
state
.db
.set_config_snippet_cleared("claude", is_cleared)
.map_err(|e| e.to_string())?;
Ok(())
}
@@ -246,6 +248,7 @@ pub async fn set_common_config_snippet(
snippet: String,
state: tauri::State<'_, crate::store::AppState>,
) -> Result<(), String> {
let is_cleared = snippet.trim().is_empty();
let old_snippet = state
.db
.get_config_snippet(&app_type)
@@ -253,11 +256,7 @@ pub async fn set_common_config_snippet(
validate_common_config_snippet(&app_type, &snippet)?;
let value = if snippet.trim().is_empty() {
None
} else {
Some(snippet)
};
let value = if is_cleared { None } else { Some(snippet) };
if matches!(app_type.as_str(), "claude" | "codex" | "gemini") {
if let Some(legacy_snippet) = old_snippet
@@ -278,6 +277,10 @@ pub async fn set_common_config_snippet(
.db
.set_config_snippet(&app_type, value)
.map_err(|e| e.to_string())?;
state
.db
.set_config_snippet_cleared(&app_type, is_cleared)
.map_err(|e| e.to_string())?;
if matches!(app_type.as_str(), "claude" | "codex" | "gemini") {
let app = AppType::from_str(&app_type).map_err(|e| e.to_string())?;
+4 -4
View File
@@ -103,16 +103,16 @@ fn import_default_config_internal(state: &AppState, app_type: AppType) -> Result
// Extract common config snippet (mirrors old startup logic in lib.rs)
if state
.db
.get_config_snippet(app_type.as_str())
.ok()
.flatten()
.is_none()
.should_auto_extract_config_snippet(app_type.as_str())?
{
match ProviderService::extract_common_config_snippet(state, app_type.clone()) {
Ok(snippet) if !snippet.is_empty() && snippet != "{}" => {
let _ = state
.db
.set_config_snippet(app_type.as_str(), Some(snippet));
let _ = state
.db
.set_config_snippet_cleared(app_type.as_str(), false);
}
_ => {}
}
+60
View File
@@ -7,6 +7,12 @@ use crate::error::AppError;
use rusqlite::params;
impl Database {
const LEGACY_COMMON_CONFIG_MIGRATED_KEY: &'static str = "common_config_legacy_migrated_v1";
fn config_snippet_cleared_key(app_type: &str) -> String {
format!("common_config_{app_type}_cleared")
}
/// 获取设置值
pub fn get_setting(&self, key: &str) -> Result<Option<String>, AppError> {
let conn = lock_conn!(self.conn);
@@ -45,6 +51,60 @@ impl Database {
self.get_setting(&format!("common_config_{app_type}"))
}
/// 检查通用配置片段是否被用户显式清空
pub fn is_config_snippet_cleared(&self, app_type: &str) -> Result<bool, AppError> {
Ok(self
.get_setting(&Self::config_snippet_cleared_key(app_type))?
.as_deref()
== Some("true"))
}
/// 设置通用配置片段是否被显式清空
pub fn set_config_snippet_cleared(
&self,
app_type: &str,
cleared: bool,
) -> Result<(), AppError> {
let key = Self::config_snippet_cleared_key(app_type);
if cleared {
self.set_setting(&key, "true")
} else {
let conn = lock_conn!(self.conn);
conn.execute("DELETE FROM settings WHERE key = ?1", params![key])
.map_err(|e| AppError::Database(e.to_string()))?;
Ok(())
}
}
/// 当前是否允许从 live 配置自动抽取通用配置片段
pub fn should_auto_extract_config_snippet(&self, app_type: &str) -> Result<bool, AppError> {
Ok(self.get_config_snippet(app_type)?.is_none()
&& !self.is_config_snippet_cleared(app_type)?)
}
/// 检查历史通用配置迁移是否已经执行过
pub fn is_legacy_common_config_migrated(&self) -> Result<bool, AppError> {
Ok(self
.get_setting(Self::LEGACY_COMMON_CONFIG_MIGRATED_KEY)?
.as_deref()
== Some("true"))
}
/// 标记历史通用配置迁移已经执行完成
pub fn set_legacy_common_config_migrated(&self, migrated: bool) -> Result<(), AppError> {
if migrated {
self.set_setting(Self::LEGACY_COMMON_CONFIG_MIGRATED_KEY, "true")
} else {
let conn = lock_conn!(self.conn);
conn.execute(
"DELETE FROM settings WHERE key = ?1",
params![Self::LEGACY_COMMON_CONFIG_MIGRATED_KEY],
)
.map_err(|e| AppError::Database(e.to_string()))?;
Ok(())
}
}
/// 设置通用配置片段
pub fn set_config_snippet(
&self,
+81 -72
View File
@@ -560,78 +560,6 @@ pub fn run() {
}
}
// 5. Auto-extract common config snippets from live files (when snippet is missing)
for app_type in crate::app_config::AppType::all() {
// Skip if snippet already exists
if app_state
.db
.get_config_snippet(app_type.as_str())
.ok()
.flatten()
.is_some()
{
continue;
}
// Try to read the live config file for this app type
let settings =
match crate::services::provider::ProviderService::read_live_settings(
app_type.clone(),
) {
Ok(s) => s,
Err(_) => continue, // No live config file, skip silently
};
// Extract common config (strip provider-specific fields)
match crate::services::provider::ProviderService::extract_common_config_snippet_from_settings(
app_type.clone(),
&settings,
) {
Ok(snippet) if !snippet.is_empty() && snippet != "{}" => {
match app_state
.db
.set_config_snippet(app_type.as_str(), Some(snippet))
{
Ok(()) => log::info!(
"✓ Auto-extracted common config snippet for {}",
app_type.as_str()
),
Err(e) => log::warn!(
"✗ Failed to save config snippet for {}: {e}",
app_type.as_str()
),
}
}
Ok(_) => log::debug!(
"○ Live config for {} has no extractable common fields",
app_type.as_str()
),
Err(e) => log::warn!(
"✗ Failed to extract config snippet for {}: {e}",
app_type.as_str()
),
}
}
// 5.1 Migrate legacy providers that relied on inferred common-config usage
for app_type in [
crate::app_config::AppType::Claude,
crate::app_config::AppType::Codex,
crate::app_config::AppType::Gemini,
] {
if let Err(e) =
crate::services::provider::ProviderService::migrate_legacy_common_config_usage_if_needed(
&app_state,
app_type.clone(),
)
{
log::warn!(
"✗ Failed to migrate legacy common-config usage for {}: {e}",
app_type.as_str()
);
}
}
// 迁移旧的 app_config_dir 配置到 Store
if let Err(e) = app_store::migrate_app_config_dir_from_settings(app.handle()) {
log::warn!("迁移 app_config_dir 失败: {e}");
@@ -816,6 +744,8 @@ pub fn run() {
}
}
initialize_common_config_snippets(&state);
// 检查 settings 表中的代理状态,自动恢复代理服务
restore_proxy_state_on_startup(&state).await;
@@ -1324,6 +1254,85 @@ async fn restore_proxy_state_on_startup(state: &store::AppState) {
}
}
fn initialize_common_config_snippets(state: &store::AppState) {
// Auto-extract common config snippets from clean live files when snippet is missing.
// This must run before proxy takeover is restored on startup, otherwise we'd read
// proxy-placeholder configs instead of the user's actual live settings.
for app_type in crate::app_config::AppType::all() {
if !state
.db
.should_auto_extract_config_snippet(app_type.as_str())
.unwrap_or(false)
{
continue;
}
let settings = match crate::services::provider::ProviderService::read_live_settings(
app_type.clone(),
) {
Ok(s) => s,
Err(_) => continue,
};
match crate::services::provider::ProviderService::extract_common_config_snippet_from_settings(
app_type.clone(),
&settings,
) {
Ok(snippet) if !snippet.is_empty() && snippet != "{}" => {
match state.db.set_config_snippet(app_type.as_str(), Some(snippet)) {
Ok(()) => {
let _ = state.db.set_config_snippet_cleared(app_type.as_str(), false);
log::info!(
"✓ Auto-extracted common config snippet for {}",
app_type.as_str()
);
}
Err(e) => log::warn!(
"✗ Failed to save config snippet for {}: {e}",
app_type.as_str()
),
}
}
Ok(_) => log::debug!(
"○ Live config for {} has no extractable common fields",
app_type.as_str()
),
Err(e) => log::warn!(
"✗ Failed to extract config snippet for {}: {e}",
app_type.as_str()
),
}
}
let should_run_legacy_migration = state
.db
.is_legacy_common_config_migrated()
.map(|done| !done)
.unwrap_or(true);
if should_run_legacy_migration {
for app_type in [
crate::app_config::AppType::Claude,
crate::app_config::AppType::Codex,
crate::app_config::AppType::Gemini,
] {
if let Err(e) = crate::services::provider::ProviderService::migrate_legacy_common_config_usage_if_needed(
state,
app_type.clone(),
) {
log::warn!(
"✗ Failed to migrate legacy common-config usage for {}: {e}",
app_type.as_str()
);
}
}
if let Err(e) = state.db.set_legacy_common_config_migrated(true) {
log::warn!("✗ Failed to persist legacy common-config migration flag: {e}");
}
}
}
// ============================================================
// 迁移错误对话框辅助函数
// ============================================================
+40
View File
@@ -614,6 +614,46 @@ impl ProviderService {
state: &AppState,
app_type: AppType,
) -> Result<(), AppError> {
if app_type.is_additive_mode() {
return sync_current_provider_for_app_to_live(state, &app_type);
}
let current_id =
match crate::settings::get_effective_current_provider(&state.db, &app_type)? {
Some(id) => id,
None => return Ok(()),
};
let providers = state.db.get_all_providers(app_type.as_str())?;
let Some(provider) = providers.get(&current_id) else {
return Ok(());
};
let takeover_enabled =
futures::executor::block_on(state.db.get_proxy_config_for_app(app_type.as_str()))
.map(|config| config.enabled)
.unwrap_or(false);
let has_live_backup =
futures::executor::block_on(state.db.get_live_backup(app_type.as_str()))
.ok()
.flatten()
.is_some();
let live_taken_over = state
.proxy_service
.detect_takeover_in_live_config_for_app(&app_type);
if takeover_enabled && (has_live_backup || live_taken_over) {
futures::executor::block_on(
state
.proxy_service
.update_live_backup_from_provider(app_type.as_str(), provider),
)
.map_err(|e| AppError::Message(format!("更新 Live 备份失败: {e}")))?;
return Ok(());
}
sync_current_provider_for_app_to_live(state, &app_type)
}
+178
View File
@@ -238,6 +238,184 @@ command = "say"
);
}
#[test]
fn sync_current_provider_for_app_keeps_live_takeover_and_updates_restore_backup() {
let _guard = test_mutex().lock().expect("acquire test mutex");
reset_test_fs();
let _home = ensure_test_home();
let mut config = MultiAppConfig::default();
{
let manager = config
.get_manager_mut(&AppType::Claude)
.expect("claude manager");
manager.current = "current-provider".to_string();
let mut provider = Provider::with_id(
"current-provider".to_string(),
"Current".to_string(),
json!({
"env": {
"ANTHROPIC_AUTH_TOKEN": "real-token",
"ANTHROPIC_BASE_URL": "https://claude.example"
}
}),
None,
);
provider.meta = Some(ProviderMeta {
common_config_enabled: Some(true),
..Default::default()
});
manager
.providers
.insert("current-provider".to_string(), provider);
}
let state = create_test_state_with_config(&config).expect("create test state");
state
.db
.set_config_snippet(
AppType::Claude.as_str(),
Some(r#"{ "includeCoAuthoredBy": false }"#.to_string()),
)
.expect("set common config snippet");
let taken_over_live = json!({
"env": {
"ANTHROPIC_BASE_URL": "http://127.0.0.1:5000",
"ANTHROPIC_AUTH_TOKEN": "PROXY_MANAGED"
}
});
let settings_path = get_claude_settings_path();
std::fs::create_dir_all(settings_path.parent().expect("settings dir")).expect("create dir");
std::fs::write(
&settings_path,
serde_json::to_string_pretty(&taken_over_live).expect("serialize taken over live"),
)
.expect("write taken over live");
futures::executor::block_on(state.db.save_live_backup("claude", "{\"env\":{}}"))
.expect("seed live backup");
let mut proxy_config = futures::executor::block_on(state.db.get_proxy_config_for_app("claude"))
.expect("get proxy config");
proxy_config.enabled = true;
futures::executor::block_on(state.db.update_proxy_config_for_app(proxy_config))
.expect("enable takeover");
ProviderService::sync_current_provider_for_app(&state, AppType::Claude)
.expect("sync current provider should succeed");
let live_after: serde_json::Value =
read_json_file(&settings_path).expect("read live settings after sync");
assert_eq!(
live_after, taken_over_live,
"sync should not overwrite live config while takeover is active"
);
let backup = futures::executor::block_on(state.db.get_live_backup("claude"))
.expect("get live backup")
.expect("backup exists");
let backup_value: serde_json::Value =
serde_json::from_str(&backup.original_config).expect("parse backup value");
assert_eq!(
backup_value
.get("includeCoAuthoredBy")
.and_then(|v| v.as_bool()),
Some(false),
"restore backup should receive the updated effective config"
);
assert_eq!(
backup_value
.get("env")
.and_then(|v| v.get("ANTHROPIC_AUTH_TOKEN"))
.and_then(|v| v.as_str()),
Some("real-token"),
"restore backup should preserve the provider token rather than proxy placeholder"
);
}
#[test]
fn explicitly_cleared_common_snippet_is_not_auto_extracted() {
let _guard = test_mutex().lock().expect("acquire test mutex");
reset_test_fs();
let _home = ensure_test_home();
let state = create_test_state().expect("create test state");
state
.db
.set_config_snippet_cleared(AppType::Claude.as_str(), true)
.expect("mark snippet explicitly cleared");
assert!(
!state
.db
.should_auto_extract_config_snippet(AppType::Claude.as_str())
.expect("check auto-extract eligibility"),
"explicitly cleared snippets should block auto-extraction"
);
state
.db
.set_config_snippet(AppType::Claude.as_str(), Some("{}".to_string()))
.expect("set snippet");
state
.db
.set_config_snippet_cleared(AppType::Claude.as_str(), false)
.expect("clear explicit-empty marker");
assert!(
!state
.db
.should_auto_extract_config_snippet(AppType::Claude.as_str())
.expect("check auto-extract after snippet saved"),
"existing snippets should also block auto-extraction"
);
}
#[test]
fn legacy_common_config_migration_flag_roundtrip() {
let _guard = test_mutex().lock().expect("acquire test mutex");
reset_test_fs();
let _home = ensure_test_home();
let state = create_test_state().expect("create test state");
assert!(
!state
.db
.is_legacy_common_config_migrated()
.expect("initial migration flag"),
"migration flag should default to false"
);
state
.db
.set_legacy_common_config_migrated(true)
.expect("set migration flag");
assert!(
state
.db
.is_legacy_common_config_migrated()
.expect("read migration flag"),
"migration flag should persist once set"
);
state
.db
.set_legacy_common_config_migrated(false)
.expect("clear migration flag");
assert!(
!state
.db
.is_legacy_common_config_migrated()
.expect("read migration flag after clear"),
"migration flag should be removable for tests/debugging"
);
}
#[test]
fn switch_packycode_gemini_updates_security_selected_type() {
let _guard = test_mutex().lock().expect("acquire test mutex");