Skip to content

fix: 支持配置目录路径中的 ${VAR} 环境变量展开,修复手动删除 skill 后 UI 残留问题#2356

Open
TuYv wants to merge 13 commits intofarion1231:mainfrom
TuYv:fix/path-expand-and-skill-sync
Open

fix: 支持配置目录路径中的 ${VAR} 环境变量展开,修复手动删除 skill 后 UI 残留问题#2356
TuYv wants to merge 13 commits intofarion1231:mainfrom
TuYv:fix/path-expand-and-skill-sync

Conversation

@TuYv
Copy link
Copy Markdown
Contributor

@TuYv TuYv commented Apr 26, 2026

改动说明

修改文件

src/settings.rs

  • 新增 expand_env_vars(),在路径解析前展开 ${VAR} 模式
  • resolve_override_path() 入口调用,原有 ~ 处理逻辑不受影响
  • 新增测试,使用 EnvGuard + #[serial] 做进程级环境变量隔离

src/services/skill.rs

  • get_all_installed() 对每个 skill 核对磁盘上的 SSOT 目录是否存在
  • 目录不存在时先调 remove_from_app() 清理各应用目录的副本/符号链接,再调 delete_skill() 删除数据库记录

验证步骤

  • 在设置页将配置目录填写为 ${HOME}/.claude,确认能正确解析到真实 home 目录,不报 IO 错误
  • 安装一个 skill,手动删除 ~/.cc-switch/skills/<名称> 目录,重启 cc-switch,确认该 skill 不再出现在列表中,且 .claude/.codex 目录中的副本也已清除
  • cargo test 全部通过(14 个测试)

Closes #2342
Closes #2279

TuYv added 2 commits April 26, 2026 22:14
… 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 TuYv changed the title fix: expand ${VAR} in config dir paths and reconcile stale skill DB entries fix: 支持配置目录路径中的 ${VAR} 环境变量展开,修复手动删除 skill 后 UI 残留问题 Apr 26, 2026
TuYv and others added 2 commits April 27, 2026 00:26
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Owner

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src-tauri/src/services/skill.rs Outdated
…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>
@TuYv
Copy link
Copy Markdown
Contributor Author

TuYv commented Apr 27, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src-tauri/src/services/skill.rs Outdated
Comment on lines +574 to +575
for app in AppType::all() {
let _ = Self::remove_from_app(&skill.directory, &app);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Limit stale-skill cleanup to managed app copies

When a SSOT folder is missing, this loop removes skill.directory from every app via remove_from_app, but remove_from_app deletes any existing folder at that path (not only managed symlinks). In the case where a user has an unmanaged/manual skill folder with the same directory name in another app, listing installed skills will unexpectedly delete that folder after SSOT cleanup is triggered. Cleanup should be scoped to apps enabled in skill.apps and/or only paths proven to be managed copies.

Useful? React with 👍 / 👎.

Comment thread src-tauri/src/services/skill.rs Outdated
… 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>
@TuYv
Copy link
Copy Markdown
Contributor Author

TuYv commented Apr 27, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src-tauri/src/services/skill.rs Outdated
Comment thread src-tauri/src/settings.rs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@TuYv
Copy link
Copy Markdown
Contributor Author

TuYv commented Apr 27, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src-tauri/src/services/skill.rs Outdated
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>
@TuYv
Copy link
Copy Markdown
Contributor Author

TuYv commented Apr 27, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src-tauri/src/services/skill.rs Outdated
TuYv and others added 3 commits April 27, 2026 14:03
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>
@TuYv
Copy link
Copy Markdown
Contributor Author

TuYv commented Apr 27, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ 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".

TuYv and others added 2 commits April 27, 2026 15:33
… 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>
@TuYv
Copy link
Copy Markdown
Contributor Author

TuYv commented Apr 27, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

ℹ️ 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".

@TuYv
Copy link
Copy Markdown
Contributor Author

TuYv commented Apr 27, 2026

CR 提到 uninstall_skill_for_app 内部也调用了 get_all_installed让我意识到#2279 的issue比想象的要大 为了避免本 PR 扩大改动范围 我另提了一个issue #2382
主要从以下角度考虑:

  1. 实际触发条件较窄——用户必须先刷新列表(已触发 reconcile),然后趁还没点击卸载之前手动删除文件夹
  2. 该问题不属于本 PR 的原始 scope(claude 、codex手动删除skill后 并且 .cc-switch中skills文件夹也删了但是CCS里面仍然存在删除的skills #2279 只要求"刷新时清理")
  3. 系统性地审计所有 get_all_installed 调用点是一项独立工作,更适合单独跟踪
    我会在后续有空的时候把这块收口掉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants