fix(watch): stop a filesystem watch feeding itself on Linux (#523)

On Linux, an idle window with a repository open ran `git status -uall` about
2.6 times a second, forever. `notify`'s inotify backend subscribes with
`WatchMask::OPEN`, so every `open(2)` under a watched directory is an event —
and `git status` opens `.git/index`, `.git/HEAD` and `refs/heads/*`, all of
which the source control watch covers. The read that answers "did this
repository change" was itself an event saying it may have changed, so each
answer scheduled the next question. Measured on the Linux runner: 78 debounce
bursts in 30 seconds, from one real change.

`Debounce`'s doc argued this was impossible because `GIT_OPTIONAL_LOCKS=0`
stops `git status` writing the index back. That covers writes; `IN_OPEN` fires
on reads. macOS FSEvents has no equivalent, which is why it was invisible on
the machine it was written on.

`is_content_change` drops `Access(Open | Read | Close(Read))` and keeps
`Access(Close(Write))`, at both the host watch and the config hot-reload watch.
Guarded by a host conformance case rather than a platform test, verified red on
the Linux runner with the filter removed.

Also here, and how the above was found: the `render_idle` tests counted frames
across a window they advance a virtual clock over, while the pane runs its own
git pipeline off a 300ms timer on that same clock that nothing advanced during
settle — so the measurement set off the pane's first `git` run and raced its
landing. `test_window::quiesce` settles both clocks, and the kernel's, before
counting. That turned #523 from a 1-in-50 flake into a deterministic failure,
which is what made the watch loop findable.

Third: `ssh_config::an_alias_added_to_an_included_file_is_seen_without_touching_the_root`
(from #528, already on main) failed on every Windows run of this branch. The
cache keys on mtime and the test writes twice inside one ~15ms Windows clock
tick, so the cache was right to say nothing changed. Test-only; the poll behind
it runs at 4Hz and real edits arrive at human speed.

Closes #523.

CHANGELOG.md

@@ -124,6 +124,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **An idle window stops re-reading the repository on Linux** — with a
+  repository open, tty7 ran `git status -uall` about two and a half times a
+  second, forever, with nothing on screen changing and nobody touching the
+  machine. The filesystem watch was feeding itself: Linux reports opening a
+  file as a watch event, every read of `.git` is an open, and reading `.git`
+  is exactly how the watch's own events were answered — so each answer
+  scheduled the next question. Reads are no longer mistaken for changes;
+  a write finishing still is. macOS and Windows were never affected, which is
+  why this survived so long.
+
 - **An alias put back in an `Include`d ssh config file is found again** — the
   check behind a parked remote workspace watched `~/.ssh/config` for changes
   and nothing else, but `Include` is common and editing an included file

crates/tty7-core/src/host/conformance.rs

@@ -66,6 +66,7 @@ macro_rules! for_each_host_case {
             search_respects_max_dirs,
             shells_are_named_and_have_a_default,
             watch_reports_create_and_delete,
+            watch_ignores_reads,
             watch_is_non_recursive,
             watch_set_dirs_adds_and_drops,
             watch_coalesces_within_window,
@@ -946,6 +947,47 @@ pub fn watch_reports_create_and_delete(h: &dyn Host, sb: &dyn Sandbox) {
     );
 }
 
+/// Reading a watched file is not a change.
+///
+/// The one case where a watch can feed itself: every consumer answers an event
+/// by reading the directory it came from, so if a read is an event, the answer
+/// asks the question again. `notify`'s inotify backend subscribes to `IN_OPEN`,
+/// which makes this reachable on Linux and only there — an idle Source Control
+/// panel ran `git status -uall` about 2.6 times a second before this was
+/// filtered (issue #523). macOS passes this trivially, which is exactly why it
+/// has to be a conformance case rather than a platform test.
+pub fn watch_ignores_reads(h: &dyn Host, sb: &dyn Sandbox) {
+    let sandbox = sb.path();
+    let f = h.join(sandbox, "read-me.txt");
+    write(h, &f, "hello");
+
+    let sub = h.watch(&[sandbox.to_path_buf()]).unwrap();
+    // The file this asserts about is created *before* the watch, so the drain
+    // has to come after the tail of that create can still arrive — FSEvents
+    // will hand one over a beat late, and a stale create is indistinguishable
+    // here from the reads below reporting.
+    std::thread::sleep(WATCH_QUIET);
+    drain(&sub);
+
+    for _ in 0..5 {
+        h.read_file(&f, 1 << 20)
+            .expect("the watched file reads back");
+    }
+    std::thread::sleep(WATCH_QUIET);
+
+    let batches = collect_batches(&sub, Duration::from_millis(200));
+    let leaked: Vec<&PathBuf> = batches
+        .iter()
+        .flatten()
+        .filter(|p| p.file_name().is_some_and(|n| n == "read-me.txt"))
+        .collect();
+    assert!(
+        leaked.is_empty(),
+        "reading a watched file reported as a change, so a watch can feed \
+         itself: {leaked:?}"
+    );
+}
+
 pub fn watch_is_non_recursive(h: &dyn Host, sb: &dyn Sandbox) {
     let sandbox = sb.path();
     let sub_dir = h.join(sandbox, "child");

crates/tty7-core/src/host/local.rs

@@ -381,6 +381,7 @@ fn local_watch(dirs: &[PathBuf], gitignore: Arc<Mutex<GitignoreChain>>) -> io::R
     let watcher = notify::recommended_watcher(move |res: notify::Result<notify::Event>| {
         if let Ok(ev) = res
             && !ev.paths.is_empty()
+            && crate::host::is_content_change(&ev.kind)
         {
             let _ = raw_tx.send(ev.paths);
         }

crates/tty7-core/src/host/mod.rs

@@ -262,6 +262,32 @@ pub trait Host: Send + Sync + 'static {
     }
 }
 
+/// Did this watch event mean something *changed*, or only that something was
+/// read?
+///
+/// The distinction does not exist on macOS, and on Linux it decides whether a
+/// watch can feed itself. `notify`'s inotify backend asks for `IN_OPEN`, so
+/// every `open(2)` under a watched directory arrives as an event — and the one
+/// thing every consumer of a watch does with an event is go and read the
+/// directory it came from. Reading `.git/index` to answer "did the repository
+/// change" is itself an event saying the repository may have changed, so the
+/// answer schedules the next question, at whatever rate the debounce allows,
+/// forever. Measured at ~2.6 `git status -uall` per second on an idle Source
+/// Control panel before this filter existed (issue #523).
+///
+/// `IN_CLOSE_WRITE` also arrives as `Access`, and that one is a real write
+/// finishing, so it stays. Everything else under `Access` is a reader.
+pub fn is_content_change(kind: &notify::EventKind) -> bool {
+    use notify::event::{AccessKind, AccessMode};
+
+    !matches!(
+        kind,
+        notify::EventKind::Access(
+            AccessKind::Open(_) | AccessKind::Read | AccessKind::Close(AccessMode::Read),
+        )
+    )
+}
+
 pub fn default_join(dir: &Path, name: &str, sep: char) -> PathBuf {
     let mut s = dir.to_string_lossy().into_owned();
     if !s.is_empty() && !s.ends_with(sep) && !s.ends_with('/') {

src/core/ssh_config.rs

@@ -1423,6 +1423,7 @@ mod tests {
         );
 
         std::fs::write(ssh.join("conf.d/work"), "Host work\n").unwrap();
+        edited_just_now(&ssh.join("conf.d/work"));
         assert!(
             cache.resolves(&cfg, &root, "work"),
             "an alias put back in an included file is found again, \
@@ -1430,6 +1431,21 @@ mod tests {
         );
     }
 
+    /// Make a write that has just happened look like one, on a filesystem
+    /// whose clock is coarser than this test is fast.
+    ///
+    /// Windows stamps files from a clock that ticks about every 15ms, so two
+    /// writes as close together as the ones above can share an mtime — and a
+    /// cache keyed on mtime is then right to say nothing changed. A real edit
+    /// arrives at human speed, and the poll behind this cache runs at 4Hz, so
+    /// the collision is an artefact of the test rather than something a user
+    /// can reach. Stated by hand instead of slept out.
+    fn edited_just_now(path: &Path) {
+        let f = std::fs::File::options().write(true).open(path).unwrap();
+        let ahead = std::time::SystemTime::now() + std::time::Duration::from_secs(1);
+        f.set_modified(ahead).unwrap();
+    }
+
     fn temp_root(name: &str) -> PathBuf {
         let dir = std::env::temp_dir().join(format!(
             "tty7-ssh-config-test-{name}-{}",

src/main.rs

@@ -43,6 +43,11 @@ fn spawn_config_watcher(cx: &mut App) {
     let watched_file = config_file.clone();
     let handler = move |res: notify::Result<notify::Event>| {
         let Ok(event) = res else { return };
+        // A reload re-reads the file, and on Linux reading it is itself an
+        // event — see `tty7_core::host::is_content_change`.
+        if !tty7_core::host::is_content_change(&event.kind) {
+            return;
+        }
         let hit = event
             .paths
             .iter()

src/terminal/git_data.rs

@@ -166,6 +166,12 @@ impl Debounce {
         self.seq
     }
 
+    /// Is an event still waiting for its probe?
+    #[cfg(test)]
+    pub fn is_open(&self) -> bool {
+        self.opened.is_some()
+    }
+
     /// What the timer should do now. `Fire` closes the burst, so it is
     /// returned exactly once however many events went into it.
     pub fn poll(&mut self, now: Instant) -> DebounceStep {
@@ -489,6 +495,17 @@ impl ScmData {
         self.subs.is_empty() && self.watches.is_empty()
     }
 
+    /// Is any repository still inside a debounce window?
+    ///
+    /// The window is measured on the real clock, and closing it costs a
+    /// frame. A test driving a virtual clock has no other way to tell a panel
+    /// that has gone quiet from one whose next repaint is merely still owed —
+    /// see `ui::app::test_window::quiesce`.
+    #[cfg(test)]
+    pub fn is_debouncing(&self) -> bool {
+        self.watches.values().any(|watch| watch.debounce.is_open())
+    }
+
     /// Repositories that have a holder but no watch, nothing on the way, and
     /// no failed attempt still resting.
     fn unwatched(&self, now: Instant) -> Vec<(HostId, PathBuf)> {

src/ui/app.rs

@@ -7853,6 +7853,88 @@ pub(crate) mod test_window {
         vcx.background_executor.run_until_parked();
         (app, vcx, stream)
     }
+
+    /// Wait until the window has actually stopped drawing — which is not the
+    /// same as having reached the state a test was waiting for.
+    ///
+    /// Called both after a settle and at the top of `draws_while_idle`: a test
+    /// that reaches its state, asserts a few things about it and only then
+    /// measures has given the setup more time to land, but not necessarily
+    /// enough, and the measurement is the place that cannot afford to be
+    /// wrong.
+    ///
+    /// Both of a `render_idle` test's clocks have to be pumped here, and they
+    /// are pumped differently.
+    ///
+    /// The pane runs its own git pipeline, separate from whatever panel is on
+    /// screen, and it hangs off a 300ms timer on the *virtual* clock — so it
+    /// never starts at all unless a test advances that clock. It used to be
+    /// `draws_while_idle`'s own `advance_clock` that started it, which put the
+    /// pane's first real `git` run, and the repaint it lands with, inside the
+    /// window being counted. Whether that repaint arrived before or after the
+    /// count then came down to how fast git ran, which is why these tests were
+    /// green here and red on a loaded CI runner (issue #523).
+    ///
+    /// What that repaint sets off in turn is timed on the *real* clock: the
+    /// landing opens a `GIT_WATCH_DEBOUNCE` burst, and closing the burst costs
+    /// another frame 250ms later. So the sleep below is load-bearing too, and
+    /// a round that drew nothing is not on its own enough to stop on — a burst
+    /// still open is a frame already owed.
+    #[cfg(unix)]
+    pub(crate) fn quiesce(vcx: &mut VisualTestContext, cwd: Option<&std::path::Path>) {
+        use crate::terminal::git_data::ScmData;
+        use crate::terminal::git_status::GitStatusCache;
+        use crate::ui::app::render_probe;
+        use crate::ui::host_ops::HostId;
+
+        /// How long quiet has to hold before it counts as quiet.
+        ///
+        /// Real time, and the only defence against the third clock in play:
+        /// the kernel's. The file tree keeps a real `inotify`/`FSEvents`
+        /// watch, and the writes a test makes while setting up its repository
+        /// are still being delivered long after every future the test can wait
+        /// on has resolved. They arrive on the channel, sit in a 200ms debounce
+        /// on the virtual clock, and are released by the next `advance_clock`
+        /// — which, without this, was the measurement's own. Any delivery
+        /// restarts the hold, so the wait is as long as the runner needs and
+        /// no longer.
+        const QUIET_HOLD: std::time::Duration = std::time::Duration::from_millis(400);
+
+        let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
+        let mut quiet_since: Option<std::time::Instant> = None;
+        loop {
+            render_probe::arm(u64::MAX);
+            vcx.executor()
+                .advance_clock(std::time::Duration::from_millis(300));
+            vcx.background_executor.run_until_parked();
+            let quiet = render_probe::draws() == 0
+                && vcx.update(|_, cx| {
+                    let owed = cx
+                        .try_global::<ScmData>()
+                        .is_some_and(ScmData::is_debouncing);
+                    let answered = cwd.is_none_or(|cwd| {
+                        cx.try_global::<GitStatusCache>()
+                            .and_then(|cache| cache.known_repo_for(HostId::LOCAL, cwd))
+                            .is_some()
+                    });
+                    !owed && answered
+                });
+            match quiet {
+                false => quiet_since = None,
+                true => {
+                    let since = *quiet_since.get_or_insert_with(std::time::Instant::now);
+                    if since.elapsed() >= QUIET_HOLD {
+                        return;
+                    }
+                }
+            }
+            assert!(
+                std::time::Instant::now() < deadline,
+                "the window never stopped drawing"
+            );
+            std::thread::sleep(std::time::Duration::from_millis(20));
+        }
+    }
 }
 
 #[cfg(all(test, unix))]

src/ui/file_tree.rs

@@ -2801,6 +2801,7 @@ mod render_idle_gpui_tests {
             std::thread::sleep(std::time::Duration::from_millis(20));
         }
         vcx.background_executor.run_until_parked();
+        test_window::quiesce(&mut vcx, Some(root));
         (app, vcx, pane)
     }
 
@@ -2887,12 +2888,18 @@ mod render_idle_gpui_tests {
     }
 
     fn draws_while_idle(vcx: &mut VisualTestContext) -> u64 {
+        test_window::quiesce(vcx, None);
         render_probe::arm(BUDGET);
         vcx.background_executor.run_until_parked();
         vcx.executor()
             .advance_clock(std::time::Duration::from_secs(3));
         vcx.background_executor.run_until_parked();
         render_probe::arm(BUDGET);
+        // No real-time exposure in the counted window, deliberately. The file
+        // tree holds a real filesystem watch, so real time is a channel input
+        // arrives on — and a test that spends it here is asking to be handed
+        // some. What has to be waited out is waited out in `quiesce` above,
+        // where a frame costs nothing.
         vcx.executor()
             .advance_clock(std::time::Duration::from_secs(9));
         vcx.background_executor.run_until_parked();

src/ui/scm/graph.rs

@@ -2212,12 +2212,18 @@ mod render_idle_gpui_tests {
     }
 
     fn draws_while_idle(vcx: &mut VisualTestContext) -> u64 {
+        test_window::quiesce(vcx, None);
         render_probe::arm(BUDGET);
         vcx.background_executor.run_until_parked();
         vcx.executor()
             .advance_clock(std::time::Duration::from_secs(3));
         vcx.background_executor.run_until_parked();
         render_probe::arm(BUDGET);
+        // No real-time exposure in the counted window, deliberately. The file
+        // tree holds a real filesystem watch, so real time is a channel input
+        // arrives on — and a test that spends it here is asking to be handed
+        // some. What has to be waited out is waited out in `quiesce` above,
+        // where a frame costs nothing.
         vcx.executor()
             .advance_clock(std::time::Duration::from_secs(9));
         vcx.background_executor.run_until_parked();
@@ -2234,6 +2240,7 @@ mod render_idle_gpui_tests {
             let page = app.update_in(vcx, |app, _, _| app.scm.graph.page.clone());
             if page.is_some() {
                 vcx.background_executor.run_until_parked();
+                test_window::quiesce(vcx, None);
                 return page;
             }
             if std::time::Instant::now() >= deadline {

src/ui/scm/panel.rs

@@ -2712,17 +2712,23 @@ mod render_idle_gpui_tests {
             );
             std::thread::sleep(std::time::Duration::from_millis(20));
         }
-        vcx.background_executor.run_until_parked();
+        test_window::quiesce(&mut vcx, Some(root));
         (app, vcx, pane)
     }
 
     fn draws_while_idle(vcx: &mut VisualTestContext) -> u64 {
+        test_window::quiesce(vcx, None);
         render_probe::arm(BUDGET);
         vcx.background_executor.run_until_parked();
         vcx.executor()
             .advance_clock(std::time::Duration::from_secs(3));
         vcx.background_executor.run_until_parked();
         render_probe::arm(BUDGET);
+        // No real-time exposure in the counted window, deliberately. The file
+        // tree holds a real filesystem watch, so real time is a channel input
+        // arrives on — and a test that spends it here is asking to be handed
+        // some. What has to be waited out is waited out in `quiesce` above,
+        // where a frame costs nothing.
         vcx.executor()
             .advance_clock(std::time::Duration::from_secs(9));
         vcx.background_executor.run_until_parked();