From 4ca87f7d2eb0b08107656c4790ea6f35a24bd192 Mon Sep 17 00:00:00 2001 From: maximus792 <08093.claver@gmail.com> Date: Mon, 7 Sep 2026 14:35:50 +0200 Subject: [PATCH 1/3] feat(computer): add optional headed browser mode --- .env.example | 6 +- agent-computer/src/browser-mode.ts | 10 ++ agent-computer/src/index.ts | 15 +++ agent-computer/src/profiles.ts | 3 + agent-computer/src/virtual-display.ts | 103 +++++++++++++++++++ agent-computer/tests/browser-mode.test.ts | 20 ++++ agent-computer/tests/headed-browser.test.ts | 86 ++++++++++++++++ agent-computer/tests/virtual-display.test.ts | 53 ++++++++++ docker-compose.yml | 5 + docs/configuration.md | 1 + supervisor/src/environment.ts | 27 +++++ supervisor/src/index.ts | 34 +----- supervisor/tests/environment.test.ts | 23 +++++ 13 files changed, 352 insertions(+), 34 deletions(-) create mode 100644 agent-computer/src/browser-mode.ts create mode 100644 agent-computer/src/virtual-display.ts create mode 100644 agent-computer/tests/browser-mode.test.ts create mode 100644 agent-computer/tests/headed-browser.test.ts create mode 100644 agent-computer/tests/virtual-display.test.ts create mode 100644 supervisor/src/environment.ts create mode 100644 supervisor/tests/environment.test.ts diff --git a/.env.example b/.env.example index 7b8f2a28b..6d46581f2 100644 --- a/.env.example +++ b/.env.example @@ -258,6 +258,11 @@ COMPUTER_TOKEN= # docker-compose.yml. Pointing it somewhere unmounted disables login persistence. # PROFILES_DIR=/profiles +# Browser process used by a Bot's computer. `headless` preserves the smaller default. `headed` runs +# full Chromium on a private virtual display; the existing live screen and take-the-wheel controls +# are still how a person sees and drives it. +# COMPUTER_BROWSER_MODE=headless + # Which Bot this computer belongs to, and therefore which profile directory it uses. # COMPUTER_BOT_ID=shared @@ -333,4 +338,3 @@ AGENT_TOOL_TOKEN= # for a deployment that has not stood up a worker. Set for one that has: openssl rand -base64 32. # Do not accept a default in production. WORKER_SHARED_SECRET= - diff --git a/agent-computer/src/browser-mode.ts b/agent-computer/src/browser-mode.ts new file mode 100644 index 000000000..fa8f7f513 --- /dev/null +++ b/agent-computer/src/browser-mode.ts @@ -0,0 +1,10 @@ +export type BrowserMode = "headless" | "headed"; + +/** Choose the browser process an operator asked for, preserving the existing default. */ +export function browserModeFromEnv(raw: string | undefined): BrowserMode { + const mode = raw?.trim() || "headless"; + if (mode === "headless" || mode === "headed") return mode; + throw new Error( + `COMPUTER_BROWSER_MODE must be headless or headed, not ${JSON.stringify(mode)}.`, + ); +} diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index 341fcc349..6f37ffb55 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -1,6 +1,7 @@ import { serve } from "bun"; import type { Page } from "playwright"; import { parseAriaSnapshot, type SnapshotElement } from "./aria-snapshot"; +import { browserModeFromEnv } from "./browser-mode"; import { actsOnTheComputer, isOpenPath, @@ -21,6 +22,7 @@ import { createProfiles, numberFromEnv, VIEWPORT } from "./profiles"; import { type InputMessage, startScreencast } from "./screencast"; import { createShell } from "./shell"; import { createViewerSlot, type ViewerSlot } from "./viewer"; +import { startVirtualDisplay } from "./virtual-display"; import { createWorkspace, WorkspaceFileError, @@ -72,6 +74,17 @@ if (!COMPUTER_TOKEN) { process.exit(1); } +const BROWSER_MODE = browserModeFromEnv(process.env.COMPUTER_BROWSER_MODE); +const VIRTUAL_DISPLAY = await startVirtualDisplay(BROWSER_MODE); +if (VIRTUAL_DISPLAY) process.env.DISPLAY = VIRTUAL_DISPLAY.name; +console.info( + JSON.stringify({ + type: "computer-browser-mode", + mode: BROWSER_MODE, + display: VIRTUAL_DISPLAY?.name ?? null, + }), +); + const PORT = numberFromEnv("PORT", 4100); const NAVIGATION_TIMEOUT_MS = numberFromEnv("NAVIGATION_TIMEOUT_MS", 30000); @@ -749,6 +762,7 @@ serve({ // deployment without it, not a failure, and it is reported rather than omitted so the // difference between "no identity here" and "identity broken" is visible. identity: await identity(), + browserMode: BROWSER_MODE, }); } @@ -1233,6 +1247,7 @@ for (const signal of ["SIGTERM", "SIGINT"] as const) { void (async () => { console.info(`${signal}: closing the browser so its profile is flushed`); await profiles.closeAll(); + await VIRTUAL_DISPLAY?.stop(); process.exit(0); })(); }); diff --git a/agent-computer/src/profiles.ts b/agent-computer/src/profiles.ts index c97f041c7..0d55cfc47 100644 --- a/agent-computer/src/profiles.ts +++ b/agent-computer/src/profiles.ts @@ -37,6 +37,7 @@ import { readdir, rm } from "node:fs/promises"; import { join } from "node:path"; import { type BrowserContext, chromium, type Page } from "playwright"; import { profileDirectoryFor } from "./bot-id"; +import { browserModeFromEnv } from "./browser-mode"; import { chooseEvictions, chooseIdle } from "./browser-eviction"; import { egressFor, egressLabel } from "./egress"; import { numberFromEnv, settleWithin } from "./env"; @@ -91,6 +92,7 @@ const SINGLETON_FILES = ["SingletonLock", "SingletonSocket", "SingletonCookie"]; * whether the browser rendering the open internet is sandboxed. */ const SANDBOX_ENABLED = process.env.COMPUTER_SANDBOX === "on"; +const BROWSER_MODE = browserModeFromEnv(process.env.COMPUTER_BROWSER_MODE); const LAUNCH_ARGS = [ ...(SANDBOX_ENABLED ? [] : ["--no-sandbox"]), @@ -380,6 +382,7 @@ export function createProfiles(root: string, onClosed: BrowserClosed) { await sweepLocks(dir); const proxy = egressFor(botId, process.env); const context = await chromium.launchPersistentContext(dir, { + headless: BROWSER_MODE === "headless", args: LAUNCH_ARGS, // Playwright launches with `--enable-automation`, which sets `navigator.webdriver` and the // "controlled by automated software" banner. Dropped for the same reason as the flag above: diff --git a/agent-computer/src/virtual-display.ts b/agent-computer/src/virtual-display.ts new file mode 100644 index 000000000..03fbf618e --- /dev/null +++ b/agent-computer/src/virtual-display.ts @@ -0,0 +1,103 @@ +import { spawn } from "node:child_process"; +import { access } from "node:fs/promises"; +import type { BrowserMode } from "./browser-mode"; + +export type DisplayProcess = { + exited: Promise; + kill: (signal?: NodeJS.Signals) => boolean; +}; + +export type DisplayRuntime = { + spawn: (command: string, args: string[]) => DisplayProcess; + ready: (socketPath: string) => Promise; + wait: (milliseconds: number) => Promise; +}; + +export type VirtualDisplay = { + name: string; + stop: () => Promise; +}; + +const DISPLAY = ":99"; +const SOCKET = "/tmp/.X11-unix/X99"; +const READY_BUDGET_MS = 5_000; +const POLL_MS = 25; +const STOP_BUDGET_MS = 2_000; + +const systemRuntime: DisplayRuntime = { + spawn(command, args) { + const child = spawn(command, args, { + stdio: ["ignore", "ignore", "inherit"], + }); + const exited = new Promise((resolve) => { + child.once("exit", (code) => resolve(code ?? 1)); + child.once("error", () => resolve(1)); + }); + return { exited, kill: (signal) => child.kill(signal) }; + }, + ready: async (socketPath) => { + try { + await access(socketPath); + return true; + } catch { + return false; + } + }, + wait: (milliseconds) => + new Promise((resolve) => setTimeout(resolve, milliseconds)), +}; + +async function stop( + process: DisplayProcess, + runtime: DisplayRuntime, +): Promise { + let exited = false; + void process.exited.then(() => { + exited = true; + }); + process.kill("SIGTERM"); + await Promise.race([process.exited, runtime.wait(STOP_BUDGET_MS)]); + if (!exited) { + process.kill("SIGKILL"); + await process.exited; + } +} + +/** Start the one local-only X display a headed computer needs. */ +export async function startVirtualDisplay( + mode: BrowserMode, + runtime: DisplayRuntime = systemRuntime, +): Promise { + if (mode === "headless") return null; + + const process = runtime.spawn("Xvfb", [ + DISPLAY, + "-screen", + "0", + "1280x800x24", + "-nolisten", + "tcp", + "-ac", + ]); + let exitCode: number | undefined; + void process.exited.then((code) => { + exitCode = code; + }); + + for (let waited = 0; waited < READY_BUDGET_MS; waited += POLL_MS) { + if (await runtime.ready(SOCKET)) { + return { name: DISPLAY, stop: () => stop(process, runtime) }; + } + if (exitCode !== undefined) { + throw new Error( + `The virtual display exited before it became ready (exit ${exitCode}).`, + ); + } + await runtime.wait(POLL_MS); + } + + await stop(process, runtime); + throw new Error( + `The virtual display did not become ready within ${READY_BUDGET_MS}ms.`, + ); +} diff --git a/agent-computer/tests/browser-mode.test.ts b/agent-computer/tests/browser-mode.test.ts new file mode 100644 index 000000000..12ef2da80 --- /dev/null +++ b/agent-computer/tests/browser-mode.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, test } from "bun:test"; +import { browserModeFromEnv } from "../src/browser-mode"; + +describe("choosing the browser people take over", () => { + test("keeps the existing headless browser when no mode is configured", () => { + expect(browserModeFromEnv(undefined)).toBe("headless"); + expect(browserModeFromEnv("")).toBe("headless"); + }); + + test("runs the full browser only when headed is requested", () => { + expect(browserModeFromEnv("headed")).toBe("headed"); + expect(browserModeFromEnv("headless")).toBe("headless"); + }); + + test("refuses a typo rather than silently changing browser behavior", () => { + expect(() => browserModeFromEnv("visible")).toThrow( + 'COMPUTER_BROWSER_MODE must be headless or headed, not "visible".', + ); + }); +}); diff --git a/agent-computer/tests/headed-browser.test.ts b/agent-computer/tests/headed-browser.test.ts new file mode 100644 index 000000000..743ca3b3c --- /dev/null +++ b/agent-computer/tests/headed-browser.test.ts @@ -0,0 +1,86 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { createServer } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +/** + * The optional full browser, driven as the container drives it. + * + * Asked for explicitly because it needs a virtual display and a real Chromium: + * + * xvfb-run -a env OPENBOT_HEADED_BROWSER=1 bun test tests/headed-browser.test.ts + */ +const asked = process.env.OPENBOT_HEADED_BROWSER === "1"; +const TOKEN = "headed-browser-test-token"; +const BOT = "headed-browser"; + +async function freePort(): Promise { + return new Promise((resolve, reject) => { + const probe = createServer(); + probe.on("error", reject); + probe.listen(0, "127.0.0.1", () => { + const address = probe.address(); + if (!address || typeof address === "string") { + reject(new Error("The port probe did not return a TCP address.")); + return; + } + probe.close(() => resolve(address.port)); + }); + }); +} + +let base = ""; +let root = ""; + +function api(path: string, init?: RequestInit) { + return fetch(`${base}${path}`, { + ...init, + headers: { + "content-type": "application/json", + "x-openbot-bot-id": BOT, + "x-openbot-computer-token": TOKEN, + ...(init?.headers ?? {}), + }, + }); +} + +beforeAll(async () => { + if (!asked) return; + root = await mkdtemp(join(tmpdir(), "agent-computer-headed-")); + const port = await freePort(); + base = `http://127.0.0.1:${port}`; + process.env.COMPUTER_TOKEN = TOKEN; + process.env.COMPUTER_BROWSER_MODE = "headed"; + process.env.PORT = String(port); + process.env.PROFILES_DIR = join(root, "profiles"); + process.env.WORKSPACE_DIR = join(root, "workspace"); + await mkdir(process.env.PROFILES_DIR, { recursive: true }); + await import(`../src/index?headed=${Date.now()}`); +}); + +afterAll(async () => { + if (!asked) return; + await api("/computers/stop", { method: "POST" }).catch(() => undefined); + await rm(root, { recursive: true, force: true }); +}, 30_000); + +describe.skipIf(!asked)("a browser a person can take over", () => { + test("is full Chromium rather than the headless shell", async () => { + const page = + "data:text/html," + + encodeURIComponent( + "", + ); + const navigated = await api("/navigate", { + method: "POST", + body: JSON.stringify({ url: page }), + }); + expect(navigated.status).toBe(200); + + const read = await api("/read"); + const body = (await read.json()) as { text?: string }; + expect(body.text).not.toContain("HeadlessChrome"); + expect(body.text).toContain("webdriver=false"); + }, 30_000); +}); diff --git a/agent-computer/tests/virtual-display.test.ts b/agent-computer/tests/virtual-display.test.ts new file mode 100644 index 000000000..d9b546a5f --- /dev/null +++ b/agent-computer/tests/virtual-display.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, test } from "bun:test"; +import { + startVirtualDisplay, + type DisplayProcess, + type DisplayRuntime, +} from "../src/virtual-display"; + +function displayRuntime(ready: boolean) { + let stopped = false; + let finish = (_code: number) => {}; + const process: DisplayProcess = { + exited: new Promise((resolve) => { + finish = resolve; + }), + kill: () => { + stopped = true; + finish(0); + return true; + }, + }; + const runtime: DisplayRuntime = { + spawn: () => process, + ready: async () => ready, + wait: async () => {}, + }; + return { runtime, stopped: () => stopped }; +} + +describe("the virtual display behind a full browser", () => { + test("does not start for the existing headless mode", async () => { + const fake = displayRuntime(true); + expect(await startVirtualDisplay("headless", fake.runtime)).toBeNull(); + expect(fake.stopped()).toBe(false); + }); + + test("stays alive for headed Chromium and stops with the computer", async () => { + const fake = displayRuntime(true); + const display = await startVirtualDisplay("headed", fake.runtime); + + expect(display?.name).toBe(":99"); + expect(fake.stopped()).toBe(false); + await display?.stop(); + expect(fake.stopped()).toBe(true); + }); + + test("refuses to launch Chromium when the display never becomes ready", async () => { + const fake = displayRuntime(false); + await expect(startVirtualDisplay("headed", fake.runtime)).rejects.toThrow( + "The virtual display did not become ready", + ); + expect(fake.stopped()).toBe(true); + }); +}); diff --git a/docker-compose.yml b/docker-compose.yml index b4bb122e9..19b2e3a20 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -101,6 +101,9 @@ services: # shutdown notes tell you to use -- hands the container an empty string instead. Reaches only # a loopback-bound port; a deployment sets a real value in .env. COMPUTER_TOKEN: ${COMPUTER_TOKEN:-openbot-dev-computer-token} + # `headed` runs full Chromium behind the same streamed screen, so a person taking the wheel + # gets a browser sites treat like their ordinary desktop browser. Headless remains the default. + COMPUTER_BROWSER_MODE: ${COMPUTER_BROWSER_MODE:-headless} # How many Bots may hold a running browser at once, and how long an untouched one is kept. # A few hundred MB each, so on a deployment with many Bots these are the difference between # a container that holds steady and one that is killed for memory. Defaults are 8 and 30 @@ -197,6 +200,8 @@ services: SUPERVISOR_TOKEN: ${SUPERVISOR_TOKEN:-openbot-dev-supervisor-token} # Handed to every computer this creates, so the server and the computers share one secret. COMPUTER_TOKEN: ${COMPUTER_TOKEN:-openbot-dev-computer-token} + # Forwarded into every per-Bot computer the supervisor creates. + COMPUTER_BROWSER_MODE: ${COMPUTER_BROWSER_MODE:-headless} COMPUTER_IMAGE: ${COMPUTER_IMAGE:-openbot-agent-computer:latest} # Which deployment the computers it creates belong to, so two stacks on one Docker host never # derive the same container and volume names for the same Bot. diff --git a/docs/configuration.md b/docs/configuration.md index 50fed72df..47ea23dc4 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -253,6 +253,7 @@ then is a row nothing will read. | `COMPUTER_TOKEN` | Secret every computer request must present. The computer refuses to start without it. | | `COMPUTER_MAX_BROWSERS` | How many Bots may hold a running browser at once. `8` by default; the least recently used is closed past it. | | `COMPUTER_BROWSER_IDLE_MS` | How long an untouched browser is kept. 30 minutes by default; `0` keeps them resident. | +| `COMPUTER_BROWSER_MODE` | `headless` by default; set to `headed` to run full Chromium on a private virtual display for human takeover. | | `COMPUTER_SUPERVISOR_URL` | Supervisor URL for per-Bot computers. If absent, Bots share `AGENT_COMPUTER_URL`. | | `SUPERVISOR_TOKEN` | Bearer token required by the supervisor. | | `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Local-only private-host browsing when `true`. A deployment running with `NODE_ENV=production` refuses to start while it is set. Cloud metadata addresses are refused either way. | diff --git a/supervisor/src/environment.ts b/supervisor/src/environment.ts new file mode 100644 index 000000000..ae3a814aa --- /dev/null +++ b/supervisor/src/environment.ts @@ -0,0 +1,27 @@ +/** + * What a computer is told about itself. + * + * Kept separate from the HTTP server so the exact environment boundary is testable without + * starting a listener or connecting to Docker. Nothing here is caller-supplied: a request says + * which Bot, never what to run or what to set. + */ +export function environmentFor( + botId: string, + env: Record = process.env, +): string[] { + const passthrough = Object.entries(env).filter(([key]) => + key.startsWith("EGRESS_PROXY"), + ); + const computerToken = env.COMPUTER_TOKEN; + const spireSocketVolume = env.SPIRE_AGENT_SOCKET_VOLUME; + const browserMode = env.COMPUTER_BROWSER_MODE; + return [ + `COMPUTER_BOT_ID=${botId}`, + ...(computerToken ? [`COMPUTER_TOKEN=${computerToken}`] : []), + ...(spireSocketVolume + ? ["SPIFFE_ENDPOINT_SOCKET=/tmp/spire-agent/public/api.sock"] + : []), + ...(browserMode ? [`COMPUTER_BROWSER_MODE=${browserMode}`] : []), + ...passthrough.map(([key, value]) => `${key}=${value ?? ""}`), + ]; +} diff --git a/supervisor/src/index.ts b/supervisor/src/index.ts index a875679a0..37d4f5296 100644 --- a/supervisor/src/index.ts +++ b/supervisor/src/index.ts @@ -1,5 +1,6 @@ import { serve } from "bun"; import { Hono } from "hono"; +import { environmentFor } from "./environment"; import { ComputerNotAnsweringError, DockerUnavailableError, @@ -69,39 +70,6 @@ if (!resolvedMemory.ok) { const memoryBytes = resolvedMemory.bytes; const spireSocketVolume = process.env.SPIRE_AGENT_SOCKET_VOLUME; -/** - * What a computer is told about itself. - * - * The egress variables come through so a Bot's traffic still leaves by the route configured for it; - * everything else a computer needs it already has. Nothing here is caller-supplied: a request says - * which Bot, never what to run or what to set. - */ -function environmentFor(botId: string): string[] { - const passthrough = Object.entries(process.env).filter(([key]) => - key.startsWith("EGRESS_PROXY"), - ); - /* - * The secret the computer demands of its callers. Handed to every container this creates, from - * this process's own environment, so the server and the computers share one secret and nothing else - * can drive a Bot's browser. Never caller-supplied: a request says which Bot, never what - * to set. - */ - const computerToken = process.env.COMPUTER_TOKEN; - return [ - // Which Bot this container is. Read by the computer as the Bot to assume when a request does not - // name one. It is normally named per request, so this is the fallback, and for a container that - // exists to be one Bot's the fallback must be that Bot rather than the shared default. - `COMPUTER_BOT_ID=${botId}`, - // Without this the computer refuses to start; it must never answer an unauthenticated caller. - ...(computerToken ? [`COMPUTER_TOKEN=${computerToken}`] : []), - // Where to ask what it is. Absent, the computer reports no identity and carries on. - ...(spireSocketVolume - ? ["SPIFFE_ENDPOINT_SOCKET=/tmp/spire-agent/public/api.sock"] - : []), - ...passthrough.map(([key, value]) => `${key}=${value ?? ""}`), - ]; -} - const app = new Hono(); app.use("*", async (context, next) => { diff --git a/supervisor/tests/environment.test.ts b/supervisor/tests/environment.test.ts new file mode 100644 index 000000000..cb7221c53 --- /dev/null +++ b/supervisor/tests/environment.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, test } from "bun:test"; +import { environmentFor } from "../src/environment"; + +describe("what the supervisor tells a computer about its browser", () => { + test("passes the deployment's browser mode to every per-Bot computer", () => { + expect( + environmentFor("invoice-collector", { + COMPUTER_TOKEN: "secret", + COMPUTER_BROWSER_MODE: "headed", + }), + ).toEqual([ + "COMPUTER_BOT_ID=invoice-collector", + "COMPUTER_TOKEN=secret", + "COMPUTER_BROWSER_MODE=headed", + ]); + }); + + test("does not invent a browser mode when the deployment left it unset", () => { + expect( + environmentFor("invoice-collector", { COMPUTER_TOKEN: "secret" }), + ).toEqual(["COMPUTER_BOT_ID=invoice-collector", "COMPUTER_TOKEN=secret"]); + }); +}); From 9495398caa727d044cd40abbc22a50ba1e3d2640 Mon Sep 17 00:00:00 2001 From: maximus792 <08093.claver@gmail.com> Date: Mon, 7 Sep 2026 14:49:48 +0200 Subject: [PATCH 2/3] fix(computer): own and monitor the virtual display --- agent-computer/src/index.ts | 35 +++++- agent-computer/src/virtual-display.ts | 114 +++++++++++++------ agent-computer/tests/headed-browser.test.ts | 5 +- agent-computer/tests/virtual-display.test.ts | 94 +++++++++++---- 4 files changed, 179 insertions(+), 69 deletions(-) diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index 6f37ffb55..b5cfa1974 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -1242,13 +1242,36 @@ console.info(`agent-computer listening on http://localhost:${PORT}`); * * `stop_grace_period` in docker-compose.yml is what gives this time to run. */ +let shuttingDown = false; + +async function shutDown(reason: string, exitCode: number): Promise { + if (shuttingDown) return; + // Set before the first await: a display exiting while Chromium flushes is part of this shutdown, + // not a second failure racing it. + shuttingDown = true; + console.info(`${reason}: closing the browser so its profile is flushed`); + await profiles.closeAll(); + await VIRTUAL_DISPLAY?.stop(); + process.exit(exitCode); +} + +if (VIRTUAL_DISPLAY) { + void VIRTUAL_DISPLAY.terminated.then(({ code, expected }) => { + if (expected || shuttingDown) return; + console.error( + JSON.stringify({ + type: "computer-virtual-display-exited", + exitCode: code, + }), + ); + // A headed Chromium cannot recover without its display. Let the container restart policy build + // the pair together again instead of advertising a healthy service whose next browser fails. + void shutDown("virtual display exited", 1); + }); +} + for (const signal of ["SIGTERM", "SIGINT"] as const) { process.on(signal, () => { - void (async () => { - console.info(`${signal}: closing the browser so its profile is flushed`); - await profiles.closeAll(); - await VIRTUAL_DISPLAY?.stop(); - process.exit(0); - })(); + void shutDown(signal, 0); }); } diff --git a/agent-computer/src/virtual-display.ts b/agent-computer/src/virtual-display.ts index 03fbf618e..1d8ee465f 100644 --- a/agent-computer/src/virtual-display.ts +++ b/agent-computer/src/virtual-display.ts @@ -1,47 +1,65 @@ import { spawn } from "node:child_process"; -import { access } from "node:fs/promises"; +import type { Readable } from "node:stream"; import type { BrowserMode } from "./browser-mode"; export type DisplayProcess = { + /** The display allocated by this exact Xvfb process, reported through -displayfd. */ + ready: Promise; exited: Promise; kill: (signal?: NodeJS.Signals) => boolean; }; export type DisplayRuntime = { spawn: (command: string, args: string[]) => DisplayProcess; - ready: (socketPath: string) => Promise; wait: (milliseconds: number) => Promise; }; export type VirtualDisplay = { name: string; + /** Resolves for both normal shutdown and a display that failed while the computer was running. */ + terminated: Promise<{ code: number; expected: boolean }>; stop: () => Promise; }; -const DISPLAY = ":99"; -const SOCKET = "/tmp/.X11-unix/X99"; const READY_BUDGET_MS = 5_000; -const POLL_MS = 25; const STOP_BUDGET_MS = 2_000; +async function allocatedDisplay(stream: Readable): Promise { + let response = ""; + for await (const chunk of stream) { + response += String(chunk); + if (response.length > 32) { + throw new Error("Xvfb returned an invalid display number."); + } + if (response.includes("\n")) break; + } + + const number = response.trim(); + if (!/^\d+$/.test(number)) { + throw new Error("Xvfb returned an invalid display number."); + } + return `:${number}`; +} + const systemRuntime: DisplayRuntime = { spawn(command, args) { const child = spawn(command, args, { - stdio: ["ignore", "ignore", "inherit"], + // fd 3 is private to this child. Xvfb writes its selected display there only after that display + // is ready, so a stale socket or another X server can never satisfy our readiness check. + stdio: ["ignore", "ignore", "inherit", "pipe"], }); const exited = new Promise((resolve) => { child.once("exit", (code) => resolve(code ?? 1)); child.once("error", () => resolve(1)); }); - return { exited, kill: (signal) => child.kill(signal) }; - }, - ready: async (socketPath) => { - try { - await access(socketPath); - return true; - } catch { - return false; - } + const displayFd = child.stdio[3] as Readable | null; + if (!displayFd) + throw new Error("Xvfb did not expose its display descriptor."); + return { + ready: allocatedDisplay(displayFd), + exited, + kill: (signal) => child.kill(signal), + }; }, wait: (milliseconds) => new Promise((resolve) => setTimeout(resolve, milliseconds)), @@ -51,27 +69,27 @@ async function stop( process: DisplayProcess, runtime: DisplayRuntime, ): Promise { - let exited = false; - void process.exited.then(() => { - exited = true; - }); process.kill("SIGTERM"); - await Promise.race([process.exited, runtime.wait(STOP_BUDGET_MS)]); - if (!exited) { + const stopped = await Promise.race([ + process.exited.then(() => true), + runtime.wait(STOP_BUDGET_MS).then(() => false), + ]); + if (!stopped) { process.kill("SIGKILL"); await process.exited; } } -/** Start the one local-only X display a headed computer needs. */ +/** Start the local-only X display a headed computer needs. */ export async function startVirtualDisplay( mode: BrowserMode, runtime: DisplayRuntime = systemRuntime, ): Promise { if (mode === "headless") return null; - const process = runtime.spawn("Xvfb", [ - DISPLAY, + const displayProcess = runtime.spawn("Xvfb", [ + "-displayfd", + "3", "-screen", "0", "1280x800x24", @@ -79,25 +97,47 @@ export async function startVirtualDisplay( "tcp", "-ac", ]); + let stopping = false; let exitCode: number | undefined; - void process.exited.then((code) => { + const exited = displayProcess.exited.then((code) => { exitCode = code; + return code; }); + const terminated = exited.then((code) => ({ code, expected: stopping })); - for (let waited = 0; waited < READY_BUDGET_MS; waited += POLL_MS) { - if (await runtime.ready(SOCKET)) { - return { name: DISPLAY, stop: () => stop(process, runtime) }; + let name: string; + try { + name = await Promise.race([ + displayProcess.ready, + exited.then((code) => { + throw new Error( + `The virtual display exited before it became ready (exit ${code}).`, + ); + }), + runtime.wait(READY_BUDGET_MS).then(() => { + throw new Error( + `The virtual display did not become ready within ${READY_BUDGET_MS}ms.`, + ); + }), + ]); + if (!/^:\d+$/.test(name)) { + throw new Error("Xvfb returned an invalid display number."); } - if (exitCode !== undefined) { - throw new Error( - `The virtual display exited before it became ready (exit ${exitCode}).`, - ); + } catch (error) { + if (exitCode === undefined) { + stopping = true; + await stop(displayProcess, runtime); } - await runtime.wait(POLL_MS); + throw error; } - await stop(process, runtime); - throw new Error( - `The virtual display did not become ready within ${READY_BUDGET_MS}ms.`, - ); + return { + name, + terminated, + stop: async () => { + if (stopping || exitCode !== undefined) return; + stopping = true; + await stop(displayProcess, runtime); + }, + }; } diff --git a/agent-computer/tests/headed-browser.test.ts b/agent-computer/tests/headed-browser.test.ts index 743ca3b3c..2a36552fd 100644 --- a/agent-computer/tests/headed-browser.test.ts +++ b/agent-computer/tests/headed-browser.test.ts @@ -9,7 +9,10 @@ import { join } from "node:path"; * * Asked for explicitly because it needs a virtual display and a real Chromium: * - * xvfb-run -a env OPENBOT_HEADED_BROWSER=1 bun test tests/headed-browser.test.ts + * OPENBOT_HEADED_BROWSER=1 bun test tests/headed-browser.test.ts + * + * The computer starts and owns Xvfb itself. Wrapping this command in xvfb-run would hide display + * allocation bugs by giving Chromium a second display it does not own. */ const asked = process.env.OPENBOT_HEADED_BROWSER === "1"; const TOKEN = "headed-browser-test-token"; diff --git a/agent-computer/tests/virtual-display.test.ts b/agent-computer/tests/virtual-display.test.ts index d9b546a5f..dd8315daf 100644 --- a/agent-computer/tests/virtual-display.test.ts +++ b/agent-computer/tests/virtual-display.test.ts @@ -5,49 +5,93 @@ import { type DisplayRuntime, } from "../src/virtual-display"; -function displayRuntime(ready: boolean) { - let stopped = false; - let finish = (_code: number) => {}; +function deferred() { + let resolve = (_value: T) => {}; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +function displayRuntime() { + const ready = deferred(); + const exited = deferred(); + const readyWait = deferred(); + const stopWait = deferred(); + const killed = deferred(); + const signals: NodeJS.Signals[] = []; + let waits = 0; const process: DisplayProcess = { - exited: new Promise((resolve) => { - finish = resolve; - }), - kill: () => { - stopped = true; - finish(0); + ready: ready.promise, + exited: exited.promise, + kill: (signal = "SIGTERM") => { + signals.push(signal); + killed.resolve(signal); return true; }, }; const runtime: DisplayRuntime = { spawn: () => process, - ready: async () => ready, - wait: async () => {}, + wait: () => (waits++ === 0 ? readyWait.promise : stopWait.promise), }; - return { runtime, stopped: () => stopped }; + return { runtime, ready, exited, readyWait, stopWait, killed, signals }; } describe("the virtual display behind a full browser", () => { test("does not start for the existing headless mode", async () => { - const fake = displayRuntime(true); + const fake = displayRuntime(); expect(await startVirtualDisplay("headless", fake.runtime)).toBeNull(); - expect(fake.stopped()).toBe(false); + expect(fake.signals).toEqual([]); }); - test("stays alive for headed Chromium and stops with the computer", async () => { - const fake = displayRuntime(true); - const display = await startVirtualDisplay("headed", fake.runtime); + test("uses the display allocated by the Xvfb process it owns", async () => { + const fake = displayRuntime(); + const starting = startVirtualDisplay("headed", fake.runtime); + fake.ready.resolve(":143"); + const display = await starting; + + expect(display?.name).toBe(":143"); + expect(fake.signals).toEqual([]); - expect(display?.name).toBe(":99"); - expect(fake.stopped()).toBe(false); - await display?.stop(); - expect(fake.stopped()).toBe(true); + const stopping = display?.stop(); + fake.exited.resolve(0); + await stopping; + expect(fake.signals).toEqual(["SIGTERM"]); + expect(await display?.terminated).toEqual({ code: 0, expected: true }); }); - test("refuses to launch Chromium when the display never becomes ready", async () => { - const fake = displayRuntime(false); - await expect(startVirtualDisplay("headed", fake.runtime)).rejects.toThrow( + test("reports an unexpected exit after the display became ready", async () => { + const fake = displayRuntime(); + const starting = startVirtualDisplay("headed", fake.runtime); + fake.ready.resolve(":7"); + const display = await starting; + + fake.exited.resolve(23); + + expect(await display?.terminated).toEqual({ code: 23, expected: false }); + }); + + test("refuses to launch Chromium when its own display exits before readiness", async () => { + const fake = displayRuntime(); + const starting = startVirtualDisplay("headed", fake.runtime); + fake.exited.resolve(17); + + await expect(starting).rejects.toThrow( + "The virtual display exited before it became ready (exit 17)", + ); + expect(fake.signals).toEqual([]); + }); + + test("stops its display after the readiness deadline", async () => { + const fake = displayRuntime(); + const starting = startVirtualDisplay("headed", fake.runtime); + fake.readyWait.resolve(); + expect(await fake.killed.promise).toBe("SIGTERM"); + fake.exited.resolve(0); + + await expect(starting).rejects.toThrow( "The virtual display did not become ready", ); - expect(fake.stopped()).toBe(true); + expect(fake.signals).toEqual(["SIGTERM"]); }); }); From 77547d255aedc690aed75dd08af99c21d22ab9fa Mon Sep 17 00:00:00 2001 From: maximus792 <08093.claver@gmail.com> Date: Mon, 7 Sep 2026 14:52:22 +0200 Subject: [PATCH 3/3] test(computer): cover forced display shutdown --- agent-computer/tests/virtual-display.test.ts | 30 +++++++++++++++++++- docs/configuration.md | 11 +++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/agent-computer/tests/virtual-display.test.ts b/agent-computer/tests/virtual-display.test.ts index dd8315daf..9898fb83e 100644 --- a/agent-computer/tests/virtual-display.test.ts +++ b/agent-computer/tests/virtual-display.test.ts @@ -19,6 +19,7 @@ function displayRuntime() { const readyWait = deferred(); const stopWait = deferred(); const killed = deferred(); + const forceKilled = deferred(); const signals: NodeJS.Signals[] = []; let waits = 0; const process: DisplayProcess = { @@ -27,6 +28,7 @@ function displayRuntime() { kill: (signal = "SIGTERM") => { signals.push(signal); killed.resolve(signal); + if (signal === "SIGKILL") forceKilled.resolve(); return true; }, }; @@ -34,7 +36,16 @@ function displayRuntime() { spawn: () => process, wait: () => (waits++ === 0 ? readyWait.promise : stopWait.promise), }; - return { runtime, ready, exited, readyWait, stopWait, killed, signals }; + return { + runtime, + ready, + exited, + readyWait, + stopWait, + killed, + forceKilled, + signals, + }; } describe("the virtual display behind a full browser", () => { @@ -94,4 +105,21 @@ describe("the virtual display behind a full browser", () => { ); expect(fake.signals).toEqual(["SIGTERM"]); }); + + test("force-kills a display that ignores the graceful stop deadline", async () => { + const fake = displayRuntime(); + const starting = startVirtualDisplay("headed", fake.runtime); + fake.ready.resolve(":8"); + const display = await starting; + + const stopping = display?.stop(); + expect(await fake.killed.promise).toBe("SIGTERM"); + fake.stopWait.resolve(); + await fake.forceKilled.promise; + fake.exited.resolve(137); + await stopping; + + expect(fake.signals).toEqual(["SIGTERM", "SIGKILL"]); + expect(await display?.terminated).toEqual({ code: 137, expected: true }); + }); }); diff --git a/docs/configuration.md b/docs/configuration.md index 47ea23dc4..f7053e3f3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -262,6 +262,17 @@ then is a row nothing will read. | `COMPUTER_RUNTIME` | Set to `runsc` to run supervised computers under gVisor. | | `COMPUTER_SANDBOX` | Set to `on` to enable Chromium's own sandbox where the host permits user namespaces. Which way it went is printed at start-up. | +Changing `COMPUTER_BROWSER_MODE` affects new supervised computers. A computer that already exists is +left running until its image changes or its container is recreated. To apply a mode-only change to +all computers while preserving their browser profiles and workspaces, apply the new supervisor +environment and remove only the owned containers (do not remove their volumes): + +```sh +docker ps -aq --filter "label=openbot.namespace=openbot" | xargs -r docker rm -f +``` + +The supervisor recreates each computer with the same named volumes on its next request. + `agent-computer` also reads: - `ACTION_TIMEOUT_MS`