fix: exit on stdin EOF and SIGTERM/SIGINT/SIGHUP, closing the browser cleanly#2117
Merged
OrKoN merged 2 commits intoMay 26, 2026
Merged
Conversation
4b24907 to
05fd545
Compare
OrKoN
reviewed
May 26, 2026
OrKoN
reviewed
May 26, 2026
OrKoN
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, generally looks good to me!
… cleanly The MCP server entrypoint had no shutdown handler. After a session called navigate_page (or anything else that launched Chrome), the Chrome subprocess kept the Node event loop ref'd, so closing stdin -- the stdio MCP convention for 'I'm done' -- did not make the server exit. Clients that close stdin to terminate the server had to fall back to SIGTERM/ SIGKILL on every page-loaded session. This change: 1. Adds closeBrowser() in browser.ts that calls browser.close() for launched instances (reaps the Chrome subprocess) and browser.disconnect() for attached instances (leaves the user's Chrome alive). No-op if no browser is active or the connection has already been dropped. 2. Registers shutdown handlers in chrome-devtools-mcp-main.ts for stdin 'end'/'close' (stdio MCP transport convention) and SIGTERM/SIGINT/ SIGHUP (clients that signal instead of closing stdin; SIGHUP for parity with src/daemon/daemon.ts). The handler is idempotent and has an unref'd 10s timeout backstop in case Chrome teardown hangs.
Spawns the binary, runs the MCP initialize handshake, calls navigate_page to force a real Chrome launch (the bug from ChromeDevTools#2116 only reproduces with an active Chrome subprocess), then triggers each shutdown path and asserts the process exits within a 3s budget vs a 15s outer hang detector. Patching the SIGTERM/SIGHUP handlers out of the production code (equivalent to handling only stdin) makes those two tests time out at 15s; the SIGINT and stdin-EOF tests still pass — confirming the test catches the regression we'd actually care about. Also fixes a floating-promise lint introduced in 05fd545 (the new disconnect() path in closeBrowser was not awaited).
7497fad to
b2311a6
Compare
Collaborator
|
There is currently an outage with GitHub Actions https://www.githubstatus.com/incidents/gnftqj9htp0g I will trigger the test jobs once it is resolved. |
OrKoN
approved these changes
May 26, 2026
Collaborator
|
Thanks! |
Merged
via the queue into
ChromeDevTools:main
with commit May 26, 2026
43b934c
34 of 38 checks passed
OrKoN
reviewed
May 26, 2026
| }); | ||
| } | ||
|
|
||
| describe('shutdown', () => { |
Collaborator
There was a problem hiding this comment.
seems to be flaky on windows https://github.com/ChromeDevTools/chrome-devtools-mcp/actions/runs/26454201052/job/77883330043?pr=2140#step:9:1235
pull Bot
pushed a commit
to oogalieboogalie/chrome-devtools-mcp
that referenced
this pull request
May 26, 2026
🤖 I have created a release *beep* *boop* --- ## [1.1.0](ChromeDevTools/chrome-devtools-mcp@chrome-devtools-mcp-v1.0.1...chrome-devtools-mcp-v1.1.0) (2026-05-26) ### 🎉 Features * add extraHttpHeaders emulation to emulate tool ([ChromeDevTools#1176](ChromeDevTools#1176)) ([6992106](ChromeDevTools@6992106)) * created cursor plugin.json setting file with release auto versioning ([ChromeDevTools#2091](ChromeDevTools#2091)) ([10c8205](ChromeDevTools@10c8205)) ### 🛠️ Fixes * Apply CPU throttling to secondary CDP session ([ChromeDevTools#2092](ChromeDevTools#2092)) ([3ade962](ChromeDevTools@3ade962)) * **cli:** address pid file creation issues ([ChromeDevTools#2124](ChromeDevTools#2124)) ([1b51a52](ChromeDevTools@1b51a52)) * exit on stdin EOF and SIGTERM/SIGINT/SIGHUP, closing the browser cleanly ([ChromeDevTools#2117](ChromeDevTools#2117)) ([43b934c](ChromeDevTools@43b934c)) * Fix throttling info in performance trace output ([ChromeDevTools#2096](ChromeDevTools#2096)) ([57f32b0](ChromeDevTools@57f32b0)) * make pageId required ([ChromeDevTools#2084](ChromeDevTools#2084)) ([d751693](ChromeDevTools@d751693)), closes [ChromeDevTools#2052](ChromeDevTools#2052) * remove duplicate .mcp.json ([ChromeDevTools#2095](ChromeDevTools#2095)) ([dbf6ba9](ChromeDevTools@dbf6ba9)) * Set viewport after updating timeouts when setting emulation ([ChromeDevTools#2134](ChromeDevTools#2134)) ([0c3ac37](ChromeDevTools@0c3ac37)) * use pinned version for plugins ([ChromeDevTools#2135](ChromeDevTools#2135)) ([8ea5f09](ChromeDevTools@8ea5f09)) * use realpath for MCP roots validation ([ChromeDevTools#2127](ChromeDevTools#2127)) ([176eb69](ChromeDevTools@176eb69)) ### 📄 Documentation * align coding agent examples with Antigravity ([ChromeDevTools#2094](ChromeDevTools#2094)) ([ce31594](ChromeDevTools@ce31594)) * fix installation instructions for VS Code ([ChromeDevTools#2087](ChromeDevTools#2087)) ([9f47df3](ChromeDevTools@9f47df3)) ### 🏗️ Refactor * remove redundant validatePath calls ([ChromeDevTools#2136](ChromeDevTools#2136)) ([521c388](ChromeDevTools@521c388)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This comment was marked as off-topic.
This comment was marked as off-topic.
samiyac
pushed a commit
to bcfmtolgahan/chrome-devtools-mcp
that referenced
this pull request
Jun 5, 2026
… cleanly (ChromeDevTools#2117) Fixes ChromeDevTools#2116. `chrome-devtools-mcp-main.ts` currently has no shutdown handler. After a session calls `navigate_page` (or anything else that launches Chrome), the Chrome subprocess keeps the Node event loop ref'd, so closing stdin (the stdio MCP convention for "I'm done") doesn't make the server exit. Callers that close stdin to terminate the server have to fall back to SIGTERM / SIGKILL on every page-loaded session — deterministically, not flakily. This change: 1. Adds `closeBrowser()` in `browser.ts` that calls `browser.close()` for launched instances (reaps the Chrome subprocess) and `browser.disconnect()` for attached instances (leaves the user's Chrome alive). No-op if no browser is active or the connection has already been dropped. 2. Registers shutdown handlers in `chrome-devtools-mcp-main.ts` for: - `stdin.on('end' | 'close')` — stdio MCP transport convention - `SIGTERM` / `SIGINT` / `SIGHUP` — clients that signal instead of closing stdin (`SIGHUP` for parity with `src/daemon/daemon.ts`) The handler is idempotent (guarded `shuttingDown` flag), and has an unref'd 10s timeout backstop in case Chrome teardown hangs (slow `beforeunload` handlers, many tabs, etc.). ### Note on scope This complements (does not replace) the client-side fixes filed against ChromeDevTools#1765, e.g. google-gemini/gemini-cli#13391 and anthropics/claude-code#42300. The MCP stdio convention is that closing stdin signals shutdown; a server that doesn't honor that forces every client to special-case it. The watchdog sub-process (`src/telemetry/watchdog/main.ts:145-146`) and the daemon (`src/daemon/daemon.ts:224-230`) both already implement this for the same reason — this PR extends the same pattern to the main entry point so all three execution paths behave consistently. ### Measurement Repro script in ChromeDevTools#2116, same env (chrome-devtools-mcp@1.0.1, Chrome 148.0.7778.178, Node v24.11.1, Linux), 10 iterations: | Scenario | Before | After | |---|---|---| | `tools/list only` (no navigation) | 10/10 clean, 30-37 ms | 10/10 clean, 29-40 ms | | `navigate example.com` | 10/10 SIGTERM at ~5080 ms | 10/10 clean at 145-180 ms | ### Notes - I didn't add a subprocess-based test for this; the existing `tests/utils.ts:runCli` infrastructure targets the `chrome-devtools` CLI, not the stdio MCP server, and a shutdown-timing test would introduce non-trivial Chrome-startup flakiness in CI. Happy to add one if maintainers want it — pointer to the right test directory appreciated.
samiyac
pushed a commit
to bcfmtolgahan/chrome-devtools-mcp
that referenced
this pull request
Jun 5, 2026
🤖 I have created a release *beep* *boop* --- ## [1.1.0](ChromeDevTools/chrome-devtools-mcp@chrome-devtools-mcp-v1.0.1...chrome-devtools-mcp-v1.1.0) (2026-05-26) ### 🎉 Features * add extraHttpHeaders emulation to emulate tool ([ChromeDevTools#1176](ChromeDevTools#1176)) ([6992106](ChromeDevTools@6992106)) * created cursor plugin.json setting file with release auto versioning ([ChromeDevTools#2091](ChromeDevTools#2091)) ([10c8205](ChromeDevTools@10c8205)) ### 🛠️ Fixes * Apply CPU throttling to secondary CDP session ([ChromeDevTools#2092](ChromeDevTools#2092)) ([3ade962](ChromeDevTools@3ade962)) * **cli:** address pid file creation issues ([ChromeDevTools#2124](ChromeDevTools#2124)) ([1b51a52](ChromeDevTools@1b51a52)) * exit on stdin EOF and SIGTERM/SIGINT/SIGHUP, closing the browser cleanly ([ChromeDevTools#2117](ChromeDevTools#2117)) ([43b934c](ChromeDevTools@43b934c)) * Fix throttling info in performance trace output ([ChromeDevTools#2096](ChromeDevTools#2096)) ([57f32b0](ChromeDevTools@57f32b0)) * make pageId required ([ChromeDevTools#2084](ChromeDevTools#2084)) ([d751693](ChromeDevTools@d751693)), closes [ChromeDevTools#2052](ChromeDevTools#2052) * remove duplicate .mcp.json ([ChromeDevTools#2095](ChromeDevTools#2095)) ([dbf6ba9](ChromeDevTools@dbf6ba9)) * Set viewport after updating timeouts when setting emulation ([ChromeDevTools#2134](ChromeDevTools#2134)) ([0c3ac37](ChromeDevTools@0c3ac37)) * use pinned version for plugins ([ChromeDevTools#2135](ChromeDevTools#2135)) ([8ea5f09](ChromeDevTools@8ea5f09)) * use realpath for MCP roots validation ([ChromeDevTools#2127](ChromeDevTools#2127)) ([176eb69](ChromeDevTools@176eb69)) ### 📄 Documentation * align coding agent examples with Antigravity ([ChromeDevTools#2094](ChromeDevTools#2094)) ([ce31594](ChromeDevTools@ce31594)) * fix installation instructions for VS Code ([ChromeDevTools#2087](ChromeDevTools#2087)) ([9f47df3](ChromeDevTools@9f47df3)) ### 🏗️ Refactor * remove redundant validatePath calls ([ChromeDevTools#2136](ChromeDevTools#2136)) ([521c388](ChromeDevTools@521c388)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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.
Fixes #2116.
chrome-devtools-mcp-main.tscurrently has no shutdown handler. After a session callsnavigate_page(or anything else that launches Chrome), the Chrome subprocess keeps the Node event loop ref'd, so closing stdin (the stdio MCP convention for "I'm done") doesn't make the server exit. Callers that close stdin to terminate the server have to fall back to SIGTERM / SIGKILL on every page-loaded session — deterministically, not flakily.This change:
Adds
closeBrowser()inbrowser.tsthat callsbrowser.close()for launched instances (reaps the Chrome subprocess) andbrowser.disconnect()for attached instances (leaves the user's Chrome alive). No-op if no browser is active or the connection has already been dropped.Registers shutdown handlers in
chrome-devtools-mcp-main.tsfor:stdin.on('end' | 'close')— stdio MCP transport conventionSIGTERM/SIGINT/SIGHUP— clients that signal instead of closing stdin (SIGHUPfor parity withsrc/daemon/daemon.ts)The handler is idempotent (guarded
shuttingDownflag), and has an unref'd 10s timeout backstop in case Chrome teardown hangs (slowbeforeunloadhandlers, many tabs, etc.).Note on scope
This complements (does not replace) the client-side fixes filed against #1765, e.g. google-gemini/gemini-cli#13391 and anthropics/claude-code#42300. The MCP stdio convention is that closing stdin signals shutdown; a server that doesn't honor that forces every client to special-case it. The watchdog sub-process (
src/telemetry/watchdog/main.ts:145-146) and the daemon (src/daemon/daemon.ts:224-230) both already implement this for the same reason — this PR extends the same pattern to the main entry point so all three execution paths behave consistently.Measurement
Repro script in #2116, same env (chrome-devtools-mcp@1.0.1, Chrome 148.0.7778.178, Node v24.11.1, Linux), 10 iterations:
tools/list only(no navigation)navigate example.comNotes
tests/utils.ts:runCliinfrastructure targets thechrome-devtoolsCLI, not the stdio MCP server, and a shutdown-timing test would introduce non-trivial Chrome-startup flakiness in CI. Happy to add one if maintainers want it — pointer to the right test directory appreciated.