From e2561edab8fb6f3ce0bf73866e2ebac826305bb8 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 14:32:04 +1100 Subject: [PATCH 01/10] fix(watch): a directory's arrival re-decides the worktree verdicts beneath it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PLAN-558, the residual of #700's review: a worktree verdict cached at X/wt survived X being removed and an ordinary tree arriving whole at X (a mv), and so did a stale bookkeeping record for X/wt — the arrival's expansion then treated the ordinary wt as a worktree and its edits were silenced. A Create or Rename of a directory now drops the worktree verdicts beneath it and supersedes the records beneath it; a real worktree inside the arriving tree still verifies as one. Worktree and ordinary verdicts are kept in separate sets so that scan stays over the few worktrees, and a full worktree set drops only directories that no longer exist. Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/worktrees.go | 99 +++++++++++++++++++++++------- internal/fswatch/worktrees_test.go | 95 +++++++++++++++++++++++++++- 2 files changed, 169 insertions(+), 25 deletions(-) diff --git a/internal/fswatch/worktrees.go b/internal/fswatch/worktrees.go index dd5e8809..5c6bfd02 100644 --- a/internal/fswatch/worktrees.go +++ b/internal/fswatch/worktrees.go @@ -1,6 +1,7 @@ package fswatch import ( + "os" "path/filepath" "strings" "sync" @@ -35,9 +36,10 @@ import ( // Lost events re-decide every verdict against fresh bookkeeping, since what was // lost may have replaced a directory. -// worktreeCacheMax bounds the "not a worktree" verdicts; past it they are -// dropped wholesale and rebuilt lazily. "Worktree" verdicts are never evicted: -// there are few, and losing one mid-removal is the leak this file prevents. +// worktreeCacheMax bounds each verdict set. Past it the "not a worktree" set is +// dropped wholesale and rebuilt lazily; the "worktree" set — few, and losing +// one mid-removal is the leak this file prevents — drops only the directories +// that no longer exist. const worktreeCacheMax = 4096 // worktreeSkip answers "is this path inside a linked worktree of root". @@ -46,8 +48,12 @@ const worktreeCacheMax = 4096 type worktreeSkip struct { root string - mu sync.Mutex - known map[string]bool // directory below root -> it is a linked worktree of root + mu sync.Mutex + // worktree and ordinary are the cached verdicts for directories below root, + // kept apart so an arriving directory can drop the worktree verdicts beneath + // it without scanning every ordinary one. + worktree map[string]struct{} + ordinary map[string]struct{} recorded map[string]struct{} // worktrees git's bookkeeping records, in root's spelling // superseded are recorded worktrees whose path an event has since created or // renamed into: the record may be stale (an admin directory outlives an @@ -67,7 +73,8 @@ type worktreeSkip struct { } func newWorktreeSkip(root string) *worktreeSkip { - s := &worktreeSkip{root: root, known: map[string]bool{}, superseded: map[string]struct{}{}, isWorktree: paths.LinkedWorktreeOf, listWorktrees: paths.LinkedWorktreesOf} + s := &worktreeSkip{root: root, isWorktree: paths.LinkedWorktreeOf, listWorktrees: paths.LinkedWorktreesOf} + s.resetLocked() s.refreshLocked() return s } @@ -117,7 +124,7 @@ func (s *worktreeSkip) arrived(dir string) bool { // record of it can both be gone. v := s.decideLocked(dir) if v { - s.known[dir] = true + s.rememberLocked(dir, true) } return v } @@ -136,10 +143,16 @@ func (s *worktreeSkip) lost() { func (s *worktreeSkip) settleLocked() { if s.dirty.Swap(false) { s.refreshLocked() - clear(s.known) + clear(s.worktree) + clear(s.ordinary) } } +// resetLocked empties every verdict. s.mu must be held (or s unshared). +func (s *worktreeSkip) resetLocked() { + s.worktree, s.ordinary, s.superseded = map[string]struct{}{}, map[string]struct{}{}, map[string]struct{}{} +} + // below is path relative to root, when it lies strictly below it. func (s *worktreeSkip) below(path string) (string, bool) { rel, err := filepath.Rel(s.root, path) @@ -161,37 +174,77 @@ func (s *worktreeSkip) forgetLocked(path string, op Op) { // the admin directory too), so the directory's Remove can come before its // children's, and the rest of the removal must stay skipped. if op.Has(Create | Rename) { - delete(s.known, path) - if _, ok := s.recorded[path]; ok { - s.superseded[path] = struct{}{} - } + s.arrivalLocked(path) } if filepath.Base(path) == ".git" { dir := filepath.Dir(path) - if dir != s.root && !s.known[dir] { // the root's own .git changes with every git command - delete(s.known, dir) + _, isWorktree := s.worktree[dir] + if dir != s.root && !isWorktree { // the root's own .git changes with every git command + delete(s.ordinary, dir) s.refreshLocked() } } } +// arrivalLocked re-decides what a directory arriving at path replaces: path +// itself and every worktree verdict and record beneath it (PLAN-558). An +// ordinary tree moved in where a worktree's parent was holds no worktree, but +// the old verdict for / would otherwise silence it — and the +// expansion of the arrival judges its subdirectories with arrived(), which a +// superseded record no longer answers. A real worktree inside the arriving +// tree still verifies as one. s.mu must be held. +func (s *worktreeSkip) arrivalLocked(path string) { + delete(s.worktree, path) + delete(s.ordinary, path) + if _, ok := s.recorded[path]; ok { + s.superseded[path] = struct{}{} + } + prefix := path + string(filepath.Separator) + for d := range s.worktree { + if strings.HasPrefix(d, prefix) { + delete(s.worktree, d) + } + } + for d := range s.recorded { + if strings.HasPrefix(d, prefix) { + s.superseded[d] = struct{}{} + } + } +} + // verdictLocked is dir's cached verdict, decided on a miss. s.mu must be held. func (s *worktreeSkip) verdictLocked(dir string) bool { - if v, ok := s.known[dir]; ok { - return v + if _, ok := s.worktree[dir]; ok { + return true } - if len(s.known) >= worktreeCacheMax { - for d, v := range s.known { - if !v { - delete(s.known, d) - } - } + if _, ok := s.ordinary[dir]; ok { + return false } v := s.decideLocked(dir) - s.known[dir] = v + s.rememberLocked(dir, v) return v } +// rememberLocked caches dir's verdict, within worktreeCacheMax. s.mu must be held. +func (s *worktreeSkip) rememberLocked(dir string, isWorktree bool) { + if !isWorktree { + if len(s.ordinary) >= worktreeCacheMax { + clear(s.ordinary) + } + s.ordinary[dir] = struct{}{} + return + } + delete(s.ordinary, dir) + if len(s.worktree) >= worktreeCacheMax { + for d := range s.worktree { + if _, err := os.Lstat(d); err != nil { + delete(s.worktree, d) + } + } + } + s.worktree[dir] = struct{}{} +} + // decideLocked decides dir from the disk: a verified worktree, or one git's // bookkeeping records. s.mu must be held. func (s *worktreeSkip) decideLocked(dir string) bool { diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index 25686af7..3f5c69f2 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -156,7 +156,8 @@ func fakeSkip(t *testing.T) (s *worktreeSkip, verified, recorded map[string]bool t.Helper() root := filepath.Join(string(filepath.Separator), "ws") verified, recorded, asked = map[string]bool{}, map[string]bool{}, map[string]int{} - s = &worktreeSkip{root: root, known: map[string]bool{}, superseded: map[string]struct{}{}} + s = &worktreeSkip{root: root} + s.resetLocked() s.isWorktree = func(r, dir string) bool { if r != root { t.Fatalf("judged against %q, want the root", r) @@ -278,7 +279,7 @@ func TestWorktreeSkip_TheCapKeepsWorktrees(t *testing.T) { t.Errorf("the cap evicted a worktree verdict (judged %d times)", asked[wt]) } s.mu.Lock() - n := len(s.known) + n := len(s.ordinary) + len(s.worktree) s.mu.Unlock() if n > worktreeCacheMax+1 { t.Errorf("the cache holds %d verdicts, past its cap", n) @@ -522,3 +523,93 @@ func TestWorktreeSkip_AnArrivedWorktreeIsRemembered(t *testing.T) { t.Error("a worktree seen only arriving leaked its removal") } } + +// TestWorktreeSkip_AParentsArrivalRedecidesWorktreesBeneathIt (PLAN-558): a +// worktree verdict cached below a directory that is removed and then arrives +// whole again — an ordinary tree moved in where a worktree's parent was — does +// not silence the ordinary directory now at that path. A real worktree that +// arrives inside it is still skipped. +func TestWorktreeSkip_AParentsArrivalRedecidesWorktreesBeneathIt(t *testing.T) { + for _, stale := range []bool{false, true} { + t.Run(map[bool]string{false: "cached verdict", true: "stale record too"}[stale], func(t *testing.T) { + s, verified, recorded, _ := fakeSkip(t) + x := filepath.Join(s.root, "agents") + wt := filepath.Join(x, "wt") + verified[wt] = true + if stale { + recorded[wt] = true + s.lost() + } + if !s.inside(filepath.Join(wt, "a.go"), Write) { + t.Fatal("control: the worktree was not skipped") + } + s.inside(x, Remove) + verified[wt] = false // an ordinary tree arrives at x, with an ordinary wt + s.inside(x, Create) + if s.arrived(wt) { + t.Error("the expansion of the arrived parent treats its ordinary wt as a worktree") + } + if s.inside(filepath.Join(wt, "edit.go"), Write) { + t.Error("an edit inside the ordinary wt was silenced by the old verdict") + } + }) + } + // Control: a real worktree inside an arriving parent is still a worktree. + s, verified, _, _ := fakeSkip(t) + x := filepath.Join(s.root, "agents") + wt := filepath.Join(x, "wt") + verified[wt] = true + s.inside(x, Create) + if !s.arrived(wt) || !s.inside(filepath.Join(wt, "b.go"), Write) { + t.Error("a worktree inside an arriving directory was not skipped") + } +} + +// TestWatcher_AnOrdinaryTreeMovedOverAWorktreesParentIsWatched (PLAN-558): +// the real-backend shape — a worktree's parent removed, an ordinary tree with +// the same layout moved into its place. +func TestWatcher_AnOrdinaryTreeMovedOverAWorktreesParentIsWatched(t *testing.T) { + root := t.TempDir() + gitRepoAt(t, root) + x := filepath.Join(root, "agents") + wt := filepath.Join(x, "wt") + populate(t, wt, "a.go") + worktreeLink(t, wt, worktreeAdmin(t, root, "wt", wt)) + c := startWatcher(t, root, Options{SkipLinkedWorktrees: true}) + if err := os.WriteFile(filepath.Join(wt, "a.go"), []byte("w"), 0o644); err != nil { + t.Fatal(err) + } + rootStillReports(t, c, root) + if err := os.RemoveAll(x); err != nil { + t.Fatal(err) + } + rootStillReports(t, c, root) + staging := filepath.Join(t.TempDir(), "agents") + populate(t, staging, "wt/plain.go") + if err := os.Rename(staging, x); err != nil { + t.Fatal(err) + } + expectEventually(t, c, filepath.Join(wt, "plain.go"), "an ordinary tree moved over a worktree's parent") +} + +// TestWorktreeSkip_AFullWorktreeSetDropsOnlyVanishedDirectories: at the cap, +// worktree verdicts for directories that are gone are dropped; one that still +// exists is kept. +func TestWorktreeSkip_AFullWorktreeSetDropsOnlyVanishedDirectories(t *testing.T) { + s, _, _, _ := fakeSkip(t) + live := t.TempDir() + s.mu.Lock() + s.rememberLocked(live, true) + for i := range worktreeCacheMax { + s.rememberLocked(filepath.Join(s.root, "gone-"+strconv.Itoa(i)), true) + } + _, kept := s.worktree[live] + n := len(s.worktree) + s.mu.Unlock() + if !kept { + t.Error("an existing worktree's verdict was dropped at the cap") + } + if n > 2 { + t.Errorf("the worktree set holds %d verdicts after the cap, want the live one and the newest", n) + } +} From 8c8b66ac2af03d534cc234b394eca19a3f19e7ed Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 14:34:24 +1100 Subject: [PATCH 02/10] test(watch): an arrival re-decides only its own subtree, and not from a stale ordinary verdict Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/worktrees_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index 3f5c69f2..2fbe1344 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -613,3 +613,28 @@ func TestWorktreeSkip_AFullWorktreeSetDropsOnlyVanishedDirectories(t *testing.T) t.Errorf("the worktree set holds %d verdicts after the cap, want the live one and the newest", n) } } + +// TestWorktreeSkip_AnArrivalTouchesOnlyItsOwnSubtree: a directory arriving at +// agents leaves a worktree under its sibling agents-old alone, and a worktree +// under it that was once cached as ordinary is re-decided, not answered by +// that stale verdict. +func TestWorktreeSkip_AnArrivalTouchesOnlyItsOwnSubtree(t *testing.T) { + s, verified, _, asked := fakeSkip(t) + sibling := filepath.Join(s.root, "agents-old", "wt") + verified[sibling] = true + s.inside(filepath.Join(sibling, "a.go"), Write) + inner := filepath.Join(s.root, "agents", "wt") + s.inside(filepath.Join(inner, "a.go"), Write) // cached ordinary + verified[inner] = true + if !s.arrived(inner) { + t.Fatal("control: the directory that became a worktree was not recognised") + } + s.inside(filepath.Join(s.root, "agents"), Create) + before := asked[sibling] + if !s.inside(filepath.Join(sibling, "b.go"), Write) || asked[sibling] != before { + t.Error("an arrival at agents re-decided a worktree under agents-old") + } + if !s.inside(filepath.Join(inner, "b.go"), Write) { + t.Error("a worktree beneath the arrival was answered by its stale ordinary verdict") + } +} From f32dbf4b3f84796475ed71f80ce43d8b378fcddb Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 14:49:37 +1100 Subject: [PATCH 03/10] =?UTF-8?q?docs(changelog):=200.26.2=20(unreleased)?= =?UTF-8?q?=20=E2=80=94=20an=20arriving=20directory=20re-decides=20the=20w?= =?UTF-8?q?orktrees=20beneath=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f41fd412..8239f9bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 0.26.2 (unreleased) + +### Fixed + +- **An ordinary directory moved in where a worktree's parent was is watched.** + The watcher remembered a linked worktree below a directory after that + directory was removed, so an ordinary tree later moved into its place (say, + `agents/` holding a plain `wt/`) had the edits inside `wt/` silenced. A + directory that arrives now re-decides every worktree beneath it; a real + worktree inside the arriving tree is still left out. + ## 0.26.1 (2026-10-16) ### Fixed From 66cb1f29789bf1d9448d5c0bb7b0eb6702fa794d Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 15:17:31 +1100 Subject: [PATCH 04/10] fix(watch): an arrival replaces worktree beliefs only where a different directory stands Review of #703 (BLOCKING): inotify reports a directory moving AWAY as Remove|Rename, and one moved away and back is the same directory; treating either as an arrival dropped the worktree verdicts beneath it and superseded their records, so the rest of a removal leaked (thousands of events measured on Linux; main leaked none). Now nothing happens unless a directory stands at the path, and a belief at or beneath it is dropped only where its directory still exists and is not the one the belief was formed for (dev+inode, taken when the verdict or record was formed). A directory being removed keeps its belief. Known residual: a directory recreated with a reused inode looks unchanged. Also: arrived() no longer re-remembers a cached worktree. The Linux-image control: the new real-backend rename-away-and-back test fails 7/10 on the previous head and passes 10/10 here. Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/worktrees.go | 88 ++++++++++++++---- internal/fswatch/worktrees_test.go | 139 ++++++++++++++++++++++++++++- 2 files changed, 206 insertions(+), 21 deletions(-) diff --git a/internal/fswatch/worktrees.go b/internal/fswatch/worktrees.go index 5c6bfd02..95fb9f86 100644 --- a/internal/fswatch/worktrees.go +++ b/internal/fswatch/worktrees.go @@ -60,20 +60,27 @@ type worktreeSkip struct { // `rm -rf` until `git worktree prune`), so only a live verification counts // there, until a .git that verifies appears. superseded map[string]struct{} + // ident is the identity of each directory a worktree verdict or record was + // formed for, taken when it was formed: an arrival replaces a belief only + // where a DIFFERENT directory now stands (arrivalLocked). + ident map[string]os.FileInfo // dirty is set by lost and consumed by the next lookup, which re-reads the // bookkeeping and re-decides every verdict once: a burst of loss signals // costs one refresh, not one per signal. dirty atomic.Bool - // isWorktree and listWorktrees are paths.LinkedWorktreeOf and - // paths.LinkedWorktreesOf, seams for tests that count or fake lookups. + // isWorktree, listWorktrees, stat and sameFile are paths.LinkedWorktreeOf, + // paths.LinkedWorktreesOf, os.Lstat and os.SameFile: seams for tests that + // count or fake lookups and directories. isWorktree func(root, dir string) bool listWorktrees func(root string) []string + stat func(string) (os.FileInfo, error) + sameFile func(a, b os.FileInfo) bool } func newWorktreeSkip(root string) *worktreeSkip { - s := &worktreeSkip{root: root, isWorktree: paths.LinkedWorktreeOf, listWorktrees: paths.LinkedWorktreesOf} + s := &worktreeSkip{root: root, isWorktree: paths.LinkedWorktreeOf, listWorktrees: paths.LinkedWorktreesOf, stat: os.Lstat, sameFile: os.SameFile} s.resetLocked() s.refreshLocked() return s @@ -142,15 +149,16 @@ func (s *worktreeSkip) lost() { // verdict. s.mu must be held. func (s *worktreeSkip) settleLocked() { if s.dirty.Swap(false) { - s.refreshLocked() clear(s.worktree) clear(s.ordinary) + s.refreshLocked() } } // resetLocked empties every verdict. s.mu must be held (or s unshared). func (s *worktreeSkip) resetLocked() { s.worktree, s.ordinary, s.superseded = map[string]struct{}{}, map[string]struct{}{}, map[string]struct{}{} + s.ident = map[string]os.FileInfo{} } // below is path relative to root, when it lies strictly below it. @@ -186,32 +194,60 @@ func (s *worktreeSkip) forgetLocked(path string, op Op) { } } -// arrivalLocked re-decides what a directory arriving at path replaces: path -// itself and every worktree verdict and record beneath it (PLAN-558). An -// ordinary tree moved in where a worktree's parent was holds no worktree, but -// the old verdict for / would otherwise silence it — and the -// expansion of the arrival judges its subdirectories with arrived(), which a -// superseded record no longer answers. A real worktree inside the arriving -// tree still verifies as one. s.mu must be held. +// arrivalLocked re-decides what a directory arriving at path replaces: the +// worktree verdicts and records at and beneath path whose directory is now a +// DIFFERENT one (PLAN-558). An ordinary tree moved in where a worktree's parent +// was holds no worktree, but the old verdict for / would otherwise +// silence it, and the arrival's expansion judges its subdirectories with +// arrived(), which a superseded record no longer answers. +// +// Identity, not the event, decides. inotify reports a directory moving AWAY as +// Remove|Rename, and one moved away and back is the same directory: dropping +// its beliefs then would leak the rest of a removal (#703's review measured +// thousands of events). So nothing happens unless a directory now stands at +// path, and a belief is dropped only where its directory still exists and is +// not the one it was formed for. A real worktree in the arriving tree verifies +// again. s.mu must be held. func (s *worktreeSkip) arrivalLocked(path string) { - delete(s.worktree, path) - delete(s.ordinary, path) - if _, ok := s.recorded[path]; ok { - s.superseded[path] = struct{}{} + fi, err := s.stat(path) + if err != nil || !fi.IsDir() { + return // nothing arrived: a move away, or a file } + delete(s.ordinary, path) prefix := path + string(filepath.Separator) + under := func(d string) bool { return d == path || strings.HasPrefix(d, prefix) } for d := range s.worktree { - if strings.HasPrefix(d, prefix) { + if under(d) && s.replacedLocked(d) { delete(s.worktree, d) } } for d := range s.recorded { - if strings.HasPrefix(d, prefix) { + if under(d) && s.replacedLocked(d) { s.superseded[d] = struct{}{} } } } +// replacedLocked reports whether the directory at d is not the one a belief +// about d was formed for. A d that is gone is not replaced (it is being +// removed, and its belief must cover the rest of the removal). s.mu must be held. +func (s *worktreeSkip) replacedLocked(d string) bool { + cur, err := s.stat(d) + if err != nil { + return false + } + old, ok := s.ident[d] + return !ok || !s.sameFile(old, cur) +} + +// identifyLocked records the identity of the directory a belief about d is +// formed for, when it exists. s.mu must be held. +func (s *worktreeSkip) identifyLocked(d string) { + if fi, err := s.stat(d); err == nil { + s.ident[d] = fi + } +} + // verdictLocked is dir's cached verdict, decided on a miss. s.mu must be held. func (s *worktreeSkip) verdictLocked(dir string) bool { if _, ok := s.worktree[dir]; ok { @@ -235,14 +271,18 @@ func (s *worktreeSkip) rememberLocked(dir string, isWorktree bool) { return } delete(s.ordinary, dir) + if _, ok := s.worktree[dir]; ok { + return + } if len(s.worktree) >= worktreeCacheMax { for d := range s.worktree { - if _, err := os.Lstat(d); err != nil { + if _, err := s.stat(d); err != nil { delete(s.worktree, d) } } } s.worktree[dir] = struct{}{} + s.identifyLocked(dir) } // decideLocked decides dir from the disk: a verified worktree, or one git's @@ -272,4 +312,16 @@ func (s *worktreeSkip) refreshLocked() { delete(s.superseded, d) // no longer recorded: nothing to supersede } } + for d := range s.recorded { + if _, ok := s.ident[d]; !ok { + s.identifyLocked(d) + } + } + for d := range s.ident { + _, rec := s.recorded[d] + _, wt := s.worktree[d] + if !rec && !wt { + delete(s.ident, d) + } + } } diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index 2fbe1344..14b3a4a6 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -148,16 +148,59 @@ func TestWatcher_DoesNotExpandAWorktreeMovedInWhole(t *testing.T) { } } -// fakeSkip is a worktreeSkip over /ws whose verified worktrees and recorded -// bookkeeping the test sets, counting the verifications it makes. +// fakeSkip is a worktreeSkip over /ws whose verified worktrees, recorded +// bookkeeping and directories (place, vanish) the test sets, counting the +// verifications it makes. var listed int // fakeSkip's bookkeeping reads, across the test binary +// fakeDir is a directory in a fakeSkip's tree; its id is its identity. +type fakeDir struct{ id int } + +func (fakeDir) Name() string { return "d" } +func (fakeDir) Size() int64 { return 0 } +func (fakeDir) Mode() os.FileMode { return os.ModeDir | 0o755 } +func (fakeDir) ModTime() time.Time { return time.Time{} } +func (fakeDir) IsDir() bool { return true } +func (fakeDir) Sys() any { return nil } + +var ( + fakeTrees = map[*worktreeSkip]map[string]int{} + fakeNextID int +) + +// place puts a NEW directory at path — a fresh identity, as a mkdir or a tree +// moved in from elsewhere gives it. +func place(s *worktreeSkip, paths ...string) { + for _, p := range paths { + fakeNextID++ + fakeTrees[s][p] = fakeNextID + } +} + +// vanish removes path and everything beneath it from the fake tree. +func vanish(s *worktreeSkip, path string) { + for p := range fakeTrees[s] { + if p == path || strings.HasPrefix(p, path+string(filepath.Separator)) { + delete(fakeTrees[s], p) + } + } +} + func fakeSkip(t *testing.T) (s *worktreeSkip, verified, recorded map[string]bool, asked map[string]int) { t.Helper() root := filepath.Join(string(filepath.Separator), "ws") verified, recorded, asked = map[string]bool{}, map[string]bool{}, map[string]int{} s = &worktreeSkip{root: root} s.resetLocked() + fakeTrees[s] = map[string]int{} + t.Cleanup(func() { delete(fakeTrees, s) }) + s.stat = func(p string) (os.FileInfo, error) { + if id, ok := fakeTrees[s][p]; ok { + return fakeDir{id}, nil + } + return nil, os.ErrNotExist + } + s.sameFile = func(a, b os.FileInfo) bool { return a.(fakeDir).id == b.(fakeDir).id } s.isWorktree = func(r, dir string) bool { if r != root { t.Fatalf("judged against %q, want the root", r) @@ -229,6 +272,7 @@ func TestWorktreeSkip_CachesAndForgets(t *testing.T) { if !in("src/z.go", Remove) { t.Error("the worktree's own Remove re-decided it mid-removal") } + place(s, at("src")) in("src", Create) if in("src/z.go", Create) { t.Error("a directory arriving at the path did not re-decide it") @@ -427,6 +471,7 @@ func TestWorktreeSkip_ARecreatedPathOutlivesItsStaleRecord(t *testing.T) { t.Fatal("control: the recorded worktree's removal was not skipped") } s.inside(wt, Remove) + place(s, wt) s.inside(wt, Create) // an ordinary directory at the same path if s.inside(filepath.Join(wt, "b.go"), Create) { t.Fatal("an ordinary directory at a removed worktree's path was skipped on a stale record") @@ -535,6 +580,7 @@ func TestWorktreeSkip_AParentsArrivalRedecidesWorktreesBeneathIt(t *testing.T) { s, verified, recorded, _ := fakeSkip(t) x := filepath.Join(s.root, "agents") wt := filepath.Join(x, "wt") + place(s, x, wt) verified[wt] = true if stale { recorded[wt] = true @@ -543,8 +589,10 @@ func TestWorktreeSkip_AParentsArrivalRedecidesWorktreesBeneathIt(t *testing.T) { if !s.inside(filepath.Join(wt, "a.go"), Write) { t.Fatal("control: the worktree was not skipped") } + vanish(s, x) s.inside(x, Remove) verified[wt] = false // an ordinary tree arrives at x, with an ordinary wt + place(s, x, wt) s.inside(x, Create) if s.arrived(wt) { t.Error("the expansion of the arrived parent treats its ordinary wt as a worktree") @@ -559,6 +607,7 @@ func TestWorktreeSkip_AParentsArrivalRedecidesWorktreesBeneathIt(t *testing.T) { x := filepath.Join(s.root, "agents") wt := filepath.Join(x, "wt") verified[wt] = true + place(s, x, wt) s.inside(x, Create) if !s.arrived(wt) || !s.inside(filepath.Join(wt, "b.go"), Write) { t.Error("a worktree inside an arriving directory was not skipped") @@ -597,7 +646,8 @@ func TestWatcher_AnOrdinaryTreeMovedOverAWorktreesParentIsWatched(t *testing.T) // exists is kept. func TestWorktreeSkip_AFullWorktreeSetDropsOnlyVanishedDirectories(t *testing.T) { s, _, _, _ := fakeSkip(t) - live := t.TempDir() + live := filepath.Join(s.root, "live") + place(s, live) s.mu.Lock() s.rememberLocked(live, true) for i := range worktreeCacheMax { @@ -623,12 +673,14 @@ func TestWorktreeSkip_AnArrivalTouchesOnlyItsOwnSubtree(t *testing.T) { sibling := filepath.Join(s.root, "agents-old", "wt") verified[sibling] = true s.inside(filepath.Join(sibling, "a.go"), Write) + place(s, sibling) inner := filepath.Join(s.root, "agents", "wt") s.inside(filepath.Join(inner, "a.go"), Write) // cached ordinary verified[inner] = true if !s.arrived(inner) { t.Fatal("control: the directory that became a worktree was not recognised") } + place(s, filepath.Join(s.root, "agents"), inner) // a tree with a worktree in it moved in s.inside(filepath.Join(s.root, "agents"), Create) before := asked[sibling] if !s.inside(filepath.Join(sibling, "b.go"), Write) || asked[sibling] != before { @@ -638,3 +690,84 @@ func TestWorktreeSkip_AnArrivalTouchesOnlyItsOwnSubtree(t *testing.T) { t.Error("a worktree beneath the arrival was answered by its stale ordinary verdict") } } + +// TestWorktreeSkip_AMoveIsNotAnArrival (#703's review): inotify reports a +// directory moving AWAY as Remove|Rename, and one moved away and back is the +// same directory. Neither drops the worktree beliefs beneath it, so the rest +// of a removal stays skipped. +func TestWorktreeSkip_AMoveIsNotAnArrival(t *testing.T) { + for _, back := range []bool{false, true} { + t.Run(map[bool]string{false: "moved away", true: "moved away and back"}[back], func(t *testing.T) { + s, verified, recorded, _ := fakeSkip(t) + x := filepath.Join(s.root, "agents") + wt := filepath.Join(x, "wt") + place(s, x, wt) + verified[wt], recorded[wt] = true, true + s.lost() + if !s.inside(filepath.Join(wt, "a.go"), Write) { + t.Fatal("control: the worktree was not skipped") + } + ids := map[string]int{x: fakeTrees[s][x], wt: fakeTrees[s][wt]} + vanish(s, x) + s.inside(x, Remove|Rename) // IN_MOVED_FROM + if back { + fakeTrees[s][x], fakeTrees[s][wt] = ids[x], ids[wt] // the same directories + s.inside(x, Create|Rename) // IN_MOVED_TO + } + verified[wt] = false // its .git is the first thing a removal takes + if !s.inside(filepath.Join(wt, "pkg", "b.go"), Remove) { + t.Error("after a move, a worktree's removal was reported") + } + s.lost() // a cold verdict leans on the record, which a move must not supersede + if !s.inside(filepath.Join(wt, "pkg", "c.go"), Remove) { + t.Error("after a move, the worktree's record was superseded") + } + }) + } +} + +// TestWorktreeSkip_AnArrivingWorktreeLosesItsOrdinaryVerdict: a directory +// once judged ordinary that arrives anew as a worktree is re-decided. +func TestWorktreeSkip_AnArrivingWorktreeLosesItsOrdinaryVerdict(t *testing.T) { + s, verified, _, _ := fakeSkip(t) + x := filepath.Join(s.root, "x") + s.inside(filepath.Join(x, "a.go"), Write) // cached ordinary + verified[x] = true + place(s, x) + s.inside(x, Create) + if !s.inside(filepath.Join(x, "b.go"), Write) { + t.Error("an arriving worktree was answered by its old ordinary verdict") + } +} + +// TestWatcher_ARenameAwayAndBackKeepsARemovalQuiet: the real-backend shape of +// #703's review — on Linux, a worktree's parent moved away and back, then the +// worktree removed, reported thousands of events. +func TestWatcher_ARenameAwayAndBackKeepsARemovalQuiet(t *testing.T) { + root := t.TempDir() + gitRepoAt(t, root) + x := filepath.Join(root, "agents") + wt := filepath.Join(x, "wt") + populate(t, wt, "a.go", "pkg/b.go", "pkg/deep/c.go") + worktreeLink(t, wt, worktreeAdmin(t, root, "wt", wt)) + c := startWatcher(t, root, Options{SkipLinkedWorktrees: true}) + if err := os.WriteFile(filepath.Join(wt, "a.go"), []byte("w"), 0o644); err != nil { + t.Fatal(err) + } + rootStillReports(t, c, root) + tmp := filepath.Join(root, "agents.tmp") + if err := os.Rename(x, tmp); err != nil { + t.Fatal(err) + } + if err := os.Rename(tmp, x); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(wt); err != nil { + t.Fatal(err) + } + rootStillReports(t, c, root) + time.Sleep(300 * time.Millisecond) + if got := eventsUnder(c, wt); len(got) != 0 { + t.Fatalf("a worktree's removal after its parent moved away and back was reported: %v", got) + } +} From 8cb1602c6a065964fe926252f1ee520df532ac4c Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 15:21:06 +1100 Subject: [PATCH 05/10] test(watch): a moved-back worktree keeps its verdict without a record to lean on Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/worktrees_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index 14b3a4a6..bd98b396 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -771,3 +771,20 @@ func TestWatcher_ARenameAwayAndBackKeepsARemovalQuiet(t *testing.T) { t.Fatalf("a worktree's removal after its parent moved away and back was reported: %v", got) } } + +// TestWorktreeSkip_AMovedBackWorktreeKeepsItsVerdict: with no git record to +// lean on, a worktree's own cached verdict carries its identity through its +// parent moving away and back. +func TestWorktreeSkip_AMovedBackWorktreeKeepsItsVerdict(t *testing.T) { + s, verified, _, _ := fakeSkip(t) + x := filepath.Join(s.root, "agents") + wt := filepath.Join(x, "wt") + place(s, x, wt) + verified[wt] = true + s.inside(filepath.Join(wt, "a.go"), Write) + s.inside(x, Create|Rename) // moved back: the same directories + verified[wt] = false + if !s.inside(filepath.Join(wt, "b.go"), Remove) { + t.Error("a worktree's verdict did not survive its parent moving away and back") + } +} From 664f794d139c337414091d8f2a02c5b9494164be Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 15:22:07 +1100 Subject: [PATCH 06/10] test(watch): a moved-back git record stays trusted in the cold case Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/worktrees_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index bd98b396..3616618e 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -788,3 +788,19 @@ func TestWorktreeSkip_AMovedBackWorktreeKeepsItsVerdict(t *testing.T) { t.Error("a worktree's verdict did not survive its parent moving away and back") } } + +// TestWorktreeSkip_AMovedBackRecordStaysTrusted: a git record with no cached +// verdict — the cold case — carries its directory's identity through a move +// away and back, so the removal that follows is still skipped. +func TestWorktreeSkip_AMovedBackRecordStaysTrusted(t *testing.T) { + s, _, recorded, _ := fakeSkip(t) + x := filepath.Join(s.root, "agents") + wt := filepath.Join(x, "wt") + place(s, x, wt) + recorded[wt] = true + s.lost() + s.inside(x, Create|Rename) // moved back: the same directories + if !s.inside(filepath.Join(wt, "b.go"), Remove) { + t.Error("a recorded worktree's removal after its parent moved away and back was reported") + } +} From 13c784fdbb5c3938f4c8e0d113c9dc7d426581bf Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 15:22:51 +1100 Subject: [PATCH 07/10] docs(changelog): the arrival rule keys on directory identity Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8239f9bd..7ce60b7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,10 @@ The watcher remembered a linked worktree below a directory after that directory was removed, so an ordinary tree later moved into its place (say, `agents/` holding a plain `wt/`) had the edits inside `wt/` silenced. A - directory that arrives now re-decides every worktree beneath it; a real - worktree inside the arriving tree is still left out. + directory that arrives now re-decides every worktree beneath it that is no + longer the same directory; a real worktree inside the arriving tree is still + left out, and a worktree whose parent was only moved away and back keeps + its removal quiet. ## 0.26.1 (2026-10-16) From 3412df1720d782670bc7cef0e18e9ac1636b711a Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 15:45:23 +1100 Subject: [PATCH 08/10] fix(watch): a directory's identity includes its birth time #703's Linux CI failed TestWatcher_AnOrdinaryTreeMovedOverAWorktreesParentIsWatched: on ext4 the tree made right after `rm -rf` reused the freed inodes, so the moved-in ordinary wt read as the old worktree (device + inode alike) and its edits were silenced. The container gate's overlayfs hands out fresh inodes, which hid it. dirID now adds the birth time where the platform reports one (statx STATX_BTIME on Linux, Birthtimespec on darwin); elsewhere it falls back to os.SameFile. Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/dirid.go | 34 ++++++++++++++++++++++ internal/fswatch/dirid_darwin.go | 17 +++++++++++ internal/fswatch/dirid_linux.go | 22 +++++++++++++++ internal/fswatch/dirid_other.go | 8 ++++++ internal/fswatch/worktrees.go | 25 ++++++++--------- internal/fswatch/worktrees_test.go | 45 +++++++++++++++++++++++++++--- 6 files changed, 134 insertions(+), 17 deletions(-) create mode 100644 internal/fswatch/dirid.go create mode 100644 internal/fswatch/dirid_darwin.go create mode 100644 internal/fswatch/dirid_linux.go create mode 100644 internal/fswatch/dirid_other.go diff --git a/internal/fswatch/dirid.go b/internal/fswatch/dirid.go new file mode 100644 index 00000000..22731a3f --- /dev/null +++ b/internal/fswatch/dirid.go @@ -0,0 +1,34 @@ +package fswatch + +import "os" + +// dirID is a directory's identity, as worktrees.go compares it: the file +// identity os.SameFile reads (device and inode) and, where the platform +// reports one, the directory's birth time. Device and inode alone are not +// enough: a filesystem such as ext4 hands a freed inode straight to the next +// directory made, so a tree recreated right after `rm -rf` reads as the old one +// (#703's CI caught exactly that). The birth time tells them apart. +type dirID struct { + fi os.FileInfo + birth int64 // nanoseconds since the epoch; valid when hasBirth + hasBirth bool +} + +// statDir is dirID's os.Lstat. +func statDir(path string) (dirID, error) { + fi, err := os.Lstat(path) + if err != nil { + return dirID{}, err + } + birth, ok := birthTime(path, fi) + return dirID{fi: fi, birth: birth, hasBirth: ok}, nil +} + +// sameDir reports whether a and b are one directory: the same file, and — when +// both carry a birth time — born at the same moment. +func sameDir(a, b dirID) bool { + if !os.SameFile(a.fi, b.fi) { + return false + } + return !a.hasBirth || !b.hasBirth || a.birth == b.birth +} diff --git a/internal/fswatch/dirid_darwin.go b/internal/fswatch/dirid_darwin.go new file mode 100644 index 00000000..7b4acec0 --- /dev/null +++ b/internal/fswatch/dirid_darwin.go @@ -0,0 +1,17 @@ +//go:build darwin + +package fswatch + +import ( + "os" + "syscall" +) + +// birthTime is the directory's birth time, which darwin's stat reports. +func birthTime(_ string, fi os.FileInfo) (int64, bool) { + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0, false + } + return st.Birthtimespec.Sec*1e9 + st.Birthtimespec.Nsec, true +} diff --git a/internal/fswatch/dirid_linux.go b/internal/fswatch/dirid_linux.go new file mode 100644 index 00000000..c26b3f66 --- /dev/null +++ b/internal/fswatch/dirid_linux.go @@ -0,0 +1,22 @@ +//go:build linux + +package fswatch + +import ( + "os" + + "golang.org/x/sys/unix" +) + +// birthTime is the directory's birth time from statx(2), which a filesystem +// reports only when it records one (ext4, xfs, btrfs and tmpfs do). +func birthTime(path string, _ os.FileInfo) (int64, bool) { + var stx unix.Statx_t + if err := unix.Statx(unix.AT_FDCWD, path, unix.AT_SYMLINK_NOFOLLOW, unix.STATX_BTIME, &stx); err != nil { + return 0, false + } + if stx.Mask&unix.STATX_BTIME == 0 { + return 0, false + } + return stx.Btime.Sec*1e9 + int64(stx.Btime.Nsec), true +} diff --git a/internal/fswatch/dirid_other.go b/internal/fswatch/dirid_other.go new file mode 100644 index 00000000..5f2f8401 --- /dev/null +++ b/internal/fswatch/dirid_other.go @@ -0,0 +1,8 @@ +//go:build !linux && !darwin + +package fswatch + +import "os" + +// birthTime is unavailable here; identity falls back to os.SameFile alone. +func birthTime(string, os.FileInfo) (int64, bool) { return 0, false } diff --git a/internal/fswatch/worktrees.go b/internal/fswatch/worktrees.go index 95fb9f86..67891835 100644 --- a/internal/fswatch/worktrees.go +++ b/internal/fswatch/worktrees.go @@ -1,7 +1,6 @@ package fswatch import ( - "os" "path/filepath" "strings" "sync" @@ -63,7 +62,7 @@ type worktreeSkip struct { // ident is the identity of each directory a worktree verdict or record was // formed for, taken when it was formed: an arrival replaces a belief only // where a DIFFERENT directory now stands (arrivalLocked). - ident map[string]os.FileInfo + ident map[string]dirID // dirty is set by lost and consumed by the next lookup, which re-reads the // bookkeeping and re-decides every verdict once: a burst of loss signals @@ -71,16 +70,16 @@ type worktreeSkip struct { dirty atomic.Bool // isWorktree, listWorktrees, stat and sameFile are paths.LinkedWorktreeOf, - // paths.LinkedWorktreesOf, os.Lstat and os.SameFile: seams for tests that - // count or fake lookups and directories. + // paths.LinkedWorktreesOf, statDir and sameDir: seams for tests that count + // or fake lookups and directories. isWorktree func(root, dir string) bool listWorktrees func(root string) []string - stat func(string) (os.FileInfo, error) - sameFile func(a, b os.FileInfo) bool + stat func(string) (dirID, error) + sameFile func(a, b dirID) bool } func newWorktreeSkip(root string) *worktreeSkip { - s := &worktreeSkip{root: root, isWorktree: paths.LinkedWorktreeOf, listWorktrees: paths.LinkedWorktreesOf, stat: os.Lstat, sameFile: os.SameFile} + s := &worktreeSkip{root: root, isWorktree: paths.LinkedWorktreeOf, listWorktrees: paths.LinkedWorktreesOf, stat: statDir, sameFile: sameDir} s.resetLocked() s.refreshLocked() return s @@ -158,7 +157,7 @@ func (s *worktreeSkip) settleLocked() { // resetLocked empties every verdict. s.mu must be held (or s unshared). func (s *worktreeSkip) resetLocked() { s.worktree, s.ordinary, s.superseded = map[string]struct{}{}, map[string]struct{}{}, map[string]struct{}{} - s.ident = map[string]os.FileInfo{} + s.ident = map[string]dirID{} } // below is path relative to root, when it lies strictly below it. @@ -206,11 +205,11 @@ func (s *worktreeSkip) forgetLocked(path string, op Op) { // its beliefs then would leak the rest of a removal (#703's review measured // thousands of events). So nothing happens unless a directory now stands at // path, and a belief is dropped only where its directory still exists and is -// not the one it was formed for. A real worktree in the arriving tree verifies +// not the one it was formed for (dirID: device, inode and birth time). A real worktree in the arriving tree verifies // again. s.mu must be held. func (s *worktreeSkip) arrivalLocked(path string) { - fi, err := s.stat(path) - if err != nil || !fi.IsDir() { + id, err := s.stat(path) + if err != nil || !id.fi.IsDir() { return // nothing arrived: a move away, or a file } delete(s.ordinary, path) @@ -243,8 +242,8 @@ func (s *worktreeSkip) replacedLocked(d string) bool { // identifyLocked records the identity of the directory a belief about d is // formed for, when it exists. s.mu must be held. func (s *worktreeSkip) identifyLocked(d string) { - if fi, err := s.stat(d); err == nil { - s.ident[d] = fi + if id, err := s.stat(d); err == nil { + s.ident[d] = id } } diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index 3616618e..8f9c62f6 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -194,13 +194,13 @@ func fakeSkip(t *testing.T) (s *worktreeSkip, verified, recorded map[string]bool s.resetLocked() fakeTrees[s] = map[string]int{} t.Cleanup(func() { delete(fakeTrees, s) }) - s.stat = func(p string) (os.FileInfo, error) { + s.stat = func(p string) (dirID, error) { if id, ok := fakeTrees[s][p]; ok { - return fakeDir{id}, nil + return dirID{fi: fakeDir{id}}, nil } - return nil, os.ErrNotExist + return dirID{}, os.ErrNotExist } - s.sameFile = func(a, b os.FileInfo) bool { return a.(fakeDir).id == b.(fakeDir).id } + s.sameFile = func(a, b dirID) bool { return a.fi.(fakeDir).id == b.fi.(fakeDir).id } s.isWorktree = func(r, dir string) bool { if r != root { t.Fatalf("judged against %q, want the root", r) @@ -804,3 +804,40 @@ func TestWorktreeSkip_AMovedBackRecordStaysTrusted(t *testing.T) { t.Error("a recorded worktree's removal after its parent moved away and back was reported") } } + +// TestSameDir_ARecreatedDirectoryOnAReusedInodeDiffers: a directory removed +// and made again is a different directory even where the filesystem reuses the +// inode (ext4 does, at once), because its birth time differs. Where the +// platform records no birth time this degrades to os.SameFile, and the test +// only asserts the same-directory half. +func TestSameDir_ARecreatedDirectoryOnAReusedInodeDiffers(t *testing.T) { + d := filepath.Join(t.TempDir(), "wt") + if err := os.Mkdir(d, 0o755); err != nil { + t.Fatal(err) + } + before, err := statDir(d) + if err != nil { + t.Fatal(err) + } + again, _ := statDir(d) + if !sameDir(before, again) { + t.Fatal("control: one directory read twice is not the same directory") + } + if !before.hasBirth { + t.Skip("no birth time on this platform or filesystem") + } + time.Sleep(10 * time.Millisecond) + if err := os.Remove(d); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(d, 0o755); err != nil { + t.Fatal(err) + } + after, err := statDir(d) + if err != nil { + t.Fatal(err) + } + if sameDir(before, after) { + t.Errorf("a recreated directory reads as the old one (reused inode: %v)", os.SameFile(before.fi, after.fi)) + } +} From eb9f61c457df707b045d5088221ae62112f78e29 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 15:47:16 +1100 Subject: [PATCH 09/10] test(watch): sameDir compares file identity and birth time Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/worktrees_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index 8f9c62f6..d7057fb2 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -841,3 +841,32 @@ func TestSameDir_ARecreatedDirectoryOnAReusedInodeDiffers(t *testing.T) { t.Errorf("a recreated directory reads as the old one (reused inode: %v)", os.SameFile(before.fi, after.fi)) } } + +// TestSameDir_ComparesFileAndBirth pins both halves of sameDir without +// depending on whether this filesystem reuses inodes. +func TestSameDir_ComparesFileAndBirth(t *testing.T) { + base := t.TempDir() + a, b := filepath.Join(base, "a"), filepath.Join(base, "b") + for _, d := range []string{a, b} { + if err := os.Mkdir(d, 0o755); err != nil { + t.Fatal(err) + } + } + fa, _ := os.Lstat(a) + fb, _ := os.Lstat(b) + at := func(fi os.FileInfo, birth int64, has bool) dirID { return dirID{fi: fi, birth: birth, hasBirth: has} } + for _, tc := range []struct { + name string + x, y dirID + want bool + }{ + {"one directory, one birth", at(fa, 1, true), at(fa, 1, true), true}, + {"one inode, two births (a reused inode)", at(fa, 1, true), at(fa, 2, true), false}, + {"two directories, one birth", at(fa, 1, true), at(fb, 1, true), false}, + {"no birth time to compare", at(fa, 1, true), at(fa, 0, false), true}, + } { + if got := sameDir(tc.x, tc.y); got != tc.want { + t.Errorf("%s: sameDir = %v, want %v", tc.name, got, tc.want) + } + } +} From 1afa07acd478d73546e2276776afe0145ba8aac0 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Fri, 16 Oct 2026 15:59:20 +1100 Subject: [PATCH 10/10] test(watch): the birth-time test fails when the wiring drops a birth the filesystem records; a gone worktree keeps its verdict through an arrival Review of #703: the reused-inode test skipped exactly when dirID's birth time went missing, so a broken statDir or darwin birthTime survived it. It now skips only where the platform genuinely records no birth time (platformRecordsBirth, checked independently). Co-Authored-By: Claude Opus 5.5 --- internal/fswatch/dirid_darwin_test.go | 6 ++++++ internal/fswatch/dirid_linux_test.go | 12 ++++++++++++ internal/fswatch/dirid_other_test.go | 6 ++++++ internal/fswatch/worktrees_test.go | 22 ++++++++++++++++++++++ 4 files changed, 46 insertions(+) create mode 100644 internal/fswatch/dirid_darwin_test.go create mode 100644 internal/fswatch/dirid_linux_test.go create mode 100644 internal/fswatch/dirid_other_test.go diff --git a/internal/fswatch/dirid_darwin_test.go b/internal/fswatch/dirid_darwin_test.go new file mode 100644 index 00000000..e28ccf46 --- /dev/null +++ b/internal/fswatch/dirid_darwin_test.go @@ -0,0 +1,6 @@ +//go:build darwin + +package fswatch + +// platformRecordsBirth: darwin's stat always carries a birth time. +func platformRecordsBirth(string) bool { return true } diff --git a/internal/fswatch/dirid_linux_test.go b/internal/fswatch/dirid_linux_test.go new file mode 100644 index 00000000..7ea0e010 --- /dev/null +++ b/internal/fswatch/dirid_linux_test.go @@ -0,0 +1,12 @@ +//go:build linux + +package fswatch + +import "golang.org/x/sys/unix" + +// platformRecordsBirth reports, independently of dirID's wiring, whether this +// filesystem records a birth time for path. +func platformRecordsBirth(path string) bool { + var stx unix.Statx_t + return unix.Statx(unix.AT_FDCWD, path, unix.AT_SYMLINK_NOFOLLOW, unix.STATX_BTIME, &stx) == nil && stx.Mask&unix.STATX_BTIME != 0 +} diff --git a/internal/fswatch/dirid_other_test.go b/internal/fswatch/dirid_other_test.go new file mode 100644 index 00000000..ec92ba7d --- /dev/null +++ b/internal/fswatch/dirid_other_test.go @@ -0,0 +1,6 @@ +//go:build !linux && !darwin + +package fswatch + +// platformRecordsBirth: dirID reads no birth time on this platform. +func platformRecordsBirth(string) bool { return false } diff --git a/internal/fswatch/worktrees_test.go b/internal/fswatch/worktrees_test.go index d7057fb2..5862e956 100644 --- a/internal/fswatch/worktrees_test.go +++ b/internal/fswatch/worktrees_test.go @@ -824,6 +824,9 @@ func TestSameDir_ARecreatedDirectoryOnAReusedInodeDiffers(t *testing.T) { t.Fatal("control: one directory read twice is not the same directory") } if !before.hasBirth { + if platformRecordsBirth(d) { + t.Fatal("the filesystem records a birth time, but dirID did not read it") + } t.Skip("no birth time on this platform or filesystem") } time.Sleep(10 * time.Millisecond) @@ -870,3 +873,22 @@ func TestSameDir_ComparesFileAndBirth(t *testing.T) { } } } + +// TestWorktreeSkip_AnArrivalKeepsTheVerdictOfAGoneWorktree: a parent arriving +// while the worktree beneath it is already gone (a removal in flight) leaves +// that worktree's verdict, so the removal's remaining events stay skipped. +func TestWorktreeSkip_AnArrivalKeepsTheVerdictOfAGoneWorktree(t *testing.T) { + s, verified, _, _ := fakeSkip(t) + x := filepath.Join(s.root, "agents") + wt := filepath.Join(x, "wt") + place(s, x, wt) + verified[wt] = true + s.inside(filepath.Join(wt, "a.go"), Write) + vanish(s, wt) + verified[wt] = false + place(s, x) // a directory arrives at x; wt is not in it + s.inside(x, Create) + if !s.inside(filepath.Join(wt, "pkg", "b.go"), Remove) { + t.Error("a gone worktree lost its verdict to its parent's arrival") + } +}