From e48a72414b593aa3b112697905432a9477725968 Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Wed, 16 Sep 2026 14:22:08 +0200 Subject: [PATCH] feat(cli): add --prune option to sandbox delete command Add a --prune flag to `openshell sandbox delete` that deletes only inactive sandboxes. This allows users to clean up terminated, or errored sandboxes while preserving active and provisioning ones. The prune filter targets sandboxes in these phases: - Error - Completed Sandboxes in other phases (e.g. Ready, Provisioning) are preserved. The flag conflicts with both --all and named sandbox arguments, ensuring clear deletion intent. Fixes: https://github.com/NVIDIA/OpenShell/issues/2594 Signed-off-by: Michael Engel --- crates/openshell-cli/src/main.rs | 11 +- crates/openshell-cli/src/run.rs | 16 ++- .../sandbox_create_lifecycle_integration.rs | 108 +++++++++++++++++- deploy/man/openshell.1.md | 4 +- docs/how-it-works/sandboxes/overview.mdx | 9 ++ skills/openshell-cli/SKILL.md | 1 + 6 files changed, 140 insertions(+), 9 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 0375b98f24..048bdd6f81 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1665,12 +1665,16 @@ enum SandboxCommands { #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Delete { /// Sandbox names. - #[arg(required_unless_present = "all", num_args = 1.., value_name = "NAME", add = ArgValueCompleter::new(completers::complete_sandbox_names))] + #[arg(required_unless_present = "all", required_unless_present = "prune", num_args = 1.., value_name = "NAME", add = ArgValueCompleter::new(completers::complete_sandbox_names))] names: Vec, /// Delete all sandboxes. - #[arg(long, conflicts_with = "names")] + #[arg(long, conflicts_with_all = ["names", "prune"])] all: bool, + + /// Delete all inactive sandboxes. + #[arg(long, conflicts_with_all = ["names", "all"])] + prune: bool, }, /// Stop a sandbox while preserving its workspace. @@ -3555,11 +3559,12 @@ async fn run_async() -> Result<()> { ) .await?; } - SandboxCommands::Delete { names, all } => { + SandboxCommands::Delete { names, all, prune } => { run::sandbox_delete( endpoint, &names, all, + prune, &cli.workspace, &tls, &ctx.name, diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 49f6f05d81..b9a241330e 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -412,7 +412,7 @@ async fn finalize_sandbox_create_session( } let names = [sandbox_name.to_string()]; - if let Err(err) = sandbox_delete(server, &names, false, workspace, tls, gateway).await { + if let Err(err) = sandbox_delete(server, &names, false, false, workspace, tls, gateway).await { if let Ok(exit_code) = session_result.as_ref() { return Err(miette::miette!( "sandbox command exited with status {exit_code}, but ephemeral cleanup failed: {err}" @@ -3883,13 +3883,14 @@ pub async fn sandbox_delete( server: &str, names: &[String], all: bool, + prune: bool, workspace: &str, tls: &TlsOptions, gateway: &str, ) -> Result<()> { let mut client = grpc_client(server, tls).await?; - let names_to_delete: Vec = if all { + let names_to_delete: Vec = if all || prune { let mut page_token = String::new(); let mut sandboxes = Vec::new(); loop { @@ -3903,7 +3904,16 @@ pub async fn sandbox_delete( .await .into_diagnostic()? .into_inner(); - sandboxes.extend(response.sandboxes); + + if prune { + sandboxes.extend(response.sandboxes.into_iter().filter(|s| { + let phase = SandboxPhase::try_from(s.phase()).unwrap_or(SandboxPhase::Unknown); + [SandboxPhase::Error, SandboxPhase::Completed].contains(&phase) + })); + } else { + sandboxes.extend(response.sandboxes); + } + if response.next_page_token.is_empty() { break; } diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 44ddc5ebe7..fabfc3dfbe 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -92,6 +92,7 @@ struct SandboxState { template_get_requests: Arc>>, template_list_requests: Arc>>, template_delete_requests: Arc>>, + sandboxes: Arc>>, } #[derive(Clone, Default)] @@ -264,7 +265,10 @@ impl OpenShell for TestOpenShell { &self, _request: tonic::Request, ) -> Result, Status> { - Ok(Response::new(ListSandboxesResponse::default())) + Ok(Response::new(ListSandboxesResponse { + sandboxes: self.state.sandboxes.lock().await.clone(), + next_page_token: String::new(), + })) } async fn create_sandbox_template( @@ -1667,6 +1671,24 @@ async fn add_provider(server: &TestServer, name: &str, provider_type: &str) { }); } +async fn add_sandbox(server: &TestServer, name: &str, phase: SandboxPhase) { + let mut sandbox = Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: format!("sandbox-{name}"), + name: name.to_string(), + created_time: None, + labels: HashMap::new(), + resource_version: 0, + annotations: HashMap::new(), + workspace: String::new(), + deletion_time: None, + }), + ..Sandbox::default() + }; + sandbox.set_phase(phase as i32); + server.openshell.state.sandboxes.lock().await.push(sandbox); +} + fn test_tls(server: &TestServer) -> TlsOptions { server.tls.with_gateway_name("openshell") } @@ -1703,6 +1725,7 @@ async fn sandbox_delete_continues_after_entry_failure() { &server.endpoint, &["failing-sandbox".to_string(), "later-sandbox".to_string()], false, + false, "default", &tls, "openshell", @@ -1772,6 +1795,89 @@ async fn sandbox_create_tolerates_an_unreachable_profile_catalog() { .await; } +#[tokio::test] +async fn sandbox_delete_all() { + let server = run_server().await; + let tls = test_tls(&server); + + add_sandbox(&server, "unspecified", SandboxPhase::Unspecified).await; + add_sandbox(&server, "provisioning", SandboxPhase::Provisioning).await; + add_sandbox(&server, "ready", SandboxPhase::Ready).await; + add_sandbox(&server, "error", SandboxPhase::Error).await; + add_sandbox(&server, "deleting", SandboxPhase::Deleting).await; + add_sandbox(&server, "stopping", SandboxPhase::Stopping).await; + add_sandbox(&server, "stopped", SandboxPhase::Stopped).await; + add_sandbox(&server, "starting", SandboxPhase::Starting).await; + add_sandbox(&server, "completed", SandboxPhase::Completed).await; + add_sandbox(&server, "unknown", SandboxPhase::Unknown).await; + + assert!( + run::sandbox_delete( + &server.endpoint, + &[], + true, + false, + "default", + &tls, + "openshell" + ) + .await + .is_ok() + ); + + assert_eq!( + deleted_names(&server).await, + vec![ + vec!["unspecified".to_string()], + vec!["provisioning".to_string()], + vec!["ready".to_string()], + vec!["error".to_string()], + vec!["deleting".to_string()], + vec!["stopping".to_string()], + vec!["stopped".to_string()], + vec!["starting".to_string()], + vec!["completed".to_string()], + vec!["unknown".to_string()], + ] + ); +} + +#[tokio::test] +async fn sandbox_delete_prune() { + let server = run_server().await; + let tls = test_tls(&server); + + add_sandbox(&server, "unspecified", SandboxPhase::Unspecified).await; + add_sandbox(&server, "provisioning", SandboxPhase::Provisioning).await; + add_sandbox(&server, "ready", SandboxPhase::Ready).await; + add_sandbox(&server, "error", SandboxPhase::Error).await; + add_sandbox(&server, "deleting", SandboxPhase::Deleting).await; + add_sandbox(&server, "stopping", SandboxPhase::Stopping).await; + add_sandbox(&server, "stopped", SandboxPhase::Stopped).await; + add_sandbox(&server, "starting", SandboxPhase::Starting).await; + add_sandbox(&server, "completed", SandboxPhase::Completed).await; + add_sandbox(&server, "unknown", SandboxPhase::Unknown).await; + + assert!( + run::sandbox_delete( + &server.endpoint, + &[], + false, + true, + "default", + &tls, + "openshell" + ) + .await + .is_ok() + ); + + assert_eq!( + deleted_names(&server).await, + vec![vec!["error".to_string()], vec!["completed".to_string()],] + ); +} + #[tokio::test] async fn sandbox_create_keeps_command_sessions_by_default() { let server = run_server().await; diff --git a/deploy/man/openshell.1.md b/deploy/man/openshell.1.md index 7dabf558ab..5662e10275 100644 --- a/deploy/man/openshell.1.md +++ b/deploy/man/openshell.1.md @@ -65,8 +65,8 @@ development task, or behind a cloud reverse proxy. **sandbox get** *NAME* : Show details for a sandbox. -**sandbox delete** *NAME* \| **--all** -: Delete one or all sandboxes. +**sandbox delete** *NAME* \| **--prune** \| **--all** +: Delete one, all inactive or all sandboxes. **sandbox connect** *NAME* \[**--editor** *EDITOR*\] : SSH into a running sandbox. diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index 2e824c730e..6ec3f4e0b8 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -953,6 +953,15 @@ Delete multiple sandboxes by listing their names in one command: openshell sandbox delete sandbox-a sandbox-b ``` +Use `--all` to delete all sandboxes. The `--prune` flag only deletes sandboxes +in `Error` or `Completed` phase (see [sandbox phases](#sandbox-lifecycle)). The +two flags are mutually exclusive. + +```shell +openshell sandbox delete --all +openshell sandbox delete --prune +``` + When a multi-sandbox delete fails for one entry, the CLI reports that sandbox's failure and continues with the remaining names. The command exits with an error after it attempts every requested deletion if any entry failed. diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 472d7fadbb..740dfc4cc8 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -477,6 +477,7 @@ openshell logs my-sandbox --since 5m openshell sandbox delete my-sandbox openshell sandbox delete sandbox-1 sandbox-2 sandbox-3 # Multiple at once openshell sandbox delete --all +openshell sandbox delete --prune # Only inactive ``` `deletion accepted` means cleanup is still pending. Inspect the sandbox until