fix(skills): discover skill directories that are symlinks - #1048
fix(skills): discover skill directories that are symlinks#1048shokosanma wants to merge 1 commit into
Conversation
Dirent.isDirectory() is false for symlinks even when the target is a directory, so ~/.cursor/skills style links were skipped during discovery. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughChangesProvider skill discovery
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/shared/utils.ts`:
- Around line 913-916: Update the recursive traversal around collectRecursive to
resolve each directory to its canonical real path and maintain a
visited-directory set; skip descent when that canonical path has already been
visited, while preserving normal symlink-to-directory traversal and
deduplication. Add a regression test covering a self-referential or
parent-directory symlink to verify traversal terminates without duplicate
results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a5d51d74-d61e-431c-bfcd-7681a3cc8997
📒 Files selected for processing (1)
server/shared/utils.ts
| // Symlinks to skill directories are common in ~/.cursor/skills; Dirent.isDirectory() | ||
| // is false for links even when the target is a directory. | ||
| if (entry.isDirectory() || entry.isSymbolicLink()) { | ||
| await collectRecursive(path.join(dirPath, entry.name)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard recursive traversal against symlink cycles.
Following every symlink without tracking visited directories allows links such as skills/loop -> .. to recurse indefinitely, producing duplicate results and potentially exhausting I/O or failing with excessively long paths. Track each directory by its canonical/real path before descending.
Proposed fix
+import { realpath } from 'node:fs/promises';
export async function findProviderSkillMarkdownFiles(...) {
const skillFiles: string[] = [];
+ const visitedDirectories = new Set<string>();
const collectRecursive = async (dirPath: string): Promise<void> => {
+ let canonicalPath: string;
+ try {
+ canonicalPath = await realpath(dirPath);
+ } catch {
+ return;
+ }
+ if (visitedDirectories.has(canonicalPath)) {
+ return;
+ }
+ visitedDirectories.add(canonicalPath);
let entries;Please also add a regression test for a self-referential or parent-directory symlink.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Symlinks to skill directories are common in ~/.cursor/skills; Dirent.isDirectory() | |
| // is false for links even when the target is a directory. | |
| if (entry.isDirectory() || entry.isSymbolicLink()) { | |
| await collectRecursive(path.join(dirPath, entry.name)); | |
| import { realpath } from 'node:fs/promises'; | |
| export async function findProviderSkillMarkdownFiles(...) { | |
| const skillFiles: string[] = []; | |
| const visitedDirectories = new Set<string>(); | |
| const collectRecursive = async (dirPath: string): Promise<void> => { | |
| let canonicalPath: string; | |
| try { | |
| canonicalPath = await realpath(dirPath); | |
| } catch { | |
| return; | |
| } | |
| if (visitedDirectories.has(canonicalPath)) { | |
| return; | |
| } | |
| visitedDirectories.add(canonicalPath); | |
| // Symlinks to skill directories are common in ~/.cursor/skills; Dirent.isDirectory() | |
| // is false for links even when the target is a directory. | |
| if (entry.isDirectory() || entry.isSymbolicLink()) { | |
| await collectRecursive(path.join(dirPath, entry.name)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/shared/utils.ts` around lines 913 - 916, Update the recursive
traversal around collectRecursive to resolve each directory to its canonical
real path and maintain a visited-directory set; skip descent when that canonical
path has already been visited, while preserving normal symlink-to-directory
traversal and deduplication. Add a regression test covering a self-referential
or parent-directory symlink to verify traversal terminates without duplicate
results.
1 participant
Summary
SKILL.mdfiles.~/.cursor/skillswhereDirent.isDirectory()is false for symlinks even when the target is a directory.Test plan
~/.cursor/skills/my-skill→ real dir withSKILL.md)Made with Cursor
Summary by CodeRabbit
SKILL.mdfiles within symlinked directories.