fix(watch): an arriving directory re-decides the worktrees beneath it

CHANGELOG.md

@@ -1,5 +1,18 @@
 # 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 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)
 
 ### Fixed

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
+}

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
+}

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 }

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
+}

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
+}

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 }

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 }

internal/fswatch/worktrees.go

@@ -35,9 +35,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,28 +47,40 @@ 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
 	// `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]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
 	// 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, 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) (dirID, error)
+	sameFile      func(a, b dirID) bool
 }
 
 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, stat: statDir, sameFile: sameDir}
+	s.resetLocked()
 	s.refreshLocked()
 	return s
 }
@@ -117,7 +130,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
 }
@@ -135,11 +148,18 @@ func (s *worktreeSkip) lost() {
 // verdict. s.mu must be held.
 func (s *worktreeSkip) settleLocked() {
 	if s.dirty.Swap(false) {
+		clear(s.worktree)
+		clear(s.ordinary)
 		s.refreshLocked()
-		clear(s.known)
 	}
 }
 
+// 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]dirID{}
+}
+
 // 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 +181,109 @@ 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: 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 <path>/<wt> 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 (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) {
+	id, err := s.stat(path)
+	if err != nil || !id.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 under(d) && s.replacedLocked(d) {
+			delete(s.worktree, d)
+		}
+	}
+	for d := range s.recorded {
+		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 id, err := s.stat(d); err == nil {
+		s.ident[d] = id
+	}
+}
+
 // 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 _, ok := s.worktree[dir]; ok {
+		return
+	}
+	if len(s.worktree) >= worktreeCacheMax {
+		for d := range s.worktree {
+			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
 // bookkeeping records. s.mu must be held.
 func (s *worktreeSkip) decideLocked(dir string) bool {
@@ -219,4 +311,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)
+		}
+	}
 }