From 9113057fde1c5b70bbc73d7bf264b28bb503dae3 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 18:06:06 +0800 Subject: [PATCH] fix(switcher): name a machine whose profile is gone (#485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A residual path of #485 that #645 did not cover, found while fact-checking #695. The switcher builds a group per machine it has a listing snapshot for, and those groups named themselves by the target's own spelling — but a `Profile` target spells itself as its config UUID. While the profile is configured the pass below rewrites the label from `available_hosts`, so the leak only showed once the profile was deleted or stopped resolving: the connecting banner, the install-progress line, the error band and the parked notice all announced a raw UUID. Groups built from the workspace store already answer this with `RemoteRef::route_label`, and a pane's own route answers it with `remote_connect::target_label`. Rather than add a third rule, split `target_label` into the listing lookup plus `label_from_hosts`, and have the switcher name its pending groups through that — reusing the listing it already builds one line further down, so this costs no extra read of `~/.ssh/config` on the render path. No new wording: an unresolvable profile reads "deleted profile" (`RemoteProfileGone`), the same as everywhere else. --- src/ui/remote_connect.rs | 47 +++++++++++++++++++++++++++---- src/ui/switcher.rs | 60 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/src/ui/remote_connect.rs b/src/ui/remote_connect.rs index b04ea116..c7e05457 100644 --- a/src/ui/remote_connect.rs +++ b/src/ui/remote_connect.rs @@ -69,11 +69,18 @@ pub fn available_hosts(cx: &App) -> Vec { /// exists, the target's own spelling when that is human-readable, and the /// deleted-profile placeholder for the bare-UUID case (#485). pub fn target_label(cx: &App, target: &RemoteTarget) -> String { - if let Some(choice) = available_hosts(cx) - .into_iter() - .find(|h| h.target == *target) - { - return choice.label; + label_from_hosts(&available_hosts(cx), target) +} + +/// `target_label`'s rule applied to a listing the caller already has. The +/// switcher builds `available_hosts` once per frame and names several targets +/// from it; re-listing per target would re-read `~/.ssh/config` off the disk +/// on the render path, which is the cost `route_label` goes out of its way to +/// avoid. One rule, two entry points — so a name shown next to a pane and the +/// same name shown in the switcher cannot drift apart (#485). +pub fn label_from_hosts(hosts: &[HostChoice], target: &RemoteTarget) -> String { + if let Some(choice) = hosts.iter().find(|h| h.target == *target) { + return choice.label.clone(); } match target { RemoteTarget::Profile { .. } => t(L10nKey::RemoteProfileGone).to_string(), @@ -835,6 +842,36 @@ pub fn restart_server_blocking(header: RouteHeader, label: &str) -> Result<(), S mod tests { use super::*; + /// The one rule for "what do we call a target we cannot resolve" (#485), + /// pinned where both `target_label` and the switcher's group listing read + /// it from. + #[test] + fn an_unresolvable_profile_is_named_never_spelled_as_its_uuid() { + let id = uuid::Uuid::new_v4(); + let gone = RemoteTarget::Profile { id }; + + let label = label_from_hosts(&[], &gone); + assert!( + !label.contains(&id.to_string()), + "a bare profile UUID reached the UI: {label}" + ); + assert_eq!(label, t(L10nKey::RemoteProfileGone)); + + // While the profile is configured, its own name wins. + let listed = vec![HostChoice { + target: gone.clone(), + label: "lager".into(), + detail: "qhw@222.29.101.16".into(), + }]; + assert_eq!(label_from_hosts(&listed, &gone), "lager"); + + // Targets that spell themselves readably never need the placeholder. + let alias = RemoteTarget::Alias { + alias: "build-box".into(), + }; + assert_eq!(label_from_hosts(&[], &alias), "build-box"); + } + fn request() -> InstallRequest { InstallRequest { host: "me@build-box:22".into(), diff --git a/src/ui/switcher.rs b/src/ui/switcher.rs index 2468a40c..a6359b85 100644 --- a/src/ui/switcher.rs +++ b/src/ui/switcher.rs @@ -716,6 +716,11 @@ impl Tty7App { } } + // Listed once for the whole frame: the pending groups below name + // themselves from it, and so does the pass that settles every group's + // link state further down. + let configured = remote_connect::available_hosts(cx); + for target in self.pending_machines() { let key = target.to_string(); if index.contains_key(&key) { @@ -723,7 +728,12 @@ impl Tty7App { } index.insert(key.clone(), groups.len()); groups.push(Group { - label: key.clone(), + // Not `key`: a `Profile` target spells itself as its config + // UUID, so a machine whose profile has been deleted would + // announce itself to the banners below by a raw UUID (#485). + // The pass below overwrites this while the profile is still + // configured; this is what is left when it is not. + label: remote_connect::label_from_hosts(&configured, &target), key, endpoint: String::new(), target: Some(target), @@ -838,7 +848,6 @@ impl Tty7App { // trouble banners under the list come out in a stable order. groups.sort_by(|a, b| a.key.is_empty().cmp(&b.key.is_empty()).reverse()); - let configured = remote_connect::available_hosts(cx); for group in &mut groups { let Some(target) = group.target.clone() else { group.link = Link::Local; @@ -3286,6 +3295,53 @@ fn glyph_col(w: f32, child: impl IntoElement) -> impl IntoElement { mod tests { use super::*; + /// #485 on the path #645 did not cover. A machine the switcher knows only + /// from a listing snapshot has no store entry to name it, so its group + /// used to be labelled by the target's own spelling — and a `Profile` + /// target spells itself as its config UUID. Delete the profile and every + /// banner under the list announced a raw UUID. + #[gpui::test] + fn a_pending_machine_whose_profile_is_gone_is_not_named_by_its_uuid( + cx: &mut gpui::TestAppContext, + ) { + use crate::core::session::RemoteTarget; + + let (app, _vcx) = crate::ui::app::test_window::harness(cx); + + // A profile id that is in no config: the state left behind when the + // profile a machine was reached through is deleted. + let id = uuid::Uuid::new_v4(); + let target = RemoteTarget::Profile { id }; + + app.update(cx, |app, _| { + app.host_snapshots.insert( + target.host_id(), + super::HostSnapshot { + target: target.clone(), + rows: Vec::new(), + }, + ); + }); + + app.update(cx, |app, cx| { + let groups = app.switcher_groups(cx); + let group = groups + .iter() + .find(|g| g.target.as_ref() == Some(&target)) + .expect("the snapshot puts its machine in the list"); + assert!( + !group.label.contains(&id.to_string()), + "the switcher named a machine by its raw profile UUID: {}", + group.label + ); + assert_eq!( + group.label, + t(L10nKey::RemoteProfileGone), + "a gone profile is named here the way a pane's route names it" + ); + }); + } + /// A wrong hostname or a stale password used to be fixable only by /// finding the same machine again in Settings (#438). The machine is on /// screen here, so its host row is too — worded for what the row can