From 7965862e668458eb9401a6fe16045a7e0bf20f78 Mon Sep 17 00:00:00 2001 From: ayxwi <274959179+xwil1@users.noreply.github.com> Date: Thu, 30 Apr 2026 21:59:47 +0800 Subject: [PATCH] Make import existing side-effect free (#2429) Co-authored-by: Xvvln <3369759202@qq.com> --- src-tauri/src/services/mcp.rs | 20 +++--- src-tauri/src/services/skill.rs | 13 ---- src-tauri/tests/mcp_commands.rs | 104 ++++++++++++++++++++++++++++++++ src-tauri/tests/skill_sync.rs | 47 +++++++++++++++ 4 files changed, 161 insertions(+), 23 deletions(-) diff --git a/src-tauri/src/services/mcp.rs b/src-tauri/src/services/mcp.rs index a0026dd87..78f92c029 100644 --- a/src-tauri/src/services/mcp.rs +++ b/src-tauri/src/services/mcp.rs @@ -268,8 +268,8 @@ impl McpService { state.db.save_mcp_server(&to_save)?; existing.insert(to_save.id.clone(), to_save.clone()); - // 同步到对应应用 live 配置 - Self::sync_server_to_apps(state, &to_save)?; + // 导入是读取已有配置，不应反向写回任何应用的 live 配置。 + // 显式编辑、启用/禁用或手动同步时再执行写回。 } } } @@ -306,8 +306,8 @@ impl McpService { state.db.save_mcp_server(&to_save)?; existing.insert(to_save.id.clone(), to_save.clone()); - // 同步到对应应用 live 配置 - Self::sync_server_to_apps(state, &to_save)?; + // 导入是读取已有配置，不应反向写回任何应用的 live 配置。 + // 显式编辑、启用/禁用或手动同步时再执行写回。 } } } @@ -344,8 +344,8 @@ impl McpService { state.db.save_mcp_server(&to_save)?; existing.insert(to_save.id.clone(), to_save.clone()); - // 同步到对应应用 live 配置 - Self::sync_server_to_apps(state, &to_save)?; + // 导入是读取已有配置，不应反向写回任何应用的 live 配置。 + // 显式编辑、启用/禁用或手动同步时再执行写回。 } } } @@ -382,8 +382,8 @@ impl McpService { state.db.save_mcp_server(&to_save)?; existing.insert(to_save.id.clone(), to_save.clone()); - // 同步到对应应用 live 配置 - Self::sync_server_to_apps(state, &to_save)?; + // 导入是读取已有配置，不应反向写回任何应用的 live 配置。 + // 显式编辑、启用/禁用或手动同步时再执行写回。 } } } @@ -420,8 +420,8 @@ impl McpService { state.db.save_mcp_server(&to_save)?; existing.insert(to_save.id.clone(), to_save.clone()); - // 同步到对应应用 live 配置 - Self::sync_server_to_apps(state, &to_save)?; + // 导入是读取已有配置，不应反向写回任何应用的 live 配置。 + // 显式编辑、启用/禁用或手动同步时再执行写回。 } } } diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 06d159677..7c2515ebf 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -1536,19 +1536,6 @@ impl SkillService { // 保存到数据库 db.save_skill(&skill)?; - // 同步到已启用的应用目录（创建 symlink 或复制文件） - for app in AppType::all() { - if skill.apps.is_enabled_for(&app) { - if let Err(e) = Self::sync_to_app_dir(&skill.directory, &app) { - log::warn!( - "导入后同步 Skill '{}' 到 {:?} 失败: {e:#}", - skill.directory, - app - ); - } - } - } - imported.push(skill); } diff --git a/src-tauri/tests/mcp_commands.rs b/src-tauri/tests/mcp_commands.rs index 5cd31f615..242b6e835 100644 --- a/src-tauri/tests/mcp_commands.rs +++ b/src-tauri/tests/mcp_commands.rs @@ -150,6 +150,110 @@ fn import_mcp_from_claude_creates_config_and_enables_servers() { ); } +#[test] +fn import_mcp_from_codex_does_not_rewrite_codex_config() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + let codex_dir = home.join(".codex"); + fs::create_dir_all(&codex_dir).expect("create codex dir"); + let config_path = codex_dir.join("config.toml"); + let original = r#"# keep user formatting intact +model = "gpt-5" + +[mcp.servers.legacy] +type = "stdio" +command = "echo" + +[mcp_servers.echo] +type = "stdio" +command = "echo" +"#; + fs::write(&config_path, original).expect("seed codex config"); + + let state = create_test_state().expect("create test state"); + let changed = McpService::import_from_codex(&state).expect("import from codex"); + assert!(changed > 0, "should import servers from Codex config"); + + let after = fs::read_to_string(&config_path).expect("read codex config"); + assert_eq!( + after, original, + "importing from Codex should not rewrite ~/.codex/config.toml" + ); +} + +#[test] +fn import_mcp_from_claude_does_not_sync_existing_codex_enabled_server() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + let codex_dir = home.join(".codex"); + fs::create_dir_all(&codex_dir).expect("create codex dir"); + let codex_config_path = codex_dir.join("config.toml"); + let codex_original = r#"[mcp.servers.keep_me] +type = "stdio" +command = "echo" +"#; + fs::write(&codex_config_path, codex_original).expect("seed codex config"); + + let claude_json = json!({ + "mcpServers": { + "shared": { + "type": "stdio", + "command": "echo" + } + } + }); + fs::write( + get_claude_mcp_path(), + serde_json::to_string_pretty(&claude_json).expect("serialize claude mcp"), + ) + .expect("seed claude mcp"); + + let state = create_test_state().expect("create test state"); + state + .db + .save_mcp_server(&McpServer { + id: "shared".to_string(), + name: "shared".to_string(), + server: json!({ + "type": "stdio", + "command": "echo" + }), + apps: McpApps { + claude: false, + codex: true, + gemini: false, + opencode: false, + hermes: false, + }, + description: None, + homepage: None, + docs: None, + tags: Vec::new(), + }) + .expect("seed existing mcp server"); + + let changed = McpService::import_from_claude(&state).expect("import from claude"); + assert_eq!(changed, 0, "existing server should not count as new"); + + let after = fs::read_to_string(&codex_config_path).expect("read codex config"); + assert_eq!( + after, codex_original, + "importing from Claude should not sync an existing Codex-enabled server" + ); + + let servers = state.db.get_all_mcp_servers().expect("get all mcp servers"); + let shared = servers.get("shared").expect("shared server exists"); + assert!( + shared.apps.claude, + "import should enable Claude in database" + ); + assert!(shared.apps.codex, "existing Codex flag should be preserved"); +} + #[test] fn import_mcp_from_claude_invalid_json_preserves_state() { use support::create_test_state; diff --git a/src-tauri/tests/skill_sync.rs b/src-tauri/tests/skill_sync.rs index bf960bbe7..fcba98a7a 100644 --- a/src-tauri/tests/skill_sync.rs +++ b/src-tauri/tests/skill_sync.rs @@ -72,6 +72,53 @@ fn import_from_apps_respects_explicit_app_selection() { ); } +#[test] +fn import_from_apps_does_not_rewrite_selected_app_directory() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + let ssot_skill_dir = home.join(".cc-switch").join("skills").join("codex-skill"); + write_skill(&ssot_skill_dir, "Stale SSOT Skill"); + fs::write(ssot_skill_dir.join("prompt.md"), "stale ssot").expect("write stale ssot prompt"); + + let codex_skill_dir = home.join(".codex").join("skills").join("codex-skill"); + write_skill(&codex_skill_dir, "Live Codex Skill"); + fs::write(codex_skill_dir.join("prompt.md"), "live codex").expect("write live codex prompt"); + + let state = create_test_state().expect("create test state"); + + let imported = SkillService::import_from_apps( + &state.db, + vec![ImportSkillSelection { + directory: "codex-skill".to_string(), + apps: SkillApps { + codex: true, + ..Default::default() + }, + }], + ) + .expect("import skills"); + + assert_eq!(imported.len(), 1, "expected exactly one imported skill"); + assert!( + imported[0].apps.codex, + "import should preserve the selected Codex app state" + ); + assert_eq!( + fs::read_to_string(codex_skill_dir.join("prompt.md")).expect("read live codex prompt"), + "live codex", + "import should not replace the app skill directory with SSOT contents" + ); + assert!( + !fs::symlink_metadata(&codex_skill_dir) + .expect("read codex skill metadata") + .file_type() + .is_symlink(), + "import should not replace the app skill directory with a managed symlink" + ); +} + #[test] fn sync_to_app_removes_disabled_and_orphaned_ssot_symlinks() { let _guard = test_mutex().lock().expect("acquire test mutex");