fix(mcp): tolerate dialogs handled outside tools#41853
Open
pravinireri wants to merge 1 commit into
Open
Conversation
Only one native dialog can be open per page, so a new dialog opening clears previously tracked dialog states. browser_handle_dialog treats known "dialog is gone" errors as a stale dialog that was already handled outside of the session, instead of failing with a protocol error. The last user-handled dialog still requires one browser_handle_dialog call to clear, since the protocol has no dialog-closed event. References: microsoft#41837
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Playwright MCP/tools dialog tracking so that dialogs that are accepted/dismissed outside the tools session (e.g., manually in headed mode or via a second attached client) don’t leave behind stale “modal state” that breaks later tool calls like browser_snapshot.
Changes:
- Clear any previously tracked dialog modal state when a new native dialog is shown (since only one native dialog can be open at a time).
- Make
browser_handle_dialogtolerate “stale dialog” errors and report the dialog as already handled instead of failing. - Add an MCP regression test that simulates dialogs being dismissed externally via a second client.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/mcp/dialogs.spec.ts | Adds coverage for dialogs dismissed outside the MCP/tools session, ensuring only the last dialog can remain stale and that handling it doesn’t fail. |
| packages/playwright-core/src/tools/backend/tab.ts | Clears previously tracked dialog modal states when a new dialog opens to avoid accumulating phantom modal state. |
| packages/playwright-core/src/tools/backend/dialogs.ts | Treats specific “already handled/no dialog” errors as non-fatal when handling a dialog, returning a user-facing result instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improves handling for dialogs that were accepted or dismissed outside of the MCP/tools session, such as manually by a user in headed mode.
Why
The tools backend tracks dialog modal states when a
dialogevent is observed, but there is no tools-level close notification when that dialog is handled outside ofbrowser_handle_dialog.This can leave stale modal state behind and cause later tool calls to report a phantom active dialog.
Change
browser_handle_dialogtolerate known stale-dialog errors, reporting that the dialog was already handled instead of failing with a protocol error.This does not fully auto-clear the final manually handled dialog because there is no cross-browser dialog-closed signal available at this layer.
Tests
npm run ctest-mcp dialogsReferences #41837.