From b35a51b605b533ed3540d612a598d4a0e61cd222 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 9 Sep 2026 17:47:07 -0700 Subject: [PATCH 1/3] Isolate innerdogfood ports and browser sessions per worktree --- .../debug-standalone-agent-browser/SKILL.md | 23 +-- AGENTS.md | 4 +- docs/specs/transport.md | 5 + scripts/spec-word-budgets.json | 2 +- standalone/scripts/dev-agent-browser.mjs | 157 ++++++++------- standalone/scripts/dev-agent-browser.test.mjs | 186 ++++++++++++++++++ 6 files changed, 292 insertions(+), 85 deletions(-) create mode 100644 standalone/scripts/dev-agent-browser.test.mjs diff --git a/.claude/skills/debug-standalone-agent-browser/SKILL.md b/.claude/skills/debug-standalone-agent-browser/SKILL.md index 45ec1c7ec..62a960751 100644 --- a/.claude/skills/debug-standalone-agent-browser/SKILL.md +++ b/.claude/skills/debug-standalone-agent-browser/SKILL.md @@ -12,9 +12,6 @@ Use this skill when you need to run Dormouse standalone in a normal browser so y Run from the repo root: ```sh -DORMOUSE_BROWSER_DEV_AB_SESSION=dormouse-debug-$(date +%s) \ -DORMOUSE_BROWSER_DEV_VITE_PORT=1550 \ -DORMOUSE_BROWSER_DEV_HOST_PORT=1552 \ pnpm innerdogfood ``` @@ -27,35 +24,29 @@ The harness: - opens the app in `agent-browser` - mirrors browser console logs as `[browser log] ...` in the harness terminal -Use unique `DORMOUSE_BROWSER_DEV_AB_SESSION`, Vite port, and host port for repeat runs to avoid stale outer-browser state and port collisions. +Port and session overrides are documented in `docs/specs/transport.md` → "Standalone browser-dev harness". ## Freshness -Before a measurement, clear any stale nested agent-browser session used by Dormouse surfaces: +Close only the browser session your test owns when you need a fresh page: ```sh -agent-browser --session dormouse.1.default close --all +agent-browser --session close +agent-browser --session open "http://localhost:/" ``` -This matters because `dor ab open ...` uses a nested agent-browser session such as `dormouse.1.default`. If it has old tabs, the first stream snapshot can be polluted with stale URLs. +Never use `close --all` or a global process-name kill: other worktrees may have live harnesses. If testing nested browser surfaces, give their `dor ab --key` a test-specific name and close only that session afterward. -**`close --all` is global, not per-session.** Despite the `--session` flag, it closes *every* agent-browser session — including the outer harness session the app runs in. That is actually the cleanest way to get a fresh blank Dormouse, but you must then re-open the outer session yourself: +If the first `open` lands on `about:blank`, issue it again and poll until the URL sticks and the xterm input exists: ```sh -agent-browser --session dormouse.1.default close --all # clears nested AND outer agent-browser --session open "http://localhost:/" -``` - -The first `open` after a `close --all` frequently lands on `about:blank` instead of navigating (the stray-about:blank race). **Issue `open` a second time** and poll until the URL sticks and the xterm input exists: - -```sh -agent-browser --session open "http://localhost:/" # often needed twice agent-browser --session eval '(()=>(!!document.querySelector("textarea.xterm-helper-textarea")&&location.href.indexOf("")>-1)?"ready":"no")()' ``` Browser console mirroring (`[browser log] ...`) keeps working after a manual re-open, so you don't lose log visibility. -Stop any running harness with Ctrl-C (or `pkill -f dev-agent-browser.mjs`) before starting another one. Do not leave background dev servers running after a timing run. +Parallel worktrees are isolated automatically (`docs/specs/transport.md` → "Standalone browser-dev harness"). Inside Dormouse, use `dor ensure -- pnpm innerdogfood`; the harness opens its browser pane. Stop only your own harness with Ctrl-C after a timing run. ## Driving Dormouse diff --git a/AGENTS.md b/AGENTS.md index ce2df3d52..61edb5cf1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,8 +9,8 @@ pnpm install # install deps pnpm build # build lib, vscode extension, Pocket, and website ``` -**Inside Dormouse, run `innerdogfood`** — `dor ensure -- pnpm innerdogfood`, then -`dor ab --key innerdogfood open http://localhost:1420` +**Inside Dormouse, run `innerdogfood`** — `dor ensure -- pnpm innerdogfood`. +The harness opens its browser pane and prints its URL and browser command (`docs/specs/transport.md` → "Standalone browser-dev harness"). **Open every PR as a draft.** Chromatic bills per snapshot and skips drafts, so diff --git a/docs/specs/transport.md b/docs/specs/transport.md index ab26e160d..720d39ef0 100644 --- a/docs/specs/transport.md +++ b/docs/specs/transport.md @@ -28,6 +28,11 @@ Optional booleans: `pnpm innerdogfood` starts the standalone sidecar directly, a localhost-only HTTP bridge, and Vite with `VITE_DORMOUSE_BROWSER_DEV_HOST`, then opens the app URL in an `agent-browser` session. The browser build uses `BrowserSidecarAdapter` instead of `TauriAdapter` whenever that env var is present. +- **Must bind OS-assigned ports for Vite and the HTTP bridge by default.** +- **Must derive the default browser key from the canonical worktree path**, stable across restarts. **Must open through `dor ab` when `DORMOUSE_SURFACE_ID` is set**, otherwise through `agent-browser`; print the actual app URL, session, and command to drive it. Inside Dormouse, `dor ensure -- pnpm innerdogfood` starts and opens the harness. +- **May pin ports with `DORMOUSE_BROWSER_DEV_VITE_PORT` / `DORMOUSE_BROWSER_DEV_HOST_PORT` and the session with `DORMOUSE_BROWSER_DEV_AB_SESSION`.** An occupied pinned port fails startup; `0` requests an OS-assigned port. Explicit overrides are the caller's isolation responsibility. +- **Must await Vite's own listener before opening the browser and use the actual ports for bridge authentication and CORS.** **Must close the bridge, Vite, and sidecar on startup failure or shutdown.** Pinned by `standalone/scripts/dev-agent-browser.test.mjs`. + The bridge is a transport shim over the same sidecar protocol, not a second PTY implementation: fire-and-forget commands `POST /__dormouse_dev_host/send`, request/response commands `POST /__dormouse_dev_host/invoke`, host→webview events as SSE on `GET /__dormouse_dev_host/events`, and browser console output mirrored to `POST /__dormouse_dev_host/console` so one terminal shows sidecar, Vite, and in-browser logs together. The Burrow rides it too, on the message names below ("Message protocol"), so the harness runs a real Burrow against a per-run temp state directory (`docs/specs/standalone.md` → "Burrow service"). **The harness must keep logging the Burrow state directory in a form the pairing walkthrough parses**, which is how the walkthrough records that path before enrollment; pinned by `lib/src/lib/mirrored-constants.test.ts`. diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index a13393796..b67132a6c 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -30,7 +30,7 @@ "docs/specs/terminal-state.md": 2350, "docs/specs/theme.md": 2150, "docs/specs/tiling-engine.md": 4500, - "docs/specs/transport.md": 4350, + "docs/specs/transport.md": 4450, "docs/specs/tutorial.md": 1900, "docs/specs/vscode.md": 7350, "docs/specs/webgl-text.md": 1200, diff --git a/standalone/scripts/dev-agent-browser.mjs b/standalone/scripts/dev-agent-browser.mjs index d7d0a89bf..2f32f3d27 100644 --- a/standalone/scripts/dev-agent-browser.mjs +++ b/standalone/scripts/dev-agent-browser.mjs @@ -1,12 +1,12 @@ #!/usr/bin/env node import http from 'node:http'; -import net from 'node:net'; import os from 'node:os'; import path from 'node:path'; -import { readFile } from 'node:fs/promises'; -import { randomBytes } from 'node:crypto'; +import { readFile, realpath } from 'node:fs/promises'; +import { createHash, randomBytes } from 'node:crypto'; +import { createServer } from 'vite'; import { fileURLToPath } from 'node:url'; -// cross-spawn, not node:child_process: this script spawns `pnpm` and +// cross-spawn, not node:child_process: this script spawns `dor` and // `agent-browser`, which are `.cmd` shims on Windows that a bare-name spawn // can't resolve (ENOENT) and Node >=22 won't run directly (EINVAL). cross-spawn // handles both and is a no-op on POSIX. See docs/specs/dor-cli.md. @@ -23,9 +23,12 @@ const sidecarDir = path.join(standaloneDir, 'sidecar'); const sidecarScript = path.join(sidecarDir, 'main.js'); const dorBinDir = path.join(sidecarDir, 'dor-cli', 'bin'); const dorEntrypoint = path.join(sidecarDir, 'dor-cli', 'dist', 'dor.js'); -const hostPort = Number(process.env.DORMOUSE_BROWSER_DEV_HOST_PORT || 1422); -const vitePort = Number(process.env.DORMOUSE_BROWSER_DEV_VITE_PORT || 1420); -const browserSession = process.env.DORMOUSE_BROWSER_DEV_AB_SESSION || 'dormouse-dev-standalone'; +// Bind port 0 directly: probing and then releasing a free port races other runs. +let hostPort = Number(process.env.DORMOUSE_BROWSER_DEV_HOST_PORT || 0); +const vitePort = Number(process.env.DORMOUSE_BROWSER_DEV_VITE_PORT || 0); +const worktreeKey = `innerdogfood-${createHash('sha256').update(await realpath(repoRoot)).digest('hex').slice(0, 16)}`; +const browserSession = process.env.DORMOUSE_BROWSER_DEV_AB_SESSION || `dormouse.1.${worktreeKey}`; +const insideDormouse = Boolean(process.env.DORMOUSE_SURFACE_ID); // Only the token: the sidecar picks the control socket path itself (hardened // per-user directory on POSIX, unguessable pipe name on Windows) and reports it // on its own stderr as `[dor-control] listening on …`, which this harness @@ -44,7 +47,7 @@ const controlToken = randomBytes(24).toString('hex'); // dev page gets it, via the URL baked into `VITE_DORMOUSE_BROWSER_DEV_HOST`. // Overloading one token would hand the bridge to every spawned shell for free. const bridgeToken = randomBytes(24).toString('hex'); -const viteOrigin = `http://localhost:${vitePort}`; +let viteOrigin; // The Burrow persists its enrollment + ACL here, under the harness's own // temp dir so a dev run never touches the installed app's state. const stateDir = path.join(os.tmpdir(), `dormouse-${process.pid}-browser-state`); @@ -53,6 +56,8 @@ const pending = new Map(); const sseClients = new Set(); let sidecar; let vite; +let hostServer; +let browser; let shuttingDown = false; let requestSeq = 0; @@ -157,6 +162,10 @@ function cors(req, res) { function startHostServer() { const server = http.createServer(async (req, res) => { + if (!viteOrigin) { + res.writeHead(404).end('not found'); + return; + } cors(req, res); if (req.method === 'OPTIONS') { res.writeHead(204).end(); @@ -215,6 +224,7 @@ function startHostServer() { server.once('error', reject); server.listen(hostPort, '127.0.0.1', () => { server.off('error', reject); + hostPort = server.address().port; resolve(server); }); }); @@ -259,84 +269,99 @@ function startSidecar() { broadcast('sidecar', { event, data }); }); createInterface({ input: sidecar.stderr }).on('line', (line) => console.error(`[sidecar] ${line}`)); + sidecar.on('error', (err) => { + console.error(err); + shutdown(1); + }); sidecar.on('exit', (code, signal) => { log(`sidecar exited code=${code} signal=${signal}`); for (const request of pending.values()) request.reject(new Error('sidecar exited')); pending.clear(); - shutdown(); + shutdown(1); }); } -function startVite() { - vite = spawn('pnpm', ['--filter', 'dormouse-standalone', 'dev'], { - cwd: repoRoot, - stdio: ['ignore', 'pipe', 'pipe'], - env: { - ...process.env, - // The token rides in the URL, so the page needs nothing else plumbed to - // it and `BrowserSidecarHost` stays the single place that knows about it. - VITE_DORMOUSE_BROWSER_DEV_HOST: `http://127.0.0.1:${hostPort}/?t=${bridgeToken}`, - DORMOUSE_BROWSER_DEV_VITE_PORT: String(vitePort), +async function startVite() { + // Own Vite in this process: listen() reports the actual bound port and close() + // tears down its watchers too. A TCP readiness probe could find another run. + vite = await createServer({ + root: standaloneDir, + // Inject only into the page, never process.env: the sidecar's PTYs must not + // inherit the HTTP bridge credential. + define: { + 'import.meta.env.VITE_DORMOUSE_BROWSER_DEV_HOST': JSON.stringify(`http://127.0.0.1:${hostPort}/?t=${bridgeToken}`), + }, + server: { + host: '127.0.0.1', port: vitePort, strictPort: true, + // Share Vite's listener, including when TAURI_DEV_HOST is inherited. + hmr: { host: 'localhost', port: 0, protocol: 'ws' }, }, }); - createInterface({ input: vite.stdout }).on('line', (line) => console.error(`[vite] ${line}`)); - createInterface({ input: vite.stderr }).on('line', (line) => console.error(`[vite] ${line}`)); - vite.on('exit', (code, signal) => { - log(`vite exited code=${code} signal=${signal}`); - shutdown(); - }); -} - -async function waitForVite() { - const deadline = Date.now() + 30000; - while (Date.now() < deadline) { - try { - await new Promise((resolve, reject) => { - const socket = net.connect(vitePort, 'localhost', resolve); - socket.once('error', reject); - socket.once('connect', () => socket.end()); - }); - return; - } catch { - await new Promise((resolve) => setTimeout(resolve, 250)); - } - } - throw new Error(`vite did not open port ${vitePort}`); + await vite.listen(); + viteOrigin = `http://localhost:${vite.httpServer.address().port}`; + log(`app URL: ${viteOrigin}`); } async function openAgentBrowser() { - const args = ['--session', browserSession]; + const binary = insideDormouse ? 'dor' : 'agent-browser'; + const identity = process.env.DORMOUSE_BROWSER_DEV_AB_SESSION + ? ['--session', browserSession] + : ['--key', worktreeKey]; + const args = insideDormouse ? ['ab', ...identity] : ['--session', browserSession]; if (process.env.DORMOUSE_BROWSER_DEV_HEADED === '1') args.push('--headed'); - args.push('open', `http://localhost:${vitePort}`); - const child = spawn('agent-browser', args, { cwd: repoRoot, stdio: ['ignore', 'pipe', 'pipe'] }); - createInterface({ input: child.stdout }).on('line', (line) => console.error(`[agent-browser] ${line}`)); - createInterface({ input: child.stderr }).on('line', (line) => console.error(`[agent-browser] ${line}`)); - await new Promise((resolve) => child.on('exit', resolve)); + args.push('open', viteOrigin); + browser = spawn(binary, args, { cwd: repoRoot, stdio: ['ignore', 'pipe', 'pipe'] }); + createInterface({ input: browser.stdout }).on('line', (line) => console.error(`[${binary}] ${line}`)); + createInterface({ input: browser.stderr }).on('line', (line) => console.error(`[${binary}] ${line}`)); + await new Promise((resolve, reject) => { + browser.once('error', reject); + browser.once('exit', (code, signal) => code === 0 + ? resolve() + : reject(new Error(`${binary} exited code=${code} signal=${signal}`))); + }); log(`agent-browser session: ${browserSession}`); - log(`try: agent-browser --session ${browserSession} snapshot -i`); + log(insideDormouse + ? `try: dor ab ${identity.join(' ')} snapshot -i` + : `try: agent-browser --session ${browserSession} snapshot -i`); } -async function shutdown() { +async function shutdown(code = 0) { if (shuttingDown) return; shuttingDown = true; for (const client of sseClients) client.end(); sseClients.clear(); - if (vite && !vite.killed) vite.kill('SIGTERM'); - if (sidecar && !sidecar.killed) sidecar.kill('SIGTERM'); - setTimeout(() => process.exit(0), 250).unref(); + hostServer?.close(); + hostServer?.closeAllConnections(); + if (browser?.pid && browser.exitCode === null && browser.signalCode === null) browser.kill('SIGTERM'); + const sidecarClosed = sidecar?.pid && sidecar.exitCode === null && sidecar.signalCode === null + ? new Promise((resolve) => { + sidecar.once('exit', resolve); + sidecar.kill('SIGTERM'); + }) : Promise.resolve(); + // Bound cleanup even when a child or open request stops responding. + const timeout = setTimeout(() => { + if (sidecar?.pid && sidecar.exitCode === null && sidecar.signalCode === null) sidecar.kill('SIGKILL'); + process.exit(code); + }, 3000); + await Promise.all([vite?.close(), sidecarClosed]); + clearTimeout(timeout); + process.exit(code); } -process.on('SIGINT', shutdown); -process.on('SIGTERM', shutdown); +process.on('SIGINT', () => shutdown()); +process.on('SIGTERM', () => shutdown()); -log(`starting browser dev host on http://127.0.0.1:${hostPort}`); -// Printed so poking the bridge by hand stays possible. Local stderr only: this -// harness never runs in CI, and the token dies with the process. -log(`bridge token: ${bridgeToken}`); -log(`try: curl -H 'content-type: application/json' -d '{"cmd":"pty_request_init"}' 'http://127.0.0.1:${hostPort}/__dormouse_dev_host/send?t=${bridgeToken}'`); -await startHostServer(); -startSidecar(); -startVite(); -await waitForVite(); -await openAgentBrowser(); -log('running; Ctrl-C to stop'); +try { + hostServer = await startHostServer(); + log(`starting browser dev host on http://127.0.0.1:${hostPort}`); + // Local stderr only; this credential dies with the process. + log(`bridge token: ${bridgeToken}`); + log(`try: curl -H 'content-type: application/json' -d '{"cmd":"pty_request_init"}' 'http://127.0.0.1:${hostPort}/__dormouse_dev_host/send?t=${bridgeToken}'`); + await startVite(); + startSidecar(); + await openAgentBrowser(); + log('running; Ctrl-C to stop'); +} catch (err) { + console.error(err); + await shutdown(1); +} diff --git a/standalone/scripts/dev-agent-browser.test.mjs b/standalone/scripts/dev-agent-browser.test.mjs new file mode 100644 index 000000000..5c720d3d4 --- /dev/null +++ b/standalone/scripts/dev-agent-browser.test.mjs @@ -0,0 +1,186 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { copyFile, mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { spawn } from 'node:child_process'; +import { setTimeout as delay } from 'node:timers/promises'; + +const scripts = path.dirname(fileURLToPath(import.meta.url)); + +// Exercise the shipped harness with real Vite and HTTP listeners. Only the PTY +// runtime and browser CLI are substitutes; tests never launch a user's browser. +async function fixture(t) { + const root = await realpath(await mkdtemp(path.join(tmpdir(), 'innerdogfood-test-'))); + const standalone = path.join(root, 'standalone'); + const bin = path.join(root, 'bin'); + await mkdir(path.join(standalone, 'scripts'), { recursive: true }); + await mkdir(path.join(standalone, 'sidecar')); + await mkdir(bin); + await symlink(path.resolve(scripts, '../node_modules'), path.join(standalone, 'node_modules'), 'junction'); + for (const name of ['dev-agent-browser.mjs', 'dev-host-guard.mjs']) { + await copyFile(path.join(scripts, name), path.join(standalone, 'scripts', name)); + } + await copyFile(path.resolve(scripts, '../vite.config.ts'), path.join(standalone, 'vite.config.ts')); + await writeFile(path.join(standalone, 'index.html'), ''); + await writeFile(path.join(standalone, 'app.js'), 'console.log(import.meta.env.VITE_DORMOUSE_BROWSER_DEV_HOST);'); + await writeFile(path.join(standalone, 'sidecar/main.js'), ` + const { createInterface } = require('node:readline'); + createInterface({ input: process.stdin }).on('line', line => { + const { event, data } = JSON.parse(line); + if (event === 'pty:getCwd') console.log(JSON.stringify({ + event: 'pty:cwd', data: { requestId: data.requestId, cwd: process.env.VITE_DORMOUSE_BROWSER_DEV_HOST || process.cwd() } + })); + }); + `); + const cli = path.join(bin, 'cli.cjs'); + await writeFile(cli, `console.log('BROWSER_ARGS ' + JSON.stringify(process.argv.slice(2))); process.exit(Number(process.env.TEST_BROWSER_EXIT || 0));`); + for (const name of ['agent-browser', 'dor']) { + if (process.platform === 'win32') { + await writeFile(path.join(bin, `${name}.cmd`), `@"${process.execPath}" "${cli}" %*\r\n`); + } else { + const quote = value => "'" + value.replaceAll("'", "'\\''") + "'"; + await writeFile(path.join(bin, name), `#!/bin/sh\nexec ${quote(process.execPath)} ${quote(cli)} "$@"\n`, { mode: 0o755 }); + } + } + const runs = []; + t.after(async () => { + await Promise.all(runs.map(run => run.stop())); + await rm(root, { recursive: true, force: true }); + }); + return { + root, + start(overrides = {}) { + const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => !/^(DORMOUSE_|VITE_|TAURI_)/.test(key))); + env.PATH = `${bin}${path.delimiter}${process.env.PATH}`; + const child = spawn(process.execPath, [path.join(standalone, 'scripts/dev-agent-browser.mjs')], { + cwd: root, env: { ...env, ...overrides }, stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + child.stdout.on('data', chunk => { output += chunk; }); + child.stderr.on('data', chunk => { output += chunk; }); + const exited = new Promise((resolve, reject) => { + child.once('error', reject); + child.once('exit', (code, signal) => resolve({ code, signal })); + }); + const run = { + child, exited, + get output() { return output; }, + async wait(pattern) { + const deadline = Date.now() + 20000; + while (Date.now() < deadline) { + const match = output.match(pattern); + if (match) return match; + if (child.exitCode !== null || child.signalCode !== null) break; + await delay(25); + } + throw new Error(`Harness did not log ${pattern}:\n${output}`); + }, + async ready() { + await this.wait(/running; Ctrl-C to stop/); + this.app = (await this.wait(/app URL: (http:\/\/localhost:\d+)/))[1]; + this.bridge = (await this.wait(/starting browser dev host on (http:\/\/127.0.0.1:\d+)/))[1]; + this.token = (await this.wait(/bridge token: ([a-f0-9]+)/))[1]; + this.session = (await this.wait(/agent-browser session: (\S+)/))[1]; + this.args = JSON.parse((await this.wait(/BROWSER_ARGS (.+)/))[1]); + return this; + }, + async stop() { + if (child.exitCode === null && child.signalCode === null) child.kill('SIGTERM'); + const timer = setTimeout(() => child.kill('SIGKILL'), 5000); + try { return await exited; } finally { clearTimeout(timer); } + }, + }; + runs.push(run); + return run; + }, + }; +} + +async function invoke(run, token = run.token, origin = run.app) { + return fetch(`${run.bridge}/__dormouse_dev_host/invoke?t=${token}`, { + method: 'POST', headers: { 'content-type': 'application/json', origin }, + body: JSON.stringify({ cmd: 'pty_get_cwd', args: { id: 'test' } }), + signal: AbortSignal.timeout(5000), + }); +} + +async function assertClosed(run) { + for (const url of [run.app, run.bridge]) { + await assert.rejects(fetch(url, { signal: AbortSignal.timeout(1000) })); + } +} + +test('parallel worktrees own ports, browser identities and bridges; stopping one preserves the other', { timeout: 60000 }, async t => { + const [a, b] = await Promise.all([fixture(t), fixture(t)]); + const [one, two] = await Promise.all([ + a.start({ DORMOUSE_SURFACE_ID: 'outer-pane', TAURI_DEV_HOST: '192.0.2.1' }).ready(), b.start().ready(), + ]); + assert.equal(new Set([one.app.split(':').at(-1), two.app.split(':').at(-1), one.bridge.split(':').at(-1), two.bridge.split(':').at(-1)]).size, 4); + assert.notEqual(one.session, two.session); + assert.notEqual(one.token, two.token); + assert.deepEqual(one.args, ['ab', '--key', one.session.replace('dormouse.1.', ''), 'open', one.app]); + assert.deepEqual(two.args, ['--session', two.session, 'open', two.app]); + for (const [run, dir, other] of [[one, a.root, two], [two, b.root, one]]) { + const js = await (await fetch(`${run.app}/app.js`)).text(); + assert.ok(js.includes(`${run.bridge}/?t=${run.token}`)); + // HMR must share this listener, even with a Tauri-specific host inherited. + await new Promise((resolve, reject) => { + const ws = new WebSocket(run.app.replace('http:', 'ws:'), 'vite-ping'); + const timer = setTimeout(() => { ws.close(); reject(new Error('HMR did not connect')); }, 5000); + ws.addEventListener('open', () => { clearTimeout(timer); ws.close(); resolve(); }); + ws.addEventListener('error', event => { clearTimeout(timer); reject(event); }); + }); + const response = await invoke(run); + assert.equal(response.status, 200); + assert.equal(response.headers.get('access-control-allow-origin'), run.app); + assert.deepEqual(await response.json(), { ok: true, result: path.join(dir, 'standalone/sidecar') }); + assert.equal((await invoke(run, other.token)).status, 404); + assert.equal((await invoke(run, run.token, other.app)).headers.get('access-control-allow-origin'), run.app); + } + assert.equal((await one.stop()).code, 0); + await assertClosed(one); + assert.equal((await invoke(two)).status, 200); + const restarted = await a.start().ready(); + assert.equal(restarted.session, one.session); +}); + +test('explicit ports and raw browser sessions are honored; occupied ports fail without adopting a peer', { timeout: 60000 }, async t => { + const a = await fixture(t); + const one = await a.start().ready(); + const hostPort = one.bridge.split(':').at(-1); + const vitePort = one.app.split(':').at(-1); + const b = await fixture(t); + const hostCollision = b.start({ DORMOUSE_BROWSER_DEV_HOST_PORT: hostPort }); + assert.equal((await hostCollision.exited).code, 1); + assert.match(hostCollision.output, /EADDRINUSE/); + const viteCollision = b.start({ DORMOUSE_BROWSER_DEV_VITE_PORT: vitePort }); + assert.equal((await viteCollision.exited).code, 1); + assert.match(viteCollision.output, /already in use/); + assert.doesNotMatch(viteCollision.output, /BROWSER_ARGS/); + const failedBridge = (await viteCollision.wait(/starting browser dev host on (http:\/\/127.0.0.1:\d+)/))[1]; + await assert.rejects(fetch(failedBridge)); + assert.equal((await invoke(one)).status, 200); + await one.stop(); + const pinned = await b.start({ + DORMOUSE_SURFACE_ID: 'outer-pane', DORMOUSE_BROWSER_DEV_AB_SESSION: 'explicit-session', + DORMOUSE_BROWSER_DEV_HOST_PORT: hostPort, DORMOUSE_BROWSER_DEV_VITE_PORT: vitePort, + }).ready(); + assert.equal(pinned.app, one.app); + assert.equal(pinned.bridge, one.bridge); + assert.deepEqual(pinned.args, ['ab', '--session', 'explicit-session', 'open', pinned.app]); +}); + +test('browser startup failure closes the harness listeners and sidecar', { timeout: 30000 }, async t => { + const a = await fixture(t); + const run = a.start({ TEST_BROWSER_EXIT: '7' }); + assert.equal((await run.exited).code, 1); + assert.match(run.output, /agent-browser exited code=7/); + assert.doesNotMatch(run.output, /running; Ctrl-C/); + run.app = (await run.wait(/app URL: (http:\/\/localhost:\d+)/))[1]; + run.bridge = (await run.wait(/starting browser dev host on (http:\/\/127.0.0.1:\d+)/))[1]; + await assertClosed(run); + const pid = Number((await run.wait(/sidecar pid=(\d+)/))[1]); + assert.throws(() => process.kill(pid, 0), { code: 'ESRCH' }); +}); From 24abefbd0502be2a614cebeb6ae7b2320c66f2f8 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 9 Sep 2026 18:10:48 -0700 Subject: [PATCH 2/3] Simplify innerdogfood browser launch and child cleanup --- docs/specs/transport.md | 2 +- standalone/scripts/dev-agent-browser.mjs | 32 ++++++++++--------- standalone/scripts/dev-agent-browser.test.mjs | 32 ++++++++++++++++++- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/docs/specs/transport.md b/docs/specs/transport.md index 720d39ef0..19429be12 100644 --- a/docs/specs/transport.md +++ b/docs/specs/transport.md @@ -31,7 +31,7 @@ Optional booleans: - **Must bind OS-assigned ports for Vite and the HTTP bridge by default.** - **Must derive the default browser key from the canonical worktree path**, stable across restarts. **Must open through `dor ab` when `DORMOUSE_SURFACE_ID` is set**, otherwise through `agent-browser`; print the actual app URL, session, and command to drive it. Inside Dormouse, `dor ensure -- pnpm innerdogfood` starts and opens the harness. - **May pin ports with `DORMOUSE_BROWSER_DEV_VITE_PORT` / `DORMOUSE_BROWSER_DEV_HOST_PORT` and the session with `DORMOUSE_BROWSER_DEV_AB_SESSION`.** An occupied pinned port fails startup; `0` requests an OS-assigned port. Explicit overrides are the caller's isolation responsibility. -- **Must await Vite's own listener before opening the browser and use the actual ports for bridge authentication and CORS.** **Must close the bridge, Vite, and sidecar on startup failure or shutdown.** Pinned by `standalone/scripts/dev-agent-browser.test.mjs`. +- **Must await Vite's own listener before opening the browser and use the actual ports for bridge authentication and CORS.** **Must close the bridge and Vite and terminate owned sidecar and browser-launch children on startup failure or shutdown**, escalating to SIGKILL after three seconds. Pinned by `standalone/scripts/dev-agent-browser.test.mjs`. The bridge is a transport shim over the same sidecar protocol, not a second PTY implementation: fire-and-forget commands `POST /__dormouse_dev_host/send`, request/response commands `POST /__dormouse_dev_host/invoke`, host→webview events as SSE on `GET /__dormouse_dev_host/events`, and browser console output mirrored to `POST /__dormouse_dev_host/console` so one terminal shows sidecar, Vite, and in-browser logs together. The Burrow rides it too, on the message names below ("Message protocol"), so the harness runs a real Burrow against a per-run temp state directory (`docs/specs/standalone.md` → "Burrow service"). diff --git a/standalone/scripts/dev-agent-browser.mjs b/standalone/scripts/dev-agent-browser.mjs index 2f32f3d27..7f3b5817e 100644 --- a/standalone/scripts/dev-agent-browser.mjs +++ b/standalone/scripts/dev-agent-browser.mjs @@ -304,10 +304,11 @@ async function startVite() { async function openAgentBrowser() { const binary = insideDormouse ? 'dor' : 'agent-browser'; - const identity = process.env.DORMOUSE_BROWSER_DEV_AB_SESSION - ? ['--session', browserSession] - : ['--key', worktreeKey]; - const args = insideDormouse ? ['ab', ...identity] : ['--session', browserSession]; + const identity = insideDormouse && !process.env.DORMOUSE_BROWSER_DEV_AB_SESSION + ? ['--key', worktreeKey] + : ['--session', browserSession]; + const args = insideDormouse ? ['ab', ...identity] : identity; + const command = `${binary} ${args.join(' ')}`; if (process.env.DORMOUSE_BROWSER_DEV_HEADED === '1') args.push('--headed'); args.push('open', viteOrigin); browser = spawn(binary, args, { cwd: repoRoot, stdio: ['ignore', 'pipe', 'pipe'] }); @@ -320,9 +321,7 @@ async function openAgentBrowser() { : reject(new Error(`${binary} exited code=${code} signal=${signal}`))); }); log(`agent-browser session: ${browserSession}`); - log(insideDormouse - ? `try: dor ab ${identity.join(' ')} snapshot -i` - : `try: agent-browser --session ${browserSession} snapshot -i`); + log(`try: ${command} snapshot -i`); } async function shutdown(code = 0) { @@ -332,18 +331,21 @@ async function shutdown(code = 0) { sseClients.clear(); hostServer?.close(); hostServer?.closeAllConnections(); - if (browser?.pid && browser.exitCode === null && browser.signalCode === null) browser.kill('SIGTERM'); - const sidecarClosed = sidecar?.pid && sidecar.exitCode === null && sidecar.signalCode === null - ? new Promise((resolve) => { - sidecar.once('exit', resolve); - sidecar.kill('SIGTERM'); - }) : Promise.resolve(); + const children = new Set([browser, sidecar].filter(child => + child?.pid && child.exitCode === null && child.signalCode === null)); + const childrenClosed = [...children].map(child => new Promise(resolve => { + child.once('exit', () => { + children.delete(child); + resolve(); + }); + child.kill('SIGTERM'); + })); // Bound cleanup even when a child or open request stops responding. const timeout = setTimeout(() => { - if (sidecar?.pid && sidecar.exitCode === null && sidecar.signalCode === null) sidecar.kill('SIGKILL'); + for (const child of children) child.kill('SIGKILL'); process.exit(code); }, 3000); - await Promise.all([vite?.close(), sidecarClosed]); + await Promise.all([vite?.close(), ...childrenClosed]); clearTimeout(timeout); process.exit(code); } diff --git a/standalone/scripts/dev-agent-browser.test.mjs b/standalone/scripts/dev-agent-browser.test.mjs index 5c720d3d4..d73373b62 100644 --- a/standalone/scripts/dev-agent-browser.test.mjs +++ b/standalone/scripts/dev-agent-browser.test.mjs @@ -35,7 +35,16 @@ async function fixture(t) { }); `); const cli = path.join(bin, 'cli.cjs'); - await writeFile(cli, `console.log('BROWSER_ARGS ' + JSON.stringify(process.argv.slice(2))); process.exit(Number(process.env.TEST_BROWSER_EXIT || 0));`); + await writeFile(cli, ` + console.log('BROWSER_ARGS ' + JSON.stringify(process.argv.slice(2))); + if (process.env.TEST_BROWSER_HANG) { + process.on('SIGTERM', () => {}); + setInterval(() => {}, 1000); + console.log('BROWSER_PID ' + process.pid); + } else { + process.exit(Number(process.env.TEST_BROWSER_EXIT || 0)); + } + `); for (const name of ['agent-browser', 'dor']) { if (process.platform === 'win32') { await writeFile(path.join(bin, `${name}.cmd`), `@"${process.execPath}" "${cli}" %*\r\n`); @@ -184,3 +193,24 @@ test('browser startup failure closes the harness listeners and sidecar', { timeo const pid = Number((await run.wait(/sidecar pid=(\d+)/))[1]); assert.throws(() => process.kill(pid, 0), { code: 'ESRCH' }); }); + +test('shutdown kills an owned browser launcher that ignores SIGTERM', { + timeout: 30000, skip: process.platform === 'win32', +}, async t => { + const a = await fixture(t); + const run = a.start({ TEST_BROWSER_HANG: '1' }); + const pid = Number((await run.wait(/BROWSER_PID (\d+)/))[1]); + t.after(() => { + try { process.kill(pid, 'SIGKILL'); } catch (err) { + if (err.code !== 'ESRCH') throw err; + } + }); + assert.equal((await run.stop()).code, 0); + // SIGKILL delivery and orphan reaping can finish just after the harness exits. + await assert.rejects(async () => { + for (let attempt = 0; attempt < 40; attempt++) { + process.kill(pid, 0); + await delay(25); + } + }, { code: 'ESRCH' }); +}); From ed086d6914f1f5a90ec79fbb12be6e590ba983f9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 9 Sep 2026 19:42:14 -0700 Subject: [PATCH 3/3] Address innerdogfood review feedback on process output and session identity --- .../debug-standalone-agent-browser/SKILL.md | 1 + docs/specs/transport.md | 2 +- pnpm-lock.yaml | 3 +++ standalone/package.json | 2 ++ standalone/scripts/dev-agent-browser.mjs | 7 ++++--- standalone/scripts/dev-agent-browser.test.mjs | 15 ++++++++++++--- 6 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.claude/skills/debug-standalone-agent-browser/SKILL.md b/.claude/skills/debug-standalone-agent-browser/SKILL.md index 62a960751..31c3b4e12 100644 --- a/.claude/skills/debug-standalone-agent-browser/SKILL.md +++ b/.claude/skills/debug-standalone-agent-browser/SKILL.md @@ -181,6 +181,7 @@ After changing the harness, run: ```sh node --check standalone/scripts/dev-agent-browser.mjs +pnpm --filter dormouse-standalone test pnpm --filter dormouse-standalone build ``` diff --git a/docs/specs/transport.md b/docs/specs/transport.md index 19429be12..c7c5222d1 100644 --- a/docs/specs/transport.md +++ b/docs/specs/transport.md @@ -48,7 +48,7 @@ The bridge is a transport shim over the same sidecar protocol, not a second PTY The harness **may omit** native-only desktop chrome (window controls, update checks) but **must preserve** every `PlatformAdapter` contract the app uses — PTY, control-request, clipboard, iframe-proxy, Burrow, agent-browser. It **must mirror** standalone's Session-persistence answer ("The governing rule"): the same `PERSIST_SESSION = false` gate as `TauriAdapter`, `persistsSession: false`, and any pre-gate `localStorage` blob deleted on `init()` (rationale). **Tauri APIs must not be required at static module-evaluation time** when `VITE_DORMOUSE_BROWSER_DEV_HOST` is set — a normal browser loads the page, not the Tauri WebView. -Source of truth: `standalone/scripts/dev-agent-browser.mjs`, `standalone/scripts/dev-host-guard.mjs`, `standalone/src/browser-sidecar-host.ts`, `standalone/src/browser-sidecar-adapter.ts`; `stepBurrow` in `scripts/pairing-walkthrough/steps.mjs`. +Source of truth: `standalone/scripts/dev-agent-browser.mjs`, `standalone/scripts/dev-host-guard.mjs`, `standalone/src/browser-sidecar-host.ts`, `standalone/src/browser-sidecar-adapter.ts`; `stepBurrow` in `scripts/pairing-walkthrough/steps.mjs`; `sessionForKey` in `dor-lib-common/src/agent-browser.ts`. ## PTY lifecycle diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 166b05e08..9de83f766 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -285,6 +285,9 @@ importers: cross-spawn: specifier: ^7.0.6 version: 7.0.6 + dor-lib-common: + specifier: workspace:* + version: link:../dor-lib-common esbuild: specifier: ^0.28.0 version: 0.28.2 diff --git a/standalone/package.json b/standalone/package.json index 53baf6395..d6f516b64 100644 --- a/standalone/package.json +++ b/standalone/package.json @@ -13,6 +13,7 @@ "stage:dor-cli": "pnpm --filter dor build && node scripts/stage-dor-cli.mjs", "stage:sidecar-proxy": "pnpm --filter remote-lib-common build && node scripts/build-sidecar-proxy.mjs", "tauri": "pnpm run stage && node scripts/tauri.mjs", + "pretest": "pnpm --filter dor-lib-common build", "test": "vitest run && node --test scripts/*.test.mjs" }, "dependencies": { @@ -37,6 +38,7 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^6.0.2", "cross-spawn": "^7.0.6", + "dor-lib-common": "workspace:*", "esbuild": "^0.28.0", "jsdom": "^29.1.1", "tailwindcss": "^4.3.0", diff --git a/standalone/scripts/dev-agent-browser.mjs b/standalone/scripts/dev-agent-browser.mjs index 7f3b5817e..2d99d85ca 100644 --- a/standalone/scripts/dev-agent-browser.mjs +++ b/standalone/scripts/dev-agent-browser.mjs @@ -5,6 +5,7 @@ import path from 'node:path'; import { readFile, realpath } from 'node:fs/promises'; import { createHash, randomBytes } from 'node:crypto'; import { createServer } from 'vite'; +import { sessionForKey } from 'dor-lib-common/agent-browser'; import { fileURLToPath } from 'node:url'; // cross-spawn, not node:child_process: this script spawns `dor` and // `agent-browser`, which are `.cmd` shims on Windows that a bare-name spawn @@ -25,9 +26,9 @@ const dorBinDir = path.join(sidecarDir, 'dor-cli', 'bin'); const dorEntrypoint = path.join(sidecarDir, 'dor-cli', 'dist', 'dor.js'); // Bind port 0 directly: probing and then releasing a free port races other runs. let hostPort = Number(process.env.DORMOUSE_BROWSER_DEV_HOST_PORT || 0); -const vitePort = Number(process.env.DORMOUSE_BROWSER_DEV_VITE_PORT || 0); +const requestedVitePort = Number(process.env.DORMOUSE_BROWSER_DEV_VITE_PORT || 0); const worktreeKey = `innerdogfood-${createHash('sha256').update(await realpath(repoRoot)).digest('hex').slice(0, 16)}`; -const browserSession = process.env.DORMOUSE_BROWSER_DEV_AB_SESSION || `dormouse.1.${worktreeKey}`; +const browserSession = process.env.DORMOUSE_BROWSER_DEV_AB_SESSION || sessionForKey(worktreeKey); const insideDormouse = Boolean(process.env.DORMOUSE_SURFACE_ID); // Only the token: the sidecar picks the control socket path itself (hardened // per-user directory on POSIX, unguessable pipe name on Windows) and reports it @@ -292,7 +293,7 @@ async function startVite() { 'import.meta.env.VITE_DORMOUSE_BROWSER_DEV_HOST': JSON.stringify(`http://127.0.0.1:${hostPort}/?t=${bridgeToken}`), }, server: { - host: '127.0.0.1', port: vitePort, strictPort: true, + host: '127.0.0.1', port: requestedVitePort, strictPort: true, // Share Vite's listener, including when TAURI_DEV_HOST is inherited. hmr: { host: 'localhost', port: 0, protocol: 'ws' }, }, diff --git a/standalone/scripts/dev-agent-browser.test.mjs b/standalone/scripts/dev-agent-browser.test.mjs index d73373b62..f6ff0759a 100644 --- a/standalone/scripts/dev-agent-browser.test.mjs +++ b/standalone/scripts/dev-agent-browser.test.mjs @@ -6,6 +6,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { spawn } from 'node:child_process'; import { setTimeout as delay } from 'node:timers/promises'; +import { sessionForKey } from 'dor-lib-common/agent-browser'; const scripts = path.dirname(fileURLToPath(import.meta.url)); @@ -67,11 +68,16 @@ async function fixture(t) { cwd: root, env: { ...env, ...overrides }, stdio: ['ignore', 'pipe', 'pipe'], }); let output = ''; + let closed = false; child.stdout.on('data', chunk => { output += chunk; }); child.stderr.on('data', chunk => { output += chunk; }); const exited = new Promise((resolve, reject) => { child.once('error', reject); - child.once('exit', (code, signal) => resolve({ code, signal })); + // close includes stdio EOF, so final diagnostics are available to assertions. + child.once('close', (code, signal) => { + closed = true; + resolve({ code, signal }); + }); }); const run = { child, exited, @@ -81,7 +87,7 @@ async function fixture(t) { while (Date.now() < deadline) { const match = output.match(pattern); if (match) return match; - if (child.exitCode !== null || child.signalCode !== null) break; + if (closed) break; await delay(25); } throw new Error(`Harness did not log ${pattern}:\n${output}`); @@ -129,7 +135,10 @@ test('parallel worktrees own ports, browser identities and bridges; stopping one assert.equal(new Set([one.app.split(':').at(-1), two.app.split(':').at(-1), one.bridge.split(':').at(-1), two.bridge.split(':').at(-1)]).size, 4); assert.notEqual(one.session, two.session); assert.notEqual(one.token, two.token); - assert.deepEqual(one.args, ['ab', '--key', one.session.replace('dormouse.1.', ''), 'open', one.app]); + const key = one.args[2]; + assert.match(key, /^innerdogfood-[a-f0-9]{16}$/); + assert.deepEqual(one.args, ['ab', '--key', key, 'open', one.app]); + assert.equal(one.session, sessionForKey(key)); assert.deepEqual(two.args, ['--session', two.session, 'open', two.app]); for (const [run, dir, other] of [[one, a.root, two], [two, b.root, one]]) { const js = await (await fetch(`${run.app}/app.js`)).text();