fix(skills): detect nested SKILL.md in unmanaged scan#1917
fix(skills): detect nested SKILL.md in unmanaged scan#1917zerone0x wants to merge 1 commit intofarion1231:mainfrom
Conversation
Fixes farion1231#1842 Co-Authored-By: Claude <[email protected]>
|
@codex review |
![chatgpt-codex-connector[bot]](https://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: 0e4b659dcb
ℹ️ 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".
| continue; | ||
| } | ||
| let dir_name = entry.file_name().to_string_lossy().to_string(); | ||
| let dir_name = rel.to_string_lossy().to_string(); |
There was a problem hiding this comment.
Using rel.to_string_lossy() as dir_name makes unmanaged nested skills keep their full relative path (for example repo/skills/foo) as InstalledSkill.directory when imported, but downstream install-state logic still compares by basename (e.g., Path::file_name() in list_skills and install). In that flow, an imported nested skill is shown as not installed and can be reinstalled under a different directory key, which desynchronizes metadata and leaves stale paths behind. Please normalize directory identity consistently (or update all consumers to treat nested keys as canonical).
Useful? React with 👍 / 👎.
| let entries = match fs::read_dir(scan_dir) { | ||
| Ok(e) => e, | ||
| Err(_) => continue, | ||
| let skill_dirs = match Self::scan_skills_in_dir(scan_dir) { |
There was a problem hiding this comment.
scan_unmanaged now calls the recursive scanner for user skill roots, but the recursion follows symlinked directories (path.is_dir()) without any visited-path guard. A symlink cycle inside a skills folder (for example loop -> ..) will recurse forever and can hang or stack-overflow unmanaged scanning. Since this path is now exercised on normal app directories, the scan should skip symlink dirs or track canonicalized visited directories.
Useful? React with 👍 / 👎.
2 participants
Summary / 概述
Support nested skill layouts when scanning for unmanaged skills by recursively finding SKILL.md and using the relative path as the directory key. This restores detection for repos that bundle multiple skills under subfolders (e.g. ai-skills/skills//SKILL.md).
Related Issue / 关联 Issue
Fixes #1842
Screenshots / 截图
N/A
Checklist / 检查清单
Checking formatting...
All matched files use Prettier code style! passes / 通过代码格式检查 (not run)