Uh oh!
There was an error while loading. Please reload this page.
fix(mcp): do not clobber chromiumSandbox from the config file - #42288
Merged
Conversation
Commander leaves `sandbox` undefined when neither `--sandbox` nor `--no-sandbox` is passed, since both flags are declared. The normalization turned that into `false`, which overrode the config file value, and turned an explicit `--sandbox` into `undefined`, which on Linux fell back to the platform default of disabling the sandbox for the chromium channel. Fixes: microsoft/playwright-mcp#1716
| .action(async options => { | ||
| // normalize the --no-sandbox option: sandbox = true => nothing was passed, sandbox = false => --no-sandbox was passed. | ||
| options.sandbox = options.sandbox === true ? undefined : false; |
There was a problem hiding this comment.
We should erase options.sandbox if there is no 'sandbox' in the argv instead.
MemberAuthor
There was a problem hiding this comment.
I don't think it's true, missing --no-sandbox results in undefined:
yurys:~/playwright(main)$ node packages/playwright-core/lib/entry/mcp.js --no-sandbox
sandbox: false
yurys:~/playwright(main)$ node packages/playwright-core/lib/entry/mcp.js sandbox: undefined
yurys:~/playwright(main)$ node packages/playwright-core/lib/entry/mcp.js --sandbox sandbox: true
There was a problem hiding this comment.
This must have changed during the monorepo migration (minimist => commander)
Pavel Feldman (pavelfeldman)
approved these changes
Aug 17, 2026
Yury Semikhatsky (yury-s)
merged commit Aug 17, 2026
be6e7cc
into
microsoft:main
15 of 17 checks passed
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Test results for "MCP"2 failed 8108 passed, 1341 skipped Merge workflow run. |
Contributor
🔴 One failure looks caused by this PRHi, I'm the Playwright bot and I took a first look at the CI failures.
DetailsCaused by this PR
Pre-existing flake / infra
Triaged by the Playwright bot - agent run |
This was referenced Aug 18, 2026
Jori Huisman (SixFive7) added a commit
to SixFive7/BrowserAI
that referenced
this pull request
Aug 19, 2026
… rather than a fix The TODO entry named three outcomes and what each one owed. This is the one it called least likely: microsoft/playwright#42288 -- "fix(mcp): do not clobber chromiumSandbox from the config file" -- deletes decorateMCPCommand's four-line normaliser outright and closes playwright-mcp#1716 as COMPLETED, merged 2026-08-17. Nothing in the product changes and nothing should. BrowserAI passes --sandbox on the command line, which works on both sides of this: before, by mapping to undefined and falling through to validateBrowserConfig's non-Linux default; after, by setting the key explicitly. The half that inverts is the config-file key, which starts working, and nothing here sets it. What it owes is the row. This is the change a golden snapshot is structurally blind to -- the tool surface and the config schema both hold still while the behaviour behind one key inverts -- so re-verification row 102 exists to be worked through at the bump that carries it. And it has not arrived yet: @playwright/mcp 0.0.79 pins playwright-core 1.63.0-alpha-2026-08-05, both older than the merge, re-resolved and confirmed unmoved today. So the kb entry keeps every measurement it has, stamped with what the shipped tree still does, and gains a note saying exactly what will stop being true and when.
This was referenced Aug 20, 2026
Jori Huisman (SixFive7) added a commit
to SixFive7/BrowserAI
that referenced
this pull request
Aug 27, 2026
… because one signal decides both The two upstream asks stay where they were filed. What was missing was the sentence that says what would change that, so `TODO.md`'s Upstream asks section now carries a third item covering #1725 and #1726 together -- the decision taken 2026-08-27, with the trigger, the destination and the evidence that pointed both ways. Neither ask body was touched and neither `Filed:` line moved: the diff is 59 insertions and no deletions at all. WHY ONE ITEM AND NOT TWO. They are two instances of the same judgment about the same tracker, filed four seconds apart. Whatever happens to either answers the question for the other, so watching them apart would only mean taking one decision twice off one piece of evidence. WHAT MOVES THEM, AND WHERE THEY GO. Either issue closed with a redirect to the monorepo, or both left untriaged while microsoft/playwright's `[MCP]:` issues go on being answered around them -- silence in one tracker while the other moves is the same answer as a redirect, given slower. They go to microsoft/playwright, one issue each, bodies unchanged, with `[MCP]` as a TITLE prefix rather than a label: #42363 and #42384 both carry it in the title and both carry no labels at all. Then #1725 and #1726 close with links to the new issues, so the trail leads forwards instead of stopping. THE EVIDENCE IS WRITTEN DOWN BOTH WAYS BECAUSE IT DOES NOT POINT ONE WAY. Every figure in the item was re-read today rather than carried over. playwright-mcp held three open issues in total on 2026-08-27 -- the pinned notice #1664 from 2026-06-29 asking that issues be filed at microsoft/playwright, and these two. The code both asks name is monorepo code already: `_computeRelativeTo` and `_writeFile` appear nowhere in the shipped payload except playwright-core/lib/coreBundle.js, where the MCP server's `Response` module now ships, and the `@playwright/mcp` package beside it is `cli.js` and `index.js` over that bundle. Against all of that stands this project's own #1716, filed at playwright-mcp seven weeks after the notice and closed as fixed nine hours and fifty-two minutes later -- and the item records which way that cuts, because its fix landed in the monorepo as microsoft/playwright#42288, so it is evidence for where the code lives and against where the report has to go. ONE MORE IS WATCHED BESIDE THEM. microsoft/playwright#42384, open since 2026-08-24, is adjacent to the first ask and not a duplicate of it: it is about which directory artifacts land in, ours is about the form of the path a tool result names them by. It is the same class of report in the other tracker, which makes how it is handled the triage signal this item waits on. NO PUBLISHED COUNT MOVED, AND THAT WAS CHECKED RATHER THAN ASSUMED. Every link the item adds is an absolute URL, which both halves of the link scan skip, so the fragment count `CLAUDE.md` publishes -- 843 relative links carrying a `#fragment` -- has the same input it had before, and the green run says so rather than a diff being counted by hand. The 27 assumed justifications counted in this file are one per italicised or named claim in the three lists inside that item, and this item is not one of them. GATE. One full run from each shell, neither overlapping the other, both green: 640/0/0 FULL RUN from PowerShell at drive letter UPPER `C:\`, 640/0/0 FULL RUN from Git Bash at LOWER `c:\`, each half reporting that the spelling it forced took. Both runs exercised every layer with no test on a degraded path. TANGENTIAL, NOT FIXED HERE. The hazard row for the discarded `chromiumSandbox` config key still says #1716 is "Reported upstream as issue 1716 and still open" and that `TODO.md` carries what each of the three outcomes needs. Neither half holds now: the issue closed COMPLETED on 2026-08-17, which kb/playwright/configuration.md already records against PR 42288, and the item it points at is gone from this file. The behaviour the row is about is unchanged -- the shipped bundle predates the fix -- so this is a stale tracker claim rather than a stale measurement, and it belongs to whoever owns that row.
Jori Huisman (SixFive7) added a commit
to SixFive7/BrowserAI
that referenced
this pull request
Aug 27, 2026
… that was already gone CORRECTION. The hazard row for the discarded `chromiumSandbox` config key ended on two sentences that were both false, and the previous commit flagged them as tangential and left them for whoever owns the row. This is that. Neither half was load-bearing on the closure, which is why the row's `Status` does not move and no published count moves with it. WHAT WAS FALSE. The row said the defect was "Reported upstream as issue 1716 and still open". playwright-mcp#1716 closed COMPLETED on 2026-08-17, fixed in the monorepo by microsoft/playwright#42288 -- *fix(mcp): do not clobber chromiumSandbox from the config file* -- which deletes the four-line normaliser outright. `kb/playwright/configuration.md` has recorded that against the PR since the day it landed, so the row was contradicted by the very article its own `Established in` cell points at. The row also said `TODO.md` "carries what each of the three outcomes needs"; that item is gone. `TODO.md` names #1716 today only as evidence about which tracker to file in, in an entirely different item about where two other asks should go. WHAT IS STILL TRUE, AND IT IS THE HALF THE ROW EXISTS FOR. The behaviour is unchanged: the shipped bundle predates the fix -- `@playwright/mcp` 0.0.79 pins `playwright-core` 1.63.0-alpha-2026-08-05, both older than the merge, and the payload rebuild in the previous commit re-confirmed both versions today rather than carrying them forward. So the config key is still discarded here, `SandboxFlagTests.TheConfigKeyIsStillDiscardedByUpstream` still guards it, and that test is what will say so on the bump that finally carries the fix rather than the discard quietly becoming a fix nobody noticed. BrowserAI is not exposed either way, because it passes the flag on the command line. AMENDED BY ADDITION, NOT REWRITTEN. The false sentence is quoted inside a `Corrected 2026-08-27 (previously ...)` clause rather than deleted, because a reader who learned the tracker was open needs to be told it is not. The row names the same symbols it named before, so the check that every symbol a row names exists is unaffected. NO COUNT MOVES. No row arrived and none crossed -- this is one row's prose, and its `Status` is the `closed 2026-08-19` it already had -- so the hazard tally has the same inputs it had before, and the green run says so rather than a diff being read by hand. The two issue references are written as plain text rather than as links, matching the row's existing style and the file's own rule about staying self-contained. GATE. Both full runs for this batch were green before either commit was made: 640/0/0 FULL RUN from PowerShell at drive letter UPPER `C:\`, 640/0/0 FULL RUN from Git Bash at LOWER `c:\`, neither overlapping the other, each declaring the spelling it forced and that it took.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
sandboxundefined when neither--sandboxnor--no-sandboxis passed (both flags are declared), so the normalization indecorateMCPCommandwas wrong.false, silently overridingbrowser.launchOptions.chromiumSandboxfrom the config file.--sandboxintoundefined, which on Linux fell back to the platform default that disables the sandbox for the chromium channel.Fixesmicrosoft/playwright-mcp#1716