From 28adc11c03cb7b19d402f0b9d72066186e85aee8 Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 18:48:04 +0800 Subject: [PATCH] fix: harden browse lifecycle and ref safety Clean up browse daemon lifecycle, freeze refs per tab, and add regression coverage for restart, cookie, network, and snapshot edge cases. --- browse/src/browser-manager.ts | 191 +++++++++++++++++------ browse/src/cli.ts | 193 ++++++++++++++++++----- browse/src/meta-commands.ts | 14 +- browse/src/read-commands.ts | 46 ++++-- browse/src/server.ts | 18 ++- browse/src/snapshot.ts | 33 +++- browse/src/write-commands.ts | 72 ++++++--- browse/test/commands.test.ts | 281 ++++++++++++++++++++++++++++++++++ browse/test/snapshot.test.ts | 84 ++++++++++ 9 files changed, 800 insertions(+), 132 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 033ed8747..2e7d6dc6e 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -7,8 +7,20 @@ * We do NOT try to self-heal — don't hide failure. */ -import { chromium, type Browser, type BrowserContext, type Page, type Locator } from 'playwright'; -import { addConsoleEntry, addNetworkEntry, networkBuffer, type LogEntry, type NetworkEntry } from './buffers'; +import { + chromium, + type Browser, + type BrowserContext, + type ElementHandle, + type Page, + type Request, +} from 'playwright'; +import { addConsoleEntry, addNetworkEntry, type NetworkEntry } from './buffers'; +import * as fs from 'fs'; + +interface BrowserSettings { + userAgent?: string | null; +} export class BrowserManager { private browser: Browser | null = null; @@ -18,11 +30,19 @@ export class BrowserManager { private nextTabId: number = 1; private extraHeaders: Record = {}; private customUserAgent: string | null = null; + private readonly settingsFile: string | null; - // ─── Ref Map (snapshot → @e1, @e2, ...) ──────────────────── - private refMap: Map = new Map(); + // ─── Ref Map (tab → snapshot refs → frozen element handles) ───────────── + private refMaps: Map>> = new Map(); + // Request object identity is stable even when multiple requests share a URL. + private requestEntries: WeakMap = new WeakMap(); + + constructor(settingsFile?: string | null) { + this.settingsFile = settingsFile ?? process.env.BROWSE_SETTINGS_FILE ?? null; + } async launch() { + this.loadSettings(); this.browser = await chromium.launch({ headless: true }); // Chromium crash → exit with clear message @@ -34,19 +54,29 @@ export class BrowserManager { this.context = await this.browser.newContext({ viewport: { width: 1280, height: 720 }, + ...(this.customUserAgent ? { userAgent: this.customUserAgent } : {}), }); + if (Object.keys(this.extraHeaders).length > 0) { + await this.context.setExtraHTTPHeaders(this.extraHeaders); + } + // Create first tab await this.newTab(); } async close() { + this.clearAllRefs(); if (this.browser) { // Remove disconnect handler to avoid exit during intentional close this.browser.removeAllListeners('disconnected'); await this.browser.close(); this.browser = null; } + this.context = null; + this.pages.clear(); + this.activeTabId = 0; + this.nextTabId = 1; } isHealthy(): boolean { @@ -63,7 +93,7 @@ export class BrowserManager { this.activeTabId = id; // Wire up console/network capture - this.wirePageEvents(page); + this.wirePageEvents(id, page); if (url) { await page.goto(url, { waitUntil: 'domcontentloaded', timeout: 15000 }); @@ -77,6 +107,7 @@ export class BrowserManager { const page = this.pages.get(tabId); if (!page) throw new Error(`Tab ${tabId} not found`); + this.clearRefs(tabId); await page.close(); this.pages.delete(tabId); @@ -143,34 +174,60 @@ export class BrowserManager { } // ─── Ref Map ────────────────────────────────────────────── - setRefMap(refs: Map) { - this.refMap = refs; + setRefMap(refs: Map>, tabId: number = this.activeTabId) { + this.clearRefs(tabId); + if (refs.size > 0) { + this.refMaps.set(tabId, refs); + } } - clearRefs() { - this.refMap.clear(); + clearRefs(tabId: number = this.activeTabId) { + const refs = this.refMaps.get(tabId); + if (!refs) return; + for (const handle of refs.values()) { + void handle.dispose().catch(() => {}); + } + this.refMaps.delete(tabId); } /** * Resolve a selector that may be a @ref (e.g., "@e3") or a CSS selector. - * Returns { locator } for refs or { selector } for CSS selectors. + * Returns { handle } for refs or { selector } for CSS selectors. */ - resolveRef(selector: string): { locator: Locator } | { selector: string } { + resolveRef(selector: string): { handle: ElementHandle } | { selector: string } { if (selector.startsWith('@e')) { const ref = selector.slice(1); // "e3" - const locator = this.refMap.get(ref); - if (!locator) { + const refMap = this.refMaps.get(this.activeTabId); + const handle = refMap?.get(ref); + if (!handle) { throw new Error( `Ref ${selector} not found. Page may have changed — run 'snapshot' to get fresh refs.` ); } - return { locator }; + return { handle }; } return { selector }; } - getRefCount(): number { - return this.refMap.size; + getRefCount(tabId: number = this.activeTabId): number { + return this.refMaps.get(tabId)?.size ?? 0; + } + + rethrowIfStaleRef(selector: string, err: unknown): never { + const message = err instanceof Error ? err.message : String(err); + const isStale = + message.includes('Element is not attached to the DOM') || + message.includes('Execution context was destroyed') || + message.includes('JSHandle is disposed') || + message.includes('Target page, context or browser has been closed'); + + if (selector.startsWith('@e') && isStale) { + // Normalize detached-handle errors back to the same stale-ref guidance + // the old locator-based implementation returned after navigation. + this.removeRef(selector); + throw new Error(`Ref ${selector} not found. Page may have changed — run 'snapshot' to get fresh refs.`); + } + throw err; } // ─── Viewport ────────────────────────────────────────────── @@ -191,17 +248,22 @@ export class BrowserManager { // For simplicity, we just store it and apply on next "restart" setUserAgent(ua: string) { this.customUserAgent = ua; + this.persistSettings(); } // ─── Console/Network/Ref Wiring ──────────────────────────── - private wirePageEvents(page: Page) { - // Clear ref map on navigation — refs point to stale elements after page change + private wirePageEvents(tabId: number, page: Page) { + // Clear this tab's ref map on navigation — refs point to stale elements after page change page.on('framenavigated', (frame) => { if (frame === page.mainFrame()) { - this.clearRefs(); + this.clearRefs(tabId); } }); + page.on('close', () => { + this.clearRefs(tabId); + }); + page.on('console', (msg) => { addConsoleEntry({ timestamp: Date.now(), @@ -211,43 +273,84 @@ export class BrowserManager { }); page.on('request', (req) => { - addNetworkEntry({ + const entry = { timestamp: Date.now(), method: req.method(), url: req.url(), - }); + }; + addNetworkEntry(entry); + this.requestEntries.set(req, entry); }); page.on('response', (res) => { - // Find matching request entry and update it - const url = res.url(); - const status = res.status(); - for (let i = networkBuffer.length - 1; i >= 0; i--) { - if (networkBuffer[i].url === url && !networkBuffer[i].status) { - networkBuffer[i].status = status; - networkBuffer[i].duration = Date.now() - networkBuffer[i].timestamp; - break; - } + const entry = this.requestEntries.get(res.request()); + if (entry) { + entry.status = res.status(); } }); - // Capture response sizes via response finished page.on('requestfinished', async (req) => { + const entry = this.requestEntries.get(req); + if (!entry) return; + try { - const res = await req.response(); - if (res) { - const url = req.url(); - const body = await res.body().catch(() => null); - const size = body ? body.length : 0; - for (let i = networkBuffer.length - 1; i >= 0; i--) { - if (networkBuffer[i].url === url && !networkBuffer[i].size) { - networkBuffer[i].size = size; - break; - } - } + const timing = req.timing(); + if (timing.responseEnd >= 0) { + entry.duration = Math.round(timing.responseEnd); } - } catch {} + const sizes = await req.sizes().catch(() => null); + if (sizes) { + entry.size = sizes.responseBodySize; + } + } catch { + } finally { + this.requestEntries.delete(req); + } + }); + + page.on('requestfailed', (req) => { + const entry = this.requestEntries.get(req); + if (entry) { + const timing = req.timing(); + if (timing.responseEnd >= 0) { + entry.duration = Math.round(timing.responseEnd); + } + } + this.requestEntries.delete(req); + }); + } + + private clearAllRefs() { + for (const tabId of [...this.refMaps.keys()]) { + this.clearRefs(tabId); + } + } + + private removeRef(selector: string, tabId: number = this.activeTabId) { + if (!selector.startsWith('@e')) return; + const ref = selector.slice(1); + const refs = this.refMaps.get(tabId); + const handle = refs?.get(ref); + if (!refs || !handle) return; + void handle.dispose().catch(() => {}); + refs.delete(ref); + if (refs.size === 0) { + this.refMaps.delete(tabId); + } + } + + private loadSettings() { + if (!this.settingsFile) return; + try { + const settings = JSON.parse(fs.readFileSync(this.settingsFile, 'utf-8')) as BrowserSettings; + this.customUserAgent = settings.userAgent ?? null; + } catch {} + } + + private persistSettings() { + if (!this.settingsFile) return; + fs.writeFileSync(this.settingsFile, JSON.stringify({ userAgent: this.customUserAgent } satisfies BrowserSettings, null, 2), { + mode: 0o600, }); } } - diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 931b85c10..2e52ce1b7 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -18,7 +18,10 @@ const BROWSE_PORT = process.env.CONDUCTOR_PORT : parseInt(process.env.BROWSE_PORT || '0', 10); const INSTANCE_SUFFIX = BROWSE_PORT ? `-${BROWSE_PORT}` : ''; const STATE_FILE = process.env.BROWSE_STATE_FILE || `/tmp/browse-server${INSTANCE_SUFFIX}.json`; +// Serialize startup so parallel agent shells don't spawn duplicate daemons. +const LOCK_FILE = `${STATE_FILE}.lock`; const MAX_START_WAIT = 8000; // 8 seconds to start +const LOCK_STALE_MS = 30_000; export function resolveServerScript( env: Record = process.env, @@ -59,6 +62,11 @@ interface ServerState { serverPath: string; } +interface StartLock { + pid: number; + createdAt: number; +} + // ─── State File ──────────────────────────────────────────────── function readState(): ServerState | null { try { @@ -78,12 +86,74 @@ function isProcessAlive(pid: number): boolean { } } -// ─── Server Lifecycle ────────────────────────────────────────── -async function startServer(): Promise { - // Clean up stale state file - try { fs.unlinkSync(STATE_FILE); } catch {} +function readLock(): StartLock | null { + try { + return JSON.parse(fs.readFileSync(LOCK_FILE, 'utf-8')) as StartLock; + } catch { + return null; + } +} + +function tryAcquireStartLock(): boolean { + try { + fs.writeFileSync( + LOCK_FILE, + JSON.stringify({ pid: process.pid, createdAt: Date.now() } satisfies StartLock), + { flag: 'wx', mode: 0o600 } + ); + return true; + } catch (err: any) { + if (err.code === 'EEXIST') return false; + throw err; + } +} + +function clearOwnedStartLock() { + const lock = readLock(); + if (lock?.pid === process.pid) { + try { fs.unlinkSync(LOCK_FILE); } catch {} + } +} + +function clearStaleStartLock() { + const lock = readLock(); + if (!lock) return; + if (!isProcessAlive(lock.pid) || Date.now() - lock.createdAt > LOCK_STALE_MS) { + try { fs.unlinkSync(LOCK_FILE); } catch {} + } +} + +async function fetchHealth(state: ServerState, timeout = 2000): Promise<{ status: string } | null> { + try { + const resp = await fetch(`http://127.0.0.1:${state.port}/health`, { + signal: AbortSignal.timeout(timeout), + }); + if (!resp.ok) return null; + return await resp.json() as { status: string }; + } catch { + return null; + } +} + +async function getHealthyState(): Promise { + const state = readState(); + if (!state || !isProcessAlive(state.pid)) { + return null; + } + const health = await fetchHealth(state); + if (health?.status === 'healthy') { + return state; + } + return null; +} + +// ─── Server Lifecycle ────────────────────────────────────────── +async function spawnServerProcess(): Promise { + const existing = readState(); + if (existing && !isProcessAlive(existing.pid)) { + try { fs.unlinkSync(STATE_FILE); } catch {} + } - // Start server as detached background process const proc = Bun.spawn(['bun', 'run', SERVER_SCRIPT], { stdio: ['ignore', 'pipe', 'pipe'], env: { ...process.env }, @@ -92,18 +162,18 @@ async function startServer(): Promise { // Don't hold the CLI open proc.unref(); - // Wait for state file to appear const start = Date.now(); while (Date.now() - start < MAX_START_WAIT) { const state = readState(); if (state && isProcessAlive(state.pid)) { - return state; + const health = await fetchHealth(state, 1000); + if (health?.status === 'healthy') { + return state; + } } await Bun.sleep(100); } - // If we get here, server didn't start in time - // Try to read stderr for error message const stderr = proc.stderr; if (stderr) { const reader = stderr.getReader(); @@ -116,31 +186,61 @@ async function startServer(): Promise { throw new Error(`Server failed to start within ${MAX_START_WAIT / 1000}s`); } -async function ensureServer(): Promise { - const state = readState(); +async function startServer(): Promise { + const start = Date.now(); + while (Date.now() - start < MAX_START_WAIT) { + const healthy = await getHealthyState(); + if (healthy) return healthy; - if (state && isProcessAlive(state.pid)) { - // Server appears alive — do a health check - try { - const resp = await fetch(`http://127.0.0.1:${state.port}/health`, { - signal: AbortSignal.timeout(2000), - }); - if (resp.ok) { - const health = await resp.json() as any; - if (health.status === 'healthy') { - return state; - } + // Another CLI process may already be starting the daemon. Wait for it + // unless the lock is stale, then take over. + clearStaleStartLock(); + if (tryAcquireStartLock()) { + try { + const healthyAfterLock = await getHealthyState(); + if (healthyAfterLock) return healthyAfterLock; + return await spawnServerProcess(); + } finally { + clearOwnedStartLock(); } - } catch { - // Health check failed — server is dead or unhealthy } + + await Bun.sleep(100); + } + + const healthy = await getHealthyState(); + if (healthy) return healthy; + throw new Error('[browse] Timed out waiting for server startup'); +} + +async function ensureServer(): Promise { + const healthy = await getHealthyState(); + if (healthy) { + return healthy; } - // Need to (re)start console.error('[browse] Starting server...'); return startServer(); } +async function waitForServerStop(pid: number, timeout = MAX_START_WAIT): Promise { + const start = Date.now(); + while (Date.now() - start < timeout) { + const alive = isProcessAlive(pid); + const stateExists = fs.existsSync(STATE_FILE); + if (!alive && !stateExists) { + return; + } + await Bun.sleep(100); + } + throw new Error('[browse] Timed out waiting for server shutdown'); +} + +function writeCommandOutput(text: string) { + process.stdout.write(text); + if (!text.endsWith('\n')) process.stdout.write('\n'); +} + // ─── Command Dispatch ────────────────────────────────────────── async function sendCommand(state: ServerState, command: string, args: string[], retries = 0): Promise { const body = JSON.stringify({ command, args }); @@ -169,19 +269,38 @@ async function sendCommand(state: ServerState, command: string, args: string[], const text = await resp.text(); if (resp.ok) { - process.stdout.write(text); - if (!text.endsWith('\n')) process.stdout.write('\n'); - } else { - // Try to parse as JSON error - try { - const err = JSON.parse(text); - console.error(err.error || text); - if (err.hint) console.error(err.hint); - } catch { - console.error(text); + writeCommandOutput(text); + + if (command === 'stop') { + // "stop" returns success before the server exits. Wait for the daemon + // and state file to disappear so the CLI only exits green on a real stop. + await waitForServerStop(state.pid); + return; } - process.exit(1); + + if (command === 'restart') { + // "restart" is "clean stop, then ensure a fresh daemon exists" — not + // "drop the socket and hope the next command recovers it." + await waitForServerStop(state.pid); + const newState = await ensureServer(); + if (newState.pid === state.pid) { + throw new Error('[browse] Restart did not replace the server process'); + } + return; + } + + return; } + + // Try to parse as JSON error + try { + const err = JSON.parse(text); + console.error(err.error || text); + if (err.hint) console.error(err.hint); + } catch { + console.error(text); + } + process.exit(1); } catch (err: any) { if (err.name === 'AbortError') { console.error('[browse] Command timed out after 30s'); @@ -220,7 +339,7 @@ Snapshot: snapshot [-i] [-c] [-d N] [-s sel] Compare: diff Multi-step: chain (reads JSON from stdin) Tabs: tabs | tab | newtab [url] | closetab [id] -Server: status | cookie = | header : +Server: status | cookie = [origin] | header : useragent | stop | restart Refs: After 'snapshot', use @e1, @e2... as selectors: diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts index 0fbe9aea4..89e538369 100644 --- a/browse/src/meta-commands.ts +++ b/browse/src/meta-commands.ts @@ -58,14 +58,20 @@ export async function handleMetaCommand( } case 'stop': { - await shutdown(); + // Return the HTTP response first so the CLI sees a clean stop, + // then shut down on the next tick. + setTimeout(() => { + void shutdown(); + }, 0); return 'Server stopped'; } case 'restart': { - // Signal that we want a restart — the CLI will detect exit and restart - console.log('[browse] Restart requested. Exiting for CLI to restart.'); - await shutdown(); + // Signal that we want a restart — return success first, then exit. + // The CLI waits for shutdown and starts a fresh daemon immediately. + setTimeout(() => { + void shutdown(); + }, 0); return 'Restarting...'; } diff --git a/browse/src/read-commands.ts b/browse/src/read-commands.ts index a473477d0..85185d180 100644 --- a/browse/src/read-commands.ts +++ b/browse/src/read-commands.ts @@ -35,8 +35,12 @@ export async function handleReadCommand( const selector = args[0]; if (selector) { const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - return await resolved.locator.innerHTML({ timeout: 5000 }); + if ('handle' in resolved) { + try { + return await resolved.handle.innerHTML(); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } return await page.innerHTML(resolved.selector); } @@ -108,12 +112,16 @@ export async function handleReadCommand( const [selector, property] = args; if (!selector || !property) throw new Error('Usage: browse css '); const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - const value = await resolved.locator.evaluate( - (el, prop) => getComputedStyle(el).getPropertyValue(prop), - property - ); - return value; + if ('handle' in resolved) { + try { + const value = await resolved.handle.evaluate( + (el, prop) => getComputedStyle(el as Element).getPropertyValue(prop), + property + ); + return value; + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } const value = await page.evaluate( ([sel, prop]) => { @@ -130,15 +138,19 @@ export async function handleReadCommand( const selector = args[0]; if (!selector) throw new Error('Usage: browse attrs '); const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - const attrs = await resolved.locator.evaluate((el) => { - const result: Record = {}; - for (const attr of el.attributes) { - result[attr.name] = attr.value; - } - return result; - }); - return JSON.stringify(attrs, null, 2); + if ('handle' in resolved) { + try { + const attrs = await resolved.handle.evaluate((el) => { + const result: Record = {}; + for (const attr of (el as Element).attributes) { + result[attr.name] = attr.value; + } + return result; + }); + return JSON.stringify(attrs, null, 2); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } const attrs = await page.evaluate((sel) => { const el = document.querySelector(sel); diff --git a/browse/src/server.ts b/browse/src/server.ts index 71e3b5c04..65377a91f 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -24,7 +24,9 @@ const BROWSE_PORT = process.env.CONDUCTOR_PORT : parseInt(process.env.BROWSE_PORT || '0', 10); // 0 = auto-scan const INSTANCE_SUFFIX = BROWSE_PORT ? `-${BROWSE_PORT}` : ''; const STATE_FILE = process.env.BROWSE_STATE_FILE || `/tmp/browse-server${INSTANCE_SUFFIX}.json`; +const SETTINGS_FILE = process.env.BROWSE_SETTINGS_FILE || `${STATE_FILE}.settings.json`; const IDLE_TIMEOUT_MS = parseInt(process.env.BROWSE_IDLE_TIMEOUT || '1800000', 10); // 30 min +const SHUTDOWN_GRACE_MS = 1000; function validateAuth(req: Request): boolean { const header = req.headers.get('authorization'); @@ -83,7 +85,7 @@ const idleCheckInterval = setInterval(() => { }, 60_000); // ─── Server ──────────────────────────────────────────────────── -const browserManager = new BrowserManager(); +const browserManager = new BrowserManager(SETTINGS_FILE); let isShuttingDown = false; // Read/write/meta command sets for routing @@ -184,11 +186,19 @@ async function shutdown() { clearInterval(idleCheckInterval); flushBuffers(); // Final flush - await browserManager.close(); - - // Clean up state file + // Remove the state file before closing Chromium so CLI stop/restart polling + // can observe shutdown progress even if browser.close() takes a moment. try { fs.unlinkSync(STATE_FILE); } catch {} + try { + // Graceful close is best-effort here. If Chromium hangs, still exit so + // stop/restart cannot wedge forever. + await Promise.race([ + browserManager.close(), + Bun.sleep(SHUTDOWN_GRACE_MS), + ]); + } catch {} + process.exit(0); } diff --git a/browse/src/snapshot.ts b/browse/src/snapshot.ts index d8d0da0fb..94f961c03 100644 --- a/browse/src/snapshot.ts +++ b/browse/src/snapshot.ts @@ -1,17 +1,17 @@ /** * Snapshot command — accessibility tree with ref-based element selection * - * Architecture (Locator map — no DOM mutation): + * Architecture (frozen handle map — no DOM mutation): * 1. page.locator(scope).ariaSnapshot() → YAML-like accessibility tree * 2. Parse tree, assign refs @e1, @e2, ... * 3. Build Playwright Locator for each ref (getByRole + nth) - * 4. Store Map on BrowserManager + * 4. Resolve each locator to an ElementHandle and store it per tab * 5. Return compact text output with refs prepended * - * Later: "click @e3" → look up Locator → locator.click() + * Later: "click @e3" → look up frozen handle → handle.click() */ -import type { Page, Locator } from 'playwright'; +import type { ElementHandle, Locator } from 'playwright'; import type { BrowserManager } from './browser-manager'; // Roles considered "interactive" for the -i flag @@ -38,6 +38,10 @@ interface ParsedNode { rawLine: string; } +function unescapeQuotedText(value: string): string { + return value.replace(/\\\\/g, '\\').replace(/\\"/g, '"'); +} + /** * Parse CLI args into SnapshotOptions */ @@ -83,7 +87,7 @@ export function parseSnapshotArgs(args: string[]): SnapshotOptions { */ function parseLine(line: string): ParsedNode | null { // Match: (indent)(- )(role)( "name")?( [props])?(: inline)? - const match = line.match(/^(\s*)-\s+(\w+)(?:\s+"([^"]*)")?(?:\s+(\[.*?\]))?\s*(?::\s*(.*))?$/); + const match = line.match(/^(\s*)-\s+(\w+)(?:\s+"((?:[^"\\]|\\.)*)")?(?:\s+(\[.*?\]))?\s*(?::\s*(.*))?$/); if (!match) { // Skip metadata lines like "- /url: /a" return null; @@ -91,7 +95,7 @@ function parseLine(line: string): ParsedNode | null { return { indent: match[1].length, role: match[2], - name: match[3] ?? null, + name: match[3] ? unescapeQuotedText(match[3]) : null, props: match[4] || '', children: match[5]?.trim() || '', rawLine: line, @@ -126,7 +130,7 @@ export async function handleSnapshot( // Parse the ariaSnapshot output const lines = ariaText.split('\n'); - const refMap = new Map(); + const refMap = new Map>(); const output: string[] = []; let refCounter = 1; @@ -190,7 +194,20 @@ export async function handleSnapshot( locator = locator.nth(seenIndex); } - refMap.set(ref, locator); + // Some accessibility nodes (for example structural text nodes) do not map + // cleanly back to a single DOM element. Skip those instead of stalling the + // whole snapshot. + let handle: ElementHandle | null = null; + try { + const count = await locator.count(); + if (count !== 1) continue; + handle = await locator.elementHandle({ timeout: 100 }); + } catch { + continue; + } + if (!handle) continue; + + refMap.set(ref, handle); // Format output line let outputLine = `${indent}@${ref} [${node.role}]`; diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index e1c91942b..ff054223b 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -42,8 +42,12 @@ export async function handleWriteCommand( const selector = args[0]; if (!selector) throw new Error('Usage: browse click '); const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - await resolved.locator.click({ timeout: 5000 }); + if ('handle' in resolved) { + try { + await resolved.handle.click({ timeout: 5000 }); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } else { await page.click(resolved.selector, { timeout: 5000 }); } @@ -55,10 +59,14 @@ export async function handleWriteCommand( case 'fill': { const [selector, ...valueParts] = args; const value = valueParts.join(' '); - if (!selector || !value) throw new Error('Usage: browse fill '); + if (!selector || args.length < 2) throw new Error('Usage: browse fill '); const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - await resolved.locator.fill(value, { timeout: 5000 }); + if ('handle' in resolved) { + try { + await resolved.handle.fill(value, { timeout: 5000 }); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } else { await page.fill(resolved.selector, value, { timeout: 5000 }); } @@ -68,10 +76,14 @@ export async function handleWriteCommand( case 'select': { const [selector, ...valueParts] = args; const value = valueParts.join(' '); - if (!selector || !value) throw new Error('Usage: browse select '); + if (!selector || args.length < 2) throw new Error('Usage: browse select '); const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - await resolved.locator.selectOption(value, { timeout: 5000 }); + if ('handle' in resolved) { + try { + await resolved.handle.selectOption(value, { timeout: 5000 }); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } else { await page.selectOption(resolved.selector, value, { timeout: 5000 }); } @@ -82,8 +94,12 @@ export async function handleWriteCommand( const selector = args[0]; if (!selector) throw new Error('Usage: browse hover '); const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - await resolved.locator.hover({ timeout: 5000 }); + if ('handle' in resolved) { + try { + await resolved.handle.hover({ timeout: 5000 }); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } else { await page.hover(resolved.selector, { timeout: 5000 }); } @@ -108,8 +124,12 @@ export async function handleWriteCommand( const selector = args[0]; if (selector) { const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - await resolved.locator.scrollIntoViewIfNeeded({ timeout: 5000 }); + if ('handle' in resolved) { + try { + await resolved.handle.scrollIntoViewIfNeeded({ timeout: 5000 }); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } else { await page.locator(resolved.selector).scrollIntoViewIfNeeded({ timeout: 5000 }); } @@ -124,8 +144,12 @@ export async function handleWriteCommand( if (!selector) throw new Error('Usage: browse wait '); const timeout = args[1] ? parseInt(args[1], 10) : 15000; const resolved = bm.resolveRef(selector); - if ('locator' in resolved) { - await resolved.locator.waitFor({ state: 'visible', timeout }); + if ('handle' in resolved) { + try { + await resolved.handle.waitForElementState('visible', { timeout }); + } catch (err) { + bm.rethrowIfStaleRef(selector, err); + } } else { await page.waitForSelector(resolved.selector, { timeout }); } @@ -142,16 +166,28 @@ export async function handleWriteCommand( case 'cookie': { const cookieStr = args[0]; - if (!cookieStr || !cookieStr.includes('=')) throw new Error('Usage: browse cookie ='); + if (!cookieStr || !cookieStr.includes('=')) throw new Error('Usage: browse cookie = [origin]'); const eq = cookieStr.indexOf('='); const name = cookieStr.slice(0, eq); const value = cookieStr.slice(eq + 1); - const url = new URL(page.url()); + let cookieUrl: string; + if (args[1]) { + try { + cookieUrl = new URL(args[1]).origin; + } catch { + throw new Error('Usage: browse cookie = [origin]'); + } + } else { + const currentUrl = page.url(); + if (currentUrl === 'about:blank') { + throw new Error('Usage: browse cookie = [origin]'); + } + cookieUrl = new URL(currentUrl).origin; + } await page.context().addCookies([{ name, value, - domain: url.hostname, - path: '/', + url: cookieUrl, }]); return `Cookie set: ${name}=${value}`; } diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 0d572bb64..4d819a726 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -36,6 +36,77 @@ afterAll(() => { setTimeout(() => process.exit(0), 500); }); +interface CliResult { + code: number; + stdout: string; + stderr: string; +} + +function reservePort(): number { + const server = Bun.serve({ + port: 0, + hostname: '127.0.0.1', + fetch: () => new Response('ok'), + }); + const { port } = server; + server.stop(); + return port; +} + +function readJson(filePath: string): T | null { + try { + return JSON.parse(fs.readFileSync(filePath, 'utf-8')) as T; + } catch { + return null; + } +} + +async function runCliCommand(args: string[], envOverrides: Record, timeout = 20000): Promise { + const cliPath = path.resolve(__dirname, '../src/cli.ts'); + return await new Promise((resolve) => { + const proc = spawn('bun', ['run', cliPath, ...args], { + timeout, + env: { + ...process.env, + ...envOverrides, + }, + }); + let stdout = ''; + let stderr = ''; + proc.stdout.on('data', (d) => stdout += d.toString()); + proc.stderr.on('data', (d) => stderr += d.toString()); + proc.on('close', (code) => resolve({ code: code ?? 1, stdout, stderr })); + }); +} + +async function waitFor(fn: () => T | null | undefined, timeout = 5000, interval = 50): Promise { + const start = Date.now(); + while (Date.now() - start < timeout) { + const value = fn(); + if (value) return value; + await Bun.sleep(interval); + } + return null; +} + +async function cleanupCliState(stateFile: string) { + const state = readJson<{ pid?: number }>(stateFile); + if (state?.pid) { + try { process.kill(state.pid, 'SIGTERM'); } catch {} + await waitFor(() => { + try { + process.kill(state.pid!, 0); + return null; + } catch { + return true; + } + }, 3000); + } + try { fs.unlinkSync(stateFile); } catch {} + try { fs.unlinkSync(`${stateFile}.lock`); } catch {} + try { fs.unlinkSync(`${stateFile}.settings.json`); } catch {} +} + // ─── Navigation ───────────────────────────────────────────────── describe('Navigation', () => { @@ -230,6 +301,28 @@ describe('Interaction', () => { const val = await handleReadCommand('js', ['document.querySelector("#name").value'], bm); expect(val).toBe('John Doe'); }); + + test('fill accepts empty string values', async () => { + await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm); + await handleWriteCommand('fill', ['#email', 'filled@example.com'], bm); + + const result = await handleWriteCommand('fill', ['#email', ''], bm); + expect(result).toContain('Filled'); + + const value = await handleReadCommand('js', ['document.querySelector("#email").value'], bm); + expect(value).toBe(''); + }); + + test('select accepts empty string values', async () => { + await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm); + await handleWriteCommand('select', ['#role', 'admin'], bm); + + const result = await handleWriteCommand('select', ['#role', ''], bm); + expect(result).toContain('Selected'); + + const value = await handleReadCommand('js', ['document.querySelector("#role").value'], bm); + expect(value).toBe(''); + }); }); // ─── SPA / Console / Network ─────────────────────────────────── @@ -268,6 +361,51 @@ describe('SPA and buffers', () => { const result = await handleReadCommand('network', ['--clear'], bm); expect(result).toContain('cleared'); }); + + test('network keeps same-url requests paired with their own size and duration', async () => { + networkBuffer.length = 0; + let apiCount = 0; + const server = Bun.serve({ + port: 0, + hostname: '127.0.0.1', + async fetch(req) { + const url = new URL(req.url); + if (url.pathname === '/double.html') { + return new Response(``, { + headers: { 'Content-Type': 'text/html' }, + }); + } + if (url.pathname === '/api/data') { + apiCount += 1; + if (apiCount === 1) { + await Bun.sleep(15); + return new Response('small', { + headers: { 'Content-Type': 'text/plain' }, + }); + } + await Bun.sleep(200); + return new Response('X'.repeat(5000), { + headers: { 'Content-Type': 'text/plain' }, + }); + } + return new Response('Not Found', { status: 404 }); + }, + }); + + await handleWriteCommand('goto', [`http://127.0.0.1:${server.port}/double.html`], bm); + await Bun.sleep(700); + + const entries = networkBuffer.filter((entry) => entry.url.endsWith('/api/data')); + try { server.stop(); } catch {} + + expect(entries).toHaveLength(2); + expect(entries[0].size).toBe(5); + expect(entries[1].size).toBe(5000); + expect(entries[0].duration).toBeLessThan(entries[1].duration!); + }); }); // ─── Cookies / Storage ────────────────────────────────────────── @@ -286,6 +424,27 @@ describe('Cookies and storage', () => { const storage = JSON.parse(result); expect(storage.localStorage.testKey).toBe('testValue'); }); + + test('cookie supports explicit origin before first navigation', async () => { + const freshBrowser = new BrowserManager(); + await freshBrowser.launch(); + + const result = await handleWriteCommand('cookie', ['session=abc123', baseUrl], freshBrowser); + expect(result).toContain('Cookie set'); + + await handleWriteCommand('goto', [baseUrl + '/basic.html'], freshBrowser); + const cookies = JSON.parse(await handleReadCommand('cookies', [], freshBrowser)); + expect(cookies.some((cookie: any) => cookie.name === 'session' && cookie.value === 'abc123')).toBe(true); + }); + + test('cookie on about:blank without explicit origin returns guidance error', async () => { + const freshBrowser = new BrowserManager(); + await freshBrowser.launch(); + + await expect(handleWriteCommand('cookie', ['session=abc123'], freshBrowser)).rejects.toThrow( + 'Usage: browse cookie = [origin]' + ); + }); }); // ─── Performance ──────────────────────────────────────────────── @@ -486,6 +645,128 @@ describe('CLI lifecycle', () => { expect(result.stdout).toContain('Status: healthy'); expect(result.stderr).toContain('Starting server'); }, 20000); + + test('stop exits cleanly and removes the state file', async () => { + const stateFile = `/tmp/browse-stop-state-${Date.now()}.json`; + const port = reservePort(); + const env = { + BROWSE_STATE_FILE: stateFile, + BROWSE_PORT: String(port), + }; + + try { + const started = await runCliCommand(['status'], env); + const startedState = readJson<{ pid: number }>(stateFile); + expect(started.code).toBe(0); + expect(startedState?.pid).toBeTruthy(); + + const stop = await runCliCommand(['stop'], env); + const pidGone = await waitFor(() => { + try { + process.kill(startedState!.pid, 0); + return null; + } catch { + return true; + } + }, 5000); + + expect(stop.code).toBe(0); + expect(stop.stdout).toContain('Server stopped'); + expect(fs.existsSync(stateFile)).toBe(false); + expect(pidGone).toBe(true); + } finally { + await cleanupCliState(stateFile); + } + }, 20000); + + test('restart exits cleanly and replaces the daemon pid', async () => { + const stateFile = `/tmp/browse-restart-state-${Date.now()}.json`; + const port = reservePort(); + const env = { + BROWSE_STATE_FILE: stateFile, + BROWSE_PORT: String(port), + }; + + try { + const started = await runCliCommand(['status'], env); + const beforeState = readJson<{ pid: number }>(stateFile); + expect(started.code).toBe(0); + expect(beforeState?.pid).toBeTruthy(); + + const restart = await runCliCommand(['restart'], env); + const afterState = await waitFor(() => { + const state = readJson<{ pid: number }>(stateFile); + return state && state.pid !== beforeState!.pid ? state : null; + }, 8000); + + expect(restart.code).toBe(0); + expect(restart.stdout).toContain('Restarting'); + expect(afterState?.pid).toBeTruthy(); + expect(afterState?.pid).not.toBe(beforeState?.pid); + } finally { + await cleanupCliState(stateFile); + } + }, 25000); + + test('parallel status calls start the daemon once', async () => { + const root = fs.mkdtempSync('/tmp/gstack-browse-wrapper-'); + const stateFile = path.join(root, 'browse-state.json'); + const startLog = path.join(root, 'server-starts.log'); + const wrapperPath = path.join(root, 'server-wrapper.ts'); + const realServerPath = path.resolve(__dirname, '../src/server.ts'); + const port = reservePort(); + + fs.writeFileSync(wrapperPath, ` + import * as fs from 'fs'; + fs.appendFileSync(${JSON.stringify(startLog)}, 'start\\n'); + await import(${JSON.stringify(realServerPath)}); + `); + + const env = { + BROWSE_STATE_FILE: stateFile, + BROWSE_PORT: String(port), + BROWSE_SERVER_SCRIPT: wrapperPath, + }; + + try { + const [first, second] = await Promise.all([ + runCliCommand(['status'], env), + runCliCommand(['status'], env), + ]); + + const state = readJson<{ pid: number }>(stateFile); + const starts = fs.readFileSync(startLog, 'utf-8').trim().split('\n').filter(Boolean); + + expect(first.code).toBe(0); + expect(second.code).toBe(0); + expect(state?.pid).toBeTruthy(); + expect(starts).toHaveLength(1); + } finally { + await cleanupCliState(stateFile); + fs.rmSync(root, { recursive: true, force: true }); + } + }, 25000); + + test('useragent applies after restart', async () => { + const stateFile = `/tmp/browse-useragent-state-${Date.now()}.json`; + const port = reservePort(); + const env = { + BROWSE_STATE_FILE: stateFile, + BROWSE_PORT: String(port), + }; + + try { + expect((await runCliCommand(['status'], env)).code).toBe(0); + expect((await runCliCommand(['useragent', 'MyAgent/1.0'], env)).code).toBe(0); + expect((await runCliCommand(['restart'], env)).code).toBe(0); + + const js = await runCliCommand(['js', 'navigator.userAgent'], env); + expect(js.code).toBe(0); + expect(js.stdout).toContain('MyAgent/1.0'); + } finally { + await cleanupCliState(stateFile); + } + }, 25000); }); // ─── Buffer bounds ────────────────────────────────────────────── diff --git a/browse/test/snapshot.test.ts b/browse/test/snapshot.test.ts index 846c82bb9..1868e4bff 100644 --- a/browse/test/snapshot.test.ts +++ b/browse/test/snapshot.test.ts @@ -17,6 +17,14 @@ let bm: BrowserManager; let baseUrl: string; const shutdown = async () => {}; +function extractRef(snapshot: string, predicate: (line: string) => boolean): string { + const line = snapshot.split('\n').find(predicate); + expect(line).toBeDefined(); + const refMatch = line!.match(/@(e\d+)/); + expect(refMatch).toBeDefined(); + return `@${refMatch![1]}`; +} + beforeAll(async () => { testServer = startTestServer(0); baseUrl = testServer.url; @@ -97,6 +105,14 @@ describe('Snapshot', () => { expect(snap1).toContain('@e1'); expect(snap2).toContain('@e1'); }); + + test('snapshot preserves accessible names with escaped quotes', async () => { + const page = bm.getPage(); + await page.setContent(``); + const result = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); + expect(result).toContain('[button]'); + expect(result).toContain('Say "Hello"'); + }); }); // ─── Ref-Based Interaction ────────────────────────────────────── @@ -175,6 +191,74 @@ describe('Ref resolution', () => { // ─── Ref Invalidation ─────────────────────────────────────────── describe('Ref invalidation', () => { + test('ref from tab 1 cannot be used from blank tab 2', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + const snap = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); + const ref = extractRef(snap, (line) => line.includes('[link]') && line.includes('"Page 1"')); + + await handleMetaCommand('newtab', [], bm, shutdown); + + await expect(handleWriteCommand('click', [ref], bm)).rejects.toThrow('snapshot'); + + const tabs = await bm.getTabListWithTitles(); + const tabOne = tabs.find((tab) => tab.id === 1); + const tabTwo = tabs.find((tab) => tab.active); + expect(tabOne?.url).toContain('/basic.html'); + expect(tabTwo?.url).toBe('about:blank'); + }); + + test('tab 1 refs still work after tab 2 navigates when switched back', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + const snap = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); + const ref = extractRef(snap, (line) => line.includes('[link]') && line.includes('"Page 1"')); + + const newTabResult = await handleMetaCommand('newtab', [baseUrl + '/forms.html'], bm, shutdown); + const tabIdMatch = newTabResult.match(/Opened tab (\d+)/); + expect(tabIdMatch).toBeDefined(); + + await handleMetaCommand('tab', ['1'], bm, shutdown); + + const result = await handleWriteCommand('click', [ref], bm); + expect(result).toContain('Clicked'); + expect(bm.getCurrentUrl()).toContain('/page1'); + }); + + test('reordering same-name elements does not retarget an existing ref', async () => { + const page = bm.getPage(); + await page.setContent(` + + + `); + const snap = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); + const ref = extractRef(snap, (line) => line.includes('[button]') && line.includes('"Delete"')); + + await page.evaluate(() => { + const btn = document.createElement('button'); + btn.id = 'new'; + btn.textContent = 'Delete'; + btn.onclick = () => { (window as any).clicked = 'new'; }; + document.body.prepend(btn); + }); + + await handleWriteCommand('click', [ref], bm); + const clicked = await handleReadCommand('js', ['window.clicked'], bm); + expect(clicked).toBe('a'); + }); + + test('removing a referenced element returns a stale ref error', async () => { + const page = bm.getPage(); + await page.setContent(` + + + `); + const snap = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); + const ref = extractRef(snap, (line) => line.includes('[button]') && line.includes('"Delete"')); + + await page.evaluate(() => document.getElementById('a')?.remove()); + + await expect(handleWriteCommand('click', [ref], bm)).rejects.toThrow('snapshot'); + }); + test('stale ref after goto returns clear error', async () => { await handleWriteCommand('goto', [baseUrl + '/snapshot.html'], bm); await handleMetaCommand('snapshot', ['-i'], bm, shutdown);