Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 58a5c33a9e09624584c25f11834d17fabae1348d and 81d99a3. 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughRemote POSIX browsing now captures 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Follow-up #8998 (stacked on this) adds a visual symlink indicator in the same browsers, so a linked directory is distinguishable from a real one — the natural next step now that they behave identically. It also extends |
…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
58a5c33 to
81d99a3
Compare
|
Taking a look! |
Summary
Fixes #8983.
In the Add Project / Add Remote Project folder browsers, a symbolic link pointing to a directory was listed as a file — no folder icon, not navigable — so any project living behind a symlinked path could not be reached or selected.
Two classification sites had the same bug:
browseServerDir(src/main/runtime/orca-runtime.ts) usedDirent.isDirectory(), which does not follow symlinks. Symlink entries are now classified by their target type viastat, mirroring the existing (and test-locked) behavior of the relayfs.readDirhandler insrc/relay/fs-handler.ts.ssh:browseDirPOSIX branch (src/main/ipc/ssh-browse.ts) derived the directory marker fromls -p, which does not dereference links. The listing now appends the/marker via[ -d ](which follows symlinks) in a plain POSIXshloop. Thelsoutput is captured into a variable first so a failingls(e.g. permission denied) still propagates through the&&chain as a non-zero exit code instead of being masked by the while-loop pipeline — preserving the existing error semantics.Deliberately unchanged: the local file-explorer
fs:readDirand runtime file listing intentionally treat symlinks as file-like to avoid macOS TCC prompts (documented and test-locked); the Windows remote fallback already classifies symlinked directories correctly viaPSIsContainer.Screenshots
Remote filesystem browser (Add Remote Project) on a real Linux SSH host, running this branch.
linked-diris a symlink toreal-dir; it now renders as a navigable directory, sorts with directories, and can be descended into and selected as a project (breadcrumb reached/tmp/orca-symlink-verify/linked-dir):Before this change the same entry rendered with a file icon and could not be entered.
Testing
pnpm lintpnpm typecheckpnpm testpnpm buildNew regression tests (both watched failing before the fix):
orca-runtime.test.ts:browseServerDirover a real temp directory containing a symlinked directory, a symlinked file, and a broken symlink — asserts symlink-to-dir classifies as a directory, symlink-to-file and broken links stay file-like, and sort order holds. Usesjunctionon Windows, matching the existing relay symlink test.ssh-browse.test.ts: locks the new POSIX listing command and asserts symlinked-directory entries parse as directories; existing command-string assertions updated via a shared helper.Manual verification: drove the built dev app end-to-end against a real Linux SSH host — browsed into the symlinked directory and confirmed it is selectable as a project. The new shell snippet was additionally exercised directly in
shagainst fixtures with spaces in names, an empty directory, a broken symlink, and a permission-denied directory (still exits non-zero).AI Review Report
Review by Claude (Fable 5), which also authored the change. Checks performed:
browseServerDirchange is purenode:fsand platform-neutral; Node reports Windows junctions as symlinks, and the new test usesjunctionon win32. The SSH change touches only the POSIX branch; the Windows PowerShell fallback (PSIsContainer, already symlink-correct) is untouched. No shortcuts, labels, or path-separator handling involved.fs.readDirpath already handled symlinks and is unchanged.shconstructs (command ls -1A,[ -d ],printf,while IFS= read -r) — no bashisms, nofind -maxdepth, nopipefail— so dash/busybox login shells are fine. Filenames with spaces are preserved; filenames containing newlines remain a pre-existing limitation of the line-based protocol, unchanged by this PR.statcalls are issued only for symlink entries and run concurrently; non-symlink entries take the previous fast path. Remote listing adds one subshell and a builtin test per entry inside the existing single exec.stat-validated, so selecting a symlinked folder keeps working as before.Security Audit
shellEscapefor the only user-controlled input (the directory path); the added shell text is a static string with no interpolation, and"$f"values come fromlsoutput inside the already-escaped target directory — no new injection surface.Notes
ELI5
Symlinked folders in Add Project looked like plain files and couldn’t be opened. They’re treated as real directories now so you can navigate into them and pick projects.