diff --git a/src/browser/cdp.test.ts b/src/browser/cdp.test.ts index 5da4b71c..924ce1ef 100644 --- a/src/browser/cdp.test.ts +++ b/src/browser/cdp.test.ts @@ -3,10 +3,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; const { MockWebSocket } = vi.hoisted(() => { class MockWebSocket { static OPEN = 1; + static urls: string[] = []; readyState = 1; private handlers = new Map void>>(); - constructor(_url: string) { + constructor(url: string) { + MockWebSocket.urls.push(url); queueMicrotask(() => this.emit('open')); } @@ -41,6 +43,7 @@ import { CDPBridge } from './cdp.js'; describe('CDPBridge cookies', () => { beforeEach(() => { vi.unstubAllEnvs(); + MockWebSocket.urls = []; }); it('filters cookies by actual domain match instead of substring match', async () => { @@ -63,4 +66,15 @@ describe('CDPBridge cookies', () => { { name: 'exact', value: '2', domain: 'example.com' }, ]); }); + + it('trims OPENCLI_CDP_ENDPOINT before opening the websocket', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', ' ws://127.0.0.1:9222/devtools/page/1 '); + + const bridge = new CDPBridge(); + vi.spyOn(bridge, 'send').mockResolvedValue({}); + + await bridge.connect(); + + expect(MockWebSocket.urls).toEqual(['ws://127.0.0.1:9222/devtools/page/1']); + }); }); diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index 0f9f59b9..ab09e5a8 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -49,7 +49,7 @@ export class CDPBridge implements IBrowserFactory { async connect(opts?: { timeout?: number; workspace?: string; cdpEndpoint?: string }): Promise { if (this._ws) throw new Error('CDPBridge is already connected. Call close() before reconnecting.'); - const endpoint = opts?.cdpEndpoint ?? process.env.OPENCLI_CDP_ENDPOINT; + const endpoint = (opts?.cdpEndpoint ?? process.env.OPENCLI_CDP_ENDPOINT)?.trim(); if (!endpoint) throw new Error('CDP endpoint not provided (pass cdpEndpoint or set OPENCLI_CDP_ENDPOINT)'); let wsUrl = endpoint; diff --git a/src/cli-browser.test.ts b/src/cli-browser.test.ts new file mode 100644 index 00000000..f10cf492 --- /dev/null +++ b/src/cli-browser.test.ts @@ -0,0 +1,63 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + mockCdpConnect, + mockBridgeConnect, +} = vi.hoisted(() => ({ + mockCdpConnect: vi.fn(), + mockBridgeConnect: vi.fn(), +})); + +vi.mock('./browser/index.js', () => ({ + BrowserBridge: class { + connect = mockBridgeConnect; + close = vi.fn(); + }, + CDPBridge: class { + connect = mockCdpConnect; + close = vi.fn(); + }, +})); + +import { createProgram } from './cli.js'; + +describe('browser manual CDP routing', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); + vi.spyOn(console, 'log').mockImplementation(() => {}); + + const page = { + evaluate: vi.fn(), + wait: vi.fn(), + }; + + mockBridgeConnect.mockResolvedValue(page); + mockCdpConnect.mockResolvedValue(page); + }); + + it('uses CDPBridge when OPENCLI_CDP_ENDPOINT is set', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', ' https://abcdef.ngrok.app '); + + const program = createProgram('', ''); + await program.parseAsync(['node', 'opencli', 'browser', 'back']); + + expect(mockCdpConnect).toHaveBeenCalledWith({ + timeout: 30, + workspace: 'browser:default', + cdpEndpoint: 'https://abcdef.ngrok.app', + }); + expect(mockBridgeConnect).not.toHaveBeenCalled(); + }); + + it('keeps BrowserBridge when OPENCLI_CDP_ENDPOINT is not set', async () => { + const program = createProgram('', ''); + await program.parseAsync(['node', 'opencli', 'browser', 'back']); + + expect(mockBridgeConnect).toHaveBeenCalledWith({ + timeout: 30, + workspace: 'browser:default', + }); + expect(mockCdpConnect).not.toHaveBeenCalled(); + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 64e63203..4363d46a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -27,9 +27,10 @@ const CLI_FILE = fileURLToPath(import.meta.url); /** Create a browser page for browser commands. Uses a dedicated browser workspace for session persistence. */ async function getBrowserPage(): Promise { - const { BrowserBridge } = await import('./browser/index.js'); - const bridge = new BrowserBridge(); - return bridge.connect({ timeout: 30, workspace: 'browser:default' }); + const cdpEndpoint = process.env.OPENCLI_CDP_ENDPOINT?.trim() || undefined; + const BrowserFactory = getBrowserFactory(); + const browser = new BrowserFactory(); + return browser.connect({ timeout: 30, workspace: 'browser:default', cdpEndpoint }); } function applyVerbose(opts: { verbose?: boolean }): void { diff --git a/src/execution-routing.test.ts b/src/execution-routing.test.ts new file mode 100644 index 00000000..e55d281a --- /dev/null +++ b/src/execution-routing.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + mockBrowserSession, + mockGetDaemonHealth, + mockProbeCDP, + mockResolveElectronEndpoint, + mockEmitHook, +} = vi.hoisted(() => ({ + mockBrowserSession: vi.fn(async (_Factory, fn) => fn({ + goto: vi.fn(), + wait: vi.fn(), + } as any)), + mockGetDaemonHealth: vi.fn(), + mockProbeCDP: vi.fn(), + mockResolveElectronEndpoint: vi.fn(), + mockEmitHook: vi.fn(), +})); + +vi.mock('./runtime.js', async () => { + const actual = await vi.importActual('./runtime.js'); + return { + ...actual, + browserSession: mockBrowserSession, + }; +}); + +vi.mock('./browser/daemon-client.js', () => ({ + getDaemonHealth: mockGetDaemonHealth, +})); + +vi.mock('./launcher.js', () => ({ + probeCDP: mockProbeCDP, + resolveElectronEndpoint: mockResolveElectronEndpoint, +})); + +vi.mock('./hooks.js', () => ({ + emitHook: mockEmitHook, +})); + +import { CDPBridge } from './browser/index.js'; +import { executeCommand } from './execution.js'; +import { cli, Strategy } from './registry.js'; + +const youtubeCommand = cli({ + site: 'youtube', + name: 'search', + description: 'search', + browser: true, + strategy: Strategy.COOKIE, + domain: 'www.youtube.com', + navigateBefore: false, + func: vi.fn(async () => 'ok'), +}); + +const cursorCommand = cli({ + site: 'cursor', + name: 'status', + description: 'status', + browser: true, + strategy: Strategy.COOKIE, + navigateBefore: false, + func: vi.fn(async () => 'ok'), +}); + +describe('executeCommand manual CDP routing', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); + mockGetDaemonHealth.mockResolvedValue({ state: 'ready', status: { extensionConnected: true } }); + mockProbeCDP.mockResolvedValue(true); + mockResolveElectronEndpoint.mockResolvedValue('http://127.0.0.1:9333'); + }); + + it('uses CDPBridge for non-Electron browser commands when OPENCLI_CDP_ENDPOINT is set', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'https://abcdef.ngrok.app'); + + await expect(executeCommand(youtubeCommand, {})).resolves.toBe('ok'); + + expect(mockGetDaemonHealth).not.toHaveBeenCalled(); + expect(mockProbeCDP).not.toHaveBeenCalled(); + expect(mockBrowserSession).toHaveBeenCalledWith( + CDPBridge, + expect.any(Function), + expect.objectContaining({ cdpEndpoint: 'https://abcdef.ngrok.app' }), + ); + }); + + it('preserves manual-endpoint validation for Electron apps', async () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + + await expect(executeCommand(cursorCommand, {})).resolves.toBe('ok'); + + expect(mockProbeCDP).toHaveBeenCalledWith(9222); + expect(mockGetDaemonHealth).not.toHaveBeenCalled(); + }); + + it('keeps Browser Bridge checks when no manual endpoint is set', async () => { + mockGetDaemonHealth.mockResolvedValue({ state: 'no-extension', status: { extensionConnected: false } }); + + await expect(executeCommand(youtubeCommand, {})).rejects.toThrow('Browser Bridge extension not connected'); + }); +}); diff --git a/src/execution.ts b/src/execution.ts index 278f788d..4718f7b7 100644 --- a/src/execution.ts +++ b/src/execution.ts @@ -14,11 +14,12 @@ import { type CliCommand, type InternalCliCommand, type Arg, type CommandArgs, g import type { IPage } from './types.js'; import { pathToFileURL } from 'node:url'; import { executePipeline } from './pipeline/index.js'; -import { AdapterLoadError, ArgumentError, CommandExecutionError, getErrorMessage } from './errors.js'; +import { AdapterLoadError, ArgumentError, BrowserConnectError, CommandExecutionError, getErrorMessage } from './errors.js'; import { isDiagnosticEnabled, collectDiagnostic, emitDiagnostic } from './diagnostic.js'; import { shouldUseBrowserSession } from './capabilityRouting.js'; import { getBrowserFactory, browserSession, runWithTimeout, DEFAULT_BROWSER_COMMAND_TIMEOUT } from './runtime.js'; import { emitHook, type HookContext } from './hooks.js'; +import { getDaemonHealth } from './browser/daemon-client.js'; import { log } from './logger.js'; import { isElectronApp } from './electron-apps.js'; import { probeCDP, resolveElectronEndpoint } from './launcher.js'; @@ -171,6 +172,24 @@ export async function executeCommand( } else { cdpEndpoint = await resolveElectronEndpoint(cmd.site); } + } else { + const manualEndpoint = process.env.OPENCLI_CDP_ENDPOINT?.trim() || undefined; + if (manualEndpoint) { + cdpEndpoint = manualEndpoint; + } else { + // Browser Bridge: fail-fast when daemon is up but extension is missing. + // 300ms timeout avoids a full 2s wait on cold-start. + const health = await getDaemonHealth({ timeout: 300 }); + if (health.state === 'no-extension') { + throw new BrowserConnectError( + 'Browser Bridge extension not connected', + 'Install the Browser Bridge:\n' + + ' 1. Download: https://github.com/jackwener/opencli/releases\n' + + ' 2. In Chrome or Chromium, open chrome://extensions → Developer Mode → Load unpacked\n' + + ' Then run: opencli doctor', + ); + } + } } ensureRequiredEnv(cmd); diff --git a/src/runtime.test.ts b/src/runtime.test.ts new file mode 100644 index 00000000..85248de8 --- /dev/null +++ b/src/runtime.test.ts @@ -0,0 +1,26 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { BrowserBridge, CDPBridge } from './browser/index.js'; +import { getBrowserFactory } from './runtime.js'; + +describe('getBrowserFactory', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + it('uses BrowserBridge by default for non-Electron sites', () => { + expect(getBrowserFactory()).toBe(BrowserBridge); + expect(getBrowserFactory('bilibili')).toBe(BrowserBridge); + }); + + it('uses CDPBridge for registered Electron apps', () => { + expect(getBrowserFactory('cursor')).toBe(CDPBridge); + }); + + it('prefers CDPBridge whenever OPENCLI_CDP_ENDPOINT is set, including zero-arg callers', () => { + vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222'); + + expect(getBrowserFactory()).toBe(CDPBridge); + expect(getBrowserFactory('bilibili')).toBe(CDPBridge); + expect(getBrowserFactory('cursor')).toBe(CDPBridge); + }); +}); diff --git a/src/runtime.ts b/src/runtime.ts index 0ea88a6a..a78d9e96 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -5,10 +5,11 @@ import { isElectronApp } from './electron-apps.js'; import { log } from './logger.js'; /** - * Returns the appropriate browser factory based on site type. - * Uses CDPBridge for registered Electron apps, otherwise BrowserBridge. + * Returns the appropriate browser factory for the current command path. + * Manual CDP endpoint overrides shared browser-factory callers. */ export function getBrowserFactory(site?: string): new () => IBrowserFactory { + if (process.env.OPENCLI_CDP_ENDPOINT?.trim()) return CDPBridge; if (site && isElectronApp(site)) return CDPBridge; return BrowserBridge; }