fix(switcher): name a machine whose profile is gone (#786)

Refs #485.

Also stops PaneWorkspace::route_header spelling a profile UUID on the restore path.

src/terminal/remote.rs

@@ -136,11 +136,24 @@ impl PaneWorkspace {
                 RouteHeader::local_stdio(program.clone(), &argv)
             }
             (_, Some(spec)) => RouteHeader::ssh((**spec).clone()),
-            (target, None) => {
-                return Err(anyhow::anyhow!(
-                    "this workspace has no SSH connection details ({target:?}), so its panes \
-                     cannot be routed"
-                ));
+            (_, None) => {
+                // Deliberately not the target: a `Profile` spells itself as
+                // its config UUID in `Display` and in `Debug` alike, and a
+                // deleted profile is exactly what empties `spec` here. This
+                // sentence is not only logged — `land_pane` hands it to the
+                // pending pane, which prints the reason verbatim under
+                // "could not reach {machine}", so the UUID reached the screen
+                // (#485). The workspace's own name is what every other
+                // surface calls this thing.
+                return Err(match self.label.as_deref() {
+                    Some(label) => anyhow::anyhow!(
+                        "{label} has no SSH connection details, so its panes cannot be routed"
+                    ),
+                    None => anyhow::anyhow!(
+                        "this workspace has no SSH connection details, so its panes \
+                         cannot be routed"
+                    ),
+                });
             }
         };
         Ok(header.for_pane())
@@ -3073,6 +3086,62 @@ mod windows_tests {
     }
 }
 
+/// Ungated on purpose: what a workspace can build a route out of is the same
+/// on every platform, and so is the name the refusal carries.
+#[cfg(test)]
+mod route_header_tests {
+    use super::*;
+    use crate::core::session::{RemoteTarget, WorkspaceId};
+
+    fn unroutable(target: RemoteTarget, label: Option<&str>) -> PaneWorkspace {
+        PaneWorkspace {
+            workspace: WorkspaceId::new(),
+            target,
+            spec: None,
+            label: label.map(str::to_string),
+            resize_echo: false,
+        }
+    }
+
+    /// A deleted profile is what empties `spec`, and the refusal built here is
+    /// what the pending pane prints verbatim under "could not reach
+    /// {machine}" — so this is one of the screens #485 is about. It used to
+    /// carry `{target:?}`, which for a `Profile` is its config UUID and
+    /// nothing else.
+    #[test]
+    fn an_unroutable_workspace_is_not_named_by_its_profile_uuid() {
+        let id = uuid::Uuid::new_v4();
+        let gone = RemoteTarget::Profile { id };
+
+        let named = unroutable(gone.clone(), Some("lager"));
+        let e = named
+            .route_header()
+            .expect_err("no spec, no route")
+            .to_string();
+        assert!(
+            !e.contains(&id.to_string()),
+            "a bare profile UUID reached the UI: {e}"
+        );
+        assert!(
+            e.contains("lager"),
+            "the entry's own name is what it is called: {e}"
+        );
+        assert!(e.contains("cannot be routed"), "{e}");
+
+        // Nothing to call it by is still no reason to print the UUID.
+        let bare = unroutable(gone, None);
+        let e = bare
+            .route_header()
+            .expect_err("no spec, no route")
+            .to_string();
+        assert!(
+            !e.contains(&id.to_string()),
+            "a bare profile UUID reached the UI: {e}"
+        );
+        assert!(e.contains("cannot be routed"), "{e}");
+    }
+}
+
 #[cfg(all(test, unix))]
 mod tests {
     use super::*;

src/ui/remote_connect.rs

@@ -69,11 +69,18 @@ pub fn available_hosts(cx: &App) -> Vec<HostChoice> {
 /// 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(),

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;
@@ -3290,6 +3299,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