Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,21 @@
example a hook-less desktop chat sharing a connector. Its calls can then be
credited to the session that did declare, so the report remains a proxy.

- **The file watcher leaves linked git worktrees out.** The OS watcher a
workspace's topology index and language servers share now drops every event
inside a linked worktree of the workspace's repository (or of one of its
submodules), never expands one that arrives whole, and stays quiet while one
is removed, since git's own records outlive the checkout. Agents' worktrees
inside a workspace made each checkout or removal a burst of thousands of
events that overflowed the watcher and forced resyncs: one workspace logged
583k lost-event warnings in a day. The index's own walk uses the same rule,
which now verifies a worktree through git's `commondir` and back-link rather
than the shape of its path, so a submodule at a path like `libs/worktrees/x`
is no longer mistaken for one and a hand-written `.git` pointer proves
nothing. A workspace that is itself a linked worktree now leaves out the
sibling worktrees nested inside it too. `[topology] index_worktrees` keeps
them, on a watcher of its own.

- **The Claude Code and Codex hooks stay inside their session's namespace.**
`plumb hooks run-claude` and `run-codex` now send the session's
`PLUMB_NAMESPACE` with every daemon request, and the daemon answers the
Expand Down
1 change: 1 addition & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,7 @@ language and resolved path, or the reason it will not run.
| `resync_batch` | int | `100` | Files the full resync extracts before pausing, to throttle CPU. `0` disables pacing. |
| `resync_pause_ms` | int | `25` | Pause (milliseconds) after each `resync_batch` files. `0` disables pacing. |
| `resync_interval_minutes` | int | `60` | Periodic full-resync **fallback**, used only when `watch = false` or the platform watcher cannot start; suppressed while the watcher is live. `0` disables. |
| `index_worktrees` | bool | `false` | Also index, and watch, the workspace's linked git worktrees (`git worktree add`, of its repository or of one of its submodules). Off by default: each is a second full copy of the tree, and agents create and remove them by the dozen. Only a worktree git's own bookkeeping verifies (its `commondir` and back-link) is left out; submodules always stay. With it off, the OS watcher the index shares with the language servers drops every event inside such a worktree; with it on, the index uses a watcher of its own. |
| `watch` | bool | `true` | OS-level file watching (`internal/fswatch`: FSEvents on macOS, inotify on Linux; other platforms fall back to `resync_interval_minutes`): re-index a file the instant it changes on disk, whoever changed it — this agent, another agent, or your editor. Replaces time-based polling; a mass change (e.g. `git checkout`) coalesces to a single paced resync via the bounded queue + overflow path. Set `false` to fall back to `resync_interval_minutes`. |

Note the two different meanings of `0` in this table: `max_file_size_bytes = 0` means *use the default*, and `extract_timeout_seconds = 0` means *use the built-in 2-minute ceiling* -- it no longer disables bounding entirely, since an unbounded parse can wedge the indexer.
Expand Down
21 changes: 17 additions & 4 deletions internal/cli/pool_descope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,25 @@ import (
// lockProbe records whether a mutex was free when probed.
type lockProbe struct{ heldOnce atomic.Bool }

// lockProbeWindow is how long mu must stay unavailable for the probe to say
// the caller holds it. One failed TryLock is not evidence: the test's own
// concurrent acquires take mu for a moment, and catching one of them flaked
// CI. A caller that holds mu across the probed call holds it throughout.
const lockProbeWindow = 200 * time.Millisecond

func (lp *lockProbe) probe(mu *sync.Mutex) {
if mu.TryLock() {
mu.Unlock()
return
deadline := time.Now().Add(lockProbeWindow)
for {
if mu.TryLock() {
mu.Unlock()
return
}
if time.Now().After(deadline) {
lp.heldOnce.Store(true)
return
}
time.Sleep(time.Millisecond)
}
lp.heldOnce.Store(true)
}

func within(t *testing.T, d time.Duration, what string, fn func()) {
Expand Down
8 changes: 5 additions & 3 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,11 @@ type TopologyConfig struct {
// each one multiplies both the index and the resync walk for no new
// information — one workspace reached 20,291 files and 753 MiB this way. It
// is an OPT-IN rather than an unconditional exclusion because the rule is
// exact: only a directory whose .git FILE records a gitdir under
// <workspace>/.git/worktrees/ is skipped, so submodules
// (<workspace>/.git/modules/) stay indexed either way.
// exact (paths.LinkedWorktreeOf): only a verified linked worktree of the
// workspace's repository or of one of its submodules is skipped, so
// submodules stay indexed either way. The OS file watcher shared by the
// index and the language servers leaves the same worktrees out; with this
// set, the index watches through a watcher of its own that keeps them.
IndexWorktrees bool `toml:"index_worktrees"`
// Watch enables OS-level file-system watching: any change to a source file —
// by this agent, another agent, or an external editor — is re-indexed at the
Expand Down
9 changes: 8 additions & 1 deletion internal/fswatch/backend_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ func (p *fseventsPump) run(batches <-chan []rawEvent) {
}
}

// skipped is everything FSEvents does not filter that the watcher drops for an
// event at path: Options.ExcludeRegex and Options.SkipLinkedWorktrees. (sgtdi
// applies the regex itself; pumpEvent checks the worktrees.)
func (w *Watcher) skipped(path string, op Op) bool {
return w.excluded(path) || w.worktrees.inside(path, op)
}

// accept takes one FSEvents record through loss detection, rebasing,
// exclusion, debouncing and directory expansion.
func (p *fseventsPump) accept(ev rawEvent, now time.Time) {
Expand All @@ -99,7 +106,7 @@ func (p *fseventsPump) accept(ev rawEvent, now time.Time) {
}
return
}
if p.w.excluded(path) {
if p.w.skipped(path, op) {
return
}
p.emit(path, op, now)
Expand Down
5 changes: 4 additions & 1 deletion internal/fswatch/backend_sgtdi.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,11 @@ func (w *Watcher) pumpEvent(ev fswatcher.WatchEvent, limitReached func(dir strin
// root's own metadata, with nothing for a consumer to do.
return op.Has(Remove | Rename)
}
if w.worktrees.inside(ev.Path, op) {
return false // sgtdi applied ExcludeRegex itself
}
w.deliver(Event{Path: ev.Path, Op: op})
if arrivedDir(ev.Path, op) {
if arrivedDir(ev.Path, op) && !w.worktrees.arrived(ev.Path) {
w.expand(ev.Path, func(child string) { w.deliver(Event{Path: child, Op: Create}) })
if !w.isFailed() && limitReached(ev.Path) {
w.degrade(limitMsg, nil)
Expand Down
4 changes: 2 additions & 2 deletions internal/fswatch/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (w *Watcher) excluded(path string) bool {
// subdirectory it cannot read) it asks for a reconcile. An entry that vanished
// mid-walk is not a loss: its removal is an event of its own.
func (w *Watcher) expandIfDir(path string, op Op, emit func(string)) {
if arrivedDir(path, op) {
if arrivedDir(path, op) && !w.worktrees.arrived(path) {
w.expand(path, emit)
}
}
Expand Down Expand Up @@ -72,7 +72,7 @@ func (w *Watcher) expand(path string, emit func(string)) {
if d.IsDir() {
// With the trailing separator, a pattern that excludes everything
// UNDER a directory (ExcludeDirsRegex's dot branch) prunes it whole.
if w.excluded(p) || w.excluded(p+string(filepath.Separator)) {
if w.excluded(p) || w.excluded(p+string(filepath.Separator)) || w.worktrees.arrived(p) {
return filepath.SkipDir
}
} else if w.excluded(p) {
Expand Down
9 changes: 9 additions & 0 deletions internal/fswatch/fswatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ type Options struct {
// base name it matches, before debouncing. It filters events only: it never
// narrows what the OS watches. Build one with ExcludeDirsRegex.
ExcludeRegex string
// SkipLinkedWorktrees drops every event inside a linked worktree of the
// root (paths.LinkedWorktreeOf), and never expands one that arrives whole
// (worktrees.go). Like ExcludeRegex it filters events only.
SkipLinkedWorktrees bool
}

// ErrUnsupported is returned by New on a platform with no backend that is safe
Expand Down Expand Up @@ -132,6 +136,7 @@ type Watcher struct {
failed chan struct{}

exclude *regexp.Regexp // Options.ExcludeRegex, nil when empty
worktrees *worktreeSkip // Options.SkipLinkedWorktrees, nil when off
expandLimit int // maxExpand, smaller in tests

done chan struct{}
Expand Down Expand Up @@ -174,6 +179,9 @@ func New(root string, opts Options) (*Watcher, error) {
expandLimit: maxExpand,
done: make(chan struct{}),
}
if opts.SkipLinkedWorktrees {
w.worktrees = newWorktreeSkip(w.root)
}
if err := w.startBackend(opts); err != nil {
return nil, err
}
Expand Down Expand Up @@ -230,6 +238,7 @@ func (w *Watcher) deliver(ev Event) {
// signalLost records that events may have been lost. Coalescing: if a signal
// is already pending, this one is folded into it.
func (w *Watcher) signalLost() {
w.worktrees.lost() // what was lost may have replaced a worktree (worktrees.go)
select {
case w.lost <- struct{}{}:
default:
Expand Down
222 changes: 222 additions & 0 deletions internal/fswatch/worktrees.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
package fswatch

import (
"path/filepath"
"strings"
"sync"
"sync/atomic"

"github.com/plumbkit/plumb/internal/paths"
)

// worktrees.go — Options.SkipLinkedWorktrees: a watcher that drops every event
// inside a linked worktree of its root. Agents create and remove worktrees
// inside a workspace by the dozen, each a full copy of the tree. Without this,
// every checkout or removal was a burst of thousands of events (and, for a
// worktree moved in whole, an expansion of all of it) that filled the event
// buffer and reported loss: on plumb-ops, a shared watcher over the agents'
// worktrees logged 583k lost-event warnings in a day.
//
// A directory is a worktree when it verifies as one (paths.LinkedWorktreeOf) or
// when git's own bookkeeping records it (paths.LinkedWorktreesOf: the admin
// directories' back-links, read when the watcher starts and again whenever a
// .git entry changes or events are lost). The second half is what keeps a
// REMOVAL quiet: `git worktree remove` and `rm -rf` delete the checkout's .git
// along with everything else, often first, and the admin directory outlives
// the checkout.
//
// Verdicts are cached per directory, so a steady stream costs a map lookup per
// ancestor. A "worktree" verdict is dropped only by an event that creates,
// removes or renames the directory itself; a .git event re-decides a "not a
// worktree" verdict (`git worktree add` creates the directory before it writes
// .git) but never downgrades a "worktree" one mid-removal, and neither does the
// directory's own Remove. An event that creates or renames into a recorded
// worktree's path re-decides it and supersedes the record, which can be stale.
// 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.
const worktreeCacheMax = 4096

// worktreeSkip answers "is this path inside a linked worktree of root".
//
// Concurrency: safe for concurrent use; mu guards every field below it.
type worktreeSkip struct {
root string

mu sync.Mutex
known map[string]bool // directory below root -> it is a linked worktree of root
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{}

// 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 func(root, dir string) bool
listWorktrees func(root string) []string
}

func newWorktreeSkip(root string) *worktreeSkip {
s := &worktreeSkip{root: root, known: map[string]bool{}, superseded: map[string]struct{}{}, isWorktree: paths.LinkedWorktreeOf, listWorktrees: paths.LinkedWorktreesOf}
s.refreshLocked()
return s
}

// inside reports whether path, the subject of an event with op, lies inside a
// linked worktree of root: whether a directory strictly between root and path
// is one. The root itself is never judged (a workspace that IS a linked
// worktree is watched), and nor is path itself, which may be a file; arrived
// reports on a directory that just arrived.
func (s *worktreeSkip) inside(path string, op Op) bool {
if s == nil {
return false
}
rel, ok := s.below(path)
if !ok {
return false
}
s.mu.Lock()
defer s.mu.Unlock()
s.settleLocked()
s.forgetLocked(path, op)
dir := s.root
parts := strings.Split(rel, string(filepath.Separator))
for _, part := range parts[:len(parts)-1] {
dir = filepath.Join(dir, part)
if s.verdictLocked(dir) {
return true
}
}
return false
}

// arrived reports whether dir, a directory that just arrived below root, is
// itself a linked worktree — which is then not expanded.
func (s *worktreeSkip) arrived(dir string) bool {
if s == nil {
return false
}
if _, ok := s.below(dir); !ok {
return false
}
s.mu.Lock()
defer s.mu.Unlock()
s.settleLocked()
// Remembered: a worktree that arrives is not expanded, so nothing inside it
// may ever be looked up before it is removed — by when its .git and git's
// record of it can both be gone.
v := s.decideLocked(dir)
if v {
s.known[dir] = true
}
return v
}

// lost marks every verdict for re-deciding: the events that would have told
// this watcher a worktree was removed and replaced (or a directory became one)
// may be among those lost. It is O(1); the next lookup does the work.
func (s *worktreeSkip) lost() {
if s != nil {
s.dirty.Store(true)
}
}

// settleLocked applies a pending lost: re-read the bookkeeping and drop every
// verdict. s.mu must be held.
func (s *worktreeSkip) settleLocked() {
if s.dirty.Swap(false) {
s.refreshLocked()
clear(s.known)
}
}

// 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)
if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
return "", false
}
return rel, true
}

// forgetLocked drops what an event at path with op may have changed. An event
// that creates or renames into path re-decides path itself. A .git event
// re-reads the bookkeeping and re-decides the directory holding it, unless that
// directory is already a worktree: its .git going away is how a removal starts.
// s.mu must be held.
func (s *worktreeSkip) forgetLocked(path string, op Op) {
// Only a directory arriving at the path re-decides it and supersedes its
// record. Its own Remove does neither: sgtdi delivers a burst's paths in no
// set order, at least its cooldown after the fact (by when git has deleted
// 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{}{}
}
}
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)
s.refreshLocked()
}
}
}

// 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 len(s.known) >= worktreeCacheMax {
for d, v := range s.known {
if !v {
delete(s.known, d)
}
}
}
v := s.decideLocked(dir)
s.known[dir] = v
return v
}

// 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 {
if s.isWorktree(s.root, dir) {
delete(s.superseded, dir) // a worktree again: its record can be trusted
return true
}
_, recorded := s.recorded[dir]
_, superseded := s.superseded[dir]
return recorded && !superseded
}

// refreshLocked re-reads git's bookkeeping, keyed in the root's own spelling
// (event paths use it; the bookkeeping is canonical). s.mu must be held.
func (s *worktreeSkip) refreshLocked() {
canonRoot := paths.Canonical(s.root)
s.recorded = map[string]struct{}{}
for _, wt := range s.listWorktrees(s.root) {
if rel, err := filepath.Rel(canonRoot, wt); err == nil {
s.recorded[filepath.Join(s.root, rel)] = struct{}{}
}
}
for d := range s.superseded {
if _, ok := s.recorded[d]; !ok {
delete(s.superseded, d) // no longer recorded: nothing to supersede
}
}
}
Loading
Loading