fix: add proactive tool rejection when dialog is open

Author: samiyac

src/tools/emulation.ts

@@ -68,6 +68,12 @@ export const emulate = definePageTool({
   },
   handler: async (request, _response, context) => {
     const page = request.page;
+    const dialog = page.getDialog();
+    if (dialog) {
+      throw new Error(
+        `A dialog is open (${dialog.type()}: ${dialog.message()}).`,
+      );
+    }
     await context.emulate(request.params, page.pptrPage);
   },
 });

src/tools/memory.ts

@@ -25,6 +25,12 @@ export const takeMemorySnapshot = definePageTool({
   handler: async (request, response, context) => {
     const page = request.page;
     context.validatePath(request.params.filePath);
+    const dialog = request.page.getDialog();
+    if (dialog) {
+      throw new Error(
+        `A dialog is open (${dialog.type()}: ${dialog.message()}).`,
+      );
+    }
 
     await page.pptrPage.captureHeapSnapshot({
       path: ensureExtension(request.params.filePath, '.heapsnapshot'),

src/tools/screenshot.ts

@@ -52,6 +52,12 @@ export const screenshot = definePageTool({
   },
   handler: async (request, response, context) => {
     context.validatePath(request.params.filePath);
+    const dialog = request.page.getDialog();
+    if (dialog) {
+      throw new Error(
+        `A dialog is open (${dialog.type()}: ${dialog.message()}).`,
+      );
+    }
     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,12 @@ in the DevTools Elements panel (if any).`,
   },
   handler: async (request, response, context) => {
     context.validatePath(request.params.filePath);
+    const dialog = request.page.getDialog();
+    if (dialog) {
+      throw new Error(
+        `A dialog is open (${dialog.type()}: ${dialog.message()}).`,
+      );
+    }
     response.includeSnapshot({
       verbose: request.params.verbose ?? false,
       filePath: request.params.filePath,
@@ -60,6 +66,12 @@ export const waitFor = definePageTool({
   },
   handler: async (request, response, context) => {
     const page = request.page;
+    const dialog = page.getDialog();
+    if (dialog) {
+      throw new Error(
+        `A dialog is open (${dialog.type()}: ${dialog.message()}).`,
+      );
+    }
     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', () => {

tests/tools/screenshot.test.js.snapshot

@@ -0,0 +1,3 @@
+exports[`screenshot > browser_take_screenshot > 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/screenshot.test.ts

@@ -10,6 +10,8 @@ import {tmpdir} from 'node:os';
 import {join} from 'node:path';
 import {describe, it} from 'node:test';
 
+import type {Dialog} from 'puppeteer-core';
+
 import {TextSnapshot} from '../../src/TextSnapshot.js';
 import {screenshot} from '../../src/tools/screenshot.js';
 import {screenshots} from '../snapshot.js';
@@ -297,5 +299,36 @@ describe('screenshot', () => {
         );
       });
     });
+
+    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(
+          screenshot.handler(
+            {
+              params: {format: 'png'},
+              page: context.getSelectedMcpPage(),
+            },
+            response,
+            context,
+          ),
+        );
+        const result = await response.handle('take_screenshot', context);
+
+        t.assert.snapshot?.(JSON.stringify(result));
+        await dialog.dismiss();
+      });
+    });
   });
 });