feat: run code-mode MCP in Vercel Sandbox - #2626
Conversation
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| process.env.STAGEHAND_BROWSER ??= "local"; | ||
|
|
||
| const result = await runStagehandAgent( | ||
| groq(process.env.VERCEL_STAGEHAND_MODEL ?? "openai/gpt-oss-120b"), |
There was a problem hiding this comment.
swap this to opus 5
… into shrey/stg-2765-codemode-vercel
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 18 files (changes from recent commits).
Confidence score: 4/5
- In
packages/integrations/examples/e2b/src/smoke.ts, ignored teardown errors can let the smoke test pass even when MCP client close fails, creating a CI blind spot for cleanup regressions; this is the main risk because failures may accumulate unnoticed over time — surface and report close errors while preserving any primary test failure signal.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/examples/e2b/src/smoke.ts">
<violation number="1" location="packages/integrations/examples/e2b/src/smoke.ts:54">
P3: Cleanup errors are currently ignored, so this smoke can report PASS even when MCP client teardown fails. Surfacing close failures (while still preserving primary test failures) would make CI catch cleanup regressions instead of masking them.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| `${JSON.stringify({ status: "PASS", tools: ["code_execute"], statePersisted: true })}\n`, | ||
| ); | ||
| } finally { | ||
| await client.close().catch(() => undefined); |
There was a problem hiding this comment.
P3: Cleanup errors are currently ignored, so this smoke can report PASS even when MCP client teardown fails. Surfacing close failures (while still preserving primary test failures) would make CI catch cleanup regressions instead of masking them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/examples/e2b/src/smoke.ts, line 54:
<comment>Cleanup errors are currently ignored, so this smoke can report PASS even when MCP client teardown fails. Surfacing close failures (while still preserving primary test failures) would make CI catch cleanup regressions instead of masking them.</comment>
<file context>
@@ -0,0 +1,72 @@
+ `${JSON.stringify({ status: "PASS", tools: ["code_execute"], statePersisted: true })}\n`,
+ );
+} finally {
+ await client.close().catch(() => undefined);
+}
+
</file context>
There was a problem hiding this comment.
Addressed in e9c6682. The smoke now preserves the primary failure, surfaces cleanup-only failures, and emits PASS only after client.close() succeeds.
shrey150
changed the title
There was a problem hiding this comment.
2 issues found across 16 files (changes from recent commits).
Confidence score: 4/5
- In
packages/integrations/examples/vercel-sandbox/src/sandbox.ts, a malformed successful/v1/sessionsresponse can throw beforesessionIdis recorded, which risks leaving a billable Browserbase session undeleted — capture the returnedid/connectUrldefensively and ensure cleanup still runs on parse/validation failures. - In
packages/integrations/examples/vercel-sandbox/tsconfig.json, permitting.tsESM specifiers can let this example drift from the repo’s.js-specifier convention, increasing runtime/loader mismatch risk outside TypeScript checks — keep TS extension imports constrained and preserve the.jsspecifier convention in emitted/imported paths.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/examples/vercel-sandbox/tsconfig.json">
<violation number="1" location="packages/integrations/examples/vercel-sandbox/tsconfig.json:4">
P2: Typechecking now allows `.ts` ESM import specifiers, so this example can drift away from the repository’s `.js`-specifier convention and become loader-dependent at runtime. Consider keeping `allowImportingTsExtensions` disabled and switching relative imports to explicit `.js` specifiers.
(Based on your team's feedback about requiring explicit .js extensions for relative ESM imports.) .</violation>
</file>
<file name="packages/integrations/examples/vercel-sandbox/src/sandbox.ts">
<violation number="1" location="packages/integrations/examples/vercel-sandbox/src/sandbox.ts:291">
P2: If the POST `/v1/sessions` call succeeds (so Browserbase actually created a billable session) but the response body is malformed (missing a string `id` or `connectUrl`), the function throws before `sessionId` is assigned. Since the cleanup block is guarded by `if (sessionId)`, the just-created Browserbase session is never released with `REQUEST_RELEASE`, leaking a live/expensive session and its browser on an error path. Assign `sessionId` (or otherwise track the created session) before validating the shape so the cleanup path can still release it.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| throw new Error(`Browserbase CDP host discovery returned ${response.status}`); | ||
| } | ||
| const session = (await response.json()) as { id?: unknown; connectUrl?: unknown }; | ||
| if (typeof session.id !== "string" || typeof session.connectUrl !== "string") { |
There was a problem hiding this comment.
P2: If the POST /v1/sessions call succeeds (so Browserbase actually created a billable session) but the response body is malformed (missing a string id or connectUrl), the function throws before sessionId is assigned. Since the cleanup block is guarded by if (sessionId), the just-created Browserbase session is never released with REQUEST_RELEASE, leaking a live/expensive session and its browser on an error path. Assign sessionId (or otherwise track the created session) before validating the shape so the cleanup path can still release it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/examples/vercel-sandbox/src/sandbox.ts, line 291:
<comment>If the POST `/v1/sessions` call succeeds (so Browserbase actually created a billable session) but the response body is malformed (missing a string `id` or `connectUrl`), the function throws before `sessionId` is assigned. Since the cleanup block is guarded by `if (sessionId)`, the just-created Browserbase session is never released with `REQUEST_RELEASE`, leaking a live/expensive session and its browser on an error path. Assign `sessionId` (or otherwise track the created session) before validating the shape so the cleanup path can still release it.</comment>
<file context>
@@ -0,0 +1,406 @@
+ throw new Error(`Browserbase CDP host discovery returned ${response.status}`);
+ }
+ const session = (await response.json()) as { id?: unknown; connectUrl?: unknown };
+ if (typeof session.id !== "string" || typeof session.connectUrl !== "string") {
+ throw new Error("Browserbase CDP host discovery returned an invalid session");
+ }
</file context>
There was a problem hiding this comment.
Addressed in e9c6682. A string session ID is retained immediately after decoding the response, before validating connectUrl, so malformed post-creation responses still take the REQUEST_RELEASE cleanup path whenever the API supplied a releasable ID.
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
… into shrey/stg-2765-codemode-vercel # Conflicts: # packages/integrations/README.md
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
2 issues found across 10 files (changes from recent commits).
Confidence score: 4/5
- In
packages/integrations/examples/vercel-sandbox/src/sandbox.ts, the credential allowlist behavior isn’t protected by a regression test, so future changes could accidentally forward extra keys (likenetworkPolicy) toSandbox.createand broaden what gets sent to the integration — add a mockedSandbox.createtest that asserts onlyteamId,projectId, andtokenare passed through. - In
packages/integrations/examples/vercel-sandbox/src/sandbox.ts, the new non-stringresolvedguard lacks fixture coverage, which risks regressions wherenull/number/object values slip past validation or fail with the wrong error path before sandbox creation — add regression cases assertingStagehandPackageArtifactErrorfor non-stringresolvedinputs.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/examples/vercel-sandbox/src/sandbox.ts">
<violation number="1" location="packages/integrations/examples/vercel-sandbox/src/sandbox.ts:122">
P3: The credential allowlist has no regression test; add a mocked `Sandbox.create` case with an extra `networkPolicy` key and assert only `teamId`, `projectId`, and `token` are forwarded.
(Based on your team's feedback about allowlisting Sandbox credentials.)</violation>
<violation number="2" location="packages/integrations/examples/vercel-sandbox/src/sandbox.ts:521">
P3: The new non-string `resolved` rejection has no regression coverage; add `resolved: null` and numeric/object fixtures asserting `StagehandPackageArtifactError` before sandbox creation.
(Based on your team's feedback about adding unit tests for changed behavior.)</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| for (const entry of Object.values(lock.packages ?? {})) { | ||
| const resolved = entry.resolved; | ||
| if ( | ||
| resolved !== undefined && |
There was a problem hiding this comment.
P3: The new non-string resolved rejection has no regression coverage; add resolved: null and numeric/object fixtures asserting StagehandPackageArtifactError before sandbox creation.
(Based on your team's feedback about adding unit tests for changed behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/examples/vercel-sandbox/src/sandbox.ts, line 521:
<comment>The new non-string `resolved` rejection has no regression coverage; add `resolved: null` and numeric/object fixtures asserting `StagehandPackageArtifactError` before sandbox creation.
(Based on your team's feedback about adding unit tests for changed behavior.) </comment>
<file context>
@@ -502,12 +509,22 @@ function assertRuntimeManifest(content: Buffer): void {
+ for (const entry of Object.values(lock.packages ?? {})) {
+ const resolved = entry.resolved;
+ if (
+ resolved !== undefined &&
+ (typeof resolved !== "string" ||
+ (!resolved.startsWith("file:") && !resolved.startsWith("https://registry.npmjs.org/")))
</file context>
There was a problem hiding this comment.
Added in 63ddf585. The contract suite now exercises resolved: null, a number, and an object, and each case asserts the fixed StagehandPackageArtifactError before any sandbox creation path. The Vercel sandbox contract suite passes 10/10.
| persistent: false, | ||
| networkPolicy: "allow-all", | ||
| tags: { purpose: "stagehand-codemode-mcp" }, | ||
| ...(vercelCredentials |
There was a problem hiding this comment.
P3: The credential allowlist has no regression test; add a mocked Sandbox.create case with an extra networkPolicy key and assert only teamId, projectId, and token are forwarded.
(Based on your team's feedback about allowlisting Sandbox credentials.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/examples/vercel-sandbox/src/sandbox.ts, line 122:
<comment>The credential allowlist has no regression test; add a mocked `Sandbox.create` case with an extra `networkPolicy` key and assert only `teamId`, `projectId`, and `token` are forwarded.
(Based on your team's feedback about allowlisting Sandbox credentials.) </comment>
<file context>
@@ -118,7 +119,13 @@ export async function createStagehandSandbox(
networkPolicy: "allow-all",
tags: { purpose: "stagehand-codemode-mcp" },
- ...options.vercelCredentials,
+ ...(vercelCredentials
+ ? {
+ teamId: vercelCredentials.teamId,
</file context>
There was a problem hiding this comment.
Added in 63ddf585. The new regression test passes an extra untrusted networkPolicy: "deny-all", mocks Sandbox.create, and proves only the allowlisted teamId, projectId, and token are forwarded while the trusted call-site policy remains allow-all and the trusted purpose tag is unchanged. The contract suite passes 10/10.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 5/5
- In
packages/integrations/examples/vercel-sandbox/scripts/pack-artifacts.mjs, the main risk is regression in failure sanitization: a future edit could re-expose rawexecFileAsyncstdout/stderr on wrapped command errors, potentially leaking noisy or sensitive subprocess output in failure paths — add a focused failing-subprocess test that asserts sanitized error output.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/examples/vercel-sandbox/scripts/pack-artifacts.mjs">
<violation number="1" location="packages/integrations/examples/vercel-sandbox/scripts/pack-artifacts.mjs:99">
P3: Failure sanitization has no regression coverage: a future change can again expose `execFileAsync` stdout/stderr without a test failing. Add a focused subprocess test that forces a wrapped command failure and asserts only the fixed typed error/message is emitted.
(Based on your team's feedback about unit tests for new behavior.)</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| async function run(file, args, cwd) { | ||
| try { | ||
| return await execFileAsync(file, args, { cwd, maxBuffer: commandMaxBuffer }); | ||
| } catch { |
There was a problem hiding this comment.
P3: Failure sanitization has no regression coverage: a future change can again expose execFileAsync stdout/stderr without a test failing. Add a focused subprocess test that forces a wrapped command failure and asserts only the fixed typed error/message is emitted.
(Based on your team's feedback about unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/examples/vercel-sandbox/scripts/pack-artifacts.mjs, line 99:
<comment>Failure sanitization has no regression coverage: a future change can again expose `execFileAsync` stdout/stderr without a test failing. Add a focused subprocess test that forces a wrapped command failure and asserts only the fixed typed error/message is emitted.
(Based on your team's feedback about unit tests for new behavior.) </comment>
<file context>
@@ -86,7 +94,11 @@ function requiredArtifact(files, pattern) {
- return execFileAsync(file, args, { cwd, maxBuffer: commandMaxBuffer });
+ try {
+ return await execFileAsync(file, args, { cwd, maxBuffer: commandMaxBuffer });
+ } catch {
+ throw new StagehandArtifactPackCommandError();
+ }
</file context>
There was a problem hiding this comment.
Added in 35c15787. The command wrapper is now isolated in an importable module and a secret-free subprocess regression forces stderr containing a sentinel token, then asserts the fixed StagehandArtifactPackCommandError name/message and proves neither stdout nor stderr is attached. The sandbox suite passes 12/12, exact packing passes, and the workspace check passes 9/9.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
1 participant
BLUF
Run the published Stagehand code-mode executable inside a Vercel Firecracker microVM and expose one framework-neutral, bearer-authenticated Streamable HTTP connection. Agent-framework adapters stay outside the VM and consume only
{ url, token, close }.This PR now stacks on the package-publication PR, #2644.
Architecture
The trusted host builds and packs the exact review checkout into
@browserbasehq/stagehandand@browserbasehq/stagehand-codemodetarballs. It also creates an npm v3 lock for those local artifacts plus exactsupergateway@3.4.3, rejects non-public registry sources, and uploads the complete artifact set. The guest verifies each uploaded SHA-256 digest and runsnpm ci --ignore-scriptsbefore lockdown. No guest-side Git checkout, branch/tag resolution, workspace install, pnpm activation, or monorepo build remains.The helper then:
stagehand-mcp;stagehand-proxyuser;api.browserbase.complus that exact CDP host;{ url, token, close }, whereclose()idempotently attempts both stop and permanent delete.The host keeps the random 32-byte bearer. The proxy bootstrap receives only its SHA-256 digest. Installed artifacts, dependencies, and bridge files become root-owned and read-only before generated JavaScript can run.
Security limits
GET /mcpreturns405; Stagehand has no server-initiated notifications, so MCP calls stream over POST. Revisit this if the server adds unsolicited notifications.The optional OCI image in #2643 remains a sibling optimization, not a dependency of this package-installed foundation.
E2E Test Matrix
pnpm checkpack:artifactsnpm ci --ignore-scriptsfrom the generated lockstagehand-codemodeandsupergatewayexecutables presenttest:contractcode_execute; two calls retain browser state and PASS is emitted only after cleanupec7a7ee1Changeset
No additional changeset. This PR adds a private integration example and CI coverage; #2644 carries the public package changeset.