From 71901d027a08927761e2034e1d6b3a7a6e9cf5bc Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 14 Jun 2026 21:33:57 -0700 Subject: [PATCH] fix(mcp): only wait for next pause in browser_resume when step or location is set browser_resume unconditionally awaited a pausedPromise that only resolved when the debugger paused again. For a plain resume (no step or location), if execution completed without hitting another breakpoint, the promise never resolved and the tool hung indefinitely. Only create and await the pausedPromise when params.step or params.location is set, since those are the only cases where waiting for re-pause is expected. Introduced in #39717 (2026-03-17). --- .../src/tools/backend/devtools.ts | 25 +++++++++++-------- tests/mcp/devtools.spec.ts | 21 ++++++++++++++++ 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/packages/playwright-core/src/tools/backend/devtools.ts b/packages/playwright-core/src/tools/backend/devtools.ts index 300793b84d903..379b756cc7b80 100644 --- a/packages/playwright-core/src/tools/backend/devtools.ts +++ b/packages/playwright-core/src/tools/backend/devtools.ts @@ -40,15 +40,19 @@ const resume = defineTool({ handle: async (context, params, response) => { const browserContext = await context.ensureBrowserContext(); - const pausedPromise = new Promise(resolve => { - const listener = () => { - if (browserContext.debugger.pausedDetails()) { - browserContext.debugger.off('pausedstatechanged', listener); - resolve(); - } - }; - browserContext.debugger.on('pausedstatechanged', listener); - }); + const waitForPause = params.step || params.location; + let pausedPromise: Promise | undefined; + if (waitForPause) { + pausedPromise = new Promise(resolve => { + const listener = () => { + if (browserContext.debugger.pausedDetails()) { + browserContext.debugger.off('pausedstatechanged', listener); + resolve(); + } + }; + browserContext.debugger.on('pausedstatechanged', listener); + }); + } if (params.location) { const [file, lineStr] = params.location.split(':'); @@ -67,7 +71,8 @@ const resume = defineTool({ } else { await browserContext.debugger.resume(); } - await pausedPromise; + if (pausedPromise) + await pausedPromise; }, }); diff --git a/tests/mcp/devtools.spec.ts b/tests/mcp/devtools.spec.ts index dc665f6d518fc..9b57f15ba7d39 100644 --- a/tests/mcp/devtools.spec.ts +++ b/tests/mcp/devtools.spec.ts @@ -93,6 +93,27 @@ test('browser_hide_highlight', async ({ boundBrowser, startClient }) => { await expect(page.locator('x-pw-highlight')).toHaveCount(0); }); +test('browser_resume completes without next breakpoint', async ({ boundBrowser, startClient }) => { + const page = await boundBrowser.newPage(); + await page.setContent(``); + const context = boundBrowser.contexts()[0]; + + // Pause the debugger before the next action. + await context.debugger.requestPause(); + const clickPromise = page.click('button'); + await new Promise(resolve => context.debugger.once('pausedstatechanged', resolve)); + expect(context.debugger.pausedDetails()).toBeTruthy(); + + // browser_resume without step or location should return without + // waiting for a subsequent pause. Before the fix, this would hang + // indefinitely because no subsequent breakpoint exists. + const { client } = await startClient({ args: [`--endpoint=default`] }); + await client.callTool({ name: 'browser_resume' }); + + await clickPromise; + expect(context.debugger.pausedDetails()).toBeNull(); +}); + test('browser_hide_highlight all', async ({ boundBrowser, startClient }) => { const page = await boundBrowser.newPage(); await page.setContent(`Go`);