From 6b2aa11e5b30a2758b9f5be92bc4f610efc17c39 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Sun, 26 Apr 2026 22:14:52 +0800 Subject: [PATCH 01/13] fix: expand env vars in config dir paths and reconcile stale skill DB entries - settings: add expand_env_vars() so ${HOME}/... and other ${VAR}/... patterns in claude/codex/gemini config dir settings are resolved correctly instead of being treated as literal path strings (#2342) - skill: get_all_installed() now checks each skill's SSOT directory exists on disk; missing entries are removed from the database so the UI no longer shows skills that were manually deleted (#2279) --- src-tauri/src/services/skill.rs | 18 ++++++++- src-tauri/src/settings.rs | 67 +++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 06d159677..0251ce013 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -556,9 +556,25 @@ impl SkillService { // ========== 统一管理方法 ========== /// 获取所有已安装的 Skills + /// + /// 同时核对磁盘：若 SSOT 目录下的 skill 文件夹已被手动删除，自动清理 DB 记录。 pub fn get_all_installed(db: &Arc) -> Result> { let skills = db.get_all_installed_skills()?; - Ok(skills.into_values().collect()) + let ssot_dir = Self::get_ssot_dir().ok(); + let mut result = Vec::new(); + for (id, skill) in skills { + let exists = ssot_dir + .as_ref() + .map(|d| d.join(&skill.directory).exists()) + .unwrap_or(true); + if exists { + result.push(skill); + } else { + let _ = db.delete_skill(&id); + log::info!("skill '{}' directory missing from SSOT, removed from database", skill.name); + } + } + Ok(result) } /// 安装 Skill diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index 704ab71eb..d71844df0 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -478,7 +478,31 @@ fn settings_store() -> &'static RwLock { SETTINGS_STORE.get_or_init(|| RwLock::new(AppSettings::load_from_file())) } +fn expand_env_vars(s: &str) -> String { + let mut result = s.to_string(); + let mut pos = 0; + loop { + let Some(rel_start) = result[pos..].find("${") else { + break; + }; + let start = pos + rel_start; + let Some(rel_end) = result[start + 2..].find('}') else { + break; + }; + let end = start + 2 + rel_end; + let var_name = result[start + 2..end].to_string(); + if let Ok(val) = std::env::var(&var_name) { + result.replace_range(start..=end, &val); + pos = start + val.len(); + } else { + pos = end + 1; + } + } + result +} + fn resolve_override_path(raw: &str) -> PathBuf { + let raw = &expand_env_vars(raw); if raw == "~" { if let Some(home) = dirs::home_dir() { return home; @@ -768,3 +792,46 @@ pub fn update_webdav_sync_status(status: WebDavSyncStatus) -> Result<(), AppErro } }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn expand_env_vars_replaces_home() { + std::env::set_var("HOME", "/Users/test"); + assert_eq!(expand_env_vars("${HOME}/.claude"), "/Users/test/.claude"); + assert_eq!(expand_env_vars("${HOME}"), "/Users/test"); + } + + #[test] + fn expand_env_vars_multiple_vars() { + std::env::set_var("FOO", "foo"); + std::env::set_var("BAR", "bar"); + assert_eq!(expand_env_vars("${FOO}/${BAR}"), "foo/bar"); + } + + #[test] + fn expand_env_vars_unknown_var_left_as_is() { + let s = "${DEFINITELY_NOT_SET_XYZ}"; + assert_eq!(expand_env_vars(s), s); + } + + #[test] + fn expand_env_vars_no_vars_unchanged() { + assert_eq!(expand_env_vars("/absolute/path"), "/absolute/path"); + assert_eq!(expand_env_vars("~/path"), "~/path"); + } + + #[test] + fn resolve_override_path_expands_home_var() { + if let Some(home) = dirs::home_dir() { + let home_str = home.to_string_lossy(); + std::env::set_var("HOME", home_str.as_ref()); + assert_eq!( + resolve_override_path("${HOME}/.claude"), + home.join(".claude") + ); + } + } +} From f88396fce01918bf1386f120b2ffddc7a6e56c65 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Sun, 26 Apr 2026 22:22:38 +0800 Subject: [PATCH 02/13] fix: address code review issues in path expand and skill sync - skill: remove synced app copies before dropping stale DB record so orphaned skill folders are not left in .claude/.codex etc directories - settings: add EnvGuard + serial_test isolation to HOME-mutating tests to prevent parallel test order-dependency --- src-tauri/src/services/skill.rs | 5 ++++ src-tauri/src/settings.rs | 43 +++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 0251ce013..2226e24a8 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -570,6 +570,11 @@ impl SkillService { if exists { result.push(skill); } else { + // Clean up app copies before removing the DB record so the + // enabled-app directories don't contain orphaned skill folders. + for app in AppType::all() { + let _ = Self::remove_from_app(&skill.directory, &app); + } let _ = db.delete_skill(&id); log::info!("skill '{}' directory missing from SSOT, removed from database", skill.name); } diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index d71844df0..8bde3410b 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -796,18 +796,49 @@ pub fn update_webdav_sync_status(status: WebDavSyncStatus) -> Result<(), AppErro #[cfg(test)] mod tests { use super::*; + use serial_test::serial; + use std::sync::{Mutex, OnceLock}; + + fn env_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + } + + struct EnvGuard { + key: &'static str, + original: Option, + } + + impl EnvGuard { + fn set(key: &'static str, value: &str) -> Self { + let original = std::env::var(key).ok(); + std::env::set_var(key, value); + Self { key, original } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + match &self.original { + Some(v) => std::env::set_var(self.key, v), + None => std::env::remove_var(self.key), + } + } + } #[test] + #[serial] fn expand_env_vars_replaces_home() { - std::env::set_var("HOME", "/Users/test"); + let _guard = env_lock().lock().unwrap(); + let _home = EnvGuard::set("HOME", "/Users/test"); assert_eq!(expand_env_vars("${HOME}/.claude"), "/Users/test/.claude"); assert_eq!(expand_env_vars("${HOME}"), "/Users/test"); } #[test] fn expand_env_vars_multiple_vars() { - std::env::set_var("FOO", "foo"); - std::env::set_var("BAR", "bar"); + let _foo = EnvGuard::set("FOO", "foo"); + let _bar = EnvGuard::set("BAR", "bar"); assert_eq!(expand_env_vars("${FOO}/${BAR}"), "foo/bar"); } @@ -824,10 +855,12 @@ mod tests { } #[test] + #[serial] fn resolve_override_path_expands_home_var() { + let _guard = env_lock().lock().unwrap(); if let Some(home) = dirs::home_dir() { - let home_str = home.to_string_lossy(); - std::env::set_var("HOME", home_str.as_ref()); + let home_str = home.to_string_lossy().to_string(); + let _home = EnvGuard::set("HOME", &home_str); assert_eq!( resolve_override_path("${HOME}/.claude"), home.join(".claude") From 5d94950f7086d968fd5610fd5b15831ed630d589 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 00:26:55 +0800 Subject: [PATCH 03/13] style: fix rustfmt line length in skill sync log statement Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/services/skill.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 2226e24a8..77e5555ef 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -576,7 +576,10 @@ impl SkillService { let _ = Self::remove_from_app(&skill.directory, &app); } let _ = db.delete_skill(&id); - log::info!("skill '{}' directory missing from SSOT, removed from database", skill.name); + log::info!( + "skill '{}' directory missing from SSOT, removed from database", + skill.name + ); } } Ok(result) From f923fcc15dce05e8311bc4c7bf4428acfdcdf4e8 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 00:32:23 +0800 Subject: [PATCH 04/13] style: rewrite loop-with-let-else as while-let to satisfy clippy Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/settings.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index 8bde3410b..999af627f 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -481,10 +481,7 @@ fn settings_store() -> &'static RwLock { fn expand_env_vars(s: &str) -> String { let mut result = s.to_string(); let mut pos = 0; - loop { - let Some(rel_start) = result[pos..].find("${") else { - break; - }; + while let Some(rel_start) = result[pos..].find("${") { let start = pos + rel_start; let Some(rel_end) = result[start + 2..].find('}') else { break; From 6e3ba390d9cd2b65d55f5517ddb837bcad49ae7f Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 09:51:33 +0800 Subject: [PATCH 05/13] fix: use try_exists() to avoid false-positive skill pruning on I/O errors Path::exists() silently returns false on permission/IO errors, which could cause valid skills to be deleted. Switch to try_exists() and only prune when the result is definitively Ok(false). Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/services/skill.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 77e5555ef..41cef8fda 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -563,13 +563,12 @@ impl SkillService { let ssot_dir = Self::get_ssot_dir().ok(); let mut result = Vec::new(); for (id, skill) in skills { - let exists = ssot_dir + let missing = ssot_dir .as_ref() - .map(|d| d.join(&skill.directory).exists()) - .unwrap_or(true); - if exists { - result.push(skill); - } else { + .and_then(|d| d.join(&skill.directory).try_exists().ok()) + .map(|exists| !exists) + .unwrap_or(false); + if missing { // Clean up app copies before removing the DB record so the // enabled-app directories don't contain orphaned skill folders. for app in AppType::all() { @@ -580,6 +579,8 @@ impl SkillService { "skill '{}' directory missing from SSOT, removed from database", skill.name ); + } else { + result.push(skill); } } Ok(result) From f82af0ade5ef93ac1177903d7e898d68a7198ff8 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 10:19:00 +0800 Subject: [PATCH 06/13] fix: scope stale-skill cleanup to enabled apps and handle DB deletion errors - Replace AppType::all() with skill.apps.enabled_apps() so only managed app copies are removed, avoiding accidental deletion of unmanaged folders - Delete DB record before touching app dirs; on failure keep skill in result list so DB and UI stay in sync instead of silently diverging Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/services/skill.rs | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 41cef8fda..2e82a31bd 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -569,16 +569,27 @@ impl SkillService { .map(|exists| !exists) .unwrap_or(false); if missing { - // Clean up app copies before removing the DB record so the - // enabled-app directories don't contain orphaned skill folders. - for app in AppType::all() { - let _ = Self::remove_from_app(&skill.directory, &app); + // Delete the DB record first. If that fails (locked/read-only DB), + // leave the skill visible and skip filesystem cleanup so the DB and + // app directories stay in sync. + match db.delete_skill(&id) { + Ok(_) => { + for app in skill.apps.enabled_apps() { + let _ = Self::remove_from_app(&skill.directory, &app); + } + log::info!( + "skill '{}' directory missing from SSOT, removed from database", + skill.name + ); + } + Err(e) => { + log::warn!( + "skill '{}' SSOT directory missing but DB deletion failed, keeping record: {}", + skill.name, e + ); + result.push(skill); + } } - let _ = db.delete_skill(&id); - log::info!( - "skill '{}' directory missing from SSOT, removed from database", - skill.name - ); } else { result.push(skill); } From 2025c41de761db48bbe54573c01ad00086845d1b Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 11:14:21 +0800 Subject: [PATCH 07/13] test: serialize expand_env_vars_multiple_vars to prevent env-var races Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/settings.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index 999af627f..ce2b3b190 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -833,7 +833,9 @@ mod tests { } #[test] + #[serial] fn expand_env_vars_multiple_vars() { + let _guard = env_lock().lock().unwrap(); let _foo = EnvGuard::set("FOO", "foo"); let _bar = EnvGuard::set("BAR", "bar"); assert_eq!(expand_env_vars("${FOO}/${BAR}"), "foo/bar"); From 9bbadab991b4892eb9096d5c767bc76ff806e659 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 13:45:34 +0800 Subject: [PATCH 08/13] fix: guard get_all_installed pruning during migrate_storage file moves migrate_storage moves skill dirs before updating skill_storage_location; if get_all_installed runs in that window it sees skills as missing in the old SSOT path and deletes their DB records. Add a MIGRATION_IN_PROGRESS AtomicBool (reset via RAII guard on any exit path) so that get_all_installed skips pruning while a migration is in flight. Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/services/skill.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 2e82a31bd..6322d2092 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::fs; use std::path::{Component, Path, PathBuf}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use tokio::time::timeout; @@ -19,6 +20,10 @@ use crate::config::get_app_config_dir; use crate::database::Database; use crate::error::format_skill_error; +/// Set to true while migrate_storage is moving files so that get_all_installed +/// does not prune DB records that are temporarily absent from the old SSOT path. +static MIGRATION_IN_PROGRESS: AtomicBool = AtomicBool::new(false); + // ========== 数据结构 ========== /// Skill 同步方式 @@ -568,7 +573,7 @@ impl SkillService { .and_then(|d| d.join(&skill.directory).try_exists().ok()) .map(|exists| !exists) .unwrap_or(false); - if missing { + if missing && !MIGRATION_IN_PROGRESS.load(Ordering::Acquire) { // Delete the DB record first. If that fails (locked/read-only DB), // leave the skill visible and skip filesystem cleanup so the DB and // app directories stay in sync. @@ -1207,6 +1212,16 @@ impl SkillService { fs::create_dir_all(&new_dir)?; // 2. 逐个移动 skill 目录 + // Guard against get_all_installed pruning records while files are in flight. + // The guard resets the flag on drop, so any early-return also clears it. + struct MigrationGuard; + impl Drop for MigrationGuard { + fn drop(&mut self) { + MIGRATION_IN_PROGRESS.store(false, Ordering::Release); + } + } + MIGRATION_IN_PROGRESS.store(true, Ordering::Release); + let _migration_guard = MigrationGuard; let skills = db.get_all_installed_skills()?; let mut result = MigrationResult { migrated_count: 0, @@ -1242,7 +1257,7 @@ impl SkillService { } } - // 3. 文件移动完成后才持久化设置 + // 3. 文件移动完成后才持久化设置（_migration_guard 在此作用域末尾自动清 flag） crate::settings::set_skill_storage_location(target)?; // 4. 刷新所有应用目录的 symlink（指向新 SSOT） From 454ad976cc74709f6ce71ff7d575f9b3b48ca4de Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 14:03:52 +0800 Subject: [PATCH 09/13] fix: check all SSOT locations before pruning stale skill records The MIGRATION_IN_PROGRESS AtomicBool only protects against in-flight races but not against a crash after files are moved and before settings are persisted: on next startup the flag is false, yet the current SSOT path no longer contains the skills. Replace the single-path check with all_ssot_dirs(), which returns both the current configured path and the alternate one. A skill is only pruned when its directory is definitively absent (try_exists() == Ok(false)) from every possible location. This handles both the race and the crash-recovery case without any mutable state, so the MIGRATION_IN_PROGRESS guard is removed. Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/services/skill.rs | 60 ++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 6322d2092..f6f61faa7 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -11,7 +11,6 @@ use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::fs; use std::path::{Component, Path, PathBuf}; -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use tokio::time::timeout; @@ -20,10 +19,6 @@ use crate::config::get_app_config_dir; use crate::database::Database; use crate::error::format_skill_error; -/// Set to true while migrate_storage is moving files so that get_all_installed -/// does not prune DB records that are temporarily absent from the old SSOT path. -static MIGRATION_IN_PROGRESS: AtomicBool = AtomicBool::new(false); - // ========== 数据结构 ========== /// Skill 同步方式 @@ -498,6 +493,33 @@ impl SkillService { Ok(dir) } + /// Returns all candidate SSOT directories (current + alternate location). + /// + /// Used by get_all_installed to avoid pruning skill records that are absent + /// from the current SSOT path but present in the alternate one — this covers + /// both the in-flight migration race and the crash-after-move / before-settings + /// recovery case where settings still point to the old path. + fn all_ssot_dirs() -> Vec { + let mut dirs = Vec::new(); + // Current configured location (may fail if home dir is unavailable) + if let Ok(d) = Self::get_ssot_dir() { + dirs.push(d); + } + // Alternate location + let alternate = match crate::settings::get_skill_storage_location() { + SkillStorageLocation::CcSwitch => { + dirs::home_dir().map(|h| h.join(".agents").join("skills")) + } + SkillStorageLocation::Unified => Some(get_app_config_dir().join("skills")), + }; + if let Some(alt) = alternate { + if !dirs.contains(&alt) { + dirs.push(alt); + } + } + dirs + } + /// 获取 Skill 卸载备份目录（~/.cc-switch/skill-backups/） fn get_backup_dir() -> Result { let dir = get_app_config_dir().join("skill-backups"); @@ -565,15 +587,17 @@ impl SkillService { /// 同时核对磁盘：若 SSOT 目录下的 skill 文件夹已被手动删除，自动清理 DB 记录。 pub fn get_all_installed(db: &Arc) -> Result> { let skills = db.get_all_installed_skills()?; - let ssot_dir = Self::get_ssot_dir().ok(); + let ssot_dirs = Self::all_ssot_dirs(); let mut result = Vec::new(); for (id, skill) in skills { - let missing = ssot_dir - .as_ref() - .and_then(|d| d.join(&skill.directory).try_exists().ok()) - .map(|exists| !exists) - .unwrap_or(false); - if missing && !MIGRATION_IN_PROGRESS.load(Ordering::Acquire) { + // Only prune when the skill dir is definitively absent from every + // possible SSOT location. Checking all locations guards against both + // in-flight migration races and crash-after-move / before-settings-persist + // restarts where files landed in the alternate path. + let missing = ssot_dirs + .iter() + .all(|d| matches!(d.join(&skill.directory).try_exists(), Ok(false))); + if missing { // Delete the DB record first. If that fails (locked/read-only DB), // leave the skill visible and skip filesystem cleanup so the DB and // app directories stay in sync. @@ -1212,16 +1236,6 @@ impl SkillService { fs::create_dir_all(&new_dir)?; // 2. 逐个移动 skill 目录 - // Guard against get_all_installed pruning records while files are in flight. - // The guard resets the flag on drop, so any early-return also clears it. - struct MigrationGuard; - impl Drop for MigrationGuard { - fn drop(&mut self) { - MIGRATION_IN_PROGRESS.store(false, Ordering::Release); - } - } - MIGRATION_IN_PROGRESS.store(true, Ordering::Release); - let _migration_guard = MigrationGuard; let skills = db.get_all_installed_skills()?; let mut result = MigrationResult { migrated_count: 0, @@ -1257,7 +1271,7 @@ impl SkillService { } } - // 3. 文件移动完成后才持久化设置（_migration_guard 在此作用域末尾自动清 flag） + // 3. 文件移动完成后才持久化设置 crate::settings::set_skill_storage_location(target)?; // 4. 刷新所有应用目录的 symlink（指向新 SSOT） From 16f5e065cf8bf1ad35de5e736cdc7f158a662304 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 14:19:02 +0800 Subject: [PATCH 10/13] fix: guard against vacuous-truth pruning when no SSOT path is resolvable Iterator::all() returns true on an empty iterator. If all_ssot_dirs() returns an empty vec (e.g. create_dir_all fails and home_dir is None), every skill would be falsely marked missing and deleted. Add an explicit !ssot_dirs.is_empty() guard so pruning is skipped when no SSOT location can be resolved. Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/services/skill.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index f6f61faa7..3d4e5331e 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -594,9 +594,12 @@ impl SkillService { // possible SSOT location. Checking all locations guards against both // in-flight migration races and crash-after-move / before-settings-persist // restarts where files landed in the alternate path. - let missing = ssot_dirs - .iter() - .all(|d| matches!(d.join(&skill.directory).try_exists(), Ok(false))); + // Vacuous truth: if no SSOT path is resolvable we cannot verify + // absence, so skip pruning to avoid data loss. + let missing = !ssot_dirs.is_empty() + && ssot_dirs + .iter() + .all(|d| matches!(d.join(&skill.directory).try_exists(), Ok(false))); if missing { // Delete the DB record first. If that fails (locked/read-only DB), // leave the skill visible and skip filesystem cleanup so the DB and From 50629547806c3c8a01421d375f35ee6643ce0885 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 14:42:22 +0800 Subject: [PATCH 11/13] refactor: move stale-skill cleanup out of get_all_installed into startup get_all_installed is restored to a pure read. A new reconcile_stale_entries function owns the pruning logic and is called once from setup() after the database is ready. Also: - extract ssot_dir_for() so all_ssot_dirs() derives both locations from a single source of truth instead of duplicating path strings - log warnings when try_exists fails or remove_from_app fails instead of silently swallowing errors Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/lib.rs | 8 ++ src-tauri/src/services/skill.rs | 131 ++++++++++++++++++-------------- 2 files changed, 83 insertions(+), 56 deletions(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 9e7b937ce..7945394ca 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -813,6 +813,14 @@ pub fn run() { let skill_service = SkillService::new(); app.manage(commands::skill::SkillServiceState(Arc::new(skill_service))); + // 清理磁盘上已不存在的 skill 记录（用户手动删除 SSOT 目录后的自愈） + { + let db = &app.state::().db; + if let Err(e) = SkillService::reconcile_stale_entries(db) { + log::warn!("reconcile_stale_entries failed: {e}"); + } + } + // 初始化 CopilotAuthManager { use crate::proxy::providers::copilot_auth::CopilotAuthManager; diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 3d4e5331e..b0b04c5a5 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -493,31 +493,31 @@ impl SkillService { Ok(dir) } - /// Returns all candidate SSOT directories (current + alternate location). - /// - /// Used by get_all_installed to avoid pruning skill records that are absent - /// from the current SSOT path but present in the alternate one — this covers - /// both the in-flight migration race and the crash-after-move / before-settings - /// recovery case where settings still point to the old path. - fn all_ssot_dirs() -> Vec { - let mut dirs = Vec::new(); - // Current configured location (may fail if home dir is unavailable) - if let Ok(d) = Self::get_ssot_dir() { - dirs.push(d); - } - // Alternate location - let alternate = match crate::settings::get_skill_storage_location() { - SkillStorageLocation::CcSwitch => { + /// Returns the SSOT path for a given storage location without creating it. + fn ssot_dir_for(loc: SkillStorageLocation) -> Option { + match loc { + SkillStorageLocation::CcSwitch => Some(get_app_config_dir().join("skills")), + SkillStorageLocation::Unified => { dirs::home_dir().map(|h| h.join(".agents").join("skills")) } - SkillStorageLocation::Unified => Some(get_app_config_dir().join("skills")), - }; - if let Some(alt) = alternate { - if !dirs.contains(&alt) { - dirs.push(alt); - } } - dirs + } + + /// Returns all candidate SSOT directories (current + alternate) without creating them. + /// + /// Checking both locations guards against in-flight migration races and + /// crash-after-move / before-settings-persist restarts where files landed + /// in the alternate path while settings still point to the old one. + fn all_ssot_dirs() -> Vec { + let current_loc = crate::settings::get_skill_storage_location(); + let alternate_loc = match current_loc { + SkillStorageLocation::CcSwitch => SkillStorageLocation::Unified, + SkillStorageLocation::Unified => SkillStorageLocation::CcSwitch, + }; + [current_loc, alternate_loc] + .into_iter() + .filter_map(Self::ssot_dir_for) + .collect() } /// 获取 Skill 卸载备份目录（~/.cc-switch/skill-backups/） @@ -582,51 +582,70 @@ impl SkillService { // ========== 统一管理方法 ========== - /// 获取所有已安装的 Skills - /// - /// 同时核对磁盘：若 SSOT 目录下的 skill 文件夹已被手动删除，自动清理 DB 记录。 + /// 获取所有已安装的 Skills（纯读，不修改任何状态） pub fn get_all_installed(db: &Arc) -> Result> { + Ok(db.get_all_installed_skills()?.into_values().collect()) + } + + /// 清理磁盘上已不存在的 skill 记录。 + /// + /// 应在应用启动时调用一次。若 skill 的 SSOT 目录在所有候选路径下均确认不存在， + /// 则删除其 DB 记录并清理各已启用 app 下的副本。 + pub fn reconcile_stale_entries(db: &Arc) -> Result<()> { let skills = db.get_all_installed_skills()?; let ssot_dirs = Self::all_ssot_dirs(); - let mut result = Vec::new(); + + if ssot_dirs.is_empty() { + log::warn!("reconcile_stale_entries: no SSOT path resolvable, skipping cleanup"); + return Ok(()); + } + for (id, skill) in skills { - // Only prune when the skill dir is definitively absent from every - // possible SSOT location. Checking all locations guards against both - // in-flight migration races and crash-after-move / before-settings-persist - // restarts where files landed in the alternate path. - // Vacuous truth: if no SSOT path is resolvable we cannot verify - // absence, so skip pruning to avoid data loss. - let missing = !ssot_dirs.is_empty() - && ssot_dirs - .iter() - .all(|d| matches!(d.join(&skill.directory).try_exists(), Ok(false))); - if missing { - // Delete the DB record first. If that fails (locked/read-only DB), - // leave the skill visible and skip filesystem cleanup so the DB and - // app directories stay in sync. - match db.delete_skill(&id) { - Ok(_) => { - for app in skill.apps.enabled_apps() { - let _ = Self::remove_from_app(&skill.directory, &app); - } - log::info!( - "skill '{}' directory missing from SSOT, removed from database", - skill.name - ); - } + let missing = ssot_dirs.iter().all(|d| { + let path = d.join(&skill.directory); + match path.try_exists() { + Ok(exists) => !exists, Err(e) => { log::warn!( - "skill '{}' SSOT directory missing but DB deletion failed, keeping record: {}", - skill.name, e + "skill '{}': could not check path {}: {e}", + skill.name, + path.display() ); - result.push(skill); + false // conservative: treat as present } } - } else { - result.push(skill); + }); + + if !missing { + continue; + } + + // Delete the DB record first. If that fails (locked/read-only DB), + // skip filesystem cleanup so DB and app directories stay in sync. + match db.delete_skill(&id) { + Ok(_) => { + for app in skill.apps.enabled_apps() { + if let Err(e) = Self::remove_from_app(&skill.directory, &app) { + log::warn!( + "skill '{}': failed to remove app copy for {app:?}: {e}", + skill.name + ); + } + } + log::info!( + "skill '{}' directory missing from SSOT, removed from database", + skill.name + ); + } + Err(e) => { + log::warn!( + "skill '{}' SSOT directory missing but DB deletion failed, keeping record: {e}", + skill.name + ); + } } } - Ok(result) + Ok(()) } /// 安装 Skill From 4fc0c32fed0a08a053b5b960a26c1e56bfe152cf Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 15:33:25 +0800 Subject: [PATCH 12/13] refactor: reconcile stale skills in get_installed_skills command, not at startup Moving reconcile_stale_entries from setup() to the get_installed_skills Tauri command so stale entries are cleaned up whenever the frontend refreshes the list, not only on next restart. get_all_installed remains a pure read; the reconcile step is explicit in the one command that serves the list to the UI. Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/commands/skill.rs | 5 +++++ src-tauri/src/lib.rs | 7 ------- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/commands/skill.rs b/src-tauri/src/commands/skill.rs index 695e8d08a..19c4d695e 100644 --- a/src-tauri/src/commands/skill.rs +++ b/src-tauri/src/commands/skill.rs @@ -33,8 +33,13 @@ fn parse_app_type(app: &str) -> Result { // ========== 统一管理命令 ========== /// 获取所有已安装的 Skills +/// +/// 每次调用前先清理磁盘上已不存在的记录，确保前端看到的列表与文件系统一致。 #[tauri::command] pub fn get_installed_skills(app_state: State<'_, AppState>) -> Result, String> { + if let Err(e) = SkillService::reconcile_stale_entries(&app_state.db) { + log::warn!("reconcile_stale_entries failed: {e}"); + } SkillService::get_all_installed(&app_state.db).map_err(|e| e.to_string()) } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 7945394ca..b6043fdb5 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -813,13 +813,6 @@ pub fn run() { let skill_service = SkillService::new(); app.manage(commands::skill::SkillServiceState(Arc::new(skill_service))); - // 清理磁盘上已不存在的 skill 记录（用户手动删除 SSOT 目录后的自愈） - { - let db = &app.state::().db; - if let Err(e) = SkillService::reconcile_stale_entries(db) { - log::warn!("reconcile_stale_entries failed: {e}"); - } - } // 初始化 CopilotAuthManager { From 9666a81ad5a0710b73a41e0aa2a31a09d174cfbe Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Mon, 27 Apr 2026 15:55:35 +0800 Subject: [PATCH 13/13] fix: tighten reconcile_stale_entries doc and skip redundant cleanup on Ok(false) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Doc comment was misleading ("应在应用启动时调用一次") since the function is now invoked from the get_installed_skills command on every refresh. Restate it as idempotent and list its side effects explicitly. - delete_skill returns Ok(false) when no row matches, which can happen if a concurrent reconcile already cleaned this entry. Skip the app-copy cleanup in that case to avoid redundant filesystem ops and log noise. Co-Authored-By: Claude Sonnet 4.6 --- src-tauri/src/services/skill.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index b0b04c5a5..6ba4c4741 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -587,10 +587,12 @@ impl SkillService { Ok(db.get_all_installed_skills()?.into_values().collect()) } - /// 清理磁盘上已不存在的 skill 记录。 + /// 清理磁盘上已不存在的 skill 记录（幂等，可重复调用）。 /// - /// 应在应用启动时调用一次。若 skill 的 SSOT 目录在所有候选路径下均确认不存在， - /// 则删除其 DB 记录并清理各已启用 app 下的副本。 + /// 若 skill 的 SSOT 目录在所有候选路径下均确认不存在，则删除其 DB 记录 + /// 并清理各已启用 app 下的副本。 + /// + /// 副作用：可能修改数据库（删除 skill 记录）和文件系统（删除 app 目录下的副本）。 pub fn reconcile_stale_entries(db: &Arc) -> Result<()> { let skills = db.get_all_installed_skills()?; let ssot_dirs = Self::all_ssot_dirs(); @@ -623,7 +625,7 @@ impl SkillService { // Delete the DB record first. If that fails (locked/read-only DB), // skip filesystem cleanup so DB and app directories stay in sync. match db.delete_skill(&id) { - Ok(_) => { + Ok(true) => { for app in skill.apps.enabled_apps() { if let Err(e) = Self::remove_from_app(&skill.directory, &app) { log::warn!( @@ -637,6 +639,9 @@ impl SkillService { skill.name ); } + Ok(false) => { + // 记录已被并发的另一次 reconcile 删除，无需重复清理 app 副本 + } Err(e) => { log::warn!( "skill '{}' SSOT directory missing but DB deletion failed, keeping record: {e}",