fix(filesystem): resolve symlink/junction targets to directory type in readDirectoryEntries - #28532
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates directory listing logic to better classify filesystem entries, especially symbolic links that point to directories.
Changes:
- Replaces synchronous map with async mapping + Promise.all to allow stat calls during classification.
- Resolves symlink targets with NFS.stat to classify symlinks-to-directories as "directory".
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| return await Promise.all( | ||
| entries.map(async (e): Promise<DirEntry> => { | ||
| let type: DirEntry["type"] | ||
| if (e.isDirectory()) { | ||
| type = "directory" | ||
| } else if (e.isSymbolicLink()) { | ||
| try { | ||
| const target = await NFS.stat(join(dirPath, e.name)) | ||
| type = target.isDirectory() ? "directory" : "symlink" | ||
| } catch { | ||
| type = "symlink" | ||
| } | ||
| } else if (e.isFile()) { | ||
| type = "file" | ||
| } else { | ||
| type = "other" | ||
| } | ||
| return { name: e.name, type } | ||
| }), | ||
| ) |
| return await Promise.all( | ||
| entries.map(async (e): Promise<DirEntry> => { | ||
| let type: DirEntry["type"] | ||
| if (e.isDirectory()) { | ||
| type = "directory" | ||
| } else if (e.isSymbolicLink()) { | ||
| try { | ||
| const target = await NFS.stat(join(dirPath, e.name)) | ||
| type = target.isDirectory() ? "directory" : "symlink" | ||
| } catch { | ||
| type = "symlink" | ||
| } | ||
| } else if (e.isFile()) { | ||
| type = "file" | ||
| } else { | ||
| type = "other" | ||
| } | ||
| return { name: e.name, type } | ||
| }), | ||
| ) |
|
The following comment was made by an LLM, it may be inaccurate: Potential duplicate found:
Why they're related: Both PRs address the same core issue of treating symlinked/junction directories as the "directory" type in readDirectoryEntries. PR #28531 appears to be the earlier attempt at the same fix. You should review whether #28531 is already merged, closed, or if this is a newer/refined version of the same fix. |
Sorry, something went wrong.
|
Not a duplicate. #28531 uses e.isDirectory() inside the ternary, which doesn't resolve Windows junction points (reparse points). isDirectory() returns false for those. This PR uses NFS.stat() to explicitly follow the link target, which correctly identifies junction-to-directory on Windows, symlink-to-directory on Linux, and aliases on macOS. The extra stat() only fires on isSymbolicLink() entries, so non-symlink entries have zero overhead. |
Sorry, something went wrong.
rg.files() in scan() was not passing --follow, so files inside symlinked directories were invisible to @file autocomplete. the remaining readDirectoryEntries consumer fixes (symlinked dirs in list() and the global home scan path) are covered by anomalyco#28532 which fixes the root cause in readDirectoryEntries itself. closes anomalyco#29080
|
Looking forward to this @danielxxomg seems like there are some merge conflicts that need to be resolved? |
Sorry, something went wrong.
rg.files() in scan() was not passing --follow, so files inside symlinked directories were invisible to @file autocomplete. the remaining readDirectoryEntries consumer fixes (symlinked dirs in list() and the global home scan path) are covered by anomalyco#28532 which fixes the root cause in readDirectoryEntries itself. closes anomalyco#29080
rg.files() in scan() was not passing --follow, so files inside symlinked directories were invisible to @file autocomplete. the remaining readDirectoryEntries consumer fixes (symlinked dirs in list() and the global home scan path) are covered by anomalyco#28532 which fixes the root cause in readDirectoryEntries itself. closes anomalyco#29080
…n readDirectoryEntries When isSymbolicLink() is true (Linux symlinks, Windows junction points like OneDrive Desktop), stat() the target to determine if it points to a directory. Previously these entries were classified as 'symlink' and silently filtered out by downstream code (file/index.ts), making symlinked directories invisible in the project picker, @file picker, and file listing on ALL platforms. This fixes the root cause instead of patching individual filter sites. Closes anomalyco#28526, also resolves anomalyco#10365 and anomalyco#16342.
danielxxomg
left a comment
There was a problem hiding this comment.
Addressing the Copilot review comments:
On DirEntry.type semantics: The intent is that callers that need to distinguish real directories from symlinked ones can check entry.uri (which contains the real path via pathToFileURL). For the directory picker and file listing use case, treating symlinks-to-directories as "directory" is the desired behavior — downstream filters skip type !== "directory", so this is the minimal fix that unblocks all consumers without changing filter logic.
On unbounded Promise.all: NFS.stat() only runs when isSymbolicLink() is true. Most directories have 0-2 symlinks, so the unbounded Promise.all is negligible in practice. The stat calls are I/O-bound, not CPU-bound.
Sorry, something went wrong.
|
Rebased this against dev. Looks like readDirectoryEntries was moved to fs-util.ts in #30447, so I updated the fix to apply there now. Tests are still green on my side: 99 pass, 0 fail Covered suites:
Also worth noting: the existing security check in entry() is still unchanged. |
Sorry, something went wrong.
|
Just wanted to share my use case for this: I create a symlink in repo work dir to my notes dir for a project in Obsidian vault, so the agent working in the repo can directly read/write notes in Obsidian vault. Another way of using it is perhaps having a virtual workspace where user symlink several projects in one place to allow agent to work on several things at once. There should still be a permission prompt when reading symlink if it's outside project dir, otherwise it becomes an easy attach surface (creating a symlink in project that points to system root elevates opencode to global file access..) |
Sorry, something went wrong.
|
Waiting to see this PR merged. There are now inconsistency between Desktop and Web. In Desktop the Electron uses native window to open a project, which accepts symlink folders, yet in Web it doesn't show these folders. |
Sorry, something went wrong.
Issue for this PR
Closes #28526
Type of change
What does this PR do?
Symlinked directories (Linux ln -s) and Windows junction points (e.g., OneDrive Desktop) are invisible in the directory picker, @file picker, and file listing because readDirectoryEntries classifies them as "symlink" and downstream filters skip non-directory types.
This fixes the root cause in packages/core/src/filesystem.ts:readDirectoryEntries: when isSymbolicLink() is true, stat() the target and return "directory" if the target is a directory. This fixes ALL consumers at once — no downstream filter changes needed.
Also resolves long-standing open issues:
How did you verify your code works?
bun test test/file/ — 87 pass, 1 skip, 0 fail. No regressions.
Screenshots / recordings
N/A, no UI changes.
Checklist