feat(projects): show a symlink indicator in Add Project file browsers - #8998
feat(projects): show a symlink indicator in Add Project file browsers#8998choihjin wants to merge 2 commits into
Conversation
…ect browsers The Add Project host folder browser classified a symlink-to-directory as a file: browseServerDir used Dirent.isDirectory(), which does not follow symlinks, and the SSH browse command relied on `ls -p`, which does not dereference links. Classify symlink entries by their target type (stat for the local/server listing, `[ -d ]` for the remote POSIX listing) so linked project folders can be descended into and selected. Fixes stablyai#8983
Symlinked entries were visually identical to real files and directories in the Add Project / Add Remote Project browsers. Render a muted link icon next to symlinked entries, matching the existing file-explorer pattern. The SSH browse protocol now carries symlink state: the POSIX listing emits a `<l|-><d|->/name` prefix (`[ -h ]` for the link flag, `[ -d ]` for the dir flag), the Windows PowerShell fallback flags links via $_.LinkType, and RemoteDirEntry / DirEntry gain isSymlink. The local/server browseServerDir path already returned isSymlink. Fixes stablyai#8989
ec38d36 to
c10f15e
Compare
📝 WalkthroughWalkthroughRemote directory entries now include 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 72a0abc4-54ef-4baf-a6b7-bc69a797ec0d
📒 Files selected for processing (16)
src/main/ipc/ssh-browse.test.tssrc/main/ipc/ssh-browse.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/sidebar/RemoteFileBrowser.paste.test.tsxsrc/renderer/src/components/sidebar/RemoteFileBrowser.symlink-indicator.test.tsxsrc/renderer/src/components/sidebar/RemoteFileBrowser.tsxsrc/renderer/src/components/sidebar/remote-file-browser-helpers.test.tssrc/renderer/src/components/sidebar/remote-file-browser-helpers.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.json
| const mapped = await Promise.all( | ||
| entries | ||
| .filter((entry) => entry.name !== '.' && entry.name !== '..') | ||
| .map(async (entry) => ({ | ||
| name: entry.name, | ||
| isDirectory: await isServerBrowseEntryDirectory(dirPath, entry), | ||
| isSymlink: entry.isSymbolicLink() | ||
| })) | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Limit concurrency to prevent EMFILE errors on directories with many symlinks.
Using an unbounded Promise.all to stat all symlinks concurrently can trigger EMFILE (too many open files) errors if the directory contains a large number of symbolic links. Because isServerBrowseEntryDirectory catches all errors and silently returns false, affected symlinked directories will incorrectly appear as non-directories in the UI.
Consider processing the entries sequentially to cap concurrent file descriptors.
💡 Proposed fix using sequential processing
- const mapped = await Promise.all(
- entries
- .filter((entry) => entry.name !== '.' && entry.name !== '..')
- .map(async (entry) => ({
- name: entry.name,
- isDirectory: await isServerBrowseEntryDirectory(dirPath, entry),
- isSymlink: entry.isSymbolicLink()
- }))
- )
+ const mapped: DirEntry[] = []
+ for (const entry of entries) {
+ if (entry.name === '.' || entry.name === '..') {
+ continue
+ }
+ mapped.push({
+ name: entry.name,
+ isDirectory: await isServerBrowseEntryDirectory(dirPath, entry),
+ isSymlink: entry.isSymbolicLink()
+ })
+ }📝 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.
| const mapped = await Promise.all( | |
| entries | |
| .filter((entry) => entry.name !== '.' && entry.name !== '..') | |
| .map(async (entry) => ({ | |
| name: entry.name, | |
| isDirectory: await isServerBrowseEntryDirectory(dirPath, entry), | |
| isSymlink: entry.isSymbolicLink() | |
| })) | |
| ) | |
| const mapped: DirEntry[] = [] | |
| for (const entry of entries) { | |
| if (entry.name === '.' || entry.name === '..') { | |
| continue | |
| } | |
| mapped.push({ | |
| name: entry.name, | |
| isDirectory: await isServerBrowseEntryDirectory(dirPath, entry), | |
| isSymlink: entry.isSymbolicLink() | |
| }) | |
| } |
There was a problem hiding this comment.
This intentionally mirrors the existing fs.readDir handler in src/relay/fs-handler.ts (lines 141–147), which uses the same unbounded Promise.all over isDirectoryEntry and is test-locked — browseServerDir was written to match that behavior.
The concurrency here is bounded by the number of symlinked entries in a single browsed directory (non-symlinks skip stat entirely), so for a project-picker listing it isn't a realistic EMFILE trigger. Adding a concurrency cap to only this side would diverge from the relay twin it deliberately matches; if the fd concern is worth addressing, it's really a pre-existing, codebase-wide item that should be a separate change applied to both handlers rather than a divergence introduced here.
Leaving as-is for parity with the established handler.
3 participants
Summary
Fixes #8989. Stacked on #8987 — that fix is the first commit here; until it merges, this PR's diff shows both commits. Once #8987 lands, this reduces to the single indicator commit.
After #8987, symlinked entries behave as directories in the Add Project / Add Remote Project browsers but are visually indistinguishable from real folders. This adds a muted link indicator next to symlinked entries, matching the in-app file explorer's existing pattern (
FileExplorerRow).To support this on remote hosts, the
ssh:browseDirlisting protocol now carries symlink state:<l|-><d|->/nameper entry — a symlink flag from[ -h ]and a directory flag from[ -d ](which follows the link, so a symlinked directory still classifies as a directory). Names can't contain/, so the first/reliably ends the two-flag prefix; any other stdout noise (motd, blank lines) is skipped by the parser.$_.LinkType(populated only for symlinks/junctions), matching Node'sisSymbolicLink()on the local/server path. It deliberately does not use the genericReparsePointattribute, which is also set on OneDrive/cloud placeholders and would mislabel ordinary files as links.RemoteDirEntry/DirEntrygainisSymlink; the local/serverbrowseServerDirpath already returned it.Screenshots
Remote file browser on a real Linux SSH host running this branch.
linked-dir→real-dirandlinked-file→real file.txtare symlinks and now show the link indicator; the real directory and file do not:Testing
pnpm lintpnpm typecheckpnpm test(30,680 passed; note: run with the shell'sGIT_CONFIG_*vars unset — an unrelated relay test asserts an exact git-config count and picks up Orca's own terminal env otherwise)pnpm buildTests:
ssh-browse.test.ts: locks the new prefix-format command, asserts symlink flags parse correctly for symlinked dirs/files and broken links, and pins the PowerShellLinkTypedetection (with an explicitnot.toContain('ReparsePoint')).RemoteFileBrowser.symlink-indicator.test.tsx(new): renders the browser and asserts the link indicator appears only on symlinked rows.DirEntryfixtures updated for the new field.Manual verification: drove the built dev app end-to-end against a real Linux SSH host — the indicator appears on exactly the symlinked entries and not on real files/dirs (screenshot above). The new POSIX listing snippet was also exercised directly in
shagainst fixtures with spaces in names, symlinked dirs/files, and broken links. The Windows PowerShell branch is covered by unit tests (no Windows SSH host available to drive live).AI Review Report
Reviewed by Claude (Fable 5, author) and independently by Codex (gpt-5.6-sol, reasoning effort high) via
codex review.[IO.FileAttributes]::ReparsePoint, which also matches OneDrive Files On-Demand placeholders and other filter-driver files — it would have shown false-positive link indicators on non-link entries, diverging from the POSIX and local paths. Switched to$_.LinkType, which is populated only for actual symlinks/junctions, and added a regression assertion. No other issues flagged.sh([ -h ],[ -d ],printf,command ls -1A) — no bashisms. Windows branch keeps the forward-slashresolvedPathand adds link detection. Icon rendering is platform-neutral.lsoutput is captured before the classify loop). TheisSymlinkfield flows through preload → renderer for both the SSH and runtime-server paths.shellEscape; the added flag/prefix logic is static text operating onlsoutput within the escaped directory. The PowerShell literal keeps its single-quote doubling (asserted in tests).Security Audit
No new IPC endpoints, auth, secrets, or dependencies. No new injection surface (see above). The change only adds classification metadata to entries of a directory the user explicitly browses.
Notes
isSymlink.ELI5
In the Add Project folder browser, symlinked folders now show a small link icon. You can tell them apart from normal directories the same way as in the main file explorer.