chore: resolve dialog races

pavelfeldman · pavelfeldman · commit 42fea4250d22 · 2025-07-15T17:57:01.000-07:00
diff --git a/src/context.ts b/src/context.ts
@@ -22,7 +22,6 @@ import { ManualPromise } from './manualPromise.js';
 import { Tab } from './tab.js';
 import { outputFile } from './config.js';
 
-import type { ImageContent, TextContent } from '@modelcontextprotocol/sdk/types.js';
 import type { ModalState, Tool, ToolActionResult } from './tools/tool.js';
 import type { FullConfig } from './config.js';
 import type { BrowserContextFactory } from './browserContextFactory.js';
@@ -136,7 +135,6 @@ export class Context {
     // Tab management is done outside of the action() call.
     const toolResult = await tool.handle(this, tool.schema.inputSchema.parse(params || {}));
     const { code, action, waitForNetwork, captureSnapshot, resultOverride } = toolResult;
-    const racingAction = action ? () => this._raceAgainstModalDialogs(action) : undefined;
 
     if (resultOverride)
       return resultOverride;
@@ -152,16 +150,17 @@ export class Context {
 
     const tab = this.currentTabOrDie();
     // TODO: race against modal dialogs to resolve clicks.
-    let actionResult: { content?: (ImageContent | TextContent)[] } | undefined;
-    try {
-      if (waitForNetwork)
-        actionResult = await waitForCompletion(this, tab, async () => racingAction?.()) ?? undefined;
-      else
-        actionResult = await racingAction?.() ?? undefined;
-    } finally {
-      if (captureSnapshot && !this._javaScriptBlocked())
-        await tab.captureSnapshot();
-    }
+    const actionResult = await this._raceAgainstModalDialogs(async () => {
+      try {
+        if (waitForNetwork)
+          return await waitForCompletion(this, tab, async () => action?.()) ?? undefined;
+        else
+          return await action?.() ?? undefined;
+      } finally {
+        if (captureSnapshot && !this._javaScriptBlocked())
+          await tab.captureSnapshot();
+      }
+    });
 
     const result: string[] = [];
     result.push(`- Ran Playwright code:
diff --git a/tests/dialogs.spec.ts b/tests/dialogs.spec.ts
@@ -16,9 +16,6 @@
 
 import { test, expect } from './fixtures.js';
 
-// https://github.com/microsoft/playwright/issues/35663
-test.skip(({ mcpBrowser, mcpHeadless }) => mcpBrowser === 'webkit' && mcpHeadless);
-
 test('alert dialog', async ({ client, server }) => {
   server.setContent('/', `<button onclick="alert('Alert')">Button</button>`, 'text/html');
   expect(await client.callTool({
@@ -64,8 +61,6 @@ await page.getByRole('button', { name: 'Button' }).click();
 });
 
 test('two alert dialogs', async ({ client, server }) => {
-  test.fixme(true, 'Race between the dialog and ariaSnapshot');
-
   server.setContent('/', `
     <title>Title</title>
     <body>
@@ -100,7 +95,18 @@ await page.getByRole('button', { name: 'Button' }).click();
     },
   });
 
-  expect(result).not.toContainTextContent('### Modal state');
+  expect(result).toContainTextContent(`
+### Modal state
+- ["alert" dialog with message "Alert 2"]: can be handled by the "browser_handle_dialog" tool`);
+
+  const result2 = await client.callTool({
+    name: 'browser_handle_dialog',
+    arguments: {
+      accept: true,
+    },
+  });
+
+  expect(result2).not.toContainTextContent('### Modal state');
 });
 
 test('confirm dialog (true)', async ({ client, server }) => {
@@ -210,3 +216,47 @@ test('prompt dialog', async ({ client, server }) => {
 - generic [active] [ref=e1]: Answer
 \`\`\``);
 });
+
+test('alert dialog w/ race', async ({ client, server }) => {
+  server.setContent('/', `<button onclick="setTimeout(() => alert('Alert'), 100)">Button</button>`, 'text/html');
+  expect(await client.callTool({
+    name: 'browser_navigate',
+    arguments: { url: server.PREFIX },
+  })).toContainTextContent('- button "Button" [ref=e2]');
+
+  expect(await client.callTool({
+    name: 'browser_click',
+    arguments: {
+      element: 'Button',
+      ref: 'e2',
+    },
+  })).toHaveTextContent(`- Ran Playwright code:
+\`\`\`js
+// Click Button
+await page.getByRole('button', { name: 'Button' }).click();
+\`\`\`
+
+### Modal state
+- ["alert" dialog with message "Alert"]: can be handled by the "browser_handle_dialog" tool`);
+
+  const result = await client.callTool({
+    name: 'browser_handle_dialog',
+    arguments: {
+      accept: true,
+    },
+  });
+
+  expect(result).not.toContainTextContent('### Modal state');
+  expect(result).toHaveTextContent(`- Ran Playwright code:
+\`\`\`js
+// <internal code to handle "alert" dialog>
+\`\`\`
+
+- Page URL: ${server.PREFIX}
+- Page Title: 
+- Page Snapshot
+\`\`\`yaml
+- button "Button" [active] [ref=e2]
+\`\`\`
+`);
+});
