Make the file watcher respect ignore rules — refresh them live, and stop subscribing to ignored trees

[Shengyu Fu (shengyfu)]

Fixes #104.

tgrep serve's watcher was ignoring ignore rules in two different ways. Both are addressed here.

Field report (Office monorepo team), which this PR is measured against. In a controlled Git repo with 40,212 physical directories but only 200 searchable files after .gitignore, the shipping build registered exactly 40,212 Linux inotify watches — every directory — and watched .git despite --exclude .git. --no-watch registered zero and started instantly. That is this bug, quantified in production, on the bundled tgrep 1.0.2. This branch carries the fix and bumps the workspace to 1.0.3, so that team can tell from tgrep --version whether a build contains it — but 1.0.3 still has to be merged and released before it reaches the bundled CLI.

1. A .ignore file written while the server was live had no effect

is_ignore_rules_file — the predicate that decides whether a change should rebuild the ignore matcher — matched .gitignore at any depth and a root p4ignore.ini, but not .ignore. So writing a .ignore never set ignore_rules_dirty and never scheduled schedule_ignore_rules_refresh. The write then fell through to the ordinary reindex path, where should_skip_watcher_path drops anything with a dot-prefixed segment — so nothing happened at all. The rules only took effect on the next server restart.

The fix is one predicate, using the filename constants tgrep-core already exports so the watcher and the walker cannot drift apart again.

2. The watcher subscribed to ignored directories and filtered on delivery

Events for ignored paths were discarded, but only after the OS had delivered them. On Linux that is too late to matter:

• inotify has no recursive mode. RecursiveMode::Recursive makes notify walk the tree and spend one watch descriptor per directory — which is exactly the 1:1 ratio the field report measured.
• A repo whose target/, node_modules/ or out/ holds most of its directories burns most of the per-user fs.inotify.max_user_watches budget on events that are immediately thrown away.
• Worse, notify's registration loop propagates the first failure with ?. A repo large enough to exhaust that budget makes watcher.watch() return an error, and the server loses its watcher entirely — silently falling back to the hourly reconcile. At 40,212 watches against a common default of 65,536 shared across every process on the machine, that ceiling is close.
• It also explains the reported startup delay: registration walks and issues a syscall per directory before the server is usable, which is why --no-watch "started immediately".

So on inotify backends the watcher now subscribes per directory:

• watchable_dirs(root, start, ..) walks once and prunes ignored / hidden / --excluded subtrees before descending. root and start are separate because ignore rules are anchored at the repository root, so a subtree discovered at runtime still has to be matched against root-relative paths.
• WatchRegistry::sync reconciles the live subscription set against that, on every ignore-matcher publish, so relaxing a rule subscribes to the tree it used to hide and tightening one drops it.
• WatchRegistry::add_all is the additive half, used for subtrees that appear at runtime. Keeping it separate is both a performance and a correctness matter: it is proportional to the new subtree rather than the whole watched set, and passing a subtree to sync would treat the entire rest of the repo as stale and unsubscribe from it.
• A directory that cannot be subscribed is reported once and skipped, instead of taking down the whole watcher.

The initial descendant sync is deferred until the ignore matcher is published. Events are dropped while gitignore_pending is set anyway, and subscribing first and narrowing afterwards would mean briefly holding exactly the watches this is meant to avoid.

state.gitignore had three publish sites and only one went through the helper; they are unified behind publish_ignore_matcher so the sync hook cannot be missed.

Measured against the reported layout

Reproducing the report's shape — 40,210 directories, ~200 searchable files, .git both hidden and --excluded:

	Directories watched	Time to compute
Before	40,212 (all of them)	walks + one syscall each
After	202	13.7 ms

The walk cost is set by the tree that survives pruning, not the physical tree: the same measurement over 8,458 directories takes 12.6 ms. Pruning happens before descent, so the ignored subtrees are never entered.

The reported .git symptom is covered twice over — it is skipped as a hidden directory and again by --exclude, both asserted in watchable_dirs_prunes_ignored_and_hidden_subtrees.

Scope caveat, stated plainly: this reduces the watch count in proportion to how much of the tree is pruned by directory-level ignore rules. A repo whose directories are mostly tracked but whose files are excluded by file patterns would still be watched, and correctly so — a new source file can appear in any of those directories. The [trace] watcher subscriptions: N directories line reports the real number so this is verifiable on any repo rather than assumed.

A pre-existing Linux bug this also fixes

The control run turned up something I was not looking for: on Linux, a file created in a directory that did not exist when the server started was never indexed. Recursive inotify registration is racy by construction — notify only adds a watch for src/fresh/ when it receives that directory's creation event, so src/fresh/deeper/new.rs, written moments later, lands before the watch exists and its event is lost until the next hourly reconcile. watch_new_subtree closes this by indexing what it finds as it subscribes.

Platform scope

Platform	Backend	Effect
Linux / Android	inotify	Ignored directories are never subscribed to. This is the real fix, and the platform the report is from.
Windows	ReadDirectoryChangesW	Single recursive subscription; per-path registration is not expressible. Delivery-time filtering remains the only lever.
macOS	FSEvents	Same as Windows.

Part 1 applies to every platform.

Testing

Unit:

• identifies_live_ignore_rule_changes extended to cover .ignore.
• watchable_dirs_prunes_ignored_and_hidden_subtrees — covers both reported .git paths (hidden and --excluded) plus a gitignored subtree.
• watchable_dirs_without_a_matcher_keeps_everything_visible — --no-ignore must not silently narrow the watch set below what gets indexed.
• watchable_dirs_anchors_rules_at_the_root_not_the_start_directory — regression for the root/start distinction; caught a real bug while writing it.
• watch_registry_add_all_is_additive_but_sync_prunes — pins the distinction whose confusion would unsubscribe a whole repo.
• skip_watcher_dir_applies_directory_semantics — a build/ rule does not match the path build under file semantics, which is why the directory check is separate.

End-to-end:

• late_dot_ignore_refreshes_the_watchers_ignore_rules.
• watcher_does_not_subscribe_to_gitignored_directories (Linux) — counts the server's real inotify watch descriptors via /proc/<pid>/fdinfo, the same way the field report measured. It first proves the watcher is live by indexing a newly created file, so a watcher that registered nothing could not pass by accident.
• watcher_indexes_files_in_directories_created_after_startup (all platforms) — a new nested directory gets indexed, a new directory under a gitignored path does not.

Both fixes were checked against a control rather than assumed to work:

• Part 1: stashing the predicate change made its e2e test fail with the intended assertion; restoring it made it pass.
• Part 2: a throwaway branch with PER_DIRECTORY_WATCHES forced to false (the old behaviour) was run through CI on ubuntu. watcher_does_not_subscribe_to_gitignored_directories failed with "watcher holds 69 inotify watches ... expected at most 12", and watcher_indexes_files_in_directories_created_after_startup failed too — which is how the pre-existing race above was found. That branch has been deleted.

cargo test --workspace, cargo clippy --all-targets -- -D warnings and cargo fmt --all --check are clean; CI is green on ubuntu, macOS and Windows.

Review round

Three races the review caught, all fixed in 6ddba29.

Watches were established after the walk they were derived from. A file written to a directory between the matcher-producing walk and its subscription was in neither the walk's results nor any event, and RECONCILE_INTERVAL is 3600s, so it stayed invisible for an hour. sync_watch_registrations now returns the directories it newly subscribed to; publish_ignore_matcher passes them through (#[must_use], so a future call site cannot quietly drop them) and the stale check rechecks them.

Placement matters: stream_merge_stale_changes replaces file_stamps wholesale, so the scan has to run after the merge or it would be discarded and would re-read every stale file on the way. background_refresh_stale is now a thin wrapper holding stale_refresh_lock + snapshot_gate across refresh_stale_locked and then the recovery, and only scans when the inner call succeeded. The two index-build publishes skip it deliberately — there "newly watched" is the whole repo and the startup stale check already does a superset.

watch_new_subtree read each directory before subscribing to it, leaving the same race one level down. It now subscribes to a level before enumerating it, releasing the registry lock before the I/O.

A subtree that arrives whole can carry its own ignore rules. A clone, a git mv, a branch switch or an unpacked archive all land that way. Those files are dot-prefixed, so the recovery scan dropped them silently and indexed the rest against rules that had never heard of the subtree. It now looks for ignore rules first and defers to a refresh instead of indexing under stale ones.

Path::is_dir follows symlinks. A link to a directory was subscribed to and walked through, indexing a target the walker never descends into and that can sit outside root. is_real_dir uses symlink_metadata and is applied at the handle_fs_event call site and inside watch_new_subtree.

Verified against a control again rather than assumed. A throwaway branch with only the ignore-rule detection disabled failed on ubuntu with exactly the intended assertion — "watcher indexed a file excluded by a .gitignore that arrived inside the same subtree" — confirming the new e2e test is not vacuous on the one platform that exercises per-directory watches. That branch has been deleted.

New coverage: watcher_honors_ignore_rules_inside_a_subtree_that_arrives_whole (e2e) and is_real_dir_rejects_a_symlink_to_a_directory (unit, #[cfg(unix)], and it asserts link.is_dir() is true so the test documents the trap rather than just the fix).
A directory removed and recreated stopped being watched, permanently. The kernel releases an inotify watch when its directory goes away and reports nothing, so the path stayed in watched with no descriptor behind it. No later sync could recover it either — the recreated path is in desired and in watched, which is indistinguishable from a live subscription — so rm -rf build && mkdir build, a branch switch or a git clean silently stopped that directory being watched for the life of the process. Fixed in 2907e1b with two cheap halves: forget clears the entry on a removal event (one hash lookup, not a prefix sweep — deleting a tree delivers one event per directory, so anything proportional to the watched set would be quadratic on a rm -rf), and watch_new_subtree re-issues subscriptions rather than trusting watched, since a directory that has just appeared is exactly where that belief is worthless. The second half covers what the first cannot: a mv away delivers no events for the descendants it carries off.

Controlled again: with both halves disabled, ubuntu fails the new unit test, and — once that is skipped so the run continues — the new e2e test watcher_rewatches_a_directory_that_is_removed_and_recreated fails with "watcher never saw a file written to a directory that was removed and recreated". It writes its file three seconds after the recreation is processed, so the subscription pass's own scan cannot mask a missing watch. Branch deleted.

The recovery scan was still discarded at three of the four sites. My justification comments for that were wrong, which the review caught. On a warm start the stale check runs on a thread spawned before start_file_watcher, so it can publish while watch_registry is still None and take no subscriptions at all; its walk is then already over by the time the startup sync runs, making that sync the first descendant pass in the process with nothing following it until the hourly reconcile. All three sites now go through spawn_recovery_scan. It runs on a thread and waits out indexing, because background_index_build publishes early in the build while the stamps are only written at the end — scanning at the publish point would read every file as changed and duplicate the whole build.

watch_new_subtree ran for every directory event, including Modify(Metadata). A recursive chmod or a branch switch over 40k directories re-walked and re-subscribed each subtree once per directory in it. Gated to Create and Modify(Name), the only kinds that can introduce a directory.

The watcher ignored the walker's per-file rules. should_skip_watcher_path filters by location only, so a binary extension or a file over --max-filesize was indexed when it arrived through the watcher even though a walk of the same tree rejects it — and the next reconcile then deleted it again. reindex_file now applies the same two rules as walk_file_metadata, and additionally drops an entry it already holds when a file stops being eligible (a file can grow past the cap, and the smaller version's trigrams would otherwise keep being served).

New coverage: watcher_applies_the_same_file_eligibility_rules_as_the_walker asserts both rejections next to an eligible file written at the same moment, so it cannot pass against a watcher that has simply stopped indexing. Confirmed decisive — short-circuiting the eligibility check to true fails it on binary_extension_marker. CI green on ubuntu, macOS and Windows.

Validated against a real enlistment

Run against Microsoft's Substrate monorepo (290,016 indexed files) with an A/B control: this branch (329ec66) and its merge-base with main (86a000c), back to back on the same machine, each against its own copy of the prebuilt 2.8 GiB index.

The reported bug reproduces there, and is fixed. With a file already indexed and an ignore rule then written over it, the control leaves it searchable indefinitely while .gitignore — the control half, always recognised — drops correctly:

Rule file	merge-base 86a000c	this branch 329ec66
.ignore	still searchable after 90 s	dropped
.gitignore	dropped	dropped

The round-3 eligibility fix matters on real data too. A 65 MiB file written into a watched tree is indexed by the merge-base — its trace log shows reindex: modified .../big.cs — even though a walk of the same tree rejects it, so the next reconcile would silently delete it again. This branch skips it, as it does a .png.

Worth noting for reviewers: PER_DIRECTORY_WATCHES is cfg!(linux), so the selective registration, forget-on-removal and event-kind gate are inert on Windows and that run cannot exercise them. It covers the two platform-independent fixes and rules out a regression.

No regression. Median visibility latency across 1/10/100/1000-file batches ranges from −17% to +4% versus the control; startup is 385.9 ms vs 405.4 ms and the stale check 2659 ms vs 2733 ms over the same 290,016 files. The 5,000-file branch-switch burst is identical on correctness — 5000/5000 observed, 0 residual, 0 queue overflows — and peak memory is lower (633 MiB vs 714 MiB).

One figure initially looked like a +24% regression in burst deletion (20.7 s vs 16.7 s). It is variance: re-running the same binary gave 15.1 s, so that build alone spans 15.1–20.7 s and the control sits inside its range. That matches the code, since the deletion path is byte-identical on Windows — the only change to it, WatchRegistry::forget, is behind PER_DIRECTORY_WATCHES.

An earlier comparison against the suite's stored baseline appeared to show a uniform +25–32% regression. That baseline was recorded three days earlier from a different worktree and its measurement floor was ~100 ms lower, which accounted for the whole difference; it is why the merge-base was built and measured instead.

Review round 4 — five reported issues, all confirmed and fixed

All five were real. Fixed in 371bc77; CI green on ubuntu / macOS / Windows.

1. Symlink escape (high). reindex_file used std::fs::metadata, which follows links, and then read through the link. The indexer walks with follow_links(false), where a symlink is neither file nor dir and is skipped, so a link inside the repository had its target's bytes indexed under the link's own path — and the target need not be under the served root. Round 1 fixed this for directories (is_real_dir); the file case one line below was missed. Now symlink_metadata, with is_file on the link's own metadata as an eligibility rule, so a link falls into the branch that drops whatever was indexed at that path before.

Control experiment: on a scratch branch with only this hunk reverted, the new test fails on both ubuntu and macOS with the watcher followed a symlink and indexed content from outside the served root; everything else passes.

2. Recovery scan gaps. reindex_files_in looked only at files directly inside each newly watched directory. It now also:

• picks up subdirectories created in the same window, filtered by subscription membership so the startup case (where the list is every directory in the repository) stays one hash lookup apiece instead of a re-walk per level;
• drops entries for files removed in the window, via a single pass over the stamps. Only directories whose read_dir succeeded are swept, and the "still there" set is built from every entry regardless of ignore rules or eligibility, so a file indexed under a laxer configuration is never deleted for being ineligible now;
• defers to an ignore-rules refresh when an ignore file landed in the window. That test is bounded at both ends by mtime: a one-sided test would fire on every scan on a network mount whose server clock runs ahead, and each firing schedules a whole-repository rewalk that arms the next one.

3. Stamp publication race. background_index_build assigned state.file_stamps after dropping the publish gate but cleared indexing before it. The recovery scan waits on indexing, then on the gate, so it ran in that window against empty stamps: every file read as changed and the whole repository was re-read — exactly what the wait exists to prevent — and the assignment then discarded the stamps the scan had just recorded. Stamps are now published inside the gate before the flag flips, and the flush takes a read guard rather than a clone (verified: nothing on the flush path touches file_stamps).

4. Over-subscription of ignored moved trees. watch_new_subtree recorded an ignore-rules file and kept descending, taking a watch descriptor per level of a tree the rules it was about to publish would exclude — the moved-in node_modules case this PR exists to fix. It now abandons the descent at the point of discovery; the refresh redoes the traversal properly.

5. Stamp-dependent drop. The "no longer eligible" branch only deleted when a stamp entry existed. It now also consults the live overlay. Deliberately not unconditional the way the removal branch is: removals are rare, but this runs for every ineligible file a recovery scan walks past, and delete_file records a tombstone and dirties the overlay even for a path that was never indexed.

Coverage

• watcher_does_not_index_through_symlinks (unix) — a link to a file outside the root is not indexed, and a real file replaced by a link loses its indexed content.
• watcher_applies_the_same_file_eligibility_rules_as_the_walker extended with a file that grows past --max-filesize after being indexed.
• Assertions that require content to be dropped now poll rather than checking once, so they do not depend on event ordering on a backend that coalesces.

Note on platform

Issues 2, 3 and 4 are on Linux-only code paths — sync_watch_registrations and spawn_recovery_scan return early unless PER_DIRECTORY_WATCHES. The Substrate run reported above was on Windows and could not have exercised them.

Review round 5

Four more findings, all valid, all fixed in 700e64e.

The eligibility check and the read were two different lookups. symlink_metadata(path) followed by std::fs::read(path) resolves the name twice, and a tree being rewritten underneath the watcher — a checkout, a build, a git mv — can put a symlink there in between. The bytes indexed were then not the ones judged eligible, which is the same escape round 4 closed, through a smaller window. There is now one handle: open_no_follow opens the path itself (O_NOFOLLOW on unix; the reparse point on Windows, where the is_file check on the handle's metadata is what rejects it), and the type, size, mtime and contents all come off it.

The ineligible-file drop was conditional on evidence that can go missing. ServerState accepts an empty stamp map when filestamps.json is missing or unreadable, and the reader can still hold the path in that state, so had_stamp || in_overlay could be false for something that is very much still searchable. The delete is now unconditional. The reason it was not before is real — live::delete_file records a tombstone and dirties the overlay even for a path that was never indexed, and this runs for every binary asset a startup scan walks past — so an existing tombstone is now taken as proof there is nothing left to do, which bounds that to one per distinct path.

since came from the wrong walk. It was the moment the subscription sync started, but the window that needs closing opens when the walk that produced the matcher starts. A nested .ignore written between the two carries an mtime that predates the later timestamp, so the recovery scan reads it as already accounted for and its subtree stays indexed under rules that never saw it. publish_ignore_matcher now takes since from the caller, and both build paths capture it before their traversal.

Events discarded during the initial build were simply lost. They cannot be applied while indexing is set — the stamps do not describe the index yet, so every path would compare as changed — but they are the only record that those paths moved, and the build's own walk misses anything written to a directory it has already passed. They are now buffered and replayed once the build publishes, as synthetic create events so they go through exactly the filtering an ordinary event gets. The buffer is capped at 100k paths and gives up as a whole on overflow (a truncated set is indistinguishable from a complete one at replay time), falling back to a full reconcile. Note this is the first recovery of any kind on Windows and macOS, where there are no per-directory subscriptions and the previous scan returned immediately.

The root was never rechecked. It is subscribed as the watcher starts, before any matcher exists, so every later sync sees it as already watched and it never appears in the newly-watched list. Nothing covered a file written to the top level while a build walk was deeper in the tree. It now costs one directory listing.

Verification

cargo fmt / cargo clippy --all-targets -D warnings / cargo test --workspace clean; CI green on ubuntu, macOS and Windows.

Four new tests, confirmed to run on Linux rather than silently skipped:

• open_no_follow_reads_a_regular_file — the metadata and the bytes come off one handle.
• open_no_follow_refuses_a_symlink (unix) — the link does not open as its target.
• deferring_more_changes_than_the_cap_gives_up_on_the_whole_set — overflow marks the buffer unusable rather than truncating it, and stays that way.
• changes_deferred_during_a_build_are_applied_once_it_publishes — an event during a build touches nothing, and the change lands after.

Control experiment: a scratch branch with only the deferral call removed, run through CI on a draft PR, to confirm the last of those fails without the fix.

Review round 6

Two more, both valid, fixed in f9b9579.

Refusing to follow the final component is not enough. O_NOFOLLOW and FILE_FLAG_OPEN_REPARSE_POINT guard the file being opened and nothing above it, so a path that arrives from an event or a replay as root/a/file still resolves through a if a is a link — and the file at the end of that is a perfectly ordinary file, outside the tree we serve. Same escape as rounds 4 and 5, one level up.

open_within_root now resolves the path a component at a time starting from the root, which is the trust anchor (a link there is the user's to have). On unix that is openat against the parent's handle with O_NOFOLLOW on every component and O_DIRECTORY on the intermediates — the name is never re-resolved, so this is race-free, not merely checked. On Windows there is no openat; each ancestor is checked by path for a reparse point (is_symlink covers junctions, since both are name-surrogate reparse tags) and the final component opened as before. That rejects a link that is actually there but not one substituted mid-flight; closing that needs NtCreateFile with a RootDirectory, which is a lot of unsafe code for a platform where creating a symlink requires a privilege the machine does not grant by default. Non-literal components (.., prefixes) are refused rather than interpreted.

Two indexers could race on one path. snapshot_gate is held for read by everything that indexes a file, so a recovery scan and the watcher worker can both be inside reindex_file for the same path, both see the same old stamp, both read — and the one that read the older content can commit last. The newer event is already consumed, so the stale version survives until the next reconcile. A new reindex_lock makes the whole check-read-commit cycle atomic. Taken per file rather than per scan, so a recovery pass and the watcher interleave instead of one waiting out the other, and searches never take it.

Also opened with O_NONBLOCK while I was there, so a fifo in the tree answers immediately instead of blocking the watcher thread until someone opens the write end.

Verification

CI green on ubuntu, macOS and Windows. Four new tests, confirmed to run on Linux:

• open_within_root_reads_a_regular_file
• open_within_root_refuses_a_symlinked_file (unix)
• open_within_root_refuses_a_symlinked_ancestor (unix) — asserts first that File::open on that path does succeed, so the test is about containment rather than about the file being unreachable
• open_within_root_refuses_paths_that_escape_or_are_not_literal

Review round 7

Five more, all the same shape: a decision made on evidence that does not support it.

Replay reconstructed the wrong event kind. Deferred paths were replayed as Create regardless of what actually happened, which is what handle_fs_event uses to decide whether to walk a new subtree. A recursive chmod or a checkout during the initial build therefore put every directory through watch_new_subtree — quadratic, and on Linux a watch descriptor per level of trees that should never have been entered. The buffer now carries a flag per path (Create / rename vs. everything else) and the replay reconstructs from it. Covered by a_deferred_metadata_change_does_not_replay_as_a_subtree_arrival.

The deferral handoff was not synchronized. handle_fs_event checked indexing, then called into defer_events_during_build, which inserted without re-checking. Between the two the build could finish and the replay swap the buffer out, leaving the event in a set nothing would ever look at again. indexing is now re-read under the buffer lock, and the function reports back so the caller handles the event normally when the build ended underneath it. That is what makes the handoff provable: the replay cannot swap until indexing is false, and cannot swap without that lock, so seeing it set while holding the lock proves the swap has not happened.

Any failure to open was treated as proof of ineligibility. reindex_file dropped the indexed entry whenever open_within_root returned an error — including EACCES, a Windows sharing violation, or a descriptor limit, none of which say anything about whether the file belongs in the index. A build holding a file open for a moment was enough to evict live content, and it contradicted the stale path, which deliberately keeps unreadable files and retries them later. Only errors that establish something structural — NotFound, NotADirectory, our own InvalidInput, and unix ELOOP — drop the entry now. Covered by an_unreadable_file_keeps_its_indexed_content.

Recovery could see an ignore file arrive but not one leave. The mtime heuristic finds a .gitignore written during the window; a deleted one leaves nothing to stat, so the matcher kept enforcing rules whose source was gone and the subtree they hid stayed unsubscribed and unindexed until an unrelated rebuild happened along — the more damaging direction of the two. publish_ignore_matcher now records the sources it built from (including root p4ignore.ini, which is a separate walker filter but invalidates the rules the same way), and a scan checks them directly at one stat apiece, once per scan rather than once per file. Covered by a_recovery_scan_notices_an_ignore_file_that_was_deleted.

Windows containment was still check-then-open. Round 6 walked the ancestors with symlink_metadata before opening, which rejects a junction that happens to be there but not one substituted between the check and the open. The file is now opened without following a final reparse point and the handle is asked where it ended up, via GetFinalPathNameByHandleW, compared against the canonicalized root. There is no second lookup to race. The per-ancestor stat walk goes away with it, so this is also one open plus one query instead of depth-many stats.

Control experiment

Reverting all three testable hunks on a scratch branch — proves_ineligible forced true, the replay kind forced to Create, the vanished-source check disabled — failed exactly the three new tests and nothing else, on all three platforms. Restoring them passes.

Review round 8

Three more, all variations on concluding something from evidence that does not support it.

read_dir''s per-entry errors were flattened away, and the directory was then recorded as swept. A name that failed to yield is simply missing from present, so sweep_removed_files read it as a deletion and tombstoned a file it had no reason to believe was gone. This was already handled correctly one level up — a failed listing continues without claiming the directory — and one level down, where an unclassifiable file_type() is inserted into present precisely so absence cannot be concluded. The per-entry case now matches: the directory joins swept only if it enumerated cleanly. Covered by a_directory_that_was_not_fully_enumerated_is_not_swept.

An indexed file replaced in place by a non-file kept its contents searchable. A mv of a fifo, a socket, or a symlink-to-directory over x.rs is not a removal — path.exists() is still true, and inotify may report only the rename destination — so is_remove misses it. It is not a regular file either, so the branch below returned after considering only whether to subscribe to a subtree, and the old x.rs stayed in the index indefinitely. Classification is now explicit: a real directory gets the subtree handling, and everything else drops what was indexed under that path. Covered by a_file_replaced_by_a_fifo_loses_its_indexed_content.

notify registers inotify watches without IN_DONT_FOLLOW. The descriptor therefore lands on whatever the name resolves to at registration time, not on the directory the walk validated earlier, so a checkout or rename in between could leave a descriptor watching an inode outside the root while the registry recorded the in-root name as covered — and, if the name were swapped back, the real directory would be considered subscribed while its events went elsewhere. The registration is now re-checked no-follow and undone on a mismatch, so a poisoned entry is retried by the next sync instead of being trusted.

This narrows the window rather than closing it, and the PR does not claim otherwise: notify takes a path rather than a handle, so a swap reverted before the check cannot be detected through its API. What remains is bounded — missed events on a real directory, which the periodic reconcile picks up — and never misplaced content, since open_within_root establishes containment from the handle it actually reads, whatever a watch descriptor happens to point at.

Control experiment

Reverting both hunks failed exactly one test — a_file_replaced_by_a_fifo_loses_its_indexed_content — and nothing else. That is the honest result, and it corrected a claim: the sweep test exercises sweep_removed_files directly, so it pins the invariant the fix depends on (a directory absent from swept tombstones nothing) rather than the wiring that withholds it. A per-entry readdir failure cannot be induced portably, so that half has no test and the doc comment now says so. The watch-registration re-check is likewise untested: it guards a race that cannot be staged deterministically.

Review round 9

Two more races in the reconcile path, both real.

The sweep deleted on stale evidence. sweep_removed_files acted on a
directory listing taken earlier in the scan, and took neither reindex_lock
nor a second look at the filesystem. A file recreated in between has already
had its create event consumed by the watcher, so deleting it here dropped it
until the next reconcile with nothing left to replay. Each candidate is now
rechecked under reindex_lock -- the lock is what makes the recheck mean
anything, since without it the file could be reindexed between the check and
the delete. The recheck uses symlink_metadata, so a path that came back as a
symlink still stays swept. The trace line now counts what was actually dropped.

The content read was unbounded. The size that qualified a file was stat'd
before its contents were read, and appending in between is what a log or a
build artifact does -- so a file could be pulled into memory whole and indexed
past --max-filesize. Extracted read_within_limit, which reads at most one
byte past the cap and reports TooLarge when that byte is there; reindex_file
then drops what the index holds rather than committing oversized content.

Tests: the_sweep_does_not_delete_a_file_that_came_back and
a_read_stops_one_byte_past_the_cap (which also covers the uncapped, under-cap
and exactly-at-cap paths).

Control (draft PR #116, since closed): with both hunks reverted, exactly
those two tests failed and nothing else. That control also showed
a_file_that_outgrows_the_cap_loses_its_indexed_content still passing -- it
pins the eligibility gate that catches growth between visits, not the new
bound, so it was renamed
a_file_that_outgrew_the_cap_between_visits_loses_its_indexed_content.

Review round 10

Three defects, all valid, plus a fourth the fix for the second one uncovered.

A removal could be undone by an in-flight reindex. The watcher's removal
branches mutated the index without reindex_lock, so a reindex_file already
holding a file's bytes could commit them after the delete -- resurrecting a
file that is gone, with a fresh stamp, so nothing afterwards disagrees and no
further event is coming to correct it. Both branches now take the lock. It stays
the caller's rather than drop_indexed_file's because reindex_file calls in
while already holding it and a Mutex is not reentrant; the three existing
sites moved onto a shared lock_reindex helper.

Ignore-file arrival was detected by mtime alone. git checkout, tar -x
and rsync -a all restore mtimes from what they unpack, so a nested
.gitignore can arrive dated months ago and sail straight past a recency test.
Absence from ignore_sources is the exact question instead -- this file did not
feed the published matcher -- and the mtime window stays for what the source
list cannot answer: an existing source that has just been edited.

Symlinked rule files were invisible. DirEntry::file_type does not follow
links, but the walker collects rule files with Path::is_file, which does. A
symlinked .gitignore therefore contributed rules that both the recovery scan
and the new-subtree descent were blind to.

And the ordering defect the macOS runner then caught. With the first version
of the fix, two of the new tests passed on Linux and failed on macOS: read_dir
promises no ordering, and on macOS .gitignore routinely comes back after its
siblings, so a per-entry check indexes part of a directory under the stale rules
before reaching the file that changes them -- and across the scan, rules in a
later directory arrive after earlier ones were already indexed. Replaced the
per-entry check with changed_ignore_rules_in, which answers for the whole
scan up front by probing for rule files by name. That is how the walker finds
them, so the two agree by construction; it is ordering-independent; and
Path::is_file follows links, which subsumes the symlink fix.

Tests: a_removal_waits_for_an_in_flight_reindex,
an_arriving_ignore_file_with_a_preserved_mtime_still_refreshes_the_matcher,
a_symlinked_ignore_file_is_still_seen_by_a_recovery_scan,
a_scan_checks_every_directory_for_rules_before_indexing_any_file (which puts
the rules in a directory later in the scan than the file, so it pins the
ordering on every platform rather than relying on macOS readdir order).

Control (draft PR #117, since closed): with the lock removed, the
known-source test dropped, is_file swapped for a no-follow check, and the
rules probe moved back inside the per-directory loop, exactly those four tests
failed -- 120 passed, 4 failed.

Review round 11

Three threads, all valid, all the same shape: a pathname was being treated as evidence about contents.

Symlinked rule-file targets were not watched. ignore_files_in collects with Path::is_file, which follows links, so a .gitignore symlinked to shared-rules contributes the target's rules. Editing the target produces an event naming shared-rules — a basename is_ignore_rules_file has no opinion about — and touches nothing whose name it does recognise, so no refresh was scheduled and the matcher stayed stale until the hourly reconcile.

A pathname is not proof of content. A source replaced in place (git checkout, tar -x, a restore from an archive) keeps its path and can carry an mtime that predates the scan window. Known name, untouched clock: neither test in changed_ignore_rules_in fired, and the subtree was indexed under rules that were never read.

Stale doc. publish_ignore_matcher still described a since parameter it no longer takes.

The fix records what the published matcher actually read — ignore_source_stamps, size and mtime per source, keyed by relative path, plus an entry for the target of any symlinked source that is itself under the root — and asks against that instead of against a set of names. The live check now also fires for a path that appears in that map; the recovery scan reports not the file the matcher read on a mismatch. The mtime window stays, because the stamps are taken when the matcher is published, which is after the walk that read the files: a write landing between the two would otherwise be recorded as if it had been read. Targets outside the root cannot be watched at all, so for those the periodic reconcile remains the backstop — now stated rather than implied.

Two tests: a_rule_file_swapped_for_an_older_one_is_not_taken_on_faith (asserts the unmodified case stays quiet first, so the check is not trivially always-on) and an_edit_to_a_symlinked_rule_files_target_schedules_a_refresh.

Control (draft PR #118, since closed): with the two decision points removed and the tests kept, ubuntu reported 124 passed; 2 failed — exactly the two new tests, nothing else.

Review round 12

Two threads, both valid, both a proxy standing in for the thing itself.

The sweep took its candidates from file_stamps alone. A stamp is not what makes a file searchable — the index is. filestamps.json is optional by design, and a missing or unreadable one is tolerated everywhere else, so a seeded index whose stamps could not be read had no sweep candidates at all: every file deleted during the unwatched window kept answering searches until the hourly reconcile. Candidates now come from the reader and the overlay as well. Reader paths already hidden by a tombstone are skipped, because delete_file tombstones unconditionally and counts a mutation, so re-deleting them would make every scan over a directory with deletions look like fresh churn and pull flushes forward. reader_paths would have allocated a copy of every path in the index to answer this, so reader_paths_matching filters in place instead.

The mtime window compared a rounded timestamp against a wall-clock instant. HFS+ and ext3 store whole seconds, FAT-derived filesystems two, so a write that followed the walk can be dated before it. For a source edited between the walk and the publication the recorded stamp matches the file, which leaves the window as the only test — and it was failing by up to two seconds. Widened by MTIME_GRANULARITY at the near end. The far end stays where it is: a future mtime is clock skew, and treating it as an edit is what loops. Over-triggering costs one rewalk that finds nothing, and it is self-limiting, since later scans take later timestamps.

Two tests, both cross-platform: a_rule_file_stamped_a_second_early_is_still_inside_the_window (asserts an hour-old source stays quiet first, so the widening cannot pass by being always-on) and the_sweep_drops_a_deleted_file_that_never_had_a_stamp.

Control: with the two changes removed and the tests kept, 107 passed; 2 failed — exactly the two new tests.

Review round 13

One thread, valid: size and mtime do not establish that a rule file is the one the matcher read. rsync -a and tar -x preserve mtime, and two different sets of rules are easily the same length, so the pair is identical across a replacement — and this path exists precisely for changes made while no event was observable.

ignore_source_stamps now records a hash of each source's bytes and changed_ignore_rules_in compares against that. It is never persisted, so the hash only has to be stable within a run.

On cost: these are rule files, a few hundred bytes each, already in the page cache from the walk that found them. That is a different proposition from hashing indexed content, where the read is the whole repository on every reconcile and a wrong stamp costs one stale file rather than a whole subtree indexed under rules that no longer exist. The per-file FileStamp scheme is left as it is for that reason.

The digest is taken at publication rather than inside the matcher builder because the ignore crate opens these files itself and does not hand back what it read. The mtime window is what covers that residual gap, and is why it stays.

Test: a_rule_file_swapped_for_one_of_the_same_size_and_age_is_still_caught, which swaps seven bytes of rules for seven different ones and restores the mtime with utimes, asserting the size and mtime really are identical before it asks. Control with the digest reverted to metadata: 128 passed; 1 failed — that test alone.

Review round 14

Four threads, all valid. Each one is a check that was asking a narrower question than the thing it was guarding.

Containment was only checked on the last component. is_real_dir refuses a symlink it is pointed at, which says nothing about how the path was reached: root/a/b is a perfectly real directory while a is a symlink to anywhere on the machine. The walker never descends through a, so nothing under it is part of the served tree — but a Create for root/a/b was enough to subscribe to it and enumerate it. open_within_root keeps that from misfiling content, so the cost is watch descriptors and work: on a large linked-in tree, exactly the inotify exhaustion this registration exists to avoid. is_contained_dir now walks down from the served root and requires a real directory at every level, and it is what watch_new_subtree and the post-registration re-check ask. The root itself is not tested — it may legitimately be reached through a link, which is the ordinary case under /var on macOS, and it is the anchor open_within_root already trusts. WatchRegistry carries the root and short-circuits through its own watched set, whose every entry was checked on the way in, so the startup sync still costs one symlink_metadata per directory instead of one per level.

exists is not the question for a vanished rule file. A .gitignore replaced by a directory, a FIFO or a socket still exists, but the walker collects sources with Path::is_file and would no longer take it, and changed_ignore_rules_in skips candidates that are not files — so nothing else could notice either, and the matcher kept enforcing rules from a file that had stopped being one. One word: is_file.

A stamp claimed more than the build could support. The background build publishes stamps from a metadata walk taken after the walk that fed the index, so a file created between the two appeared in the stamps and in no index. Every later check then agreed it was current: reindex_file returns early on a matching stamp, and every automatic caller of the reconcile passes compare_index_membership = false, so it compares stamps alone. The file would have stayed unsearchable until something changed it again. stamps_for_index_members now publishes stamps only for paths the reader or the overlay actually holds; anything else is left unstamped and is picked up as new by the next event or scan.

The digest was taken after the matcher had already read. GitignoreBuilder::add opens each source itself, inside the build, and hands nothing back — so digests taken at publication describe whatever is on disk when the build finishes. An mtime-preserving atomic replace in that window left the matcher on the old rules while the stamps recorded the new bytes, and pathname, timestamp and digest then all agreed there was nothing to reread. publish_ignore_matcher now takes the build as a closure and digests the sources on both sides of it. A mismatch still publishes — no matcher at all means indexing ignored paths — but marks the matcher stale and schedules a refresh. It converges: a filesystem that has stopped moving produces matching digests on the next pass.

Four tests: a_directory_below_a_symlinked_one_is_not_subscribed_to (unix), a_rule_file_replaced_by_a_directory_counts_as_gone, stamps_are_published_only_for_what_the_build_indexed, and a_rule_file_rewritten_during_the_build_marks_the_matcher_stale — the last asserting that an unraced build stays quiet first, so it cannot pass by marking everything stale.

Control (draft PR #121, since closed): with the four decision points reverted and the tests kept, macOS reported 127 passed; 4 failed — exactly the four new tests, nothing else.

Review round 15

Three more threads, all three real.

A whole directory that vanishes takes nothing with it (High). The recovery
sweep decided a file was gone from its immediate parent's listing. When a
directory is deleted or moved away entirely, its descendants have no enumerated
parent, so none of them were ever candidates — and no event names them either:
a removal delivers one event for the directory, and a move away delivers
nothing at all for what was inside it. Those files answered searches until the
hourly reconcile. reindex_files_in now tracks the directories it saw present
and the ones whose listing failed, derives the vanished ones from a parent
listing that did succeed, and sweep_removed_files walks ancestors so a file
under a vanished directory at any depth is swept. The ancestor walk only runs
when something actually vanished, since that closure runs once per indexed path
in the repository.

Subscriptions were established in hash order (Medium). Round 14 justified
WatchRegistry::contained's fast path by claiming sync feeds directories
parent-first. It does not — desired is a HashSet. Unordered, a child
usually arrives before its parent, gets no watched parent to lean on, and walks
every ancestor with a symlink_metadata per level. On a 40k-directory monorepo
that is hundreds of thousands of syscalls in the path that exists to make
startup cheap. sync now sorts by component count before subscribing; a parent
is always strictly shallower than its children, so depth order is enough.

A build stamped files it had not indexed (Medium). The stamp map comes from
a metadata walk taken after the content walk that fed the index. A file
written between the two is stamped with the new size and mtime while the index
holds the old bytes — and that claim is load-bearing in exactly the place that
should have repaired it, since reindex_file returns early on a matching stamp
and the reconcile behind it compares the same stamps. The replay of the very
event that reported the write read nothing, and the old content stayed
searchable indefinitely. The deferred-event buffer already names those paths, so
their stamps are withheld at publication (both in background_index_build and
in the bootstrap path, which has the same race). When that buffer has
overflowed it names nothing and no file can be told apart from what changed, so
no stamp from that build is published at all and the reconcile overflow already
schedules re-reads the tree instead.

Three tests, all cross-platform:
a_directory_that_went_away_whole_takes_its_files_with_it,
subscriptions_are_established_from_the_root_down,
a_file_written_during_the_build_is_not_stamped_by_it.

Control: with only the three decision points reverted and the tests kept,
exactly those three fail (112 passed / 3 failed) and nothing else moves.

Review round 16

One thread, and it is the same class of defect round 14 fixed for
subscriptions: symlink_metadata refuses to follow only the final
component. The sweep used it to ask "is this path back on disk?" before
dropping a candidate, so a directory that vanished and returned as a link to
another tree made root/gone-dir/a.rs resolve to an ordinary file outside the
root. The recheck read that as a return, kept the stale in-root entry, and
nothing ever corrected it — a linked-in tree is not walked and not watched, so
no descendant event exists.

The recheck now goes through open_within_root, the same contract
reindex_file opens under, which resolves every ancestor without following a
link (race-free on unix via openat). Errors are classified with the existing
proves_ineligible, so "gone, escaped, or not a directory on the way" sweeps
while a descriptor limit or a sharing violation preserves the entry for the
next reconcile rather than evicting live content — matching how reindex_file
already treats the same failures.

Test: a_path_that_returns_through_a_symlinked_ancestor_is_still_swept.
Control (draft PR, since closed): with only the recheck reverted, Linux CI
reports 136 passed; 1 failed — that test and nothing else.

Review round 17

Five claims, at head e585f37. All five reproduce. Fixed in c07eb81.

1. High — an ignore refresh can race a resumed build

background_index_build publishes its matcher part-way through Phase 2 and
holds no gate while doing it (bootstrap_index_build does, which is why only
the resumed path is affected). When that publish sees changed rules it schedules
a refresh, and the refresh thread took snapshot_gate.write() uncontended and
replaced file_stamps wholesale from its own walk — after which the build
overwrote them from a walk that predates the new rules. The result is an index
and a stamp map describing two different trees, with no scan left to notice,
because both sides believe they finished.

spawn_recovery_scan already waits out indexing for exactly this reason. The
refresh worker now does the same, ahead of background_refresh_stale, so the
guard covers every caller. A wait rather than a lock: it cannot deadlock against
the build, and nothing is lost by waiting, because the build is still walking
the tree the refresh would walk.

Test: an_ignore_refresh_waits_for_a_running_build.

2. High — the watcher omitted git's case-insensitivity narrowing

Both walks apply git_ignorecase_filter as a filter_entry (walker.rs:257,
walker.rs:498), which hides trees that a core.ignorecase repository treats
as matching an ignore rule under a different case. IgnoreMatcher had no such
field and is_ignored never consulted it, so should_skip_watcher_entry and
watchable_dirs admitted exactly the trees the walk excluded: the watcher
subscribed to them, indexed their files, and the next stale check evicted them
again. Reproduced on Windows.

IgnoreMatcher now carries the same CaseInsensitiveIgnore, applied where the
walk applies it — last, and to whitelisted paths too, because a filter_entry
rejection is not undone by a whitelist rule. It is a pure narrowing, so it can
only ever add exclusions. serve exposes none of the search-time
--no-ignore-* flags, so the watcher's matcher is built with the full flag set.

Tests: the_point_query_matcher_hides_exactly_what_the_walk_hides, and
the_point_query_matcher_follows_the_case_sensitivity_gate guarding the other
direction — a case-sensitive repository must have nothing hidden from it.

3. High — overflow could leave dead watches recorded as live

Two halves. A native drop — IN_Q_OVERFLOW, a lost ReadDirectoryChangesW
buffer — arrived on notify's error branch, which only logged; just the channel's
TrySendError::Full set overflowed, so the loss that most needs a reconcile
triggered none at all. And after any overflow the dropped removal events left
watched recording descriptors the kernel had already released, which every
later sync skipped as already present: a directory removed and recreated
during the gap stayed permanently unsubscribed.

The error branch now reconciles too, and both branches set a flag that one
following sync consumes to re-issue subscriptions once, through the existing
resubscribe_all — whose doc already described this poisoning, but which
nothing on the reconcile path called.

Test: a_forced_sync_retires_a_subscription_the_kernel_already_dropped.

4. Medium — populated directories moved in stayed unindexed off Linux

watch_new_subtree both subscribes and enumerates, but its call site in
handle_fs_event was gated on PER_DIRECTORY_WATCHES. A recursive backend
reports a moved-in tree as a single event for the directory and never describes
the contents, so on Windows and macOS a mv of a populated tree from outside
the root — a checkout, an unpacked archive — left every file unindexed until the
hourly reconcile. Reproduced on Windows.

The enumeration now runs everywhere; only the subscribing half stays behind
PER_DIRECTORY_WATCHES, where it belongs.

Test: a_populated_directory_that_arrives_whole_is_indexed_on_every_platform.

5. Medium — ignore-source tracking had gaps, and mis-resolved the exclude

ignore_sources_of listed only the walk's .gitignore/.ignore files and the
root p4ignore.ini, omitting the ancestor rule files and the repository exclude
that the published matcher does enforce — so editing one changed what the
watcher filtered with no digest change to notice it.

Separately, both the matcher builder and CaseInsensitiveIgnore::new resolved
the exclude at a literal .git/info/exclude. In a linked worktree or a
submodule .git is a file holding a gitdir: pointer, and that directory holds
a commondir naming the repository every worktree shares — which is where the
one info/exclude lives. WalkBuilder follows that chain internally, so the
walk honored the exclude and the watcher did not, in exactly the layouts where
the two differ.

Both are fixed: repo_exclude_path follows gitdir → commondir → info/exclude
the way the walk does, and ignore_sources_of tracks the ancestors and the
exclude alongside everything else. Both helpers filter on is_file(), because
reindex_files_in treats a listed source that does not exist as vanished and
would otherwise re-refresh forever.

Deliberately still untracked: the user's global gitignore. The ignore crate
resolves it through git config precedence and does not expose which path it
chose, so guessing would report a vanished source on every scan. The reasoning
is in the ignore_sources_of doc comment.

Tests: ignore_sources_include_the_rules_that_live_outside_the_tree,
the_repository_exclude_is_found_through_a_worktree_pointer,
a_relative_commondir_resolves_against_the_worktree_git_dir.

Control

All eight tests are cross-platform, so the control ran locally. Reverting only
the six decision points and keeping the tests: 115 passed; 4 failed in
tgrep-cli and 207 passed; 3 failed in tgrep-core — those seven and nothing
else, every other suite still green. the_point_query_matcher_follows_the_case_sensitivity_gate
passes under the control by construction, as a guard against over-application
should. Restored: 119 / 210 / 101 / 211, all green.

Review round 18

Three claims, at head e585f37. All three reproduce. Fixed in a784a01.

1. High — a transient stat failure was read as a deletion

Path::exists and Path::is_file fold every metadata error into false, so a
file held open by a build, a Windows sharing violation, or a momentary EACCES
read as "gone" at one site and "no longer a regular file" at the other. Both
branches then evicted content that was still valid. reindex_file deliberately
preserves entries through exactly those failures, but it never got the chance:
the drop happens earlier in handle_fs_event, before it is ever called.

Rather than swap in try_exists at two sites and let them drift again, the
policy is now explicit. One stat feeds classify_event_target, which answers
through the same proves_ineligible contract the recovery sweep uses:
NotFound, ELOOP, NotADirectory and InvalidInput are a removal, anything
else is Unknown and concludes nothing. Unknown leaves the index alone and
lets the stale path retry, which is what it already does for unreadable files.

Test: an_unreadable_path_is_not_treated_as_a_deletion.

2. High — the tracked-file exemption froze

A regression from round 17. CaseInsensitiveIgnore caches the tracked-file set
behind a OnceLock, which was sound while it was a walk-local object built,
used and dropped inside one traversal. Retaining it in the watcher's long-lived
matcher changed its lifetime without changing its freshness model, and there a
git add -f or a git rm --cached rewrites only .git/index — hidden, so no
ignore source changes and nothing republishes. Both directions hurt: a newly
tracked file stays hidden from the watcher, and an untracked-again one keeps
being indexed and then evicted by the next stale check.

Now an RwLock<TrackedCache> keyed on the index's identity — (mtime, len) of
the file git_dir() resolves to, the same pair git's own racy-index handling
relies on, and git installs a new index by renaming index.lock over it so
every rewrite lands as a new mtime. The common path stays a read lock, and the
laziness is preserved: the identity stat only runs for paths the
case-insensitive rules have already claimed, which for most repositories is
none.

Test: the_tracked_exemption_reloads_when_the_git_index_changes.

3. Medium — a rule file symlinked into a hidden directory was unobservable

handle_fs_event recognises an event naming a symlinked source's target
rather than a name rules usually go by, and asserted in a comment that such
targets are watched "because only those are watched". On a per-directory backend
they were not: nothing subscribes to build/ when the rules hide it, so editing
the file the matcher was built from produced no event at all and the matcher
stayed stale until the hourly reconcile.

Windows and macOS were already correct — the subtree watch is native and
recursive, the event arrives, and the stamps lookup runs before
should_skip_watcher_path, so the rules hiding the directory do not suppress
it. The hole was Linux-only.

ignore_target_dirs now resolves each symlinked source to its in-root target
and adds that file's own directory — not its subtree — to the desired set.
watchable_dirs is untouched, so nothing descends into the ignored tree;
should_skip_watcher_path still discards everything else delivered from there;
and the registry's contained check already falls back to a full containment
test when a parent is not watched, so no ancestor chain is required. Normally
the set is empty and this costs nothing.

Not taken: dropping the target entry instead. That would break the case where it
does work — a target that is itself a watched .gitignore/.ignore — without
making the matcher any fresher. Targets outside the root remain deliberately
uncovered, since subscribing there means watching outside the tree the server
was asked to serve.

Tests: a_rule_file_symlinked_into_a_hidden_directory_is_still_watched, and
ordinary_and_outside_rule_files_add_no_subscriptions guarding against
over-subscribing.

Control

All four tests are cross-platform, so the control ran locally. Reverting only
the three decision points and keeping the tests: 120 passed; 2 failed in
tgrep-cli and 210 passed; 1 failed in tgrep-core — those three and nothing
else, every other suite still green. The over-subscription guard passes under
the control, as it should. Restored: 122 / 211 / 101 / 211, all green.

[Copilot]

Pull request overview

Adds live .ignore change detection so the watcher refreshes its matcher and reconciles indexed files.

Changes:

• Recognizes root and nested .ignore files as ignore-rule sources.
• Adds unit and end-to-end regression coverage.
• Extracts shared watcher test helpers.

Show a summary per file

File	Description
tgrep-cli/src/serve.rs	Detects .ignore changes and tests classification.
tgrep-cli/tests/watcher_dot_ignore.rs	Verifies live matcher refresh and index reconciliation.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

tgrep-cli/src/serve.rs:1949

• This union treats every path in registry.watched as still subscribed, but inotify automatically removes a watch when a directory is deleted and the remove path never removes it from this set. If that directory is recreated before a full sync, it is already present in the union, so sync does not re-register it; this pass indexes files currently inside it, but later writes are invisible. Remove deleted directories (and descendants) from the registry on removal, or force a newly created subtree root to be re-watched.

        let union: std::collections::HashSet<PathBuf> =
            registry.watched.union(&desired).cloned().collect();
        registry.sync(&union);

tgrep-cli/src/serve.rs:1968

• A pre-populated directory moved into the watched tree can contain a nested .ignore or .gitignore, but adding an inotify watch does not emit events for existing files. This recovery scan classifies that dotfile as skipped and never marks the ignore rules dirty, then indexes its siblings using the old matcher. Detect ignore-rule files during the scan and schedule the same matcher refresh used by normal watcher events so nested rules take effect.

            let rel = rel.to_string_lossy().replace('\\', "/");
            let skip = {
                let gitignore = state.gitignore.read().unwrap();
                should_skip_watcher_path(&rel, &state.exclude_dirs, gitignore.as_ref())
            };

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

tgrep-cli/src/serve.rs:1791

• When a watched directory is deleted, inotify drops its descriptor but the path remains in watched. If that path is later recreated, this check treats the stale entry as an active subscription and skips watch(), so subsequent changes under the recreated directory are lost. Remove the deleted/moved path and its descendants from the registry (best-effort unwatching them) when processing removal or move-out events.

            if self.watched.contains(dir) {
                continue;
            }

tgrep-cli/src/serve.rs:2125

• This branch is reached for every accepted Modify event whose path is a directory, not just for a directory that appeared. A metadata change such as chmod on a large directory therefore recursively walks the entire subtree and stats every file; repeated directory metadata events can turn ordinary watcher activity into repeated O(tree) scans. Restrict subtree discovery to folder-create and rename-to events.

            if PER_DIRECTORY_WATCHES && path.is_dir() {

tgrep-cli/src/serve.rs:1870

• This enumerates the complete tree before any descendant subscriptions are installed. With non-recursive inotify, a directory created after its parent was enumerated but before add_all watches that parent produces no observed event and is absent from desired, leaving it unwatched until the hourly reconcile. Subscribe to each directory before enumerating it, or perform a post-subscription directory reconciliation that also adds newly discovered directories.

    let mut stack = vec![start.to_path_buf()];
    while let Some(dir) = stack.pop() {
        let Ok(entries) = std::fs::read_dir(&dir) else {

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

tgrep-cli/src/serve.rs:1790

• watched can contain stale entries after a directory is deleted: inotify drops the kernel watch automatically, but remove events never prune this set. If that path is recreated before a full sync observes it absent, add_all skips it here even though there is no OS subscription, so later files under the recreated directory are missed. Remove the deleted path and its descendants from registry bookkeeping on removal, or otherwise force re-registration for recreated directories.

            if self.watched.contains(dir) {
                continue;

tgrep-cli/tests/watcher_watch_registration.rs:200

• This does not actually wait for descendant subscriptions: normal_source_marker was already persisted by the initial index command, so it is searchable even while gitignore_pending is true. The subsequently added file can also be picked up by the startup stale reconciliation rather than by an inotify event, allowing an implementation that watches only the root to satisfy both the positive control and the low watch-count assertion. Wait until the expected non-ignored directory watches are present (or expose an explicit matcher/subscription-ready signal) before creating the control file.

    // Positive control, and a wait for the watcher to be live: subscriptions
    // are deferred until the ignore matcher is published, so counting before
    // that would pass for the wrong reason.
    assert!(
        wait_for_match(port, "normal_source_marker", Duration::from_secs(30)),
        "expected the seeded source files to be searchable"
    );

tgrep-cli/src/serve.rs:2125

• Path::is_dir() follows symlinks, so a directory symlink created at runtime enters watch_new_subtree. Because that helper always includes and reads its start path, it subscribes through the link and indexes its target, contradicting the walker’s default follow_links = false behavior and the watchable_dirs contract. Check symlink_metadata(path).file_type().is_dir() so only actual directories are added.

            if PER_DIRECTORY_WATCHES && path.is_dir() {

tgrep-cli/src/serve.rs:1985

• This recovery scan does not recognize ignore-rule files. If a newly created subtree is populated with a nested .ignore/.gitignore before its parent creation event is handled, the ignore-file event predates the new subscription; this code then skips that dotfile using the old matcher and indexes sibling content without ever scheduling a matcher refresh. Detect discovered ignore sources and trigger a stale refresh before accepting the subtree's files.

            let skip = {
                let gitignore = state.gitignore.read().unwrap();
                should_skip_watcher_path(&rel, &state.exclude_dirs, gitignore.as_ref())
            };

tgrep-cli/src/serve.rs:1916

• The initial subscription sync has a race: it finishes the full directory walk before registering any descendant. On the deferred startup path, the stale walk has already completed, so a file or directory created after its parent was scanned but before registry.sync subscribes is neither in that snapshot nor delivered by inotify; it can remain unindexed/unwatched until the hourly reconcile. Register directories while traversing, or run a post-registration metadata reconciliation after all watches are active.

        let gitignore = state.gitignore.read().unwrap();
        watchable_dirs(root, root, &state.exclude_dirs, gitignore.as_ref())
    };
    let total = desired.len();
    let (added, removed) = registry.sync(&desired);

• Files reviewed: 4/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

tgrep-cli/src/serve.rs:1585

• publish_ignore_matcher can run before this registry is installed because run spawns the stale/build thread first. In that ordering the publish reports no newly watched directories; this branch then adds all descendants but discards them. A write after the matcher-producing walk passed a directory and before this sync is therefore in neither the walk nor an event, and the bootstrap path returns without another stale check. Start the watcher before spawning the producer, or perform the same recovery scan/reconcile for the directories added here.

        // The newly watched directories are deliberately not rechecked here.
        // This is every directory in the repository, the index was built or
        // opened moments ago, and the stale check that follows startup
        // reconciles the same drift while holding `snapshot_gate` — which this
        // path does not hold and must not take, since the watcher has to be
        // receiving events before that check runs.
        let _ = sync_watch_registrations(&state, root);

tgrep-cli/src/serve.rs:1993

• This recovery pass silently skips an ignore-rules file because dot-prefixed paths satisfy should_skip_watcher_path. If a newly visible directory receives .ignore/.gitignore after the stale walk visited it but before its new subscription is installed, this is the only recovery scan that can see that write; the matcher remains stale and neighboring files may be indexed under the wrong rules. Detect is_ignore_rules_file here and schedule a refresh before indexing these directories, as watch_new_subtree already does for the equivalent runtime-subtree race.

            let skip = {
                let gitignore = state.gitignore.read().unwrap();
                should_skip_watcher_path(&rel, &state.exclude_dirs, gitignore.as_ref())
            };
            if !skip {
                reindex_file(state, &path, &rel);
            }

• Files reviewed: 4/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

tgrep-cli/src/serve.rs:2181

• The loop does not react to found_ignore_rules until after traversing the entire subtree. A moved-in tree with a root .gitignore excluding a large node_modules/ therefore subscribes to every descendant under stale rules before refreshing, potentially exhausting inotify—the failure this change is intended to prevent. Schedule the refresh and return after the current level is enumerated, before descending into next.

        level = next;

tgrep-cli/src/serve.rs:3194

• This discards the recovery set on the assumption that a startup stale check follows, but background_index_build returns immediately when bootstrap_index_build succeeds; no such check runs. A file written after the builder's walk but before these descendant subscriptions is therefore neither in the built index nor observable as an event. Preserve and scan the newly watched directories after loading stamps, or run an immediate stale reconciliation before declaring indexing complete.

        let _ = publish_ignore_matcher(state, root, matcher);

tgrep-cli/src/serve.rs:3291

• No stale check follows this publish either: execution indexes the original walk.files and later replaces file_stamps from a metadata walk. A file created in a newly subscribed directory after the path walk can be absent from new_files while that later metadata walk records it as current; periodic reconciliation does not compare index membership, so it can remain unindexed indefinitely. Carry the returned directories to a recovery scan after final stamp publication, or perform an immediate membership-aware stale check.

        let _ = publish_ignore_matcher(state, root, matcher);

tgrep-cli/src/serve.rs:2184

• Files discovered in a newly arrived subtree are sent directly to reindex_file, which does not enforce state.max_file_size or the binary-extension filter used by both indexing walks. A populated subtree can thus add files that a fresh index would exclude, making the live index diverge immediately. Apply the same file eligibility checks before collecting/reindexing these files, preferably through the shared reindex helper.

    for (path, rel) in &files {
        reindex_file(state, path, rel);

• Files reviewed: 4/5 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: 3 · 1

New issues introduced by this change (3)

Severity	Finding
	tgrep-cli/​src/​serve.rs — symlink_metadata only refuses a symlink in the final component; it still follows symlinked…
	tgrep-cli/​src/​serve.rs — Existence is not enough to prove this is still an ignore-rule source. If a previously read…
	tgrep-cli/​src/​serve.rs — A matching stamp does not prove this path is actually indexed. The background build derives content…

Pre-existing issues (1)

Severity	Finding
	tgrep-cli/​src/​serve.rs — Size plus mtime does not establish that this is the content the matcher read. An archive/rsync… View comment

Issues resolved since last review (2)

Severity	Finding
	tgrep-cli/​src/​serve.rs — The removal candidates come only from file_stamps, but the active reader/overlay can contain… View resolved comment
	tgrep-cli/​src/​serve.rs — This time comparison misses edits on filesystems with coarse mtimes. since has subsecond… View resolved comment

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: 4 · 1

New issues introduced by this change (1)

Severity	Finding
	tgrep-cli/​src/​serve.rs — The digest is captured after the matcher has already read each source. If an unwatched nested rule…

Pre-existing issues (4)

Severity	Finding
	tgrep-cli/​src/​serve.rs — A matching stamp does not prove this path is actually indexed. The background build derives content… View comment
	tgrep-cli/​src/​serve.rs — symlink_metadata only refuses a symlink in the final component; it still follows symlinked… View comment
	tgrep-cli/​src/​serve.rs — Size plus mtime does not establish that this is the content the matcher read. An archive/rsync… View comment
	tgrep-cli/​src/​serve.rs — Existence is not enough to prove this is still an ignore-rule source. If a previously read… View comment

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: 4 · 1

New issues introduced by this change (1)

Severity	Finding
	tgrep-cli/​src/​serve.rs — The sweep only considers files whose immediate parent was enumerated. If a whole directory d…

Pre-existing issues (4)

Severity	Finding
	tgrep-cli/​src/​serve.rs — The digest is captured after the matcher has already read each source. If an unwatched nested rule… View comment
	tgrep-cli/​src/​serve.rs — A matching stamp does not prove this path is actually indexed. The background build derives content… View comment
	tgrep-cli/​src/​serve.rs — symlink_metadata only refuses a symlink in the final component; it still follows symlinked… View comment
	tgrep-cli/​src/​serve.rs — Existence is not enough to prove this is still an ignore-rule source. If a previously read… View comment

Issues resolved since last review (1)

Severity	Finding
	tgrep-cli/​src/​serve.rs — Size plus mtime does not establish that this is the content the matcher read. An archive/rsync… View resolved comment

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: 2 · 1

New issues introduced by this change (2)

Severity	Finding
	tgrep-cli/​src/​serve.rs — This publishes a post-build metadata stamp even for a path whose watcher event was deferred during…
	tgrep-cli/​src/​serve.rs — desired is a HashSet, so this iteration is not parent-first even though contained relies on…

Pre-existing issues (1)

Severity	Finding
	tgrep-cli/​src/​serve.rs — The sweep only considers files whose immediate parent was enumerated. If a whole directory d… View comment

Issues resolved since last review (4)

Severity	Finding
	tgrep-cli/​src/​serve.rs — The digest is captured after the matcher has already read each source. If an unwatched nested rule… View resolved comment
	tgrep-cli/​src/​serve.rs — A matching stamp does not prove this path is actually indexed. The background build derives content… View resolved comment
	tgrep-cli/​src/​serve.rs — Existence is not enough to prove this is still an ignore-rule source. If a previously read… View resolved comment
	tgrep-cli/​src/​serve.rs — symlink_metadata only refuses a symlink in the final component; it still follows symlinked… View resolved comment

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	tgrep-cli/​src/​serve.rs — This no-follow check protects only the final component. If a vanished directory is replaced by a…

Issues resolved since last review (7)

Severity	Finding
	tgrep-cli/​src/​serve.rs — desired is a HashSet, so this iteration is not parent-first even though contained relies on… View resolved comment
	tgrep-cli/​src/​serve.rs — This publishes a post-build metadata stamp even for a path whose watcher event was deferred during… View resolved comment
	tgrep-cli/​src/​serve.rs — The sweep only considers files whose immediate parent was enumerated. If a whole directory d… View resolved comment
	tgrep-cli/​src/​serve.rs — The digest is captured after the matcher has already read each source. If an unwatched nested rule… View resolved comment
	tgrep-cli/​src/​serve.rs — A matching stamp does not prove this path is actually indexed. The background build derives content… View resolved comment
	tgrep-cli/​src/​serve.rs — Existence is not enough to prove this is still an ignore-rule source. If a previously read… View resolved comment
	tgrep-cli/​src/​serve.rs — symlink_metadata only refuses a symlink in the final component; it still follows symlinked… View resolved comment

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: None

Issues resolved since last review (1)

Severity	Finding
	tgrep-cli/​src/​serve.rs — This no-follow check protects only the final component. If a vanished directory is replaced by a… View resolved comment

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: 1 · 2

New issues introduced by this change (3)

Severity	Finding
	tgrep-cli/​src/​serve.rs — Path::exists() maps metadata errors to false, so a transient EACCES or sharing error is…
	tgrep-core/​src/​gitignore.rs — CaseInsensitiveIgnore is now retained inside the long-lived watcher matcher, but its OnceLock…
	tgrep-cli/​src/​serve.rs — Recording an in-root symlink target does not guarantee that its edits are observable. For example,…

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: None

Issues resolved since last review (3)

Severity	Finding
	tgrep-cli/​src/​serve.rs — Recording an in-root symlink target does not guarantee that its edits are observable. For example,… View resolved comment
	tgrep-core/​src/​gitignore.rs — CaseInsensitiveIgnore is now retained inside the long-lived watcher matcher, but its OnceLock… View resolved comment
	tgrep-cli/​src/​serve.rs — Path::exists() maps metadata errors to false, so a transient EACCES or sharing error is… View resolved comment

Suppressed comments (1)

tgrep-core/src/gitignore.rs:563

• Reloading the tracked-file cache on demand does not make git add -f or git rm --cached live. Those operations modify only .git/index, which the watcher filters (and does not subscribe to on Linux), so no call to excludes occurs for the affected source path and the active index remains stale until periodic reconciliation. The watcher must observe Git index identity changes and schedule a refresh/reindex; the point-query unit test only proves that a later query would reload the cache.

        let identity = index_identity(&self.repo_root);
        {
            let cache = self.tracked.read().unwrap();
            if cache.loaded_from.as_ref() == Some(&identity) {
                return Self::hides(cache.tracked.as_ref(), relative, is_dir);