feat(skills): follow symlinked directories for user and repo skills #525
Conversation
Previously, symlinks were unconditionally skipped during skill discovery. This made it impossible to symlink skill directories from a shared location (e.g., a dotfiles repo or cross-agent skill collection). Changes: - Follow symlinked directories for User and Repo scoped skills - System skills still skip symlinks (security: managed by the tool) - Add cycle detection to prevent infinite loops from circular symlinks - Symlinked files are not followed (only symlinked directories) - Add unit tests for all three scenarios This aligns with user expectations - if you can write to ~/.code/skills/, you can already create real directories there, so symlinks pose no additional security risk for user-scoped skills. Fixes common workflow where developers symlink skills from: - A shared dotfiles repository - A cross-agent skills collection (Claude Code, Codex, Cursor, etc.) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![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: 593d2a3815
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| use super::*; | ||
| use std::os::unix::fs::symlink; | ||
| use tempfile::TempDir; |
There was a problem hiding this comment.
These new tests unconditionally import std::os::unix::fs::symlink, which is unavailable on Windows. As a result, cargo test on Windows will fail to compile this module even before running tests. If Windows CI/devs run tests (the repo ships Windows builds), this becomes an immediate test break. Consider adding #[cfg(unix)] to the test module or providing a Windows-specific symlink helper (e.g., std::os::windows::fs::symlink_dir) to keep tests portable.
Useful? React with 👍 / 👎.
Uses std::os::unix::fs::symlink on Unix and std::os::windows::fs::symlink_dir on Windows, allowing tests to run on both platforms.
Cygnusfear
force-pushed
the
feat/follow-symlinked-skills
branch
from
2e2f32d to
10cbe90
Compare
|
Thanks for the review! Fixed in 10cbe90. Added a cross-platform
Tests now run on both platforms. |
1 participant
Enables skill discovery to follow symlinked directories, allowing developers to symlink skills from shared locations (dotfiles repos, cross-agent skill collections).
Key Changes
How Symlink Following Works
flowchart TD A[Discover Skills] --> B{Is Symlink?} B -->|No| C[Process normally] B -->|Yes| D{Scope?} D -->|System| E[Skip - security] D -->|User/Repo| F{Points to dir?} F -->|No| G[Skip file symlinks] F -->|Yes| H{Already visited?} H -->|Yes| I[Skip - cycle detection] H -->|No| J[Add to queue & visited set]Related Issues
Related (upstream):
This PR brings the symlink fix to Every Code, since the upstream Codex rejected it.
Testing
follows_symlinked_directory_for_user_scope,ignores_symlinked_directory_for_system_scope,handles_circular_symlink_without_infinite_loopFiles Changed
code-rs/core/src/skills/loader.rs- Symlink following logic + testsdocs/skills.md- Updated docs to reflect new behavior