release-please / release-please (push) Has been cancelled
Compile and run tests / Tests on macos-latest with node 22 (push) Has been cancelled
Compile and run tests / Tests on ubuntu-latest with node 22 (push) Has been cancelled
Compile and run tests / Tests on windows-latest with node 22 (push) Has been cancelled
Compile and run tests / Tests on macos-latest with node 24 (push) Has been cancelled
Compile and run tests / Tests on ubuntu-latest with node 24 (push) Has been cancelled
Compile and run tests / Tests on windows-latest with node 24 (push) Has been cancelled
Compile and run tests / Tests on macos-latest with node 26 (push) Has been cancelled
Compile and run tests / Tests on ubuntu-latest with node 26 (push) Has been cancelled
Compile and run tests / Tests on windows-latest with node 26 (push) Has been cancelled
Check code before submitting / [Required] Check correct format (push) Has been cancelled
Check code before submitting / [Required] Check docs updated (push) Has been cancelled
Compile and run tests / [Required] Tests passed (push) Has been cancelled
Assortment of various flakiness conditions found running tests in a loop
locally:
This PR introduces a comprehensive set of hermetic retry layers and
aggressive
timeout handlers across the test suite to insulate it from random
Chromium
startup hangs, CDP deadlocks, and Puppeteer lifecycle flakes. It
guarantees that
temporary browser infrastructure failures are automatically retried
without
failing the CI, while actual code assertion failures still fail fast.
### Test Harness & Retry Improvements
• tests/utils.ts: Rewrote withBrowser to include a 30-second internal
timeout and
a 3-attempt retry loop. If Chromium locks up or disconnects (Target
closed /
socket hang up), the browser is forcibly evicted (via SIGKILL if
browser.close()
hangs) and the test setup is cleanly retried.
• tests/index.test.ts: Wrapped withClient (used by E2E tests) in a
3-attempt
retry loop to handle the daemon/Chromium hanging during launch and
triggering the
60-second MCP client timeout.
• tests/browser.test.ts: Added a safeClose helper that imposes a
2-second timeout
before SIGKILLing browsers, and wrapped raw Puppeteer tests in
runWithRetry to
handle startup hangs.
• tests/shutdown.test.ts: Added a setupServerWithRetry helper to prevent
random
60s RPC timeouts when the server's Chrome instance hangs during boot.
### Flaky Operations & Navigation Fixes
• src/tools/performance.ts & tests/tools/performance.test.ts: Replaced
the
notoriously flaky waitUntil: ['networkidle0'] with 'load' when
navigating to
about:blank in performance_start_trace. This prevents random 10-second
Navigation
timeout exceeded errors. Also stubbed goto in the associated unit tests
for
better hermeticity.
• src/McpContext.ts: Wrapped browser.installExtension() with a 15-second
timeout
to prevent deadlocks when an extension fails to load.
• tests/tools/extensions.test.ts: Removed flaky headless UI navigations
to
chrome://extensions in favor of using the context.listExtensions() API.
• tests/tools/pages.test.js.snapshot: Synced test snapshots to reflect
updated
environment baselines.
As discussed in #2366: `get_tab_id` gates on
`--experimentalInteropTools`, but its result only went into
`structuredContent`, which is dropped unless
`--experimentalStructuredContent` is also set – so with interop alone
the tool always returned an empty text response.
This appends a `Tab ID: <id>` line to the text response and keeps
`structuredContent.tabId` as-is – `key: value` so it stays trivially
parseable.
## Testing
The existing `returns the tab id` test asserted empty `responseLines`,
which pinned the old behavior – now it asserts the `Tab ID:` line.
Verified end-to-end over stdio with `--experimentalInteropTools` alone:
the text response is `Tab ID: <id>` where it was `""` before.
Given we run test locally and that this does not change often one can
just run the command once in a while to fix the local setup.
```sh
Failed tests:
✖ matches snapshot if exists (0.78188ms)
Error: THIRD_PARTY_NOTICES does not exist, run `npm ci && npm run bundle`
at TestContext.<anonymous> (file:///usr/local/google/home/nvitkov/chrome-devtools-mcp/build/tests/third_party_notices.test.js:13:19)
at Test.runInAsyncScope (node:async_hooks:227:14)
at Test.run (node:internal/test_runner/test:1325:25)
at Test.start (node:internal/test_runner/test:1191:17)
at node:internal/test_runner/test:1792:71
at node:internal/per_context/primordials:466:82
at new Promise (<anonymous>)
at new SafePromise (node:internal/per_context/primordials:435:3)
at node:internal/per_context/primordials:466:9
at Array.map (<anonymous>)
✖ THIRD_PARTY_NOTICES (1.41613ms)
'1 subtest failed'
```
Add a get_heapsnapshot_object_details MCP tool which lets the agent
query all known information about a node in the heap snapshot.
Co-authored-by: Dominik Inführ <dinfuehr@chromium.org>
Emits native contexts and their sizes with the get_heapsnapshot_summary
MCP tool. This should help the agent get an overview about which native
contexts consume the most memory.
Co-authored-by: Dominik Inführ <dinfuehr@chromium.org>
`list_network_requests` and `list_console_messages` treat an explicit
`pageIdx: 0` as "no pagination" and dump every result, while `pageIdx:
1` and up paginate correctly.
The guards in `setIncludeNetworkRequests`/`setIncludeConsoleData` build
the pagination options with `options?.pageSize || options?.pageIdx`, so
a bare `{pageIdx: 0}` collapses to `undefined` — 0 is falsy — and
`paginate()` never runs. That's a bit odd since `paginate()` already
decides for itself when there's nothing to paginate
(`noPaginationOptions` checks `pageSize === undefined && pageIdx ===
undefined`), so page 0 is a perfectly valid first page there. The memory
tools, which pass their options straight through to `paginate()`, handle
`pageIdx: 0` fine — only these two guards get it wrong, so an agent
walking pages 0, 1, 2… gets the full list on page 0 and a 20-item window
from page 1 on.
Switched both guards to an explicit `!== undefined` check so `pageIdx:
0` reaches `paginate()` and gets the default page size like every other
page. Both-omitted still short-circuits to the full list, so nothing
else changes. Added a regression test for the `{pageIdx: 0}` case.
Closes#2339
Reconnecting after a browser restart builds a fresh `McpContext`, which
restarted the page id counter at 1 – ids from before the restart
silently resolved to unrelated pages of the new browser (repro in
#2339).
Two changes:
- The new context continues the id counter where the previous one left
off (`startingPageId` option), so a stale id now fails with the existing
"No page found" error and the agent re-lists
- The first response after a reconnect carries a one-time note ("the
browser was restarted or reconnected since the last call. Page ids have
changed..."), like the #2308 fallback note. Since tool errors run
through the same response formatting, the note shows up together with
the very error the stale id produces
Verified against the #2339 repro: the stale `select_page` now returns
the note plus "No page found", and the next listing shows the new
browser's pages under fresh ids.
One thing I noticed but left alone: The replaced context isn't
`dispose()`d on reconnect – pre-existing, and everything it holds is
tied to the dead browser anyway. Happy to add that here if you'd like.
- remove obsolete devtools page detection step
- move isolated context processing out of the browser pages fetching
- move filtering of pages out of getPages(). That should just be a
getter.
## What
`press_key` currently presses each modifier down, presses the main key,
then releases the modifiers — in three sequential steps with no
`try/finally`:
```ts
for (const modifier of modifiers) {
await page.pptrPage.keyboard.down(modifier);
}
await page.pptrPage.keyboard.press(key); // if this rejects…
for (const modifier of modifiers.toReversed()) {
await page.pptrPage.keyboard.up(modifier); // …this never runs
}
```
If `keyboard.press(key)` rejects — a CDP hiccup, a target crash, or a
dropped connection — the release loop is skipped and the modifier keys
are left **logically held down in the browser**.
`waitForEventsAfterAction` re-throws the action error, so nothing
downstream releases them either.
This is the unpaired-`keyDown` class of defect asked about in #2309
(*"whether any code path sends a keyDown without a guaranteed matching
keyUp on an error/timeout branch"*). This PR fixes the one concrete
instance of it in this repo.
## Fix
Wrap the down/press sequence in `try/finally` and track which modifiers
were actually pressed, releasing each held modifier even when the main
press throws. Only modifiers whose `keyboard.down()` succeeded are
released, so a failure *while* pressing a modifier doesn't emit a
spurious `keyUp`.
## Test
Adds a regression test (real browser, keydown/keyup logging) that
injects a `press()` failure mid-sequence and asserts both modifiers are
still released. It fails on `main` (`['dControl','dShift']` — no keyups)
and passes with the fix (`['dControl','dShift','uShift','uControl']`).
Verified locally: full `tests/tools/input.test.ts` suite passes, `npm
run typecheck`, eslint, and prettier all clean.
## Scope note re: #2309
I want to be precise about what this does and does not address. This
closes a **browser-level** stuck-key path: leaked keys here live in
Chromium's input state (CDP `Input.dispatchKeyEvent` is injected into
the renderer), so the observable effect is a modifier stuck **within the
driven page**. The report in #2309 is a bare `Space` that repeats
**system-wide** and survives physically unplugging the keyboard — that
symptom is at the OS input layer, which CDP-injected input doesn't route
through, so I don't claim this fully explains that case (details and a
non-reboot workaround are in a comment on the issue). Still, an unpaired
keyDown on an error branch is a real defect worth closing on its own,
and it's exactly the code path the issue asked to audit.
Prepared with AI assistance (Claude Code) and verified against a local
build before submission.
Follow-up to #2304, and to #2328 which fixed the symptom.
After #2333, `getPages()` – what `list_pages` presents – is a filtered
view over `#mcpPages` that excludes `devtools://` frontends (unless
`experimentalDevToolsDebugging` is set; they enter `#mcpPages` via
`handleDevToolsAsPage`). `getPageById()` still searches all of
`#mcpPages`, though – so `select_page`, and every other tool that takes
a `pageId`, can target a page `list_pages` never showed.
#2328 stopped that from silently stealing a still-open selection. This
goes to the root: `getPageById()` now resolves through `getPages()`, so
an unlisted page isn't targetable in the first place. Page ids only ever
reach the client through the listing, so an unlisted id has no
legitimate source. `experimentalDevToolsDebugging` is unaffected – the
listing already includes devtools frontends there.
All five callers (`select_page`, `close_page`, `evaluate_script`,
`get_tab_id`, and the generic `pageId` handler) benefit uniformly. Also
trims the now-impossible example from the fallback comment – an unlisted
`devtools://` page can no longer be selected.
Test: with DevTools open, the frontend page is tracked but unlisted; the
new test asserts no id outside `getPages()` resolves through
`getPageById()`.
Refs: #2304
This exposes DevTools named filters in get_heapsnapshot_details and
get_heapsnapshot_class_nodes tools. This allows the client to list e.g.
all objects retained through detached DOM objects.
Co-authored-by: Dominik Inführ <dinfuehr@chromium.org>
## What
`generateToolMetrics` reads an enum parameter's values by unwrapping
**at most
one** `optional` wrapper:
```ts
if (schema._def.values?.length > 0) {
values = schema._def.values;
} else {
values = schema._def.innerType._def.values; // only one level
}
```
An enum parameter wrapped in `.default().optional()` (or any nested
`optional`/`default`/`nullable` combination) therefore falls through to
`schema._def.innerType._def.values === undefined`, and `npm run
update-metrics`
(run as part of `npm run gen`) crashes:
```
TypeError: Cannot read properties of undefined (reading '0')
at validateEnumHomogeneity (build/src/telemetry/metricsRegistry.js:12)
at generateToolMetrics (build/src/telemetry/metricsRegistry.js:59)
```
## Why it matters
`npm run gen` is required whenever a tool is added or changed (per
`CONTRIBUTING.md`). Declaring an enum parameter with a default — a
natural,
documented zod pattern — silently breaks docs/metrics generation.
## Fix
Add `getEnumValues()` in `transformation.ts` that recursively unwraps
`optional` / `default` / `nullable` / `effects` wrappers, mirroring the
existing
`getZodType()`, and use it in `generateToolMetrics`. Behavior is
unchanged for
the existing bare-enum and single-`optional` cases.
## Tests
- `getEnumValues` unit tests: bare enum, `optional`, `default`, both
orders of
`default`+`optional`, and throws for a non-enum type.
- `generateToolMetrics` regression test with
`zod.enum(...).default(...).optional()`.
- `npm run typecheck`, the telemetry tests, and `npm run check-format`
pass.
---
First-time contributor here — happy to sign the CLA.
Follow-up to #2304.
`createPagesSnapshot()` reselects `#pages[0]` whenever the selected page
is missing from `#pages`, even when that page is still open. So a live
selection can be silently swapped, not only a closed one.
The case that hit us is a `devtools://` page:
- the server connects with `handleDevToolsAsPage: true`
(`src/browser.ts`), so DevTools frontends are pages and land in
`#mcpPages`
- `#pages` filters `devtools://` out (`src/McpContext.ts`, unless
`experimentalDevToolsDebugging` is set)
- `getPageById()` (used by `select_page`) reads `#mcpPages`, not
`#pages`, so a devtools page can be selected even though `list_pages`
never lists it
- the next snapshot finds it missing from `#pages` and, because the
fallback keys on `#pages` membership rather than `isClosed()`, reselects
`#pages[0]` with the page still open (`isClosed() === false`)
This reproduces on a normal Chrome (details and a repro in #2304).
This change gates the fallback on `isClosed()`, as proposed in #2304. A
still-open selection is kept; a genuinely closed page still
auto-advances, so the browser-like behavior for closed tabs is
unchanged.
One consequence: the "is no longer listed" wording added in #2308
becomes unreachable, since a live page missing from the list no longer
triggers a fallback. I left it in place to keep this diff focused, but
I'm happy to simplify it here or in a follow-up.
The unit test that covered the missing-but-open case now asserts the
selection is retained.
Refs: #2304
## Summary
validatePath() in McpContext returned immediately, with no restriction
at all, whenever roots() returned undefined. roots() only returns
undefined when the connecting MCP client never negotiates the optional
roots capability during initialize, which any minimal client can trigger
simply by omitting it from its declared capabilities.
Since roots() already always appends the OS temp directory to whatever
explicit roots are configured, this change makes it return that same
default (temp directory only) instead of undefined when no roots have
been set. This removes the early return in validatePath() entirely, so
path validation now runs unconditionally rather than being conditional
on whether the connecting client happened to negotiate a capability it
was never required to declare per the MCP spec.
Any filePath-accepting tool (take_screenshot, saveFile, and the
performance/Lighthouse export tools that route through the same check)
had its only path-traversal guard silently disabled for the lifetime of
a connection whenever the client omitted the optional roots capability.
Since this server is designed to let an LLM drive a browser, and browsed
page content is not trusted input, this meant a client that simply
doesn't implement roots (a plausible, non-adversarial default for
lightweight or custom MCP clients) removed the only boundary preventing
the connected agent from writing to any path the process can reach.
Added a test that exercises the actual default state of roots (never
calling setRoots()) directly, since the existing tests always call
setRoots(), even with an empty array, before validating. Verified
locally with a minimal MCP client that declares no capabilities: before
this change, take_screenshot with a filePath outside any root wrote a
real file to an arbitrary path with no error; after this change, the
same call is rejected with the existing Access denied error. Also
verified that a client that does declare roots is unaffected, and that
writes to the OS temp directory continue to succeed with no roots
negotiated, matching prior behavior for that path.
Follow-up to #2304 (the "note" part discussed there).
When the selected page disappears from the page list,
`createPagesSnapshot()` silently re-selects the first page. The agent
gets no signal: if its next call is `list_pages` (which is what the
closed-page error message recommends), the listing already shows the new
selection and every subsequent tool call runs against a page the agent
never picked.
This PR records the automatic fallback and surfaces it as a one-line
note in the pages section of the same response:
```
## Pages
Note: the previously selected page was closed. Page 1 is now selected.
1: about:blank [selected]
```
For a selected page that is missing from the list without being closed,
the note reads "is no longer listed" instead. If the expectation from
#2304 holds (a page stays listed as long as it is not closed), that
wording never renders; if the transient case discussed there does occur
in the wild, the note will make it visible.
The fallback behavior itself is unchanged (as discussed in #2304, closed
tabs keep the browser-like auto-selection). No note is emitted on first
connect, when nothing was selected before.
Tests: two unit tests for the fallback bookkeeping (closed page, regular
selection), one for the missing-but-open case via a stubbed page list,
and the one affected snapshot updated (`close_page` now includes the
note).
Refs: #2304
Fixes output path validation so tools cannot validate one path and then
write to a different canonical target after extension enforcement.
Changes:
- Resolve dangling symlinks to their target path during
canonicalization.
- Validate the final extension-enforced output path before writing.
- Apply the same final-path validation to heap snapshots and
screencasts.
- Add regression coverage for dangling symlinks that point outside
configured roots.
Validation:
- npm run format
- npm run check-format
- npm run test tests/utils/files.test.ts
- npm run test tests/roots.test.ts
- npm run test tests/tools/memory.test.ts tests/tools/screencast.test.ts
Note: I also ran the full npm test suite locally. The targeted tests
above passed, but the full suite hit local WSL daemon/e2e startup
timeouts while waiting for daemon.pid / server_start, which appear
unrelated to this path-validation change.
---------
Co-authored-by: huynhtrungcsc <huynhtrungcsc@users.noreply.github.com>
Fixes flakiness in pages.test.ts where unawaited page.evaluate()
promises triggering dialogs would resolve after the test ended, causing
TargetCloseError when the next test closed the pages.
TAG=agy
CONV=dc6c530f-5d71-4a91-9440-6a7006d168e6
Co-authored-by: Piotr Paulski <piotrpaulski@chromium.org>
Instead of two separate MCP tools, we can have one instead which takes
classIndex as optional argument.
Co-authored-by: Dominik Inführ <dinfuehr@chromium.org>
This commit adds two MCP tools for comparing heap snapshots.
`compare_heapsnapshot_summary` compares two memory snapshot and returns
which classes have new/deleted objects.
`compare_heapsnapshot_class_nodes` can then be used to list the object
ids added and deleted for a specific class.
Co-authored-by: Dominik Inführ <dinfuehr@chromium.org>
Since toon dependency is only needed for `--experimentalToonFormat`
flag, we can make it optional to decrease package size and security
footprint for users that don't use it.
Testing npx optional peer dependency resolution before the release:
```
npm i -g verdaccio
verdaccio
```
On a separate terminal:
```
npm adduser --registry http://localhost:4873/
# follow prompts to create user and login
# replace the published package in verdaccio
npm unpublish chrome-devtools-mcp@1.4.0 --force --registry http://localhost:4873
npm publish --registry http://localhost:4873
# clear npx cache
rm -rf ~/.npm/_npx
# run the server from commandline and observe both packages being installed:
npx --registry http://localhost:4873 --package chrome-devtools-mcp@latest --package @toon-format/toon chrome-devtools-mcp --experimentalToonFormat
```
Paste the following commands (each line separately) to manually interact
with the mcp server and observe TOON formatted response directly
### 1. Initialize the session
{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"test-client","version":"1.0.0"}}}
### 2. Confirm initialization
{"jsonrpc":"2.0","method":"notifications/initialized"}
### 3. Navigate to Google (this will already create snapshot in most
recent versions)
{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"navigate_page","arguments":{"url":"https://google.com"}}}
### 4. Take Snapshot (if not returned by the previous command)
{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"take_snapshot","arguments":{}}}
---------
Co-authored-by: Piotr Paulski <piotrpaulski@chromium.org>
## Summary
Fixes#2230.
- Sets `windowsHide: true` on the detached update-check process so
Windows does not flash a console window.
- Also hides the `npm config get registry` subprocess used by the
updater helper.
- Covers the update-check spawn options in the existing tests.
## Verification
- `NODE_OPTIONS=--max-old-space-size=4096 npm run build`
- `npm run test:no-build -- tests/check-for-updates.test.ts`
- `NODE_OPTIONS=--max-old-space-size=4096 npx eslint
src/utils/check-for-updates.ts src/bin/check-latest-version.ts
tests/check-for-updates.test.ts`
- `npx prettier --check src/utils/check-for-updates.ts
src/bin/check-latest-version.ts tests/check-for-updates.test.ts`
- `git diff --check`
Note: plain `npm run build` and full-repo `npm run check-format` hit the
local Node heap limit in this runner; the same build passed with the
heap limit raised, and touched-file lint/format checks passed.
Co-authored-by: cyphercodes <cyphercodes@users.noreply.github.com>
## Problem
`get_network_request` returns, from the **same call**, both a
human-readable
text block (`toStringDetailed()`) and a
`structuredContent.networkRequest`
object (`toJSONDetailed()`) — emitted together in `McpResponse.ts`. For
a
request that went through HTTP redirects, the redirect chain comes out
in
**opposite orders** in the two representations:
- `toJSONDetailed()` (`NetworkFormatter.ts`) reverses `redirectChain()`
once →
newest→oldest in the JSON.
- the text formatter then reverses it **a second time** → oldest→newest
in the
text.
Because each path calls `redirectChain()` separately and Puppeteer
returns a
fresh copy on every call (`HTTPRequest#redirectChain()` does
`this._redirectChain.slice()`), the two reverses operate on different
arrays and
don't cancel. So a consumer reading the text and a consumer parsing the
structured JSON from the same response see contradictory redirect
orders.
## Solution
Drop the redundant `.reverse()` in the text formatter so the rendered
text uses
the order already produced by `toJSONDetailed()`. Both representations
are now
consistent (newest→oldest), and `structuredContent` is unchanged.
## Why the existing tests didn't catch it
- The existing "handles redirect chain" test uses a **single-element**
chain,
where reversing is a no-op.
- `getMockRequest().redirectChain()` returned the **same array
reference** on
every call, unlike real Puppeteer — so the two reverses accidentally
agreed in
tests. This PR makes the mock return a fresh copy per call (matching
Puppeteer)
and adds a regression test with a multi-element chain that asserts the
text and
JSON orders match.
## Testing
- `npm test` for the formatter suite passes. New test
`renders the redirect chain in the same order in text and JSON` is
**red**
before the fix (text `[first, second]` vs JSON `[second, first]`) and
**green**
after, with no change to existing snapshots.
- `npm run typecheck` and Prettier/ESLint are clean.
No existing issue tracked this; found via code inspection and confirmed
empirically.
Fixes#2206
### Problem
`screencast_start` matched the requested file extension with a
**case-sensitive** `endsWith()` against `['.webm', '.mp4']` and
**silently fell back to `.mp4`** when nothing matched. Combined with
`ensureExtension()` (which replaces the extension), a request for
`demo.WEBM` was recorded as **MP4** to **`demo.mp4`** — a different
format *and* path than requested — and any unsupported extension (e.g.
`recording.avi`) silently became `.mp4`.
Separately, when `screencast_start` is called without a `filePath`, it
creates a temp directory via `mkdtemp()`. If `page.screencast()` then
throws (e.g. ffmpeg missing), that directory was leaked.
### Changes
Two commits:
1. **`fix: match screencast extension case-insensitively and reject
unsupported ones`** — match via `path.extname().toLowerCase()`; reject
an explicitly requested but unsupported extension with an explicit error
listing the supported formats; a missing extension still defaults to
`.mp4`.
2. **`fix: clean up screencast temp directory when recording fails to
start`** — remove the generated temp dir in the `catch` handler, but
only when we own the generated path (never when the caller supplied
`filePath`).
| requested | before | after |
| --------------- | --------------- | -------------- |
| `demo.WEBM` | mp4 → `demo.mp4`| webm → `demo.webm` |
| `recording.avi` | mp4 → `recording.mp4` | error (rejected) |
| `demo.webm` | webm → `demo.webm` | unchanged |
| *(no filePath)* | mp4 temp | unchanged |
The matched extension is normalized to lower case (`demo.WEBM` →
`demo.webm`).
### Testing
Added three regression tests to `tests/tools/screencast.test.ts` using
the existing `sinon`/`withMcpContext` harness. Verified locally against
Chrome for Testing 149 (`PUPPETEER_EXECUTABLE_PATH`):
- With the fix reverted, the two extension tests fail (uppercase `.WEBM`
→ mp4, `.avi` not rejected) and the cleanup test fails (temp dir left
behind) — i.e. they fail for the right reason.
- With the fix applied, the full `screencast.test.ts` suite passes
(11/11).
- `tsc --noEmit` and `npm run check-format` (eslint + prettier) are
clean.
> Note: I ran the `screencast` test file (which stubs `page.screencast`)
plus typecheck/lint locally; the rest of the browser-based suite I left
to CI.
### Notes for reviewers
- I chose to **`throw`** for an unsupported explicit extension
(consistent with the ffmpeg-missing `throw` in the same handler and with
the issue's "reject with an explicit error"). Happy to switch to the
softer `appendResponseLine(...) + return` style used by the in-progress
guard if you'd prefer.
- The two commits are independent and can be split if you'd rather take
them separately.
- I left the pre-existing `as \`${string}.webm\`` assertion on
`resolvedPath` untouched to keep the diff focused, though it's slightly
misleading now that the default is `.mp4`.
---------
Co-authored-by: Nicholas Roscino <nroscino@google.com>
## Summary
Adds **opt-in** CLI flags so operators can cap the size of screenshots
returned by `take_screenshot` before they are embedded in the MCP
response. Refs #879.
The flags address two related symptoms reported when MCP clients display
screenshots inline:
1. **Per-image dimension limit**: hosted LLM APIs commonly reject images
exceeding per-image dimension constraints (typical caps are in the
2000-8000 px range, sometimes scaling down further when many images are
in the same request). This is the exact error reported in #879.
2. **Cumulative request size**: after many captures, the cumulative
base64 payload eventually pushes a request over the per-call body size
limit imposed by the LLM API.
Both can be mitigated at the source by reducing format/quality and
downscaling the capture.
## New flags (all opt-in)
- `--screenshot-format <jpeg|png|webp>`: override the default format
used by `take_screenshot` when the caller does not specify one
- `--screenshot-quality <0-100>`: override the default JPEG/WebP
quality. Ignored for PNG
- `--screenshot-max-width <px>`: downscale screenshots wider than this
before they are returned
- `--screenshot-max-height <px>`: downscale screenshots taller than
this. Combines with `--screenshot-max-width`; the smaller scale wins so
both bounds are respected while preserving aspect ratio
For the exact error in #879, the recipe is `--screenshot-max-width=8000
--screenshot-max-height=8000` (or a smaller value such as `2000` if many
images may end up in the same request, depending on the operator's
chosen API).
## Implementation
- Resizing leverages Puppeteer's `clip.scale` (CDP
`Page.captureScreenshot`), so **no new dependencies**.
- Source dimensions per capture mode:
- viewport: `page.viewport()`
- full page: `document.documentElement.scrollWidth/scrollHeight` via
`page.evaluate()`
- element (`uid`): `elementHandle.boundingBox()`
- For element and full-page captures with a downscale clip, the call
routes through `page.screenshot({clip})` so the scale parameter applies.
`captureBeyondViewport` is left to Puppeteer's default (`true` when a
clip is set), preserving correct behavior for elements below the fold
and full-page captures.
- ~150 lines of source code, ~200 lines of new tests.
## Backwards compatibility
**Fully opt-in**: when no flags are set, `take_screenshot` returns the
exact same bytes as before. No behavioral change for existing users.
## Design alignment
- Aligned with the **"Reference over Value"** principle in
`docs/design-principles.md`: the existing 2 MB threshold still routes
oversized screenshots to a temporary file. This change only reduces the
size of the **inline base64 fallback path**, which the principles
document calls out as an acceptable exception when MCP clients display
images natively.
- The MCP server **hardcodes no LLM-specific size limits**. Operators
pick the values that match their client/model combination. This keeps
the maintenance surface here minimal as model limits evolve, and is
intended as a **complement to, not a replacement for**, fixes in the MCP
client itself.
## Addressing concerns raised in #879
> "It's not feasible for us to maintain this. Limits will change when
models change." (@natorion)
The flags are pure parameters; nothing about the upstream LLM is encoded
in the server. When a vendor raises (or lowers) a limit, no code change
is needed here, only the operator's CLI args change.
> "`filePath` / `page_resize` already work as a workaround." (@OrKoN)
`filePath` is great when the call site knows it's about to take a huge
screenshot, but as you noted earlier in the thread, an oversized image
already in the request history keeps causing failures even on subsequent
calls. `page_resize` works but mutates the page being debugged. The
resize in this PR happens **between Puppeteer and the MCP response**, so
the inspected page is untouched and the failure mode is prevented at the
source.
> "Should be fixed client side."
Agreed, this PR is intended as a complement, not a substitute. A
client-side fix (e.g. compaction evicts/downsamples old images) handles
the cumulative case for *any* MCP. A server-side cap handles the
per-call dimension limit for users who hit it before compaction can kick
in. The two address overlapping but distinct failure modes.
Happy to drop or rework any of this if the maintainers prefer a
different shape, for example making the threshold automatic from a
single `--max-image-bytes` knob, or rejecting the PR entirely in favor
of waiting for a client-side fix. Just wanted to put a concrete option
on the table.
## Tests
Added 6 new tests:
- `honors screenshotFormat default from CLI args`
- `keeps "png" as default format when no CLI override is set`
- `downscales viewport screenshot when screenshotMaxWidth is set`
- `downscales using the smaller scale when both max-width and max-height
are set`
- `does not resize when source is smaller than the max bounds`
- `downscales full page screenshot when screenshotMaxWidth is set`
All 627 tests in the suite pass. `npm run typecheck` and `npm run
check-format` are clean.
## Notes for reviewers
- The dimensions compared against `--screenshot-max-width/height` are
**CSS pixels** (`page.viewport()`), not raw bitmap pixels. With
`deviceScaleFactor > 1` (HiDPI emulation) the actual bitmap may still be
larger. Happy to clarify this in the option description if preferred.
- For element captures with a downscale clip, the call routes through
`page.screenshot({clip})` instead of `element.screenshot()`. Same-frame
elements are correct (boundingBox returns main-frame coords). I have
**not** exercised this path against cross-origin iframe elements; let me
know if you'd like a fallback there.
- The PR is currently in **Draft** state pending CLA verification and
any feedback on the framing above.
Refs #879
Closes https://github.com/ChromeDevTools/chrome-devtools-mcp/issues/879
Adding the get_heapsnapshot_dominators MCP tool to show the dominators
for a given node. In combination with get_heapsnapshot_retaining_paths
this should help understand what keeps an object reachable and thus
alive.
Co-authored-by: Dominik Inführ <dinfuehr@chromium.org>
Co-authored-by: Nicholas Roscino <nroscino@google.com>
## Summary
`screencast_stop` returns an empty response when no recording is active,
making it impossible for the calling agent to distinguish "stopped
successfully" from "nothing was recording."
`screencast_start` already handles its inverse case with an explicit
error (`"a screencast recording is already in progress"`), so this makes
`stop` consistent.
## Change
Added an error message when `screencast_stop` is called without an
active recording:
```ts
if (!data) {
response.appendResponseLine(
'Error: no active screencast recording to stop.',
);
return;
}
```
## Before
Empty tool response — agent cannot tell what happened.
## After
`Error: no active screencast recording to stop.`
---------
Co-authored-by: Nicholas Roscino <nroscino@google.com>
Alex Rudenko