Skip to content

fix(watch): the shared file watcher leaves linked git worktrees out - #700

Merged
atlas-from-plumb merged 12 commits into
mainfrom
atlas/watch-skip-worktrees
Oct 16, 2026
Merged

atlas-from-plumb merged 12 commits into
mainfrom
atlas/watch-skip-worktrees

Conversation

@atlas-from-plumb

Copy link
Copy Markdown
Collaborator

What

Follow-up from PLAN-491 and PLAN-538: the OS watcher a workspace's topology index and language servers share still reported every event inside the agents' linked worktrees. Each git worktree add/remove (or a worktree moved in whole, which the watcher expands) was a burst of thousands of events that overflowed the buffer and forced resyncs. plumb-ops logged 583k lost-event warnings in a day.

  • fswatch.Options.SkipLinkedWorktrees (internal/fswatch/worktrees.go) drops every event inside a linked worktree of the root, on both backends, and never expands one that arrives whole or sits inside a directory that arrives whole. The verdict is cached per directory and re-decided on any event for the directory or its .git entry, because git worktree add writes .git after creating the directory. The root and an event's own path are never judged.
  • The rule is the new paths.LinkedWorktreeOf. It verifies through commondir plus git's back-link (paths.CheckoutAt): a worktree of the root repository or of one of its submodules. The topology resync walk now uses it too, so the index and the watcher cannot disagree. Two behaviour changes in the index's rule, both deliberate:
    • a submodule at a path with a worktrees segment is no longer pruned (the residual false positive PLAN-491's review noted);
    • a hand-written .git pointer in the worktree shape, with no admin dir behind it, is no longer trusted.
  • topology.SharedWatchOptions sets the option. [topology] index_worktrees watches through options of its own that keep the worktrees. That costs a second OS watcher, for opt-in users only.
  • Residual: on Linux, sgtdi's own walk still registers inotify watches inside worktrees, since its exclusion regex is fixed at start. The events are dropped, but the watches are spent.

Verification

  • paths: TestLinkedWorktreeOf: a worktree of the root ✓; a worktree of a submodule ✓; the submodule itself, a libs/worktrees/foo submodule, an unrelated nested repo and its worktree, a forged pointer, a plain dir and the root ✗. A root that is not a checkout ✗.
  • fswatch (real backend):
    • an existing worktree, with a skip-off control;
    • a worktree created in place (.git written after the directory);
    • one moved in whole;
    • a nested worktree inside a moved-in directory;
    • cache and invalidation unit tests.
  • topology: shared and opt-in options, and OpenWith with index_worktrees opening its own watcher.
  • Mutation: 14 mutants across the backends, expansion, cache, option wiring, rule and store, all KILLED.

🤖 Generated with Claude Code

atlas-from-plumb added a commit that referenced this pull request Oct 16, 2026
Review of #700: a .git Remove event re-decided its directory from the disk,
where .git was already gone, so the rest of a `git worktree remove` or
`rm -rf` burst leaked (147 events warm, ~760 cold or with .git deleted first).

A directory is now a worktree when it verifies OR when git's bookkeeping
records it (paths.LinkedWorktreesOf: every admin directory's gitdir
back-link, for the root repository and its submodules), which outlives the
checkout during a removal. The bookkeeping is read at start, on a .git event
and on loss. A "worktree" verdict is dropped only by an event that creates,
removes or renames the directory itself; a .git event never downgrades one;
the cap evicts only "not a worktree" verdicts; a loss re-decides everything.
LinkedWorktreeOf stats dir/.git before resolving anything, and moves with the
new lister to internal/paths/worktrees.go.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
atlas-from-plumb added a commit that referenced this pull request Oct 16, 2026
…ory; loss is O(1)

Re-review of #700:
- an admin directory outlives an `rm -rf` until `git worktree prune`, and a
  `git worktree remove` leaves the cached record until the next re-read, so an
  ordinary directory created at the old path was silenced. A Create or Rename
  of a recorded path now supersedes the record: only a live verification
  counts there until a .git that verifies appears.
- lost() re-read the bookkeeping and cleared the cache on every call, and
  sgtdi signals loss once per dropped event. It now only marks the verdicts
  dirty; the next lookup re-reads once.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
atlas-from-plumb added a commit that referenced this pull request Oct 16, 2026
Re-review of #700: sgtdi can deliver a removed directory's own Remove before
its children's (it flushes due paths in map order), and superseding the
record there leaked the rest of the removal. Only a Create or Rename of the
path supersedes it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
atlas-from-plumb and others added 12 commits October 16, 2026 13:45
Agents create and remove worktrees inside a workspace by the dozen; the OS
watcher the topology index and the language servers share reported every
event inside them, and a worktree created or moved in whole was a burst of
thousands of events (an expansion of the whole checkout) that overflowed the
buffer and forced resyncs — 583k lost-event warnings in a day on plumb-ops.

fswatch.Options.SkipLinkedWorktrees drops every event inside a linked
worktree of the root and never expands one that arrives. The verdict is
cached per directory and re-decided when the directory or its .git entry
changes, since `git worktree add` writes .git after creating the directory.
The rule is paths.LinkedWorktreeOf, which verifies through commondir and git's
back-link; the topology resync walk now uses it too, so the two never
disagree, a submodule at a path containing a `worktrees` segment is no longer
mistaken for a worktree, and a hand-written .git pointer proves nothing.
SharedWatchOptions sets it; [topology] index_worktrees watches through options
of its own that keep the worktrees.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…watches on its own options

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Review of #700: a .git Remove event re-decided its directory from the disk,
where .git was already gone, so the rest of a `git worktree remove` or
`rm -rf` burst leaked (147 events warm, ~760 cold or with .git deleted first).

A directory is now a worktree when it verifies OR when git's bookkeeping
records it (paths.LinkedWorktreesOf: every admin directory's gitdir
back-link, for the root repository and its submodules), which outlives the
checkout during a removal. The bookkeeping is read at start, on a .git event
and on loss. A "worktree" verdict is dropped only by an event that creates,
removes or renames the directory itself; a .git event never downgrades one;
the cap evicts only "not a worktree" verdicts; a loss re-decides everything.
LinkedWorktreeOf stats dir/.git before resolving anything, and moves with the
new lister to internal/paths/worktrees.go.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ne TryLock

TestTopologyPool_OpenIsUnlockedAndCoalesced flaked on macOS CI: a single
failed TryLock caught the test's own concurrent Acquire holding the pool mutex
for a moment. A caller that holds it across the probed open holds it for the
whole window.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ory; loss is O(1)

Re-review of #700:
- an admin directory outlives an `rm -rf` until `git worktree prune`, and a
  `git worktree remove` leaves the cached record until the next re-read, so an
  ordinary directory created at the old path was silenced. A Create or Rename
  of a recorded path now supersedes the record: only a live verification
  counts there until a .git that verifies appears.
- lost() re-read the bookkeeping and cleared the cache on every call, and
  sgtdi signals loss once per dropped event. It now only marks the verdicts
  dirty; the next lookup re-reads once.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Re-review of #700: sgtdi can deliver a removed directory's own Remove before
its children's (it flushes due paths in map order), and superseding the
record there leaked the rest of the removal. Only a Create or Rename of the
path supersedes it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…t re-decide it

The Linux gate leaked an event from TestWatcher_GitWorktreeAddAndRemoveAreQuiet.
sgtdi processes a burst at least its cooldown after the fact and in no set
order, so a quick add-then-remove was decided after git had deleted the
admin directory: the worktree had arrived whole (never expanded, nothing
inside it looked up), so nothing remembered it. arrived() now caches a
worktree verdict, and only a Create or Rename at the path re-decides it — the
directory's own Remove can come before its children's.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@atlas-from-plumb
atlas-from-plumb force-pushed the atlas/watch-skip-worktrees branch from 9143dce to 112bcb7 Compare October 16, 2026 02:46
@atlas-from-plumb
atlas-from-plumb merged commit 514aed8 into main Oct 16, 2026
9 checks passed
@atlas-from-plumb
atlas-from-plumb deleted the atlas/watch-skip-worktrees branch October 16, 2026 03:05
atlas-from-plumb added a commit that referenced this pull request Oct 16, 2026
Hooks stay inside their session's namespace (#698); context-hint uptake credits
a hint's use on a connection its session alone declared (#699); the shared file
watcher leaves linked git worktrees out (#700); an [lsp] restart can no longer
land on a superseded config (#701).


Plumb-Session: forest-ridge

Co-authored-by: Claude Opus 5.5 <[email protected]>
atlas-from-plumb added a commit that referenced this pull request Oct 16, 2026
…neath it

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 <[email protected]>
atlas-from-plumb added a commit that referenced this pull request Oct 16, 2026
…#703)

* fix(watch): a directory's arrival re-decides the worktree verdicts beneath it

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 <[email protected]>

* test(watch): an arrival re-decides only its own subtree, and not from a stale ordinary verdict

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* docs(changelog): 0.26.2 (unreleased) — an arriving directory re-decides the worktrees beneath it

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* 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 <[email protected]>

* test(watch): a moved-back worktree keeps its verdict without a record to lean on

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* test(watch): a moved-back git record stays trusted in the cold case

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* docs(changelog): the arrival rule keys on directory identity

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* 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 <[email protected]>

* test(watch): sameDir compares file identity and birth time

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants