diff --git a/crates/tty7-core/src/core/config.rs b/crates/tty7-core/src/core/config.rs index 17f9e532..fce87281 100644 --- a/crates/tty7-core/src/core/config.rs +++ b/crates/tty7-core/src/core/config.rs @@ -614,7 +614,7 @@ impl Default for Config { link_url: true, link_file_open: Some(LinkFileOpen::Internal), link_file_command: None, - ssh_loopback_forward: false, + ssh_loopback_forward: true, cursor_blink: true, scrollback_limit: 10_000, new_tab_position: NewTabPosition::AfterCurrent, diff --git a/crates/tty7-core/src/daemon/control.rs b/crates/tty7-core/src/daemon/control.rs index e2ec9f0a..72bb557f 100644 --- a/crates/tty7-core/src/daemon/control.rs +++ b/crates/tty7-core/src/daemon/control.rs @@ -126,6 +126,11 @@ pub mod feature { pub const HOST_RPC: &str = "host-rpc"; pub const MACHINE_TREE: &str = "machine-tree"; pub const STDIO_BRIDGE: &str = "stdio-bridge"; + /// The peer can say what is running inside one of its panes, and what that + /// is listening on. Without it a remote pane's processes and ports are + /// simply unknown here: the pane lives in the peer's registry, and the + /// local daemon this client would otherwise ask has never heard of it. + pub const PANE_PROCS: &str = "pane-procs"; } pub use crate::host::{Entry, MTime, Meta, Output, SearchHit}; @@ -309,6 +314,12 @@ pub enum ControlRequest { }, AgentStates, + /// What is running inside one of the peer's panes, and what it is + /// listening on. `pane_id` is the peer's own id for it — the same one the + /// client spawned the pane with. + PaneProcs { + pane_id: u64, + }, Routes, Status, } @@ -353,7 +364,11 @@ impl ControlRequest { | RepoRoot { .. } | WatchOpen { .. } | WatchSet { .. } - | WatchClose { .. } => Duration::from_secs(5), + | WatchClose { .. } + // A poll, on a two-second timer: waiting longer than the gap + // between asks would only stack up requests behind a peer that has + // stopped answering. + | PaneProcs { .. } => Duration::from_secs(5), ReadFile { .. } | WriteFile { .. } => Duration::from_secs(30), CreateFileNew { .. } | CreateDir { .. } | Rename { .. } | Remove { .. } => { Duration::from_secs(10) @@ -430,6 +445,7 @@ pub enum ReplyOk { TabTree(Box), Panes(Vec), AgentStates(Vec), + PaneProcs(crate::daemon::protocol::PaneProcs), Routes(Vec), Status(ServerStatus), } @@ -1496,6 +1512,7 @@ mod tests { workspace: None, }, ControlRequest::AgentStates, + ControlRequest::PaneProcs { pane_id: 7 }, ControlRequest::Routes, ControlRequest::Status, ] @@ -2226,6 +2243,7 @@ mod tests { s(5), ), (R::AgentStates, s(5)), + (R::PaneProcs { pane_id: 7 }, s(5)), (R::Routes, s(5)), (R::Status, s(5)), ]; diff --git a/crates/tty7-core/src/daemon/server.rs b/crates/tty7-core/src/daemon/server.rs index 04a10fb6..6c5d7992 100644 --- a/crates/tty7-core/src/daemon/server.rs +++ b/crates/tty7-core/src/daemon/server.rs @@ -101,6 +101,10 @@ impl crate::host::server::PaneDirectory for Registry { self.list() } + fn pane_procs(&self, pane_id: u64) -> crate::daemon::protocol::PaneProcs { + self.get(pane_id).map(|p| p.procs()).unwrap_or_default() + } + fn agent_states(&self) -> Vec { let panes: Vec> = self.panes.lock().unwrap().values().cloned().collect(); let mut states: Vec<_> = panes.iter().filter_map(|p| p.agent_state()).collect(); diff --git a/crates/tty7-core/src/daemon/ssh/forward.rs b/crates/tty7-core/src/daemon/ssh/forward.rs index c1e28ead..726ff9f1 100644 --- a/crates/tty7-core/src/daemon/ssh/forward.rs +++ b/crates/tty7-core/src/daemon/ssh/forward.rs @@ -237,6 +237,17 @@ fn loop_exit_status(exit: LoopExit) -> ForwardStatus { ) } +/// The reason a just-started forward is not listening, if it is not. +/// +/// Only ever a bind failure at this point: everything else that can stop a +/// forward happens later, in the accept loop. +fn bind_error(status: &SharedStatus) -> Option { + match &*status.lock().unwrap() { + ForwardStatus::Error(e) => Some(e.clone()), + ForwardStatus::Listening => None, + } +} + /// A forward that never got as far as a listening socket. It has no task, so /// nothing will ever move it off this status. fn bind_failed(rule: &SshForwardRule, e: io::Error) -> SharedStatus { @@ -611,18 +622,36 @@ impl SshForwardRegistry { if let Some(local_port) = self.find_auto_local(owner, remote_host, remote_port) { return Ok(LoopbackForward { local_port }); } - let rule = SshForwardRule { + let mut rule = SshForwardRule { kind: SshForwardKind::Local, bind_host: "127.0.0.1".to_string(), - bind_port: 0, + // The far side's own number first. An automatic forward used to + // always bind 0, so the remote's :3000 came out on a different + // five-digit port every session — an address nobody could predict, + // guess or bookmark. When the number is free here, keeping it makes + // localhost:3000 mean what it says. + bind_port: remote_port, target_host: remote_host.to_string(), target_port: remote_port, description: Some(format!("localhost link → :{remote_port}")), }; let id = self.next_id.fetch_add(1, Ordering::Relaxed); - let (bind_port, status, cancel) = self.start_local(&conn, &rule).await; - if let ForwardStatus::Error(e) = &*status.lock().unwrap() { - return Err(io::Error::other(e.clone())); + let (mut bind_port, mut status, mut cancel) = self.start_local(&conn, &rule).await; + // Taken here, or a privileged port this process may not bind. Neither + // is a reason to fail: 0 asks the OS for one that works, which is what + // this did before it tried for the matching number. + if let Some(e) = bind_error(&status) { + if rule.bind_port == 0 { + return Err(io::Error::other(e)); + } + log::debug!( + "loopback forward could not keep :{remote_port} locally ({e}); asking the OS for a port" + ); + rule.bind_port = 0; + (bind_port, status, cancel) = self.start_local(&conn, &rule).await; + if let Some(e) = bind_error(&status) { + return Err(io::Error::other(e)); + } } let entry = ForwardEntry { id, diff --git a/crates/tty7-core/src/host/mod.rs b/crates/tty7-core/src/host/mod.rs index dcb0a724..6bd0a7de 100644 --- a/crates/tty7-core/src/host/mod.rs +++ b/crates/tty7-core/src/host/mod.rs @@ -260,6 +260,20 @@ pub trait Host: Send + Sync + 'static { fn is_connected(&self) -> bool { true } + + /// What is running inside one of this host's panes, and what it is + /// listening on — or `None` where this host cannot say. + /// + /// `None` is not "nothing is running": it is the answer from a host whose + /// panes are somebody else's to describe. The local host gives it, because + /// its panes belong to the daemon the caller asks directly; so does a peer + /// too old to know the request. Callers must keep the two apart — an empty + /// list means the pane really is serving nothing, and drawing "no ports" + /// over "we could not ask" is how a remote pane came to look idle while a + /// dev server was up in it. + fn pane_procs(&self, _pane_id: u64) -> Option { + None + } } /// Did this watch event mean something *changed*, or only that something was diff --git a/crates/tty7-core/src/host/remote.rs b/crates/tty7-core/src/host/remote.rs index 71b70061..7183abe8 100644 --- a/crates/tty7-core/src/host/remote.rs +++ b/crates/tty7-core/src/host/remote.rs @@ -253,6 +253,30 @@ impl Host for RemoteHost { }) } + /// The peer owns these panes' PTYs, so it is the one that can walk their + /// process trees. A peer that does not announce the feature is not asked: + /// it would answer `Err` and the caller cannot tell that apart from a pane + /// serving nothing. + fn pane_procs(&self, pane_id: u64) -> Option { + if !self + .peer() + .has_feature(crate::daemon::control::feature::PANE_PROCS) + { + return None; + } + match self.call(ControlRequest::PaneProcs { pane_id }) { + Ok(ReplyOk::PaneProcs(procs)) => Some(procs), + Ok(other) => { + log::warn!("PaneProcs answered with {other:?}"); + None + } + Err(e) => { + log::debug!("could not read pane {pane_id}'s processes: {e}"); + None + } + } + } + fn remove(&self, p: &Path, recursive: bool) -> io::Result<()> { self.expect_unit(ControlRequest::Remove { path: wire_path(p), diff --git a/crates/tty7-core/src/host/server.rs b/crates/tty7-core/src/host/server.rs index 8f7d7e5e..1f2fa862 100644 --- a/crates/tty7-core/src/host/server.rs +++ b/crates/tty7-core/src/host/server.rs @@ -28,6 +28,12 @@ pub trait PaneDirectory: Send + Sync { fn pane_count(&self) -> u64; fn panes(&self) -> Vec; fn agent_states(&self) -> Vec; + /// What is running inside one pane, and what it is listening on. + /// + /// Answered by whoever owns the pane's PTY, which for a remote workspace + /// is this process and not the client's own daemon: the client asks over + /// the control link precisely because the processes are here. + fn pane_procs(&self, pane_id: u64) -> crate::daemon::protocol::PaneProcs; } #[derive(Clone, Default)] @@ -368,6 +374,12 @@ fn handshake( feature::HOST_RPC.to_string(), feature::STDIO_BRIDGE.to_string(), ]; + // Only where there are panes to ask about. A control peer serving no panes + // would answer every ask with an empty list, which reads to a client as + // "nothing is listening" rather than as "I cannot tell you". + if services.panes.is_some() { + features.push(feature::PANE_PROCS.to_string()); + } if services.machine.is_some() { features.push(feature::MACHINE_TREE.to_string()); } @@ -831,6 +843,15 @@ fn run_request( ), Vec::new(), ), + ControlRequest::PaneProcs { pane_id } => ( + ReplyOk::PaneProcs( + conn.panes + .as_ref() + .map(|p| p.pane_procs(pane_id)) + .unwrap_or_default(), + ), + Vec::new(), + ), ControlRequest::Routes => ( ReplyOk::Routes(crate::daemon::ssh::SshManager::global().routes()), Vec::new(), @@ -1676,6 +1697,23 @@ mod aggregate_tests { self.panes.clone() } + fn pane_procs(&self, pane_id: u64) -> crate::daemon::protocol::PaneProcs { + crate::daemon::protocol::PaneProcs { + procs: vec![crate::daemon::protocol::ProcEntry { + pid: 900 + pane_id as u32, + name: "node".into(), + depth: 0, + foreground: true, + }], + ports: vec![crate::daemon::protocol::PortEntry { + port: 3000, + pid: 900 + pane_id as u32, + name: "node".into(), + addr: "*".into(), + }], + } + } + fn agent_states(&self) -> Vec { vec![PaneAgentState { pane_id: 7, @@ -1750,6 +1788,45 @@ mod aggregate_tests { assert_eq!(states[0].state.session_id.as_deref(), Some("sess-7")); } + /// A remote workspace's pane runs on the peer, so the peer is the only + /// one that can walk its process tree — the client's own daemon has never + /// heard of the pane. Without this request its ports were simply invisible. + #[test] + fn a_peer_says_what_is_listening_inside_one_of_its_panes() { + let services = Services { + panes: Some(Arc::new(ThreePanesOneAgent { panes: Vec::new() })), + ..Services::none() + }; + let client = client_with(services); + + assert!( + client.hello().has_feature(feature::PANE_PROCS), + "a peer that serves panes has to say it can describe them" + ); + let ReplyOk::PaneProcs(procs) = client + .call(ControlRequest::PaneProcs { pane_id: 4 }) + .unwrap() + else { + panic!("PaneProcs must answer with ReplyOk::PaneProcs"); + }; + assert_eq!(procs.ports.len(), 1); + assert_eq!(procs.ports[0].port, 3000); + assert_eq!( + procs.ports[0].pid, 904, + "the answer is about the pane that was asked for" + ); + } + + /// The feature is the client's only way to tell "nothing is listening" + /// from "nobody here can tell you", and a process serving no panes is the + /// second. Announcing it anyway would draw an empty Ports list over a + /// question that was never answered. + #[test] + fn a_process_with_no_panes_does_not_claim_it_can_list_ports() { + let client = client_with(Services::none()); + assert!(!client.hello().has_feature(feature::PANE_PROCS)); + } + #[test] fn machine_get_overlays_live_pane_titles() { let dir = tempfile::TempDir::new().unwrap(); diff --git a/docs/reference/configuration.mdx b/docs/reference/configuration.mdx index 75d593e8..3aa90be5 100644 --- a/docs/reference/configuration.mdx +++ b/docs/reference/configuration.mdx @@ -139,7 +139,7 @@ angle brackets, and a tab: | `link_url` | bool | `true` | Underline and open URLs on ⌘/Ctrl-click. | | `link_file_open` | string | `internal` | What a file link opens: `internal` (tty7's editor, at the line), `system` (the OS file association), `command`. A file on another machine always uses `internal`. | | `link_file_command` | string | — | Command for file links under `link_file_open: command`. `{path}`, `{line}`, `{column}` are substituted; a flag whose value is missing is dropped. | -| `ssh_loopback_forward` | bool | `false` | Open `localhost:PORT` links through a temporary forward when the pane is in SSH. | +| `ssh_loopback_forward` | bool | `true` | Forward the ports a remote pane starts serving, and open its `localhost:PORT` links through those forwards. | ## Notifications diff --git a/docs/remote/port-forwarding.mdx b/docs/remote/port-forwarding.mdx index 99e1b1aa..a74518fc 100644 --- a/docs/remote/port-forwarding.mdx +++ b/docs/remote/port-forwarding.mdx @@ -3,6 +3,37 @@ title: "Port forwarding" description: "Local, remote, and dynamic forwards — preconfigured or added mid-session." --- +## Ports you did not ask to forward + +When a pane is on a remote machine, tty7 watches what its processes are +listening on and forwards those ports for you. Start a dev server on the remote +`:3000` and a moment later a notice says it is at `http://localhost:3000` — +same number, no rule to write. + +The **Ports** section of the Info panel is the whole list: one row per listener, +the process that owns it, and where it comes out on this machine. The globe +opens it in a browser, the copy tile takes the address that works from here, +and ✕ takes the forward back down. + +Ports are forwarded once each. Remove one and it stays removed — the offer is +made a single time per port, not on every poll. + +The local port matches the remote one whenever it is free here. When it is not — +something else on this machine already has `:3000` — the OS picks another and +the row says which. + +Turn the whole thing off under **Settings → Terminal → Links → Forward remote +ports** (`ssh_loopback_forward: false`). + + + In a remote workspace the ports are found by the **`tty7-server` on that + machine** — the panes are its, and this machine's daemon has never heard of + them. A server too old to answer says so in the Ports section rather than + showing an empty list; updating it is what fixes that. Panes that ssh'd + somewhere from inside a local pane are a different case: those processes + belong to no tty7 server, so nothing lists them. + + ## The three kinds | | What it does | @@ -25,23 +56,26 @@ later `8080 → 3000` explains nothing. ## Adding one mid-session -*SSH: Port Forwarding* in the command palette opens the **Forwards** panel for -the current connection. Add a rule there and it starts immediately; remove it -and it stops. These live only as long as the session unless you save them into -the profile. +The **+** on the Ports section — or *SSH: Port Forwarding* in the command +palette — opens a form asking for one thing: the port the remote is serving on. +It comes out here under the same number, and the form says so before you commit +to it. - - The forwards panel +**Advanced** opens the rest of the grammar: remote and dynamic forwards, a bind +host other than loopback, a target on some third machine, a description. You +need it about as often as you need `ssh -R`. + +These live only as long as the session unless you save them into the profile. + + + The ports section -## The one-click shortcut +## Links in the terminal -⌘-clicking a `localhost:PORT` link inside an SSH pane can open a -temporary forward for exactly that port and then open the browser — turn on -**Settings → Terminal → Links → Forward SSH loopback links**. - -That is the right tool for "let me look at this dev server once". For something -you use every day, put it in the profile. +⌘-clicking a `localhost:PORT` link inside a remote pane opens the +forward for that port if there is not one yet, then the browser. Same machinery +as the Ports list, reached from the output instead of the panel. ## Jump hosts and proxies diff --git a/docs/terminal/links.mdx b/docs/terminal/links.mdx index 021e29e4..89a6fc7f 100644 --- a/docs/terminal/links.mdx +++ b/docs/terminal/links.mdx @@ -67,12 +67,16 @@ that did not moves to the built-in editor. ⌘-clicking `localhost:3000` opens it in your browser, which is only useful if the server is on this machine. -When the pane is inside an SSH session it usually is not. Turn on **Settings → -Terminal → Links → Forward SSH loopback links** (`ssh_loopback_forward: true`) -and tty7 opens a temporary port forward through that connection first, so the -link reaches the server on the remote machine. +When the pane is inside an SSH session it usually is not, so tty7 opens a port +forward through that connection first and points the browser at the local end. +The forward is built on the click if it is not there already. + +This is on by default; **Settings → Terminal → Links → Forward remote ports** +(`ssh_loopback_forward`) turns it off, along with the automatic forwarding of +ports a remote pane starts serving. - For a forward you want to keep, set one up properly instead — - [port forwarding](/remote/port-forwarding). + Every port a remote pane is listening on is listed in the Info panel, whether + or not its URL was ever printed — see [port + forwarding](/remote/port-forwarding). diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 176533db..785587f6 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -481,6 +481,18 @@ enum LinkAt { None, } +/// What it takes to open one of a pane's loopback ports from this machine. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum PortRoute { + /// The port is already reachable here under its own number. + Direct, + /// A local forward has to exist first; the pane's `ForwardRoute` builds it. + Forward, + /// Another machine's port, with no way to reach it from here. + #[default] + Blocked, +} + #[derive(Clone, Debug, PartialEq)] pub(super) enum LoopbackPlan { Direct, @@ -489,6 +501,26 @@ pub(super) enum LoopbackPlan { ForwardOnWorkspace(Box), } +/// What a pane's loopback port takes to open, given how its links would be +/// forwarded and whether the daemon listing it is this machine's. +/// +/// Deliberately not "is the pane local": a remote pane's :3000 is perfectly +/// reachable once a forward exists, and building that forward is something +/// this app already knows how to do. Answering only "local or not" is what +/// left the Ports list showing a port it then refused to open. +pub(crate) fn port_route_of(plan: &LoopbackPlan, local: bool) -> PortRoute { + match plan { + // WSL shares this machine's loopback, so its ports are already here + // under the same number. + LoopbackPlan::NoForwardNeeded => PortRoute::Direct, + LoopbackPlan::ForwardOnPane(_) | LoopbackPlan::ForwardOnWorkspace(_) => PortRoute::Forward, + // No plan and no forwarding: this machine's own ports open, and + // another machine's do not. + LoopbackPlan::Direct if local => PortRoute::Direct, + LoopbackPlan::Direct => PortRoute::Blocked, + } +} + pub(super) fn loopback_plan( enabled: bool, workspace: Option<&crate::terminal::PaneWorkspace>, @@ -5643,12 +5675,9 @@ impl TerminalView { } let forwarded = match &plan { - LoopbackPlan::ForwardOnPane(pane_id) => RemoteTerminal::ensure_loopback_forward( - *pane_id, - loopback.forward_host(), - loopback.port, - ), - LoopbackPlan::ForwardOnWorkspace(ws) => self.ensure_workspace_loopback(ws, &loopback), + LoopbackPlan::ForwardOnPane(_) | LoopbackPlan::ForwardOnWorkspace(_) => self + .forward_route() + .ensure_loopback(loopback.forward_host(), loopback.port), LoopbackPlan::Direct | LoopbackPlan::NoForwardNeeded => unreachable!("handled above"), }; match forwarded { @@ -5666,27 +5695,14 @@ impl TerminalView { } } - fn ensure_workspace_loopback( - &self, - ws: &crate::terminal::PaneWorkspace, - loopback: &super::loopback::LoopbackUrl, - ) -> anyhow::Result { - let req = RemoteTerminal::workspace_request( - ws, - self.pane_id, - crate::daemon::protocol::WorkspaceOp::EnsureLoopback { - remote_host: loopback.forward_host().to_string(), - remote_port: loopback.port, - }, - ) - .ok_or_else(|| anyhow::anyhow!("this workspace has no SSH connection to forward over"))?; - match RemoteTerminal::on_workspace(req)? { - crate::daemon::protocol::DaemonMsg::LoopbackForward(f) => Ok(f), - other => Err(anyhow::anyhow!("unexpected reply: {other:?}")), - } + /// How forward requests about this pane reach the daemon that owns them — + /// through the workspace when there is one, and by pane id when there is + /// not. The Ports list and the port watcher build the same thing. + pub(crate) fn forward_route(&self) -> crate::ui::app::ForwardRoute { + crate::ui::app::ForwardRoute::new(self.pane_id, self.workspace.clone()) } - fn loopback_plan(&self, cx: &mut Context) -> LoopbackPlan { + fn loopback_plan(&self, cx: &gpui::App) -> LoopbackPlan { loopback_plan( cx.global::().ssh_loopback_forward, self.workspace.as_ref(), @@ -5695,10 +5711,21 @@ impl TerminalView { ) } - fn can_forward_loopback(&self, cx: &mut Context) -> bool { + fn can_forward_loopback(&self, cx: &gpui::App) -> bool { !matches!(self.loopback_plan(cx), LoopbackPlan::Direct) } + /// How a loopback port this pane is serving can be reached from here. + /// + /// The Ports list asks this about every listener it found, and it is a + /// different question from "is this pane local": a remote pane's :3000 is + /// perfectly reachable once a forward exists, and building that forward is + /// something this app already knows how to do. Answering only "local or + /// not" is what left the list showing a port it refused to open. + pub(crate) fn port_route(&self, cx: &gpui::App) -> PortRoute { + port_route_of(&self.loopback_plan(cx), self.host_id().is_local()) + } + pub fn hover_link_at( &mut self, col: usize, @@ -7579,9 +7606,10 @@ mod tests { assert!(!out.contains(" …"), "{out:?}"); } use super::{ - COMPLETION_MENU_MAX_W, LoopbackPlan, RawInput, SelectEndCopy, Typeahead, WheelRoute, - clipboard_paste_text, compose_notification_title, cwd_is_on_host, display_width, - is_typeahead_interrupt, link_path_style, loopback_plan, observe_typeahead_for_owner, + COMPLETION_MENU_MAX_W, LoopbackPlan, PortRoute, RawInput, SelectEndCopy, Typeahead, + WheelRoute, clipboard_paste_text, compose_notification_title, cwd_is_on_host, + display_width, is_typeahead_interrupt, link_path_style, loopback_plan, + observe_typeahead_for_owner, }; use super::{SCROLL_ANIM_FRAME, scroll_anim_step}; use super::{ @@ -7818,6 +7846,34 @@ mod tests { ); } + /// What the Ports list asks about every listener it found. The middle + /// case is the one that matters: a remote pane's port is not unreachable, + /// it is one forward away. + #[test] + fn a_remote_port_is_a_forward_away_rather_than_out_of_reach() { + use super::port_route_of; + assert_eq!( + port_route_of(&LoopbackPlan::ForwardOnPane(7), false), + PortRoute::Forward + ); + assert_eq!( + port_route_of(&LoopbackPlan::NoForwardNeeded, false), + PortRoute::Direct, + "WSL serves onto this machine's own loopback" + ); + assert_eq!( + port_route_of(&LoopbackPlan::Direct, true), + PortRoute::Direct, + "this machine's ports open with no help" + ); + assert_eq!( + port_route_of(&LoopbackPlan::Direct, false), + PortRoute::Blocked, + "another machine's port with forwarding turned off is not ours to \ + open — opening it here would reach some unrelated local service" + ); + } + #[test] fn wsl_workspace_needs_no_forward() { let w = ws( diff --git a/src/ui/app.rs b/src/ui/app.rs index 7a7e972c..59df219f 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -763,10 +763,18 @@ pub(crate) struct LoopbackForwardPanelState { /// Why the last Add or Save did not take, in the far side's own words. /// Cleared the moment the form is closed or the edit is abandoned. pub(crate) mf_error: Option, - /// Return, on each of the five boxes. Held here for the same reason the - /// sftp form holds its own: a live subscription on a box nothing is - /// showing would answer Return for a form that is gone. + /// Return, on each of the boxes. Held here for the same reason the sftp + /// form holds its own: a live subscription on a box nothing is showing + /// would answer Return for a form that is gone. pub(crate) mf_subs: Vec, + /// Whether the form is showing all five fields rather than the one. + /// + /// Almost every forward anyone builds by hand is "bring the remote's :3000 + /// over here", which is one number — and asking for five fields to collect + /// one number is what made the panel feel like paperwork. The rest of the + /// `ssh -L` grammar is still here, one disclosure away, for the forwards + /// that really do need it. + pub(crate) mf_advanced: bool, } pub struct Tty7App { @@ -1427,6 +1435,7 @@ impl Tty7App { mf_editing: None, mf_error: None, mf_subs: Vec::new(), + mf_advanced: false, }, sftp_panel, right_panel: Default::default(), @@ -2794,6 +2803,7 @@ impl Tty7App { pub(crate) fn managed_forward_fields(&self, cx: &gpui::App) -> ForwardFields { let val = |input: &Entity| input.read(cx).value().to_string(); ForwardFields { + advanced: self.loopback_panel.mf_advanced, kind: self.loopback_panel.mf_kind, bind_host: val(&self.loopback_panel.mf_bind_host), bind_port: val(&self.loopback_panel.mf_bind_port), @@ -2809,9 +2819,8 @@ impl Tty7App { window: &mut Window, cx: &mut Context, ) { - use crate::daemon::protocol::ForwardStatus; - - let Some(rule) = self.managed_forward_fields(cx).collect() else { + let fields = self.managed_forward_fields(cx); + let Some(rule) = fields.collect() else { // Add is disabled while the fields do not make a rule and the form // already says what is missing, so there is nothing to do here and // nothing left to explain. @@ -2838,31 +2847,32 @@ impl Tty7App { self.loopback_panel.managed = list; } - let before: Vec = self.loopback_panel.managed.iter().map(|m| m.id).collect(); - let mut failure = None; - match route.add(rule) { - // The request never got an answer. An empty list here is not "this - // pane has no forwards", it is "nobody said" — assigning it is what - // used to blank the panel on a dropped connection. - None => failure = Some(t(L10nKey::ForwardRequestFailed).to_string()), - Some(list) => { - // A rule that could not be started is registered all the same, - // with the reason in its status, so whether the add worked is a - // question about the entry it appended rather than about - // whether the call returned. - let broken = added_forward(&before, &list).and_then(|added| match &added.status { - ForwardStatus::Error(msg) => Some((added.id, msg.clone())), - ForwardStatus::Listening => None, - }); - self.loopback_panel.managed = list; - if let Some((id, msg)) = broken { - if let Some(list) = route.remove(id) { - self.loopback_panel.managed = list; - } - failure = Some(msg); + // The short form aims at the same number on both ends because that is + // the address people can predict. When it is already taken here, the + // useful answer is another port rather than a complaint: somebody who + // typed one number to forward one port has not been asked to care + // which local port it lands on, and the row says where it came out. + let retry_free_port = !fields.advanced && rule.bind_port != 0; + let mut failure = match self.place_forward(&route, rule.clone()) { + PlaceOutcome::Placed => None, + // Nobody answered, so nothing was bound and nothing would be bound + // by asking again — a second round trip would only spend another + // timeout on the way to the same sentence. + PlaceOutcome::Unreachable(msg) => Some(msg), + PlaceOutcome::Rejected(msg) if !retry_free_port => Some(msg), + PlaceOutcome::Rejected(_) => { + match self.place_forward( + &route, + crate::daemon::protocol::SshForwardRule { + bind_port: 0, + ..rule.clone() + }, + ) { + PlaceOutcome::Placed => None, + PlaceOutcome::Rejected(msg) | PlaceOutcome::Unreachable(msg) => Some(msg), } } - } + }; if let Some(msg) = failure { // Put back what the edit took out, so the worst a failed Save can @@ -2900,6 +2910,41 @@ impl Tty7App { cx.notify(); } + /// Ask the far side for one rule, and say what became of it. + /// + /// A rule that could not be started is registered all the same, with the + /// reason in its status, so whether the add worked is a question about the + /// entry it appended rather than about whether the call returned. The dead + /// entry is taken back out — a forward listed as listening on nothing is + /// worse than no forward. + fn place_forward( + &mut self, + route: &ForwardRoute, + rule: crate::daemon::protocol::SshForwardRule, + ) -> PlaceOutcome { + use crate::daemon::protocol::ForwardStatus; + + let before: Vec = self.loopback_panel.managed.iter().map(|m| m.id).collect(); + // The request never got an answer. An empty list here is not "this + // pane has no forwards", it is "nobody said" — assigning it is what + // used to blank the panel on a dropped connection. + let Some(list) = route.add(rule) else { + return PlaceOutcome::Unreachable(t(L10nKey::ForwardRequestFailed).to_string()); + }; + let broken = added_forward(&before, &list).and_then(|added| match &added.status { + ForwardStatus::Error(msg) => Some((added.id, msg.clone())), + ForwardStatus::Listening => None, + }); + self.loopback_panel.managed = list; + let Some((id, msg)) = broken else { + return PlaceOutcome::Placed; + }; + if let Some(list) = route.remove(id) { + self.loopback_panel.managed = list; + } + PlaceOutcome::Rejected(msg) + } + pub(crate) fn edit_managed_forward( &mut self, forward: crate::daemon::protocol::ManagedForward, @@ -2909,6 +2954,10 @@ impl Tty7App { self.loopback_panel.mf_kind = forward.kind; self.loopback_panel.form_pane_id = Some(forward.pane_id); self.loopback_panel.mf_error = None; + // A rule that already exists is shown whole: the short form cannot + // spell a bind host or a remote forward, so editing one through it + // would silently rewrite the parts it cannot see. + self.loopback_panel.mf_advanced = true; let target_port = if forward.target_port == 0 { String::new() } else { @@ -2974,12 +3023,19 @@ impl Tty7App { } pub(crate) fn show_ssh_forwards(&mut self, window: &mut Window, cx: &mut Context) { - let Some((pane_id, _)) = self.active_connected_native_ssh_pane(window, cx) else { + // Whatever the panel would let this pane forward, which is a wider set + // than "a connected native-ssh pane": a pane in a remote workspace + // forwards over the workspace's own connection, and the command used + // to do nothing at all there while the panel beside it worked. + let Some(ctx) = self.pane_forward_ctx(window, cx) else { return; }; + if ctx.route.is_none() { + return; + } self.set_right_panel_tab(crate::core::config::RightPanelTab::Info, cx); - if self.loopback_panel.form_pane_id != Some(pane_id) { - self.toggle_managed_forward_form(pane_id, window, cx); + if self.loopback_panel.form_pane_id != Some(ctx.pane_id) { + self.toggle_managed_forward_form(ctx.pane_id, window, cx); } } @@ -2994,6 +3050,8 @@ impl Tty7App { return; } self.loopback_panel.form_pane_id = Some(pane_id); + self.loopback_panel.mf_advanced = false; + self.loopback_panel.mf_kind = crate::daemon::protocol::SshForwardKind::Local; self.cancel_managed_forward_edit(window, cx); self.refresh_managed_forwards(pane_id, cx); self.arm_managed_forward_form(pane_id, window, cx); @@ -3041,6 +3099,12 @@ impl Tty7App { inputs[0].update(cx, |s, cx| s.focus(window, cx)); } + pub(crate) fn toggle_managed_forward_advanced(&mut self, cx: &mut Context) { + self.loopback_panel.mf_advanced = !self.loopback_panel.mf_advanced; + self.loopback_panel.mf_error = None; + cx.notify(); + } + pub(crate) fn close_managed_forward_form( &mut self, window: &mut Window, @@ -7149,7 +7213,43 @@ pub(crate) struct ForwardRoute { workspace: Option, } +/// What came of asking the far side to put one forward up. +enum PlaceOutcome { + Placed, + /// The far side answered and the rule could not be started — a bind that + /// collided, a port this process may not have. Another bind port might. + Rejected(String), + /// Nobody answered. Nothing was bound, and nothing about the rule is what + /// went wrong. + Unreachable(String), +} + +/// Who a set of forwards belongs to on the far side. +/// +/// A workspace's forwards outlive any one of its panes and are shared between +/// all of them, so "have we already offered to forward :3000" is a question +/// about the workspace — asking it per pane made switching tabs re-announce +/// every port the workspace had already forwarded. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(crate) enum ForwardOwnerKey { + Pane(u64), + Workspace(crate::core::session::WorkspaceId), +} + impl ForwardRoute { + pub(crate) fn new(pane_id: u64, workspace: Option) -> Self { + Self { pane_id, workspace } + } + + /// Which side of the daemon's own forward registry this route addresses — + /// the same split `ForwardOwner` makes there. + pub(crate) fn owner_key(&self) -> ForwardOwnerKey { + match &self.workspace { + Some(ws) => ForwardOwnerKey::Workspace(ws.workspace), + None => ForwardOwnerKey::Pane(self.pane_id), + } + } + fn workspace_op( &self, op: crate::daemon::protocol::WorkspaceOp, @@ -7213,6 +7313,37 @@ impl ForwardRoute { Self::forwards(crate::terminal::RemoteTerminal::on_workspace(req)).unwrap_or_default() } + /// The local port that reaches `remote_host:remote_port` over this route, + /// building the forward if there is not one yet. + /// + /// The far side keeps one automatic forward per endpoint and hands the + /// same port back on the next ask, so callers may treat this as "what is + /// the address here" rather than as an action with a cost — which is what + /// lets the Ports list call it on a click and the watcher call it on a + /// port it has only just noticed. + pub(crate) fn ensure_loopback( + &self, + remote_host: &str, + remote_port: u16, + ) -> anyhow::Result { + let Some(req) = self.workspace_op(crate::daemon::protocol::WorkspaceOp::EnsureLoopback { + remote_host: remote_host.to_string(), + remote_port, + }) else { + return crate::terminal::RemoteTerminal::ensure_loopback_forward( + self.pane_id, + remote_host, + remote_port, + ); + }; + match crate::terminal::RemoteTerminal::on_workspace(req)? { + crate::daemon::protocol::DaemonMsg::LoopbackForward(f) => Ok(f), + other => Err(anyhow::anyhow!( + "unexpected reply to EnsureLoopback: {other:?}" + )), + } + } + pub(crate) fn remove( &self, forward_id: u64, @@ -7240,6 +7371,10 @@ impl Render for Tty7App { self.touch_active_tab(); self.declare_displayed_panes(cx); self.scm_sync_watchers(window, cx); + // Keeps looking for new listening ports on the pane in front, panel + // open or not — a port that appears while the panel is shut is exactly + // the one worth forwarding unasked. + self.sync_port_watch(window, cx); if cx.has_active_drag() { crate::ui::reorder::clear_pending(&self.reorder); crate::ui::pane_drag::clear_landing(&self.pane_drag); @@ -10592,6 +10727,9 @@ mod managed_forward_gpui_tests { app.update_in(&mut vcx, |app, window, cx| { app.loopback_panel.managed = vec![listening(1)]; app.loopback_panel.form_pane_id = Some(1); + // These three fields are the long form's; without this the short + // form would read them as the one number it asks for. + app.loopback_panel.mf_advanced = true; let typed: [(&gpui::Entity, &str); 3] = [ (&app.loopback_panel.mf_bind_port, "9000"), (&app.loopback_panel.mf_target_host, "127.0.0.1"), @@ -10628,6 +10766,7 @@ mod managed_forward_gpui_tests { app.loopback_panel.managed = vec![listening(1)]; app.loopback_panel.form_pane_id = Some(1); app.loopback_panel.mf_editing = Some(listening(1)); + app.loopback_panel.mf_advanced = true; let typed: [(&gpui::Entity, &str); 3] = [ (&app.loopback_panel.mf_bind_port, "8080"), (&app.loopback_panel.mf_target_host, "10.0.0.6"), diff --git a/src/ui/forwards.rs b/src/ui/forwards.rs index 2adb55e7..d4395529 100644 --- a/src/ui/forwards.rs +++ b/src/ui/forwards.rs @@ -1,15 +1,13 @@ use gpui::{AnyElement, Context, Div, Entity, FontWeight, Stateful, div, prelude::*, px, rems}; use gpui_component::button::{Button, ButtonVariants as _}; use gpui_component::input::Input; -use gpui_component::{ - ActiveTheme as _, Disableable as _, Icon, IconName, Sizable as _, h_flex, v_flex, -}; +use gpui_component::{ActiveTheme as _, Disableable as _, IconName, Sizable as _, h_flex, v_flex}; use crate::daemon::protocol::{ForwardStatus, ManagedForward, SshForwardKind, SshForwardRule}; use crate::terminal::view::TerminalView; -use crate::ui::app::{CONTENT_INSET, TILE_GLYPH_SM, TILE_SIZE_SM, Tty7App}; +use crate::ui::app::{CONTENT_INSET, Tty7App}; use crate::ui::i18n::{L10nKey, t, t_fmt}; -use crate::ui::right_panel::{META, TEXT, TEXT_MONO}; +use crate::ui::right_panel::{META, TEXT_MONO}; /// How far a forward's target endpoint fades when the rule is Dynamic and has /// no target to name. The rules editor in Settings and the live Forwards panel @@ -23,6 +21,9 @@ pub(crate) const NO_TARGET_FADE: f32 = 0.4; /// rule, and `forward_form` needs to know whether there is one yet — that is /// what decides whether Add is live and whether the form says what is missing. pub(crate) struct ForwardFields { + /// Whether the whole `ssh -L` grammar is on screen. With it off the form + /// is one number and the rule is derived from it. + pub(crate) advanced: bool, pub(crate) kind: SshForwardKind, pub(crate) bind_host: String, pub(crate) bind_port: String, @@ -41,6 +42,9 @@ impl ForwardFields { /// OS to pick the port, and there is nowhere in either form to say which /// one it picked. pub(crate) fn collect(&self) -> Option { + if !self.advanced { + return self.simple_rule(); + } let bind_port: u16 = self.bind_port.trim().parse().ok().filter(|p| *p > 0)?; let (target_host, target_port) = if self.kind == SshForwardKind::Dynamic { (String::new(), 0) @@ -69,10 +73,36 @@ impl ForwardFields { }) } + /// The rule the short form's single number describes: bring the far + /// side's own `:port` over to the same number here. + /// + /// The bind port is that same number rather than 0 so the address is one + /// anybody can predict — remote :3000 is localhost:3000. When it is taken + /// on this machine `add_managed_forward` retries with 0 and the OS picks, + /// which is a better answer than handing the collision back to be solved + /// by hand. + fn simple_rule(&self) -> Option { + let port: u16 = self.target_port.trim().parse().ok().filter(|p| *p > 0)?; + Some(SshForwardRule { + kind: SshForwardKind::Local, + bind_host: "127.0.0.1".to_string(), + bind_port: port, + target_host: "localhost".to_string(), + target_port: port, + // No field for it on screen, so nothing to carry: a description + // left over from a trip through the advanced form is not something + // this rule was given. + description: None, + }) + } + /// Whether the form is still empty enough that saying what is missing /// would be nagging rather than helping — the same restraint the settings /// sheet shows through `ForwardRuleForm::is_blank`. pub(crate) fn is_blank(&self) -> bool { + if !self.advanced { + return self.target_port.trim().is_empty(); + } [ &self.bind_host, &self.bind_port, @@ -210,73 +240,7 @@ impl Tty7App { Some(bar.into_any_element()) } - pub(crate) fn forwards_section( - &self, - pane_id: Option, - cx: &mut Context, - ) -> Option { - let pane_id = pane_id?; - let open = self.loopback_panel.form_pane_id == Some(pane_id); - // The section's own affordance, and the same 24px chrome tile the Info - // tab's cwd actions use. It used to be built by hand — a 32px tile - // forced down to 24 and then set `.xsmall()`, which quietly overrode - // the 13px the icon asked for with the button size's own 12, so the - // glyph never was the size the code claimed. `chrome_tile_sized` - // derives it instead: `TILE_GLYPH_SM / BUTTON_ICON_SCALE` of the button - // size, the same pair every other 24px tile in the panel is on. - let add = crate::ui::tab_strip::chrome_tile_sized( - Button::new(("ssh-forward-add-toggle", pane_id)) - .icon(Icon::empty().path("icons/plus.svg")), - TILE_SIZE_SM, - TILE_GLYPH_SM, - open, - cx, - ) - .rounded_md() - .tooltip(if open { - t(L10nKey::Cancel) - } else { - t(L10nKey::ForwardTooltipAdd) - }) - .on_click(cx.listener(move |this, _, window, cx| { - this.toggle_managed_forward_form(pane_id, window, cx) - })) - .into_any_element(); - - let managed: Vec = self - .loopback_panel - .managed - .iter() - .filter(|m| m.pane_id == pane_id) - .cloned() - .collect(); - - let mono = cx.theme().mono_font_family.clone(); - let mut list = v_flex().px(px(CONTENT_INSET - 4.)).py(px(2.)).gap(px(1.)); - for forward in &managed { - list = list.child(self.forward_row(forward, &mono, cx)); - } - - Some( - v_flex() - .child(self.panel_subtitle(t(L10nKey::ForwardPanelTitle), true, Some(add), cx)) - .when(managed.is_empty() && !open, |this| { - this.child( - div() - .px(px(CONTENT_INSET)) - .py(px(2.)) - .text_size(rems(TEXT)) - .text_color(cx.theme().muted_foreground) - .child(crate::ui::i18n::t(crate::ui::i18n::L10nKey::None)), - ) - }) - .when(!managed.is_empty(), |this| this.child(list)) - .when(open, |this| this.child(self.forward_form(pane_id, cx))) - .into_any_element(), - ) - } - - fn forward_row( + pub(crate) fn forward_row( &self, forward: &ManagedForward, mono: &gpui::SharedString, @@ -389,19 +353,27 @@ impl Tty7App { ) } - fn forward_form(&self, pane_id: u64, cx: &mut Context) -> Div { + /// The form that adds a forward. + /// + /// Short by default — one number, because "bring the remote's :3000 over + /// here" is what nearly every hand-built forward is, and collecting one + /// number through five fields is what made this read like paperwork. The + /// full `ssh -L` grammar is one disclosure away for the rest. + pub(crate) fn forward_form(&self, pane_id: u64, cx: &mut Context) -> Div { let theme = cx.theme(); let muted = theme.muted_foreground; let danger = theme.danger; let sf = cx.global::().sidebar; let kind = self.loopback_panel.mf_kind; let editing = self.loopback_panel.mf_editing.is_some(); + let advanced = self.loopback_panel.mf_advanced; let fields = self.managed_forward_fields(cx); // The form used to accept a click on Add and then do nothing at all // when the fields did not make a rule. Now Add is only live when there // is something to add, and the line below the form says what is still // missing — but not while the form has barely been touched. - let complete = fields.collect().is_some(); + let rule = fields.collect(); + let complete = rule.is_some(); let incomplete = !complete && !fields.is_blank(); let selected = match kind { SshForwardKind::Local => 0, @@ -445,49 +417,87 @@ impl Tty7App { } }), ) - .child(self.segmented_on( - sf, - "ssh-managed-forward-kind", - &[ - t(L10nKey::ForwardLocal), - t(L10nKey::ForwardRemote), - t(L10nKey::ForwardDynamic), - ], - selected, - cx, - move |this, ix, _window, cx| { - let kind = match ix { - 1 => SshForwardKind::Remote, - 2 => SshForwardKind::Dynamic, - _ => SshForwardKind::Local, - }; - this.set_managed_forward_kind(kind, cx); - }, - )) - .child(pair( - t(L10nKey::ForwardBindLabel), - &self.loopback_panel.mf_bind_host, - &self.loopback_panel.mf_bind_port, - )) - .child( - div() - .opacity(if needs_target { 1.0 } else { NO_TARGET_FADE }) - .child(pair( - if needs_target { - t(L10nKey::ForwardToLabel) - } else { - t(L10nKey::ForwardSocksLabel) - }, - &self.loopback_panel.mf_target_host, - &self.loopback_panel.mf_target_port, - )), - ) - .child(Input::new(&self.loopback_panel.mf_description).xsmall()) + .when(!advanced, |form| { + form.child( + h_flex() + .items_center() + .gap(px(6.)) + .child( + div() + .flex_none() + .text_size(rems(META)) + .text_color(muted) + .child(t(L10nKey::ForwardPortLabel)), + ) + .child( + div() + .w(px(64.)) + .child(Input::new(&self.loopback_panel.mf_target_port).xsmall()), + ) + // Where the port will come out, said before the click + // rather than after it. The number is the same one on + // both ends unless it is taken here, which is the case + // the row itself reports once the forward exists. + .children(rule.as_ref().map(|r| { + div() + .flex_1() + .min_w_0() + .truncate() + .text_size(rems(META)) + .text_color(muted) + .child(t_fmt( + L10nKey::ForwardPortHere, + &[("port", &r.bind_port.to_string())], + )) + })), + ) + }) + .when(advanced, |form| { + form.child(self.segmented_on( + sf, + "ssh-managed-forward-kind", + &[ + t(L10nKey::ForwardLocal), + t(L10nKey::ForwardRemote), + t(L10nKey::ForwardDynamic), + ], + selected, + cx, + move |this, ix, _window, cx| { + let kind = match ix { + 1 => SshForwardKind::Remote, + 2 => SshForwardKind::Dynamic, + _ => SshForwardKind::Local, + }; + this.set_managed_forward_kind(kind, cx); + }, + )) + .child(pair( + t(L10nKey::ForwardBindLabel), + &self.loopback_panel.mf_bind_host, + &self.loopback_panel.mf_bind_port, + )) + .child( + div() + .opacity(if needs_target { 1.0 } else { NO_TARGET_FADE }) + .child(pair( + if needs_target { + t(L10nKey::ForwardToLabel) + } else { + t(L10nKey::ForwardSocksLabel) + }, + &self.loopback_panel.mf_target_host, + &self.loopback_panel.mf_target_port, + )), + ) + .child(Input::new(&self.loopback_panel.mf_description).xsmall()) + }) .when(incomplete, |form| { form.child(div().text_size(rems(META)).text_color(danger).child( - match needs_target { - true => t(L10nKey::SettingsFwdNeedsBoth), - false => t(L10nKey::SettingsFwdNeedsListen), + match (advanced, needs_target) { + (false, _) => t(L10nKey::ForwardNeedsPort), + (true, true) => t(L10nKey::SettingsFwdNeedsBoth), + (true, false) => t(L10nKey::SettingsFwdNeedsListen), }, )) }) @@ -496,31 +506,54 @@ impl Tty7App { }) .child( h_flex() - .justify_end() + .items_center() + .justify_between() .gap(px(4.)) .pt(px(1.)) + // An edit is always shown whole — the short form cannot + // spell what an existing rule may contain — so there is + // nothing to disclose and no toggle to offer. + .child(div().when(!editing, |slot| { + slot.child( + Button::new(("ssh-managed-forward-advanced", pane_id)) + .label(if advanced { + t(L10nKey::ForwardSimpleToggle) + } else { + t(L10nKey::ForwardAdvancedToggle) + }) + .ghost() + .xsmall() + .on_click(cx.listener(|this, _, _window, cx| { + this.toggle_managed_forward_advanced(cx) + })), + ) + })) .child( - Button::new(("ssh-managed-forward-cancel", pane_id)) - .label(t(L10nKey::Cancel)) - .ghost() - .xsmall() - .on_click(cx.listener(move |this, _, window, cx| { - this.close_managed_forward_form(window, cx) - })), - ) - .child( - Button::new(("ssh-managed-forward-add", pane_id)) - .label(if editing { - t(L10nKey::Save) - } else { - t(L10nKey::ForwardAdd) - }) - .primary() - .xsmall() - .disabled(!complete) - .on_click(cx.listener(move |this, _, window, cx| { - this.add_managed_forward(pane_id, window, cx) - })), + h_flex() + .gap(px(4.)) + .child( + Button::new(("ssh-managed-forward-cancel", pane_id)) + .label(t(L10nKey::Cancel)) + .ghost() + .xsmall() + .on_click(cx.listener(move |this, _, window, cx| { + this.close_managed_forward_form(window, cx) + })), + ) + .child( + Button::new(("ssh-managed-forward-add", pane_id)) + .label(if editing { + t(L10nKey::Save) + } else { + t(L10nKey::ForwardAdd) + }) + .primary() + .xsmall() + .disabled(!complete) + .on_click(cx.listener(move |this, _, window, cx| { + this.add_managed_forward(pane_id, window, cx) + })), + ), ), ) } @@ -532,6 +565,7 @@ mod tests { fn fields(kind: SshForwardKind, bind_port: &str, host: &str, port: &str) -> ForwardFields { ForwardFields { + advanced: true, kind, bind_host: "127.0.0.1".to_string(), bind_port: bind_port.to_string(), @@ -606,6 +640,42 @@ mod tests { ); } + /// The short form: one number, and the rule it makes reaches the far + /// side's own loopback and comes out here under the same number. + #[test] + fn one_number_is_a_whole_rule_in_the_short_form() { + let mut form = fields(SshForwardKind::Local, "", "", "3000"); + form.advanced = false; + // Whatever a trip through the advanced form left behind is not part of + // what the short form was asked for. + form.bind_host = "0.0.0.0".to_string(); + form.description = "left over".to_string(); + let rule = form.collect().expect("a port is enough"); + assert_eq!(rule.kind, SshForwardKind::Local); + assert_eq!(rule.bind_host, "127.0.0.1"); + assert_eq!(rule.bind_port, 3000, "the address has to be predictable"); + assert_eq!(rule.target_host, "localhost"); + assert_eq!(rule.target_port, 3000); + assert_eq!(rule.description, None); + } + + #[test] + fn the_short_form_is_blank_until_the_port_is_typed() { + let mut form = fields(SshForwardKind::Local, "8080", "10.0.0.5", ""); + form.advanced = false; + assert!( + form.is_blank(), + "fields the short form does not show cannot make it dirty" + ); + assert!(form.collect().is_none()); + form.target_port = "http".to_string(); + assert!( + !form.is_blank(), + "a typed port that is not one still counts" + ); + assert!(form.collect().is_none()); + } + #[test] fn a_socks_proxy_needs_nothing_but_a_port_to_listen_on() { let rule = fields(SshForwardKind::Dynamic, "1080", "", "") diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 1fa0fba8..38f98aab 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -450,9 +450,9 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::SettingsDetectUrlsDesc => { "Underline links on hover and open them on {modifier}-click." } - L10nKey::ForwardSshLoopbackLinks => "Forward SSH loopback links", + L10nKey::ForwardSshLoopbackLinks => "Forward remote ports", L10nKey::SettingsForwardSshLoopbackLinksDesc => { - "When a pane is in SSH, open localhost links through a temporary port forward." + "Over SSH, forward the ports a pane starts serving and open its localhost links here." } L10nKey::SettingsOpenFilesInternal => "Built-in editor", L10nKey::SettingsOpenFilesSystem => "Default app", @@ -796,7 +796,7 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::SettingsSearchFontLigaturesKeywords => "typography glyph fira", L10nKey::SettingsSearchFontSizeKeywords => "typography text bigger smaller zoom", L10nKey::SettingsSearchForwardSshLoopbackLinksKeywords => { - "ssh remote port tunnel localhost forward links" + "ssh remote port tunnel localhost forward links ports autoforward detect" } L10nKey::SettingsSearchGrokBuildKeywords => { "agent integration hooks install xai grok build" @@ -954,7 +954,6 @@ pub fn translate_en(key: L10nKey) -> &'static str { "Could not upload the pasted image to {host}: {error}" } L10nKey::LinkFileOpenFailed => "Could not open {path}: {error}", - L10nKey::ForwardPanelTitle => "Forwards", L10nKey::ForwardDisconnected => "Disconnected", L10nKey::ForwardDisconnectedFrom => "Disconnected from {host}", L10nKey::SshEditProfile => "Edit connection…", @@ -967,6 +966,11 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::ForwardToLabel => "to", L10nKey::ForwardSocksLabel => "SOCKS", L10nKey::ForwardAdd => "Add", + L10nKey::ForwardPortLabel => "Remote port", + L10nKey::ForwardPortHere => "opens at localhost:{port}", + L10nKey::ForwardNeedsPort => "A port is a number from 1 to 65535.", + L10nKey::ForwardAdvancedToggle => "Advanced", + L10nKey::ForwardSimpleToggle => "Simple", L10nKey::ForwardRequestFailed => "Could not reach the session — nothing changed.", L10nKey::FileTreePlaceholderFileName => "file name", L10nKey::FileTreePlaceholderFolderName => "folder name", @@ -1057,6 +1061,8 @@ pub fn translate_en(key: L10nKey) -> &'static str { } L10nKey::PanelProcessesSubtitle => "Processes", L10nKey::PanelPortsSubtitle => "Ports", + L10nKey::PanelPortsUnsupported => "That machine's tty7-server is too old to list ports.", + L10nKey::PortAutoForwarded => "Remote :{port} is now http://localhost:{local}", L10nKey::PanelCwd => "cwd", L10nKey::PanelShell => "shell", L10nKey::PanelSsh => "ssh", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 5de5a95e..1cfbd67c 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -461,9 +461,9 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::SettingsDetectUrlsDesc => { "ホバーでリンクに下線を表示し、{modifier}+クリックで開く" } - L10nKey::ForwardSshLoopbackLinks => "SSH ループバックリンクを転送", + L10nKey::ForwardSshLoopbackLinks => "リモートポートを転送", L10nKey::SettingsForwardSshLoopbackLinksDesc => { - "ペインが SSH 接続中の場合、一時的なポートフォワード経由で localhost リンクを開く" + "SSH 接続中、ペインが待ち受けを始めたポートを自動転送し、localhost リンクをこの端末で開く" } L10nKey::SettingsOpenFilesInternal => "内蔵エディタ", L10nKey::SettingsOpenFilesSystem => "デフォルトアプリ", @@ -831,7 +831,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { "タイポグラフィ 文字 拡大 縮小 ズーム font size typography text bigger smaller zoom" } L10nKey::SettingsSearchForwardSshLoopbackLinksKeywords => { - "ssh リモート ポート トンネル localhost フォワード リンク forward ssh loopback links tunnel" + "ssh リモート ポート トンネル localhost フォワード リンク 自動転送 forward ssh loopback links tunnel ports" } L10nKey::SettingsSearchGrokBuildKeywords => { "エージェント 統合 フック インストール xai grok build agent integration hooks install" @@ -1015,7 +1015,6 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { "貼り付けた画像を {host} にアップロードできませんでした: {error}" } L10nKey::LinkFileOpenFailed => "{path} を開けませんでした: {error}", - L10nKey::ForwardPanelTitle => "ポートフォワード", L10nKey::ForwardDisconnected => "切断済み", L10nKey::ForwardDisconnectedFrom => "{host} から切断されました", L10nKey::SshEditProfile => "接続を編集…", @@ -1028,6 +1027,11 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::ForwardToLabel => "転送先", L10nKey::ForwardSocksLabel => "SOCKS", L10nKey::ForwardAdd => "追加", + L10nKey::ForwardPortLabel => "リモートポート", + L10nKey::ForwardPortHere => "localhost:{port} で開きます", + L10nKey::ForwardNeedsPort => "ポートは 1 から 65535 までの数字です。", + L10nKey::ForwardAdvancedToggle => "詳細", + L10nKey::ForwardSimpleToggle => "シンプル", L10nKey::ForwardRequestFailed => "セッションに届きませんでした。何も変更していません", L10nKey::FileTreePlaceholderFileName => "ファイル名", L10nKey::FileTreePlaceholderFolderName => "フォルダ名", @@ -1121,6 +1125,8 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { } L10nKey::PanelProcessesSubtitle => "プロセス", L10nKey::PanelPortsSubtitle => "ポート", + L10nKey::PanelPortsUnsupported => "リモートの tty7-server が古く、ポートを列挙できません。", + L10nKey::PortAutoForwarded => "リモートの :{port} は http://localhost:{local} で開けます", L10nKey::PanelCwd => "作業ディレクトリ", L10nKey::PanelShell => "シェル", L10nKey::PanelSsh => "ssh", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 430ca53c..ce1a4668 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -692,7 +692,6 @@ l10n_keys! { SftpTransferListFailed, SftpImagePasteUploadFailed, LinkFileOpenFailed, - ForwardPanelTitle, ForwardDisconnected, ForwardDisconnectedFrom, SshEditProfile, @@ -705,6 +704,11 @@ l10n_keys! { ForwardToLabel, ForwardSocksLabel, ForwardAdd, + ForwardPortLabel, + ForwardPortHere, + ForwardNeedsPort, + ForwardAdvancedToggle, + ForwardSimpleToggle, ForwardRequestFailed, FileTreePlaceholderFileName, FileTreePlaceholderFolderName, @@ -779,6 +783,8 @@ l10n_keys! { PanelTurnNoScrollback, PanelProcessesSubtitle, PanelPortsSubtitle, + PanelPortsUnsupported, + PortAutoForwarded, PanelCwd, PanelShell, PanelSsh, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 51625979..e0802603 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -396,9 +396,9 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::SettingsLinks => "链接", L10nKey::DetectUrls => "检测 URL", L10nKey::SettingsDetectUrlsDesc => "悬停时给链接加下划线，通过 {modifier}+点击 打开。", - L10nKey::ForwardSshLoopbackLinks => "转发 SSH 回环链接", + L10nKey::ForwardSshLoopbackLinks => "转发远程端口", L10nKey::SettingsForwardSshLoopbackLinksDesc => { - "当窗格处于 SSH 中时，通过临时端口转发打开 localhost 链接。" + "SSH 会话里，自动转发窗格开始监听的端口，并在本机打开它的 localhost 链接。" } L10nKey::SettingsOpenFilesInternal => "内置编辑器", L10nKey::SettingsOpenFilesSystem => "默认应用", @@ -738,7 +738,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { "字号 字体大小 文字 放大 缩小 typography font size bigger smaller zoom" } L10nKey::SettingsSearchForwardSshLoopbackLinksKeywords => { - "SSH回环链接 端口转发 隧道 localhost 转发 forward ssh loopback links tunnel" + "SSH回环链接 端口转发 隧道 localhost 转发 自动转发 端口检测 forward ssh loopback links tunnel ports" } L10nKey::SettingsSearchGrokBuildKeywords => { "Grok Build agent 集成 hook 安装 xai grok build agent integration hooks install" @@ -920,7 +920,6 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::SftpTransferListFailed => "无法获取传输状态：{error}", L10nKey::SftpImagePasteUploadFailed => "无法将粘贴的图片上传到 {host}：{error}", L10nKey::LinkFileOpenFailed => "无法打开 {path}：{error}", - L10nKey::ForwardPanelTitle => "端口转发", L10nKey::ForwardDisconnected => "已断开", L10nKey::ForwardDisconnectedFrom => "与 {host} 的连接已断开", L10nKey::SshEditProfile => "编辑连接…", @@ -933,6 +932,11 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::ForwardToLabel => "到", L10nKey::ForwardSocksLabel => "SOCKS", L10nKey::ForwardAdd => "添加", + L10nKey::ForwardPortLabel => "远程端口", + L10nKey::ForwardPortHere => "在 localhost:{port} 打开", + L10nKey::ForwardNeedsPort => "端口是 1 到 65535 之间的数字。", + L10nKey::ForwardAdvancedToggle => "高级", + L10nKey::ForwardSimpleToggle => "简单", L10nKey::ForwardRequestFailed => "联系不上这个会话——什么都没有改动。", L10nKey::FileTreePlaceholderFileName => "文件名", L10nKey::FileTreePlaceholderFolderName => "文件夹名", @@ -1010,6 +1014,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::PanelTurnNoScrollback => "这一轮画在 alt screen 上，scrollback 里没有留下它。", L10nKey::PanelProcessesSubtitle => "进程", L10nKey::PanelPortsSubtitle => "端口", + L10nKey::PanelPortsUnsupported => "对端的 tty7-server 太旧，列不出端口。", + L10nKey::PortAutoForwarded => "远程 :{port} 现在是 http://localhost:{local}", L10nKey::PanelCwd => "工作目录", L10nKey::PanelShell => "shell", L10nKey::PanelSsh => "ssh", diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index bf77398f..be6d3cfd 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -2,17 +2,18 @@ use gpui::{AnyElement, Context, Window, div, prelude::*, px, rems}; use gpui_component::button::Button; use gpui_component::input::Input; use gpui_component::{ - ActiveTheme as _, Icon, IconName, InteractiveElementExt as _, Sizable as _, h_flex, v_flex, + ActiveTheme as _, Icon, IconName, InteractiveElementExt as _, Sizable as _, WindowExt as _, + h_flex, v_flex, }; use std::path::PathBuf; use crate::core::config::{Config, RightPanelTab}; -use crate::daemon::protocol::PaneProcs; +use crate::daemon::protocol::{ManagedForward, PaneProcs}; use crate::ui::app::{ - CONTENT_INSET, TILE_GLYPH_XS, TILE_SIZE_XS, Tty7App, tile_trailing_inset, - tile_trailing_inset_sm, + CONTENT_INSET, TILE_GLYPH_SM, TILE_GLYPH_XS, TILE_SIZE_SM, TILE_SIZE_XS, Tty7App, + tile_trailing_inset, tile_trailing_inset_sm, }; -use crate::ui::i18n::{L10nKey, t}; +use crate::ui::i18n::{L10nKey, t, t_fmt}; use crate::ui::scrollbar::with_vertical_scrollbar; pub(crate) const MIN_WIDTH: f32 = 216.; @@ -90,6 +91,17 @@ pub(crate) const ROW_GLYPH: f32 = crate::ui::app::TILE_GLYPH; /// 6px under Info is a panel whose rows visibly do not belong to each other. pub(crate) const ROW_INSET: f32 = 4.; +/// Whether this forward is the one that reaches `port` on the far side. +/// +/// Local forwards only, and only those aimed at the far host's own loopback: +/// a forward to some third machine happens to carry the same number, and +/// pairing it with the port row would claim it leads somewhere it does not. +pub(crate) fn forwards_port(m: &ManagedForward, port: u16) -> bool { + m.kind == crate::daemon::protocol::SshForwardKind::Local + && m.target_port == port + && crate::daemon::protocol::PortEntry::reaches_loopback(&m.target_host) +} + /// The strip the row and group action buttons live in, revealed by hovering /// `row`. /// @@ -147,6 +159,31 @@ pub(crate) struct RightPanelState { pub(crate) procs_loading: bool, pub(crate) procs_gen: u64, pub(crate) procs_forwards: Option, + /// Who to ask about `procs_pane` — `None` means this machine's daemon. + pub(crate) procs_host: Option, + /// Whether the host that owns this pane cannot describe its processes at + /// all: an older `tty7-server` on the far end, which does not know the + /// request. Kept apart from an empty list, because "nothing is listening" + /// and "nobody could tell us" are different sentences and the panel has to + /// say which one it means. + pub(crate) procs_unsupported: bool, + /// How `procs_pane`'s loopback ports can be reached from this machine. + /// Read by the Ports list to decide what a click on a port does, and by + /// the watch to decide whether it has to keep looking with the panel shut. + pub(crate) port_route: crate::terminal::view::PortRoute, + /// The remote ports already forwarded unasked, per set of forwards. + /// + /// Kept so that a forward the user then deletes is not immediately rebuilt + /// by the next poll — the automatic offer is made once per port, and after + /// that the port is theirs to forward or not. + /// + /// Keyed by owner rather than held for the pane in front, because a + /// workspace's forwards are shared by all of its panes: switching tabs + /// would otherwise re-announce every port the workspace had already + /// forwarded, and switching back would re-offer what was just dismissed. + /// A few `u16` per connection is not worth reclaiming. + pub(crate) auto_forwarded: + std::collections::HashMap>, pub(crate) scroll: gpui::ScrollHandle, pub(crate) tree_scroll: gpui::ScrollHandle, /// A path the tree should scroll onto, and how many more renders it may @@ -163,8 +200,35 @@ pub(crate) struct RightPanelState { /// a moment. pub(crate) const TREE_REVEAL_RENDERS: u8 = 60; +/// How often the process and port list is re-read while it is on screen — +/// close enough that a process appearing feels immediate. const PROCS_POLL: std::time::Duration = std::time::Duration::from_millis(2000); +/// How often it is re-read with the panel shut, where nobody is watching the +/// list and the only question is whether a new port has appeared. A couple of +/// extra seconds nobody can feel, against a query that crosses the network on +/// every remote pane. +const PORT_WATCH_POLL: std::time::Duration = std::time::Duration::from_millis(5000); + +/// How many newly-seen ports one poll may forward. A dev server brings up one +/// or two; a number this side of a dozen is a process opening listeners in a +/// loop, and forwarding all of them helps nobody. +const AUTO_FORWARD_BURST: usize = 4; + +/// What the Ports and Forwards sections are describing. +#[derive(Clone)] +pub(crate) struct PaneForwardCtx { + pub(crate) pane_id: u64, + /// `Some` when the pane has somewhere to hold managed forwards. + pub(crate) route: Option, + pub(crate) port_route: crate::terminal::view::PortRoute, + /// The host that owns this pane's processes, when that is not this + /// machine. A remote workspace's panes live in the peer's registry, so the + /// local daemon — which is what `query_procs` asks — has never heard of + /// them and answers with an empty list. + pub(crate) host: Option, +} + /// What a session row draws in its value column. /// /// Every row used to be a `(&str, String)` pair rendered identically, and the @@ -645,14 +709,10 @@ impl Tty7App { fn render_panel_info(&mut self, window: &mut Window, cx: &mut Context) -> AnyElement { let title = self.panel_title(t(L10nKey::PanelInfoTitle), None, None, window, cx); let mut rows: Vec = Vec::new(); + // Which pane the sections below describe, and what can be done with + // its ports. Worked out once, by the same call the watch uses. + let ctx = self.pane_forward_ctx(window, cx); let mut pane_id: Option = None; - let mut forwards_pane: Option = None; - // Whether the ports below are this machine's. They are listed by the - // daemon that owns the pane, so what decides it is which machine that - // daemon runs on — not whether the shell has since ssh'd somewhere, - // which would hide the browser tile on a `ssh -L` pane whose forwarded - // listener is on this machine and reachable. - let mut local_pane = false; // Where the `changes` row's counts lead. Same source as the sidebar's, // and gated on the same setting, so turning the preview off turns it // off in both places rather than in one of them. @@ -666,7 +726,6 @@ impl Tty7App { if let Some(leaf) = tab.detail_pane(window, cx) { let view = leaf.read(cx); pane_id = Some(view.pane_id); - local_pane = view.host_id().is_local(); diff_target = crate::ui::tab_sidebar::diff_click_cwd( cx.global::(), view.git_status_cwd() @@ -702,16 +761,6 @@ impl Tty7App { if let Some(ssh) = view.ssh_spec() { rows.push(InfoRow::text(t(L10nKey::PanelSsh), ssh.host.clone()).copyable()); } - let connected_ssh = view - .remote_context() - .is_some_and(|c| c.kind == crate::daemon::protocol::RemoteKind::NativeSsh) - && matches!( - view.ssh_phase(), - Some(crate::daemon::protocol::SshPhase::Connected) - ); - if connected_ssh || view.workspace().is_some() { - forwards_pane = Some(view.pane_id); - } git = view.git_status(cx); detail_pane = Some(leaf); } @@ -751,9 +800,6 @@ impl Tty7App { ); } - let route = forwards_pane.map(|id| self.forward_route(id, cx)); - self.sync_procs(pane_id, route, cx); - let label_w = info_label_column(&rows, window, cx); // Rows pad themselves back out to `CONTENT_INSET`, so their hover fill // bleeds past the text on both sides — the geometry the Source Control @@ -768,8 +814,7 @@ impl Tty7App { .child(list) .children(self.turns_section(detail_pane.as_ref(), cx)) .children(self.procs_section(pane_id, cx)) - .children(self.ports_section(pane_id, local_pane, cx)) - .children(self.forwards_section(forwards_pane, cx)) + .children(self.ports_section(ctx.as_ref(), cx)) .into_any_element(); self.panel_scroll(inner, title) } @@ -1183,34 +1228,59 @@ impl Tty7App { ) } - /// The listening ports of the pane's processes. + /// The ports this pane is serving, and what it takes to reach them. /// - /// `local` is whether the daemon that listed these ports is this machine's, - /// and it is what decides whether the browser tile appears: a port on a - /// remote host is not this machine's port, and opening it here is not a - /// near miss, it is a different service. It is deliberately about the - /// *host* and not about whether the shell has ssh'd somewhere — the ports - /// come from the pane's own process tree either way, so a `ssh -L` pane's - /// forwarded listener really is on this machine and really does open. + /// One list, not two. Ports and forwards used to be separate sections, so + /// a remote :3000 and the forward that reaches :3000 sat under different + /// headings with nothing saying they were the same thing — and the ports + /// half offered no way to build the forward the other half was for. A row + /// is a port here, and the forward, when there is one, is where that row + /// says it comes out. fn ports_section( &self, - pane_id: Option, - local: bool, + ctx: Option<&PaneForwardCtx>, cx: &mut Context, ) -> Option { - let ports = &self.procs(pane_id)?.ports; - if ports.is_empty() { + let ctx = ctx?; + let pane_id = ctx.pane_id; + let ports = self + .procs(Some(pane_id)) + .map(|p| p.ports.clone()) + .unwrap_or_default(); + let forwards: Vec = self + .loopback_panel + .managed + .iter() + .filter(|m| m.pane_id == pane_id) + .cloned() + .collect(); + let form_open = self.loopback_panel.form_pane_id == Some(pane_id); + // A pane that cannot hold a forward and is serving nothing has no + // section: the heading alone would be an empty promise. + if ports.is_empty() && forwards.is_empty() && ctx.route.is_none() { return None; } + let sf = cx.global::().sidebar; let mono = cx.theme().mono_font_family.clone(); + let openable = ctx.port_route != crate::terminal::view::PortRoute::Blocked; let mut list = v_flex().px(px(CONTENT_INSET - ROW_INSET)).py(px(1.)); + // Which forwards a port row has already accounted for; whatever is + // left over gets a row of its own below. + let mut paired: Vec = Vec::new(); + for (i, p) in ports.iter().enumerate() { - // "What is this pane serving, and where" is the question the - // section answers, and the next thing anyone does with the answer - // is go there — so the row hands over an address instead of making - // it something to read off the screen and retype. - let authority = p.authority(); + let forward = forwards.iter().find(|m| forwards_port(m, p.port)); + if let Some(f) = forward { + paired.push(f.id); + } + // What a click and a copy are about: the address that works from + // here. Once a forward exists that is the local end of it, not the + // far side's own spelling of the port. + let here = forward.map(|f| f.bind_port); + let authority = here + .map(|local| format!("127.0.0.1:{local}")) + .unwrap_or_else(|| p.authority()); // Keyed by the row, not by the port: `listening_ports` drops a // duplicate only when the port *and* the pid match, so a // pre-forking server — nginx, gunicorn, a node cluster — puts one @@ -1221,9 +1291,10 @@ impl Tty7App { let id = gpui::SharedString::from(format!("panel-port-{}-{}", p.port, p.pid)); let mut tiles_wide = 1; let mut actions = action_strip(&id, sf.hover); - if local { + if openable { tiles_wide += 1; - let url = format!("http://{authority}"); + let port = p.port; + let direct = p.authority(); actions = actions.child( self.info_tile( ("panel-port-open", i), @@ -1231,7 +1302,9 @@ impl Tty7App { t(L10nKey::PanelOpenInBrowser), cx, ) - .on_click(move |_, _window, cx| cx.open_url(&url)), + .on_click(cx.listener(move |this, _, _window, cx| { + this.open_pane_port(port, direct.clone(), cx) + })), ); } actions = actions.child( @@ -1248,6 +1321,21 @@ impl Tty7App { } }), ); + if let Some(f) = forward { + tiles_wide += 1; + let forward_id = f.id; + actions = actions.child( + self.info_tile( + ("panel-port-unforward", i), + IconName::Close, + t(L10nKey::ForwardTooltipRemove), + cx, + ) + .on_click(cx.listener(move |this, _, _window, cx| { + this.remove_managed_forward(pane_id, forward_id, cx) + })), + ); + } list = list.child( h_flex() .id(id.clone()) @@ -1279,41 +1367,327 @@ impl Tty7App { .text_color(cx.theme().muted_foreground) .child(p.name.clone()), ) + // Where the port comes out on this machine. Just the + // number: the host is always this machine's loopback, and + // spelling it out on every row would bury the one part + // that differs. + .children(here.map(|local| { + div() + .flex_none() + .text_size(rems(META_MONO)) + .font_family(mono.clone()) + .text_color(cx.theme().muted_foreground.opacity(0.8)) + .child(format!("→ :{local}")) + })) .child(actions), ); } + + // Forwards no port row spoke for: the remote and dynamic ones, and any + // local forward pointed somewhere this pane is not itself serving. + for forward in forwards.iter().filter(|m| !paired.contains(&m.id)) { + list = list.child(self.forward_row(forward, &mono, cx)); + } + + let add = ctx.route.is_some().then(|| { + crate::ui::tab_strip::chrome_tile_sized( + Button::new(("ssh-forward-add-toggle", pane_id)) + .icon(Icon::empty().path("icons/plus.svg")), + TILE_SIZE_SM, + TILE_GLYPH_SM, + form_open, + cx, + ) + .rounded_md() + .tooltip(if form_open { + t(L10nKey::Cancel) + } else { + t(L10nKey::ForwardTooltipAdd) + }) + .on_click(cx.listener(move |this, _, window, cx| { + this.toggle_managed_forward_form(pane_id, window, cx) + })) + .into_any_element() + }); + Some( v_flex() - .child(self.panel_subtitle(t(L10nKey::PanelPortsSubtitle), true, None, cx)) + .child(self.panel_subtitle(t(L10nKey::PanelPortsSubtitle), true, add, cx)) + .when( + ports.is_empty() && forwards.is_empty() && !form_open, + |this| { + // "Nothing is listening" and "nobody could tell us" + // look identical on screen unless the panel says which + // one it means — and the second is a fixable thing: + // the far end is running a server too old to answer. + let (text, tone) = match self.right_panel.procs_unsupported { + true => ( + t(L10nKey::PanelPortsUnsupported), + cx.theme().muted_foreground, + ), + false => (t(L10nKey::None), cx.theme().muted_foreground), + }; + this.child( + div() + .px(px(CONTENT_INSET)) + .py(px(2.)) + .text_size(rems(TEXT)) + .text_color(tone) + .child(text), + ) + }, + ) .child(list) + .when(form_open, |this| this.child(self.forward_form(pane_id, cx))) .into_any_element(), ) } + /// Open one of this pane's ports in a browser, building the forward it + /// needs first when it needs one. + /// + /// The forward is the part the user should not have to think about: they + /// asked to see :3000, and where :3000 has to be tunnelled to be seen, + /// that is this function's problem and not theirs. + pub(crate) fn open_pane_port( + &mut self, + port: u16, + direct_authority: String, + cx: &mut Context, + ) { + use crate::terminal::view::PortRoute; + match self.right_panel.port_route { + PortRoute::Blocked => {} + PortRoute::Direct => cx.open_url(&format!("http://{direct_authority}")), + PortRoute::Forward => { + if let Some(f) = self + .loopback_panel + .managed + .iter() + .find(|m| forwards_port(m, port)) + { + cx.open_url(&format!("http://127.0.0.1:{}", f.bind_port)); + return; + } + let Some(route) = self.right_panel.procs_forwards.clone() else { + return; + }; + let owner = route.owner_key(); + cx.spawn(async move |this, cx| { + let built = cx + .background_executor() + .spawn(async move { route.ensure_loopback("127.0.0.1", port) }) + .await; + let _ = this.update_in(cx, |app, window, cx| match built { + Ok(f) => { + // Claimed, so the watch does not offer this port a + // second time after the user has just opened it. + app.right_panel + .auto_forwarded + .entry(owner) + .or_default() + .insert(port); + cx.open_url(&format!("http://127.0.0.1:{}", f.local_port)); + if let Some(pane_id) = app.right_panel.procs_pane { + app.refresh_managed_forwards(pane_id, cx); + } + } + Err(e) => window.push_notification( + t_fmt( + L10nKey::LoopbackForwardFailed, + &[("port", &port.to_string()), ("error", &e.to_string())], + ), + cx, + ), + }); + }) + .detach(); + } + } + } + fn procs(&self, pane_id: Option) -> Option<&PaneProcs> { (pane_id.is_some() && self.right_panel.procs_pane == pane_id) .then_some(self.right_panel.procs.as_ref())? } + /// What the Ports and Forwards sections are describing: which pane, how + /// forward requests about it reach a daemon, and whether the loopback + /// ports it is serving can be opened from this machine. + /// + /// One answer for both readers. The panel draws from it and the watch polls + /// from it, and when they were each working it out for themselves the panel + /// could offer to open a port the watch had already given up on. + pub(crate) fn pane_forward_ctx( + &self, + window: &Window, + cx: &gpui::App, + ) -> Option { + let leaf = self.tabs.get(self.active)?.detail_pane(window, cx)?; + let view = leaf.read(cx); + // A pane holds managed forwards once it has somewhere to hold them: + // a live native-ssh connection, or the workspace's shared one. + let connected_ssh = view + .remote_context() + .is_some_and(|c| c.kind == crate::daemon::protocol::RemoteKind::NativeSsh) + && matches!( + view.ssh_phase(), + Some(crate::daemon::protocol::SshPhase::Connected) + ); + Some(PaneForwardCtx { + pane_id: view.pane_id, + route: (connected_ssh || view.workspace().is_some()).then(|| view.forward_route()), + port_route: view.port_route(cx), + host: (!view.host_id().is_local()) + .then(|| view.host(cx)) + .flatten(), + }) + } + + /// Point the port watch at whatever pane is in front, once a frame. + /// + /// Driven from the app's own render rather than the panel's: the watch has + /// to run with the panel shut, which is exactly when a new port appearing + /// is worth saying something about. + pub(crate) fn sync_port_watch(&mut self, window: &mut Window, cx: &mut Context) { + let ctx = self.pane_forward_ctx(window, cx); + self.right_panel.port_route = ctx + .as_ref() + .map_or(crate::terminal::view::PortRoute::Blocked, |c| c.port_route); + let pane_id = ctx.as_ref().map(|c| c.pane_id); + let host = ctx.as_ref().and_then(|c| c.host.clone()); + let route = ctx.and_then(|c| c.route); + self.sync_procs(pane_id, route, host, cx); + } + + /// Whether the process/port poll should run at all this round. + fn procs_wanted(&self) -> bool { + (self.right_panel_visible && self.right_panel_tab == RightPanelTab::Info) + || self.watching_ports() + } + + /// Whether the poll has to keep going with the panel closed: on a pane + /// whose ports need forwarding, noticing a new listener *is* the feature, + /// and a shut panel is not a reason to stop looking. + fn watching_ports(&self) -> bool { + self.right_panel.port_route == crate::terminal::view::PortRoute::Forward + && self.right_panel.procs_forwards.is_some() + } + + /// Forward the loopback ports this poll saw for the first time, and say so. + /// + /// Once per port, not once per poll: a forward the user then deletes stays + /// deleted. Only ports bound somewhere this machine could reach through the + /// tunnel — a listener pinned to one of the far host's own interfaces is a + /// different service, and guessing at it would build a forward to nothing. + fn auto_forward_ports(&mut self, cx: &mut Context) { + if !self.watching_ports() { + return; + } + let Some(route) = self.right_panel.procs_forwards.clone() else { + return; + }; + // Read out before the ledger is touched: the ports are behind the same + // borrow the `seen` entry needs. + let listening: Vec = match self.right_panel.procs.as_ref() { + Some(procs) => procs + .ports + .iter() + .filter(|p| crate::daemon::protocol::PortEntry::reaches_loopback(&p.addr)) + .map(|p| p.port) + .collect(), + None => return, + }; + let owner = route.owner_key(); + let seen = self.right_panel.auto_forwarded.entry(owner).or_default(); + let mut fresh: Vec = Vec::new(); + for port in listening { + if fresh.len() >= AUTO_FORWARD_BURST { + break; + } + // Two rows may name one port — a pre-forking server puts one per + // worker on screen — and they want one forward between them. + if seen.contains(&port) || fresh.contains(&port) { + continue; + } + fresh.push(port); + } + if fresh.is_empty() { + return; + } + // Claimed before the request goes out, so the next poll — two seconds + // away, and this round trip crosses a network — does not ask again. + seen.extend(fresh.iter().copied()); + cx.spawn(async move |this, cx| { + let built = cx + .background_executor() + .spawn(async move { + fresh + .into_iter() + .map(|port| { + let local = route + .ensure_loopback("127.0.0.1", port) + .map(|f| f.local_port); + (port, local) + }) + .collect::>() + }) + .await; + let _ = this.update_in(cx, |app, window, cx| { + for (port, local) in built { + match local { + Ok(local) => window.push_notification( + t_fmt( + L10nKey::PortAutoForwarded, + &[("port", &port.to_string()), ("local", &local.to_string())], + ), + cx, + ), + Err(e) => { + // Usually a connection that is not up yet. Let the + // next poll try again rather than writing the port + // off for the life of the pane. + log::debug!("could not auto-forward :{port}: {e}"); + if let Some(seen) = app.right_panel.auto_forwarded.get_mut(&owner) { + seen.remove(&port); + } + } + } + } + cx.notify(); + }); + }) + .detach(); + } + fn sync_procs( &mut self, pane_id: Option, forwards: Option, + host: Option, cx: &mut Context, ) { let Some(pane_id) = pane_id else { return }; self.right_panel.procs_forwards = forwards.clone(); + self.right_panel.procs_host = host.clone(); if self.right_panel.procs_pane != Some(pane_id) { self.right_panel.procs_pane = Some(pane_id); self.right_panel.procs = None; self.loopback_panel.managed.clear(); self.right_panel.procs_gen += 1; self.right_panel.procs_loading = false; + self.right_panel.procs_unsupported = false; } - if !self.right_panel.procs_loading { + // Asked before the first query as well as before every later one. This + // used to be reached only from the Info panel's own render, where the + // panel being open was implied; driven from the app's render it is not, + // and starting a round trip per frame for a pane nobody is watching is + // both wasted IPC and, under a test executor, a queue that never + // empties. + if !self.right_panel.procs_loading && self.procs_wanted() { self.right_panel.procs_loading = true; let generation = self.right_panel.procs_gen; - self.spawn_procs_query(pane_id, generation, forwards, cx); + self.spawn_procs_query(pane_id, generation, forwards, host, cx); } } @@ -1322,6 +1696,7 @@ impl Tty7App { pane_id: u64, generation: u64, forwards: Option, + host: Option, cx: &mut Context, ) { cx.spawn(async move |this, cx| { @@ -1329,7 +1704,19 @@ impl Tty7App { let (procs, managed) = cx .background_executor() .spawn(async move { - let procs = crate::terminal::RemoteTerminal::query_procs(pane_id); + // A remote workspace's pane runs on the peer, so the peer + // is the only one that can walk its process tree; the local + // daemon does not have the pane at all and would answer + // with an empty list. `None` back from the host means it + // could not be asked, which is not the same as an empty + // answer — see `Host::pane_procs`. + let procs = match &host { + Some(host) => { + use crate::ui::host_ops::Host as _; + host.pane_procs(pane_id) + } + None => Some(crate::terminal::RemoteTerminal::query_procs(pane_id)), + }; let managed = route.map(|r| r.list()).unwrap_or_default(); (procs, managed) }) @@ -1339,31 +1726,47 @@ impl Tty7App { if app.right_panel.procs_gen != generation { return false; } - app.right_panel.procs = Some(procs); + app.right_panel.procs_unsupported = procs.is_none(); + // A host that could not answer leaves the last list it did + // answer with in place: blanking it on one failed poll + // would make the panel flicker on a link that hiccups. + if let Some(procs) = procs { + app.right_panel.procs = Some(procs); + } if forwards.is_some() { app.loopback_panel.managed = managed; } cx.notify(); - let wanted = - app.right_panel_visible && app.right_panel_tab == RightPanelTab::Info; + let wanted = app.procs_wanted(); if !wanted { app.right_panel.procs_loading = false; } wanted }) .unwrap_or(false); + // After the list has landed, so the ports it forwards are the ones + // this poll actually saw. + let _ = this.update(cx, |app, cx| app.auto_forward_ports(cx)); if !keep_polling { return; } - cx.background_executor().timer(PROCS_POLL).await; + let gap = this + .read_with(cx, |app, _| { + match app.right_panel_visible && app.right_panel_tab == RightPanelTab::Info { + true => PROCS_POLL, + false => PORT_WATCH_POLL, + } + }) + .unwrap_or(PORT_WATCH_POLL); + cx.background_executor().timer(gap).await; let _ = this.update(cx, |app, cx| { if app.right_panel.procs_gen != generation { return; } - let wanted = app.right_panel_visible && app.right_panel_tab == RightPanelTab::Info; - if wanted { + if app.procs_wanted() { let forwards = app.right_panel.procs_forwards.clone(); - app.spawn_procs_query(pane_id, generation, forwards, cx); + let host = app.right_panel.procs_host.clone(); + app.spawn_procs_query(pane_id, generation, forwards, host, cx); } else { app.right_panel.procs_loading = false; } @@ -1501,7 +1904,50 @@ fn turn_is_jumpable(row: Option, alt_now: bool) -> bool { #[cfg(test)] mod tests { - use super::{InfoRow, InfoValue, turn_is_jumpable}; + use super::{InfoRow, InfoValue, forwards_port, turn_is_jumpable}; + use crate::daemon::protocol::{ForwardStatus, ManagedForward, SshForwardKind}; + + fn forward(kind: SshForwardKind, target_host: &str, target_port: u16) -> ManagedForward { + ManagedForward { + id: 1, + pane_id: 7, + kind, + bind_host: "127.0.0.1".to_string(), + bind_port: 51000, + target_host: target_host.to_string(), + target_port, + description: None, + status: ForwardStatus::Listening, + } + } + + /// A port row and the forward that reaches it are one line, so this is + /// what decides whether a forward is *that* row's or a line of its own. + #[test] + fn a_port_row_claims_only_the_forward_that_reaches_it() { + assert!(forwards_port( + &forward(SshForwardKind::Local, "localhost", 3000), + 3000 + )); + assert!( + forwards_port(&forward(SshForwardKind::Local, "127.0.0.1", 3000), 3000), + "the far side's loopback spells itself several ways" + ); + assert!( + !forwards_port(&forward(SshForwardKind::Local, "localhost", 3000), 8080), + "a different port is a different row" + ); + assert!( + !forwards_port(&forward(SshForwardKind::Local, "10.0.0.5", 3000), 3000), + "same number, another machine — pairing them would claim it leads \ + somewhere it does not" + ); + assert!( + !forwards_port(&forward(SshForwardKind::Remote, "localhost", 3000), 3000), + "a remote forward listens on the far side, so it is not how this \ + port is reached from here" + ); + } fn diff(added: u32, removed: u32, open: bool) -> InfoRow { InfoRow {