From 3c2a22ffefccbf305a9fd6e0666710ece0fffaa7 Mon Sep 17 00:00:00 2001 From: SaladDay <92240037+SaladDay@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:28:52 +0000 Subject: [PATCH] feat(skills): add configurable storage location Co-authored-by: ShatterDusk --- src-tauri/src/cli/commands/skills.rs | 50 +- src-tauri/src/cli/i18n.rs | 76 ++ src-tauri/src/cli/mod.rs | 31 + src-tauri/src/cli/tui/app.rs | 2 +- src-tauri/src/cli/tui/app/app_state.rs | 7 +- src-tauri/src/cli/tui/app/content_config.rs | 8 + src-tauri/src/cli/tui/app/helpers.rs | 14 + .../cli/tui/app/overlay_handlers/dialogs.rs | 5 + .../cli/tui/app/overlay_handlers/pickers.rs | 49 + src-tauri/src/cli/tui/app/tests.rs | 51 + src-tauri/src/cli/tui/app/types.rs | 8 + src-tauri/src/cli/tui/help.rs | 10 + src-tauri/src/cli/tui/mod.rs | 1 + src-tauri/src/cli/tui/runtime_actions/mod.rs | 70 ++ .../src/cli/tui/runtime_actions/skills.rs | 25 +- .../src/cli/tui/runtime_systems/handlers.rs | 70 ++ .../src/cli/tui/runtime_systems/types.rs | 7 + .../src/cli/tui/runtime_systems/workers.rs | 16 + src-tauri/src/cli/tui/ui/config.rs | 8 + src-tauri/src/cli/tui/ui/overlay/pickers.rs | 46 + src-tauri/src/cli/tui/ui/overlay/render.rs | 8 + src-tauri/src/cli/tui/ui/tests.rs | 9 +- src-tauri/src/config.rs | 7 + src-tauri/src/lib.rs | 7 +- src-tauri/src/services/mod.rs | 2 +- src-tauri/src/services/skill.rs | 1008 ++++++++++++++++- src-tauri/src/services/webdav_sync/archive.rs | 160 ++- src-tauri/src/settings.rs | 19 + src-tauri/src/test_support.rs | 5 + src-tauri/tests/settings_current_provider.rs | 8 + src-tauri/tests/settings_visible_apps.rs | 8 + src-tauri/tests/skills_service.rs | 562 ++++++++- src-tauri/tests/support.rs | 2 + 33 files changed, 2299 insertions(+), 60 deletions(-) diff --git a/src-tauri/src/cli/commands/skills.rs b/src-tauri/src/cli/commands/skills.rs index be379205d..9d4e21d15 100644 --- a/src-tauri/src/cli/commands/skills.rs +++ b/src-tauri/src/cli/commands/skills.rs @@ -5,9 +5,10 @@ use crate::app_config::{AppType, SkillApps}; use crate::cli::commands::app_targets::{ app_target_names, app_targets_or_default, parse_app_targets, supported_app_target_labels, }; +use crate::cli::i18n::texts; use crate::cli::ui::{create_table, highlight, info, success}; use crate::error::AppError; -use crate::services::skill::{ImportSkillSelection, SkillRepo, SyncMethod}; +use crate::services::skill::{ImportSkillSelection, SkillRepo, SkillStorageLocation, SyncMethod}; use crate::services::SkillService; #[derive(Subcommand)] @@ -110,6 +111,12 @@ pub enum SkillsCommand { #[arg(value_enum)] method: Option, }, + /// Get or change the managed Skills storage location + StorageLocation { + /// Optional location to set (omit to show current) + #[arg(value_enum)] + location: Option, + }, /// Manage skill repositories #[command(subcommand)] Repos(SkillReposCommand), @@ -164,6 +171,7 @@ pub fn execute(cmd: SkillsCommand, app: Option) -> Result<(), AppError> SkillsCommand::ImportFromApps { apps, directories } => import_from_apps(apps, directories), SkillsCommand::Info { spec } => show_skill_info(&spec), SkillsCommand::SyncMethod { method } => sync_method(method), + SkillsCommand::StorageLocation { location } => storage_location(location), SkillsCommand::Repos(repos_cmd) => execute_repos(repos_cmd), } } @@ -619,6 +627,46 @@ fn sync_method(method: Option) -> Result<(), AppError> { Ok(()) } +fn storage_location(location: Option) -> Result<(), AppError> { + let Some(target) = location else { + println!( + "{}", + match crate::settings::get_skill_storage_location() { + SkillStorageLocation::CcSwitch => "cc-switch", + SkillStorageLocation::Unified => "unified", + } + ); + return Ok(()); + }; + + let result = SkillService::migrate_storage(target)?; + let location = texts::tui_skills_storage_location_name(target); + let summary = if result.errors.is_empty() { + texts::tui_toast_skills_storage_location_set( + location, + result.migrated_count, + result.skipped_count, + ) + } else { + texts::tui_toast_skills_storage_location_partial( + location, + result.migrated_count, + result.skipped_count, + result.errors.len(), + ) + }; + + if result.errors.is_empty() { + println!("{}", success(&summary)); + Ok(()) + } else { + Err(AppError::Message(format!( + "{summary}\n{}", + result.errors.join("\n") + ))) + } +} + fn parse_repo_spec(raw: &str) -> Result { let raw = raw.trim().trim_end_matches('/'); if raw.is_empty() { diff --git a/src-tauri/src/cli/i18n.rs b/src-tauri/src/cli/i18n.rs index 7b073fda1..7231425dd 100644 --- a/src-tauri/src/cli/i18n.rs +++ b/src-tauri/src/cli/i18n.rs @@ -5653,6 +5653,53 @@ pub mod texts { } } + pub fn tui_settings_skills_storage_location_label() -> &'static str { + if is_chinese() { + "技能存储位置" + } else { + "Skill storage location" + } + } + + pub fn tui_skills_storage_location_title() -> &'static str { + if is_chinese() { + "选择技能存储位置" + } else { + "Select Skill Storage Location" + } + } + + pub fn tui_skills_storage_location_name( + location: crate::services::skill::SkillStorageLocation, + ) -> &'static str { + match location { + crate::services::skill::SkillStorageLocation::CcSwitch => { + "CC Switch (~/.cc-switch/skills)" + } + crate::services::skill::SkillStorageLocation::Unified => "Unified (~/.agents/skills)", + } + } + + pub fn tui_confirm_skills_storage_location( + location: crate::services::skill::SkillStorageLocation, + count: usize, + ) -> String { + let target = tui_skills_storage_location_name(location); + if is_chinese() { + format!("将把 {count} 个已管理技能迁移到 {target}，并刷新各应用的技能部署。继续吗？") + } else { + format!("Move {count} managed Skill(s) to {target} and refresh their app deployments?") + } + } + + pub fn tui_skills_storage_location_loading() -> &'static str { + if is_chinese() { + "正在迁移技能并刷新应用部署..." + } else { + "Moving Skills and refreshing app deployments..." + } + } + pub fn tui_skills_sync_method_name(method: crate::services::skill::SyncMethod) -> &'static str { match method { crate::services::skill::SyncMethod::Auto => { @@ -8601,6 +8648,35 @@ pub mod texts { } } + pub fn tui_toast_skills_storage_location_set( + location: &str, + migrated: usize, + skipped: usize, + ) -> String { + if is_chinese() { + format!("技能存储位置已切换为 {location}：迁移 {migrated} 个，跳过 {skipped} 个。") + } else { + format!("Skill storage set to {location}: {migrated} moved, {skipped} skipped.") + } + } + + pub fn tui_toast_skills_storage_location_partial( + location: &str, + migrated: usize, + skipped: usize, + failed: usize, + ) -> String { + if is_chinese() { + format!( + "技能存储已切换为 {location}：迁移 {migrated} 个，跳过 {skipped} 个，失败 {failed} 个。" + ) + } else { + format!( + "Skill storage set to {location}: {migrated} moved, {skipped} skipped, {failed} failed." + ) + } + } + pub fn tui_toast_repo_spec_empty() -> &'static str { if is_chinese() { "仓库不能为空。" diff --git a/src-tauri/src/cli/mod.rs b/src-tauri/src/cli/mod.rs index 7ffed351a..3f6794f7c 100644 --- a/src-tauri/src/cli/mod.rs +++ b/src-tauri/src/cli/mod.rs @@ -1855,6 +1855,37 @@ mod tests { } } + #[test] + fn parses_skills_storage_location_query_and_values() { + let query = Cli::parse_from(["cc-switch", "skills", "storage-location"]); + assert!(matches!( + query.command, + Some(Commands::Skills( + super::commands::skills::SkillsCommand::StorageLocation { location: None } + )) + )); + + let unified = Cli::parse_from(["cc-switch", "skills", "storage-location", "unified"]); + assert!(matches!( + unified.command, + Some(Commands::Skills( + super::commands::skills::SkillsCommand::StorageLocation { + location: Some(crate::services::skill::SkillStorageLocation::Unified) + } + )) + )); + + let cc_switch = Cli::parse_from(["cc-switch", "skills", "storage-location", "cc_switch"]); + assert!(matches!( + cc_switch.command, + Some(Commands::Skills( + super::commands::skills::SkillsCommand::StorageLocation { + location: Some(crate::services::skill::SkillStorageLocation::CcSwitch) + } + )) + )); + } + #[test] fn parses_manual_skill_update_commands() { let check = Cli::parse_from(["cc-switch", "skills", "check-updates"]); diff --git a/src-tauri/src/cli/tui/app.rs b/src-tauri/src/cli/tui/app.rs index 2b2717d7f..2b3ff8159 100644 --- a/src-tauri/src/cli/tui/app.rs +++ b/src-tauri/src/cli/tui/app.rs @@ -7,7 +7,7 @@ use crate::app_config::AppType; use crate::cli::i18n::current_language; use crate::cli::i18n::texts; use crate::cli::i18n::Language; -use crate::services::skill::SyncMethod; +use crate::services::skill::{SkillStorageLocation, SyncMethod}; use super::data::UiData; use super::form::{ diff --git a/src-tauri/src/cli/tui/app/app_state.rs b/src-tauri/src/cli/tui/app/app_state.rs index 70ab880c9..7542878fa 100644 --- a/src-tauri/src/cli/tui/app/app_state.rs +++ b/src-tauri/src/cli/tui/app/app_state.rs @@ -66,6 +66,9 @@ pub enum Action { SkillsSetSyncMethod { method: SyncMethod, }, + SkillsSetStorageLocation { + location: SkillStorageLocation, + }, SkillsDiscover { query: String, source: SkillsDiscoverSource, @@ -500,6 +503,7 @@ pub enum SettingsItem { PreferredEditor, VisibleAppsMode, VisibleApps, + SkillsStorageLocation, OpenClawConfigDir, ManagedAccounts, SkipClaudeOnboarding, @@ -512,7 +516,7 @@ pub enum SettingsItem { } impl SettingsItem { - pub const ALL: [SettingsItem; 15] = [ + pub const ALL: [SettingsItem; 16] = [ SettingsItem::ManagedAccounts, SettingsItem::Language, SettingsItem::Theme, @@ -520,6 +524,7 @@ impl SettingsItem { SettingsItem::PreferredEditor, SettingsItem::VisibleAppsMode, SettingsItem::VisibleApps, + SettingsItem::SkillsStorageLocation, SettingsItem::OpenClawConfigDir, SettingsItem::SkipClaudeOnboarding, SettingsItem::ClaudePluginIntegration, diff --git a/src-tauri/src/cli/tui/app/content_config.rs b/src-tauri/src/cli/tui/app/content_config.rs index f2e258859..4f7b1cb14 100644 --- a/src-tauri/src/cli/tui/app/content_config.rs +++ b/src-tauri/src/cli/tui/app/content_config.rs @@ -929,6 +929,14 @@ impl App { }; Action::None } + Some(SettingsItem::SkillsStorageLocation) => { + self.overlay = Overlay::SkillsStorageLocationPicker { + selected: storage_location_picker_index( + crate::settings::get_skill_storage_location(), + ), + }; + Action::None + } Some(SettingsItem::OpenClawConfigDir) => { let buffer = crate::settings::get_settings() .openclaw_config_dir diff --git a/src-tauri/src/cli/tui/app/helpers.rs b/src-tauri/src/cli/tui/app/helpers.rs index 212348a8a..ce145fdff 100644 --- a/src-tauri/src/cli/tui/app/helpers.rs +++ b/src-tauri/src/cli/tui/app/helpers.rs @@ -1803,6 +1803,20 @@ pub(crate) fn sync_method_for_picker_index(index: usize) -> SyncMethod { } } +pub(crate) fn storage_location_picker_index(location: SkillStorageLocation) -> usize { + match location { + SkillStorageLocation::CcSwitch => 0, + SkillStorageLocation::Unified => 1, + } +} + +pub(crate) fn storage_location_for_picker_index(index: usize) -> SkillStorageLocation { + match index { + 1 => SkillStorageLocation::Unified, + _ => SkillStorageLocation::CcSwitch, + } +} + pub(crate) fn openclaw_tools_profile_picker_index(profile: Option<&str>) -> Option { OPENCLAW_TOOLS_PROFILE_PICKER_VALUES .iter() diff --git a/src-tauri/src/cli/tui/app/overlay_handlers/dialogs.rs b/src-tauri/src/cli/tui/app/overlay_handlers/dialogs.rs index c9953f03a..e115cb47d 100644 --- a/src-tauri/src/cli/tui/app/overlay_handlers/dialogs.rs +++ b/src-tauri/src/cli/tui/app/overlay_handlers/dialogs.rs @@ -114,6 +114,11 @@ impl App { ConfirmAction::SkillsUninstall { directory } => Action::SkillsUninstall { directory: directory.clone(), }, + ConfirmAction::SkillsMigrateStorage { location } => { + Action::SkillsSetStorageLocation { + location: *location, + } + } ConfirmAction::SkillsRepoRemove { owner, name } => Action::SkillsRepoRemove { owner: owner.clone(), name: name.clone(), diff --git a/src-tauri/src/cli/tui/app/overlay_handlers/pickers.rs b/src-tauri/src/cli/tui/app/overlay_handlers/pickers.rs index fe574fba4..a13396909 100644 --- a/src-tauri/src/cli/tui/app/overlay_handlers/pickers.rs +++ b/src-tauri/src/cli/tui/app/overlay_handlers/pickers.rs @@ -85,6 +85,9 @@ impl App { if let Some(action) = self.handle_sync_method_picker_key(key, data) { return Some(action); } + if let Some(action) = self.handle_storage_location_picker_key(key, data) { + return Some(action); + } if let Some(action) = self.handle_claude_api_format_picker_key(key, data) { return Some(action); } @@ -285,6 +288,52 @@ impl App { }) } + fn handle_storage_location_picker_key( + &mut self, + key: KeyEvent, + data: &UiData, + ) -> Option { + let Overlay::SkillsStorageLocationPicker { selected } = &mut self.overlay else { + return None; + }; + + Some(match key.code { + KeyCode::Esc => { + self.close_overlay(); + Action::None + } + KeyCode::Up => { + *selected = selected.saturating_sub(1); + Action::None + } + KeyCode::Down => { + *selected = (*selected + 1).min(1); + Action::None + } + KeyCode::Enter => { + let location = storage_location_for_picker_index(*selected); + if location == crate::settings::get_skill_storage_location() { + self.overlay = Overlay::None; + Action::SkillsSetStorageLocation { location } + } else if data.skills.installed.is_empty() { + self.overlay = Overlay::None; + Action::SkillsSetStorageLocation { location } + } else { + self.overlay = Overlay::Confirm(ConfirmOverlay { + title: texts::tui_confirm_title().to_string(), + message: texts::tui_confirm_skills_storage_location( + location, + data.skills.installed.len(), + ), + action: ConfirmAction::SkillsMigrateStorage { location }, + }); + Action::None + } + } + _ => Action::None, + }) + } + fn handle_claude_api_format_picker_key( &mut self, key: KeyEvent, diff --git a/src-tauri/src/cli/tui/app/tests.rs b/src-tauri/src/cli/tui/app/tests.rs index 174c41138..c23ef7142 100644 --- a/src-tauri/src/cli/tui/app/tests.rs +++ b/src-tauri/src/cli/tui/app/tests.rs @@ -10896,6 +10896,57 @@ mod tests { )); } + #[test] + #[serial(home_settings)] + fn settings_skills_storage_location_opens_picker_and_confirms_migration() { + let temp_home = TempDir::new().expect("create temp home"); + let _env = TestEnvGuard::isolated(temp_home.path()); + + let mut app = App::new(Some(AppType::Claude)); + app.route = Route::Settings; + app.focus = Focus::Content; + app.settings_idx = SettingsItem::ALL + .iter() + .position(|item| matches!(item, SettingsItem::SkillsStorageLocation)) + .expect("SkillsStorageLocation missing from SettingsItem::ALL"); + + assert!(matches!( + app.on_key(key(KeyCode::Enter), &UiData::default()), + Action::None + )); + assert!(matches!( + app.overlay, + Overlay::SkillsStorageLocationPicker { selected: 0 } + )); + + assert!(matches!( + app.on_key(key(KeyCode::Enter), &UiData::default()), + Action::SkillsSetStorageLocation { + location: SkillStorageLocation::CcSwitch + } + )); + assert!(matches!(app.overlay, Overlay::None)); + + app.overlay = Overlay::SkillsStorageLocationPicker { selected: 1 }; + let mut data = UiData::default(); + data.skills + .installed + .push(installed_skill("managed", "Managed")); + assert!(matches!( + app.on_key(key(KeyCode::Enter), &data), + Action::None + )); + assert!(matches!( + app.overlay, + Overlay::Confirm(ConfirmOverlay { + action: ConfirmAction::SkillsMigrateStorage { + location: SkillStorageLocation::Unified + }, + .. + }) + )); + } + #[test] #[serial(home_settings)] fn visible_apps_picker_rejects_zero_selection_without_closing() { diff --git a/src-tauri/src/cli/tui/app/types.rs b/src-tauri/src/cli/tui/app/types.rs index 88f2ebb30..56ee282b6 100644 --- a/src-tauri/src/cli/tui/app/types.rs +++ b/src-tauri/src/cli/tui/app/types.rs @@ -4343,6 +4343,9 @@ pub enum ConfirmAction { SkillsUninstall { directory: String, }, + SkillsMigrateStorage { + location: crate::services::skill::SkillStorageLocation, + }, SkillsRepoRemove { owner: String, name: String, @@ -4730,6 +4733,9 @@ pub enum Overlay { SkillsSyncMethodPicker { selected: usize, }, + SkillsStorageLocationPicker { + selected: usize, + }, McpKeyValuePicker { kind: crate::cli::tui::form::McpKeyValueKind, selected: usize, @@ -4896,6 +4902,7 @@ impl Overlay { | Overlay::SkillsAppsPicker { .. } | Overlay::SkillsImportPicker { .. } | Overlay::SkillsSyncMethodPicker { .. } + | Overlay::SkillsStorageLocationPicker { .. } | Overlay::McpKeyValuePicker { .. } | Overlay::McpTypePicker { .. } | Overlay::SpeedtestResult { .. } @@ -4939,6 +4946,7 @@ impl Overlay { | Overlay::SkillsAppsPicker { .. } | Overlay::SkillsImportPicker { .. } | Overlay::SkillsSyncMethodPicker { .. } + | Overlay::SkillsStorageLocationPicker { .. } | Overlay::McpKeyValuePicker { .. } | Overlay::McpTypePicker { .. } | Overlay::Loading { .. } diff --git a/src-tauri/src/cli/tui/help.rs b/src-tauri/src/cli/tui/help.rs index 8e05b988c..a5dc602c2 100644 --- a/src-tauri/src/cli/tui/help.rs +++ b/src-tauri/src/cli/tui/help.rs @@ -52,6 +52,7 @@ enum HelpTarget { Sessions, FailoverQueue, PreferredEditor, + SkillStorageLocation, GlobalOutboundProxy, CodexOfficialAuthPreservation, CodexUnifiedSessionHistory, @@ -112,6 +113,7 @@ fn current_help_target(app: &App) -> HelpTarget { provider_local_proxy_overlay_target(app, LocalProxySettingsField::UserAgent) } Overlay::ExternalEditorPicker { .. } => HelpTarget::PreferredEditor, + Overlay::SkillsStorageLocationPicker { .. } => HelpTarget::SkillStorageLocation, Overlay::ClaudeModelPicker { .. } => { provider_field_overlay_target(app, ProviderAddField::ClaudeModelConfig) } @@ -157,6 +159,7 @@ fn current_help_target(app: &App) -> HelpTarget { if matches!(app.route, super::route::Route::Settings) && matches!(app.focus, Focus::Content) { match SettingsItem::ALL.get(app.settings_idx) { Some(SettingsItem::PreferredEditor) => return HelpTarget::PreferredEditor, + Some(SettingsItem::SkillsStorageLocation) => return HelpTarget::SkillStorageLocation, Some(SettingsItem::OutboundProxy) => return HelpTarget::GlobalOutboundProxy, Some(SettingsItem::PreserveCodexOfficialAuth) => { return HelpTarget::CodexOfficialAuthPreservation; @@ -336,6 +339,13 @@ fn help_for_target(target: HelpTarget, app: &App, data: &UiData) -> HelpContent "cc-switch detects executable common editors only when you open this setting; detection launches nothing and does not affect startup time. Results are choices only: nothing is saved until you explicitly press Enter, and no editor is selected automatically. Valid VISUAL and EDITOR commands also appear in the list.\nCustom commands are executed directly without a shell, with the temporary file path appended as the final argument. Quote paths or arguments that contain spaces; leave custom input empty to clear the selection.\nGUI editors need a wait flag, such as code --wait, or the temporary file could be removed when the launcher exits early. A configured command reports launch failures instead of silently switching editors.", ), ), + HelpTarget::SkillStorageLocation => HelpContent::new( + texts::tui_settings_skills_storage_location_label(), + help_lines( + "选择 CC Switch 管理目录或通用的 ~/.agents/skills 目录作为已管理技能的唯一主存储。切换时只迁移数据库中记录的技能，并刷新已启用应用的链接或副本；不会导入、认领或移动未管理目录。Unified 中存在未管理目录时，云同步会拒绝替换整个目录，请先显式导入或切回 CC Switch 存储。\n如果目标位置已有同名目录（即使内容相同），迁移也会停止并保留当前设置；只有带有本次迁移凭据的中断副本可以自动续跑。应用刷新失败时会保留旧副本；再次选择当前存储位置可重试修复。迁移期间请勿同时运行其他技能安装或更新命令。", + "Choose either CC Switch's managed directory or the shared ~/.agents/skills directory as the single source of truth for managed Skills. Switching moves only Skills recorded in the database and refreshes links or copies for enabled apps; unmanaged directories are not imported, claimed, or moved. If Unified contains unmanaged directories, cloud sync refuses to replace the root; import them explicitly or switch back to CC Switch storage first.\nMigration stops and keeps the current setting when the target already has a same-named directory, even with identical content; only an interrupted copy carrying this migration's receipt can resume automatically. If app refresh is partial, the old copy is retained; select the current location again to retry reconciliation. Do not run another Skill install or update command while migration is active.", + ), + ), HelpTarget::GlobalOutboundProxy => HelpContent::new( crate::t!("Global Outbound Proxy", "全局出站代理"), help_lines( diff --git a/src-tauri/src/cli/tui/mod.rs b/src-tauri/src/cli/tui/mod.rs index 0db8e5d06..cd19db2b6 100644 --- a/src-tauri/src/cli/tui/mod.rs +++ b/src-tauri/src/cli/tui/mod.rs @@ -2226,6 +2226,7 @@ fn cache_invalidation_for_action(action: &Action) -> CacheInvalidation { | Action::SkillsDiscover { .. } | Action::SkillsCheckUpdates | Action::SkillsUpdate { .. } + | Action::SkillsSetStorageLocation { .. } | Action::SkillsOpenImport | Action::SkillsScanUnmanaged | Action::EditorDiscard diff --git a/src-tauri/src/cli/tui/runtime_actions/mod.rs b/src-tauri/src/cli/tui/runtime_actions/mod.rs index 53d5cc389..b60c10a19 100644 --- a/src-tauri/src/cli/tui/runtime_actions/mod.rs +++ b/src-tauri/src/cli/tui/runtime_actions/mod.rs @@ -850,6 +850,9 @@ pub(crate) fn handle_action( Action::SkillsUninstall { directory } => skills::uninstall(&mut ctx, directory), Action::SkillsSync { app: scope } => skills::sync(&mut ctx, scope), Action::SkillsSetSyncMethod { method } => skills::set_sync_method(&mut ctx, method), + Action::SkillsSetStorageLocation { location } => { + skills::set_storage_location(&mut ctx, location) + } Action::SkillsDiscover { query, source, @@ -1235,6 +1238,38 @@ mod tests { ) } + fn run_action_with_skills( + app: &mut App, + data: &mut UiData, + skills_req_tx: &mpsc::Sender, + action: Action, + ) -> Result<(), AppError> { + let mut terminal = TuiTerminal::new_for_test().expect("create terminal"); + let mut proxy_loading = RequestTracker::default(); + let mut webdav_loading = RequestTracker::default(); + let mut update_check = RequestTracker::default(); + + handle_action( + &mut terminal, + app, + data, + None, + None, + Some(skills_req_tx), + None, + &mut proxy_loading, + None, + None, + None, + &mut webdav_loading, + None, + &mut update_check, + None, + None, + action, + ) + } + fn app_with_base_session_manifest() -> (App, TempDir) { let manifest_dir = tempfile::tempdir().expect("manifest fixture directory"); let store = crate::session_manager::paged_manifest::PagedManifestStore::open_at( @@ -1291,6 +1326,41 @@ mod tests { ); } + #[test] + fn storage_migration_retires_a_pending_discovery_result() { + let mut app = App::new(Some(AppType::Claude)); + app.skills_discover_active_request_id = Some(7); + app.skills_discover_loading = true; + let mut data = UiData::default(); + let (tx, rx) = mpsc::channel(); + + run_action_with_skills( + &mut app, + &mut data, + &tx, + Action::SkillsSetStorageLocation { + location: crate::services::skill::SkillStorageLocation::Unified, + }, + ) + .expect("queue storage migration"); + + assert!(app.skills_discover_active_request_id.is_none()); + assert!(!app.skills_discover_loading); + assert!(matches!( + rx.recv().expect("migration request"), + SkillsReq::MigrateStorage { + target: crate::services::skill::SkillStorageLocation::Unified + } + )); + assert!(matches!( + app.overlay, + Overlay::Loading { + kind: super::super::app::LoadingKind::SkillOperation, + .. + } + )); + } + #[test] fn leaving_sessions_invalidates_the_active_cost_overlay() { let (mut app, _manifest_dir) = app_with_base_session_manifest(); diff --git a/src-tauri/src/cli/tui/runtime_actions/skills.rs b/src-tauri/src/cli/tui/runtime_actions/skills.rs index 19385f805..6794b8182 100644 --- a/src-tauri/src/cli/tui/runtime_actions/skills.rs +++ b/src-tauri/src/cli/tui/runtime_actions/skills.rs @@ -2,7 +2,7 @@ use crate::app_config::{AppType, SkillApps}; use crate::cli::i18n::texts; use crate::error::AppError; use crate::services::{ - skill::{ImportSkillSelection, SyncMethod}, + skill::{ImportSkillSelection, SkillStorageLocation, SyncMethod}, SkillService, }; @@ -164,6 +164,29 @@ pub(super) fn set_sync_method( Ok(()) } +pub(super) fn set_storage_location( + ctx: &mut RuntimeActionContext<'_>, + target: SkillStorageLocation, +) -> Result<(), AppError> { + let Some(tx) = ctx.skills_req_tx else { + return Err(AppError::Message( + texts::tui_error_skills_worker_unavailable().to_string(), + )); + }; + tx.send(super::super::runtime_systems::SkillsReq::MigrateStorage { target }) + .map_err(|e| AppError::Message(e.to_string()))?; + // Discovery shares this worker but has no blocking overlay. Invalidate a + // queued result so it cannot dismiss the migration overlay when it lands. + ctx.app.skills_discover_active_request_id = None; + ctx.app.skills_discover_loading = false; + ctx.app.overlay = Overlay::Loading { + kind: LoadingKind::SkillOperation, + title: texts::tui_skills_storage_location_title().to_string(), + message: texts::tui_skills_storage_location_loading().to_string(), + }; + Ok(()) +} + pub(super) fn discover( ctx: &mut RuntimeActionContext<'_>, query: String, diff --git a/src-tauri/src/cli/tui/runtime_systems/handlers.rs b/src-tauri/src/cli/tui/runtime_systems/handlers.rs index 3272f7f7f..d9213a6f0 100644 --- a/src-tauri/src/cli/tui/runtime_systems/handlers.rs +++ b/src-tauri/src/cli/tui/runtime_systems/handlers.rs @@ -1426,6 +1426,39 @@ pub(crate) fn handle_skills_msg( ); } }, + SkillsMsg::StorageMigrated { target, result } => match result { + Ok(result) => { + app.overlay = Overlay::None; + *data = UiData::load(&app.app_type)?; + invalidation = CacheInvalidation::DataReloaded; + let location = texts::tui_skills_storage_location_name(target); + if result.errors.is_empty() { + app.push_toast( + texts::tui_toast_skills_storage_location_set( + location, + result.migrated_count, + result.skipped_count, + ), + ToastKind::Success, + ); + } else { + app.push_copyable_toast( + texts::tui_toast_skills_storage_location_partial( + location, + result.migrated_count, + result.skipped_count, + result.errors.len(), + ), + ToastKind::Warning, + result.errors.join("\n"), + ); + } + } + Err(err) => { + app.overlay = Overlay::None; + app.push_toast(err, ToastKind::Error); + } + }, } Ok(invalidation) @@ -2055,6 +2088,43 @@ mod tests { ); } + #[test] + #[serial_test::serial(home_settings)] + fn skill_storage_migration_feedback_preserves_partial_error_details() { + let temp = tempfile::tempdir().expect("create isolated home"); + let _env = crate::test_support::TestEnvGuard::isolated(temp.path()); + let mut settings = crate::settings::AppSettings::default(); + settings.skill_storage_location = crate::services::skill::SkillStorageLocation::Unified; + crate::settings::update_settings(settings).expect("set unified storage"); + + let mut app = App::new(Some(AppType::Claude)); + app.overlay = Overlay::Loading { + kind: LoadingKind::SkillOperation, + title: "Migrating".to_string(), + message: "Working".to_string(), + }; + let mut data = UiData::default(); + handle_skills_msg( + &mut app, + &mut data, + SkillsMsg::StorageMigrated { + target: crate::services::skill::SkillStorageLocation::Unified, + result: Ok(crate::services::skill::MigrationResult { + migrated_count: 1, + skipped_count: 0, + errors: vec!["claude/managed: deployment preserved".to_string()], + }), + }, + ) + .expect("handle partial migration"); + + assert!(matches!(app.overlay, Overlay::None)); + assert_eq!( + app.toast.as_ref().and_then(|toast| toast.copy_text()), + Some("claude/managed: deployment preserved") + ); + } + #[test] fn partial_skill_deployment_keeps_the_tui_retry_marker() { let mut app = App::new(Some(AppType::Claude)); diff --git a/src-tauri/src/cli/tui/runtime_systems/types.rs b/src-tauri/src/cli/tui/runtime_systems/types.rs index 75c5a0cab..edf01ca15 100644 --- a/src-tauri/src/cli/tui/runtime_systems/types.rs +++ b/src-tauri/src/cli/tui/runtime_systems/types.rs @@ -482,6 +482,9 @@ pub(crate) enum SkillsReq { Update { ids: Vec, }, + MigrateStorage { + target: crate::services::skill::SkillStorageLocation, + }, } pub(crate) enum SkillsMsg { @@ -501,6 +504,10 @@ pub(crate) enum SkillsMsg { SkillsUpdated { result: Result, }, + StorageMigrated { + target: crate::services::skill::SkillStorageLocation, + result: Result, + }, } #[derive(Debug, Clone)] diff --git a/src-tauri/src/cli/tui/runtime_systems/workers.rs b/src-tauri/src/cli/tui/runtime_systems/workers.rs index b5e8d046d..445f3fa5b 100644 --- a/src-tauri/src/cli/tui/runtime_systems/workers.rs +++ b/src-tauri/src/cli/tui/runtime_systems/workers.rs @@ -3782,6 +3782,12 @@ fn skills_worker_loop(rx: mpsc::Receiver, tx: mpsc::Sender result: Err(err.clone()), }); } + SkillsReq::MigrateStorage { target } => { + let _ = tx.send(SkillsMsg::StorageMigrated { + target, + result: Err(err.clone()), + }); + } } } return; @@ -3823,6 +3829,12 @@ fn skills_worker_loop(rx: mpsc::Receiver, tx: mpsc::Sender result: Err(err.clone()), }); } + SkillsReq::MigrateStorage { target } => { + let _ = tx.send(SkillsMsg::StorageMigrated { + target, + result: Err(err.clone()), + }); + } } } return; @@ -3927,6 +3939,10 @@ fn skills_worker_loop(rx: mpsc::Receiver, tx: mpsc::Sender let result = Ok(rt.block_on(service.update_skills(&ids))); let _ = tx.send(SkillsMsg::SkillsUpdated { result }); } + SkillsReq::MigrateStorage { target } => { + let result = SkillService::migrate_storage(target).map_err(|e| e.to_string()); + let _ = tx.send(SkillsMsg::StorageMigrated { target, result }); + } } } } diff --git a/src-tauri/src/cli/tui/ui/config.rs b/src-tauri/src/cli/tui/ui/config.rs index 1517dab68..e61a912f9 100644 --- a/src-tauri/src/cli/tui/ui/config.rs +++ b/src-tauri/src/cli/tui/ui/config.rs @@ -20,6 +20,7 @@ fn settings_section(item: SettingsItem) -> SettingsSection { | SettingsItem::PreferredEditor => SettingsSection::General, SettingsItem::VisibleAppsMode | SettingsItem::VisibleApps + | SettingsItem::SkillsStorageLocation | SettingsItem::OpenClawConfigDir => SettingsSection::Applications, SettingsItem::SkipClaudeOnboarding | SettingsItem::ClaudePluginIntegration @@ -3479,6 +3480,13 @@ pub(super) fn render_settings( texts::tui_settings_visible_apps_label().to_string(), visible_apps_summary(&visible_apps), ), + super::app::SettingsItem::SkillsStorageLocation => ( + texts::tui_settings_skills_storage_location_label().to_string(), + texts::tui_skills_storage_location_name( + crate::settings::get_skill_storage_location(), + ) + .to_string(), + ), super::app::SettingsItem::OpenClawConfigDir => ( texts::tui_settings_openclaw_config_dir_label().to_string(), openclaw_config_dir.clone().unwrap_or_else(|| { diff --git a/src-tauri/src/cli/tui/ui/overlay/pickers.rs b/src-tauri/src/cli/tui/ui/overlay/pickers.rs index 57bed5522..c4a8b5ac6 100644 --- a/src-tauri/src/cli/tui/ui/overlay/pickers.rs +++ b/src-tauri/src/cli/tui/ui/overlay/pickers.rs @@ -2509,6 +2509,52 @@ pub(super) fn render_skills_sync_method_picker_overlay( frame.render_stateful_widget(list, body_area, &mut state); } +pub(super) fn render_skills_storage_location_picker_overlay( + frame: &mut Frame<'_>, + content_area: Rect, + theme: &theme::Theme, + selected: usize, +) { + let locations = [ + crate::services::skill::SkillStorageLocation::CcSwitch, + crate::services::skill::SkillStorageLocation::Unified, + ]; + let body_area = overlay_frame( + frame, + content_area, + theme, + texts::tui_skills_storage_location_title(), + &[ + ("↑↓", texts::tui_key_select()), + ("Enter", texts::tui_key_apply()), + ("Esc", texts::tui_key_cancel()), + ], + OverlaySize::FitRows { + width: OVERLAY_FIXED_LG.0, + body_rows: locations.len() as u16, + }, + overlay_border_style(theme, false), + ); + let current = crate::settings::get_skill_storage_location(); + let items = locations.into_iter().map(|location| { + let marker = if location == current { + texts::tui_marker_active() + } else { + texts::tui_marker_inactive() + }; + ListItem::new(Line::from(Span::raw(format!( + "{marker} {}", + texts::tui_skills_storage_location_name(location) + )))) + }); + let list = List::new(items) + .highlight_style(selection_style(theme)) + .highlight_symbol(highlight_symbol(theme)); + let mut state = ListState::default(); + state.select(Some(selected)); + frame.render_stateful_widget(list, body_area, &mut state); +} + #[expect( clippy::too_many_arguments, reason = "app picker renderer receives list state and display labels" diff --git a/src-tauri/src/cli/tui/ui/overlay/render.rs b/src-tauri/src/cli/tui/ui/overlay/render.rs index 7944b0fe0..08e990a78 100644 --- a/src-tauri/src/cli/tui/ui/overlay/render.rs +++ b/src-tauri/src/cli/tui/ui/overlay/render.rs @@ -272,6 +272,14 @@ pub(crate) fn render_overlay( *selected, ) } + Overlay::SkillsStorageLocationPicker { selected } => { + super::pickers::render_skills_storage_location_picker_overlay( + frame, + content_area, + theme, + *selected, + ) + } Overlay::McpKeyValuePicker { kind, selected } => { super::mcp_key_value::render_mcp_key_value_picker_overlay( frame, diff --git a/src-tauri/src/cli/tui/ui/tests.rs b/src-tauri/src/cli/tui/ui/tests.rs index 5f05ae85a..f1f8d8c91 100644 --- a/src-tauri/src/cli/tui/ui/tests.rs +++ b/src-tauri/src/cli/tui/ui/tests.rs @@ -4962,6 +4962,10 @@ fn settings_page_groups_items_with_unlabeled_dividers() { let managed_accounts = line_index(&content, texts::tui_settings_managed_accounts_title()); let editor = line_index(&content, texts::tui_settings_preferred_editor_label()); let visible_apps = line_index(&content, texts::tui_settings_visible_apps_mode_label()); + let skill_storage = line_index( + &content, + texts::tui_settings_skills_storage_location_label(), + ); let openclaw_dir = line_index(&content, texts::tui_settings_openclaw_config_dir_label()); let claude_integration = line_index(&content, texts::enable_claude_plugin_integration_label()); let codex_login = line_index(&content, texts::codex_preserve_official_auth_label()); @@ -4974,7 +4978,10 @@ fn settings_page_groups_items_with_unlabeled_dividers() { "{content}" ); assert!( - dividers[0] < visible_apps && openclaw_dir < dividers[1], + dividers[0] < visible_apps + && visible_apps < skill_storage + && skill_storage < openclaw_dir + && openclaw_dir < dividers[1], "{content}" ); assert!( diff --git a/src-tauri/src/config.rs b/src-tauri/src/config.rs index 5c7c86b7c..4279fdb40 100644 --- a/src-tauri/src/config.rs +++ b/src-tauri/src/config.rs @@ -13,6 +13,13 @@ pub(crate) fn home_dir() -> Option { return Some(home); } + if let Some(home) = env::var_os("CC_SWITCH_TEST_HOME") { + let home = PathBuf::from(home); + if !home.as_os_str().is_empty() && !home.to_string_lossy().trim().is_empty() { + return Some(home); + } + } + dirs::home_dir() } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 9243643ba..2815a8ca6 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -82,9 +82,10 @@ pub use services::{ reapply_current_codex_official_live, AuthService, ConfigService, CredentialStatus, EndpointLatency, ExtraUsage, GlobalOutboundProxyConfig, HealthStatus, ImportSkillSelection, ManagedAuthAccount, ManagedAuthDeviceCodeResponse, ManagedAuthStatus, McpService, - PromptService, ProviderService, ProxyService, QuotaTier, S3RemoteInfo, S3SyncService, - S3SyncSummary, SkillService, SpeedtestService, StreamCheckConfig, StreamCheckResult, - StreamCheckService, SubscriptionQuota, SyncDecision, WebDavSyncService, WebDavSyncSummary, + MigrationResult, PromptService, ProviderService, ProxyService, QuotaTier, S3RemoteInfo, + S3SyncService, S3SyncSummary, SkillService, SkillStorageLocation, SpeedtestService, + StreamCheckConfig, StreamCheckResult, StreamCheckService, SubscriptionQuota, SyncDecision, + WebDavSyncService, WebDavSyncSummary, }; pub use settings::{ get_enable_claude_plugin_integration, get_s3_sync_settings, get_skip_claude_onboarding, diff --git a/src-tauri/src/services/mod.rs b/src-tauri/src/services/mod.rs index 5d6d01bf2..47884bd47 100644 --- a/src-tauri/src/services/mod.rs +++ b/src-tauri/src/services/mod.rs @@ -51,7 +51,7 @@ pub use prompt::PromptService; pub use provider::{reapply_current_codex_official_live, ProviderService}; pub use proxy::ProxyService; pub use s3_sync::{S3RemoteInfo, S3SyncService, S3SyncSummary}; -pub use skill::{ImportSkillSelection, SkillService}; +pub use skill::{ImportSkillSelection, MigrationResult, SkillService, SkillStorageLocation}; pub use speedtest::{EndpointLatency, SpeedtestService}; pub use stream_check::{HealthStatus, StreamCheckConfig, StreamCheckResult, StreamCheckService}; pub use subscription::{CredentialStatus, ExtraUsage, QuotaTier, SubscriptionQuota}; diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 810923f0d..e08d64df0 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -10,13 +10,15 @@ use chrono::{DateTime, Utc}; use futures::future::join_all; use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; +use std::ffi::OsStr; use std::fs; +use std::io::{Read, Write}; use std::path::{Component, Path, PathBuf}; use tokio::time::timeout; use crate::app_config::AppType; pub use crate::app_config::{InstalledSkill, SkillApps, UnmanagedSkill}; -use crate::config::{create_managed_config_dir_all, get_app_config_dir}; +use crate::config::{create_managed_config_dir_all, get_app_config_dir, write_json_file}; use crate::database::Database; use crate::error::{format_skill_error, AppError}; @@ -26,6 +28,7 @@ const MAX_SKILL_ARCHIVE_TOTAL_BYTES: u64 = 512 * 1024 * 1024; const MAX_SKILL_ARCHIVE_DOWNLOAD_BYTES: u64 = 128 * 1024 * 1024; const MAX_SKILL_ARCHIVE_PATH_DEPTH: usize = 64; const SKILL_ARCHIVE_ENTRY_COST: u64 = 4096; +const STORAGE_MIGRATION_JOURNAL_FILE: &str = "skill-storage-migration.json"; fn default_skills_index_version() -> u32 { SKILLS_INDEX_VERSION @@ -120,6 +123,43 @@ pub enum SyncMethod { Copy, } +/// Location of the managed Skills single source of truth (SSOT). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[cfg_attr(feature = "cli", derive(clap::ValueEnum))] +#[serde(rename_all = "snake_case")] +pub enum SkillStorageLocation { + /// CC Switch managed directory (`~/.cc-switch/skills/`). + #[default] + #[cfg_attr(feature = "cli", value(alias = "cc_switch"))] + CcSwitch, + /// Shared Agent Skills directory (`~/.agents/skills/`). + Unified, +} + +/// Result of moving managed Skills between SSOT locations. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub struct MigrationResult { + pub migrated_count: usize, + pub skipped_count: usize, + pub errors: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct StorageMigrationJournal { + source: SkillStorageLocation, + target: SkillStorageLocation, + token: String, + hashes: HashMap, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum MigrationDeploymentAction { + Refresh, + AlreadyCurrent, +} + /// Explicit app matrix submitted when importing unmanaged skills. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -585,63 +625,155 @@ impl SkillService { // Paths // --------------------------------------------------------------------- + fn ssot_dir_for(location: SkillStorageLocation) -> Result { + match location { + SkillStorageLocation::CcSwitch => Ok(get_app_config_dir().join("skills")), + SkillStorageLocation::Unified => crate::config::home_dir() + .map(|home| home.join(".agents").join("skills")) + .ok_or_else(|| { + AppError::Message(format_skill_error( + "GET_HOME_DIR_FAILED", + &[], + Some("checkPermission"), + )) + }), + } + } + + fn validate_storage_root(location: SkillStorageLocation, root: &Path) -> Result<(), AppError> { + if location != SkillStorageLocation::Unified { + return Ok(()); + } + + let agents_dir = root.parent().ok_or_else(|| { + AppError::InvalidInput(format!( + "Invalid Unified Skill storage path: {}", + root.display() + )) + })?; + for path in [agents_dir, root] { + match fs::symlink_metadata(path) { + Ok(metadata) if metadata.file_type().is_symlink() => { + return Err(AppError::InvalidInput(format!( + "Unified Skill storage cannot use a symbolic link: {}", + path.display() + ))); + } + Ok(metadata) if !metadata.is_dir() => { + return Err(AppError::InvalidInput(format!( + "Unified Skill storage requires a directory: {}", + path.display() + ))); + } + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(AppError::io(path, error)), + } + } + Ok(()) + } + pub fn get_ssot_dir() -> Result { - let dir = get_app_config_dir().join("skills"); + let location = crate::settings::get_skill_storage_location(); + let dir = Self::ssot_dir_for(location)?; + Self::validate_storage_root(location, &dir)?; create_managed_config_dir_all(&dir)?; + Self::validate_storage_root(location, &dir)?; Ok(dir) } pub fn get_app_skills_dir(app: &AppType) -> Result { - // Override directories follow the same pattern as upstream: /skills - match app { - AppType::Claude => { - if let Some(custom) = crate::settings::get_claude_override_dir() { - return Ok(custom.join("skills")); - } - } - AppType::Codex => { - if let Some(custom) = crate::settings::get_codex_override_dir() { - return Ok(custom.join("skills")); - } - } - AppType::Gemini => { - if let Some(custom) = crate::settings::get_gemini_override_dir() { - return Ok(custom.join("skills")); - } - } - AppType::OpenCode => { - if let Some(custom) = crate::settings::get_opencode_override_dir() { - return Ok(custom.join("skills")); - } - } - AppType::Hermes => { - if let Some(custom) = crate::settings::get_hermes_override_dir() { - return Ok(custom.join("skills")); + // Reuse each app's authoritative config-dir resolver so settings and + // environment overrides (notably CLAUDE_CONFIG_DIR/CODEX_HOME) agree. + Ok(match app { + AppType::Claude => crate::config::get_claude_config_dir().join("skills"), + AppType::Codex => crate::codex_config::get_codex_config_dir().join("skills"), + AppType::Gemini => crate::gemini_config::get_gemini_dir().join("skills"), + AppType::OpenCode => crate::opencode_config::get_opencode_dir().join("skills"), + AppType::Hermes => crate::hermes_config::get_hermes_dir().join("skills"), + AppType::OpenClaw => crate::openclaw_config::get_openclaw_dir().join("skills"), + }) + } + + fn comparable_path(path: &Path) -> PathBuf { + fn normalize(path: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + let _ = normalized.pop(); + } + Component::Prefix(_) | Component::RootDir | Component::Normal(_) => { + normalized.push(component.as_os_str()); + } } } - AppType::OpenClaw => { - if let Some(custom) = crate::settings::get_openclaw_override_dir() { - return Ok(custom.join("skills")); + normalized + } + + let path = if path.is_absolute() { + path.to_path_buf() + } else { + std::env::current_dir() + .unwrap_or_else(|_| PathBuf::from(".")) + .join(path) + }; + + // Resolve the longest existing prefix before applying a remaining + // suffix. In particular, `link/..` must follow `link` first instead of + // being collapsed lexically, matching filesystem path resolution. + for ancestor in path.ancestors() { + if let Ok(mut resolved) = ancestor.canonicalize() { + let suffix = path + .strip_prefix(ancestor) + .unwrap_or_else(|_| Path::new("")); + for component in suffix.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + let _ = resolved.pop(); + } + Component::Prefix(_) | Component::RootDir | Component::Normal(_) => { + resolved.push(component.as_os_str()); + } + } } + return normalize(&resolved); } } - let home = dirs::home_dir().ok_or_else(|| { - AppError::Message(format_skill_error( - "GET_HOME_DIR_FAILED", - &[], - Some("checkPermission"), - )) - })?; + normalize(&path) + } - Ok(match app { - AppType::Claude => home.join(".claude").join("skills"), - AppType::Codex => home.join(".codex").join("skills"), - AppType::Gemini => home.join(".gemini").join("skills"), - AppType::OpenCode => home.join(".config").join("opencode").join("skills"), - AppType::Hermes => home.join(".hermes").join("skills"), - AppType::OpenClaw => home.join(".openclaw").join("skills"), - }) + fn paths_equal(left: &Path, right: &Path) -> bool { + Self::comparable_path(left) == Self::comparable_path(right) + } + + fn paths_overlap(left: &Path, right: &Path) -> bool { + let left = Self::comparable_path(left); + let right = Self::comparable_path(right); + left.starts_with(&right) || right.starts_with(&left) + } + + fn get_distinct_app_skills_dir(ssot_dir: &Path, app: &AppType) -> Result { + let app_dir = Self::get_app_skills_dir(app)?; + if Self::paths_overlap(ssot_dir, &app_dir) { + return Err(AppError::InvalidInput(format!( + "Skill storage directory cannot overlap the {} Skills directory: {} and {}", + app.as_str(), + ssot_dir.display(), + app_dir.display() + ))); + } + Ok(app_dir) + } + + fn validate_skill_storage_destination(ssot_dir: &Path) -> Result<(), AppError> { + for app in Self::supported_skill_apps() { + Self::get_distinct_app_skills_dir(ssot_dir, &app)?; + } + Ok(()) } // --------------------------------------------------------------------- @@ -923,7 +1055,7 @@ impl SkillService { ))); } - let app_dir = Self::get_app_skills_dir(app)?; + let app_dir = Self::get_distinct_app_skills_dir(&ssot_dir, app)?; // D5: allow creating target app dirs during skills sync. fs::create_dir_all(&app_dir).map_err(|e| AppError::io(&app_dir, e))?; @@ -958,13 +1090,14 @@ impl SkillService { return Ok(()); } - let source = Self::get_ssot_dir()?.join(directory); + let ssot_dir = Self::get_ssot_dir()?; + let source = ssot_dir.join(directory); if !source.is_dir() { return Err(AppError::Message(format!( "Skill does not exist in SSOT: {directory}" ))); } - let app_dir = Self::get_app_skills_dir(app)?; + let app_dir = Self::get_distinct_app_skills_dir(&ssot_dir, app)?; fs::create_dir_all(&app_dir).map_err(|e| AppError::io(&app_dir, e))?; let dest = app_dir.join(directory); if source == dest { @@ -1018,7 +1151,8 @@ impl SkillService { return Ok(()); } - let app_dir = Self::get_app_skills_dir(app)?; + let ssot_dir = Self::get_ssot_dir()?; + let app_dir = Self::get_distinct_app_skills_dir(&ssot_dir, app)?; let path = app_dir.join(directory); if path.exists() || Self::is_symlink(&path) { Self::remove_path(&path)?; @@ -1067,6 +1201,628 @@ impl SkillService { Ok(()) } + fn migration_tree_hash_with_ignored_root_file( + dir: &Path, + ignored_root_file: Option<&OsStr>, + ) -> Result { + use sha2::{Digest, Sha256}; + + fn update_framed(hasher: &mut Sha256, bytes: &[u8]) { + hasher.update((bytes.len() as u64).to_le_bytes()); + hasher.update(bytes); + } + + fn visit( + root: &Path, + current: &Path, + ignored_root_file: Option<&OsStr>, + hasher: &mut Sha256, + ) -> Result<(), AppError> { + let mut entries = fs::read_dir(current) + .map_err(|error| AppError::io(current, error))? + .collect::, _>>() + .map_err(|error| AppError::io(current, error))?; + entries.sort_by_key(|entry| entry.file_name()); + + for entry in entries { + let path = entry.path(); + if current == root + && ignored_root_file.is_some_and(|ignored| entry.file_name() == ignored) + { + continue; + } + let relative = path.strip_prefix(root).unwrap_or(&path); + let file_type = entry + .file_type() + .map_err(|error| AppError::io(&path, error))?; + if file_type.is_symlink() { + return Err(AppError::InvalidInput(format!( + "Skill storage migration does not follow symbolic links: {}", + path.display() + ))); + } + if file_type.is_dir() { + hasher.update(b"D"); + update_framed(hasher, relative.as_os_str().as_encoded_bytes()); + visit(root, &path, ignored_root_file, hasher)?; + } else if file_type.is_file() { + hasher.update(b"F"); + update_framed(hasher, relative.as_os_str().as_encoded_bytes()); + let mut file = + fs::File::open(&path).map_err(|error| AppError::io(&path, error))?; + let mut content_hasher = Sha256::new(); + let mut content_len = 0u64; + let mut buffer = [0u8; 16 * 1024]; + loop { + let read = file + .read(&mut buffer) + .map_err(|error| AppError::io(&path, error))?; + if read == 0 { + break; + } + content_len = content_len.saturating_add(read as u64); + content_hasher.update(&buffer[..read]); + } + hasher.update(content_len.to_le_bytes()); + hasher.update(content_hasher.finalize()); + } else { + return Err(AppError::InvalidInput(format!( + "Unsupported file type in Skill storage migration: {}", + path.display() + ))); + } + } + Ok(()) + } + + let metadata = fs::symlink_metadata(dir).map_err(|error| AppError::io(dir, error))?; + if metadata.file_type().is_symlink() || !metadata.is_dir() { + return Err(AppError::InvalidInput(format!( + "Skill storage migration requires a real directory: {}", + dir.display() + ))); + } + let mut hasher = Sha256::new(); + hasher.update(b"cc-switch-skill-tree-v2\0"); + visit(dir, dir, ignored_root_file, &mut hasher)?; + Ok(format!("{:x}", hasher.finalize())) + } + + fn migration_tree_hash(dir: &Path) -> Result { + Self::migration_tree_hash_with_ignored_root_file(dir, None) + } + + pub(crate) fn validate_managed_skill_tree(dir: &Path) -> Result<(), AppError> { + Self::migration_tree_hash(dir).map(|_| ()) + } + + fn migration_tree_hash_if_present(path: &Path) -> Result, AppError> { + match fs::symlink_metadata(path) { + Ok(_) => Self::migration_tree_hash(path).map(Some), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(AppError::io(path, error)), + } + } + + fn copy_migration_tree(src: &Path, dest: &Path) -> Result<(), AppError> { + let metadata = fs::symlink_metadata(src).map_err(|error| AppError::io(src, error))?; + if metadata.file_type().is_symlink() || !metadata.is_dir() { + return Err(AppError::InvalidInput(format!( + "Skill storage migration requires a real directory: {}", + src.display() + ))); + } + + fs::create_dir(dest).map_err(|error| AppError::io(dest, error))?; + let mut entries = fs::read_dir(src) + .map_err(|error| AppError::io(src, error))? + .collect::, _>>() + .map_err(|error| AppError::io(src, error))?; + entries.sort_by_key(|entry| entry.file_name()); + for entry in entries { + let source = entry.path(); + let target = dest.join(entry.file_name()); + let file_type = entry + .file_type() + .map_err(|error| AppError::io(&source, error))?; + if file_type.is_symlink() { + return Err(AppError::InvalidInput(format!( + "Skill storage migration does not follow symbolic links: {}", + source.display() + ))); + } + if file_type.is_dir() { + Self::copy_migration_tree(&source, &target)?; + } else if file_type.is_file() { + fs::copy(&source, &target).map_err(|error| AppError::io(&target, error))?; + } else { + return Err(AppError::InvalidInput(format!( + "Unsupported file type in Skill storage migration: {}", + source.display() + ))); + } + } + Ok(()) + } + + fn migration_marker_name(journal: &StorageMigrationJournal) -> String { + format!(".cc-switch-migration-{}", journal.token) + } + + fn migration_marker_matches( + target: &Path, + journal: &StorageMigrationJournal, + ) -> Result { + let marker = target.join(Self::migration_marker_name(journal)); + match fs::symlink_metadata(&marker) { + Ok(metadata) if metadata.is_file() && !metadata.file_type().is_symlink() => { + let token = + fs::read_to_string(&marker).map_err(|error| AppError::io(&marker, error))?; + if token == journal.token { + Ok(true) + } else { + Err(AppError::InvalidInput(format!( + "Skill migration marker does not match the active migration: {}", + marker.display() + ))) + } + } + Ok(_) => Err(AppError::InvalidInput(format!( + "Invalid Skill migration marker: {}", + marker.display() + ))), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(AppError::io(&marker, error)), + } + } + + fn migration_target_hash( + target: &Path, + journal: &StorageMigrationJournal, + ) -> Result { + if Self::migration_marker_matches(target, journal)? { + let marker_name = Self::migration_marker_name(journal); + Self::migration_tree_hash_with_ignored_root_file(target, Some(OsStr::new(&marker_name))) + } else { + Self::migration_tree_hash(target) + } + } + + fn stage_migration_copy( + src: &Path, + dest: &Path, + target_root: &Path, + journal: &StorageMigrationJournal, + ) -> Result<(), AppError> { + let staging = tempfile::Builder::new() + .prefix(".cc-switch-skill-migration-") + .tempdir_in(target_root) + .map_err(|error| AppError::io(target_root, error))?; + let next = staging.path().join("next"); + Self::copy_migration_tree(src, &next)?; + if Self::migration_tree_hash(src)? != Self::migration_tree_hash(&next)? { + return Err(AppError::Message(format!( + "Skill changed while it was being copied: {}", + src.display() + ))); + } + let marker = next.join(Self::migration_marker_name(journal)); + let mut marker_file = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&marker) + .map_err(|error| AppError::io(&marker, error))?; + marker_file + .write_all(journal.token.as_bytes()) + .map_err(|error| AppError::io(&marker, error))?; + marker_file + .flush() + .map_err(|error| AppError::io(&marker, error))?; + fs::rename(&next, dest).map_err(|error| AppError::io(dest, error))?; + Ok(()) + } + + fn migration_deployment_action( + destination: &Path, + old_source: &Path, + new_source: &Path, + expected_hash: &str, + ) -> Result { + let metadata = match fs::symlink_metadata(destination) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(MigrationDeploymentAction::Refresh); + } + Err(error) => return Err(AppError::io(destination, error)), + }; + if metadata.file_type().is_symlink() { + let target = + fs::read_link(destination).map_err(|error| AppError::io(destination, error))?; + let target = if target.is_absolute() { + target + } else { + destination + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(target) + }; + if Self::paths_equal(&target, old_source) || Self::paths_equal(&target, new_source) { + return Ok(MigrationDeploymentAction::Refresh); + } + return Err(AppError::InvalidInput(format!( + "Refusing to replace an unmanaged Skill deployment: {}", + destination.display() + ))); + } + if metadata.is_dir() && Self::migration_tree_hash(destination)? == expected_hash { + // An ordinary directory has no ownership proof. If it already has + // the current content, leave it byte-for-byte untouched. + return Ok(MigrationDeploymentAction::AlreadyCurrent); + } + Err(AppError::InvalidInput(format!( + "Refusing to replace an unmanaged Skill deployment: {}", + destination.display() + ))) + } + + fn sync_migrated_skill_to_app( + directory: &str, + app: &AppType, + method: SyncMethod, + old_root: &Path, + new_root: &Path, + ) -> Result<(), AppError> { + let source = new_root.join(directory); + let expected_hash = Self::migration_tree_hash(&source)?; + let app_dir = Self::get_distinct_app_skills_dir(new_root, app)?; + let destination = app_dir.join(directory); + let action = Self::migration_deployment_action( + &destination, + &old_root.join(directory), + &source, + &expected_hash, + )?; + if action == MigrationDeploymentAction::AlreadyCurrent { + return Ok(()); + } + Self::sync_updated_skill_to_app(directory, app, method) + } + + fn storage_migration_journal_path() -> PathBuf { + get_app_config_dir().join(STORAGE_MIGRATION_JOURNAL_FILE) + } + + fn load_storage_migration_journal() -> Result, AppError> { + let path = Self::storage_migration_journal_path(); + let raw = match fs::read_to_string(&path) { + Ok(raw) => raw, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(AppError::io(&path, error)), + }; + serde_json::from_str(&raw).map(Some).map_err(|error| { + AppError::InvalidInput(format!( + "Invalid Skill storage migration journal {}: {error}", + path.display() + )) + }) + } + + fn save_storage_migration_journal(journal: &StorageMigrationJournal) -> Result<(), AppError> { + write_json_file(&Self::storage_migration_journal_path(), journal) + } + + fn clear_storage_migration_journal() -> Result<(), AppError> { + let path = Self::storage_migration_journal_path(); + match fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(AppError::io(&path, error)), + } + } + + fn ensure_journal_matches_index( + journal: &StorageMigrationJournal, + index: &SkillsIndex, + ) -> Result<(), AppError> { + if journal.hashes.len() == index.skills.len() + && index + .skills + .keys() + .all(|directory| journal.hashes.contains_key(directory)) + { + return Ok(()); + } + Err(AppError::InvalidInput( + "Managed Skills changed during an unfinished storage migration; finish or recover that migration before installing or removing Skills" + .to_string(), + )) + } + + fn remove_storage_migration_markers( + root: &Path, + index: &SkillsIndex, + journal: &StorageMigrationJournal, + ) -> Result<(), AppError> { + let marker_name = Self::migration_marker_name(journal); + for skill in index.skills.values() { + let target = root.join(&skill.directory); + if Self::migration_marker_matches(&target, journal)? { + let marker = target.join(&marker_name); + fs::remove_file(&marker).map_err(|error| AppError::io(&marker, error))?; + } + } + Ok(()) + } + + /// Move managed Skills between the CC Switch and shared Agent Skills SSOTs. + /// Target copies are verified before the setting changes. Old copies remain + /// available until every enabled app deployment has been refreshed. + pub fn migrate_storage(target: SkillStorageLocation) -> Result { + let current = crate::settings::get_skill_storage_location(); + let mut journal = Self::load_storage_migration_journal()?; + if current == target && journal.is_none() { + return Ok(MigrationResult::default()); + } + + let new_dir = Self::ssot_dir_for(target)?; + let old_location = match target { + SkillStorageLocation::CcSwitch => SkillStorageLocation::Unified, + SkillStorageLocation::Unified => SkillStorageLocation::CcSwitch, + }; + let old_dir = Self::ssot_dir_for(old_location)?; + Self::validate_storage_root(old_location, &old_dir)?; + Self::validate_storage_root(target, &new_dir)?; + if Self::paths_overlap(&old_dir, &new_dir) { + return Err(AppError::InvalidInput(format!( + "Skill storage directories cannot be equal or overlap: {} and {}", + old_dir.display(), + new_dir.display() + ))); + } + Self::validate_skill_storage_destination(&old_dir)?; + Self::validate_skill_storage_destination(&new_dir)?; + create_managed_config_dir_all(&new_dir)?; + Self::validate_storage_root(target, &new_dir)?; + + let index = Self::load_index()?; + if let Some(active) = &journal { + if active.source != old_location + || active.target != target + || (current != active.source && current != active.target) + { + return Err(AppError::InvalidInput(format!( + "An unfinished Skill storage migration from {:?} to {:?} must be reconciled before starting another migration", + active.source, active.target + ))); + } + if current == active.source { + Self::ensure_journal_matches_index(active, &index)?; + } + } + + let mut copies = Vec::new(); + let mut result = MigrationResult::default(); + + // A first switch never infers ownership from content equality. Persist + // an intent before copying; an embedded token then proves which target + // directories were created by an interrupted attempt. + if journal.is_none() && current != target { + let mut hashes = HashMap::new(); + for skill in index.skills.values() { + Self::validate_update_directory(&skill.directory)?; + let src = old_dir.join(&skill.directory); + let dst = new_dir.join(&skill.directory); + let source_hash = Self::migration_tree_hash_if_present(&src)?.ok_or_else(|| { + AppError::InvalidInput(format!( + "Managed Skill is missing from the current storage: {}", + skill.directory + )) + })?; + match fs::symlink_metadata(&dst) { + Ok(_) => { + return Err(AppError::InvalidInput(format!( + "Refusing to claim a pre-existing Skill migration target: {}", + dst.display() + ))); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(AppError::io(&dst, error)), + } + hashes.insert(skill.directory.clone(), source_hash); + } + let next = StorageMigrationJournal { + source: old_location, + target, + token: uuid::Uuid::new_v4().to_string(), + hashes, + }; + Self::save_storage_migration_journal(&next)?; + journal = Some(next); + } + + // Preflight the complete managed set before writing a target directory. + for skill in index.skills.values() { + Self::validate_update_directory(&skill.directory)?; + let src = old_dir.join(&skill.directory); + let dst = new_dir.join(&skill.directory); + match Self::migration_tree_hash_if_present(&dst)? { + Some(_) if current == target => { + if let Some(active) = &journal { + if Self::migration_marker_matches(&dst, active)? { + let expected = + active.hashes.get(&skill.directory).ok_or_else(|| { + AppError::InvalidInput(format!( + "Skill is missing from the active migration journal: {}", + skill.directory + )) + })?; + if &Self::migration_target_hash(&dst, active)? != expected { + return Err(AppError::InvalidInput(format!( + "Interrupted Skill migration target changed: {}", + dst.display() + ))); + } + } + } + } + Some(_) => { + let active = journal.as_ref().ok_or_else(|| { + AppError::InvalidInput(format!( + "Refusing to claim a pre-existing Skill migration target: {}", + dst.display() + )) + })?; + if !Self::migration_marker_matches(&dst, active)? { + return Err(AppError::InvalidInput(format!( + "Refusing to claim a pre-existing Skill migration target: {}", + dst.display() + ))); + } + let expected = active.hashes.get(&skill.directory).ok_or_else(|| { + AppError::InvalidInput(format!( + "Skill is missing from the active migration journal: {}", + skill.directory + )) + })?; + if &Self::migration_target_hash(&dst, active)? != expected { + return Err(AppError::InvalidInput(format!( + "Interrupted Skill migration target changed: {}", + dst.display() + ))); + } + if Self::migration_tree_hash(&src)? != *expected { + return Err(AppError::InvalidInput(format!( + "Skill changed during an unfinished storage migration: {}", + skill.directory + ))); + } + result.skipped_count += 1; + } + None => { + let active = journal.as_ref().ok_or_else(|| { + AppError::InvalidInput(format!( + "Managed Skill is missing from both storage locations: {}", + skill.directory + )) + })?; + let expected = active.hashes.get(&skill.directory).ok_or_else(|| { + AppError::InvalidInput(format!( + "Skill is missing from the active migration journal: {}", + skill.directory + )) + })?; + if Self::migration_tree_hash(&src)? != *expected { + return Err(AppError::InvalidInput(format!( + "Skill changed during an unfinished storage migration: {}", + skill.directory + ))); + } + copies.push((src, dst)); + } + } + } + + for (src, dst) in copies { + let active = journal + .as_ref() + .expect("copies require a migration journal"); + Self::stage_migration_copy(&src, &dst, &new_dir, active)?; + result.migrated_count += 1; + } + + if current != target { + crate::settings::set_skill_storage_location(target)?; + } + if let Some(active) = &journal { + Self::remove_storage_migration_markers(&new_dir, &index, active)?; + } + + // Reconcile per Skill/app through the existing staged deployment path. + // Unknown destinations are preserved and reported instead of overwritten. + for skill in index.skills.values() { + for app in Self::supported_skill_apps() { + if skill.apps.is_enabled_for(&app) { + if let Err(error) = Self::sync_migrated_skill_to_app( + &skill.directory, + &app, + index.sync_method, + &old_dir, + &new_dir, + ) { + result.errors.push(format!( + "{}/{}: {error}", + app.as_str(), + skill.directory + )); + } + } + } + } + + // Keep the old SSOT as a working fallback whenever deployment is partial. + // A same-target retry re-enters this path, repairs deployment, then cleans it. + if result.errors.is_empty() { + if let Some(active) = journal.as_ref() { + for skill in index.skills.values() { + let Some(expected_source_hash) = active.hashes.get(&skill.directory) else { + // This Skill was added after settings moved to the target; + // it has no old migration-owned copy to clean up. + continue; + }; + let source = old_dir.join(&skill.directory); + let target_path = new_dir.join(&skill.directory); + let source_hash = match Self::migration_tree_hash_if_present(&source) { + Ok(Some(hash)) => hash, + Ok(None) => continue, + Err(error) => { + result.errors.push(format!( + "{}: could not verify the old copy; it was preserved: {error}", + skill.directory + )); + continue; + } + }; + let target_hash = match Self::migration_tree_hash(&target_path) { + Ok(hash) => hash, + Err(error) => { + result.errors.push(format!( + "{}: could not verify the new copy; the old copy was preserved: {error}", + skill.directory + )); + continue; + } + }; + let source_is_original = source_hash == *expected_source_hash; + if (current == active.target && !source_is_original) + || (current != active.target && source_hash != target_hash) + { + result.errors.push(format!( + "{}: old and new copies differ; the old copy was preserved", + skill.directory + )); + continue; + } + if let Err(error) = Self::remove_path(&source) { + result.errors.push(format!( + "{}: failed to remove the old copy: {error}", + skill.directory + )); + } + } + } + } + + if result.errors.is_empty() && journal.is_some() { + if let Err(error) = Self::clear_storage_migration_journal() { + result.errors.push(format!( + "failed to clear the completed Skill storage migration journal: {error}" + )); + } + } + + Ok(result) + } + pub fn list_installed() -> Result, AppError> { let mut index = Self::load_index()?; let _ = Self::migrate_ssot_if_pending(&mut index)?; @@ -1137,6 +1893,27 @@ impl SkillService { ))) } + fn reject_unmanaged_install_collision( + destination: &Path, + directory: &str, + new_repo: &str, + ) -> Result<(), AppError> { + if crate::settings::get_skill_storage_location() == SkillStorageLocation::Unified + && fs::symlink_metadata(destination).is_ok() + { + return Err(AppError::Message(format_skill_error( + "SKILL_DIRECTORY_CONFLICT", + &[ + ("directory", directory), + ("existing_repo", "unmanaged local directory"), + ("new_repo", new_repo), + ], + Some("importOrUninstallFirst"), + ))); + } + Ok(()) + } + fn source_path_from_readme( skill: &InstalledSkill, downloaded_branch: Option<&str>, @@ -1759,6 +2536,8 @@ impl SkillService { // Ensure SSOT dir and install files. let ssot_dir = Self::get_ssot_dir()?; let dest = ssot_dir.join(&install_name); + let new_repo = format!("{}/{}", discoverable.repo_owner, discoverable.repo_name); + Self::reject_unmanaged_install_collision(&dest, &install_name, &new_repo)?; let mut installed_branch = discoverable.repo_branch.clone(); let mut installed_readme_url = discoverable.readme_url.clone(); if !dest.exists() { @@ -2551,6 +3330,137 @@ mod tests { assert!(SkillService::validate_update_directory("").is_err()); } + #[test] + fn migration_tree_hash_has_unambiguous_entry_framing() { + let temp = tempfile::tempdir().expect("create hash fixtures"); + let first = temp.path().join("first"); + let second = temp.path().join("second"); + fs::create_dir_all(&first).expect("create first tree"); + fs::create_dir_all(&second).expect("create second tree"); + fs::write(first.join("a"), b"X").expect("write first a"); + fs::write(first.join("b"), b"Y").expect("write first b"); + fs::write(second.join("a"), b"X\0b\0file\0Y").expect("write framed collision fixture"); + + assert_ne!( + SkillService::migration_tree_hash(&first).expect("hash first tree"), + SkillService::migration_tree_hash(&second).expect("hash second tree") + ); + } + + #[cfg(unix)] + #[test] + fn migration_tree_hash_preserves_non_utf8_names() { + use std::os::unix::ffi::OsStringExt; + + let temp = tempfile::tempdir().expect("create hash fixtures"); + let first = temp.path().join("first"); + let second = temp.path().join("second"); + fs::create_dir_all(&first).expect("create first tree"); + fs::create_dir_all(&second).expect("create second tree"); + fs::write( + first.join(std::ffi::OsString::from_vec(vec![b'a', 0x80])), + b"same", + ) + .expect("write first non-UTF8 file"); + fs::write( + second.join(std::ffi::OsString::from_vec(vec![b'a', 0x81])), + b"same", + ) + .expect("write second non-UTF8 file"); + + assert_ne!( + SkillService::migration_tree_hash(&first).expect("hash first tree"), + SkillService::migration_tree_hash(&second).expect("hash second tree") + ); + } + + #[test] + #[serial_test::serial(home_settings)] + fn storage_migration_rejects_physically_identical_roots() { + let temp = tempfile::tempdir().expect("create isolated home"); + let _env = crate::test_support::TestEnvGuard::isolated(temp.path()); + std::env::set_var("CC_SWITCH_CONFIG_DIR", temp.path().join(".agents")); + crate::settings::reload_test_settings(); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("the two storage choices resolve to the same root"); + + assert!(!temp.path().join(".agents").join("skills").exists()); + } + + #[test] + #[serial_test::serial(home_settings)] + fn interrupted_storage_copy_resumes_only_with_its_persisted_marker() { + let temp = tempfile::tempdir().expect("create isolated home"); + let _env = crate::test_support::TestEnvGuard::isolated(temp.path()); + let old_root = SkillService::get_ssot_dir().expect("resolve old SSOT"); + let old_skill = old_root.join("managed"); + fs::create_dir_all(&old_skill).expect("create old Skill"); + fs::write(old_skill.join("SKILL.md"), "managed").expect("write old Skill"); + + let db = Database::init().expect("initialize database"); + let mut skill = repository_skill(None); + skill.id = "local:managed".to_string(); + skill.name = "Managed".to_string(); + skill.directory = "managed".to_string(); + skill.repo_owner = None; + skill.repo_name = None; + skill.repo_branch = None; + db.save_skill(&skill).expect("save managed Skill"); + + let new_root = + SkillService::ssot_dir_for(SkillStorageLocation::Unified).expect("resolve target SSOT"); + fs::create_dir_all(&new_root).expect("create target root"); + let new_skill = new_root.join("managed"); + SkillService::copy_migration_tree(&old_skill, &new_skill) + .expect("simulate completed copy before interruption"); + let journal = StorageMigrationJournal { + source: SkillStorageLocation::CcSwitch, + target: SkillStorageLocation::Unified, + token: "interrupted-test-token".to_string(), + hashes: HashMap::from([( + "managed".to_string(), + SkillService::migration_tree_hash(&old_skill).expect("hash source"), + )]), + }; + fs::write( + new_skill.join(SkillService::migration_marker_name(&journal)), + &journal.token, + ) + .expect("write migration marker"); + SkillService::save_storage_migration_journal(&journal).expect("save migration journal"); + + let result = SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect("resume interrupted migration"); + + assert!(result.errors.is_empty()); + assert!(!old_skill.exists()); + assert!(new_skill.join("SKILL.md").is_file()); + assert!(!new_skill + .join(SkillService::migration_marker_name(&journal)) + .exists()); + assert!(SkillService::load_storage_migration_journal() + .expect("read cleared journal") + .is_none()); + } + + #[test] + #[serial_test::serial(home_settings)] + fn unified_install_rejects_an_existing_unmanaged_directory() { + let temp = tempfile::tempdir().expect("create isolated home"); + let _env = crate::test_support::TestEnvGuard::isolated(temp.path()); + let mut settings = crate::settings::AppSettings::default(); + settings.skill_storage_location = SkillStorageLocation::Unified; + crate::settings::update_settings(settings).expect("enable unified storage"); + let destination = SkillService::get_ssot_dir() + .expect("resolve unified storage") + .join("personal"); + fs::create_dir_all(&destination).expect("create unmanaged directory"); + + SkillService::reject_unmanaged_install_collision(&destination, "personal", "owner/repo") + .expect_err("install must not silently claim an unmanaged directory"); + } + #[test] fn skills_sh_api_skill_maps_github_source() { let skill = skills_sh_api_skill_to_discoverable(SkillsShApiSkill { diff --git a/src-tauri/src/services/webdav_sync/archive.rs b/src-tauri/src/services/webdav_sync/archive.rs index 202f30960..a0057cb00 100644 --- a/src-tauri/src/services/webdav_sync/archive.rs +++ b/src-tauri/src/services/webdav_sync/archive.rs @@ -9,7 +9,7 @@ use tempfile::{tempdir, TempDir}; use zip::{write::SimpleFileOptions, DateTime}; use crate::error::AppError; -use crate::services::skill::SkillService; +use crate::services::skill::{SkillService, SkillStorageLocation}; const MAX_ZIP_ENTRIES: usize = 10_000; const MAX_ZIP_EXTRACT_BYTES: u64 = 512 * 1024 * 1024; // 512 MB @@ -45,6 +45,7 @@ pub struct SkillsBackup { impl SkillsBackup { pub fn backup_current_skills() -> Result { let ssot = SkillService::get_ssot_dir()?; + ensure_unified_ssot_is_fully_managed(&ssot)?; let tmp = tempdir().map_err(|e| { io_context_localized( "webdav.sync.skills_backup_tmpdir_failed", @@ -76,12 +77,54 @@ impl SkillsBackup { } } +fn unified_storage_enabled() -> bool { + crate::settings::get_skill_storage_location() == SkillStorageLocation::Unified +} + +fn ensure_unified_ssot_is_fully_managed(ssot: &Path) -> Result<(), AppError> { + if !unified_storage_enabled() || !ssot.exists() { + return Ok(()); + } + let managed = SkillService::load_index()?.skills; + for entry in fs::read_dir(ssot).map_err(|error| AppError::io(ssot, error))? { + let entry = entry.map_err(|error| AppError::io(ssot, error))?; + let file_name = entry.file_name(); + let name = file_name.to_str().ok_or_else(|| { + AppError::localized( + "sync.unified_skills_non_utf8_name", + format!( + "Unified 技能目录包含无法安全识别的名称：{}", + entry.path().display() + ), + format!( + "Unified Skill storage contains a name that cannot be identified safely: {}", + entry.path().display() + ), + ) + })?; + if !managed.contains_key(name) { + return Err(AppError::localized( + "sync.unified_skills_contains_unmanaged", + format!( + "Unified 技能目录包含未管理目录 {name}，云同步不会替换它。请先显式导入，或切回 CC Switch 存储。" + ), + format!( + "Unified Skill storage contains the unmanaged directory {name}; cloud sync will not replace it. Import it explicitly or use CC Switch storage." + ), + )); + } + SkillService::validate_managed_skill_tree(&entry.path())?; + } + Ok(()) +} + // --------------------------------------------------------------------------- // ZIP 打包 // --------------------------------------------------------------------------- pub fn zip_skills_ssot(dest_path: &Path) -> Result<(), AppError> { let source = SkillService::get_ssot_dir()?; + ensure_unified_ssot_is_fully_managed(&source)?; if let Some(parent) = dest_path.parent() { fs::create_dir_all(parent).map_err(|e| AppError::io(parent, e))?; } @@ -289,8 +332,20 @@ pub fn restore_skills_zip(raw: &[u8]) -> Result<(), AppError> { } let ssot = SkillService::get_ssot_dir()?; + ensure_unified_ssot_is_fully_managed(&ssot)?; let bak = ssot.with_extension("bak"); + if unified_storage_enabled() && bak.exists() { + return Err(AppError::localized( + "sync.unified_skills_backup_collision", + format!("Unified 技能恢复不会覆盖已有备份目录：{}", bak.display()), + format!( + "Unified Skill restore will not replace the existing backup directory: {}", + bak.display() + ), + )); + } + // 原子替换：先 rename 到 .bak，再 copy，失败则回滚 if ssot.exists() { if bak.exists() { @@ -392,6 +447,7 @@ fn copy_dir_recursive_inner( #[cfg(test)] mod tests { use super::*; + use crate::settings::{update_settings, AppSettings}; use tempfile::tempdir; #[test] @@ -473,4 +529,106 @@ mod tests { "should not write when the first chunk exceeds limit" ); } + + #[test] + #[serial_test::serial(home_settings)] + fn unified_archive_refuses_to_touch_unmanaged_directories() { + let temp = tempdir().expect("create isolated home"); + let _env = crate::test_support::TestEnvGuard::isolated(temp.path()); + let mut settings = AppSettings::default(); + settings.skill_storage_location = SkillStorageLocation::Unified; + update_settings(settings).expect("enable unified Skill storage"); + + let ssot = SkillService::get_ssot_dir().expect("resolve unified SSOT"); + fs::create_dir_all(ssot.join("personal")).expect("create unmanaged Skill"); + fs::write(ssot.join("personal/SKILL.md"), "personal").expect("write unmanaged Skill"); + + let snapshot = temp.path().join("skills.zip"); + let error = zip_skills_ssot(&snapshot) + .expect_err("cloud archive must not claim an unmanaged unified directory"); + + assert!(error.to_string().contains("personal")); + assert_eq!( + fs::read_to_string(ssot.join("personal/SKILL.md")) + .expect("read preserved unmanaged Skill"), + "personal" + ); + + let replacement = temp.path().join("replacement.zip"); + let file = fs::File::create(&replacement).expect("create replacement archive"); + zip::ZipWriter::new(file) + .finish() + .expect("finish replacement archive"); + restore_skills_zip(&fs::read(&replacement).expect("read replacement archive")) + .expect_err("cloud restore must not replace an unmanaged unified directory"); + assert_eq!( + fs::read_to_string(ssot.join("personal/SKILL.md")) + .expect("read preserved unmanaged Skill after restore refusal"), + "personal" + ); + + fs::remove_dir_all(ssot.join("personal")).expect("remove unmanaged fixture"); + let sibling_backup = ssot.with_extension("bak"); + fs::create_dir_all(&sibling_backup).expect("create external sibling backup"); + fs::write(sibling_backup.join("keep.txt"), "external").expect("write external backup"); + restore_skills_zip(&fs::read(&replacement).expect("read replacement archive")) + .expect_err("Unified restore must not remove a pre-existing sibling backup"); + assert_eq!( + fs::read_to_string(sibling_backup.join("keep.txt")) + .expect("read preserved sibling backup"), + "external" + ); + } + + #[cfg(unix)] + #[test] + #[serial_test::serial(home_settings)] + fn unified_archive_rejects_links_and_special_files_inside_managed_skills() { + use std::os::unix::fs::symlink; + use std::os::unix::net::UnixListener; + + let temp = tempdir().expect("create isolated home"); + let _env = crate::test_support::TestEnvGuard::isolated(temp.path()); + let mut settings = AppSettings::default(); + settings.skill_storage_location = SkillStorageLocation::Unified; + update_settings(settings).expect("enable unified Skill storage"); + + let ssot = SkillService::get_ssot_dir().expect("resolve unified SSOT"); + let managed = ssot.join("managed"); + fs::create_dir_all(&managed).expect("create managed Skill"); + fs::write(managed.join("SKILL.md"), "managed").expect("write managed Skill"); + let db = crate::database::Database::init().expect("initialize database"); + db.save_skill(&crate::app_config::InstalledSkill { + id: "local:managed".to_string(), + name: "Managed".to_string(), + description: None, + directory: "managed".to_string(), + readme_url: None, + repo_owner: None, + repo_name: None, + repo_branch: None, + apps: crate::app_config::SkillApps::default(), + installed_at: 0, + content_hash: None, + updated_at: 0, + }) + .expect("register managed Skill"); + + let external = temp.path().join("external.txt"); + fs::write(&external, "external").expect("write external fixture"); + let nested_link = managed.join("external-link"); + symlink(&external, &nested_link).expect("create nested symlink"); + let snapshot = temp.path().join("linked.zip"); + zip_skills_ssot(&snapshot).expect_err("archive must reject nested symlinks"); + assert_eq!( + fs::read_to_string(&external).expect("read external file"), + "external" + ); + + fs::remove_file(&nested_link).expect("remove symlink fixture"); + let socket_path = managed.join("special.sock"); + let _listener = UnixListener::bind(&socket_path).expect("create special file fixture"); + zip_skills_ssot(&snapshot).expect_err("archive must reject special files"); + assert!(socket_path.exists()); + } } diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index fc9169207..f926221a6 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -578,6 +578,9 @@ pub struct AppSettings { /// Skills 同步方式（auto|symlink|copy） #[serde(default)] pub skill_sync_method: crate::services::skill::SyncMethod, + /// Skills SSOT location: cc_switch (default) or unified (~/.agents/skills/). + #[serde(default)] + pub skill_storage_location: crate::services::skill::SkillStorageLocation, #[serde(default, skip_serializing_if = "Option::is_none")] pub security: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -646,6 +649,7 @@ impl Default for AppSettings { unify_codex_migrate_existing: None, usage_auto_sync: default_usage_auto_sync(), skill_sync_method: crate::services::skill::SyncMethod::default(), + skill_storage_location: crate::services::skill::SkillStorageLocation::default(), security: None, webdav_sync: None, s3_sync: None, @@ -1252,6 +1256,21 @@ pub fn set_skill_sync_method(method: crate::services::skill::SyncMethod) -> Resu update_settings(settings) } +pub fn get_skill_storage_location() -> crate::services::skill::SkillStorageLocation { + settings_store() + .read() + .map(|settings| settings.skill_storage_location) + .unwrap_or_default() +} + +pub fn set_skill_storage_location( + location: crate::services::skill::SkillStorageLocation, +) -> Result<(), AppError> { + let mut settings = get_settings(); + settings.skill_storage_location = location; + update_settings(settings) +} + pub fn get_webdav_sync_settings() -> Option { settings_store() .read() diff --git a/src-tauri/src/test_support.rs b/src-tauri/src/test_support.rs index a72a17482..a102cb853 100644 --- a/src-tauri/src/test_support.rs +++ b/src-tauri/src/test_support.rs @@ -36,6 +36,7 @@ pub(crate) struct TestEnvGuard { home: PathBuf, old_home: Option, old_userprofile: Option, + old_test_home: Option, old_cc_switch_config_dir: Option, old_claude_config_dir: Option, old_codex_home: Option, @@ -47,6 +48,7 @@ impl TestEnvGuard { let lock = lock_test_home_and_settings(); let old_home = std::env::var_os("HOME"); let old_userprofile = std::env::var_os("USERPROFILE"); + let old_test_home = std::env::var_os("CC_SWITCH_TEST_HOME"); let old_cc_switch_config_dir = std::env::var_os("CC_SWITCH_CONFIG_DIR"); let old_claude_config_dir = std::env::var_os("CLAUDE_CONFIG_DIR"); let old_codex_home = std::env::var_os("CODEX_HOME"); @@ -54,6 +56,7 @@ impl TestEnvGuard { std::env::set_var("HOME", home); std::env::set_var("USERPROFILE", home); + std::env::set_var("CC_SWITCH_TEST_HOME", home); std::env::set_var("CC_SWITCH_CONFIG_DIR", home.join(".cc-switch")); std::env::set_var("CLAUDE_CONFIG_DIR", home.join(".claude")); std::env::set_var("CODEX_HOME", home.join(".codex")); @@ -66,6 +69,7 @@ impl TestEnvGuard { home: home.to_path_buf(), old_home, old_userprofile, + old_test_home, old_cc_switch_config_dir, old_claude_config_dir, old_codex_home, @@ -83,6 +87,7 @@ impl Drop for TestEnvGuard { cleanup_test_processes_under(&self.home); restore_env("HOME", &self.old_home); restore_env("USERPROFILE", &self.old_userprofile); + restore_env("CC_SWITCH_TEST_HOME", &self.old_test_home); restore_env("CC_SWITCH_CONFIG_DIR", &self.old_cc_switch_config_dir); restore_env("CLAUDE_CONFIG_DIR", &self.old_claude_config_dir); restore_env("CODEX_HOME", &self.old_codex_home); diff --git a/src-tauri/tests/settings_current_provider.rs b/src-tauri/tests/settings_current_provider.rs index 79279c282..4143feda6 100644 --- a/src-tauri/tests/settings_current_provider.rs +++ b/src-tauri/tests/settings_current_provider.rs @@ -164,6 +164,14 @@ mod services { Symlink, Copy, } + + #[derive(Debug, Clone, Copy, Serialize, Deserialize, Default)] + #[serde(rename_all = "snake_case")] + pub enum SkillStorageLocation { + #[default] + CcSwitch, + Unified, + } } pub mod webdav { diff --git a/src-tauri/tests/settings_visible_apps.rs b/src-tauri/tests/settings_visible_apps.rs index e5050ed77..221c219fc 100644 --- a/src-tauri/tests/settings_visible_apps.rs +++ b/src-tauri/tests/settings_visible_apps.rs @@ -167,6 +167,14 @@ mod services { Symlink, Copy, } + + #[derive(Debug, Clone, Copy, Serialize, Deserialize, Default)] + #[serde(rename_all = "snake_case")] + pub enum SkillStorageLocation { + #[default] + CcSwitch, + Unified, + } } pub mod webdav { diff --git a/src-tauri/tests/skills_service.rs b/src-tauri/tests/skills_service.rs index f5ee33edc..91ac98662 100644 --- a/src-tauri/tests/skills_service.rs +++ b/src-tauri/tests/skills_service.rs @@ -1,4 +1,7 @@ -use cc_switch_lib::{AppType, Database, ImportSkillSelection, SkillApps, SkillService}; +use cc_switch_lib::{ + update_settings, AppSettings, AppType, Database, ImportSkillSelection, SkillApps, SkillService, + SkillStorageLocation, +}; #[path = "support.rs"] mod support; @@ -13,6 +16,55 @@ fn write_skill_md(dir: &std::path::Path, name: &str, description: &str) { .expect("write SKILL.md"); } +fn register_managed_skill(directory: &str, apps: SkillApps) { + let imported = SkillService::import_from_apps(vec![ImportSkillSelection { + directory: directory.to_string(), + apps, + }]) + .expect("register managed skill through the public import API"); + assert_eq!(imported.len(), 1); +} + +fn persisted_settings(home: &std::path::Path) -> AppSettings { + let raw = std::fs::read_to_string(home.join(".cc-switch").join("settings.json")) + .expect("read persisted settings"); + serde_json::from_str(&raw).expect("parse persisted settings") +} + +fn remove_test_path(path: &std::path::Path) { + let Ok(metadata) = std::fs::symlink_metadata(path) else { + return; + }; + if metadata.file_type().is_symlink() || metadata.is_file() { + std::fs::remove_file(path).expect("remove test file"); + } else { + std::fs::remove_dir_all(path).expect("remove test directory"); + } +} + +struct EnvVarGuard { + key: &'static str, + previous: Option, +} + +impl EnvVarGuard { + fn set(key: &'static str, value: &std::path::Path) -> Self { + let previous = std::env::var_os(key); + std::env::set_var(key, value); + Self { key, previous } + } +} + +impl Drop for EnvVarGuard { + fn drop(&mut self) { + if let Some(previous) = &self.previous { + std::env::set_var(self.key, previous); + } else { + std::env::remove_var(self.key); + } + } +} + #[test] fn list_installed_triggers_initial_ssot_migration() { let _guard = lock_test_mutex(); @@ -350,3 +402,511 @@ fn pending_migration_with_existing_managed_list_does_not_claim_unmanaged_skills( "unmanaged skill should remain unmanaged (not added to db)" ); } + +#[test] +fn storage_migration_moves_only_managed_skills_and_refreshes_apps() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_root = SkillService::get_ssot_dir().expect("create managed SSOT root"); + let new_root = home.join(".agents").join("skills"); + write_skill_md(&old_root.join("managed"), "Managed", "Managed by CC Switch"); + write_skill_md(&old_root.join("unmanaged"), "Unmanaged", "Leave in place"); + register_managed_skill("managed", SkillApps::only(&AppType::Claude)); + + let result = + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("migrate storage"); + + assert_eq!(result.migrated_count, 1); + assert!(result.errors.is_empty()); + assert!(!old_root.join("managed").exists()); + assert!(new_root.join("managed").join("SKILL.md").is_file()); + assert!(old_root.join("unmanaged").join("SKILL.md").is_file()); + assert!(!new_root.join("unmanaged").exists()); + assert!( + home.join(".claude") + .join("skills") + .join("managed") + .join("SKILL.md") + .is_file(), + "enabled app deployment should be refreshed" + ); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::Unified + ); + + let reversed = + SkillService::migrate_storage(SkillStorageLocation::CcSwitch).expect("migrate back"); + assert_eq!(reversed.migrated_count, 1); + assert!(reversed.errors.is_empty()); + assert!(old_root.join("managed").join("SKILL.md").is_file()); + assert!(!new_root.join("managed").exists()); + assert!(old_root.join("unmanaged").join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[test] +fn storage_migration_same_target_preserves_an_opposite_identical_copy() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + let new_skill = home.join(".agents").join("skills").join("managed"); + write_skill_md(&old_skill, "Managed", "Managed source"); + register_managed_skill("managed", SkillApps::default()); + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("initial migration"); + write_skill_md(&old_skill, "Managed", "Managed source"); + + let result = + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("same-target no-op"); + + assert_eq!(result.migrated_count, 0); + assert_eq!(result.skipped_count, 0); + assert!(result.errors.is_empty()); + assert!(old_skill.join("SKILL.md").is_file()); + assert!(new_skill.join("SKILL.md").is_file()); +} + +#[test] +fn storage_migration_same_target_does_not_adopt_an_opposite_copy() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Opposite copy"); + register_managed_skill("managed", SkillApps::default()); + let mut settings = AppSettings::default(); + settings.skill_storage_location = SkillStorageLocation::Unified; + update_settings(settings).expect("point settings at Unified without a journal"); + + let result = + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("same-target no-op"); + + assert_eq!(result.migrated_count, 0); + assert!(result.errors.is_empty()); + assert!(old_skill.join("SKILL.md").is_file()); + assert!(!home.join(".agents").join("skills").join("managed").exists()); +} + +#[test] +fn storage_migration_rejects_different_target_content_before_moving() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + let new_skill = home.join(".agents").join("skills").join("managed"); + write_skill_md(&old_skill, "Managed", "Original content"); + write_skill_md(&new_skill, "Managed", "Different content"); + register_managed_skill("managed", SkillApps::default()); + + let error = SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("different target content must stop migration"); + + assert!(error.to_string().contains("pre-existing")); + assert!(old_skill.join("SKILL.md").is_file()); + assert!(new_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[test] +fn storage_migration_does_not_claim_an_identical_unknown_target() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + let new_skill = home.join(".agents").join("skills").join("managed"); + write_skill_md(&old_skill, "Managed", "Same content"); + write_skill_md(&new_skill, "Managed", "Same content"); + register_managed_skill("managed", SkillApps::default()); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("content equality must not imply ownership"); + + assert!(old_skill.join("SKILL.md").is_file()); + assert!(new_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[cfg(unix)] +#[test] +fn storage_migration_leaves_an_identical_plain_app_directory_untouched() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + let deployed = home.join(".claude").join("skills").join("managed"); + write_skill_md(&old_skill, "Managed", "Same deployment"); + write_skill_md(&deployed, "Managed", "Same deployment"); + register_managed_skill("managed", SkillApps::only(&AppType::Claude)); + + let before = std::fs::symlink_metadata(&deployed).expect("read deployment metadata"); + assert!(before.is_dir()); + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect("migrate without claiming the app directory"); + let after = std::fs::symlink_metadata(&deployed).expect("read deployment metadata again"); + + use std::os::unix::fs::MetadataExt; + assert!(after.is_dir()); + assert_eq!( + before.ino(), + after.ino(), + "plain deployment must not be replaced" + ); +} + +#[test] +fn storage_migration_compares_hidden_content() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + let new_skill = home.join(".agents").join("skills").join("managed"); + write_skill_md(&old_skill, "Managed", "Same visible content"); + write_skill_md(&new_skill, "Managed", "Same visible content"); + std::fs::write(old_skill.join(".env"), "TOKEN=old").expect("write old hidden content"); + std::fs::write(new_skill.join(".env"), "TOKEN=new").expect("write new hidden content"); + register_managed_skill("managed", SkillApps::default()); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("different hidden content must stop migration"); + + assert!(old_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[test] +fn storage_migration_does_not_adopt_a_target_when_the_current_source_is_missing() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Original"); + register_managed_skill("managed", SkillApps::default()); + remove_test_path(&old_skill); + let new_skill = home.join(".agents").join("skills").join("managed"); + write_skill_md(&new_skill, "Managed", "Unknown target"); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("a target without the current source cannot be identified safely"); + + assert!(new_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[cfg(unix)] +#[test] +fn storage_migration_rejects_symbolic_links_without_following_them() { + use std::os::unix::fs::symlink; + + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&skill, "Managed", "Contains an external link"); + let external = home.join("private"); + std::fs::create_dir_all(&external).expect("create external directory"); + std::fs::write(external.join("secret"), "do not copy").expect("write external file"); + symlink(&external, skill.join("linked-private")).expect("create external symlink"); + register_managed_skill("managed", SkillApps::default()); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("migration must reject a nested symlink"); + + assert!(!home.join(".agents").join("skills").join("managed").exists()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[test] +fn storage_migration_preserves_an_unknown_app_destination_and_retries() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Managed source"); + register_managed_skill("managed", SkillApps::only(&AppType::Claude)); + SkillService::sync_all_enabled(Some(&AppType::Claude)).expect("create app deployment"); + + let deployed = home.join(".claude").join("skills").join("managed"); + remove_test_path(&deployed); + write_skill_md(&deployed, "Personal", "Do not replace"); + + let partial = + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("move SSOT safely"); + assert_eq!(partial.errors.len(), 1); + assert_eq!( + std::fs::read_to_string(deployed.join("SKILL.md")).expect("read preserved deployment"), + "---\nname: Personal\ndescription: Do not replace\n---\n\n# Personal\n" + ); + assert!(old_skill.exists(), "old source is the deployment fallback"); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::Unified + ); + + remove_test_path(&deployed); + let retried = + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("retry deployment"); + assert!(retried.errors.is_empty()); + assert!(deployed.join("SKILL.md").is_file()); + assert!( + !old_skill.exists(), + "successful retry cleans the old source" + ); +} + +#[test] +fn storage_migration_retry_accepts_changes_in_the_current_target() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Original source"); + register_managed_skill("managed", SkillApps::only(&AppType::Claude)); + let deployed = home.join(".claude").join("skills").join("managed"); + write_skill_md(&deployed, "Personal", "Block initial deployment"); + + let partial = + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("partial migration"); + assert_eq!(partial.errors.len(), 1); + let new_skill = home.join(".agents").join("skills").join("managed"); + write_skill_md(&new_skill, "Managed", "Updated in current storage"); + + let imported = home.join(".gemini").join("skills").join("added-later"); + write_skill_md(&imported, "Added Later", "New managed Skill"); + register_managed_skill("added-later", SkillApps::only(&AppType::Gemini)); + remove_test_path(&deployed); + + let retried = + SkillService::migrate_storage(SkillStorageLocation::Unified).expect("retry migration"); + + assert!(retried.errors.is_empty(), "{:?}", retried.errors); + assert!(!old_skill.exists()); + assert!(new_skill.join("SKILL.md").is_file()); + assert!(home + .join(".agents") + .join("skills") + .join("added-later") + .join("SKILL.md") + .is_file()); +} + +#[test] +fn storage_migration_rejects_an_app_directory_alias() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Keep safe"); + register_managed_skill("managed", SkillApps::only(&AppType::Codex)); + let mut settings = AppSettings::default(); + settings.codex_config_dir = Some(home.join(".agents").display().to_string()); + update_settings(settings).expect("set Codex override"); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("aliased destination must be rejected"); + + assert!(old_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[test] +fn storage_migration_rejects_an_app_directory_nested_under_the_ssot() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Keep safe"); + register_managed_skill("managed", SkillApps::only(&AppType::Codex)); + let mut settings = AppSettings::default(); + settings.codex_config_dir = Some( + home.join(".agents") + .join("skills") + .join("nested") + .display() + .to_string(), + ); + update_settings(settings).expect("set nested Codex override"); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("nested storage roots must be rejected"); + + assert!(old_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[test] +fn storage_migration_alias_check_honors_codex_home() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Keep safe"); + register_managed_skill("managed", SkillApps::only(&AppType::Codex)); + let codex_home = home.join(".agents"); + std::fs::create_dir_all(&codex_home).expect("create CODEX_HOME"); + let _env = EnvVarGuard::set("CODEX_HOME", &codex_home); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("CODEX_HOME alias must be rejected"); + + assert!(old_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[cfg(unix)] +#[test] +fn app_sync_resolves_a_symlink_before_parent_components() { + use std::os::unix::fs::symlink; + + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Keep safe"); + register_managed_skill("managed", SkillApps::only(&AppType::Claude)); + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect("move to Unified before configuring the alias"); + + let nested = home.join(".agents").join("nested"); + std::fs::create_dir_all(&nested).expect("create symlink target"); + let alias = home.join("alias"); + symlink(&nested, &alias).expect("create config alias"); + let claude_override = alias.join(".."); + let _env = EnvVarGuard::set("CLAUDE_CONFIG_DIR", &claude_override); + + SkillService::sync_all_enabled(Some(&AppType::Claude)) + .expect_err("resolved app and SSOT roots overlap"); + + let skill = home.join(".agents").join("skills").join("managed"); + assert!(skill.join("SKILL.md").is_file()); + assert!(std::fs::symlink_metadata(&skill) + .expect("read preserved Skill") + .is_dir()); +} + +#[test] +fn storage_migration_rejects_overlap_with_the_current_ssot() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Keep safe"); + register_managed_skill("managed", SkillApps::only(&AppType::Claude)); + let _env = EnvVarGuard::set("CLAUDE_CONFIG_DIR", &home.join(".cc-switch")); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("the current SSOT must not double as an app deployment root"); + + assert!(old_skill.join("SKILL.md").is_file()); + assert_eq!( + persisted_settings(home).skill_storage_location, + SkillStorageLocation::CcSwitch + ); +} + +#[cfg(unix)] +#[test] +fn storage_migration_rejects_a_symlinked_unified_root() { + use std::os::unix::fs::symlink; + + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + write_skill_md(&old_skill, "Managed", "Keep safe"); + register_managed_skill("managed", SkillApps::default()); + let external = home.join("external-agents"); + std::fs::create_dir_all(&external).expect("create external directory"); + symlink(&external, home.join(".agents")).expect("link Unified root elsewhere"); + + SkillService::migrate_storage(SkillStorageLocation::Unified) + .expect_err("Unified root symlinks must be rejected"); + + assert!(old_skill.join("SKILL.md").is_file()); + assert!(!external.join("skills").exists()); +} + +#[test] +fn app_sync_rejects_an_ssot_alias_without_deleting_the_skill() { + let _guard = lock_test_mutex(); + reset_test_fs(); + let home = ensure_test_home(); + let old_skill = SkillService::get_ssot_dir() + .expect("create managed SSOT root") + .join("managed"); + let skill = home.join(".agents").join("skills").join("managed"); + write_skill_md(&old_skill, "Managed", "Keep safe"); + register_managed_skill("managed", SkillApps::only(&AppType::Codex)); + std::fs::create_dir_all(skill.parent().expect("unified skill root")) + .expect("create unified skill root"); + std::fs::rename(&old_skill, &skill).expect("move skill to unified SSOT"); + let mut settings = AppSettings::default(); + settings.skill_storage_location = SkillStorageLocation::Unified; + settings.codex_config_dir = Some(home.join(".agents").display().to_string()); + update_settings(settings).expect("set aliased paths"); + + SkillService::sync_all_enabled(Some(&AppType::Codex)) + .expect_err("sync must reject aliased roots"); + + assert!(skill.join("SKILL.md").is_file()); +} diff --git a/src-tauri/tests/support.rs b/src-tauri/tests/support.rs index 2e5ed1123..c30de9859 100644 --- a/src-tauri/tests/support.rs +++ b/src-tauri/tests/support.rs @@ -23,6 +23,7 @@ pub fn ensure_test_home() -> &'static Path { std::env::set_var("HOME", home); #[cfg(windows)] std::env::set_var("USERPROFILE", home); + std::env::set_var("CC_SWITCH_TEST_HOME", home); std::env::set_var("XDG_CONFIG_HOME", home.join(".config")); std::env::set_var("XDG_RUNTIME_DIR", home.join(".runtime")); std::env::set_var("XDG_STATE_HOME", home.join(".state")); @@ -40,6 +41,7 @@ pub fn reset_test_fs() { ".claude", ".codex", ".cc-switch", + ".agents", ".gemini", ".openclaw", ".config",