Repository navigation
feat(sandbox): support AgentENV auto-pause and resume - #3757
Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-3757-050b9394c8c7.docs.buildwithfern.com/nemo/gym |
|
/ok to test 8daf471 |
|
/claude review |
|
/ok to test b1d98d0 |
|
/claude review |
|
SHIP — low-risk, well-scoped E2B lifecycle change. Reviewed the full diff across the four changed files. Summary of what I checked and one non-blocking note. What is correct:
NOTE (author's call, defense-in-depth): The status mapping keys on the literal strings Ran the pinned base/head revision check before and after analysis; both matched |
|
/claude review |
|
/ok to test ba72878 |
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP: LGTM. I found nothing wrong in the provider change itself. I'm leaving two non-blocking NOTEs on the new real-SDK tests.
What I checked:
- Config and validation. YAML and mapping values for
create.auto_resume(defaultfalse) reach the dataclass. Non-boolean values are rejected inE2BCreateConfig.__post_init__. - Create paths.
create()and the REST path in_create_with_extension_paramssend the same lifecycle fortrueandfalse, and neither sends one fornull. - Status.
status()maps running, paused, unrecognized and transient-failure cases correctly. The real-SDK test confirms it sends only the control-plane GET. - Status consumers. The sandbox facade passes status through unchanged. OSWorld's readiness loop keeps polling on
PAUSEDorUNKNOWN, so neither state makes a live sandbox look dead. - In-repo AgentENV profile.
osworld_agentenv.yamldoesn't setauto_resume, so its sandboxes are now deleted when the TTL expires. That's the leak fix the PR describes, and the compatibility notes cover it. - Docs. The docs page, the provider README and the
e2b.yamlcomments match the head code. - Earlier threads. The current head addresses the earlier review threads: the default value, docs consistency, the resume floor, the 410 behavior, connect resetting the timeout, and the real-SDK status test.
One behavior change to keep in mind: if the control plane reports a sandbox as running but its envd is dead, status() now returns RUNNING where it used to return STOPPED. In that case OSWorld readiness waits out its own timeout instead of failing early. The PR states this, so I'm not counting it as a finding.
On rollouts, the description still lists a model-driven rollout as pending. AGENTS.md's smoke-rollout rule is aimed at environment and agent changes, and the live AgentENV smoke run exercised the changed create, status and close paths, so I'm not treating the pending rollout as a merge gate.
One of my three review passes (docs accuracy) finished without its report reaching me, so I checked the docs page, README and YAML comments against the code myself.
Review generated by claude. This was a full review. Request a full review with /review reset=true.
Expose an optional coupled pause/resume lifecycle policy, enable it in the AgentENV profile, and query control-plane status without waking paused sandboxes. Cover SDK request serialization and document the server-side resume timeout floor. Signed-off-by: Hemil Desai <hemildesai@users.noreply.github.com>
Signed-off-by: Hemil Desai <hemildesai@users.noreply.github.com>
Signed-off-by: Hemil Desai <hemildesai@users.noreply.github.com>
E2B 2.42+ (including the locked 2.46) sends no lifecycle fields unless the caller sets them, and AgentENV then pauses timed-out sandboxes without auto-resume and never evicts them, so crashed or cancelled runs leak paused VMs. Default create.auto_resume to false, which sends kill-on-timeout explicitly; null keeps the SDK/server default and is documented as unsafe on AgentENV. Folds in #4598. Review follow-ups: update the timeout/close wording and config snippet in the docs and provider README, describe the post-resume minimum timeout as an operator setting counted from the resume, note the 410 during pause/resume and that connect() resets the timeout to 300 s, pin the AgentENV link, and add a real-SDK test that status sends only the control-plane GET. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Hemil Desai <hemildesai@users.noreply.github.com>
With create.custom_extension_params set, the provider creates through POST /sandboxes, which sent no lifecycle. Sandboxes created that way (for example sandbank opt-ins) therefore missed the kill-on-timeout default and could stay paused on AgentENV. Send the same autoPause and autoResume fields the SDK create sends, and test both paths against the real SDK serialization. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Hemil Desai <hemildesai@users.noreply.github.com>
The real-SDK lifecycle and status tests relied on an earlier test to install Gym's transport; run alone they would reach the real E2B API. Each now installs the transport itself. Add a 404 case showing that a sandbox killed at its timeout reads STOPPED through the real SDK. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Hemil Desai <hemildesai@users.noreply.github.com>
322e70b to
050b939
Compare
|
/ok to test 050b939 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/41531613587 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/41534673535 |
| timeout_s: float | None = 3600.0 | ||
| # True pauses/resumes; False kills at timeout; None sends no lifecycle. E2B 2.42+ then leaves | ||
| # the choice to the server, and AgentENV pauses without resume and never evicts the sandbox. | ||
| auto_resume: bool | None = field(default=False, kw_only=True) |
There was a problem hiding this comment.
Now that pyproject.toml requires e2b>=2.46.0 (since #4822), null always means "send no lifecycle fields". On hosted E2B that behaves like false. On AgentENV it is the paused-and-never-reaped case this PR fixes. Is there a gateway that rejects autoPause/autoResume and needs null?
- If not, a plain
boolremoves the footgun, in line with CLAUDE.md's "do not add options for hypothetical future needs". - If it stays:
- give that reason in the
nullrow of the docs table (e2b.mdx#L229-L231). - add
Nonetotest_real_sdk_serializes_lifecycle_policyand assert thatautoPauseandautoResumeare absent from the request body. Today, a change that makes the REST create path send the kill fields fornullstill passes every test.
- give that reason in the
Either way, the 2.36–2.41 history here, in e2b.yaml#L67-L68 and in e2b.mdx#L233-L235 describes versions the floor no longer allows. "the locked 2.46" will also go stale with the next lock bump. Saying that omitting the fields leaves the choice to the server (AgentENV pauses without resume) should be enough. A related, older line at e2b.mdx#L24 still says e2b>=2.36.0.
| them the full ``ApiParams`` raises ``TypeError: unexpected keyword | ||
| argument 'api_key'``. | ||
| """ | ||
| """Return the request timeout; data-plane methods reject connection parameters.""" |
There was a problem hiding this comment.
The new one-liner no longer holds for every caller. status() now passes these params to get_info, a control-plane call that accepts the full ApiParams (e2b 2.46 sandbox_async/main.py#L662-L675). It also drops the reason the helper exists. Suggest keeping the old explanation and adding get_info:
| """Return the request timeout; data-plane methods reject connection parameters.""" | |
| """Per-request options for calls on an existing sandbox object. | |
| ``commands.run`` and ``files.*`` take ``request_timeout`` only; handing them the full | |
| ``ApiParams`` raises ``TypeError: unexpected keyword argument 'api_key'``. ``get_info`` | |
| accepts ``ApiParams``, but the sandbox already carries the connection config, so the | |
| request timeout is enough. | |
| """ |
| sandbox that is already gone as closed, and clears the local handle only after confirmed cleanup. Serialized handles | ||
| carry the E2B sandbox ID for reconnection through another configured provider instance. E2B's | ||
| public connect operation may extend a near-expiry sandbox to its SDK-default connection lifetime | ||
| (300 seconds in E2B 2.36). |
There was a problem hiding this comment.
Nit: this line runs to 116 characters while the rest of the page wraps at about 96. A reflow, which also drops the version from the old 300-second note (2.46 still uses 300):
| sandbox that is already gone as closed, and clears the local handle only after confirmed cleanup. Serialized handles | |
| carry the E2B sandbox ID for reconnection through another configured provider instance. E2B's | |
| public connect operation may extend a near-expiry sandbox to its SDK-default connection lifetime | |
| (300 seconds in E2B 2.36). | |
| sandbox that is already gone as closed, and clears the local handle only after confirmed | |
| cleanup. Serialized handles carry the E2B sandbox ID for reconnection through another | |
| configured provider instance. E2B's public connect operation may extend a near-expiry sandbox | |
| to its SDK-default connection lifetime (300 seconds). |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/41537455431 |
…ilience Resolve status() against #3757's control-plane get_info: a not-found answer still reads as UNKNOWN while the sandbox is new. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Hemil Desai <hemildesai@users.noreply.github.com>
What does this PR do?
Add optional automatic pause and traffic-triggered resume support to Gym's E2B provider for AgentENV. A sandbox that reaches its TTL can retain its state and resume when the agent next uses it, instead of becoming unusable until manually resumed or deleted.
e2b.create.auto_resume, defaultfalse:falseexplicitly sends kill-on-timeout plus disabled auto-resume;truesends pause-on-timeout plus auto-resume;nullsends no lifecycle and leaves the action to the SDK/service.falseby default: E2B 2.36–2.41 default to kill-on-timeout, but 2.42+ (including the locked 2.46) omit the lifecycle fields. AgentENV then pauses timed-out sandboxes without auto-resume and never evicts them, so a crashed or cancelled run leaks paused VMs. We hit this in practice: sandboxes from a crashed client stayed paused for hours past their TTL. This folds in fix(sandbox): kill E2B sandboxes at timeout instead of pausing them #4598.PAUSEDwithout sending an envd health request that would wake the VM.No separate issue is needed for this focused integration of AgentENV's existing auto-eviction and proxy auto-resume APIs.
Validation
python -m pytest tests/unit_tests/test_e2b_provider.py tests/unit_tests/test_e2b_build.py tests/unit_tests/test_sandbox.py tests/unit_tests/test_sandbox_connect.py tests/unit_tests/test_sandbox_access.py --no-cov -q -o addopts='': 221 passed, 2 skipped, with locked E2B 2.46.0. The skips are existing opt-in sandbox survival tests.python -m pytest tests/unit_tests/test_e2b_provider.py --no-cov -q -o addopts=''with minimum E2B 2.36.0: 112 passed.mainand pass with this change.pre-commit run --all-files: passed.cd fern && npm run check -- --warnings: zero errors; missing-redirect checks skipped because Fern authentication is unavailable.git diff --check: passed.CI classification is full because this changes the shared sandbox provider. The complete core/server suites were not run locally.
Follow-up commit (kill by default, review fixes):
test_e2b_provider.py+test_e2b_build.py+ sandbox tests: 224 passed, 2 skipped with locked E2B 2.46.0;test_e2b_provider.py115 passed with E2B 2.36.0.test_create_kills_at_timeout_by_defaultfails with the oldNonedefault; newtest_real_sdk_status_only_reads_control_planeasserts that status sends onlyGET /sandboxes/{id}through the real SDK (2.36.0 and 2.46.0).pre-commit run --all-filesandfern check --warnings: passed (0 errors; the redirect check is skipped without Fern auth).Rollouts
Live provider smoke passed with both E2B 2.36.0 and locked E2B 2.46.0 on an AgentENV deployment, using the
osworld-slimtemplate and certificate-verified HTTPS with test-local CA trust:PAUSEDstate at expiry; repeated status checks did not wake it.auto_resume: falsedeletes at expiry (control-plane 404) rather than retaining a paused VM.lifecycle={"on_timeout": "kill"}on AgentENV and e2b 2.53.1, were gone at 95 s (GET 404, not in the paused listing). Before the change, same-shape sandboxes were still listed as paused hours after their TTL.Compatibility and benchmark impact
Behavior change: sandboxes are now deleted at timeout by default on every SDK version, which matches E2B's documented default and the 2.36–2.41 behavior. Only E2B 2.42+ against a server that defaults to pausing (AgentENV) changes, from a never-reaped pause to deletion. Set
auto_resume: trueto opt into pause and resume, ornullto restore the previous "send nothing" behavior. No benchmark profile is changed. The new dataclass parameter is keyword-only to preserve positional compatibility. E2B status now reflects control-plane state rather than guest-health reachability.TTL remains a lifetime deadline, not an idle timer. AgentENV currently imposes a server-side
EnsureMinimum(300s)after proxy auto-resume; this PR does not change that upstream floor. Paused state retains storage, so normal cleanup is still required. No dataset, verifier, or scoring changes; benchmark-result impact has not been measured.Checklist
🤖 Generated with Claude Code