fix: add proactive tool rejection when dialog is open

Author: samiyac

src/McpPage.ts

@@ -81,6 +81,14 @@ export class McpPage implements ContextPage {
     this.#dialog = undefined;
   }
 
+  throwIfDialogOpen(): void {
+    if (this.#dialog) {
+      throw new Error(
+        `A dialog is open (${this.#dialog.type()}: ${this.#dialog.message()}).`,
+      );
+    }
+  }
+
   getInPageTools(): ToolGroup<ToolDefinition> | undefined {
     return this.inPageTools;
   }

src/tools/ToolDefinition.ts

@@ -255,6 +255,7 @@ export type ContextPage = Readonly<{
 
   getDialog(): Dialog | undefined;
   clearDialog(): void;
+  throwIfDialogOpen(): void;
   waitForEventsAfterAction(
     action: () => Promise<unknown>,
     options?: {timeout?: number; handleDialog?: 'accept' | 'dismiss' | string},

src/tools/emulation.ts

@@ -68,6 +68,7 @@ export const emulate = definePageTool({
   },
   handler: async (request, _response, context) => {
     const page = request.page;
+    page.throwIfDialogOpen();
     await context.emulate(request.params, page.pptrPage);
   },
 });

src/tools/memory.ts

@@ -25,6 +25,7 @@ export const takeMemorySnapshot = definePageTool({
   handler: async (request, response, context) => {
     const page = request.page;
     context.validatePath(request.params.filePath);
+    page.throwIfDialogOpen();
 
     await page.pptrPage.captureHeapSnapshot({
       path: ensureExtension(request.params.filePath, '.heapsnapshot'),

src/tools/screenshot.ts

@@ -52,6 +52,7 @@ export const screenshot = definePageTool({
   },
   handler: async (request, response, context) => {
     context.validatePath(request.params.filePath);
+    request.page.throwIfDialogOpen();
     if (request.params.uid && request.params.fullPage) {
       throw new Error('Providing both "uid" and "fullPage" is not allowed.');
     }

src/tools/snapshot.ts

@@ -35,6 +35,7 @@ in the DevTools Elements panel (if any).`,
   },
   handler: async (request, response, context) => {
     context.validatePath(request.params.filePath);
+    request.page.throwIfDialogOpen();
     response.includeSnapshot({
       verbose: request.params.verbose ?? false,
       filePath: request.params.filePath,
@@ -60,6 +61,7 @@ export const waitFor = definePageTool({
   },
   handler: async (request, response, context) => {
     const page = request.page;
+    page.throwIfDialogOpen();
     await context.waitForTextOnPage(
       request.params.text,
       request.params.timeout,

tests/tools/emulation.test.js.snapshot

@@ -0,0 +1,3 @@
+exports[`emulation > when dialog is open 1`] = `
+{"content":[{"type":"text","text":"# Open dialog\\nalert: test dialog.\\nCall handle_dialog to handle it before continuing."}],"structuredContent":{"dialog":{"type":"alert","message":"test dialog","defaultValue":""}}}
+`;

tests/tools/emulation.test.ts

@@ -7,6 +7,8 @@
 import assert from 'node:assert';
 import {beforeEach, describe, it} from 'node:test';
 
+import type {Dialog} from 'puppeteer-core';
+
 import {emulate} from '../../src/tools/emulation.js';
 import {
   geolocationTransform,
@@ -680,4 +682,35 @@ describe('emulation', () => {
       });
     });
   });
+
+  it('when dialog is open', async t => {
+    await withMcpContext(async (response, context) => {
+      const page = context.getSelectedPptrPage();
+      await page.setContent('<h1>Test</h1>');
+      const dialogPromise = new Promise<Dialog>(resolve => {
+        page.on('dialog', dialog => resolve(dialog));
+      });
+      page.evaluate(() => {
+        alert('test dialog');
+      });
+      const dialog = await dialogPromise;
+
+      await assert.rejects(
+        emulate.handler(
+          {
+            params: {
+              userAgent: 'Test Agent',
+            },
+            page: context.getSelectedMcpPage(),
+          },
+          response,
+          context,
+        ),
+      );
+      const result = await response.handle('emulate', context);
+
+      t.assert.snapshot?.(JSON.stringify(result));
+      await dialog.dismiss();
+    });
+  });
 });

tests/tools/memory.test.js.snapshot

@@ -198,3 +198,7 @@ Static Data: {
   "maxJSObjectId": 54005
 }
 `;
+
+exports[`memory > take_memory_snapshot > when dialog is open 1`] = `
+{"content":[{"type":"text","text":"# Open dialog\\nalert: test dialog.\\nCall handle_dialog to handle it before continuing."}],"structuredContent":{"dialog":{"type":"alert","message":"test dialog","defaultValue":""}}}
+`;

tests/tools/memory.test.ts

@@ -11,6 +11,8 @@ import {tmpdir} from 'node:os';
 import {join} from 'node:path';
 import {describe, it} from 'node:test';
 
+import type {Dialog} from 'puppeteer-core';
+
 import {
   takeMemorySnapshot,
   exploreMemorySnapshot,
@@ -40,6 +42,34 @@ describe('memory', () => {
         }
       });
     });
+
+    it('when dialog is open', async t => {
+      await withMcpContext(async (response, context) => {
+        const page = context.getSelectedPptrPage();
+        await page.setContent('<h1>Test</h1>');
+        const dialogPromise = new Promise<Dialog>(resolve => {
+          page.on('dialog', dialog => resolve(dialog));
+        });
+        page.evaluate(() => {
+          alert('test dialog');
+        });
+        const dialog = await dialogPromise;
+        const filePath = join(tmpdir(), 'test-dialog.heapsnapshot');
+
+        await assert.rejects(
+          takeMemorySnapshot.handler(
+            {params: {filePath}, page: context.getSelectedMcpPage()},
+            response,
+            context,
+          ),
+        );
+        await rm(filePath, {force: true});
+        const result = await response.handle('take_memory_snapshot', context);
+
+        t.assert.snapshot?.(JSON.stringify(result));
+        await dialog.dismiss();
+      });
+    });
   });
 
   describe('load_memory_snapshot', () => {