Repository navigation
chore(sdk): share sandbox create response mapping and command handle setup - #1941
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: d027dca 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 |
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
Package ArtifactsBuilt from 68e65e0. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.52.1-devin-1791183890-sdk-dead-code.0.tgzCLI ( npm install ./e2b-cli-2.21.1-devin-1791183890-sdk-dead-code.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.1-devin-1791183890-sdk-dead-code.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-devin-1791183890-sdk-dead-code.0.tgzPython SDK ( pip install ./e2b-2.52.0+devin.1791183890.sdk.dead.code-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.1+devin.1791183890.sdk.dead.code-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.6.0+devin.1791183890.sdk.dead.code-py3-none-any.whl |
There was a problem hiding this comment.
TASTE.md review: complies, 0 violations. Checked T-1/T-2 (sync/async parity; shared logic goes in sandbox/), T-3a, T-18 (generated Sandbox model mapped to SDK-owned SandboxCreateResponse), T-28 (sync _create_handle doesn't take on_stdout/on_stderr/request_timeout, which T-28 allows), T-54 and T-69/T-71. The public surface is unchanged and both new helpers are internal.
There was a problem hiding this comment.
LGTM — straightforward, behavior-preserving refactor.
What was reviewed: the new to_sandbox_create_response() helper in sandbox/sandbox_api.py and its six call sites in the sync/async sandbox_api.py create/connect/fork paths — the extracted logic is byte-for-byte identical to the inlined code it replaces, just parameterized by the sandbox object. The new _create_handle() in sync/async commands/command.py — compared the extracted try/except body against the original _start/connect implementations line by line; the only parameterized difference is the action string used in extract_start_pid error messages, and all other arguments (on_stdout/on_stderr in async, generator typing in sync) are threaded through unchanged.
Extended reasoning...
Single-commit, Python-SDK-only refactor touching sandbox_api.py (sync/async/shared) and commands/command.py (sync/async) to deduplicate repeated SandboxCreateResponse mapping and command-handle setup logic; no new logic, no touched auth/crypto surface, just code motion with a parameter added for the one varying string. Diffed each extracted block against its six prior inlined copies and confirmed exact equivalence, and a changeset is included as required by CLAUDE.md for python-sdk public-surface-adjacent changes even though behavior is unchanged.
|
check comments above |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@mishushakov Addressed Codex's parity comment in d027dca: the JS SDK now has the same shared The earlier red Staging Python jobs came from staging infra, not this change: |
|
CI on d027dca: 41 passed, 2 test jobs failed (plus the
A re-run of these two jobs should clear them. |
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
SandboxCreateResponsemapping: the same 15-line "APISandboxmodel →SandboxCreateResponse" block (narrowdomain/envd_access_token/traffic_access_tokenfromUnsettoNone) was copied 6 times:_create_sandbox,_cls_connectand each_cls_forkresult, in bothsandbox_sync/sandbox_api.pyandsandbox_async/sandbox_api.py. It is now one helper,to_sandbox_create_response(sandbox), in the sharede2b/sandbox/sandbox_api.py.createSandbox,connectSandboxand eachforkSandboxresult injs-sdk/src/sandbox/sandboxApi.tsbuilt the same object literal. They now call a module-privatetoSandboxCreateResponse(sandbox). The internalSandboxForkResponsetype is nowSandboxCreateResponse | Error, and neither type is exported.Commands._start/Commands.connect(sync and async): both ended with the same "read start event →extract_start_pid→ buildCommandHandle/ 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 JSCommands.createHandlefrom chore(sdk): remove dead constant/attribute and de-duplicate Commands handle setup #1907. Theextract_start_piderror messages ("start process" / "connect to process") are kept.Patch changeset for
@e2b/python-sdkande2b.Not repeated here (already in open draft PRs)
#1766 and #1873 already cover the JS/Python
uploadUrl/downloadUrlsigning duplication, the JS volume/secret response tails,handleProcessStartEvent/handleWatchDirStartEvent, the CLIinit/migratelanguage list and the CLI's superfluousexports. Those were not duplicated here.Public-API candidates deliberately left in place
Sandbox.default_sandbox_timeout: nothing 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.create/ JSSandboxOpts.secure: deprecated, accepted and ignored on purpose.stream_idle_timeout(Filesystem.read,Volume.read_file) andrequest_timeout(synccommands.connect/_start,pty.create/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(); Pythonget_mcp_url,ConnectionConfig.set_integration(used by tests/integrations).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.DockerfFileFinalParserInterface: it is the return type of theDockerfileParserInterface.set_start_cmdprotocol and appears in the docs.Left as-is (not mechanical)
Commands.kill/Pty.kill(and the JSCommands/Ptyequivalents) are near-identical, but they sit on separate public classes.make_dir/get_info/update_metadata/write_filehave similar response tails, but not identical ones:update_metadataskips theVolumeErrorcheck. Merging them would change behaviour.Template.buildvsbuild_in_backgroundshare signature and docs only. CLItemplate delete/publishshare a short interactive-selection block with different prompts.Verification
make lint,make format,make typecheck(ruff + ty) on python-sdk all pass.tests/shared,tests/test_*.py, sync/asynctest_config_propagation(734 passed).commands/,test_connect,test_fork,test_create,test_snapshot,test_secure(93 passed).pnpm run format,pnpm run lint,pnpm run typecheckpass. vitesttests/sandbox/{create,connect,fork,apiDefaults,lifecyclePayload,lifecycleRequest,configPropagation,iam,onResumeRequest,snapshot-api,secure,snapshot,kill}.test.tspass.Link to Devin session: https://app.devin.ai/sessions/4e5922ccd81b4c6e83225fa6d158e84b
Open in Devin Desktop: https://app.devin.ai/desktop/session/4e5922ccd81b4c6e83225fa6d158e84b?variant=devin