From 821d2bc75807fd48e76fe8cd7f3f98b30f65f5a2 Mon Sep 17 00:00:00 2001 From: Mac McDonald Date: Mon, 19 Oct 2026 22:02:46 -0700 Subject: [PATCH 01/11] fix: show repository owner avatars for workspaces on SSH devices Sidebar repository icons were resolved only for the local endpoint, because the owner lookup ran `git config` in the workspace directory on this machine. A remote device's rows always fell back to the generic GitHub mark. Avatar requests are now keyed by a `Checkout`: a local directory read by local Git, or, for a saved SSH device, the Git directory its daemon reported, read over the same bounded, noninteractive SSH config read the pull request lookup already uses. Remote directories are still never resolved against this machine's repositories, and other socket targets get no lookup. Worktrees share their common Git directory, so one SSH read covers every worktree of a repository on a host. Co-Authored-By: Claude Opus 5.5 --- crates/herdr-gpui/src/avatars.rs | 172 ++++++++++++++---- crates/herdr-gpui/src/avatars/tests.rs | 67 ++++--- crates/herdr-gpui/src/avatars/tests/remote.rs | 124 +++++++++++++ crates/herdr-gpui/src/endpoint/polling.rs | 11 +- crates/herdr-gpui/src/sidebar/render.rs | 6 +- 5 files changed, 316 insertions(+), 64 deletions(-) create mode 100644 crates/herdr-gpui/src/avatars/tests/remote.rs diff --git a/crates/herdr-gpui/src/avatars.rs b/crates/herdr-gpui/src/avatars.rs index 4b461d59f..70c3f3d58 100644 --- a/crates/herdr-gpui/src/avatars.rs +++ b/crates/herdr-gpui/src/avatars.rs @@ -1,10 +1,11 @@ use gpui::{Image, ImageFormat}; mod cache; +use herdr_client::{ConnectTarget, protocol::ClientShellWorkspace}; use std::{ collections::HashMap, sync::{Arc, mpsc}, thread, - time::{Duration, SystemTime}, + time::{Duration, Instant, SystemTime}, }; const MAX_REQUESTS: usize = 128; @@ -12,30 +13,89 @@ 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); + +/// 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 }, + } + } +} + +type Slot = Option>; + /// Memory front end to the shared public-image disk cache. pub struct Avatars { - requests: mpsc::SyncSender, - results: mpsc::Receiver<(String, Option>)>, + requests: mpsc::SyncSender>, + results: mpsc::Receiver<(Checkout, Slot)>, // 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>, } impl Avatars { pub fn new() -> Self { - let (requests, incoming) = mpsc::sync_channel::(MAX_REQUESTS); + let (requests, 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, Some(image))).is_err() { break; } @@ -43,69 +103,109 @@ impl Avatars { }); // Deliberately detach: dropping the sender ends the worker without a UI join. thread::spawn(move || { - for cwd in incoming { - let url = repo_owner(&cwd) + for checkout in incoming { + let url = repo_owner(checkout.as_deref()) .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))) + .send((checkout.clone(), cached.map(|(image, _)| image))) .is_err() { break; } if !fresh && let Some(url) = url { - let _ = refresh.try_send((cwd, url)); + let _ = refresh.try_send((checkout, url)); } } }); Self { requests, results, - images: HashMap::new(), + local: HashMap::new(), + remote: HashMap::new(), } } - /// Enqueue each exact cwd at most once, including failed resolutions. - pub fn request(&mut self, cwd: &str) { - if self.images.len() >= MAX_REQUESTS { + /// Enqueue each exact checkout at most once, including failed resolutions. + pub(crate) fn request(&mut self, checkout: Checkout<&str>) { + let len = self.local.len() + self.remote.values().map(HashMap::len).sum::(); + if len >= MAX_REQUESTS || self.slot(checkout).is_some() { 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); + if self.requests.try_send(checkout.to_owned()).is_ok() { + self.insert(checkout.to_owned(), None); } } /// Drain completed requests without blocking. Misses also count as updates. pub fn poll(&mut self) -> bool { let mut changed = false; - for (cwd, image) in self.results.try_iter() { - self.images.insert(cwd, image); + while let Ok((checkout, image)) = self.results.try_recv() { + self.insert(checkout, image); 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> { + self.slot(checkout).cloned().flatten() } -} -fn repo_owner(cwd: &str) -> Option { - 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); + 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), } } - None + + 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); + } + } + } +} + +/// The GitHub owner behind a checkout's `upstream` remote, else its `origin`. +fn repo_owner(checkout: Checkout<&str>) -> Option { + let deadline = Instant::now() + REMOTE_TIMEOUT; + ["remote.upstream.url", "remote.origin.url"] + .into_iter() + .find_map(|key| { + 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. + Checkout::Ssh { target, git_dir } => herdr_client::remote_config_value( + target, + git_dir, + key, + deadline.checked_duration_since(Instant::now())?, + || false, + ) + .ok() + .flatten(), + }?; + github_owner(&url) + }) +} + +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/tests.rs b/crates/herdr-gpui/src/avatars/tests.rs index 3f46b0d7f..84b722a27 100644 --- a/crates/herdr-gpui/src/avatars/tests.rs +++ b/crates/herdr-gpui/src/avatars/tests.rs @@ -1,6 +1,7 @@ #![allow(clippy::expect_used)] use super::*; +mod remote; mod remote_userinfo; // Exercises the disk cache; other platforms have no cache root. @@ -82,19 +83,17 @@ 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).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.request(checkout); + let deadline = Instant::now() + Duration::from_secs(10); + while avatars.image(checkout).is_none() { avatars.poll(); - assert!( - std::time::Instant::now() < deadline, - "avatar worker timed out" - ); + 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 +183,57 @@ fn accepts_only_supported_content_types() { } } -#[test] -fn deduplicates_pending_hits_and_misses_and_drains_results() { +type Results = mpsc::Sender<(Checkout, Slot)>; + +/// 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 { + let avatars = Avatars { requests, results, - images: HashMap::new(), + local: HashMap::new(), + remote: HashMap::new(), }; + (avatars, incoming, outgoing) +} + +#[test] +fn deduplicates_pending_hits_and_misses_and_drains_results() { + let (mut avatars, incoming, outgoing) = detached(); assert!(!avatars.poll()); - assert!(avatars.image("hit").is_none()); + assert!(avatars.image(Checkout::Local("hit")).is_none()); for cwd in ["hit", "miss", "hit", "miss"] { - avatars.request(cwd); + avatars.request(Checkout::Local(cwd)); } - 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!( + outgoing + .send((Checkout::Local("hit".into()), Some(image.clone()))) + .is_ok() + ); + assert!( + outgoing + .send((Checkout::Local("miss".into()), None)) + .is_ok() + ); assert!(avatars.poll()); assert!(!avatars.poll()); 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")); + avatars.request(Checkout::Local("miss")); assert!(incoming.try_recv().is_err()); } 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..6bf60eb27 --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/remote.rs @@ -0,0 +1,124 @@ +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 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); + } + 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(), Some(image.clone()))) + .is_ok() + ); + assert!(outgoing.send((other.to_owned(), None)).is_ok()); + assert!(avatars.poll()); + 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 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 }); + } + 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, + }); + assert!(owner.is_some(), "no GitHub owner read over SSH"); + eprintln!("Owner read over SSH: {owner:?}"); +} diff --git a/crates/herdr-gpui/src/endpoint/polling.rs b/crates/herdr-gpui/src/endpoint/polling.rs index 60e5ed53a..e2547d1eb 100644 --- a/crates/herdr-gpui/src/endpoint/polling.rs +++ b/crates/herdr-gpui/src/endpoint/polling.rs @@ -1,7 +1,7 @@ //! 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 std::time::Instant; @@ -79,14 +79,17 @@ impl HerdrWindow { Instant::now(), ); changed = changed.max(updated); - // Remote cwd strings must never be resolved against this machine's Git repos. + // 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 && let (Some(avatars), Some(snapshot)) = (&mut self.avatars, &endpoint.live.snapshot) { + let target = &endpoint.connection.target; for workspace in &snapshot.workspaces { - avatars.request(&workspace.new_workspace_cwd); + if let Some(checkout) = Checkout::of(index == 0, target, workspace) { + avatars.request(checkout); + } } } if endpoint.enabled diff --git a/crates/herdr-gpui/src/sidebar/render.rs b/crates/herdr-gpui/src/sidebar/render.rs index 23e93ad21..fd7555db9 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, }; @@ -393,10 +394,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, From 89839f87078b5d809765f53f1b0d5ab9e4782e63 Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:34:16 -0700 Subject: [PATCH 02/11] fix: retry repository avatars for SSH devices that could not be read A failed SSH read of a remote checkout's Git configuration was cached like a checkout without a GitHub owner, so a device that was briefly unreachable when its first snapshot arrived kept the generic mark until the app restarted. The owner lookup now returns an error only when the host could not be read. Such a checkout is marked unreachable with a retry time 60 seconds later; the next sidebar snapshot update after that asks again. A host read without a GitHub owner is still cached for good, and a retry reuses its entry, so it never counts against the request bound as a new one. --- crates/herdr-gpui/src/avatars.rs | 122 ++++++++++++------ crates/herdr-gpui/src/avatars/tests.rs | 29 +++-- crates/herdr-gpui/src/avatars/tests/remote.rs | 15 ++- crates/herdr-gpui/src/avatars/tests/retry.rs | 96 ++++++++++++++ crates/herdr-gpui/src/endpoint/polling.rs | 3 +- crates/herdr-gpui/src/window.rs | 6 +- 6 files changed, 214 insertions(+), 57 deletions(-) create mode 100644 crates/herdr-gpui/src/avatars/tests/retry.rs diff --git a/crates/herdr-gpui/src/avatars.rs b/crates/herdr-gpui/src/avatars.rs index 70c3f3d58..bcbc02486 100644 --- a/crates/herdr-gpui/src/avatars.rs +++ b/crates/herdr-gpui/src/avatars.rs @@ -16,6 +16,11 @@ 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 waits before a sidebar +/// update may ask again. A host that is briefly down then gets its avatar +/// without a restart, and one that stays down costs one read per interval. +const RETRY_AFTER: Duration = Duration::from_secs(60); + /// A checkout whose repository owner's avatar a sidebar row shows. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub(crate) enum Checkout { @@ -68,12 +73,25 @@ impl Checkout { } } -type Slot = Option>; +/// What is known about one checkout's avatar. +#[derive(Clone, Debug)] +enum Slot { + /// Requested and not yet answered. + Pending, + /// 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`. + 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>>; /// Memory front end to the shared public-image disk cache. pub struct Avatars { requests: mpsc::SyncSender>, - results: mpsc::Receiver<(Checkout, Slot)>, + results: mpsc::Receiver<(Checkout, Lookup)>, // Presence deduplicates pending requests as well as resolved hits and misses. local: HashMap, // Keyed by SSH target, then Git directory, so a row looks up without allocating. @@ -95,7 +113,7 @@ impl Avatars { .or_insert_with(|| download(&agent, &url)) .clone(); if let Some(image) = image - && updated.send((checkout, Some(image))).is_err() + && updated.send((checkout, Ok(Some(image)))).is_err() { break; } @@ -104,12 +122,21 @@ impl Avatars { // Deliberately detach: dropping the sender ends the worker without a UI join. thread::spawn(move || { for checkout in incoming { - let url = repo_owner(checkout.as_deref()) - .map(|owner| format!("https://avatars.githubusercontent.com/{owner}?size=48")); + let url = match repo_owner(checkout.as_deref()) { + Ok(owner) => owner.map(|owner| { + format!("https://avatars.githubusercontent.com/{owner}?size=48") + }), + Err(error) => { + if outgoing.send((checkout, Err(error))).is_err() { + break; + } + continue; + } + }; let cached = url.as_deref().and_then(cached_image); let fresh = cached.as_ref().is_some_and(|(_, fresh)| *fresh); if outgoing - .send((checkout.clone(), cached.map(|(image, _)| image))) + .send((checkout.clone(), Ok(cached.map(|(image, _)| image)))) .is_err() { break; @@ -127,29 +154,47 @@ impl Avatars { } } - /// Enqueue each exact checkout at most once, including failed resolutions. - pub(crate) fn request(&mut self, checkout: Checkout<&str>) { - let len = self.local.len() + self.remote.values().map(HashMap::len).sum::(); - if len >= MAX_REQUESTS || self.slot(checkout).is_some() { - 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 self.requests.try_send(checkout.to_owned()).is_ok() { - self.insert(checkout.to_owned(), None); + self.insert(checkout.to_owned(), Slot::Pending); } } - /// 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; - while let Ok((checkout, image)) = self.results.try_recv() { - self.insert(checkout, image); + while let Ok((checkout, lookup)) = self.results.try_recv() { + let slot = match lookup { + Ok(image) => Slot::Resolved(image), + Err(_) => Slot::Unreachable { + retry_at: now + RETRY_AFTER, + }, + }; + self.insert(checkout, slot); changed = true; } changed } pub(crate) fn image(&self, checkout: Checkout<&str>) -> Option> { - self.slot(checkout).cloned().flatten() + match self.slot(checkout)? { + Slot::Resolved(image) => image.clone(), + Slot::Pending | Slot::Unreachable { .. } => None, + } } fn slot(&self, checkout: Checkout<&str>) -> Option<&Slot> { @@ -175,27 +220,30 @@ impl Avatars { } /// The GitHub owner behind a checkout's `upstream` remote, else its `origin`. -fn repo_owner(checkout: Checkout<&str>) -> Option { +/// Fails only when a remote host could not be read; local Git failures and +/// absent remotes are `None`. +fn repo_owner(checkout: Checkout<&str>) -> herdr_client::Result> { let deadline = Instant::now() + REMOTE_TIMEOUT; - ["remote.upstream.url", "remote.origin.url"] - .into_iter() - .find_map(|key| { - 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. - Checkout::Ssh { target, git_dir } => herdr_client::remote_config_value( - target, - git_dir, - key, - deadline.checked_duration_since(Instant::now())?, - || false, - ) - .ok() - .flatten(), - }?; - github_owner(&url) - }) + for key in ["remote.upstream.url", "remote.origin.url"] { + 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. + 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)?, + || false, + )?, + }; + if let Some(owner) = url.as_deref().and_then(github_owner) { + return Ok(Some(owner)); + } + } + Ok(None) } fn local_config(cwd: &str, key: &str) -> Option { diff --git a/crates/herdr-gpui/src/avatars/tests.rs b/crates/herdr-gpui/src/avatars/tests.rs index 84b722a27..aa59c9929 100644 --- a/crates/herdr-gpui/src/avatars/tests.rs +++ b/crates/herdr-gpui/src/avatars/tests.rs @@ -3,6 +3,7 @@ use super::*; mod remote; mod remote_userinfo; +mod retry; // Exercises the disk cache; other platforms have no cache root. #[cfg(any(unix, windows))] @@ -84,12 +85,15 @@ fn profile_avatar_urls_never_accept_credentials_or_other_hosts() { fn loads_local_repository_owner_avatar() { let cwd = std::env::var("HERDR_AVATAR_TEST_REPO").expect("set a local GitHub repository path"); let checkout = Checkout::Local(cwd.as_str()); - let owner = repo_owner(checkout).expect("repository has a GitHub origin"); + let owner = repo_owner(checkout) + .ok() + .flatten() + .expect("repository has a GitHub origin"); let mut avatars = Avatars::new(); - avatars.request(checkout); + avatars.request(checkout, Instant::now()); let deadline = Instant::now() + Duration::from_secs(10); while avatars.image(checkout).is_none() { - avatars.poll(); + avatars.poll(Instant::now()); assert!(Instant::now() < deadline, "avatar worker timed out"); thread::sleep(Duration::from_millis(20)); } @@ -183,7 +187,7 @@ fn accepts_only_supported_content_types() { } } -type Results = mpsc::Sender<(Checkout, Slot)>; +type Results = mpsc::Sender<(Checkout, Lookup)>; /// An `Avatars` whose worker ends are the test's: the request receiver and /// the result sender. @@ -202,10 +206,11 @@ pub(super) fn detached() -> (Avatars, mpsc::Receiver>, Results) #[test] fn deduplicates_pending_hits_and_misses_and_drains_results() { let (mut avatars, incoming, outgoing) = detached(); - assert!(!avatars.poll()); + 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(Checkout::Local(cwd)); + avatars.request(Checkout::Local(cwd), now); } assert_eq!( incoming.try_iter().collect::>(), @@ -217,23 +222,23 @@ fn deduplicates_pending_hits_and_misses_and_drains_results() { let image = Arc::new(Image::empty()); assert!( outgoing - .send((Checkout::Local("hit".into()), Some(image.clone()))) + .send((Checkout::Local("hit".into()), Ok(Some(image.clone())))) .is_ok() ); assert!( outgoing - .send((Checkout::Local("miss".into()), None)) + .send((Checkout::Local("miss".into()), Ok(None))) .is_ok() ); - assert!(avatars.poll()); - assert!(!avatars.poll()); + assert!(avatars.poll(now)); + assert!(!avatars.poll(now)); assert!( avatars .image(Checkout::Local("hit")) .is_some_and(|cached| Arc::ptr_eq(&cached, &image)) ); assert!(avatars.image(Checkout::Local("miss")).is_none()); - avatars.request(Checkout::Local("hit")); - avatars.request(Checkout::Local("miss")); + 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/remote.rs b/crates/herdr-gpui/src/avatars/tests/remote.rs index 6bf60eb27..cab9e380f 100644 --- a/crates/herdr-gpui/src/avatars/tests/remote.rs +++ b/crates/herdr-gpui/src/avatars/tests/remote.rs @@ -60,6 +60,7 @@ fn only_the_local_daemon_resolves_directories_with_local_git() { #[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", @@ -70,7 +71,7 @@ fn remote_avatars_are_kept_per_host_and_apart_from_local_paths() { git_dir, }; for checkout in [devbox, other, Checkout::Local(git_dir), devbox, other] { - avatars.request(checkout); + avatars.request(checkout, now); } assert_eq!( incoming.try_iter().collect::>(), @@ -83,11 +84,11 @@ fn remote_avatars_are_kept_per_host_and_apart_from_local_paths() { let image = Arc::new(Image::empty()); assert!( outgoing - .send((devbox.to_owned(), Some(image.clone()))) + .send((devbox.to_owned(), Ok(Some(image.clone())))) .is_ok() ); - assert!(outgoing.send((other.to_owned(), None)).is_ok()); - assert!(avatars.poll()); + assert!(outgoing.send((other.to_owned(), Ok(None))).is_ok()); + assert!(avatars.poll(now)); assert!( avatars .image(devbox) @@ -100,12 +101,13 @@ fn remote_avatars_are_kept_per_host_and_apart_from_local_paths() { #[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 }); + avatars.request(Checkout::Ssh { target, git_dir }, now); } assert_eq!(incoming.try_iter().count(), MAX_REQUESTS); } @@ -118,7 +120,8 @@ fn reads_the_repository_owner_over_ssh() { let owner = repo_owner(Checkout::Ssh { target: &target, git_dir: &git_dir, - }); + }) + .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..1eea7c9ab --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/retry.rs @@ -0,0 +1,96 @@ +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)), Ok(None))); +} diff --git a/crates/herdr-gpui/src/endpoint/polling.rs b/crates/herdr-gpui/src/endpoint/polling.rs index e2547d1eb..322f93301 100644 --- a/crates/herdr-gpui/src/endpoint/polling.rs +++ b/crates/herdr-gpui/src/endpoint/polling.rs @@ -86,9 +86,10 @@ impl HerdrWindow { (&mut self.avatars, &endpoint.live.snapshot) { let target = &endpoint.connection.target; + let now = Instant::now(); for workspace in &snapshot.workspaces { if let Some(checkout) = Checkout::of(index == 0, target, workspace) { - avatars.request(checkout); + avatars.request(checkout, now); } } } 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()) { From f1ca89db7d010c835f417fd6510307489a37f497 Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:37:21 -0700 Subject: [PATCH 03/11] test(sidebar): expect group members' labels to take the arrow column cc727de8 stopped reserving an empty chevron column on rows that draw no chevron, and updated the headless width checks, but the native paint probe still subtracted ARROW_RESERVE from a group member's label. The Windows native GUI job, the only CI job that runs it, then failed on every branch with "child label width: actual 179px, expected 161px". --- crates/herdr-gpui/src/sidebar/layout_tests.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/herdr-gpui/src/sidebar/layout_tests.rs b/crates/herdr-gpui/src/sidebar/layout_tests.rs index 0c7f45493..d8d07c202 100644 --- a/crates/herdr-gpui/src/sidebar/layout_tests.rs +++ b/crates/herdr-gpui/src/sidebar/layout_tests.rs @@ -371,11 +371,12 @@ impl PaintedText { // describe the previous layout. Compute the sidebar column directly. let left = px(bar + look.content_x() + layout.child_indent() + super::STATUS_WIDTH + layout.gap()); + // A group member draws no chevron, so its label also takes the + // arrow column its head reserves. let width = px(look.content_width(super::SIDEBAR_WIDTH) - super::STATUS_WIDTH - layout.gap() - - layout.child_indent() - - super::ARROW_RESERVE); + - layout.child_indent()); ensure!( near(self.bounds.left(), left), "child label left: actual {:?}, expected {left:?}", From 2e7ac189c931bef8f8686320ecae67a19320fe45 Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:48:08 -0700 Subject: [PATCH 04/11] test(sidebar): give the child probe's fixtures the arrow column The headless checks of the native child probe built their painted labels at the old width, which still gave up the arrow column. Each now adds ARROW_RESERVE (18px) to match the probe's expectation. --- .../herdr-gpui/src/sidebar/layout_tests/probe_bar.rs | 2 +- .../src/sidebar/layout_tests/probe_scale.rs | 6 +++--- crates/herdr-gpui/src/sidebar/layout_tests/probes.rs | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/herdr-gpui/src/sidebar/layout_tests/probe_bar.rs b/crates/herdr-gpui/src/sidebar/layout_tests/probe_bar.rs index 194ac4340..4b819b03e 100644 --- a/crates/herdr-gpui/src/sidebar/layout_tests/probe_bar.rs +++ b/crates/herdr-gpui/src/sidebar/layout_tests/probe_bar.rs @@ -7,7 +7,7 @@ fn native_child_probe_expects_the_label_beside_the_activity_bar() { use crate::config::{Density, LayoutMode}; let mode = LayoutMode::from(Density::Normal); let bar = crate::sidebar::activity::BAR_WIDTH; - let bounds = Bounds::new(point(px(44. + bar), px(100.)), size(px(161.), px(16.))); + let bounds = Bounds::new(point(px(44. + bar), px(100.)), size(px(179.), px(16.))); let probe = PaintedText { bounds, mask: bounds, diff --git a/crates/herdr-gpui/src/sidebar/layout_tests/probe_scale.rs b/crates/herdr-gpui/src/sidebar/layout_tests/probe_scale.rs index 3b3c2f14f..e1575f65b 100644 --- a/crates/herdr-gpui/src/sidebar/layout_tests/probe_scale.rs +++ b/crates/herdr-gpui/src/sidebar/layout_tests/probe_scale.rs @@ -7,7 +7,7 @@ use super::*; fn child_probe_allows_less_than_one_device_pixel_of_snapping() { use crate::config::{Density, LayoutMode}; let mode = LayoutMode::from(Density::Normal); - let bounds = Bounds::new(point(px(44.), px(100.)), size(px(161.), px(16.))); + let bounds = Bounds::new(point(px(44.), px(100.)), size(px(179.), px(16.))); let mut probe = PaintedText { bounds, mask: bounds, @@ -16,10 +16,10 @@ fn child_probe_allows_less_than_one_device_pixel_of_snapping() { width: px(94.), clipped: false, }; - probe.bounds.size.width = px(241. / 1.5); + probe.bounds.size.width = px(268. / 1.5); probe.mask = probe.bounds; probe.verify_child("sidebar-child", mode, 0., 1.5).unwrap(); - probe.bounds.size.width = px(160.); + probe.bounds.size.width = px(178.); probe.mask = probe.bounds; assert!( probe diff --git a/crates/herdr-gpui/src/sidebar/layout_tests/probes.rs b/crates/herdr-gpui/src/sidebar/layout_tests/probes.rs index 248b55a8a..0e52ac286 100644 --- a/crates/herdr-gpui/src/sidebar/layout_tests/probes.rs +++ b/crates/herdr-gpui/src/sidebar/layout_tests/probes.rs @@ -6,16 +6,16 @@ fn native_child_probe_reports_geometry_and_glyph_failures_without_panicking() { // Rounded rows move content in by the highlight's inset, the density's // gap, on both edges. for (mode, left, width) in [ - (LayoutMode::from(Density::Comfortable), 56., 145.), - (LayoutMode::from(Density::Normal), 44., 161.), - (LayoutMode::from(Density::Compact), 38., 169.), + (LayoutMode::from(Density::Comfortable), 56., 163.), + (LayoutMode::from(Density::Normal), 44., 179.), + (LayoutMode::from(Density::Compact), 38., 187.), ( LayoutMode::new(Density::Comfortable, Style::Rounded), 64., - 129., + 147., ), - (LayoutMode::new(Density::Normal, Style::Rounded), 50., 149.), - (LayoutMode::new(Density::Compact, Style::Rounded), 42., 161.), + (LayoutMode::new(Density::Normal, Style::Rounded), 50., 167.), + (LayoutMode::new(Density::Compact, Style::Rounded), 42., 179.), ] { let bounds = Bounds::new(point(px(left), px(100.)), size(px(width), px(16.))); let mut probe = PaintedText { From 0c653561d2fe3e466eae5d4f5cfaee227efd746b Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:33:56 -0700 Subject: [PATCH 05/11] fix: keep slow SSH hosts from delaying local repository avatars SSH owner reads shared the single lookup worker with local Git lookups. A host that stopped answering after its workspace snapshot arrived held that worker for up to the 10 s read budget per repository, so several queued remote repositories could delay a newly opened local workspace's avatar by about a minute, even when its image was already on disk. Local lookups now have their own queue and worker, and SSH reads run on a separate pool of four readers, so a single stalled host cannot block other devices either. The 128-entry request bound still spans both. --- crates/herdr-gpui/src/avatars.rs | 89 +++++++++++++------ crates/herdr-gpui/src/avatars/tests.rs | 4 +- .../herdr-gpui/src/avatars/tests/workers.rs | 67 ++++++++++++++ 3 files changed, 132 insertions(+), 28 deletions(-) create mode 100644 crates/herdr-gpui/src/avatars/tests/workers.rs diff --git a/crates/herdr-gpui/src/avatars.rs b/crates/herdr-gpui/src/avatars.rs index bcbc02486..cd6e86dbd 100644 --- a/crates/herdr-gpui/src/avatars.rs +++ b/crates/herdr-gpui/src/avatars.rs @@ -3,7 +3,7 @@ mod cache; use herdr_client::{ConnectTarget, protocol::ClientShellWorkspace}; use std::{ collections::HashMap, - sync::{Arc, mpsc}, + sync::{Arc, Mutex, mpsc}, thread, time::{Duration, Instant, SystemTime}, }; @@ -21,6 +21,11 @@ const REMOTE_TIMEOUT: Duration = Duration::from_secs(10); /// without a restart, and one that stays down costs one read per interval. 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 { @@ -88,9 +93,15 @@ enum Slot { /// 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>, + // 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. local: HashMap, @@ -100,7 +111,8 @@ pub struct Avatars { 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::<(Checkout, String)>(MAX_REQUESTS); let updated = outgoing.clone(); @@ -119,35 +131,30 @@ impl Avatars { } } }); - // Deliberately detach: dropping the sender ends the worker without a UI join. - thread::spawn(move || { - for checkout in incoming { - let url = match repo_owner(checkout.as_deref()) { - Ok(owner) => owner.map(|owner| { - format!("https://avatars.githubusercontent.com/{owner}?size=48") - }), - Err(error) => { - if outgoing.send((checkout, Err(error))).is_err() { - break; - } - continue; + // 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, outgoing, refresh) = + (remote_incoming.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, &outgoing, &refresh) { + break; } - }; - 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() - { - break; } - if !fresh && let Some(url) = url { - let _ = refresh.try_send((checkout, url)); + }); + } + thread::spawn(move || { + for checkout in local_incoming { + if !answer(checkout, &outgoing, &refresh) { + break; } } }); Self { - requests, + local_requests, + remote_requests, results, local: HashMap::new(), remote: HashMap::new(), @@ -168,7 +175,11 @@ impl Avatars { } } } - if self.requests.try_send(checkout.to_owned()).is_ok() { + 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); } } @@ -219,6 +230,30 @@ impl Avatars { } } +/// 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, outgoing: &Results, refresh: &Refresh) -> bool { + let url = match repo_owner(checkout.as_deref()) { + 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 +} + /// The GitHub owner behind a checkout's `upstream` remote, else its `origin`. /// Fails only when a remote host could not be read; local Git failures and /// absent remotes are `None`. diff --git a/crates/herdr-gpui/src/avatars/tests.rs b/crates/herdr-gpui/src/avatars/tests.rs index aa59c9929..d5b232977 100644 --- a/crates/herdr-gpui/src/avatars/tests.rs +++ b/crates/herdr-gpui/src/avatars/tests.rs @@ -4,6 +4,7 @@ use super::*; mod remote; mod remote_userinfo; mod retry; +mod workers; // Exercises the disk cache; other platforms have no cache root. #[cfg(any(unix, windows))] @@ -195,7 +196,8 @@ pub(super) fn detached() -> (Avatars, mpsc::Receiver>, Results) let (requests, incoming) = mpsc::sync_channel(MAX_REQUESTS); let (outgoing, results) = mpsc::channel(); let avatars = Avatars { - requests, + local_requests: requests.clone(), + remote_requests: requests, results, local: HashMap::new(), remote: HashMap::new(), 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..6a4776678 --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/workers.rs @@ -0,0 +1,67 @@ +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(), + }; + (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()] + ); +} From df7ecc5447267e17fe308507bb79bdd17a85df26 Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:41:23 -0700 Subject: [PATCH 06/11] test(updater): give relaunch waits room on a loaded machine relaunch_retries_then_alerts_when_open_keeps_failing failed twice in local `just ci` runs at load average ~67: the script had logged "open failed" but had not yet run the alert when the 10 s wait expired. That path sleeps 3.25 s between about a dozen process spawns. The deadline bounds only a failing wait, so passing runs are no slower. --- crates/herdr-gpui/src/updater/brew/tests/relaunch.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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) { From 5bbdea3bdbd602b24c9ab6824d52e630856e7f4f Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:54:35 -0700 Subject: [PATCH 07/11] test(sidebar): time the hover menu's early checks from before the drift resting_on_a_workspace_opens_its_menu_once polled at Instant::now() plus half the 600 ms dwell and expected the menu to stay closed, but the drift move had stamped its own Instant::now() earlier. At load average ~65, more than 300 ms passed between the two and the menu opened. The early checks now poll at an instant taken before the move, which can never reach the dwell. --- .../sidebar/layout_tests/workspace_menu.rs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) 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); From 702f02b92c5ac760ea8c876fa771d8cd0f46ff9f Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Tue, 20 Oct 2026 10:10:49 -0700 Subject: [PATCH 08/11] fix: read SSH repository avatars only while their device is connected Remote avatar reads kept their own 60 s retry timer, independent of the window's connection to the device. A read could dial a host the window already knew was down, and a device that came back waited for that timer before its rows got avatars. Avatars now follow each saved SSH endpoint's connection. While no endpoint on a host is connected, nothing is requested for it, reads queued for it never dial, and running reads are cancelled through the SSH read's cancel hook. When a connection comes up with a snapshot, the host's unreadable checkouts are due at once, so that first snapshot asks for them again. A read that fails while the device stays connected still waits 60 s. Another session's endpoint on the same host keeps the host connected. --- crates/herdr-gpui/src/avatars.rs | 56 +++++++++---- crates/herdr-gpui/src/avatars/links.rs | 69 ++++++++++++++++ crates/herdr-gpui/src/avatars/tests.rs | 5 +- .../herdr-gpui/src/avatars/tests/reconnect.rs | 82 +++++++++++++++++++ crates/herdr-gpui/src/avatars/tests/remote.rs | 11 ++- crates/herdr-gpui/src/avatars/tests/retry.rs | 5 +- .../herdr-gpui/src/avatars/tests/workers.rs | 2 + crates/herdr-gpui/src/endpoint.rs | 6 ++ crates/herdr-gpui/src/endpoint/polling.rs | 10 ++- 9 files changed, 225 insertions(+), 21 deletions(-) create mode 100644 crates/herdr-gpui/src/avatars/links.rs create mode 100644 crates/herdr-gpui/src/avatars/tests/reconnect.rs diff --git a/crates/herdr-gpui/src/avatars.rs b/crates/herdr-gpui/src/avatars.rs index cd6e86dbd..cf5e6ed60 100644 --- a/crates/herdr-gpui/src/avatars.rs +++ b/crates/herdr-gpui/src/avatars.rs @@ -1,5 +1,6 @@ use gpui::{Image, ImageFormat}; mod cache; +mod links; use herdr_client::{ConnectTarget, protocol::ClientShellWorkspace}; use std::{ collections::HashMap, @@ -16,9 +17,9 @@ 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 waits before a sidebar -/// update may ask again. A host that is briefly down then gets its avatar -/// without a restart, and one that stays down costs one read per interval. +/// 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 @@ -85,7 +86,8 @@ enum Slot { Pending, /// 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`. + /// The host could not be read; asked again from `retry_at`, or as soon as + /// its device reconnects. Unreachable { retry_at: Instant }, } @@ -107,6 +109,9 @@ pub struct Avatars { 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 { @@ -131,23 +136,29 @@ impl Avatars { } } }); + 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, outgoing, refresh) = - (remote_incoming.clone(), outgoing.clone(), refresh.clone()); + 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, &outgoing, &refresh) { + if !answer(checkout, &offline, &outgoing, &refresh) { break; } } }); } + let local_offline = offline.clone(); thread::spawn(move || { for checkout in local_incoming { - if !answer(checkout, &outgoing, &refresh) { + if !answer(checkout, &local_offline, &outgoing, &refresh) { break; } } @@ -158,6 +169,8 @@ impl Avatars { results, local: HashMap::new(), remote: HashMap::new(), + links: HashMap::new(), + offline, } } @@ -191,6 +204,9 @@ impl Avatars { while let Ok((checkout, lookup)) = self.results.try_recv() { 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(_) => Slot::Unreachable { retry_at: now + RETRY_AFTER, }, @@ -233,8 +249,13 @@ impl Avatars { /// 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, outgoing: &Results, refresh: &Refresh) -> bool { - let url = match repo_owner(checkout.as_deref()) { +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")) } @@ -255,15 +276,22 @@ fn answer(checkout: Checkout, outgoing: &Results, refresh: &Refresh) -> } /// The GitHub owner behind a checkout's `upstream` remote, else its `origin`. -/// Fails only when a remote host could not be read; local Git failures and -/// absent remotes are `None`. -fn repo_owner(checkout: Checkout<&str>) -> herdr_client::Result> { +/// 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"] { 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, @@ -271,7 +299,7 @@ fn repo_owner(checkout: Checkout<&str>) -> herdr_client::Result> deadline .checked_duration_since(Instant::now()) .ok_or(herdr_client::Error::SshTimeout)?, - || false, + || links::is_offline(offline, target), )?, }; if let Some(owner) = url.as_deref().and_then(github_owner) { diff --git a/crates/herdr-gpui/src/avatars/links.rs b/crates/herdr-gpui/src/avatars/links.rs new file mode 100644 index 000000000..1d21b91fa --- /dev/null +++ b/crates/herdr-gpui/src/avatars/links.rs @@ -0,0 +1,69 @@ +//! 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, 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 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 d5b232977..561ad38ce 100644 --- a/crates/herdr-gpui/src/avatars/tests.rs +++ b/crates/herdr-gpui/src/avatars/tests.rs @@ -1,6 +1,7 @@ #![allow(clippy::expect_used)] use super::*; +mod reconnect; mod remote; mod remote_userinfo; mod retry; @@ -86,7 +87,7 @@ fn profile_avatar_urls_never_accept_credentials_or_other_hosts() { fn loads_local_repository_owner_avatar() { let cwd = std::env::var("HERDR_AVATAR_TEST_REPO").expect("set a local GitHub repository path"); let checkout = Checkout::Local(cwd.as_str()); - let owner = repo_owner(checkout) + let owner = repo_owner(checkout, &Default::default()) .ok() .flatten() .expect("repository has a GitHub origin"); @@ -201,6 +202,8 @@ pub(super) fn detached() -> (Avatars, mpsc::Receiver>, Results) results, local: HashMap::new(), remote: HashMap::new(), + links: HashMap::new(), + offline: links::Offline::default(), }; (avatars, incoming, outgoing) } 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..2209c4928 --- /dev/null +++ b/crates/herdr-gpui/src/avatars/tests/reconnect.rs @@ -0,0 +1,82 @@ +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()]); +} diff --git a/crates/herdr-gpui/src/avatars/tests/remote.rs b/crates/herdr-gpui/src/avatars/tests/remote.rs index cab9e380f..21e02d0b5 100644 --- a/crates/herdr-gpui/src/avatars/tests/remote.rs +++ b/crates/herdr-gpui/src/avatars/tests/remote.rs @@ -117,10 +117,13 @@ fn requests_stop_at_the_bound_across_hosts() { 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, - }) + 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 index 1eea7c9ab..6cba2d0aa 100644 --- a/crates/herdr-gpui/src/avatars/tests/retry.rs +++ b/crates/herdr-gpui/src/avatars/tests/retry.rs @@ -92,5 +92,8 @@ fn an_unreachable_retry_is_not_refused_by_the_request_bound() { 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)), Ok(None))); + 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 index 6a4776678..1d3626d70 100644 --- a/crates/herdr-gpui/src/avatars/tests/workers.rs +++ b/crates/herdr-gpui/src/avatars/tests/workers.rs @@ -27,6 +27,8 @@ fn split( results, local: HashMap::new(), remote: HashMap::new(), + links: HashMap::new(), + offline: links::Offline::default(), }; (avatars, local, ssh) } 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 322f93301..8d9fa0090 100644 --- a/crates/herdr-gpui/src/endpoint/polling.rs +++ b/crates/herdr-gpui/src/endpoint/polling.rs @@ -3,7 +3,7 @@ use super::{ACTIVATION_TIMEOUT, LOCAL, Redraw}; use crate::{HerdrWindow, avatars::Checkout}; use gpui::{ClipboardItem, Context}; -use herdr_client::RemoteHost; +use herdr_client::{ConnectTarget, RemoteHost}; use std::time::Instant; impl HerdrWindow { @@ -79,9 +79,17 @@ impl HerdrWindow { Instant::now(), ); changed = changed.max(updated); + // 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 + && endpoint.online_since().is_some() && let (Some(avatars), Some(snapshot)) = (&mut self.avatars, &endpoint.live.snapshot) { From 78586689ce36604766a7577625103eb939ecb5e8 Mon Sep 17 00:00:00 2001 From: Fabien Penso Date: Tue, 20 Oct 2026 17:46:51 -0700 Subject: [PATCH 09/11] fix: retry SSH avatars a reconnect missed and stop reads for removed devices A read still pending when its device reconnected was skipped by that connection's snapshot, and a later failure left it waiting for another endpoint update. Pending slots now record when they were asked, and a failure that arrives after a newer connection asks again at once. Devices removed from the catalog were never followed again, so their link kept the host marked online and queued reads still dialed it. The poll loop now drops links for endpoints that left the catalog. --- crates/herdr-gpui/src/avatars.rs | 20 +++++-- crates/herdr-gpui/src/avatars/links.rs | 30 +++++++++- .../herdr-gpui/src/avatars/tests/reconnect.rs | 58 +++++++++++++++++++ crates/herdr-gpui/src/endpoint/polling.rs | 6 ++ 4 files changed, 108 insertions(+), 6 deletions(-) diff --git a/crates/herdr-gpui/src/avatars.rs b/crates/herdr-gpui/src/avatars.rs index cf5e6ed60..a53ce5059 100644 --- a/crates/herdr-gpui/src/avatars.rs +++ b/crates/herdr-gpui/src/avatars.rs @@ -82,8 +82,8 @@ impl Checkout { /// What is known about one checkout's avatar. #[derive(Clone, Debug)] enum Slot { - /// Requested and not yet answered. - Pending, + /// 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 @@ -193,7 +193,7 @@ impl Avatars { Checkout::Ssh { .. } => &self.remote_requests, }; if requests.try_send(checkout.to_owned()).is_ok() { - self.insert(checkout.to_owned(), Slot::Pending); + self.insert(checkout.to_owned(), Slot::Pending { asked: now }); } } @@ -202,16 +202,26 @@ impl Avatars { pub fn poll(&mut self, now: Instant) -> bool { let mut changed = false; 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, }, }; - self.insert(checkout, slot); + if reconnected { + self.insert(checkout.clone(), slot); + self.request(checkout.as_deref(), now); + } else { + self.insert(checkout, slot); + } changed = true; } changed @@ -220,7 +230,7 @@ impl Avatars { pub(crate) fn image(&self, checkout: Checkout<&str>) -> Option> { match self.slot(checkout)? { Slot::Resolved(image) => image.clone(), - Slot::Pending | Slot::Unreachable { .. } => None, + Slot::Pending { .. } | Slot::Unreachable { .. } => None, } } diff --git a/crates/herdr-gpui/src/avatars/links.rs b/crates/herdr-gpui/src/avatars/links.rs index 1d21b91fa..f047164ec 100644 --- a/crates/herdr-gpui/src/avatars/links.rs +++ b/crates/herdr-gpui/src/avatars/links.rs @@ -1,6 +1,6 @@ //! 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, Slot}; +use super::{Avatars, Checkout, Slot}; use std::{ collections::{HashMap, HashSet}, sync::{Arc, Mutex}, @@ -53,6 +53,34 @@ impl Avatars { } } + /// 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) { diff --git a/crates/herdr-gpui/src/avatars/tests/reconnect.rs b/crates/herdr-gpui/src/avatars/tests/reconnect.rs index 2209c4928..605b3c64a 100644 --- a/crates/herdr-gpui/src/avatars/tests/reconnect.rs +++ b/crates/herdr-gpui/src/avatars/tests/reconnect.rs @@ -80,3 +80,61 @@ fn a_read_stopped_by_a_disconnect_is_due_on_the_next_request() { 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/endpoint/polling.rs b/crates/herdr-gpui/src/endpoint/polling.rs index 8d9fa0090..7e2f3790b 100644 --- a/crates/herdr-gpui/src/endpoint/polling.rs +++ b/crates/herdr-gpui/src/endpoint/polling.rs @@ -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); From 69c4acaac3d8260ef27bcaf9d041e68ea585a77c Mon Sep 17 00:00:00 2001 From: Mac <22130+theontho@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:16:48 -0700 Subject: [PATCH 10/11] test: keep login shell and closed-port tests stable under a loaded suite The login shell timeout test gave the shell one second to start and record its pids, which a busy machine missed. The closed-port test dropped its listener, so a parallel test could be given the port or a probe could connect to itself; an accepted connection now keeps the port in use. --- crates/herdr-gpui/src/login_env.rs | 6 ++++-- crates/herdr-gpui/src/network_quality/tests.rs | 13 ++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) 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. From 9fea12c5894d332877a66d937958e8f61a66875f Mon Sep 17 00:00:00 2001 From: Fabien Penso Date: Tue, 20 Oct 2026 21:46:26 -0700 Subject: [PATCH 11/11] refactor: give the pane process list its own error type Merging main left crates/herdr-gpui/src/error.rs at 1001 lines, over the file size limit. The process list's eight failures move to `processes::Error`, reached through one transparent root variant, as AGENTS.md asks of a feature's failures. Messages are unchanged. --- .../control/client/local/processes/tests.rs | 4 +-- crates/herdr-gpui/src/error.rs | 18 ++---------- crates/herdr-gpui/src/pane_menu/processes.rs | 2 +- crates/herdr-gpui/src/processes.rs | 29 +++++++++++-------- crates/herdr-gpui/src/processes/error.rs | 21 ++++++++++++++ crates/herdr-gpui/src/processes/tests.rs | 9 ++++-- 6 files changed, 49 insertions(+), 34 deletions(-) create mode 100644 crates/herdr-gpui/src/processes/error.rs 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/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/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)) )); }