From 8ebaf0da6eafb25e76c0d46fe0ebd8dedc8d3db5 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sun, 11 Oct 2026 07:03:45 +0300 Subject: [PATCH] wave E stage 3: demos run in the app's own Chromium (browser-service helper) Demos execute JS and need a browser; the app's own binary now provides one: main.cjs boots a hidden window + CDP port under --browser-service (before the single-instance lock - a helper is deliberately a second instance of this binary), lifetime tied to the server's stdin (EOF = quit, no orphans). headless.ts connects over CDP as PRIMARY (owner ruling 2026-10-10), system Chrome stays the fallback, graceful refusal when neither. Electron's CDP has no Target.createTarget, so the helper's pre-created window is THE page, reused and reset per job behind a serialized queue. PDFs and SVGs never reach a browser anymore (native since stages 1-2); stale Chrome wording in model-facing tool copy made engine-neutral. Live-proofed end-to-end from the dev tree (helper seen by process watch, js-ran 42 probe, no orphan). Gates 1523/1523. --- electron/main.cjs | 74 +++++++++--- src/agent/subagents/demo-builder.ts | 7 +- src/agent/subagents/headless.ts | 176 +++++++++++++++++++++++++--- src/agent/subagents/pdf-reader.ts | 2 +- src/agent/subagents/visual-maker.ts | 5 +- tests/browser-service.test.ts | 42 +++++++ 6 files changed, 266 insertions(+), 40 deletions(-) create mode 100644 tests/browser-service.test.ts diff --git a/electron/main.cjs b/electron/main.cjs index 6fa4216..c634f10 100644 --- a/electron/main.cjs +++ b/electron/main.cjs @@ -57,25 +57,63 @@ function serverEntry() { } // ─── Single instance: two instances would fight over the data dir ───────── -// Dev shell (LH_DEV_URL) runs BESIDE the packaged app, not instead of it: -// its own userData so (a) the single-instance lock — checked below — doesn't -// silently kill it, and (b) it never reads/writes the real app's -// localStorage, auth.json, or logs. Must run before the lock check. -if (process.env.LH_DEV_URL) { - app.setPath("userData", `${app.getPath("userData")}-dev`); -} - -if (!app.requestSingleInstanceLock()) { - app.quit(); -} else { - app.on("second-instance", () => { - const win = BrowserWindow.getAllWindows()[0]; - if (win) { - if (win.isMinimized()) win.restore(); - win.focus(); +// ─── Browser-service helper (Wave E stage 3, owner 2026-10-10) ──────── +// The render engines for agent jobs: PDFs and SVGs rasterize natively +// (pdfjs+Skia, resvg — no browser); DEMOS execute JS and need one. The +// app's own Chromium serves that via this helper — the server spawns +// --browser-service +// and connects over CDP (system Chrome stays the fallback). Handshake: +// the helper prints "BROWSER_SERVICE_CDP " on stdout once the CDP +// endpoint listens. Lifetime: stdin stays open; parent death = EOF = quit. +// MUST run before the single-instance lock — a helper is a second +// instance of this very binary, and before the LH_DEV_URL redirect — the +// helper owns no user data at all. +if (process.argv.includes("--browser-service")) { + (async () => { + try { + app.dock?.hide(); + const port = await freePort(); + // Must land before app-ready for Chromium to pick it up. + app.commandLine.appendSwitch("remote-debugging-port", String(port)); + await app.whenReady(); + // A hidden window keeps the app lifecycle ordinary on every + // platform (no window-all-closed handler is installed here). + const win = new BrowserWindow({ show: false }); + await win.loadURL("about:blank"); + console.log(`BROWSER_SERVICE_CDP ${port}`); + // Parent-tied lifetime: the server holds our stdin open. + process.stdin.resume(); + process.stdin.on("end", () => app.quit()); + process.stdin.on("close", () => app.quit()); + } catch (err) { + console.error(`browser-service failed: ${err?.stack ?? err}`); + app.exit(1); } - }); - main(); + })(); +} else { + // Dev shell (LH_DEV_URL) runs BESIDE the packaged app, not instead of it: + // its own userData so (a) the single-instance lock — checked below — doesn't + // silently kill it, and (b) it never reads/writes the real app's + // localStorage, auth.json, or logs. Must run before the lock check. + if (process.env.LH_DEV_URL) { + app.setPath("userData", `${app.getPath("userData")}-dev`); + } + + // The whole normal boot — lock included — lives in this else: a helper is + // a SECOND instance of this very binary on purpose (the running app owns + // the lock), so it must never reach this check. + if (!app.requestSingleInstanceLock()) { + app.quit(); + } else { + app.on("second-instance", () => { + const win = BrowserWindow.getAllWindows()[0]; + if (win) { + if (win.isMinimized()) win.restore(); + win.focus(); + } + }); + main(); + } } let serverProcess = null; diff --git a/src/agent/subagents/demo-builder.ts b/src/agent/subagents/demo-builder.ts index 8be94cd..1d203df 100644 --- a/src/agent/subagents/demo-builder.ts +++ b/src/agent/subagents/demo-builder.ts @@ -16,7 +16,8 @@ import { healToolRecord } from "../tools/heal.ts"; * * Shape: a ToolLoopAgent with two executable tools (write_demo / run_demo) — * the engine is injectable (tests use a scripted faux engine) and the headless - * API is injectable (tests use a fake; production uses system Chrome). + * API is injectable (tests use a fake; production uses the app's own + * Chromium browser service, system Chrome as fallback). * * Failure semantics (decided in the W6 brief): `tested: true` means "the * verify loop ran"; on failure the result is { title, html: "", tested: true, @@ -64,7 +65,7 @@ Build quality: THE HOUSE RULE — never deliver untested: 1. Call write_demo with the complete HTML document. -2. Call run_demo. It loads the file in headless Chrome, waits ~2s, and reports console errors, uncaught +2. Call run_demo. It loads the file in a headless browser, waits ~2s, and reports console errors, uncaught errors (window.onerror), and failed requests. Optionally pass a probe: a JS expression evaluated in the page (e.g. "typeof window.simulate === 'function'" or a state assertion) to check deeper. 3. If run_demo reports problems, FIX the document and call write_demo again, then run_demo again. @@ -119,7 +120,7 @@ export function createDemoBuilderTools( name: "run_demo", label: "run_demo", description: - "Headlessly execute the current demo in Chrome and report console errors, uncaught errors, and failed requests. " + + "Headlessly execute the current demo in a browser and report console errors, uncaught errors, and failed requests. " + "Optionally pass a probe expression to assert on page state.", parameters: runDemoInput, execute: serverExecute("run_demo", runDemoInput, async ({ probe }) => { diff --git a/src/agent/subagents/headless.ts b/src/agent/subagents/headless.ts index 0b8c93d..3c66d13 100644 --- a/src/agent/subagents/headless.ts +++ b/src/agent/subagents/headless.ts @@ -1,4 +1,5 @@ import * as fs from "node:fs"; +import { createRequire } from "node:module"; import * as os from "node:os"; import * as path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -104,10 +105,150 @@ export function resolveChromeExecutablePath(): string | null { } const NO_CHROME_MESSAGE = - "No Chrome/Chromium found for headless testing. Install Google Chrome or set CHROME_PATH to a Chrome executable. " + - "(Verification is mandatory — demos and visuals are never delivered untested.)"; + "No render engine available: the app's own browser helper failed to start and no system Chrome/Chromium was found. " + + "Install Google Chrome or set CHROME_PATH to a Chrome executable. " + + "(Verification is mandatory — demos are never delivered untested.)"; -async function launchBrowser(extraArgs: string[] = []): Promise { +// ─── The app's own Chromium as a browser service (Wave E stage 3) ──────────── +// Demos execute JS and need a browser. PRIMARY: this very app's binary +// spawned with --browser-service (main.cjs boots a hidden window with a CDP +// port and prints "BROWSER_SERVICE_CDP "). FALLBACK: system Chrome. +// The helper's stdin is held open by this process; server death = EOF = the +// helper quits — no orphans. PDFs and SVGs never come here (native now). + +/** Handshake line parser (pure; exported for tests). */ +export function parseBrowserServiceLine(line: string): number | null { + const m = line.match(/^BROWSER_SERVICE_CDP (\d+)$/); + if (!m) return null; + const port = Number(m[1]); + return port >= 1 && port <= 65535 ? port : null; +} + +/** Spawn env sanitizer (pure; exported for tests): the helper must run as a + * FULL Electron app — ELECTRON_RUN_AS_NODE must not leak — and must not + * inherit the dev-shell userData redirect. */ +export function browserServiceSpawnEnv( + env: Record, +): Record { + const out: Record = { ...env }; + delete out.ELECTRON_RUN_AS_NODE; + delete out.LH_DEV_URL; + return out; +} + +/** The helper binary: inside the packaged app the server runs under + * ELECTRON_RUN_AS_NODE so process.execPath IS the app binary; in dev + * (plain node) the electron package resolves to the dev binary path. */ +export function browserServiceBinary(): string | null { + if (process.versions.electron) return process.execPath; + try { + const req = createRequire(import.meta.url); + const p = req("electron") as unknown as string; + return typeof p === "string" && p.length > 0 ? p : null; + } catch { + return null; + } +} + +const BROWSER_SERVICE_TIMEOUT_MS = 15_000; +let browserServiceProc: import("node:child_process").ChildProcess | null = null; + +async function connectBrowserService(): Promise { + const puppeteer = (await import("puppeteer-core")).default; + const { spawn } = await import("node:child_process"); + let port: number | null = null; + + if (browserServiceProc && browserServiceProc.exitCode === null) { + // Helper alive from a previous run — reuse its endpoint. + port = (browserServiceProc as { __cdpPort?: number }).__cdpPort ?? null; + } + if (port === null) { + const binary = browserServiceBinary(); + if (!binary) return null; + // Packaged: execPath runs the bundled app (flag reaches main.cjs). + // Dev: the bare electron binary needs the repo root as the app arg — + // flagless it would open Electron's default app instead of our shell. + const appArgs = process.versions.electron ? ["--browser-service"] : [ + // src/agent/subagents → repo root + path.join(path.dirname(fileURLToPath(import.meta.url)), "..", "..", ".."), + "--browser-service", + ]; + const proc = spawn(binary, appArgs, { + // Cast: the sanitizer is pure over a loose record; NODE_ENV is + // always present in a real process.env. + env: browserServiceSpawnEnv(process.env) as NodeJS.ProcessEnv, + stdio: ["pipe", "pipe", "pipe"] as const, + }); + browserServiceProc = proc; + port = await new Promise((resolve) => { + let buf = ""; + const timer = setTimeout(() => { + proc.kill(); + resolve(null); + }, BROWSER_SERVICE_TIMEOUT_MS); + const onData = (chunk: Buffer) => { + buf += chunk.toString("utf8"); + const line = buf.split("\n").find((l) => parseBrowserServiceLine(l) !== null); + if (line !== undefined) { + clearTimeout(timer); + resolve(parseBrowserServiceLine(line)); + } + }; + proc.stdout.on("data", onData); + proc.once("exit", () => { + clearTimeout(timer); + resolve(null); + }); + }); + if (port === null) { + browserServiceProc = null; + return null; + } + (proc as { __cdpPort?: number }).__cdpPort = port; + } + + try { + const version = await fetch(`http://127.0.0.1:${port}/json/version`); + const info = (await version.json()) as { webSocketDebuggerUrl?: string }; + if (!info.webSocketDebuggerUrl) return null; + // connect ≠ launch: browser.close() below merely disconnects — the + // helper stays resident for the next job (stdin-held lifetime). + return await puppeteer.connect({ + browserWSEndpoint: info.webSocketDebuggerUrl, + defaultViewport: null, + }); + } catch { + return null; + } +} + +/** Serialized helper-page use: the helper owns ONE page target (Electron's + * CDP has no Target.createTarget — pages must be windows the helper made), + * so concurrent jobs queue here instead of fighting over one webContents. */ +let helperPageChain: Promise = Promise.resolve(); + +async function withBrowserPage(fn: (page: import("puppeteer-core").Page) => Promise): Promise { + // Helper first (owner ruling 2026-10-10: the app's own Chromium primary, + // system Chrome fallback) — deterministic across users, zero install. + const helper = await connectBrowserService(); + if (helper) { + const run = async (): Promise => { + const pages = await helper.pages(); + const page = pages.find((p) => p.target().type() === "page"); + if (!page) throw new Error("browser service: no page target"); + try { + return await fn(page); + } finally { + // Drop all page state (JS heap, listeners) between jobs. + await page.goto("about:blank").catch(() => {}); + } + }; + const next = helperPageChain.then(run, run); + helperPageChain = next.catch(() => {}); + return next; + } + + // Chrome fallback: per-job launch, close on the way out (old semantics). const puppeteer = (await import("puppeteer-core")).default; const executablePath = resolveChromeExecutablePath(); const browser = await puppeteer.launch({ @@ -115,9 +256,14 @@ async function launchBrowser(extraArgs: string[] = []): Promise {}); + } } /** A temp dir per invocation; removed in finally. */ @@ -139,9 +285,8 @@ async function runDemo(html: string, opts: RunDemoOptions = {}): Promise { page.on("console", (msg) => { if (msg.type() === "error") consoleErrors.push(msg.text()); }); @@ -162,8 +307,8 @@ async function runDemo(html: string, opts: RunDemoOptions = {}): Promise {}); try { fs.rmSync(dir, { recursive: true, force: true }); } catch { @@ -289,9 +434,7 @@ async function renderSvg(svg: string, opts: RenderSvgOptions = {}): Promise { const { width, height } = svgViewport(svg, opts); await page.setViewport({ width, height }); await page.goto(pathToFileURL(file).href, { waitUntil: "load", timeout: 30_000 }); @@ -300,14 +443,13 @@ async function renderSvg(svg: string, opts: RenderSvgOptions = {}): Promise {}); + }).finally(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort cleanup */ } - } + }); } // ─── PDF page rasterization (pdfjs paints, Skia hosts) ───────────────── @@ -381,10 +523,12 @@ function PATHS_nodeModules(): string { export const realHeadless: HeadlessApi = { available() { - return resolveChromeExecutablePath() !== null; + // The helper binary exists by construction in the packaged app; in dev + // it resolves via the electron package. System Chrome widens coverage. + return browserServiceBinary() !== null || resolveChromeExecutablePath() !== null; }, unavailableReason() { - return resolveChromeExecutablePath() === null ? NO_CHROME_MESSAGE : null; + return browserServiceBinary() === null && resolveChromeExecutablePath() === null ? NO_CHROME_MESSAGE : null; }, runDemo, renderSvg, diff --git a/src/agent/subagents/pdf-reader.ts b/src/agent/subagents/pdf-reader.ts index eebe8b3..82f6307 100644 --- a/src/agent/subagents/pdf-reader.ts +++ b/src/agent/subagents/pdf-reader.ts @@ -167,7 +167,7 @@ export async function runPdfReaderAgent( const maxIterations = opts.maxIterations ?? MAX_ITERATIONS; const pdfPath = input.path; - // Deterministic upfront failure: unreadable / unparseable / no Chrome + // Deterministic upfront failure: unreadable / unparseable // for renders — the brief never starts on a broken source. const openError = await probeReadable(pdfPath); if (openError) { diff --git a/src/agent/subagents/visual-maker.ts b/src/agent/subagents/visual-maker.ts index 6aede8b..8fe68ff 100644 --- a/src/agent/subagents/visual-maker.ts +++ b/src/agent/subagents/visual-maker.ts @@ -8,7 +8,8 @@ import { getHeadless, type HeadlessApi } from "./headless.ts"; /** * The visual-maker sub-agent (vision §2.5): authors ONE precise SVG from a - * brief, renders it to a PNG in the SAME headless Chrome, LOOKS at the + * brief, renders it to a PNG with the same rasterizer the transcript + * will use, LOOKS at the * result, and iterates until correct — a direct port of the vault POC's * svg-maker doctrine ("the one rule that matters: verify by looking"). * @@ -64,7 +65,7 @@ Your three tools: - edit_svg({ old_text, new_text }) — a surgical exact-match replacement in the current SVG: old_text must appear EXACTLY ONCE; it is replaced by new_text. Call write_svg first. For anything beyond a one-spot fix (new elements, restructured layout), write_svg the whole document again. -- render_svg({}) — rasterize the current SVG in headless Chrome. The render arrives as an image in the +- render_svg({}) — rasterize the current SVG to a PNG. The render arrives as an image in the message right after the tool result. THE ONE RULE THAT MATTERS MOST: verify by looking. You are done only when you have LOOKED at the diff --git a/tests/browser-service.test.ts b/tests/browser-service.test.ts new file mode 100644 index 0000000..80568f9 --- /dev/null +++ b/tests/browser-service.test.ts @@ -0,0 +1,42 @@ +/** + * Wave E stage 3 (owner 2026-10-10): demos run in the app's own Chromium + * via a --browser-service helper (main.cjs boots a hidden window with a CDP + * port), system Chrome as fallback. These pin the pure halves of that + * chain: the handshake line parser and the spawn-env sanitizer. The live + * chain (spawn → connect → pages() → run) is exercised on dev machines, + * not in the suite. + */ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { browserServiceSpawnEnv, parseBrowserServiceLine } from "../src/agent/subagents/headless.ts"; + +describe("browser-service helper (pure halves)", () => { + it("parses the handshake line into a port", () => { + assert.equal(parseBrowserServiceLine("BROWSER_SERVICE_CDP 54321"), 54321); + }); + + it("rejects non-handshake lines", () => { + assert.equal(parseBrowserServiceLine("DevTools listening on ws://127.0.0.1:57983/devtools/browser/x"), null); + assert.equal(parseBrowserServiceLine(""), null); + assert.equal(parseBrowserServiceLine("BROWSER_SERVICE_CDP 0"), null); + assert.equal(parseBrowserServiceLine("BROWSER_SERVICE_CDP 99999"), null); + }); + + it("spawn env strips ELECTRON_RUN_AS_NODE — the helper must be a full app", () => { + const env = browserServiceSpawnEnv({ + ELECTRON_RUN_AS_NODE: "1", + LH_DEV_URL: "http://localhost:3000", + PATH: "/usr/bin", + }); + assert.equal("ELECTRON_RUN_AS_NODE" in env, false); + assert.equal("LH_DEV_URL" in env, false); + assert.equal(env.PATH, "/usr/bin"); + }); + + it("spawn env never mutates the caller's env object", () => { + const original = { ELECTRON_RUN_AS_NODE: "1", KEEP: "yes" }; + browserServiceSpawnEnv(original); + assert.equal(original.ELECTRON_RUN_AS_NODE, "1"); + assert.equal(original.KEEP, "yes"); + }); +});