Repository navigation
chore(sdk): remove dead constant/attribute and de-duplicate Commands handle setup - #1907
Conversation
… handle setup Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
🦋 Changeset detectedLatest commit: 2b7e5e5 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 c3c30a1. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.51.1-devin-1790579016-sdk-dead-code.0.tgzCLI ( npm install ./e2b-cli-2.20.1-devin-1790579016-sdk-dead-code.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.1-devin-1790579016-sdk-dead-code.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-devin-1790579016-sdk-dead-code.0.tgzPython SDK ( pip install ./e2b-2.51.0+devin.1790579016.sdk.dead.code-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.0+devin.1790579016.sdk.dead.code-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.6.0+devin.1790579016.sdk.dead.code-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: complies — 0 violations (checked T-1/T-2 parity, T-23 option types, T-26/T-27 handles & callbacks, T-47 named defaults, T-54 entry-point exports; DEFAULT_SANDBOX_TIMEOUT_MS was unused and never re-exported from index.ts, and createHandle is a behavior-preserving private helper).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good — straightforward dead-code cleanup with no behavior change. Reviewed the removed DEFAULT_SANDBOX_TIMEOUT_MS constant and _envd_api_url attribute assignments (confirmed unused elsewhere in the repo, and envd_api_url parameter itself is still used to build the RPC/streaming clients), and the createHandle extraction in Commands (verified the run/start and connect code paths were byte-for-byte identical before the refactor, and CommandConnectOpts is structurally compatible with the new helper's Pick<CommandStartOpts, 'onStdout' | 'onStderr'> parameter type). Sync and async Python filesystem files were updated in parallel as required.
Extended reasoning...
Diff touches connectionConfig.ts, Commands.run/connect in js-sdk, and Filesystem.init in both python-sdk sync/async modules; no auth, crypto, or data-exposure surface involved. Confirmed via git show of the base commit that the pre-refactor connect and start blocks were identical duplicated logic, so the new createHandle helper is a faithful extraction, and confirmed both removed items (constant and attribute) have zero remaining references in the repo. Small, mechanical, self-contained change with an included changeset, deciding factor for approval.
…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>
![https://static.devin.ai/assets/gh-devin-review-dark.svg?v=4"><img](https://static.devin.ai/assets/gh-devin-review-dark.svg?v=4"><img){kind=link}
1 participant
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
DEFAULT_SANDBOX_TIMEOUT_MS(src/connectionConfig.ts): Remove SDK-side defaults from API request payloads #1749 removed its last use when it dropped the SDK-side 300s default. It isn't re-exported fromindex.tsand isn't referenced anywhere in the monorepo.Filesystem._envd_api_url(sandbox_sync/filesystem/filesystem.py,sandbox_async/filesystem/filesystem.py): the attribute was set in__init__but never read. Theenvd_api_urlconstructor parameter stays because it is still passed to the RPC and streaming clients.Commands.connect/Commands.start(src/sandbox/commands/index.ts): these had the same 20-linehandleProcessStartEvent → new CommandHandle(...)/cleanup + handleRpcErrorWithHealthCheckblock (a jscpd clone). It now lives in one private helper:e2band@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:
Sandbox.default_sandbox_timeout = 300: nothing in the SDK reads it since Remove SDK-side defaults from API request payloads #1749, but it is a public class attribute andtests/test_client.pyasserts it.secure=onSandbox.createand JSSandboxOpts.secure: deprecated, accepted and ignored on purpose.stream_idle_timeouton syncFilesystem.read/Volume.read_file, andrequest_timeouton sync streamingcommands.connect/pty.create/pty.connect: the docstrings say these are ignored, and they are kept so the signatures match async.Sandbox.getMcpUrl()/getMcpToken(), deprecatedSandboxApi.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).connectSandbox,listSandboxes,listSandboxLogs,transformTemplateData, token-refresh helpers,format.tshelpers,User*/Columntypes. Knip flags these because nothing imports them, but each one is used inside its own module. The superfluousexportwas left alone to keep the diff small.Pty.kill/Pty.connectin the js-sdk are near-clones of theCommandsversions, but they sit in a separate class and use theonDataPTY callback. Sharing that code would need a larger refactor than this mechanical cleanup.Verification
pnpm run lint,pnpm run formatandpnpm run typecheckpass on all packages.tests/sandbox/commandsplus the unit/connectionConfig projects.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