fix: 支持配置目录路径中的 ${VAR} 环境变量展开，修复手动删除 skill 后 UI 残留问题#2356
fix: 支持配置目录路径中的 ${VAR} 环境变量展开，修复手动删除 skill 后 UI 残留问题#2356TuYv wants to merge 13 commits intofarion1231:mainfrom
Conversation
… 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 (farion1231#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 (farion1231#2279)
- 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
TuYv
changed the title
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f923fcc15d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rors 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 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e3ba390d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… 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 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f82af0ade5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2025c41de7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bbadab991
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
… 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 <noreply@anthropic.com>
…n Ok(false)
- 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 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
CR 提到 uninstall_skill_for_app 内部也调用了 get_all_installed让我意识到#2279 的issue比想象的要大 为了避免本 PR 扩大改动范围 我另提了一个issue #2382
|
2 participants
改动说明
${HOME}及其他${VAR}形式的环境变量展开。此前填写${HOME}/.claude会被当作字面路径，导致 IO 错误。~/.cc-switch/skills/<name>目录后，get_all_installed会检测到 SSOT 目录不存在，自动清理.claude/.codex等应用目录中的 skill 副本，并删除数据库中的残留记录。UI 不再显示已不存在于磁盘的 skill。修改文件
src/settings.rsexpand_env_vars()，在路径解析前展开${VAR}模式resolve_override_path()入口调用，原有~处理逻辑不受影响EnvGuard+#[serial]做进程级环境变量隔离src/services/skill.rsget_all_installed()对每个 skill 核对磁盘上的 SSOT 目录是否存在remove_from_app()清理各应用目录的副本/符号链接，再调delete_skill()删除数据库记录验证步骤
${HOME}/.claude，确认能正确解析到真实 home 目录，不报 IO 错误~/.cc-switch/skills/<名称>目录，重启 cc-switch，确认该 skill 不再出现在列表中，且.claude/.codex目录中的副本也已清除cargo test全部通过（14 个测试）Closes #2342
Closes #2279