fix: restore BrowserManager.navigate() and package entry point (#748)
Root cause: package.json `main` pointed at `dist/daemon.js` (the internal daemon process), so programmatic consumers of the package received the daemon module instead of a usable API. Additionally, `BrowserManager.launch()` required IPC-only fields (`id`, `action`), and `navigate()` existed only as a private function inside actions.ts. Changes: - Add src/index.ts as the public package entry point - Add BrowserLaunchOptions type (Pick<LaunchCommand> minus id/action/engine) to decouple the programmatic API from the IPC wire protocol - Change launch() signature from LaunchCommand to BrowserLaunchOptions - Add BrowserManager.navigate(url, options?) — consolidates domain check, scoped-header setup, and page.goto() into one reusable method; auto- recovers a new page when all pages have been closed (stale session) - Add BrowserManager.getUrl() and getTitle() convenience methods - Update package.json: main → ./dist/index.js, add types and exports["."] - Add tests: navigate() with headers, waitUntil, allowedDomains blocking, allowedDomains allow, non-http(s) scheme blocking, getUrl/getTitle, and compile-time type assertions verifying the public entry exports the right API surface (direct repro of #307) Fixes #307 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
640d259130
commit
1129a3e7fc
+10
-1
@@ -3,7 +3,16 @@
|
||||
"version": "0.18.0",
|
||||
"description": "Headless browser automation CLI for AI agents",
|
||||
"type": "module",
|
||||
"main": "dist/daemon.js",
|
||||
"main": "./dist/index.js",
|
||||
"types": "./dist/index.d.ts",
|
||||
"exports": {
|
||||
".": {
|
||||
"import": "./dist/index.js",
|
||||
"types": "./dist/index.d.ts",
|
||||
"default": "./dist/index.js"
|
||||
},
|
||||
"./package.json": "./package.json"
|
||||
},
|
||||
"files": [
|
||||
"dist",
|
||||
"bin",
|
||||
|
||||
+4
-17
@@ -621,23 +621,12 @@ async function handleNavigate(
|
||||
command: NavigateCommand,
|
||||
browser: BrowserManager
|
||||
): Promise<Response<NavigateData>> {
|
||||
browser.checkDomainAllowed(command.url);
|
||||
|
||||
const page = browser.getPage();
|
||||
|
||||
// If headers are provided, set up scoped headers for this origin
|
||||
if (command.headers && Object.keys(command.headers).length > 0) {
|
||||
await browser.setScopedHeaders(command.url, command.headers);
|
||||
}
|
||||
|
||||
await page.goto(command.url, {
|
||||
waitUntil: command.waitUntil ?? 'load',
|
||||
const result = await browser.navigate(command.url, {
|
||||
headers: command.headers,
|
||||
waitUntil: command.waitUntil,
|
||||
});
|
||||
|
||||
return successResponse(command.id, {
|
||||
url: page.url(),
|
||||
title: await page.title(),
|
||||
});
|
||||
return successResponse(command.id, result);
|
||||
}
|
||||
|
||||
async function handleClick(command: ClickCommand, browser: BrowserManager): Promise<Response> {
|
||||
@@ -1892,8 +1881,6 @@ async function handleStateLoad(
|
||||
}
|
||||
|
||||
await browser.launch({
|
||||
id: command.id,
|
||||
action: 'launch',
|
||||
headless: true,
|
||||
autoStateFilePath: command.path,
|
||||
});
|
||||
|
||||
+97
-6
@@ -1,5 +1,16 @@
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest';
|
||||
import {
|
||||
describe,
|
||||
it,
|
||||
expect,
|
||||
expectTypeOf,
|
||||
beforeAll,
|
||||
afterAll,
|
||||
beforeEach,
|
||||
afterEach,
|
||||
vi,
|
||||
} from 'vitest';
|
||||
import { BrowserManager, getDefaultTimeout } from './browser.js';
|
||||
import type { BrowserManager as PublicBrowserManager, BrowserLaunchOptions } from './index.js';
|
||||
import { executeCommand } from './actions.js';
|
||||
import { chromium } from 'playwright-core';
|
||||
import os from 'node:os';
|
||||
@@ -40,18 +51,16 @@ describe('BrowserManager', () => {
|
||||
|
||||
it('should be no-op when relaunching with same options', async () => {
|
||||
const browserInstance = browser.getBrowser();
|
||||
await browser.launch({ id: 'test', action: 'launch', headless: true });
|
||||
await browser.launch({ headless: true });
|
||||
expect(browser.getBrowser()).toBe(browserInstance);
|
||||
});
|
||||
|
||||
it('should reconnect when CDP port changes', async () => {
|
||||
const newBrowser = new BrowserManager();
|
||||
await newBrowser.launch({ id: 'test', action: 'launch', headless: true });
|
||||
await newBrowser.launch({ headless: true });
|
||||
expect(newBrowser.getBrowser()).not.toBeNull();
|
||||
|
||||
await expect(
|
||||
newBrowser.launch({ id: 'test', action: 'launch', cdpPort: 59999 })
|
||||
).rejects.toThrow();
|
||||
await expect(newBrowser.launch({ cdpPort: 59999 })).rejects.toThrow();
|
||||
|
||||
expect(newBrowser.getBrowser()).toBeNull();
|
||||
await newBrowser.close();
|
||||
@@ -275,11 +284,93 @@ describe('BrowserManager', () => {
|
||||
expect(page.url()).toBe('https://example.com/');
|
||||
});
|
||||
|
||||
it('should navigate via BrowserManager API', async () => {
|
||||
const result = await browser.navigate('https://example.com');
|
||||
expect(result).toEqual({
|
||||
url: 'https://example.com/',
|
||||
title: 'Example Domain',
|
||||
});
|
||||
});
|
||||
|
||||
it('should navigate with custom headers without throwing', async () => {
|
||||
const result = await browser.navigate('https://example.com', {
|
||||
headers: { 'X-Custom-Header': 'test-value' },
|
||||
});
|
||||
expect(result.url).toBe('https://example.com/');
|
||||
expect(result.title).toBe('Example Domain');
|
||||
});
|
||||
|
||||
it('should navigate with waitUntil option', async () => {
|
||||
const result = await browser.navigate('https://example.com', {
|
||||
waitUntil: 'domcontentloaded',
|
||||
});
|
||||
expect(result.url).toBe('https://example.com/');
|
||||
});
|
||||
|
||||
it('should get page title', async () => {
|
||||
const page = browser.getPage();
|
||||
const title = await page.title();
|
||||
expect(title).toBe('Example Domain');
|
||||
});
|
||||
|
||||
it('should expose current URL and title via BrowserManager API', async () => {
|
||||
await browser.navigate('https://example.com');
|
||||
await expect(browser.getUrl()).resolves.toBe('https://example.com/');
|
||||
await expect(browser.getTitle()).resolves.toBe('Example Domain');
|
||||
});
|
||||
});
|
||||
|
||||
describe('navigate() domain filtering', () => {
|
||||
it('should block navigation to a domain outside allowedDomains', async () => {
|
||||
const restricted = new BrowserManager();
|
||||
await restricted.launch({ headless: true, allowedDomains: ['example.com'] });
|
||||
try {
|
||||
await expect(restricted.navigate('https://httpbin.org')).rejects.toThrow(
|
||||
'Navigation blocked'
|
||||
);
|
||||
} finally {
|
||||
await restricted.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('should allow navigation within allowedDomains', async () => {
|
||||
const restricted = new BrowserManager();
|
||||
await restricted.launch({ headless: true, allowedDomains: ['example.com'] });
|
||||
try {
|
||||
const result = await restricted.navigate('https://example.com');
|
||||
expect(result.url).toBe('https://example.com/');
|
||||
} finally {
|
||||
await restricted.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('should block non-http(s) schemes regardless of allowedDomains', async () => {
|
||||
const restricted = new BrowserManager();
|
||||
await restricted.launch({ headless: true, allowedDomains: ['example.com'] });
|
||||
try {
|
||||
await expect(restricted.navigate('ftp://example.com')).rejects.toThrow(
|
||||
'Navigation blocked'
|
||||
);
|
||||
} finally {
|
||||
await restricted.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('navigate() public API types (issue #307)', () => {
|
||||
it('BrowserManager exported from package entry has navigate method', () => {
|
||||
// Compile-time proof: if this file type-checks, the public API surface is correct.
|
||||
// navigate() must exist on BrowserManager — absence was the bug in #307.
|
||||
expectTypeOf<InstanceType<typeof PublicBrowserManager>>().toHaveProperty('navigate');
|
||||
expectTypeOf<InstanceType<typeof PublicBrowserManager>>().toHaveProperty('launch');
|
||||
});
|
||||
|
||||
it('BrowserLaunchOptions does not require id or action', () => {
|
||||
// id and action are IPC-only fields that must not leak into the public API
|
||||
expectTypeOf<BrowserLaunchOptions>().not.toHaveProperty('id');
|
||||
expectTypeOf<BrowserLaunchOptions>().not.toHaveProperty('action');
|
||||
expectTypeOf<BrowserLaunchOptions>().not.toHaveProperty('engine');
|
||||
});
|
||||
});
|
||||
|
||||
describe('element interaction', () => {
|
||||
|
||||
+74
-1
@@ -97,6 +97,36 @@ export interface ScreencastOptions {
|
||||
everyNthFrame?: number;
|
||||
}
|
||||
|
||||
export interface NavigateOptions {
|
||||
waitUntil?: 'load' | 'domcontentloaded' | 'networkidle';
|
||||
headers?: Record<string, string>;
|
||||
}
|
||||
|
||||
export type BrowserLaunchOptions = Pick<
|
||||
LaunchCommand,
|
||||
| 'headless'
|
||||
| 'viewport'
|
||||
| 'browser'
|
||||
| 'headers'
|
||||
| 'executablePath'
|
||||
| 'cdpPort'
|
||||
| 'cdpUrl'
|
||||
| 'autoConnect'
|
||||
| 'extensions'
|
||||
| 'profile'
|
||||
| 'storageState'
|
||||
| 'proxy'
|
||||
| 'args'
|
||||
| 'userAgent'
|
||||
| 'provider'
|
||||
| 'ignoreHTTPSErrors'
|
||||
| 'allowFileAccess'
|
||||
| 'colorScheme'
|
||||
| 'downloadPath'
|
||||
| 'allowedDomains'
|
||||
| 'autoStateFilePath'
|
||||
>;
|
||||
|
||||
interface TrackedRequest {
|
||||
url: string;
|
||||
method: string;
|
||||
@@ -458,6 +488,49 @@ export class BrowserManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Navigate the active page to a URL and return the resolved URL + title.
|
||||
* If the browser is launched but all pages have been closed, a new page is
|
||||
* created automatically before navigating (stale-session recovery).
|
||||
*/
|
||||
async navigate(
|
||||
url: string,
|
||||
options: NavigateOptions = {}
|
||||
): Promise<{ url: string; title: string }> {
|
||||
this.checkDomainAllowed(url);
|
||||
await this.ensurePage();
|
||||
|
||||
if (options.headers && Object.keys(options.headers).length > 0) {
|
||||
await this.setScopedHeaders(url, options.headers);
|
||||
}
|
||||
|
||||
const page = this.getPage();
|
||||
await page.goto(url, {
|
||||
waitUntil: options.waitUntil ?? 'load',
|
||||
});
|
||||
|
||||
return {
|
||||
url: page.url(),
|
||||
title: await page.title(),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the active page URL.
|
||||
*/
|
||||
async getUrl(): Promise<string> {
|
||||
await this.ensurePage();
|
||||
return this.getPage().url();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the active page title.
|
||||
*/
|
||||
async getTitle(): Promise<string> {
|
||||
await this.ensurePage();
|
||||
return this.getPage().title();
|
||||
}
|
||||
|
||||
/**
|
||||
* Switch back to main frame
|
||||
*/
|
||||
@@ -1357,7 +1430,7 @@ export class BrowserManager {
|
||||
* Launch the browser with the specified options
|
||||
* If already launched, this is a no-op (browser stays open)
|
||||
*/
|
||||
async launch(options: LaunchCommand): Promise<void> {
|
||||
async launch(options: BrowserLaunchOptions): Promise<void> {
|
||||
// Determine CDP endpoint: prefer cdpUrl over cdpPort for flexibility
|
||||
const cdpEndpoint = options.cdpUrl ?? (options.cdpPort ? String(options.cdpPort) : undefined);
|
||||
const hasExtensions = !!options.extensions?.length;
|
||||
|
||||
@@ -503,8 +503,6 @@ export async function startDaemon(options?: {
|
||||
? colorSchemeEnv
|
||||
: undefined;
|
||||
await manager.launch({
|
||||
id: 'auto',
|
||||
action: 'launch' as const,
|
||||
headless:
|
||||
process.env.AGENT_BROWSER_HEADED !== '1' &&
|
||||
process.env.AGENT_BROWSER_HEADED !== 'true',
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
export { BrowserManager, getDefaultTimeout } from './browser.js';
|
||||
export type {
|
||||
BrowserLaunchOptions,
|
||||
NavigateOptions,
|
||||
ScreencastFrame,
|
||||
ScreencastOptions,
|
||||
} from './browser.js';
|
||||
export { IOSManager } from './ios-manager.js';
|
||||
export { executeCommand } from './actions.js';
|
||||
export type { Command, LaunchCommand, NavigateCommand, Response } from './types.js';
|
||||
export {
|
||||
cleanupSocket,
|
||||
getAppDir,
|
||||
getConnectionInfo,
|
||||
getPidFile,
|
||||
getPortFile,
|
||||
getPortForSession,
|
||||
getSession,
|
||||
getSocketDir,
|
||||
getSocketPath,
|
||||
getStreamPortFile,
|
||||
isDaemonRunning,
|
||||
safeWrite,
|
||||
setSession,
|
||||
startDaemon,
|
||||
} from './daemon.js';
|
||||
Reference in New Issue
Block a user