feat: add Stagehand code execution tool - #2619
Conversation
|
There was a problem hiding this comment.
9 issues found across 18 files
Confidence score: 3/5
- In
packages/integrations/src/codemode/executor.ts(close(),ensureStagehand()) andpackages/integrations/src/codemode/config.ts(stagehandCodeConfigFromEnv()), raw lifecycle/config errors can bubble up unsanitized, which risks leaking internal details to MCP stdio users and makes failures harder to control consistently — wrap and sanitize these exceptions before surfacing them. packages/integrations/src/codemode/config.tscurrently prefersGEMINI_API_KEYwhen both Google env vars are set, which can select the wrong credential versus repo conventions and cause confusing auth behavior — switch to the canonical key-precedence order.- In
packages/integrations/src/codemode/index.ts, newly publicStagehandCodeExecutor/executeCodepaths appear to missflowLoggerinstrumentation, creating observability gaps for debugging and audit trails on the new surface area — add flowLogger hooks across the exposed execution path. packages/integrations/src/codemode/executor.tsandpackages/integrations/src/codemode/tool-contract.tshave inconsistent/duplicated byte-limit handling, including mid-UTF-8 truncation and possible post-truncation size drift invalue.preview, which can produce malformed text and inaccurate size bounds — centralize one byte-limit constant and apply UTF-8-safe truncation with a final byte-length clamp.
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/src/codemode/executor.ts">
<violation number="1" location="packages/integrations/src/codemode/executor.ts:80">
P1: Custom agent: **Exception and error message sanitization**
`close()` and `ensureStagehand()` throw generic `AggregateError` instances wrapping raw, unsanitized errors from Playwright/Stagehand lifecycle operations. The `.errors` array can expose CDP URLs, session details, or credentials to callers instead of surfacing a sanitized, typed error. Convert these to individually typed error classes (like the `failure()` helper used elsewhere) and sanitize or replace the inner error details before raising them to users.</violation>
<violation number="2" location="packages/integrations/src/codemode/executor.ts:193">
P3: When a console log entry or a returned value is truncated at the byte budget, the code slices the UTF-8 buffer at an arbitrary byte offset. If the cut lands mid-character, `Buffer.toString()` substitutes a single replacement character (U+FFFD), so the last log line (or result preview) can end with an invalid/broken character, and the byte accounting can actually overshoot the declared limit (a lone 1-byte slice expands to a 3-byte replacement char). Consider truncating on a character boundary instead so the bounded log/preview is always valid UTF-8 text that fits the budget.</violation>
<violation number="3" location="packages/integrations/src/codemode/executor.ts:228">
P2: Oversized `value.preview` can exceed the configured byte limit when truncation lands mid-character. Applying the same post-decode byte-length clamp here keeps the result-size bound accurate.</violation>
</file>
<file name="packages/integrations/src/codemode/index.ts">
<violation number="1" location="packages/integrations/src/codemode/index.ts:2">
P2: Custom agent: **Ensure all public methods added to the stagehand class, agent, or understudy (page, locator, etc.) interfaces are properly instrumented with the flowLogger**
New public `StagehandCodeExecutor` and `executeStagehandSnippet` interfaces exported here drive discrete browser actions (e.g., ensuring a Stagehand instance, resolving pages, executing user code against `page`/`context`/`stagehand`), but the implementation files contain no flowLogger instrumentation. Rule cb36c727 requires significant new public interfaces that trigger discrete browser steps to be tracked via flowLogger decorators or manual logging. Consider adding `@logStagehandStep` (or equivalent manual `SessionFileLogger` calls) to the `execute()` entry point and the snippet execution path so these operations are observable alongside existing `act()`/`observe()`/`extract()` traces.</violation>
<violation number="2" location="packages/integrations/src/codemode/index.ts:10">
P2: The codemode barrel now publishes all types, which widens the API contract and makes internal type refactors breaking for consumers. Prefer explicit `export type { ... }` for the intended public subset.
(Based on your team's feedback about avoiding exposing internal types as public APIs.)</violation>
</file>
<file name="packages/integrations/tests/stdio-server.test.ts">
<violation number="1" location="packages/integrations/tests/stdio-server.test.ts:105">
P2: The assertion on stderr can be flaky: waitForExit resolves on the child's 'exit' event, which Node fires before the piped stdio streams are drained, so the stderr 'data' listener may not have received the error text yet when the assertion runs. Consider resolving waitForExit on the 'close' event (which fires after stdio streams close and also supplies code/signal) so this test reliably sees the captured output.</violation>
</file>
<file name="packages/integrations/src/codemode/config.ts">
<violation number="1" location="packages/integrations/src/codemode/config.ts:15">
P2: Custom agent: **Exception and error message sanitization**
These configuration validation errors propagate directly to users during MCP stdio server startup because `stagehandCodeConfigFromEnv()` is called without any wrapping sanitization boundary in `stdio-server.ts`. The rule requires individually typed error classes, not generic `new Error()`. Consider defining a typed error class (for example, `StagehandCodeConfigError`) and throwing that instead so callers and monitoring can distinguish config failures from other errors.</violation>
<violation number="2" location="packages/integrations/src/codemode/config.ts:83">
P2: When both Google env vars are set, this picks `GEMINI_API_KEY` first, which diverges from the repo’s canonical Google-key precedence and can select an unintended credential. Aligning to canonical-first order keeps code-mode behavior consistent with eval-native initialization.</violation>
</file>
<file name="packages/integrations/src/codemode/tool-contract.ts">
<violation number="1" location="packages/integrations/src/codemode/tool-contract.ts:16">
P3: The snippet size limit is enforced in two places with two separately-maintained 100_000-byte constants: the tool-contract schema (`new TextEncoder().encode(code).byteLength <= 100_000`) and the executor's `MAX_CODE_BYTES`. Since these files are reviewed/maintained independently, they can silently drift apart, making the documented and error-message limit inconsistent with what actually gets rejected. Consider exporting a single shared `MAX_CODE_BYTES` constant from tool-contract (or types) and referencing it in both the schema refine and the executor's `validate()` so the bound stays in sync.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Host as Agent Framework / Host
participant Exec as StagehandCodeExecutor
participant Queue as Serial Queue
participant Snip as executeStagehandSnippet
participant Stagehand as Stagehand Instance
participant Browser as Browser (local/Browserbase)
participant Page as Page
participant MCP as MCP Server / Tool
participant Config as stagehandCodeConfigFromEnv
Note over Host,Config: Startup: Configuration and Browser Selection
Host->>Config: Read environment variables
Config->>Config: Determine browser type (local/browserbase)
alt STAGEHAND_BROWSER=local
Config->>Config: Set local headless browser
else STAGEHAND_BROWSER=browserbase
Config->>Config: Validate BROWSERBASE_API_KEY present
Config->>Config: Forward project ID if set
else No explicit setting
alt BROWSERBASE_API_KEY exists
Config->>Config: Select Browserbase
else
Config->>Config: Select local headless
end
end
Config->>Config: Resolve model name and API key
alt Explicit STAGEHAND_MODEL_NAME
Config->>Config: Use provider-specific API key
alt Provider is Anthropic
Config->>Config: Add dangerous-direct-browser-access header
end
else No model name, Google key present
Config->>Config: Default to google/gemini-2.5-flash-lite
end
Config-->>Host: Return StagehandCodeConfig
Note over Host,MCP: Execution Flow
Host->>Exec: new StagehandCodeExecutor(config)
Host->>Exec: execute({ code }, signal?)
Note over Exec,Queue: Serialization and Validation
Exec->>Exec: validate input (size, non-empty)
alt Invalid input
Exec-->>Host: Return failure with kind="validation"
else Valid input
Exec->>Queue: Chain onto FIFO queue
Queue-->>Exec: Wait for previous operations
end
Note over Exec,Stagehand: Lazy Browser Initialization
Exec->>Exec: ensureStagehand()
alt Stagehand not yet created
Exec->>Browser: Launch (local or browserbase)
alt Launch successful
Exec->>Stagehand: Stagehand.create(browser, config)
alt Stagehand create fails
Exec->>Browser: Close browser
Exec-->>Host: Return failure with kind="runtime"
end
else Launch fails
Exec-->>Host: Return failure with kind="runtime"
end
end
Exec-->>Exec: Stagehand instance ready
Note over Exec,Page: Snippet Execution
Exec->>Page: Get active page (or first/new)
alt Signal aborted before execution
Exec-->>Host: Return failure with kind="aborted"
else Signal not aborted
Exec->>Snip: executeStagehandSnippet({ code, page, context, stagehand, console })
Snip->>Snip: Create AsyncFunction with bindings
Note over Snip: Injects page, context, stagehand, z (Zod), console
Snip->>Page: Execute snippet code
alt Snippet succeeds
Page-->>Snip: Return value
Snip-->>Exec: Return value
Exec->>Page: Read page state (URL, title)
Exec-->>Host: Return success with page state and value
else Snippet throws
Page-->>Snip: Throw error
Snip-->>Exec: Throw error
Exec->>Exec: normalizeError (redact secrets)
Exec-->>Host: Return failure with kind="runtime"
end
end
Note over Exec: Cleanup and Shutdown
Host->>Exec: close()
Exec->>Exec: Drain queued operations
Exec->>Stagehand: stagehand.close()
Exec->>Browser: browser.close()
alt Both close fail
Exec-->>Host: AggregateError
end
Exec-->>Host: Cleanup complete
Note over MCP,MCP: MCP Tool Registration
Host->>MCP: createCodeModeMcpServer(executor)
MCP->>MCP: registerTool("code_execute", schema, handler)
MCP-->>Host: McpServer ready
Host->>MCP: connect to stdio transport
Note over MCP,Host: Tool calls flow through MCP protocol
Host->>MCP: callTool("code_execute", { code })
MCP->>Exec: executor.execute(input, signal)
Exec-->>MCP: result
MCP->>MCP: Format as text + structured content
alt result.ok
MCP-->>Host: isError=false, content=JSON
else
MCP-->>Host: isError=true, content=JSON
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (browser) { | ||
| await browser.close().catch((error) => errors.push(error)); | ||
| } | ||
| if (errors.length > 0) { |
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
close() and ensureStagehand() throw generic AggregateError instances wrapping raw, unsanitized errors from Playwright/Stagehand lifecycle operations. The .errors array can expose CDP URLs, session details, or credentials to callers instead of surfacing a sanitized, typed error. Convert these to individually typed error classes (like the failure() helper used elsewhere) and sanitize or replace the inner error details before raising them to users.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/executor.ts, line 80:
<comment>`close()` and `ensureStagehand()` throw generic `AggregateError` instances wrapping raw, unsanitized errors from Playwright/Stagehand lifecycle operations. The `.errors` array can expose CDP URLs, session details, or credentials to callers instead of surfacing a sanitized, typed error. Convert these to individually typed error classes (like the `failure()` helper used elsewhere) and sanitize or replace the inner error details before raising them to users.</comment>
<file context>
@@ -0,0 +1,308 @@
+ if (browser) {
+ await browser.close().catch((error) => errors.push(error));
+ }
+ if (errors.length > 0) {
+ throw new AggregateError(errors, "Failed to close Stagehand code mode.");
+ }
</file context>
| return { | ||
| truncated: true, | ||
| original_bytes: bytes, | ||
| preview: Buffer.from(serialized).subarray(0, MAX_RESULT_BYTES).toString(), |
There was a problem hiding this comment.
P2: Oversized value.preview can exceed the configured byte limit when truncation lands mid-character. Applying the same post-decode byte-length clamp here keeps the result-size bound accurate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/executor.ts, line 228:
<comment>Oversized `value.preview` can exceed the configured byte limit when truncation lands mid-character. Applying the same post-decode byte-length clamp here keeps the result-size bound accurate.</comment>
<file context>
@@ -0,0 +1,308 @@
+ return {
+ truncated: true,
+ original_bytes: bytes,
+ preview: Buffer.from(serialized).subarray(0, MAX_RESULT_BYTES).toString(),
+ };
+}
</file context>
| codeExecuteResultText, | ||
| codeExecuteSchema, | ||
| } from "./tool-contract.js"; | ||
| export * from "./types.js"; |
There was a problem hiding this comment.
P2: The codemode barrel now publishes all types, which widens the API contract and makes internal type refactors breaking for consumers. Prefer explicit export type { ... } for the intended public subset.
(Based on your team's feedback about avoiding exposing internal types as public APIs.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/index.ts, line 10:
<comment>The codemode barrel now publishes all types, which widens the API contract and makes internal type refactors breaking for consumers. Prefer explicit `export type { ... }` for the intended public subset.
(Based on your team's feedback about avoiding exposing internal types as public APIs.) </comment>
<file context>
@@ -0,0 +1,10 @@
+ codeExecuteResultText,
+ codeExecuteSchema,
+} from "./tool-contract.js";
+export * from "./types.js";
</file context>
| stderr += chunk.toString(); | ||
| }); | ||
|
|
||
| const exit = await waitForExit(child); |
There was a problem hiding this comment.
P2: The assertion on stderr can be flaky: waitForExit resolves on the child's 'exit' event, which Node fires before the piped stdio streams are drained, so the stderr 'data' listener may not have received the error text yet when the assertion runs. Consider resolving waitForExit on the 'close' event (which fires after stdio streams close and also supplies code/signal) so this test reliably sees the captured output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/tests/stdio-server.test.ts, line 105:
<comment>The assertion on stderr can be flaky: waitForExit resolves on the child's 'exit' event, which Node fires before the piped stdio streams are drained, so the stderr 'data' listener may not have received the error text yet when the assertion runs. Consider resolving waitForExit on the 'close' event (which fires after stdio streams close and also supplies code/signal) so this test reliably sees the captured output.</comment>
<file context>
@@ -88,7 +95,20 @@ describe("built code-mode stdio host", () => {
+ stderr += chunk.toString();
+ });
+
+ const exit = await waitForExit(child);
+
+ expect(exit.code).not.toBe(0);
</file context>
| return nonEmpty(env.ANTHROPIC_API_KEY); | ||
| case "google": | ||
| return ( | ||
| nonEmpty(env.GEMINI_API_KEY) ?? |
There was a problem hiding this comment.
P2: When both Google env vars are set, this picks GEMINI_API_KEY first, which diverges from the repo’s canonical Google-key precedence and can select an unintended credential. Aligning to canonical-first order keeps code-mode behavior consistent with eval-native initialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/config.ts, line 83:
<comment>When both Google env vars are set, this picks `GEMINI_API_KEY` first, which diverges from the repo’s canonical Google-key precedence and can select an unintended credential. Aligning to canonical-first order keeps code-mode behavior consistent with eval-native initialization.</comment>
<file context>
@@ -0,0 +1,99 @@
+ return nonEmpty(env.ANTHROPIC_API_KEY);
+ case "google":
+ return (
+ nonEmpty(env.GEMINI_API_KEY) ??
+ nonEmpty(env.GOOGLE_GENERATIVE_AI_API_KEY) ??
+ nonEmpty(env.GOOGLE_API_KEY)
</file context>
| requestedBrowser !== "local" && | ||
| requestedBrowser !== "browserbase" | ||
| ) { | ||
| throw new Error('STAGEHAND_BROWSER must be either "local" or "browserbase".'); |
There was a problem hiding this comment.
P2: Custom agent: Exception and error message sanitization
These configuration validation errors propagate directly to users during MCP stdio server startup because stagehandCodeConfigFromEnv() is called without any wrapping sanitization boundary in stdio-server.ts. The rule requires individually typed error classes, not generic new Error(). Consider defining a typed error class (for example, StagehandCodeConfigError) and throwing that instead so callers and monitoring can distinguish config failures from other errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/config.ts, line 15:
<comment>These configuration validation errors propagate directly to users during MCP stdio server startup because `stagehandCodeConfigFromEnv()` is called without any wrapping sanitization boundary in `stdio-server.ts`. The rule requires individually typed error classes, not generic `new Error()`. Consider defining a typed error class (for example, `StagehandCodeConfigError`) and throwing that instead so callers and monitoring can distinguish config failures from other errors.</comment>
<file context>
@@ -0,0 +1,99 @@
+ requestedBrowser !== "local" &&
+ requestedBrowser !== "browserbase"
+ ) {
+ throw new Error('STAGEHAND_BROWSER must be either "local" or "browserbase".');
+ }
+
</file context>
| @@ -0,0 +1,10 @@ | |||
| export { stagehandCodeConfigFromEnv } from "./config.js"; | |||
| export { StagehandCodeExecutor, type StagehandCodeExecutorOptions } from "./executor.js"; | |||
There was a problem hiding this comment.
P2: Custom agent: Ensure all public methods added to the stagehand class, agent, or understudy (page, locator, etc.) interfaces are properly instrumented with the flowLogger
New public StagehandCodeExecutor and executeStagehandSnippet interfaces exported here drive discrete browser actions (e.g., ensuring a Stagehand instance, resolving pages, executing user code against page/context/stagehand), but the implementation files contain no flowLogger instrumentation. Rule cb36c727 requires significant new public interfaces that trigger discrete browser steps to be tracked via flowLogger decorators or manual logging. Consider adding @logStagehandStep (or equivalent manual SessionFileLogger calls) to the execute() entry point and the snippet execution path so these operations are observable alongside existing act()/observe()/extract() traces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/index.ts, line 2:
<comment>New public `StagehandCodeExecutor` and `executeStagehandSnippet` interfaces exported here drive discrete browser actions (e.g., ensuring a Stagehand instance, resolving pages, executing user code against `page`/`context`/`stagehand`), but the implementation files contain no flowLogger instrumentation. Rule cb36c727 requires significant new public interfaces that trigger discrete browser steps to be tracked via flowLogger decorators or manual logging. Consider adding `@logStagehandStep` (or equivalent manual `SessionFileLogger` calls) to the `execute()` entry point and the snippet execution path so these operations are observable alongside existing `act()`/`observe()`/`extract()` traces.</comment>
<file context>
@@ -0,0 +1,10 @@
+export { stagehandCodeConfigFromEnv } from "./config.js";
+export { StagehandCodeExecutor, type StagehandCodeExecutorOptions } from "./executor.js";
+export { connectCodeModeStdio, createCodeModeMcp, createCodeModeMcpServer } from "./mcp-server.js";
+export { executeStagehandSnippet } from "./snippet.js";
</file context>
| .string() | ||
| .refine((code) => code.trim().length > 0, "code must contain JavaScript source") | ||
| .refine( | ||
| (code) => new TextEncoder().encode(code).byteLength <= 100_000, |
There was a problem hiding this comment.
P3: The snippet size limit is enforced in two places with two separately-maintained 100_000-byte constants: the tool-contract schema (new TextEncoder().encode(code).byteLength <= 100_000) and the executor's MAX_CODE_BYTES. Since these files are reviewed/maintained independently, they can silently drift apart, making the documented and error-message limit inconsistent with what actually gets rejected. Consider exporting a single shared MAX_CODE_BYTES constant from tool-contract (or types) and referencing it in both the schema refine and the executor's validate() so the bound stays in sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/tool-contract.ts, line 16:
<comment>The snippet size limit is enforced in two places with two separately-maintained 100_000-byte constants: the tool-contract schema (`new TextEncoder().encode(code).byteLength <= 100_000`) and the executor's `MAX_CODE_BYTES`. Since these files are reviewed/maintained independently, they can silently drift apart, making the documented and error-message limit inconsistent with what actually gets rejected. Consider exporting a single shared `MAX_CODE_BYTES` constant from tool-contract (or types) and referencing it in both the schema refine and the executor's `validate()` so the bound stays in sync.</comment>
<file context>
@@ -0,0 +1,71 @@
+ .string()
+ .refine((code) => code.trim().length > 0, "code must contain JavaScript source")
+ .refine(
+ (code) => new TextEncoder().encode(code).byteLength <= 100_000,
+ "code must be at most 100000 UTF-8 bytes",
+ )
</file context>
| if (logBytes >= MAX_LOG_BYTES) return; | ||
| const text = formatLog(values); | ||
| const remaining = MAX_LOG_BYTES - logBytes; | ||
| const bounded = Buffer.from(text).subarray(0, remaining).toString(); |
There was a problem hiding this comment.
P3: When a console log entry or a returned value is truncated at the byte budget, the code slices the UTF-8 buffer at an arbitrary byte offset. If the cut lands mid-character, Buffer.toString() substitutes a single replacement character (U+FFFD), so the last log line (or result preview) can end with an invalid/broken character, and the byte accounting can actually overshoot the declared limit (a lone 1-byte slice expands to a 3-byte replacement char). Consider truncating on a character boundary instead so the bounded log/preview is always valid UTF-8 text that fits the budget.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/src/codemode/executor.ts, line 193:
<comment>When a console log entry or a returned value is truncated at the byte budget, the code slices the UTF-8 buffer at an arbitrary byte offset. If the cut lands mid-character, `Buffer.toString()` substitutes a single replacement character (U+FFFD), so the last log line (or result preview) can end with an invalid/broken character, and the byte accounting can actually overshoot the declared limit (a lone 1-byte slice expands to a 3-byte replacement char). Consider truncating on a character boundary instead so the bounded log/preview is always valid UTF-8 text that fits the budget.</comment>
<file context>
@@ -0,0 +1,308 @@
+ if (logBytes >= MAX_LOG_BYTES) return;
+ const text = formatLog(values);
+ const remaining = MAX_LOG_BYTES - logBytes;
+ const bounded = Buffer.from(text).subarray(0, remaining).toString();
+ logBytes += Buffer.byteLength(bounded);
+ logs.push({ level, text: bounded });
</file context>
Why
This second stack layer adds the independently usable code-execution product core on top of the package and MCP host from #2597. Keeping execution separate from generated agent guidance lets reviewers focus on browser ownership, configuration, schemas, queueing, redaction, and lifecycle behavior.
Stack
code_execute, Stagehand executor, local/Browserbase configuration, schemas, and runtime testsSKILL.md,REFERENCE.md, generated exports, package assets, and guidance loading checksWhat changed
StagehandCodeExecutorwith lazy browser startup and one long-lived session per executorcode_executepage,context,stagehand, Zod, and a bounded console into async JavaScript snippetsLocal and remote startup
The stdio process reads startup configuration from its environment:
STAGEHAND_BROWSER=localSTAGEHAND_BROWSER=browserbaseBROWSERBASE_API_KEY.BROWSERBASE_PROJECT_IDis forwarded when present. Explicit model names select only their matching provider key, and Google-key precedence matches the eval-native configuration.Timeout and cancellation boundary
An abort signal can cancel queued work before its snippet begins. Arbitrary JavaScript already executing in-process cannot be safely preempted. If code blocks the Node event loop, the owning framework must terminate the entire child process tree, escalate to
SIGKILLafter its deadline, and create a replacement process. Killing only the Node process can leave a local browser descendant alive.Intentionally not included
SKILL.mdorREFERENCE.mdE2E Test Matrix
pnpm --filter @browserbasehq/stagehand-integrations typecheck && pnpm --filter @browserbasehq/stagehand-integrations testcode_executewas discovered and state persisted across both calls.code_executewas discovered and both remote pages persisted.pnpm exec turbo run fmt:check lint typecheck --concurrency=1Changeset
None. This changes a private workspace package and does not publish a release.