diff --git a/src/firefox/core.ts b/src/firefox/core.ts index 032085fe..19c31471 100644 --- a/src/firefox/core.ts +++ b/src/firefox/core.ts @@ -157,6 +157,9 @@ const BINARY_NOT_FOUND_HINT = 'Unable to detect Firefox binary automatically, please provide the full path via ' + '--firefox-path'; +/** Leave prompts open until accept_dialog or dismiss_dialog handles them. */ +const UNHANDLED_PROMPT_BEHAVIOR = 'ignore'; + export class FirefoxCore { private currentContextId: string | null = null; private detectedBinaryPath: string | undefined; @@ -218,6 +221,7 @@ export class FirefoxCore { const caps = new Capabilities(); caps.set('webSocketUrl', true); + caps.set('unhandledPromptBehavior', UNHANDLED_PROMPT_BEHAVIOR); caps.set('moz:firefoxOptions', mozOptions); if (this.options.acceptInsecureCerts) { caps.set('acceptInsecureCerts', true); @@ -275,6 +279,7 @@ export class FirefoxCore { // session in a locked state that blocks reconnection. const caps = new Capabilities(); caps.set('webSocketUrl', true); + caps.set('unhandledPromptBehavior', UNHANDLED_PROMPT_BEHAVIOR); // createSession() returns synchronously; the session is established async under the hood. // Passing geckodriverPath to ServiceBuilder prevents getBinaryPaths() from running, @@ -317,6 +322,7 @@ export class FirefoxCore { // Standard path: launch a new Firefox via selenium-webdriver const firefoxOptions = new firefox.Options(); firefoxOptions.enableBidi(); + firefoxOptions.setAlertBehavior(UNHANDLED_PROMPT_BEHAVIOR); if (this.options.headless) { firefoxOptions.addArguments('-headless'); diff --git a/src/firefox/pages.ts b/src/firefox/pages.ts index 48fec917..182cd9ce 100644 --- a/src/firefox/pages.ts +++ b/src/firefox/pages.ts @@ -36,6 +36,18 @@ export function isReadinessState(value: unknown): value is BrowsingContext.Readi return READINESS_STATES.includes(value as BrowsingContext.ReadinessState); } +/** + * Keep NoSuchAlertError as is so callers can tell it apart: Marionette sends it with an empty message. + */ +function dialogError(action: 'accept' | 'dismiss', error: unknown): Error { + if (error instanceof Error && error.name === 'NoSuchAlertError') { + return error; + } + return new Error( + `Failed to ${action} dialog: ${error instanceof Error ? error.message : String(error)}` + ); +} + export class PageManagement { constructor( private driver: WebDriver, @@ -108,9 +120,7 @@ export class PageManagement { } await alert.accept(); } catch (error) { - throw new Error( - `Failed to accept dialog: ${error instanceof Error ? error.message : String(error)}` - ); + throw dialogError('accept', error); } } @@ -122,9 +132,7 @@ export class PageManagement { const alert = await this.driver.switchTo().alert(); await alert.dismiss(); } catch (error) { - throw new Error( - `Failed to dismiss dialog: ${error instanceof Error ? error.message : String(error)}` - ); + throw dialogError('dismiss', error); } } diff --git a/src/tools/utilities.ts b/src/tools/utilities.ts index 372239e7..4dbcd82e 100644 --- a/src/tools/utilities.ts +++ b/src/tools/utilities.ts @@ -92,10 +92,8 @@ export const handleAcceptDialog = defineToolHandler(async function handleAcceptD await firefox.acceptDialog(promptText); return successResponse(promptText ? `Accepted: "${promptText}"` : 'Accepted'); } catch (error) { - const errorMsg = (error as Error).message; - // Concise error for no active dialog - if (errorMsg.includes('no such alert') || errorMsg.includes('No dialog')) { + if ((error as Error).name === 'NoSuchAlertError') { throw new Error('No active dialog'); } @@ -113,10 +111,8 @@ export const handleDismissDialog = defineToolHandler(async function handleDismis await firefox.dismissDialog(); return successResponse('Dismissed'); } catch (error) { - const errorMsg = (error as Error).message; - // Concise error for no active dialog - if (errorMsg.includes('no such alert') || errorMsg.includes('No dialog')) { + if ((error as Error).name === 'NoSuchAlertError') { throw new Error('No active dialog'); } diff --git a/tests/firefox/connect-existing.test.ts b/tests/firefox/connect-existing.test.ts index 50ce1136..412fda06 100644 --- a/tests/firefox/connect-existing.test.ts +++ b/tests/firefox/connect-existing.test.ts @@ -136,6 +136,7 @@ describe('FirefoxCore connect() BiDi endpoint check', () => { // Mocks for the launch path (used by the launch-mode test only) const mockEnableBidi = vi.fn(); + const mockSetAlertBehavior = vi.fn(); const mockOptionsAddArguments = vi.fn(); // Fake socket for the Marionette port probe: emits `event` on the next tick. @@ -183,6 +184,7 @@ describe('FirefoxCore connect() BiDi endpoint check', () => { default: { Options: class { enableBidi = mockEnableBidi; + setAlertBehavior = mockSetAlertBehavior; addArguments = mockOptionsAddArguments; windowSize = vi.fn(); setBinary = vi.fn(); @@ -373,4 +375,34 @@ describe('FirefoxCore connect() BiDi endpoint check', () => { expect(mockCreateSession).not.toHaveBeenCalled(); expect(core.getCurrentContextId()).toBe('mock-context-id'); }); + + describe('unhandled prompt behavior', () => { + it('should keep prompts open in launch mode', async () => { + const { FirefoxCore } = await import('@/firefox/core.js'); + + await new FirefoxCore({ headless: true }).connect(); + + expect(mockSetAlertBehavior).toHaveBeenCalledWith('ignore'); + }); + + it('should keep prompts open in connect-existing mode', async () => { + mockCreateSession.mockReturnValue( + makeDriver({ browserVersion: '142.0', webSocketUrl: 'ws://127.0.0.1:9222/session/abc' }) + ); + const { FirefoxCore } = await import('@/firefox/core.js'); + + await new FirefoxCore({ connectExisting: true, marionettePort: 2828 }).connect(); + + expect(mockCapabilitiesSet).toHaveBeenCalledWith('unhandledPromptBehavior', 'ignore'); + }); + + it('should keep prompts open in Android mode', async () => { + mockCreateSession.mockReturnValue(makeDriver({ browserVersion: '142.0' })); + const { FirefoxCore } = await import('@/firefox/core.js'); + + await new FirefoxCore({ androidDevice: 'auto', androidWipeAppData: true }).connect(); + + expect(mockCapabilitiesSet).toHaveBeenCalledWith('unhandledPromptBehavior', 'ignore'); + }); + }); }); diff --git a/tests/firefox/core-prefs.test.ts b/tests/firefox/core-prefs.test.ts index 9fdb9617..66792650 100644 --- a/tests/firefox/core-prefs.test.ts +++ b/tests/firefox/core-prefs.test.ts @@ -27,6 +27,7 @@ describe('FirefoxCore prefs via firefoxOptions', () => { default: { Options: class { enableBidi = vi.fn(); + setAlertBehavior = vi.fn(); addArguments = vi.fn(); setBinary = vi.fn(); setAcceptInsecureCerts = vi.fn(); diff --git a/tests/firefox/core.test.ts b/tests/firefox/core.test.ts index 26685234..c3920296 100644 --- a/tests/firefox/core.test.ts +++ b/tests/firefox/core.test.ts @@ -313,6 +313,7 @@ describe('FirefoxCore connect() profile handling', () => { const mockAddArguments = vi.fn(); const mockSetProfile = vi.fn(); const mockEnableBidi = vi.fn(); + const mockSetAlertBehavior = vi.fn(); const mockSetBinary = vi.fn(); const mockWindowSize = vi.fn(); const mockSetAcceptInsecureCerts = vi.fn(); @@ -327,6 +328,7 @@ describe('FirefoxCore connect() profile handling', () => { default: { Options: class { enableBidi = mockEnableBidi; + setAlertBehavior = mockSetAlertBehavior; addArguments = mockAddArguments; setProfile = mockSetProfile; setBinary = mockSetBinary; @@ -453,6 +455,7 @@ describe('FirefoxCore connect() binary lookup failure', () => { default: { Options: class { enableBidi = vi.fn(); + setAlertBehavior = vi.fn(); addArguments = vi.fn(); setBinary = vi.fn(); windowSize = vi.fn(); diff --git a/tests/firefox/pages.test.ts b/tests/firefox/pages.test.ts index dcf6727a..5f34582e 100644 --- a/tests/firefox/pages.test.ts +++ b/tests/firefox/pages.test.ts @@ -464,4 +464,54 @@ describe('PageManagement', () => { expect(driver.close).not.toHaveBeenCalled(); }); }); + + describe('dialogs', () => { + function createDialogMocks(alert: Record) { + const driver = { + switchTo: vi.fn(() => ({ alert: vi.fn().mockResolvedValue(alert) })), + } as any; + const pages = new PageManagement(driver, vi.fn(), vi.fn(), vi.fn()); + return { pages, driver }; + } + + function noSuchAlertError(): Error { + const error = new Error(''); + error.name = 'NoSuchAlertError'; + return error; + } + + it('types the prompt text before accepting', async () => { + const alert = { sendKeys: vi.fn(), accept: vi.fn(), dismiss: vi.fn() }; + const { pages } = createDialogMocks(alert); + + await pages.acceptDialog('hello'); + + expect(alert.sendKeys).toHaveBeenCalledWith('hello'); + expect(alert.accept).toHaveBeenCalledOnce(); + }); + + it('keeps NoSuchAlertError unchanged when there is no dialog', async () => { + const error = noSuchAlertError(); + const alert = { + sendKeys: vi.fn(), + accept: vi.fn().mockRejectedValue(error), + dismiss: vi.fn().mockRejectedValue(error), + }; + const { pages } = createDialogMocks(alert); + + await expect(pages.acceptDialog()).rejects.toBe(error); + await expect(pages.dismissDialog()).rejects.toBe(error); + }); + + it('wraps other errors with the action that failed', async () => { + const alert = { + accept: vi.fn().mockRejectedValue(new Error('boom')), + dismiss: vi.fn().mockRejectedValue(new Error('bang')), + }; + const { pages } = createDialogMocks(alert); + + await expect(pages.acceptDialog()).rejects.toThrow('Failed to accept dialog: boom'); + await expect(pages.dismissDialog()).rejects.toThrow('Failed to dismiss dialog: bang'); + }); + }); }); diff --git a/tests/integration/dialog.integration.test.ts b/tests/integration/dialog.integration.test.ts new file mode 100644 index 00000000..15de435a --- /dev/null +++ b/tests/integration/dialog.integration.test.ts @@ -0,0 +1,111 @@ +/** + * Integration tests for accept_dialog and dismiss_dialog + * Tests with real Firefox browser in headless mode. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import type { McpToolResponse } from '@/types/common.js'; +import type { FirefoxClient } from '@/firefox/index.js'; +import { handleAcceptDialog, handleDismissDialog } from '@/tools/utilities.js'; +import { createTestFirefox, closeFirefox, waitFor } from '../helpers/firefox.js'; + +// The dialog tools resolve the browser through the server's global getFirefox(), +// so it is redirected to the Firefox instance launched by this test. +const testState = vi.hoisted(() => ({ firefox: null as FirefoxClient | null })); + +vi.mock('../../src/index.js', () => ({ + getFirefox: async () => testState.firefox, +})); + +const PAGE = 'data:text/html,

dialogs

'; + +function responseText(response: McpToolResponse): string { + return response.content.map((item) => (item.text as string) ?? '').join('\n'); +} + +describe('Dialog Integration Tests', () => { + let firefox: FirefoxClient; + + beforeAll(async () => { + firefox = await createTestFirefox(); + testState.firefox = firefox; + }); + + afterAll(async () => { + testState.firefox = null; + await closeFirefox(firefox); + }); + + async function evaluateString(expression: string): Promise { + return String(await firefox.evaluate(expression)); + } + + /** + * Opens a dialog from a timer so the evaluation returns, then checks that + * the page is still blocked in the dialog after the browser had time to + * close it on its own. + */ + async function openDialog(call: string): Promise { + await firefox.navigate(PAGE); + await firefox.evaluate( + `window.result = 'pending'; setTimeout(() => { window.result = ${call}; }, 50); 1` + ); + await new Promise((resolve) => setTimeout(resolve, 500)); + expect(await evaluateString('String(window.result)')).toBe('pending'); + } + + async function resultOnceSettled(): Promise { + await waitFor(async () => (await evaluateString('String(window.result)')) !== 'pending', 5000); + return evaluateString('JSON.stringify(window.result)'); + } + + it('should leave a confirm open until it is accepted', async () => { + await openDialog("confirm('Proceed?')"); + + const response = await handleAcceptDialog({}); + + expect(response.isError).toBeUndefined(); + expect(responseText(response)).toBe('Accepted'); + expect(await resultOnceSettled()).toBe('true'); + }, 30000); + + it('should leave a confirm open until it is dismissed', async () => { + await openDialog("confirm('Proceed?')"); + + const response = await handleDismissDialog({}); + + expect(response.isError).toBeUndefined(); + expect(responseText(response)).toBe('Dismissed'); + expect(await resultOnceSettled()).toBe('false'); + }, 30000); + + it('should return the text given to accept_dialog from a prompt', async () => { + await openDialog("prompt('Name?')"); + + const response = await handleAcceptDialog({ promptText: 'Ada' }); + + expect(response.isError).toBeUndefined(); + expect(await resultOnceSettled()).toBe('"Ada"'); + }, 30000); + + it('should return null from a prompt that is dismissed', async () => { + await openDialog("prompt('Name?')"); + + const response = await handleDismissDialog({}); + + expect(response.isError).toBeUndefined(); + expect(await resultOnceSettled()).toBe('null'); + }, 30000); + + it('should report no active dialog when none is open', async () => { + await firefox.navigate(PAGE); + + const accepted = await handleAcceptDialog({}); + const dismissed = await handleDismissDialog({}); + + expect(accepted.isError).toBe(true); + expect(responseText(accepted)).toBe('Error: No active dialog'); + expect(dismissed.isError).toBe(true); + expect(responseText(dismissed)).toBe('Error: No active dialog'); + }, 30000); +}); diff --git a/tests/tools/utilities.test.ts b/tests/tools/utilities.test.ts index 8f3235b8..f2b993ab 100644 --- a/tests/tools/utilities.test.ts +++ b/tests/tools/utilities.test.ts @@ -2,9 +2,11 @@ * Unit tests for utilities tools */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { acceptDialogTool, + handleAcceptDialog, + handleDismissDialog, dismissDialogTool, navigateHistoryTool, setViewportSizeTool, @@ -59,4 +61,55 @@ describe('Utilities Tools', () => { expect(required).toContain('height'); }); }); + + describe('Dialog handlers', () => { + const acceptDialog = vi.fn(); + const dismissDialog = vi.fn(); + + function errorText(result: { content: Array<{ type: string; text?: string }> }): string { + return result.content[0]?.text ?? ''; + } + + beforeEach(() => { + acceptDialog.mockReset(); + dismissDialog.mockReset(); + vi.doMock('../../src/index.js', () => ({ + getFirefox: vi.fn().mockResolvedValue({ acceptDialog, dismissDialog }), + })); + }); + + it('accept_dialog reports no active dialog for NoSuchAlertError with an empty message', async () => { + const error = new Error(''); + error.name = 'NoSuchAlertError'; + acceptDialog.mockRejectedValue(error); + + const result = await handleAcceptDialog({}); + + expect(result.isError).toBe(true); + expect(errorText(result)).toBe('Error: No active dialog'); + }); + + it('dismiss_dialog reports no active dialog for NoSuchAlertError with an empty message', async () => { + const error = new Error(''); + error.name = 'NoSuchAlertError'; + dismissDialog.mockRejectedValue(error); + + const result = await handleDismissDialog({}); + + expect(result.isError).toBe(true); + expect(errorText(result)).toBe('Error: No active dialog'); + }); + + it('does not treat other errors as a missing dialog', async () => { + acceptDialog.mockRejectedValue(new Error('Failed to accept dialog: no such alert')); + dismissDialog.mockRejectedValue(new Error('Failed to dismiss dialog: boom')); + + expect(errorText(await handleAcceptDialog({}))).toBe( + 'Error: Failed to accept dialog: no such alert' + ); + expect(errorText(await handleDismissDialog({}))).toBe( + 'Error: Failed to dismiss dialog: boom' + ); + }); + }); });