fix(filesystem): resolve symlink/junction targets to directory type in readDirectoryEntries#28532
Conversation
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
mapwith async mapping +Promise.allto allowstatcalls during classification. - Resolves symlink targets with
NFS.statto classify symlinks-to-directories as"directory".
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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 |
|
Not a duplicate. #28531 uses This PR uses |
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? |
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
force-pushed
the
fix/symlink-directory-listing-clean
branch
from
8559f60 to
eccb75a
Compare
danielxxomg
left a comment
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.
|
Rebased this against Looks like Tests are still green on my side:
Covered suites:
Also worth noting: the existing security check in |
|
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..) |
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 becausereadDirectoryEntriesclassifies them as"symlink"and downstream filters skip non-directory types.This fixes the root cause in
packages/core/src/filesystem.ts:readDirectoryEntries: whenisSymbolicLink()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