fix(cli): check port availability before starting SSH forward (#309)

Author: drew

crates/openshell-cli/src/main.rs

@@ -1093,9 +1093,9 @@ enum SandboxCommands {
         policy: Option<String>,
 
         /// Forward a local port to the sandbox before the initial command or shell starts.
-        /// Keeps the sandbox alive.
+        /// Accepts [bind_address:]port (e.g. 8080, 0.0.0.0:8080). Keeps the sandbox alive.
         #[arg(long, conflicts_with = "no_keep")]
-        forward: Option<u16>,
+        forward: Option<String>,
 
         /// Allocate a pseudo-terminal for the remote command.
         /// Defaults to auto-detection (on when stdin and stdout are terminals).
@@ -1359,8 +1359,8 @@ enum ForwardCommands {
     /// Start forwarding a local port to a sandbox.
     #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
     Start {
-        /// Port to forward (used as both local and remote port).
-        port: u16,
+        /// Port to forward: [bind_address:]port (e.g. 8080, 0.0.0.0:8080).
+        port: String,
 
         /// Sandbox name (defaults to last-used sandbox).
         #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))]
@@ -1377,7 +1377,7 @@ enum ForwardCommands {
         /// Port that was forwarded.
         port: u16,
 
-        /// Sandbox name (defaults to last-used sandbox).
+        /// Sandbox name (auto-detected from active forwards if omitted).
         #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))]
         name: Option<String>,
     },
@@ -1575,8 +1575,19 @@ async fn main() -> Result<()> {
             command: Some(fwd_cmd),
         }) => match fwd_cmd {
             ForwardCommands::Stop { port, name } => {
-                let gateway_name = resolve_gateway_name(&cli.gateway).unwrap_or_default();
-                let name = resolve_sandbox_name(name, &gateway_name)?;
+                let name = match name {
+                    Some(n) => n,
+                    None => match run::find_forward_by_port(port)? {
+                        Some(n) => {
+                            eprintln!("→ Found forward on sandbox '{n}'");
+                            n
+                        }
+                        None => {
+                            eprintln!("{} No active forward found for port {port}", "!".yellow(),);
+                            return Ok(());
+                        }
+                    },
+                };
                 if run::stop_forward(&name, port)? {
                     eprintln!(
                         "{} Stopped forward of port {port} for sandbox {name}",
@@ -1600,12 +1611,20 @@ async fn main() -> Result<()> {
                         .max()
                         .unwrap_or(7)
                         .max(7);
+                    let bind_width = forwards
+                        .iter()
+                        .map(|f| f.bind_addr.len())
+                        .max()
+                        .unwrap_or(4)
+                        .max(4);
                     println!(
-                        "{:<width$} {:<8} {:<10} STATUS",
+                        "{:<nw$} {:<bw$} {:<8} {:<10} STATUS",
                         "SANDBOX",
+                        "BIND",
                         "PORT",
                         "PID",
-                        width = name_width,
+                        nw = name_width,
+                        bw = bind_width,
                     );
                     for f in &forwards {
                         let status = if f.alive {
@@ -1614,12 +1633,14 @@ async fn main() -> Result<()> {
                             "dead".red().to_string()
                         };
                         println!(
-                            "{:<width$} {:<8} {:<10} {}",
+                            "{:<nw$} {:<bw$} {:<8} {:<10} {}",
                             f.sandbox,
+                            f.bind_addr,
                             f.port,
                             f.pid,
                             status,
-                            width = name_width,
+                            nw = name_width,
+                            bw = bind_width,
                         );
                     }
                 }
@@ -1629,18 +1650,20 @@ async fn main() -> Result<()> {
                 name,
                 background,
             } => {
+                let spec = openshell_core::forward::ForwardSpec::parse(&port)?;
                 let ctx = resolve_gateway(&cli.gateway, &cli.gateway_endpoint)?;
                 let mut tls = tls.with_gateway_name(&ctx.name);
                 apply_edge_auth(&mut tls, &ctx.name);
                 let name = resolve_sandbox_name(name, &ctx.name)?;
-                run::sandbox_forward(&ctx.endpoint, &name, port, background, &tls).await?;
+                run::sandbox_forward(&ctx.endpoint, &name, &spec, background, &tls).await?;
                 if background {
                     eprintln!(
-                        "{} Forwarding port {port} to sandbox {name} in the background",
+                        "{} Forwarding port {} to sandbox {name} in the background",
                         "✓".green().bold(),
+                        spec.port,
                     );
-                    eprintln!("  Access at: http://127.0.0.1:{port}/");
-                    eprintln!("  Stop with: openshell forward stop {port} {name}");
+                    eprintln!("  Access at: {}", spec.access_url());
+                    eprintln!("  Stop with: openshell forward stop {} {name}", spec.port);
                 }
             }
         },
@@ -1864,6 +1887,9 @@ async fn main() -> Result<()> {
                     });
 
                     let editor = editor.map(Into::into);
+                    let forward = forward
+                        .map(|s| openshell_core::forward::ForwardSpec::parse(&s))
+                        .transpose()?;
                     let keep = keep || !no_keep || editor.is_some() || forward.is_some();
 
                     // For `sandbox create`, a missing cluster is not fatal — the

crates/openshell-cli/src/run.rs

@@ -48,7 +48,9 @@ pub use crate::ssh::{
     sandbox_connect, sandbox_connect_editor, sandbox_exec, sandbox_forward, sandbox_ssh_proxy,
     sandbox_ssh_proxy_by_name, sandbox_sync_down, sandbox_sync_up, sandbox_sync_up_files,
 };
-pub use openshell_core::forward::{list_forwards, stop_forward, stop_forwards_for_sandbox};
+pub use openshell_core::forward::{
+    find_forward_by_port, list_forwards, stop_forward, stop_forwards_for_sandbox,
+};
 
 /// Convert a sandbox phase integer to a human-readable string.
 fn phase_name(phase: i32) -> &'static str {
@@ -1726,7 +1728,7 @@ pub async fn sandbox_create_with_bootstrap(
     ssh_key: Option<&str>,
     providers: &[String],
     policy: Option<&str>,
-    forward: Option<u16>,
+    forward: Option<openshell_core::forward::ForwardSpec>,
     command: &[String],
     tty_override: Option<bool>,
     bootstrap_override: Option<bool>,
@@ -1767,7 +1769,10 @@ pub async fn sandbox_create_with_bootstrap(
     .await
 }
 
-fn sandbox_should_persist(keep: bool, forward: Option<u16>) -> bool {
+fn sandbox_should_persist(
+    keep: bool,
+    forward: Option<&openshell_core::forward::ForwardSpec>,
+) -> bool {
     keep || forward.is_some()
 }
 
@@ -1808,7 +1813,7 @@ pub async fn sandbox_create(
     ssh_key: Option<&str>,
     providers: &[String],
     policy: Option<&str>,
-    forward: Option<u16>,
+    forward: Option<openshell_core::forward::ForwardSpec>,
     command: &[String],
     tty_override: Option<bool>,
     bootstrap_override: Option<bool>,
@@ -1821,6 +1826,12 @@ pub async fn sandbox_create(
         ));
     }
 
+    // Check port availability *before* creating the sandbox so we don't
+    // leave an orphaned sandbox behind when the forward would fail.
+    if let Some(ref spec) = forward {
+        openshell_core::forward::check_port_available(spec)?;
+    }
+
     // Try connecting to the gateway. If the connection fails due to a
     // connectivity error and bootstrap is allowed, start a new gateway.
     //
@@ -1918,7 +1929,7 @@ pub async fn sandbox_create(
         .ok_or_else(|| miette::miette!("sandbox missing from response"))?;
 
     let interactive = std::io::stdout().is_terminal();
-    let persist = sandbox_should_persist(keep, forward);
+    let persist = sandbox_should_persist(keep, forward.as_ref());
     let sandbox_name = sandbox.name.clone();
 
     // Record this sandbox as the last-used for the active gateway only when it
@@ -2195,21 +2206,25 @@ pub async fn sandbox_create(
             // If --forward was requested, start the background port forward
             // *before* running the command so that long-running processes
             // (e.g. `openclaw gateway`) are reachable immediately.
-            if let Some(port) = forward {
+            if let Some(ref spec) = forward {
                 sandbox_forward(
                     &effective_server,
                     &sandbox_name,
-                    port,
+                    spec,
                     true, // background
                     &effective_tls,
                 )
                 .await?;
                 eprintln!(
-                    "  {} Forwarding port {port} to sandbox {sandbox_name} in the background\n",
+                    "  {} Forwarding port {} to sandbox {sandbox_name} in the background\n",
                     "\u{2713}".green().bold(),
+                    spec.port,
+                );
+                eprintln!("  Access at: {}", spec.access_url());
+                eprintln!(
+                    "  Stop with: openshell forward stop {} {sandbox_name}",
+                    spec.port,
                 );
-                eprintln!("  Access at: http://127.0.0.1:{port}/");
-                eprintln!("  Stop with: openshell forward stop {port} {sandbox_name}",);
             }
 
             if let Some(editor) = editor {
@@ -4420,7 +4435,8 @@ mod tests {
 
     #[test]
     fn sandbox_should_persist_when_forward_is_requested() {
-        assert!(sandbox_should_persist(false, Some(8080)));
+        let spec = openshell_core::forward::ForwardSpec::new(8080);
+        assert!(sandbox_should_persist(false, Some(&spec)));
     }
 
     #[test]

crates/openshell-cli/src/ssh.rs

@@ -307,10 +307,12 @@ pub async fn sandbox_connect_editor(
 pub async fn sandbox_forward(
     server: &str,
     name: &str,
-    port: u16,
+    spec: &openshell_core::forward::ForwardSpec,
     background: bool,
     tls: &TlsOptions,
 ) -> Result<()> {
+    openshell_core::forward::check_port_available(spec)?;
+
     let session = ssh_session_config(server, name, tls).await?;
 
     let mut command = TokioCommand::from(ssh_base_command(&session.proxy_command));
@@ -319,7 +321,7 @@ pub async fn sandbox_forward(
         .arg("-o")
         .arg("ExitOnForwardFailure=yes")
         .arg("-L")
-        .arg(format!("{port}:127.0.0.1:{port}"));
+        .arg(spec.ssh_forward_arg());
 
     if background {
         // SSH -f: fork to background after authentication.
@@ -332,14 +334,16 @@ pub async fn sandbox_forward(
         .stdout(Stdio::inherit())
         .stderr(Stdio::inherit());
 
+    let port = spec.port;
+
     let status = if background {
         command.status().await.into_diagnostic()?
     } else {
         let mut child = command.spawn().into_diagnostic()?;
         match tokio::time::timeout(FOREGROUND_FORWARD_STARTUP_GRACE_PERIOD, child.wait()).await {
             Ok(status) => status.into_diagnostic()?,
             Err(_) => {
-                eprintln!("{}", foreground_forward_started_message(name, port));
+                eprintln!("{}", foreground_forward_started_message(name, spec));
                 child.wait().await.into_diagnostic()?
             }
         }
@@ -352,7 +356,7 @@ pub async fn sandbox_forward(
     if background {
         // SSH has forked — find its PID and record it.
         if let Some(pid) = find_ssh_forward_pid(&session.sandbox_id, port) {
-            write_forward_pid(name, port, pid, &session.sandbox_id)?;
+            write_forward_pid(name, port, pid, &session.sandbox_id, &spec.bind_addr)?;
         } else {
             eprintln!(
                 "{} Could not discover backgrounded SSH process; \
@@ -365,10 +369,15 @@ pub async fn sandbox_forward(
     Ok(())
 }
 
-fn foreground_forward_started_message(name: &str, port: u16) -> String {
+fn foreground_forward_started_message(
+    name: &str,
+    spec: &openshell_core::forward::ForwardSpec,
+) -> String {
     format!(
-        "{} Forwarding port {port} to sandbox {name}\n  Access at: http://127.0.0.1:{port}/\n  Press Ctrl+C to stop\n  {}",
+        "{} Forwarding port {} to sandbox {name}\n  Access at: {}\n  Press Ctrl+C to stop\n  {}",
         "✓".green().bold(),
+        spec.port,
+        spec.access_url(),
         "Hint: pass --background to start forwarding without blocking your terminal".dimmed(),
     )
 }
@@ -1130,7 +1139,8 @@ mod tests {
 
     #[test]
     fn foreground_forward_started_message_includes_port_and_stop_hint() {
-        let message = foreground_forward_started_message("demo", 8080);
+        let spec = openshell_core::forward::ForwardSpec::new(8080);
+        let message = foreground_forward_started_message("demo", &spec);
         assert!(message.contains("Forwarding port 8080 to sandbox demo"));
         assert!(message.contains("Access at: http://127.0.0.1:8080/"));
         assert!(message.contains("sandbox demo"));
@@ -1139,4 +1149,12 @@ mod tests {
             "Hint: pass --background to start forwarding without blocking your terminal"
         ));
     }
+
+    #[test]
+    fn foreground_forward_started_message_custom_bind_addr() {
+        let spec = openshell_core::forward::ForwardSpec::parse("0.0.0.0:3000").unwrap();
+        let message = foreground_forward_started_message("demo", &spec);
+        assert!(message.contains("Forwarding port 3000 to sandbox demo"));
+        assert!(message.contains("Access at: http://localhost:3000/"));
+    }
 }

crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs

@@ -704,7 +704,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
         None,
         &[],
         None,
-        Some(8080),
+        Some(openshell_core::forward::ForwardSpec::new(8080)),
         &["echo".to_string(), "OK".to_string()],
         Some(false),
         Some(false),