fix(sandbox): keep UnixLocal workspace-root removal off the event loop

[ranjan-del]

Summary

This pull request moves the UnixLocal workspace-root removal off the event loop, completing the set of operations #4700 converted.

UnixLocalSandboxClient.delete() removed the workspace root with a direct shutil.rmtree call inside an async def:

try:
    shutil.rmtree(Path(inner.state.manifest.root), ignore_errors=False)
except FileNotFoundError:
    pass
except Exception:
    pass

Removing the root walks the whole workspace tree, so the loop was held for the full removal and no other task could advance. rm(recursive=True), persist_workspace, and hydrate_workspace already hand that class of work to run_blocking_workspace_io, whose docstring states the contract: it runs the function in a worker thread and keeps ownership until that worker finishes. Its module docstring explains why that matters over plain asyncio.to_thread, which does not stop its worker when the awaiting task is cancelled. #4700 introduced the helper and converted those three sites. git blame puts this line at 2d665c9a6 (2026-04-15) and git log -L returns only that commit for it, so #4700 did not touch it. The rationale for preferring the helper over bare to_thread is in the #4700 description, which also records that it is why #4678 was closed.

Scope note, since this is a rmtree/archive-class fix rather than a sweep: other inline synchronous filesystem calls remain in this module, notably the shutil.copyfileobj in write and the os.scandir in ls. Those are bounded by a caller-supplied payload and a single directory rather than by a recursive tree walk, and I have left them alone here.

shutil.rmtree relies on its stdlib default ignore_errors=False, because the helper forwards positional arguments only. Errors are therefore still raised and still swallowed by the unchanged except clauses. When no cancellation is observed, task.result() re-raises the worker's original exception object, so the types reaching those clauses are the same; when a cancellation is observed the helper raises that CancelledError instead. This matches how the sibling rm(recursive=True) call site invokes it.

Present in v0.22.1, not only on main.

Released-behavior delta, disclosed

delete() can now raise CancelledError. Scoped precisely, because it depends on the manifest:

Session shape	v0.22.1	This change
No ephemeral mount targets	No suspension point in delete(), so it returned the session	Can raise CancelledError; under asyncio.wait_for the caller still waits for the full removal and then receives TimeoutError
Has ephemeral mount targets	Already awaited mount_entry.unmount(...), so cancellation could already be delivered before the removal was reached	Unchanged in kind; one further cancellation point

The removal always runs to completion, because the helper keeps the worker owned, so nothing leaks and the workspace is still removed. Delivering cancellation is impossible without a suspension point, so this is inherent to the fix. It is the same trade-off #4700 accepted for the three sibling sites. Note that CancelledError derives from BaseException, so neither the method's own except Exception nor a caller's contextlib.suppress(Exception) absorbs it.

Worth flagging for the in-SDK path: the sole caller, _SandboxSessionResources.cleanup, wraps the call in except BaseException, records it as cleanup_error if no earlier error was recorded, and still runs _aclose_dependencies() in a finally, so dependency teardown stays complete. It then re-raises, and at SandboxRuntimeSessionManager.cleanup a non-None cleanup_error skips serialize_resume_state(). That skip is pre-existing for any cleanup error; what is new is that cancellation can now reach it from here, so a cancelled cleanup loses the serialized sandbox resume state. Happy to guard that separately if you would rather it were absorbed.

Otherwise unchanged: the workspace_root_owned and unmount_failed early returns and the unmount ordering are byte-identical to v0.22.1, and the same session is returned on every path that returns at all. The pre-existing TypeError guard for a foreign session type is untouched.

Test plan

Added test_client_delete_keeps_workspace_removal_off_the_event_loop in tests/sandbox/test_unix_local.py. It builds the session through the public client.resume(state), replaces shutil.rmtree with a wrapper that still performs the real removal, and uses an event handshake rather than wall-clock timing: the worker signals that the removal has begun and then waits for a loop-side observer to answer, so the assertion covers the removal itself rather than the whole delete() call.

That scoping matters. An earlier draft counted loop progress across the whole call, which an await elsewhere in delete(), such as the ephemeral unmount loop, could satisfy on its own. Verified against both regressions:

Source under test	Result
This change	passes
Inline shutil.rmtree	assert [False] == [True]
Inline shutil.rmtree plus an unrelated await asyncio.sleep(0.01) in delete()	assert [False] == [True]

The test also asserts the workspace root is really gone and that the same session is returned, so it pins the removal target, not just the dispatch mechanism.

Ownership-on-cancellation is not separately pinned here; it is a property of run_blocking_workspace_io and is already covered for hydrate_workspace by the test #4700 added.

.agents/skills/code-change-verification/scripts/run.sh completes with exit 0: make format, make lint (All checks passed!), make typecheck (mypy 0 errors, pyright no issues found in 310 source files), and make tests (9539 passed, 29 skipped). The pre-existing tests/sandbox/test_runtime.py -k delete tests also pass unchanged.

Issue number

No existing issue. Found while auditing the module for remaining instances of the pattern #4700 addressed. Happy to open one first if you would prefer the discussion to start there.

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

Not using Codex, so the last item does not apply.

[seratch]

Thanks for tracing this and documenting the cancellation behavior. The direct rmtree call does block the event loop, and reusing the existing helper is a focused approach.

Before proceeding, please provide a realistic workspace example showing meaningful delays to another coroutine during deletion, comparing v0.22.2 with this change without artificially delaying rmtree. The handshake test establishes scheduling behavior, while #4675's evidence covers archive creation and extraction. Deletion-specific impact would help justify this change, including the newly reachable cancellation path that skips resume-state serialization.

[ranjan-del]

Fair ask. The handshake test only establishes scheduling, and #4675's numbers are about archive and extract, so here is a deletion-specific measurement.

What varies

Only the line under test. I toggle it on disk between runs and restore it from git afterwards:

baseline   shutil.rmtree(Path(inner.state.manifest.root), ignore_errors=False)
this PR    await run_blocking_workspace_io(shutil.rmtree, Path(inner.state.manifest.root))

I used this branch's parent as the baseline rather than checking out the v0.22.2 tag, so the difference between the two arms is that line and nothing else. The tag carries the same call byte for byte (v0.22.2:src/agents/sandbox/sandboxes/unix_local.py:1274), so the baseline arm is the released code path.

shutil.rmtree is not patched, wrapped, subclassed or delayed anywhere in the harness.

Workspace

npm install of typescript 5.7.2, eslint 9.17.0, webpack 5.97.1, jest 29.7.0, express 4.21.2, react and react-dom 18.3.1. 8,711 files, 97 MB. Replicated as packages/app-N for the larger sizes, which is the shape of a monorepo an agent has actually built in.

Measurement

Session built through the public client.resume(state), torn down with client.delete(session). A second coroutine ticks on an absolute 5 ms schedule and records how late each tick really ran. It stands in for whatever else is on the loop: a streamed response, an MCP session, a tracing exporter.

Five trials per cell, arms interleaved so neither systematically runs on a warmer machine. Medians. Apple M1, macOS 26.6.2, APFS, Python 3.13.14.

files	arm	delete	worst tick lateness	ticks delivered / due
8,759	baseline	405 ms	406 ms	1 / 81
8,759	this PR	410 ms	1.0 ms	84 / 82
26,277	baseline	1,200 ms	1,201 ms	1 / 240
26,277	this PR	1,266 ms	5.2 ms	255 / 253
52,554	baseline	2,275 ms	2,276 ms	1 / 455
52,554	this PR	2,370 ms	1.0 ms	476 / 474

The 1 is not rounding. On the baseline the loop delivers exactly one tick for the entire deletion, the one that lands as it comes back, and worst lateness tracks total delete time to within a millisecond at every size. With the change the loop holds its schedule and delivers what the elapsed time was due.

With no delete in flight, worst lateness is under 1 ms on both arms, so what the table shows is the deletion and not the harness.

Cost

Deletion itself gets slower: +1.2% at 8.7k files, +5.5% at 26k, +4.2% at 52k. Same worker doing the same unlinks either way, so that is the thread hop and the wait loop. Stating it rather than burying it.

The fixture understates this

Each trial workspace is built with cp -Rc, so the files are APFS clones. The same workspace built with a plain cp -R deletes in 538 ms against 388 ms for the clone, both on the baseline. The real stall is larger than the table, not smaller.

Cancellation

Fired from a plain thread with loop.call_soon_threadsafe(task.cancel), the way a signal handler or a shutdown watchdog would, 150 ms into a removal that takes around 400 ms. Nothing patches rmtree here either; the cancel lands mid-removal because the removal outlasts the delay.

arm	what delete() did	workspace
baseline	returned normally, the caller's except BaseException saw nothing	removed
this PR	raised CancelledError	removed

That is the delta I described in the PR body, measured rather than reasoned about. On the baseline the cancel cannot be delivered at all, because there is no suspension point to deliver it at. The removal completes on both arms, so nothing leaks either way.

Resume state

Driving the real SandboxRuntimeSessionManager.cleanup() with a resource whose cleanup raises:

resource cleanup	serialize_resume_state() called	resume state
succeeds	yes	returned
raises CancelledError	no	lost
raises RuntimeError	no	lost

So the skip is not new, any cleanup error already reaches it. What this PR adds is a way for cancellation to be that error.

If you would rather it stayed unreachable, absorbing CancelledError in delete() once the removal has completed is small and I can fold it in here or leave it for a separate PR. I did not want to make that call on my own since it changes cancellation semantics for the caller.

The harness is four short scripts. Happy to paste them or push them somewhere if you want to rerun any of this.