From 088097f182552b20a361de916656e335517072bb Mon Sep 17 00:00:00 2001 From: Lucky <1646721+luckydududu@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:49:58 +0000 Subject: [PATCH] fix(browser): honor OPENCLI_CDP_ENDPOINT for website CLIs (remote-Chrome mode) docs/advanced/remote-chrome.md promises that setting OPENCLI_CDP_ENDPOINT lets OpenCLI drive a remote Chrome (headless servers, CI), but execution only honored the variable for Electron apps -- website CLIs always went through the Browser Bridge extension, which cannot be installed in many remote/headless setups. - runtime: getBrowserFactory routes any site over CDPBridge when OPENCLI_CDP_ENDPOINT is set (Electron behavior unchanged) - execution: pass the manual endpoint for non-Electron sites; no localhost port probe since the endpoint may be on another machine -- CDPBridge validates it when fetching /json - cdp: website sessions open a dedicated tab via /json/new (PUT with GET fallback for pre-111 Chrome) instead of attaching to whichever existing tab ranks first, and close it on close(); ws:// endpoints and the attach path keep their previous behavior - tests: dedicated-tab lifecycle (create/close, PUT->GET fallback, legacy attach unchanged) against a real local HTTP server; factory routing unit tests - docs: describe website-CLI support and dedicated-tab semantics Verified end-to-end against a real remote Chrome over the network: twitter whoami and timeline --type for-you return correct data with no extension installed, and no tabs leak into the shared browser. --- docs/advanced/remote-chrome.md | 12 ++++ src/browser/cdp.test.ts | 106 +++++++++++++++++++++++++++++++++ src/browser/cdp.ts | 71 ++++++++++++++++++---- src/execution.ts | 13 +++- src/runtime.test.ts | 19 ++++++ src/runtime.ts | 9 ++- 6 files changed, 215 insertions(+), 15 deletions(-) create mode 100644 src/runtime.test.ts diff --git a/docs/advanced/remote-chrome.md b/docs/advanced/remote-chrome.md index 9f2695ce4..925604648 100644 --- a/docs/advanced/remote-chrome.md +++ b/docs/advanced/remote-chrome.md @@ -37,6 +37,18 @@ Use `127.0.0.1` instead of `localhost` in the SSH command to avoid IPv6 resoluti export OPENCLI_CDP_ENDPOINT="http://127.0.0.1:9222" ``` +With the endpoint set, website CLIs (e.g. `opencli twitter timeline`) run over CDP +directly — the Browser Bridge extension is not required in this mode. Each command +opens a **dedicated tab** on the remote browser and closes it when done, so your +existing tabs are never hijacked. Electron app CLIs honor the same variable as before. + +::: tip +The endpoint may also be a `ws://` URL pointing at a specific target. In that case +OpenCLI attaches to that exact target instead of opening a dedicated tab +(`OPENCLI_CDP_TARGET` filters targets when using an `http://` endpoint without a +dedicated tab, e.g. for Electron apps). +::: + ### 4. Verify ```bash diff --git a/src/browser/cdp.test.ts b/src/browser/cdp.test.ts index 6ad3b8365..14adb319c 100644 --- a/src/browser/cdp.test.ts +++ b/src/browser/cdp.test.ts @@ -97,3 +97,109 @@ describe('CDPBridge cookies', () => { ]); }); }); + +describe('CDPBridge dedicated targets (remote Chrome)', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + async function withDevtoolsServer( + handler: (req: { method: string; url: string }) => { status: number; body: string }, + fn: (base: string, seen: Array<{ method: string; url: string }>) => Promise, + ): Promise { + const { createServer } = await import('node:http'); + const seen: Array<{ method: string; url: string }> = []; + const server = createServer((req, res) => { + const entry = { method: req.method ?? '', url: req.url ?? '' }; + seen.push(entry); + const { status, body } = handler(entry); + res.writeHead(status, { 'Content-Type': 'application/json' }); + res.end(body); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + const port = typeof address === 'object' && address ? address.port : 0; + try { + await fn(`http://127.0.0.1:${port}`, seen); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + } + + it('opens a dedicated tab instead of attaching to an existing one, and closes it on close()', async () => { + await withDevtoolsServer( + ({ method, url }) => { + if (method === 'PUT' && url.startsWith('/json/new')) { + return { status: 200, body: JSON.stringify({ id: 'T1', webSocketDebuggerUrl: 'ws://127.0.0.1:9/devtools/page/T1' }) }; + } + if (method === 'GET' && url === '/json/close/T1') { + return { status: 200, body: 'Target is closing' }; + } + return { status: 500, body: '{}' }; + }, + async (base, seen) => { + const bridge = new CDPBridge(); + vi.spyOn(bridge, 'send').mockResolvedValue({}); + + await bridge.connect({ cdpEndpoint: base, dedicatedTarget: true }); + await bridge.close(); + + expect(seen.map((r) => `${r.method} ${r.url.split('?')[0]}`)).toEqual([ + 'PUT /json/new', + 'GET /json/close/T1', + ]); + }, + ); + }); + + it('falls back to GET /json/new for Chrome versions that reject PUT', async () => { + await withDevtoolsServer( + ({ method, url }) => { + if (url.startsWith('/json/new')) { + if (method === 'PUT') return { status: 405, body: 'Using unsafe HTTP verb' }; + return { status: 200, body: JSON.stringify({ id: 'T2', webSocketDebuggerUrl: 'ws://127.0.0.1:9/devtools/page/T2' }) }; + } + if (url === '/json/close/T2') return { status: 200, body: 'Target is closing' }; + return { status: 500, body: '{}' }; + }, + async (base, seen) => { + const bridge = new CDPBridge(); + vi.spyOn(bridge, 'send').mockResolvedValue({}); + + await bridge.connect({ cdpEndpoint: base, dedicatedTarget: true }); + await bridge.close(); + + expect(seen.map((r) => `${r.method} ${r.url.split('?')[0]}`)).toEqual([ + 'PUT /json/new', + 'GET /json/new', + 'GET /json/close/T2', + ]); + }, + ); + }); + + it('keeps the legacy attach behaviour when dedicatedTarget is not requested', async () => { + await withDevtoolsServer( + ({ method, url }) => { + if (method === 'GET' && url === '/json') { + return { + status: 200, + body: JSON.stringify([ + { id: 'P1', type: 'page', url: 'https://example.com', webSocketDebuggerUrl: 'ws://127.0.0.1:9/devtools/page/P1' }, + ]), + }; + } + return { status: 500, body: '{}' }; + }, + async (base, seen) => { + const bridge = new CDPBridge(); + vi.spyOn(bridge, 'send').mockResolvedValue({}); + + await bridge.connect({ cdpEndpoint: base }); + await bridge.close(); + + expect(seen.map((r) => `${r.method} ${r.url}`)).toEqual(['GET /json']); + }, + ); + }); +}); diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index 7ac2aa7bc..971923b33 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -21,6 +21,7 @@ import { getAllElectronApps } from '../electron-apps.js'; import { BasePage } from './base-page.js'; export interface CDPTarget { + id?: string; type?: string; url?: string; title?: string; @@ -52,8 +53,10 @@ export class CDPBridge implements IBrowserFactory { private _idCounter = 0; private _pending = new Map void; reject: (err: Error) => void; timer: ReturnType }>(); private _eventListeners = new Map void>>(); + private _httpBase: string | null = null; + private _createdTargetId: string | null = null; - async connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; idleTimeout?: number; windowMode?: 'foreground' | 'background'; surface?: 'browser' | 'adapter'; siteSession?: 'ephemeral' | 'persistent' }): Promise { + async connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; idleTimeout?: number; windowMode?: 'foreground' | 'background'; surface?: 'browser' | 'adapter'; siteSession?: 'ephemeral' | 'persistent'; dedicatedTarget?: boolean }): Promise { if (this._ws) throw new Error('CDPBridge is already connected. Call close() before reconnecting.'); const endpoint = opts?.cdpEndpoint ?? process.env.OPENCLI_CDP_ENDPOINT; @@ -61,12 +64,25 @@ export class CDPBridge implements IBrowserFactory { 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'); + const httpBase = endpoint.replace(/\/$/, ''); + if (opts?.dedicatedTarget) { + // Website sessions on a shared browser must not hijack whichever tab + // happens to rank first — open a dedicated tab and clean it up on close(). + const created = await createDedicatedCDPTarget(httpBase); + if (!created.webSocketDebuggerUrl) { + throw new Error('CDP endpoint created a target without webSocketDebuggerUrl'); + } + this._httpBase = httpBase; + this._createdTargetId = created.id ?? null; + wsUrl = created.webSocketDebuggerUrl; + } else { + const targets = await fetchJsonDirect(`${httpBase}/json`) as CDPTarget[]; + const target = selectCDPTarget(targets); + if (!target || !target.webSocketDebuggerUrl) { + throw new Error('No inspectable targets found at CDP endpoint'); + } + wsUrl = target.webSocketDebuggerUrl; } - wsUrl = target.webSocketDebuggerUrl; } return new Promise((resolve, reject) => { @@ -137,6 +153,13 @@ export class CDPBridge implements IBrowserFactory { } this._pending.clear(); this._eventListeners.clear(); + if (this._httpBase && this._createdTargetId) { + // Best-effort: remove the tab we opened so repeated runs don't litter the + // shared browser. Ignore failures — the browser may already be gone. + await closeDedicatedCDPTarget(this._httpBase, this._createdTargetId).catch(() => {}); + this._httpBase = null; + this._createdTargetId = null; + } } async send(method: string, params: Record = {}, timeoutMs: number = CDP_SEND_TIMEOUT): Promise { @@ -556,10 +579,10 @@ export const __test__ = { scoreCDPTarget, }; -function fetchJsonDirect(url: string): Promise { +function fetchJsonDirect(url: string, method: 'GET' | 'PUT' = 'GET'): Promise { return new Promise((resolve, reject) => { const parsed = new URL(url); - const request = (parsed.protocol === 'https:' ? httpsRequest : httpRequest)(parsed, (res) => { + const request = (parsed.protocol === 'https:' ? httpsRequest : httpRequest)(parsed, { method }, (res) => { const statusCode = res.statusCode ?? 0; if (statusCode < 200 || statusCode >= 300) { res.resume(); @@ -570,10 +593,12 @@ function fetchJsonDirect(url: string): Promise { const chunks: Buffer[] = []; res.on('data', (chunk) => chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))); res.on('end', () => { + const body = Buffer.concat(chunks).toString('utf8'); try { - resolve(JSON.parse(Buffer.concat(chunks).toString('utf8'))); - } catch (error) { - reject(error instanceof Error ? error : new Error(String(error))); + resolve(JSON.parse(body)); + } catch { + // /json/close (and some older endpoints) reply with plain text. + resolve(body); } }); }); @@ -583,3 +608,27 @@ function fetchJsonDirect(url: string): Promise { request.end(); }); } + +/** + * Open a dedicated tab on a shared browser via the DevTools HTTP API. + * Chrome 111+ requires PUT for /json/new; older versions only accept GET — + * try PUT first and fall back so both generations work. + */ +export async function createDedicatedCDPTarget(httpBase: string): Promise { + const newUrl = `${httpBase}/json/new?url=about:blank`; + let created: unknown; + try { + created = await fetchJsonDirect(newUrl, 'PUT'); + } catch { + created = await fetchJsonDirect(newUrl, 'GET'); + } + if (!isRecord(created) || typeof created.webSocketDebuggerUrl !== 'string') { + throw new Error('CDP /json/new did not return an inspectable target'); + } + return created as CDPTarget; +} + +/** Best-effort close of a tab previously opened by createDedicatedCDPTarget. */ +export async function closeDedicatedCDPTarget(httpBase: string, targetId: string): Promise { + await fetchJsonDirect(`${httpBase}/json/close/${targetId}`, 'GET'); +} diff --git a/src/execution.ts b/src/execution.ts index 585bc0344..77f5eb8d9 100644 --- a/src/execution.ts +++ b/src/execution.ts @@ -235,11 +235,12 @@ export async function executeCommand( try { if (shouldUseBrowserSession(cmd)) { const electron = isElectronApp(cmd.site); + const manualEndpoint = process.env.OPENCLI_CDP_ENDPOINT; let cdpEndpoint: string | undefined; + let dedicatedTarget = false; if (electron) { // Electron apps: respect manual endpoint override, then try auto-detect - const manualEndpoint = process.env.OPENCLI_CDP_ENDPOINT; if (manualEndpoint) { const port = Number(new URL(manualEndpoint).port); if (!await probeCDP(port)) { @@ -252,6 +253,14 @@ export async function executeCommand( } else { cdpEndpoint = await resolveElectronEndpoint(cmd.site); } + } else if (manualEndpoint) { + // Remote-Chrome mode for website CLIs (docs/advanced/remote-chrome.md): + // route over CDP instead of the Browser Bridge extension. No localhost + // port probe here — the endpoint may live on another machine, and + // CDPBridge validates it when it fetches /json. A dedicated tab keeps + // us from hijacking whatever the user has open in the shared browser. + cdpEndpoint = manualEndpoint; + dedicatedTarget = true; } const BrowserFactory = getBrowserFactory(cmd.site); @@ -411,7 +420,7 @@ export async function executeCommand( if (!keepTab) await page.closeWindow?.().catch(() => {}); throw err; } - }, { session, cdpEndpoint, ...profileRouting, windowMode, surface: 'adapter', siteSession }); + }, { session, cdpEndpoint, dedicatedTarget, ...profileRouting, windowMode, surface: 'adapter', siteSession }); } catch (err) { browserRunError = err; throw err; diff --git a/src/runtime.test.ts b/src/runtime.test.ts new file mode 100644 index 000000000..2182d0230 --- /dev/null +++ b/src/runtime.test.ts @@ -0,0 +1,19 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { getBrowserFactory } from './runtime.js'; +import { CDPBridge } from './browser/cdp.js'; +import { BrowserBridge } from './browser/bridge.js'; + +describe('getBrowserFactory', () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it('routes website CLIs through the Browser Bridge extension by default', () => { + expect(getBrowserFactory('twitter')).toBe(BrowserBridge); + }); + + it('routes website CLIs over CDP when OPENCLI_CDP_ENDPOINT is set (remote-Chrome mode)', () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + expect(getBrowserFactory('twitter')).toBe(CDPBridge); + }); +}); diff --git a/src/runtime.ts b/src/runtime.ts index 1e1c229f1..1e9937c3a 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -12,6 +12,10 @@ export { DEFAULT_BROWSER_COMMAND_TIMEOUT, DEFAULT_BROWSER_CONNECT_TIMEOUT }; */ export function getBrowserFactory(site?: string): new () => IBrowserFactory { if (site && isElectronApp(site)) return CDPBridge; + // Remote-Chrome mode (docs/advanced/remote-chrome.md): an explicit endpoint + // routes website CLIs over CDP too, so headless/server environments work + // without the Browser Bridge extension. + if (process.env.OPENCLI_CDP_ENDPOINT) return CDPBridge; return BrowserBridge; } @@ -54,14 +58,14 @@ export function withTimeoutMs( /** Interface for browser factory (BrowserBridge or test mocks) */ export interface IBrowserFactory { - connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; preferredContextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent' }): Promise; + connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; preferredContextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent'; dedicatedTarget?: boolean }): Promise; close(): Promise; } export async function browserSession( BrowserFactory: new () => IBrowserFactory, fn: (page: IPage) => Promise, - opts: { session?: string; cdpEndpoint?: string; contextId?: string; preferredContextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent' } = {}, + opts: { session?: string; cdpEndpoint?: string; contextId?: string; preferredContextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent'; dedicatedTarget?: boolean } = {}, ): Promise { const browser = new BrowserFactory(); try { @@ -69,6 +73,7 @@ export async function browserSession( timeout: DEFAULT_BROWSER_CONNECT_TIMEOUT, session: opts.session, cdpEndpoint: opts.cdpEndpoint, + dedicatedTarget: opts.dedicatedTarget, contextId: opts.contextId, preferredContextId: opts.preferredContextId, idleTimeout: opts.idleTimeout,