Skip to content

fix(files): list the changed files of a project behind a symlink - #1387

Open
brynne8 wants to merge 2 commits into
agegr:mainfrom
brynne8:fix/git-status-symlinked-cwd
Open

brynne8 wants to merge 2 commits into
agegr:mainfrom
brynne8:fix/git-status-symlinked-cwd

Conversation

@brynne8

@brynne8 brynne8 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

What it does

A project reached through a symlink had an empty changed-file list: no changed badge, no line counts, no entries at all.

  • resolveExistingPath() (lib/paths.ts): resolves the nearest existing ancestor and keeps the remainder as written, so a deleted file still works. samePath() stays lexical and says so; this is the resolving half.
  • getGitStatus() (lib/git-changes.ts): rebases every path git reports onto the cwd the caller asked about, and takes the line-stat pathspec from the resolved cwd.
  • getGitFileDiff() (lib/git-changes.ts): compares and derives its pathspec from the resolved pair, so a path named through a link still resolves to the file git means — and a path that resolves outside the repository stays refused.

A folder that is not in a repository keeps answering isGitRepository: false with nothing listed, and that is now pinned by a test.

Why it matters (the broken behaviour)

On the base commit, lib/git-changes.ts:

  • getGitStatus(cwd) asks git for the repository root with git rev-parse --show-toplevel (findRepositoryRoot, lib/git-changes.ts:30-32). That answer is the repository's real path.
  • It computes each file's path as path.resolve(repositoryRoot, entry.path) and keeps it only if isWithinPath(cwd, filePath) — a string comparison against the cwd the caller passed (lib/git-changes.ts:123-124).
  • The line-stat pathspec is derived the same way (readTrackedLineStats, lib/git-changes.ts:61-66, called with the caller's cwd at lib/git-changes.ts:120).
  • The diff lookup repeats the shape: getGitFileDiff() compares the file against the repository root as strings (lib/git-changes.ts:207-209).

On macOS those two never match when the project is behind a link: git says /private/var/folders/…/project (the real path), the caller says /var/folders/…/project. path.resolve does not resolve symlinks, so every file is filtered out and files comes back empty — and the diff lookup answers supported: false. The same is true for /tmp (a link to /private/tmp) and for any project the operator opened through a link of their own.

How to reproduce (on main, before the fix)

  1. Make a repository and modify a tracked file in it:

    mkdir -p /tmp/pi-web-repro/repo && cd /tmp/pi-web-repro/repo
    git init -q && echo one > a.txt && git add a.txt && git commit -qm seed
    echo two > a.txt
    
  2. Link to it and open Pi Web on the link:

    ln -s /tmp/pi-web-repro/repo /tmp/pi-web-repro/link
    
  3. Open /tmp/pi-web-repro/link in Pi Web.

Before the fix the Git panel is empty — no changed badge on a.txt, no line counts, no entries — although the file is modified. The same happens for any project reached through /tmp or ~/Library/Caches, which are links on macOS. After the fix the file is listed as M with its line counts.

Testing

Node 26.7.0 (source ~/.nvm/nvm.sh && nvm use 26.7.0), in the worktree.

  • npm test — pass: tests 2874 / suites 16 / pass 2873 / fail 0 / skipped 1 (the one skip is the pre-existing Windows-only junction test in app/api/git/diff/route.test.mjs).
  • node_modules/.bin/tsc --noEmit — clean, no output.
  • npm run lint — clean, no output.

The new test fails on the untouched base commit and passes with the fix. On the base commit (git checkout origin/main -- lib/git-changes.ts lib/paths.ts, tests kept):

$ node --experimental-strip-types --test lib/git-changes.test.mjs
ℹ tests 5
ℹ pass 3
ℹ fail 2

✖ a repository reached through a link reports files under the folder it was given (173.405166ms)
  AssertionError [ERR_ASSERTION]: a row's path carries its change; listed []
      at TestContext.<anonymous> (file:///…/lib/git-changes.test.mjs:99:12)

✖ a repository above the folder asked about contributes no rows (123.972458ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected

  + []
  - [
  -   '/var/folders/…/pi-web-git-linked-kF6oU4/link-repo/sub/b.txt'
  - ]

With the fix:

$ node --experimental-strip-types --test lib/git-changes.test.mjs lib/paths.test.mjs app/api/git/diff/route.test.mjs
ℹ tests 20
ℹ pass 19
ℹ fail 0
ℹ skipped 1

The test builds a repository in a temporary folder, makes a symlink of its own to it (so it works on any POSIX system, without relying on /tmp being a link) and is skipped on Windows.

Provenance

Ported by hand from the Bun-running fork https://github.com/brynne8/pi-web-bun, commit 478d5178 ("fix(files): name a changed file by the path the tree was given"). The fork's commit also touched lib/git-revert.ts — a write path — which has the same cause (it derives the checkout's path from the unresolved pair). That part is deliberately not part of this PR and should be done separately.

Notes

  • Read path only: the status listing and the diff lookup. No new setting, no new dependency, no user-facing string, no change to pi or to what pi writes to session files.
  • Access is not widened: a file that resolves outside the repository the caller asked about is still refused (getGitFileDiff drops any candidate path that leaves the repository, and the routes' allowed-root rules are unchanged).

`git rev-parse --show-toplevel` answers with the repository's real path, while
the caller asks about the cwd it was given: `/tmp` and `/var/folders/…` are
links on macOS, so the two spell the same folder differently. Every path git
reported was then resolved against the real root and kept only if it matched
the caller's cwd as a string, which never happens through a link — a project
reached that way had an empty changed-file list: no changed badge, no line
counts, no entries at all.

- `resolveExistingPath()` (`lib/paths.ts`) resolves the nearest existing
  ancestor and keeps the remainder as written, so a deleted file works too.
  `samePath()` stays lexical and says so; this is the resolving half.
- `getGitStatus()` rebases every reported path onto the cwd it was asked
  about, and takes its line-stat pathspec from the resolved cwd.
- `getGitFileDiff()` compares and derives its pathspec from the resolved pair,
  so a path named through a link still resolves to the file Git means, and a
  path that leaves the repository stays refused.
@brynne8

brynne8 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

The first run's e2e job was red on an unrelated flake — e2e/session-navigation.mjs timed out waiting for a browser page event (Timeout 30000ms exceeded while waiting for event "page"), with no Git or file access involved:

✖ two tabs keep their own sessions through reloads; tab memory and ?session= are not crossed
  browserContext.waitForEvent: Timeout 30000ms exceeded while waiting for event "page"
      at newTab (/home/runner/work/pi-web/pi-web/e2e/session-navigation.mjs:61:28)

The checks job (lint, tsc --noEmit, npm test) was green in that run. I cannot re-run the upstream workflow from a fork (it needs admin rights), so I pushed an empty commit to re-trigger it; the second run is green in both jobs (checks pass, e2e pass). The empty commit changes no file — the diff is the one commit above.

This branch has not been deployed

No deployments
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.

1 participant