From e247ab9bdfc07336eb8e0e07095287b7ade93c44 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:35:46 +0800 Subject: [PATCH] fix(remote): stop the unroutable-pane refusal spelling a profile UUID (#485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The switcher's group labels were not the last surface a deleted profile reached as a raw UUID. `spec_for` returns `Err` once the profile leaves the config, so `pane_workspace_for` builds a `PaneWorkspace` with no spec, `route_header` refuses to route it — and that refusal carried `{target:?}`, which for a `Profile` target is its config UUID and nothing else. The string is not only logged. `land_pane` hands a failed spawn to the pending pane, which prints the reason verbatim under "could not reach {machine}", so opening (or restoring) a workspace whose profile was deleted put the UUID on screen under a banner that had just gone to the trouble of not saying it. Name it the way everything else does: the workspace's own label, which `pane_workspace_for` already fills from `route_label` when the entry has no name of its own, and which survives the deletion via the route snapshot. The new test is ungated — the route a workspace builds is platform-neutral, and this file's main test module is unix-only. --- src/terminal/remote.rs | 79 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 5 deletions(-) diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 86d517dc..d7cf82ce 100644 --- a/src/terminal/remote.rs +++ b/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::*;