diff --git a/src/firefox/index.ts b/src/firefox/index.ts index 1782af3..c539282 100644 --- a/src/firefox/index.ts +++ b/src/firefox/index.ts @@ -11,7 +11,7 @@ import { remoteValueToNative } from '../utils/remote-value.js'; import { ConsoleEvents, NetworkEvents, DebuggingEvents, DownloadEvents } from './events/index.js'; import type { NetworkBodyResult } from './events/network.js'; import { DomInteractions } from './dom.js'; -import { PageManagement } from './pages.js'; +import { PageManagement, type ReadinessState } from './pages.js'; import { SnapshotManager, type Snapshot, type SnapshotOptions } from './snapshot/index.js'; /** @@ -232,11 +232,11 @@ export class FirefoxClient { // Pages / Navigation // ============================================================================ - async navigate(url: string): Promise { + async navigate(url: string, wait?: ReadinessState): Promise { if (!this.pages) { throw new Error('Not connected'); } - await this.pages.navigate(url); + await this.pages.navigate(url, wait); } async navigateBack(): Promise { @@ -302,11 +302,11 @@ export class FirefoxClient { return await this.pages.selectTab(index); } - async createNewPage(url: string): Promise { + async createNewPage(url: string, wait?: ReadinessState): Promise { if (!this.pages) { throw new Error('Not connected'); } - return await this.pages.createNewPage(url); + return await this.pages.createNewPage(url, wait); } async closeTab(index: number): Promise { diff --git a/src/firefox/pages.ts b/src/firefox/pages.ts index ee21c77..0729b9f 100644 --- a/src/firefox/pages.ts +++ b/src/firefox/pages.ts @@ -21,6 +21,20 @@ export function isCommonScheme(url: string): boolean { export type BiDiCommandFn = (method: string, params: Record) => Promise; +/** + * WebDriver BiDi browsingContext.ReadinessState. + * - "none": return as soon as navigation starts + * - "interactive": wait for DOMContentLoaded + * - "complete": wait for the load event, including subresources + */ +export const READINESS_STATES = ['none', 'interactive', 'complete'] as const; + +export type ReadinessState = (typeof READINESS_STATES)[number]; + +export function isReadinessState(value: unknown): value is ReadinessState { + return READINESS_STATES.includes(value as ReadinessState); +} + export class PageManagement { constructor( private driver: WebDriver, @@ -31,16 +45,22 @@ export class PageManagement { /** * Navigate to URL using BiDi + * + * @param url - Target URL + * @param waitOverride - Explicit readiness state to wait for. When omitted, + * common schemes wait for "interactive" and uncommon schemes do not wait. */ - async navigate(url: string): Promise { + async navigate(url: string, waitOverride?: ReadinessState): Promise { const contextId = this.getCurrentContextId(); if (!contextId) { throw new Error(`Cannot navigate: no browsing context ID`); } // Default wait time is "interactive" (DOMContentLoaded). - // All uncommon schemes use wait time "none" - const wait = isCommonScheme(url) ? 'interactive' : 'none'; + // All uncommon schemes use wait time "none". + // An explicit override is honoured for every scheme: silently downgrading it + // would discard what the caller asked for with no way to tell. + const wait: ReadinessState = waitOverride ?? (isCommonScheme(url) ? 'interactive' : 'none'); // Navigate using direct BiDi await this.sendBiDiCommand('browsingContext.navigate', { @@ -186,13 +206,13 @@ export class PageManagement { /** * Create new page (tab) */ - async createNewPage(url: string): Promise { + async createNewPage(url: string, waitOverride?: ReadinessState): Promise { await this.driver.switchTo().newWindow('tab'); const handles = await this.driver.getAllWindowHandles(); const newIdx = handles.length - 1; this.setCurrentContextId(handles[newIdx]!); this.cachedSelectedIdx = newIdx; - await this.navigate(url); + await this.navigate(url, waitOverride); return newIdx; } diff --git a/src/tools/pages.ts b/src/tools/pages.ts index 1082aec..ebc4b4d 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -10,10 +10,46 @@ import { } from '../utils/response-helpers.js'; import { saveOutput } from '../utils/save-output.js'; import { defineModule } from './module.js'; +import { READINESS_STATES, isReadinessState, type ReadinessState } from '../firefox/pages.js'; import type { McpToolResponse } from '../types/common.js'; const DEFAULT_MAX_CONTENT_CHARS = 20_000; +const WAIT_DESCRIPTION = + "When to return: 'none' (navigation started), 'interactive' (DOMContentLoaded), " + + "'complete' (load event fired, including subresources). Omit for the default: " + + "'interactive' for http/https/data/blob/file, 'none' for other schemes. Use " + + "'complete' when the page must be fully loaded, e.g. before stopping a performance recording."; + +const waitSchema = { + type: 'string', + enum: [...READINESS_STATES], + description: WAIT_DESCRIPTION, +}; + +/** + * Validate the optional `wait` argument. + * + * An unknown value is rejected rather than ignored: silently falling back to the + * default would leave the caller believing it waited for something it did not. + */ +function parseWait(value: unknown): ReadinessState | undefined { + if (value === undefined || value === null) { + return undefined; + } + if (!isReadinessState(value)) { + throw new Error( + `wait must be one of ${READINESS_STATES.join(', ')} (got ${JSON.stringify(value)})` + ); + } + return value; +} + +/** Echo the readiness state back only when the caller asked for one. */ +function waitSuffix(wait: ReadinessState | undefined): string { + return wait ? ` (waited for: ${wait})` : ''; +} + // Tool definitions export const listPagesTool = { name: 'list_pages', @@ -40,6 +76,7 @@ export const newPageTool = { type: 'string', description: 'Target URL', }, + wait: waitSchema, }, required: ['url'], }, @@ -58,6 +95,7 @@ export const navigatePageTool = { type: 'string', description: 'Target URL', }, + wait: waitSchema, }, required: ['url'], }, @@ -175,18 +213,20 @@ export async function handleListPages(_args: unknown): Promise export async function handleNewPage(args: unknown): Promise { try { - const { url } = args as { url: string }; + const { url, wait } = args as { url: string; wait?: unknown }; if (!url || typeof url !== 'string') { throw new Error('url parameter is required and must be a string'); } + const waitFor = parseWait(wait); + const { getFirefox } = await import('../index.js'); const firefox = await getFirefox(); - const newIdx = await firefox.createNewPage(url); + const newIdx = await firefox.createNewPage(url, waitFor); - return successResponse(`new page [${newIdx}] → ${url}`); + return successResponse(`new page [${newIdx}] → ${url}${waitSuffix(waitFor)}`); } catch (error) { return errorResponse(error as Error); } @@ -194,12 +234,14 @@ export async function handleNewPage(args: unknown): Promise { export async function handleNavigatePage(args: unknown): Promise { try { - const { url } = args as { url: string }; + const { url, wait } = args as { url: string; wait?: unknown }; if (!url || typeof url !== 'string') { throw new Error('url parameter is required and must be a string'); } + const waitFor = parseWait(wait); + const { getFirefox } = await import('../index.js'); const firefox = await getFirefox(); @@ -213,9 +255,9 @@ export async function handleNavigatePage(args: unknown): Promise { }); }); +describe('isReadinessState', () => { + it('accepts the three BiDi readiness states', () => { + expect(READINESS_STATES).toEqual(['none', 'interactive', 'complete']); + for (const state of READINESS_STATES) { + expect(isReadinessState(state)).toBe(true); + } + }); + + it('rejects anything else, including the DOM event name "load"', () => { + expect(isReadinessState('load')).toBe(false); + expect(isReadinessState('COMPLETE')).toBe(false); + expect(isReadinessState(undefined)).toBe(false); + expect(isReadinessState(true)).toBe(false); + }); +}); + // -- PageManagement ----------------------------------------------------------- describe('PageManagement', () => { @@ -128,6 +149,57 @@ describe('PageManagement', () => { wait: 'none', }); }); + + it('honours an explicit wait for common schemes', async () => { + const { pages, sendBiDiCommand } = createMocks(); + + await pages.navigate(HTTPS_URL, 'complete'); + expect(sendBiDiCommand).toHaveBeenCalledWith('browsingContext.navigate', { + context: 'ctx-1', + url: HTTPS_URL, + wait: 'complete', + }); + }); + + it('honours an explicit wait for uncommon schemes rather than downgrading it', async () => { + const { pages, sendBiDiCommand } = createMocks(); + + await pages.navigate(MOZ_EXT_URL, 'complete'); + expect(sendBiDiCommand).toHaveBeenCalledWith('browsingContext.navigate', { + context: 'ctx-1', + url: MOZ_EXT_URL, + wait: 'complete', + }); + + await pages.navigate('about:blank', 'interactive'); + expect(sendBiDiCommand).toHaveBeenCalledWith('browsingContext.navigate', { + context: 'ctx-1', + url: 'about:blank', + wait: 'interactive', + }); + }); + + it('allows an explicit wait to opt out of waiting on a common scheme', async () => { + const { pages, sendBiDiCommand } = createMocks(); + + await pages.navigate(HTTPS_URL, 'none'); + expect(sendBiDiCommand).toHaveBeenCalledWith('browsingContext.navigate', { + context: 'ctx-1', + url: HTTPS_URL, + wait: 'none', + }); + }); + + it('keeps the scheme-based default when no wait is given', async () => { + const { pages, sendBiDiCommand } = createMocks(); + + await pages.navigate(HTTPS_URL, undefined); + expect(sendBiDiCommand).toHaveBeenCalledWith('browsingContext.navigate', { + context: 'ctx-1', + url: HTTPS_URL, + wait: 'interactive', + }); + }); }); describe('createNewPage', () => { @@ -170,5 +242,32 @@ describe('PageManagement', () => { wait: 'none', }); }); + + it('forwards an explicit wait override to navigate', async () => { + const switchToMock = vi + .fn() + .mockReturnValue({ newWindow: vi.fn().mockResolvedValue(undefined) }); + const getAllWindowHandlesMock = vi.fn().mockResolvedValue(['handle-1', 'handle-2']); + + const driver = { + switchTo: switchToMock, + getAllWindowHandles: getAllWindowHandlesMock, + } as any; + + const sendBiDiCommand = vi.fn().mockResolvedValue({}); + const pages = new PageManagement( + driver, + vi.fn().mockReturnValue('handle-2'), + vi.fn(), + sendBiDiCommand + ); + + await pages.createNewPage(HTTPS_URL, 'complete'); + expect(sendBiDiCommand).toHaveBeenCalledWith('browsingContext.navigate', { + context: 'handle-2', + url: HTTPS_URL, + wait: 'complete', + }); + }); }); }); diff --git a/tests/tools/pages.test.ts b/tests/tools/pages.test.ts index 8ae58c6..f013061 100644 --- a/tests/tools/pages.test.ts +++ b/tests/tools/pages.test.ts @@ -64,6 +64,20 @@ describe('Pages Tools', () => { expect(properties?.url).toBeDefined(); }); + it.each([ + ['navigatePageTool', navigatePageTool], + ['newPageTool', newPageTool], + ])('%s should expose an optional wait enum', (_name, tool) => { + const schema = tool.inputSchema as { + properties?: Record; + required?: string[]; + }; + expect(schema.properties?.wait).toBeDefined(); + expect(schema.properties?.wait.type).toBe('string'); + expect(schema.properties?.wait.enum).toEqual(['none', 'interactive', 'complete']); + expect(schema.required).not.toContain('wait'); + }); + it('closePageTool should require pageIdx', () => { const { properties, required } = closePageTool.inputSchema; expect(properties).toBeDefined(); @@ -148,4 +162,64 @@ describe('Pages Tools', () => { expect(text).toContain('Preview:'); }); }); + + describe('Navigation handlers: wait argument', () => { + let navigate: ReturnType; + let createNewPage: ReturnType; + + beforeEach(() => { + navigate = vi.fn().mockResolvedValue(undefined); + createNewPage = vi.fn().mockResolvedValue(1); + + vi.doMock('../../src/index.js', () => ({ + args: {}, + getFirefox: vi.fn().mockResolvedValue({ + navigate, + createNewPage, + refreshTabs: vi.fn().mockResolvedValue(undefined), + getTabs: vi.fn().mockReturnValue([{ url: 'about:blank', title: 'blank' }]), + getSelectedTabIdx: vi.fn().mockReturnValue(0), + }), + })); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.resetModules(); + }); + + it('passes an explicit wait through to navigate', async () => { + const { handleNavigatePage } = await import('../../src/tools/pages.js'); + const result = await handleNavigatePage({ url: 'https://example.com', wait: 'complete' }); + + expect(navigate).toHaveBeenCalledWith('https://example.com', 'complete'); + const text = (result.content[0] as { type: 'text'; text: string }).text; + expect(text).toContain('waited for: complete'); + }); + + it('leaves the default in place when wait is omitted', async () => { + const { handleNavigatePage } = await import('../../src/tools/pages.js'); + const result = await handleNavigatePage({ url: 'https://example.com' }); + + expect(navigate).toHaveBeenCalledWith('https://example.com', undefined); + const text = (result.content[0] as { type: 'text'; text: string }).text; + expect(text).not.toContain('waited for'); + }); + + it('passes an explicit wait through to new_page', async () => { + const { handleNewPage } = await import('../../src/tools/pages.js'); + await handleNewPage({ url: 'https://example.com', wait: 'complete' }); + + expect(createNewPage).toHaveBeenCalledWith('https://example.com', 'complete'); + }); + + it('rejects an unknown wait value instead of silently ignoring it', async () => { + const { handleNavigatePage } = await import('../../src/tools/pages.js'); + const result = await handleNavigatePage({ url: 'https://example.com', wait: 'load' }); + + const text = (result.content[0] as { type: 'text'; text: string }).text; + expect(text).toContain('wait must be one of none, interactive, complete'); + expect(navigate).not.toHaveBeenCalled(); + }); + }); });