From 6d08a44c089873093f31dd181f7e29fa085c9b96 Mon Sep 17 00:00:00 2001 From: coolxll Date: Wed, 25 Mar 2026 12:54:59 +0800 Subject: [PATCH 1/8] feat: support browser-level CDP websocket fallback --- src/browser/cdp.test.ts | 42 ++++++++- src/browser/cdp.ts | 195 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 219 insertions(+), 18 deletions(-) diff --git a/src/browser/cdp.test.ts b/src/browser/cdp.test.ts index 480f32ae..7680a527 100644 --- a/src/browser/cdp.test.ts +++ b/src/browser/cdp.test.ts @@ -36,7 +36,7 @@ vi.mock('ws', () => ({ WebSocket: MockWebSocket, })); -import { CDPBridge } from './cdp.js'; +import { CDPBridge, __test__ } from './cdp.js'; describe('CDPBridge cookies', () => { beforeEach(() => { @@ -64,3 +64,43 @@ describe('CDPBridge cookies', () => { ]); }); }); + +describe('CDP browser websocket helpers', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + it('accepts browser-level targets that only expose targetId', () => { + const target = __test__.selectCDPTarget([ + { + targetId: 'page-1', + type: 'page', + title: 'Hacker News', + url: 'https://news.ycombinator.com/', + }, + { + targetId: 'devtools-1', + type: 'page', + title: 'DevTools - localhost:9222', + url: 'devtools://devtools/bundled/inspector.html', + }, + ]); + + expect(target?.targetId).toBe('page-1'); + }); + + it('parses browser websocket URLs from DevToolsActivePort content', () => { + const wsUrl = __test__.parseBrowserWebSocketUrlFromActivePort( + '9222', + '127.0.0.1', + '9222\n/devtools/browser/abc-123\n', + ); + + expect(wsUrl).toBe('ws://127.0.0.1:9222/devtools/browser/abc-123'); + }); + + it('detects browser-level websocket endpoints', () => { + expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/browser/abc')).toBe(true); + expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/page/abc')).toBe(false); + }); +}); diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index 14b1054f..761260ea 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -11,6 +11,9 @@ import { WebSocket, type RawData } from 'ws'; import { request as httpRequest } from 'node:http'; import { request as httpsRequest } from 'node:https'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; import type { BrowserCookie, IPage, ScreenshotOptions, SnapshotOptions, WaitOptions } from '../types.js'; import { wrapForEval } from './utils.js'; import { generateSnapshotJs, scrollToRefJs, getFormStateJs } from './dom-snapshot.js'; @@ -28,6 +31,7 @@ import { import { isRecord, saveBase64ToFile } from '../utils.js'; export interface CDPTarget { + targetId?: string; type?: string; url?: string; title?: string; @@ -49,6 +53,7 @@ const CDP_SEND_TIMEOUT = 30_000; export class CDPBridge { private _ws: WebSocket | null = null; + private _sessionId: string | null = null; private _idCounter = 0; private _pending = new Map void; reject: (err: Error) => void; timer: ReturnType }>(); private _eventListeners = new Map void>>(); @@ -59,25 +64,27 @@ export class CDPBridge { const endpoint = process.env.OPENCLI_CDP_ENDPOINT; if (!endpoint) throw new Error('OPENCLI_CDP_ENDPOINT is not set'); - let wsUrl = endpoint; - if (endpoint.startsWith('http')) { - const targets = await fetchJsonDirect(`${endpoint.replace(/\/$/, '')}/json`) as CDPTarget[]; - const target = selectCDPTarget(targets); - if (!target || !target.webSocketDebuggerUrl) { - throw new Error('No inspectable targets found at CDP endpoint'); - } - wsUrl = target.webSocketDebuggerUrl; - } + const connection = await resolveConnectionEndpoint(endpoint); return new Promise((resolve, reject) => { - const ws = new WebSocket(wsUrl); - const timeoutMs = (opts?.timeout ?? 10) * 1000; + const ws = new WebSocket(connection.wsUrl); + const timeoutMs = (opts?.timeout ?? 10) * 1000; // opts.timeout is in seconds const timeout = setTimeout(() => reject(new Error('CDP connect timeout')), timeoutMs); ws.on('open', async () => { - clearTimeout(timeout); - this._ws = ws; try { + clearTimeout(timeout); + this._ws = ws; + if (connection.browserLevel) { + await this.attachToBestTarget(); + } + } catch (err) { + reject(err); + return; + } + + try { + // Register stealth script to run before any page JS on every navigation. await this.send('Page.enable'); await this.send('Page.addScriptToEvaluateOnNewDocument', { source: generateStealthJs() }); } catch {} @@ -103,6 +110,9 @@ export class CDPBridge { } } if (msg.method) { + if (this._sessionId && msg.sessionId && msg.sessionId !== this._sessionId) { + return; + } const listeners = this._eventListeners.get(msg.method); if (listeners) { for (const fn of listeners) fn(msg.params); @@ -118,6 +128,7 @@ export class CDPBridge { this._ws.close(); this._ws = null; } + this._sessionId = null; for (const p of this._pending.values()) { clearTimeout(p.timer); p.reject(new Error('CDP connection closed')); @@ -126,7 +137,13 @@ export class CDPBridge { this._eventListeners.clear(); } - async send(method: string, params: Record = {}, timeoutMs: number = CDP_SEND_TIMEOUT): Promise { + /** Send a CDP command with timeout guard (P0 fix #4) */ + async send( + method: string, + params: Record = {}, + timeoutMs: number = CDP_SEND_TIMEOUT, + opts: { root?: boolean } = {}, + ): Promise { if (!this._ws || this._ws.readyState !== WebSocket.OPEN) { throw new Error('CDP connection is not open'); } @@ -137,7 +154,11 @@ export class CDPBridge { reject(new Error(`CDP command '${method}' timed out after ${timeoutMs / 1000}s`)); }, timeoutMs); this._pending.set(id, { resolve, reject, timer }); - this._ws!.send(JSON.stringify({ id, method, params })); + const payload: Record = { id, method, params }; + if (this._sessionId && !opts.root) { + payload.sessionId = this._sessionId; + } + this._ws!.send(JSON.stringify(payload)); }); } @@ -168,6 +189,28 @@ export class CDPBridge { this.on(event, handler); }); } + + private async attachToBestTarget(): Promise { + const result = await this.send('Target.getTargets', {}, CDP_SEND_TIMEOUT, { root: true }); + const targetInfos = isRecord(result) && Array.isArray(result.targetInfos) + ? result.targetInfos as CDPTarget[] + : []; + const target = selectCDPTarget(targetInfos); + if (!target?.targetId) { + throw new Error('No inspectable targets found at CDP endpoint'); + } + + await this.send('Target.activateTarget', { targetId: target.targetId }, CDP_SEND_TIMEOUT, { root: true }).catch(() => {}); + const attachResult = await this.send('Target.attachToTarget', { + targetId: target.targetId, + flatten: true, + }, CDP_SEND_TIMEOUT, { root: true }); + const sessionId = isRecord(attachResult) ? attachResult.sessionId : undefined; + if (typeof sessionId !== 'string' || !sessionId) { + throw new Error(`Failed to attach to CDP target '${target.targetId}'`); + } + this._sessionId = sessionId; + } } class CDPPage implements IPage { @@ -295,7 +338,7 @@ class CDPPage implements IPage { } async newTab(): Promise { - await this.bridge.send('Target.createTarget', { url: 'about:blank' }); + await this.bridge.send('Target.createTarget', { url: 'about:blank' }, CDP_SEND_TIMEOUT, { root: true }); } async selectTab(_index: number): Promise { @@ -335,6 +378,121 @@ function matchesCookieDomain(cookieDomain: string, targetDomain: string): boolea || normalizedTargetDomain.endsWith(`.${normalizedCookieDomain}`); } +async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: string; browserLevel: boolean }> { + if (endpoint === 'auto') { + const wsUrl = resolveAnyBrowserWebSocketUrl(); + if (wsUrl) { + return { wsUrl, browserLevel: true }; + } + throw new Error('Failed to auto-discover a local browser CDP websocket. Start Chrome with --remote-debugging-port and retry, or set OPENCLI_CDP_ENDPOINT explicitly.'); + } + + if (endpoint.startsWith('ws://') || endpoint.startsWith('wss://')) { + return { + wsUrl: endpoint, + browserLevel: isBrowserLevelWebSocket(endpoint), + }; + } + + if (!endpoint.startsWith('http://') && !endpoint.startsWith('https://')) { + return { wsUrl: endpoint, browserLevel: false }; + } + + const normalized = endpoint.replace(/\/$/, ''); + try { + const targets = await fetchJsonDirect(`${normalized}/json`) as CDPTarget[]; + const target = selectCDPTarget(targets); + if (!target?.webSocketDebuggerUrl) { + throw new Error('No inspectable targets found at CDP endpoint'); + } + return { wsUrl: target.webSocketDebuggerUrl, browserLevel: false }; + } catch { + // Fall through to browser-level websocket discovery. + } + + const browserWsUrl = resolveBrowserWebSocketUrl(normalized); + if (browserWsUrl) { + return { wsUrl: browserWsUrl, browserLevel: true }; + } + + throw new Error('Failed to resolve an inspectable target from CDP endpoint'); +} + +function resolveBrowserWebSocketUrl(endpoint: string): string | null { + let parsed: URL; + try { + parsed = new URL(endpoint); + } catch { + return null; + } + + const host = parsed.hostname.toLowerCase(); + if (!['127.0.0.1', 'localhost'].includes(host)) return null; + const port = parsed.port || (parsed.protocol === 'https:' ? '443' : '80'); + + for (const filePath of getDevToolsActivePortCandidates()) { + try { + const content = fs.readFileSync(filePath, 'utf-8'); + const wsUrl = parseBrowserWebSocketUrlFromActivePort(port, host, content); + if (wsUrl) return wsUrl; + } catch { + // Try the next candidate. + } + } + + return null; +} + +function resolveAnyBrowserWebSocketUrl(host: string = '127.0.0.1'): string | null { + for (const filePath of getDevToolsActivePortCandidates()) { + try { + const content = fs.readFileSync(filePath, 'utf-8'); + const wsUrl = parseAnyBrowserWebSocketUrlFromActivePort(content, host); + if (wsUrl) return wsUrl; + } catch { + // Try the next candidate. + } + } + + return null; +} + +function parseBrowserWebSocketUrlFromActivePort(port: string, host: string, content: string): string | null { + const lines = content.trim().split(/\r?\n/).map(line => line.trim()).filter(Boolean); + if (lines.length < 2) return null; + if (lines[0] !== port) return null; + if (!lines[1].startsWith('/devtools/browser/')) return null; + return `ws://${host}:${port}${lines[1]}`; +} + +function parseAnyBrowserWebSocketUrlFromActivePort(content: string, host: string): string | null { + const lines = content.trim().split(/\r?\n/).map(line => line.trim()).filter(Boolean); + if (lines.length < 2) return null; + if (!/^\d+$/.test(lines[0])) return null; + if (!lines[1].startsWith('/devtools/browser/')) return null; + return `ws://${host}:${lines[0]}${lines[1]}`; +} + +function getDevToolsActivePortCandidates(): string[] { + const candidates: string[] = []; + if (process.platform === 'win32') { + const localAppData = process.env.LOCALAPPDATA ?? path.join(os.homedir(), 'AppData', 'Local'); + candidates.push(path.join(localAppData, 'Google', 'Chrome', 'User Data', 'DevToolsActivePort')); + candidates.push(path.join(localAppData, 'Microsoft', 'Edge', 'User Data', 'DevToolsActivePort')); + } else if (process.platform === 'darwin') { + candidates.push(path.join(os.homedir(), 'Library', 'Application Support', 'Google', 'Chrome', 'DevToolsActivePort')); + candidates.push(path.join(os.homedir(), 'Library', 'Application Support', 'Microsoft Edge', 'DevToolsActivePort')); + } else { + candidates.push(path.join(os.homedir(), '.config', 'google-chrome', 'DevToolsActivePort')); + candidates.push(path.join(os.homedir(), '.config', 'chromium', 'DevToolsActivePort')); + candidates.push(path.join(os.homedir(), '.config', 'microsoft-edge', 'DevToolsActivePort')); + } + return candidates; +} + +function isBrowserLevelWebSocket(endpoint: string): boolean { + return endpoint.includes('/devtools/browser/'); +} function selectCDPTarget(targets: CDPTarget[]): CDPTarget | undefined { const preferredPattern = compilePreferredPattern(process.env.OPENCLI_CDP_TARGET); @@ -350,7 +508,7 @@ function selectCDPTarget(targets: CDPTarget[]): CDPTarget | undefined { } function scoreCDPTarget(target: CDPTarget, preferredPattern?: RegExp): number { - if (!target.webSocketDebuggerUrl) return Number.NEGATIVE_INFINITY; + if (!target.webSocketDebuggerUrl && !target.targetId) return Number.NEGATIVE_INFINITY; const type = (target.type ?? '').toLowerCase(); const url = (target.url ?? '').toLowerCase(); @@ -406,6 +564,9 @@ function escapeRegExp(value: string): string { export const __test__ = { selectCDPTarget, scoreCDPTarget, + parseBrowserWebSocketUrlFromActivePort, + parseAnyBrowserWebSocketUrlFromActivePort, + isBrowserLevelWebSocket, }; function fetchJsonDirect(url: string): Promise { From d1de0fc5863a2814f14d446f5f8ee7f59b53792f Mon Sep 17 00:00:00 2001 From: coolxll Date: Wed, 25 Mar 2026 13:22:01 +0800 Subject: [PATCH 2/8] feat: add explicit browser CDP mode --- docs/advanced/cdp.md | 11 ++++ docs/guide/troubleshooting.md | 1 + src/browser/cdp.test.ts | 9 +++ src/cli.ts | 111 +++++++++++++++++++--------------- src/commanderAdapter.test.ts | 43 +++++++++++++ src/commanderAdapter.ts | 13 +++- src/runtime.test.ts | 46 ++++++++++++++ src/runtime.ts | 44 ++++++++++++++ 8 files changed, 229 insertions(+), 49 deletions(-) create mode 100644 src/commanderAdapter.test.ts create mode 100644 src/runtime.test.ts diff --git a/docs/advanced/cdp.md b/docs/advanced/cdp.md index 09079f31..dc2bedaa 100644 --- a/docs/advanced/cdp.md +++ b/docs/advanced/cdp.md @@ -101,3 +101,14 @@ opencli bilibili hot --limit 5 # Test a command > *Tip: If you provide a standard HTTP/HTTPS CDP endpoint, OpenCLI requests the `/json` target list and picks the most likely inspectable app/page target automatically. If multiple app targets exist, you can further narrow selection with `OPENCLI_CDP_TARGET` (for example `antigravity` or `codex`).* If you plan to use this setup frequently, you can persist the environment variable by adding the `export` line to your `~/.bashrc` or `~/.zshrc` on the server. + +## Local Shortcut: `--browser-cdp` + +For local browser-backed commands, you can bypass the daemon/extension path explicitly with `--browser-cdp`. + +```bash +opencli cascade https://news.ycombinator.com --browser-cdp +opencli explore https://linux.do --browser-cdp +``` + +When this flag is present, OpenCLI forces the browser command through Chrome CDP directly. If `OPENCLI_CDP_ENDPOINT` is not already set, OpenCLI will try to auto-discover a local Chrome/Edge debugging session from `DevToolsActivePort`. diff --git a/docs/guide/troubleshooting.md b/docs/guide/troubleshooting.md index e5d310fe..da9adaf1 100644 --- a/docs/guide/troubleshooting.md +++ b/docs/guide/troubleshooting.md @@ -6,6 +6,7 @@ - Ensure the opencli Browser Bridge extension is installed and **enabled** in `chrome://extensions`. - Run `opencli doctor` to diagnose connectivity. +- If Chrome is already running with `--remote-debugging-port`, browser-backed commands can bypass the daemon path explicitly with `--browser-cdp`. ### Empty data or 'Unauthorized' error diff --git a/src/browser/cdp.test.ts b/src/browser/cdp.test.ts index 7680a527..7a9433ae 100644 --- a/src/browser/cdp.test.ts +++ b/src/browser/cdp.test.ts @@ -99,6 +99,15 @@ describe('CDP browser websocket helpers', () => { expect(wsUrl).toBe('ws://127.0.0.1:9222/devtools/browser/abc-123'); }); + it('parses auto-discovered browser websocket URLs from DevToolsActivePort content', () => { + const wsUrl = __test__.parseAnyBrowserWebSocketUrlFromActivePort( + '9333\n/devtools/browser/abc-123\n', + '127.0.0.1', + ); + + expect(wsUrl).toBe('ws://127.0.0.1:9333/devtools/browser/abc-123'); + }); + it('detects browser-level websocket endpoints', () => { expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/browser/abc')).toBe(true); expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/page/abc')).toBe(false); diff --git a/src/cli.ts b/src/cli.ts index 6d005800..10ba9027 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -10,7 +10,7 @@ import chalk from 'chalk'; import { type CliCommand, fullName, getRegistry, strategyLabel } from './registry.js'; import { serializeCommand, formatArgSummary } from './serialization.js'; import { render as renderOutput } from './output.js'; -import { getBrowserFactory, browserSession } from './runtime.js'; +import { getBrowserFactory, browserSession, extractBrowserEnvOverrides, withBrowserEnvOverrides } from './runtime.js'; import { PKG_VERSION } from './version.js'; import { printCompletionScript } from './completion.js'; import { loadExternalClis, executeExternalCli, installExternalCli, registerExternalCli, isBinaryInstalled } from './external.js'; @@ -133,22 +133,25 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .option('--wait ', '', '3') .option('--auto', 'Enable interactive fuzzing') .option('--click ', 'Comma-separated labels to click before fuzzing') + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) .action(async (url, opts) => { - const { exploreUrl, renderExploreSummary } = await import('./explore.js'); - const clickLabels = opts.click - ? opts.click.split(',').map((s: string) => s.trim()) - : undefined; - const workspace = `explore:${inferHost(url, opts.site)}`; - const result = await exploreUrl(url, { - BrowserFactory: getBrowserFactory(), - site: opts.site, - goal: opts.goal, - waitSeconds: parseFloat(opts.wait), - auto: opts.auto, - clickLabels, - workspace, + await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + const { exploreUrl, renderExploreSummary } = await import('./explore.js'); + const clickLabels = opts.click + ? opts.click.split(',').map((s: string) => s.trim()) + : undefined; + const workspace = `explore:${inferHost(url, opts.site)}`; + const result = await exploreUrl(url, { + BrowserFactory: getBrowserFactory(), + site: opts.site, + goal: opts.goal, + waitSeconds: parseFloat(opts.wait), + auto: opts.auto, + clickLabels, + workspace, + }); + console.log(renderExploreSummary(result)); }); - console.log(renderExploreSummary(result)); }); program @@ -167,18 +170,21 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .argument('') .option('--goal ') .option('--site ') + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) .action(async (url, opts) => { - const { generateCliFromUrl, renderGenerateSummary } = await import('./generate.js'); - const workspace = `generate:${inferHost(url, opts.site)}`; - const r = await generateCliFromUrl({ - url, - BrowserFactory: getBrowserFactory(), - goal: opts.goal, - site: opts.site, - workspace, + await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + const { generateCliFromUrl, renderGenerateSummary } = await import('./generate.js'); + const workspace = `generate:${inferHost(url, opts.site)}`; + const r = await generateCliFromUrl({ + url, + BrowserFactory: getBrowserFactory(), + goal: opts.goal, + site: opts.site, + workspace, + }); + console.log(renderGenerateSummary(r)); + process.exitCode = r.ok ? 0 : 1; }); - console.log(renderGenerateSummary(r)); - process.exitCode = r.ok ? 0 : 1; }); // ── Built-in: record ───────────────────────────────────────────────────── @@ -191,18 +197,21 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .option('--out ', 'Output directory for candidates') .option('--poll ', 'Poll interval in milliseconds', '2000') .option('--timeout ', 'Auto-stop after N milliseconds (default: 60000)', '60000') + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) .action(async (url, opts) => { - const { recordSession, renderRecordSummary } = await import('./record.js'); - const result = await recordSession({ - BrowserFactory: getBrowserFactory(), - url, - site: opts.site, - outDir: opts.out, - pollMs: parseInt(opts.poll, 10), - timeoutMs: parseInt(opts.timeout, 10), + await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + const { recordSession, renderRecordSummary } = await import('./record.js'); + const result = await recordSession({ + BrowserFactory: getBrowserFactory(), + url, + site: opts.site, + outDir: opts.out, + pollMs: parseInt(opts.poll, 10), + timeoutMs: parseInt(opts.timeout, 10), + }); + console.log(renderRecordSummary(result)); + process.exitCode = result.candidateCount > 0 ? 0 : 1; }); - console.log(renderRecordSummary(result)); - process.exitCode = result.candidateCount > 0 ? 0 : 1; }); program @@ -210,18 +219,21 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .description('Strategy cascade: find simplest working strategy') .argument('') .option('--site ') + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) .action(async (url, opts) => { - const { cascadeProbe, renderCascadeResult } = await import('./cascade.js'); - const workspace = `cascade:${inferHost(url, opts.site)}`; - const result = await browserSession(getBrowserFactory(), async (page) => { - try { - const siteUrl = new URL(url); - await page.goto(`${siteUrl.protocol}//${siteUrl.host}`); - await page.wait(2); - } catch {} - return cascadeProbe(page, url); - }, { workspace }); - console.log(renderCascadeResult(result)); + await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + const { cascadeProbe, renderCascadeResult } = await import('./cascade.js'); + const workspace = `cascade:${inferHost(url, opts.site)}`; + const result = await browserSession(getBrowserFactory(), async (page) => { + try { + const siteUrl = new URL(url); + await page.goto(`${siteUrl.protocol}//${siteUrl.host}`); + await page.wait(2); + } catch {} + return cascadeProbe(page, url); + }, { workspace }); + console.log(renderCascadeResult(result)); + }); }); // ── Built-in: doctor / completion ────────────────────────────────────────── @@ -439,9 +451,12 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .command('serve') .description('Start Anthropic-compatible API proxy for Antigravity') .option('--port ', 'Server port (default: 8082)', '8082') + .option('--browser-cdp', 'Auto-discover a local Chrome CDP session instead of requiring OPENCLI_CDP_ENDPOINT', false) .action(async (opts) => { - const { startServe } = await import('./clis/antigravity/serve.js'); - await startServe({ port: parseInt(opts.port) }); + await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + const { startServe } = await import('./clis/antigravity/serve.js'); + await startServe({ port: parseInt(opts.port) }); + }); }); // ── Dynamic adapter commands ────────────────────────────────────────────── diff --git a/src/commanderAdapter.test.ts b/src/commanderAdapter.test.ts new file mode 100644 index 00000000..14807198 --- /dev/null +++ b/src/commanderAdapter.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; +import { Command } from 'commander'; +import { Strategy, type CliCommand } from './registry.js'; +import { registerCommandToProgram } from './commanderAdapter.js'; + +function buildCommand(overrides: Partial = {}): CliCommand { + return { + site: 'demo', + name: 'run', + description: 'demo command', + strategy: Strategy.COOKIE, + browser: true, + args: [], + ...overrides, + }; +} + +describe('registerCommandToProgram', () => { + it('adds --browser-cdp to browser-backed commands', () => { + const siteCmd = new Command('demo'); + + registerCommandToProgram(siteCmd, buildCommand()); + + const subCmd = siteCmd.commands[0]; + const longFlags = subCmd.options.map(option => option.long); + + expect(longFlags).toContain('--browser-cdp'); + }); + + it('does not add --browser-cdp to non-browser commands', () => { + const siteCmd = new Command('demo'); + + registerCommandToProgram(siteCmd, buildCommand({ + browser: false, + strategy: Strategy.PUBLIC, + })); + + const subCmd = siteCmd.commands[0]; + const longFlags = subCmd.options.map(option => option.long); + + expect(longFlags).not.toContain('--browser-cdp'); + }); +}); diff --git a/src/commanderAdapter.ts b/src/commanderAdapter.ts index a4fda706..71b11f36 100644 --- a/src/commanderAdapter.ts +++ b/src/commanderAdapter.ts @@ -17,6 +17,7 @@ import { formatRegistryHelpText } from './serialization.js'; import { render as renderOutput } from './output.js'; import { executeCommand } from './execution.js'; import { CliError, ERROR_ICONS, getErrorMessage } from './errors.js'; +import { extractBrowserEnvOverrides, withBrowserEnvOverrides } from './runtime.js'; /** * Register a single CliCommand as a Commander subcommand. @@ -43,6 +44,13 @@ export function registerCommandToProgram(siteCmd: Command, cmd: CliCommand): voi subCmd .option('-f, --format ', 'Output format: table, json, yaml, md, csv', 'table') .option('-v, --verbose', 'Debug output', false); + if (cmd.browser) { + subCmd.option( + '--browser-cdp', + 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', + false, + ); + } subCmd.addHelpText('after', formatRegistryHelpText(cmd)); @@ -70,7 +78,10 @@ export function registerCommandToProgram(siteCmd: Command, cmd: CliCommand): voi const format = typeof optionsRecord.format === 'string' ? optionsRecord.format : 'table'; if (verbose) process.env.OPENCLI_VERBOSE = '1'; - const result = await executeCommand(cmd, kwargs, verbose); + const result = await withBrowserEnvOverrides( + extractBrowserEnvOverrides(optionsRecord), + () => executeCommand(cmd, kwargs, verbose), + ); if (verbose && (!result || (Array.isArray(result) && result.length === 0))) { console.error(chalk.yellow('[Verbose] Warning: Command returned an empty result.')); diff --git a/src/runtime.test.ts b/src/runtime.test.ts new file mode 100644 index 00000000..792e8b11 --- /dev/null +++ b/src/runtime.test.ts @@ -0,0 +1,46 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { BrowserBridge, CDPBridge } from './browser/index.js'; +import { extractBrowserEnvOverrides, getBrowserFactory, withBrowserEnvOverrides } from './runtime.js'; + +describe('runtime browser CDP overrides', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + delete process.env.OPENCLI_CDP_ENDPOINT; + }); + + it('extracts the browser-cdp flag from commander-style options', () => { + expect(extractBrowserEnvOverrides({ browserCdp: true })).toEqual({ browserCdp: true }); + expect(extractBrowserEnvOverrides({ 'browser-cdp': 'true' })).toEqual({ browserCdp: true }); + expect(extractBrowserEnvOverrides({ browserCdp: false })).toEqual({ browserCdp: false }); + }); + + it('uses CDPBridge when OPENCLI_CDP_ENDPOINT is set to auto', () => { + expect(getBrowserFactory()).toBe(BrowserBridge); + + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'auto'); + + expect(getBrowserFactory()).toBe(CDPBridge); + }); + + it('temporarily enables automatic CDP discovery for browser-cdp commands', async () => { + let seenEndpoint: string | undefined; + + await withBrowserEnvOverrides({ browserCdp: true }, async () => { + seenEndpoint = process.env.OPENCLI_CDP_ENDPOINT; + return 'ok'; + }); + + expect(seenEndpoint).toBe('auto'); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); + }); + + it('does not overwrite an explicit CDP endpoint', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + + await withBrowserEnvOverrides({ browserCdp: true }, async () => { + expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); + }); + + expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); + }); +}); diff --git a/src/runtime.ts b/src/runtime.ts index f3b63961..02eab238 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -10,6 +10,50 @@ export function getBrowserFactory(): new () => IBrowserFactory { return (process.env.OPENCLI_CDP_ENDPOINT ? CDPBridge : BrowserBridge) as unknown as new () => IBrowserFactory; } +export interface BrowserEnvOverrides { + browserCdp?: boolean; +} + +export function extractBrowserEnvOverrides(options: Record): BrowserEnvOverrides { + return { + browserCdp: readBooleanOption(options, ['browser-cdp', 'browserCdp']), + }; +} + +export async function withBrowserEnvOverrides( + overrides: BrowserEnvOverrides, + fn: () => Promise, +): Promise { + if (!overrides.browserCdp || process.env.OPENCLI_CDP_ENDPOINT) { + return fn(); + } + + const previousEndpoint = process.env.OPENCLI_CDP_ENDPOINT; + process.env.OPENCLI_CDP_ENDPOINT = 'auto'; + try { + return await fn(); + } finally { + if (previousEndpoint === undefined) { + delete process.env.OPENCLI_CDP_ENDPOINT; + } else { + process.env.OPENCLI_CDP_ENDPOINT = previousEndpoint; + } + } +} + +function readBooleanOption(options: Record, names: string[]): boolean | undefined { + for (const name of names) { + const value = options[name]; + if (typeof value === 'boolean') return value; + if (typeof value === 'string') { + const normalized = value.trim().toLowerCase(); + if (normalized === 'true' || normalized === '1') return true; + if (normalized === 'false' || normalized === '0') return false; + } + } + return undefined; +} + function parseEnvTimeout(envVar: string, fallback: number): number { const raw = process.env[envVar]; if (raw === undefined) return fallback; From 3644d377bd94ee925d3e824d8c188acc216890ec Mon Sep 17 00:00:00 2001 From: coolxll Date: Wed, 25 Mar 2026 13:26:52 +0800 Subject: [PATCH 3/8] docs: add PR verification screenshot --- docs/images/pr-408-browser-cdp-mode.svg | 27 +++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 docs/images/pr-408-browser-cdp-mode.svg diff --git a/docs/images/pr-408-browser-cdp-mode.svg b/docs/images/pr-408-browser-cdp-mode.svg new file mode 100644 index 00000000..fab98267 --- /dev/null +++ b/docs/images/pr-408-browser-cdp-mode.svg @@ -0,0 +1,27 @@ + + + + + + + + PR #408 verification: browser websocket fallback + explicit --browser-cdp mode + + + + $ curl http://127.0.0.1:9222/json + HTTP/1.1 404 Not Found + $ type "%LOCALAPPDATA%\Google\Chrome\User Data\DevToolsActivePort" + 9222 + /devtools/browser/abc-123-example + Newer Chrome remote debugging sessions can expose a browser-level websocket even when the classic /json target list is unavailable. + + + + $ opencli cascade https://news.ycombinator.com --browser-cdp + Strategy Cascade: cookie (30% confidence) + ❌ public + ❌ cookie + ❌ header + The command reached browser execution through direct CDP mode instead of failing at the daemon / extension connection stage. + From 4cf851ffddd66859e9cad388cf87f2ec33c360f8 Mon Sep 17 00:00:00 2001 From: coolxll Date: Wed, 25 Mar 2026 13:39:22 +0800 Subject: [PATCH 4/8] fix: open fresh page for auto browser CDP mode --- src/browser/cdp.test.ts | 8 +++++++ src/browser/cdp.ts | 50 ++++++++++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/browser/cdp.test.ts b/src/browser/cdp.test.ts index 7a9433ae..32fca80a 100644 --- a/src/browser/cdp.test.ts +++ b/src/browser/cdp.test.ts @@ -112,4 +112,12 @@ describe('CDP browser websocket helpers', () => { expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/browser/abc')).toBe(true); expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/page/abc')).toBe(false); }); + + it('prefers a fresh target for auto-discovered browser sessions without an explicit target hint', () => { + expect(__test__.shouldPreferNewBrowserTarget('auto')).toBe(true); + + vi.stubEnv('OPENCLI_CDP_TARGET', 'linux.do'); + expect(__test__.shouldPreferNewBrowserTarget('auto')).toBe(false); + expect(__test__.shouldPreferNewBrowserTarget('http://127.0.0.1:9222')).toBe(false); + }); }); diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index 761260ea..458a5418 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -76,7 +76,7 @@ export class CDPBridge { clearTimeout(timeout); this._ws = ws; if (connection.browserLevel) { - await this.attachToBestTarget(); + await this.attachToBrowserTarget(connection.preferNewTarget); } } catch (err) { reject(err); @@ -190,24 +190,35 @@ export class CDPBridge { }); } - private async attachToBestTarget(): Promise { - const result = await this.send('Target.getTargets', {}, CDP_SEND_TIMEOUT, { root: true }); - const targetInfos = isRecord(result) && Array.isArray(result.targetInfos) - ? result.targetInfos as CDPTarget[] - : []; - const target = selectCDPTarget(targetInfos); - if (!target?.targetId) { + private async attachToBrowserTarget(preferNewTarget: boolean = false): Promise { + let targetId: string | undefined; + + if (preferNewTarget) { + const created = await this.send('Target.createTarget', { url: 'about:blank' }, CDP_SEND_TIMEOUT, { root: true }); + targetId = isRecord(created) && typeof created.targetId === 'string' ? created.targetId : undefined; + } + + if (!targetId) { + const result = await this.send('Target.getTargets', {}, CDP_SEND_TIMEOUT, { root: true }); + const targetInfos = isRecord(result) && Array.isArray(result.targetInfos) + ? result.targetInfos as CDPTarget[] + : []; + const target = selectCDPTarget(targetInfos); + targetId = target?.targetId; + } + + if (!targetId) { throw new Error('No inspectable targets found at CDP endpoint'); } - await this.send('Target.activateTarget', { targetId: target.targetId }, CDP_SEND_TIMEOUT, { root: true }).catch(() => {}); + await this.send('Target.activateTarget', { targetId }, CDP_SEND_TIMEOUT, { root: true }).catch(() => {}); const attachResult = await this.send('Target.attachToTarget', { - targetId: target.targetId, + targetId, flatten: true, }, CDP_SEND_TIMEOUT, { root: true }); const sessionId = isRecord(attachResult) ? attachResult.sessionId : undefined; if (typeof sessionId !== 'string' || !sessionId) { - throw new Error(`Failed to attach to CDP target '${target.targetId}'`); + throw new Error(`Failed to attach to CDP target '${targetId}'`); } this._sessionId = sessionId; } @@ -378,11 +389,11 @@ function matchesCookieDomain(cookieDomain: string, targetDomain: string): boolea || normalizedTargetDomain.endsWith(`.${normalizedCookieDomain}`); } -async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: string; browserLevel: boolean }> { +async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: string; browserLevel: boolean; preferNewTarget: boolean }> { if (endpoint === 'auto') { const wsUrl = resolveAnyBrowserWebSocketUrl(); if (wsUrl) { - return { wsUrl, browserLevel: true }; + return { wsUrl, browserLevel: true, preferNewTarget: shouldPreferNewBrowserTarget(endpoint) }; } throw new Error('Failed to auto-discover a local browser CDP websocket. Start Chrome with --remote-debugging-port and retry, or set OPENCLI_CDP_ENDPOINT explicitly.'); } @@ -391,11 +402,12 @@ async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: str return { wsUrl: endpoint, browserLevel: isBrowserLevelWebSocket(endpoint), + preferNewTarget: false, }; } if (!endpoint.startsWith('http://') && !endpoint.startsWith('https://')) { - return { wsUrl: endpoint, browserLevel: false }; + return { wsUrl: endpoint, browserLevel: false, preferNewTarget: false }; } const normalized = endpoint.replace(/\/$/, ''); @@ -405,14 +417,14 @@ async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: str if (!target?.webSocketDebuggerUrl) { throw new Error('No inspectable targets found at CDP endpoint'); } - return { wsUrl: target.webSocketDebuggerUrl, browserLevel: false }; + return { wsUrl: target.webSocketDebuggerUrl, browserLevel: false, preferNewTarget: false }; } catch { // Fall through to browser-level websocket discovery. } const browserWsUrl = resolveBrowserWebSocketUrl(normalized); if (browserWsUrl) { - return { wsUrl: browserWsUrl, browserLevel: true }; + return { wsUrl: browserWsUrl, browserLevel: true, preferNewTarget: false }; } throw new Error('Failed to resolve an inspectable target from CDP endpoint'); @@ -493,6 +505,11 @@ function getDevToolsActivePortCandidates(): string[] { function isBrowserLevelWebSocket(endpoint: string): boolean { return endpoint.includes('/devtools/browser/'); } + +function shouldPreferNewBrowserTarget(endpoint: string): boolean { + return endpoint === 'auto' && !process.env.OPENCLI_CDP_TARGET?.trim(); +} + function selectCDPTarget(targets: CDPTarget[]): CDPTarget | undefined { const preferredPattern = compilePreferredPattern(process.env.OPENCLI_CDP_TARGET); @@ -567,6 +584,7 @@ export const __test__ = { parseBrowserWebSocketUrlFromActivePort, parseAnyBrowserWebSocketUrlFromActivePort, isBrowserLevelWebSocket, + shouldPreferNewBrowserTarget, }; function fetchJsonDirect(url: string): Promise { From 276218bb243e20f3e7d6b8ad823c7950dead0aa7 Mon Sep 17 00:00:00 2001 From: coolxll Date: Wed, 25 Mar 2026 13:48:24 +0800 Subject: [PATCH 5/8] feat: support global browser CDP default --- docs/advanced/cdp.md | 8 ++++++++ docs/guide/troubleshooting.md | 1 + src/cli.ts | 15 ++++++++++----- src/commanderAdapter.ts | 5 ++++- src/runtime.test.ts | 24 ++++++++++++++++++++++++ src/runtime.ts | 31 ++++++++++++++++++++++++------- 6 files changed, 71 insertions(+), 13 deletions(-) diff --git a/docs/advanced/cdp.md b/docs/advanced/cdp.md index dc2bedaa..aca13d07 100644 --- a/docs/advanced/cdp.md +++ b/docs/advanced/cdp.md @@ -112,3 +112,11 @@ opencli explore https://linux.do --browser-cdp ``` When this flag is present, OpenCLI forces the browser command through Chrome CDP directly. If `OPENCLI_CDP_ENDPOINT` is not already set, OpenCLI will try to auto-discover a local Chrome/Edge debugging session from `DevToolsActivePort`. + +If you want this behavior globally, set: + +```bash +export OPENCLI_BROWSER_CDP=1 +``` + +With that environment variable enabled, browser-backed commands use the same direct-CDP path by default. You can still disable it for one command with `--no-browser-cdp`. diff --git a/docs/guide/troubleshooting.md b/docs/guide/troubleshooting.md index da9adaf1..181850c8 100644 --- a/docs/guide/troubleshooting.md +++ b/docs/guide/troubleshooting.md @@ -7,6 +7,7 @@ - Ensure the opencli Browser Bridge extension is installed and **enabled** in `chrome://extensions`. - Run `opencli doctor` to diagnose connectivity. - If Chrome is already running with `--remote-debugging-port`, browser-backed commands can bypass the daemon path explicitly with `--browser-cdp`. +- If you want browser-backed commands to use that path by default, set `OPENCLI_BROWSER_CDP=1`. ### Empty data or 'Unauthorized' error diff --git a/src/cli.ts b/src/cli.ts index 10ba9027..b4ccf1f6 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -133,7 +133,8 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .option('--wait ', '', '3') .option('--auto', 'Enable interactive fuzzing') .option('--click ', 'Comma-separated labels to click before fuzzing') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') + .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') .action(async (url, opts) => { await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { const { exploreUrl, renderExploreSummary } = await import('./explore.js'); @@ -170,7 +171,8 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .argument('') .option('--goal ') .option('--site ') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') + .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') .action(async (url, opts) => { await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { const { generateCliFromUrl, renderGenerateSummary } = await import('./generate.js'); @@ -197,7 +199,8 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .option('--out ', 'Output directory for candidates') .option('--poll ', 'Poll interval in milliseconds', '2000') .option('--timeout ', 'Auto-stop after N milliseconds (default: 60000)', '60000') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') + .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') .action(async (url, opts) => { await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { const { recordSession, renderRecordSummary } = await import('./record.js'); @@ -219,7 +222,8 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .description('Strategy cascade: find simplest working strategy') .argument('') .option('--site ') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', false) + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') + .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') .action(async (url, opts) => { await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { const { cascadeProbe, renderCascadeResult } = await import('./cascade.js'); @@ -451,7 +455,8 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .command('serve') .description('Start Anthropic-compatible API proxy for Antigravity') .option('--port ', 'Server port (default: 8082)', '8082') - .option('--browser-cdp', 'Auto-discover a local Chrome CDP session instead of requiring OPENCLI_CDP_ENDPOINT', false) + .option('--browser-cdp', 'Auto-discover a local Chrome CDP session instead of requiring OPENCLI_CDP_ENDPOINT') + .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') .action(async (opts) => { await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { const { startServe } = await import('./clis/antigravity/serve.js'); diff --git a/src/commanderAdapter.ts b/src/commanderAdapter.ts index 71b11f36..5e65ac41 100644 --- a/src/commanderAdapter.ts +++ b/src/commanderAdapter.ts @@ -48,7 +48,10 @@ export function registerCommandToProgram(siteCmd: Command, cmd: CliCommand): voi subCmd.option( '--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', - false, + ); + subCmd.option( + '--no-browser-cdp', + 'Disable direct Chrome CDP mode for this command, even if enabled globally', ); } diff --git a/src/runtime.test.ts b/src/runtime.test.ts index 792e8b11..be5f950a 100644 --- a/src/runtime.test.ts +++ b/src/runtime.test.ts @@ -34,6 +34,30 @@ describe('runtime browser CDP overrides', () => { expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); }); + it('uses global OPENCLI_BROWSER_CDP=1 when no per-command override is provided', async () => { + vi.stubEnv('OPENCLI_BROWSER_CDP', '1'); + + let seenEndpoint: string | undefined; + await withBrowserEnvOverrides({}, async () => { + seenEndpoint = process.env.OPENCLI_CDP_ENDPOINT; + }); + + expect(seenEndpoint).toBe('auto'); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); + }); + + it('lets an explicit command option disable a global browser CDP default', async () => { + vi.stubEnv('OPENCLI_BROWSER_CDP', '1'); + + let seenEndpoint: string | undefined; + await withBrowserEnvOverrides({ browserCdp: false }, async () => { + seenEndpoint = process.env.OPENCLI_CDP_ENDPOINT; + }); + + expect(seenEndpoint).toBeUndefined(); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); + }); + it('does not overwrite an explicit CDP endpoint', async () => { vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); diff --git a/src/runtime.ts b/src/runtime.ts index 02eab238..ab1c2538 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -24,7 +24,7 @@ export async function withBrowserEnvOverrides( overrides: BrowserEnvOverrides, fn: () => Promise, ): Promise { - if (!overrides.browserCdp || process.env.OPENCLI_CDP_ENDPOINT) { + if (!isBrowserCdpEnabled(overrides) || process.env.OPENCLI_CDP_ENDPOINT) { return fn(); } @@ -41,15 +41,32 @@ export async function withBrowserEnvOverrides( } } +function isBrowserCdpEnabled(overrides: BrowserEnvOverrides): boolean { + if (typeof overrides.browserCdp === 'boolean') { + return overrides.browserCdp; + } + return readBooleanEnv('OPENCLI_BROWSER_CDP') ?? false; +} + function readBooleanOption(options: Record, names: string[]): boolean | undefined { for (const name of names) { const value = options[name]; - if (typeof value === 'boolean') return value; - if (typeof value === 'string') { - const normalized = value.trim().toLowerCase(); - if (normalized === 'true' || normalized === '1') return true; - if (normalized === 'false' || normalized === '0') return false; - } + const parsed = parseBooleanValue(value); + if (parsed !== undefined) return parsed; + } + return undefined; +} + +function readBooleanEnv(name: string): boolean | undefined { + return parseBooleanValue(process.env[name]); +} + +function parseBooleanValue(value: unknown): boolean | undefined { + if (typeof value === 'boolean') return value; + if (typeof value === 'string') { + const normalized = value.trim().toLowerCase(); + if (normalized === 'true' || normalized === '1' || normalized === 'yes' || normalized === 'on') return true; + if (normalized === 'false' || normalized === '0' || normalized === 'no' || normalized === 'off') return false; } return undefined; } From 4c1588f44749d1e334c4ae2b840429dfea462108 Mon Sep 17 00:00:00 2001 From: coolxll Date: Thu, 26 Mar 2026 11:23:42 +0800 Subject: [PATCH 6/8] fix: consolidate browser CDP overrides --- src/browser/cdp.test.ts | 91 +++++++++++++++++++- src/browser/cdp.ts | 162 ++++++++++++++++++++++++++++------- src/browserEnvOptions.ts | 31 +++++++ src/cli.ts | 65 +++++++------- src/commanderAdapter.test.ts | 145 ++++++++++++++++++++++++------- src/commanderAdapter.ts | 23 ++--- src/registry.ts | 2 + src/runtime.test.ts | 140 ++++++++++++++++++++++-------- src/runtime.ts | 117 ++++++++++++++++--------- 9 files changed, 596 insertions(+), 180 deletions(-) create mode 100644 src/browserEnvOptions.ts diff --git a/src/browser/cdp.test.ts b/src/browser/cdp.test.ts index 32fca80a..6f92c922 100644 --- a/src/browser/cdp.test.ts +++ b/src/browser/cdp.test.ts @@ -89,6 +89,25 @@ describe('CDP browser websocket helpers', () => { expect(target?.targetId).toBe('page-1'); }); + it('filters browser-level attach targets to pages only', () => { + const target = __test__.selectBrowserPageTarget([ + { + targetId: 'worker-1', + type: 'service_worker', + title: 'Service Worker', + url: 'https://news.ycombinator.com/sw.js', + }, + { + targetId: 'page-1', + type: 'page', + title: 'Hacker News', + url: 'https://news.ycombinator.com/', + }, + ]); + + expect(target?.targetId).toBe('page-1'); + }); + it('parses browser websocket URLs from DevToolsActivePort content', () => { const wsUrl = __test__.parseBrowserWebSocketUrlFromActivePort( '9222', @@ -108,12 +127,24 @@ describe('CDP browser websocket helpers', () => { expect(wsUrl).toBe('ws://127.0.0.1:9333/devtools/browser/abc-123'); }); + it('extracts browser websocket URLs from /json/version payloads', () => { + expect(__test__.extractBrowserWebSocketUrlFromVersionPayload({ + webSocketDebuggerUrl: 'ws://127.0.0.1:9222/devtools/browser/abc', + })).toBe('ws://127.0.0.1:9222/devtools/browser/abc'); + }); + it('detects browser-level websocket endpoints', () => { expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/browser/abc')).toBe(true); expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/page/abc')).toBe(false); }); - it('prefers a fresh target for auto-discovered browser sessions without an explicit target hint', () => { + it('accepts IPv6 loopback hosts for local browser websocket fallback', () => { + expect(__test__.isLoopbackHost('[::1]')).toBe(true); + expect(__test__.isLoopbackHost('::1')).toBe(true); + expect(__test__.isLoopbackHost('192.168.0.10')).toBe(false); + }); + + it('prefers a fresh target only for auto-discovered browser sessions without an explicit target hint', () => { expect(__test__.shouldPreferNewBrowserTarget('auto')).toBe(true); vi.stubEnv('OPENCLI_CDP_TARGET', 'linux.do'); @@ -121,3 +152,61 @@ describe('CDP browser websocket helpers', () => { expect(__test__.shouldPreferNewBrowserTarget('http://127.0.0.1:9222')).toBe(false); }); }); + +describe('CDPBridge lifecycle', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + it('closes owned blank targets before disconnecting', async () => { + const bridge = new CDPBridge() as any; + bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; + bridge._ownedTargetId = 'target-1'; + + const sendSpy = vi.spyOn(bridge, 'send').mockResolvedValue({ success: true }); + + await bridge.close(); + + expect(sendSpy).toHaveBeenCalledWith( + 'Target.closeTarget', + { targetId: 'target-1' }, + expect.any(Number), + { root: true }, + ); + }); + + it('resets internal command ids on close', async () => { + const bridge = new CDPBridge() as any; + bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; + bridge._ownedTargetId = 'target-1'; + bridge._idCounter = 42; + + vi.spyOn(bridge, 'send').mockResolvedValue({ success: true }); + + await bridge.close(); + + expect(bridge._idCounter).toBe(0); + }); +}); + +describe('CDPPage navigation', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + it('surfaces Page.navigate errorText directly', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'ws://127.0.0.1:9222/devtools/page/1'); + + const bridge = new CDPBridge(); + vi.spyOn(bridge, 'send') + .mockResolvedValueOnce({}) + .mockResolvedValueOnce({}) + .mockResolvedValueOnce({}) + .mockResolvedValueOnce({ errorText: 'net::ERR_NAME_NOT_RESOLVED' }); + vi.spyOn(bridge, 'waitForEvent').mockRejectedValue(new Error('should not be awaited after errorText')); + + const page = await bridge.connect(); + + await expect(page.goto('http://oops-typo.com')).rejects.toThrow('net::ERR_NAME_NOT_RESOLVED'); + }); +}); diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index 458a5418..d8b7138e 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -28,7 +28,6 @@ import { networkRequestsJs, waitForDomStableJs, } from './dom-helpers.js'; -import { isRecord, saveBase64ToFile } from '../utils.js'; export interface CDPTarget { targetId?: string; @@ -49,11 +48,13 @@ interface RuntimeEvaluateResult { }; } -const CDP_SEND_TIMEOUT = 30_000; +const CDP_SEND_TIMEOUT = 30_000; // 30s per command +const CDP_CLOSE_TIMEOUT = 1_500; export class CDPBridge { private _ws: WebSocket | null = null; private _sessionId: string | null = null; + private _ownedTargetId: string | null = null; private _idCounter = 0; private _pending = new Map void; reject: (err: Error) => void; timer: ReturnType }>(); private _eventListeners = new Map void>>(); @@ -79,6 +80,7 @@ export class CDPBridge { await this.attachToBrowserTarget(connection.preferNewTarget); } } catch (err) { + await this.close().catch(() => {}); reject(err); return; } @@ -87,7 +89,9 @@ export class CDPBridge { // Register stealth script to run before any page JS on every navigation. await this.send('Page.enable'); await this.send('Page.addScriptToEvaluateOnNewDocument', { source: generateStealthJs() }); - } catch {} + } catch { + // Non-fatal: stealth is best-effort + } resolve(new CDPPage(this)); }); @@ -99,6 +103,7 @@ export class CDPBridge { ws.on('message', (data: RawData) => { try { const msg = JSON.parse(data.toString()); + // Handle command responses if (msg.id && this._pending.has(msg.id)) { const entry = this._pending.get(msg.id)!; clearTimeout(entry.timer); @@ -109,6 +114,7 @@ export class CDPBridge { entry.resolve(msg.result); } } + // Handle CDP events if (msg.method) { if (this._sessionId && msg.sessionId && msg.sessionId !== this._sessionId) { return; @@ -118,12 +124,18 @@ export class CDPBridge { for (const fn of listeners) fn(msg.params); } } - } catch {} + } catch { + // ignore parsing errors + } }); }); } async close(): Promise { + if (this._ownedTargetId && this._ws && this._ws.readyState === WebSocket.OPEN) { + await this.send('Target.closeTarget', { targetId: this._ownedTargetId }, CDP_CLOSE_TIMEOUT, { root: true }).catch(() => {}); + } + this._ownedTargetId = null; if (this._ws) { this._ws.close(); this._ws = null; @@ -135,6 +147,7 @@ export class CDPBridge { } this._pending.clear(); this._eventListeners.clear(); + this._idCounter = 0; } /** Send a CDP command with timeout guard (P0 fix #4) */ @@ -162,19 +175,19 @@ export class CDPBridge { }); } + /** Listen for a CDP event */ on(event: string, handler: (params: unknown) => void): void { let set = this._eventListeners.get(event); - if (!set) { - set = new Set(); - this._eventListeners.set(event, set); - } + if (!set) { set = new Set(); this._eventListeners.set(event, set); } set.add(handler); } + /** Remove a CDP event listener */ off(event: string, handler: (params: unknown) => void): void { this._eventListeners.get(event)?.delete(handler); } + /** Wait for a CDP event to fire (one-shot) */ waitForEvent(event: string, timeoutMs: number = 15_000): Promise { return new Promise((resolve, reject) => { const timer = setTimeout(() => { @@ -190,12 +203,13 @@ export class CDPBridge { }); } - private async attachToBrowserTarget(preferNewTarget: boolean = false): Promise { + private async attachToBrowserTarget(preferNewTarget: boolean): Promise { let targetId: string | undefined; if (preferNewTarget) { const created = await this.send('Target.createTarget', { url: 'about:blank' }, CDP_SEND_TIMEOUT, { root: true }); targetId = isRecord(created) && typeof created.targetId === 'string' ? created.targetId : undefined; + if (targetId) this._ownedTargetId = targetId; } if (!targetId) { @@ -203,7 +217,7 @@ export class CDPBridge { const targetInfos = isRecord(result) && Array.isArray(result.targetInfos) ? result.targetInfos as CDPTarget[] : []; - const target = selectCDPTarget(targetInfos); + const target = selectBrowserPageTarget(targetInfos); targetId = target?.targetId; } @@ -228,14 +242,29 @@ class CDPPage implements IPage { private _pageEnabled = false; constructor(private bridge: CDPBridge) {} + /** Navigate with proper load event waiting (P1 fix #3) */ async goto(url: string, options?: { waitUntil?: 'load' | 'none'; settleMs?: number }): Promise { if (!this._pageEnabled) { await this.bridge.send('Page.enable'); this._pageEnabled = true; } - const loadPromise = this.bridge.waitForEvent('Page.loadEventFired', 30_000).catch(() => {}); - await this.bridge.send('Page.navigate', { url }); - await loadPromise; + const loadPromise = this.bridge.waitForEvent('Page.loadEventFired', 30_000); + const navigateResult = await this.bridge.send('Page.navigate', { url }); + const navigationError = isRecord(navigateResult) && typeof navigateResult.errorText === 'string' + ? navigateResult.errorText.trim() + : ''; + if (navigationError) { + void loadPromise.catch(() => {}); + throw new Error(`Navigation failed for ${url}: ${navigationError}`); + } + try { + await loadPromise; + } catch (error) { + logVerbose(`[cdp] Timed out waiting for Page.loadEventFired after navigating to ${url}`, normalizeError(error)); + // Do not fail on load timeout alone; SPAs and long-polling pages may never emit a clean load event. + } + // Smart settle: use DOM stability detection instead of fixed sleep. + // settleMs is now a timeout cap (default 1000ms), not a fixed wait. if (options?.waitUntil !== 'none') { const maxMs = options?.settleMs ?? 1000; await this.evaluate(waitForDomStableJs(maxMs, Math.min(500, maxMs))); @@ -247,7 +276,7 @@ class CDPPage implements IPage { const result = await this.bridge.send('Runtime.evaluate', { expression, returnByValue: true, - awaitPromise: true, + awaitPromise: true }) as RuntimeEvaluateResult; if (result.exceptionDetails) { throw new Error('Evaluate error: ' + (result.exceptionDetails.exception?.description || 'Unknown exception')); @@ -276,6 +305,8 @@ class CDPPage implements IPage { return this.evaluate(snapshotJs); } + // ── Shared DOM operations (P1 fix #5 — using dom-helpers.ts) ── + async click(ref: string): Promise { await this.evaluate(clickJs(ref)); } @@ -298,12 +329,12 @@ class CDPPage implements IPage { async wait(options: number | WaitOptions): Promise { if (typeof options === 'number') { - await new Promise((resolve) => setTimeout(resolve, options * 1000)); + await new Promise(resolve => setTimeout(resolve, options * 1000)); return; } if (typeof options.time === 'number') { const waitTime = options.time; - await new Promise((resolve) => setTimeout(resolve, waitTime * 1000)); + await new Promise(resolve => setTimeout(resolve, waitTime * 1000)); return; } if (options.text) { @@ -312,6 +343,8 @@ class CDPPage implements IPage { } } + // ── Implemented methods (P1 fix #2) ── + async scroll(direction: string = 'down', amount: number = 500): Promise { await this.evaluate(scrollJs(direction, amount)); } @@ -375,6 +408,8 @@ class CDPPage implements IPage { } } +import { isRecord, saveBase64ToFile } from '../utils.js'; + function isCookie(value: unknown): value is BrowserCookie { return isRecord(value) && typeof value.name === 'string' @@ -389,20 +424,27 @@ function matchesCookieDomain(cookieDomain: string, targetDomain: string): boolea || normalizedTargetDomain.endsWith(`.${normalizedCookieDomain}`); } -async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: string; browserLevel: boolean; preferNewTarget: boolean }> { +async function resolveConnectionEndpoint( + endpoint: string, +): Promise<{ wsUrl: string; browserLevel: boolean; preferNewTarget: boolean }> { if (endpoint === 'auto') { const wsUrl = resolveAnyBrowserWebSocketUrl(); - if (wsUrl) { - return { wsUrl, browserLevel: true, preferNewTarget: shouldPreferNewBrowserTarget(endpoint) }; + if (!wsUrl) { + throw new Error('Failed to auto-discover a local browser CDP websocket. Start Chrome with --remote-debugging-port and retry, or set OPENCLI_CDP_ENDPOINT explicitly.'); } - throw new Error('Failed to auto-discover a local browser CDP websocket. Start Chrome with --remote-debugging-port and retry, or set OPENCLI_CDP_ENDPOINT explicitly.'); + return { + wsUrl, + browserLevel: true, + preferNewTarget: shouldPreferNewBrowserTarget(endpoint), + }; } if (endpoint.startsWith('ws://') || endpoint.startsWith('wss://')) { + const browserLevel = isBrowserLevelWebSocket(endpoint); return { wsUrl: endpoint, - browserLevel: isBrowserLevelWebSocket(endpoint), - preferNewTarget: false, + browserLevel, + preferNewTarget: browserLevel && shouldPreferNewBrowserTarget(endpoint), }; } @@ -411,6 +453,9 @@ async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: str } const normalized = endpoint.replace(/\/$/, ''); + let discoveryError: Error | undefined; + let versionError: Error | undefined; + try { const targets = await fetchJsonDirect(`${normalized}/json`) as CDPTarget[]; const target = selectCDPTarget(targets); @@ -418,16 +463,39 @@ async function resolveConnectionEndpoint(endpoint: string): Promise<{ wsUrl: str throw new Error('No inspectable targets found at CDP endpoint'); } return { wsUrl: target.webSocketDebuggerUrl, browserLevel: false, preferNewTarget: false }; - } catch { - // Fall through to browser-level websocket discovery. + } catch (error) { + discoveryError = normalizeError(error); + logVerbose(`[cdp] Failed to resolve page websocket from ${normalized}/json`, discoveryError); + } + + try { + const payload = await fetchJsonDirect(`${normalized}/json/version`); + const browserWsUrl = extractBrowserWebSocketUrlFromVersionPayload(payload); + if (browserWsUrl) { + return { + wsUrl: browserWsUrl, + browserLevel: true, + preferNewTarget: shouldPreferNewBrowserTarget(endpoint), + }; + } + } catch (error) { + versionError = normalizeError(error); + logVerbose(`[cdp] Failed to resolve browser websocket from ${normalized}/json/version`, versionError); } const browserWsUrl = resolveBrowserWebSocketUrl(normalized); if (browserWsUrl) { - return { wsUrl: browserWsUrl, browserLevel: true, preferNewTarget: false }; + return { + wsUrl: browserWsUrl, + browserLevel: true, + preferNewTarget: shouldPreferNewBrowserTarget(endpoint), + }; } - throw new Error('Failed to resolve an inspectable target from CDP endpoint'); + const detail = [discoveryError?.message, versionError?.message].filter(Boolean).join('; '); + throw new Error(detail + ? `Failed to resolve an inspectable target from CDP endpoint (${detail})` + : 'Failed to resolve an inspectable target from CDP endpoint'); } function resolveBrowserWebSocketUrl(endpoint: string): string | null { @@ -439,7 +507,7 @@ function resolveBrowserWebSocketUrl(endpoint: string): string | null { } const host = parsed.hostname.toLowerCase(); - if (!['127.0.0.1', 'localhost'].includes(host)) return null; + if (!isLoopbackHost(host)) return null; const port = parsed.port || (parsed.protocol === 'https:' ? '443' : '80'); for (const filePath of getDevToolsActivePortCandidates()) { @@ -474,7 +542,7 @@ function parseBrowserWebSocketUrlFromActivePort(port: string, host: string, cont if (lines.length < 2) return null; if (lines[0] !== port) return null; if (!lines[1].startsWith('/devtools/browser/')) return null; - return `ws://${host}:${port}${lines[1]}`; + return `ws://${formatHostForUrl(host)}:${port}${lines[1]}`; } function parseAnyBrowserWebSocketUrlFromActivePort(content: string, host: string): string | null { @@ -482,7 +550,7 @@ function parseAnyBrowserWebSocketUrlFromActivePort(content: string, host: string if (lines.length < 2) return null; if (!/^\d+$/.test(lines[0])) return null; if (!lines[1].startsWith('/devtools/browser/')) return null; - return `ws://${host}:${lines[0]}${lines[1]}`; + return `ws://${formatHostForUrl(host)}:${lines[0]}${lines[1]}`; } function getDevToolsActivePortCandidates(): string[] { @@ -510,6 +578,26 @@ function shouldPreferNewBrowserTarget(endpoint: string): boolean { return endpoint === 'auto' && !process.env.OPENCLI_CDP_TARGET?.trim(); } +function isLoopbackHost(host: string): boolean { + return ['127.0.0.1', 'localhost', '::1', '[::1]'].includes(host); +} + +function formatHostForUrl(host: string): string { + return host.includes(':') && !host.startsWith('[') ? `[${host}]` : host; +} + +function extractBrowserWebSocketUrlFromVersionPayload(payload: unknown): string | null { + if (!isRecord(payload)) return null; + const wsUrl = payload.webSocketDebuggerUrl; + return typeof wsUrl === 'string' && isBrowserLevelWebSocket(wsUrl) ? wsUrl : null; +} + +function selectBrowserPageTarget(targets: CDPTarget[]): CDPTarget | undefined { + return selectCDPTarget(targets.filter(target => (target.type ?? '').toLowerCase() === 'page')); +} + +// ── CDP target selection (unchanged) ── + function selectCDPTarget(targets: CDPTarget[]): CDPTarget | undefined { const preferredPattern = compilePreferredPattern(process.env.OPENCLI_CDP_TARGET); @@ -580,10 +668,13 @@ function escapeRegExp(value: string): string { export const __test__ = { selectCDPTarget, + selectBrowserPageTarget, scoreCDPTarget, parseBrowserWebSocketUrlFromActivePort, parseAnyBrowserWebSocketUrlFromActivePort, + extractBrowserWebSocketUrlFromVersionPayload, isBrowserLevelWebSocket, + isLoopbackHost, shouldPreferNewBrowserTarget, }; @@ -594,7 +685,7 @@ function fetchJsonDirect(url: string): Promise { const statusCode = res.statusCode ?? 0; if (statusCode < 200 || statusCode >= 300) { res.resume(); - reject(new Error(`Failed to fetch CDP targets: HTTP ${statusCode}`)); + reject(new Error(`HTTP ${statusCode}`)); return; } @@ -604,7 +695,7 @@ function fetchJsonDirect(url: string): Promise { try { resolve(JSON.parse(Buffer.concat(chunks).toString('utf8'))); } catch (error) { - reject(error instanceof Error ? error : new Error(String(error))); + reject(normalizeError(error)); } }); }); @@ -614,3 +705,12 @@ function fetchJsonDirect(url: string): Promise { request.end(); }); } + +function logVerbose(message: string, error?: Error): void { + if (process.env.OPENCLI_VERBOSE !== '1') return; + console.error(error ? `${message}: ${error.message}` : message); +} + +function normalizeError(error: unknown): Error { + return error instanceof Error ? error : new Error(String(error)); +} diff --git a/src/browserEnvOptions.ts b/src/browserEnvOptions.ts new file mode 100644 index 00000000..ac0b0f04 --- /dev/null +++ b/src/browserEnvOptions.ts @@ -0,0 +1,31 @@ +import { Command } from 'commander'; +import { extractBrowserEnvOverrides, withBrowserEnvOverrides } from './runtime.js'; + +export interface BrowserEnvOptionConfig { + allowBrowserCdp?: boolean; +} + +export function addBrowserEnvOverrideOptions( + command: Command, + config: BrowserEnvOptionConfig = {}, +): Command { + command + .option('--cdp-endpoint ', 'Override the CDP endpoint for this command') + .option('--cdp-target ', 'Prefer a CDP target whose title or URL matches this pattern'); + + if (config.allowBrowserCdp) { + command + .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') + .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally'); + } + + return command; +} + +export async function runWithBrowserEnvOptions( + options: Record | null | undefined, + fn: () => Promise, + config: BrowserEnvOptionConfig = {}, +): Promise { + return withBrowserEnvOverrides(extractBrowserEnvOverrides(options), fn, config); +} diff --git a/src/cli.ts b/src/cli.ts index b4ccf1f6..b2920c6f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -10,7 +10,8 @@ import chalk from 'chalk'; import { type CliCommand, fullName, getRegistry, strategyLabel } from './registry.js'; import { serializeCommand, formatArgSummary } from './serialization.js'; import { render as renderOutput } from './output.js'; -import { getBrowserFactory, browserSession, extractBrowserEnvOverrides, withBrowserEnvOverrides } from './runtime.js'; +import { getBrowserFactory, browserSession } from './runtime.js'; +import { addBrowserEnvOverrideOptions, runWithBrowserEnvOptions } from './browserEnvOptions.js'; import { PKG_VERSION } from './version.js'; import { printCompletionScript } from './completion.js'; import { loadExternalClis, executeExternalCli, installExternalCli, registerExternalCli, isBinaryInstalled } from './external.js'; @@ -123,7 +124,8 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { // ── Built-in: explore / synthesize / generate / cascade ─────────────────── - program + addBrowserEnvOverrideOptions( + program .command('explore') .alias('probe') .description('Explore a website: discover APIs, stores, and recommend strategies') @@ -132,11 +134,11 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { .option('--goal ') .option('--wait ', '', '3') .option('--auto', 'Enable interactive fuzzing') - .option('--click ', 'Comma-separated labels to click before fuzzing') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') - .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') + .option('--click ', 'Comma-separated labels to click before fuzzing'), + { allowBrowserCdp: true }, + ) .action(async (url, opts) => { - await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + await runWithBrowserEnvOptions(opts, async () => { const { exploreUrl, renderExploreSummary } = await import('./explore.js'); const clickLabels = opts.click ? opts.click.split(',').map((s: string) => s.trim()) @@ -152,7 +154,7 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { workspace, }); console.log(renderExploreSummary(result)); - }); + }, { allowBrowserCdp: true }); }); program @@ -165,16 +167,17 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { console.log(renderSynthesizeSummary(synthesizeFromExplore(target, { top: parseInt(opts.top) }))); }); - program + addBrowserEnvOverrideOptions( + program .command('generate') .description('One-shot: explore → synthesize → register') .argument('') .option('--goal ') - .option('--site ') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') - .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') + .option('--site '), + { allowBrowserCdp: true }, + ) .action(async (url, opts) => { - await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + await runWithBrowserEnvOptions(opts, async () => { const { generateCliFromUrl, renderGenerateSummary } = await import('./generate.js'); const workspace = `generate:${inferHost(url, opts.site)}`; const r = await generateCliFromUrl({ @@ -186,23 +189,24 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { }); console.log(renderGenerateSummary(r)); process.exitCode = r.ok ? 0 : 1; - }); + }, { allowBrowserCdp: true }); }); // ── Built-in: record ───────────────────────────────────────────────────── - program + addBrowserEnvOverrideOptions( + program .command('record') .description('Record API calls from a live browser session → generate YAML candidates') .argument('', 'URL to open and record') .option('--site ', 'Site name (inferred from URL if omitted)') .option('--out ', 'Output directory for candidates') .option('--poll ', 'Poll interval in milliseconds', '2000') - .option('--timeout ', 'Auto-stop after N milliseconds (default: 60000)', '60000') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') - .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') + .option('--timeout ', 'Auto-stop after N milliseconds (default: 60000)', '60000'), + { allowBrowserCdp: true }, + ) .action(async (url, opts) => { - await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + await runWithBrowserEnvOptions(opts, async () => { const { recordSession, renderRecordSummary } = await import('./record.js'); const result = await recordSession({ BrowserFactory: getBrowserFactory(), @@ -214,18 +218,19 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { }); console.log(renderRecordSummary(result)); process.exitCode = result.candidateCount > 0 ? 0 : 1; - }); + }, { allowBrowserCdp: true }); }); - program + addBrowserEnvOverrideOptions( + program .command('cascade') .description('Strategy cascade: find simplest working strategy') .argument('') - .option('--site ') - .option('--browser-cdp', 'Connect directly to a local Chrome CDP session and bypass the daemon/extension') - .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') + .option('--site '), + { allowBrowserCdp: true }, + ) .action(async (url, opts) => { - await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + await runWithBrowserEnvOptions(opts, async () => { const { cascadeProbe, renderCascadeResult } = await import('./cascade.js'); const workspace = `cascade:${inferHost(url, opts.site)}`; const result = await browserSession(getBrowserFactory(), async (page) => { @@ -237,7 +242,7 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { return cascadeProbe(page, url); }, { workspace }); console.log(renderCascadeResult(result)); - }); + }, { allowBrowserCdp: true }); }); // ── Built-in: doctor / completion ────────────────────────────────────────── @@ -451,14 +456,14 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { // ── Antigravity serve (long-running, special case) ──────────────────────── const antigravityCmd = program.command('antigravity').description('antigravity commands'); - antigravityCmd + addBrowserEnvOverrideOptions( + antigravityCmd .command('serve') .description('Start Anthropic-compatible API proxy for Antigravity') - .option('--port ', 'Server port (default: 8082)', '8082') - .option('--browser-cdp', 'Auto-discover a local Chrome CDP session instead of requiring OPENCLI_CDP_ENDPOINT') - .option('--no-browser-cdp', 'Disable direct Chrome CDP mode for this command, even if enabled globally') + .option('--port ', 'Server port (default: 8082)', '8082'), + ) .action(async (opts) => { - await withBrowserEnvOverrides(extractBrowserEnvOverrides(opts), async () => { + await runWithBrowserEnvOptions(opts, async () => { const { startServe } = await import('./clis/antigravity/serve.js'); await startServe({ port: parseInt(opts.port) }); }); diff --git a/src/commanderAdapter.test.ts b/src/commanderAdapter.test.ts index 14807198..cbc98a27 100644 --- a/src/commanderAdapter.test.ts +++ b/src/commanderAdapter.test.ts @@ -1,43 +1,130 @@ -import { describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { Command } from 'commander'; -import { Strategy, type CliCommand } from './registry.js'; -import { registerCommandToProgram } from './commanderAdapter.js'; +import type { CliCommand } from './registry.js'; + +const { mockExecuteCommand, mockRender } = vi.hoisted(() => ({ + mockExecuteCommand: vi.fn(), + mockRender: vi.fn(), +})); -function buildCommand(overrides: Partial = {}): CliCommand { - return { - site: 'demo', - name: 'run', - description: 'demo command', - strategy: Strategy.COOKIE, - browser: true, - args: [], - ...overrides, - }; -} +vi.mock('./execution.js', () => ({ + executeCommand: mockExecuteCommand, +})); + +vi.mock('./output.js', () => ({ + render: mockRender, +})); + +import { registerCommandToProgram } from './commanderAdapter.js'; describe('registerCommandToProgram', () => { - it('adds --browser-cdp to browser-backed commands', () => { - const siteCmd = new Command('demo'); + beforeEach(() => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + process.exitCode = undefined; + }); + + it('applies command-level CDP overrides only while a browser command executes', async () => { + const seen: Array<{ endpoint?: string; target?: string }> = []; + mockExecuteCommand.mockImplementation(async () => { + seen.push({ + endpoint: process.env.OPENCLI_CDP_ENDPOINT, + target: process.env.OPENCLI_CDP_TARGET, + }); + return []; + }); + + const cmd: CliCommand = { + site: 'antigravity', + name: 'status', + description: 'status', + browser: true, + supportsBrowserCdp: false, + args: [], + }; + + const program = new Command(); + const siteCmd = program.command('antigravity'); + registerCommandToProgram(siteCmd, cmd); + + await program.parseAsync([ + 'node', + 'opencli', + 'antigravity', + 'status', + '--cdp-endpoint', + 'http://127.0.0.1:9333', + '--cdp-target', + 'launchpad', + ]); + + expect(mockExecuteCommand).toHaveBeenCalledWith(cmd, {}, false); + expect(seen).toEqual([ + { + endpoint: 'http://127.0.0.1:9333', + target: 'launchpad', + }, + ]); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); + expect(process.env.OPENCLI_CDP_TARGET).toBeUndefined(); + }); + + it('enables browser-cdp auto mode only for supported browser commands', async () => { + const seen: Array<{ endpoint?: string }> = []; + mockExecuteCommand.mockImplementation(async () => { + seen.push({ endpoint: process.env.OPENCLI_CDP_ENDPOINT }); + return []; + }); + + const cmd: CliCommand = { + site: 'linux-do', + name: 'categories', + description: 'categories', + browser: true, + supportsBrowserCdp: true, + args: [], + }; - registerCommandToProgram(siteCmd, buildCommand()); + const program = new Command(); + const siteCmd = program.command('linux-do'); + registerCommandToProgram(siteCmd, cmd); - const subCmd = siteCmd.commands[0]; - const longFlags = subCmd.options.map(option => option.long); + await program.parseAsync([ + 'node', + 'opencli', + 'linux-do', + 'categories', + '--browser-cdp', + ]); - expect(longFlags).toContain('--browser-cdp'); + expect(seen).toEqual([{ endpoint: 'auto' }]); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); }); - it('does not add --browser-cdp to non-browser commands', () => { - const siteCmd = new Command('demo'); + it('does not register browser-cdp flags for desktop-style commands', async () => { + const cmd: CliCommand = { + site: 'cursor', + name: 'ask', + description: 'ask', + browser: true, + supportsBrowserCdp: false, + args: [], + }; - registerCommandToProgram(siteCmd, buildCommand({ - browser: false, - strategy: Strategy.PUBLIC, - })); + const program = new Command(); + program.exitOverride(); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const siteCmd = program.command('cursor'); + registerCommandToProgram(siteCmd, cmd); - const subCmd = siteCmd.commands[0]; - const longFlags = subCmd.options.map(option => option.long); + await expect(program.parseAsync([ + 'node', + 'opencli', + 'cursor', + 'ask', + '--browser-cdp', + ])).rejects.toThrow(); - expect(longFlags).not.toContain('--browser-cdp'); + errorSpy.mockRestore(); }); }); diff --git a/src/commanderAdapter.ts b/src/commanderAdapter.ts index 5e65ac41..abe4a31a 100644 --- a/src/commanderAdapter.ts +++ b/src/commanderAdapter.ts @@ -17,7 +17,7 @@ import { formatRegistryHelpText } from './serialization.js'; import { render as renderOutput } from './output.js'; import { executeCommand } from './execution.js'; import { CliError, ERROR_ICONS, getErrorMessage } from './errors.js'; -import { extractBrowserEnvOverrides, withBrowserEnvOverrides } from './runtime.js'; +import { addBrowserEnvOverrideOptions, runWithBrowserEnvOptions } from './browserEnvOptions.js'; /** * Register a single CliCommand as a Commander subcommand. @@ -45,14 +45,7 @@ export function registerCommandToProgram(siteCmd: Command, cmd: CliCommand): voi .option('-f, --format ', 'Output format: table, json, yaml, md, csv', 'table') .option('-v, --verbose', 'Debug output', false); if (cmd.browser) { - subCmd.option( - '--browser-cdp', - 'Connect directly to a local Chrome CDP session and bypass the daemon/extension', - ); - subCmd.option( - '--no-browser-cdp', - 'Disable direct Chrome CDP mode for this command, even if enabled globally', - ); + addBrowserEnvOverrideOptions(subCmd, { allowBrowserCdp: cmd.supportsBrowserCdp !== false }); } subCmd.addHelpText('after', formatRegistryHelpText(cmd)); @@ -80,11 +73,13 @@ export function registerCommandToProgram(siteCmd: Command, cmd: CliCommand): voi const verbose = optionsRecord.verbose === true; const format = typeof optionsRecord.format === 'string' ? optionsRecord.format : 'table'; if (verbose) process.env.OPENCLI_VERBOSE = '1'; - - const result = await withBrowserEnvOverrides( - extractBrowserEnvOverrides(optionsRecord), - () => executeCommand(cmd, kwargs, verbose), - ); + const result = cmd.browser + ? await runWithBrowserEnvOptions( + optionsRecord, + () => executeCommand(cmd, kwargs, verbose), + { allowBrowserCdp: cmd.supportsBrowserCdp !== false }, + ) + : await executeCommand(cmd, kwargs, verbose); if (verbose && (!result || (Array.isArray(result) && result.length === 0))) { console.error(chalk.yellow('[Verbose] Warning: Command returned an empty result.')); diff --git a/src/registry.ts b/src/registry.ts index a864e9f2..f9d001cd 100644 --- a/src/registry.ts +++ b/src/registry.ts @@ -37,6 +37,7 @@ export interface CliCommand { domain?: string; strategy?: Strategy; browser?: boolean; + supportsBrowserCdp?: boolean; args: Arg[]; columns?: string[]; func?: (page: IPage, kwargs: CommandArgs, debug?: boolean) => Promise; @@ -88,6 +89,7 @@ export function cli(opts: CliOptions): CliCommand { domain: opts.domain, strategy, browser, + supportsBrowserCdp: opts.supportsBrowserCdp ?? (browser && !(opts.domain === 'localhost' && strategy === Strategy.UI)), args: opts.args ?? [], columns: opts.columns, func: opts.func, diff --git a/src/runtime.test.ts b/src/runtime.test.ts index be5f950a..96ad8883 100644 --- a/src/runtime.test.ts +++ b/src/runtime.test.ts @@ -1,70 +1,142 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { BrowserBridge, CDPBridge } from './browser/index.js'; -import { extractBrowserEnvOverrides, getBrowserFactory, withBrowserEnvOverrides } from './runtime.js'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { extractBrowserEnvOverrides, withBrowserEnvOverrides } from './runtime.js'; -describe('runtime browser CDP overrides', () => { +describe('browser env overrides', () => { beforeEach(() => { vi.unstubAllEnvs(); - delete process.env.OPENCLI_CDP_ENDPOINT; }); - it('extracts the browser-cdp flag from commander-style options', () => { - expect(extractBrowserEnvOverrides({ browserCdp: true })).toEqual({ browserCdp: true }); - expect(extractBrowserEnvOverrides({ 'browser-cdp': 'true' })).toEqual({ browserCdp: true }); - expect(extractBrowserEnvOverrides({ browserCdp: false })).toEqual({ browserCdp: false }); + afterEach(() => { + vi.unstubAllEnvs(); }); - it('uses CDPBridge when OPENCLI_CDP_ENDPOINT is set to auto', () => { - expect(getBrowserFactory()).toBe(BrowserBridge); - - vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'auto'); + it('extracts browser overrides from commander-style option names', () => { + expect(extractBrowserEnvOverrides({ + 'browser-cdp': true, + 'cdp-endpoint': ' http://127.0.0.1:9333 ', + 'cdp-target': ' antigravity ', + })).toEqual({ + browserCdp: true, + cdpEndpoint: 'http://127.0.0.1:9333', + cdpTarget: 'antigravity', + }); + }); - expect(getBrowserFactory()).toBe(CDPBridge); + it('rejects invalid cdp endpoint values early', () => { + expect(() => extractBrowserEnvOverrides({ + 'cdp-endpoint': 'foobar', + })).toThrow('Invalid --cdp-endpoint value'); }); - it('temporarily enables automatic CDP discovery for browser-cdp commands', async () => { + it('temporarily applies overrides and restores previous values', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + vi.stubEnv('OPENCLI_CDP_TARGET', 'codex'); + let seenEndpoint: string | undefined; + let seenTarget: string | undefined; - await withBrowserEnvOverrides({ browserCdp: true }, async () => { + await withBrowserEnvOverrides({ + cdpEndpoint: 'http://127.0.0.1:9333', + cdpTarget: 'antigravity', + }, async () => { seenEndpoint = process.env.OPENCLI_CDP_ENDPOINT; - return 'ok'; + seenTarget = process.env.OPENCLI_CDP_TARGET; }); - expect(seenEndpoint).toBe('auto'); - expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); + expect(seenEndpoint).toBe('http://127.0.0.1:9333'); + expect(seenTarget).toBe('antigravity'); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); + expect(process.env.OPENCLI_CDP_TARGET).toBe('codex'); }); - it('uses global OPENCLI_BROWSER_CDP=1 when no per-command override is provided', async () => { - vi.stubEnv('OPENCLI_BROWSER_CDP', '1'); + it('leaves unrelated browser env unchanged when an override is omitted', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + vi.stubEnv('OPENCLI_CDP_TARGET', 'cursor'); let seenEndpoint: string | undefined; - await withBrowserEnvOverrides({}, async () => { + let seenTarget: string | undefined; + + await withBrowserEnvOverrides({ + cdpEndpoint: 'http://127.0.0.1:9333', + }, async () => { seenEndpoint = process.env.OPENCLI_CDP_ENDPOINT; + seenTarget = process.env.OPENCLI_CDP_TARGET; }); - expect(seenEndpoint).toBe('auto'); - expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); + expect(seenEndpoint).toBe('http://127.0.0.1:9333'); + expect(seenTarget).toBe('cursor'); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); + expect(process.env.OPENCLI_CDP_TARGET).toBe('cursor'); }); - it('lets an explicit command option disable a global browser CDP default', async () => { - vi.stubEnv('OPENCLI_BROWSER_CDP', '1'); + it('prefers command-level browser-cdp auto mode over existing env defaults', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); let seenEndpoint: string | undefined; - await withBrowserEnvOverrides({ browserCdp: false }, async () => { + await withBrowserEnvOverrides({ + browserCdp: true, + }, async () => { seenEndpoint = process.env.OPENCLI_CDP_ENDPOINT; + }, { allowBrowserCdp: true }); + + expect(seenEndpoint).toBe('auto'); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); + }); + + it('restores outer overrides correctly across nested calls', async () => { + const seen: Array<{ label: string; endpoint?: string; target?: string }> = []; + + await withBrowserEnvOverrides({ + cdpEndpoint: 'http://127.0.0.1:9333', + cdpTarget: 'outer', + }, async () => { + seen.push({ + label: 'outer-before', + endpoint: process.env.OPENCLI_CDP_ENDPOINT, + target: process.env.OPENCLI_CDP_TARGET, + }); + + await withBrowserEnvOverrides({ + cdpEndpoint: 'http://127.0.0.1:9444', + cdpTarget: 'inner', + }, async () => { + seen.push({ + label: 'inner', + endpoint: process.env.OPENCLI_CDP_ENDPOINT, + target: process.env.OPENCLI_CDP_TARGET, + }); + }); + + seen.push({ + label: 'outer-after', + endpoint: process.env.OPENCLI_CDP_ENDPOINT, + target: process.env.OPENCLI_CDP_TARGET, + }); }); - expect(seenEndpoint).toBeUndefined(); + expect(seen).toEqual([ + { label: 'outer-before', endpoint: 'http://127.0.0.1:9333', target: 'outer' }, + { label: 'inner', endpoint: 'http://127.0.0.1:9444', target: 'inner' }, + { label: 'outer-after', endpoint: 'http://127.0.0.1:9333', target: 'outer' }, + ]); expect(process.env.OPENCLI_CDP_ENDPOINT).toBeUndefined(); + expect(process.env.OPENCLI_CDP_TARGET).toBeUndefined(); }); - it('does not overwrite an explicit CDP endpoint', async () => { - vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + it('honors global browser cdp default only for commands that allow it', async () => { + vi.stubEnv('OPENCLI_BROWSER_CDP', '1'); - await withBrowserEnvOverrides({ browserCdp: true }, async () => { - expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); - }); + let seenAllowed: string | undefined; + await withBrowserEnvOverrides({}, async () => { + seenAllowed = process.env.OPENCLI_CDP_ENDPOINT; + }, { allowBrowserCdp: true }); - expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); + let seenDisallowed: string | undefined; + await withBrowserEnvOverrides({}, async () => { + seenDisallowed = process.env.OPENCLI_CDP_ENDPOINT; + }, { allowBrowserCdp: false }); + + expect(seenAllowed).toBe('auto'); + expect(seenDisallowed).toBeUndefined(); }); }); diff --git a/src/runtime.ts b/src/runtime.ts index ab1c2538..25edee23 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -2,6 +2,16 @@ import { BrowserBridge, CDPBridge } from './browser/index.js'; import type { IPage } from './types.js'; import { TimeoutError } from './errors.js'; +export type BrowserEnvOverrides = { + browserCdp?: boolean; + cdpEndpoint?: string; + cdpTarget?: string; +}; + +export interface BrowserEnvOverrideConfig { + allowBrowserCdp?: boolean; +} + /** * Returns the appropriate browser factory based on environment config. * Uses CDPBridge when OPENCLI_CDP_ENDPOINT is set, otherwise BrowserBridge. @@ -10,76 +20,101 @@ export function getBrowserFactory(): new () => IBrowserFactory { return (process.env.OPENCLI_CDP_ENDPOINT ? CDPBridge : BrowserBridge) as unknown as new () => IBrowserFactory; } -export interface BrowserEnvOverrides { - browserCdp?: boolean; -} - -export function extractBrowserEnvOverrides(options: Record): BrowserEnvOverrides { +export function extractBrowserEnvOverrides(options?: Record | null): BrowserEnvOverrides { + const input = options ?? {}; return { - browserCdp: readBooleanOption(options, ['browser-cdp', 'browserCdp']), + browserCdp: readBooleanOption(input['browser-cdp'] ?? input.browserCdp), + cdpEndpoint: readCdpEndpointOption(input['cdp-endpoint'] ?? input.cdpEndpoint), + cdpTarget: readStringOption(input['cdp-target'] ?? input.cdpTarget), }; } export async function withBrowserEnvOverrides( overrides: BrowserEnvOverrides, fn: () => Promise, + config: BrowserEnvOverrideConfig = {}, ): Promise { - if (!isBrowserCdpEnabled(overrides) || process.env.OPENCLI_CDP_ENDPOINT) { - return fn(); + const effectiveEndpoint = resolveEffectiveCdpEndpoint(overrides, config); + const pairs: Array<[key: 'OPENCLI_CDP_ENDPOINT' | 'OPENCLI_CDP_TARGET', value: string | undefined]> = [ + ['OPENCLI_CDP_ENDPOINT', effectiveEndpoint], + ['OPENCLI_CDP_TARGET', overrides.cdpTarget], + ]; + const previous = new Map(); + + for (const [key, value] of pairs) { + if (value === undefined) continue; + previous.set(key, process.env[key]); + process.env[key] = value; } - const previousEndpoint = process.env.OPENCLI_CDP_ENDPOINT; - process.env.OPENCLI_CDP_ENDPOINT = 'auto'; try { return await fn(); } finally { - if (previousEndpoint === undefined) { - delete process.env.OPENCLI_CDP_ENDPOINT; - } else { - process.env.OPENCLI_CDP_ENDPOINT = previousEndpoint; + for (const [key, value] of pairs) { + if (value === undefined) continue; + const prior = previous.get(key); + if (prior === undefined) delete process.env[key]; + else process.env[key] = prior; } } } -function isBrowserCdpEnabled(overrides: BrowserEnvOverrides): boolean { - if (typeof overrides.browserCdp === 'boolean') { - return overrides.browserCdp; +function parseEnvTimeout(envVar: string, fallback: number): number { + const raw = process.env[envVar]; + if (raw === undefined) return fallback; + const parsed = parseInt(raw, 10); + if (Number.isNaN(parsed) || parsed <= 0) { + console.error(`[runtime] Invalid ${envVar}="${raw}", using default ${fallback}s`); + return fallback; } - return readBooleanEnv('OPENCLI_BROWSER_CDP') ?? false; + return parsed; } -function readBooleanOption(options: Record, names: string[]): boolean | undefined { - for (const name of names) { - const value = options[name]; - const parsed = parseBooleanValue(value); - if (parsed !== undefined) return parsed; - } +function readStringOption(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + const trimmed = value.trim(); + return trimmed ? trimmed : undefined; +} + +function readBooleanOption(value: unknown): boolean | undefined { + if (typeof value === 'boolean') return value; + if (typeof value !== 'string') return undefined; + const normalized = value.trim().toLowerCase(); + if (['1', 'true', 'yes', 'on'].includes(normalized)) return true; + if (['0', 'false', 'no', 'off'].includes(normalized)) return false; return undefined; } function readBooleanEnv(name: string): boolean | undefined { - return parseBooleanValue(process.env[name]); + return readBooleanOption(process.env[name]); } -function parseBooleanValue(value: unknown): boolean | undefined { - if (typeof value === 'boolean') return value; - if (typeof value === 'string') { - const normalized = value.trim().toLowerCase(); - if (normalized === 'true' || normalized === '1' || normalized === 'yes' || normalized === 'on') return true; - if (normalized === 'false' || normalized === '0' || normalized === 'no' || normalized === 'off') return false; - } - return undefined; +function readCdpEndpointOption(value: unknown): string | undefined { + const normalized = readStringOption(value); + if (!normalized) return undefined; + if (normalized === 'auto') return normalized; + if (/^(https?|wss?):\/\//.test(normalized)) return normalized; + throw new Error('Invalid --cdp-endpoint value. Expected http://, https://, ws://, or wss:// URL.'); } -function parseEnvTimeout(envVar: string, fallback: number): number { - const raw = process.env[envVar]; - if (raw === undefined) return fallback; - const parsed = parseInt(raw, 10); - if (Number.isNaN(parsed) || parsed <= 0) { - console.error(`[runtime] Invalid ${envVar}="${raw}", using default ${fallback}s`); - return fallback; +function resolveEffectiveCdpEndpoint( + overrides: BrowserEnvOverrides, + config: BrowserEnvOverrideConfig, +): string | undefined { + if (overrides.cdpEndpoint) { + if (overrides.cdpEndpoint === 'auto' && !config.allowBrowserCdp) { + throw new Error('The "auto" CDP endpoint is only supported for browser CDP commands.'); + } + return overrides.cdpEndpoint; } - return parsed; + + if (!config.allowBrowserCdp) return undefined; + + if (typeof overrides.browserCdp === 'boolean') { + return overrides.browserCdp ? 'auto' : undefined; + } + + return readBooleanEnv('OPENCLI_BROWSER_CDP') ? 'auto' : undefined; } export const DEFAULT_BROWSER_CONNECT_TIMEOUT = parseEnvTimeout('OPENCLI_BROWSER_CONNECT_TIMEOUT', 30); From 920069d9ead1747d28aad5771f95693b5abe2b90 Mon Sep 17 00:00:00 2001 From: coolxll Date: Thu, 26 Mar 2026 23:19:08 +0800 Subject: [PATCH 7/8] fix: harden browser cdp fallback --- src/browser/cdp.test.ts | 230 ++++++++++++++++++++++++++++++++++- src/browser/cdp.ts | 166 ++++++++++++++++++++++--- src/build-manifest.test.ts | 24 +++- src/build-manifest.ts | 21 ++++ src/commanderAdapter.test.ts | 75 +++++++++++- src/commanderAdapter.ts | 38 ++++-- src/discovery.ts | 19 ++- src/engine.test.ts | 30 +++++ src/registry.test.ts | 32 +++++ src/registry.ts | 37 +++++- src/runtime.test.ts | 14 +++ src/runtime.ts | 9 +- 12 files changed, 655 insertions(+), 40 deletions(-) diff --git a/src/browser/cdp.test.ts b/src/browser/cdp.test.ts index 6f92c922..48543e50 100644 --- a/src/browser/cdp.test.ts +++ b/src/browser/cdp.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'node:fs'; const { MockWebSocket } = vi.hoisted(() => { class MockWebSocket { @@ -38,6 +39,14 @@ vi.mock('ws', () => ({ import { CDPBridge, __test__ } from './cdp.js'; +function clearPersistentTargetRegistry(): void { + try { + fs.unlinkSync(__test__.persistentTargetRegistryPath); + } catch { + // Ignore missing file. + } +} + describe('CDPBridge cookies', () => { beforeEach(() => { vi.unstubAllEnvs(); @@ -68,6 +77,8 @@ describe('CDPBridge cookies', () => { describe('CDP browser websocket helpers', () => { beforeEach(() => { vi.unstubAllEnvs(); + vi.restoreAllMocks(); + clearPersistentTargetRegistry(); }); it('accepts browser-level targets that only expose targetId', () => { @@ -89,8 +100,14 @@ describe('CDP browser websocket helpers', () => { expect(target?.targetId).toBe('page-1'); }); - it('filters browser-level attach targets to pages only', () => { - const target = __test__.selectBrowserPageTarget([ + it('keeps browser-level attach compatible with app and webview targets', () => { + const target = __test__.selectBrowserAttachTarget([ + { + targetId: 'app-1', + type: 'app', + title: 'Codex Desktop', + url: 'file:///app/index.html', + }, { targetId: 'worker-1', type: 'service_worker', @@ -105,7 +122,7 @@ describe('CDP browser websocket helpers', () => { }, ]); - expect(target?.targetId).toBe('page-1'); + expect(target?.targetId).toBe('app-1'); }); it('parses browser websocket URLs from DevToolsActivePort content', () => { @@ -133,6 +150,20 @@ describe('CDP browser websocket helpers', () => { })).toBe('ws://127.0.0.1:9222/devtools/browser/abc'); }); + it('rewrites loopback /json/version browser websocket URLs onto proxied endpoints', () => { + expect(__test__.rewriteBrowserWebSocketUrlForEndpoint( + 'https://demo.ngrok.app', + 'ws://127.0.0.1:9222/devtools/browser/abc', + )).toBe('wss://demo.ngrok.app/devtools/browser/abc'); + }); + + it('rewrites loopback /json page websocket URLs onto proxied endpoints too', () => { + expect(__test__.rewriteBrowserWebSocketUrlForEndpoint( + 'https://demo.ngrok.app', + 'ws://127.0.0.1:9222/devtools/page/abc', + )).toBe('wss://demo.ngrok.app/devtools/page/abc'); + }); + it('detects browser-level websocket endpoints', () => { expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/browser/abc')).toBe(true); expect(__test__.isBrowserLevelWebSocket('ws://127.0.0.1:9222/devtools/page/abc')).toBe(false); @@ -151,17 +182,38 @@ describe('CDP browser websocket helpers', () => { expect(__test__.shouldPreferNewBrowserTarget('auto')).toBe(false); expect(__test__.shouldPreferNewBrowserTarget('http://127.0.0.1:9222')).toBe(false); }); + + it('normalizes blank workspaces away before persistence decisions', () => { + expect(__test__.normalizeWorkspaceKey(' site:twitter ')).toBe('site:twitter'); + expect(__test__.normalizeWorkspaceKey(' ')).toBeUndefined(); + }); + + it('matches cached browser targets for app/webview tabs too', () => { + const target = __test__.selectTargetById([ + { + targetId: 'webview-1', + type: 'webview', + title: 'Embedded App', + url: 'https://embedded.example/app', + }, + ], 'webview-1'); + + expect(target?.targetId).toBe('webview-1'); + }); }); describe('CDPBridge lifecycle', () => { beforeEach(() => { vi.unstubAllEnvs(); + vi.restoreAllMocks(); + clearPersistentTargetRegistry(); }); it('closes owned blank targets before disconnecting', async () => { const bridge = new CDPBridge() as any; bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; bridge._ownedTargetId = 'target-1'; + bridge._closeOwnedTargetOnClose = true; const sendSpy = vi.spyOn(bridge, 'send').mockResolvedValue({ success: true }); @@ -175,10 +227,29 @@ describe('CDPBridge lifecycle', () => { ); }); + it('keeps workspace-persistent targets open on disconnect', async () => { + const bridge = new CDPBridge() as any; + bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; + bridge._ownedTargetId = 'target-1'; + bridge._closeOwnedTargetOnClose = false; + + const sendSpy = vi.spyOn(bridge, 'send').mockResolvedValue({ success: true }); + + await bridge.close(); + + expect(sendSpy).not.toHaveBeenCalledWith( + 'Target.closeTarget', + expect.anything(), + expect.anything(), + expect.anything(), + ); + }); + it('resets internal command ids on close', async () => { const bridge = new CDPBridge() as any; bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; bridge._ownedTargetId = 'target-1'; + bridge._closeOwnedTargetOnClose = true; bridge._idCounter = 42; vi.spyOn(bridge, 'send').mockResolvedValue({ success: true }); @@ -187,6 +258,127 @@ describe('CDPBridge lifecycle', () => { expect(bridge._idCounter).toBe(0); }); + + it('reuses a stored workspace target before creating a new browser target', async () => { + const bridge = new CDPBridge() as any; + bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; + + fs.writeFileSync( + __test__.persistentTargetRegistryPath, + JSON.stringify({ + [__test__.makePersistentTargetRegistryKey('auto', 'site:twitter')]: 'target-keep', + }), + 'utf8', + ); + + const sendSpy = vi.spyOn(bridge, 'send').mockImplementation(async (...args: unknown[]) => { + const method = args[0] as string; + switch (method) { + case 'Target.getTargets': + return { + targetInfos: [ + { targetId: 'target-keep', type: 'page', title: 'X', url: 'https://x.com/home' }, + ], + }; + case 'Target.activateTarget': + return {}; + case 'Target.attachToTarget': + return { sessionId: 'session-1' }; + default: + return {}; + } + }); + + await bridge.attachToBrowserTarget({ + preferNewTarget: true, + workspace: 'site:twitter', + endpointKey: 'auto', + }); + + expect(sendSpy).not.toHaveBeenCalledWith( + 'Target.createTarget', + expect.anything(), + expect.anything(), + expect.anything(), + ); + expect(bridge._sessionId).toBe('session-1'); + }); + + it('lets explicit target hints override a stored workspace target', async () => { + const bridge = new CDPBridge() as any; + bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; + + vi.stubEnv('OPENCLI_CDP_TARGET', 'second'); + fs.writeFileSync( + __test__.persistentTargetRegistryPath, + JSON.stringify({ + [__test__.makePersistentTargetRegistryKey('auto', 'site:twitter')]: 'target-keep', + }), + 'utf8', + ); + + const activated: string[] = []; + vi.spyOn(bridge, 'send').mockImplementation(async (...args: unknown[]) => { + const method = args[0] as string; + const params = args[1] as Record | undefined; + switch (method) { + case 'Target.getTargets': + return { + targetInfos: [ + { targetId: 'target-keep', type: 'page', title: 'First Tab', url: 'https://x.com/home' }, + { targetId: 'target-second', type: 'page', title: 'Second Match', url: 'https://second.example' }, + ], + }; + case 'Target.activateTarget': + if (params && typeof params.targetId === 'string') activated.push(params.targetId); + return {}; + case 'Target.attachToTarget': + return { sessionId: 'session-2' }; + default: + return {}; + } + }); + + await bridge.attachToBrowserTarget({ + preferNewTarget: true, + workspace: 'site:twitter', + endpointKey: 'auto', + }); + + expect(activated).toEqual(['target-second']); + expect(fs.readFileSync(__test__.persistentTargetRegistryPath, 'utf8')).toContain('"auto::site:twitter": "target-second"'); + }); + + it('persists newly created workspace targets for later reuse', async () => { + const bridge = new CDPBridge() as any; + bridge._ws = { readyState: MockWebSocket.OPEN, close: vi.fn() }; + + vi.spyOn(bridge, 'send').mockImplementation(async (...args: unknown[]) => { + const method = args[0] as string; + switch (method) { + case 'Target.getTargets': + return { targetInfos: [] }; + case 'Target.createTarget': + return { targetId: 'target-new' }; + case 'Target.activateTarget': + return {}; + case 'Target.attachToTarget': + return { sessionId: 'session-1' }; + default: + return {}; + } + }); + + await bridge.attachToBrowserTarget({ + preferNewTarget: true, + workspace: 'site:twitter', + endpointKey: 'auto', + }); + + expect(fs.existsSync(__test__.persistentTargetRegistryPath)).toBe(true); + expect(fs.readFileSync(__test__.persistentTargetRegistryPath, 'utf8')).toContain('"auto::site:twitter": "target-new"'); + expect(bridge._closeOwnedTargetOnClose).toBe(false); + }); }); describe('CDPPage navigation', () => { @@ -209,4 +401,36 @@ describe('CDPPage navigation', () => { await expect(page.goto('http://oops-typo.com')).rejects.toThrow('net::ERR_NAME_NOT_RESOLVED'); }); + + it('does not leave an unhandled rejection behind when Page.navigate throws', async () => { + vi.useFakeTimers(); + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'ws://127.0.0.1:9222/devtools/page/1'); + + const bridge = new CDPBridge(); + vi.spyOn(bridge, 'send') + .mockResolvedValueOnce({}) + .mockResolvedValueOnce({}) + .mockResolvedValueOnce({}) + .mockRejectedValueOnce(new Error('Target closed')); + vi.spyOn(bridge, 'waitForEvent').mockImplementation(() => + new Promise((_, reject) => setTimeout(() => reject(new Error('load timeout')), 10)) + ); + + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => { + unhandled.push(reason); + }; + process.on('unhandledRejection', onUnhandled); + + try { + const page = await bridge.connect(); + await expect(page.goto('https://example.com')).rejects.toThrow('Target closed'); + await vi.runAllTimersAsync(); + await Promise.resolve(); + expect(unhandled).toEqual([]); + } finally { + process.off('unhandledRejection', onUnhandled); + vi.useRealTimers(); + } + }); }); diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index d8b7138e..31c30ad1 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -50,11 +50,13 @@ interface RuntimeEvaluateResult { const CDP_SEND_TIMEOUT = 30_000; // 30s per command const CDP_CLOSE_TIMEOUT = 1_500; +const PERSISTENT_TARGET_REGISTRY_PATH = path.join(os.tmpdir(), 'opencli-cdp-targets.json'); export class CDPBridge { private _ws: WebSocket | null = null; private _sessionId: string | null = null; private _ownedTargetId: string | null = null; + private _closeOwnedTargetOnClose = false; private _idCounter = 0; private _pending = new Map void; reject: (err: Error) => void; timer: ReturnType }>(); private _eventListeners = new Map void>>(); @@ -77,7 +79,11 @@ export class CDPBridge { clearTimeout(timeout); this._ws = ws; if (connection.browserLevel) { - await this.attachToBrowserTarget(connection.preferNewTarget); + await this.attachToBrowserTarget({ + preferNewTarget: connection.preferNewTarget, + workspace: normalizeWorkspaceKey(opts?.workspace), + endpointKey: endpoint, + }); } } catch (err) { await this.close().catch(() => {}); @@ -132,10 +138,11 @@ export class CDPBridge { } async close(): Promise { - if (this._ownedTargetId && this._ws && this._ws.readyState === WebSocket.OPEN) { + if (this._ownedTargetId && this._closeOwnedTargetOnClose && this._ws && this._ws.readyState === WebSocket.OPEN) { await this.send('Target.closeTarget', { targetId: this._ownedTargetId }, CDP_CLOSE_TIMEOUT, { root: true }).catch(() => {}); } this._ownedTargetId = null; + this._closeOwnedTargetOnClose = false; if (this._ws) { this._ws.close(); this._ws = null; @@ -203,21 +210,40 @@ export class CDPBridge { }); } - private async attachToBrowserTarget(preferNewTarget: boolean): Promise { + private async attachToBrowserTarget(opts: { preferNewTarget: boolean; workspace?: string; endpointKey: string }): Promise { let targetId: string | undefined; + const targetInfos = await this.listBrowserTargets(); - if (preferNewTarget) { + if (hasExplicitTargetHint()) { + const hintedTarget = selectBrowserAttachTarget(targetInfos); + targetId = hintedTarget?.targetId; + if (targetId && opts.workspace) { + setPersistentTargetId(opts.endpointKey, opts.workspace, targetId); + } + } else if (opts.workspace) { + const storedTargetId = getPersistentTargetId(opts.endpointKey, opts.workspace); + const storedTarget = selectTargetById(targetInfos, storedTargetId); + if (storedTarget?.targetId) { + targetId = storedTarget.targetId; + } else if (storedTargetId) { + clearPersistentTargetId(opts.endpointKey, opts.workspace); + } + } + + if (!targetId && opts.preferNewTarget) { const created = await this.send('Target.createTarget', { url: 'about:blank' }, CDP_SEND_TIMEOUT, { root: true }); targetId = isRecord(created) && typeof created.targetId === 'string' ? created.targetId : undefined; - if (targetId) this._ownedTargetId = targetId; + if (targetId) { + this._ownedTargetId = targetId; + this._closeOwnedTargetOnClose = !opts.workspace; + if (opts.workspace) { + setPersistentTargetId(opts.endpointKey, opts.workspace, targetId); + } + } } if (!targetId) { - const result = await this.send('Target.getTargets', {}, CDP_SEND_TIMEOUT, { root: true }); - const targetInfos = isRecord(result) && Array.isArray(result.targetInfos) - ? result.targetInfos as CDPTarget[] - : []; - const target = selectBrowserPageTarget(targetInfos); + const target = selectBrowserAttachTarget(targetInfos); targetId = target?.targetId; } @@ -236,6 +262,13 @@ export class CDPBridge { } this._sessionId = sessionId; } + + private async listBrowserTargets(): Promise { + const result = await this.send('Target.getTargets', {}, CDP_SEND_TIMEOUT, { root: true }); + return isRecord(result) && Array.isArray(result.targetInfos) + ? result.targetInfos as CDPTarget[] + : []; + } } class CDPPage implements IPage { @@ -249,12 +282,14 @@ class CDPPage implements IPage { this._pageEnabled = true; } const loadPromise = this.bridge.waitForEvent('Page.loadEventFired', 30_000); + // Guard the event wait immediately so a navigation transport failure does not + // leave a late unhandled rejection behind. + void loadPromise.catch(() => {}); const navigateResult = await this.bridge.send('Page.navigate', { url }); const navigationError = isRecord(navigateResult) && typeof navigateResult.errorText === 'string' ? navigateResult.errorText.trim() : ''; if (navigationError) { - void loadPromise.catch(() => {}); throw new Error(`Navigation failed for ${url}: ${navigationError}`); } try { @@ -462,7 +497,11 @@ async function resolveConnectionEndpoint( if (!target?.webSocketDebuggerUrl) { throw new Error('No inspectable targets found at CDP endpoint'); } - return { wsUrl: target.webSocketDebuggerUrl, browserLevel: false, preferNewTarget: false }; + return { + wsUrl: rewriteBrowserWebSocketUrlForEndpoint(normalized, target.webSocketDebuggerUrl) ?? target.webSocketDebuggerUrl, + browserLevel: false, + preferNewTarget: false, + }; } catch (error) { discoveryError = normalizeError(error); logVerbose(`[cdp] Failed to resolve page websocket from ${normalized}/json`, discoveryError); @@ -470,7 +509,10 @@ async function resolveConnectionEndpoint( try { const payload = await fetchJsonDirect(`${normalized}/json/version`); - const browserWsUrl = extractBrowserWebSocketUrlFromVersionPayload(payload); + const browserWsUrl = rewriteBrowserWebSocketUrlForEndpoint( + normalized, + extractBrowserWebSocketUrlFromVersionPayload(payload), + ); if (browserWsUrl) { return { wsUrl: browserWsUrl, @@ -578,6 +620,58 @@ function shouldPreferNewBrowserTarget(endpoint: string): boolean { return endpoint === 'auto' && !process.env.OPENCLI_CDP_TARGET?.trim(); } +function normalizeWorkspaceKey(workspace?: string): string | undefined { + const trimmed = workspace?.trim(); + return trimmed ? trimmed : undefined; +} + +function makePersistentTargetRegistryKey(endpointKey: string, workspace: string): string { + return `${endpointKey}::${workspace}`; +} + +function readPersistentTargetRegistry(): Record { + try { + const raw = fs.readFileSync(PERSISTENT_TARGET_REGISTRY_PATH, 'utf8'); + const parsed = JSON.parse(raw); + if (!isRecord(parsed)) return {}; + return Object.fromEntries( + Object.entries(parsed).filter((entry): entry is [string, string] => typeof entry[0] === 'string' && typeof entry[1] === 'string'), + ); + } catch { + return {}; + } +} + +function writePersistentTargetRegistry(registry: Record): void { + try { + fs.writeFileSync(PERSISTENT_TARGET_REGISTRY_PATH, JSON.stringify(registry, null, 2), 'utf8'); + } catch { + // Best-effort cache only. + } +} + +function getPersistentTargetId(endpointKey: string, workspace: string): string | undefined { + const registry = readPersistentTargetRegistry(); + return registry[makePersistentTargetRegistryKey(endpointKey, workspace)]; +} + +function setPersistentTargetId(endpointKey: string, workspace: string, targetId: string): void { + const registry = readPersistentTargetRegistry(); + registry[makePersistentTargetRegistryKey(endpointKey, workspace)] = targetId; + writePersistentTargetRegistry(registry); +} + +function clearPersistentTargetId(endpointKey: string, workspace: string): void { + const registry = readPersistentTargetRegistry(); + delete registry[makePersistentTargetRegistryKey(endpointKey, workspace)]; + writePersistentTargetRegistry(registry); +} + +function selectTargetById(targets: CDPTarget[], targetId?: string): CDPTarget | undefined { + if (!targetId) return undefined; + return targets.find((target) => target.targetId === targetId && isBrowserAttachableTarget(target)); +} + function isLoopbackHost(host: string): boolean { return ['127.0.0.1', 'localhost', '::1', '[::1]'].includes(host); } @@ -592,8 +686,37 @@ function extractBrowserWebSocketUrlFromVersionPayload(payload: unknown): string return typeof wsUrl === 'string' && isBrowserLevelWebSocket(wsUrl) ? wsUrl : null; } -function selectBrowserPageTarget(targets: CDPTarget[]): CDPTarget | undefined { - return selectCDPTarget(targets.filter(target => (target.type ?? '').toLowerCase() === 'page')); +function rewriteBrowserWebSocketUrlForEndpoint(endpoint: string, wsUrl: string | null): string | null { + if (!wsUrl) return null; + + let endpointUrl: URL; + let browserWsUrl: URL; + try { + endpointUrl = new URL(endpoint); + browserWsUrl = new URL(wsUrl); + } catch { + return wsUrl; + } + + if (isLoopbackHost(endpointUrl.hostname.toLowerCase())) return wsUrl; + if (!isLoopbackHost(browserWsUrl.hostname.toLowerCase())) return wsUrl; + + browserWsUrl.protocol = endpointUrl.protocol === 'https:' ? 'wss:' : 'ws:'; + browserWsUrl.hostname = endpointUrl.hostname; + browserWsUrl.port = endpointUrl.port; + browserWsUrl.username = endpointUrl.username; + browserWsUrl.password = endpointUrl.password; + return browserWsUrl.toString(); +} + +function isBrowserAttachableTarget(target: CDPTarget): boolean { + const type = (target.type ?? '').toLowerCase(); + if (!type) return true; + return ['page', 'app', 'webview', 'iframe'].includes(type); +} + +function selectBrowserAttachTarget(targets: CDPTarget[]): CDPTarget | undefined { + return selectCDPTarget(targets.filter(isBrowserAttachableTarget)); } // ── CDP target selection (unchanged) ── @@ -662,20 +785,31 @@ function compilePreferredPattern(raw: string | undefined): RegExp | undefined { return new RegExp(escapeRegExp(value.toLowerCase())); } +function hasExplicitTargetHint(): boolean { + return !!process.env.OPENCLI_CDP_TARGET?.trim(); +} + function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } export const __test__ = { selectCDPTarget, - selectBrowserPageTarget, + selectBrowserAttachTarget, + selectTargetById, + isBrowserAttachableTarget, scoreCDPTarget, parseBrowserWebSocketUrlFromActivePort, parseAnyBrowserWebSocketUrlFromActivePort, extractBrowserWebSocketUrlFromVersionPayload, + rewriteBrowserWebSocketUrlForEndpoint, isBrowserLevelWebSocket, isLoopbackHost, shouldPreferNewBrowserTarget, + hasExplicitTargetHint, + normalizeWorkspaceKey, + makePersistentTargetRegistryKey, + persistentTargetRegistryPath: PERSISTENT_TARGET_REGISTRY_PATH, }; function fetchJsonDirect(url: string): Promise { diff --git a/src/build-manifest.test.ts b/src/build-manifest.test.ts index 263f5922..9aa051e4 100644 --- a/src/build-manifest.test.ts +++ b/src/build-manifest.test.ts @@ -129,7 +129,6 @@ describe('manifest helper rules', () => { expect(scanTs(file, 'demo')).toBeNull(); }); - it('keeps literal domain and navigateBefore for TS adapters', () => { const file = path.join(process.cwd(), 'src', 'clis', 'xueqiu', 'fund-holdings.ts'); const entry = scanTs(file, 'xueqiu'); @@ -166,4 +165,27 @@ describe('manifest helper rules', () => { replacedBy: 'opencli demo new', }); }); + + it('derives supportsBrowserCdp for desktop-style TS adapters', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'opencli-manifest-')); + tempDirs.push(dir); + const file = path.join(dir, 'ask.ts'); + fs.writeFileSync(file, ` +import { cli, Strategy } from '../../registry.js'; +cli({ + site: 'doubao-app', + name: 'ask', + description: 'ask', + domain: 'doubao-app', + strategy: Strategy.UI, + browser: true, +}); +`); + + expect(scanTs(file, 'doubao-app')).toEqual(expect.objectContaining({ + site: 'doubao-app', + name: 'ask', + supportsBrowserCdp: false, + })); + }); }); diff --git a/src/build-manifest.ts b/src/build-manifest.ts index 93be7b76..4d20fa3f 100644 --- a/src/build-manifest.ts +++ b/src/build-manifest.ts @@ -14,6 +14,7 @@ import * as path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import yaml from 'js-yaml'; import { getErrorMessage } from './errors.js'; +import { Strategy, deriveSupportsBrowserCdp } from './registry.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const CLIS_DIR = path.resolve(__dirname, 'clis'); @@ -26,6 +27,7 @@ export interface ManifestEntry { domain?: string; strategy: string; browser: boolean; + supportsBrowserCdp?: boolean; args: Array<{ name: string; type?: string; @@ -52,6 +54,12 @@ import type { YamlCliDefinition } from './yaml-schema.js'; import { isRecord } from './utils.js'; +function toStrategyEnum(strategy: string | undefined, browser: boolean): Strategy { + if (!strategy) return browser ? Strategy.COOKIE : Strategy.PUBLIC; + const key = strategy.toUpperCase() as keyof typeof Strategy; + return Strategy[key] ?? (browser ? Strategy.COOKIE : Strategy.PUBLIC); +} + function extractBalancedBlock( source: string, @@ -197,6 +205,11 @@ function scanYaml(filePath: string, site: string): ManifestEntry | null { domain: cliDef.domain, strategy: strategy.toLowerCase(), browser, + supportsBrowserCdp: deriveSupportsBrowserCdp({ + browser, + strategy: Strategy[strategy as keyof typeof Strategy], + domain: cliDef.domain, + }), args, columns: cliDef.columns, pipeline: cliDef.pipeline, @@ -252,6 +265,14 @@ export function scanTs(filePath: string, site: string): ManifestEntry | null { if (browserMatch) entry.browser = browserMatch[1] === 'true'; else entry.browser = entry.strategy !== 'public'; + const supportsBrowserCdpMatch = src.match(/supportsBrowserCdp\s*:\s*(true|false)/); + entry.supportsBrowserCdp = deriveSupportsBrowserCdp({ + browser: entry.browser, + strategy: toStrategyEnum(entry.strategy, entry.browser), + domain: entry.domain, + supportsBrowserCdp: supportsBrowserCdpMatch ? supportsBrowserCdpMatch[1] === 'true' : undefined, + }); + // Extract columns const colMatch = src.match(/columns\s*:\s*\[([^\]]*)\]/); if (colMatch) { diff --git a/src/commanderAdapter.test.ts b/src/commanderAdapter.test.ts index cbc98a27..bb33c62c 100644 --- a/src/commanderAdapter.test.ts +++ b/src/commanderAdapter.test.ts @@ -17,7 +17,80 @@ vi.mock('./output.js', () => ({ import { registerCommandToProgram } from './commanderAdapter.js'; -describe('registerCommandToProgram', () => { +describe('commanderAdapter bool normalization', () => { + const cmd: CliCommand = { + site: 'paperreview', + name: 'submit', + description: 'Submit a PDF', + browser: false, + args: [ + { name: 'pdf', positional: true, required: true, help: 'Path to the paper PDF' }, + { name: 'dry-run', type: 'bool', default: false, help: 'Validate only' }, + { name: 'prepare-only', type: 'bool', default: false, help: 'Prepare only' }, + ], + func: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + mockExecuteCommand.mockResolvedValue([]); + process.exitCode = undefined; + }); + + it('normalizes explicit false string values to false', async () => { + const program = new Command(); + const siteCmd = program.command('paperreview'); + registerCommandToProgram(siteCmd, cmd); + + await program.parseAsync(['node', 'opencli', 'paperreview', 'submit', './paper.pdf', '--dry-run', 'false']); + + expect(mockExecuteCommand).toHaveBeenCalledWith( + cmd, + expect.objectContaining({ + pdf: './paper.pdf', + 'dry-run': false, + 'prepare-only': false, + }), + false, + ); + }); + + it('normalizes valueless bool flags to true', async () => { + const program = new Command(); + const siteCmd = program.command('paperreview'); + registerCommandToProgram(siteCmd, cmd); + + await program.parseAsync(['node', 'opencli', 'paperreview', 'submit', './paper.pdf', '--prepare-only']); + + expect(mockExecuteCommand).toHaveBeenCalledWith( + cmd, + expect.objectContaining({ + pdf: './paper.pdf', + 'dry-run': false, + 'prepare-only': true, + }), + false, + ); + }); + + it('rejects invalid bool strings before execution', async () => { + const program = new Command(); + const siteCmd = program.command('paperreview'); + registerCommandToProgram(siteCmd, cmd); + const stderr = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await program.parseAsync(['node', 'opencli', 'paperreview', 'submit', './paper.pdf', '--dry-run', 'maybe']); + + expect(mockExecuteCommand).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + expect(stderr).toHaveBeenCalledWith(expect.stringContaining('"dry-run" must be either "true" or "false".')); + + stderr.mockRestore(); + }); +}); + +describe('registerCommandToProgram browser env overrides', () => { beforeEach(() => { vi.clearAllMocks(); vi.unstubAllEnvs(); diff --git a/src/commanderAdapter.ts b/src/commanderAdapter.ts index 251ea008..5d7fa13f 100644 --- a/src/commanderAdapter.ts +++ b/src/commanderAdapter.ts @@ -19,6 +19,18 @@ import { executeCommand } from './execution.js'; import { CliError, ERROR_ICONS, getErrorMessage } from './errors.js'; import { addBrowserEnvOverrideOptions, runWithBrowserEnvOptions } from './browserEnvOptions.js'; +export function normalizeArgValue(argType: string | undefined, value: unknown, name: string): unknown { + if (argType !== 'bool') return value; + if (typeof value === 'boolean') return value; + if (value == null || value === '') return false; + + const normalized = String(value).trim().toLowerCase(); + if (normalized === 'true') return true; + if (normalized === 'false') return false; + + throw new CliError('ARGUMENT', `"${name}" must be either "true" or "false".`); +} + /** * Register a single CliCommand as a Commander subcommand. */ @@ -56,21 +68,21 @@ export function registerCommandToProgram(siteCmd: Command, cmd: CliCommand): voi const optionsRecord = typeof actionOpts === 'object' && actionOpts !== null ? actionOpts as Record : {}; const startTime = Date.now(); - // ── Collect kwargs ────────────────────────────────────────────────── - const kwargs: Record = {}; - for (let i = 0; i < positionalArgs.length; i++) { - const v = actionArgs[i]; - if (v !== undefined) kwargs[positionalArgs[i].name] = v; - } - for (const arg of cmd.args) { - if (arg.positional) continue; - const camelName = arg.name.replace(/-([a-z])/g, (_m, ch: string) => ch.toUpperCase()); - const v = optionsRecord[arg.name] ?? optionsRecord[camelName]; - if (v !== undefined) kwargs[arg.name] = v; - } - // ── Execute + render ──────────────────────────────────────────────── try { + // ── Collect kwargs ──────────────────────────────────────────────── + const kwargs: Record = {}; + for (let i = 0; i < positionalArgs.length; i++) { + const v = actionArgs[i]; + if (v !== undefined) kwargs[positionalArgs[i].name] = v; + } + for (const arg of cmd.args) { + if (arg.positional) continue; + const camelName = arg.name.replace(/-([a-z])/g, (_m, ch: string) => ch.toUpperCase()); + const v = optionsRecord[arg.name] ?? optionsRecord[camelName]; + if (v !== undefined) kwargs[arg.name] = normalizeArgValue(arg.type, v, arg.name); + } + const verbose = optionsRecord.verbose === true; const format = typeof optionsRecord.format === 'string' ? optionsRecord.format : 'table'; if (verbose) process.env.OPENCLI_VERBOSE = '1'; diff --git a/src/discovery.ts b/src/discovery.ts index 90f6a0ac..bdd98b9d 100644 --- a/src/discovery.ts +++ b/src/discovery.ts @@ -13,7 +13,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; import yaml from 'js-yaml'; -import { type CliCommand, type InternalCliCommand, type Arg, Strategy, registerCommand } from './registry.js'; +import { type CliCommand, type InternalCliCommand, type Arg, Strategy, deriveSupportsBrowserCdp, registerCommand } from './registry.js'; import { getErrorMessage } from './errors.js'; import { log } from './logger.js'; import type { ManifestEntry } from './build-manifest.js'; @@ -72,6 +72,12 @@ async function loadFromManifest(manifestPath: string, clisDir: string): Promise< domain: entry.domain, strategy, browser: entry.browser, + supportsBrowserCdp: deriveSupportsBrowserCdp({ + browser: entry.browser, + strategy, + domain: entry.domain, + supportsBrowserCdp: entry.supportsBrowserCdp, + }), args: entry.args ?? [], columns: entry.columns, pipeline: entry.pipeline, @@ -94,6 +100,12 @@ async function loadFromManifest(manifestPath: string, clisDir: string): Promise< domain: entry.domain, strategy, browser: entry.browser ?? true, + supportsBrowserCdp: deriveSupportsBrowserCdp({ + browser: entry.browser ?? true, + strategy, + domain: entry.domain, + supportsBrowserCdp: entry.supportsBrowserCdp, + }), args: entry.args ?? [], columns: entry.columns, timeoutSeconds: entry.timeout, @@ -184,6 +196,11 @@ async function registerYamlCli(filePath: string, defaultSite: string): Promise { + const tempBuildRoot = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'opencli-manifest-flags-')); + const distDir = path.join(tempBuildRoot, 'dist'); + const manifestPath = path.join(tempBuildRoot, 'cli-manifest.json'); + + try { + await fs.promises.mkdir(distDir, { recursive: true }); + await fs.promises.writeFile(manifestPath, JSON.stringify([ + { + site: 'doubao-app', + name: 'ask', + description: 'ask', + domain: 'doubao-app', + strategy: 'ui', + browser: true, + supportsBrowserCdp: false, + args: [], + type: 'ts', + modulePath: 'doubao-app/ask.js', + }, + ])); + + await discoverClis(distDir); + + expect(getRegistry().get('doubao-app/ask')?.supportsBrowserCdp).toBe(false); + } finally { + await fs.promises.rm(tempBuildRoot, { recursive: true, force: true }); + } + }); }); describe('discoverPlugins', () => { diff --git a/src/registry.test.ts b/src/registry.test.ts index dfe5d2e7..6d401626 100644 --- a/src/registry.test.ts +++ b/src/registry.test.ts @@ -53,6 +53,38 @@ describe('cli() registration', () => { expect(cmd.strategy).toBe(Strategy.PUBLIC); }); + it('disables browser-cdp by default for local desktop UI targets', () => { + const localhostCmd = cli({ + site: 'test-registry', + name: 'localhost-ui', + domain: 'localhost', + strategy: Strategy.UI, + browser: true, + }); + const appCmd = cli({ + site: 'test-registry', + name: 'app-ui', + domain: 'doubao-app', + strategy: Strategy.UI, + browser: true, + }); + + expect(localhostCmd.supportsBrowserCdp).toBe(false); + expect(appCmd.supportsBrowserCdp).toBe(false); + }); + + it('keeps browser-cdp enabled for real website UI targets', () => { + const cmd = cli({ + site: 'test-registry', + name: 'remote-ui', + domain: 'x.com', + strategy: Strategy.UI, + browser: true, + }); + + expect(cmd.supportsBrowserCdp).toBe(true); + }); + it('overwrites existing command on re-registration', () => { cli({ site: 'test-registry', name: 'overwrite', description: 'v1' }); cli({ site: 'test-registry', name: 'overwrite', description: 'v2' }); diff --git a/src/registry.ts b/src/registry.ts index 3da8cdf5..ee24d0c0 100644 --- a/src/registry.ts +++ b/src/registry.ts @@ -30,6 +30,11 @@ export interface RequiredEnv { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- kwargs from CLI parsing are inherently untyped export type CommandArgs = Record; +export interface RequiredEnv { + name: string; + help?: string; +} + export interface CliCommand { site: string; name: string; @@ -76,6 +81,31 @@ export interface CliOptions extends Partial { expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); }); + it('honors --no-browser-cdp by clearing an inherited endpoint during the command', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + + let seenEndpoint: string | undefined; + await withBrowserEnvOverrides({ + browserCdp: false, + }, async () => { + seenEndpoint = process.env.OPENCLI_CDP_ENDPOINT; + }, { allowBrowserCdp: true }); + + expect(seenEndpoint).toBeUndefined(); + expect(process.env.OPENCLI_CDP_ENDPOINT).toBe('http://127.0.0.1:9222'); + }); + it('restores outer overrides correctly across nested calls', async () => { const seen: Array<{ label: string; endpoint?: string; target?: string }> = []; diff --git a/src/runtime.ts b/src/runtime.ts index 25edee23..b0298a93 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -35,7 +35,7 @@ export async function withBrowserEnvOverrides( config: BrowserEnvOverrideConfig = {}, ): Promise { const effectiveEndpoint = resolveEffectiveCdpEndpoint(overrides, config); - const pairs: Array<[key: 'OPENCLI_CDP_ENDPOINT' | 'OPENCLI_CDP_TARGET', value: string | undefined]> = [ + const pairs: Array<[key: 'OPENCLI_CDP_ENDPOINT' | 'OPENCLI_CDP_TARGET', value: string | null | undefined]> = [ ['OPENCLI_CDP_ENDPOINT', effectiveEndpoint], ['OPENCLI_CDP_TARGET', overrides.cdpTarget], ]; @@ -44,7 +44,8 @@ export async function withBrowserEnvOverrides( for (const [key, value] of pairs) { if (value === undefined) continue; previous.set(key, process.env[key]); - process.env[key] = value; + if (value === null) delete process.env[key]; + else process.env[key] = value; } try { @@ -100,7 +101,7 @@ function readCdpEndpointOption(value: unknown): string | undefined { function resolveEffectiveCdpEndpoint( overrides: BrowserEnvOverrides, config: BrowserEnvOverrideConfig, -): string | undefined { +): string | null | undefined { if (overrides.cdpEndpoint) { if (overrides.cdpEndpoint === 'auto' && !config.allowBrowserCdp) { throw new Error('The "auto" CDP endpoint is only supported for browser CDP commands.'); @@ -111,7 +112,7 @@ function resolveEffectiveCdpEndpoint( if (!config.allowBrowserCdp) return undefined; if (typeof overrides.browserCdp === 'boolean') { - return overrides.browserCdp ? 'auto' : undefined; + return overrides.browserCdp ? 'auto' : null; } return readBooleanEnv('OPENCLI_BROWSER_CDP') ? 'auto' : undefined; From 295b8de469f5dfdb4a6df2be59354cec4dc568db Mon Sep 17 00:00:00 2001 From: coolxll Date: Thu, 26 Mar 2026 23:33:43 +0800 Subject: [PATCH 8/8] fix: drop duplicated commander arg normalization --- src/commanderAdapter.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/commanderAdapter.ts b/src/commanderAdapter.ts index e6f11fac..5d7fa13f 100644 --- a/src/commanderAdapter.ts +++ b/src/commanderAdapter.ts @@ -31,18 +31,6 @@ export function normalizeArgValue(argType: string | undefined, value: unknown, n throw new CliError('ARGUMENT', `"${name}" must be either "true" or "false".`); } -export function normalizeArgValue(argType: string | undefined, value: unknown, name: string): unknown { - if (argType !== 'bool') return value; - if (typeof value === 'boolean') return value; - if (value == null || value === '') return false; - - const normalized = String(value).trim().toLowerCase(); - if (normalized === 'true') return true; - if (normalized === 'false') return false; - - throw new CliError('ARGUMENT', `"${name}" must be either "true" or "false".`); -} - /** * Register a single CliCommand as a Commander subcommand. */