Skip to content

fix(mcp): identify downloads explicitly#41933

Merged
Skn0tt merged 2 commits into
mainfrom
skn0tt-fix-mcp-crash-recovery
Jul 23, 2026
Merged

fix(mcp): identify downloads explicitly#41933
Skn0tt merged 2 commits into
mainfrom
skn0tt-fix-mcp-crash-recovery

Conversation

@Skn0tt

@Skn0tt Skn0tt commented Jul 22, 2026

Copy link
Copy Markdown
Member

This PR stops treating every Chromium net::ERR_ABORTED navigation as a possible download. Since Chromium 140, released in September 2025, Page.navigate reports downloads explicitly, which Playwright surfaces as Download is starting.

Previously, MCP waited up to three seconds for a download event before returning net::ERR_ABORTED. This didn’t change the result, but happened to give the renderer’s crash event time to arrive. The error is now returned immediately. Crash recovery stays the same: once the event arrives, a subsequent MCP call resets the page to about:blank and reports Page crashed and was reset to about:blank.

The crash tests now poll for recovery instead of relying on the download wait, and explicitly await the crash event when asserting tab state.

Chromium reports both downloads and renderer crashes as ERR_ABORTED. Wait for either event so crash recovery cannot race the next tool call.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0d6d0323-d581-4df1-ae11-bf2d9302c1d2
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Hi, I'm the Playwright bot and I took a first look at the CI failures here.

🟢 The one failure is a pre-existing flake — this PR is clear

The single red test, [chrome] › mcp/cli-navigation.spec.ts:56 › goto chrome:// page that closes the tab does not crash the response, has been flaking on mcp-windows-latest-chrome for weeks, on SHAs this PR can't be responsible for. Nothing here looks caused by your change.

Details

Pre-existing flake / infra

  • [chrome] › mcp/cli-navigation.spec.ts:56 › goto chrome:// page that closes the tab does not crash the response @mcp-windows-latest-chrome — pre-existing flake. In the aggregated CI results this test fails 41 of 348 runs (~12%) on mcp-windows-latest-chrome, and never once on the other five MCP bots (0/356 chromium-ubuntu, 0/348 chromium-windows, 0/351 chromium-macos, 0/356 chrome-ubuntu, 0/351 chrome-macos). The failures span 24 unrelated PRs plus many push runs on main — e.g. PR test: fixme partitioned cookie API test on WebKit macOS < 26 #41691 (sha 22ff0ef), PR fix(mcp): await init page before screenshot #41646 (fa1c703), push 39a418a — all predating this branch, all with the identical signature: expect(received).toContain(expected) looking for the "No open tabs. Navigate to a URL to create one." substring. It's isolated to the real-Chrome channel on Windows, where the chrome://extensions/ page occasionally doesn't tear the tab down in time. Your run (#29913780666, PR fix(mcp): identify downloads explicitly #41933) is just one more hit of the same flake.

On the diff itself

Worth noting your change does touch this exact code path — Tab navigation now races a crash event against the download event on net::ERR_ABORTED. But the failure predates the PR with an unchanged rate and message, so it isn't a regression from your rewrite. If anything, waiting on the crash event is the kind of thing that could eventually reduce this flake; it doesn't introduce it.

I'm a first pass, so treat the green as "no PR-caused failures found," not a guarantee the branch is bug-free.

Triaged by the Playwright bot - agent run

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 296e429d-05c3-4f64-be3f-a199a1e00d65
@Skn0tt Skn0tt changed the title fix(mcp): wait for crash after aborted navigation fix(mcp): identify downloads explicitly Jul 22, 2026
@Skn0tt
Skn0tt marked this pull request as ready for review July 22, 2026 14:43
@Skn0tt
Skn0tt requested a review from dgozman July 22, 2026 14:45
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

7786 passed, 1262 skipped


Merge workflow run.

@Skn0tt
Skn0tt merged commit 15c4f55 into main Jul 23, 2026
18 checks passed
@Skn0tt
Skn0tt deleted the skn0tt-fix-mcp-crash-recovery branch July 23, 2026 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants