Skip to content

Commit d6302ec

Browse files
committed
fix: honor manual cdp endpoints
1 parent 714df64 commit d6302ec

8 files changed

Lines changed: 235 additions & 8 deletions

File tree

src/browser/cdp.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
33
const { MockWebSocket } = vi.hoisted(() => {
44
class MockWebSocket {
55
static OPEN = 1;
6+
static urls: string[] = [];
67
readyState = 1;
78
private handlers = new Map<string, Array<(...args: unknown[]) => void>>();
89

9-
constructor(_url: string) {
10+
constructor(url: string) {
11+
MockWebSocket.urls.push(url);
1012
queueMicrotask(() => this.emit('open'));
1113
}
1214

@@ -41,6 +43,7 @@ import { CDPBridge } from './cdp.js';
4143
describe('CDPBridge cookies', () => {
4244
beforeEach(() => {
4345
vi.unstubAllEnvs();
46+
MockWebSocket.urls = [];
4447
});
4548

4649
it('filters cookies by actual domain match instead of substring match', async () => {
@@ -63,4 +66,15 @@ describe('CDPBridge cookies', () => {
6366
{ name: 'exact', value: '2', domain: 'example.com' },
6467
]);
6568
});
69+
70+
it('trims OPENCLI_CDP_ENDPOINT before opening the websocket', async () => {
71+
vi.stubEnv('OPENCLI_CDP_ENDPOINT', ' ws://127.0.0.1:9222/devtools/page/1 ');
72+
73+
const bridge = new CDPBridge();
74+
vi.spyOn(bridge, 'send').mockResolvedValue({});
75+
76+
await bridge.connect();
77+
78+
expect(MockWebSocket.urls).toEqual(['ws://127.0.0.1:9222/devtools/page/1']);
79+
});
6680
});

src/browser/cdp.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export class CDPBridge implements IBrowserFactory {
4949
async connect(opts?: { timeout?: number; workspace?: string; cdpEndpoint?: string }): Promise<IPage> {
5050
if (this._ws) throw new Error('CDPBridge is already connected. Call close() before reconnecting.');
5151

52-
const endpoint = opts?.cdpEndpoint ?? process.env.OPENCLI_CDP_ENDPOINT;
52+
const endpoint = (opts?.cdpEndpoint ?? process.env.OPENCLI_CDP_ENDPOINT)?.trim();
5353
if (!endpoint) throw new Error('CDP endpoint not provided (pass cdpEndpoint or set OPENCLI_CDP_ENDPOINT)');
5454

5555
let wsUrl = endpoint;

src/cli-browser.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
const {
4+
mockCdpConnect,
5+
mockBridgeConnect,
6+
} = vi.hoisted(() => ({
7+
mockCdpConnect: vi.fn(),
8+
mockBridgeConnect: vi.fn(),
9+
}));
10+
11+
vi.mock('./browser/index.js', () => ({
12+
BrowserBridge: class {
13+
connect = mockBridgeConnect;
14+
close = vi.fn();
15+
},
16+
CDPBridge: class {
17+
connect = mockCdpConnect;
18+
close = vi.fn();
19+
},
20+
}));
21+
22+
import { createProgram } from './cli.js';
23+
24+
describe('browser manual CDP routing', () => {
25+
beforeEach(() => {
26+
vi.unstubAllEnvs();
27+
vi.clearAllMocks();
28+
vi.spyOn(console, 'log').mockImplementation(() => {});
29+
30+
const page = {
31+
evaluate: vi.fn(),
32+
wait: vi.fn(),
33+
};
34+
35+
mockBridgeConnect.mockResolvedValue(page);
36+
mockCdpConnect.mockResolvedValue(page);
37+
});
38+
39+
it('uses CDPBridge when OPENCLI_CDP_ENDPOINT is set', async () => {
40+
vi.stubEnv('OPENCLI_CDP_ENDPOINT', ' https://abcdef.ngrok.app ');
41+
42+
const program = createProgram('', '');
43+
await program.parseAsync(['node', 'opencli', 'browser', 'back']);
44+
45+
expect(mockCdpConnect).toHaveBeenCalledWith({
46+
timeout: 30,
47+
workspace: 'browser:default',
48+
cdpEndpoint: 'https://abcdef.ngrok.app',
49+
});
50+
expect(mockBridgeConnect).not.toHaveBeenCalled();
51+
});
52+
53+
it('keeps BrowserBridge when OPENCLI_CDP_ENDPOINT is not set', async () => {
54+
const program = createProgram('', '');
55+
await program.parseAsync(['node', 'opencli', 'browser', 'back']);
56+
57+
expect(mockBridgeConnect).toHaveBeenCalledWith({
58+
timeout: 30,
59+
workspace: 'browser:default',
60+
});
61+
expect(mockCdpConnect).not.toHaveBeenCalled();
62+
});
63+
});

src/cli.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ const CLI_FILE = fileURLToPath(import.meta.url);
2626

2727
/** Create a browser page for browser commands. Uses a dedicated browser workspace for session persistence. */
2828
async function getBrowserPage(): Promise<import('./types.js').IPage> {
29-
const { BrowserBridge } = await import('./browser/index.js');
30-
const bridge = new BrowserBridge();
31-
return bridge.connect({ timeout: 30, workspace: 'browser:default' });
29+
const cdpEndpoint = process.env.OPENCLI_CDP_ENDPOINT?.trim() || undefined;
30+
const BrowserFactory = getBrowserFactory();
31+
const browser = new BrowserFactory();
32+
return browser.connect({ timeout: 30, workspace: 'browser:default', cdpEndpoint });
3233
}
3334

3435
function applyVerbose(opts: { verbose?: boolean }): void {

src/execution-routing.test.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
const {
4+
mockBrowserSession,
5+
mockGetDaemonHealth,
6+
mockProbeCDP,
7+
mockResolveElectronEndpoint,
8+
mockEmitHook,
9+
} = vi.hoisted(() => ({
10+
mockBrowserSession: vi.fn(async (_Factory, fn) => fn({
11+
goto: vi.fn(),
12+
wait: vi.fn(),
13+
} as any)),
14+
mockGetDaemonHealth: vi.fn(),
15+
mockProbeCDP: vi.fn(),
16+
mockResolveElectronEndpoint: vi.fn(),
17+
mockEmitHook: vi.fn(),
18+
}));
19+
20+
vi.mock('./runtime.js', async () => {
21+
const actual = await vi.importActual<typeof import('./runtime.js')>('./runtime.js');
22+
return {
23+
...actual,
24+
browserSession: mockBrowserSession,
25+
};
26+
});
27+
28+
vi.mock('./browser/daemon-client.js', () => ({
29+
getDaemonHealth: mockGetDaemonHealth,
30+
}));
31+
32+
vi.mock('./launcher.js', () => ({
33+
probeCDP: mockProbeCDP,
34+
resolveElectronEndpoint: mockResolveElectronEndpoint,
35+
}));
36+
37+
vi.mock('./hooks.js', () => ({
38+
emitHook: mockEmitHook,
39+
}));
40+
41+
import { CDPBridge } from './browser/index.js';
42+
import { executeCommand } from './execution.js';
43+
import { cli, Strategy } from './registry.js';
44+
45+
const youtubeCommand = cli({
46+
site: 'youtube',
47+
name: 'search',
48+
description: 'search',
49+
browser: true,
50+
strategy: Strategy.COOKIE,
51+
domain: 'www.youtube.com',
52+
navigateBefore: false,
53+
func: vi.fn(async () => 'ok'),
54+
});
55+
56+
const cursorCommand = cli({
57+
site: 'cursor',
58+
name: 'status',
59+
description: 'status',
60+
browser: true,
61+
strategy: Strategy.COOKIE,
62+
navigateBefore: false,
63+
func: vi.fn(async () => 'ok'),
64+
});
65+
66+
describe('executeCommand manual CDP routing', () => {
67+
beforeEach(() => {
68+
vi.unstubAllEnvs();
69+
vi.clearAllMocks();
70+
mockGetDaemonHealth.mockResolvedValue({ state: 'ready', status: { extensionConnected: true } });
71+
mockProbeCDP.mockResolvedValue(true);
72+
mockResolveElectronEndpoint.mockResolvedValue('http://127.0.0.1:9333');
73+
});
74+
75+
it('uses CDPBridge for non-Electron browser commands when OPENCLI_CDP_ENDPOINT is set', async () => {
76+
vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'https://abcdef.ngrok.app');
77+
78+
await expect(executeCommand(youtubeCommand, {})).resolves.toBe('ok');
79+
80+
expect(mockGetDaemonHealth).not.toHaveBeenCalled();
81+
expect(mockProbeCDP).not.toHaveBeenCalled();
82+
expect(mockBrowserSession).toHaveBeenCalledWith(
83+
CDPBridge,
84+
expect.any(Function),
85+
expect.objectContaining({ cdpEndpoint: 'https://abcdef.ngrok.app' }),
86+
);
87+
});
88+
89+
it('preserves manual-endpoint validation for Electron apps', async () => {
90+
vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222');
91+
92+
await expect(executeCommand(cursorCommand, {})).resolves.toBe('ok');
93+
94+
expect(mockProbeCDP).toHaveBeenCalledWith(9222);
95+
expect(mockGetDaemonHealth).not.toHaveBeenCalled();
96+
});
97+
98+
it('keeps Browser Bridge checks when no manual endpoint is set', async () => {
99+
mockGetDaemonHealth.mockResolvedValue({ state: 'no-extension', status: { extensionConnected: false } });
100+
101+
await expect(executeCommand(youtubeCommand, {})).rejects.toThrow('Browser Bridge extension not connected');
102+
});
103+
});

src/execution.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@ import { type CliCommand, type InternalCliCommand, type Arg, type CommandArgs, S
1414
import type { IPage } from './types.js';
1515
import { pathToFileURL } from 'node:url';
1616
import { executePipeline } from './pipeline/index.js';
17-
import { AdapterLoadError, ArgumentError, CommandExecutionError, getErrorMessage } from './errors.js';
17+
import { AdapterLoadError, ArgumentError, BrowserConnectError, CommandExecutionError, getErrorMessage } from './errors.js';
1818
import { isDiagnosticEnabled, collectDiagnostic, emitDiagnostic } from './diagnostic.js';
1919
import { shouldUseBrowserSession } from './capabilityRouting.js';
2020
import { getBrowserFactory, browserSession, runWithTimeout, DEFAULT_BROWSER_COMMAND_TIMEOUT } from './runtime.js';
2121
import { emitHook, type HookContext } from './hooks.js';
22+
import { getDaemonHealth } from './browser/daemon-client.js';
2223
import { log } from './logger.js';
2324
import { isElectronApp } from './electron-apps.js';
2425
import { probeCDP, resolveElectronEndpoint } from './launcher.js';
@@ -174,6 +175,24 @@ export async function executeCommand(
174175
} else {
175176
cdpEndpoint = await resolveElectronEndpoint(cmd.site);
176177
}
178+
} else {
179+
const manualEndpoint = process.env.OPENCLI_CDP_ENDPOINT?.trim() || undefined;
180+
if (manualEndpoint) {
181+
cdpEndpoint = manualEndpoint;
182+
} else {
183+
// Browser Bridge: fail-fast when daemon is up but extension is missing.
184+
// 300ms timeout avoids a full 2s wait on cold-start.
185+
const health = await getDaemonHealth({ timeout: 300 });
186+
if (health.state === 'no-extension') {
187+
throw new BrowserConnectError(
188+
'Browser Bridge extension not connected',
189+
'Install the Browser Bridge:\n' +
190+
' 1. Download: https://github.com/jackwener/opencli/releases\n' +
191+
' 2. In Chrome or Chromium, open chrome://extensions → Developer Mode → Load unpacked\n' +
192+
' Then run: opencli doctor',
193+
);
194+
}
195+
}
177196
}
178197

179198
ensureRequiredEnv(cmd);

src/runtime.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { BrowserBridge, CDPBridge } from './browser/index.js';
3+
import { getBrowserFactory } from './runtime.js';
4+
5+
describe('getBrowserFactory', () => {
6+
beforeEach(() => {
7+
vi.unstubAllEnvs();
8+
});
9+
10+
it('uses BrowserBridge by default for non-Electron sites', () => {
11+
expect(getBrowserFactory()).toBe(BrowserBridge);
12+
expect(getBrowserFactory('bilibili')).toBe(BrowserBridge);
13+
});
14+
15+
it('uses CDPBridge for registered Electron apps', () => {
16+
expect(getBrowserFactory('cursor')).toBe(CDPBridge);
17+
});
18+
19+
it('prefers CDPBridge whenever OPENCLI_CDP_ENDPOINT is set, including zero-arg callers', () => {
20+
vi.stubEnv('OPENCLI_CDP_ENDPOINT', 'http://127.0.0.1:9222');
21+
22+
expect(getBrowserFactory()).toBe(CDPBridge);
23+
expect(getBrowserFactory('bilibili')).toBe(CDPBridge);
24+
expect(getBrowserFactory('cursor')).toBe(CDPBridge);
25+
});
26+
});

src/runtime.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ import { TimeoutError } from './errors.js';
44
import { isElectronApp } from './electron-apps.js';
55

66
/**
7-
* Returns the appropriate browser factory based on site type.
8-
* Uses CDPBridge for registered Electron apps, otherwise BrowserBridge.
7+
* Returns the appropriate browser factory for the current command path.
8+
* Manual CDP endpoint overrides shared browser-factory callers.
99
*/
1010
export function getBrowserFactory(site?: string): new () => IBrowserFactory {
11+
if (process.env.OPENCLI_CDP_ENDPOINT?.trim()) return CDPBridge;
1112
if (site && isElectronApp(site)) return CDPBridge;
1213
return BrowserBridge;
1314
}

0 commit comments

Comments
 (0)