From c98d61d9a33eb3abf17e83c39410dfc3b56a8cef Mon Sep 17 00:00:00 2001 From: Jesse Herrick Date: Sat, 3 Oct 2026 19:25:48 -0400 Subject: [PATCH 1/3] Do not index a directory the watcher could not read or classify walkDirectories returned silently when it could not read a directory, for example when the process had no file descriptor left. The subtree below it was then not watched, not marked failed and never retried, and the Create handler reported every file in it as if it were a plain directory, even when it was a worktree that had been moved into place. Such a directory is now marked failed: the coverage report says that it is not covered, the retry reads it again, and a retry clears the failure only when it could read it. Its files are not reported from the Create event; the reconcile that follows restored coverage indexes them. Both index walks (WalkElixirFiles and CollectElixirFilesParallel) now skip a directory whose .git file names no git directory yet, as the watchers do, so a reconcile does not index a worktree that git is still moving. This is a likely cause of a rare failure of TestFSNotifyWatcherSkipsWorktreeAddedWhileRunning under load. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 + internal/parser/parser.go | 6 ++- internal/parser/parser_test.go | 24 ++++++++++ internal/workspace/watch_fsnotify.go | 50 ++++++++++++++++---- internal/workspace/watch_test.go | 71 ++++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b69b5a6..d8d3df9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ ### Fixed +- **A worktree moved into the project, or a directory that cannot be read, is no longer indexed by mistake** — `git worktree move` renames the directory and then writes its `.git` file again in place, so a watcher or a walk could find the file empty and index the whole worktree. Such a directory is now treated as a worktree until git is done, by the watchers and by both index walks. A directory that the watcher cannot read (for example when the process has no file descriptor left) used to be skipped silently: it was not watched, not retried, and its files were reported as if it were a plain directory. It is now marked as not covered, so the coverage report says so and the retry reads it again, and its files are indexed only once Dexter knows what it is + - **A def inside a macro's `quote` is no longer indexed as a function of the macro's module** — `defmacro route(...) do quote do def handle(...) end end` made the index say that the DSL module defines `handle/2`, which it does not. A consumer that imports the DSL then resolved `Consumer.handle` into the macro's body. Such a def is now skipped, so the call goes to the line in the consumer that declared it, from the compiled BEAM. What `__using__` injects, and a quote in a helper function, are still indexed as before. The index is rebuilt once after the upgrade - **Compressed BEAM files are read** — a module compiled with the `compressed` option, as some Erlang dependencies are, is a gzip stream around the BEAM container. Dexter rejected it as an invalid BEAM, so its exports were missing from completion and generated-function navigation. It is now decompressed, with the same size limit as an uncompressed file diff --git a/internal/parser/parser.go b/internal/parser/parser.go index e356b5d..b1b15c1 100644 --- a/internal/parser/parser.go +++ b/internal/parser/parser.go @@ -192,7 +192,9 @@ func WalkElixirFiles(root string, fn func(path string, d fs.DirEntry) error) err if err != nil { return nil } - if !isRoot && HasLinkedWorktreeGitFile(dir, entries) { + // A worktree that git is still moving has an empty .git file; it is + // skipped like a settled one, as the watchers treat it. + if !isRoot && (HasLinkedWorktreeGitFile(dir, entries) || HasUnsettledGitFile(dir, entries)) { return nil } for _, e := range entries { @@ -477,7 +479,7 @@ func CollectElixirFilesParallel(root string) []string { if err != nil { return } - if dir != root && HasLinkedWorktreeGitFile(dir, entries) { + if dir != root && (HasLinkedWorktreeGitFile(dir, entries) || HasUnsettledGitFile(dir, entries)) { return } diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go index b09aa0f..6491473 100644 --- a/internal/parser/parser_test.go +++ b/internal/parser/parser_test.go @@ -3886,3 +3886,27 @@ func TestHeredocLineContinuationClosesHeredoc(t *testing.T) { }) } } + +// git worktree move renames a worktree and then writes its .git file again in +// place, so for a moment the file is empty. Both walkers skip such a +// directory, as the watchers do, rather than index a worktree on the move. +func TestWalkAndCollectSkipUnsettledGitFile(t *testing.T) { + app := t.TempDir() + for path, content := range map[string]string{ + filepath.Join(app, "lib", "app.ex"): "defmodule App do\nend\n", + filepath.Join(app, "moving", "lib", "moving.ex"): "defmodule Moving do\nend\n", + filepath.Join(app, "moving", ".git"): "", + } { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + want := []string{filepath.Join(app, "lib", "app.ex")} + walked, collected := walkedAndCollected(t, app) + if !reflect.DeepEqual(walked, want) || !reflect.DeepEqual(collected, want) { + t.Errorf("Walk = %v, Collect = %v; want %v", walked, collected, want) + } +} diff --git a/internal/workspace/watch_fsnotify.go b/internal/workspace/watch_fsnotify.go index f40f670..2bdcc45 100644 --- a/internal/workspace/watch_fsnotify.go +++ b/internal/workspace/watch_fsnotify.go @@ -25,7 +25,10 @@ type fsnotifyWatcher struct { add func(string) error remove func(string) error watchList func() []string - wg sync.WaitGroup + // readDir reads a directory for the walk; nil uses parser.ReadDirUnsorted. + // Tests replace it to make a read fail. + readDir func(string) ([]fs.DirEntry, error) + wg sync.WaitGroup // tops are the nested worktrees found so far. Only a top itself is watched, // for its .git file. pending holds tops whose .git file went away; the retry @@ -47,6 +50,13 @@ func (w *fsnotifyWatcher) Degraded() bool { return len(w.failed) > 0 } +func (w *fsnotifyWatcher) isFailed(path string) bool { + w.mu.Lock() + defer w.mu.Unlock() + _, ok := w.failed[path] + return ok +} + func (w *fsnotifyWatcher) failedDirectories() []string { w.mu.Lock() paths := make([]string, 0, len(w.failed)) @@ -104,13 +114,24 @@ func (w *fsnotifyWatcher) Close() error { // whole tree because one directory could not be watched would leave a large // repository with no native watching at all, which is far worse. func (w *fsnotifyWatcher) watchTree(root string) int { - return w.walkDirectories(root, true) + watched, _ := w.walkDirectories(root, true) + return watched } -func (w *fsnotifyWatcher) walkDirectories(root string, includeRoot bool) int { +// walkDirectories watches root's tree and reports whether root itself could be +// read. A directory that cannot be read, for example because the process has +// no file descriptor left, cannot be told apart from a nested worktree and its +// subdirectories cannot be watched, so it is marked failed: the retry timer +// walks it again, and the coverage report says that the tree is not covered. +func (w *fsnotifyWatcher) walkDirectories(root string, includeRoot bool) (int, bool) { if info, err := os.Lstat(root); err != nil || !info.IsDir() || skipWatchDir(info.Name()) { - return 0 + return 0, true + } + readDir := w.readDir + if readDir == nil { + readDir = parser.ReadDirUnsorted } + rootRead := true watched := 0 var walk func(dir string) walk = func(dir string) { @@ -127,8 +148,13 @@ func (w *fsnotifyWatcher) walkDirectories(root string, includeRoot bool) int { if dir != w.root && w.tops.has(dir) { return } - entries, err := parser.ReadDirUnsorted(dir) + entries, err := readDir(dir) if err != nil { + log.Printf("Warning: cannot read %s to watch it: %v", dir, err) + w.setFailed(dir, true) + if dir == root { + rootRead = false + } return } // The entries show a nested worktree without another syscall. @@ -151,7 +177,7 @@ func (w *fsnotifyWatcher) walkDirectories(root string, includeRoot bool) int { } } walk(root) - return watched + return watched, rootRead } // unwatchBelow drops the watches below dir, which turned out to be a nested @@ -263,8 +289,10 @@ func (w *fsnotifyWatcher) retryPaths(paths []string) { } // The recovered parent watch closes the race with this walk. Add any // descendants created while the parent had no coverage before restoring. - w.walkDirectories(path, false) - w.setFailed(path, false) + // A directory that still cannot be read stays failed. + if _, read := w.walkDirectories(path, false); read { + w.setFailed(path, false) + } } } @@ -336,6 +364,12 @@ func (w *fsnotifyWatcher) handle(ev fsnotify.Event) { if w.tops.has(path) { return } + // A directory that could not be read is not known to be plain: + // it can be a worktree moved into place. Its files are indexed by + // the reconcile that follows when coverage comes back. + if w.isFailed(path) { + return + } _ = parser.WalkElixirFiles(path, func(file string, _ fs.DirEntry) error { w.onChange(file) return nil diff --git a/internal/workspace/watch_test.go b/internal/workspace/watch_test.go index 99776c9..60d46e8 100644 --- a/internal/workspace/watch_test.go +++ b/internal/workspace/watch_test.go @@ -3,12 +3,14 @@ package workspace import ( "errors" "fmt" + "io/fs" "os" "os/exec" "path/filepath" "slices" "strings" "sync" + "syscall" "testing" "time" @@ -734,3 +736,72 @@ func (env *watchedRepo) changedPaths() []string { } return paths } + +// A directory that cannot be read, for example when the process has no file +// descriptor left, cannot be classified: it may be a worktree that was moved +// into place. Its files are not reported from the Create event; the directory +// is marked failed, so the coverage report says so and the retry reads it +// again. A retry classifies it, and a plain directory reports its restored +// coverage, which makes the runtime index it. +func TestWatcherDoesNotReportDirectoryItCouldNotRead(t *testing.T) { + for _, tc := range []struct { + name string + worktree bool + }{ + {"worktree moved into place", true}, + {"plain directory", false}, + } { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "arrived") + if tc.worktree { + makeLinkedWorktree(t, root, dir) + } else { + if err := os.MkdirAll(filepath.Join(dir, "lib"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "lib", "plain.ex"), []byte("defmodule Plain do\nend\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + stub := &watchAddStub{failing: map[string]bool{}} + var coverage []bool + w, changed, _ := newRecordingWatcher(root, stub) + w.onCoverageChange = func(degraded bool) { coverage = append(coverage, degraded) } + unreadable := true + w.readDir = func(path string) ([]fs.DirEntry, error) { + if unreadable && path == dir { + return nil, syscall.EMFILE + } + return parser.ReadDirUnsorted(path) + } + + w.handle(fsnotify.Event{Name: dir, Op: fsnotify.Create}) + if len(*changed) != 0 { + t.Fatalf("reported %v from a directory that could not be read", *changed) + } + if !slices.Equal(w.failedDirectories(), []string{dir}) || !slices.Equal(coverage, []bool{true}) { + t.Fatalf("failed = %v, coverage = %v; want %s failed and one degraded report", w.failedDirectories(), coverage, dir) + } + + // A retry while the directory still cannot be read keeps it failed. + w.retryFailed() + if !slices.Equal(w.failedDirectories(), []string{dir}) { + t.Fatalf("failed = %v after a retry that could not read %s", w.failedDirectories(), dir) + } + + unreadable = false + w.retryFailed() + if len(w.failedDirectories()) != 0 || !slices.Equal(coverage, []bool{true, false}) { + t.Fatalf("failed = %v, coverage = %v after the directory became readable", w.failedDirectories(), coverage) + } + if w.tops.has(dir) != tc.worktree { + t.Errorf("tops.has(%s) = %v, want %v", dir, w.tops.has(dir), tc.worktree) + } + if len(*changed) != 0 { + t.Errorf("reported %v; the coverage reconcile indexes the directory", *changed) + } + }) + } +} From 1181bba387bb06c2b81b7b9a6e73b3c510f4945f Mon Sep 17 00:00:00 2001 From: Jesse Herrick Date: Sat, 3 Oct 2026 19:48:24 -0400 Subject: [PATCH 2/3] Classify a directory from one read of its .git file git 2.48 and later rename a worktree on `git worktree move` and then write its .git file again in place (truncate, then write). The watcher's walk asked two questions with two reads: "is this a worktree?" and "is its .git file still being written?". The first read could see the truncated file and the second the complete one, so a worktree was neither, and the walk reported every file in it as a plain directory. With git 2.55 on Linux this made TestFSNotifyWatcherSkipsWorktreeAddedWhileRunning fail in 2 of 20 runs; git 2.47, which does not write the file again, never failed. parser.GitFile and parser.GitFileFromEntries now return one state (none, plain, worktree, unsettled) from one read of the file. The fsnotify walk and pending check, the FSEvents handler and its later check, both index walks, and the runtime's removal of a reported worktree use it. The cost is the same: no syscall for a directory without a .git entry, and one read for a directory with one. Tests: TestGitFileStates (each state) and TestGitFileReadsTheFileOnce (serves an empty file on the first read and a complete one on the second, as git does; one call must read once and answer "unsettled"). On Linux, the watcher test now passes 100 of 100 runs with git 2.55 and 50 of 50 with git 2.47. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- internal/parser/parser.go | 110 ++++++++++++++------ internal/parser/parser_test.go | 76 ++++++++++++++ internal/workspace/runtime.go | 4 +- internal/workspace/watch_fsnotify.go | 31 +++--- internal/workspace/watch_platform_darwin.go | 37 ++++--- 6 files changed, 196 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8d3df9..6876ab5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ ### Fixed -- **A worktree moved into the project, or a directory that cannot be read, is no longer indexed by mistake** — `git worktree move` renames the directory and then writes its `.git` file again in place, so a watcher or a walk could find the file empty and index the whole worktree. Such a directory is now treated as a worktree until git is done, by the watchers and by both index walks. A directory that the watcher cannot read (for example when the process has no file descriptor left) used to be skipped silently: it was not watched, not retried, and its files were reported as if it were a plain directory. It is now marked as not covered, so the coverage report says so and the retry reads it again, and its files are indexed only once Dexter knows what it is +- **A worktree moved into the project, or a directory that cannot be read, is no longer indexed by mistake** — `git worktree move` renames the directory and then writes its `.git` file again in place, so a watcher or a walk could find the file empty and index the whole worktree. Such a directory is now treated as a worktree until git is done, by the watchers and by both index walks, and each check reads the `.git` file once: two reads could see the empty file and then the complete one, and answer that a worktree was neither a worktree nor still being written. This happened with git 2.48 and later, which write the file again after the move; older git does not. A directory that the watcher cannot read (for example when the process has no file descriptor left) used to be skipped silently: it was not watched, not retried, and its files were reported as if it were a plain directory. It is now marked as not covered, so the coverage report says so and the retry reads it again, and its files are indexed only once Dexter knows what it is - **A def inside a macro's `quote` is no longer indexed as a function of the macro's module** — `defmacro route(...) do quote do def handle(...) end end` made the index say that the DSL module defines `handle/2`, which it does not. A consumer that imports the DSL then resolved `Consumer.handle` into the macro's body. Such a def is now skipped, so the call goes to the line in the consumer that declared it, from the compiled BEAM. What `__using__` injects, and a quote in a helper function, are still indexed as before. The index is rebuilt once after the upgrade diff --git a/internal/parser/parser.go b/internal/parser/parser.go index b1b15c1..bd62f5a 100644 --- a/internal/parser/parser.go +++ b/internal/parser/parser.go @@ -1,6 +1,7 @@ package parser import ( + "io" "io/fs" "os" "path/filepath" @@ -194,7 +195,7 @@ func WalkElixirFiles(root string, fn func(path string, d fs.DirEntry) error) err } // A worktree that git is still moving has an empty .git file; it is // skipped like a settled one, as the watchers treat it. - if !isRoot && (HasLinkedWorktreeGitFile(dir, entries) || HasUnsettledGitFile(dir, entries)) { + if !isRoot && GitFileFromEntries(dir, entries).Nested() { return nil } for _, e := range entries { @@ -225,43 +226,83 @@ func skipDir(name string) bool { return name == "_build" || name == ".git" || name == "node_modules" } -// HasLinkedWorktreeGitFile reports whether dir, whose entries are given, is the -// top of a linked git worktree. Such a checkout nested inside the project (e.g. -// Claude Code's .claude/worktrees/) is a full copy of the repository, and -// indexing it would duplicate every definition. Scanning the entries already -// read costs no syscall; only a directory that has a .git file pays one read. -func HasLinkedWorktreeGitFile(dir string, entries []fs.DirEntry) bool { - for _, e := range entries { - if e.Name() == ".git" { - return !e.IsDir() && isLinkedWorktreeGitFile(filepath.Join(dir, ".git")) - } - } - return false +// openGitFile opens a .git file for reading. Tests replace it to count the +// reads and to serve the states git leaves while it writes the file. +var openGitFile = func(path string) (io.ReadCloser, error) { return os.Open(path) } + +// GitFileState is what a directory's .git entry says about the directory. +type GitFileState int + +const ( + // NoGitFile: the directory has no .git file. A .git directory, as in a + // repository's own root or an old-style submodule, also counts as none. + NoGitFile GitFileState = iota + // PlainGitFile: a .git file that names a git directory that is not a + // linked worktree's, such as a submodule's. The directory is indexed. + PlainGitFile + // WorktreeGitFile: the top of a linked git worktree. Such a checkout nested + // inside the project (e.g. Claude Code's .claude/worktrees/) is a full copy + // of the repository, and indexing it would duplicate every definition. + WorktreeGitFile + // UnsettledGitFile: a .git file that names no git directory yet. Newer + // git (2.48 and later) renames a worktree on `git worktree move` and then + // writes its .git file again in place, so for a moment the file is empty. + // Such a directory is treated as a worktree and checked again later. + UnsettledGitFile +) + +// Nested reports whether a walk must leave the directory out: a worktree, +// or what may be one that git has not finished writing. +func (s GitFileState) Nested() bool { + return s == WorktreeGitFile || s == UnsettledGitFile } -// UnsettledGitFile reports whether dir holds a .git file that names no git -// directory yet. git rewrites a worktree's .git file in place when it moves the -// worktree, so for a moment after the rename the file is empty, and a directory -// that is a worktree looks like a plain one. Such a directory is checked again -// once git is done, rather than indexed. -func UnsettledGitFile(dir string) bool { - info, err := os.Lstat(filepath.Join(dir, ".git")) +// GitFile classifies dir by its .git entry. The file is read once and every +// check uses that read: two reads can see two states while git writes the +// file (empty, then complete), and then answer neither "worktree" nor +// "unsettled" for a worktree. +func GitFile(dir string) GitFileState { + path := filepath.Join(dir, ".git") + info, err := os.Lstat(path) if err != nil || !info.Mode().IsRegular() { - return false + return NoGitFile } - _, ok := gitdirFromFile(filepath.Join(dir, ".git")) - return !ok + return gitFileState(path) } -// HasUnsettledGitFile is UnsettledGitFile for a directory whose entries are -// already read, so a directory without a .git file costs no syscall. -func HasUnsettledGitFile(dir string, entries []fs.DirEntry) bool { +// GitFileFromEntries is GitFile for a directory whose entries are already +// read: a directory without a .git file costs no syscall, and one with a .git +// file pays one read. +func GitFileFromEntries(dir string, entries []fs.DirEntry) GitFileState { for _, e := range entries { if e.Name() == ".git" { - return !e.IsDir() && UnsettledGitFile(dir) + if e.IsDir() { + return NoGitFile + } + return gitFileState(filepath.Join(dir, ".git")) } } - return false + return NoGitFile +} + +func gitFileState(path string) GitFileState { + gitdir, ok := gitdirFromFile(path) + if !ok { + // A file that cannot be read, or that names no git directory, is + // what git leaves while it writes the file. + return UnsettledGitFile + } + if linkedWorktreeGitdir(path, gitdir) { + return WorktreeGitFile + } + return PlainGitFile +} + +// HasLinkedWorktreeGitFile reports whether dir, whose entries are given, is the +// top of a linked git worktree. Scanning the entries already read costs no +// syscall; only a directory that has a .git file pays one read. +func HasLinkedWorktreeGitFile(dir string, entries []fs.DirEntry) bool { + return GitFileFromEntries(dir, entries) == WorktreeGitFile } // isLinkedWorktreeGitFile reports whether the .git file at path belongs to a @@ -269,9 +310,12 @@ func HasUnsettledGitFile(dir string, entries []fs.DirEntry) bool { // any other directory. Only a directory that has a .git file pays for this check. func isLinkedWorktreeGitFile(path string) bool { gitdir, ok := gitdirFromFile(path) - if !ok { - return false - } + return ok && linkedWorktreeGitdir(path, gitdir) +} + +// linkedWorktreeGitdir reports whether gitdir, read from the .git file at path, +// is a linked worktree's admin directory. +func linkedWorktreeGitdir(path, gitdir string) bool { // Git gives each linked worktree an admin directory with a commondir file, // and a submodule's has none. This also covers worktrees of bare // repositories. After git prunes the admin directory, only its place under @@ -303,7 +347,7 @@ func isLinkedWorktreeGitFile(path string) bool { // resolved against the file's directory. It reports false when path is not such // a file, which includes a .git directory. func gitdirFromFile(path string) (string, bool) { - f, err := os.Open(path) + f, err := openGitFile(path) if err != nil { return "", false } @@ -479,7 +523,7 @@ func CollectElixirFilesParallel(root string) []string { if err != nil { return } - if dir != root && (HasLinkedWorktreeGitFile(dir, entries) || HasUnsettledGitFile(dir, entries)) { + if dir != root && GitFileFromEntries(dir, entries).Nested() { return } diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go index 6491473..10b0b85 100644 --- a/internal/parser/parser_test.go +++ b/internal/parser/parser_test.go @@ -2,6 +2,7 @@ package parser import ( "fmt" + "io" "io/fs" "os" "os/exec" @@ -3910,3 +3911,78 @@ func TestWalkAndCollectSkipUnsettledGitFile(t *testing.T) { t.Errorf("Walk = %v, Collect = %v; want %v", walked, collected, want) } } + +// GitFile tells each kind of .git entry apart, from one read of the file. +func TestGitFileStates(t *testing.T) { + app, wt, _ := gitRepoWithNestedWorktree(t) + write := func(dir, content string) string { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ".git"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return dir + } + if err := os.MkdirAll(filepath.Join(app, ".git", "modules", "shared"), 0o755); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name string + dir string + want GitFileState + }{ + {"repository root (.git directory)", app, NoGitFile}, + {"no .git", filepath.Join(app, "lib"), NoGitFile}, + {"linked worktree", wt, WorktreeGitFile}, + {"submodule", write(filepath.Join(app, "deps", "shared"), "gitdir: ../../.git/modules/shared\n"), PlainGitFile}, + {"empty .git file", write(filepath.Join(app, "moving"), ""), UnsettledGitFile}, + {"not a gitdir line", write(filepath.Join(app, "odd"), "something else\n"), UnsettledGitFile}, + } { + if got := GitFile(tc.dir); got != tc.want { + t.Errorf("%s: GitFile = %v, want %v", tc.name, got, tc.want) + } + entries, err := os.ReadDir(tc.dir) + if err != nil { + t.Fatal(err) + } + if got := GitFileFromEntries(tc.dir, entries); got != tc.want { + t.Errorf("%s: GitFileFromEntries = %v, want %v", tc.name, got, tc.want) + } + } +} + +// git 2.48 and later rename a worktree on `git worktree move` and then write +// its .git file again in place: a reader can find it empty, and a moment later +// complete. One classification must use one read, so that it cannot see the +// empty file for one question and the complete file for the next, and then +// answer that a worktree is neither a worktree nor unsettled. +func TestGitFileReadsTheFileOnce(t *testing.T) { + _, wt, _ := gitRepoWithNestedWorktree(t) + content, err := os.ReadFile(filepath.Join(wt, ".git")) + if err != nil { + t.Fatal(err) + } + reads := 0 + previous := openGitFile + t.Cleanup(func() { openGitFile = previous }) + openGitFile = func(string) (io.ReadCloser, error) { + reads++ + if reads == 1 { + return io.NopCloser(strings.NewReader("")), nil + } + return io.NopCloser(strings.NewReader(string(content))), nil + } + if got := GitFile(wt); got != UnsettledGitFile || reads != 1 { + t.Errorf("GitFile = %v after %d reads, want %v after one read", got, reads, UnsettledGitFile) + } + reads = 0 + entries, err := os.ReadDir(wt) + if err != nil { + t.Fatal(err) + } + if got := GitFileFromEntries(wt, entries); got != UnsettledGitFile || reads != 1 { + t.Errorf("GitFileFromEntries = %v after %d reads, want %v after one read", got, reads, UnsettledGitFile) + } +} diff --git a/internal/workspace/runtime.go b/internal/workspace/runtime.go index f2c8eb3..50f464f 100644 --- a/internal/workspace/runtime.go +++ b/internal/workspace/runtime.go @@ -612,7 +612,9 @@ func (r *Runtime) reconcilePath(path string) error { // A watcher reports a directory when it turns out to be a nested // worktree. Files indexed from it before its .git file appeared, as cp -r // can do, are removed with one range read of the path index. - if path != r.root && parser.IsLinkedWorktree(path) { + // A worktree that git is still moving counts too: its .git file can + // be empty when the watcher reports it. + if path != r.root && parser.GitFile(path).Nested() { under, err := r.store.ListFilePathsUnder(path) if err != nil { return err diff --git a/internal/workspace/watch_fsnotify.go b/internal/workspace/watch_fsnotify.go index 2bdcc45..e4ea200 100644 --- a/internal/workspace/watch_fsnotify.go +++ b/internal/workspace/watch_fsnotify.go @@ -158,17 +158,21 @@ func (w *fsnotifyWatcher) walkDirectories(root string, includeRoot bool) (int, b return } // The entries show a nested worktree without another syscall. - if dir != w.root && parser.HasLinkedWorktreeGitFile(dir, entries) { - w.tops.add(dir) - return - } - // A worktree that git is still moving has an empty .git file. It is a - // top until the pending check says otherwise, so that its files are - // not reported in the meantime. - if dir != w.root && parser.HasUnsettledGitFile(dir, entries) { - w.tops.add(dir) - w.markPending(dir) - return + // One read of the .git file classifies the directory: two reads can + // see an empty file and then a complete one while git writes it. + if dir != w.root { + switch parser.GitFileFromEntries(dir, entries) { + case parser.WorktreeGitFile: + w.tops.add(dir) + return + case parser.UnsettledGitFile: + // A worktree that git is still moving has an empty .git file. It + // is a top until the pending check says otherwise, so that its + // files are not reported in the meantime. + w.tops.add(dir) + w.markPending(dir) + return + } } for _, e := range entries { if e.IsDir() && !skipWatchDir(e.Name()) { @@ -210,11 +214,12 @@ func (w *fsnotifyWatcher) checkPending() { delete(w.pending, dir) info, err := os.Stat(dir) if err == nil && info.IsDir() { - if parser.IsLinkedWorktree(dir) { + state := parser.GitFile(dir) + if state == parser.WorktreeGitFile { continue } // git may still be writing the .git file of a worktree it moved. - if parser.UnsettledGitFile(dir) || recordedWorktree(w.root, dir) { + if state == parser.UnsettledGitFile || recordedWorktree(w.root, dir) { recorded = append(recorded, dir) continue } diff --git a/internal/workspace/watch_platform_darwin.go b/internal/workspace/watch_platform_darwin.go index ad0565b..92d46ab 100644 --- a/internal/workspace/watch_platform_darwin.go +++ b/internal/workspace/watch_platform_darwin.go @@ -144,21 +144,25 @@ func (w *fseventsWatcher) handle(event fsevents.Event) { // A worktree moved or copied into place is a top from now on. The // runtime is told once, as for a new .git file, to drop anything // indexed from it; it needs no full reconcile. - if flags&(fsevents.ItemCreated|fsevents.ItemRenamed) != 0 && parser.IsLinkedWorktree(path) { - if w.tops.add(path) { - w.callbacks.PathChanged(path) - } - return - } - // A worktree that git is still moving has an empty .git file. It is a - // top until it is checked again, so that no full reconcile indexes it - // in the meantime. - if flags&(fsevents.ItemCreated|fsevents.ItemRenamed) != 0 && parser.UnsettledGitFile(path) { - if w.tops.add(path) { - w.callbacks.PathChanged(path) + if flags&(fsevents.ItemCreated|fsevents.ItemRenamed) != 0 { + // One read of the .git file classifies the directory: two reads can + // see an empty file and then a complete one while git writes it. + switch parser.GitFile(path) { + case parser.WorktreeGitFile: + if w.tops.add(path) { + w.callbacks.PathChanged(path) + } + return + case parser.UnsettledGitFile: + // A worktree that git is still moving has an empty .git file. + // It is a top until it is checked again, so that no full + // reconcile indexes it in the meantime. + if w.tops.add(path) { + w.callbacks.PathChanged(path) + } + w.checkTopLater(path) + return } - w.checkTopLater(path) - return } if flags&(fsevents.ItemCreated|fsevents.ItemRemoved|fsevents.ItemRenamed) != 0 { w.callbacks.FullReconcile() @@ -203,11 +207,12 @@ func (w *fseventsWatcher) checkTopLater(dir string) { } info, err := os.Stat(dir) if err == nil && info.IsDir() { - if parser.IsLinkedWorktree(dir) { + state := parser.GitFile(dir) + if state == parser.WorktreeGitFile { return } // git may still be writing the .git file of a worktree it moved. - if parser.UnsettledGitFile(dir) || recordedWorktree(w.root, dir) { + if state == parser.UnsettledGitFile || recordedWorktree(w.root, dir) { w.checkTopLater(dir) return } From 69703fa2119f44973cadbbe76f8b677e451fc758 Mon Sep 17 00:00:00 2001 From: Jesse Herrick Date: Sat, 3 Oct 2026 19:54:56 -0400 Subject: [PATCH 3/3] Index a readable directory that cannot be watched, and agree on .git The Create handler skipped indexing any directory marked failed, but a directory is also marked failed when its watch cannot be added (the inotify watch limit) although it was read and is known to be plain. When the watcher was already degraded, no coverage edge followed, so such a directory was never indexed. The handler now skips only a directory that could not be read. GitFileFromEntries read any .git entry that is not a directory, while GitFile reads only a regular file. A .git symlink could then be "unsettled" in a walk and "none" in the later check. Both now read only a regular file. Co-Authored-By: Claude Opus 5.5 --- internal/parser/parser.go | 4 +++- internal/parser/parser_test.go | 18 +++++++++++++++++ internal/workspace/watch_fsnotify.go | 16 ++++++--------- internal/workspace/watch_test.go | 30 ++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/internal/parser/parser.go b/internal/parser/parser.go index bd62f5a..de66534 100644 --- a/internal/parser/parser.go +++ b/internal/parser/parser.go @@ -276,7 +276,9 @@ func GitFile(dir string) GitFileState { func GitFileFromEntries(dir string, entries []fs.DirEntry) GitFileState { for _, e := range entries { if e.Name() == ".git" { - if e.IsDir() { + // The same rule as GitFile: only a regular file is read. A .git + // directory, a symlink or another kind of entry counts as none. + if !e.Type().IsRegular() { return NoGitFile } return gitFileState(filepath.Join(dir, ".git")) diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go index 10b0b85..9ca429c 100644 --- a/internal/parser/parser_test.go +++ b/internal/parser/parser_test.go @@ -3939,6 +3939,7 @@ func TestGitFileStates(t *testing.T) { {"submodule", write(filepath.Join(app, "deps", "shared"), "gitdir: ../../.git/modules/shared\n"), PlainGitFile}, {"empty .git file", write(filepath.Join(app, "moving"), ""), UnsettledGitFile}, {"not a gitdir line", write(filepath.Join(app, "odd"), "something else\n"), UnsettledGitFile}, + {"symlinked .git", symlinkedGit(t, app), NoGitFile}, } { if got := GitFile(tc.dir); got != tc.want { t.Errorf("%s: GitFile = %v, want %v", tc.name, got, tc.want) @@ -3986,3 +3987,20 @@ func TestGitFileReadsTheFileOnce(t *testing.T) { t.Errorf("GitFileFromEntries = %v after %d reads, want %v after one read", got, reads, UnsettledGitFile) } } + +// symlinkedGit makes a directory whose .git is a symlink to an empty file. +func symlinkedGit(t *testing.T, app string) string { + t.Helper() + dir := filepath.Join(app, "linked") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + target := filepath.Join(app, "empty-gitfile") + if err := os.WriteFile(target, nil, 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(dir, ".git")); err != nil { + t.Fatal(err) + } + return dir +} diff --git a/internal/workspace/watch_fsnotify.go b/internal/workspace/watch_fsnotify.go index e4ea200..a515147 100644 --- a/internal/workspace/watch_fsnotify.go +++ b/internal/workspace/watch_fsnotify.go @@ -50,13 +50,6 @@ func (w *fsnotifyWatcher) Degraded() bool { return len(w.failed) > 0 } -func (w *fsnotifyWatcher) isFailed(path string) bool { - w.mu.Lock() - defer w.mu.Unlock() - _, ok := w.failed[path] - return ok -} - func (w *fsnotifyWatcher) failedDirectories() []string { w.mu.Lock() paths := make([]string, 0, len(w.failed)) @@ -362,7 +355,8 @@ func (w *fsnotifyWatcher) handle(ev fsnotify.Event) { if skipWatchDir(base) { return } - if added := w.watchTree(path); added == 0 { + added, read := w.walkDirectories(path, true) + if added == 0 { log.Printf("Warning: no directory under %s could be watched", path) } w.retryFailedUnder(path) @@ -371,8 +365,10 @@ func (w *fsnotifyWatcher) handle(ev fsnotify.Event) { } // A directory that could not be read is not known to be plain: // it can be a worktree moved into place. Its files are indexed by - // the reconcile that follows when coverage comes back. - if w.isFailed(path) { + // the reconcile that follows when a retry reads it. A directory + // that was read but could not be watched is known to be plain, and + // is indexed now. + if !read { return } _ = parser.WalkElixirFiles(path, func(file string, _ fs.DirEntry) error { diff --git a/internal/workspace/watch_test.go b/internal/workspace/watch_test.go index 60d46e8..637db92 100644 --- a/internal/workspace/watch_test.go +++ b/internal/workspace/watch_test.go @@ -805,3 +805,33 @@ func TestWatcherDoesNotReportDirectoryItCouldNotRead(t *testing.T) { }) } } + +// A directory that could be read but not watched, as at the inotify watch +// limit, is known to be plain, so its files are indexed at once. This holds +// also when the watcher is already degraded and no coverage edge is reported. +func TestWatcherIndexesReadableDirectoryItCouldNotWatch(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "arrived") + if err := os.MkdirAll(filepath.Join(dir, "lib"), 0o755); err != nil { + t.Fatal(err) + } + file := filepath.Join(dir, "lib", "plain.ex") + if err := os.WriteFile(file, []byte("defmodule Plain do\nend\n"), 0o644); err != nil { + t.Fatal(err) + } + stub := &watchAddStub{failing: map[string]bool{dir: true, filepath.Join(dir, "lib"): true}} + w, changed, _ := newRecordingWatcher(root, stub) + var coverage []bool + w.onCoverageChange = func(degraded bool) { coverage = append(coverage, degraded) } + // Already degraded by another directory. + w.setFailed(filepath.Join(root, "elsewhere"), true) + coverage = nil + + w.handle(fsnotify.Event{Name: dir, Op: fsnotify.Create}) + if !slices.Contains(*changed, file) { + t.Errorf("reported %v, want %s from a readable directory that could not be watched", *changed, file) + } + if len(coverage) != 0 { + t.Errorf("coverage edges %v while already degraded, want none", coverage) + } +}