From 8ff3d531a2ce7bb5dfb0a5355606fc0bef8099c6 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Sat, 26 Sep 2026 14:16:42 -0400 Subject: [PATCH 01/21] fs: apply FSEvents watch changes on a watcher thread On macOS every FSEvents watch or unwatch stops the stream and starts a new one over all watched paths, and the start waits on fseventsd, which can take seconds when it is busy. Callers made these changes on whatever thread they ran on, including the main thread when a Project was created or dropped. Native watches on macOS now go through a watcher thread that applies each burst of changes with one update_paths call, so one restart covers the burst, and a watch removed before it was applied never reaches the stream. RealFs::watch still waits (without blocking a thread) until its root watch is in place. inotify, Windows, and poll changes stay on the caller's thread, so a caller that adds a watch and then reads the directory never misses a change in between. --- crates/fs/src/fs.rs | 11 +- crates/fs/src/fs_watcher.rs | 1155 ++++++++++++++++++++++++++--------- 2 files changed, 877 insertions(+), 289 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 59fe449b185b1c..30cb35c9b482b4 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -1075,13 +1075,15 @@ impl Fs for RealFs { let (tx, rx) = async_channel::unbounded(); let pending_paths: Arc>> = Default::default(); - let watcher: Arc = Arc::new(fs_watcher::FsWatcher::new( + let watcher = Arc::new(fs_watcher::FsWatcher::new( executor.clone(), tx.clone(), pending_paths.clone(), )); - if let Err(e) = watcher.add(path) { + // Awaited rather than queued, so a change made after `watch` returns is + // never missed; the OS work runs on the watcher thread either way. + if let Err(e) = watcher.add_and_wait(path).await { log::warn!("Failed to watch {}:\n{e}", path.display()); } @@ -1097,11 +1099,12 @@ impl Fs for RealFs { target = SanitizedPath::new(&canonical).as_path().to_path_buf(); } } - watcher.add(&target).ok(); + watcher.add_and_wait(&target).await.ok(); if let Some(parent) = target.parent() { - watcher.add(parent).log_err(); + watcher.add_and_wait(parent).await.log_err(); } } + let watcher: Arc = watcher; ( Box::pin(rx.filter_map({ diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 80393e7d784c57..a828c0c6c63922 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -1,15 +1,16 @@ +use futures::channel::oneshot; use gpui::{BackgroundExecutor, Task}; use notify::{Event, EventKind}; use parking_lot::Mutex; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, fs, ops::DerefMut, path::Path, sync::{Arc, LazyLock, OnceLock}, time::{Duration, Instant}, }; -use util::{ResultExt, paths::SanitizedPath}; +use util::paths::SanitizedPath; use crate::{PathEvent, PathEventKind, Watcher}; @@ -49,22 +50,65 @@ impl FsWatcher { } } - fn add_existing_path(&self, path: Arc) -> anyhow::Result<()> { + fn add_existing_path(&self, path: Arc) -> Option { let case_insensitive = case_insensitive_path(&path); let key = WatchKey::for_registration(SanitizedPath::new(&path), case_insensitive); - if self.registrations.lock().contains_key(&key) { + // A registration whose OS watch failed is gone from the global watcher; + // adding the path again retries it. + if let Some(existing) = self.registrations.lock().get(&key) + && global_watcher().is_registered(existing.id) + { log::trace!("path to watch is already watched: {path:?}"); - return Ok(()); + return None; } - if let Some(registration) = register_existing_path( + let (registration, applied) = register_existing_path( path, case_insensitive, self.tx.clone(), self.pending_path_events.clone(), - )? { - self.registrations.lock().insert(key, registration); + )?; + self.registrations.lock().insert(key, registration); + applied + } + + /// Adds `path` and waits until its OS watch is in place, so no change made + /// after this returns can be missed. Waiting never blocks a thread. + pub async fn add_and_wait(&self, path: &Path) -> anyhow::Result<()> { + let Some(applied) = self.add_path(path) else { + return Ok(()); + }; + // Cancelled means the registration was removed before its watch was + // applied, so there is nothing left to wait for. + applied.await.unwrap_or(Ok(())) + } + + fn add_path(&self, path: &Path) -> Option { + log::trace!("watcher add: {path:?}"); + + let path: Arc = path.into(); + if path_covered_by_recursive_registration( + &self.registrations.lock(), + SanitizedPath::new(&path), + ) { + log::trace!("path to watch is covered by an existing registration: {path:?}"); + return None; } - Ok(()) + + if self + .pending_registrations + .lock() + .contains_key(path.as_ref()) + { + log::trace!("path to watch is already pending: {path:?}"); + return None; + } + + if fs::symlink_metadata(path.as_ref()).is_err() { + self.add_pending_path(path); + return None; + } + + self.add_existing_path(path) } fn add_pending_path(&self, path: Arc) { @@ -103,33 +147,11 @@ impl Drop for FsWatcher { } impl Watcher for FsWatcher { + /// Queues the watch and returns without waiting for the OS; a failure is + /// logged by the watcher thread. Use [`FsWatcher::add_and_wait`] to wait. fn add(&self, path: &std::path::Path) -> anyhow::Result<()> { - log::trace!("watcher add: {path:?}"); - - let path: Arc = path.into(); - if path_covered_by_recursive_registration( - &self.registrations.lock(), - SanitizedPath::new(&path), - ) { - log::trace!("path to watch is covered by an existing registration: {path:?}"); - return Ok(()); - } - - if self - .pending_registrations - .lock() - .contains_key(path.as_ref()) - { - log::trace!("path to watch is already pending: {path:?}"); - return Ok(()); - } - - if fs::symlink_metadata(path.as_ref()).is_err() { - self.add_pending_path(path); - return Ok(()); - } - - self.add_existing_path(path) + self.add_path(path); + Ok(()) } fn remove(&self, path: &std::path::Path) -> anyhow::Result<()> { @@ -206,7 +228,7 @@ fn register_existing_path( case_insensitive: bool, tx: async_channel::Sender<()>, pending_path_events: Arc>>, -) -> anyhow::Result> { +) -> Option<(FsWatcherRegistration, Option)> { let mode = if requires_poll_watcher(path.as_ref()) { log::info!( "Using poll watcher ({}ms interval) for {}", @@ -220,7 +242,7 @@ fn register_existing_path( }; let root_path = SanitizedPath::new_arc(path.as_ref()); let path_for_callback = path.clone(); - let Some(registration_id) = global_watcher().add( + let registration = global_watcher().add( path, mode, case_insensitive, @@ -235,14 +257,14 @@ fn register_existing_path( event, ); }, - )? - else { - return Ok(None); - }; - Ok(Some(FsWatcherRegistration { - id: registration_id, - mode, - })) + )?; + Some(( + FsWatcherRegistration { + id: registration.id, + mode, + }, + registration.applied, + )) } #[cfg(target_os = "linux")] @@ -505,7 +527,13 @@ async fn poll_path_until_created( tx.clone(), pending_path_events.clone(), ) { - Ok(Some(registration)) => { + Some((registration, applied)) => { + if let Some(applied) = applied + && let Ok(Err(error)) = applied.await + { + log::warn!("failed to watch newly-created path {path:?}: {error}; retrying"); + continue; + } { let mut pending_registrations = pending_registrations.lock(); if pending_registrations.remove(path.as_ref()).is_none() { @@ -530,10 +558,7 @@ async fn poll_path_until_created( ); return; } - Ok(None) => {} - Err(error) => { - log::warn!("failed to watch newly-created path {path:?}: {error}; retrying"); - } + None => {} } } } @@ -674,6 +699,21 @@ fn is_covered_rescan(kind: Option, path: &Path, ancestor: &Path) #[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct WatcherRegistrationId(u32); +/// Identifies one lifetime of an OS watch on a path: from the watch command +/// that asked for it to the unwatch command that releases it. Commands of the +/// same epoch may cancel out; commands of different epochs never do. +#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)] +struct WatchEpoch(u64); + +/// Resolves once the OS watch a registration asked for is in place (or failed). +/// A registration removed before its watch was applied resolves as cancelled. +pub(crate) type WatchApplied = oneshot::Receiver>; + +pub(crate) struct Registration { + id: WatcherRegistrationId, + applied: Option, +} + struct WatcherRegistrationState { callback: Arc, key: WatchKey, @@ -683,7 +723,10 @@ struct WatcherRegistrationState { struct PathRegistrationState { watcher_ids: Vec, - has_os_watcher: bool, + /// The OS watch this path owns, or `None` when a recursive ancestor covers it. + os_watch: Option, + /// Registrations waiting for `os_watch` to be applied. Empty once it is. + waiters: Vec>>, } /// The registered watch paths for one watcher mode, keyed by [`WatchKey`] so that @@ -707,8 +750,8 @@ impl WatchPaths { self.0.entry(key) } - fn remove(&mut self, key: &WatchKey) { - self.0.remove(key); + fn remove(&mut self, key: &WatchKey) -> Option { + self.0.remove(key) } /// True if a recursive registration on a strict ancestor already covers @@ -740,12 +783,14 @@ impl WatchPaths { } } +#[derive(Default)] struct WatcherState { watchers: HashMap, native_path_registrations: WatchPaths, poll_path_registrations: WatchPaths, cooldown_until: Option, last_registration: WatcherRegistrationId, + last_epoch: WatchEpoch, } impl WatcherState { @@ -754,6 +799,22 @@ impl WatcherState { .is_some_and(|cooldown_until| cooldown_until > Instant::now()) } + fn start_native_watch_limit_cooldown(&mut self, path: &Path) { + let should_log = !self.is_native_watch_limit_cooldown_active(); + self.cooldown_until = Some(Instant::now() + *NATIVE_WATCH_LIMIT_COOLDOWN); + if should_log { + log::warn!( + "OS file watch limit reached while watching {path:?}; skipping new native file watcher registrations for {} seconds", + NATIVE_WATCH_LIMIT_COOLDOWN.as_secs() + ); + } + } + + fn next_epoch(&mut self) -> WatchEpoch { + self.last_epoch = WatchEpoch(self.last_epoch.0 + 1); + self.last_epoch + } + fn path_registrations(&mut self, mode: WatcherMode) -> &mut WatchPaths { match mode { WatcherMode::Native => &mut self.native_path_registrations, @@ -761,10 +822,9 @@ impl WatcherState { } } - fn remove_registration( - &mut self, - id: WatcherRegistrationId, - ) -> Option<(Arc, WatcherMode)> { + /// Drops a registration and, when it was the last one on a path that owns + /// an OS watch, returns the unwatch command that releases it. + fn remove_registration(&mut self, id: WatcherRegistrationId) -> Option { let registration_state = self.watchers.remove(&id)?; let path_registrations = self.path_registrations(registration_state.mode); let path_state = path_registrations.get_mut(®istration_state.key)?; @@ -773,97 +833,431 @@ impl WatcherState { return None; } - let was_actually_watched = path_state.has_os_watcher; - path_registrations.remove(®istration_state.key); + let path_state = path_registrations.remove(®istration_state.key)?; + Some(WatchCommand { + op: WatchOp::Unwatch, + mode: registration_state.mode, + path: registration_state.path, + key: registration_state.key, + epoch: path_state.os_watch?, + }) + } - was_actually_watched.then_some((registration_state.path, registration_state.mode)) + /// Records the outcome of a watch command. A failed watch is rolled back, + /// so its path reads as unregistered and a later add retries it. Returns + /// whether the command's registration was still present. + fn finish_watch(&mut self, command: &WatchCommand, result: &anyhow::Result<()>) -> bool { + let path_registrations = self.path_registrations(command.mode); + let Some(path_state) = path_registrations + .get_mut(&command.key) + .filter(|path_state| path_state.os_watch == Some(command.epoch)) + else { + return false; + }; + let waiters = std::mem::take(&mut path_state.waiters); + if result.is_err() { + let path_state = path_registrations + .remove(&command.key) + .expect("path state was just found"); + for id in path_state.watcher_ids { + self.watchers.remove(&id); + } + } + for waiter in waiters { + let outcome = match result { + Ok(()) => Ok(()), + Err(error) => Err(anyhow::anyhow!("{error:#}")), + }; + waiter.send(outcome).ok(); + } + true } } +/// A native or poll backend. Every change goes through `update_paths`, so a +/// batch of path changes costs one backend restart where the backend has one +/// (FSEvents restarts its stream on every change). trait WatchBackend: Send { - fn watch(&mut self, path: &Path, mode: notify::RecursiveMode) -> notify::Result<()>; - fn unwatch(&mut self, path: &Path) -> notify::Result<()>; + fn update_paths(&mut self, ops: Vec) -> Result<(), notify::UpdatePathsError>; } impl WatchBackend for T { - fn watch(&mut self, path: &Path, mode: notify::RecursiveMode) -> notify::Result<()> { - notify::Watcher::watch(self, path, mode) + fn update_paths(&mut self, ops: Vec) -> Result<(), notify::UpdatePathsError> { + notify::Watcher::update_paths(self, ops) } +} + +type DispatchEvent = (WatcherMode, Result); + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum WatchOp { + Watch, + Unwatch, +} + +struct WatchCommand { + op: WatchOp, + mode: WatcherMode, + path: Arc, + key: WatchKey, + epoch: WatchEpoch, +} - fn unwatch(&mut self, path: &Path) -> notify::Result<()> { - notify::Watcher::unwatch(self, path) +impl WatchCommand { + fn path_op(&self) -> notify::PathOp { + let path = self.path.as_path().to_path_buf(); + match (self.op, self.mode) { + (WatchOp::Unwatch, _) => notify::PathOp::unwatch(path), + (WatchOp::Watch, WatcherMode::Poll) => notify::PathOp::watch_recursive(path), + (WatchOp::Watch, WatcherMode::Native) + if cfg!(any(target_os = "windows", target_os = "macos")) => + { + notify::PathOp::watch_recursive(path) + } + (WatchOp::Watch, WatcherMode::Native) => notify::PathOp::watch_non_recursive(path), + } } } -type DispatchEvent = (WatcherMode, Result); +enum WorkerMessage { + Command(WatchCommand), + #[cfg(test)] + Flush(std::sync::mpsc::Sender<()>), +} -pub struct GlobalWatcher { - state: Mutex, +/// The OS backends, one lock per mode so a slow native batch never holds up +/// a poll change. +struct WatchBackends { + native: Mutex, String>>, + poll: Mutex>>, + make_poll: fn() -> notify::Result>, +} + +/// Applies watch commands to the backends and records their outcome in the +/// shared state. Runs on the watcher thread for queued modes and on the +/// caller's thread for the rest. +struct WatchApplier { + state: Arc>, + backends: WatchBackends, + /// Epochs whose watch failed after their registration was already removed, + /// so the unwatch still in flight for them has nothing to release. + failed_epochs: Mutex>, +} + +impl WatchApplier { + /// A watch followed by the unwatch of the same epoch never reaches the + /// backend; an unwatch for an epoch whose watch failed has nothing to do. + fn cancel_paired(&self, commands: Vec) -> Vec { + let mut failed_epochs = self.failed_epochs.lock(); + let mut kept: Vec> = Vec::with_capacity(commands.len()); + let mut unapplied_watches = HashMap::new(); + for command in commands { + match command.op { + WatchOp::Watch => { + unapplied_watches.insert(command.epoch, kept.len()); + kept.push(Some(command)); + } + WatchOp::Unwatch => { + if let Some(index) = unapplied_watches.remove(&command.epoch) { + kept[index] = None; + } else if !failed_epochs.remove(&command.epoch) { + kept.push(Some(command)); + } + } + } + } + kept.into_iter().flatten().collect() + } + + /// Applies `commands` for one mode as one `update_paths` batch, retrying + /// what a failed op left unapplied. Never holds the state lock while the + /// backend runs. + fn apply(&self, mode: WatcherMode, commands: Vec) { + let mut pending = self.cancel_paired(commands); + if pending.is_empty() { + return; + } + let mut finished = Vec::new(); + { + let mut native; + let mut poll; + let backend: &mut Box = match mode { + WatcherMode::Native => { + native = self.backends.native.lock(); + match native.as_mut() { + Ok(backend) => backend, + Err(error) => { + let message = format!("native file watcher unavailable: {error}"); + drop(native); + for command in pending { + self.finish(command, Err(anyhow::anyhow!("{message}"))); + } + return; + } + } + } + WatcherMode::Poll => { + poll = self.backends.poll.lock(); + if poll.is_none() { + match (self.backends.make_poll)() { + Ok(backend) => *poll = Some(backend), + Err(error) => { + drop(poll); + let message = error.to_string(); + for command in pending { + self.finish(command, Err(anyhow::anyhow!("{message}"))); + } + return; + } + } + } + poll.as_mut().expect("poll watcher initialized") + } + }; + while !pending.is_empty() { + let ops = pending.iter().map(WatchCommand::path_op).collect(); + let Err(error) = backend.update_paths(ops) else { + finished.extend(pending.drain(..).map(|command| (command, Ok(())))); + break; + }; + let notify::UpdatePathsError { + source, + origin, + remaining, + } = error; + let Some(_) = origin else { + // Every op was applied but the backend failed to restart. + let message = source.to_string(); + finished.extend( + pending + .drain(..) + .map(|command| (command, Err(anyhow::anyhow!("{message}")))), + ); + break; + }; + let failed_index = pending.len() - remaining.len() - 1; + let unapplied = pending.split_off(failed_index + 1); + let failed = pending.pop().expect("the failed op is in the batch"); + finished.extend(pending.drain(..).map(|command| (command, Ok(())))); + finished.push((failed, Err(source.into()))); + pending = unapplied; + } + } + for (command, result) in finished { + self.finish(command, result); + } + } + + fn finish(&self, command: WatchCommand, result: anyhow::Result<()>) { + match (command.op, &result) { + (WatchOp::Unwatch, Err(error)) if !is_watch_not_found_error(error) => { + log::warn!("failed to unwatch {:?}: {error:#}", command.path.as_path()); + } + (WatchOp::Unwatch, _) => {} + (WatchOp::Watch, _) => { + let mut state = self.state.lock(); + if let Err(error) = &result { + if command.mode == WatcherMode::Native && is_max_files_watch_error(error) { + state.start_native_watch_limit_cooldown(command.path.as_path()); + } else { + log::warn!("failed to watch {:?}: {error:#}", command.path.as_path()); + } + } + let registered = state.finish_watch(&command, &result); + if result.is_err() && !registered { + self.failed_epochs.lock().insert(command.epoch); + } + } + } + } +} - // DANGER: never keep state lock while holding watcher lock - // two mutexes because calling watcher.add triggers watcher.event, which needs watchers. - native_watcher: Mutex>>, - poll_watcher: Mutex>>, +/// Owns the queued modes' OS work. An FSEvents change stops the stream and +/// starts a new one over every watched path, and the start waits on +/// `fseventsd`, which can take seconds when it is busy; on this thread that +/// wait never blocks a caller. Commands queued while a batch runs are applied +/// together as the next batch, which costs one restart. +fn run_watch_worker( + applier: Arc, + messages: std::sync::mpsc::Receiver, +) { + while let Ok(first) = messages.recv() { + let mut commands = Vec::new(); + #[cfg(test)] + let mut flushes = Vec::new(); + for message in std::iter::once(first).chain(messages.try_iter()) { + match message { + WorkerMessage::Command(command) => commands.push(command), + #[cfg(test)] + WorkerMessage::Flush(done) => flushes.push(done), + } + } + let (native, poll): (Vec<_>, Vec<_>) = commands + .into_iter() + .partition(|command| command.mode == WatcherMode::Native); + applier.apply(WatcherMode::Native, native); + applier.apply(WatcherMode::Poll, poll); + #[cfg(test)] + for done in flushes { + done.send(()).ok(); + } + } +} + +/// Which modes go through the watcher thread. Only FSEvents needs it: its +/// changes restart a stream and wait on `fseventsd`. inotify, Windows, and poll +/// changes are applied on the caller's thread, so a caller that adds a watch +/// and then reads the directory never misses a change in between. +fn queued_in_production(mode: WatcherMode) -> bool { + mode == WatcherMode::Native && cfg!(target_os = "macos") +} + +pub struct GlobalWatcher { + state: Arc>, + applier: Arc, + commands: std::sync::mpsc::Sender, + queued: fn(WatcherMode) -> bool, + native_available: Result<(), String>, event_tx: async_channel::Sender, } impl GlobalWatcher { - #[must_use] + fn new( + event_tx: async_channel::Sender, + native: Result, String>, + poll: Option>, + make_poll: fn() -> notify::Result>, + queued: fn(WatcherMode) -> bool, + ) -> Self { + let state = Arc::new(Mutex::new(WatcherState::default())); + let native_available = native.as_ref().map(|_| ()).map_err(Clone::clone); + let applier = Arc::new(WatchApplier { + state: state.clone(), + backends: WatchBackends { + native: Mutex::new(native), + poll: Mutex::new(poll), + make_poll, + }, + failed_epochs: Mutex::default(), + }); + let (commands, messages) = std::sync::mpsc::channel(); + let worker_applier = applier.clone(); + std::thread::Builder::new() + .name("fs-watcher".to_owned()) + .spawn(move || run_watch_worker(worker_applier, messages)) + .expect("failed to spawn fs watcher thread"); + Self { + state, + applier, + commands, + queued, + native_available, + event_tx, + } + } + + /// Registers `cb` for events under `path`. Never blocks on FSEvents: a new + /// native watch on macOS is queued for the watcher thread, and `applied` + /// resolves once it is in place. Other modes are applied before this + /// returns. `None` while the native watch limit cooldown is active. fn add( &self, path: Arc, mode: WatcherMode, case_insensitive: bool, cb: impl Fn(¬ify::Event) + Send + Sync + 'static, - ) -> anyhow::Result> { + ) -> Option { let path = SanitizedPath::from_arc(path); let key = WatchKey::for_registration(&path, case_insensitive); - let mut state = self.state.lock(); - let (path_already_covered, path_already_registered) = { + let (registration, command) = { + let mut state = self.state.lock(); let registrations_for_mode = state.path_registrations(mode); - ( - registrations_for_mode.covered_by_recursive_ancestor(&path, mode), - registrations_for_mode.contains(&key), - ) - }; - - if !path_already_covered && !path_already_registered { - if mode == WatcherMode::Native && state.is_native_watch_limit_cooldown_active() { - return Ok(None); + let covered = registrations_for_mode.covered_by_recursive_ancestor(&path, mode); + let registered = registrations_for_mode.contains(&key); + if !covered + && !registered + && mode == WatcherMode::Native + && state.is_native_watch_limit_cooldown_active() + { + return None; } - drop(state); - match self.watch(path.as_path(), mode) { - Ok(()) => {} - Err(error) if mode == WatcherMode::Native && is_max_files_watch_error(&error) => { - self.start_native_watch_limit_cooldown(path.as_path()); - return Ok(None); - } - Err(error) => return Err(error), - } - state = self.state.lock(); - } + let id = state.last_registration; + state.last_registration = WatcherRegistrationId(id.0 + 1); + state.watchers.insert( + id, + WatcherRegistrationState { + callback: Arc::new(cb), + key: key.clone(), + path: path.clone(), + mode, + }, + ); - let id = state.last_registration; - state.last_registration = WatcherRegistrationId(id.0 + 1); + let (waiter, applied) = oneshot::channel(); + if registered { + let path_state = state + .path_registrations(mode) + .get_mut(&key) + .expect("registered path has state"); + path_state.watcher_ids.push(id); + // The path's OS watch is still queued: wait for the same one. + let applied = (!path_state.waiters.is_empty()).then(|| { + path_state.waiters.push(waiter); + applied + }); + return Some(Registration { id, applied }); + } - let registration_state = WatcherRegistrationState { - callback: Arc::new(cb), - key: key.clone(), - path, - mode, + let os_watch = (!covered).then(|| state.next_epoch()); + state + .path_registrations(mode) + .entry(key.clone()) + .or_insert(PathRegistrationState { + watcher_ids: vec![id], + os_watch, + waiters: if os_watch.is_some() { + vec![waiter] + } else { + Vec::new() + }, + }); + let Some(epoch) = os_watch else { + return Some(Registration { id, applied: None }); + }; + let command = WatchCommand { + op: WatchOp::Watch, + mode, + path, + key, + epoch, + }; + ( + Registration { + id, + applied: Some(applied), + }, + command, + ) }; - state.watchers.insert(id, registration_state); - state - .path_registrations(mode) - .entry(key) - .and_modify(|registration| registration.watcher_ids.push(id)) - .or_insert_with(|| PathRegistrationState { - watcher_ids: vec![id], - has_os_watcher: !path_already_covered, - }); + self.submit(command); + Some(registration) + } + + /// Whether `id` is still registered. A registration disappears when it is + /// removed or when the OS watch it asked for failed. + fn is_registered(&self, id: WatcherRegistrationId) -> bool { + self.state.lock().watchers.contains_key(&id) + } - Ok(Some(id)) + fn submit(&self, command: WatchCommand) { + if !(self.queued)(command.mode) { + self.applier.apply(command.mode, vec![command]); + return; + } + if self.commands.send(WorkerMessage::Command(command)).is_err() { + log::error!("fs watcher thread has exited; dropping a watch change"); + } } fn enqueue(&self, mode: WatcherMode, event: Result) { @@ -931,114 +1325,23 @@ impl GlobalWatcher { } } - fn start_native_watch_limit_cooldown(&self, path: &Path) { - let mut state = self.state.lock(); - let now = Instant::now(); - let should_log = !state.is_native_watch_limit_cooldown_active(); - state.cooldown_until = Some(now + *NATIVE_WATCH_LIMIT_COOLDOWN); - if should_log { - log::warn!( - "OS file watch limit reached while watching {path:?}; skipping new native file watcher registrations for {} seconds", - NATIVE_WATCH_LIMIT_COOLDOWN.as_secs() - ); - } - } - + /// Drops a registration. Never blocks on FSEvents: releasing the OS watch + /// it owned is queued for the watcher thread like any other change. pub fn remove(&self, id: WatcherRegistrationId) { - let mut state = self.state.lock(); - let Some((path, mode)) = state.remove_registration(id) else { - return; - }; - drop(state); - self.unwatch(path.as_path(), mode).log_err(); - } - - fn watch(&self, path: &Path, mode: WatcherMode) -> anyhow::Result<()> { - match mode { - WatcherMode::Native => { - self.ensure_native_watcher()?; - self.native_watcher - .lock() - .as_mut() - .expect("native watcher initialized") - .watch( - path, - if cfg!(any(target_os = "windows", target_os = "macos")) { - notify::RecursiveMode::Recursive - } else { - notify::RecursiveMode::NonRecursive - }, - )?; - } - WatcherMode::Poll => { - self.ensure_poll_watcher()?; - self.poll_watcher - .lock() - .as_mut() - .expect("poll watcher initialized") - .watch(path, notify::RecursiveMode::Recursive)?; - } - } - - Ok(()) - } - - fn unwatch(&self, path: &Path, mode: WatcherMode) -> anyhow::Result<()> { - let watcher = match mode { - WatcherMode::Native => self - .native_watcher - .lock() - .as_mut() - .map(|watcher| watcher.unwatch(path)), - WatcherMode::Poll => self - .poll_watcher - .lock() - .as_mut() - .map(|watcher| watcher.unwatch(path)), - }; - - match watcher { - // inotify auto-removes a watch when its directory is deleted, so a - // later unwatch races that and fails with a benign error. Either way - // the path is no longer watched, which is all we wanted. - Some(Err(error)) if !matches!(error.kind, notify::ErrorKind::WatchNotFound) => { - Err(error.into()) - } - _ => Ok(()), + let command = self.state.lock().remove_registration(id); + if let Some(command) = command { + self.submit(command); } } - fn ensure_native_watcher(&self) -> anyhow::Result<()> { - if self.native_watcher.lock().is_some() { - return Ok(()); - } - - // CORE excludes Access events, which Zed discards anyway. Without this, - // the default mask subscribes to inotify OPEN/CLOSE_* on Linux, so every - // file read in a watched directory would queue events, increasing the - // risk of queue overflows (and thus full rescans) under read-heavy - // workloads like grep or language server indexing. - let config = notify::Config::default().with_event_kinds(notify::EventKindMask::CORE); - let watcher = ::new( - |event| global_watcher().enqueue(WatcherMode::Native, event), - config, - )?; - *self.native_watcher.lock() = Some(Box::new(watcher)); - Ok(()) - } - - fn ensure_poll_watcher(&self) -> anyhow::Result<()> { - if self.poll_watcher.lock().is_some() { - return Ok(()); - } - - let config = notify::Config::default().with_poll_interval(*POLL_INTERVAL); - let watcher = notify::PollWatcher::new( - |event| global_watcher().enqueue(WatcherMode::Poll, event), - config, - )?; - *self.poll_watcher.lock() = Some(Box::new(watcher)); - Ok(()) + /// Blocks until the watcher thread has applied every command sent so far. + #[cfg(test)] + fn flush(&self) { + let (done, applied) = std::sync::mpsc::channel(); + self.commands + .send(WorkerMessage::Flush(done)) + .expect("fs watcher thread is running"); + applied.recv().expect("fs watcher thread flushed"); } } @@ -1048,6 +1351,15 @@ fn is_max_files_watch_error(error: &anyhow::Error) -> bool { .is_some_and(|error| matches!(&error.kind, notify::ErrorKind::MaxFilesWatch)) } +/// inotify auto-removes a watch when its directory is deleted, so a later +/// unwatch races that and fails with a benign error. Either way the path is no +/// longer watched, which is all we wanted. +fn is_watch_not_found_error(error: &anyhow::Error) -> bool { + error + .downcast_ref::() + .is_some_and(|error| matches!(&error.kind, notify::ErrorKind::WatchNotFound)) +} + static POLL_INTERVAL: LazyLock = LazyLock::new(|| { let poll_ms: u64 = std::env::var("ZED_FILE_WATCHER_POLL_MS") .ok() @@ -1083,21 +1395,42 @@ fn global_watcher() -> &'static GlobalWatcher { } }) .expect("failed to spawn fs watcher dispatch thread"); - GlobalWatcher { - state: Mutex::new(WatcherState { - watchers: Default::default(), - native_path_registrations: Default::default(), - poll_path_registrations: Default::default(), - cooldown_until: None, - last_registration: Default::default(), - }), - native_watcher: Mutex::new(None), - poll_watcher: Mutex::new(None), + GlobalWatcher::new( event_tx, - } + new_native_backend().map_err(|error| error.to_string()), + None, + new_poll_backend, + queued_in_production, + ) }) } +/// Creating the backend starts no OS watch (FSEvents starts a stream only +/// once it has a path), so this is cheap on whichever thread first needs the +/// global watcher. +fn new_native_backend() -> notify::Result> { + // CORE excludes Access events, which Zed discards anyway. Without this, + // the default mask subscribes to inotify OPEN/CLOSE_* on Linux, so every + // file read in a watched directory would queue events, increasing the + // risk of queue overflows (and thus full rescans) under read-heavy + // workloads like grep or language server indexing. + let config = notify::Config::default().with_event_kinds(notify::EventKindMask::CORE); + let watcher = ::new( + |event| global_watcher().enqueue(WatcherMode::Native, event), + config, + )?; + Ok(Box::new(watcher)) +} + +fn new_poll_backend() -> notify::Result> { + let config = notify::Config::default().with_poll_interval(*POLL_INTERVAL); + let watcher = notify::PollWatcher::new( + |event| global_watcher().enqueue(WatcherMode::Poll, event), + config, + )?; + Ok(Box::new(watcher)) +} + #[cfg(test)] mod tests { use super::*; @@ -1122,67 +1455,97 @@ mod tests { watched_paths: HashSet, watch_calls: Vec, unwatch_calls: Vec, + /// Ops per `update_paths` call, i.e. per backend restart. + batches: Vec, fail_with_watch_limit: bool, + /// How long each `update_paths` call takes, like a busy `fseventsd`. + delay: Duration, } struct SharedFakeWatchBackend(Arc>); impl WatchBackend for SharedFakeWatchBackend { - fn watch(&mut self, path: &Path, _mode: notify::RecursiveMode) -> notify::Result<()> { - let path = path.to_path_buf(); + fn update_paths( + &mut self, + ops: Vec, + ) -> Result<(), notify::UpdatePathsError> { + // Whether this batch fails is decided when it starts, as it would + // be for an OS call already in flight. + let (delay, fail) = { + let backend = self.0.lock(); + (backend.delay, backend.fail_with_watch_limit) + }; + std::thread::sleep(delay); let mut backend = self.0.lock(); - backend.watch_calls.push(path.clone()); - if backend.fail_with_watch_limit { - return Err(notify::Error::new(notify::ErrorKind::MaxFilesWatch)); + backend.batches.push(ops.len()); + let mut ops = ops.into_iter(); + while let Some(op) = ops.next() { + let result = match &op { + notify::PathOp::Watch(path, _) => { + backend.watch_calls.push(path.clone()); + if fail { + Err(notify::Error::new(notify::ErrorKind::MaxFilesWatch)) + } else { + backend.watched_paths.insert(path.clone()); + Ok(()) + } + } + notify::PathOp::Unwatch(path) => { + backend.unwatch_calls.push(path.clone()); + if backend.watched_paths.remove(path) { + Ok(()) + } else { + Err(notify::Error::generic("path was not watched")) + } + } + }; + if let Err(source) = result { + return Err(notify::UpdatePathsError { + source, + origin: Some(op), + remaining: ops.collect(), + }); + } } - backend.watched_paths.insert(path); Ok(()) } + } - fn unwatch(&mut self, path: &Path) -> notify::Result<()> { - let path = path.to_path_buf(); - let mut backend = self.0.lock(); - backend.unwatch_calls.push(path.clone()); - if backend.watched_paths.remove(&path) { - Ok(()) - } else { - Err(notify::Error::generic("path was not watched")) - } - } + fn no_poll_backend() -> notify::Result> { + Err(notify::Error::generic("no poll backend in this test")) } fn test_watcher(poll_watcher: Arc>) -> GlobalWatcher { test_watcher_with_backends(None, Some(poll_watcher)) } + /// Queues every mode on the watcher thread, the way macOS queues FSEvents. fn test_watcher_with_backends( native_watcher: Option>>, poll_watcher: Option>>, ) -> GlobalWatcher { - // Tests call `handle_event` directly to exercise dispatch synchronously, + watcher_with(native_watcher, poll_watcher, |_| true) + } + + fn watcher_with( + native_watcher: Option>>, + poll_watcher: Option>>, + queued: fn(WatcherMode) -> bool, + ) -> GlobalWatcher { + // Tests call `dispatch` directly to exercise dispatch synchronously, // rather than going through the OS watcher callbacks, so nothing is ever // sent on this channel; the receiver can just be dropped. let (event_tx, _event_rx) = async_channel::unbounded(); - GlobalWatcher { - state: Mutex::new(WatcherState { - watchers: Default::default(), - native_path_registrations: Default::default(), - poll_path_registrations: Default::default(), - cooldown_until: None, - last_registration: Default::default(), - }), - native_watcher: Mutex::new( - native_watcher.map(|watcher| { - Box::new(SharedFakeWatchBackend(watcher)) as Box - }), - ), - poll_watcher: Mutex::new( - poll_watcher.map(|watcher| { - Box::new(SharedFakeWatchBackend(watcher)) as Box - }), - ), + GlobalWatcher::new( event_tx, - } + native_watcher + .map(|watcher| Box::new(SharedFakeWatchBackend(watcher)) as Box) + .ok_or_else(|| "no native backend in this test".to_owned()), + poll_watcher + .map(|watcher| Box::new(SharedFakeWatchBackend(watcher)) as Box), + no_poll_backend, + queued, + ) } struct TestCase { @@ -1202,15 +1565,15 @@ mod tests { let parent_registration = watcher .add(parent.as_ref().into(), WatcherMode::Poll, false, |_| {}) - .expect("add parent watch") .expect("parent watch registered"); let child_registration = watcher .add(child.as_ref().into(), WatcherMode::Poll, false, |_| {}) - .expect("add covered child watch") .expect("child watch registered"); + watcher.flush(); - watcher.remove(parent_registration); - watcher.remove(child_registration); + watcher.remove(parent_registration.id); + watcher.remove(child_registration.id); + watcher.flush(); let backend = backend.lock(); assert_eq!(backend.watch_calls, &[parent.to_path_buf()]); @@ -1230,18 +1593,242 @@ mod tests { let first_registration = watcher .add(first_path.clone(), WatcherMode::Native, false, |_| {}) - .expect("native watch limit is handled"); - let second_registration = watcher - .add(second_path, WatcherMode::Native, false, |_| {}) - .expect("native watch limit backoff is handled"); + .expect("the first registration is queued"); + watcher.flush(); + let second_registration = watcher.add(second_path, WatcherMode::Native, false, |_| {}); - assert!(first_registration.is_none()); - assert!(second_registration.is_none()); + assert!( + !watcher.is_registered(first_registration.id), + "a registration whose watch hit the limit is rolled back" + ); + assert!( + second_registration.is_none(), + "the cooldown skips new native registrations" + ); let native_backend = native_backend.lock(); assert_eq!(native_backend.watch_calls, &[first_path.to_path_buf()]); } + fn slow_backend(delay: Duration) -> Arc> { + Arc::new(Mutex::new(FakeWatchBackend { + delay, + ..Default::default() + })) + } + + fn poll_path(path: &str) -> Arc { + Arc::::from(Path::new(path)) + } + + #[test] + fn only_fsevents_is_queued_in_production() { + assert_eq!( + queued_in_production(WatcherMode::Native), + cfg!(target_os = "macos") + ); + assert!(!queued_in_production(WatcherMode::Poll)); + } + + #[test] + fn an_inline_mode_is_watched_before_add_returns() { + let backend = Arc::new(Mutex::new(FakeWatchBackend::default())); + let watcher = watcher_with(None, Some(backend.clone()), |_| false); + + let registration = watcher + .add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + + // A caller that adds a watch and then reads the directory must not miss + // a change made in between, so nothing may be left to a later thread. + assert!( + backend + .lock() + .watched_paths + .contains(&PathBuf::from("/repo/root")) + ); + let mut applied = registration + .applied + .expect("a new OS watch reports its outcome"); + assert!(matches!(applied.try_recv(), Ok(Some(Ok(()))))); + + watcher.remove(registration.id); + assert_eq!( + backend.lock().unwatch_calls, + vec![PathBuf::from("/repo/root")] + ); + } + + #[test] + fn add_and_remove_never_wait_for_the_os() { + let backend = slow_backend(Duration::from_millis(500)); + let watcher = test_watcher(backend.clone()); + + let started = Instant::now(); + let first = watcher + .add(poll_path("/repo/a"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let second = watcher + .add(poll_path("/repo/b"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + watcher.remove(first.id); + watcher.remove(second.id); + let elapsed = started.elapsed(); + + assert!( + elapsed < Duration::from_millis(100), + "add/remove blocked the caller for {elapsed:?} behind a 500 ms backend" + ); + watcher.flush(); + } + + #[test] + fn changes_queued_while_the_backend_is_busy_apply_as_one_batch() { + let backend = slow_backend(Duration::from_millis(200)); + let watcher = test_watcher(backend.clone()); + + watcher + .add(poll_path("/repo/first"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + // Let the worker start applying the first watch before queueing more. + std::thread::sleep(Duration::from_millis(50)); + for ix in 0..20 { + watcher + .add( + poll_path(&format!("/repo/dir-{ix}")), + WatcherMode::Poll, + false, + |_| {}, + ) + .expect("registered"); + } + watcher.flush(); + + assert_eq!(backend.lock().batches, vec![1, 20]); + assert_eq!(backend.lock().watched_paths.len(), 21); + } + + #[test] + fn a_watch_removed_before_it_was_applied_never_reaches_the_backend() { + let backend = slow_backend(Duration::from_millis(200)); + let watcher = test_watcher(backend.clone()); + + watcher + .add(poll_path("/repo/busy"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + std::thread::sleep(Duration::from_millis(50)); + let transient = watcher + .add( + poll_path("/repo/transient"), + WatcherMode::Poll, + false, + |_| {}, + ) + .expect("registered"); + watcher.remove(transient.id); + watcher.flush(); + + let backend = backend.lock(); + assert_eq!(backend.watch_calls, vec![PathBuf::from("/repo/busy")]); + assert!(backend.unwatch_calls.is_empty()); + } + + #[test] + fn applied_resolves_after_the_backend_applied_the_watch() { + let backend = slow_backend(Duration::from_millis(100)); + let watcher = test_watcher(backend.clone()); + + let registration = watcher + .add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let applied = registration.applied.expect("a new OS watch is awaited"); + + smol::block_on(applied) + .expect("not cancelled") + .expect("watch applied"); + assert!( + backend + .lock() + .watched_paths + .contains(&PathBuf::from("/repo/root")), + "applied resolved before the backend watched the path" + ); + } + + #[test] + fn applied_reports_a_failed_watch_and_rolls_it_back() { + let backend = Arc::new(Mutex::new(FakeWatchBackend { + fail_with_watch_limit: true, + ..Default::default() + })); + let watcher = test_watcher(backend); + + let registration = watcher + .add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let result = smol::block_on(registration.applied.expect("awaited")).expect("not cancelled"); + + assert!(result.is_err()); + assert!(!watcher.is_registered(registration.id)); + let retry = watcher.add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}); + assert!( + retry.and_then(|retry| retry.applied).is_some(), + "a rolled-back path asks for a new OS watch" + ); + watcher.flush(); + } + + #[test] + fn a_second_registration_waits_for_the_pending_watch_it_shares() { + let backend = slow_backend(Duration::from_millis(100)); + let watcher = test_watcher(backend.clone()); + + let first = watcher + .add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let second = watcher + .add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + + for applied in [first.applied, second.applied] { + smol::block_on(applied.expect("both wait for the shared watch")) + .expect("not cancelled") + .expect("watch applied"); + } + assert_eq!(backend.lock().watch_calls.len(), 1); + } + + #[test] + fn the_unwatch_of_a_failed_watch_does_not_cancel_a_newer_watch() { + let backend = Arc::new(Mutex::new(FakeWatchBackend { + fail_with_watch_limit: true, + delay: Duration::from_millis(200), + ..Default::default() + })); + let watcher = test_watcher(backend.clone()); + let path = poll_path("/repo/root"); + + let failing = watcher + .add(path.clone(), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + // The worker is now applying (and failing) the first watch. + std::thread::sleep(Duration::from_millis(50)); + watcher.remove(failing.id); + backend.lock().fail_with_watch_limit = false; + let replacement = watcher + .add(path.clone(), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + watcher.flush(); + + let backend = backend.lock(); + assert!(watcher.is_registered(replacement.id)); + assert!(backend.watched_paths.contains(path.as_ref())); + assert!( + backend.unwatch_calls.is_empty(), + "the failed watch left nothing to unwatch" + ); + } + fn modify_event(path: &str) -> notify::Event { notify::Event { paths: vec![PathBuf::from(path)], @@ -1265,9 +1852,9 @@ mod tests { fired.lock().push(label.clone()); }, ) - .expect("add watch") .expect("watch registered"); } + watcher.flush(); (watcher, fired) } @@ -1347,7 +1934,6 @@ mod tests { true, cb, ) - .expect("add") .expect("registered"); // Event arrives lowercased (as TSGO/macOS may report it). @@ -1369,7 +1955,6 @@ mod tests { true, cb, ) - .expect("add") .expect("registered"); // The watched directory was renamed to a different casing; events now @@ -1389,7 +1974,6 @@ mod tests { false, cb, ) - .expect("add") .expect("registered"); // On a case-sensitive volume these are genuinely different directories. @@ -1408,7 +1992,6 @@ mod tests { true, |_| {}, ) - .expect("add") .expect("registered"); watcher .add( @@ -1417,9 +2000,10 @@ mod tests { true, |_| {}, ) - .expect("add") .expect("registered"); + watcher.flush(); + // The second, differently-cased spelling reuses the same OS watch. assert_eq!(backend.lock().watch_calls.len(), 1); } @@ -1430,7 +2014,6 @@ mod tests { let watcher = test_watcher(backend.clone()); watcher .add(Path::new("/Repo").into(), WatcherMode::Poll, true, |_| {}) - .expect("add") .expect("registered"); watcher .add( @@ -1439,7 +2022,8 @@ mod tests { true, |_| {}, ) - .expect("add"); + .expect("registered"); + watcher.flush(); // The child is covered by the recursive parent despite the case mismatch. assert_eq!(backend.lock().watch_calls, vec![PathBuf::from("/Repo")]); @@ -1484,7 +2068,6 @@ mod tests { false, move |_| fired.lock().push(()), ) - .expect("add watch") .expect("watch registered"); } @@ -1562,6 +2145,8 @@ mod tests { pub fn global(f: impl FnOnce(&GlobalWatcher) -> T) -> anyhow::Result { let global_watcher = global_watcher(); - global_watcher.ensure_native_watcher()?; + if let Err(error) = &global_watcher.native_available { + anyhow::bail!("native file watcher unavailable: {error}"); + } Ok(f(global_watcher)) } From 1856e49b5a95e3df1d01dce1f44df72ea8d975d7 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Sat, 26 Sep 2026 16:24:41 -0400 Subject: [PATCH 02/21] fs, worktree: wait for new watches the scanner depends on Watcher::add may return before a new FSEvents watch is applied. The worktree scanner added a linked worktree's common and repository directories, the reftable directory, symlink targets outside the root, and external directories with add, then trusted them as watched; those paths usually sit outside the recursively watched root, so their watches are new. They now use Watcher::add_and_wait, which resolves once the watch is in place without blocking a thread (other implementations fall back to add). Adds tests for a batch whose middle op fails and for the watcher thread exiting with its watcher. --- crates/fs/src/fs.rs | 8 +++ crates/fs/src/fs_watcher.rs | 93 ++++++++++++++++++++++++++++----- crates/worktree/src/worktree.rs | 40 +++++++++----- 3 files changed, 114 insertions(+), 27 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 30cb35c9b482b4..3c0071fee893d8 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -71,6 +71,14 @@ use std::ffi::OsStr; pub trait Watcher: Send + Sync { fn add(&self, path: &Path) -> Result<()>; fn remove(&self, path: &Path) -> Result<()>; + + /// Add `path` and resolve once its OS watch is in place, so no change made + /// after it resolves can be missed. `add` may return before then (a new + /// FSEvents watch is applied on a watcher thread). Waiting never blocks a + /// thread. + fn add_and_wait<'a>(&'a self, path: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async move { self.add(path) }) + } } #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)] diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index a828c0c6c63922..f75cfbd51e2fcf 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -71,17 +71,6 @@ impl FsWatcher { applied } - /// Adds `path` and waits until its OS watch is in place, so no change made - /// after this returns can be missed. Waiting never blocks a thread. - pub async fn add_and_wait(&self, path: &Path) -> anyhow::Result<()> { - let Some(applied) = self.add_path(path) else { - return Ok(()); - }; - // Cancelled means the registration was removed before its watch was - // applied, so there is nothing left to wait for. - applied.await.unwrap_or(Ok(())) - } - fn add_path(&self, path: &Path) -> Option { log::trace!("watcher add: {path:?}"); @@ -148,12 +137,27 @@ impl Drop for FsWatcher { impl Watcher for FsWatcher { /// Queues the watch and returns without waiting for the OS; a failure is - /// logged by the watcher thread. Use [`FsWatcher::add_and_wait`] to wait. + /// logged by the watcher thread. Use [`Watcher::add_and_wait`] to wait. fn add(&self, path: &std::path::Path) -> anyhow::Result<()> { self.add_path(path); Ok(()) } + fn add_and_wait<'a>( + &'a self, + path: &'a std::path::Path, + ) -> futures::future::BoxFuture<'a, anyhow::Result<()>> { + let applied = self.add_path(path); + Box::pin(async move { + let Some(applied) = applied else { + return Ok(()); + }; + // Cancelled means the registration was removed before its watch was + // applied, so there is nothing left to wait for. + applied.await.unwrap_or(Ok(())) + }) + } + fn remove(&self, path: &std::path::Path) -> anyhow::Result<()> { log::trace!("remove watched path: {path:?}"); self.pending_registrations.lock().remove(path); @@ -1458,6 +1462,8 @@ mod tests { /// Ops per `update_paths` call, i.e. per backend restart. batches: Vec, fail_with_watch_limit: bool, + /// Watches of these paths fail; every other op succeeds. + failing_paths: HashSet, /// How long each `update_paths` call takes, like a busy `fseventsd`. delay: Duration, } @@ -1483,7 +1489,7 @@ mod tests { let result = match &op { notify::PathOp::Watch(path, _) => { backend.watch_calls.push(path.clone()); - if fail { + if fail || backend.failing_paths.contains(path) { Err(notify::Error::new(notify::ErrorKind::MaxFilesWatch)) } else { backend.watched_paths.insert(path.clone()); @@ -1621,6 +1627,67 @@ mod tests { Arc::::from(Path::new(path)) } + #[test] + fn a_failed_op_in_a_batch_leaves_the_ops_around_it_applied() { + let backend = slow_backend(Duration::from_millis(200)); + backend + .lock() + .failing_paths + .insert(PathBuf::from("/repo/bad")); + let watcher = test_watcher(backend.clone()); + + watcher + .add(poll_path("/repo/busy"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + std::thread::sleep(Duration::from_millis(50)); + let first = watcher + .add(poll_path("/repo/first"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let bad = watcher + .add(poll_path("/repo/bad"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let last = watcher + .add(poll_path("/repo/last"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + watcher.flush(); + + let backend = backend.lock(); + // The busy watch, then one batch that stops at the failure, then the + // untried op retried on its own. + assert_eq!(backend.batches, vec![1, 3, 1]); + assert!(backend.watched_paths.contains(Path::new("/repo/first"))); + assert!(backend.watched_paths.contains(Path::new("/repo/last"))); + assert!(!backend.watched_paths.contains(Path::new("/repo/bad"))); + assert!(watcher.is_registered(first.id)); + assert!(watcher.is_registered(last.id)); + assert!( + !watcher.is_registered(bad.id), + "only the failed watch is rolled back" + ); + } + + #[test] + fn the_watcher_thread_exits_when_the_watcher_is_dropped() { + let backend = Arc::new(Mutex::new(FakeWatchBackend::default())); + let watcher = test_watcher(backend.clone()); + watcher + .add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + watcher.flush(); + + drop(watcher); + + // The thread owns a handle to the backend until it exits. + let deadline = Instant::now() + Duration::from_secs(5); + while Arc::strong_count(&backend) > 1 { + assert!( + Instant::now() < deadline, + "the watcher thread outlived its watcher" + ); + std::thread::sleep(Duration::from_millis(10)); + } + } + #[test] fn only_fsevents_is_queued_in_production() { assert_eq!( diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 55d161aea6f38f..f4bf44f3f811e1 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -3347,12 +3347,16 @@ impl BackgroundScannerState { let (repository_dir_abs_path, common_dir_abs_path) = discover_git_paths(&dot_git_abs_path, fs).await; + // Awaited: the common and repository directories of a linked worktree + // usually sit outside the watched root, so their watches are new. watcher - .add(&common_dir_abs_path) + .add_and_wait(&common_dir_abs_path) + .await .context("failed to add common directory to watcher") .log_err(); watcher - .add(&repository_dir_abs_path) + .add_and_wait(&repository_dir_abs_path) + .await .context("failed to add repository directory to watcher") .log_err(); @@ -3361,7 +3365,8 @@ impl BackgroundScannerState { let reftable_path = common_dir_abs_path.join("reftable"); if fs.is_dir(&reftable_path).await { watcher - .add(&reftable_path) + .add_and_wait(&reftable_path) + .await .context("failed to add reftable directory to watcher") .log_err(); } @@ -4181,7 +4186,7 @@ impl BackgroundScanner { let state = self.state.lock().await; for target in state.symlink_paths_by_target.keys() { if !target.starts_with(root_abs_path.as_path()) { - self.watcher.add(target).log_err(); + self.watcher.add_and_wait(target).await.log_err(); } } for repo in state.snapshot.git_repositories.values() { @@ -4189,13 +4194,19 @@ impl BackgroundScanner { .common_dir_abs_path .starts_with(root_abs_path.as_path()) { - self.watcher.add(&repo.common_dir_abs_path).log_err(); + self.watcher + .add_and_wait(&repo.common_dir_abs_path) + .await + .log_err(); } if !repo .repository_dir_abs_path .starts_with(root_abs_path.as_path()) { - self.watcher.add(&repo.repository_dir_abs_path).log_err(); + self.watcher + .add_and_wait(&repo.repository_dir_abs_path) + .await + .log_err(); } } drop(state); @@ -5158,15 +5169,16 @@ impl BackgroundScanner { // the lock must not be held across the await point below. drop(state); let watched_abs_path: Option> = if job.is_external { - self.fs - .canonicalize(job.abs_path.as_ref()) - .await - .ok() - .map(|canonical| { + match self.fs.canonicalize(job.abs_path.as_ref()).await { + Ok(canonical) => { + // An external directory is outside the watched root, so its + // watch is new; wait for it before its entries are trusted. let canonical: Arc = canonical.into(); - self.watcher.add(&canonical).log_err(); - canonical - }) + self.watcher.add_and_wait(&canonical).await.log_err(); + Some(canonical) + } + Err(_) => None, + } } else { self.watcher.add(job.abs_path.as_ref()).log_err(); Some(job.abs_path.clone()) From 679edc3cf29b1f0302822f05faf43a2ea96e3a27 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Sat, 10 Oct 2026 12:04:24 -0400 Subject: [PATCH 03/21] workspace, project_panel: embedder hooks for window title, name validation, toast stack --- crates/project_panel/src/project_panel.rs | 33 +++++++++++++++++ crates/workspace/src/workspace.rs | 45 +++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 646c52568a5c98..7f51bc0d5dc901 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -185,6 +185,10 @@ impl Default for ProjectPanelContextMenuPolicy { } } +/// Host-supplied check of a name typed into the panel's rename / new-entry +/// editor: `Some(message)` refuses it, shown inline, and keeps the editor open. +pub type NameValidator = std::sync::Arc Option>; + pub struct ProjectPanel { project: Entity, fs: Arc, @@ -216,6 +220,7 @@ pub struct ProjectPanel { update_visible_entries_task: UpdateVisibleEntriesTask, undo_manager: UndoManager, context_menu_policy: ProjectPanelContextMenuPolicy, + name_validator: Option, state: State, } @@ -942,6 +947,7 @@ impl ProjectPanel { update_visible_entries_task: Default::default(), undo_manager: UndoManager::new(workspace.weak_handle(), weak_project_panel, &cx), context_menu_policy: ProjectPanelContextMenuPolicy::default(), + name_validator: None, }; this.update_visible_entries(None, false, false, window, cx); @@ -1823,6 +1829,16 @@ impl ProjectPanel { return; }; + if let Some(message) = self + .name_validator + .as_ref() + .and_then(|validate| validate(filename.as_unix_str())) + { + edit_state.validation_state = ValidationState::Error(message); + cx.notify(); + return; + } + if let Some(worktree) = self .project .read(cx) @@ -1883,6 +1899,15 @@ impl ProjectPanel { if filename.trim().is_empty() { return None; } + if let Some(message) = self + .name_validator + .as_ref() + .and_then(|validate| validate(filename.trim().trim_start_matches(['/', '\\']))) + { + edit_state.validation_state = ValidationState::Error(message); + cx.notify(); + return None; + } let filename_indicates_dir = if path_style.is_windows() { filename.ends_with('/') || filename.ends_with('\\') @@ -4010,6 +4035,14 @@ impl ProjectPanel { }) } + /// Installs a check run on every name typed into the rename and new-entry + /// editor, so a name the host cannot accept is refused inline instead of + /// failing after the editor closed. + pub fn set_name_validator(&mut self, validator: Option, cx: &mut Context) { + self.name_validator = validator; + cx.notify(); + } + /// Updates context-menu action visibility for this project-panel surface. /// /// Standalone Zed leaves the default policy in place; embedded hosts call diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 6ae8c36d1f0cf6..b1cecf173d71de 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1381,6 +1381,7 @@ pub struct Workspace { auto_watch: AutoWatch, window_edited: bool, last_window_title: Option, + manages_window_title: bool, dirty_items: HashMap, active_call: Option<(GlobalAnyActiveCall, Vec)>, leader_updates_tx: mpsc::UnboundedSender<(PeerId, proto::UpdateFollowers)>, @@ -1833,6 +1834,7 @@ impl Workspace { dispatching_keystrokes: Default::default(), window_edited: false, last_window_title: None, + manages_window_title: true, dirty_items: Default::default(), active_call, database_id: workspace_id, @@ -6157,7 +6159,16 @@ impl Workspace { self.apply_window_title(window, cx); } + /// Whether this workspace writes the OS window title. An embedder that owns + /// the window's chrome turns it off and sets the title itself. + pub fn set_manages_window_title(&mut self, manages: bool) { + self.manages_window_title = manages; + } + fn apply_window_title(&mut self, window: &mut Window, cx: &mut App) { + if !self.manages_window_title { + return; + } let project = self.project().read(cx); let mut title = String::new(); @@ -6297,6 +6308,12 @@ impl Workspace { } fn render_notifications(&self, _window: &mut Window, _cx: &mut Context) -> Option
{ + self.notification_stack() + } + + /// The toast stack, for an embedder that renders a bare workspace instead + /// of Zed's own window chrome. Without it every `show_error` is invisible. + pub fn notification_stack(&self) -> Option
{ if self.notifications.is_empty() { None } else { @@ -11395,6 +11412,34 @@ mod tests { assert_eq!(cx.window_title().as_deref(), Some("root2 — one.txt")); } + #[gpui::test] + async fn test_embedder_can_take_over_the_window_title(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root1", json!({ "one.txt": "" })).await; + let project = Project::test(fs, ["root1".as_ref()], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + workspace.update(cx, |workspace, _| workspace.set_manages_window_title(false)); + cx.update(|window, _| window.set_window_title("Embedder Title")); + + let item = cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new(1, "one.txt", cx)]) + }); + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(item), None, true, window, cx); + workspace.refresh_window_state(window, cx); + }); + assert_eq!(cx.window_title().as_deref(), Some("Embedder Title")); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.set_manages_window_title(true); + workspace.refresh_window_state(window, cx); + }); + assert_eq!(cx.window_title().as_deref(), Some("root1 — one.txt")); + } + #[gpui::test] async fn test_document_path_updates_with_active_item(cx: &mut TestAppContext) { init_test(cx); From 2a352a7527aa5b5f660a424485538d534e8cac96 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Sat, 10 Oct 2026 12:32:09 -0400 Subject: [PATCH 04/21] project, project_panel, search, git_ui: read-only local projects, embedded find bar, binary diff cards --- crates/git_ui/src/commit_diff_preview.rs | 204 ++++++++++++++++++++-- crates/project/src/buffer_store.rs | 16 ++ crates/project/src/project.rs | 32 ++++ crates/project_panel/src/project_panel.rs | 25 ++- crates/search/src/buffer_search.rs | 42 ++++- 5 files changed, 295 insertions(+), 24 deletions(-) diff --git a/crates/git_ui/src/commit_diff_preview.rs b/crates/git_ui/src/commit_diff_preview.rs index 67d4f9dd4ab761..fa8134a68129d7 100644 --- a/crates/git_ui/src/commit_diff_preview.rs +++ b/crates/git_ui/src/commit_diff_preview.rs @@ -3,7 +3,9 @@ use buffer_diff::BufferDiff; use editor::{Editor, EditorEvent, MultiBuffer}; use gpui::{ AnyElement, App, AppContext as _, AsyncApp, Context, Entity, EventEmitter, FocusHandle, - Focusable, Font, IntoElement, Render, SharedString, Task, Window, + Focusable, Font, ImageSource, InteractiveElement as _, IntoElement, ParentElement as _, Render, + SharedString, StatefulInteractiveElement as _, Styled as _, Task, Window, div, img, + prelude::FluentBuilder as _, px, }; use language::{ Buffer, Capability, DiskState, File as LanguageFile, HighlightedText, OffsetRangeExt, Point, @@ -18,7 +20,8 @@ use std::{ sync::Arc, time::Instant, }; -use ui::{Color, Icon, IconName, Label, LabelCommon as _}; +use theme::ActiveTheme as _; +use ui::{Color, Icon, IconName, Label, LabelCommon as _, h_flex, v_flex}; use util::ResultExt as _; use util::paths::PathStyle; use util::rel_path::RelPath; @@ -38,6 +41,22 @@ pub enum CommitDiffPreviewStatus { Typechange, } +/// One side of a binary change: its size, and the picture when it is an image +/// small enough to show. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct BinaryBlob { + pub size: u64, + pub image: Option>, +} + +/// Both sides of a binary file's change. An absent side did not exist (the file +/// was added or deleted). +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct BinaryPreview { + pub old: Option, + pub new: Option, +} + #[derive(Clone, Debug)] pub struct CommitDiffPreviewFile { pub display_path: Arc, @@ -47,6 +66,8 @@ pub struct CommitDiffPreviewFile { pub old_text: Option>, pub new_text: Option>, pub is_binary: bool, + /// What a binary file looked like before and after, when known. + pub binary: Option, pub is_truncated: bool, pub load_error: Option>, } @@ -72,9 +93,19 @@ pub struct CommitDiffPreview { editor: Entity, title: SharedString, file_count: usize, + /// Binary files, drawn as before/after cards above the text diff instead of + /// as fake text. + binary_cards: Vec, + has_text_entries: bool, _populate_task: Task>, } +struct BinaryCard { + display_path: Arc, + status: CommitDiffPreviewStatus, + binary: BinaryPreview, +} + struct PreviewEntry { index: usize, display_path: Arc, @@ -166,7 +197,8 @@ impl CommitDiffPreviewFile { status, old_text, new_text, - is_binary, + is_binary: _, + binary: _, is_truncated, load_error, } = self; @@ -187,17 +219,6 @@ impl CommitDiffPreviewFile { }; } - if is_binary { - return PreviewEntry { - index, - display_path: display_path.clone(), - metadata_path, - was_deleted, - old_text: None, - new_text: format!("Binary file: {display_path}\n").into(), - }; - } - if is_truncated { return PreviewEntry { index, @@ -276,11 +297,23 @@ impl CommitDiffPreview { }); let language_registry = project.read(cx).languages().clone(); - let entries = files + let (binary_files, text_files): (Vec<_>, Vec<_>) = files .into_iter() .enumerate() + .partition(|(_, file)| file.is_binary && file.load_error.is_none()); + let binary_cards = binary_files + .into_iter() + .map(|(_, file)| BinaryCard { + display_path: file.display_path, + status: file.status, + binary: file.binary.unwrap_or_default(), + }) + .collect::>(); + let entries = text_files + .into_iter() .map(|(index, file)| file.into_entry(index)) .collect::>(); + let has_text_entries = !entries.is_empty(); let context_lines = options.context_lines; let populate_task = cx.spawn(async move |_, cx| { populate_entries(multibuffer, entries, context_lines, language_registry, cx) @@ -294,6 +327,8 @@ impl CommitDiffPreview { editor, title: options.title, file_count, + binary_cards, + has_text_entries, _populate_task: populate_task, } } @@ -608,9 +643,114 @@ impl Item for CommitDiffPreview { } } +/// "12 B", "3.4 KB", "5.0 MB". +fn format_binary_size(bytes: u64) -> String { + const KB: f64 = 1024.0; + let size = bytes as f64; + if size < KB { + format!("{bytes} B") + } else if size < KB * KB { + format!("{:.1} KB", size / KB) + } else { + format!("{:.1} MB", size / (KB * KB)) + } +} + +fn status_label(status: &CommitDiffPreviewStatus) -> &'static str { + match status { + CommitDiffPreviewStatus::Added => "Added", + CommitDiffPreviewStatus::Deleted => "Deleted", + CommitDiffPreviewStatus::Modified => "Modified", + CommitDiffPreviewStatus::Renamed => "Renamed", + CommitDiffPreviewStatus::Copied => "Copied", + CommitDiffPreviewStatus::Typechange => "Type changed", + } +} + +/// What the "before" or "after" half of a binary card says when it has no +/// picture: the size, or that nothing was there. +fn binary_blob_caption(blob: Option<&BinaryBlob>) -> String { + match blob { + None => "\u{2014}".to_string(), + Some(blob) if blob.image.is_some() => format_binary_size(blob.size), + Some(blob) => format!("Binary, {} (no preview)", format_binary_size(blob.size)), + } +} + +fn render_binary_side( + label: &'static str, + blob: Option<&BinaryBlob>, + cx: &App, +) -> impl IntoElement { + let border = cx.theme().colors().border; + let picture = blob.and_then(|blob| blob.image.clone()).map(|image| { + div() + .border_1() + .border_color(border) + .child( + img(ImageSource::Image(image)) + .max_w(px(360.)) + .max_h(px(360.)), + ) + .into_any_element() + }); + v_flex() + .gap_1() + .child(Label::new(label).color(Color::Muted)) + .children(picture) + .child(Label::new(binary_blob_caption(blob))) +} + +fn render_binary_card(card: &BinaryCard, cx: &App) -> impl IntoElement { + let colors = cx.theme().colors(); + v_flex() + .gap_2() + .p_3() + .border_1() + .border_color(colors.border) + .rounded_md() + .child(Label::new(format!( + "{} \u{2014} {}", + card.display_path, + status_label(&card.status) + ))) + .child( + h_flex() + .gap_4() + .items_start() + .when( + card.binary.old.is_some() || card.binary.new.is_none(), + |row| row.child(render_binary_side("Before", card.binary.old.as_ref(), cx)), + ) + .when( + card.binary.new.is_some() || card.binary.old.is_none(), + |row| row.child(render_binary_side("After", card.binary.new.as_ref(), cx)), + ), + ) +} + impl Render for CommitDiffPreview { - fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { - self.editor.clone() + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + if self.binary_cards.is_empty() { + return self.editor.clone().into_any_element(); + } + let cards = self + .binary_cards + .iter() + .map(|card| render_binary_card(card, cx).into_any_element()) + .collect::>(); + v_flex() + .id("commit-diff-binary-cards") + .size_full() + .gap_3() + .p_3() + .bg(cx.theme().colors().editor_background) + .overflow_y_scroll() + .children(cards) + .when(self.has_text_entries, |this| { + this.child(div().flex_1().min_h(px(240.)).child(self.editor.clone())) + }) + .into_any_element() } } @@ -651,6 +791,7 @@ mod tests { old_text: Some("old".into()), new_text: Some("new".into()), is_binary: false, + binary: None, is_truncated: false, load_error: None, } @@ -672,4 +813,33 @@ mod tests { "crates/cherrypick-ui/src/app_shell.rs" ); } + + #[test] + fn binary_sizes_read_naturally() { + assert_eq!(format_binary_size(12), "12 B"); + assert_eq!(format_binary_size(3482), "3.4 KB"); + assert_eq!(format_binary_size(5 * 1024 * 1024), "5.0 MB"); + } + + #[test] + fn a_binary_side_says_what_it_has() { + let image = Arc::new(gpui::Image::from_bytes( + gpui::ImageFormat::Png, + vec![1, 2, 3], + )); + let shown = BinaryBlob { + size: 2048, + image: Some(image), + }; + let too_big = BinaryBlob { + size: 30 * 1024 * 1024, + image: None, + }; + assert_eq!(binary_blob_caption(None), "\u{2014}"); + assert_eq!(binary_blob_caption(Some(&shown)), "2.0 KB"); + assert_eq!( + binary_blob_caption(Some(&too_big)), + "Binary, 30.0 MB (no preview)" + ); + } } diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index f9076753998e93..ced0c3160be255 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -42,6 +42,9 @@ pub struct BufferStore { shared_buffers: HashMap>, non_searchable_buffers: HashSet, project_search: RemoteProjectSearchState, + /// Every buffer loaded from disk is read-only (an embedder showing a + /// snapshot or a remote tree it cannot write back to). + force_read_only: bool, } #[derive(Default)] @@ -684,6 +687,9 @@ impl LocalBufferStore { }; this.update(cx, |this, cx| { this.add_buffer(buffer.clone(), cx)?; + if this.force_read_only { + buffer.update(cx, |buffer, cx| buffer.set_capability(Capability::Read, cx)); + } let buffer_id = buffer.read(cx).remote_id(); if let Some(file) = File::from_dyn(buffer.read(cx).file()) { let project_path = ProjectPath { @@ -798,6 +804,7 @@ impl BufferStore { shared_buffers: Default::default(), loading_buffers: Default::default(), non_searchable_buffers: Default::default(), + force_read_only: false, worktree_store, project_search: Default::default(), } @@ -824,6 +831,7 @@ impl BufferStore { loading_buffers: Default::default(), shared_buffers: Default::default(), non_searchable_buffers: Default::default(), + force_read_only: false, worktree_store, project_search: Default::default(), } @@ -1010,6 +1018,14 @@ impl BufferStore { Ok(()) } + /// Make every buffer, open now or loaded later, read-only. + pub fn set_force_read_only(&mut self, cx: &mut Context) { + self.force_read_only = true; + for buffer in self.buffers().collect::>() { + buffer.update(cx, |buffer, cx| buffer.set_capability(Capability::Read, cx)); + } + } + pub fn buffers(&self) -> impl '_ + Iterator> { self.opened_buffers .values() diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 9ae67a85633287..e971e442a877e0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -232,6 +232,8 @@ pub struct Project { client_subscriptions: Vec, worktree_store: Entity, buffer_store: Entity, + /// A local project an embedder has made read-only (see `set_local_read_only`). + local_read_only: bool, context_server_store: Entity, image_store: Entity, lsp_store: Entity, @@ -1337,6 +1339,7 @@ impl Project { context_server_store, join_project_response_message_id: 0, client_state: ProjectClientState::Local, + local_read_only: false, git_store, client_subscriptions: Vec::new(), _subscriptions: vec![cx.on_release(Self::release)], @@ -1566,6 +1569,7 @@ impl Project { dap_store, join_project_response_message_id: 0, client_state: ProjectClientState::Local, + local_read_only: false, git_store, agent_server_store, client_subscriptions: Vec::new(), @@ -1881,6 +1885,7 @@ impl Project { remote_id, replica_id, }, + local_read_only: false, bookmark_store: bookmark_store.clone(), breakpoint_store: breakpoint_store.clone(), dap_store: dap_store.clone(), @@ -2540,6 +2545,9 @@ impl Project { is_directory: bool, cx: &mut Context, ) -> Task> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } let project_path = project_path.into(); let Some(worktree) = self.worktree_for_id(project_path.worktree_id, cx) else { return Task::ready(Err(anyhow!(format!( @@ -2558,6 +2566,9 @@ impl Project { new_project_path: ProjectPath, cx: &mut Context, ) -> Task>> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } self.worktree_store.update(cx, |worktree_store, cx| { worktree_store.copy_entry(entry_id, new_project_path, cx) }) @@ -2573,6 +2584,9 @@ impl Project { new_path: ProjectPath, cx: &mut Context, ) -> Task> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } let worktree_store = self.worktree_store.clone(); let Some((worktree, old_path, is_dir)) = worktree_store .read(cx) @@ -2652,6 +2666,9 @@ impl Project { trash: bool, cx: &mut Context, ) -> Option>>> { + if !self.capability().editable() { + return Some(Task::ready(Err(anyhow!("This project is read-only")))); + } let worktree = self.worktree_for_entry(entry_id, cx)?; cx.emit(Event::DeletedEntry(worktree.read(cx).id(), entry_id)); worktree.update(cx, |worktree, cx| { @@ -2959,10 +2976,25 @@ impl Project { pub fn capability(&self) -> Capability { match &self.client_state { ProjectClientState::Collab { capability, .. } => *capability, + ProjectClientState::Shared { .. } | ProjectClientState::Local + if self.local_read_only => + { + Capability::Read + } ProjectClientState::Shared { .. } | ProjectClientState::Local => Capability::ReadWrite, } } + /// Make this local project read-only: its buffers cannot be edited and + /// creating, renaming, copying or deleting entries is refused. For + /// embedders that show a snapshot or a tree they cannot write back to. + pub fn set_local_read_only(&mut self, cx: &mut Context) { + self.local_read_only = true; + self.buffer_store + .update(cx, |buffer_store, cx| buffer_store.set_force_read_only(cx)); + cx.notify(); + } + #[inline] pub fn is_read_only(&self, cx: &App) -> bool { self.is_disconnected(cx) || !self.capability().editable() diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 7f51bc0d5dc901..9614aba99ca11a 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -2151,6 +2151,9 @@ impl ProjectPanel { } fn add_entry(&mut self, is_dir: bool, window: &mut Window, cx: &mut Context) { + if self.project.read(cx).is_read_only(cx) { + return; + } let Some((worktree_id, entry_id)) = self .selection .map(|entry| (entry.worktree_id, entry.entry_id)) @@ -2244,11 +2247,17 @@ impl ProjectPanel { } } - pub fn undo(&mut self, _: &Undo, _window: &mut Window, _cx: &mut Context) { + pub fn undo(&mut self, _: &Undo, _window: &mut Window, cx: &mut Context) { + if self.project.read(cx).is_read_only(cx) { + return; + } self.undo_manager.undo().log_err(); } - pub fn redo(&mut self, _: &Redo, _window: &mut Window, _cx: &mut Context) { + pub fn redo(&mut self, _: &Redo, _window: &mut Window, cx: &mut Context) { + if self.project.read(cx).is_read_only(cx) { + return; + } self.undo_manager.redo().log_err(); } @@ -2258,6 +2267,9 @@ impl ProjectPanel { window: &mut Window, cx: &mut Context, ) { + if self.project.read(cx).is_read_only(cx) { + return; + } if let Some(SelectedEntry { worktree_id, entry_id, @@ -2527,6 +2539,9 @@ impl ProjectPanel { window: &mut Window, cx: &mut Context, ) { + if self.project.read(cx).is_read_only(cx) { + return; + } maybe!({ let items_to_delete = self.disjoint_effective_entries(cx); if items_to_delete.is_empty() { @@ -3200,6 +3215,9 @@ impl ProjectPanel { } fn cut(&mut self, _: &Cut, _: &mut Window, cx: &mut Context) { + if self.project.read(cx).is_read_only(cx) { + return; + } let entries = self.disjoint_effective_entries(cx); if !entries.is_empty() { self.write_entries_to_system_clipboard(&entries, cx); @@ -3282,6 +3300,9 @@ impl ProjectPanel { } fn paste(&mut self, _: &Paste, window: &mut Window, cx: &mut Context) { + if self.project.read(cx).is_read_only(cx) { + return; + } if let Some(external_paths) = self.external_paths_from_system_clipboard(cx) { let target_entry_id = self .selection diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 5e20f2e7ecf6e3..6c913d4f556fe8 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -97,6 +97,9 @@ pub struct BufferSearchBar { replace_enabled: bool, selection_search_enabled: Option, scroll_handle: ScrollHandle, + /// Sized by the toolbar it sits in rather than by the whole window (an + /// embedder that shows the bar in a pane narrower than the window). + embedded: bool, regex_language: Option>, splittable_editor: Option>, _splittable_editor_subscription: Option, @@ -343,11 +346,21 @@ impl Render for BufferSearchBar { let container_width = window.viewport_size().width; let input_width = SearchInputWidth::calc_width(container_width); - let input_base_styles = - |border_color| input_base_styles(border_color, |div| div.w(input_width)); + let embedded = self.embedded; + // Embedded: the input takes what the toolbar leaves (never below ~12 + // rem) and the option buttons wrap under it, instead of every width + // being derived from the window. + let size_input = move |div: Div| -> Div { + if embedded { + div.flex_1().min_w_48() + } else { + div.w(input_width) + } + }; + let input_base_styles = |border_color| input_base_styles(border_color, size_input); let input_style = if find_in_results { - filter_search_results_input(query_border, |div| div.w(input_width), cx) + filter_search_results_input(query_border, size_input, cx) } else { input_base_styles(query_border) }; @@ -387,7 +400,7 @@ impl Render for BufferSearchBar { let mode_column = h_flex() .gap_1() - .min_w_64() + .when(!embedded, |this| this.min_w_64()) .when(replacement, |this| { this.child(render_action_button( "buffer-search-bar-toggle", @@ -493,6 +506,7 @@ impl Render for BufferSearchBar { let search_line = h_flex() .w_full() .gap_2() + .when(embedded, |el| el.flex_wrap()) .when(find_in_results, |el| el.child(alignment_element())) .when(!find_in_results && has_collapse_button, |el| { el.pl_0p5().child(collapse_expand_button.expect("button")) @@ -510,7 +524,7 @@ impl Render for BufferSearchBar { let focus_handle = self.replacement_editor.read(cx).focus_handle(cx); let replace_actions = h_flex() - .min_w_64() + .when(!embedded, |this| this.min_w_64()) .gap_1() .child(render_action_button( "buffer-search-replace-button", @@ -854,6 +868,15 @@ impl BufferSearchBar { )); } + /// Size the bar from the toolbar it is shown in, not the window. For an + /// embedder whose editor pane is narrower than the window. + pub fn set_embedded(&mut self, embedded: bool, cx: &mut Context) { + if self.embedded != embedded { + self.embedded = embedded; + cx.notify(); + } + } + pub fn new( languages: Option>, window: &mut Window, @@ -929,6 +952,7 @@ impl BufferSearchBar { replace_enabled: false, selection_search_enabled: None, scroll_handle: ScrollHandle::new(), + embedded: false, regex_language: None, splittable_editor: None, _splittable_editor_subscription: None, @@ -1031,6 +1055,14 @@ impl BufferSearchBar { false } + /// Put the cursor in the query box if the bar is open. For an embedder + /// that opens the bar from a button, where a later click can move focus. + pub fn focus_query_editor(&mut self, window: &mut Window, cx: &mut Context) { + if !self.dismissed { + self.focus(&self.query_editor.focus_handle(cx), window, cx); + } + } + pub fn toggle(&mut self, action: &Deploy, window: &mut Window, cx: &mut Context) { if self.is_dismissed() { self.deploy(action, None, window, cx); From 7a5666bf7bb5f4b248d34f85522da4893ff16354 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Sat, 10 Oct 2026 17:38:03 -0400 Subject: [PATCH 05/21] project, editor, project_panel, git_ui: forced read-only buffers, save guard, raw-name validation, capped binary cards --- crates/editor/src/items.rs | 5 ++ crates/git_ui/src/commit_diff_preview.rs | 82 ++++++++++++++++++----- crates/project/src/buffer_store.rs | 10 ++- crates/project/src/project.rs | 8 ++- crates/project_panel/src/project_panel.rs | 35 +++++++--- 5 files changed, 110 insertions(+), 30 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 9a8682b07ac35d..52bee992da7476 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -928,6 +928,11 @@ impl Item for Editor { window: &mut Window, cx: &mut Context, ) -> Task> { + // A read-only project (a snapshot, a remote tree) has nothing to write: + // skip formatting and the save prompt entirely. + if project.read(cx).is_read_only(cx) { + return Task::ready(Ok(())); + } // Add meta data tracking # of auto saves if options.autosave { self.report_editor_event(ReportEditorEvent::Saved { auto_saved: true }, None, cx); diff --git a/crates/git_ui/src/commit_diff_preview.rs b/crates/git_ui/src/commit_diff_preview.rs index fa8134a68129d7..2cf49f6596608f 100644 --- a/crates/git_ui/src/commit_diff_preview.rs +++ b/crates/git_ui/src/commit_diff_preview.rs @@ -43,7 +43,7 @@ pub enum CommitDiffPreviewStatus { /// One side of a binary change: its size, and the picture when it is an image /// small enough to show. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Hash, PartialEq, Eq)] pub struct BinaryBlob { pub size: u64, pub image: Option>, @@ -51,12 +51,24 @@ pub struct BinaryBlob { /// Both sides of a binary file's change. An absent side did not exist (the file /// was added or deleted). -#[derive(Clone, Debug, Default, PartialEq, Eq)] +#[derive(Clone, Debug, Default, Hash, PartialEq, Eq)] pub struct BinaryPreview { pub old: Option, pub new: Option, } +impl BinaryPreview { + /// Bytes of picture data held for both sides (what a cache must account for). + pub fn image_bytes(&self) -> usize { + [&self.old, &self.new] + .into_iter() + .flatten() + .filter_map(|blob| blob.image.as_ref()) + .map(|image| image.bytes.len()) + .sum() + } +} + #[derive(Clone, Debug)] pub struct CommitDiffPreviewFile { pub display_path: Arc, @@ -100,12 +112,29 @@ pub struct CommitDiffPreview { _populate_task: Task>, } +/// The most binary cards drawn at once; the rest are counted in a closing line. +/// Cards are laid out and painted every frame, and a commit can touch hundreds +/// of binary files. +const MAX_BINARY_CARDS: usize = 40; + struct BinaryCard { - display_path: Arc, - status: CommitDiffPreviewStatus, + title: SharedString, + old_caption: SharedString, + new_caption: SharedString, binary: BinaryPreview, } +impl BinaryCard { + fn new(display_path: &str, status: &CommitDiffPreviewStatus, binary: BinaryPreview) -> Self { + Self { + title: format!("{display_path} \u{2014} {}", status_label(status)).into(), + old_caption: binary_blob_caption(binary.old.as_ref()).into(), + new_caption: binary_blob_caption(binary.new.as_ref()).into(), + binary, + } + } +} + struct PreviewEntry { index: usize, display_path: Arc, @@ -303,10 +332,12 @@ impl CommitDiffPreview { .partition(|(_, file)| file.is_binary && file.load_error.is_none()); let binary_cards = binary_files .into_iter() - .map(|(_, file)| BinaryCard { - display_path: file.display_path, - status: file.status, - binary: file.binary.unwrap_or_default(), + .map(|(_, file)| { + BinaryCard::new( + &file.display_path, + &file.status, + file.binary.unwrap_or_default(), + ) }) .collect::>(); let entries = text_files @@ -680,6 +711,7 @@ fn binary_blob_caption(blob: Option<&BinaryBlob>) -> String { fn render_binary_side( label: &'static str, blob: Option<&BinaryBlob>, + caption: &SharedString, cx: &App, ) -> impl IntoElement { let border = cx.theme().colors().border; @@ -698,7 +730,7 @@ fn render_binary_side( .gap_1() .child(Label::new(label).color(Color::Muted)) .children(picture) - .child(Label::new(binary_blob_caption(blob))) + .child(Label::new(caption.clone())) } fn render_binary_card(card: &BinaryCard, cx: &App) -> impl IntoElement { @@ -709,22 +741,32 @@ fn render_binary_card(card: &BinaryCard, cx: &App) -> impl IntoElement { .border_1() .border_color(colors.border) .rounded_md() - .child(Label::new(format!( - "{} \u{2014} {}", - card.display_path, - status_label(&card.status) - ))) + .child(Label::new(card.title.clone())) .child( h_flex() .gap_4() .items_start() .when( card.binary.old.is_some() || card.binary.new.is_none(), - |row| row.child(render_binary_side("Before", card.binary.old.as_ref(), cx)), + |row| { + row.child(render_binary_side( + "Before", + card.binary.old.as_ref(), + &card.old_caption, + cx, + )) + }, ) .when( card.binary.new.is_some() || card.binary.old.is_none(), - |row| row.child(render_binary_side("After", card.binary.new.as_ref(), cx)), + |row| { + row.child(render_binary_side( + "After", + card.binary.new.as_ref(), + &card.new_caption, + cx, + )) + }, ), ) } @@ -734,9 +776,11 @@ impl Render for CommitDiffPreview { if self.binary_cards.is_empty() { return self.editor.clone().into_any_element(); } + let hidden = self.binary_cards.len().saturating_sub(MAX_BINARY_CARDS); let cards = self .binary_cards .iter() + .take(MAX_BINARY_CARDS) .map(|card| render_binary_card(card, cx).into_any_element()) .collect::>(); v_flex() @@ -747,6 +791,12 @@ impl Render for CommitDiffPreview { .bg(cx.theme().colors().editor_background) .overflow_y_scroll() .children(cards) + .when(hidden > 0, |this| { + this.child( + Label::new(format!("\u{2026} and {hidden} more binary files")) + .color(Color::Muted), + ) + }) .when(self.has_text_entries, |this| { this.child(div().flex_1().min_h(px(240.)).child(self.editor.clone())) }) diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index ced0c3160be255..f011d760f77402 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -688,7 +688,9 @@ impl LocalBufferStore { this.update(cx, |this, cx| { this.add_buffer(buffer.clone(), cx)?; if this.force_read_only { - buffer.update(cx, |buffer, cx| buffer.set_capability(Capability::Read, cx)); + buffer.update(cx, |buffer, cx| { + buffer.set_capability(Capability::ReadOnly, cx) + }); } let buffer_id = buffer.read(cx).remote_id(); if let Some(file) = File::from_dyn(buffer.read(cx).file()) { @@ -705,7 +707,7 @@ impl LocalBufferStore { } else { settings.is_path_read_only(&project_path.path) }; - if is_read_only { + if is_read_only && !this.force_read_only { buffer.update(cx, |buffer, cx| { buffer.set_capability(Capability::Read, cx); }); @@ -1022,7 +1024,9 @@ impl BufferStore { pub fn set_force_read_only(&mut self, cx: &mut Context) { self.force_read_only = true; for buffer in self.buffers().collect::>() { - buffer.update(cx, |buffer, cx| buffer.set_capability(Capability::Read, cx)); + buffer.update(cx, |buffer, cx| { + buffer.set_capability(Capability::ReadOnly, cx) + }); } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e971e442a877e0..70edb49659d868 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2979,7 +2979,7 @@ impl Project { ProjectClientState::Shared { .. } | ProjectClientState::Local if self.local_read_only => { - Capability::Read + Capability::ReadOnly } ProjectClientState::Shared { .. } | ProjectClientState::Local => Capability::ReadWrite, } @@ -3323,6 +3323,9 @@ impl Project { } pub fn save_buffer(&self, buffer: Entity, cx: &mut Context) -> Task> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } self.buffer_store .update(cx, |buffer_store, cx| buffer_store.save_buffer(buffer, cx)) } @@ -3333,6 +3336,9 @@ impl Project { path: ProjectPath, cx: &mut Context, ) -> Task> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } self.buffer_store.update(cx, |buffer_store, cx| { buffer_store.save_buffer_as(buffer.clone(), path, cx) }) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 9614aba99ca11a..f53bc2935bb376 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1829,11 +1829,11 @@ impl ProjectPanel { return; }; - if let Some(message) = self - .name_validator - .as_ref() - .and_then(|validate| validate(filename.as_unix_str())) - { + if let Some(message) = Self::name_rejection( + self.name_validator.as_ref(), + filename.as_unix_str(), + PathStyle::Posix, + ) { edit_state.validation_state = ValidationState::Error(message); cx.notify(); return; @@ -1888,6 +1888,10 @@ impl ProjectPanel { let is_new_entry = edit_state.is_new_entry(); let mut filename = self.filename_editor.read(cx).text(cx); let path_style = self.project.read(cx).path_style(cx); + // The host judges what was typed, before trailing dots and spaces are + // quietly dropped below. + let host_rejection = + Self::name_rejection(self.name_validator.as_ref(), &filename, path_style); if path_style.is_windows() { // on windows, trailing dots are ignored in paths // this can cause project panel to create a new entry with a trailing dot @@ -1899,11 +1903,7 @@ impl ProjectPanel { if filename.trim().is_empty() { return None; } - if let Some(message) = self - .name_validator - .as_ref() - .and_then(|validate| validate(filename.trim().trim_start_matches(['/', '\\']))) - { + if let Some(message) = host_rejection { edit_state.validation_state = ValidationState::Error(message); cx.notify(); return None; @@ -4056,6 +4056,21 @@ impl ProjectPanel { }) } + /// The host validator's verdict on a typed name. Windows separators are + /// normalised first, so `src\\new.rs` is the nested path it means. + fn name_rejection( + validator: Option<&NameValidator>, + typed: &str, + path_style: PathStyle, + ) -> Option { + let validate = validator?; + if path_style.is_windows() { + validate(typed.replace('\\', "/").trim_start_matches('/')) + } else { + validate(typed.trim_start_matches('/')) + } + } + /// Installs a check run on every name typed into the rename and new-entry /// editor, so a name the host cannot accept is refused inline instead of /// failing after the editor closed. From 8797bad998a63dd739eda22fe03984de5eaeade0 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Sun, 11 Oct 2026 23:28:06 -0400 Subject: [PATCH 06/21] fs, project, editor, project_panel, git_ui, workspace: review fixes for read-only projects and queued watches A second add_and_wait for a path whose watch an earlier add queued (or a path a queued ancestor covers) now waits for that watch. A pending-path poll task cancelled mid-wait releases its registration. Forcing a project read-only keeps buffers with unsaved edits editable and refuses to save them, instead of reporting them clean; every buffer added afterwards is forced read-only. restore_entry and Discard Changes are refused in a read-only project. Re-enabling window title management rewrites the title. The inline name check uses the project's path style and the submitted name. Binary cards past the cap are counted, not held. --- crates/editor/src/items.rs | 18 +++- crates/fs/src/fs_watcher.rs | 100 ++++++++++++++++++---- crates/git_ui/src/commit_diff_preview.rs | 9 +- crates/project/src/buffer_store.rs | 14 ++- crates/project/src/project.rs | 3 + crates/project_panel/src/project_panel.rs | 15 ++-- crates/workspace/src/workspace.rs | 8 +- 7 files changed, 136 insertions(+), 31 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 52bee992da7476..878c720229ea8a 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -929,9 +929,23 @@ impl Item for Editor { cx: &mut Context, ) -> Task> { // A read-only project (a snapshot, a remote tree) has nothing to write: - // skip formatting and the save prompt entirely. + // skip formatting and the save prompt entirely. Edits made before the + // project became read-only cannot be written either, and saying so keeps + // the item from closing as if they had been saved. if project.read(cx).is_read_only(cx) { - return Task::ready(Ok(())); + let has_unsaved_edits = self + .buffer() + .read(cx) + .all_buffers() + .iter() + .any(|buffer| buffer.read(cx).is_dirty()); + return Task::ready(if has_unsaved_edits { + Err(anyhow!( + "This project is read-only, so these edits cannot be saved" + )) + } else { + Ok(()) + }); } // Add meta data tracking # of auto saves if options.autosave { diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index f75cfbd51e2fcf..821030c0bb6807 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -59,7 +59,8 @@ impl FsWatcher { && global_watcher().is_registered(existing.id) { log::trace!("path to watch is already watched: {path:?}"); - return None; + // Its OS watch may still be queued by an earlier `add`. + return global_watcher().applied_waiter(existing.id); } let (registration, applied) = register_existing_path( path, @@ -75,12 +76,12 @@ impl FsWatcher { log::trace!("watcher add: {path:?}"); let path: Arc = path.into(); - if path_covered_by_recursive_registration( - &self.registrations.lock(), - SanitizedPath::new(&path), - ) { + let covering = + recursive_registration_covering(&self.registrations.lock(), SanitizedPath::new(&path)); + if let Some(covering) = covering { log::trace!("path to watch is covered by an existing registration: {path:?}"); - return None; + // The covering watch may still be queued by an earlier `add`. + return global_watcher().applied_waiter(covering.id); } if self @@ -176,22 +177,25 @@ impl Watcher for FsWatcher { } } -/// Whether a recursive registration on a strict ancestor of `path` already covers -/// it. Both key spellings are probed so a folded registration still matches; only -/// poll watches and native macOS/Windows watches are recursive. -fn path_covered_by_recursive_registration( +/// The recursive registration on a strict ancestor of `path` that already covers +/// it, if any. Both key spellings are probed so a folded registration still +/// matches; only poll watches and native macOS/Windows watches are recursive. +fn recursive_registration_covering( registrations: &HashMap, path: &SanitizedPath, -) -> bool { - path.as_path().ancestors().skip(1).any(|ancestor| { +) -> Option { + path.as_path().ancestors().skip(1).find_map(|ancestor| { let ancestor = SanitizedPath::unchecked_new(ancestor); [WatchKey::exact(ancestor), WatchKey::folded(ancestor)] .iter() - .any(|key| { - registrations.get(key).is_some_and(|registration| { - registration.mode == WatcherMode::Poll - || cfg!(any(target_os = "windows", target_os = "macos")) - }) + .find_map(|key| { + registrations + .get(key) + .filter(|registration| { + registration.mode == WatcherMode::Poll + || cfg!(any(target_os = "windows", target_os = "macos")) + }) + .copied() }) }) } @@ -532,6 +536,9 @@ async fn poll_path_until_created( pending_path_events.clone(), ) { Some((registration, applied)) => { + // Removing the pending path drops this task, possibly while it + // waits below; the guard releases the registration then. + let mut unclaimed = UnclaimedRegistration(Some(registration.id)); if let Some(applied) = applied && let Ok(Err(error)) = applied.await { @@ -541,10 +548,10 @@ async fn poll_path_until_created( { let mut pending_registrations = pending_registrations.lock(); if pending_registrations.remove(path.as_ref()).is_none() { - global_watcher().remove(registration.id); return; } registrations.lock().insert(key, registration); + unclaimed.0 = None; } enqueue_path_events( &tx, @@ -567,6 +574,18 @@ async fn poll_path_until_created( } } +/// A global registration no `FsWatcher` owns yet. Dropping it removes the +/// registration, so a task cancelled mid-wait cannot leak a watch. +struct UnclaimedRegistration(Option); + +impl Drop for UnclaimedRegistration { + fn drop(&mut self) { + if let Some(id) = self.0.take() { + global_watcher().remove(id); + } + } +} + fn enqueue_path_events( tx: &smol::channel::Sender<()>, pending_path_events: &Arc>>, @@ -1248,6 +1267,24 @@ impl GlobalWatcher { Some(registration) } + /// A receiver for the OS watch behind registration `id` if it is still + /// queued, so a second `add_and_wait` for a path an earlier `add` already + /// registered waits for the same watch instead of returning at once. + fn applied_waiter(&self, id: WatcherRegistrationId) -> Option { + let mut state = self.state.lock(); + let (key, mode) = state + .watchers + .get(&id) + .map(|watcher| (watcher.key.clone(), watcher.mode))?; + let path_state = state.path_registrations(mode).get_mut(&key)?; + if path_state.waiters.is_empty() { + return None; + } + let (waiter, applied) = oneshot::channel(); + path_state.waiters.push(waiter); + Some(applied) + } + /// Whether `id` is still registered. A registration disappears when it is /// removed or when the OS watch it asked for failed. fn is_registered(&self, id: WatcherRegistrationId) -> bool { @@ -1822,6 +1859,33 @@ mod tests { ); } + #[test] + fn a_second_waiter_for_a_queued_registration_waits_for_the_same_watch() { + let backend = slow_backend(Duration::from_millis(200)); + let watcher = test_watcher(backend.clone()); + + let registration = watcher + .add(poll_path("/repo/root"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + // An earlier plain `add` dropped its receiver; a later `add_and_wait` + // for the same path must still wait for the watch. + drop(registration.applied); + let applied = watcher + .applied_waiter(registration.id) + .expect("the watch is still queued"); + + smol::block_on(applied) + .expect("not cancelled") + .expect("watch applied"); + assert!( + backend + .lock() + .watched_paths + .contains(&PathBuf::from("/repo/root")), + "the second waiter resolved before the backend watched the path" + ); + } + #[test] fn applied_reports_a_failed_watch_and_rolls_it_back() { let backend = Arc::new(Mutex::new(FakeWatchBackend { diff --git a/crates/git_ui/src/commit_diff_preview.rs b/crates/git_ui/src/commit_diff_preview.rs index 2cf49f6596608f..9b17d9be0ed427 100644 --- a/crates/git_ui/src/commit_diff_preview.rs +++ b/crates/git_ui/src/commit_diff_preview.rs @@ -108,6 +108,9 @@ pub struct CommitDiffPreview { /// Binary files, drawn as before/after cards above the text diff instead of /// as fake text. binary_cards: Vec, + /// Binary files past [`MAX_BINARY_CARDS`]: counted, never held, so their + /// image data is not kept for cards that are not drawn. + hidden_binary_files: usize, has_text_entries: bool, _populate_task: Task>, } @@ -330,8 +333,10 @@ impl CommitDiffPreview { .into_iter() .enumerate() .partition(|(_, file)| file.is_binary && file.load_error.is_none()); + let hidden_binary_files = binary_files.len().saturating_sub(MAX_BINARY_CARDS); let binary_cards = binary_files .into_iter() + .take(MAX_BINARY_CARDS) .map(|(_, file)| { BinaryCard::new( &file.display_path, @@ -359,6 +364,7 @@ impl CommitDiffPreview { title: options.title, file_count, binary_cards, + hidden_binary_files, has_text_entries, _populate_task: populate_task, } @@ -776,11 +782,10 @@ impl Render for CommitDiffPreview { if self.binary_cards.is_empty() { return self.editor.clone().into_any_element(); } - let hidden = self.binary_cards.len().saturating_sub(MAX_BINARY_CARDS); + let hidden = self.hidden_binary_files; let cards = self .binary_cards .iter() - .take(MAX_BINARY_CARDS) .map(|card| render_binary_card(card, cx).into_any_element()) .collect::>(); v_flex() diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index f011d760f77402..93a17877b04fcc 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -967,6 +967,11 @@ impl BufferStore { } fn add_buffer(&mut self, buffer_entity: Entity, cx: &mut Context) -> Result<()> { + if self.force_read_only { + buffer_entity.update(cx, |buffer, cx| { + buffer.set_capability(Capability::ReadOnly, cx) + }); + } let buffer = buffer_entity.read(cx); let remote_id = buffer.remote_id(); let path = File::from_dyn(buffer.file()).map(|file| ProjectPath { @@ -1020,12 +1025,17 @@ impl BufferStore { Ok(()) } - /// Make every buffer, open now or loaded later, read-only. + /// Make every buffer, open now or created or loaded later, read-only. A + /// buffer that already has unsaved edits keeps its capability: `ReadOnly` + /// buffers report themselves clean, so flipping it would let the editor close + /// without asking and lose the edits. Saving it is refused instead. pub fn set_force_read_only(&mut self, cx: &mut Context) { self.force_read_only = true; for buffer in self.buffers().collect::>() { buffer.update(cx, |buffer, cx| { - buffer.set_capability(Capability::ReadOnly, cx) + if !buffer.is_dirty() { + buffer.set_capability(Capability::ReadOnly, cx) + } }); } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 70edb49659d868..c82f991c11a1ce 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2683,6 +2683,9 @@ impl Project { trash_entry: TrashedEntry, cx: &mut Context<'_, Self>, ) -> Task> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } let Some(worktree) = self.worktree_for_id(worktree_id, cx) else { return Task::ready(Err(anyhow!("No worktree for id {worktree_id:?}"))); }; diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index f53bc2935bb376..f2e13f16cd0522 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1798,6 +1798,7 @@ impl ProjectPanel { } fn populate_validation_error(&mut self, cx: &mut Context) { + let path_style = self.project.read(cx).path_style(cx); let edit_state = match self.state.edit_state.as_mut() { Some(state) => state, None => return, @@ -1829,11 +1830,11 @@ impl ProjectPanel { return; }; - if let Some(message) = Self::name_rejection( - self.name_validator.as_ref(), - filename.as_unix_str(), - PathStyle::Posix, - ) { + // The same name and path style Enter validates with, so the inline + // error and the confirmation never disagree. + if let Some(message) = + Self::name_rejection(self.name_validator.as_ref(), trimmed_filename, path_style) + { edit_state.validation_state = ValidationState::Error(message); cx.notify(); return; @@ -2347,6 +2348,10 @@ impl ProjectPanel { window: &mut Window, cx: &mut Context, ) { + // Discarding changes checks files out into the working tree. + if self.project.read(cx).is_read_only(cx) { + return; + } maybe!({ let selection = self.selection?; let project = self.project.read(cx); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index b1cecf173d71de..ed0aa151bb3c7e 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -6159,10 +6159,14 @@ impl Workspace { self.apply_window_title(window, cx); } - /// Whether this workspace writes the OS window title. An embedder that owns - /// the window's chrome turns it off and sets the title itself. + /// Whether this workspace writes the OS window title and the window's + /// document path. An embedder that owns the window's chrome turns it off and + /// sets both itself. Turning it back on forgets the last title written, so + /// the next update rewrites the window even if the computed title is the + /// same as before the embedder changed it. pub fn set_manages_window_title(&mut self, manages: bool) { self.manages_window_title = manages; + self.last_window_title = None; } fn apply_window_title(&mut self, window: &mut Window, cx: &mut App) { From d98dd0365feeb93744ee3df286b33539b84cca4a Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Sun, 11 Oct 2026 23:48:30 -0400 Subject: [PATCH 07/21] fs, project: wait for covering watches across watchers; refuse buffer-store saves when forced read-only A path covered by an ancestor watch another FsWatcher queued now waits for that watch, and a local covering record whose watch failed no longer suppresses descendants. BufferStore::save_buffer and save_buffer_as refuse when the project was forced read-only, so staging cannot write buffers behind the project-level guards. --- crates/fs/src/fs_watcher.rs | 67 +++++++++++++++++++++++++++++- crates/project/src/buffer_store.rs | 6 +++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 821030c0bb6807..061c4b4bd8c6e6 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -195,6 +195,9 @@ fn recursive_registration_covering( registration.mode == WatcherMode::Poll || cfg!(any(target_os = "windows", target_os = "macos")) }) + // A watch that failed is gone from the global watcher; the + // local record of it must not keep covering its descendants. + .filter(|registration| global_watcher().is_registered(registration.id)) .copied() }) }) @@ -790,6 +793,29 @@ impl WatchPaths { }) } + /// The state of the nearest recursive ancestor registration of `path` whose + /// OS watch is still queued, so a covered path can wait for that watch. + fn pending_covering_state_mut( + &mut self, + path: &SanitizedPath, + mode: WatcherMode, + ) -> Option<&mut PathRegistrationState> { + if mode != WatcherMode::Poll && !cfg!(any(target_os = "windows", target_os = "macos")) { + return None; + } + let key = path.as_path().ancestors().skip(1).find_map(|ancestor| { + let ancestor = SanitizedPath::unchecked_new(ancestor); + [WatchKey::exact(ancestor), WatchKey::folded(ancestor)] + .into_iter() + .find(|key| { + self.0 + .get(key) + .is_some_and(|state| !state.waiters.is_empty()) + }) + })?; + self.0.get_mut(&key) + } + /// Collects the watcher ids of every registration whose directory is an /// ancestor of (or equal to) `path`. Both exact and folded keys are probed, /// so a real-cased event path matches a folded registration and vice versa. @@ -1246,7 +1272,18 @@ impl GlobalWatcher { }, }); let Some(epoch) = os_watch else { - return Some(Registration { id, applied: None }); + // A recursive ancestor's watch covers this path. If that watch is + // still queued, wait for it too: until it is in place nothing is + // watching the path, whichever `FsWatcher` asked for the ancestor. + let (ancestor_waiter, ancestor_applied) = oneshot::channel(); + let applied = state + .path_registrations(mode) + .pending_covering_state_mut(&path, mode) + .map(|ancestor| { + ancestor.waiters.push(ancestor_waiter); + ancestor_applied + }); + return Some(Registration { id, applied }); }; let command = WatchCommand { op: WatchOp::Watch, @@ -1886,6 +1923,34 @@ mod tests { ); } + #[test] + fn a_path_covered_by_a_queued_ancestor_waits_for_the_ancestors_watch() { + let backend = slow_backend(Duration::from_millis(200)); + let watcher = test_watcher(backend.clone()); + + let _ancestor = watcher + .add(poll_path("/repo"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + // Another `FsWatcher` asks for a child while the ancestor's watch is + // still queued: there is no OS watch of its own, but it must not report + // ready before the ancestor's is in place. + let child = watcher + .add(poll_path("/repo/sub"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let applied = child.applied.expect("waits for the covering watch"); + + smol::block_on(applied) + .expect("not cancelled") + .expect("watch applied"); + assert!( + backend + .lock() + .watched_paths + .contains(&PathBuf::from("/repo")), + "the child resolved before the ancestor was watched" + ); + } + #[test] fn applied_reports_a_failed_watch_and_rolls_it_back() { let backend = Arc::new(Mutex::new(FakeWatchBackend { diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index 93a17877b04fcc..da18bf5ea39fd7 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -932,6 +932,9 @@ impl BufferStore { buffer: Entity, cx: &mut Context, ) -> Task> { + if self.force_read_only { + return Task::ready(Err(anyhow!("This project is read-only"))); + } match &mut self.state { BufferStoreState::Local(this) => this.save_buffer(buffer, cx), BufferStoreState::Remote(this) => this.save_remote_buffer(buffer, None, cx), @@ -944,6 +947,9 @@ impl BufferStore { path: ProjectPath, cx: &mut Context, ) -> Task> { + if self.force_read_only { + return Task::ready(Err(anyhow!("This project is read-only"))); + } let old_file = buffer.read(cx).file().cloned(); let task = match &self.state { BufferStoreState::Local(this) => this.save_buffer_as(buffer.clone(), path, cx), From 730de0338716fcd92e2fea4abe87d2b143c72782 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 00:04:23 -0400 Subject: [PATCH 08/21] fs, project, project_panel: covered watches follow their ancestor, external drops refused when read-only A repeat registration of a path an ancestor covers waits for the ancestor's queued watch. When a watch fails, the paths it covered are unregistered so their owners retry and get a watch of their own, and a poll task no longer trusts a local record whose watch failed. File drops into a read-only project are ignored. set_local_read_only documents that it is a UI policy for projects on a read-only filesystem. --- crates/fs/src/fs_watcher.rs | 45 ++++++++++++++++++++++- crates/project/src/project.rs | 12 +++++- crates/project_panel/src/project_panel.rs | 4 ++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 061c4b4bd8c6e6..6e7c0593da525f 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -527,7 +527,12 @@ async fn poll_path_until_created( let case_insensitive = case_insensitive_path(path.as_ref()); let key = WatchKey::for_registration(SanitizedPath::new(&path), case_insensitive); - if registrations.lock().contains_key(&key) { + // A record whose watch has since failed does not count: register again. + if registrations + .lock() + .get(&key) + .is_some_and(|existing| global_watcher().is_registered(existing.id)) + { pending_registrations.lock().remove(path.as_ref()); return; } @@ -911,6 +916,29 @@ impl WatcherState { for id in path_state.watcher_ids { self.watchers.remove(&id); } + // Paths this watch covered have no OS watch of their own. Drop them + // too, so their owners see them unregistered and retry, which now + // installs a watch for each. + let covered: Vec<_> = self + .watchers + .iter() + .filter(|(_, watcher)| { + watcher.mode == command.mode + && watcher.path.as_path() != command.path.as_path() + && watcher.path.as_path().starts_with(command.path.as_path()) + }) + .map(|(id, watcher)| (*id, watcher.key.clone())) + .collect(); + for (id, key) in covered { + let paths = self.path_registrations(command.mode); + if paths + .get_mut(&key) + .is_some_and(|state| state.os_watch.is_none()) + { + paths.remove(&key); + self.watchers.remove(&id); + } + } } for waiter in waiters { let outcome = match result { @@ -1250,11 +1278,26 @@ impl GlobalWatcher { .get_mut(&key) .expect("registered path has state"); path_state.watcher_ids.push(id); + // A covered path has no OS watch of its own: its readiness is its + // ancestor's. + let covered_by_ancestor = path_state.os_watch.is_none(); // The path's OS watch is still queued: wait for the same one. let applied = (!path_state.waiters.is_empty()).then(|| { path_state.waiters.push(waiter); applied }); + let applied = if applied.is_none() && covered_by_ancestor { + let (ancestor_waiter, ancestor_applied) = oneshot::channel(); + state + .path_registrations(mode) + .pending_covering_state_mut(&path, mode) + .map(|ancestor| { + ancestor.waiters.push(ancestor_waiter); + ancestor_applied + }) + } else { + applied + }; return Some(Registration { id, applied }); } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index c82f991c11a1ce..463ca7c647b9d7 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2989,8 +2989,16 @@ impl Project { } /// Make this local project read-only: its buffers cannot be edited and - /// creating, renaming, copying or deleting entries is refused. For - /// embedders that show a snapshot or a tree they cannot write back to. + /// creating, renaming, copying, deleting or restoring entries, saving, and + /// dropping files in are refused. For embedders that show a snapshot or a + /// tree they cannot write back to. + /// + /// This is a policy for the UI, not a security boundary: it is meant for a + /// project whose filesystem is itself read-only (a virtual one), which + /// rejects whatever a language server, a task or another embedder-owned + /// path writes. A workspace restoring unsaved editor contents from a + /// database is not supported on a read-only project (read-only buffers + /// report themselves clean). pub fn set_local_read_only(&mut self, cx: &mut Context) { self.local_read_only = true; self.buffer_store diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index f2e13f16cd0522..5e41adc861a41f 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -4596,6 +4596,10 @@ impl ProjectPanel { window: &mut Window, cx: &mut Context, ) { + // Dropping files in copies them into the project's tree. + if self.project.read(cx).is_read_only(cx) { + return; + } let mut paths: Vec> = paths.iter().map(|path| Arc::from(path.clone())).collect(); let open_file_after_drop = paths.len() == 1 && paths[0].is_file(); From a7d46d848713babbb8a9b6edb4ee48d1af2335ce Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 00:18:54 -0400 Subject: [PATCH 09/21] fs, project, editor: applied_waiter distinguishes a vanished registration; dirty buffers become Read; autosave stays quiet when read-only applied_waiter reports a missing registration separately from an applied watch, and callers register again when it vanished. Buffers with unsaved edits become Capability::Read when a project is forced read-only: not editable, still dirty, so closing still asks. An autosave tick in a read-only project with unsaved edits no longer fails every time; an explicit save still says so. --- crates/editor/src/items.rs | 4 ++- crates/fs/src/fs_watcher.rs | 39 ++++++++++++++++++------------ crates/project/src/buffer_store.rs | 18 +++++++++----- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 878c720229ea8a..32c9588596d06c 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -939,7 +939,9 @@ impl Item for Editor { .all_buffers() .iter() .any(|buffer| buffer.read(cx).is_dirty()); - return Task::ready(if has_unsaved_edits { + // An autosave tick cannot do anything about it and must not fail + // on every tick; an explicit save says so. + return Task::ready(if has_unsaved_edits && !options.autosave { Err(anyhow!( "This project is read-only, so these edits cannot be saved" )) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 6e7c0593da525f..0ac2074e73564f 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -55,12 +55,13 @@ impl FsWatcher { let key = WatchKey::for_registration(SanitizedPath::new(&path), case_insensitive); // A registration whose OS watch failed is gone from the global watcher; // adding the path again retries it. - if let Some(existing) = self.registrations.lock().get(&key) - && global_watcher().is_registered(existing.id) + let existing = self.registrations.lock().get(&key).copied(); + if let Some(existing) = existing + && let Some(applied) = global_watcher().applied_waiter(existing.id) { log::trace!("path to watch is already watched: {path:?}"); // Its OS watch may still be queued by an earlier `add`. - return global_watcher().applied_waiter(existing.id); + return applied; } let (registration, applied) = register_existing_path( path, @@ -78,10 +79,12 @@ impl FsWatcher { let path: Arc = path.into(); let covering = recursive_registration_covering(&self.registrations.lock(), SanitizedPath::new(&path)); - if let Some(covering) = covering { + if let Some(covering) = covering + && let Some(applied) = global_watcher().applied_waiter(covering.id) + { log::trace!("path to watch is covered by an existing registration: {path:?}"); // The covering watch may still be queued by an earlier `add`. - return global_watcher().applied_waiter(covering.id); + return applied; } if self @@ -1347,21 +1350,26 @@ impl GlobalWatcher { Some(registration) } - /// A receiver for the OS watch behind registration `id` if it is still - /// queued, so a second `add_and_wait` for a path an earlier `add` already - /// registered waits for the same watch instead of returning at once. - fn applied_waiter(&self, id: WatcherRegistrationId) -> Option { + /// For registration `id`: `None` when it no longer exists (removed, or its + /// watch failed), `Some(None)` when its OS watch is already in place, and + /// `Some(Some(receiver))` while that watch is still queued. Both facts come + /// from one lock, so a caller can tell a vanished registration from an + /// applied one and register again. + fn applied_waiter(&self, id: WatcherRegistrationId) -> Option> { let mut state = self.state.lock(); let (key, mode) = state .watchers .get(&id) .map(|watcher| (watcher.key.clone(), watcher.mode))?; - let path_state = state.path_registrations(mode).get_mut(&key)?; - if path_state.waiters.is_empty() { - return None; - } - let (waiter, applied) = oneshot::channel(); - path_state.waiters.push(waiter); + let applied = state + .path_registrations(mode) + .get_mut(&key) + .filter(|path_state| !path_state.waiters.is_empty()) + .map(|path_state| { + let (waiter, applied) = oneshot::channel(); + path_state.waiters.push(waiter); + applied + }); Some(applied) } @@ -1952,6 +1960,7 @@ mod tests { drop(registration.applied); let applied = watcher .applied_waiter(registration.id) + .expect("the registration exists") .expect("the watch is still queued"); smol::block_on(applied) diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index da18bf5ea39fd7..126b49b8798295 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -1032,16 +1032,22 @@ impl BufferStore { } /// Make every buffer, open now or created or loaded later, read-only. A - /// buffer that already has unsaved edits keeps its capability: `ReadOnly` - /// buffers report themselves clean, so flipping it would let the editor close - /// without asking and lose the edits. Saving it is refused instead. + /// buffer that already has unsaved edits becomes `Read` (not editable, but + /// still dirty): `ReadOnly` buffers report themselves clean, so flipping it + /// would let the editor close without asking and lose the edits. Saving it + /// is refused. pub fn set_force_read_only(&mut self, cx: &mut Context) { self.force_read_only = true; for buffer in self.buffers().collect::>() { buffer.update(cx, |buffer, cx| { - if !buffer.is_dirty() { - buffer.set_capability(Capability::ReadOnly, cx) - } + // `Read` stops editing but, unlike `ReadOnly`, keeps reporting + // unsaved edits, so closing still asks. + let capability = if buffer.is_dirty() { + Capability::Read + } else { + Capability::ReadOnly + }; + buffer.set_capability(capability, cx) }); } } From 5013d7c4517af177c8b415de3c981069edcc1845 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 00:32:09 -0400 Subject: [PATCH 10/21] fs, editor: covered registrations wait on their ancestor, failed-watch cleanup removes every registration and spares independently covered paths; read-only projects cannot be unlocked per file applied_waiter follows a covered path to the ancestor watch covering it. When a watch fails, the paths it covered are removed with all of their registrations (matched through exact and folded keys) unless another watch with its own OS watch still covers them. Toggling read-only on a buffer cannot restore write capability while the project is read-only. --- crates/editor/src/editor.rs | 6 +++ crates/editor/src/items.rs | 6 +++ crates/fs/src/fs_watcher.rs | 84 +++++++++++++++++++++++++------------ 3 files changed, 69 insertions(+), 27 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 1c8709fed86b78..b3feb7e0ca97a1 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5738,11 +5738,17 @@ impl Editor { _: &mut Window, cx: &mut Context, ) { + // A project an embedder made read-only cannot be unlocked per file. + let project_read_only = self + .project + .as_ref() + .is_some_and(|project| project.read(cx).is_read_only(cx)); if let Some(buffer) = self.buffer.read(cx).as_singleton() { buffer.update(cx, |buffer, cx| { buffer.set_capability( match buffer.capability() { Capability::ReadWrite => Capability::Read, + Capability::Read if project_read_only => Capability::Read, Capability::Read => Capability::ReadWrite, Capability::ReadOnly => Capability::ReadOnly, }, diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 32c9588596d06c..8dd1c8235a2fb8 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -888,11 +888,17 @@ impl Item for Editor { // Note: this mirrors the logic in `Editor::toggle_read_only`, but is reachable // without relying on focus-based action dispatch. fn toggle_read_only(&mut self, window: &mut Window, cx: &mut Context) { + // A project an embedder made read-only cannot be unlocked per file. + let project_read_only = self + .project + .as_ref() + .is_some_and(|project| project.read(cx).is_read_only(cx)); if let Some(buffer) = self.buffer.read(cx).as_singleton() { buffer.update(cx, |buffer, cx| { buffer.set_capability( match buffer.capability() { Capability::ReadWrite => Capability::Read, + Capability::Read if project_read_only => Capability::Read, Capability::Read => Capability::ReadWrite, Capability::ReadOnly => Capability::ReadOnly, }, diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 0ac2074e73564f..0779d7ddf17aee 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -801,6 +801,20 @@ impl WatchPaths { }) } + /// Whether a strict ancestor of `path` has an OS watch of its own. + fn has_own_watch_ancestor(&self, path: &SanitizedPath) -> bool { + path.as_path().ancestors().skip(1).any(|ancestor| { + let ancestor = SanitizedPath::unchecked_new(ancestor); + [WatchKey::exact(ancestor), WatchKey::folded(ancestor)] + .iter() + .any(|key| { + self.0 + .get(key) + .is_some_and(|state| state.os_watch.is_some()) + }) + }) + } + /// The state of the nearest recursive ancestor registration of `path` whose /// OS watch is still queued, so a covered path can wait for that watch. fn pending_covering_state_mut( @@ -919,27 +933,34 @@ impl WatcherState { for id in path_state.watcher_ids { self.watchers.remove(&id); } - // Paths this watch covered have no OS watch of their own. Drop them - // too, so their owners see them unregistered and retry, which now + // Paths this watch covered have no OS watch of their own, unless + // another watch still covers them. Drop the rest, every registration + // of each, so their owners see them unregistered and retry, which // installs a watch for each. - let covered: Vec<_> = self - .watchers - .iter() - .filter(|(_, watcher)| { - watcher.mode == command.mode - && watcher.path.as_path() != command.path.as_path() - && watcher.path.as_path().starts_with(command.path.as_path()) - }) - .map(|(id, watcher)| (*id, watcher.key.clone())) - .collect(); - for (id, key) in covered { + let mut affected: Vec<(WatchKey, Arc)> = Vec::new(); + for watcher in self.watchers.values() { + let under_failed_path = watcher.mode == command.mode + && watcher.path.as_path().ancestors().skip(1).any(|ancestor| { + let ancestor = SanitizedPath::unchecked_new(ancestor); + WatchKey::exact(ancestor) == command.key + || WatchKey::folded(ancestor) == command.key + }); + if under_failed_path && !affected.iter().any(|(key, _)| *key == watcher.key) { + affected.push((watcher.key.clone(), watcher.path.clone())); + } + } + for (key, path) in affected { let paths = self.path_registrations(command.mode); - if paths + let covered_only = paths .get_mut(&key) - .is_some_and(|state| state.os_watch.is_none()) + .is_some_and(|state| state.os_watch.is_none()); + if covered_only + && !paths.has_own_watch_ancestor(&path) + && let Some(state) = paths.remove(&key) { - paths.remove(&key); - self.watchers.remove(&id); + for id in state.watcher_ids { + self.watchers.remove(&id); + } } } } @@ -1357,19 +1378,28 @@ impl GlobalWatcher { /// applied one and register again. fn applied_waiter(&self, id: WatcherRegistrationId) -> Option> { let mut state = self.state.lock(); - let (key, mode) = state + let (key, mode, path) = state .watchers .get(&id) - .map(|watcher| (watcher.key.clone(), watcher.mode))?; - let applied = state - .path_registrations(mode) + .map(|watcher| (watcher.key.clone(), watcher.mode, watcher.path.clone()))?; + let paths = state.path_registrations(mode); + // A covered path has no OS watch of its own: its readiness is that of + // the ancestor watch covering it. + let covered = paths .get_mut(&key) - .filter(|path_state| !path_state.waiters.is_empty()) - .map(|path_state| { - let (waiter, applied) = oneshot::channel(); - path_state.waiters.push(waiter); - applied - }); + .is_some_and(|path_state| path_state.os_watch.is_none()); + let pending = if covered { + paths.pending_covering_state_mut(&path, mode) + } else { + paths + .get_mut(&key) + .filter(|path_state| !path_state.waiters.is_empty()) + }; + let applied = pending.map(|path_state| { + let (waiter, applied) = oneshot::channel(); + path_state.waiters.push(waiter); + applied + }); Some(applied) } From 8b0db55caf6ca9a227a0edc0c1e99dac245b9475 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 01:09:48 -0400 Subject: [PATCH 11/21] fs: removing an ancestor watch gives its covered registrations a watch of their own When the last registration on a watched path is removed, registered paths that watch was covering and that nothing else covers are promoted to their own OS watch, shortest first, instead of being left registered and unwatched. --- crates/fs/src/fs_watcher.rs | 114 ++++++++++++++++++++++++++++++------ 1 file changed, 97 insertions(+), 17 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 0779d7ddf17aee..2fe177c84c692f 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -893,25 +893,77 @@ impl WatcherState { } } - /// Drops a registration and, when it was the last one on a path that owns - /// an OS watch, returns the unwatch command that releases it. - fn remove_registration(&mut self, id: WatcherRegistrationId) -> Option { - let registration_state = self.watchers.remove(&id)?; - let path_registrations = self.path_registrations(registration_state.mode); - let path_state = path_registrations.get_mut(®istration_state.key)?; + /// Drops a registration. When it was the last one on a path that owns an OS + /// watch, returns the unwatch command that releases it, followed by a watch + /// command for each path that watch was covering and that nothing else + /// covers: those registrations stay, so they need a watch of their own. + fn remove_registration(&mut self, id: WatcherRegistrationId) -> Vec { + let Some(registration_state) = self.watchers.remove(&id) else { + return Vec::new(); + }; + let mode = registration_state.mode; + let paths = self.path_registrations(mode); + let Some(path_state) = paths.get_mut(®istration_state.key) else { + return Vec::new(); + }; path_state.watcher_ids.retain(|&existing| existing != id); if !path_state.watcher_ids.is_empty() { - return None; + return Vec::new(); } - - let path_state = path_registrations.remove(®istration_state.key)?; - Some(WatchCommand { + let Some(path_state) = paths.remove(®istration_state.key) else { + return Vec::new(); + }; + let Some(epoch) = path_state.os_watch else { + return Vec::new(); + }; + let mut commands = vec![WatchCommand { op: WatchOp::Unwatch, - mode: registration_state.mode, - path: registration_state.path, - key: registration_state.key, - epoch: path_state.os_watch?, - }) + mode, + path: registration_state.path.clone(), + key: registration_state.key.clone(), + epoch, + }]; + commands.extend(self.promote_covered(®istration_state.key, mode)); + commands + } + + /// Gives an OS watch to every registered path that the watch on `released` + /// was covering and that no other watch covers, shortest paths first so a + /// path under another promoted one stays covered by it. + fn promote_covered(&mut self, released: &WatchKey, mode: WatcherMode) -> Vec { + let mut covered: Vec<(WatchKey, Arc)> = Vec::new(); + for watcher in self.watchers.values() { + let under_released = watcher.mode == mode + && watcher.path.as_path().ancestors().skip(1).any(|ancestor| { + let ancestor = SanitizedPath::unchecked_new(ancestor); + WatchKey::exact(ancestor) == *released + || WatchKey::folded(ancestor) == *released + }); + if under_released && !covered.iter().any(|(key, _)| *key == watcher.key) { + covered.push((watcher.key.clone(), watcher.path.clone())); + } + } + covered.sort_by_key(|(_, path)| path.as_path().components().count()); + let mut commands = Vec::new(); + for (key, path) in covered { + let epoch = self.next_epoch(); + let paths = self.path_registrations(mode); + let needs_watch = paths + .get_mut(&key) + .is_some_and(|state| state.os_watch.is_none()) + && !paths.has_own_watch_ancestor(&path); + if needs_watch && let Some(state) = paths.get_mut(&key) { + state.os_watch = Some(epoch); + commands.push(WatchCommand { + op: WatchOp::Watch, + mode, + path, + key, + epoch, + }); + } + } + commands } /// Records the outcome of a watch command. A failed watch is rolled back, @@ -1487,8 +1539,8 @@ impl GlobalWatcher { /// Drops a registration. Never blocks on FSEvents: releasing the OS watch /// it owned is queued for the watcher thread like any other change. pub fn remove(&self, id: WatcherRegistrationId) { - let command = self.state.lock().remove_registration(id); - if let Some(command) = command { + let commands = self.state.lock().remove_registration(id); + for command in commands { self.submit(command); } } @@ -2033,6 +2085,34 @@ mod tests { ); } + #[test] + fn a_path_covered_by_a_removed_ancestor_gets_a_watch_of_its_own() { + let backend = slow_backend(Duration::from_millis(200)); + let watcher = test_watcher(backend.clone()); + + // Keep the worker busy so the ancestor is still queued when it is removed. + watcher + .add(poll_path("/busy"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + std::thread::sleep(Duration::from_millis(50)); + let ancestor = watcher + .add(poll_path("/repo"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + let child = watcher + .add(poll_path("/repo/sub"), WatcherMode::Poll, false, |_| {}) + .expect("registered"); + watcher.remove(ancestor.id); + watcher.flush(); + + assert!(watcher.is_registered(child.id)); + let backend = backend.lock(); + assert!( + backend.watched_paths.contains(&PathBuf::from("/repo/sub")), + "the child lost its coverage and was given no watch" + ); + assert!(!backend.watched_paths.contains(&PathBuf::from("/repo"))); + } + #[test] fn applied_reports_a_failed_watch_and_rolls_it_back() { let backend = Arc::new(Mutex::new(FakeWatchBackend { From e4dd8aaac2af8f5bf79ff8fa9f15cb29cb2dc8b6 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 01:32:56 -0400 Subject: [PATCH 12/21] fs, project: promoted watches stay pending until applied and go in before the ancestor is released; code actions are refused when read-only --- crates/fs/src/fs_watcher.rs | 18 ++++++++++++------ crates/project/src/project.rs | 7 +++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 2fe177c84c692f..10b2b85fe18094 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -894,9 +894,9 @@ impl WatcherState { } /// Drops a registration. When it was the last one on a path that owns an OS - /// watch, returns the unwatch command that releases it, followed by a watch - /// command for each path that watch was covering and that nothing else - /// covers: those registrations stay, so they need a watch of their own. + /// watch, returns a watch command for each path that watch was covering and + /// that nothing else covers (those registrations stay, so they need a watch + /// of their own), followed by the unwatch command that releases the watch. fn remove_registration(&mut self, id: WatcherRegistrationId) -> Vec { let Some(registration_state) = self.watchers.remove(&id) else { return Vec::new(); @@ -916,14 +916,16 @@ impl WatcherState { let Some(epoch) = path_state.os_watch else { return Vec::new(); }; - let mut commands = vec![WatchCommand { + // The covered paths get their own watch first and the ancestor's watch is + // released last, so there is no moment when they are watched by neither. + let mut commands = self.promote_covered(®istration_state.key, mode); + commands.push(WatchCommand { op: WatchOp::Unwatch, mode, path: registration_state.path.clone(), key: registration_state.key.clone(), epoch, - }]; - commands.extend(self.promote_covered(®istration_state.key, mode)); + }); commands } @@ -954,6 +956,10 @@ impl WatcherState { && !paths.has_own_watch_ancestor(&path); if needs_watch && let Some(state) = paths.get_mut(&key) { state.os_watch = Some(epoch); + // Pending until the backend acknowledges the watch: a later + // `applied_waiter` must wait for it. Nobody holds the receiver. + let (waiter, _) = oneshot::channel(); + state.waiters.push(waiter); commands.push(WatchCommand { op: WatchOp::Watch, mode, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 463ca7c647b9d7..86fe91ec7291c5 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4502,6 +4502,10 @@ impl Project { push_to_history: bool, cx: &mut Context, ) -> Task> { + // A code action edits buffers and files. + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } self.lsp_store.update(cx, |lsp_store, cx| { lsp_store.apply_code_action(buffer_handle, action, push_to_history, cx) }) @@ -4514,6 +4518,9 @@ impl Project { push_to_history: bool, cx: &mut Context, ) -> Task> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } self.lsp_store.update(cx, |lsp_store, cx| { lsp_store.apply_code_action_kind(buffers, kind, push_to_history, cx) }) From fd41ef31530e47c76b656bb9c86118a56c5c1452 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 01:47:34 -0400 Subject: [PATCH 13/21] fs: add_and_wait rechecks its registration after a cancelled wait When the watch a covered path was waiting on is released before it is applied, add_and_wait waits for the path's own watch if it is still registered, and only returns once that one is applied. --- crates/fs/src/fs_watcher.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 10b2b85fe18094..3cefa1e988d50f 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -104,6 +104,15 @@ impl FsWatcher { self.add_existing_path(path) } + /// A receiver for the watch this watcher's registration of `path` is still + /// waiting on, or `None` when it is not registered or its watch is in place. + fn pending_watch(&self, path: &Path) -> Option { + let case_insensitive = case_insensitive_path(path); + let key = WatchKey::for_registration(SanitizedPath::new(path), case_insensitive); + let existing = self.registrations.lock().get(&key).copied()?; + global_watcher().applied_waiter(existing.id).flatten() + } + fn add_pending_path(&self, path: Arc) { let mut pending_registrations = self.pending_registrations.lock(); if pending_registrations.contains_key(path.as_ref()) { @@ -153,12 +162,22 @@ impl Watcher for FsWatcher { ) -> futures::future::BoxFuture<'a, anyhow::Result<()>> { let applied = self.add_path(path); Box::pin(async move { - let Some(applied) = applied else { + let Some(mut applied) = applied else { return Ok(()); }; - // Cancelled means the registration was removed before its watch was - // applied, so there is nothing left to wait for. - applied.await.unwrap_or(Ok(())) + loop { + match applied.await { + Ok(result) => return result, + // Cancelled: the watch this was waiting for was released + // before it was applied. If this path is still registered + // (a covering watch went away and it got its own), wait for + // that one; if not, it was removed and there is nothing left. + Err(_cancelled) => match self.pending_watch(path) { + Some(next) => applied = next, + None => return Ok(()), + }, + } + } }) } From d05a869a9e3730930d25bbde0047bf2bb18eed82 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 01:49:48 -0400 Subject: [PATCH 14/21] worktree, search: queue deferred watches together; embedded replace row wraps The deferred worktree watches (external symlink targets, git dirs) are all queued before any is awaited, so the watcher can batch them into one FSEvents restart. The embedded replacement row wraps like the search row. --- crates/search/src/buffer_search.rs | 1 + crates/worktree/src/worktree.rs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 6c913d4f556fe8..c28f38a5fc715c 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -546,6 +546,7 @@ impl Render for BufferSearchBar { h_flex() .w_full() .gap_2() + .when(embedded, |this| this.flex_wrap()) .when(has_collapse_button, |this| this.child(alignment_element())) .child(replace_column) .child(replace_actions) diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index f4bf44f3f811e1..d09c2231ba0eeb 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -4184,9 +4184,12 @@ impl BackgroundScanner { fs_events_rx = Box::pin(events.map(|events| events.into_iter().collect())); let state = self.state.lock().await; + // Queue every watch before waiting on any: each submission restarts + // the FSEvents stream, and the watcher batches what is already queued. + let mut watches = Vec::new(); for target in state.symlink_paths_by_target.keys() { if !target.starts_with(root_abs_path.as_path()) { - self.watcher.add_and_wait(target).await.log_err(); + watches.push(self.watcher.add_and_wait(target)); } } for repo in state.snapshot.git_repositories.values() { @@ -4194,21 +4197,18 @@ impl BackgroundScanner { .common_dir_abs_path .starts_with(root_abs_path.as_path()) { - self.watcher - .add_and_wait(&repo.common_dir_abs_path) - .await - .log_err(); + watches.push(self.watcher.add_and_wait(&repo.common_dir_abs_path)); } if !repo .repository_dir_abs_path .starts_with(root_abs_path.as_path()) { - self.watcher - .add_and_wait(&repo.repository_dir_abs_path) - .await - .log_err(); + watches.push(self.watcher.add_and_wait(&repo.repository_dir_abs_path)); } } + for result in futures::future::join_all(watches).await { + result.log_err(); + } drop(state); } From 10d0167286ecfbc65ebe91cc041d21e57164d52d Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 01:50:46 -0400 Subject: [PATCH 15/21] fs: retry native backend creation instead of caching its first failure --- crates/fs/src/fs_watcher.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 3cefa1e988d50f..8a60e424ee08f9 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -1107,6 +1107,10 @@ enum WorkerMessage { /// a poll change. struct WatchBackends { native: Mutex, String>>, + /// Creates the native backend again after a failed attempt, so a temporary + /// shortage (inotify instances, file descriptors) does not disable native + /// watching for the life of the process. `None` keeps the result as given. + make_native: Option notify::Result>>, poll: Mutex>>, make_poll: fn() -> notify::Result>, } @@ -1162,6 +1166,11 @@ impl WatchApplier { let backend: &mut Box = match mode { WatcherMode::Native => { native = self.backends.native.lock(); + if native.is_err() + && let Some(make_native) = self.backends.make_native + { + *native = make_native().map_err(|error| error.to_string()); + } match native.as_mut() { Ok(backend) => backend, Err(error) => { @@ -1303,6 +1312,7 @@ impl GlobalWatcher { fn new( event_tx: async_channel::Sender, native: Result, String>, + make_native: Option notify::Result>>, poll: Option>, make_poll: fn() -> notify::Result>, queued: fn(WatcherMode) -> bool, @@ -1313,6 +1323,7 @@ impl GlobalWatcher { state: state.clone(), backends: WatchBackends { native: Mutex::new(native), + make_native, poll: Mutex::new(poll), make_poll, }, @@ -1634,6 +1645,7 @@ fn global_watcher() -> &'static GlobalWatcher { GlobalWatcher::new( event_tx, new_native_backend().map_err(|error| error.to_string()), + Some(new_native_backend), None, new_poll_backend, queued_in_production, @@ -1779,6 +1791,7 @@ mod tests { native_watcher .map(|watcher| Box::new(SharedFakeWatchBackend(watcher)) as Box) .ok_or_else(|| "no native backend in this test".to_owned()), + None, poll_watcher .map(|watcher| Box::new(SharedFakeWatchBackend(watcher)) as Box), no_poll_backend, From 950e7c943c4f693f779f396a5522c86f40248a0f Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 02:03:03 -0400 Subject: [PATCH 16/21] fs: a pending-path poll task re-asks its registration after a cancelled wait --- crates/fs/src/fs_watcher.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 8a60e424ee08f9..e4ff659fa53efc 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -569,9 +569,22 @@ async fn poll_path_until_created( // Removing the pending path drops this task, possibly while it // waits below; the guard releases the registration then. let mut unclaimed = UnclaimedRegistration(Some(registration.id)); - if let Some(applied) = applied - && let Ok(Err(error)) = applied.await - { + let mut waiting = applied; + let mut failure = None; + while let Some(applied) = waiting.take() { + match applied.await { + Ok(Err(error)) => failure = Some(error), + Ok(Ok(())) => {} + // Cancelled: the watch waited on was released first. Ask + // again where this registration stands, and report + // success only once its own watch is in place. + Err(_cancelled) => match global_watcher().applied_waiter(registration.id) { + Some(next) => waiting = next, + None => failure = Some(anyhow::anyhow!("its watch was released")), + }, + } + } + if let Some(error) = failure { log::warn!("failed to watch newly-created path {path:?}: {error}; retrying"); continue; } From db76ffe32b0054ed79d16ba0bad0ac1b3435bf34 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 02:05:11 -0400 Subject: [PATCH 17/21] fs, worktree, project: native availability retried, vanished registrations reported, failed external watches not recorded, renames refused when read-only global() creates the native backend again after a failure. add_and_wait tells a removed registration, an applied watch and a failed one apart after a cancelled wait. An external directory is recorded as watched only after its watch succeeded. Project::perform_rename refuses when the project is read-only. --- crates/fs/src/fs_watcher.rs | 54 ++++++++++++++++++++++++++------- crates/project/src/project.rs | 3 ++ crates/worktree/src/worktree.rs | 9 ++++-- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index e4ff659fa53efc..59019737ed7515 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -29,6 +29,17 @@ pub struct FsWatcher { pending_registrations: Arc, Task<()>>>>, } +/// Where a registration's OS watch stands. +enum WatchState { + /// Still being installed: wait on this receiver. + Pending(WatchApplied), + Applied, + /// This watcher no longer holds the registration (it removed it). + Removed, + /// The registration is gone from the global watcher: its watch failed. + Failed, +} + #[derive(Clone, Copy)] struct FsWatcherRegistration { id: WatcherRegistrationId, @@ -104,13 +115,18 @@ impl FsWatcher { self.add_existing_path(path) } - /// A receiver for the watch this watcher's registration of `path` is still - /// waiting on, or `None` when it is not registered or its watch is in place. - fn pending_watch(&self, path: &Path) -> Option { + /// Where this watcher's registration of `path` stands now. + fn pending_watch(&self, path: &Path) -> WatchState { let case_insensitive = case_insensitive_path(path); let key = WatchKey::for_registration(SanitizedPath::new(path), case_insensitive); - let existing = self.registrations.lock().get(&key).copied()?; - global_watcher().applied_waiter(existing.id).flatten() + let Some(existing) = self.registrations.lock().get(&key).copied() else { + return WatchState::Removed; + }; + match global_watcher().applied_waiter(existing.id) { + None => WatchState::Failed, + Some(None) => WatchState::Applied, + Some(Some(next)) => WatchState::Pending(next), + } } fn add_pending_path(&self, path: Arc) { @@ -173,8 +189,13 @@ impl Watcher for FsWatcher { // (a covering watch went away and it got its own), wait for // that one; if not, it was removed and there is nothing left. Err(_cancelled) => match self.pending_watch(path) { - Some(next) => applied = next, - None => return Ok(()), + WatchState::Pending(next) => applied = next, + WatchState::Applied | WatchState::Removed => return Ok(()), + WatchState::Failed => { + return Err(anyhow::anyhow!( + "the watch for {path:?} was released before it was applied" + )); + } }, } } @@ -1317,7 +1338,6 @@ pub struct GlobalWatcher { applier: Arc, commands: std::sync::mpsc::Sender, queued: fn(WatcherMode) -> bool, - native_available: Result<(), String>, event_tx: async_channel::Sender, } @@ -1331,7 +1351,6 @@ impl GlobalWatcher { queued: fn(WatcherMode) -> bool, ) -> Self { let state = Arc::new(Mutex::new(WatcherState::default())); - let native_available = native.as_ref().map(|_| ()).map_err(Clone::clone); let applier = Arc::new(WatchApplier { state: state.clone(), backends: WatchBackends { @@ -1353,7 +1372,6 @@ impl GlobalWatcher { applier, commands, queued, - native_available, event_tx, } } @@ -1472,6 +1490,20 @@ impl GlobalWatcher { Some(registration) } + /// Whether the native backend can be used. A backend that failed to start is + /// created again here, so a temporary shortage (inotify instances, file + /// descriptors) does not disable native watching for the life of the process. + fn native_available(&self) -> Result<(), String> { + let backends = &self.applier.backends; + let mut native = backends.native.lock(); + if native.is_err() + && let Some(make_native) = backends.make_native + { + *native = make_native().map_err(|error| error.to_string()); + } + native.as_ref().map(|_| ()).map_err(Clone::clone) + } + /// For registration `id`: `None` when it no longer exists (removed, or its /// watch failed), `Some(None)` when its OS watch is already in place, and /// `Some(Some(receiver))` while that watch is still queued. Both facts come @@ -2554,7 +2586,7 @@ mod tests { pub fn global(f: impl FnOnce(&GlobalWatcher) -> T) -> anyhow::Result { let global_watcher = global_watcher(); - if let Err(error) = &global_watcher.native_available { + if let Err(error) = global_watcher.native_available() { anyhow::bail!("native file watcher unavailable: {error}"); } Ok(f(global_watcher)) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 86fe91ec7291c5..22e999492c9f81 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4548,6 +4548,9 @@ impl Project { new_name: String, cx: &mut Context, ) -> Task> { + if !self.capability().editable() { + return Task::ready(Err(anyhow!("This project is read-only"))); + } let push_to_history = true; let position = position.to_point_utf16(buffer.read(cx)); self.request_lsp( diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index d09c2231ba0eeb..9ef90a9506bbac 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -5174,8 +5174,13 @@ impl BackgroundScanner { // An external directory is outside the watched root, so its // watch is new; wait for it before its entries are trusted. let canonical: Arc = canonical.into(); - self.watcher.add_and_wait(&canonical).await.log_err(); - Some(canonical) + // Recorded as watched only once it is: a failed watch has + // been rolled back and must not be reported as active. + self.watcher + .add_and_wait(&canonical) + .await + .log_err() + .map(|()| canonical) } Err(_) => None, } From 22087878432a964e39d2efa8624614a09137c147 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 02:08:07 -0400 Subject: [PATCH 18/21] fs: registration changes publish their commands in order; add_and_wait waits for a pending path that has appeared A producer mutex holds from a registration change to the submission of its commands, so an unwatch cannot overtake the watch it follows. add_and_wait gives a poll task that is claiming a path that has just appeared time to do so and then waits for its watch. --- crates/fs/src/fs_watcher.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 59019737ed7515..925f80a683f16d 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -29,6 +29,10 @@ pub struct FsWatcher { pending_registrations: Arc, Task<()>>>>, } +/// How many 50 ms polls `add_and_wait` gives a poll task to claim a path that +/// has appeared (about 30 seconds, longer than any poll interval). +const PENDING_CLAIM_POLLS: usize = 600; + /// Where a registration's OS watch stands. enum WatchState { /// Still being installed: wait on this receiver. @@ -176,8 +180,24 @@ impl Watcher for FsWatcher { &'a self, path: &'a std::path::Path, ) -> futures::future::BoxFuture<'a, anyhow::Result<()>> { - let applied = self.add_path(path); + let mut applied = self.add_path(path); Box::pin(async move { + // A missing path is watched by a poll task once it appears. If it + // already has, that task is claiming it right now: wait for the claim + // (bounded) and then for its watch, instead of reporting a watch that + // is not installed yet. + for _ in 0..PENDING_CLAIM_POLLS { + if applied.is_some() + || !self.pending_registrations.lock().contains_key(path) + || fs::symlink_metadata(path).is_err() + { + break; + } + self.executor + .timer(std::time::Duration::from_millis(50)) + .await; + applied = self.add_path(path); + } let Some(mut applied) = applied else { return Ok(()); }; @@ -1339,6 +1359,11 @@ pub struct GlobalWatcher { commands: std::sync::mpsc::Sender, queued: fn(WatcherMode) -> bool, event_tx: async_channel::Sender, + /// Held from a registration change to the submission of its commands, so two + /// changes cannot publish their commands in the opposite order to the state + /// transitions they came from (an unwatch overtaking its watch). The state + /// lock itself is still released before any OS work. + publish: Mutex<()>, } impl GlobalWatcher { @@ -1373,6 +1398,7 @@ impl GlobalWatcher { commands, queued, event_tx, + publish: Mutex::new(()), } } @@ -1387,6 +1413,7 @@ impl GlobalWatcher { case_insensitive: bool, cb: impl Fn(¬ify::Event) + Send + Sync + 'static, ) -> Option { + let _publishing = self.publish.lock(); let path = SanitizedPath::from_arc(path); let key = WatchKey::for_registration(&path, case_insensitive); let (registration, command) = { @@ -1620,6 +1647,7 @@ impl GlobalWatcher { /// Drops a registration. Never blocks on FSEvents: releasing the OS watch /// it owned is queued for the watcher thread like any other change. pub fn remove(&self, id: WatcherRegistrationId) { + let _publishing = self.publish.lock(); let commands = self.state.lock().remove_registration(id); for command in commands { self.submit(command); From 92bb248c3689362b9d87e74760da6e0f443d69ad Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 02:24:58 -0400 Subject: [PATCH 19/21] fs: a covered path waits on its nearest covering watch; an unclaimed pending path past the deadline is an error The ancestor a covered path waits on is the nearest one that owns an OS watch, not whichever farther one is still queued. add_and_wait reports a timeout instead of success when an existing path has still not been claimed by its poll task. --- crates/fs/src/fs_watcher.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 925f80a683f16d..5e86277b633a0a 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -198,6 +198,15 @@ impl Watcher for FsWatcher { .await; applied = self.add_path(path); } + if applied.is_none() + && self.pending_registrations.lock().contains_key(path) + && fs::symlink_metadata(path).is_ok() + { + // Still unclaimed after the deadline: its watch is not in place. + return Err(anyhow::anyhow!( + "the watch for {path:?} was not installed in time" + )); + } let Some(mut applied) = applied else { return Ok(()); }; @@ -898,6 +907,9 @@ impl WatchPaths { if mode != WatcherMode::Poll && !cfg!(any(target_os = "windows", target_os = "macos")) { return None; } + // The nearest ancestor with an OS watch of its own is what covers this + // path: it is waited on if still queued, and nothing is waited on if it + // is applied (a farther, unrelated ancestor must not decide the outcome). let key = path.as_path().ancestors().skip(1).find_map(|ancestor| { let ancestor = SanitizedPath::unchecked_new(ancestor); [WatchKey::exact(ancestor), WatchKey::folded(ancestor)] @@ -905,10 +917,12 @@ impl WatchPaths { .find(|key| { self.0 .get(key) - .is_some_and(|state| !state.waiters.is_empty()) + .is_some_and(|state| state.os_watch.is_some()) }) })?; - self.0.get_mut(&key) + self.0 + .get_mut(&key) + .filter(|state| !state.waiters.is_empty()) } /// Collects the watcher ids of every registration whose directory is an From 97113e157be05589a250cd539ef950c8c1ac1235 Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 14:20:31 -0400 Subject: [PATCH 20/21] fix(fs): saturate the failed-op index when a watch batch partially applies --- crates/fs/src/fs_watcher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 5e86277b633a0a..62d6151e81a8c9 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -1290,7 +1290,7 @@ impl WatchApplier { ); break; }; - let failed_index = pending.len() - remaining.len() - 1; + let failed_index = pending.len().saturating_sub(remaining.len() + 1); let unapplied = pending.split_off(failed_index + 1); let failed = pending.pop().expect("the failed op is in the batch"); finished.extend(pending.drain(..).map(|command| (command, Ok(())))); From c5a1d6959899dde3467c60b59b53838ef9391bce Mon Sep 17 00:00:00 2001 From: soyboyscout Date: Mon, 12 Oct 2026 15:36:30 -0400 Subject: [PATCH 21/21] chore(project): drop the comment that restates local_read_only --- crates/project/src/project.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 22e999492c9f81..e975370d9e6b59 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -232,7 +232,6 @@ pub struct Project { client_subscriptions: Vec, worktree_store: Entity, buffer_store: Entity, - /// A local project an embedder has made read-only (see `set_local_read_only`). local_read_only: bool, context_server_store: Entity, image_store: Entity,