fix: resolve page ids only among listed pages (#2332)

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

src/McpContext.ts

@@ -509,9 +509,11 @@ export class McpContext implements Context {
   }
 
   getPageById(pageId: number): McpPage {
-    const page = Array.from(this.#mcpPages.values()).find(
-      mcpPage => mcpPage.id === pageId,
-    );
+    // Resolve only among listed pages (`getPages()`), so an id that
+    // `list_pages` never showed cannot be targeted (e.g. a `devtools://`
+    // frontend, which the listing excludes unless `experimentalDevToolsDebugging`
+    // is set).
+    const page = this.getPages().find(mcpPage => mcpPage.id === pageId);
     if (!page) {
       throw new Error('No page found');
     }
@@ -640,8 +642,7 @@ export class McpContext implements Context {
 
     // Only fall back when the selected page is actually gone. Gating on
     // `isClosed()` instead of `pages` membership avoids silently swapping a
-    // live page that is momentarily missing from the snapshot, e.g., a
-    // `devtools://` page, which is selectable but filtered out of `pages` above.
+    // live page that is momentarily missing from the snapshot.
     this.#selectedPageFallback = undefined;
     if (
       (!this.#selectedPage || this.#selectedPage.pptrPage.isClosed()) &&

tests/McpContext.test.ts

@@ -96,6 +96,26 @@ describe('McpContext', () => {
         const page = await context.newPage();
         await context.createPagesSnapshot();
         assert.ok(page.devToolsPage);
+
+        // A devtools page is tracked but excluded from the listing, so its id
+        // must not resolve through `getPageById()` (which backs `select_page`
+        // and every other pageId tool). A second snapshot guarantees the
+        // devtools page is enrolled.
+        await context.createPagesSnapshot();
+        const listed = context.getPages();
+        assert.ok(
+          listed.every(
+            mcpPage => !mcpPage.pptrPage.url().startsWith('devtools://'),
+          ),
+          'listing should exclude devtools pages',
+        );
+        const listedIds = new Set(listed.map(mcpPage => mcpPage.id));
+        for (let id = 1; id < 30; id++) {
+          if (listedIds.has(id)) {
+            continue;
+          }
+          assert.throws(() => context.getPageById(id), /No page found/);
+        }
       },
       {
         autoOpenDevTools: true,