diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2a599367..a0ba8b663 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -422,8 +422,8 @@ jobs: - name: Install browser shell test dependencies run: pnpm install --frozen-lockfile --ignore-scripts - - name: Browser launch shell tests - run: pnpm --dir lib exec vitest run src/host/browser-launch-env.test.ts + - name: Browser host and launch shell tests + run: pnpm --dir lib exec vitest run src/host/browser-launch-env.test.ts src/host/agent-browser-host.test.ts # node-pty ships the Windows addon and ConPTY DLL as prebuilds, so the # install above suffices. The Linux suite cannot exercise native close. diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 782de341e..e6aad5f43 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -226,7 +226,8 @@ Source of truth: `parseBrowserRequest` and `createBrowserHost` in `lib/src/host/ agent-browser alone has a per-session daemon, state files beside its socket, and one fixed argv per operation; `--headed` is a no-op against a live daemon, so only a relaunch changes the mode. Its daemon's stream server and its browser's CDP port accept unauthenticated loopback TCP connections from any local process, refusing only a foreign `Origin`; Dormouse sets neither listener's flags (`docs/specs/security.md` -> "Known gaps"). -- **Must restart a headless daemon's screencast (`screencast_stop`, `screencast_start` on its own socket) when its first viewer socket opens and at once when it reports another tab active**, once per change per browser, and stop it when the last closes: only an asked-for switch rebinds it, and a restart never moves the active tab (rationale). **Must take any refusal as leaving the stream as it was**, so an agent-browser without these actions degrades to a stale pane, not an error. +- **Must restart a headless daemon's screencast (`screencast_stop`, `screencast_start`) when its first viewer connects or its active tab changes**, once per change per browser, and stop it when its last viewer closes. **Never change the active tab during a restart** (rationale). **Must leave the stream unchanged on refusal.** +- **Must send those actions only to the proven daemon's Unix socket or Windows loopback port in `.port`**, refusing missing, stale, or invalid port metadata. - **Never spawn a daemon outside a launch's own steps**: any CLI verb starts one at `about:blank`, so `attach` reads the live port from `.pid` / `.stream` and a port probe, and every other verb runs only on proof of a live daemon (`liveDaemon`) — a pid file from this boot, alive, beside a stream port that accepts. - **Never signal a pid without that proof** (rationale). - **Must run a launch's `open` in the binding's project directory while it exists**, so a relaunch reads the same `./agent-browser.json` the `dor agent-browser` there did. diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index c3cf14b5b..cefb2b912 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -142,6 +142,8 @@ ## agent-browser +Windows daemon endpoints (2026-10-21): agent-browser 0.27.0 and 0.31.1 write the actual loopback control port to `.port` ([upstream `resolve_port`](https://github.com/vercel-labs/agent-browser/blob/v0.31.1/cli/src/connection.rs)). Reading that file avoids copying the CLI's fallback hash, whose input changed between those versions. The host previously refused all Windows screencast actions; a TCP daemon fixture reproduced the missing restart and now exercises the same action protocol as the Unix socket fixture. + **Why the screencast is restarted, not the tab selected.** agent-browser 0.31.1's daemon rebinds its CDP screencast only on a switch someone asks for (2026-10-18/19, pages showing a 100 ms clock): after a `target=_blank` click it reported the popup active and sent 1–2 changed frames in 2 s, none of the popup; a new stream client did not rebind it, so a viewer that reconnects restarts too. Selecting the tab (`tab t2`) rebound it, but raced an agent's own switches: each `tab tN` already rebinds, and the host's select landed a CLI spawn later, so `tab new` then `tab t1` could be followed by the host's `tab t2` (review of #1176). Waiting for frames that did not come could not tell a stuck stream either: the old page, still painting, sent 1 changed frame in 2 s after a popup (static pages none, 6 of 6). `screencast_start` on the daemon's socket rebound the stuck stream (21 changed frames in 2 s) without changing the active tab, matched the stream's own frames (1280×577 JPEG at about the same weight), and left later `tab tN` switches streaming (21–22 frames). A second start answers `Screencast already active` and stays on its page, so each restart stops first, which answers `No screencast active` when none runs; a restart on a healthy stream kept streaming (20 frames). An explicit screencast kept capturing with no stream client (its stop still answered `stopped`), so the last viewer stops it; a reconnect without a restart then saw no frames from a tab changed meanwhile. The CLI has no verb for either action, and an unknown one answers `Not yet implemented`. Playwright kept the opener current after a popup and rebound within one 750 ms refresh after a close (@playwright/cli 1.64-alpha). **Why a pid needs proof before a signal.** A relaunch reads the daemon's pid from `.pid`, which nothing removes: after a reboot, or once that daemon died, the number can belong to any process, and SIGTERM then SIGKILL would reach it (review of #777, 2026-09). A file older than the boot is stale for certain; a live pid beside a stream port that accepts is the daemon as far as its state files can tell. diff --git a/lib/src/host/agent-browser-host.test.ts b/lib/src/host/agent-browser-host.test.ts index df8ced4d4..8004751e5 100644 --- a/lib/src/host/agent-browser-host.test.ts +++ b/lib/src/host/agent-browser-host.test.ts @@ -84,7 +84,7 @@ function useTempSocketDir(prefix: string): void { }); } -function writeState(session: string, ext: 'pid' | 'stream', value: number): void { +function writeState(session: string, ext: 'pid' | 'stream' | 'port', value: number): void { writeFileSync(join(process.env.AGENT_BROWSER_SOCKET_DIR!, `${session}.${ext}`), `${value}\n`); } @@ -545,6 +545,12 @@ describe('agent-browser host attach', () => { writeState(session, 'pid', process.pid); writeState(session, 'stream', await closedPort()); }], + ...['65536', '123garbage', '1.5', '-1'].map((value): [string, () => Promise] => [ + `an invalid stream port ${value}`, async () => { + running(session); + writeFileSync(join(process.env.AGENT_BROWSER_SOCKET_DIR!, `${session}.stream`), value); + }, + ]), ['a live pid and stream from before this boot', async () => { running(session); for (const ext of ['pid', 'stream']) utimesSync(join(process.env.AGENT_BROWSER_SOCKET_DIR!, `${session}.${ext}`), 0, 0); @@ -821,7 +827,13 @@ describe('agent-browser host viewer', () => { }); client.on('error', () => {}); }); - await new Promise((resolve) => server.listen(join(process.env.AGENT_BROWSER_SOCKET_DIR!, `${session}.sock`), resolve)); + await new Promise((resolve, reject) => { + server.once('error', reject); + if (process.platform === 'win32') server.listen(0, '127.0.0.1', resolve); + else server.listen(join(process.env.AGENT_BROWSER_SOCKET_DIR!, `${session}.sock`), resolve); + }); + const address = server.address(); + if (address && typeof address !== 'string') writeState(session, 'port', address.port); onTestFinished(() => new Promise((resolve) => server.close(() => resolve()))); return { actions }; } @@ -915,6 +927,17 @@ describe('agent-browser host viewer', () => { await vi.waitFor(async () => expect((await ab(host, { op: 'diagnostics' } as BrowserOp, { session })).diagnostics?.screencast).toMatchObject({ refusals: 1 }), { timeout: DAEMON_ACTION_TIMEOUT_MS + 1000 }); }, 10_000); + it.runIf(process.platform === 'win32').each(['missing', 'malformed', 'out of range', 'before boot'])('refuses a Windows daemon port that is %s', async (condition) => { + const { host, viewer, actions } = await streaming(); + const file = join(process.env.AGENT_BROWSER_SOCKET_DIR!, `${session}.port`); + if (condition === 'missing') await fsp.unlink(file); + else if (condition === 'before boot') utimesSync(file, new Date(0), new Date(0)); + else writeFileSync(file, condition === 'malformed' ? '123garbage' : '65536'); + await viewer(); + await vi.waitFor(async () => expect((await ab(host, { op: 'diagnostics' } as BrowserOp, { session })).diagnostics?.screencast).toMatchObject({ refusals: 1, last: { outcome: 'refused: no valid daemon port' } })); + expect(actions).toEqual([]); + }); + it('reaches no socket for a daemon its state files do not prove', async () => { const daemon = await fakeServer(); const { actions } = await daemonSocket(); diff --git a/lib/src/host/agent-browser-host.ts b/lib/src/host/agent-browser-host.ts index b3da23a73..db802cb06 100644 --- a/lib/src/host/agent-browser-host.ts +++ b/lib/src/host/agent-browser-host.ts @@ -283,14 +283,14 @@ export function createAgentBrowserProvider(deps: AgentBrowserProviderDeps = {}): // up when the daemon is killed, so a reader must know which daemon wrote it. // One written before this boot describes no process now running: its pid // is whatever process has that number since, so it reads as absent. - async function readStateNumber(b: ProviderBinding, ext: 'pid' | 'stream'): Promise { + async function readStateNumber(b: ProviderBinding, ext: 'pid' | 'stream' | 'port'): Promise { try { const file = path.join(agentBrowserStateDir(b), `${b.session}.${ext}`); const [stat, text] = await Promise.all([fs.stat(file), fs.readFile(file, 'utf8')]); // `os.uptime` is whole seconds on some platforms: a second's slack. if (stat.mtimeMs < Date.now() - os.uptime() * 1000 - 1000) return undefined; - const value = Number.parseInt(text.trim(), 10); - return Number.isInteger(value) && value > 0 ? value : undefined; + const value = Number(text.trim()); + return Number.isSafeInteger(value) && value > 0 && (ext === 'pid' || value <= 65535) ? value : undefined; } catch { return undefined; // absent (no daemon yet, custom dir, or an older CLI) } @@ -397,16 +397,19 @@ export function createAgentBrowserProvider(deps: AgentBrowserProviderDeps = {}): * `stream`: a newline-delimited JSON request and one answer line, bounded. * The CLI has no verb for the screencast actions (agent-browser 0.31.1), so * this is the host's one use of the socket. Anything but a `success: true` - * answer — no daemon, no socket (Windows has none), an error, a timeout, an + * answer — no daemon, no endpoint, an error, a timeout, an * action a later agent-browser renamed — is a refusal, with its reason. */ async function daemonAction(b: ProviderBinding, stream: number, request: Record): Promise { - if (process.platform === 'win32') return 'no daemon socket on Windows'; if ((await liveDaemon(b))?.stream !== stream) return 'not the live daemon of this stream'; + // Windows daemons publish their actual TCP port; do not guess the CLI's + // version-dependent fallback hash when that endpoint is absent or stale. + const port = process.platform === 'win32' ? await readStateNumber(b, 'port') : undefined; + if (process.platform === 'win32' && port === undefined) return 'no valid daemon port'; const file = path.join(agentBrowserStateDir(b), `${b.session}.sock`); return new Promise((resolve) => { let answer = ''; - const socket = net.connect(file); + const socket = port === undefined ? net.connect(file) : net.connect({ host: '127.0.0.1', port }); const settle = (refusal: string | undefined) => { socket.destroy(); resolve(refusal); }; socket.setTimeout(DAEMON_ACTION_TIMEOUT_MS, () => settle('no answer in time')); socket.once('error', (error) => settle(error.message)); diff --git a/scripts/outbound-lint.mjs b/scripts/outbound-lint.mjs index 154190105..63029b0a8 100644 --- a/scripts/outbound-lint.mjs +++ b/scripts/outbound-lint.mjs @@ -218,7 +218,7 @@ const OUTBOUND_SITES = { // --- loopback --- 'lib/src/host/agent-browser-host.ts': { class: 'loopback', - reason: 'Dials agent-browser\'s stream and the browser\'s CDP on 127.0.0.1 only (viewStream, askCdpEndpoint), and the daemon\'s own Unix socket beside the state files liveDaemon proves, for its fixed screencast actions only (daemonAction).', + reason: 'Dials agent-browser\'s stream and the browser\'s CDP on 127.0.0.1 only (viewStream, askCdpEndpoint), and the daemon\'s own Unix socket or Windows .port on 127.0.0.1 beside the state files liveDaemon proves, for its fixed screencast actions only (daemonAction).', }, 'lib/src/host/browser-viewer.ts': { class: 'loopback',