diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index f94dd83919..4ce4d66923 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -1327,6 +1327,14 @@ impl SkillService { restored_skill.apps = SkillApps::only(current_app); restored_skill.updated_at = 0; + let enabled_dirs = existing_skills + .values() + .filter(|skill| skill.apps.is_enabled_for(current_app)) + .map(|skill| skill.directory.clone()) + .chain(std::iter::once(restored_skill.directory.clone())) + .collect::>(); + Self::ensure_no_app_path_conflicts(current_app, &enabled_dirs)?; + Self::copy_dir_recursive(&backup_skill_dir, &restore_path)?; // 重新计算内容哈希 @@ -1359,14 +1367,27 @@ impl SkillService { /// 启用:复制到应用目录 /// 禁用:从应用目录删除 pub fn toggle_app(db: &Arc, id: &str, app: &AppType, enabled: bool) -> Result<()> { + let existing_skills = db.get_all_installed_skills()?; + // 获取当前 skill - let mut skill = db - .get_installed_skill(id)? + let mut skill = existing_skills + .get(id) + .cloned() .ok_or_else(|| anyhow!("Skill not found: {id}"))?; // 更新状态 skill.apps.set_enabled_for(app, enabled); + if enabled { + let enabled_dirs = existing_skills + .values() + .filter(|existing| existing.id != skill.id && existing.apps.is_enabled_for(app)) + .map(|existing| existing.directory.clone()) + .chain(std::iter::once(skill.directory.clone())) + .collect::>(); + Self::ensure_no_app_path_conflicts(app, &enabled_dirs)?; + } + // 同步文件 if enabled { Self::sync_to_app_dir(&skill.directory, app)?; @@ -1389,8 +1410,10 @@ impl SkillService { let managed_skills = db.get_all_installed_skills()?; let managed_dirs: HashSet = managed_skills .values() - .map(|s| s.directory.clone()) + .map(|s| Self::normalize_skill_key(&s.directory)) .collect(); + let managed_app_sources = Self::collect_managed_app_sources(managed_skills.values()); + let ssot_dir = Self::get_ssot_dir().ok(); // 收集所有待扫描的目录及其来源标签 let mut scan_sources: Vec<(PathBuf, String)> = Vec::new(); @@ -1409,6 +1432,58 @@ impl SkillService { let mut unmanaged: HashMap = HashMap::new(); for (scan_dir, label) in &scan_sources { + if let Ok(app) = label.parse::() { + let entries = Self::list_app_skill_entries(&app, scan_dir)?; + for (directory, path) in entries { + let normalized_directory = Self::normalize_skill_key(&directory); + let normalized_app_path = Self::app_relative_skill_path(&app, &directory) + .map(|relative| Self::normalize_skill_path(&relative)); + let is_managed_mapped_entry = if let (Some(app_path), Some(ssot_dir)) = + (normalized_app_path.as_ref(), ssot_dir.as_ref()) + { + if normalized_directory == *app_path { + managed_app_sources + .get(app.as_str()) + .and_then(|sources| sources.get(app_path)) + .map(|managed_directory| { + Self::is_synced_entry_for_source( + &path, + &ssot_dir.join(managed_directory), + ) + .unwrap_or(false) + }) + .unwrap_or(false) + } else { + false + } + } else { + false + }; + + // 对Claude这类会改写live目录结构的app,除了完整目录外 + // 还要按实际映射后的叶子路径判断是否已被CC Switch管理,避免同步出的leaf目录 + // 再次被扫成unmanaged;但外部嵌套目录仍应保留为可见的unmanaged skill。 + if managed_dirs.contains(&normalized_directory) || is_managed_mapped_entry { + continue; + } + + let skill_md = path.join("SKILL.md"); + let (name, description) = Self::read_skill_name_desc(&skill_md, &directory); + + unmanaged + .entry(directory.clone()) + .and_modify(|s| s.found_in.push(label.clone())) + .or_insert(UnmanagedSkill { + directory, + name, + description, + found_in: vec![label.clone()], + path: path.display().to_string(), + }); + } + continue; + } + let entries = match fs::read_dir(scan_dir) { Ok(e) => e, Err(_) => continue, @@ -1455,6 +1530,21 @@ impl SkillService { let ssot_dir = Self::get_ssot_dir()?; let agents_lock = parse_agents_lock(); let mut imported = Vec::new(); + let existing_skills = db.get_all_installed_skills()?; + + let claude_enabled_dirs = existing_skills + .values() + .filter(|skill| skill.apps.claude) + .map(|skill| skill.directory.clone()) + .chain( + imports + .iter() + .filter(|selection| selection.apps.claude) + .map(|selection| selection.directory.clone()), + ) + .collect::>(); + // Claude live目录会扁平到叶子skill名,这里提前拦截同名冲突,避免导入后互相覆盖 + Self::ensure_no_app_path_conflicts(&AppType::Claude, &claude_enabled_dirs)?; // 将 lock 文件中发现的仓库保存到 skill_repos save_repos_from_lock( @@ -1597,10 +1687,19 @@ impl SkillService { let app_dir = Self::get_app_skills_dir(app)?; fs::create_dir_all(&app_dir)?; - let dest = app_dir.join(directory); + let dest = Self::app_skill_dest_path(app, &app_dir, directory); + if let Some(parent) = dest.parent() { + fs::create_dir_all(parent)?; + } - // 如果已存在则先删除(无论是 symlink 还是真实目录) + // 仅允许覆盖由 CC Switch 同步出来的目标,避免误删用户已有目录。 if dest.exists() || Self::is_symlink(&dest) { + if !Self::is_synced_entry_for_source(&dest, &source)? { + return Err(anyhow!( + "{app:?} skill 目标路径已存在且不是由 CC Switch 管理: {}", + dest.display() + )); + } Self::remove_path(&dest)?; } @@ -1693,7 +1792,7 @@ impl SkillService { /// 从应用目录删除 Skill(支持 symlink 和真实目录) pub fn remove_from_app(directory: &str, app: &AppType) -> Result<()> { let app_dir = Self::get_app_skills_dir(app)?; - let skill_path = app_dir.join(directory); + let skill_path = Self::app_skill_dest_path(app, &app_dir, directory); if skill_path.exists() || Self::is_symlink(&skill_path) { Self::remove_path(&skill_path)?; @@ -1708,31 +1807,57 @@ impl SkillService { let skills = db.get_all_installed_skills()?; let ssot_dir = Self::get_ssot_dir()?; let app_dir = Self::get_app_skills_dir(app)?; + let enabled_dirs = skills + .values() + .filter(|skill| skill.apps.is_enabled_for(app)) + .map(|skill| skill.directory.clone()) + .collect::>(); + Self::ensure_no_app_path_conflicts(app, &enabled_dirs)?; - let indexed_skills: HashMap = skills + let enabled_indexed_skills: HashMap = skills + .values() + .filter_map(|skill| { + if !skill.apps.is_enabled_for(app) { + return None; + } + let relative = Self::app_relative_skill_path(app, &skill.directory)?; + Some((Self::normalize_skill_path(&relative), skill)) + }) + .collect(); + let managed_app_paths: HashSet = skills + .values() + .filter_map(|skill| { + if !skill.apps.is_enabled_for(app) { + return None; + } + Self::app_relative_skill_path(app, &skill.directory) + .map(|relative| Self::normalize_skill_path(&relative)) + }) + .collect(); + let managed_original_paths: HashSet = skills .values() - .map(|skill| (skill.directory.to_lowercase(), skill)) + .filter(|skill| skill.apps.is_enabled_for(app)) + .map(|skill| Self::normalize_skill_key(&skill.directory)) .collect(); if app_dir.exists() { - for entry in fs::read_dir(&app_dir)? { - let entry = entry?; - let path = entry.path(); - let dir_name = entry.file_name().to_string_lossy().to_string(); + for (relative_path, path) in Self::list_existing_app_skill_entries(app, &app_dir)? { + let normalized_relative = Self::normalize_skill_key(&relative_path); - if dir_name.starts_with('.') { + if enabled_indexed_skills.contains_key(&normalized_relative) { continue; } - if let Some(skill) = indexed_skills.get(&dir_name.to_lowercase()) { - if !skill.apps.is_enabled_for(app) { - Self::remove_path(&path)?; - } - continue; - } + let should_remove = managed_app_paths.contains(&normalized_relative) + || (matches!(app, AppType::Claude) + && managed_original_paths.contains(&normalized_relative)) + || Self::is_symlink_to_ssot(&path, &ssot_dir); - if Self::is_symlink_to_ssot(&path, &ssot_dir) { + if should_remove { Self::remove_path(&path)?; + if matches!(app, AppType::Claude) { + Self::prune_empty_parent_dirs(&app_dir, &path)?; + } } } } @@ -1746,6 +1871,291 @@ impl SkillService { Ok(()) } + fn app_skill_dest_path(app: &AppType, app_dir: &Path, directory: &str) -> PathBuf { + app_dir.join( + Self::app_relative_skill_path(app, directory) + .unwrap_or_else(|| PathBuf::from(directory)), + ) + } + + fn ensure_no_app_path_conflicts(app: &AppType, directories: &[String]) -> Result<()> { + let mut path_to_directories: HashMap> = HashMap::new(); + + for directory in directories { + let Some(relative) = Self::app_relative_skill_path(app, directory) else { + continue; + }; + path_to_directories + .entry(Self::normalize_skill_path(&relative)) + .or_default() + .push(directory.clone()); + } + + if let Some((leaf_path, conflicts)) = path_to_directories + .into_iter() + .find(|(_, directories)| directories.len() > 1) + { + return Err(anyhow!( + "{app:?} skills 目标路径冲突: {leaf_path} <- {}", + conflicts.join(", ") + )); + } + + Ok(()) + } + + fn app_relative_skill_path(app: &AppType, directory: &str) -> Option { + match app { + // Claude的skills目录按叶子skill进行暴露 + // 对于类似superpowers/这类分组目录,同步时映射到顶层skill + AppType::Claude => Path::new(directory) + .file_name() + .map(PathBuf::from) + .filter(|name| !name.as_os_str().is_empty()), + _ => Some(PathBuf::from(directory)), + } + } + + fn list_app_skill_entries(app: &AppType, app_dir: &Path) -> Result> { + match app { + AppType::Claude => { + // Claude可能会把一组skills放在分组目录下 + // 这里递归扫描所有包含SKILL.md的叶子目录,并保留相对路径用于导入 + let mut entries = Vec::new(); + for skill_dir in Self::scan_skills_in_dir(app_dir)? { + let Some(relative) = skill_dir.strip_prefix(app_dir).ok() else { + continue; + }; + let directory = relative + .components() + .filter_map(|component| match component { + Component::Normal(name) => Some(name.to_string_lossy().to_string()), + _ => None, + }) + .collect::>() + .join("/"); + if directory.is_empty() { + continue; + } + entries.push((directory, skill_dir)); + } + Ok(entries) + } + _ => { + let mut entries = Vec::new(); + let read_dir = match fs::read_dir(app_dir) { + Ok(entries) => entries, + Err(_) => return Ok(entries), + }; + + for entry in read_dir.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + let dir_name = entry.file_name().to_string_lossy().to_string(); + if dir_name.starts_with('.') { + continue; + } + if !path.join("SKILL.md").exists() { + continue; + } + entries.push((dir_name, path)); + } + + Ok(entries) + } + } + } + + fn list_existing_app_skill_entries( + app: &AppType, + app_dir: &Path, + ) -> Result> { + match app { + AppType::Claude => Self::list_app_skill_entries(app, app_dir), + _ => { + let mut entries = Vec::new(); + let read_dir = match fs::read_dir(app_dir) { + Ok(entries) => entries, + Err(_) => return Ok(entries), + }; + + for entry in read_dir.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + let dir_name = entry.file_name().to_string_lossy().to_string(); + if dir_name.starts_with('.') { + continue; + } + entries.push((dir_name, path)); + } + + Ok(entries) + } + } + } + + fn normalize_skill_key(value: &str) -> String { + value.replace('\\', "/").to_lowercase() + } + + fn collect_managed_app_sources<'a, I>(skills: I) -> HashMap> + where + I: IntoIterator, + { + let skills = skills.into_iter().collect::>(); + + AppType::all() + .map(|app| { + let paths = skills + .iter() + .filter_map(|skill| { + if !skill.apps.is_enabled_for(&app) { + return None; + } + Self::app_relative_skill_path(&app, &skill.directory).map(|relative| { + ( + Self::normalize_skill_path(&relative), + skill.directory.clone(), + ) + }) + }) + .collect(); + (app.as_str().to_string(), paths) + }) + .collect() + } + + fn is_synced_entry_for_source(path: &Path, source: &Path) -> Result { + if Self::is_symlink(path) { + return Ok(Self::is_symlink_to_source(path, source)); + } + + if !path.exists() { + return Ok(false); + } + + Self::paths_match(source, path) + } + + fn is_symlink_to_source(path: &Path, source: &Path) -> bool { + if !Self::is_symlink(path) { + return false; + } + + let Ok(target) = fs::read_link(path) else { + return false; + }; + + let resolved = path + .parent() + .map(|parent| { + if target.is_absolute() { + target.clone() + } else { + parent.join(&target) + } + }) + .unwrap_or(target.clone()); + + let canonical_source = source + .canonicalize() + .unwrap_or_else(|_| source.to_path_buf()); + let canonical_target = resolved.canonicalize().unwrap_or(resolved); + + canonical_target == canonical_source + } + + fn paths_match(source: &Path, target: &Path) -> Result { + let source_meta = match fs::metadata(source) { + Ok(meta) => meta, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(err) => return Err(err.into()), + }; + let target_meta = match fs::metadata(target) { + Ok(meta) => meta, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(err) => return Err(err.into()), + }; + + if source_meta.is_dir() != target_meta.is_dir() + || source_meta.is_file() != target_meta.is_file() + { + return Ok(false); + } + + if source_meta.is_file() { + if source_meta.len() != target_meta.len() { + return Ok(false); + } + return Ok(fs::read(source)? == fs::read(target)?); + } + + if !source_meta.is_dir() { + return Ok(false); + } + + let mut source_entries = HashSet::new(); + for entry in fs::read_dir(source)? { + let entry = entry?; + let name = entry.file_name(); + source_entries.insert(name.clone()); + + if !Self::paths_match(&entry.path(), &target.join(&name))? { + return Ok(false); + } + } + + for entry in fs::read_dir(target)? { + let entry = entry?; + let name = entry.file_name(); + if source_entries.contains(&name) || Self::is_ignored_sync_extra(&name) { + continue; + } + return Ok(false); + } + + Ok(true) + } + + fn is_ignored_sync_extra(name: &std::ffi::OsStr) -> bool { + name.to_string_lossy().starts_with('.') + } + + fn normalize_skill_path(path: &Path) -> String { + path.components() + .filter_map(|component| match component { + Component::Normal(name) => Some(name.to_string_lossy().to_string()), + _ => None, + }) + .collect::>() + .join("/") + .to_lowercase() + } + + fn prune_empty_parent_dirs(root: &Path, path: &Path) -> Result<()> { + let mut current = path.parent(); + + while let Some(dir) = current { + if dir == root || !dir.starts_with(root) { + break; + } + + let mut entries = fs::read_dir(dir)?; + if entries.next().is_some() { + break; + } + + fs::remove_dir(dir)?; + current = dir.parent(); + } + + Ok(()) + } + // ========== 发现功能(保留原有逻辑)========== /// 列出所有可发现的技能(从仓库获取) diff --git a/src-tauri/tests/skill_sync.rs b/src-tauri/tests/skill_sync.rs index b1e0c3a61a..eb4665b455 100644 --- a/src-tauri/tests/skill_sync.rs +++ b/src-tauri/tests/skill_sync.rs @@ -74,6 +74,36 @@ fn import_from_apps_respects_explicit_app_selection() { ); } +#[test] +fn scan_unmanaged_detects_nested_claude_skills() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + write_skill( + &home + .join(".claude") + .join("skills") + .join("superpowers") + .join("brainstorming"), + "Brainstorming", + ); + + let state = create_test_state().expect("create test state"); + let unmanaged = SkillService::scan_unmanaged(&state.db).expect("scan unmanaged skills"); + + let skill = unmanaged + .iter() + .find(|skill| skill.directory == "superpowers/brainstorming") + .expect("nested Claude skill should be discovered"); + + assert_eq!(skill.name, "Brainstorming"); + assert!( + skill.found_in.iter().any(|source| source == "claude"), + "nested skill should be reported as coming from Claude" + ); +} + #[test] fn sync_to_app_removes_disabled_and_orphaned_ssot_symlinks() { let _guard = test_mutex().lock().expect("acquire test mutex"); @@ -127,6 +157,106 @@ fn sync_to_app_removes_disabled_and_orphaned_ssot_symlinks() { ); } +#[test] +fn import_from_apps_accepts_nested_claude_skill_paths() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + write_skill( + &home + .join(".claude") + .join("skills") + .join("superpowers") + .join("brainstorming"), + "Brainstorming", + ); + + let state = create_test_state().expect("create test state"); + let imported = SkillService::import_from_apps( + &state.db, + vec![ImportSkillSelection { + directory: "superpowers/brainstorming".to_string(), + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + }], + ) + .expect("import nested Claude skill"); + + assert_eq!(imported.len(), 1); + assert_eq!(imported[0].directory, "superpowers/brainstorming"); + assert!( + home.join(".cc-switch") + .join("skills") + .join("superpowers") + .join("brainstorming") + .join("SKILL.md") + .exists(), + "nested Claude skill should be copied into SSOT with its relative path" + ); +} + +#[test] +fn import_from_apps_rejects_conflicting_claude_leaf_names() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + write_skill( + &home + .join(".claude") + .join("skills") + .join("superpowers") + .join("brainstorming"), + "Superpowers Brainstorming", + ); + write_skill( + &home + .join(".claude") + .join("skills") + .join("tools") + .join("brainstorming"), + "Tools Brainstorming", + ); + + let state = create_test_state().expect("create test state"); + let error = SkillService::import_from_apps( + &state.db, + vec![ + ImportSkillSelection { + directory: "superpowers/brainstorming".to_string(), + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + }, + ImportSkillSelection { + directory: "tools/brainstorming".to_string(), + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + }, + ], + ) + .expect_err("conflicting Claude leaf names should be rejected"); + + assert!( + error + .to_string() + .contains("Claude skills 目标路径冲突: brainstorming"), + "unexpected error: {error:#}" + ); +} + #[test] fn uninstall_skill_creates_backup_before_removing_ssot() { let _guard = test_mutex().lock().expect("acquire test mutex"); @@ -278,6 +408,349 @@ fn restore_skill_backup_restores_files_to_ssot_and_current_app() { ); } +#[test] +fn sync_to_claude_flattens_nested_skill_paths_to_leaf_directory() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + let ssot_skill_dir = home + .join(".cc-switch") + .join("skills") + .join("superpowers") + .join("brainstorming"); + write_skill(&ssot_skill_dir, "Brainstorming"); + + let state = create_test_state().expect("create test state"); + state + .db + .save_skill(&InstalledSkill { + id: "local:superpowers/brainstorming".to_string(), + name: "Brainstorming".to_string(), + description: Some("Nested Claude skill".to_string()), + directory: "superpowers/brainstorming".to_string(), + repo_owner: None, + repo_name: None, + repo_branch: None, + readme_url: None, + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + installed_at: 789, + content_hash: None, + updated_at: 0, + }) + .expect("save nested skill"); + + SkillService::sync_to_app(&state.db, &AppType::Claude).expect("sync Claude skills"); + + assert!( + home.join(".claude") + .join("skills") + .join("brainstorming") + .join("SKILL.md") + .exists(), + "nested skill should be exposed at Claude top level using its leaf directory" + ); + assert!( + !home + .join(".claude") + .join("skills") + .join("superpowers") + .exists(), + "Claude live dir should not keep the grouping directory after sync" + ); +} + +#[test] +fn sync_to_claude_cleans_legacy_nested_dir_and_scan_unmanaged_does_not_repeat() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + write_skill( + &home + .join(".claude") + .join("skills") + .join("superpowers") + .join("brainstorming"), + "Brainstorming", + ); + + let state = create_test_state().expect("create test state"); + SkillService::import_from_apps( + &state.db, + vec![ImportSkillSelection { + directory: "superpowers/brainstorming".to_string(), + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + }], + ) + .expect("import nested Claude skill"); + + SkillService::sync_to_app(&state.db, &AppType::Claude).expect("sync Claude skills"); + + assert!( + home.join(".claude") + .join("skills") + .join("brainstorming") + .join("SKILL.md") + .exists(), + "synced Claude leaf directory should exist" + ); + assert!( + !home + .join(".claude") + .join("skills") + .join("superpowers") + .join("brainstorming") + .exists(), + "legacy nested Claude directory should be removed after sync" + ); + assert!( + !home + .join(".claude") + .join("skills") + .join("superpowers") + .exists(), + "empty Claude grouping directory should be pruned after cleanup" + ); + + let unmanaged = SkillService::scan_unmanaged(&state.db).expect("scan unmanaged skills"); + assert!( + unmanaged.iter().all(|skill| { + skill.directory != "brainstorming" && skill.directory != "superpowers/brainstorming" + }), + "managed nested Claude skill should not reappear as unmanaged after sync" + ); +} + +#[test] +fn real_claude_leaf_skill_conflicting_with_managed_nested_skill_still_appears_as_unmanaged() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + write_skill( + &home.join(".claude").join("skills").join("brainstorming"), + "Real Claude Brainstorming", + ); + + let ssot_skill_dir = home + .join(".cc-switch") + .join("skills") + .join("superpowers") + .join("brainstorming"); + write_skill(&ssot_skill_dir, "Managed Brainstorming"); + + let state = create_test_state().expect("create test state"); + state + .db + .save_skill(&InstalledSkill { + id: "local:superpowers/brainstorming".to_string(), + name: "Managed Brainstorming".to_string(), + description: Some("Enabled for Claude".to_string()), + directory: "superpowers/brainstorming".to_string(), + repo_owner: None, + repo_name: None, + repo_branch: None, + readme_url: None, + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + installed_at: 1, + content_hash: None, + updated_at: 0, + }) + .expect("save managed nested skill"); + + let unmanaged = SkillService::scan_unmanaged(&state.db).expect("scan unmanaged skills"); + assert!( + unmanaged.iter().any(|skill| skill.directory == "brainstorming"), + "real Claude leaf skill should stay visible as unmanaged even when a managed nested Claude skill maps to the same leaf path" + ); +} + +#[test] +fn sync_to_claude_rejects_overwriting_real_leaf_skill_with_managed_nested_skill() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + let real_leaf_dir = home.join(".claude").join("skills").join("brainstorming"); + write_skill(&real_leaf_dir, "Real Claude Brainstorming"); + + let ssot_skill_dir = home + .join(".cc-switch") + .join("skills") + .join("superpowers") + .join("brainstorming"); + write_skill(&ssot_skill_dir, "Managed Brainstorming"); + + let state = create_test_state().expect("create test state"); + state + .db + .save_skill(&InstalledSkill { + id: "local:superpowers/brainstorming".to_string(), + name: "Managed Brainstorming".to_string(), + description: Some("Enabled for Claude".to_string()), + directory: "superpowers/brainstorming".to_string(), + repo_owner: None, + repo_name: None, + repo_branch: None, + readme_url: None, + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + installed_at: 1, + content_hash: None, + updated_at: 0, + }) + .expect("save managed nested skill"); + + let error = SkillService::sync_to_app(&state.db, &AppType::Claude) + .expect_err("sync should refuse to overwrite a real Claude leaf skill"); + assert!( + error + .to_string() + .contains("目标路径已存在且不是由 CC Switch 管理"), + "unexpected error: {error:#}" + ); + assert!( + fs::read_to_string(real_leaf_dir.join("SKILL.md")) + .expect("read real Claude skill") + .contains("Real Claude Brainstorming"), + "real Claude leaf skill should remain untouched after the rejected sync" + ); +} + +#[test] +fn codex_only_nested_skill_does_not_block_real_claude_leaf_skill() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + write_skill( + &home.join(".claude").join("skills").join("brainstorming"), + "Claude Brainstorming", + ); + + let ssot_skill_dir = home + .join(".cc-switch") + .join("skills") + .join("superpowers") + .join("brainstorming"); + write_skill(&ssot_skill_dir, "Codex Brainstorming"); + + let state = create_test_state().expect("create test state"); + state + .db + .save_skill(&InstalledSkill { + id: "local:superpowers/brainstorming".to_string(), + name: "Codex Brainstorming".to_string(), + description: Some("Only enabled for Codex".to_string()), + directory: "superpowers/brainstorming".to_string(), + repo_owner: None, + repo_name: None, + repo_branch: None, + readme_url: None, + apps: SkillApps { + claude: false, + codex: true, + gemini: false, + opencode: false, + }, + installed_at: 1, + content_hash: None, + updated_at: 0, + }) + .expect("save codex-only skill"); + + let unmanaged = SkillService::scan_unmanaged(&state.db).expect("scan unmanaged skills"); + assert!( + unmanaged.iter().any(|skill| skill.directory == "brainstorming"), + "real Claude leaf skill should still appear as unmanaged when only a Codex skill shares its leaf name" + ); + + SkillService::sync_to_app(&state.db, &AppType::Claude).expect("sync Claude skills"); + assert!( + home.join(".claude") + .join("skills") + .join("brainstorming") + .join("SKILL.md") + .exists(), + "sync_to_app(Claude) should not delete a real Claude skill just because a Codex-only skill shares its leaf name" + ); +} + +#[test] +fn external_nested_claude_skill_with_conflicting_leaf_still_appears_as_unmanaged() { + let _guard = test_mutex().lock().expect("acquire test mutex"); + reset_test_fs(); + let home = ensure_test_home(); + + let ssot_skill_dir = home + .join(".cc-switch") + .join("skills") + .join("superpowers") + .join("brainstorming"); + write_skill(&ssot_skill_dir, "Managed Brainstorming"); + + write_skill( + &home + .join(".claude") + .join("skills") + .join("tools") + .join("brainstorming"), + "External Brainstorming", + ); + + let state = create_test_state().expect("create test state"); + state + .db + .save_skill(&InstalledSkill { + id: "local:superpowers/brainstorming".to_string(), + name: "Managed Brainstorming".to_string(), + description: Some("Enabled for Claude".to_string()), + directory: "superpowers/brainstorming".to_string(), + repo_owner: None, + repo_name: None, + repo_branch: None, + readme_url: None, + apps: SkillApps { + claude: true, + codex: false, + gemini: false, + opencode: false, + }, + installed_at: 1, + content_hash: None, + updated_at: 0, + }) + .expect("save managed claude skill"); + + let unmanaged = SkillService::scan_unmanaged(&state.db).expect("scan unmanaged skills"); + assert!( + unmanaged + .iter() + .any(|skill| skill.directory == "tools/brainstorming"), + "external nested Claude skill should remain visible as unmanaged even if its leaf path conflicts with a managed Claude skill" + ); +} + #[test] fn delete_skill_backup_removes_backup_directory() { let _guard = test_mutex().lock().expect("acquire test mutex"); diff --git a/src/components/skills/SkillCard.tsx b/src/components/skills/SkillCard.tsx index 2b47c40cad..56ffd8f2e3 100644 --- a/src/components/skills/SkillCard.tsx +++ b/src/components/skills/SkillCard.tsx @@ -18,7 +18,7 @@ type SkillCardSkill = DiscoverableSkill & { installed: boolean }; interface SkillCardProps { skill: SkillCardSkill; - onInstall: (directory: string) => Promise; + onInstall: (skill: SkillCardSkill) => Promise; onUninstall: (directory: string) => Promise; installs?: number; } @@ -35,7 +35,7 @@ export function SkillCard({ const handleInstall = async () => { setLoading(true); try { - await onInstall(skill.directory); + await onInstall(skill); } finally { setLoading(false); } diff --git a/src/components/skills/SkillsPage.tsx b/src/components/skills/SkillsPage.tsx index c24d68e944..f6fa2a4799 100644 --- a/src/components/skills/SkillsPage.tsx +++ b/src/components/skills/SkillsPage.tsx @@ -28,6 +28,10 @@ import { useRemoveSkillRepo, useSearchSkillsSh, } from "@/hooks/useSkills"; +import { + buildInstalledSkillIdentityKey, + buildSkillIdentityKey, +} from "@/lib/api/skills"; import type { AppId } from "@/lib/api/types"; import type { DiscoverableSkill, @@ -121,12 +125,7 @@ export const SkillsPage = forwardRef( const installedKeys = useMemo(() => { if (!installedSkills) return new Set(); return new Set( - installedSkills.map((s) => { - // 构建唯一 key:directory + repoOwner + repoName - const owner = s.repoOwner?.toLowerCase() || ""; - const name = s.repoName?.toLowerCase() || ""; - return `${s.directory.toLowerCase()}:${owner}:${name}`; - }), + installedSkills.map((skill) => buildInstalledSkillIdentityKey(skill)), ); }, [installedSkills]); @@ -148,15 +147,11 @@ export const SkillsPage = forwardRef( const skills: DiscoverableSkillItem[] = useMemo(() => { if (!discoverableSkills) return []; return discoverableSkills.map((d) => { - // 同时处理 / 和 \ 路径分隔符(兼容 Windows 和 Unix) - const installName = - d.directory.split(/[/\\]/).pop()?.toLowerCase() || - d.directory.toLowerCase(); - // 使用 directory + repoOwner + repoName 组合判断是否已安装 - const key = `${installName}:${d.repoOwner.toLowerCase()}:${d.repoName.toLowerCase()}`; return { ...d, - installed: installedKeys.has(key), + installed: installedKeys.has( + buildSkillIdentityKey(d.directory, d.repoOwner, d.repoName), + ), }; }); }, [discoverableSkills, installedKeys]); @@ -194,27 +189,7 @@ export const SkillsPage = forwardRef( readmeUrl: s.readmeUrl, }); - const handleInstall = async (directory: string) => { - let skill: DiscoverableSkill | undefined; - - if (searchSource === "skillssh") { - const found = accumulatedResults.find((s) => s.directory === directory); - if (found) { - skill = toDiscoverableSkill(found); - } - } else { - skill = discoverableSkills?.find( - (s) => - s.directory === directory || - s.directory.split("/").pop() === directory, - ); - } - - if (!skill) { - toast.error(t("skills.notFound")); - return; - } - + const handleInstall = async (skill: DiscoverableSkill) => { try { await installMutation.mutateAsync({ skill, diff --git a/src/components/skills/UnifiedSkillsPanel.tsx b/src/components/skills/UnifiedSkillsPanel.tsx index 701af49292..78e0b579fc 100644 --- a/src/components/skills/UnifiedSkillsPanel.tsx +++ b/src/components/skills/UnifiedSkillsPanel.tsx @@ -30,6 +30,10 @@ import { import type { AppId } from "@/lib/api/types"; import { ConfirmDialog } from "@/components/ConfirmDialog"; import { settingsApi, skillsApi } from "@/lib/api"; +import { + buildInstalledSkillIdentityKey, + getInstalledSkillDirectory, +} from "@/lib/api/skills"; import { toast } from "sonner"; import { MCP_SKILLS_APP_IDS } from "@/config/appConfig"; import { AppCountBar } from "@/components/common/AppCountBar"; @@ -138,11 +142,7 @@ const UnifiedSkillsPanel = React.forwardRef< message: t("skills.uninstallConfirm", { name: skill.name }), onConfirm: async () => { try { - // 构建 skillKey 用于更新 discoverable 缓存 - const installName = - skill.directory.split(/[/\\]/).pop()?.toLowerCase() || - skill.directory.toLowerCase(); - const skillKey = `${installName}:${skill.repoOwner?.toLowerCase() || ""}:${skill.repoName?.toLowerCase() || ""}`; + const skillKey = buildInstalledSkillIdentityKey(skill); const result = await uninstallMutation.mutateAsync({ id: skill.id, @@ -504,6 +504,10 @@ const InstalledSkillListItem: React.FC = ({ } return t("skills.local"); }, [skill.repoOwner, skill.repoName, t]); + const installedDirectory = getInstalledSkillDirectory(skill); + const nestedDirectory = /[\\/]/.test(installedDirectory) + ? installedDirectory + : null; return ( @@ -541,6 +545,14 @@ const InstalledSkillListItem: React.FC = ({ {skill.description}

)} + {nestedDirectory && ( +

+ {nestedDirectory} +

+ )} ( ["skills", "discoverable"], (oldData) => { if (!oldData) return oldData; return oldData.map((s) => { - if (s.key === skillKey) { + if ( + buildSkillIdentityKey(s.directory, s.repoOwner, s.repoName) === + skillKey + ) { return { ...s, installed: true }; } return s; @@ -135,7 +140,10 @@ export function useUninstallSkill() { (oldData) => { if (!oldData) return oldData; return oldData.map((s) => { - if (s.key === skillKey) { + if ( + buildSkillIdentityKey(s.directory, s.repoOwner, s.repoName) === + skillKey + ) { return { ...s, installed: false }; } return s; diff --git a/src/lib/api/skills.ts b/src/lib/api/skills.ts index 456e57b670..fde8737f9b 100644 --- a/src/lib/api/skills.ts +++ b/src/lib/api/skills.ts @@ -122,6 +122,40 @@ export interface SkillRepo { enabled: boolean; } +function normalizeSkillKeyPart(value?: string): string { + return (value ?? "").replace(/\\/g, "/").toLowerCase(); +} + +export function buildSkillIdentityKey( + directory: string, + repoOwner?: string, + repoName?: string, +): string { + return `${normalizeSkillKeyPart(directory)}:${normalizeSkillKeyPart(repoOwner)}:${normalizeSkillKeyPart(repoName)}`; +} + +export function getInstalledSkillDirectory( + skill: Pick, +): string { + const separatorIndex = skill.id.indexOf(":"); + if (separatorIndex === -1) { + return skill.directory; + } + + const directoryFromId = skill.id.slice(separatorIndex + 1); + return directoryFromId || skill.directory; +} + +export function buildInstalledSkillIdentityKey( + skill: Pick, +): string { + return buildSkillIdentityKey( + getInstalledSkillDirectory(skill), + skill.repoOwner, + skill.repoName, + ); +} + // ========== API ========== export const skillsApi = { diff --git a/tests/components/SkillsPage.test.tsx b/tests/components/SkillsPage.test.tsx new file mode 100644 index 0000000000..5624c27c1b --- /dev/null +++ b/tests/components/SkillsPage.test.tsx @@ -0,0 +1,168 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi, beforeEach } from "vitest"; + +import { SkillsPage } from "@/components/skills/SkillsPage"; + +let discoverableSkillsData: any[] = []; +let installedSkillsData: any[] = []; +const installSkillMock = vi.fn(); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => key, + }), +})); + +vi.mock("sonner", () => ({ + toast: { + success: vi.fn(), + error: vi.fn(), + info: vi.fn(), + }, +})); + +vi.mock("@/components/ui/button", () => ({ + Button: ({ children, ...props }: any) => ( + + ), +})); + +vi.mock("@/components/ui/input", () => ({ + Input: (props: any) => , +})); + +vi.mock("@/components/ui/select", () => ({ + Select: ({ children }: any) =>
{children}
, + SelectContent: ({ children }: any) =>
{children}
, + SelectItem: ({ children }: any) =>
{children}
, + SelectTrigger: ({ children }: any) =>
{children}
, + SelectValue: () => null, +})); + +vi.mock("@/components/skills/RepoManagerPanel", () => ({ + RepoManagerPanel: () => null, +})); + +vi.mock("@/components/skills/SkillCard", () => ({ + SkillCard: ({ skill, onInstall }: any) => ( +
+ {skill.name} + {skill.installed ? "installed" : "uninstalled"} + +
+ ), +})); + +vi.mock("@/hooks/useSkills", () => ({ + useDiscoverableSkills: () => ({ + data: discoverableSkillsData, + isLoading: false, + isFetching: false, + refetch: vi.fn(), + }), + useInstalledSkills: () => ({ + data: installedSkillsData, + }), + useInstallSkill: () => ({ + mutateAsync: installSkillMock, + }), + useSkillRepos: () => ({ + data: [], + refetch: vi.fn(), + }), + useAddSkillRepo: () => ({ + mutateAsync: vi.fn(), + }), + useRemoveSkillRepo: () => ({ + mutateAsync: vi.fn(), + }), + useSearchSkillsSh: () => ({ + data: undefined, + isLoading: false, + isFetching: false, + }), +})); + +describe("SkillsPage", () => { + beforeEach(() => { + installSkillMock.mockReset(); + discoverableSkillsData = [ + { + key: "owner/repo:superpowers/using-superpowers", + name: "using-superpowers", + description: "Nested skill", + directory: "superpowers/using-superpowers", + repoOwner: "owner", + repoName: "repo", + repoBranch: "main", + }, + ]; + installedSkillsData = [ + { + id: "owner/repo:superpowers/using-superpowers", + name: "using-superpowers", + description: "Nested skill", + directory: "using-superpowers", + repoOwner: "owner", + repoName: "repo", + apps: { + claude: true, + codex: false, + gemini: false, + opencode: false, + openclaw: false, + }, + installedAt: 1, + }, + ]; + }); + + it("marks nested discoverable skills as installed using the full directory key", () => { + render(); + + expect(screen.getByText("using-superpowers")).toBeInTheDocument(); + expect(screen.getByText("installed")).toBeInTheDocument(); + }); + + it("installs the exact discoverable skill even when directories are duplicated across repos", async () => { + installSkillMock.mockResolvedValue({}); + discoverableSkillsData = [ + { + key: "owner-a/repo-a:shared/skill", + name: "shared-skill-a", + description: "Repo A", + directory: "shared/skill", + repoOwner: "owner-a", + repoName: "repo-a", + repoBranch: "main", + }, + { + key: "owner-b/repo-b:shared/skill", + name: "shared-skill-b", + description: "Repo B", + directory: "shared/skill", + repoOwner: "owner-b", + repoName: "repo-b", + repoBranch: "main", + }, + ]; + installedSkillsData = []; + + render(); + + fireEvent.click(screen.getByRole("button", { name: "install-repo-b" })); + + await waitFor(() => { + expect(installSkillMock).toHaveBeenCalledWith({ + skill: expect.objectContaining({ + key: "owner-b/repo-b:shared/skill", + repoOwner: "owner-b", + repoName: "repo-b", + }), + currentApp: "claude", + }); + }); + }); +}); diff --git a/tests/components/UnifiedSkillsPanel.test.tsx b/tests/components/UnifiedSkillsPanel.test.tsx index 38518ad9b2..fb346cda89 100644 --- a/tests/components/UnifiedSkillsPanel.test.tsx +++ b/tests/components/UnifiedSkillsPanel.test.tsx @@ -1,5 +1,11 @@ import { createRef } from "react"; -import { render, screen, waitFor, act } from "@testing-library/react"; +import { + render, + screen, + waitFor, + act, + fireEvent, +} from "@testing-library/react"; import { describe, expect, it, vi, beforeEach } from "vitest"; import UnifiedSkillsPanel, { @@ -13,6 +19,8 @@ const importSkillsMock = vi.fn(); const installFromZipMock = vi.fn(); const deleteSkillBackupMock = vi.fn(); const restoreSkillBackupMock = vi.fn(); +let installedSkillsData: any[] = []; +let unmanagedSkillsData: any[] = []; vi.mock("sonner", () => ({ toast: { @@ -24,7 +32,7 @@ vi.mock("sonner", () => ({ vi.mock("@/hooks/useSkills", () => ({ useInstalledSkills: () => ({ - data: [], + data: installedSkillsData, isLoading: false, }), useSkillBackups: () => ({ @@ -47,15 +55,7 @@ vi.mock("@/hooks/useSkills", () => ({ mutateAsync: uninstallSkillMock, }), useScanUnmanagedSkills: () => ({ - data: [ - { - directory: "shared-skill", - name: "Shared Skill", - description: "Imported from Claude", - foundIn: ["claude"], - path: "/tmp/shared-skill", - }, - ], + data: unmanagedSkillsData, refetch: scanUnmanagedMock, }), useImportSkillsFromApps: () => ({ @@ -75,18 +75,26 @@ vi.mock("@/hooks/useSkills", () => ({ }), })); +vi.mock("@/components/common/AppCountBar", () => ({ + AppCountBar: ({ totalLabel }: { totalLabel: string }) => ( +
{totalLabel}
+ ), +})); + describe("UnifiedSkillsPanel", () => { beforeEach(() => { + installedSkillsData = []; + unmanagedSkillsData = [ + { + directory: "shared-skill", + name: "Shared Skill", + description: "Imported from Claude", + foundIn: ["claude"], + path: "/tmp/shared-skill", + }, + ]; scanUnmanagedMock.mockResolvedValue({ - data: [ - { - directory: "shared-skill", - name: "Shared Skill", - description: "Imported from Claude", - foundIn: ["claude"], - path: "/tmp/shared-skill", - }, - ], + data: unmanagedSkillsData, }); toggleSkillAppMock.mockReset(); uninstallSkillMock.mockReset(); @@ -117,4 +125,70 @@ describe("UnifiedSkillsPanel", () => { expect(screen.getByText("/tmp/shared-skill")).toBeInTheDocument(); }); }); + + it("shows nested installed skill directory context", async () => { + installedSkillsData = [ + { + id: "owner/repo:superpowers/using-superpowers", + name: "using-superpowers", + description: "Imported from Claude", + directory: "using-superpowers", + repoOwner: "owner", + repoName: "repo", + apps: { + claude: true, + codex: false, + gemini: false, + opencode: false, + openclaw: false, + }, + installedAt: 1, + }, + ]; + + render( + {}} currentApp="claude" />, + ); + + expect(screen.getByText("using-superpowers")).toBeInTheDocument(); + expect( + screen.getByText("superpowers/using-superpowers"), + ).toBeInTheDocument(); + }); + + it("uninstalls nested skills using the full directory identity key", async () => { + uninstallSkillMock.mockResolvedValue({}); + installedSkillsData = [ + { + id: "owner/repo:superpowers/using-superpowers", + name: "using-superpowers", + description: "Imported from Claude", + directory: "using-superpowers", + repoOwner: "owner", + repoName: "repo", + apps: { + claude: true, + codex: false, + gemini: false, + opencode: false, + openclaw: false, + }, + installedAt: 1, + }, + ]; + + render( + {}} currentApp="claude" />, + ); + + fireEvent.click(screen.getByTitle("skills.uninstall")); + fireEvent.click(screen.getByRole("button", { name: "common.confirm" })); + + await waitFor(() => { + expect(uninstallSkillMock).toHaveBeenCalledWith({ + id: "owner/repo:superpowers/using-superpowers", + skillKey: "superpowers/using-superpowers:owner:repo", + }); + }); + }); });