From 0dee70e5d318b974b8a2e1999f1dd9fd626e033f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Tue, 20 Oct 2026 21:45:04 +0100 Subject: [PATCH] Bound diff watches to Git-tracked directories --- CHANGELOG.md | 2 + src/app/sessions.rs | 14 +- src/app/state.rs | 4 +- src/diff_watch.rs | 328 ++++++++++++++++++++++++++++++++++++++------ 4 files changed, 303 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7519665..55c0f1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Notable changes to Agentaps are recorded here. ### Fixed +- Diff watching skips ignored and untracked dependency trees, preserves updates for tracked files, and avoids following directory symlinks to prevent runaway memory use. + - Linux desktop notifications share a single connection and worker, replace repeated alerts for the same session, and release tracking when closed, activated, or dismissed. - Reduce Linux memory retained after temporary worker allocations by returning free heap tails to the operating system sooner. diff --git a/src/app/sessions.rs b/src/app/sessions.rs index 7cefd85..f15e76a 100644 --- a/src/app/sessions.rs +++ b/src/app/sessions.rs @@ -756,9 +756,17 @@ impl Workspace { } } let mut watched_change = false; - while let Ok(generation) = self.diff.watch_rx.try_recv() { - watched_change |= - self.diff.watched_project.is_some() && generation == self.diff.watch_generation; + let mut rescan_watch = false; + while let Ok(change) = self.diff.watch_rx.try_recv() { + if self.diff.watched_project.is_some() + && change.generation == self.diff.watch_generation + { + watched_change = true; + rescan_watch |= change.rescan; + } + } + if rescan_watch { + self.diff.watched_project = None; } let now = Instant::now(); if watched_change { diff --git a/src/app/state.rs b/src/app/state.rs index 1b852eb..f38799e 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -73,8 +73,8 @@ pub(super) struct DiffState { pub(super) tx: Sender<(u64, DiffLoadResult)>, pub(super) rx: Receiver<(u64, DiffLoadResult)>, pub(super) request_id: u64, - pub(super) watch_tx: Sender, - pub(super) watch_rx: Receiver, + pub(super) watch_tx: Sender, + pub(super) watch_rx: Receiver, pub(super) watcher_tx: Sender<(u64, notify::Result)>, pub(super) watcher_rx: Receiver<(u64, notify::Result)>, pub(super) watch_generation: u64, diff --git a/src/diff_watch.rs b/src/diff_watch.rs index a69727b..2bfe7ef 100644 --- a/src/diff_watch.rs +++ b/src/diff_watch.rs @@ -1,30 +1,128 @@ use notify::{Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher}; -use std::{collections::HashSet, path::Path, sync::mpsc::Sender}; +use std::{ + collections::BTreeSet, + fs, + path::{Path, PathBuf}, + sync::mpsc::Sender, +}; -/// Watch the checkout and the Git files that can change its HEAD or index. -pub fn start(path: &Path, generation: u64, tx: Sender) -> notify::Result { - let mut watcher = notify::recommended_watcher(move |event: notify::Result| { - let is_access = event - .as_ref() - .is_ok_and(|event| matches!(event.kind, EventKind::Access(_))); - if !is_access { - let _ = tx.send(generation); +pub struct Change { + pub generation: u64, + pub rescan: bool, +} + +/// The diff contains tracked files only. Watch their parent directories rather +/// than recursively traversing ignored build outputs, dependencies, or symlinks. +#[derive(Default)] +struct WatchPlan { + files: BTreeSet, + directories: BTreeSet, + git_directories: BTreeSet, +} + +impl WatchPlan { + fn new(path: &Path) -> notify::Result { + let path = path.canonicalize()?; + let repo = gix::discover(&path).map_err(git_error)?; + let workdir = repo + .workdir() + .ok_or_else(|| notify::Error::generic("Git repository has no working directory"))? + .canonicalize()?; + let index = repo.index_or_load_from_head_or_empty().map_err(git_error)?; + let mut plan = Self::default(); + plan.directories.insert(path.clone()); + for entry in index.entries() { + let file = workdir.join(gix::path::from_bstr(entry.path(&index)).as_ref()); + if !file.starts_with(&path) { + continue; + } + for parent in file.ancestors().skip(1) { + if !parent.starts_with(&path) { + break; + } + plan.directories.insert(parent.to_path_buf()); + } + plan.files.insert(file); } - })?; - watcher.watch(path, RecursiveMode::Recursive)?; - - if let Ok(repo) = gix::discover(path) { - let mut watched = HashSet::new(); - for (git_path, mode) in [ - (repo.git_dir().to_path_buf(), RecursiveMode::NonRecursive), - (repo.common_dir().to_path_buf(), RecursiveMode::NonRecursive), - (repo.common_dir().join("refs"), RecursiveMode::Recursive), - ] { - if git_path.exists() && !git_path.starts_with(path) && watched.insert(git_path.clone()) + for root in [repo.git_dir(), repo.common_dir()] { + let root = root.canonicalize()?; + plan.git_directories.insert(root.clone()); + let mut pending = vec![root.join("refs")]; + while let Some(directory) = pending.pop() { + if !fs::symlink_metadata(&directory).is_ok_and(|meta| meta.is_dir()) { + continue; + } + if !plan.git_directories.insert(directory.clone()) { + continue; + } + for entry in fs::read_dir(directory)? { + let entry = entry?; + if entry.file_type()?.is_dir() { + pending.push(entry.path()); + } + } + } + } + // A linked worktree's .git file can itself be replaced. + plan.files.insert(path.join(".git")); + Ok(plan) + } + + fn change(&self, event: &Event) -> Option { + if matches!(event.kind, EventKind::Access(_)) { + return None; + } + let mut changed = false; + for path in &event.paths { + if self.directories.contains(path) + || self.git_directories.contains(path) + || path + .parent() + .is_some_and(|parent| self.git_directories.contains(parent)) + || path.file_name().is_some_and(|name| name == ".git") { - watcher.watch(&git_path, mode)?; + return Some(true); } + changed |= self.files.contains(path); } + changed.then_some(false) + } +} + +fn git_error(error: impl std::fmt::Display) -> notify::Error { + notify::Error::generic(&error.to_string()) +} + +pub fn start( + path: &Path, + generation: u64, + tx: Sender, +) -> notify::Result { + let root = path.canonicalize()?; + let plan = WatchPlan::new(&root)?; + let mut directories = plan.git_directories.clone(); + for directory in &plan.directories { + // Checking every ancestor also prevents descending into a tracked + // directory that has been replaced by a symlink in the working tree. + if directory + .ancestors() + .take_while(|ancestor| ancestor.starts_with(&root)) + .all(|ancestor| fs::symlink_metadata(ancestor).is_ok_and(|meta| meta.is_dir())) + { + directories.insert(directory.clone()); + } + } + let mut watcher = notify::recommended_watcher(move |event: notify::Result| { + let rescan = match event { + Ok(event) => plan.change(&event), + Err(_) => Some(true), + }; + if let Some(rescan) = rescan { + let _ = tx.send(Change { generation, rescan }); + } + })?; + for directory in directories { + watcher.watch(&directory, RecursiveMode::NonRecursive)?; } Ok(watcher) } @@ -32,30 +130,180 @@ pub fn start(path: &Path, generation: u64, tx: Sender) -> notify::Result tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + git(dir.path(), &["init", "-q"]); + dir + } #[test] - fn reports_checkout_write() { - let nonce = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos(); - let path = std::env::temp_dir().join(format!( - "agentaps-diff-watch-{}-{nonce}", - std::process::id() - )); - fs::create_dir(&path).unwrap(); + fn reports_tracked_checkout_write() { + let dir = repository(); + let path = dir.path(); + fs::create_dir(path.join("src")).unwrap(); + fs::write(path.join("src/tracked.txt"), "before").unwrap(); + git(path, &["add", "."]); let (tx, rx) = mpsc::channel(); - let watcher = start(&path, 7, tx).unwrap(); + let _watcher = start(path, 7, tx).unwrap(); + fs::write(path.join("src/tracked.txt"), "changed").unwrap(); + assert_eq!( + rx.recv_timeout(Duration::from_secs(5)).unwrap().generation, + 7 + ); + } - fs::write(path.join("changed.txt"), "changed").unwrap(); - assert_eq!(rx.recv_timeout(Duration::from_secs(5)).unwrap(), 7); + #[test] + fn excludes_ignored_trees_but_keeps_tracked_files() { + let dir = repository(); + let path = dir.path().canonicalize().unwrap(); + fs::write(path.join(".gitignore"), ".devenv/\nignored/\n").unwrap(); + fs::create_dir_all(path.join(".devenv/profile/lib/node_modules")).unwrap(); + fs::create_dir_all(path.join("ignored/nested")).unwrap(); + fs::write(path.join("ignored/nested/tracked.txt"), "tracked").unwrap(); + git(&path, &["add", "-f", "ignored/nested/tracked.txt"]); + let plan = WatchPlan::new(&path).unwrap(); + assert!( + !plan + .directories + .iter() + .any(|dir| dir.starts_with(path.join(".devenv"))) + ); + assert!(plan.directories.contains(&path.join("ignored/nested"))); + let modify = |file: &str| { + Event::new(EventKind::Modify(notify::event::ModifyKind::Any)).add_path(path.join(file)) + }; + assert_eq!( + plan.change(&modify("ignored/nested/tracked.txt")), + Some(false) + ); + assert_eq!(plan.change(&modify("ignored/nested/untracked.txt")), None); + assert_eq!(plan.change(&modify(".devenv/profile/ignored.txt")), None); + let (tx, rx) = mpsc::channel(); + let _watcher = start(&path, 7, tx).unwrap(); + fs::write( + path.join(".devenv/profile/lib/node_modules/output.txt"), + "ignored", + ) + .unwrap(); + assert!(rx.recv_timeout(Duration::from_millis(200)).is_err()); + fs::write(path.join("ignored/nested/tracked.txt"), "changed").unwrap(); + assert_eq!( + rx.recv_timeout(Duration::from_secs(5)).unwrap().generation, + 7 + ); + } + #[test] + fn index_changes_request_a_new_watch_plan() { + let dir = repository(); + let path = dir.path(); + let (tx, rx) = mpsc::channel(); + let watcher = start(path, 3, tx).unwrap(); + fs::create_dir(path.join("new")).unwrap(); + fs::write(path.join("new/tracked.txt"), "new").unwrap(); + git(path, &["add", "."]); + let change = rx.recv_timeout(Duration::from_secs(5)).unwrap(); + assert_eq!(change.generation, 3); + assert!(change.rescan); drop(watcher); - fs::remove_dir_all(path).unwrap(); + let (tx, rx) = mpsc::channel(); + let _watcher = start(path, 4, tx).unwrap(); + fs::write(path.join("new/tracked.txt"), "changed").unwrap(); + assert_eq!( + rx.recv_timeout(Duration::from_secs(5)).unwrap().generation, + 4 + ); + } + + #[test] + fn recreating_a_missing_tracked_directory_requests_a_new_watch_plan() { + let dir = repository(); + let path = dir.path(); + fs::create_dir_all(path.join("src/nested")).unwrap(); + fs::write(path.join("src/nested/tracked.txt"), "before").unwrap(); + git(path, &["add", "."]); + fs::remove_dir_all(path.join("src")).unwrap(); + let (tx, rx) = mpsc::channel(); + let _watcher = start(path, 7, tx).unwrap(); + fs::create_dir(path.join("src")).unwrap(); + assert!(rx.recv_timeout(Duration::from_secs(5)).unwrap().rescan); + } + + #[test] + fn watches_linked_worktree_index_and_shared_refs() { + let dir = repository(); + let path = dir.path(); + fs::write(path.join("tracked.txt"), "before").unwrap(); + git(path, &["add", "."]); + git( + path, + &[ + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "commit", + "-qm", + "Initial", + ], + ); + let worktree = tempfile::tempdir().unwrap(); + git( + path, + &[ + "worktree", + "add", + "-qb", + "linked", + worktree.path().to_str().unwrap(), + ], + ); + let plan = WatchPlan::new(worktree.path()).unwrap(); + assert!( + plan.git_directories + .contains(&path.join(".git/refs/heads").canonicalize().unwrap()) + ); + fs::write(worktree.path().join("tracked.txt"), "changed").unwrap(); + let (tx, rx) = mpsc::channel(); + let _watcher = start(worktree.path(), 9, tx).unwrap(); + git(worktree.path(), &["add", "."]); + let change = rx.recv_timeout(Duration::from_secs(5)).unwrap(); + assert_eq!(change.generation, 9); + assert!(change.rescan); + } + + #[cfg(unix)] + #[test] + fn does_not_follow_dependency_or_replaced_directory_symlinks() { + let dir = repository(); + let outside = tempfile::tempdir().unwrap(); + let path = dir.path(); + fs::create_dir(path.join("src")).unwrap(); + fs::write(path.join("src/tracked.txt"), "before").unwrap(); + git(path, &["add", "."]); + fs::remove_file(path.join("src/tracked.txt")).unwrap(); + fs::remove_dir(path.join("src")).unwrap(); + std::os::unix::fs::symlink(outside.path(), path.join("src")).unwrap(); + std::os::unix::fs::symlink(outside.path(), path.join("dependencies")).unwrap(); + let (tx, rx) = mpsc::channel(); + let _watcher = start(path, 7, tx).unwrap(); + fs::write(outside.path().join("tracked.txt"), "outside").unwrap(); + assert!(rx.recv_timeout(Duration::from_millis(200)).is_err()); } }