Bug 2081600 - Return early when a dialog is opened during evaluate_script or click_by_uid

src/firefox/core.ts

@@ -3,6 +3,7 @@
  */
 
 import { Builder, Browser, Capabilities, WebDriver } from 'selenium-webdriver';
+import { UserPromptHandler } from 'selenium-webdriver/lib/capabilities.js';
 import firefox from 'selenium-webdriver/firefox.js';
 import {
   mkdirSync,
@@ -290,6 +291,7 @@ export class FirefoxCore {
       if (this.options.acceptInsecureCerts) {
         caps.set('acceptInsecureCerts', true);
       }
+      caps.setAlertBehavior(UserPromptHandler.IGNORE);
 
       const serviceBuilder = new firefox.ServiceBuilder(geckodriverPath);
       this.driver = firefox.Driver.createSession(caps, serviceBuilder.build());
@@ -364,6 +366,7 @@ export class FirefoxCore {
       // session in a locked state that blocks reconnection.
       const caps = new Capabilities();
       caps.set('webSocketUrl', true);
+      caps.setAlertBehavior(UserPromptHandler.IGNORE);
 
       // createSession() returns synchronously; the session is established async under the hood.
       // Passing geckodriverPath to ServiceBuilder prevents getBinaryPaths() from running,
@@ -409,6 +412,7 @@ export class FirefoxCore {
       // Standard path: launch a new Firefox via selenium-webdriver
       const firefoxOptions = new firefox.Options();
       firefoxOptions.enableBidi();
+      firefoxOptions.setAlertBehavior(UserPromptHandler.IGNORE);
 
       if (this.options.headless) {
         firefoxOptions.addArguments('-headless');

src/firefox/events/dialogs.ts

@@ -0,0 +1,72 @@
+/**
+ * Dialog events.
+ */
+
+import type { BrowsingContext } from 'webdriver-bidi-protocol';
+import type { BiDiFacade } from '../bidi.js';
+import { logDebug } from '../../utils/logger.js';
+
+export class DialogEvents {
+  private dialogs = new Map<
+    BrowsingContext.BrowsingContext,
+    BrowsingContext.UserPromptOpenedParameters
+  >();
+  private subscribed = false;
+
+  constructor(private bidi: BiDiFacade) {}
+
+  /**
+   * Subscribe to BiDi dialog events.
+   */
+  async subscribe(): Promise<void> {
+    if (this.subscribed) {
+      return;
+    }
+
+    await this.bidi.subscribe([
+      'browsingContext.userPromptOpened',
+      'browsingContext.userPromptClosed',
+    ]);
+
+    this.bidi.on('browsingContext.userPromptOpened', (dialog) => {
+      this.dialogs.set(dialog.context, dialog);
+    });
+
+    this.bidi.on('browsingContext.userPromptClosed', ({ context }) => {
+      this.dialogs.delete(context);
+    });
+
+    this.subscribed = true;
+    logDebug('Dialog listener ready');
+  }
+
+  getDialog(context: BrowsingContext.BrowsingContext) {
+    return this.dialogs.get(context);
+  }
+
+  async raceAgainstDialogOpened<T>(
+    fn: () => Promise<T>,
+    dialogOpenedResult: (dialog: BrowsingContext.UserPromptOpenedParameters) => T
+  ) {
+    let removeListener!: () => void;
+    const dialogOpened = new Promise<T>((resolve) => {
+      const listener = (dialog: BrowsingContext.UserPromptOpenedParameters) =>
+        resolve(dialogOpenedResult(dialog));
+      this.bidi.on('browsingContext.userPromptOpened', listener);
+      removeListener = () => this.bidi.off('browsingContext.userPromptOpened', listener);
+    });
+    try {
+      const promise = fn();
+      void dialogOpened.then(() =>
+        promise.catch((error) =>
+          logDebug(
+            `Tool call errored after its handler returned early due to a dialog opening: ${String(error)}`
+          )
+        )
+      );
+      return await Promise.race([promise, dialogOpened]);
+    } finally {
+      removeListener();
+    }
+  }
+}

src/firefox/events/index.ts

@@ -5,4 +5,5 @@
 export { ConsoleEvents, type ConsoleEventsOptions } from './console.js';
 export { NetworkEvents, type NetworkEventsOptions } from './network.js';
 export { DebuggingEvents } from './debugging.js';
+export { DialogEvents } from './dialogs.js';
 export { DownloadEvents } from './downloads.js';

src/firefox/index.ts

@@ -8,7 +8,13 @@ import { FirefoxCore } from './core.js';
 import { BiDiFacade, type FirefoxExtension } from './bidi.js';
 import { listExtensions, type ListExtensionsFilters } from './webextension.js';
 import { logDebug } from '../utils/logger.js';
-import { ConsoleEvents, NetworkEvents, DebuggingEvents, DownloadEvents } from './events/index.js';
+import {
+  ConsoleEvents,
+  NetworkEvents,
+  DebuggingEvents,
+  DownloadEvents,
+  DialogEvents,
+} from './events/index.js';
 import type { NetworkBodyResult } from './events/network.js';
 import { DomInteractions } from './dom.js';
 import { PageManagement } from './pages.js';
@@ -25,6 +31,7 @@ export class FirefoxClient {
   private consoleEvents: ConsoleEvents | null = null;
   private networkEvents: NetworkEvents | null = null;
   private debuggingEvents: DebuggingEvents | null = null;
+  private dialogEvents: DialogEvents | null = null;
   private downloadEvents: DownloadEvents | null = null;
   private dom: DomInteractions | null = null;
   private pages: PageManagement | null = null;
@@ -106,6 +113,14 @@ export class FirefoxClient {
       this.debuggingEvents = null;
     }
 
+    this.dialogEvents = new DialogEvents(this.bidi);
+    try {
+      await this.dialogEvents.subscribe();
+    } catch {
+      logDebug('Unable to subscribe to dialog events');
+      this.dialogEvents = null;
+    }
+
     this.downloadEvents = new DownloadEvents(this.bidi);
     try {
       await this.downloadEvents.subscribe();
@@ -239,6 +254,23 @@ export class FirefoxClient {
     return await this.pages.setViewportSize(width, height);
   }
 
+  async raceAgainstDialogOpened<T>(
+    fn: () => Promise<T>,
+    dialogOpenedResult: (dialog: BrowsingContext.UserPromptOpenedParameters) => T
+  ) {
+    if (!this.dialogEvents) {
+      throw new Error('Dialog events not available');
+    }
+    return await this.dialogEvents.raceAgainstDialogOpened(fn, dialogOpenedResult);
+  }
+
+  getCurrentDialog() {
+    if (!this.dialogEvents) {
+      throw new Error('Dialog events not available');
+    }
+    return this.dialogEvents.getDialog(this.getContext());
+  }
+
   async acceptDialog(promptText?: string): Promise<void> {
     if (!this.pages) {
       throw new Error('Not connected');

src/tools/input.ts

@@ -6,6 +6,7 @@
 import { successResponse } from '../utils/response-helpers.js';
 import { handleUidError } from '../utils/uid-helpers.js';
 import { defineModule, defineToolHandler, type ToolDefinition } from './module.js';
+import { dialogOpenedResponse } from './utilities.js';
 import type { McpToolResponse } from '../types/common.js';
 
 // Tool definitions
@@ -213,12 +214,14 @@ export const handleClickByUid = defineToolHandler<typeof clickByUidTool>(
     const { getFirefox } = await import('../index.js');
     const firefox = await getFirefox();
 
-    try {
-      await firefox.clickByUid(uid, dblClick);
-      return successResponse(`${dblClick ? 'dblclick' : 'click'} ${uid}`);
-    } catch (error) {
-      throw handleUidError(error as Error, uid);
-    }
+    return await firefox.raceAgainstDialogOpened(async () => {
+      try {
+        await firefox.clickByUid(uid, dblClick);
+        return successResponse(`${dblClick ? 'dblclick' : 'click'} ${uid}`);
+      } catch (error) {
+        throw handleUidError(error as Error, uid);
+      }
+    }, dialogOpenedResponse);
   }
 );
 

src/tools/script.ts

@@ -8,6 +8,7 @@ import { remoteValueToNative } from '../utils/remote-value.js';
 import { validateFunction } from '../utils/js-validation.js';
 import { saveOutput } from '../utils/save-output.js';
 import { defineModule, defineToolHandler, type ToolDefinition } from './module.js';
+import { dialogOpenedResponse } from './utilities.js';
 import type { McpToolResponse } from '../types/common.js';
 import { BiDiTimeoutError } from '../utils/errors.js';
 
@@ -76,98 +77,100 @@ export const handleEvaluateScript = defineToolHandler<typeof evaluateScriptTool>
     const { getFirefox } = await import('../index.js');
     const firefox = await getFirefox();
 
-    const scriptTimeout = timeout ?? DEFAULT_TIMEOUT;
+    return await firefox.raceAgainstDialogOpened(async () => {
+      const scriptTimeout = timeout ?? DEFAULT_TIMEOUT;
 
-    // Prepare arguments: resolve UIDs to references shared ids if provided
-    const resolvedArgs: Script.LocalValue[] = [];
-    if (fnArgs && fnArgs.length > 0) {
-      for (const arg of fnArgs) {
-        try {
-          resolvedArgs.push(await firefox.resolveUidToElement(arg.uid));
-        } catch (error) {
-          const errorMsg = (error as Error).message;
+      // Prepare arguments: resolve UIDs to references shared ids if provided
+      const resolvedArgs: Script.LocalValue[] = [];
+      if (fnArgs && fnArgs.length > 0) {
+        for (const arg of fnArgs) {
+          try {
+            resolvedArgs.push(await firefox.resolveUidToElement(arg.uid));
+          } catch (error) {
+            const errorMsg = (error as Error).message;
 
-          // Provide friendly error for stale UIDs
-          if (
-            errorMsg.includes('stale') ||
-            errorMsg.includes('Snapshot') ||
-            errorMsg.includes('UID')
-          ) {
-            throw new Error(
-              `UID "${arg.uid}" is invalid or from an old snapshot.\n\n` +
-                'The page may have changed since the snapshot was taken.\n' +
-                'Please call take_snapshot to get fresh UIDs and try again.'
-            );
-          }
+            // Provide friendly error for stale UIDs
+            if (
+              errorMsg.includes('stale') ||
+              errorMsg.includes('Snapshot') ||
+              errorMsg.includes('UID')
+            ) {
+              throw new Error(
+                `UID "${arg.uid}" is invalid or from an old snapshot.\n\n` +
+                  'The page may have changed since the snapshot was taken.\n' +
+                  'Please call take_snapshot to get fresh UIDs and try again.'
+              );
+            }
 
-          throw new Error(`Failed to resolve UID "${arg.uid}": ${errorMsg}`);
+            throw new Error(`Failed to resolve UID "${arg.uid}": ${errorMsg}`);
+          }
         }
       }
-    }
 
-    // Execute with resolved args (empty array if no args)
-    const context = firefox.getCurrentContextId();
-    if (!context) {
-      throw new Error('No active browsing context');
-    }
-    let result;
-    try {
-      result = await firefox.sendBiDiCommand(
-        'script.callFunction',
-        {
-          functionDeclaration: fnString,
-          awaitPromise: true,
-          arguments: resolvedArgs,
-          target: {
-            context,
-            ...(sandbox !== undefined && { sandbox }),
+      // Execute with resolved args (empty array if no args)
+      const context = firefox.getCurrentContextId();
+      if (!context) {
+        throw new Error('No active browsing context');
+      }
+      let result;
+      try {
+        result = await firefox.sendBiDiCommand(
+          'script.callFunction',
+          {
+            functionDeclaration: fnString,
+            awaitPromise: true,
+            arguments: resolvedArgs,
+            target: {
+              context,
+              ...(sandbox !== undefined && { sandbox }),
+            },
           },
-        },
-        { timeout: scriptTimeout }
-      );
-    } catch (error) {
-      if (error instanceof BiDiTimeoutError) {
-        return errorResponse(
-          new Error(
-            `Script execution timed out (exceeded ${scriptTimeout}ms).\n\n` +
-              'The function may contain an infinite loop or be waiting for a slow operation.\n' +
-              'Try simplifying the script or increasing the timeout parameter.'
-          )
+          { timeout: scriptTimeout }
         );
+      } catch (error) {
+        if (error instanceof BiDiTimeoutError) {
+          return errorResponse(
+            new Error(
+              `Script execution timed out (exceeded ${scriptTimeout}ms).\n\n` +
+                'The function may contain an infinite loop or be waiting for a slow operation.\n' +
+                'Try simplifying the script or increasing the timeout parameter.'
+            )
+          );
+        }
+        throw error;
       }
-      throw error;
-    }
 
-    if (result.type === 'success') {
-      // JSON.stringify returns undefined for an undefined script result
-      const json = JSON.stringify(remoteValueToNative(result.result), null, 2) ?? 'undefined';
+      if (result.type === 'success') {
+        // JSON.stringify returns undefined for an undefined script result
+        const json = JSON.stringify(remoteValueToNative(result.result), null, 2) ?? 'undefined';
 
-      if (saveTo) {
-        const saved = await saveOutput(
-          json,
-          saveTo === true ? undefined : saveTo,
-          'evaluate-script'
-        );
-        let output = `Script ran on page. Result saved to: ${saved.path} (${(saved.bytes / 1024).toFixed(1)}KB)`;
-        const excerpt = previewExcerpt(json, preview);
-        if (excerpt) {
-          output += '\nPreview:\n```json\n' + excerpt + '\n```';
+        if (saveTo) {
+          const saved = await saveOutput(
+            json,
+            saveTo === true ? undefined : saveTo,
+            'evaluate-script'
+          );
+          let output = `Script ran on page. Result saved to: ${saved.path} (${(saved.bytes / 1024).toFixed(1)}KB)`;
+          const excerpt = previewExcerpt(json, preview);
+          if (excerpt) {
+            output += '\nPreview:\n```json\n' + excerpt + '\n```';
+          }
+          return successResponse(output);
         }
-        return successResponse(output);
-      }
 
-      return successResponse('Script ran on page and returned:\n```json\n' + json + '\n```');
-    } else {
-      const exceptionDetails = result.exceptionDetails;
-      return errorResponse(
-        new Error(
-          `Script execution failed: ${exceptionDetails.text}\n\n` +
-            '```json\n' +
-            JSON.stringify(remoteValueToNative(exceptionDetails.exception), null, 2) +
-            '\n```'
-        )
-      );
-    }
+        return successResponse('Script ran on page and returned:\n```json\n' + json + '\n```');
+      } else {
+        const exceptionDetails = result.exceptionDetails;
+        return errorResponse(
+          new Error(
+            `Script execution failed: ${exceptionDetails.text}\n\n` +
+              '```json\n' +
+              JSON.stringify(remoteValueToNative(exceptionDetails.exception), null, 2) +
+              '\n```'
+          )
+        );
+      }
+    }, dialogOpenedResponse);
   }
 );