Skip to content

fix(mcp): refuse browser_close when the browser context is shared - #42495

Merged
Yury Semikhatsky (yury-s) merged 3 commits into
microsoft:mainfrom
yury-s:fix-42363
Sep 1, 2026
Merged

fix(mcp): refuse browser_close when the browser context is shared#42495
Yury Semikhatsky (yury-s) merged 3 commits into
microsoft:mainfrom
yury-s:fix-42363

Conversation

@yury-s

@yury-s Yury Semikhatsky (yury-s) commented Sep 1, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #42363

browser_close disposes the backend, but with a shared browser no
'disconnected' event fires, so the server kept handing out the disposed
backend. Let the server ask the backend whether it disposed itself after
each tool call and drop the cached one if so. Also detach the backend
listeners from the browser context on dispose so they do not accumulate
on a long-lived shared context.

This replaces the approach from microsoft#42365 (reverted in microsoft#42492), which made
dispose() emit the 'disconnected' event on explicit disposal.

Fixes: microsoft#42363
With --shared-browser-context the context belongs to all connected
clients, so one client closing it only pretend-closed: the backend was
disposed while every page stayed open and the next call silently rebuilt
it. Return an error to browser_close instead.
With browser_close refused on a shared context, every remaining close
path closes the context or the browser, so the existing 'disconnected'
event already clears the cached backend.
@yury-s Yury Semikhatsky (yury-s) changed the title fix(mcp): do not reuse the cached backend after it is disposed fix(mcp): refuse browser_close when the browser context is shared Sep 1, 2026
@github-actions

This comment has been minimized.

@yury-s
Yury Semikhatsky (yury-s) merged commit 9e3157b into microsoft:main Sep 1, 2026
15 of 17 checks passed
@yury-s
Yury Semikhatsky (yury-s) deleted the fix-42363 branch September 1, 2026 21:58
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [chrome] › mcp/annotate.spec.ts:349 › should annotate when context has no fixed viewport @mcp-windows-latest-chrome
❌ [chromium] › mcp/annotate.spec.ts:269 › should start dashboard and annotate when no dashboard is running @mcp-windows-latest-chromium

8273 passed, 1371 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🟢 CI is clear — both failures are pre-existing flakes

Hi, I'm the Playwright bot and I took a first look at the failing CI on this PR.

Both failures are in mcp/annotate.spec.ts, which this PR doesn't touch. The diff only changes browser_close for shared browser contexts (tools/backend/common.ts, tools/backend/context.ts) plus an http.spec.ts test — nothing the annotate flow exercises. Both tests have a long history of flaking on unrelated SHAs and push commits, so this run is just the same noise, not something you broke.

Details

Overall: no failure reaches the PR's change. Both are known low-rate flakes.

Pre-existing flake / infra

Neither test touches browser_close or shared-context handling, so this PR can't be the cause.

Triaged by the Playwright bot - agent run

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.

[MCP]: browser_close caches a disposed backend with shared HTTP clients

2 participants