feat(sandbox): add environment variable support#111
Conversation
🦋 Changeset detectedLatest commit: 2649a6b The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 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 |
|
@codex review |
|
@greptile-apps review |
Greptile SummaryThis PR adds full environment variable support across the sandbox SDK and CLI: persisted vars baked in at create-time or managed post-creation via
Confidence Score: 4/5Safe to merge with low risk outside of the setEnv/unsetEnv path; the env body format sent to the MC PUT endpoint needs verification before those operations can be relied on in production. The create/exec/ssh env injection paths are well-constructed and the previously unguarded AGENT_TOKEN case is now correctly handled. The only genuine uncertainty is in setContainerEnv: it sends a flat Record body to a PUT endpoint while every other MC API interaction in the file uses Array-of-{name,value} objects, and the (client as any) cast means no compile-time check catches a format mismatch. If the endpoint requires the array format, sandbox env set, env delete, and unsetEnv would all fail at runtime on every invocation. packages/sandbox/src/provision.ts — specifically the setContainerEnv body format and the lack of type coverage for the PUT endpoint. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CLI
participant envArgs as env-args.ts
participant Sandbox as sandbox.ts
participant Provision as provision.ts
participant MC as MC API
note over CLI,MC: sandbox create --env KEY=VAL --env-file .env
CLI->>envArgs: collectEnv(entries, envFile)
envArgs-->>CLI: "Record<string,string>"
CLI->>Sandbox: "Sandbox.create({ env })"
Sandbox->>Sandbox: assertValidEnvKey + reserved check
Sandbox->>Provision: "createApp(client, { env })"
Provision->>MC: POST /apps (environmentVariables array)
MC-->>Provision: appId
Provision-->>Sandbox: appId
Sandbox-->>CLI: Sandbox instance
note over CLI,MC: sandbox env set name KEY=VAL
CLI->>Sandbox: sandbox.setEnv(vars)
Sandbox->>Sandbox: assertValidEnvKey + reserved check
Sandbox->>Provision: getApp(client, appId)
MC-->>Provision: App (with environmentVariables array)
Provision-->>Sandbox: app
Sandbox->>Provision: extractEnv(app) flat map
Sandbox->>Provision: setContainerEnv(client, appId, containerId, merged)
Provision->>MC: "PUT /apps/{appId}/containers/{containerId}/env (flat map body)"
MC-->>Provision: success/error
Provision-->>Sandbox: void or SandboxError
note over CLI,MC: sandbox exec name --env KEY=VAL -- cmd
CLI->>envArgs: collectEnv(entries, envFile)
envArgs-->>CLI: "Record<string,string>"
CLI->>CLI: envPrefix(vars)
CLI->>CLI: ssh remote cmd with inline prefix
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CLI
participant envArgs as env-args.ts
participant Sandbox as sandbox.ts
participant Provision as provision.ts
participant MC as MC API
note over CLI,MC: sandbox create --env KEY=VAL --env-file .env
CLI->>envArgs: collectEnv(entries, envFile)
envArgs-->>CLI: "Record<string,string>"
CLI->>Sandbox: "Sandbox.create({ env })"
Sandbox->>Sandbox: assertValidEnvKey + reserved check
Sandbox->>Provision: "createApp(client, { env })"
Provision->>MC: POST /apps (environmentVariables array)
MC-->>Provision: appId
Provision-->>Sandbox: appId
Sandbox-->>CLI: Sandbox instance
note over CLI,MC: sandbox env set name KEY=VAL
CLI->>Sandbox: sandbox.setEnv(vars)
Sandbox->>Sandbox: assertValidEnvKey + reserved check
Sandbox->>Provision: getApp(client, appId)
MC-->>Provision: App (with environmentVariables array)
Provision-->>Sandbox: app
Sandbox->>Provision: extractEnv(app) flat map
Sandbox->>Provision: setContainerEnv(client, appId, containerId, merged)
Provision->>MC: "PUT /apps/{appId}/containers/{containerId}/env (flat map body)"
MC-->>Provision: success/error
Provision-->>Sandbox: void or SandboxError
note over CLI,MC: sandbox exec name --env KEY=VAL -- cmd
CLI->>envArgs: collectEnv(entries, envFile)
envArgs-->>CLI: "Record<string,string>"
CLI->>CLI: envPrefix(vars)
CLI->>CLI: ssh remote cmd with inline prefix
Reviews (2): Last reviewed commit: "style(sandbox): organize imports and for..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: a2be486e9e
ℹ️ 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".
| export function withEnvOptions<T>(yargs: Argv<T>): Argv<T> { | ||
| return yargs | ||
| .option("env", { | ||
| alias: "e", |
There was a problem hiding this comment.
Because sandbox exec registers the shared env option with alias -e, yargs treats -e after the sandbox name as a configured CLI option rather than part of the variadic remote command; unknown-options-as-args only preserves options that are not configured. A common command like bunny sandbox exec s node -e 'console.log(1)' now has -e consumed as --env, so collectEnv tries to parse the JS snippet as KEY=VALUE or the remote command loses its flag. Keep the short alias off commands that forward arbitrary argv, or require env parsing only before an explicit delimiter.
Useful? React with 👍 / 👎.
| if (output === "json") { | ||
| logger.log(JSON.stringify({ removed, missing }, null, 2)); | ||
| return; |
There was a problem hiding this comment.
In --output json, this returns success before the existing text-mode check for removed.length === 0. When every requested key is absent, e.g. sandbox env delete s MISSING -o json, scripts receive exit 0 with { removed: [], missing: [...] } even though no variable was removed and the non-JSON path treats the same case as a UserError. Move the zero-removed check before the JSON output so exit semantics match.
Useful? React with 👍 / 👎.
| onDebug: (msg) => logger.debug(msg, true), | ||
| name: sandboxName, | ||
| region, | ||
| env: envVars, |
There was a problem hiding this comment.
The new create-time env forwarding allows --env AGENT_TOKEN=... to reach sandbox provisioning, while the SDK generates its own agent token for SSH and later setEnv explicitly reserves AGENT_TOKEN. Passing it at creation sends a duplicate or overriding AGENT_TOKEN to Magic Containers, which can leave the container using a different SSH token than the one saved locally. Apply the same reserved-key check before forwarding create env vars.
Useful? React with 👍 / 👎.
| process.exitCode = await withSshEnv(record, async (env) => { | ||
| const proc = Bun.spawn( | ||
| sshArgs(record, `cd ${WORKPLACE} && exec bash -l`, { tty: true }), | ||
| sshArgs(record, `cd ${WORKPLACE} && ${prefix}exec bash -l`, { |
There was a problem hiding this comment.
When a user supplies a temporary PATH, this builds a remote command like PATH='...' exec bash -l; the remote shell applies that assignment before the exec builtin resolves bash, so bunny sandbox ssh s -e PATH=/custom can fail to start the session with bash: not found instead of opening a shell with the requested environment. Use an absolute /bin/bash (or apply the env after the shell starts) so the session bootstrap is not affected by the injected PATH.
Useful? React with 👍 / 👎.
| } | ||
| const app = await getApp(this.client, this.appId); | ||
| const env = extractEnv(app); | ||
| const removed = keys.filter((key) => key in env); |
There was a problem hiding this comment.
For a delete request whose missing key is inherited from Object.prototype (for example constructor or toString), key in env is true even though the sandbox variable is not set. That makes unsetEnv report the variable as removed and call the full environment replacement endpoint, unnecessarily redeploying/restarting the sandbox. Use Object.hasOwn(env, key) or a null-prototype map for this membership test.
Useful? React with 👍 / 👎.
| if (eq === -1) continue; | ||
| const key = body.slice(0, eq).trim(); | ||
| assertValidKey(key); | ||
| let value = body.slice(eq + 1).trim(); |
There was a problem hiding this comment.
When a dotenv file uses a standard unquoted inline comment such as PORT=8080 # web server, this parser keeps the comment text in the value and sends 8080 # web server to sandbox create, sandbox exec, or sandbox env set. That can break sandboxes that reuse existing .env files with annotated values; strip inline comments for unquoted values before storing them.
Useful? React with 👍 / 👎.
…jection breaking shell lookup
|
@greptile-apps review |
3 participants
No description provided.