Skip to content

fix(files-widget): navigate directory symlinks in /readfiles (#9)#36

Merged
tmustier merged 2 commits into
mainfrom
feat/files-widget-symlinks
Apr 19, 2026
Merged

fix(files-widget): navigate directory symlinks in /readfiles (#9)#36
tmustier merged 2 commits into
mainfrom
feat/files-widget-symlinks

Conversation

@tmustier
Copy link
Copy Markdown
Owner

@tmustier tmustier commented Apr 19, 2026

Fixes #9.

Problem

The current /readfiles tree mishandles directory symlinks.

Non-git folders

readdir({ withFileTypes: true }) returns symlink Dirents where:

entry.isDirectory() === false
entry.isFile() === false
entry.isSymbolicLink() === true

The current scan loop only branches on entry.isDirectory(), so a symlinked folder like link-to-dir -> target-dir is 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:

  • filesystem scan path (files-widget/browser.ts)
    • detect entry.isSymbolicLink()
    • stat() the target to determine whether it resolves to a file or directory
    • route symlinked directories into the directory branch so they can be expanded
  • git-tree path (files-widget/file-tree.ts + browser.ts::ensureNode)
    • classify symlinked directories with lstatSync() + statSync()
    • create them as lazy directory nodes
    • allow toggleDir() to scan children === undefined dirs even in repo mode
  • repo-mode scan depth
    • in git repos, expanding a symlinked directory only scans one level on demand
    • nested directories stay lazy until explicitly expanded, so links into large trees (iCloud/Drive/$HOME) do not trigger a broad recursive crawl

Also:

  • add a subtle marker so symlinks are visually distinct in the tree
  • store realPath/parent on nodes and guard scans against ancestor-realpath cycles (foo -> ., foo -> ..) so expanding a symlinked folder doesn't recurse forever

Verification

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 14ms

Release notes

  • @tmustier/pi-files-widget 0.1.17 → 0.1.18
  • pi-extensions 0.1.33 → 0.1.34
  • files-widget/CHANGELOG.md includes release notes + credit to @xapids

One extra note: files-widget/CHANGELOG.md on main already 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.

@tmustier
Copy link
Copy Markdown
Owner Author

Automated review (pr-reviewer subagent)

Findings

  • [P2] Expanding a symlinked folder in repo mode can trigger a broad recursive filesystem crawl
    File: files-widget/browser.ts:404
    Repo mode still uses scanState.mode === "none", and shouldAutoScan() treats that the same as full scan mode (depth <= MAX_TREE_DEPTH). Combined with the new toggleDir() behavior, expanding one lazy symlinked directory in a git repo now auto-enqueues recursive scans of every nested directory under the link target. If the link points at something large like iCloud/Drive or $HOME, this bypasses the existing safe-mode protections and can make /readfiles unexpectedly expensive.
    What should change: In repo mode, treat symlink-directory expansion as an on-demand one-level scan (or apply the safe-mode limits) instead of recursively auto-scanning descendants immediately.

Verdict

needs attention — the symlink fix is sound overall, but it introduces a real performance regression when a repo contains symlinked directories to large trees.

@tmustier
Copy link
Copy Markdown
Owner Author

Automated re-review (pr-reviewer subagent)

Findings

Code looks good. I did not find any new P0–P3 issues, and the prior repo-mode crawl P2 is addressed: shouldAutoScan() now disables recursive auto-scan in repo mode, so expanding a symlinked folder only loads one level and keeps deeper directories lazy until explicitly expanded.

Verdict

correct — the previous performance regression is fixed, and the remaining symlink handling changes look consistent and low-risk.

thomas added 2 commits April 19, 2026 22:51
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.
@tmustier tmustier force-pushed the feat/files-widget-symlinks branch from be6c82f to dd14cf3 Compare April 19, 2026 21:51
@tmustier tmustier merged commit fbc746d into main Apr 19, 2026
1 check passed
@tmustier tmustier deleted the feat/files-widget-symlinks branch April 19, 2026 21:52
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread files-widget/browser.ts
Comment on lines 581 to 583
const relPath = normalizeGitPath(relative(cwd, node.path));
node.gitStatus = gitStatus.get(relPath);
node.diffStats = diffStats.get(relPath);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread files-widget/browser.ts
Comment on lines +800 to 801
if (node.expanded && node.children === undefined) {
enqueueScan(node, getNodeDepth(node, cwd), true);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Symlinks not being shown on MacOS

2 participants