Add Vercel sandbox provider#700
Open
ColeMurray wants to merge 6 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis pull request adds comprehensive Vercel Sandbox support as an alternative to Modal and Daytona backends. The implementation includes a REST client for the Vercel API, runtime bootstrap tooling, base snapshot management, a full SandboxProvider implementation, and integration into the control plane session lifecycle and repo-image routing, plus Terraform automation and documentation. ChangesVercel Sandbox Provider Implementation
Terraform and CI/CD Automation
Documentation and Frontend Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/control-plane/src/routes/repo-images.ts (1)
84-112:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAuthenticate build callbacks before updating D1.
The new Vercel coordinator signs callbacks with
Authorization: Bearer <timestamp>.<hmac>, but neither handler validates that header. Anyone who can guess abuild_idcan mark builds ready/failed, and the success path can cascade into deleting a provider image. Verify the HMAC and timestamp before mutating build state.Also applies to: 176-195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/routes/repo-images.ts` around lines 84 - 112, Extract and validate the Authorization header in both handleBuildComplete and the corresponding failure handler (handleBuildFailed) before making any DB mutations: read the raw request body (before parseJsonBody), parse Authorization of form "Bearer <timestamp>.<hmac>", ensure timestamp is within an allowed window (e.g., ±5 minutes), compute an HMAC using the shared coordinator secret from env (e.g., env.COORDINATOR_SECRET) over the same data the coordinator signs (timestamp + "." + rawBody or timestamp + "." + build_id depending on coordinator spec), compare the computed HMAC to the provided one using a constant-time comparison, and return 401 on missing/invalid signature or expired timestamp; only proceed to parseJsonBody and update D1 when validation succeeds.
🧹 Nitpick comments (1)
packages/control-plane/src/sandbox/providers/vercel-provider.ts (1)
477-483: ⚡ Quick winHoist these default command timeouts into named constants.
The new timeout defaults are being restated inline instead of being defined once and reused, which makes drift between provider commands more likely as this file grows.
As per coding guidelines, "Define each default timeout value exactly once as a named constant and import it everywhere rather than restating literal values" and "Do not use bare timeout variable names; always encode the unit (seconds for Python, milliseconds for TypeScript) in the variable name."
Also applies to: 496-501
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/sandbox/providers/vercel-provider.ts` around lines 477 - 483, Hoist the literal timeout values used in calls like this.client.runCommandAndWait({ sessionId, command: "sudo", args: [...], timeoutMs: 30_000 }) (and the similar call at lines ~496-501) into a named, exported constant (e.g. VERCEL_PROVIDER_COMMAND_TIMEOUT_MS) and import/use that constant instead of the magic number; follow the unit-encoding rule in the constant name (use _MS for TypeScript timeouts). Update all runCommandAndWait invocations in vercel-provider.ts to reference the new constant and remove the inline numeric literals so the default timeout is defined exactly once and reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/terraform.yml:
- Around line 312-348: Update the error message in the "Build Vercel base
snapshot" step so it references the actual env var names used elsewhere
(VERCEL_TOKEN and VERCEL_PROJECT_ID) instead of VERCEL_SANDBOX_TOKEN and
VERCEL_SANDBOX_PROJECT_ID; modify the check block that currently echoes
"VERCEL_SANDBOX_TOKEN and VERCEL_SANDBOX_PROJECT_ID are required when
SANDBOX_PROVIDER=vercel" to echo the correct variable names (VERCEL_TOKEN and
VERCEL_PROJECT_ID) so it matches the env keys set and the build script's
requiredEnv calls.
In `@docs/VERCEL_SANDBOX_PROVIDER.md`:
- Line 25: Update the phrasing in the sentence "For GitHub Actions based
deployment, configure the matching repository secrets:" to hyphenate the
compound modifier; change it to "For GitHub Actions-based deployment, configure
the matching repository secrets:" so the compound adjective "GitHub
Actions-based" is clear and grammatically correct.
In `@packages/control-plane/scripts/build-vercel-base-snapshot.ts`:
- Around line 18-25: The script currently allows a missing or invalid --output
value to silently fall back to stdout; modify getArgValue/main so that when
"--output" is present but no following value exists or the following token
starts with "-" (another flag), the script fails fast: detect this condition
after calling getArgValue("--output") in main (or enhance getArgValue to
validate the next token), print a clear error to stderr (including the fact that
--output is missing/invalid) and exit with a non-zero status (e.g., throw or
process.exit(1)) so CI fails instead of continuing.
In `@packages/control-plane/src/sandbox/providers/vercel-provider.ts`:
- Around line 477-485: The remote tunnel-writer invocation using
this.client.runCommandAndWait(...) (with sessionId, VERCEL_PYTHON_BIN and the
script that calls Path.write_text(...)) currently assumes failures will throw;
instead inspect the result returned by runCommandAndWait and fail the launch on
non-zero exit. Modify the caller around runCommandAndWait to capture its return
value (exitCode / status / stdout/stderr fields), check for a non-zero exit code
or error status, log the remote stderr with context, and throw or return an
error so sandbox launch stops when Path.write_text(...) fails.
- Around line 705-721: The happy-path for event "image_build.complete" snapshots
and calls callback but never shuts down the running sandbox, leaving proc alive
until BUILD_TIMEOUT_SECONDS; after snapshot_session() and before returning,
explicitly stop the session by terminating proc (use
proc.send_signal(signal.SIGTERM) then wait with a short timeout and fall back to
proc.kill() and wait) or call the provider's existing cleanup API (e.g.,
stopSandbox/stopSession) to release resources, and ensure any teardown succeeds
before returning from the handler.
- Around line 309-322: The PATH is hard-coded to /vercel/runtimes/node24/bin in
the env construction (Object.assign block) causing mismatched runtime when
this.providerConfig.runtime (or VERCEL_RUNTIME) differs; update the env builders
that set PATH (the block that assigns HOME, NODE_ENV, PATH, etc., and the
similar block around lines 356-377) to interpolate the configured runtime
instead of node24—use this.providerConfig.runtime (or
config.runtime/VERCEL_RUNTIME) to build the runtimePath (e.g.,
`/vercel/runtimes/${runtime}/bin`) and use that value for PATH so the sandbox
runtime and PATH remain consistent.
- Around line 366-376: The sandbox-wide env currently exposes sensitive
Vercel/control-plane secrets (OI_INTERNAL_CALLBACK_SECRET, OI_VERCEL_TOKEN) via
buildBuildEnvVars(), making them visible to repo builds run by
buildCoordinatorScript(); remove these keys from the sandbox-level environment
and instead inject them only into the coordinator/process that performs remote
API callbacks (e.g., pass secrets into the coordinator runtime or a dedicated
callback handler) so repo build env (env=os.environ.copy() used in
buildCoordinatorScript()) never contains OI_INTERNAL_CALLBACK_SECRET or
OI_VERCEL_TOKEN; update buildBuildEnvVars() to omit those two keys (and the same
for the other occurrences called out), and wire secret access into the specific
functions that perform callback/API calls rather than the repo build
environment.
In `@packages/control-plane/src/sandbox/vercel-bootstrap.ts`:
- Line 73: The generated git credential helper script incorrectly references the
shell variable ${VERCEL_PYTHON_BIN} (a build-time TypeScript constant) which is
not defined at runtime; update the script generation so it embeds the actual
Python binary path value instead of the shell variable. Specifically, change the
string written to /usr/local/bin/oi-git-credentials in the code that constructs
that script so the command uses the resolved VERCEL_PYTHON_BIN constant (the
build-time value) and still calls -m
sandbox_runtime.credentials.git_credential_helper "$@" — ensure the resulting
script contains the hardcoded path rather than a ${...} shell variable
reference.
In `@packages/control-plane/src/sandbox/vercel-client.ts`:
- Around line 236-274: The requestText function currently buffers the entire
response body (in requestText) which causes memory exhaustion for NDJSON command
streams; change requestText to consume response.body as a stream (use
response.body.getReader() or a streaming text/NDJSON parser) and process chunks
incrementally, splitting on newlines, parsing each JSON envelope as it arrives,
retaining only the most recent envelope with envelope.type === "command" (or the
"command" field) and discarding prior ones, and if response.ok is false still
read/skips the streamed body then throw VercelSandboxApiError with the captured
status and last error text; ensure headers and error handling logic around
VercelSandboxApiError and httpStatus remain unchanged.
In `@packages/control-plane/src/session/durable-object.ts`:
- Around line 764-766: RepoImageLookup currently only stores provider_image_id
and base_sha which lets SandboxLifecycleManager call the wrong provider after
SANDBOX_PROVIDER changes; update persistence and lookup logic so RepoImageLookup
includes the backend/provider identifier (e.g., provider_name or backend enum)
and ensure code paths that populate or read repoImageLookup (the block guarded
by supportsRepoImageBackend and any callers in SandboxLifecycleManager) filter
by that backend or delete incompatible rows before using the opaque
provider_image_id; also add migration/cleanup logic when reading existing rows
to clear or ignore entries whose stored backend does not match the active
SANDBOX_PROVIDER.
---
Outside diff comments:
In `@packages/control-plane/src/routes/repo-images.ts`:
- Around line 84-112: Extract and validate the Authorization header in both
handleBuildComplete and the corresponding failure handler (handleBuildFailed)
before making any DB mutations: read the raw request body (before
parseJsonBody), parse Authorization of form "Bearer <timestamp>.<hmac>", ensure
timestamp is within an allowed window (e.g., ±5 minutes), compute an HMAC using
the shared coordinator secret from env (e.g., env.COORDINATOR_SECRET) over the
same data the coordinator signs (timestamp + "." + rawBody or timestamp + "." +
build_id depending on coordinator spec), compare the computed HMAC to the
provided one using a constant-time comparison, and return 401 on missing/invalid
signature or expired timestamp; only proceed to parseJsonBody and update D1 when
validation succeeds.
---
Nitpick comments:
In `@packages/control-plane/src/sandbox/providers/vercel-provider.ts`:
- Around line 477-483: Hoist the literal timeout values used in calls like
this.client.runCommandAndWait({ sessionId, command: "sudo", args: [...],
timeoutMs: 30_000 }) (and the similar call at lines ~496-501) into a named,
exported constant (e.g. VERCEL_PROVIDER_COMMAND_TIMEOUT_MS) and import/use that
constant instead of the magic number; follow the unit-encoding rule in the
constant name (use _MS for TypeScript timeouts). Update all runCommandAndWait
invocations in vercel-provider.ts to reference the new constant and remove the
inline numeric literals so the default timeout is defined exactly once and
reused.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7baa04ca-d251-4aad-84cc-602580fa6bf8
📒 Files selected for processing (35)
.github/workflows/terraform.ymldocs/GETTING_STARTED.mddocs/HOW_IT_WORKS.mddocs/IMAGE_PREBUILD.mddocs/SECRETS.mddocs/VERCEL_SANDBOX_PROVIDER.mdpackages/control-plane/package.jsonpackages/control-plane/scripts/build-vercel-base-snapshot.tspackages/control-plane/src/routes/repo-images.tspackages/control-plane/src/sandbox/index.tspackages/control-plane/src/sandbox/lifecycle/manager.test.tspackages/control-plane/src/sandbox/lifecycle/manager.tspackages/control-plane/src/sandbox/provider-name.test.tspackages/control-plane/src/sandbox/provider-name.tspackages/control-plane/src/sandbox/providers/vercel-provider.test.tspackages/control-plane/src/sandbox/providers/vercel-provider.tspackages/control-plane/src/sandbox/vercel-base-snapshot.test.tspackages/control-plane/src/sandbox/vercel-base-snapshot.tspackages/control-plane/src/sandbox/vercel-bootstrap.tspackages/control-plane/src/sandbox/vercel-client.test.tspackages/control-plane/src/sandbox/vercel-client.tspackages/control-plane/src/session/durable-object.tspackages/control-plane/src/types.tspackages/web/src/app/api/repo-images/[owner]/[name]/toggle/route.tspackages/web/src/app/api/repo-images/[owner]/[name]/trigger/route.tspackages/web/src/app/api/repo-images/route.tspackages/web/src/components/settings/images-settings.tsxpackages/web/src/lib/sandbox-provider.test.tspackages/web/src/lib/sandbox-provider.tsterraform/README.mdterraform/environments/production/locals.tfterraform/environments/production/outputs.tfterraform/environments/production/terraform.tfvars.exampleterraform/environments/production/variables.tfterraform/environments/production/workers-control-plane.tf
Comment on lines
+312
to
+348
| - name: Build Vercel base snapshot | ||
| id: vercel_base_snapshot | ||
| run: | | ||
| if [ "${SANDBOX_PROVIDER}" != "vercel" ]; then | ||
| echo "snapshot_id=${VERCEL_BASE_SNAPSHOT_ID:-}" >> "$GITHUB_OUTPUT" | ||
| echo "Skipping Vercel base snapshot build because SANDBOX_PROVIDER=${SANDBOX_PROVIDER}" | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [ -z "${VERCEL_TOKEN:-}" ] || [ -z "${VERCEL_PROJECT_ID:-}" ]; then | ||
| echo "VERCEL_SANDBOX_TOKEN and VERCEL_SANDBOX_PROJECT_ID are required when SANDBOX_PROVIDER=vercel" | ||
| exit 1 | ||
| fi | ||
|
|
||
| npm run build:vercel-base-snapshot -w @open-inspect/control-plane | ||
|
|
||
| output_file="${RUNNER_TEMP}/vercel-base-snapshot-id" | ||
| node packages/control-plane/dist/vercel-base-snapshot.js --output "$output_file" | ||
| snapshot_id="$(tr -d '\r\n' < "$output_file")" | ||
|
|
||
| if [ -z "$snapshot_id" ]; then | ||
| echo "Vercel base snapshot builder did not write a snapshot ID" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Built Vercel base snapshot: $snapshot_id" | ||
| echo "snapshot_id=$snapshot_id" >> "$GITHUB_OUTPUT" | ||
| env: | ||
| SANDBOX_PROVIDER: "${{ secrets.SANDBOX_PROVIDER || 'modal' }}" | ||
| VERCEL_BASE_SNAPSHOT_ID: ${{ secrets.VERCEL_BASE_SNAPSHOT_ID }} | ||
| VERCEL_TOKEN: ${{ secrets.VERCEL_SANDBOX_TOKEN }} | ||
| VERCEL_PROJECT_ID: ${{ secrets.VERCEL_SANDBOX_PROJECT_ID }} | ||
| VERCEL_TEAM_ID: ${{ secrets.VERCEL_SANDBOX_TEAM_ID }} | ||
| VERCEL_RUNTIME: "${{ secrets.VERCEL_SANDBOX_RUNTIME || 'node24' }}" | ||
| VERCEL_RUNTIME_REPO_URL: "${{ secrets.VERCEL_RUNTIME_REPO_URL || 'https://github.com/ColeMurray/background-agents.git' }}" | ||
| VERCEL_RUNTIME_REPO_REF: "${{ secrets.VERCEL_RUNTIME_REPO_REF || 'main' }}" | ||
| VERCEL_SANDBOX_API_BASE_URL: ${{ secrets.VERCEL_SANDBOX_API_BASE_URL }} |
There was a problem hiding this comment.
Fix error message to use correct environment variable names.
The error message at line 322 references VERCEL_SANDBOX_TOKEN and VERCEL_SANDBOX_PROJECT_ID, but the actual environment variables set at lines 342-343 are VERCEL_TOKEN and VERCEL_PROJECT_ID (without the SANDBOX prefix). This matches the naming expected by the build script (context snippet 1 shows requiredEnv("VERCEL_TOKEN") and requiredEnv("VERCEL_PROJECT_ID")).
📝 Proposed fix
- if [ -z "${VERCEL_TOKEN:-}" ] || [ -z "${VERCEL_PROJECT_ID:-}" ]; then
- echo "VERCEL_SANDBOX_TOKEN and VERCEL_SANDBOX_PROJECT_ID are required when SANDBOX_PROVIDER=vercel"
+ if [ -z "${VERCEL_TOKEN:-}" ] || [ -z "${VERCEL_PROJECT_ID:-}" ]; then
+ echo "VERCEL_TOKEN and VERCEL_PROJECT_ID are required when SANDBOX_PROVIDER=vercel"
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Build Vercel base snapshot | |
| id: vercel_base_snapshot | |
| run: | | |
| if [ "${SANDBOX_PROVIDER}" != "vercel" ]; then | |
| echo "snapshot_id=${VERCEL_BASE_SNAPSHOT_ID:-}" >> "$GITHUB_OUTPUT" | |
| echo "Skipping Vercel base snapshot build because SANDBOX_PROVIDER=${SANDBOX_PROVIDER}" | |
| exit 0 | |
| fi | |
| if [ -z "${VERCEL_TOKEN:-}" ] || [ -z "${VERCEL_PROJECT_ID:-}" ]; then | |
| echo "VERCEL_SANDBOX_TOKEN and VERCEL_SANDBOX_PROJECT_ID are required when SANDBOX_PROVIDER=vercel" | |
| exit 1 | |
| fi | |
| npm run build:vercel-base-snapshot -w @open-inspect/control-plane | |
| output_file="${RUNNER_TEMP}/vercel-base-snapshot-id" | |
| node packages/control-plane/dist/vercel-base-snapshot.js --output "$output_file" | |
| snapshot_id="$(tr -d '\r\n' < "$output_file")" | |
| if [ -z "$snapshot_id" ]; then | |
| echo "Vercel base snapshot builder did not write a snapshot ID" | |
| exit 1 | |
| fi | |
| echo "Built Vercel base snapshot: $snapshot_id" | |
| echo "snapshot_id=$snapshot_id" >> "$GITHUB_OUTPUT" | |
| env: | |
| SANDBOX_PROVIDER: "${{ secrets.SANDBOX_PROVIDER || 'modal' }}" | |
| VERCEL_BASE_SNAPSHOT_ID: ${{ secrets.VERCEL_BASE_SNAPSHOT_ID }} | |
| VERCEL_TOKEN: ${{ secrets.VERCEL_SANDBOX_TOKEN }} | |
| VERCEL_PROJECT_ID: ${{ secrets.VERCEL_SANDBOX_PROJECT_ID }} | |
| VERCEL_TEAM_ID: ${{ secrets.VERCEL_SANDBOX_TEAM_ID }} | |
| VERCEL_RUNTIME: "${{ secrets.VERCEL_SANDBOX_RUNTIME || 'node24' }}" | |
| VERCEL_RUNTIME_REPO_URL: "${{ secrets.VERCEL_RUNTIME_REPO_URL || 'https://github.com/ColeMurray/background-agents.git' }}" | |
| VERCEL_RUNTIME_REPO_REF: "${{ secrets.VERCEL_RUNTIME_REPO_REF || 'main' }}" | |
| VERCEL_SANDBOX_API_BASE_URL: ${{ secrets.VERCEL_SANDBOX_API_BASE_URL }} | |
| - name: Build Vercel base snapshot | |
| id: vercel_base_snapshot | |
| run: | | |
| if [ "${SANDBOX_PROVIDER}" != "vercel" ]; then | |
| echo "snapshot_id=${VERCEL_BASE_SNAPSHOT_ID:-}" >> "$GITHUB_OUTPUT" | |
| echo "Skipping Vercel base snapshot build because SANDBOX_PROVIDER=${SANDBOX_PROVIDER}" | |
| exit 0 | |
| fi | |
| if [ -z "${VERCEL_TOKEN:-}" ] || [ -z "${VERCEL_PROJECT_ID:-}" ]; then | |
| echo "VERCEL_TOKEN and VERCEL_PROJECT_ID are required when SANDBOX_PROVIDER=vercel" | |
| exit 1 | |
| fi | |
| npm run build:vercel-base-snapshot -w `@open-inspect/control-plane` | |
| output_file="${RUNNER_TEMP}/vercel-base-snapshot-id" | |
| node packages/control-plane/dist/vercel-base-snapshot.js --output "$output_file" | |
| snapshot_id="$(tr -d '\r\n' < "$output_file")" | |
| if [ -z "$snapshot_id" ]; then | |
| echo "Vercel base snapshot builder did not write a snapshot ID" | |
| exit 1 | |
| fi | |
| echo "Built Vercel base snapshot: $snapshot_id" | |
| echo "snapshot_id=$snapshot_id" >> "$GITHUB_OUTPUT" | |
| env: | |
| SANDBOX_PROVIDER: "${{ secrets.SANDBOX_PROVIDER || 'modal' }}" | |
| VERCEL_BASE_SNAPSHOT_ID: ${{ secrets.VERCEL_BASE_SNAPSHOT_ID }} | |
| VERCEL_TOKEN: ${{ secrets.VERCEL_SANDBOX_TOKEN }} | |
| VERCEL_PROJECT_ID: ${{ secrets.VERCEL_SANDBOX_PROJECT_ID }} | |
| VERCEL_TEAM_ID: ${{ secrets.VERCEL_SANDBOX_TEAM_ID }} | |
| VERCEL_RUNTIME: "${{ secrets.VERCEL_SANDBOX_RUNTIME || 'node24' }}" | |
| VERCEL_RUNTIME_REPO_URL: "${{ secrets.VERCEL_RUNTIME_REPO_URL || 'https://github.com/ColeMurray/background-agents.git' }}" | |
| VERCEL_RUNTIME_REPO_REF: "${{ secrets.VERCEL_RUNTIME_REPO_REF || 'main' }}" | |
| VERCEL_SANDBOX_API_BASE_URL: ${{ secrets.VERCEL_SANDBOX_API_BASE_URL }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/terraform.yml around lines 312 - 348, Update the error
message in the "Build Vercel base snapshot" step so it references the actual env
var names used elsewhere (VERCEL_TOKEN and VERCEL_PROJECT_ID) instead of
VERCEL_SANDBOX_TOKEN and VERCEL_SANDBOX_PROJECT_ID; modify the check block that
currently echoes "VERCEL_SANDBOX_TOKEN and VERCEL_SANDBOX_PROJECT_ID are
required when SANDBOX_PROVIDER=vercel" to echo the correct variable names
(VERCEL_TOKEN and VERCEL_PROJECT_ID) so it matches the env keys set and the
build script's requiredEnv calls.
| # vercel_sandbox_team_id = "team_..." # optional for team projects | ||
| ``` | ||
|
|
||
| For GitHub Actions based deployment, configure the matching repository secrets: |
There was a problem hiding this comment.
Hyphenate compound modifier for clarity.
Use “GitHub Actions-based deployment” instead of “GitHub Actions based deployment.”
🧰 Tools
🪛 LanguageTool
[grammar] ~25-~25: Use a hyphen to join words.
Context: ...or team projects ``` For GitHub Actions based deployment, configure the matching...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/VERCEL_SANDBOX_PROVIDER.md` at line 25, Update the phrasing in the
sentence "For GitHub Actions based deployment, configure the matching repository
secrets:" to hyphenate the compound modifier; change it to "For GitHub
Actions-based deployment, configure the matching repository secrets:" so the
compound adjective "GitHub Actions-based" is clear and grammatically correct.
Comment on lines
+18
to
+25
| function getArgValue(name: string): string | undefined { | ||
| const index = process.argv.indexOf(name); | ||
| if (index === -1) return undefined; | ||
| return process.argv[index + 1]; | ||
| } | ||
|
|
||
| async function main(): Promise<void> { | ||
| const outputPath = getArgValue("--output"); |
There was a problem hiding this comment.
Treat a missing --output value as an error.
If --output is the last token, getArgValue() returns undefined and the script silently falls back to stdout; if the next token is another flag, it will try to use that flag name as a path. Fail fast here so CI does not report success without writing the requested file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/scripts/build-vercel-base-snapshot.ts` around lines 18
- 25, The script currently allows a missing or invalid --output value to
silently fall back to stdout; modify getArgValue/main so that when "--output" is
present but no following value exists or the following token starts with "-"
(another flag), the script fails fast: detect this condition after calling
getArgValue("--output") in main (or enhance getArgValue to validate the next
token), print a clear error to stderr (including the fact that --output is
missing/invalid) and exit with a non-zero status (e.g., throw or
process.exit(1)) so CI fails instead of continuing.
Comment on lines
+309
to
+322
| Object.assign(envVars, { | ||
| HOME: "/root", | ||
| NODE_ENV: "development", | ||
| PATH: "/root/.bun/bin:/usr/local/bin:/usr/bin:/bin:/vercel/runtimes/node24/bin", | ||
| PYTHONPATH: "/app", | ||
| PYTHONUNBUFFERED: "1", | ||
| NODE_PATH: "/usr/lib/node_modules:/usr/local/lib/node_modules", | ||
| SANDBOX_ID: config.sandboxId, | ||
| CONTROL_PLANE_URL: config.controlPlaneUrl, | ||
| SANDBOX_AUTH_TOKEN: config.sandboxAuthToken, | ||
| REPO_OWNER: config.repoOwner, | ||
| REPO_NAME: config.repoName, | ||
| SESSION_CONFIG: JSON.stringify(sessionConfig), | ||
| }); |
There was a problem hiding this comment.
Use the configured runtime when composing PATH.
createSandbox() honors this.providerConfig.runtime, but both env builders hard-code /vercel/runtimes/node24/bin. Setting VERCEL_RUNTIME to anything else creates a sandbox with one runtime and a PATH pointing at another or nonexistent runtime directory.
Also applies to: 356-377
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/sandbox/providers/vercel-provider.ts` around lines
309 - 322, The PATH is hard-coded to /vercel/runtimes/node24/bin in the env
construction (Object.assign block) causing mismatched runtime when
this.providerConfig.runtime (or VERCEL_RUNTIME) differs; update the env builders
that set PATH (the block that assigns HOME, NODE_ENV, PATH, etc., and the
similar block around lines 356-377) to interpolate the configured runtime
instead of node24—use this.providerConfig.runtime (or
config.runtime/VERCEL_RUNTIME) to build the runtimePath (e.g.,
`/vercel/runtimes/${runtime}/bin`) and use that value for PATH so the sandbox
runtime and PATH remain consistent.
Comment on lines
+366
to
+376
| IMAGE_BUILD_MODE: "true", | ||
| SESSION_CONFIG: JSON.stringify({ branch: config.defaultBranch }), | ||
| OI_VERCEL_BUILD_ID: config.buildId, | ||
| OI_VERCEL_CALLBACK_URL: config.callbackUrl, | ||
| OI_INTERNAL_CALLBACK_SECRET: this.providerConfig.internalCallbackSecret ?? "", | ||
| OI_VERCEL_TOKEN: this.providerConfig.token, | ||
| OI_VERCEL_TEAM_ID: this.providerConfig.teamId ?? "", | ||
| OI_VERCEL_API_BASE_URL: this.providerConfig.apiBaseUrl ?? "https://vercel.com/api", | ||
| OI_VERCEL_SNAPSHOT_EXPIRATION_MS: String( | ||
| this.providerConfig.snapshotExpirationMs ?? DEFAULT_SNAPSHOT_EXPIRATION_MS | ||
| ), |
There was a problem hiding this comment.
Do not put Vercel/control-plane credentials in the sandbox-wide env.
buildBuildEnvVars() injects OI_INTERNAL_CALLBACK_SECRET and OI_VERCEL_TOKEN into the sandbox itself, and buildCoordinatorScript() runs the repo build with env=os.environ.copy(). Any repo code executed during the build can read/exfiltrate those values, which lets an untrusted repo forge build callbacks and call the Vercel API outside the intended flow.
Also applies to: 684-691
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/sandbox/providers/vercel-provider.ts` around lines
366 - 376, The sandbox-wide env currently exposes sensitive Vercel/control-plane
secrets (OI_INTERNAL_CALLBACK_SECRET, OI_VERCEL_TOKEN) via buildBuildEnvVars(),
making them visible to repo builds run by buildCoordinatorScript(); remove these
keys from the sandbox-level environment and instead inject them only into the
coordinator/process that performs remote API callbacks (e.g., pass secrets into
the coordinator runtime or a dedicated callback handler) so repo build env
(env=os.environ.copy() used in buildCoordinatorScript()) never contains
OI_INTERNAL_CALLBACK_SECRET or OI_VERCEL_TOKEN; update buildBuildEnvVars() to
omit those two keys (and the same for the other occurrences called out), and
wire secret access into the specific functions that perform callback/API calls
rather than the repo build environment.
Comment on lines
+477
to
+485
| await this.client.runCommandAndWait( | ||
| { | ||
| sessionId, | ||
| command: "sudo", | ||
| args: ["-E", VERCEL_PYTHON_BIN, "-c", script], | ||
| timeoutMs: 30_000, | ||
| }, | ||
| correlation | ||
| ); |
There was a problem hiding this comment.
Check the tunnel writer command's exit code.
runCommandAndWait() only throws on transport/parsing failures; a non-zero remote exit still resolves. If the Path.write_text(...) command fails here, sandbox launch continues with missing .tunnels.env state and no surfaced error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/sandbox/providers/vercel-provider.ts` around lines
477 - 485, The remote tunnel-writer invocation using
this.client.runCommandAndWait(...) (with sessionId, VERCEL_PYTHON_BIN and the
script that calls Path.write_text(...)) currently assumes failures will throw;
instead inspect the result returned by runCommandAndWait and fail the launch on
non-zero exit. Modify the caller around runCommandAndWait to capture its return
value (exitCode / status / stdout/stderr fields), check for a non-zero exit code
or error status, log the remote stderr with context, and throw or return an
error so sandbox launch stops when Path.write_text(...) fails.
Comment on lines
+705
to
+721
| elif event == "image_build.complete": | ||
| if not head_sha: | ||
| head_sha = read_head_sha() | ||
| proc.send_signal(signal.SIGTERM) | ||
| try: | ||
| proc.wait(timeout=20) | ||
| except subprocess.TimeoutExpired: | ||
| proc.kill() | ||
| proc.wait(timeout=10) | ||
| snapshot_id = snapshot_session() | ||
| callback("", { | ||
| "build_id": build_id, | ||
| "provider_image_id": snapshot_id, | ||
| "base_sha": head_sha, | ||
| "build_duration_seconds": round(time.time() - started, 3), | ||
| }) | ||
| return |
There was a problem hiding this comment.
Successful repo-image builds never explicitly stop the Vercel session.
The happy path snapshots the session, posts the callback, and returns immediately. That leaves each successful build sandbox alive until BUILD_TIMEOUT_SECONDS expires, which burns quota/capacity on every build instead of releasing it as soon as the image is ready.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/sandbox/providers/vercel-provider.ts` around lines
705 - 721, The happy-path for event "image_build.complete" snapshots and calls
callback but never shuts down the running sandbox, leaving proc alive until
BUILD_TIMEOUT_SECONDS; after snapshot_session() and before returning, explicitly
stop the session by terminating proc (use proc.send_signal(signal.SIGTERM) then
wait with a short timeout and fall back to proc.kill() and wait) or call the
provider's existing cleanup API (e.g., stopSandbox/stopSession) to release
resources, and ensure any teardown succeeds before returning from the handler.
| sudo chmod -R a+rX /app/sandbox_runtime | ||
| sudo ${VERCEL_PYTHON_BIN} -m pip install --break-system-packages -e packages/sandbox-runtime || sudo ${VERCEL_PYTHON_BIN} -m pip install -e packages/sandbox-runtime | ||
|
|
||
| printf '%s\\n' '#!/bin/sh' 'exec ${VERCEL_PYTHON_BIN} -m sandbox_runtime.credentials.git_credential_helper "$@"' | sudo tee /usr/local/bin/oi-git-credentials >/dev/null |
There was a problem hiding this comment.
Undefined shell variable in generated git credential helper.
Line 73 generates a shell script that references ${VERCEL_PYTHON_BIN}, but this is not defined as a shell environment variable in the generated bootstrap script. The TypeScript constant VERCEL_PYTHON_BIN is only available at build-time, not in the runtime shell environment. When the git credential helper is invoked, the shell will expand ${VERCEL_PYTHON_BIN} to an empty string, causing the Python command to fail.
🐛 Proposed fix
Replace the shell variable reference with the actual hardcoded path:
-printf '%s\\n' '#!/bin/sh' 'exec ${VERCEL_PYTHON_BIN} -m sandbox_runtime.credentials.git_credential_helper "$@"' | sudo tee /usr/local/bin/oi-git-credentials >/dev/null
+printf '%s\\n' '#!/bin/sh' 'exec /usr/bin/python3.12 -m sandbox_runtime.credentials.git_credential_helper "$@"' | sudo tee /usr/local/bin/oi-git-credentials >/dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| printf '%s\\n' '#!/bin/sh' 'exec ${VERCEL_PYTHON_BIN} -m sandbox_runtime.credentials.git_credential_helper "$@"' | sudo tee /usr/local/bin/oi-git-credentials >/dev/null | |
| printf '%s\\n' '#!/bin/sh' 'exec /usr/bin/python3.12 -m sandbox_runtime.credentials.git_credential_helper "$@"' | sudo tee /usr/local/bin/oi-git-credentials >/dev/null |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/sandbox/vercel-bootstrap.ts` at line 73, The
generated git credential helper script incorrectly references the shell variable
${VERCEL_PYTHON_BIN} (a build-time TypeScript constant) which is not defined at
runtime; update the script generation so it embeds the actual Python binary path
value instead of the shell variable. Specifically, change the string written to
/usr/local/bin/oi-git-credentials in the code that constructs that script so the
command uses the resolved VERCEL_PYTHON_BIN constant (the build-time value) and
still calls -m sandbox_runtime.credentials.git_credential_helper "$@" — ensure
the resulting script contains the hardcoded path rather than a ${...} shell
variable reference.
Comment on lines
+236
to
+274
| private async requestText( | ||
| path: string, | ||
| init: RequestInit, | ||
| correlation: CorrelationContext | undefined, | ||
| endpoint: string | ||
| ): Promise<string> { | ||
| const startTime = Date.now(); | ||
| let httpStatus: number | undefined; | ||
| let outcome: "success" | "error" = "error"; | ||
|
|
||
| try { | ||
| const url = new URL(`${this.apiBaseUrl}${path}`); | ||
| if (this.config.teamId) { | ||
| url.searchParams.set("teamId", this.config.teamId); | ||
| } | ||
|
|
||
| const headers = new Headers(init.headers); | ||
| headers.set("Authorization", `Bearer ${this.config.token}`); | ||
| headers.set("Content-Type", headers.get("Content-Type") || "application/json"); | ||
| headers.set("User-Agent", USER_AGENT); | ||
| if (correlation?.trace_id) headers.set("x-trace-id", correlation.trace_id); | ||
| if (correlation?.request_id) headers.set("x-request-id", correlation.request_id); | ||
| if (correlation?.session_id) headers.set("x-session-id", correlation.session_id); | ||
| if (correlation?.sandbox_id) headers.set("x-sandbox-id", correlation.sandbox_id); | ||
|
|
||
| const response = await fetch(url.toString(), { ...init, headers }); | ||
| httpStatus = response.status; | ||
| const text = await response.text(); | ||
| if (!response.ok) { | ||
| throw new VercelSandboxApiError( | ||
| `Vercel Sandbox API error: ${response.status} ${text}`, | ||
| response.status, | ||
| text | ||
| ); | ||
| } | ||
|
|
||
| outcome = "success"; | ||
| return text; | ||
| } finally { |
There was a problem hiding this comment.
Stream waited command output instead of buffering the whole body.
requestText() reads the entire NDJSON stream into memory before parsing it. That path is already used by packages/control-plane/src/sandbox/providers/vercel-provider.ts during runtime bootstrap, so a verbose install can exhaust Worker memory and fail bootstrap/base-snapshot creation even when the remote command itself succeeds. Parse the response incrementally and keep only the latest command envelope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/sandbox/vercel-client.ts` around lines 236 - 274,
The requestText function currently buffers the entire response body (in
requestText) which causes memory exhaustion for NDJSON command streams; change
requestText to consume response.body as a stream (use response.body.getReader()
or a streaming text/NDJSON parser) and process chunks incrementally, splitting
on newlines, parsing each JSON envelope as it arrives, retaining only the most
recent envelope with envelope.type === "command" (or the "command" field) and
discarding prior ones, and if response.ok is false still read/skips the streamed
body then throw VercelSandboxApiError with the captured status and last error
text; ensure headers and error handling logic around VercelSandboxApiError and
httpStatus remain unchanged.
Comment on lines
+764
to
+766
| // Create repo image lookup if D1 is available and the provider supports repo images. | ||
| let repoImageLookup: RepoImageLookup | undefined; | ||
| if (this.env.DB && sandboxBackend === "modal") { | ||
| if (this.env.DB && supportsRepoImageBackend(sandboxBackend)) { |
There was a problem hiding this comment.
Persist repo-image backend provenance before broadening this lookup.
RepoImageLookup still only carries { provider_image_id, base_sha }, and SandboxLifecycleManager passes that opaque ID to whichever provider is active. After switching SANDBOX_PROVIDER between Modal and Vercel, existing rows can be restored or deleted with the wrong provider API. Store/filter by backend (or clear incompatible rows) before enabling supportsRepoImageBackend() here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/session/durable-object.ts` around lines 764 - 766,
RepoImageLookup currently only stores provider_image_id and base_sha which lets
SandboxLifecycleManager call the wrong provider after SANDBOX_PROVIDER changes;
update persistence and lookup logic so RepoImageLookup includes the
backend/provider identifier (e.g., provider_name or backend enum) and ensure
code paths that populate or read repoImageLookup (the block guarded by
supportsRepoImageBackend and any callers in SandboxLifecycleManager) filter by
that backend or delete incompatible rows before using the opaque
provider_image_id; also add migration/cleanup logic when reading existing rows
to clear or ignore entries whose stored backend does not match the active
SANDBOX_PROVIDER.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SANDBOX_PROVIDER=vercelNotes
The provider intentionally leaves Vercel sandbox resources unset. Vercel therefore uses its default sandbox size, currently observed as 2 vCPU / 4 GB RAM. Resource sizing can be added later by threading a
resources.vcpusvalue through the Vercel create-sandbox request.Validation
npm cinpm run build -w @open-inspect/sharednpm test -w @open-inspect/control-plane -- src/sandbox/vercel-base-snapshot.test.ts src/sandbox/providers/vercel-provider.test.ts src/sandbox/vercel-client.test.ts src/sandbox/provider-name.test.ts src/sandbox/lifecycle/manager.test.tsnpm test -w @open-inspect/web -- src/lib/sandbox-provider.test.tsnpm run build:vercel-base-snapshot -w @open-inspect/control-planenpm run typecheck -w @open-inspect/control-planenpm run lint -w @open-inspect/control-planenpm run build -w @open-inspect/control-planenpm run typecheck -w @open-inspect/webnpm run lint -w @open-inspect/webterraform fmt -check -recursive terraformterraform -chdir=terraform/environments/production validate -no-colorNote: local Terraform validation required a temporary darwin_arm64 Cloudflare provider checksum from
terraform init; that lock-file change was removed before commit.Summary by CodeRabbit
Release Notes
New Features
Documentation
Infrastructure