Skip to content

Commit bfcbfa4

Browse files
committed
Git browser: fix virtual portal eviction
- Frontend: `FilePane`'s "directory still exists" poll calls `pathExists` to detect externally deleted dirs (macOS FSEvents misses some deletes). Virtual `.git/<category>/...` paths don't exist on disk, so the poll evicted users back to `.git/` after 2 seconds. New `isVirtualGitPath` helper (in `file-explorer/git/path-detection.ts`) lets the poll early-return for virtual paths. Cache freshness for virtual listings already flows through `git-state-changed` and the backend's `invalidate_virtual_listings`. - Backend: `listing/streaming.rs` started a `notify` watcher on every listing's directory. Virtual paths don't exist on disk, so `notify` errored with "No path was found" on every nav into the portal. Skip `start_watching` when `git::is_virtual(path)` — invalidation already flows through the per-repo `.git/HEAD` / `refs/` / `packed-refs` watchers. - Verified end-to-end via the Cmdr MCP: navigated `right` pane to `.git/branches/main`, waited 8 s, pane stayed put. Backend log shows zero "Failed to watch path" warns since the rebuild. - Tests: `path-detection.test.ts` covers each of the seven virtual categories, raw passthrough, real `.git` internals (HEAD, refs), and lookalike paths. - Docs: updated `file-explorer/git/CLAUDE.md` and `file_system/git/CLAUDE.md` with gotchas covering both fixes.
1 parent af64689 commit bfcbfa4

6 files changed

Lines changed: 131 additions & 10 deletions

File tree

apps/desktop/src-tauri/src/file_system/git/CLAUDE.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,3 +307,13 @@ runs of the test bodies themselves are fine because they only read.
307307
expensive call in the chip pipeline
308308
**Why**: On 50k files it dominates the ~60 ms total. Don't add more work
309309
on the chip refresh path without re-benchmarking.
310+
311+
**Gotcha**: Listings on virtual portal paths must skip `start_watching`
312+
**Why**: `listing/streaming.rs` starts a `notify` watcher on the listing's
313+
directory. For virtual paths (`.git/branches/...` etc.) the on-disk path
314+
doesn't exist, so `notify` errors with "No path was found" and the warn
315+
log spams every navigation. The fix: skip the watcher start when
316+
`git::is_virtual(path)`. Cache invalidation for virtual listings flows
317+
through `git::watcher::invalidate_virtual_listings` (via the per-repo
318+
`.git/HEAD`, `refs/`, `packed-refs` watchers), so no notify watch is
319+
needed on the virtual side.

apps/desktop/src-tauri/src/file_system/listing/streaming.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,8 +504,13 @@ pub(crate) async fn read_directory_with_progress(
504504
);
505505
}
506506

507-
// Get the volume from VolumeManager to check if it supports watching
508-
if let Some(volume) = crate::file_system::get_volume_manager().get(volume_id)
507+
// Get the volume from VolumeManager to check if it supports watching.
508+
// Virtual git portal paths (`.git/branches/...` and friends) don't
509+
// exist on disk, so `notify` would error with "No path was found".
510+
// Cache invalidation for those listings flows through
511+
// `git::watcher::invalidate_virtual_listings` instead.
512+
if !crate::file_system::git::is_virtual(path)
513+
&& let Some(volume) = crate::file_system::get_volume_manager().get(volume_id)
509514
&& volume.supports_watching()
510515
&& let Err(e) = start_watching(listing_id, path)
511516
{

apps/desktop/src/lib/file-explorer/git/CLAUDE.md

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,16 @@ copy via the FriendlyError pipeline.
99

1010
## File map
1111

12-
| File | Role |
13-
| ----------------------- | --------------------------------------------------------------------------------------------------------------------- |
14-
| `RepoChip.svelte` | Single-line pill rendering branch + ahead/behind/dirty in the header |
15-
| `RepoChip.test.ts` | Snapshot-style tests for the six visual states |
16-
| `git-store.svelte.ts` | Per-repo reactive `RepoInfo` map. `subscribeToRepo(repoRoot)` is the live channel; `lookupRepoInfo(path)` is one-shot |
17-
| `status-column.ts` | Pure helpers: `glyphFor`, `labelFor`, `fetchStatusMap`. No reactivity |
18-
| `status-column.test.ts` | Tests for `glyphFor`, `labelFor`, and `fetchStatusMap` (mocks the IPC envelope) |
19-
| `git-store.test.ts` | Tests for refcounted subscribe/unsubscribe and `lookupRepoInfo` envelope unwrapping |
12+
| File | Role |
13+
| ------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
14+
| `RepoChip.svelte` | Single-line pill rendering branch + ahead/behind/dirty in the header |
15+
| `RepoChip.test.ts` | Snapshot-style tests for the six visual states |
16+
| `git-store.svelte.ts` | Per-repo reactive `RepoInfo` map. `subscribeToRepo(repoRoot)` is the live channel; `lookupRepoInfo(path)` is one-shot |
17+
| `status-column.ts` | Pure helpers: `glyphFor`, `labelFor`, `fetchStatusMap`. No reactivity |
18+
| `status-column.test.ts` | Tests for `glyphFor`, `labelFor`, and `fetchStatusMap` (mocks the IPC envelope) |
19+
| `git-store.test.ts` | Tests for refcounted subscribe/unsubscribe and `lookupRepoInfo` envelope unwrapping |
20+
| `path-detection.ts` | `isVirtualGitPath(path)` — shared regex matching the seven backend `Cat` segments. Used to skip filesystem-bound polls (and future similar checks) on virtual portal paths |
21+
| `path-detection.test.ts` | Tests for `isVirtualGitPath` (each category, raw passthrough, real `.git` internals, lookalikes) |
2022

2123
## Lifecycle
2224

@@ -76,6 +78,12 @@ information gain. We treat the user setting as "show when meaningful" rather tha
7678

7779
## Gotchas
7880

81+
**Gotcha**: `FilePane`'s "directory still exists" poll evicts users back to `.git/` on virtual paths if not skipped.
82+
**Why**: `pathExists(currentPath)` returns false for `.git/branches/main/...` because those paths only exist in the
83+
portal, not on disk. After two consecutive false readings the poll calls `navigateToFallback`. The poll body now early-
84+
returns via `isVirtualGitPath(currentPath)` so virtual paths stay put. Cache freshness for virtual listings already
85+
flows through `git-state-changed` and the backend's `invalidate_virtual_listings`.
86+
7987
## Redirect navigation (M3)
8088

8189
`FileEntry.redirectToPath` is honoured in `FilePane.svelte::handleNavigate`. When set, opening the entry navigates to
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { describe, it, expect } from 'vitest'
2+
import { isVirtualGitPath } from './path-detection'
3+
4+
describe('isVirtualGitPath', () => {
5+
it('treats the `.git` directory itself as real', () => {
6+
expect(isVirtualGitPath('/Users/me/repo/.git')).toBe(false)
7+
})
8+
9+
it('treats raw `.git` internals as real', () => {
10+
// `.git/HEAD` and `.git/refs/...` are real on-disk files. Only the
11+
// seven category names route to the virtual portal.
12+
expect(isVirtualGitPath('/Users/me/repo/.git/HEAD')).toBe(false)
13+
expect(isVirtualGitPath('/Users/me/repo/.git/refs/heads/main')).toBe(false)
14+
expect(isVirtualGitPath('/Users/me/repo/.git/objects/pack')).toBe(false)
15+
expect(isVirtualGitPath('/Users/me/repo/.git/config')).toBe(false)
16+
})
17+
18+
it('detects each of the seven virtual categories', () => {
19+
const root = '/Users/me/repo/.git'
20+
expect(isVirtualGitPath(`${root}/branches`)).toBe(true)
21+
expect(isVirtualGitPath(`${root}/tags`)).toBe(true)
22+
expect(isVirtualGitPath(`${root}/commits`)).toBe(true)
23+
expect(isVirtualGitPath(`${root}/stash`)).toBe(true)
24+
expect(isVirtualGitPath(`${root}/worktrees`)).toBe(true)
25+
expect(isVirtualGitPath(`${root}/submodules`)).toBe(true)
26+
expect(isVirtualGitPath(`${root}/raw`)).toBe(true)
27+
})
28+
29+
it('detects subpaths inside a virtual category', () => {
30+
expect(isVirtualGitPath('/Users/me/repo/.git/branches/main')).toBe(true)
31+
expect(isVirtualGitPath('/Users/me/repo/.git/branches/main/src/foo.rs')).toBe(true)
32+
expect(isVirtualGitPath('/Users/me/repo/.git/branches/feature/foo')).toBe(true)
33+
expect(isVirtualGitPath('/Users/me/repo/.git/tags/v1.0.0')).toBe(true)
34+
expect(isVirtualGitPath('/Users/me/repo/.git/commits/abc123/Cargo.toml')).toBe(true)
35+
})
36+
37+
it('treats the `raw` passthrough as virtual even though contents are real-FS', () => {
38+
// `raw/` routes through `list_raw` in the backend; conceptually the
39+
// path is still part of the portal, so frontend FS-bound polls
40+
// should skip it.
41+
expect(isVirtualGitPath('/Users/me/repo/.git/raw/HEAD')).toBe(true)
42+
expect(isVirtualGitPath('/Users/me/repo/.git/raw/refs/heads/main')).toBe(true)
43+
})
44+
45+
it('ignores paths that look similar but are not under `.git`', () => {
46+
expect(isVirtualGitPath('/Users/me/projects/branches/main')).toBe(false)
47+
expect(isVirtualGitPath('/Users/me/repo/git/branches/main')).toBe(false)
48+
expect(isVirtualGitPath('/Users/me/repo/.gitignore')).toBe(false)
49+
})
50+
51+
it('returns false for normal paths', () => {
52+
expect(isVirtualGitPath('/Users/me/foo/bar')).toBe(false)
53+
expect(isVirtualGitPath('/')).toBe(false)
54+
expect(isVirtualGitPath('')).toBe(false)
55+
})
56+
57+
it('does not match when the segment after `.git/` only starts with a category name', () => {
58+
// Exact category match required: `.git/branches-of-foo` is not virtual.
59+
expect(isVirtualGitPath('/Users/me/repo/.git/branches-of-foo')).toBe(false)
60+
expect(isVirtualGitPath('/Users/me/repo/.git/raw_logs')).toBe(false)
61+
})
62+
})
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Shared helper for spotting paths inside the virtual `.git` portal.
3+
*
4+
* The backend exposes seven categories under `.git/`: `branches`, `tags`,
5+
* `commits`, `stash`, `worktrees`, `submodules`, and `raw`. Subpaths that
6+
* route through any of these are virtual — they don't exist on disk in the
7+
* shape Cmdr presents them. The `.git` directory itself (and other real
8+
* `.git/` internals like `HEAD` or `refs/heads/main`) stay real.
9+
*
10+
* The frontend uses this to skip filesystem-bound bookkeeping (the
11+
* "directory still exists" poll, future similar checks) on virtual paths.
12+
*
13+
* The seven category names mirror `Cat` in
14+
* `apps/desktop/src-tauri/src/file_system/git/path.rs`. Keep them in sync.
15+
*/
16+
17+
const VIRTUAL_GIT_CATEGORIES = ['branches', 'tags', 'commits', 'stash', 'worktrees', 'submodules', 'raw'] as const
18+
19+
const VIRTUAL_GIT_PATH_REGEX = new RegExp(`/\\.git/(?:${VIRTUAL_GIT_CATEGORIES.join('|')})(?:/|$)`)
20+
21+
/**
22+
* Returns `true` when `path` lives under one of the seven virtual git
23+
* categories. Returns `false` for the `.git` root itself, for raw git
24+
* internals like `.git/HEAD` or `.git/refs/heads/main`, and for normal
25+
* non-git paths.
26+
*/
27+
export function isVirtualGitPath(path: string): boolean {
28+
return VIRTUAL_GIT_PATH_REGEX.test(path)
29+
}

apps/desktop/src/lib/file-explorer/pane/FilePane.svelte

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import { splitPathSegments } from '../navigation/path-segments'
6666
import RepoChip from '../git/RepoChip.svelte'
6767
import { lookupRepoInfo, subscribeToRepo, unsubscribeFromRepo, type RepoInfo } from '../git/git-store.svelte'
68+
import { isVirtualGitPath } from '../git/path-detection'
6869
import { getSetting, onSpecificSettingChange } from '$lib/settings'
6970
import ErrorPane from './ErrorPane.svelte'
7071
import VolumeUnreachableBanner from './VolumeUnreachableBanner.svelte'
@@ -1845,6 +1846,12 @@
18451846
// Poll to detect externally deleted directories (macOS FSEvents doesn't notify)
18461847
dirExistsPollInterval = setInterval(() => {
18471848
if (!listingId || loading || isNetworkView || isMtpView) return
1849+
// Virtual `.git/<category>/...` paths don't exist on disk, so
1850+
// `pathExists` always returns false and the poll would evict
1851+
// the user back to `.git/`. The git watcher keeps these
1852+
// listings fresh via `git-state-changed` and the
1853+
// `directory-diff` events from `invalidate_virtual_listings`.
1854+
if (isVirtualGitPath(currentPath)) return
18481855
void pathExists(currentPath).then((exists) => {
18491856
if (exists) {
18501857
dirNotExistsCount = 0

0 commit comments

Comments
 (0)