From d0ef037584e0387f3d82318efd8ee44b246f0f9e 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, stopped, or errored sandboxes while preserving active and provisioning ones. The prune filter targets sandboxes in these phases: - Unspecified - Unknown - Error - Deleting - Stopped - Completed Active sandboxes (Ready, Provisioning, Starting) 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 +- skills/openshell-cli/SKILL.md | 1 + 5 files changed, 131 insertions(+), 9 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 9f55658efc..b609dfe970 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1476,12 +1476,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. @@ -3332,11 +3336,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 098c52c1cf..30513c3255 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -408,7 +408,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}" @@ -3350,13 +3350,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 { @@ -3370,7 +3371,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 033bd2479e..0093aba5ee 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -82,6 +82,7 @@ struct SandboxState { template_get_requests: Arc>>, template_list_requests: Arc>>, template_delete_requests: Arc>>, + sandboxes: Arc>>, } #[derive(Clone, Default)] @@ -210,7 +211,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( @@ -1486,6 +1490,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") } @@ -1522,6 +1544,7 @@ async fn sandbox_delete_continues_after_entry_failure() { &server.endpoint, &["failing-sandbox".to_string(), "later-sandbox".to_string()], false, + false, "default", &tls, "openshell", @@ -1587,6 +1610,89 @@ async fn sandbox_create_tolerates_an_unreachable_profile_catalog() { ); } +#[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/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 057874fa3a..6a1f35156b 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -416,6 +416,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