fix(files-widget): navigate directory symlinks in /readfiles (#9)#36
Conversation
Automated review (pr-reviewer subagent)Findings
Verdictneeds attention — the symlink fix is sound overall, but it introduces a real performance regression when a repo contains symlinked directories to large trees. |
Automated re-review (pr-reviewer subagent)FindingsCode looks good. I did not find any new P0–P3 issues, and the prior repo-mode crawl P2 is addressed: Verdictcorrect — the previous performance regression is fixed, and the remaining symlink handling changes look consistent and low-risk. |
Directory symlinks currently break in two ways:
- in non-git folders, readdir({ withFileTypes: true }) returns symlink Dirents
with isDirectory() === false, so /readfiles renders symlinked folders as
inert files that cannot be expanded
- in git repos, tracked/untracked symlinked folders are built from git file
lists and can end up as empty or non-navigable nodes
Fix both code paths:
- browser scan path: detect Dirent.isSymbolicLink(), stat() the target, and
route symlinked directories into the directory branch
- git tree path: classify symlinked directories during buildFileTreeFromPaths
and lazily scan them on expand
- add a small ↗ marker so symlinks are visually distinct
- guard symlink scans against ancestor realpath cycles (foo -> . / foo -> ..)
so expansion does not recurse forever
Also:
- update README to mention the symlink marker/behavior
- files-widget 0.1.16 -> 0.1.17
- root pi-extensions 0.1.32 -> 0.1.33
Closes #9. Thanks to @xapids for reporting.
Addresses pr-reviewer P2: when a git repo contains a symlink into a large tree, expanding it should not auto-enqueue recursive scans of every nested directory. In repo mode, lazy directory scans are now one level deep; nested dirs remain collapsed and scan only when the user expands them.
be6c82f to
dd14cf3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bc7123e98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const relPath = normalizeGitPath(relative(cwd, node.path)); | ||
| node.gitStatus = gitStatus.get(relPath); | ||
| node.diffStats = diffStats.get(relPath); |
There was a problem hiding this comment.
Resolve symlink-expanded paths before git status lookup
After this change, expanding a directory symlink in a git repo creates nodes under the symlink alias path (for example link/foo.ts), but status/diff lookup still keys strictly by relative(cwd, node.path). Git status is reported for the canonical repo path (for example target/foo.ts), so these expanded nodes never get gitStatus/diffStats. As a result, modified files viewed through symlink directories appear clean and can be omitted from changed-only workflows.
Useful? React with 👍 / 👎.
| if (node.expanded && node.children === undefined) { | ||
| enqueueScan(node, getNodeDepth(node, cwd), true); |
There was a problem hiding this comment.
Gate repo symlink scans with safe-mode limits
This now triggers filesystem scans in repo mode whenever an expandable node has children === undefined. For directory symlinks that resolve to very large trees (for example links to parent/root-like paths), expansion can enqueue a large recursive scan without the safe-mode guard used for normal non-repo root scans, which can stall the UI and spike memory. The behavior is input-dependent but reproducible with large symlink targets.
Useful? React with 👍 / 👎.
Fixes #9.
Problem
The current
/readfilestree mishandles directory symlinks.Non-git folders
readdir({ withFileTypes: true })returns symlinkDirents where:The current scan loop only branches on
entry.isDirectory(), so a symlinked folder likelink-to-dir -> target-diris rendered as a plain file and cannot be expanded.Git repos
The git-tree path (
buildFileTreeFromPaths(...)) also lacked explicit symlink handling, so tracked / untracked directory symlinks could end up as empty or non-navigable nodes.Fix
Handle directory symlinks in both code paths:
files-widget/browser.ts)entry.isSymbolicLink()stat()the target to determine whether it resolves to a file or directoryfiles-widget/file-tree.ts+browser.ts::ensureNode)lstatSync()+statSync()toggleDir()to scanchildren === undefineddirs even in repo modeAlso:
↗marker so symlinks are visually distinct in the treerealPath/parenton nodes and guard scans against ancestor-realpath cycles (foo -> .,foo -> ..) so expanding a symlinked folder doesn't recurse foreverVerification
Runtime sanity check: git-tree builder now classifies directory symlinks correctly
$ npx -y tsx /tmp/symlink-tree-test-check.ts {"name":"link-to-dir","path":"/tmp/symlink-tree-test/link-to-dir","isDirectory":true,"isSymlink":true,"hasChildrenArray":false,"realPath":"/private/tmp/symlink-tree-test/target-dir"} {"name":"link-to-file","path":"/tmp/symlink-tree-test/link-to-file","isDirectory":false,"isSymlink":true,"hasChildrenArray":false,"realPath":null} {"name":"real-file.txt","path":"/tmp/symlink-tree-test/real-file.txt","isDirectory":false,"isSymlink":false,"hasChildrenArray":false,"realPath":null}Bundle/syntax sanity check
$ npx -y esbuild files-widget/browser.ts --bundle --platform=node --format=esm \ --external:@mariozechner/pi-coding-agent --external:@mariozechner/pi-tui \ --outfile=/tmp/files-widget-browser-bundle.js ⚡ Done in 14msRelease notes
@tmustier/pi-files-widget0.1.17 → 0.1.18pi-extensions0.1.33 → 0.1.34files-widget/CHANGELOG.mdincludes release notes + credit to @xapidsOne extra note:
files-widget/CHANGELOG.mdonmainalready released the multiline comment editor / markdown toggle / comment confirmation work in 0.1.17. This PR now layers the symlink fix on top as 0.1.18.