Skip to content

fix: actually terminate remote browser session in browser_close for CDP-connected browsers#56

Open
jonnyparris wants to merge 2 commits into
cloudflare:mainfrom
jonnyparris:close-browser-cdp-fix
Open

fix: actually terminate remote browser session in browser_close for CDP-connected browsers#56
jonnyparris wants to merge 2 commits into
cloudflare:mainfrom
jonnyparris:close-browser-cdp-fix

Conversation

@jonnyparris
Copy link
Copy Markdown
Member

@jonnyparris jonnyparris commented Apr 21, 2026

Summary

When this MCP is connected to a remote Chromium via CDP (via the cdpEndpoint config), calling browser_close doesn't actually close the remote browser — it just disconnects the MCP-side WebSocket. The remote browser keeps running until the underlying service's keep-alive timeout expires.

Playwright's own docs confirm this:

In case this browser is connected to, [browser.close()] clears all created contexts belonging to this browser and disconnects from the browser server.

So browser_close is effectively a no-op against the remote in CDP mode. Today it's also described as "Close the page" which further undersells what the tool is supposed to do.

Fix

Override the close path in CdpContextFactory so it:

  1. Opens a browser-level CDP session via browser.newBrowserCDPSession()
  2. Sends the Browser.close CDP command, which terminates the remote Chromium instance
  3. Falls through to the existing browserContext.close() + browser.close() disconnect

Only CdpContextFactory is affected — IsolatedContextFactory, PersistentContextFactory, RemoteContextFactory, and BrowserServerContextFactory keep their existing behaviour because their browser.close() semantics are already correct for their respective transports.

Also updated the browser_close tool description from "Close the page" to describe what the tool actually does.

Tests

Added a test in tests/cdp.spec.ts that:

  1. Starts a CDP server
  2. Connects the MCP via --cdp-endpoint
  3. Calls browser_close
  4. Asserts the remote browser fires its disconnected event within 5s (i.e. the process is actually gone, not just our WS dropped)

Passes on chrome and chromium. Skipped on firefox/webkit (CDP is Chromium-only). All existing CDP and core tests continue to pass.

Note

This is a standalone bug fix — the tool's observable behaviour should match its name and description. Whether the remote endpoint honours the CDP Browser.close command is an orthogonal concern of whatever serves the CDP endpoint.

…ted browsers

When connected to a remote Chromium via CDP, Playwright's browser.close()
only disconnects the local WebSocket — the remote browser keeps running
until the remote service's keep-alive timeout expires.

Override the close path in CdpContextFactory to first send the CDP
Browser.close command (which terminates the remote instance) before
disconnecting. Also update the browser_close tool description from
'Close the page' to reflect what it actually does.
@jonnyparris jonnyparris force-pushed the close-browser-cdp-fix branch from cfa3212 to 90d2217 Compare April 21, 2026 19:47
@jonnyparris jonnyparris marked this pull request as ready for review April 22, 2026 07:37
Copy link
Copy Markdown
Collaborator

@ruifigueira ruifigueira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment, I don't think we should merge this as is. Maybe add a different tool to actually close the browser?

…lose

Address review feedback: keep browser_close behaviour unchanged
(matches upstream Microsoft semantics — 'close the page', tear down the
MCP context) and add a separate browser_terminate tool that explicitly
sends CDP Browser.close to release remote CDP-backed sessions (e.g.
Cloudflare Browser Rendering) instead of waiting for the keep-alive to
expire.

The CDP Browser.close path now lives on an optional terminate() hook
exposed by CreateContextResult. Only CdpContextFactory implements it;
for every other transport browser_terminate falls back to the normal
close path, so there is no behaviour change for persistent, isolated,
remote, or browser-server modes.
@jonnyparris
Copy link
Copy Markdown
Member Author

Good point — revised with option A.

browser_close is back to its pre-PR behaviour ("close the page", tears down the MCP context via context.close()). Added a separate browser_terminate tool that explicitly sends CDP Browser.close to release remote CDP-backed sessions (Cloudflare Browser Rendering, Browserless, any self-hosted CDP target) instead of waiting for the keep-alive to expire.

Implementation:

  • CreateContextResult gains an optional terminate() hook
  • Only CdpContextFactory implements it; every other transport (persistent, isolated, remote, browser-server) falls back to the normal close path, so there's no behaviour change outside CDP mode
  • Context gets a matching public terminate() method used by the new tool

Test in tests/cdp.spec.ts renamed to cover browser_terminate and still asserts the remote disconnected event fires within 5s. Full test suite (core, capabilities, cdp) passes on chrome.

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