diff --git a/crates/herdr-gpui/src/avatars.rs b/crates/herdr-gpui/src/avatars.rs index 4b461d59f..a53ce5059 100644 --- a/crates/herdr-gpui/src/avatars.rs +++ b/crates/herdr-gpui/src/avatars.rs @@ -1,10 +1,12 @@ use gpui::{Image, ImageFormat}; mod cache; +mod links; +use herdr_client::{ConnectTarget, protocol::ClientShellWorkspace}; use std::{ collections::HashMap, - sync::{Arc, mpsc}, + sync::{Arc, Mutex, mpsc}, thread, - time::{Duration, SystemTime}, + time::{Duration, Instant, SystemTime}, }; const MAX_REQUESTS: usize = 128; @@ -12,100 +14,319 @@ const MAX_REQUESTS: usize = 128; /// A bound on each remote path segment, above GitHub's own name limits. const MAX_SEGMENT: usize = 100; +/// The whole budget for reading one remote checkout's Git configuration. +const REMOTE_TIMEOUT: Duration = Duration::from_secs(10); + +/// How long a checkout whose host could not be read, while its device stayed +/// connected, waits before a sidebar update may ask again. A device that +/// reconnects is asked at once instead (see `Avatars::follow`). +const RETRY_AFTER: Duration = Duration::from_secs(60); + +/// SSH reads that may run at once. A host that stops answering holds a +/// reader for up to `REMOTE_TIMEOUT`, so a few readers keep one slow host +/// from stalling every other device's avatars. +const REMOTE_READERS: usize = 4; + +/// A checkout whose repository owner's avatar a sidebar row shows. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(crate) enum Checkout { + /// A workspace directory on this machine, read by local Git. + Local(S), + /// The Git directory a saved SSH device's daemon reported, read there. + Ssh { target: S, git_dir: S }, +} + +impl<'a> Checkout<&'a str> { + /// The checkout behind `workspace` on the device reached by `target`. + /// Only the local daemon's directories are read by this machine's Git; a + /// saved SSH device is read over SSH by the Git directory its daemon + /// reported, so it needs worktree metadata. Other devices may be forwarded + /// from an unknown machine and get none. + pub(crate) fn of( + local: bool, + target: &'a ConnectTarget, + workspace: &'a ClientShellWorkspace, + ) -> Option { + match target { + ConnectTarget::Ssh { target, .. } => { + workspace.worktree.as_ref().map(|tree| Self::Ssh { + target, + git_dir: &tree.key, + }) + } + _ if local => Some(Self::Local(&workspace.new_workspace_cwd)), + _ => None, + } + } + + fn to_owned(self) -> Checkout { + match self { + Self::Local(cwd) => Checkout::Local(cwd.to_owned()), + Self::Ssh { target, git_dir } => Checkout::Ssh { + target: target.to_owned(), + git_dir: git_dir.to_owned(), + }, + } + } +} + +impl Checkout { + fn as_deref(&self) -> Checkout<&str> { + match self { + Self::Local(cwd) => Checkout::Local(cwd), + Self::Ssh { target, git_dir } => Checkout::Ssh { target, git_dir }, + } + } +} + +/// What is known about one checkout's avatar. +#[derive(Clone, Debug)] +enum Slot { + /// Requested at `asked` and not yet answered. + Pending { asked: Instant }, + /// The owner's avatar, or `None` for a checkout without a GitHub owner. + Resolved(Option>), + /// The host could not be read; asked again from `retry_at`, or as soon as + /// its device reconnects. + Unreachable { retry_at: Instant }, +} + +/// A worker's answer for one checkout: an error only when the host could +/// not be read, never when it was read and has no GitHub owner. +type Lookup = herdr_client::Result>>; + +type Results = mpsc::SyncSender<(Checkout, Lookup)>; +type Refresh = mpsc::SyncSender<(Checkout, String)>; + /// Memory front end to the shared public-image disk cache. pub struct Avatars { - requests: mpsc::SyncSender, - results: mpsc::Receiver<(String, Option>)>, + // Local Git answers at once while an SSH read may take `REMOTE_TIMEOUT`, + // so each has its own queue and a slow host never delays a local avatar. + local_requests: mpsc::SyncSender>, + remote_requests: mpsc::SyncSender>, + results: mpsc::Receiver<(Checkout, Lookup)>, // Presence deduplicates pending requests as well as resolved hits and misses. - images: HashMap>>, + local: HashMap, + // Keyed by SSH target, then Git directory, so a row looks up without allocating. + remote: HashMap>, + // Each connected SSH endpoint's host and when its connection came up. + links: HashMap, + offline: links::Offline, } impl Avatars { pub fn new() -> Self { - let (requests, incoming) = mpsc::sync_channel::(MAX_REQUESTS); + let (local_requests, local_incoming) = mpsc::sync_channel(MAX_REQUESTS); + let (remote_requests, remote_incoming) = mpsc::sync_channel(MAX_REQUESTS); let (outgoing, results) = mpsc::sync_channel(MAX_REQUESTS * 2); - let (refresh, network) = mpsc::sync_channel::<(String, String)>(MAX_REQUESTS); + let (refresh, network) = mpsc::sync_channel::<(Checkout, String)>(MAX_REQUESTS); let updated = outgoing.clone(); thread::spawn(move || { let agent = avatar_agent(); let mut owners = HashMap::new(); - for (cwd, url) in network { + for (checkout, url) in network { let image = owners .entry(url.clone()) .or_insert_with(|| download(&agent, &url)) .clone(); if let Some(image) = image - && updated.send((cwd, Some(image))).is_err() + && updated.send((checkout, Ok(Some(image)))).is_err() { break; } } }); - // Deliberately detach: dropping the sender ends the worker without a UI join. + let offline = links::Offline::default(); + // Deliberately detach: dropping the senders ends the workers without a UI join. + let remote_incoming = Arc::new(Mutex::new(remote_incoming)); + for _ in 0..REMOTE_READERS { + let (incoming, offline, outgoing, refresh) = ( + remote_incoming.clone(), + offline.clone(), + outgoing.clone(), + refresh.clone(), + ); + thread::spawn(move || { + // The lock is held only while waiting, never across a read. + while let Some(checkout) = incoming.lock().ok().and_then(|rx| rx.recv().ok()) { + if !answer(checkout, &offline, &outgoing, &refresh) { + break; + } + } + }); + } + let local_offline = offline.clone(); thread::spawn(move || { - for cwd in incoming { - let url = repo_owner(&cwd) - .map(|owner| format!("https://avatars.githubusercontent.com/{owner}?size=48")); - let cached = url.as_deref().and_then(cached_image); - let fresh = cached.as_ref().is_some_and(|(_, fresh)| *fresh); - if outgoing - .send((cwd.clone(), cached.map(|(image, _)| image))) - .is_err() - { + for checkout in local_incoming { + if !answer(checkout, &local_offline, &outgoing, &refresh) { break; } - if !fresh && let Some(url) = url { - let _ = refresh.try_send((cwd, url)); - } } }); Self { - requests, + local_requests, + remote_requests, results, - images: HashMap::new(), + local: HashMap::new(), + remote: HashMap::new(), + links: HashMap::new(), + offline, } } - /// Enqueue each exact cwd at most once, including failed resolutions. - pub fn request(&mut self, cwd: &str) { - if self.images.len() >= MAX_REQUESTS { - return; + /// Enqueue each exact checkout at most once, including ones without a + /// GitHub owner. A host that could not be read is asked again once its + /// retry time has passed at `now`. + pub(crate) fn request(&mut self, checkout: Checkout<&str>, now: Instant) { + match self.slot(checkout) { + Some(Slot::Unreachable { retry_at }) if now >= *retry_at => {} + Some(_) => return, + None => { + let len = self.local.len() + self.remote.values().map(HashMap::len).sum::(); + if len >= MAX_REQUESTS { + return; + } + } } - if let std::collections::hash_map::Entry::Vacant(entry) = self.images.entry(cwd.to_owned()) - && self.requests.try_send(entry.key().clone()).is_ok() - { - entry.insert(None); + let requests = match checkout { + Checkout::Local(_) => &self.local_requests, + Checkout::Ssh { .. } => &self.remote_requests, + }; + if requests.try_send(checkout.to_owned()).is_ok() { + self.insert(checkout.to_owned(), Slot::Pending { asked: now }); } } - /// Drain completed requests without blocking. Misses also count as updates. - pub fn poll(&mut self) -> bool { + /// Drain completed requests without blocking. Misses also count as + /// updates; a host that could not be read is retried after `now`. + pub fn poll(&mut self, now: Instant) -> bool { let mut changed = false; - for (cwd, image) in self.results.try_iter() { - self.images.insert(cwd, image); + while let Ok((checkout, lookup)) = self.results.try_recv() { + // A read that failed after its device reconnected missed that + // connection's snapshot, which found it still pending, so it is + // asked again here rather than waiting for another snapshot. + let reconnected = lookup.is_err() && self.reconnected_since_asked(checkout.as_deref()); + let slot = match lookup { + Ok(image) => Slot::Resolved(image), + // Stopped because its device disconnected: due again as soon + // as one is connected, since only then is it asked for. + Err(herdr_client::Error::SshCancelled) => Slot::Unreachable { retry_at: now }, + Err(_) if reconnected => Slot::Unreachable { retry_at: now }, + Err(_) => Slot::Unreachable { + retry_at: now + RETRY_AFTER, + }, + }; + if reconnected { + self.insert(checkout.clone(), slot); + self.request(checkout.as_deref(), now); + } else { + self.insert(checkout, slot); + } changed = true; } changed } - pub fn image(&self, cwd: &str) -> Option> { - self.images.get(cwd).cloned().flatten() + pub(crate) fn image(&self, checkout: Checkout<&str>) -> Option> { + match self.slot(checkout)? { + Slot::Resolved(image) => image.clone(), + Slot::Pending { .. } | Slot::Unreachable { .. } => None, + } + } + + fn slot(&self, checkout: Checkout<&str>) -> Option<&Slot> { + match checkout { + Checkout::Local(cwd) => self.local.get(cwd), + Checkout::Ssh { target, git_dir } => self.remote.get(target)?.get(git_dir), + } + } + + fn insert(&mut self, checkout: Checkout, image: Slot) { + match checkout { + Checkout::Local(cwd) => { + self.local.insert(cwd, image); + } + Checkout::Ssh { target, git_dir } => { + self.remote + .entry(target) + .or_default() + .insert(git_dir, image); + } + } + } +} + +/// Look up one checkout's owner and send what the disk cache holds for it, +/// queueing a download when that is missing or stale. False once the UI end +/// is gone. +fn answer( + checkout: Checkout, + offline: &links::Offline, + outgoing: &Results, + refresh: &Refresh, +) -> bool { + let url = match repo_owner(checkout.as_deref(), offline) { + Ok(owner) => { + owner.map(|owner| format!("https://avatars.githubusercontent.com/{owner}?size=48")) + } + Err(error) => return outgoing.send((checkout, Err(error))).is_ok(), + }; + let cached = url.as_deref().and_then(cached_image); + let fresh = cached.as_ref().is_some_and(|(_, fresh)| *fresh); + if outgoing + .send((checkout.clone(), Ok(cached.map(|(image, _)| image)))) + .is_err() + { + return false; + } + if !fresh && let Some(url) = url { + let _ = refresh.try_send((checkout, url)); } + true } -fn repo_owner(cwd: &str) -> Option { +/// The GitHub owner behind a checkout's `upstream` remote, else its `origin`. +/// Fails only when a remote host could not be read, or went offline before +/// or during the read; local Git failures and absent remotes are `None`. +fn repo_owner( + checkout: Checkout<&str>, + offline: &links::Offline, +) -> herdr_client::Result> { + let deadline = Instant::now() + REMOTE_TIMEOUT; for key in ["remote.upstream.url", "remote.origin.url"] { - if let Ok(output) = herdr_client::new_command("git") - .args(["-C", cwd, "config", "--default", "", "--get", key]) - .output() - && output.status.success() - && let Ok(text) = std::str::from_utf8(&output.stdout) - && let Some(owner) = github_owner(text.trim_end_matches(['\r', '\n'])) - { - return Some(owner); + let url = match checkout { + Checkout::Local(cwd) => local_config(cwd, key), + // The upstream read may use most of the budget; origin gets only + // what is left. + // A read queued before its device disconnected never dials. + Checkout::Ssh { target, .. } if links::is_offline(offline, target) => { + return Err(herdr_client::Error::SshCancelled); + } + Checkout::Ssh { target, git_dir } => herdr_client::remote_config_value( + target, + git_dir, + key, + deadline + .checked_duration_since(Instant::now()) + .ok_or(herdr_client::Error::SshTimeout)?, + || links::is_offline(offline, target), + )?, + }; + if let Some(owner) = url.as_deref().and_then(github_owner) { + return Ok(Some(owner)); } } - None + Ok(None) +} + +fn local_config(cwd: &str, key: &str) -> Option { + let output = herdr_client::new_command("git") + .args(["-C", cwd, "config", "--default", "", "--get", key]) + .output() + .ok() + .filter(|output| output.status.success())?; + let text = std::str::from_utf8(&output.stdout).ok()?; + Some(text.trim_end_matches(['\r', '\n']).to_owned()) } fn github_owner(remote: &str) -> Option { diff --git a/crates/herdr-gpui/src/avatars/links.rs b/crates/herdr-gpui/src/avatars/links.rs new file mode 100644 index 000000000..f047164ec --- /dev/null +++ b/crates/herdr-gpui/src/avatars/links.rs @@ -0,0 +1,97 @@ +//! Which saved SSH devices are connected, so remote avatar reads follow the +//! window's own reconnection rather than probing a host it knows is down. +use super::{Avatars, Checkout, Slot}; +use std::{ + collections::{HashMap, HashSet}, + sync::{Arc, Mutex}, + time::Instant, +}; + +/// Hosts with no live connection, shared with the SSH readers so a read +/// queued or running for one stops instead of waiting out its timeout. +pub(super) type Offline = Arc>>; + +pub(super) fn is_offline(offline: &Offline, target: &str) -> bool { + offline.lock().is_ok_and(|hosts| hosts.contains(target)) +} + +impl Avatars { + /// Follow one saved SSH device's connection: `online` is when its current + /// connection came up with a snapshot, `None` while it is down. While no + /// device on a host is connected nothing is read there, and reads queued + /// or running for it stop. Each new connection makes the host's unread + /// checkouts due at once, so the snapshot it brings asks for them again. + /// + /// Called on every poll; it allocates and locks only on a change. + pub(crate) fn follow(&mut self, endpoint: &str, target: &str, online: Option) { + let Some(since) = online else { + self.unlink(endpoint); + return; + }; + if self + .links + .get(endpoint) + .is_some_and(|(host, at)| host == target && *at == since) + { + return; + } + self.unlink(endpoint); + self.links + .insert(endpoint.to_owned(), (target.to_owned(), since)); + if let Ok(mut hosts) = self.offline.lock() { + hosts.remove(target); + } + for slot in self + .remote + .get_mut(target) + .into_iter() + .flat_map(HashMap::values_mut) + { + if let Slot::Unreachable { retry_at } = slot { + *retry_at = since; + } + } + } + + /// Forget the connections of endpoints that left the catalog, which are + /// never followed again; `kept` says whether an endpoint is still saved. + pub(crate) fn keep_links(&mut self, mut kept: impl FnMut(&str) -> bool) { + let gone: Vec = self + .links + .keys() + .filter(|endpoint| !kept(endpoint)) + .cloned() + .collect(); + for endpoint in gone { + self.unlink(&endpoint); + } + } + + /// Whether a pending SSH read's host has connected again since it was + /// asked for. + pub(super) fn reconnected_since_asked(&self, checkout: Checkout<&str>) -> bool { + let Checkout::Ssh { target, .. } = checkout else { + return false; + }; + let Some(Slot::Pending { asked }) = self.slot(checkout) else { + return false; + }; + self.links + .values() + .any(|(host, since)| host == target && since > asked) + } + + /// Forget an endpoint's connection; its host goes offline once no other + /// endpoint (another session on the same host) is connected to it. + fn unlink(&mut self, endpoint: &str) { + let Some((target, _)) = self.links.remove(endpoint) else { + return; + }; + if self.links.values().any(|(host, _)| *host == target) { + return; + } + if let Ok(mut hosts) = self.offline.lock() { + hosts.insert(target); + } + } +} diff --git a/crates/herdr-gpui/src/avatars/tests.rs b/crates/herdr-gpui/src/avatars/tests.rs index 3f46b0d7f..561ad38ce 100644 --- a/crates/herdr-gpui/src/avatars/tests.rs +++ b/crates/herdr-gpui/src/avatars/tests.rs @@ -1,7 +1,11 @@ #![allow(clippy::expect_used)] use super::*; +mod reconnect; +mod remote; mod remote_userinfo; +mod retry; +mod workers; // Exercises the disk cache; other platforms have no cache root. #[cfg(any(unix, windows))] @@ -82,19 +86,20 @@ fn profile_avatar_urls_never_accept_credentials_or_other_hosts() { #[ignore = "requires HERDR_AVATAR_TEST_REPO and network access to GitHub avatars"] fn loads_local_repository_owner_avatar() { let cwd = std::env::var("HERDR_AVATAR_TEST_REPO").expect("set a local GitHub repository path"); - let owner = repo_owner(&cwd).expect("repository has a GitHub origin"); + let checkout = Checkout::Local(cwd.as_str()); + let owner = repo_owner(checkout, &Default::default()) + .ok() + .flatten() + .expect("repository has a GitHub origin"); let mut avatars = Avatars::new(); - avatars.request(&cwd); - let deadline = std::time::Instant::now() + Duration::from_secs(10); - while avatars.image(&cwd).is_none() { - avatars.poll(); - assert!( - std::time::Instant::now() < deadline, - "avatar worker timed out" - ); + avatars.request(checkout, Instant::now()); + let deadline = Instant::now() + Duration::from_secs(10); + while avatars.image(checkout).is_none() { + avatars.poll(Instant::now()); + assert!(Instant::now() < deadline, "avatar worker timed out"); thread::sleep(Duration::from_millis(20)); } - let image = avatars.image(&cwd).expect("owner avatar downloaded"); + let image = avatars.image(checkout).expect("owner avatar downloaded"); assert!(!image.bytes.is_empty()); eprintln!("Avatar loaded for {owner}: {} bytes", image.bytes.len()); } @@ -184,33 +189,61 @@ fn accepts_only_supported_content_types() { } } -#[test] -fn deduplicates_pending_hits_and_misses_and_drains_results() { +type Results = mpsc::Sender<(Checkout, Lookup)>; + +/// An `Avatars` whose worker ends are the test's: the request receiver and +/// the result sender. +pub(super) fn detached() -> (Avatars, mpsc::Receiver>, Results) { let (requests, incoming) = mpsc::sync_channel(MAX_REQUESTS); let (outgoing, results) = mpsc::channel(); - let mut avatars = Avatars { - requests, + let avatars = Avatars { + local_requests: requests.clone(), + remote_requests: requests, results, - images: HashMap::new(), + local: HashMap::new(), + remote: HashMap::new(), + links: HashMap::new(), + offline: links::Offline::default(), }; - assert!(!avatars.poll()); - assert!(avatars.image("hit").is_none()); + (avatars, incoming, outgoing) +} + +#[test] +fn deduplicates_pending_hits_and_misses_and_drains_results() { + let (mut avatars, incoming, outgoing) = detached(); + let now = Instant::now(); + assert!(!avatars.poll(now)); + assert!(avatars.image(Checkout::Local("hit")).is_none()); for cwd in ["hit", "miss", "hit", "miss"] { - avatars.request(cwd); + avatars.request(Checkout::Local(cwd), now); } - assert_eq!(incoming.try_iter().collect::>(), ["hit", "miss"]); + assert_eq!( + incoming.try_iter().collect::>(), + [ + Checkout::Local("hit".into()), + Checkout::Local("miss".into()) + ] + ); let image = Arc::new(Image::empty()); - assert!(outgoing.send(("hit".into(), Some(image.clone()))).is_ok()); - assert!(outgoing.send(("miss".into(), None)).is_ok()); - assert!(avatars.poll()); - assert!(!avatars.poll()); + assert!( + outgoing + .send((Checkout::Local("hit".into()), Ok(Some(image.clone())))) + .is_ok() + ); + assert!( + outgoing + .send((Checkout::Local("miss".into()), Ok(None))) + .is_ok() + ); + assert!(avatars.poll(now)); + assert!(!avatars.poll(now)); assert!( avatars - .image("hit") + .image(Checkout::Local("hit")) .is_some_and(|cached| Arc::ptr_eq(&cached, &image)) ); - assert!(avatars.image("miss").is_none()); - avatars.request("hit"); - avatars.request("miss"); + assert!(avatars.image(Checkout::Local("miss")).is_none()); + avatars.request(Checkout::Local("hit"), now); + avatars.request(Checkout::Local("miss"), now); assert!(incoming.try_recv().is_err()); } diff --git a/crates/herdr-gpui/src/avatars/tests/reconnect.rs b/crates/herdr-gpui/src/avatars/tests/reconnect.rs new file mode 100644 index 000000000..605b3c64a --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/reconnect.rs @@ -0,0 +1,140 @@ +use super::*; + +const DEVBOX: Checkout<&str> = Checkout::Ssh { + target: "devbox", + git_dir: "/home/user/src/app/.git", +}; + +#[test] +fn a_reconnected_device_is_asked_again_at_once() { + let (mut avatars, incoming, outgoing) = detached(); + let start = Instant::now(); + let online = start - Duration::from_secs(1); + avatars.follow("dev", "devbox", Some(online)); + avatars.request(DEVBOX, start); + assert_eq!(incoming.try_iter().count(), 1); + assert!( + outgoing + .send((DEVBOX.to_owned(), Err(herdr_client::Error::SshTimeout))) + .is_ok() + ); + assert!(avatars.poll(start)); + + // The same connection does not shorten the retry interval. + avatars.follow("dev", "devbox", Some(online)); + avatars.request(DEVBOX, start + Duration::from_secs(1)); + assert!(incoming.try_recv().is_err()); + + avatars.follow("dev", "devbox", None); + let back = start + Duration::from_secs(2); + avatars.follow("dev", "devbox", Some(back)); + avatars.request(DEVBOX, back); + assert_eq!(incoming.try_iter().collect::>(), [DEVBOX.to_owned()]); +} + +#[test] +fn a_disconnected_host_stops_its_reads_until_one_device_is_back() { + let (mut avatars, _incoming, _outgoing) = detached(); + let now = Instant::now(); + avatars.follow("a", "devbox", Some(now)); + avatars.follow("b", "devbox", Some(now)); + assert!(!links::is_offline(&avatars.offline, "devbox")); + + // Another session on the same host keeps it connected. + avatars.follow("a", "devbox", None); + assert!(!links::is_offline(&avatars.offline, "devbox")); + avatars.follow("b", "devbox", None); + assert!(links::is_offline(&avatars.offline, "devbox")); + assert!(matches!( + repo_owner(DEVBOX, &avatars.offline), + Err(herdr_client::Error::SshCancelled) + )); + + avatars.follow("a", "devbox", Some(now)); + assert!(!links::is_offline(&avatars.offline, "devbox")); +} + +#[test] +fn an_endpoint_pointed_at_another_host_releases_the_old_one() { + let (mut avatars, _incoming, _outgoing) = detached(); + let now = Instant::now(); + avatars.follow("dev", "old", Some(now)); + avatars.follow("dev", "new", Some(now)); + assert!(links::is_offline(&avatars.offline, "old")); + assert!(!links::is_offline(&avatars.offline, "new")); +} + +#[test] +fn a_read_stopped_by_a_disconnect_is_due_on_the_next_request() { + let (mut avatars, incoming, outgoing) = detached(); + let now = Instant::now(); + avatars.request(DEVBOX, now); + assert_eq!(incoming.try_iter().count(), 1); + assert!( + outgoing + .send((DEVBOX.to_owned(), Err(herdr_client::Error::SshCancelled))) + .is_ok() + ); + assert!(avatars.poll(now)); + assert!(avatars.image(DEVBOX).is_none()); + avatars.request(DEVBOX, now); + assert_eq!(incoming.try_iter().collect::>(), [DEVBOX.to_owned()]); +} + +#[test] +fn a_read_failing_after_its_device_reconnected_is_asked_again_at_once() { + let (mut avatars, incoming, outgoing) = detached(); + let start = Instant::now(); + avatars.follow("dev", "devbox", Some(start)); + avatars.request(DEVBOX, start); + assert_eq!(incoming.try_iter().count(), 1); + + // The device drops and comes back while the read is still pending, so + // the new connection's snapshot cannot ask for it again. + let back = start + Duration::from_secs(1); + avatars.follow("dev", "devbox", None); + avatars.follow("dev", "devbox", Some(back)); + avatars.request(DEVBOX, back); + assert!(incoming.try_recv().is_err()); + + for error in [ + herdr_client::Error::SshCancelled, + herdr_client::Error::SshTimeout, + ] { + assert!(outgoing.send((DEVBOX.to_owned(), Err(error))).is_ok()); + assert!(avatars.poll(back + Duration::from_secs(1))); + assert_eq!(incoming.try_iter().collect::>(), [DEVBOX.to_owned()]); + // Asked after the reconnection, so the retry is pending again with + // the current connection. + avatars.follow("dev", "devbox", None); + avatars.follow("dev", "devbox", Some(back + Duration::from_secs(2))); + } +} + +#[test] +fn a_read_failing_on_the_same_connection_waits_for_its_retry() { + let (mut avatars, incoming, outgoing) = detached(); + let start = Instant::now(); + avatars.follow("dev", "devbox", Some(start)); + avatars.request(DEVBOX, start); + assert_eq!(incoming.try_iter().count(), 1); + assert!( + outgoing + .send((DEVBOX.to_owned(), Err(herdr_client::Error::SshTimeout))) + .is_ok() + ); + assert!(avatars.poll(start)); + assert!(incoming.try_recv().is_err()); +} + +#[test] +fn a_device_removed_from_the_catalog_takes_its_host_offline() { + let (mut avatars, _incoming, _outgoing) = detached(); + let now = Instant::now(); + avatars.follow("a", "devbox", Some(now)); + avatars.follow("b", "devbox", Some(now)); + avatars.keep_links(|id| id == "b"); + assert!(!links::is_offline(&avatars.offline, "devbox")); + avatars.keep_links(|_| false); + assert!(links::is_offline(&avatars.offline, "devbox")); +} diff --git a/crates/herdr-gpui/src/avatars/tests/remote.rs b/crates/herdr-gpui/src/avatars/tests/remote.rs new file mode 100644 index 000000000..21e02d0b5 --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/remote.rs @@ -0,0 +1,130 @@ +use super::*; +use herdr_client::protocol::ClientShellWorktree; +use std::path::PathBuf; + +fn workspace(worktree: Option<&str>) -> ClientShellWorkspace { + let mut workspace = crate::sidebar::layout_tests::snapshot(1) + .workspaces + .remove(0); + workspace.new_workspace_cwd = "/home/user/src/app".into(); + workspace.worktree = worktree.map(|key| ClientShellWorktree { + key: key.into(), + label: "app".into(), + is_linked_worktree: false, + }); + workspace +} + +fn ssh(target: &str) -> ConnectTarget { + ConnectTarget::Ssh { + target: target.into(), + session: "default".into(), + } +} + +#[test] +fn ssh_devices_read_the_reported_git_directory_on_the_host() { + let workspace = workspace(Some("/home/user/src/app/.git")); + let target = ssh("devbox"); + // An SSH device is read there, wherever it sits in the device list. + for local in [false, true] { + assert_eq!( + Checkout::of(local, &target, &workspace), + Some(Checkout::Ssh { + target: "devbox", + git_dir: "/home/user/src/app/.git", + }) + ); + } +} + +#[test] +fn ssh_workspaces_without_worktree_metadata_get_no_checkout() { + // The directory is the host's, so it is never resolved by local Git. + assert_eq!(Checkout::of(false, &ssh("devbox"), &workspace(None)), None); +} + +#[test] +fn only_the_local_daemon_resolves_directories_with_local_git() { + let workspace = workspace(Some("/home/user/src/app/.git")); + assert_eq!( + Checkout::of(true, &ConnectTarget::Local, &workspace), + Some(Checkout::Local("/home/user/src/app")) + ); + // A socket may be forwarded from an unknown machine. + let socket = ConnectTarget::Socket(PathBuf::from("/tmp/herdr.sock")); + assert_eq!(Checkout::of(false, &socket, &workspace), None); + assert_eq!(Checkout::of(false, &ConnectTarget::Local, &workspace), None); +} + +#[test] +fn remote_avatars_are_kept_per_host_and_apart_from_local_paths() { + let (mut avatars, incoming, outgoing) = detached(); + let now = Instant::now(); + let git_dir = "/home/user/src/app/.git"; + let devbox = Checkout::Ssh { + target: "devbox", + git_dir, + }; + let other = Checkout::Ssh { + target: "other", + git_dir, + }; + for checkout in [devbox, other, Checkout::Local(git_dir), devbox, other] { + avatars.request(checkout, now); + } + assert_eq!( + incoming.try_iter().collect::>(), + [ + devbox.to_owned(), + other.to_owned(), + Checkout::Local(git_dir.into()) + ] + ); + let image = Arc::new(Image::empty()); + assert!( + outgoing + .send((devbox.to_owned(), Ok(Some(image.clone())))) + .is_ok() + ); + assert!(outgoing.send((other.to_owned(), Ok(None))).is_ok()); + assert!(avatars.poll(now)); + assert!( + avatars + .image(devbox) + .is_some_and(|cached| Arc::ptr_eq(&cached, &image)) + ); + assert!(avatars.image(other).is_none()); + assert!(avatars.image(Checkout::Local(git_dir)).is_none()); +} + +#[test] +fn requests_stop_at_the_bound_across_hosts() { + let (mut avatars, incoming, _outgoing) = detached(); + let now = Instant::now(); + let dirs = (0..=MAX_REQUESTS) + .map(|index| format!("/repo/{index}/.git")) + .collect::>(); + for (index, git_dir) in dirs.iter().enumerate() { + let target = if index % 2 == 0 { "devbox" } else { "other" }; + avatars.request(Checkout::Ssh { target, git_dir }, now); + } + assert_eq!(incoming.try_iter().count(), MAX_REQUESTS); +} + +#[test] +#[ignore = "requires HERDR_AVATAR_TEST_SSH, HERDR_AVATAR_TEST_GIT_DIR, and noninteractive SSH"] +fn reads_the_repository_owner_over_ssh() { + let target = std::env::var("HERDR_AVATAR_TEST_SSH").expect("set a saved SSH target"); + let git_dir = std::env::var("HERDR_AVATAR_TEST_GIT_DIR").expect("set the host's Git dir"); + let owner = repo_owner( + Checkout::Ssh { + target: &target, + git_dir: &git_dir, + }, + &Default::default(), + ) + .expect("host read over SSH"); + assert!(owner.is_some(), "no GitHub owner read over SSH"); + eprintln!("Owner read over SSH: {owner:?}"); +} diff --git a/crates/herdr-gpui/src/avatars/tests/retry.rs b/crates/herdr-gpui/src/avatars/tests/retry.rs new file mode 100644 index 000000000..6cba2d0aa --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/retry.rs @@ -0,0 +1,99 @@ +use super::*; + +const DEVBOX: Checkout<&str> = Checkout::Ssh { + target: "devbox", + git_dir: "/home/user/src/app/.git", +}; + +#[test] +fn an_unreachable_host_is_asked_again_only_after_the_retry_interval() { + let (mut avatars, incoming, outgoing) = detached(); + let start = Instant::now(); + avatars.request(DEVBOX, start); + assert_eq!(incoming.try_iter().count(), 1); + assert!( + outgoing + .send((DEVBOX.to_owned(), Err(herdr_client::Error::SshTimeout))) + .is_ok() + ); + assert!(avatars.poll(start)); + assert!(avatars.image(DEVBOX).is_none()); + + // A sidebar update before the interval ends must not read the host again. + avatars.request(DEVBOX, start + RETRY_AFTER - Duration::from_millis(1)); + assert!(incoming.try_recv().is_err()); + + avatars.request(DEVBOX, start + RETRY_AFTER); + assert_eq!(incoming.try_iter().collect::>(), [DEVBOX.to_owned()]); + // The retry is pending again, so another update does not queue a third read. + avatars.request(DEVBOX, start + RETRY_AFTER * 2); + assert!(incoming.try_recv().is_err()); + + let image = Arc::new(Image::empty()); + assert!( + outgoing + .send((DEVBOX.to_owned(), Ok(Some(image.clone())))) + .is_ok() + ); + assert!(avatars.poll(start + RETRY_AFTER)); + assert!( + avatars + .image(DEVBOX) + .is_some_and(|cached| Arc::ptr_eq(&cached, &image)) + ); +} + +#[test] +fn a_host_read_without_a_github_owner_is_never_asked_again() { + let (mut avatars, incoming, outgoing) = detached(); + let start = Instant::now(); + avatars.request(DEVBOX, start); + assert!(outgoing.send((DEVBOX.to_owned(), Ok(None))).is_ok()); + assert!(avatars.poll(start)); + assert_eq!(incoming.try_iter().count(), 1); + avatars.request(DEVBOX, start + RETRY_AFTER * 10); + assert!(incoming.try_recv().is_err()); +} + +#[test] +fn an_unreachable_retry_is_not_refused_by_the_request_bound() { + let (mut avatars, incoming, outgoing) = detached(); + let start = Instant::now(); + let dirs = (0..MAX_REQUESTS) + .map(|index| format!("/repo/{index}/.git")) + .collect::>(); + for git_dir in &dirs { + avatars.request( + Checkout::Ssh { + target: "devbox", + git_dir, + }, + start, + ); + } + assert_eq!(incoming.try_iter().count(), MAX_REQUESTS); + let first = Checkout::Ssh { + target: "devbox", + git_dir: &dirs[0], + }; + assert!( + outgoing + .send((first.to_owned(), Err(herdr_client::Error::SshClosed))) + .is_ok() + ); + assert!(avatars.poll(start)); + // The bound limits distinct checkouts; retrying one already counted is + // not a new entry. + avatars.request(first, start + RETRY_AFTER); + assert_eq!(incoming.try_iter().collect::>(), [first.to_owned()]); +} + +#[test] +fn local_checkouts_never_fail_the_lookup() { + let missing = std::env::temp_dir().join("herdr-avatar-no-such-checkout"); + let cwd = missing.to_str().expect("UTF-8 temporary directory"); + assert!(matches!( + repo_owner(Checkout::Local(cwd), &Default::default()), + Ok(None) + )); +} diff --git a/crates/herdr-gpui/src/avatars/tests/workers.rs b/crates/herdr-gpui/src/avatars/tests/workers.rs new file mode 100644 index 000000000..1d3626d70 --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/workers.rs @@ -0,0 +1,69 @@ +use super::*; + +const LOCAL: Checkout<&str> = Checkout::Local("/home/user/src/app"); + +fn ssh(git_dir: &str) -> Checkout<&str> { + Checkout::Ssh { + target: "devbox", + git_dir, + } +} + +/// An `Avatars` whose local and SSH queues the test holds apart, with room +/// for `remote` SSH requests. +fn split( + remote: usize, +) -> ( + Avatars, + mpsc::Receiver>, + mpsc::Receiver>, +) { + let (local_requests, local) = mpsc::sync_channel(MAX_REQUESTS); + let (remote_requests, ssh) = mpsc::sync_channel(remote); + let (_outgoing, results) = mpsc::channel(); + let avatars = Avatars { + local_requests, + remote_requests, + results, + local: HashMap::new(), + remote: HashMap::new(), + links: HashMap::new(), + offline: links::Offline::default(), + }; + (avatars, local, ssh) +} + +#[test] +fn local_and_ssh_lookups_use_separate_queues() { + let (mut avatars, local, remote) = split(MAX_REQUESTS); + let now = Instant::now(); + avatars.request(ssh("/a/.git"), now); + avatars.request(LOCAL, now); + assert_eq!(local.try_iter().collect::>(), [LOCAL.to_owned()]); + assert_eq!( + remote.try_iter().collect::>(), + [ssh("/a/.git").to_owned()] + ); +} + +#[test] +fn a_backed_up_ssh_queue_does_not_refuse_local_lookups() { + let (mut avatars, local, remote) = split(1); + let now = Instant::now(); + avatars.request(ssh("/a/.git"), now); + // The SSH queue is full, so this one is refused and left to a later update. + avatars.request(ssh("/b/.git"), now); + avatars.request(LOCAL, now); + assert_eq!(local.try_iter().collect::>(), [LOCAL.to_owned()]); + assert_eq!( + remote.try_iter().collect::>(), + [ssh("/a/.git").to_owned()] + ); + + // The refused checkout was not recorded as pending, so it is asked again. + avatars.request(ssh("/b/.git"), now); + assert_eq!( + remote.try_iter().collect::>(), + [ssh("/b/.git").to_owned()] + ); +} diff --git a/crates/herdr-gpui/src/control/client/local/processes/tests.rs b/crates/herdr-gpui/src/control/client/local/processes/tests.rs index 6ff8ebed9..7127b1177 100644 --- a/crates/herdr-gpui/src/control/client/local/processes/tests.rs +++ b/crates/herdr-gpui/src/control/client/local/processes/tests.rs @@ -68,11 +68,11 @@ fn sampling_waits_for_measured_cpu_or_returns_the_latest() { let (watch, sender, _commands) = Watch::fake(); sender - .send(Update::Listing(Err(crate::Error::ProcessesNoRoot))) + .send(Update::Listing(Err(crate::processes::Error::NoRoot.into()))) .unwrap(); assert!(matches!( sample(&watch, Duration::from_secs(5)), - Err(crate::Error::ProcessesNoRoot) + Err(crate::Error::Processes(crate::processes::Error::NoRoot)) )); let (watch, _sender, _commands) = Watch::fake(); diff --git a/crates/herdr-gpui/src/endpoint.rs b/crates/herdr-gpui/src/endpoint.rs index b62c238ab..100f5f359 100644 --- a/crates/herdr-gpui/src/endpoint.rs +++ b/crates/herdr-gpui/src/endpoint.rs @@ -387,6 +387,12 @@ impl Endpoint { self.retry_at = self.retry_at.min(now); } + /// When the current connection came up with a snapshot, while it is up. + /// Each reconnection gives a new instant. + pub(crate) fn online_since(&self) -> Option { + self.online_since + } + /// Why the endpoint is reconnecting, while it is. pub(crate) fn outage(&self) -> Option<&str> { self.outage.as_deref() diff --git a/crates/herdr-gpui/src/endpoint/polling.rs b/crates/herdr-gpui/src/endpoint/polling.rs index 60e5ed53a..7e2f3790b 100644 --- a/crates/herdr-gpui/src/endpoint/polling.rs +++ b/crates/herdr-gpui/src/endpoint/polling.rs @@ -1,9 +1,9 @@ //! The window's periodic endpoint work: draining each connection, activating //! the selected surface, and probing sessions. use super::{ACTIVATION_TIMEOUT, LOCAL, Redraw}; -use crate::HerdrWindow; +use crate::{HerdrWindow, avatars::Checkout}; use gpui::{ClipboardItem, Context}; -use herdr_client::RemoteHost; +use herdr_client::{ConnectTarget, RemoteHost}; use std::time::Instant; impl HerdrWindow { @@ -62,6 +62,12 @@ impl HerdrWindow { // OSC 52 writes are drained per endpoint so they are written once and // never linger in a live state that a later poll would re-read. let mut clipboard_writes = std::collections::VecDeque::new(); + // A device removed from the catalog is never followed again, so its + // avatar reads stop here rather than dialing a deleted host. + if let Some(avatars) = &mut self.avatars { + let endpoints = &self.endpoints; + avatars.keep_links(|id| endpoints.iter().any(|endpoint| endpoint.id == id)); + } for (index, endpoint) in self.endpoints.iter_mut().enumerate() { let updated = endpoint.poll(Instant::now()); clipboard_writes.append(&mut endpoint.live.clipboard_writes); @@ -79,14 +85,26 @@ impl HerdrWindow { Instant::now(), ); changed = changed.max(updated); - // Remote cwd strings must never be resolved against this machine's Git repos. + // An SSH device's avatar reads follow its connection, before any + // request below, so a reconnection's first snapshot asks again. + if let (Some(avatars), ConnectTarget::Ssh { target, .. }) = + (&mut self.avatars, &endpoint.connection.target) + { + avatars.follow(&endpoint.id, target, endpoint.online_since()); + } + // Remote cwd strings must never be resolved against this machine's + // Git repos; `Checkout::of` reads a saved SSH device's over SSH. if updated == Redraw::Window - && index == 0 + && endpoint.online_since().is_some() && let (Some(avatars), Some(snapshot)) = (&mut self.avatars, &endpoint.live.snapshot) { + let target = &endpoint.connection.target; + let now = Instant::now(); for workspace in &snapshot.workspaces { - avatars.request(&workspace.new_workspace_cwd); + if let Some(checkout) = Checkout::of(index == 0, target, workspace) { + avatars.request(checkout, now); + } } } if endpoint.enabled diff --git a/crates/herdr-gpui/src/error.rs b/crates/herdr-gpui/src/error.rs index 5631372f6..07aa91723 100644 --- a/crates/herdr-gpui/src/error.rs +++ b/crates/herdr-gpui/src/error.rs @@ -683,22 +683,8 @@ pub enum Error { PrMetadata, #[error("No local worktree matches the daemon branch.")] PrMissingWorktree, - #[error("Herdr's answer about the pane's process was not understood.")] - ProcessesAnswer(#[source] serde_json::Error), - #[error("Still ending the last processes. Try again in a moment.")] - ProcessesBusy, - #[error("The herdr executable's path is not valid UTF-8.")] - ProcessesExecutable, - #[error("Herdr did not name a process for this pane.")] - ProcessesNoRoot, - #[error("Could not ask Herdr which process the pane runs.")] - ProcessesQuery(#[source] herdr_client::Error), - #[error("The pane's process has exited.")] - ProcessesRootExited, - #[error("The process list stopped updating. Reopen it to try again.")] - ProcessesStopped, - #[error("Could not start watching the pane's processes.")] - ProcessesWorker(#[source] io::Error), + #[error(transparent)] + Processes(#[from] crate::processes::Error), #[error("PR lookup supports GitHub.com origins only.")] PrOrigin, #[error("Could not {operation} for Git PR lookup.")] diff --git a/crates/herdr-gpui/src/login_env.rs b/crates/herdr-gpui/src/login_env.rs index 09bdcfdab..9aee471f9 100644 --- a/crates/herdr-gpui/src/login_env.rs +++ b/crates/herdr-gpui/src/login_env.rs @@ -460,8 +460,10 @@ mod unix { let (dir, path) = shell("/bin/sleep 60 &\nprintf '%s %s' \"$$\" \"$!\" > \"$0.pids\"\nwait"); let start = Instant::now(); - assert!(probe(path.as_os_str(), false, false, Duration::from_secs(1)).is_none()); - assert!(start.elapsed() < Duration::from_secs(5)); + // Room for a loaded machine to start the shell and record its pids + // before the timeout fires; the sleep outlasts it either way. + assert!(probe(path.as_os_str(), false, false, Duration::from_secs(3)).is_none()); + assert!(start.elapsed() < Duration::from_secs(10)); let pids = std::fs::read_to_string(dir.path().join("shell.pids")).unwrap(); for pid in pids.split_whitespace() { let deadline = Instant::now() + Duration::from_secs(2); diff --git a/crates/herdr-gpui/src/network_quality/tests.rs b/crates/herdr-gpui/src/network_quality/tests.rs index 5a3048c2b..4cbee9ae1 100644 --- a/crates/herdr-gpui/src/network_quality/tests.rs +++ b/crates/herdr-gpui/src/network_quality/tests.rs @@ -37,11 +37,14 @@ fn a_listening_port_answers() { #[test] fn a_closed_port_is_lost() { - // Bound then dropped, so nothing listens there. - let target: SocketAddr = TcpListener::bind("127.0.0.1:0") - .expect("bind") - .local_addr() - .expect("address"); + // The listener is dropped so nothing listens there, while the connection + // it accepted keeps the port in use: a parallel test cannot be given it + // for its own listener, and a probe cannot connect to itself through it. + let listener = TcpListener::bind("127.0.0.1:0").expect("bind"); + let target: SocketAddr = listener.local_addr().expect("address"); + let _client = std::net::TcpStream::connect(target).expect("connect"); + let _held = listener.accept().expect("accept"); + drop(listener); // Windows retries a refused connect for about two seconds before // reporting it, so the kind may be a refusal or a timeout; either way // the probe is lost. diff --git a/crates/herdr-gpui/src/pane_menu/processes.rs b/crates/herdr-gpui/src/pane_menu/processes.rs index d36eca191..e01ea1899 100644 --- a/crates/herdr-gpui/src/pane_menu/processes.rs +++ b/crates/herdr-gpui/src/pane_menu/processes.rs @@ -273,7 +273,7 @@ impl HerdrWindow { processes .watch .as_ref() - .ok_or(crate::Error::ProcessesStopped)? + .ok_or(crate::processes::Error::Stopped)? .kill(processes.chosen.iter().copied().collect()) }); match sent { diff --git a/crates/herdr-gpui/src/processes.rs b/crates/herdr-gpui/src/processes.rs index 30031f1ea..f370f6e21 100644 --- a/crates/herdr-gpui/src/processes.rs +++ b/crates/herdr-gpui/src/processes.rs @@ -9,12 +9,15 @@ //! pids mean nothing here. The worker samples every couple of seconds while a //! pane's process list is open, and stops when it closes. +mod error; mod tree; #[cfg(test)] mod tests; -use crate::{Error, Result}; +pub use error::Error; + +use crate::Result; use herdr_client::{ConnectTarget, ScriptHost, ScriptLimits, run_script, shell_quote}; use std::{ collections::HashMap, @@ -76,7 +79,7 @@ impl Daemon { /// The script asking this daemon about `pane`'s process. fn script(&self, executable: &Path, pane: &str) -> Result { - let executable = executable.to_str().ok_or(Error::ProcessesExecutable)?; + let executable = executable.to_str().ok_or(Error::Executable)?; let session = match self { Self::Default => String::new(), Self::Session(name) => format!(" --session {}", shell_quote(name)), @@ -104,12 +107,12 @@ impl Daemon { QUERY_LIMITS, cancelled, ) - .map_err(Error::ProcessesQuery)?; + .map_err(Error::Query)?; serde_json::from_slice::>( &output, ) .map(|envelope| envelope.result.process_info) - .map_err(Error::ProcessesAnswer) + .map_err(|source| Error::Answer(source).into()) } } @@ -156,7 +159,7 @@ impl Watch { thread::Builder::new() .name("herdr-pane-processes".into()) .spawn(move || run(&daemon, &pane, &received, &sender, &shared)) - .map_err(Error::ProcessesWorker)?; + .map_err(Error::Worker)?; Ok(Self { commands, updates, @@ -169,9 +172,12 @@ impl Watch { pub(crate) fn kill(&self, victims: Vec) -> Result<()> { self.commands .try_send(Command::Kill(victims)) - .map_err(|error| match error { - mpsc::TrySendError::Full(_) => Error::ProcessesBusy, - mpsc::TrySendError::Disconnected(_) => Error::ProcessesStopped, + .map_err(|error| { + match error { + mpsc::TrySendError::Full(_) => Error::Busy, + mpsc::TrySendError::Disconnected(_) => Error::Stopped, + } + .into() }) } @@ -180,7 +186,7 @@ impl Watch { match self.updates.try_recv() { Ok(update) => Ok(Some(update)), Err(mpsc::TryRecvError::Empty) => Ok(None), - Err(mpsc::TryRecvError::Disconnected) => Err(Error::ProcessesStopped), + Err(mpsc::TryRecvError::Disconnected) => Err(Error::Stopped.into()), } } @@ -259,7 +265,7 @@ fn sample( cancelled: &AtomicBool, ) -> Result { let info = daemon.process_info(pane, cancelled)?; - let root = info.shell_pid.ok_or(Error::ProcessesNoRoot)?; + let root = info.shell_pid.ok_or(Error::NoRoot)?; refresh(system); let previous = std::mem::take(sightings); let entries = system @@ -286,8 +292,7 @@ fn sample( cpu: measured.then(|| process.cpu_usage().max(0.)), memory: process.memory(), }); - tree::listing(entries, root, &info.foreground_pids(), ROW_LIMIT) - .ok_or(Error::ProcessesRootExited) + tree::listing(entries, root, &info.foreground_pids(), ROW_LIMIT).ok_or(Error::RootExited.into()) } /// Records that `identity` is listed now, given how often the last listing diff --git a/crates/herdr-gpui/src/processes/error.rs b/crates/herdr-gpui/src/processes/error.rs new file mode 100644 index 000000000..a2160b6c4 --- /dev/null +++ b/crates/herdr-gpui/src/processes/error.rs @@ -0,0 +1,21 @@ +//! Why a pane's process list could not be read or acted on. + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Herdr's answer about the pane's process was not understood.")] + Answer(#[source] serde_json::Error), + #[error("Still ending the last processes. Try again in a moment.")] + Busy, + #[error("The herdr executable's path is not valid UTF-8.")] + Executable, + #[error("Herdr did not name a process for this pane.")] + NoRoot, + #[error("Could not ask Herdr which process the pane runs.")] + Query(#[source] herdr_client::Error), + #[error("The pane's process has exited.")] + RootExited, + #[error("The process list stopped updating. Reopen it to try again.")] + Stopped, + #[error("Could not start watching the pane's processes.")] + Worker(#[source] std::io::Error), +} diff --git a/crates/herdr-gpui/src/processes/tests.rs b/crates/herdr-gpui/src/processes/tests.rs index 72a797fad..434c8f0ab 100644 --- a/crates/herdr-gpui/src/processes/tests.rs +++ b/crates/herdr-gpui/src/processes/tests.rs @@ -237,15 +237,18 @@ fn a_watch_reports_a_stopped_worker_and_a_busy_one() { // One kill at a time: the first is still with the worker. assert!(matches!( watch.kill(vec![victim]), - Err(Error::ProcessesBusy) + Err(crate::Error::Processes(Error::Busy)) )); assert_eq!(commands.recv().unwrap(), Command::Kill(vec![victim])); drop(updates); - assert!(matches!(watch.try_update(), Err(Error::ProcessesStopped))); + assert!(matches!( + watch.try_update(), + Err(crate::Error::Processes(Error::Stopped)) + )); drop(commands); assert!(matches!( watch.kill(vec![victim]), - Err(Error::ProcessesStopped) + Err(crate::Error::Processes(Error::Stopped)) )); } diff --git a/crates/herdr-gpui/src/sidebar/layout_tests/workspace_menu.rs b/crates/herdr-gpui/src/sidebar/layout_tests/workspace_menu.rs index 066575902..7d5c98f0c 100644 --- a/crates/herdr-gpui/src/sidebar/layout_tests/workspace_menu.rs +++ b/crates/herdr-gpui/src/sidebar/layout_tests/workspace_menu.rs @@ -229,15 +229,17 @@ fn resting_on_a_workspace_opens_its_menu_once(cx: &mut gpui::TestAppContext) { cx.simulate_resize(size(px(900.), px(700.))); cx.update(|window, cx| full_draw(window, cx).clear(cx)); let row = cx.debug_bounds("row-herdr").unwrap().center(); + let settle_at = + |view: &Entity, cx: &mut gpui::VisualTestContext, now: std::time::Instant| { + cx.update(|window, cx| { + view.update(cx, |view, cx| view.poll_hover_menu(now, window, cx)); + full_draw(window, cx).clear(cx); + }); + }; let settle = |view: &Entity, cx: &mut gpui::VisualTestContext, elapsed: std::time::Duration| { - cx.update(|window, cx| { - view.update(cx, |view, cx| { - view.poll_hover_menu(std::time::Instant::now() + elapsed, window, cx); - }); - full_draw(window, cx).clear(cx); - }); + settle_at(view, cx, std::time::Instant::now() + elapsed); }; // Entering the row alone is not a rest: the pointer has not moved yet. @@ -250,10 +252,13 @@ fn resting_on_a_workspace_opens_its_menu_once(cx: &mut gpui::TestAppContext) { assert!(view.read_with(cx, |view, _| view.menu.page.is_none())); // Drifting inside the row restarts the dwell rather than opening early. + // The early checks are timed from before the move, so a slow machine + // cannot carry them past the dwell. + let drifted = std::time::Instant::now(); cx.simulate_mouse_move(row + point(px(8.), px(0.)), None, Modifiers::default()); - settle(&view, cx, std::time::Duration::ZERO); + settle_at(&view, cx, drifted); assert!(view.read_with(cx, |view, _| view.menu.page.is_none())); - settle(&view, cx, super::super::HOVER_MENU_DELAY / 2); + settle_at(&view, cx, drifted + super::super::HOVER_MENU_DELAY / 2); assert!(view.read_with(cx, |view, _| view.menu.page.is_none())); settle(&view, cx, super::super::HOVER_MENU_DELAY); diff --git a/crates/herdr-gpui/src/sidebar/render.rs b/crates/herdr-gpui/src/sidebar/render.rs index cc3afa73f..43fe08544 100644 --- a/crates/herdr-gpui/src/sidebar/render.rs +++ b/crates/herdr-gpui/src/sidebar/render.rs @@ -19,6 +19,7 @@ use super::{ }; use crate::{ Command, HerdrWindow, NavigationTarget, + avatars::Checkout, config::{AgentsPlacement, FontConfig, Theme}, fonts::StyledFont, }; @@ -394,10 +395,11 @@ impl HerdrWindow { // Not a Git checkout, so no mark that reads as one. RowIcon::Blank } else { + let target = &endpoint.connection.target; self.avatars .as_ref() - .filter(|_| endpoint_index == 0) - .and_then(|avatars| avatars.image(&workspace.new_workspace_cwd)) + .zip(Checkout::of(endpoint_index == 0, target, workspace)) + .and_then(|(avatars, checkout)| avatars.image(checkout)) .map_or(RowIcon::Mark, RowIcon::Avatar) }, fold, diff --git a/crates/herdr-gpui/src/updater/brew/tests/relaunch.rs b/crates/herdr-gpui/src/updater/brew/tests/relaunch.rs index 178472fab..7ec0f4b53 100644 --- a/crates/herdr-gpui/src/updater/brew/tests/relaunch.rs +++ b/crates/herdr-gpui/src/updater/brew/tests/relaunch.rs @@ -105,7 +105,10 @@ impl Tools { } fn wait_for(&self, line: &str) -> anyhow::Result> { - let deadline = Instant::now() + Duration::from_secs(10); + // Only a failing wait runs this long. The failing-open path alone + // sleeps over three seconds between a dozen process spawns, which a + // loaded test machine can stretch past ten. + let deadline = Instant::now() + Duration::from_secs(30); while Instant::now() < deadline { let events = self.events(); if events.iter().any(|event| event == line) { diff --git a/crates/herdr-gpui/src/window.rs b/crates/herdr-gpui/src/window.rs index b2e4905f3..096b518ae 100644 --- a/crates/herdr-gpui/src/window.rs +++ b/crates/herdr-gpui/src/window.rs @@ -509,7 +509,11 @@ impl HerdrWindow { } cx.notify(); } - if self.avatars.as_mut().is_some_and(|avatars| avatars.poll()) { + if self + .avatars + .as_mut() + .is_some_and(|avatars| avatars.poll(std::time::Instant::now())) + { cx.notify(); } if let Some(chrome) = self.sidebar_preferences.as_mut().and_then(|p| p.loaded()) {