Repository navigation
feat(cli): add sandbox prune command to delete ERROR-phase sandboxes #2594
Description
Activity
Implementation in: https://github.com/engelmi/OpenShell/tree/add-sandbox-prune-command
Hey Michael — took a proper look at
add-sandbox-prune-command, both a manual pass and a more thorough automated review. Good news: the overall approach is sound (reusingstate.compute.delete_sandboxinstead of reimplementing cleanup was the right call). A few things worth fixing before this goes up as a real PR, roughly in priority order:Two real bugs:
--all-workspacesdoesn't work as shipped — the CLI sends the default workspace name alongsideall_workspaces: true, and the server rejects that exact combination as mutually exclusive. Looks like it was only tested by constructing server requests directly, never through the actual CLI flag.- The SDK's top-level
prune_sandboxes()will hitpermission_deniedfor any non-admin caller — it sends an empty workspace expecting server-side default resolution, but authorization runs on the raw empty string before that normalization happens. This is masked because all the added tests use a helper that grants platform-admin to every principal.
Worth addressing before merge:
3. The sandbox listing uses a single fixedlimit=1000, offset=0call with no pagination —handle_list_sandboxesin the same file already has the cursor-based pattern for this, worth reusing so large workspaces don't silently drop sandboxes past the first page.
4. The delete loop processes sandboxes sequentially.delete_sandboxalready does its own per-sandbox locking, so nothing requires serializing the batch — bounded concurrency would avoid multiplying driver round-trip latency and repeated lock contention for a big prune.
5. Docs —docs/sandboxes/manage-sandboxes.mdxalready has a Delete/Lifecycle section this should extend.Minor:
6. Under--all-workspaces, the last-sandbox cache gets cleared using the CLI's configured workspace rather than each sandbox's actual origin workspace — moot until #1's fixed, but will resurface once it is.
7. There's an unrelatedmise.lockdiff (ansccacheblock) worth dropping.One more thing, not a fix, just context: I've been working with Drew on the RFC 0012 isolation-backend rollout, and there's an open question relevant to this design — does the reconciliation loop detect a half-created supervisor/workload pair under the new topology, and is
ERRORphase well-defined for that case? Still waiting on his answer, but wanted you to know it's in flight since your prune-by-ERROR-phase design leans on that being settled.Happy to help work through any of these if useful — this is close.
@sjenning since you're a maintainer here — could you take a look and weigh in on whether this overall design (a dedicated
PruneSandboxesRPC, best-effort per-sandbox deletion,ERROR-phase filtering) is something you'd be open to accepting once the issues above are addressed?I think I can say that we don't want to implement a new gRPC interface for this. Especially when it can be done client side with something like
for i in $(openshell sandbox list | grep -i error | cut -f1 -d' '); openshell sandbox delete $i; doneAt the very most, we could implement this in the CLI/SDK as a client helper
It seems this was covered and discounted in the alternatives
Alternatives Considered
Client-side filtering - CLI lists all sandboxes, filters for ERROR, then issues individual DeleteSandbox RPCs. Simpler (no proto changes), but introduces a TOCTOU race between list and delete, produces N round trips, and pushes logic into every client (CLI, SDK, Python SDK).But I think this is the way.
openshell sandbox delete --allalready works this way. Maybe aopenshell sandbox delete --prunewhere it is a phase filtered version of--all.I verified this as well. I'd suggest pivoting this to a CLI/SDK only sandbox prune command which would list with a phase filter or filter client side after listing, same as --all does today, then loop through the existing delete_sandbox path. Should be a much smaller diff than the RPC based version, and inherits correctness from code that's already there. @engelmi
Thanks for the feedback! @politerealism @sjenning
I'll prepare a PR for with the discussed--pruneoption to thedeletecommand on the client-side CLI.- added a commit that references this issue
on Sep 16, 2026 @politerealism @sjenning Opened PR #3378 to implement the
--pruneflag. I diverged a bit from the discussed filtering by not only deleting sandboxes in theERRORphase, but also in other "inactive" phases such asCompleted. Please have a look and let me know what you think.- added 8 commits that reference this issue
on Sep 17, 2026
Problem Statement
When sandboxes fail during provisioning or runtime, they remain in the ERROR phase indefinitely. Users currently have two options:
sandbox delete <name>, orsandbox delete --allThere is no way to clean up only the broken sandboxes while leaving healthy ones running.
This is especially painful in environments where multiple sandboxes are running concurrently - a single failed experiment shouldn't require manually identifying and deleting each errored sandbox, and
--all is too destructive.Proposed Design
Add
openshell sandbox prune- a CLI command backed by a new PruneSandboxes gRPC RPC that performs server-side filtering and deletion. The semantics are similar topodman container pruneanddocker container prune.Proto:
Alternatives Considered
Agent Investigation
→ gateway handler (handle_delete_sandbox) → ComputeRuntime::delete_sandbox()
→ compute driver. The handler is a thin authz wrapper; all complexity lives in
the compute layer which is safe to reuse.
checking phase is SandboxPhase::try_from(sandbox.phase()).ok() ==
Some(SandboxPhase::Error).
(reconcile_store_with_backend) already prunes orphan sandboxes on a timer, but
does not prune by phase — this is a separate concern.
filtering would require fetching all sandboxes.
Checklist