fix: list_pages should work after selected page is closed#1145
fix: list_pages should work after selected page is closed#1145zyzyzyryxy merged 3 commits intoChromeDevTools:mainfrom
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. |
|
Thank you for your contribution! The fix looks good, however we need your CLA status cleared first and there are some tests that need updating. |
list_pages was defined with definePageTool which marks it as pageScoped. This causes the handler dispatch in index.ts to call getSelectedMcpPage() before invoking the handler. When the selected page has been closed, getSelectedPptrPage() throws an error instead of returning the page list. list_pages doesn't actually use the page parameter — its handler only calls response.setIncludePages(true). Change it to defineTool so it bypasses the page-scoped check and works regardless of whether a page is currently selected. Fixes ChromeDevTools#1138
eefe843 to
98cc530
Compare
|
Thanks for the review! I've:
Please let me know if anything else needs updating. |
|
The cla/google check is still failing - please see its description for a checklist of what is still needed - I think it might be about different email being associated with your commits than what you provided. Also, tests in tests/tools/pages.test.ts need an update, they stopped compiling after your change. It should be enough to remove an extra page parameter in a few places. |
Adds a test that verifies list_pages works correctly when the currently selected page has been closed externally. This exercises the definePageTool → defineTool change that removes the page-scoped requirement from list_pages. Ref ChromeDevTools#1138
98cc530 to
e547ae8
Compare
|
Updated — removed the extra Regarding CLA: I'll check the email association and re-sign if needed. |
e547ae8 to
5a3e0f7
Compare
|
CLA seem to be sorted out, but there are still some checks failing. Please review them and fix. |
…ools#1145) ## Problem After closing the currently selected page, calling `list_pages` throws an error: ``` The selected page has been closed. Call list_pages to see open pages. ``` This creates a deadlock: the error message tells users to call `list_pages`, but `list_pages` itself throws the same error. ## Root Cause `list_pages` is defined with `definePageTool`, which marks it as `pageScoped: true`. The handler dispatch in `index.ts` (line ~186) calls `context.getSelectedMcpPage()` for all page-scoped tools **before** invoking the handler. When the selected page is closed, `getSelectedPptrPage()` throws. However, `list_pages` doesn't actually use the `page` parameter — its handler only calls `response.setIncludePages(true)`. ## Fix Change `list_pages` from `definePageTool` to `defineTool`. This bypasses the page-scoped check while preserving all existing behavior since the handler never used the page reference. ## Testing Reproduced the issue following the steps in ChromeDevTools#1138: 1. Call `list_pages` → returns page list ✅ 2. Close the selected page 3. Call `list_pages` → now returns updated page list instead of throwing ✅ Fixes ChromeDevTools#1138 --------- Co-authored-by: Piotr Paulski <piotrpaulski@chromium.org>
🤖 I have created a release *beep* *boop* --- ## [0.21.0](chrome-devtools-mcp-v0.20.3...chrome-devtools-mcp-v0.21.0) (2026-04-01) ### 🎉 Features * add a skill for detecting memory leaks using take_memory_snapshot tool ([#1162](#1162)) ([d19a235](d19a235)) ### 🛠️ Fixes * **cli:** avoid defaulting to isolated when userDataDir is provided ([#1258](#1258)) ([73e1e24](73e1e24)) * list_pages should work after selected page is closed ([#1145](#1145)) ([2664455](2664455)) * mark lighthouse and memory as non-read-only ([#1769](#1769)) ([bec9dae](bec9dae)) * **telemetry:** record client name ([9a47b65](9a47b65)) * versioning for Claude Code plugin ([#1233](#1233)) ([966b86f](966b86f)) * wrap .mcp.json config in mcpServers key ([#1246](#1246)) ([c7948cf](c7948cf)) ### 📄 Documentation * Command Code CLI instructions for MCP server ([0a7c0a7](0a7c0a7)) * provide disclaimer about supported browsers ([#1237](#1237)) ([8676b72](8676b72)) ### 🏗️ Refactor * set process titles for easier debugging ([#1770](#1770)) ([0fe3896](0fe3896)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: Alex Rudenko <alexrudenko@chromium.org>
🤖 I have created a release *beep* *boop* --- ## [0.21.0](chrome-devtools-mcp-v0.20.3...chrome-devtools-mcp-v0.21.0) (2026-04-01) ### 🎉 Features * add a skill for detecting memory leaks using take_memory_snapshot tool ([#1162](#1162)) ([d19a235](d19a235)) ### 🛠️ Fixes * **cli:** avoid defaulting to isolated when userDataDir is provided ([#1258](#1258)) ([73e1e24](73e1e24)) * list_pages should work after selected page is closed ([#1145](#1145)) ([2664455](2664455)) * mark lighthouse and memory as non-read-only ([#1769](#1769)) ([bec9dae](bec9dae)) * **telemetry:** record client name ([9a47b65](9a47b65)) * versioning for Claude Code plugin ([#1233](#1233)) ([966b86f](966b86f)) * wrap .mcp.json config in mcpServers key ([#1246](#1246)) ([c7948cf](c7948cf)) ### 📄 Documentation * Command Code CLI instructions for MCP server ([0a7c0a7](0a7c0a7)) * provide disclaimer about supported browsers ([#1237](#1237)) ([8676b72](8676b72)) ### 🏗️ Refactor * set process titles for easier debugging ([#1770](#1770)) ([0fe3896](0fe3896)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: Alex Rudenko <alexrudenko@chromium.org>
Problem
After closing the currently selected page, calling
list_pagesthrows an error:This creates a deadlock: the error message tells users to call
list_pages, butlist_pagesitself throws the same error.Root Cause
list_pagesis defined withdefinePageTool, which marks it aspageScoped: true. The handler dispatch inindex.ts(line ~186) callscontext.getSelectedMcpPage()for all page-scoped tools before invoking the handler. When the selected page is closed,getSelectedPptrPage()throws.However,
list_pagesdoesn't actually use thepageparameter — its handler only callsresponse.setIncludePages(true).Fix
Change
list_pagesfromdefinePageTooltodefineTool. This bypasses the page-scoped check while preserving all existing behavior since the handler never used the page reference.Testing
Reproduced the issue following the steps in #1138:
list_pages→ returns page list ✅list_pages→ now returns updated page list instead of throwing ✅Fixes #1138