Files
shockbot/utils/browser.ts
T
Colin McDonnell 8e36f76cfa postrun: thread AgentRunContext through the retry loop instead of repackaging (#652)
* postrun: thread AgentRunContext through the retry loop instead of repackaging

drop the per-gate plumbing in `runPostRunRetryLoop`: the loop now receives
`ctx: AgentRunContext` whole and reads `ctx.stopScript` + `ctx.toolState.*`
directly. `getUnsubmittedReview` becomes a pure utility in postRun.ts
instead of a closure shipped over `AgentRunContext`. `AgentRunContext`
loses 4 fields that duplicated `toolState` (`summaryFilePath`,
`summarySeed`, `learningsFilePath`, `getUnsubmittedReview`) and gains
`toolState: ToolState`. both harness call sites collapse from 11 lines to
7; main.ts deletes the inline closure.

`ToolState` and friends move from `action/mcp/server.ts` to
`action/toolState.ts` so non-MCP code (agents, post-run loop) stops
importing run-state types from the MCP server module.

no behavior change. 503/503 tests green.

* toolState: relocate `CommentableLines` to break dep cycle with mcp/review

`action/toolState.ts` was importing `CommentableLines` from
`mcp/review.ts`, which pulled the entire MCP server compile graph (24
files) into any consumer of `ToolState` — including `cf-worker-indexing`
via the `pullfrog/internal` re-export chain through `utils/log.ts` →
`agents/shared.ts` → `toolState.ts`. that exposed a pre-existing TS
error in `mcp/issueEvents.ts` (octokit types resolve differently under
cf-worker's `moduleResolution: bundler`).

move `CommentableLines` (a small `{ RIGHT: Set<number>; LEFT: Set<number> }`
state-shape type) to `toolState.ts` where it's used; re-export from
`mcp/review.ts` for back-compat with test and call-site imports. cuts
cf-worker's mcp/ compile inclusion from 24 files back to 0.

* postRun: drop mock-heavy retry-loop tests; keep pure gate predicate

`runPostRunRetryLoop` and `executeStopHook` were covered by ~560 lines
of mock-heavy regression-gate tests that stubbed `spawn` / `getGitStatus`
and fabricated `AgentRunContext` to drive orchestration paths. per
AGENTS.md ("prefer no test over a mock-heavy test that only catches the
most obvious form of regression") and the empirical track record — the
one real production failure of this code path (#646) was a missing npm
release, not a logic bug a unit test could catch — the value-to-ceremony
ratio is poor. delete them.

keep only the pure predicate: `getUnsubmittedReview(toolState)` is a
decision function whose four input conditions have user-visible
consequences when wrong. 5 assertions, no mocks, no ctx fabrication.

488 tests still pass.

* toolState: import PrepResult from prep/types.ts, not the barrel

same dep-cycle class as the previous CommentableLines fix. importing
PrepResult from prep/index.ts pulled prep/installNodeDependencies.ts
into the Next.js production build's typecheck graph (via
pullfrog/internal → utils/log.ts → agents/shared.ts → toolState.ts →
prep/index.ts → installNodeDependencies.ts), and Next.js's stricter
NODE_ENV-required ProcessEnv shape rejected an existing
`env: { PATH: ... }` literal.

prep/types.ts is a leaf module with zero imports — re-routing the type
import severs the chain. Vercel preview deploy goes from Error → Ready;
preview-sync stops racing the deploy.
2026-05-11 18:47:08 +00:00

134 lines
4.5 KiB
TypeScript

import { execFileSync, spawnSync } from "node:child_process";
import { existsSync } from "node:fs";
import { dirname } from "node:path";
import type { ToolState } from "../toolState.ts";
import { log } from "./cli.ts";
import { filterEnv } from "./secrets.ts";
import { getDevDependencyVersion } from "./version.ts";
// agent-browser already discovers chrome via `which` and the playwright cache as fallbacks,
// so this list only needs to cover the GHA ubuntu-latest runner where we know the exact path.
const CHROME_PATHS = ["/usr/bin/google-chrome-stable"];
let systemChromePath: string | undefined;
function findSystemChromePath(): string | undefined {
if (typeof systemChromePath === "string") {
// return cached result but normalize to undefined if empty
return systemChromePath || undefined;
}
for (const p of CHROME_PATHS) {
if (existsSync(p)) {
systemChromePath = p;
log.info(`found system chrome: ${p}`);
return p;
}
}
// set to an empty string to indicate no system chrome found
// and to avoid repeated lookups
systemChromePath = "";
log.info(`no system chrome found (checked: ${CHROME_PATHS.join(", ")})`);
}
function buildEnv(): Record<string, string> {
const env: Record<string, string> = { ...filterEnv() };
const chromePath = findSystemChromePath();
if (chromePath) {
env.AGENT_BROWSER_EXECUTABLE_PATH = chromePath;
}
return env;
}
/**
* ensure the agent-browser daemon is running by issuing a real command.
*
* agent-browser is stateful — it manages a persistent browser process via a
* daemon that communicates over a Unix socket. we start the daemon here,
* outside of ShellTool, because ShellTool's child process lifecycle would
* kill it between invocations and the daemon must survive across calls.
*
* despite ShellTool commands running inside unshare-sandboxed namespaces,
* they can still reach this daemon because the Unix socket is discoverable
* regardless of unshare's PID/mount isolation. starting the daemon in the
* host namespace keeps it alive while sandboxed shells come and go.
*
* agent-browser auto-starts its daemon on the first CLI invocation and
* keeps it alive via the socket for subsequent commands.
* we run `open about:blank` as the seed command to trigger this.
* idempotent — only runs once.
*/
export function ensureBrowserDaemon(toolState: ToolState): string | undefined {
if (toolState.browserDaemon) {
return toolState.browserDaemon.error;
}
const agentBrowserVersion = getDevDependencyVersion("agent-browser");
log.info(`installing agent-browser@${agentBrowserVersion}...`);
const install = spawnSync("npm", ["install", "-g", `agent-browser@${agentBrowserVersion}`], {
stdio: "pipe",
encoding: "utf-8",
});
if (install.status !== 0) {
const error = `agent-browser install failed: ${(install.stderr || install.stdout || "unknown error").trim()}`;
log.error(error);
toolState.browserDaemon = { error };
return error;
}
log.info("agent-browser installed");
let binDir: string;
try {
const binPath = execFileSync("which", ["agent-browser"], { encoding: "utf-8" }).trim();
binDir = dirname(binPath);
log.info(`agent-browser binary: ${binPath}`);
} catch {
const error = "agent-browser binary not found in PATH after install";
log.error(error);
toolState.browserDaemon = { error };
return error;
}
const env = buildEnv();
// `open about:blank` triggers daemon auto-start and returns once the daemon + browser are ready
log.info("starting browser daemon...");
const seed = spawnSync("agent-browser", ["open", "about:blank"], {
env,
stdio: "pipe",
encoding: "utf-8",
timeout: 30_000,
});
if (seed.status !== 0) {
const output = (seed.stderr || seed.stdout || "unknown error").trim();
const error = `agent-browser open about:blank failed (exit=${seed.status}): ${output}`;
log.error(error);
toolState.browserDaemon = { error };
return error;
}
log.debug(`seed command done (exit=0): ${(seed.stdout || "").trim()}`);
toolState.browserDaemon = { binDir };
log.info("browser daemon ready");
}
export function closeBrowserDaemon(toolState: ToolState): void {
if (!toolState.browserDaemon?.binDir) {
delete toolState.browserDaemon;
return;
}
delete toolState.browserDaemon;
try {
log.info("closing browser daemon...");
spawnSync("agent-browser", ["close"], {
env: filterEnv(),
stdio: "pipe",
timeout: 10_000,
});
log.info("browser daemon closed");
} catch {
// best-effort
}
}