Repository navigation
Remove SDK-side defaults from API request payloads - #1749
Conversation
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
🦋 Changeset detectedLatest commit: 11d377d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 9674edd. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.50.1-devin-1787318715-remove-sdk-defaults.0.tgzCLI ( npm install ./e2b-cli-2.20.1-devin-1787318715-remove-sdk-defaults.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.1-devin-1787318715-remove-sdk-defaults.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-devin-1787318715-remove-sdk-defaults.0.tgzPython SDK ( pip install ./e2b-2.50.0+devin.1787318715.remove.sdk.defaults-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.0+devin.1787318715.remove.sdk.defaults-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.5.0+devin.1787318715.remove.sdk.defaults-py3-none-any.whl |
There was a problem hiding this comment.
TASTE.md review of the SDK-side default removal.
Checked: parity across JS / sync Python / async Python (T-1, T-2), where defaults live and how they are documented (T-47), server-owns-validation (T-52), absence-is-undefined / Optional[...] = None at the boundary (T-20), and docstring/JSDoc completeness (T-69, T-71, T-72).
The mechanics are clean and applied symmetrically across all three surfaces (UNSET in Python, omitted keys in JS), and the new tests pin both the omit and the explicit-value paths. 4 violations flagged inline, all in the documentation and validation edges rather than the payload change itself.
Not tied to a changed line:
connect/_cls_connectstill applies the SDK-side 300 s default (apiOpts?.timeoutMs ?? DEFAULT_SANDBOX_TIMEOUT_MSinsandboxApi.ts,timeout or SandboxBase.default_sandbox_timeoutin both Pythonsandbox_api.py). After this PRcreate/forkdefer to the API whileconnectdoes not, so the sametimeoutknob has two different "unset" behaviours. Either moveconnectover too or say in the changeset why it keeps a client default.- Several new doc lines restate the server's current value in prose ("currently enabled", "currently allowed", "currently 1", "currently a full memory snapshot"). That is the drift T-47 exists to avoid, without the machine-readable
@defaulttag that made the value greppable. If the value is worth documenting, document it as@default; if it isn't, say only that the SDK omits the field and the API decides.
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
The BYOP surface from #1688 diverged for callers that bypass the types. Python raised InvalidArgumentException on a proxy without a string address; JS rebuilt the body from the known fields, so `egressProxy` passed as a bare string sent `{}` and the caller got an API error naming a field they never left out. Mirror the guard in buildEgressProxyBody, the way buildIamBody already does for untyped token maps. Both SDKs also forwarded a null/None username or password as a JSON null, which the API rejects — `{"username": os.environ.get(...)}` on an unset variable is the way that happens. Read it as "no credentials", the same reading both already gave `egressProxy: null` itself, and normalize a null username coming back out of getInfo so SandboxEgressProxyInfo.username cannot be a null its type forbids. The get_info example published in both CHANGELOGs for 2.41.0 subscripts `info.network["egress_proxy"]`, which KeyErrors on every sandbox without a proxy — SandboxNetworkInfo is total=False and the key is only set when one is configured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oxy input" This reverts commit b984e34.
Fixture sandboxes previously inherited the SDK's 300s create default; after removing SDK-side defaults they would fall back to the API's 15s default, making long-running integration tests flaky. Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
|
check comments |
…out default, simplify order docs Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
|
resolve conflicts |
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
|
Resolved — merged |
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Sync the v2 create/connect routes into spec/openapi.yml and regenerate the
JS and Python API clients. Sandbox.create posts to /v2/sandboxes
(NewSandboxV2, no secure field: envd access is always secured) and
Sandbox.connect posts to /v2/sandboxes/{id}/connect with an optional body,
so omitted timeouts fall back to the API's 300s default. Drops the secure
option from create and the Python ConnectSandboxBody shim.
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
…tests at v2 create Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
secure stays in the create signatures (JS, Python sync/async, desktop-python) so existing callers don't hit an invalid-argument error, but it is ignored and never serialized into NewSandboxV2. Template.getBuildStatus no longer presets logsOffset=0; when omitted the API default applies. Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
…ct spec Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
…browser suite Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
…handle setup (#1907) ## Summary Scheduled dead-code audit of `packages/js-sdk/src`, `packages/python-sdk/e2b`, `packages/cli/src`. Tools: knip, `tsc --noUnusedLocals/--noUnusedParameters`, vulture, ruff (`ARG`/`F`), jscpd, then grep across the monorepo, tests and the docs repo. Generated code (`*.gen.ts`, `*_pb.ts`, `*_connect.ts`, `e2b/api/client*`, `e2b/volume/client`, `mcp.d.ts`) was not included. No behaviour change. ### Removed / de-duplicated - **js-sdk `DEFAULT_SANDBOX_TIMEOUT_MS`** (`src/connectionConfig.ts`): #1749 removed its last use when it dropped the SDK-side 300s default. It isn't re-exported from `index.ts` and isn't referenced anywhere in the monorepo. - **python-sdk `Filesystem._envd_api_url`** (`sandbox_sync/filesystem/filesystem.py`, `sandbox_async/filesystem/filesystem.py`): the attribute was set in `__init__` but never read. The `envd_api_url` constructor parameter stays because it is still passed to the RPC and streaming clients. - **js-sdk `Commands.connect` / `Commands.start`** (`src/sandbox/commands/index.ts`): these had the same 20-line `handleProcessStartEvent → new CommandHandle(...)` / `cleanup + handleRpcErrorWithHealthCheck` block (a jscpd clone). It now lives in one private helper: ```ts private async createHandle(events, clearStartTimeout, cleanup, opts?: Pick<CommandStartOpts, 'onStdout' | 'onStderr'>): Promise<CommandHandle> ``` - Patch changeset for `e2b` and `@e2b/python-sdk`. ### Public-API candidates deliberately left in place These have no references inside the repo but users can reach them, so removing them would be a breaking change: - python-sdk `Sandbox.default_sandbox_timeout = 300`: nothing in the SDK reads it since #1749, but it is a public class attribute and `tests/test_client.py` asserts it. - python-sdk `secure=` on `Sandbox.create` and JS `SandboxOpts.secure`: deprecated, accepted and ignored on purpose. - python-sdk `stream_idle_timeout` on sync `Filesystem.read` / `Volume.read_file`, and `request_timeout` on sync streaming `commands.connect` / `pty.create` / `pty.connect`: the docstrings say these are ignored, and they are kept so the signatures match async. - Carried over from #1811 with no change: js-sdk `Sandbox.getMcpUrl()`/`getMcpToken()`, deprecated `SandboxApi.getFullInfo()`, `ConnectionConfig.envdPort`, `TemplateBase.toJSON()`, `LogEntry.toString()`, and exported types referenced only in their own file (`Branded`, `PtyCreateOpts`, `PtyConnectOpts`, `FilesystemRequestOpts`, `FilesystemListOpts`, `WatchOpts`, `SandboxUrlOpts`, `CallableTemplate`, `DockerfileParse*`, `BasicBuildOptions`, `*Registry`, `UploadBody`, `VolumeApiPaths`, `GitHubMcpServer`). - cli: `connectSandbox`, `listSandboxes`, `listSandboxLogs`, `transformTemplateData`, token-refresh helpers, `format.ts` helpers, `User*` / `Column` types. Knip flags these because nothing imports them, but each one is used inside its own module. The superfluous `export` was left alone to keep the diff small. - Not changed: `Pty.kill` / `Pty.connect` in the js-sdk are near-clones of the `Commands` versions, but they sit in a separate class and use the `onData` PTY callback. Sharing that code would need a larger refactor than this mechanical cleanup. ### Verification - `pnpm run lint`, `pnpm run format` and `pnpm run typecheck` pass on all packages. - js-sdk vitest: `tests/sandbox/commands` plus the unit/connectionConfig projects. - python-sdk pytest: `tests/{sync,async}/sandbox_*/files` (123 tests against live sandboxes) plus the offline filesystem/client tests. Link to Devin session: https://app.devin.ai/sessions/d3d4f4a2a02d491184ca8d51b4867404 Open in Devin Desktop: https://app.devin.ai/desktop/session/d3d4f4a2a02d491184ca8d51b4867404?variant=devin Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…setup (#1941) ## Summary Scheduled dead/duplicated-code sweep of `packages/js-sdk/src`, `packages/python-sdk/e2b`, `packages/cli/src`, using knip, `tsc --noUnusedLocals/--noUnusedParameters`, vulture, ruff (`F`/`ARG`) and jscpd. Generated clients (`*.gen.ts`, `*_pb*`, `*_connect*`, `e2b/api/client`, `e2b/volume/client`) were excluded. Every candidate was then grepped across the monorepo, its tests and the docs repo. Behaviour is unchanged. Nothing was removed or renamed in the public API. This run found **no new dead code**. The tools flagged nothing in the JS SDK or CLI beyond what is already listed below. Everything vulture/ruff flagged in Python turned out to be either used or public API. This PR only fixes two duplications that were added or still exist since #1907. Per AGENTS.md, the response-mapping change is applied in JS too. ### De-duplicated - **`SandboxCreateResponse` mapping**: the same 15-line "API `Sandbox` model → `SandboxCreateResponse`" block (narrow `domain` / `envd_access_token` / `traffic_access_token` from `Unset` to `None`) was copied 6 times: `_create_sandbox`, `_cls_connect` and each `_cls_fork` result, in both `sandbox_sync/sandbox_api.py` and `sandbox_async/sandbox_api.py`. It is now one helper, `to_sandbox_create_response(sandbox)`, in the shared `e2b/sandbox/sandbox_api.py`. - **JS equivalent**: `createSandbox`, `connectSandbox` and each `forkSandbox` result in `js-sdk/src/sandbox/sandboxApi.ts` built the same object literal. They now call a module-private `toSandboxCreateResponse(sandbox)`. The internal `SandboxForkResponse` type is now `SandboxCreateResponse | Error`, and neither type is exported. - **`Commands._start` / `Commands.connect`** (sync and async): both ended with the same "read start event → `extract_start_pid` → build `CommandHandle` / on error close stream + `handle_rpc_exception_with_health`" block. It now lives in a private `_create_handle(events, action, ...)`. This is the Python version of the JS `Commands.createHandle` from #1907. The `extract_start_pid` error messages ("start process" / "connect to process") are kept. Patch changeset for `@e2b/python-sdk` and `e2b`. ### Not repeated here (already in open draft PRs) #1766 and #1873 already cover the JS/Python `uploadUrl`/`downloadUrl` signing duplication, the JS volume/secret response tails, `handleProcessStartEvent`/`handleWatchDirStartEvent`, the CLI `init`/`migrate` language list and the CLI's superfluous `export`s. Those were not duplicated here. ### Public-API candidates deliberately left in place - Python `Sandbox.default_sandbox_timeout`: nothing reads it since #1749, but it is a public class attribute and `tests/test_client.py` asserts it. - Python `secure=` on `Sandbox.create` / JS `SandboxOpts.secure`: deprecated, accepted and ignored on purpose. - Python sync `stream_idle_timeout` (`Filesystem.read`, `Volume.read_file`) and `request_timeout` (sync `commands.connect`/`_start`, `pty.create`/`connect`): the docstrings say these are ignored, and they are kept so the signatures match async. - JS `Sandbox.getMcpUrl()`/`getMcpToken()`, deprecated `SandboxApi.getFullInfo()`, `ConnectionConfig.envdPort`, `TemplateBase.toJSON()`, `LogEntry.toString()`; Python `get_mcp_url`, `ConnectionConfig.set_integration` (used by tests/integrations). - JS types that knip reports as unused exports (`Branded`, `PtyCreateOpts`, `PtyConnectOpts`, `FilesystemRequestOpts`, `FilesystemListOpts`, `WatchOpts`, `SandboxUrlOpts`, `McpServer`, `GitHubMcpServer`, `CallableTemplate`, `DockerfileParse*`, `BasicBuildOptions`, `*Registry`, `UploadBody`, `VolumeApiPaths`): they appear in public signatures and the generated SDK reference. - Python `DockerfFileFinalParserInterface`: it is the return type of the `DockerfileParserInterface.set_start_cmd` protocol and appears in the docs. ### Left as-is (not mechanical) - Python `Commands.kill` / `Pty.kill` (and the JS `Commands`/`Pty` equivalents) are near-identical, but they sit on separate public classes. - Python volume `make_dir`/`get_info`/`update_metadata`/`write_file` have similar response tails, but not identical ones: `update_metadata` skips the `VolumeError` check. Merging them would change behaviour. - `Template.build` vs `build_in_background` share signature and docs only. CLI `template delete`/`publish` share a short interactive-selection block with different prompts. ### Verification - `make lint`, `make format`, `make typecheck` (ruff + ty) on python-sdk all pass. - Offline: `tests/shared`, `tests/test_*.py`, sync/async `test_config_propagation` (734 passed). - Live against E2B: sync/async `commands/`, `test_connect`, `test_fork`, `test_create`, `test_snapshot`, `test_secure` (93 passed). - js-sdk: `pnpm run format`, `pnpm run lint`, `pnpm run typecheck` pass. vitest `tests/sandbox/{create,connect,fork,apiDefaults,lifecyclePayload,lifecycleRequest,configPropagation,iam,onResumeRequest,snapshot-api,secure,snapshot,kill}.test.ts` pass. Link to Devin session: https://app.devin.ai/sessions/4e5922ccd81b4c6e83225fa6d158e84b Open in Devin Desktop: https://app.devin.ai/desktop/session/4e5922ccd81b4c6e83225fa6d158e84b?variant=devin <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/e2b-dev/e2b/pull/1941" target="_blank"><picture><source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-devin-review-dark.svg?v=4"><img src="https://static.devin.ai/assets/gh-devin-review-light.svg?v=4" alt="Devin Review"></picture></a> <!-- devin-review-badge-end --> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>


Summary
Remove SDK-side defaults from API request payloads (JS + Python sync/async) so omitted options are absent from requests and the API defaults apply. Explicit values — including
false/0— are still serialized unchanged.Fields no longer preset when omitted:
timeout(was 300s),secure(wastrue),allow_internet_accesstimeout(was 300s)POST /sandboxes/{id}/fork:timeout(was 300s),count(was1)POST /sandboxes/{id}/pause:memory(wastrue)cpuCount/memoryMB(were 2 / 1024)Create/connect now use the v2 endpoints from belt #3425 (
spec/openapi.ymlsynced via thespec/runtime-refpin; JS + Python clients regenerated with the pinned generators):Consequently the
secureoption onSandbox.createis deprecated: it stays in the signature (JSSandboxOpts.secure, Python/desktopsecure=) so existing callers keep working, but it is ignored — every sandbox is secured, so the SDK has nothing to send. Downstream packages follow:e2b-desktop(Python) no longer presetsallow_internet_access=True; the code-interpreter test fixtures stop passingsecure. The PythonConnectSandboxBodyshim and the JS cast that worked around the required v1 connecttimeoutare gone too — the generated v2 models already make it optional.Template.getBuildStatus/Template.get_build_statusalso stop presettinglogsOffset: 0— omitted, the query param is left out and the API default applies.Per updated TASTE T-52 (client validation must not mirror backend business rules), the client-side fork
count >= 1pre-validation is also removed — an invalid count now surfaces as the API's own 400 error instead of a client-sideInvalidArgumentError/InvalidArgumentException.Belt #3425 is merged and exported:
spec/runtime-refis bumped to the runtime commit carrying the v2 routes, andmake codegenat that pin reproduces the tracked spec and generated clients byte-for-byte. The live integration suites stay red until the API deploy with the v2 routes reaches staging/production (they 404 on/v2/sandboxesuntil then).Exact-request tests added/updated in
packages/js-sdk/tests/sandbox/apiDefaults.test.ts,packages/js-sdk/tests/template/apiDefaults.test.ts, andpackages/python-sdk/tests/shared/{sandbox,template}/test_api_defaults.py; the msw/monkeypatch-mocked suites now intercept the v2 routes.Usage stays the same; only the outgoing payloads/routes change:
Linear: SDK-346
Link to Devin session: https://app.devin.ai/sessions/1921bb3818604f3a95b0db1272cff3cd
Open in Devin Desktop: https://app.devin.ai/desktop/session/1921bb3818604f3a95b0db1272cff3cd?variant=devin
Requested by: @mishushakov