Skip to content

fix: resolve page ids only among listed pages#2332

Merged
OrKoN merged 2 commits into
ChromeDevTools:mainfrom
thomasbachem:fix/getpagebyid-respect-pages
Jul 10, 2026
Merged

fix: resolve page ids only among listed pages#2332
OrKoN merged 2 commits into
ChromeDevTools:mainfrom
thomasbachem:fix/getpagebyid-respect-pages

Conversation

@thomasbachem

@thomasbachem thomasbachem commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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

@OrKoN

OrKoN commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR. I think the existence of #pages and #mcpPages is rather and oversight and the root cause of this issue and I will fix it by refactoring the logic accordingly so this fix would not be needed.

@OrKoN

OrKoN commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

I believe this would be addressed by #2333 but I think we can still land the test from this PR if you would be up to rebasing.

@thomasbachem
thomasbachem force-pushed the fix/getpagebyid-respect-pages branch from 3bda50d to 4ac019e Compare July 9, 2026 15:01
@thomasbachem

Copy link
Copy Markdown
Contributor Author

Rebased. The refactor does simplify this a lot – after #2333 the fix is a one-liner. But the test still fails on current main without it: getPageById() searches all of #mcpPages, while getPages() – what list_pages shows – filters devtools:// frontends out. So an unlisted id still resolves, and a test-only PR would be red.

I've kept the minimal change of resolving through getPages() so the two agree and the test lands green – happy to drop it if you'd rather fold that into a follow-up.

@OrKoN
OrKoN enabled auto-merge July 9, 2026 15:18
@thomasbachem

Copy link
Copy Markdown
Contributor Author

Thanks! Before I touch the approved branch: the red CI is two things, one of them mine.

macOS/Node 26: My new test was a second autoOpenDevTools test. Those share a cached browser (withBrowser keys on launch options) and close the previous test's pages on entry – tearing down the earlier devtools test's still-attaching DevTools target (the Target.attachToTarget unhandledRejection). Couldn't repro locally in 40 runs, but the race is there in every run – it just needs unlucky timing. Fixed by folding my assertions into the existing devtools test – with a single autoOpenDevTools test, nothing closes its pages mid-run.

Windows/Node 22: Unrelated timing flakes (exits within budget on stdin EOF, emulates viewport) – same class as the timeouts on main's last two runs.

Rebased onto latest main as well. OK to force-push over your approval?

@OrKoN
OrKoN force-pushed the fix/getpagebyid-respect-pages branch from 4ac019e to 487beba Compare July 10, 2026 09:48
auto-merge was automatically disabled July 10, 2026 10:17

Head branch was pushed to by a user without write access

@thomasbachem

Copy link
Copy Markdown
Contributor Author

Pushed the test fold from my comment above on top of your rebase – one autoOpenDevTools test again, so the teardown race from the macOS run can't recur.

@OrKoN
OrKoN enabled auto-merge July 10, 2026 10:54
@OrKoN
OrKoN added this pull request to the merge queue Jul 10, 2026
Merged via the queue into ChromeDevTools:main with commit eb04951 Jul 10, 2026
27 of 29 checks passed
@thomasbachem
thomasbachem deleted the fix/getpagebyid-respect-pages branch July 10, 2026 11:30
kku1993 pushed a commit to kku1993/chrome-devtools-mcp that referenced this pull request Jul 14, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.6.0](ChromeDevTools/chrome-devtools-mcp@chrome-devtools-mcp-v1.5.0...chrome-devtools-mcp-v1.6.0)
(2026-07-14)


### 🎉 Features

* add experimentalGcfFormat flag for GCF-encoded tool responses
([ChromeDevTools#2235](ChromeDevTools#2235))
([3d21389](ChromeDevTools@3d21389))
* Print object count and total sizes in get_heapsnapshot_details
([ChromeDevTools#2325](ChromeDevTools#2325))
([15a6b78](ChromeDevTools@15a6b78))
* support --allow-unrestricted-paths configuration
([ChromeDevTools#2296](ChromeDevTools#2296))
([6e56c02](ChromeDevTools@6e56c02))
* Support filter with heap snapshots aggregates
([ChromeDevTools#2323](ChromeDevTools#2323))
([2812902](ChromeDevTools@2812902))
* update Lighthouse to 13.4.0
([ChromeDevTools#2317](ChromeDevTools#2317))
([ffc6060](ChromeDevTools@ffc6060))


### 🛠️ Fixes

* enforce .gz instead of json.gz in performance tools
([ChromeDevTools#2305](ChromeDevTools#2305))
([b06e39b](ChromeDevTools@b06e39b))
* keep a still-open selected page instead of falling back to the first
page
([ChromeDevTools#2328](ChromeDevTools#2328))
([c645eee](ChromeDevTools@c645eee)),
closes
[ChromeDevTools#2304](ChromeDevTools#2304)
* keep page ids unique across browser reconnects
([ChromeDevTools#2345](ChromeDevTools#2345))
([3e8d922](ChromeDevTools@3e8d922))
* paginate page 0 in list_network_requests and list_console_messages
([ChromeDevTools#2359](ChromeDevTools#2359))
([d0025b3](ChromeDevTools@d0025b3))
* release held modifiers when press_key key event fails
([ChromeDevTools#2347](ChromeDevTools#2347))
([78ccb19](ChromeDevTools@78ccb19))
* report when the selected page was auto-replaced by the fallback
([ChromeDevTools#2308](ChromeDevTools#2308))
([2c16ac3](ChromeDevTools@2c16ac3)),
closes
[ChromeDevTools#2304](ChromeDevTools#2304)
* resolve page ids only among listed pages
([ChromeDevTools#2332](ChromeDevTools#2332))
([eb04951](ChromeDevTools@eb04951)),
closes
[ChromeDevTools#2304](ChromeDevTools#2304)
* **snapshot:** resolve element ids on the correct snapshot
([ChromeDevTools#2295](ChromeDevTools#2295))
([b703f2c](ChromeDevTools@b703f2c))
* **telemetry:** resolve enum values through nested schema wrappers
([ChromeDevTools#2315](ChromeDevTools#2315))
([c065fd9](ChromeDevTools@c065fd9))
* Wait until daemon is started
([ChromeDevTools#2327](ChromeDevTools#2327))
([ed7e95d](ChromeDevTools@ed7e95d))


### 📄 Documentation

* add Grok Build CLI configuration section
([ChromeDevTools#2294](ChromeDevTools#2294))
([aa4be07](ChromeDevTools@aa4be07))
* update memory leak debugging skill
([ChromeDevTools#2330](ChromeDevTools#2330))
([c1736a0](ChromeDevTools@c1736a0))


### ⚡ Performance

* concurrent I/O in Root Path Resolution
([ChromeDevTools#2279](ChromeDevTools#2279))
([b2c63e6](ChromeDevTools@b2c63e6))


### 🏗️ Refactor

* clean up McpContext getters
([ChromeDevTools#2340](ChromeDevTools#2340))
([5b33deb](ChromeDevTools@5b33deb))
* clean up more of the context interface
([ChromeDevTools#2335](ChromeDevTools#2335))
([9cd734b](ChromeDevTools@9cd734b))
* clean up page management
([ChromeDevTools#2333](ChromeDevTools#2333))
([16db01f](ChromeDevTools@16db01f))
* clean up page snapshot generation
([ChromeDevTools#2348](ChromeDevTools#2348))
([68cfce2](ChromeDevTools@68cfce2))
* make collectors work per page
([ChromeDevTools#2324](ChromeDevTools#2324))
([9bc61b4](ChromeDevTools@9bc61b4))
* move and rename files
([ChromeDevTools#2355](ChromeDevTools#2355))
([9c3542b](ChromeDevTools@9c3542b))
* move DevTools universe to McpPage
([ChromeDevTools#2341](ChromeDevTools#2341))
([c006c9b](ChromeDevTools@c006c9b))
* move remaining McpContext getters
([ChromeDevTools#2342](ChromeDevTools#2342))
([58ba174](ChromeDevTools@58ba174))
* remove isolated context getter
([ChromeDevTools#2336](ChromeDevTools#2336))
([8a4ddb3](ChromeDevTools@8a4ddb3))
* Use array instead of Map for idToClassKey
([ChromeDevTools#2321](ChromeDevTools#2321))
([ff53b7b](ChromeDevTools@ff53b7b))
* use helper for Dialog handle
([ChromeDevTools#2334](ChromeDevTools#2334))
([64005f9](ChromeDevTools@64005f9))
* use response page in formatting
([ChromeDevTools#2349](ChromeDevTools#2349))
([c53c1ec](ChromeDevTools@c53c1ec))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants