## 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.
## Summary
- Input tools (`click`, `fill`, `press_key`, `hover`, `drag`,
`type_text`, `fill_form`, `click_at`) and `evaluate_script` now append a
`Page navigated to <url>.` line to the response when the action triggers
a cross-document navigation.
- `WaitForHelper.waitForEventsAfterAction` returns `{navigated:
boolean}` instead of `void`, surfacing the navigation signal that was
already being detected internally.
- No change to `navigate_page` or `new_page` since they already report
the URL explicitly.
Fixes#243
## Why
Today, if a `click` causes a page navigation, the response says
*"Successfully clicked on the element"* with no indication that the page
URL changed. The agent has to make an extra `list_pages` call to
discover where it landed. This saves that round-trip for every
navigation-triggering action.
## Design
The existing `waitForNavigationStarted` in `WaitForHelper` already knows
whether a cross-document navigation started. We propagate that signal as
`{navigated: boolean}` through the return value of
`waitForEventsAfterAction` → `McpPage` → `ContextPage` interface, and
let each handler append the URL line when `navigated` is true.
Same-document (history API) navigations remain filtered out by the
existing `waitForNavigationStarted` logic, matching current behavior.
Click-opens-new-tab is a separate concern (#367).
## Test plan
- [x] New test: click on a link that causes navigation → response
includes `Page navigated to <url>.`
- [x] New test: click on a button that doesn't navigate → no navigation
line in response
- [x] Full test suite (563 tests) passes
- [x] TypeScript typecheck clean
- [x] ESLint + Prettier clean
Fixes#1942
Verified using `npm run eval --
scripts/eval_scenarios/fill_select_and_checkboxes_test.ts`
Without this change, I observed 7 runs using fill_form for all controls
at once, 14 runs using click to select checkboxes and 10 runs that did
nothing (total 31 runs)
After this change: 9 fill_form using runs (passes), 1 click based
approach and 10 no-attempt fails (20 runs total)
Depending how we count the no-attempt runs, its either increase from 23%
to 45% or 33% to 90% in eval pass rate.
Co-authored-by: Piotr Paulski <piotrpaulski@chromium.org>
## Summary
Teach the `click` tool to handle accessibility snapshot targets that
point at an `option` inside a native, single-select `<select>` element.
When the target is a native select option, `click` now selects that
option through the owning `<select>` element instead of asking Puppeteer
to click an option node that has no clickable box while the dropdown is
collapsed. Other option-like targets, including custom ARIA options,
continue through the normal click path.
## Motivation
Fixes#1941.
The text snapshot can expose native `<option>` nodes with stable uids.
This makes it natural for an agent to call `click` on an option uid
after seeing the desired option in the snapshot.
However, a collapsed native `<select>` does not expose its child
`<option>` nodes as directly clickable page boxes. In that state, the
option can exist in the DOM and accessibility tree while still having no
clickable geometry. Puppeteer therefore waits for the option to become
interactive and eventually times out.
The existing `fill` tool already handles selecting native `<select>`
options. This change keeps that guidance explicit in the `click` tool
description, while also making `click(option_uid)` robust for the native
select case that appears in the snapshot.
## Changes
- Added a constrained native select fallback in `click`:
- Only runs for single-click requests.
- Only runs when the snapshot node role is `option`.
- Only handles real `HTMLOptionElement` nodes owned by a native
`<select>`.
- Does not handle disabled selects, disabled options, disabled
optgroups, or multi-selects.
- Falls back to the existing locator click behavior for all other
targets.
- Dispatches `input` and `change` events when selecting a different
native option.
- Updated the `click` tool description to steer agents toward `fill` for
native `<select>` option selection.
- Regenerated CLI/tool reference docs with `npm run gen`.
- Added regression coverage for:
- Clicking an option uid in a collapsed native `<select>`.
- Clicking an option uid inside a native `<optgroup>`.
- Clicking a custom ARIA `role="option"` element through the normal
click path.
## Test Plan
Passed:
```bash
npm run test -- tests/tools/input.test.ts
npm run check-format
```
Also attempted:
```bash
npm run test
```
The full suite hit unrelated local failures outside this change area:
- `tests/tools/network.test.ts`: snapshot ordering difference for
redirected requests.
- `tests/tools/screenshot.test.ts`: `Page.captureScreenshot` failed for
the large full-page screenshot case with `Page is too large`.
The changed input tool tests pass locally.
## Risk
Low. The fallback is intentionally narrow:
- It is gated by the accessibility role being `option`.
- It verifies the DOM node is an actual `HTMLOptionElement`.
- It only applies to native, non-disabled, single-select `<select>`
controls.
- It does not reinterpret custom combobox/listbox implementations.
- It preserves the existing locator click path for non-native option
targets.
The main behavior change is that `click(option_uid)` can now succeed for
native collapsed dropdowns that previously timed out.
## Related Issue
Closes#1941.
## Maintainer Context
This targets a small but high-impact mismatch between the snapshot
representation and browser interaction semantics.
The snapshot correctly exposes native options because they exist in the
accessibility tree. The click implementation previously treated that uid
like any other clickable element, but collapsed native options do not
have normal clickable layout boxes. This PR makes the native select case
explicit without expanding `click` into a general custom dropdown
heuristic.
The tool description still recommends `fill` for native `<select>`
selection so agents are guided toward the more direct tool. The code
fallback exists for the common case where an agent already selected an
option uid from the snapshot.
Improvements for handling in-page tool responses. In order to
successfully pass an in-page tool response from the page context to the
MCP server, the response needs to be serializable. The code walks the
response object an performs the following changes:
- DOM elements are stashed onto the window object and replaced with an
ID. On the MCP server side this ID is used to map back to the
corresponding UID in the page snapshot generated from the accessibility
tree.
- Circular references are replaced with a string.
- Class instances (which can be complex or non-serializable) are
replaced with a string.
- Functions are replaced with a string.
If the in-page tool response contains DOM elements which are not part of
the page snapshot, a new snapshot is created add the missing elements
are added explicitly.
- `chrome-devtools-mcp.js` is the `npx chrome-devtools-mcp`
- `chrome-devtools.js` is the new CLI
- `-cli-options.js` is the corresponding options
- all these files are in the bin folder to indicate they are executable
- renamed getSelectedPage to getSelectedPptrPage
- removes getSelectedPptrPage from tool interfaces
- moves dialog handling to McpPage
- makes responses to be optionally McpPage-scoped
`type_text` is useful for pages that do not have a11y and, thus, uid is
not known. It's also useful for testing user-like keyboard input and
testing the focus state changes. The `fill` tools force the focus change
and require an uid and therefore are not suitable for these tasks.
I split this PR off from my "create one DevTools universe per page" PR
in preparation. This allows tests to re-use browser instances without
creating an `McpContext`.
Drive-by: Move mocked browser/page into utils.ts.
This PR prevents license notices being dropped when creating package for
publication.
This can happen when first import in the file is type-only import that
gets removed during build. When there is no empty line between the
license block comment and such import, the comment is treated as related
to the import and gets removed alongside it.
Adding an empty line between copyright notice and the import fixes the
issue.
Co-authored-by: Piotr Paulski <piotrpaulski@chromium.org>
Liohtml