Conversation
A local run gets its own directory under
{workspace_base}/automation-runs/{run_id}, where it extracts its tarball
and builds a uv environment -- reported as ~520 MB per run when
UV_CACHE_DIR sits on another filesystem and uv cannot hardlink. Nothing
removed it on terminal state, for two reasons that compound:
- LocalAgentServerBackend.cleanup_after_verification only logged and
returned, so the directory outlived the run it belonged to.
- _should_cleanup_sandbox_after_terminal required run.sandbox_id, which
a local run never has, so the watchdog never called the backend at all.
The backend now deletes the directory, reusing the guarded deletion the
retention purge already had (extracted to utils/workspace.py so a
recursive delete is guarded in exactly one place: linked roots and run
directories are refused, as is anything resolving outside the runs root,
and a non-UUID run id never reaches it). The predicate now asks the
backend instead of the sandbox id, and keeps holding a keep_alive run --
which is what excludes a continue_conversation sandbox.
The existing retention sweep (OpenHands#277) stays the backstop for the runs the
watchdog never terminalizes: completed by callback, cancelled, crashed.
Fixes OpenHands#422
Claude-Session: https://claude.ai/code/session_01CC1C6MQsjZHysGccmVg7Fz
Contributor
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In local mode every run gets
{workspace_base}/automation-runs/{run_id}, extracts its tarball there and builds a uv environment in it. Nothing removed that directory when the run ended, for two reasons that compound:LocalAgentServerBackend.cleanup_after_verificationonly loggedLocal mode: skipping cleanup (persistent server)and returned._should_cleanup_sandbox_after_terminalrequiredrun.sandbox_id, which a local run never has, so the watchdog's terminal-cleanup path never reached the backend either.The reporter saw six runs leave 2.6 GB on a PVC-backed
~/.openhands(~520 MB each, becauseUV_CACHE_DIRwas on another filesystem and uv could not hardlink).Change
LocalAgentServerBackend.cleanup_after_verificationnow deletes the run's workspace directory. The agent server itself is still left alone, as it is persistent and externally managed._should_cleanup_after_terminal) asks the backend rather than the sandbox id:keep_alive is not True and (bool(run.sandbox_id) or backend.is_local_mode). Cloud behaviour is unchanged, and akeep_aliverun — which is how acontinue_conversationsandbox is held — is still excluded.openhands/automation/utils/workspace.py, so both callers share one implementation: a linked runs root or run directory is refused, so is anything resolving outside the runs root, and a run id that is not a UUID never reachesrmtree. Nothing raises; failures are logged.No new setting. The retention sweep added in #277 (
AUTOMATION_WORKSPACE_RETENTION_SECONDS, default 7 days) already covers the runs the watchdog never terminalizes — completed by callback, cancelled, or crashed — and its docstring now says so. An operator who wants those reclaimed in hours rather than days can lower that value today; happy to change the default in a follow-up if you'd rather ship a shorter one.Deliberately not deleting the workspace from
complete_run: the SDK sends that callback from the workspace context manager's__exit__, so the run's own process and the bash chain's EXIT trap are still live in that directory. The retention sweep is the safe place for that case.HUMAN:
Reproduced the leak on a self-hosted Agent Canvas 1.16.0 before the change: six finished runs left 2.6 GB under ~/.openhands/workspaces/automation-runs and nothing ever removed them. Ran the branch through the full unit suite and all linters locally (1630 passed, ruff and pyright clean, pre-commit hooks pass) and verified through the new tests that the guarded delete refuses traversal and symlinked roots.
Testing'
New tests cover: cleanup removes only the target run's directory; a non-UUID run id leaves a sibling directory untouched; a missing directory does not raise; and the predicate for a local run with no sandbox id (cleans up), the same run with
keep_alive(does not), and a cloud run with no sandbox id (unchanged).Fixes #422
https://claude.ai/code/session_01CC1C6MQsjZHysGccmVg7Fz