fix(terminal): persistent sandbox envs survive between turns#6370
Closed
malaiwah wants to merge 1 commit into
Closed
fix(terminal): persistent sandbox envs survive between turns#6370malaiwah wants to merge 1 commit into
malaiwah wants to merge 1 commit into
Conversation
`_cleanup_task_resources` was unconditionally calling `cleanup_vm()` at the end of every `run_conversation` (i.e. every user turn), tearing down the docker/daytona/modal sandbox container regardless of its `persistent_filesystem` setting. This contradicted the documented intent of `terminal.lifetime_seconds` (idle reaper) and `container_persistent`, and caused per-turn loss of `/workspace`, `~/.config`, agent CLI auth state, and any other content living inside the sandbox. The unconditional teardown was introduced in fbd3a2f ("prevent leakage of morph instances between tasks", 2025-11-04) to plug a Morph backend leak, two days after `lifetime_seconds` shipped in faecbdd. It was later refactored into `_cleanup_task_resources` in 70dd3a1 without changing semantics. Code and docs have disagreed since. Fix: introduce `terminal_tool.is_persistent_env(task_id)` and skip the per-turn `cleanup_vm` when the active env is persistent. The idle reaper (`_cleanup_inactive_envs`) still tears persistent envs down once `terminal.lifetime_seconds` is exceeded. Non-persistent backends (Morph) are unchanged — still torn down per turn, preserving the original leak-prevention intent.
Contributor
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.
Fixes #6369.
Problem
HermesAgentLoop._cleanup_task_resourcesunconditionally callscleanup_vm(task_id)at the end of everyrun_conversation(every user turn), tearing down docker / daytona / modal sandbox containers regardless ofpersistent_filesystem. This contradictsterminal.lifetime_seconds(the idle reaper) andcontainer_persistent, and causes per-turn loss of/workspace,~/.config, sub-agent CLI auth state, and any in-sandbox content.The unconditional teardown was introduced in fbd3a2f ("prevent leakage of morph instances between tasks", 2025-11-04) to plug a Morph backend leak — two days after
lifetime_secondsshipped in faecbdd. It was later refactored into_cleanup_task_resourcesin 70dd3a1 without changing semantics. Code and docs have disagreed since.Fix
terminal_tool.is_persistent_env(task_id)— small helper that returnsTrueif the active env has_persistent=True._cleanup_task_resources, skipcleanup_vmwhen the env is persistent. The idle reaper (_cleanup_inactive_envs) still tears it down onterminal.lifetime_secondsexpiry.Diff is 36 lines across 2 files.
Verification
Repro from #6369 now shows the same container hostname across turns, and
/workspacewrites persist untilterminal.lifetime_secondsof inactivity.