-
Notifications
You must be signed in to change notification settings - Fork 426
Fix discovery of symlinked skills #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Remove isDirectory() check to allow symlinks. The existsSync() call on SKILL.md already follows symlinks, so no explicit symlink handling is needed. This allows skills symlinked into e.g. ~/.claude/skills to be discovered by `openskills list`, `openskills sync` and `openskills manage`. Fixes numman-ali#3
…k support Cover: - Skill discovery in project and global directories - Symlinked skill directories - Deduplication and priority handling (project over global) - SKILL.md parsing and skill metadata extraction - Both regular and symlinked skill scenarios Include test isolation and cleanup.
6b9988c to
56cdecb
Compare
|
Thank you for the PR @akaihola, have you checked that symlinked skills have no issue being access by your coding agent? ie when they use the bash tools to read referenced files, or run scripts as part of resources of a skill, it does so without issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for symlinked skill directories, allowing skills to be symlinked from external locations while maintaining compatibility with Claude Code's skills system.
- Removes the
isDirectory()check to allow symlinks to be recognized as valid skill entries - Adds comprehensive test coverage for symlinked skills including edge cases like deduplication and priority handling
- Updates documentation to mention symlink support
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils/skills.ts | Removes isDirectory() check to support symlinked directories as valid skill entries |
| tests/utils/skills.test.ts | Adds comprehensive test suite covering symlinked skills, deduplication, and priority handling |
| README.md | Updates documentation to mention symlink support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
I verified that the agent in Zed is able to read and use symlinked skills. It also ran scripts in the skill successfully using the absolute path pointing inside the symlinked directory. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add `description` parameter to `createSymlinkedSkill` helper, and remove the redundant `createSkill` call in symlinked skill test. Pass `description` directly to `createSymlinkedSkill` instead.
|
@numman-ali should I refine this further to help prepare for merging? |
|
Thank you for your help Released in v1.3.0 |
Fixes #3
Symlinked skill directories are no longer skipped. This allows skills symlinked into e.g.
~/.claude/skillsto be discovered byopenskills list,openskills syncandopenskills manage.Implementation
In
findAllSkills(), removeisDirectory()check to allow symlinks.The
existsSync()call onSKILL.mdalready follows symlinks, so no explicit symlink handling is needed.Tests
skills.test.tsadds tests forfindSkill()andfindAllSkills(), including symlink support.The added tests cover:
Test isolation and cleanup are included.