fix(cli): forward explicit false options on start - #2702
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
ecbfe09 to
b5bc4c5
Compare
yangfan-yf-yf
left a comment
There was a problem hiding this comment.
I independently checked d23d057 on Windows with Node 24.14.0 and installed Chrome 152.0.7977.83, using isolated test profiles and the package test scripts.
The start/stop e2e file plus daemon utils yielded 29 passed, 0 failed, 4 platform skips. In a counterfactual run reverting only the serializer call, the four e2e tests yielded 3 passed and 1 failed: forwards an explicit headless=false option failed because --no-headless was absent. Restoring the patch makes that case pass. This supports using the same CLI option defaults for serialization as for parsing.
I also ran the complete test suite on this head after restoring the source: 981 passed, 6 failed, 5 skipped (992 tests, concurrency 2). The six failures were the missing snapshot in should get request from previous navigations, Page.captureScreenshot: Page is too large in the large full-page screenshot test, and four EPERM symlink creation failures in resolveCanonicalPath. These same failures were observed in my earlier 099c068 run; I am not reporting a green full suite or attributing them to this patch. The daemon timeout failures from that earlier run did not recur.
For reproducibility, the frontend source was materialized from the official archive at the pinned 9b6645d16f95b2cd441aab609b53ec7d9d5fef81 commit, and dependencies came from this head's lockfile. No test expectations or source changes were retained for the full-suite run. I have not verified this on Linux or macOS.
Summary
chrome-devtools startarguments against the CLI defaults.--headless=falseas--no-headless.Problem
The CLI defaults
headlessto true while the MCP server defaults it to false. The start command parsed an explicit false value correctly, but then serialized it against the server defaults. Because false matched the server default, the argument was dropped. The daemon subsequently reparsed the missing option in CLI mode and restoredheadless=true.Using the CLI option definitions for serialization keeps the default compact while forwarding explicit values that differ from the CLI default.
Testing
npm run test tests/e2e/chrome-devtools-start-stop.test.tsnpm run formatnpm run test(all relevant tests passed; the local full-suite run only failed the unrelated large full-page screenshot case because Chrome returnedPage is too large)