fix(cli): pass options after the command through to the remote command in sandbox exec - #1759
fix(cli): pass options after the command through to the remote command in sandbox exec#1759devin-ai-integration[bot] wants to merge 4 commits into
Conversation
…d in sandbox exec 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:
|
🦋 Changeset detectedLatest commit: 3450fda The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 1508bec. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.45.1-devin-1787399758-cli-exec-passthrough.0.tgzCLI ( npm install ./e2b-cli-2.17.2-devin-1787399758-cli-exec-passthrough.0.tgzPython SDK ( pip install ./e2b-2.45.1+devin.1787399758.cli.exec.passthrough-py3-none-any.whl |
There was a problem hiding this comment.
TASTE.md review (CLI-only change, so most SDK-surface rules don't apply). Checked: T-43 (shell escaping of composed command strings), T-59 (argument validation errors), T-12 (acronym casing), T-15/T-19/T-22 (type & naming shape), T-47 (named constants). 2 findings, 1 of them a TASTE violation.
The pass-through mechanics themselves look right: passThroughOptions() requires enablePositionalOptions() on every ancestor, and both the root program and sandbox got it. I verified locally that the -- strip in buildCommand is load-bearing (commander keeps the -- in the variadic operand list, so removing the strip makes exec <id> -- codex exec --help send -- codex exec --help).
Not tied to a changed line:
<sandboxID>in the pre-existing.argument()name (and now echoed in the new help text) contradicts T-12 — identifiers spell acronyms as words (sandboxId), all-caps is only for prose ("sandbox ID"). Out of scope for this PR, but worth fixing in a follow-up so the new help text isn't cementing the old spelling.- Enabling positional options on the root
programis a global parsing change: any future root-level option will have to be passed before the subcommand name. Fine today (the root command declares no options), just noting the blast radius is wider thansandbox exec. - T-43 is satisfied for the multi-token path via
shellQuote; note the single-token path still passes the string through unquoted by design (shell-string form), which the change preserves.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5f1e6d825
ℹ️ 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".
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
|
check comments |
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
|
check comments |
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Summary
e2b sbx exec <id> codex exec "prompt" --helpprinted e2b's own help instead of running codex: commander parsed every flag after the command as an e2b option.Fix is commander's positional-options mode:
programandsandboxCommandget.enablePositionalOptions(),execCommandgets.passThroughOptions(), so option parsing stops at the first operand (the sandbox ID). e2b flags now must precede the command — the same contract asdocker exec/ssh.buildCommandalso drops a leading--, which commander no longer strips once the operand terminates parsing.Verified against a live sandbox plus new unit tests in
packages/cli/tests/commands/sandbox/exec_passthrough.test.ts.Link to Devin session: https://app.devin.ai/sessions/19811be8d8b14437b7e2f4038eae99d8
Requested by: @mishushakov