feat: onResume option on connect/resume - #1800
Conversation
Picks up `memory` on ConnectSandbox and ResumedSandbox, which the API serves and the spec no longer marks unimplemented. The rest is churn the pin had fallen behind: the removed access-token models, httpsPorts and the reworded transform rules on the network config, and Rig schemas that survive the tag filter as orphan components. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
Resuming a memory-inclusive snapshot whose memory image wedges the guest had no self-serve way out. `onResume: 'reboot'` (`on_resume` in Python) cold-boots from disk state instead, and the memory snapshot is never touched, so a later restore still works. 'restore' is the API's own default and travels as an absent `memory` field; only the 'reboot' literal sends `memory: false`, so an unrecognized value from an untyped caller falls back to a memory restore rather than silently cold-booting. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
PR SummaryMedium Risk Overview OpenAPI and generated clients pick up optional Reviewed by Cursor Bugbot for commit 9ab04ef. Bugbot is set up for automated code reviews on this repo. Configure here. |
🦋 Changeset detectedLatest commit: 9ab04ef The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Package ArtifactsBuilt from 50135b4. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.46.2-feat-on-resume-option.0.tgzCLI ( npm install ./e2b-cli-2.18.1-feat-on-resume-option.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.7.3-feat-on-resume-option.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.3.5-feat-on-resume-option.0.tgzPython SDK ( pip install ./e2b-2.46.4+feat.on.resume.option-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.9.3+feat.on.resume.option-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.4.6+feat.on.resume.option-py3-none-any.whl |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
TASTE.md review
Checked the SDK-facing changes (packages/js-sdk/src/sandbox/sandboxApi.ts, index.ts, packages/python-sdk/e2b/sandbox/sandbox_api.py, sandbox_sync/, sandbox_async/, __init__.py, and the new tests) against: parity (T-1, T-2, T-10), API shape (T-3, T-6, T-14, T-15, T-19, T-22, T-23), boundary mapping (T-18), entry-point exports (T-54), client-side validation (T-52), and docs (T-69–T-74). Generated files (schema.gen.ts, api/client/models/*, spec/) were not judged.
Violations: 1 (low severity, same issue mirrored in sync/async Python — flagged inline on both).
- T-3 / T-1d —
on_resumelands as another positional-with-default in the Pythonconnect()signatures, extending the existingtimeoutinto a chain of optional positionals (Sandbox.connect(id, 300, "reboot")now type-checks). Since JS carries it inSandboxConnectOpts, the Python mirror should be keyword-only.timeout/loggerare pre-existing and out of scope, so a*separator can't be dropped in without moving them; noted as a design-debt item rather than something to block on.
Everything else lines up well: 'restore' | 'reboot' instead of a memory: boolean mirror of the wire field (T-14); the wire boolean is mapped at the boundary and the generated ConnectSandbox type never reaches users (T-18); SandboxOnResume is a named, exported type (export type in JS, __all__ in Python — T-23, T-54); no client-side check of whether the deployment supports filesystem-only resume (T-52); all four Python connect docstrings plus the JS option JSDoc document the parameter, default, and rejection behavior (T-69, T-74). Observation, not a violation: onResume/on_resume reuses the on<Event> shape T-27 reserves for callbacks, but onTimeout already set that precedent for lifecycle action config, so consistency wins here.
Landing it after `timeout` made `Sandbox.connect(id, 300, "reboot")` a valid call, while JS only accepts the option through SandboxConnectOpts. Placing it after `logger` behind a `*` separator keeps both pre-existing parameters positional, so nothing existing moves. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
The gate short-circuits a memoryless snapshot before it consults the feature flag, and a running sandbox never reaches the gate, so "not enabled rejects reboot" was only true for a paused snapshot that holds memory. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
Replaces the placeholder pin with the commit that actually carries it, so the tracked specs come from the Copybara fetch. Also picks up the freeze endpoint's mode parameter and vanished counts on the envd spec; both are x-internal, so no SDK client changes. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
The docs said a reboot leaves the memory snapshot restorable later. It does not: while the sandbox runs, connect is a keep-alive; a later pause writes a new snapshot over the row; kill ends the resume path. The guarantee that does hold is for a reboot that fails to start, so the docs now say that and point at pausing again to keep a snapshot. The keyword-only guard asserted the wrong signature. class_method_variant wraps the instance method, so inspect.signature resolved to `(self, timeout, *, on_resume)` and the static-by-id chain was never inspected — dropping the `*` from _cls_connect_sandbox left the whole suite green while `connect(id, 300, logger, "reboot")` became valid again. Each form is now asserted directly. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
Saying a reboot that fails to start leaves the sandbox paused held for the three failures after dispatch, but not for the two refusals before it. A sandbox whose start is already in flight is refused outright and is not paused, so a caller following that advice retries into a keep-alive that returns 200 without rebooting — the server's own message on that path says to retry once it is running or paused. The docs now scope the retryable claim to a reboot that started, and say a running or starting sandbox has to be paused before it can be rebooted, which nothing said before. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
Two claims about reachability were wrong. A running sandbox is not returned unchanged — connect keeps it alive and extends its timeout, as the option above it already documents — and telling the caller to pause a sandbox whose start is in flight sends them into a second refusal, because a reservation holds no store entry and pause answers that it is already paused. The option is ignored for a running sandbox, so pausing first is the way to reboot one; a request that would drop memory is refused mid-start, so the start has to finish. Both bounds are scoped to requests that would actually drop memory, since a snapshot without memory joins a start and cold-boots regardless. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
mishushakov
left a comment
mishushakov
left a comment
There was a problem hiding this comment.
all looks good, however i'd shorten the comments in the doc strings and probably exclude Rig from the type-gen (this is probably a separate issue?)
Review feedback: the option docs carried the whole rescue contract (in-flight refusal, failed-boot state, pause-again guidance). Keep what a caller needs at the call site — the two values, the crash-consistency caveat, flag-off rejection, and the no-op cases — and leave the rest to the resume docs. Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
| * | ||
| * @default 'restore' | ||
| */ | ||
| onResume?: SandboxOnResume |
There was a problem hiding this comment.
Shortened the docstrings in 9ab04ef — JSDoc and all six Python :param on_resume entries now carry just the two values, the crash-consistency caveat, the flag-off rejection and the no-op cases; the rest of the contract moves to the resume docs.
Agreed on Rig being separate: the SDK bundles in redocly.yaml filter operations by tag but never prune component schemas (remove-unused-components is on for envd/volume only), so every schema leaks into both generated clients. Opening a separate PR that turns it on for js-sdk and python-sdk — drops 20/23 unused schemas (Rig*, Admin*, Node*, TeamAPIKey*, VolumeToken…), none referenced from SDK source.
|
Rig / unused-schema pruning is #1803 ( |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ab04ef8a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 'e2b': minor | ||
| '@e2b/python-sdk': minor |
There was a problem hiding this comment.
For consumers that import CreatedAccessToken or NewAccessToken from the Python generated models, or reference those schemas through the root-exported JavaScript components type, installing this minor release causes runtime import failures or downstream TypeScript compilation failures because this same commit deletes those public symbols. Either preserve compatibility while adding onResume, or mark both affected packages for a major release rather than allowing ordinary minor upgrades to receive the removals.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good — a small, well-scoped feature addition with matching implementations across JS and both Python sync/async SDKs.
What was reviewed: the onResume/on_resume wiring from connect() through to the ConnectSandbox request body (memory: false only for 'reboot', otherwise omitted) in JS and Python sync/async; parity across sandboxApi.ts, sandbox_sync/sandbox_api.py, and sandbox_async/sandbox_api.py; the spec regeneration (spec/infra-ref bump plus consistent memory field additions in openapi.yml, envd.yaml-derived Python models, and schema.gen.ts); and the new unit tests, which cover default/restore/reboot/unrecognized-value cases plus keyword-only-parameter enforcement on both static and instance connect forms.
Extended reasoning...
Overview
The PR adds an onResume/on_resume option ('restore' | 'reboot') to Sandbox.connect() across JS and Python (sync + async). 'reboot' sends memory: false in the ConnectSandbox request body to cold-boot from disk state instead of restoring the memory snapshot; 'restore' (the default) omits the field entirely, matching the API's own default. The change also bumps spec/infra-ref and regenerates spec/openapi.yml, spec/envd/envd.yaml, the JS schema.gen.ts, and several unrelated Python generated models that had fallen behind the pin (rig/network/access-token models). A changeset was added for both e2b and @ e2b/python-sdk.
Security risks
None identified. This is a client-side option that only toggles an optional boolean field in an existing authenticated request; it does not touch auth, crypto, or permission logic. The untyped-caller fallback behavior (only the literal 'reboot' string triggers memory: false) is a deliberate safety choice that avoids silently dropping memory data on unexpected input, and this exact behavior is tested.
Level of scrutiny
This warranted a moderate level of scrutiny: it changes public SDK surface across three language runtimes (JS, Python sync, Python async) and touches auto-generated spec files that must never be hand-edited. I verified: (1) the spec/infra-ref pin was bumped and the memory field additions in openapi.yml/schema.gen.ts/Python models are consistent with a genuine codegen regeneration rather than a hand edit; (2) the JS and both Python implementations wire onResume/on_resume through to the request body identically (memory: false iff 'reboot', else UNSET/undefined); (3) an inline review comment from devin-ai-integration about making on_resume keyword-only was addressed by a real follow-up commit (5155388) plus dedicated regression tests (test_on_resume_is_keyword_only), not merely self-resolved without action.
Other factors
Test coverage is strong: 5 JS tests and roughly 10 Python tests cover default/restore/reboot/unrecognized-value cases across sync, async, static-by-id, and instance forms, plus explicit keyword-only-parameter enforcement tests to prevent positional-argument foot-guns. The PR author's description of mutation-testing each assertion adds confidence, though I did not independently re-run that mutation testing. The docstrings went through several iterative refinements across commits, all narrowing scope/wording rather than changing behavior. No CODEOWNERS file was found restricting these paths. Given the small, mechanical, well-tested nature of the change and that the one substantive reviewer objection was demonstrably fixed with a commit and tests, this is approvable without requiring further human review.
2 participants
Resuming a paused sandbox always restores its memory snapshot, so a snapshot whose memory image wedges the guest has no self-serve way out.
onResume: 'reboot'cold-boots from the sandbox's disk state instead. The reboot neither modifies nor deletes the memory snapshot, so a reboot that fails to start leaves the sandbox paused and the resume retryable; after a successful reboot the way to keep a snapshot is to pause again.'restore'remains the default and keeps travelling as an omittedmemoryfield — only the'reboot'literal sendsmemory: false, so an unrecognized value from an untyped caller falls back to a memory restore rather than silently cold-booting.Disk state carries crash-recovery semantics, so writes not flushed before the pause may be lost. Where filesystem-only resume is not enabled, a
'reboot'that would actually drop memory is rejected with an error rather than quietly restoring it. The no-op cases (a snapshot holding no memory, a sandbox still running) never reach that gate and succeed either way — the API short-circuits them before it consults the flag.The spec pin is real:
spec/infra-refpoints at the infra commit that dropsx-not-implementedfrom these fields, and the tracked specs come from the Copybara fetch at that commit rather than from anything placed by hand.redocly.yaml'sfilter-outprunes the marked property, so until that marker was gone codegen produced clients with nomemoryfield at all — which is why this PR waited on it.Generated filesis green.The rest of the spec diff is churn the pin had fallen behind by: removed access-token models,
httpsPortsand reworded transform rules on the network config,Rig*schemas that survive the tag filter as orphan components, and the resume request body becoming optional.Verified: 5 JS + 10 Python request-shape tests over sync, async, instance and static-by-id forms, each assertion mutation-checked (flip the literal to
!== 'restore', forcefalse, drop the guard, drop each forwarding hop — every mutant reddens exactly the intended row).pnpm run lint,formatandtypecheckclean. The credential-backed suites can't run without anE2B_API_KEY: locally they fail identically tomain(231 JS, 426 Python, byte-identical failure sets), so CI's run is the real check there.