fix: preserve unread page fallback across snapshots and deduplicate page disposal (#3161)

- Preserves an unread selected-page fallback across consecutive
`createPagesSnapshot()` calls until `getSelectedPageFallback()` consumes
it.
- Page 2 is selected. A click or evaluate_script action opens a new tab
and closes Page 2 (window.close()).
- McpResponse.handle sees this.#attachedWaitForResult?.openedPages &&
!this.#includePages and calls await context.createPagesSnapshot().
- That snapshot sees Page 2 is closed, silently switches #selectedPage
to Page 1, and records #selectedPageFallback = {wasClosed: true} — but
click does not format the page list (#includePages is false), so
getSelectedPageFallback() is not read.
- When the agent next calls list_pages, createPagesSnapshot() overwrites
#selectedPageFallback = undefined, so the fallback note is never shown.
- Clears any pending fallback when `selectPage()` is called explicitly.
- Deduplicates `McpPage` disposal in `McpContext.dispose()` via
`#allMcpPages()` so the selected page is not disposed twice.
- Checking `this.#selectedPage?.target === target` mirrors
McpContext.#createMcpPage() so #isPageTarget() still reads
this.#selectedPage.url() (via pptrPage.url()) rather than falling back
to a potentially stale target.url().

Extracted from another PR where it will be re-used

src/McpContext.ts

@@ -105,6 +105,7 @@ export class McpContext implements Context {
   #mcpPages = new Map<Target, McpPage>();
   #selectedPage?: McpPage;
   #selectedPageFallback?: {wasClosed: boolean};
+  #selectedPageFallbackRead = false;
 
   #serviceWorkerConsoleCollector: ServiceWorkerConsoleCollector;
 
@@ -154,15 +155,22 @@ export class McpContext implements Context {
     this.browser.on('targetdestroyed', this.#onTargetDestroyed);
   }
 
+  #allMcpPages(): Set<McpPage> {
+    const pages = new Set(this.#mcpPages.values());
+    if (this.#selectedPage) {
+      pages.add(this.#selectedPage);
+    }
+    return pages;
+  }
+
   dispose(): void {
     this.browser.off('targetcreated', this.#onTargetCreated);
     this.browser.off('targetdestroyed', this.#onTargetDestroyed);
 
     this.#serviceWorkerConsoleCollector.dispose();
-    for (const mcpPage of this.#mcpPages.values()) {
+    for (const mcpPage of this.#allMcpPages()) {
       mcpPage.dispose();
     }
-    this.#selectedPage?.dispose();
     this.#mcpPages.clear();
     this.#workers.clear();
     // Isolated contexts are intentionally not closed here.
@@ -500,6 +508,12 @@ export class McpContext implements Context {
   }
 
   selectPage(newPage: McpPage): void {
+    this.#selectedPageFallback = undefined;
+    this.#selectedPageFallbackRead = false;
+    this.#selectPage(newPage);
+  }
+
+  #selectPage(newPage: McpPage): void {
     if (
       this.#selectedPage &&
       this.#selectedPage !== newPage &&
@@ -514,10 +528,12 @@ export class McpContext implements Context {
   /**
    * Returns details about the last page snapshot automatically replacing the
    * selection because the selected page disappeared from the page list, or
-   * `undefined` if the snapshot left the selection intact. Recomputed on every
-   * createPagesSnapshot() call.
+   * `undefined` if the snapshot left the selection intact. Cleared on the next
+   * createPagesSnapshot() call after it has been read, or when selectPage() is
+   * called explicitly.
    */
   getSelectedPageFallback(): {wasClosed: boolean} | undefined {
+    this.#selectedPageFallbackRead = true;
     return this.#selectedPageFallback;
   }
 
@@ -622,7 +638,10 @@ 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.
-    this.#selectedPageFallback = undefined;
+    if (this.#selectedPageFallbackRead) {
+      this.#selectedPageFallback = undefined;
+      this.#selectedPageFallbackRead = false;
+    }
     if ((!this.#selectedPage || this.#selectedPage.isClosed()) && pages[0]) {
       // Record the automatic change so the response can surface it. Skipped on
       // first connect, when there was no prior selection to replace.
@@ -631,7 +650,7 @@ export class McpContext implements Context {
           wasClosed: this.#selectedPage.isClosed(),
         };
       }
-      this.selectPage(pages[0]);
+      this.#selectPage(pages[0]);
     }
 
     if (this.#selectedPage && !this.#selectedPage.isClosed()) {
@@ -642,7 +661,9 @@ export class McpContext implements Context {
   }
 
   #isPageTarget(target: Target): boolean {
-    const existingMcpPage = this.#mcpPages.get(target);
+    const existingMcpPage =
+      this.#mcpPages.get(target) ??
+      (this.#selectedPage?.target === target ? this.#selectedPage : undefined);
     const url = existingMcpPage ? existingMcpPage.url() : target.url();
     if (
       !this.#options.experimentalDevToolsDebugging &&

tests/McpContext.test.ts

@@ -19,10 +19,20 @@ import {McpContext} from '../src/McpContext.js';
 import {McpPage} from '../src/McpPage.js';
 import {HeapSnapshotManager} from '../src/processors/HeapSnapshotManager.js';
 import {TextSnapshot} from '../src/TextSnapshot.js';
-import {type HTTPResponse} from '../src/third_party/index.js';
+import {
+  type Browser,
+  type HTTPResponse,
+  type Target,
+  TargetType,
+} from '../src/third_party/index.js';
 import type {TraceResult} from '../src/processors/PerformanceTrace.js';
 import {resolveCanonicalPath} from '../src/utils/files.js';
 
+import {
+  createMockPuppeteerBrowser,
+  createMockPuppeteerPage,
+  createMockPuppeteerTarget,
+} from './mocks.js';
 import {serverHooks, TestServer} from './server.js';
 import {
   assertNoServiceWorkerReported,
@@ -50,6 +60,34 @@ describe('McpContext', () => {
     sinon.restore();
   });
 
+  function createPageTarget() {
+    const page = createMockPuppeteerPage();
+    page.emulateFocusedPage.resolves();
+    const target = createMockPuppeteerTarget({page});
+    target.type.returns(TargetType.PAGE);
+    return {page, target};
+  }
+
+  async function createMockContext(options?: {
+    browser?: sinon.SinonStubbedInstance<Browser>;
+    targets?: Target[];
+  }) {
+    const browser = options?.browser ?? createMockPuppeteerBrowser();
+    const {target: defaultTarget} = createPageTarget();
+    browser.targets.returns(options?.targets ?? [defaultTarget]);
+    browser.browserContexts.returns([]);
+    return await McpContext.from(
+      browser,
+      undefined,
+      {
+        experimentalDevToolsDebugging: false,
+        performanceCrux: true,
+        heapSnapshotManager: new HeapSnapshotManager(),
+      },
+      Locator,
+    );
+  }
+
   it('list pages', async () => {
     await withMcpContext(async (_response, context) => {
       const page = context.getSelectedMcpPage();
@@ -297,6 +335,134 @@ describe('McpContext', () => {
     });
   });
 
+  it('preserves an unread fallback across snapshots until read', async () => {
+    const browser = createMockPuppeteerBrowser();
+    const {target: firstTarget} = createPageTarget();
+    const {page: secondPptrPage, target: secondTarget} = createPageTarget();
+    const context = await createMockContext({
+      browser,
+      targets: [firstTarget, secondTarget],
+    });
+    const [, secondPage] = context.getPages();
+    assert.ok(secondPage);
+    context.selectPage(secondPage);
+    await secondPage.init();
+
+    secondPptrPage.isClosed.returns(true);
+    browser.targets.returns([firstTarget]);
+
+    // First snapshot detects the fallback.
+    await context.createPagesSnapshot();
+    // Second snapshot runs before the fallback has been read by the response
+    // formatter.
+    await context.createPagesSnapshot();
+
+    assert.deepStrictEqual(context.getSelectedPageFallback(), {
+      wasClosed: true,
+    });
+
+    await context.createPagesSnapshot();
+    assert.strictEqual(context.getSelectedPageFallback(), undefined);
+  });
+
+  it('overwrites an unread fallback when a second fallback occurs before it is read', async () => {
+    const browser = createMockPuppeteerBrowser();
+    const {page: firstPptrPage, target: firstTarget} = createPageTarget();
+    const {target: secondTarget} = createPageTarget();
+    const {page: thirdPptrPage, target: thirdTarget} = createPageTarget();
+    const context = await createMockContext({
+      browser,
+      targets: [firstTarget, secondTarget, thirdTarget],
+    });
+    const [firstPage, secondPage, thirdPage] = context.getPages();
+    assert.ok(firstPage);
+    assert.ok(secondPage);
+    assert.ok(thirdPage);
+    context.selectPage(thirdPage);
+    await thirdPage.init();
+
+    // First snapshot: thirdPage triggers a fallback to firstPage (recording
+    // wasClosed: false on the third isClosed() call so it is distinguishable
+    // from the second fallback).
+    thirdPptrPage.isClosed
+      .onFirstCall()
+      .returns(true)
+      .onSecondCall()
+      .returns(true)
+      .returns(false);
+    browser.targets.returns([firstTarget, secondTarget]);
+    await context.createPagesSnapshot();
+    assert.ok(context.isPageSelected(firstPage));
+
+    // Second snapshot before getSelectedPageFallback() is read: the newly
+    // selected firstPage also closes and falls back to secondPage.
+    firstPptrPage.isClosed.returns(true);
+    browser.targets.returns([secondTarget]);
+    await context.createPagesSnapshot();
+    assert.ok(context.isPageSelected(secondPage));
+
+    assert.deepStrictEqual(context.getSelectedPageFallback(), {
+      wasClosed: true,
+    });
+
+    await context.createPagesSnapshot();
+    assert.strictEqual(context.getSelectedPageFallback(), undefined);
+  });
+
+  it('clears the fallback when selectPage is called explicitly', async () => {
+    const browser = createMockPuppeteerBrowser();
+    const {target: firstTarget} = createPageTarget();
+    const {page: secondPptrPage, target: secondTarget} = createPageTarget();
+    const context = await createMockContext({
+      browser,
+      targets: [firstTarget, secondTarget],
+    });
+    const [firstPage, secondPage] = context.getPages();
+    assert.ok(firstPage);
+    assert.ok(secondPage);
+    context.selectPage(secondPage);
+    await secondPage.init();
+
+    secondPptrPage.isClosed.returns(true);
+    browser.targets.returns([firstTarget]);
+    await context.createPagesSnapshot();
+
+    context.selectPage(firstPage);
+    assert.strictEqual(context.getSelectedPageFallback(), undefined);
+  });
+
+  it('disposes the selected page only once when context is disposed', async () => {
+    const context = await createMockContext();
+    const page = context.getSelectedMcpPage();
+    const disposeSpy = sinon.spy(page, 'dispose');
+
+    context.dispose();
+
+    sinon.assert.calledOnceWithExactly(disposeSpy);
+  });
+
+  it('disposes a selected page that closed after being pruned from the snapshot', async () => {
+    const browser = createMockPuppeteerBrowser();
+    const {page: pptrPage, target} = createPageTarget();
+    const context = await createMockContext({
+      browser,
+      targets: [target],
+    });
+    const page = context.getSelectedMcpPage();
+    const disposeSpy = sinon.spy(page, 'dispose');
+
+    // Prune the target from #mcpPages while the selected page is still open.
+    browser.targets.returns([]);
+    await context.createPagesSnapshot();
+    sinon.assert.notCalled(disposeSpy);
+
+    // The selected page closes before another snapshot runs.
+    pptrPage.isClosed.returns(true);
+    context.dispose();
+
+    sinon.assert.calledOnceWithExactly(disposeSpy);
+  });
+
   it('does not report a fallback for a regular selection', async () => {
     await withMcpContext(async (_response, context) => {
       await context.newPage();