feat(integrations): add Grok Build integration + Experimental ACP - #2772
Open
antonvishal wants to merge 2 commits into
Open
feat(integrations): add Grok Build integration + Experimental ACP#2772antonvishal wants to merge 2 commits into
antonvishal wants to merge 2 commits into
Conversation
|
Contributor
|
This PR is from an external contributor and must be approved by a stagehand team member with write access before CI can run. |
github-actions
Bot
added
external-contributor
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
antonvishal
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 25 files
Confidence score: 2/5
packages/integrations/grok-build/src/agent.tsmay print ACP or runtime failures containing API keys, CDP URLs, project IDs, or other secrets, creating a direct information-exposure risk; sanitize or redact messages beforehandleFailurereports them.packages/integrations/core/src/acp/facade-launcher.tscan expose an uncaught exception and stack trace when the server path is missing; replace it with a typed Stagehand error and controlled user-facing handling.packages/integrations/core/src/acp/env.tsduplicates theSTAGEHAND_/BROWSERBASE_filtering loop, so future allowlist changes could diverge between integrations; centralize the filtering in one shared helper.- Lower-risk follow-up remains in
packages/docs/v4/integrations/grok-build.mdx,packages/integrations/grok-build/README.md, andpackages/integrations/grok-build/tests/agent.test.ts: align the prose with the active-voice guide, usepnpm install --frozen-lockfile, and make the path assertion Windows-safe.
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/grok-build/README.md">
<violation number="1" location="packages/integrations/grok-build/README.md:10">
P2: The quickstart uses a non-reproducible install command. Replace `pnpm install` with `pnpm install --frozen-lockfile` so users get deterministic dependency resolution that matches the committed lockfile.
(Based on your team's feedback about frozen lockfile pnpm integration installs.)</violation>
</file>
<file name="packages/integrations/grok-build/tests/agent.test.ts">
<violation number="1" location="packages/integrations/grok-build/tests/agent.test.ts:115">
P3: On Windows, require.resolve() returns a backslash-separated path (e.g. C:\repo\node_modules\@xai-official\grok\bin\grok.cjs), so the forward-slash substring "@xai-official/grok/bin/grok" will never match and this test fails despite a valid executable. Normalize separators before asserting, for example by asserting against executable.replaceAll("\\", "/") or by splitting on the platform separator and joining with "/".</violation>
</file>
<file name="packages/docs/v4/integrations/grok-build.mdx">
<violation number="1" location="packages/docs/v4/integrations/grok-build.mdx:72">
P2: Custom agent: **Stagehand docs prose guide**
These added sentences use agentless passive voice for capability restrictions, contrary to the guide’s active-voice requirement. Name the responsible component: the profile denies capabilities, the client rejects permissions, and the launcher disables imports and does not load user or project resources.</violation>
</file>
<file name="packages/integrations/grok-build/src/agent.ts">
<violation number="1" location="packages/integrations/grok-build/src/agent.ts:239">
P1: Custom agent: **Exception and error message sanitization**
The Grok CLI prints ACP and runtime failure messages without sanitization. If an error includes an API key, CDP URL, project ID, or other secret, `handleFailure` exposes it verbatim; route the message through the shared sanitizer before writing it to stderr.</violation>
</file>
<file name="packages/integrations/core/src/acp/facade-launcher.ts">
<violation number="1" location="packages/integrations/core/src/acp/facade-launcher.ts:6">
P2: Custom agent: **Exception and error message sanitization**
When the launcher has no server path, this uncaught `new Error()` exposes a generic exception and stack trace to the user. Throw a typed Stagehand error class instead, as required by the error sanitization rule.</violation>
</file>
<file name="packages/integrations/core/src/acp/env.ts">
<violation number="1" location="packages/integrations/core/src/acp/env.ts:1">
P3: This adds a third copy of the same `STAGEHAND_`/`BROWSERBASE_` allowlist loop, which increases drift risk when the allowlist rules change. Reuse one shared helper so all integrations enforce the same filtering behavior.</violation>
</file>
Architecture diagram
sequenceDiagram
participant CLI as Grok CLI (user CLI)
participant ACP as ACP Client (Stagehand)
participant AGENT as Grok Agent Process
participant FACE as Facade MCP Server (Stagehand)
participant STAGE as Stagehand Core
participant BROWSER as Browser
Note over CLI,ACP: User runs: pnpm --dir packages/integrations/grok-build start -- "task"
CLI->>ACP: runGrokBuild(instruction)
ACP->>ACP: Create disposable runtime (home, cwd, GROK_HOME, agent profile)
ACP->>AGENT: Spawn grok CLI with ACP stdio + restricted tools
ACP->>AGENT: ACP initialize
AGENT-->>ACP: Initialize response with auth methods
alt XAI_API_KEY exists
ACP->>AGENT: Authenticate via xai.api_key
else cached auth.json only
ACP->>AGENT: Authenticate via cached_token
end
ACP->>AGENT: session.new with Stagehand facade as only MCP server
AGENT->>FACE: Launch facade stdio server (via launcher binary)
FACE->>STAGE: Initialize Stagehand (lazily start browser)
STAGE->>BROWSER: Start persistent browser session
ACP->>AGENT: Prompt with task + Stagehand instructions
AGENT->>AGENT: Discover MCP tools (run, snapshot, screenshot)
AGENT->>ACP: Permission request for Stagehand tool
alt Tool is stagehand__run/snapshot/screenshot
ACP->>AGENT: Allow once
else Other tool (shell, file, etc.)
ACP->>AGENT: Reject or cancel
end
AGENT->>FACE: MCP tool call (e.g., stagehand__run)
FACE->>STAGE: Execute browser automation
STAGE->>BROWSER: Navigate, act, snapshot, screenshot
BROWSER-->>STAGE: Results
STAGE-->>FACE: Tool result
FACE-->>AGENT: Tool response
AGENT-->>ACP: Stream text messages (agent updates)
ACP-->>CLI: Final assistant response
Note over ACP,AGENT: On abort/signal
ACP->>AGENT: ACP session.cancel
ACP->>AGENT: SIGTERM to process tree
opt No graceful exit
ACP->>AGENT: SIGKILL to process tree
end
ACP->>ACP: Clean up disposable runtime directories
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| function handleFailure(error: unknown): void { | ||
| // oxlint-disable-next-line no-console -- CLI example reports failures to stderr. | ||
| console.error(error instanceof Error ? error.message : error); |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
The Grok CLI prints ACP and runtime failure messages without sanitization. If an error includes an API key, CDP URL, project ID, or other secret, handleFailure exposes it verbatim; route the message through the shared sanitizer before writing it to stderr.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/grok-build/src/agent.ts, line 239:
<comment>The Grok CLI prints ACP and runtime failure messages without sanitization. If an error includes an API key, CDP URL, project ID, or other secret, `handleFailure` exposes it verbatim; route the message through the shared sanitizer before writing it to stderr.</comment>
<file context>
@@ -0,0 +1,241 @@
+
+function handleFailure(error: unknown): void {
+ // oxlint-disable-next-line no-console -- CLI example reports failures to stderr.
+ console.error(error instanceof Error ? error.message : error);
+ process.exitCode = 1;
+}
</file context>
| From the repository root: | ||
|
|
||
| ```bash | ||
| pnpm install |
Contributor
There was a problem hiding this comment.
P2: The quickstart uses a non-reproducible install command. Replace pnpm install with pnpm install --frozen-lockfile so users get deterministic dependency resolution that matches the committed lockfile.
(Based on your team's feedback about frozen lockfile pnpm integration installs.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/grok-build/README.md, line 10:
<comment>The quickstart uses a non-reproducible install command. Replace `pnpm install` with `pnpm install --frozen-lockfile` so users get deterministic dependency resolution that matches the committed lockfile.
(Based on your team's feedback about frozen lockfile pnpm integration installs.) </comment>
<file context>
@@ -0,0 +1,25 @@
+From the repository root:
+
+```bash
+pnpm install
+pnpm exec turbo run build --filter @browserbasehq/stagehand-integrations
+export XAI_API_KEY=xai-...
</file context>
Suggested change
| pnpm install | |
| pnpm install --frozen-lockfile |
| @@ -0,0 +1,96 @@ | |||
| --- | |||
Contributor
There was a problem hiding this comment.
P2: Custom agent: Stagehand docs prose guide
These added sentences use agentless passive voice for capability restrictions, contrary to the guide’s active-voice requirement. Name the responsible component: the profile denies capabilities, the client rejects permissions, and the launcher disables imports and does not load user or project resources.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/docs/v4/integrations/grok-build.mdx, line 72:
<comment>These added sentences use agentless passive voice for capability restrictions, contrary to the guide’s active-voice requirement. Name the responsible component: the profile denies capabilities, the client rejects permissions, and the launcher disables imports and does not load user or project resources.</comment>
<file context>
@@ -0,0 +1,96 @@
+
+The example opens one ACP session with Grok and mounts exactly one MCP server: the Stagehand facade. That server owns the browser for the full prompt turn, so `run`, `snapshot`, and `screenshot` share page state. Preserve that lifetime if you adapt the integration; a new process per tool call starts a new browser.
+
+Grok is restricted to a browser-only tool surface, matching the Claude and Codex examples. An agent profile allowlists only `search_tool` and `use_tool` (required to discover and invoke lazy MCP tools). Shell, file, plan, subagent, memory, and web-search capabilities are denied. The client approves one-time permission only for `stagehand__run`, `stagehand__snapshot`, and `stagehand__screenshot`; everything else is rejected or cancelled.
+
+Each run uses a disposable workspace, home directory, and `GROK_HOME`. Compatibility MCP imports from Claude/Cursor are disabled, and user or project MCP servers, plugins, hooks, skills, and rules are not loaded. A minimal launcher creates the actual facade runtime with non-empty `STAGEHAND_*` and `BROWSERBASE_*` variables plus basic OS values needed to launch Node and local Chrome; `XAI_API_KEY` and unrelated host secrets are unavailable to the facade. The ACP bridge terminates the full agent process tree on cancellation or failure.
</file context>
| import { buildAcpFacadeRuntimeEnv } from "./env.js"; | ||
|
|
||
| const [facadeServerPath, ...facadeArgs] = process.argv.slice(2); | ||
| if (!facadeServerPath) throw new Error("Stagehand ACP facade launcher requires a server path."); |
Contributor
There was a problem hiding this comment.
P2: Custom agent: Exception and error message sanitization
When the launcher has no server path, this uncaught new Error() exposes a generic exception and stack trace to the user. Throw a typed Stagehand error class instead, as required by the error sanitization rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/acp/facade-launcher.ts, line 6:
<comment>When the launcher has no server path, this uncaught `new Error()` exposes a generic exception and stack trace to the user. Throw a typed Stagehand error class instead, as required by the error sanitization rule.</comment>
<file context>
@@ -0,0 +1,31 @@
+import { buildAcpFacadeRuntimeEnv } from "./env.js";
+
+const [facadeServerPath, ...facadeArgs] = process.argv.slice(2);
+if (!facadeServerPath) throw new Error("Stagehand ACP facade launcher requires a server path.");
+
+// ACP agents may merge an MCP server's declared env with their own environment.
</file context>
| @@ -0,0 +1,36 @@ | |||
| export function buildAcpFacadeEnv(source: NodeJS.ProcessEnv = process.env): Record<string, string> { | |||
Contributor
There was a problem hiding this comment.
P3: This adds a third copy of the same STAGEHAND_/BROWSERBASE_ allowlist loop, which increases drift risk when the allowlist rules change. Reuse one shared helper so all integrations enforce the same filtering behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/acp/env.ts, line 1:
<comment>This adds a third copy of the same `STAGEHAND_`/`BROWSERBASE_` allowlist loop, which increases drift risk when the allowlist rules change. Reuse one shared helper so all integrations enforce the same filtering behavior.</comment>
<file context>
@@ -0,0 +1,36 @@
+export function buildAcpFacadeEnv(source: NodeJS.ProcessEnv = process.env): Record<string, string> {
+ const env: Record<string, string> = {};
+ for (const [key, value] of Object.entries(source)) {
</file context>
| it("resolves the packaged Grok executable", async () => { | ||
| const executable = resolveGrokExecutable(); | ||
| await expect(access(executable)).resolves.toBeUndefined(); | ||
| expect(executable).toContain("@xai-official/grok/bin/grok"); |
Contributor
There was a problem hiding this comment.
P3: On Windows, require.resolve() returns a backslash-separated path (e.g. C:\repo\node_modules@xai-official\grok\bin\grok.cjs), so the forward-slash substring "@xai-official/grok/bin/grok" will never match and this test fails despite a valid executable. Normalize separators before asserting, for example by asserting against executable.replaceAll("\", "/") or by splitting on the platform separator and joining with "/".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/grok-build/tests/agent.test.ts, line 115:
<comment>On Windows, require.resolve() returns a backslash-separated path (e.g. C:\repo\node_modules\@xai-official\grok\bin\grok.cjs), so the forward-slash substring "@xai-official/grok/bin/grok" will never match and this test fails despite a valid executable. Normalize separators before asserting, for example by asserting against executable.replaceAll("\\", "/") or by splitting on the platform separator and joining with "/".</comment>
<file context>
@@ -0,0 +1,212 @@
+ it("resolves the packaged Grok executable", async () => {
+ const executable = resolveGrokExecutable();
+ await expect(access(executable)).resolves.toBeUndefined();
+ expect(executable).toContain("@xai-official/grok/bin/grok");
+ });
+});
</file context>
Suggested change
| expect(executable).toContain("@xai-official/grok/bin/grok"); | |
| expect(executable.replaceAll("\\", "/")).toContain("@xai-official/grok/bin/grok"); |
Contributor
There was a problem hiding this comment.
2 issues found across 8 files (changes from recent commits).
Confidence score: 3/5
- In
packages/integrations/core/src/acp/agent-process.ts, raw Node spawn errors can be propagated throughrunAcpFacadeAgent, exposing unsanitized details to callers; sanitize or replace startup error messages before rethrowing. - In
packages/integrations/grok-build/src/agent.ts, Windows Git Bash environments may resolve.grokunder the wrong home directory and miss cached login state; preferUSERPROFILEon Windows and useHOMEonly as a fallback.
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/core/src/acp/agent-process.ts">
<violation number="1" location="packages/integrations/core/src/acp/agent-process.ts:27">
P1: Custom agent: **Exception and error message sanitization**
When ACP startup fails, `child.on("error", reject)` forwards Node's raw spawn error into `runAcpFacadeAgent`, which rethrows its message through generic `new Error(...)`. Wrap spawn failures in a dedicated typed error with a sanitized message before exposing the rejection.</violation>
</file>
<file name="packages/integrations/grok-build/src/agent.ts">
<violation number="1" location="packages/integrations/grok-build/src/agent.ts:61">
P2: On Windows, `HOME` can be a Git Bash path while `USERPROFILE` is the actual Windows home, so this selects the wrong `.grok` directory and misses cached login. Prefer `USERPROFILE` on Windows, with `HOME` as the fallback.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }); | ||
| const started = new Promise<void>((resolve, reject) => { | ||
| child.once("spawn", resolve); | ||
| child.on("error", reject); |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
When ACP startup fails, child.on("error", reject) forwards Node's raw spawn error into runAcpFacadeAgent, which rethrows its message through generic new Error(...). Wrap spawn failures in a dedicated typed error with a sanitized message before exposing the rejection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/acp/agent-process.ts, line 27:
<comment>When ACP startup fails, `child.on("error", reject)` forwards Node's raw spawn error into `runAcpFacadeAgent`, which rethrows its message through generic `new Error(...)`. Wrap spawn failures in a dedicated typed error with a sanitized message before exposing the rejection.</comment>
<file context>
@@ -21,6 +22,10 @@ export function spawnAcpAgentProcess(options: {
});
+ const started = new Promise<void>((resolve, reject) => {
+ child.once("spawn", resolve);
+ child.on("error", reject);
+ });
child.stderr.pipe(options.stderr, { end: false });
</file context>
| export function resolveGrokAuthHome(env: NodeJS.ProcessEnv): string | undefined { | ||
| const configured = env.GROK_HOME?.trim(); | ||
| if (configured) return configured; | ||
| const userHome = env.HOME?.trim() || env.USERPROFILE?.trim(); |
Contributor
There was a problem hiding this comment.
P2: On Windows, HOME can be a Git Bash path while USERPROFILE is the actual Windows home, so this selects the wrong .grok directory and misses cached login. Prefer USERPROFILE on Windows, with HOME as the fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/grok-build/src/agent.ts, line 61:
<comment>On Windows, `HOME` can be a Git Bash path while `USERPROFILE` is the actual Windows home, so this selects the wrong `.grok` directory and misses cached login. Prefer `USERPROFILE` on Windows, with `HOME` as the fallback.</comment>
<file context>
@@ -55,6 +55,13 @@ export function resolveGrokExecutable(): string {
+export function resolveGrokAuthHome(env: NodeJS.ProcessEnv): string | undefined {
+ const configured = env.GROK_HOME?.trim();
+ if (configured) return configured;
+ const userHome = env.HOME?.trim() || env.USERPROFILE?.trim();
+ return userHome ? join(userHome, ".grok") : undefined;
+}
</file context>
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.
What
Adds a Grok Build example that can use Stagehand through the existing facade.
Grok Build exposes ACP rather than a TypeScript SDK for driving the agent, so this adds a small ACP runner under
integrations/core. The Grok-specific package only handles its CLI setup, authentication, and tool mapping.The ACP process runs with the user's existing Grok login or
XAI_API_KEY. The Stagehand MCP process receives only the environment variables it needs.Question for maintainers
I put the ACP runner in
integrations/corebecause it is not specific to Grok another ACP-compatible agent could use it by providing its own profile.Does that feel like the right place for it?
If ACP is something we want to support as an integration path, I can also add a general ACP docs page. For this PR, I kept the documentation focused on Grok Build.
Testing
pnpm checkSummary by cubic
Adds a Grok Build integration that runs the Stagehand facade over ACP and introduces a reusable ACP runner in
@browserbasehq/stagehand-integrations. Grok Build now gets one persistent browser withrun,snapshot, andscreenshot, with strict env isolation and a hardened lifecycle.packages/integrations/core/src/acp(exported as@browserbasehq/stagehand-integrations/acp) that initializes/authenticates, mounts the facade as the only MCP server, approves only facade tool calls, cancels on abort, and terminates the full agent process tree if unresponsive (including on Windows).STAGEHAND_*andBROWSERBASE_*plus minimal OS runtime vars reach the facade; host secrets likeXAI_API_KEYnever do.packages/integrations/grok-buildthat resolves the packaged@xai-official/grokbinary, creates a disposable home/workspace and isolatedGROK_HOME, usesXAI_API_KEYif set or copies only cachedauth.jsonwhen available, deny-lists shell/file/subagent/plan/memory/web-search, and allowlistssearch_tool/use_tool.stagehand__run|snapshot|screenshot; all other tool requests are rejected or cancelled, and non-active-session permission prompts are cancelled.process.onfor repeated signal forwarding; the ACP bridge sends cancel, then SIGTERM/SIGKILL with a grace period, and cleans up descendants.@agentclientprotocol/sdkin core,@xai-official/grokin the example; workspace and Turbo tasks updated. To try locally: build@browserbasehq/stagehand-integrations, setXAI_API_KEY(or rungrok login), then run thepackages/integrations/grok-buildexample.Written for commit 304c403. Summary will update on new commits.