fix(cli): add --no-keep for ephemeral sandbox create cleanup (#258)

Author: pimlock

.agents/skills/openshell-cli/SKILL.md

@@ -147,8 +147,8 @@ Key flags:
 - `--provider`: Attach one or more providers (repeatable)
 - `--policy`: Custom policy YAML (otherwise uses built-in default or `OPENSHELL_SANDBOX_POLICY` env var)
 - `--upload <PATH>[:<DEST>]`: Upload local files into the sandbox (default dest: `/sandbox`)
-- `--keep`: Keep sandbox alive after the command exits (useful for non-interactive commands)
-- `--forward <PORT>`: Forward a local port (implies `--keep`)
+- `--no-keep`: Delete the sandbox after the initial command or shell exits
+- `--forward <PORT>`: Forward a local port and keep the sandbox alive
 
 ### List and inspect sandboxes
 
@@ -236,10 +236,10 @@ Create sandbox with initial policy
 ### Step 1: Create sandbox with initial policy
 
 ```bash
-openshell sandbox create --name dev --policy ./initial-policy.yaml --keep -- claude
+openshell sandbox create --name dev --policy ./initial-policy.yaml -- claude
 ```
 
-Use `--keep` so the sandbox stays alive for iteration. The user can work in the sandbox via a separate shell.
+Sandboxes stay alive by default for iteration. Add `--no-keep` only when the sandbox should be deleted automatically after the initial session.
 
 ### Step 2: Monitor logs for denied actions
 
@@ -320,7 +320,7 @@ Build a custom container image and run it as a sandbox.
 ### Step 1: Create a sandbox from a Dockerfile
 
 ```bash
-openshell sandbox create --from ./Dockerfile --keep --name my-app
+openshell sandbox create --from ./Dockerfile --name my-app
 ```
 
 The `--from` flag accepts a Dockerfile path, a directory containing a Dockerfile, a full image reference (e.g. `myregistry.com/img:tag`), or a community sandbox name (e.g. `openclaw`).
@@ -359,13 +359,13 @@ To update the container:
 
 ```bash
 openshell sandbox delete my-app
-openshell sandbox create --from ./Dockerfile --keep --name my-app --forward 8080
+openshell sandbox create --from ./Dockerfile --name my-app --forward 8080
 ```
 
 ### Shortcut: Create with port forward in one command
 
 ```bash
-openshell sandbox create --from ./Dockerfile --forward 8080 --keep -- ./start-server.sh
+openshell sandbox create --from ./Dockerfile --forward 8080 -- ./start-server.sh
 ```
 
 The `--forward` flag starts a background port forward before the command runs, so the service is reachable immediately.
@@ -389,7 +389,7 @@ openshell sandbox create \
   --provider github \
   --provider claude \
   --policy ./dev-policy.yaml \
-  --keep
+  # sandbox create keeps the sandbox alive by default
 ```
 
 ### Step 2: User connects in a separate shell
@@ -540,13 +540,13 @@ $ openshell sandbox upload --help
 | List/switch gateways | `openshell gateway select [name]` |
 | Create sandbox (interactive) | `openshell sandbox create` |
 | Create sandbox with tool | `openshell sandbox create -- claude` |
-| Create with custom policy | `openshell sandbox create --policy ./p.yaml --keep` |
+| Create with custom policy | `openshell sandbox create --policy ./p.yaml` |
 | Connect to sandbox | `openshell sandbox connect <name>` |
 | Stream live logs | `openshell logs <name> --tail` |
 | Pull current policy | `openshell policy get <name> --full > p.yaml` |
 | Push updated policy | `openshell policy set <name> --policy p.yaml --wait` |
 | Policy revision history | `openshell policy list <name>` |
-| Create sandbox from Dockerfile | `openshell sandbox create --from ./Dockerfile --keep` |
+| Create sandbox from Dockerfile | `openshell sandbox create --from ./Dockerfile` |
 | Forward a port | `openshell forward start <port> <name> -d` |
 | Upload files to sandbox | `openshell sandbox upload <name> <path>` |
 | Download files from sandbox | `openshell sandbox download <name> <path>` |

.agents/skills/openshell-cli/cli-reference.md

@@ -165,10 +165,10 @@ Create a sandbox, wait for readiness, then connect or execute the trailing comma
 | `--name <NAME>` | Sandbox name (auto-generated if omitted) |
 | `--from <SOURCE>` | Sandbox source: community name, Dockerfile path, directory, or image reference (BYOC) |
 | `--upload <PATH>[:<DEST>]` | Upload local files into sandbox (default dest: `/sandbox`) |
-| `--keep` | Keep sandbox alive after non-interactive commands finish |
+| `--no-keep` | Delete sandbox after the initial command or shell exits |
 | `--provider <NAME>` | Provider to attach (repeatable) |
 | `--policy <PATH>` | Path to custom policy YAML |
-| `--forward <PORT>` | Forward local port to sandbox (implies `--keep`) |
+| `--forward <PORT>` | Forward local port to sandbox (keeps the sandbox alive) |
 | `--remote <USER@HOST>` | SSH destination for auto-bootstrap |
 | `--ssh-key <PATH>` | SSH private key for auto-bootstrap |
 | `--tty` | Force pseudo-terminal allocation |

Cargo.lock

@@ -153,6 +153,8 @@ The `sandbox exec` path is identical to interactive connect except:
 - The command string is passed as the final SSH argument
 - The sandbox daemon routes it through `exec_request()` instead of `shell_request()`, spawning `/bin/bash -lc <command>`
 
+When `openshell sandbox create` launches a `--no-keep` command or shell, it keeps the CLI process alive instead of `exec()`-ing into SSH so it can delete the sandbox after SSH exits. The default create flow, along with `--forward`, keeps the sandbox running.
+
 ### Port Forwarding (`forward start`)
 
 `openshell forward start <port> <name>` opens a local SSH tunnel so connections to `127.0.0.1:<port>`

architecture/sandbox-connect.md

@@ -62,6 +62,7 @@ tokio-tungstenite = { workspace = true }
 
 # Streams
 futures = { workspace = true }
+nix = { workspace = true }
 
 # URL parsing
 url = { workspace = true }

crates/navigator-cli/Cargo.toml

@@ -1038,13 +1038,17 @@ enum SandboxCommands {
         #[arg(long, requires = "upload", help_heading = "UPLOAD FLAGS")]
         no_git_ignore: bool,
 
-        /// Keep the sandbox alive after non-interactive commands.
-        #[arg(long)]
+        /// Deprecated compatibility flag. Sandboxes are kept by default.
+        #[arg(long, hide = true, conflicts_with = "no_keep")]
         keep: bool,
 
+        /// Delete the sandbox after the initial command or shell exits.
+        #[arg(long, conflicts_with_all = ["keep", "editor", "forward"])]
+        no_keep: bool,
+
         /// Launch a remote editor after the sandbox is ready.
-        /// Implies `--keep` and installs OpenShell-managed SSH config.
-        #[arg(long, value_enum)]
+        /// Keeps the sandbox alive and installs OpenShell-managed SSH config.
+        #[arg(long, value_enum, conflicts_with = "no_keep")]
         editor: Option<CliEditor>,
 
         /// SSH destination for remote bootstrap (e.g., user@hostname).
@@ -1066,9 +1070,9 @@ enum SandboxCommands {
         #[arg(long, value_hint = ValueHint::FilePath)]
         policy: Option<String>,
 
-        /// Forward a local port to the sandbox after the command finishes.
-        /// Implies --keep for non-interactive commands.
-        #[arg(long)]
+        /// Forward a local port to the sandbox before the initial command or shell starts.
+        /// Keeps the sandbox alive.
+        #[arg(long, conflicts_with = "no_keep")]
         forward: Option<u16>,
 
         /// Allocate a pseudo-terminal for the remote command.
@@ -1785,6 +1789,7 @@ async fn main() -> Result<()> {
                     upload,
                     no_git_ignore,
                     keep,
+                    no_keep,
                     editor,
                     remote,
                     ssh_key,
@@ -1834,7 +1839,7 @@ async fn main() -> Result<()> {
                     });
 
                     let editor = editor.map(Into::into);
-                    let keep = keep || editor.is_some();
+                    let keep = keep || !no_keep || editor.is_some() || forward.is_some();
 
                     // For `sandbox create`, a missing cluster is not fatal — the
                     // bootstrap flow inside `sandbox_create` can deploy one.

crates/navigator-cli/src/main.rs

@@ -1743,6 +1743,32 @@ pub async fn sandbox_create_with_bootstrap(
     .await
 }
 
+fn sandbox_should_persist(keep: bool, forward: Option<u16>) -> bool {
+    keep || forward.is_some()
+}
+
+async fn finalize_sandbox_create_session(
+    server: &str,
+    sandbox_name: &str,
+    persist: bool,
+    session_result: Result<()>,
+    tls: &TlsOptions,
+) -> Result<()> {
+    if persist {
+        return session_result;
+    }
+
+    let names = [sandbox_name.to_string()];
+    if let Err(err) = sandbox_delete(server, &names, false, tls).await {
+        if session_result.is_ok() {
+            return Err(err);
+        }
+        eprintln!("Failed to delete sandbox {sandbox_name}: {err}");
+    }
+
+    session_result
+}
+
 /// Create a sandbox with default settings.
 #[allow(clippy::too_many_arguments)]
 pub async fn sandbox_create(
@@ -1863,10 +1889,12 @@ 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 sandbox_name = sandbox.name.clone();
 
-    // Record this sandbox as the last-used for the active gateway.
-    if let Some(gateway) = effective_tls.gateway_name() {
+    // Record this sandbox as the last-used for the active gateway only when it
+    // is expected to persist beyond the initial session.
+    if persist && let Some(gateway) = effective_tls.gateway_name() {
         let _ = save_last_sandbox(gateway, &sandbox_name);
     }
 
@@ -2171,45 +2199,60 @@ pub async fn sandbox_create(
             }
 
             if command.is_empty() {
-                return sandbox_connect(&effective_server, &sandbox_name, &effective_tls).await;
+                let connect_result = if persist {
+                    sandbox_connect(&effective_server, &sandbox_name, &effective_tls).await
+                } else {
+                    crate::ssh::sandbox_connect_without_exec(
+                        &effective_server,
+                        &sandbox_name,
+                        &effective_tls,
+                    )
+                    .await
+                };
+
+                return finalize_sandbox_create_session(
+                    &effective_server,
+                    &sandbox_name,
+                    persist,
+                    connect_result,
+                    &effective_tls,
+                )
+                .await;
             }
 
             // Resolve TTY mode: explicit --tty / --no-tty wins, otherwise
             // auto-detect from the local terminal.
             let tty = tty_override.unwrap_or_else(|| {
                 std::io::stdin().is_terminal() && std::io::stdout().is_terminal()
             });
-            let exec_result = sandbox_exec(
-                &effective_server,
-                &sandbox_name,
-                command,
-                tty,
-                &effective_tls,
-            )
-            .await;
-
-            if forward.is_some() {
-                exec_result?;
-                return Ok(());
-            }
-
-            if !interactive
-                && !keep
-                && let Err(err) = sandbox_delete(
+            let exec_result = if persist {
+                sandbox_exec(
                     &effective_server,
-                    std::slice::from_ref(&sandbox_name),
-                    false,
+                    &sandbox_name,
+                    command,
+                    tty,
                     &effective_tls,
                 )
                 .await
-            {
-                if exec_result.is_ok() {
-                    return Err(err);
-                }
-                eprintln!("Failed to delete sandbox {sandbox_name}: {err}");
-            }
+            } else {
+                crate::ssh::sandbox_exec_without_exec(
+                    &effective_server,
+                    &sandbox_name,
+                    command,
+                    tty,
+                    &effective_tls,
+                )
+                .await
+            };
 
-            exec_result
+            finalize_sandbox_create_session(
+                &effective_server,
+                &sandbox_name,
+                persist,
+                exec_result,
+                &effective_tls,
+            )
+            .await
         }
         SandboxPhase::Error => {
             if last_error_reason.is_empty() {
@@ -4158,7 +4201,7 @@ mod tests {
         GatewayControlTarget, TlsOptions, format_gateway_select_header,
         format_gateway_select_items, gateway_auth_label, gateway_select_with, gateway_type_label,
         git_sync_files, http_health_check, inferred_provider_type, parse_credential_pairs,
-        resolve_gateway_control_target_from,
+        resolve_gateway_control_target_from, sandbox_should_persist,
     };
     use crate::TEST_ENV_LOCK;
     use hyper::StatusCode;
@@ -4312,6 +4355,21 @@ mod tests {
         assert_eq!(result, Some("claude".to_string()));
     }
 
+    #[test]
+    fn sandbox_should_persist_defaults_to_persistent() {
+        assert!(sandbox_should_persist(true, None));
+    }
+
+    #[test]
+    fn sandbox_should_not_persist_when_no_keep_is_set() {
+        assert!(!sandbox_should_persist(false, None));
+    }
+
+    #[test]
+    fn sandbox_should_persist_when_forward_is_requested() {
+        assert!(sandbox_should_persist(false, Some(8080)));
+    }
+
     fn init_git_repo(path: &Path) {
         let status = Command::new("git")
             .args(["init"])