From 0593ae0201ea83c64fb253e0e790f00dcaa698fd Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 17:54:52 +0800 Subject: [PATCH 1/7] fix: harden browse install and lifecycle checks --- browse/src/cli.ts | 46 +++++++++++++++++++----- browse/test/commands.test.ts | 68 ++++++++++++++++++++++++++++-------- setup | 29 ++++++++++++--- 3 files changed, 115 insertions(+), 28 deletions(-) diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 8a7c4dc..931b85c 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -18,13 +18,39 @@ 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`; -// When compiled, import.meta.dir is virtual. Use env var or well-known path. -const SERVER_SCRIPT = process.env.BROWSE_SERVER_SCRIPT - || (import.meta.dir.startsWith('/') && !import.meta.dir.includes('$bunfs') - ? path.resolve(import.meta.dir, 'server.ts') - : path.resolve(process.env.HOME || '/tmp', '.claude/skills/gstack/browse/src/server.ts')); const MAX_START_WAIT = 8000; // 8 seconds to start +export function resolveServerScript( + env: Record = process.env, + metaDir: string = import.meta.dir, + execPath: string = process.execPath +): string { + if (env.BROWSE_SERVER_SCRIPT) { + return env.BROWSE_SERVER_SCRIPT; + } + + // Dev mode: cli.ts runs directly from browse/src + if (metaDir.startsWith('/') && !metaDir.includes('$bunfs')) { + const direct = path.resolve(metaDir, 'server.ts'); + if (fs.existsSync(direct)) { + return direct; + } + } + + // Compiled binary: derive the source tree from browse/dist/browse + if (execPath) { + const adjacent = path.resolve(path.dirname(execPath), '..', 'src', 'server.ts'); + if (fs.existsSync(adjacent)) { + return adjacent; + } + } + + // Legacy fallback for user-level installs + return path.resolve(env.HOME || '/tmp', '.claude/skills/gstack/browse/src/server.ts'); +} + +const SERVER_SCRIPT = resolveServerScript(); + interface ServerState { pid: number; port: number; @@ -215,7 +241,9 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: await sendCommand(state, command, commandArgs); } -main().catch((err) => { - console.error(`[browse] ${err.message}`); - process.exit(1); -}); +if (import.meta.main) { + main().catch((err) => { + console.error(`[browse] ${err.message}`); + process.exit(1); + }); +} diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 151e943..0d572bb 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -8,6 +8,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { startTestServer } from './test-server'; import { BrowserManager } from '../src/browser-manager'; +import { resolveServerScript } from '../src/cli'; import { handleReadCommand } from '../src/read-commands'; import { handleWriteCommand } from '../src/write-commands'; import { handleMetaCommand } from '../src/meta-commands'; @@ -420,33 +421,70 @@ describe('Status', () => { }); }); -// ─── CLI retry guard ──────────────────────────────────────────── +// ─── CLI server script resolution ─────────────────────────────── -describe('CLI retry guard', () => { - test('sendCommand aborts after repeated connection failures', async () => { - // Write a fake state file pointing to a port that refuses connections - const stateFile = '/tmp/browse-server.json'; - const origState = fs.existsSync(stateFile) ? fs.readFileSync(stateFile, 'utf-8') : null; +describe('CLI server script resolution', () => { + test('prefers adjacent browse/src/server.ts for compiled project installs', () => { + const root = fs.mkdtempSync('/tmp/gstack-cli-'); + const execPath = path.join(root, '.claude/skills/gstack/browse/dist/browse'); + const serverPath = path.join(root, '.claude/skills/gstack/browse/src/server.ts'); - fs.writeFileSync(stateFile, JSON.stringify({ port: 1, token: 'fake', pid: 999999 })); + fs.mkdirSync(path.dirname(execPath), { recursive: true }); + fs.mkdirSync(path.dirname(serverPath), { recursive: true }); + fs.writeFileSync(serverPath, '// test server\n'); + + const resolved = resolveServerScript( + { HOME: path.join(root, 'empty-home') }, + '$bunfs/root', + execPath + ); + + expect(resolved).toBe(serverPath); + + fs.rmSync(root, { recursive: true, force: true }); + }); +}); + +// ─── CLI lifecycle ────────────────────────────────────────────── + +describe('CLI lifecycle', () => { + test('dead state file triggers a clean restart', async () => { + const stateFile = `/tmp/browse-test-state-${Date.now()}.json`; + fs.writeFileSync(stateFile, JSON.stringify({ + port: 1, + token: 'fake', + pid: 999999, + })); const cliPath = path.resolve(__dirname, '../src/cli.ts'); - const result = await new Promise<{ code: number; stderr: string }>((resolve) => { + const result = await new Promise<{ code: number; stdout: string; stderr: string }>((resolve) => { const proc = spawn('bun', ['run', cliPath, 'status'], { timeout: 15000, - env: { ...process.env }, + env: { + ...process.env, + BROWSE_STATE_FILE: stateFile, + BROWSE_PORT_START: '9520', + }, }); + 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, stderr })); + proc.on('close', (code) => resolve({ code: code ?? 1, stdout, stderr })); }); - // Restore original state file - if (origState) fs.writeFileSync(stateFile, origState); - else if (fs.existsSync(stateFile)) fs.unlinkSync(stateFile); + let restartedPid: number | null = null; + if (fs.existsSync(stateFile)) { + restartedPid = JSON.parse(fs.readFileSync(stateFile, 'utf-8')).pid; + fs.unlinkSync(stateFile); + } + if (restartedPid) { + try { process.kill(restartedPid, 'SIGTERM'); } catch {} + } - // Should fail, not loop forever - expect(result.code).not.toBe(0); + expect(result.code).toBe(0); + expect(result.stdout).toContain('Status: healthy'); + expect(result.stderr).toContain('Starting server'); }, 20000); }); diff --git a/setup b/setup index 81ec2d6..73c6503 100755 --- a/setup +++ b/setup @@ -4,11 +4,32 @@ set -e GSTACK_DIR="$(cd "$(dirname "$0")" && pwd)" SKILLS_DIR="$(dirname "$GSTACK_DIR")" +BROWSE_BIN="$GSTACK_DIR/browse/dist/browse" # 1. Build browse binary if needed -if [ ! -x "$GSTACK_DIR/browse/dist/browse" ]; then +NEEDS_BUILD=0 +if [ ! -x "$BROWSE_BIN" ]; then + NEEDS_BUILD=1 +elif [ -n "$(find "$GSTACK_DIR/browse/src" -type f -newer "$BROWSE_BIN" -print -quit 2>/dev/null)" ]; then + NEEDS_BUILD=1 +elif [ "$GSTACK_DIR/package.json" -nt "$BROWSE_BIN" ]; then + NEEDS_BUILD=1 +elif [ -f "$GSTACK_DIR/bun.lock" ] && [ "$GSTACK_DIR/bun.lock" -nt "$BROWSE_BIN" ]; then + NEEDS_BUILD=1 +fi + +if [ "$NEEDS_BUILD" -eq 1 ]; then echo "Building browse binary..." - cd "$GSTACK_DIR" && bun install && bun run build + ( + cd "$GSTACK_DIR" + bun install + bun run build + ) +fi + +if [ ! -x "$BROWSE_BIN" ]; then + echo "gstack setup failed: browse binary missing at $BROWSE_BIN" >&2 + exit 1 fi # 2. Only create skill symlinks if we're inside a .claude/skills directory @@ -30,12 +51,12 @@ if [ "$SKILLS_BASENAME" = "skills" ]; then done echo "gstack ready." - echo " browse: $GSTACK_DIR/browse/dist/browse" + echo " browse: $BROWSE_BIN" if [ ${#linked[@]} -gt 0 ]; then echo " linked skills: ${linked[*]}" fi else echo "gstack ready." - echo " browse: $GSTACK_DIR/browse/dist/browse" + echo " browse: $BROWSE_BIN" echo " (skipped skill symlinks — not inside .claude/skills/)" fi From 28adc11c03cb7b19d402f0b9d72066186e85aee8 Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 18:48:04 +0800 Subject: [PATCH 2/7] 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 | 191 ++++++++++++++++++----- 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, 799 insertions(+), 131 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 033ed87..2e7d6dc 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 (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(); - // ─── Ref Map (snapshot → @e1, @e2, ...) ──────────────────── - private refMap: Map = new Map(); + 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 931b85c..2e52ce1 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 { } } +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 startServer(): Promise { - // Clean up stale state file - try { fs.unlinkSync(STATE_FILE); } catch {} +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 0fbe9ae..89e5383 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 a473477..85185d1 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 71e3b5c..65377a9 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 d8d0da0..94f961c 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 e1c9194..ff05422 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 0d572bb..4d819a7 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 846c82b..1868e4b 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); From 49df376a35c6e817dfbe1f62ad0648c97bcedfb6 Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 18:48:04 +0800 Subject: [PATCH 3/7] chore: bump version and changelog (v0.0.2) Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 15 +++++++++++++++ VERSION | 2 +- package.json | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4561c69..c5f50c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## 0.0.2 — 2026-03-12 + +### Changed + +- Reworked `/browse` daemon lifecycle so `stop` and `restart` complete cleanly, serialize startup, and persist user-agent changes across restarts. +- Switched snapshot refs from global live locators to tab-local frozen element handles so refs fail stale instead of drifting across tabs or SPA rerenders. +- Updated network capture to track requests by Playwright request identity and report response size/timing without loading response bodies into memory. + +### Fixed + +- `browse cookie` now supports an explicit origin before first navigation and returns clear guidance on `about:blank`. +- `browse fill` and `browse select` now accept explicit empty-string values. +- Snapshot parsing now preserves accessible names containing escaped quotes. +- Added regression coverage for lifecycle, ref safety, cookie semantics, empty values, quoted names, and same-URL network attribution. + ## 0.0.1 — 2026-03-11 Initial release. diff --git a/VERSION b/VERSION index 8acdd82..4e379d2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.0.1 +0.0.2 diff --git a/package.json b/package.json index 9f3bb20..d486f91 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "0.0.1", + "version": "0.0.2", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", From a4f6e3f67bfd47b5c00ba19fb9de4f05e1cddfd2 Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 19:01:19 +0800 Subject: [PATCH 4/7] fix: stop waiting on recreated state files Wait for the requested daemon pid to exit instead of treating raw state-file existence as part of shutdown completion. --- browse/src/cli.ts | 4 +--- browse/test/commands.test.ts | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 2e52ce1..714413b 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -226,9 +226,7 @@ async function ensureServer(): Promise { 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) { + if (!isProcessAlive(pid)) { return; } await Bun.sleep(100); diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 4d819a7..79cdb06 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -767,6 +767,39 @@ describe('CLI lifecycle', () => { await cleanupCliState(stateFile); } }, 25000); + + test('stop ignores recreated state file once the target pid exits', async () => { + const stateFile = `/tmp/browse-stop-recreated-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; port: number; token: string; startedAt: string; serverPath: string }>(stateFile); + expect(started.code).toBe(0); + expect(startedState?.pid).toBeTruthy(); + + const stopPromise = runCliCommand(['stop'], env); + await waitFor(() => !fs.existsSync(stateFile) ? true : null, 5000); + + fs.writeFileSync(stateFile, JSON.stringify({ + pid: 999999, + port, + token: 'fake-token', + startedAt: new Date().toISOString(), + serverPath: '/tmp/fake-server.ts', + })); + + const stop = await stopPromise; + expect(stop.code).toBe(0); + expect(stop.stdout).toContain('Server stopped'); + } finally { + await cleanupCliState(stateFile); + } + }, 20000); }); // ─── Buffer bounds ────────────────────────────────────────────── From 7a693dc052134e6e64fa0bfe9e219140597df679 Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 19:06:42 +0800 Subject: [PATCH 5/7] fix: keep snapshot refs contiguous --- browse/src/snapshot.ts | 3 +-- browse/test/snapshot.test.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/browse/src/snapshot.ts b/browse/src/snapshot.ts index 94f961c..3ff2ab5 100644 --- a/browse/src/snapshot.ts +++ b/browse/src/snapshot.ts @@ -168,8 +168,6 @@ export async function handleSnapshot( // Compact filter: skip elements with no name and no inline content that aren't interactive if (opts.compact && !isInteractive && !node.name && !node.children) continue; - // Assign ref - const ref = `e${refCounter++}`; const indent = ' '.repeat(depth); // Build Playwright locator @@ -207,6 +205,7 @@ export async function handleSnapshot( } if (!handle) continue; + const ref = `e${refCounter++}`; refMap.set(ref, handle); // Format output line diff --git a/browse/test/snapshot.test.ts b/browse/test/snapshot.test.ts index 1868e4b..72afe65 100644 --- a/browse/test/snapshot.test.ts +++ b/browse/test/snapshot.test.ts @@ -25,6 +25,14 @@ function extractRef(snapshot: string, predicate: (line: string) => boolean): str return `@${refMatch![1]}`; } +function extractRefNumbers(snapshot: string): number[] { + return snapshot + .split('\n') + .map((line) => line.match(/@e(\d+)/)) + .filter((match): match is RegExpMatchArray => Boolean(match)) + .map((match) => Number(match[1])); +} + beforeAll(async () => { testServer = startTestServer(0); baseUrl = testServer.url; @@ -113,6 +121,15 @@ describe('Snapshot', () => { expect(result).toContain('[button]'); expect(result).toContain('Say "Hello"'); }); + + test('snapshot emits contiguous refs when skipped nodes are not materialized', async () => { + await handleWriteCommand('goto', [baseUrl + '/snapshot.html'], bm); + const result = await handleMetaCommand('snapshot', [], bm, shutdown); + const refs = extractRefNumbers(result); + + expect(refs.length).toBeGreaterThan(0); + expect(refs).toEqual(Array.from({ length: refs.length }, (_, index) => index + 1)); + }); }); // ─── Ref-Based Interaction ────────────────────────────────────── From 5152aca65eac6439bc91f9991d21efdc0fcfdc15 Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 19:21:13 +0800 Subject: [PATCH 6/7] fix: keep browse state until daemon exit --- browse/src/server.ts | 9 +++--- browse/test/commands.test.ts | 62 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/browse/src/server.ts b/browse/src/server.ts index 65377a9..80553e7 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -186,10 +186,6 @@ async function shutdown() { clearInterval(idleCheckInterval); flushBuffers(); // Final flush - // 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. @@ -199,6 +195,11 @@ async function shutdown() { ]); } catch {} + // Keep the state file until exit. Other CLI processes treat it as proof that + // this PID still owns the port, so deleting it early can trigger a bogus + // replacement start while the old daemon is still shutting down. + try { fs.unlinkSync(STATE_FILE); } catch {} + process.exit(0); } diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 79cdb06..8843848 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -107,6 +107,15 @@ async function cleanupCliState(stateFile: string) { try { fs.unlinkSync(`${stateFile}.settings.json`); } catch {} } +function isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + // ─── Navigation ───────────────────────────────────────────────── describe('Navigation', () => { @@ -800,6 +809,59 @@ describe('CLI lifecycle', () => { await cleanupCliState(stateFile); } }, 20000); + + test('status during shutdown reuses the live daemon instead of racing a replacement', async () => { + const root = fs.mkdtempSync('/tmp/gstack-browse-shutdown-window-'); + const stateFile = path.join(root, 'browse-state.json'); + const shutdownMarker = path.join(root, 'shutdown-started'); + const wrapperPath = path.join(root, 'server-wrapper.ts'); + const realServerPath = path.resolve(__dirname, '../src/server.ts'); + const browserManagerPath = path.resolve(__dirname, '../src/browser-manager.ts'); + const port = reservePort(); + + fs.writeFileSync(wrapperPath, ` + import * as fs from 'fs'; + import { BrowserManager } from ${JSON.stringify(browserManagerPath)}; + + const originalClose = BrowserManager.prototype.close; + BrowserManager.prototype.close = async function (...args) { + fs.writeFileSync(${JSON.stringify(shutdownMarker)}, 'closing'); + await Bun.sleep(1500); + return await originalClose.apply(this, args); + }; + + await import(${JSON.stringify(realServerPath)}); + `); + + const env = { + BROWSE_STATE_FILE: stateFile, + BROWSE_PORT: String(port), + BROWSE_SERVER_SCRIPT: wrapperPath, + }; + + try { + const started = await runCliCommand(['status'], env); + const startedState = readJson<{ pid: number }>(stateFile); + expect(started.code).toBe(0); + expect(startedState?.pid).toBeTruthy(); + + const stopPromise = runCliCommand(['stop'], env, 15000); + const shutdownStarted = await waitFor(() => fs.existsSync(shutdownMarker) ? true : null, 5000); + expect(shutdownStarted).toBe(true); + expect(isPidAlive(startedState!.pid)).toBe(true); + + const status = await runCliCommand(['status'], env, 12000); + const stop = await stopPromise; + + expect(status.code).toBe(0); + expect(status.stdout).toContain('Status: healthy'); + expect(stop.code).toBe(0); + expect(stop.stdout).toContain('Server stopped'); + } finally { + await cleanupCliState(stateFile); + fs.rmSync(root, { recursive: true, force: true }); + } + }, 30000); }); // ─── Buffer bounds ────────────────────────────────────────────── From 24c3e20aa056a0e67b25314ac98393416aff0f09 Mon Sep 17 00:00:00 2001 From: morluto Date: Thu, 12 Mar 2026 19:43:04 +0800 Subject: [PATCH 7/7] fix: keep nth indices stable across snapshot filters --- browse/src/snapshot.ts | 19 +++++++++++++++---- browse/test/snapshot.test.ts | 29 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/browse/src/snapshot.ts b/browse/src/snapshot.ts index 3ff2ab5..73a9d25 100644 --- a/browse/src/snapshot.ts +++ b/browse/src/snapshot.ts @@ -146,6 +146,11 @@ export async function handleSnapshot( roleNameCounts.set(key, (roleNameCounts.get(key) || 0) + 1); } + function markRoleNameSeen(node: ParsedNode) { + const key = `${node.role}:${node.name || ''}`; + roleNameSeen.set(key, (roleNameSeen.get(key) || 0) + 1); + } + // Second pass: assign refs and build locators for (const line of lines) { const node = parseLine(line); @@ -155,18 +160,24 @@ export async function handleSnapshot( const isInteractive = INTERACTIVE_ROLES.has(node.role); // Depth filter - if (opts.depth !== undefined && depth > opts.depth) continue; + if (opts.depth !== undefined && depth > opts.depth) { + // Skipped nodes still occupy nth() slots for later same-name matches. + markRoleNameSeen(node); + continue; + } // Interactive filter: skip non-interactive but still count for locator indices if (opts.interactive && !isInteractive) { // Still track for nth() counts - const key = `${node.role}:${node.name || ''}`; - roleNameSeen.set(key, (roleNameSeen.get(key) || 0) + 1); + markRoleNameSeen(node); continue; } // Compact filter: skip elements with no name and no inline content that aren't interactive - if (opts.compact && !isInteractive && !node.name && !node.children) continue; + if (opts.compact && !isInteractive && !node.name && !node.children) { + markRoleNameSeen(node); + continue; + } const indent = ' '.repeat(depth); diff --git a/browse/test/snapshot.test.ts b/browse/test/snapshot.test.ts index 72afe65..c7b14c4 100644 --- a/browse/test/snapshot.test.ts +++ b/browse/test/snapshot.test.ts @@ -299,4 +299,33 @@ describe('Ref invalidation', () => { await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); expect(bm.getRefCount()).toBe(0); }); + + test('depth filter still freezes the correct nth same-name element', async () => { + const page = bm.getPage(); + await page.setContent(` +
+ + `); + + const snap = await handleMetaCommand('snapshot', ['-i', '-d', '1'], bm, shutdown); + const ref = extractRef(snap, (line) => line.includes('[button]') && line.includes('"Save"')); + + await handleWriteCommand('click', [ref], bm); + const clicked = await handleReadCommand('js', ['window.clicked'], bm); + expect(clicked).toBe('outer'); + }); + + test('compact filter still freezes the correct nth unnamed element', async () => { + const page = bm.getPage(); + await page.setContent(` +

+

Hello

+ `); + + const snap = await handleMetaCommand('snapshot', ['-c'], bm, shutdown); + const ref = extractRef(snap, (line) => line.includes('[paragraph]') && line.includes('Hello')); + + const html = await handleReadCommand('html', [ref], bm); + expect(html).toBe('Hello'); + }); });