autoresume: lifecycle component in sdk#1146
Conversation
🦋 Changeset detectedLatest commit: 515f9b7 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 dc25c40. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.13.2-lifecycle-in-sdk.0.tgzCLI ( npm install ./e2b-cli-2.8.1-lifecycle-in-sdk.0.tgzPython SDK ( pip install ./e2b-2.14.1+lifecycle.in.sdk-py3-none-any.whl |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
adding the changeset should not have caused tests to fail, tests seem a bit flaky for some reason (not related to this PR i don't think) |
matthewlouisbrockman
requested review from
ValentaTomas,
jakubno and
mishushakov
as code owners
|
@codex review |
![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: f1e0ba8559
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
got the issues - the end to end tests now pass on staging / fail on prod (expected; client proxy not live on prod yet so should fail) |
| """ | ||
|
|
||
|
|
||
| def get_auto_resume_policy(lifecycle: Optional[SandboxLifecycle]) -> Optional[str]: |
There was a problem hiding this comment.
I'd just do this inline, no need for another function
| assert False, "Sandbox not found" | ||
|
|
||
|
|
||
| def test_lifecycle_auto_resume_policy_mapping(): |
mishushakov
left a comment
mishushakov
left a comment
There was a problem hiding this comment.
approved but:
- unnecessary function get_auto_resume_policy which we don't have in JS.
- i am confused you'd call auto_resume policy "any" - it's too vague, most people won't be able to infer the meaning of the setting (I know it's only on backend, but still).
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| secure: opts?.secure ?? true, | ||
| allow_internet_access: opts?.allowInternetAccess ?? true, | ||
| network: opts?.network, | ||
| ...(autoPause !== undefined ? { autoPause } : {}), |
There was a problem hiding this comment.
Conditional autoPause !== undefined is always true
Low Severity
The autoPause variable is computed as lifecycle.onTimeout === 'pause', which always produces a boolean — never undefined. The conditional autoPause !== undefined on the spread is therefore always true, making the ternary dead code. This was likely written by analogy with the autoResumePolicy check below (where undefined IS a real possibility), but it's misleading here since a reader might incorrectly assume there's a path where autoPause is intentionally excluded from the body.
Additional Locations (1)
|
|
plan's it goes to |
| autoResume?: { | ||
| policy: 'any' | 'off' | ||
| } | ||
| } = { |
There was a problem hiding this comment.
Redundant type intersection for already-defined schema field
Low Severity
The type intersection & { autoResume?: { policy: 'any' | 'off' } } on the body variable is redundant because components['schemas']['NewSandbox'] already includes autoResume?: components["schemas"]["SandboxAutoResumeConfig"] which resolves to exactly { policy: "any" | "off" }. This adds unnecessary type complexity and misleadingly implies the generated schema doesn't support autoResume, when it already does.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| return None | ||
|
|
||
| auto_resume = lifecycle.get("auto_resume") | ||
| return "any" if auto_resume else "off" |
There was a problem hiding this comment.
Reviewer-acknowledged function still present after agreed removal
Low Severity
The get_auto_resume_policy helper function was explicitly flagged for removal/inlining in the PR review, and the author acknowledged they would remove it, but it remains in the final diff along with its duplicated tests. The JS SDK inlines the equivalent logic directly in createSandbox, creating an unnecessary asymmetry between the two SDKs. The function is only a few lines and the logic is simple enough to inline at the two call sites in sandbox_async/sandbox_api.py and sandbox_sync/sandbox_api.py.
Additional Locations (2)
|
staging not deployed yet so new API not up |
|
Sticking with the developer preview while cache issue and timeout parameter decision pending. |
2 participants
Implements the
lifecycleprop onSandbox.create, taking over and deprecating thebeta_pausefunctionality.Currently supports:
on_timeout:kill(default) |pause. Controls what should happen to the sandbox when it hits end of life. Pause allows for resumingauto_resume: False (default) | True. Whether the sandbox should autoresume on trafficIntended for additional functionality as we update the backend to support additional props. Blocked from deploying until the API and client-proxy are deployed but for pre-approval.
(Meant to be extended later as add more capabilities to the API)
Note
Medium Risk
Changes the API contract and request payload shape for sandbox auto-resume and alters lifecycle/timeout behavior mapping, which could break older integrations if backend/client versions are mismatched.
Overview
Adds a new
lifecycleconfiguration onSandbox.create(JS + Python) to control what happens at timeout (killvspause) and whether paused sandboxes auto-resume on traffic (auto_resume).Deprecates
betaPause/beta_pauseand the JSautoPausecreate option in favor of the new lifecycle semantics, updates connect/pause call paths accordingly, and expands tests to cover resume-on-connect and auto-resume behaviors.Updates the OpenAPI contract and generated clients so
autoResumeis now an object with anenabled: booleanflag (removing the previous policy enum), and bumps SDK versions via a changeset.Written by Cursor Bugbot for commit 515f9b7. This will update automatically on new commits. Configure here.