fix: keep page ids unique across browser reconnects (#2345)

Closes #2339

Reconnecting after a browser restart builds a fresh `McpContext`, which
restarted the page id counter at 1 – ids from before the restart
silently resolved to unrelated pages of the new browser (repro in
#2339).

Two changes:

- The new context continues the id counter where the previous one left
off (`startingPageId` option), so a stale id now fails with the existing
"No page found" error and the agent re-lists
- The first response after a reconnect carries a one-time note ("the
browser was restarted or reconnected since the last call. Page ids have
changed..."), like the #2308 fallback note. Since tool errors run
through the same response formatting, the note shows up together with
the very error the stale id produces

Verified against the #2339 repro: the stale `select_page` now returns
the note plus "No page found", and the next listing shows the new
browser's pages under fresh ids.

One thing I noticed but left alone: The replaced context isn't
`dispose()`d on reconnect – pre-existing, and everything it holds is
tied to the dead browser anyway. Happy to add that here if you'd like.

Author: thomasbachem

src/McpContext.ts

@@ -55,8 +55,17 @@ interface McpContextOptions {
   // capability. When false (default), file-writing tools are restricted to the
   // OS temp directory. When true, the previous permissive behavior is restored.
   allowUnrestrictedPaths?: boolean;
+  // Whether this context replaces a previous one after a browser reconnect.
+  // Surfaces a one-time note in the next response.
+  reconnected?: boolean;
 }
 
+// Page ids are handed out from a process-wide counter so they stay unique
+// across all contexts, in particular across browser reconnects. An id issued
+// before a reconnect then fails to resolve instead of hitting an unrelated
+// page of the reconnected browser.
+let nextPageId = 1;
+
 export class McpContext implements Context {
   browser: Browser;
   logger: Logger;
@@ -78,7 +87,7 @@ export class McpContext implements Context {
   #screenRecorderData: {recorder: ScreenRecorder; filePath: string} | null =
     null;
 
-  #nextPageId = 1;
+  #reconnectNotice = false;
   #extensionPages = new WeakMap<Target, Page>();
 
   #extensionServiceWorkerMap = new WeakMap<Target, string>();
@@ -109,6 +118,7 @@ export class McpContext implements Context {
     this.#locatorClass = locatorClass;
     this.#options = options;
     this.#allowUnrestrictedPaths = options.allowUnrestrictedPaths ?? false;
+    this.#reconnectNotice = options.reconnected ?? false;
 
     this.#serviceWorkerConsoleCollector = new ServiceWorkerConsoleCollector(
       this.browser,
@@ -185,6 +195,10 @@ export class McpContext implements Context {
     return context;
   }
 
+  static resetPageIdsForTesting(): void {
+    nextPageId = 1;
+  }
+
   roots(): Root[] {
     return [
       ...(this.#roots ?? []),
@@ -358,6 +372,17 @@ export class McpContext implements Context {
     return page;
   }
 
+  /**
+   * Returns true once if this context was created by reconnecting after the
+   * previous browser connection was lost, so the next response can surface a
+   * note. Cleared on first call.
+   */
+  consumeReconnectNotice(): boolean {
+    const notice = this.#reconnectNotice;
+    this.#reconnectNotice = false;
+    return notice;
+  }
+
   getPageById(pageId: number): McpPage {
     const page = this.#mcpPages.values().find(mcpPage => mcpPage.id === pageId);
     if (!page) {
@@ -449,7 +474,7 @@ export class McpContext implements Context {
   async #createMcpPage(page: Page): Promise<McpPage> {
     let mcpPage = this.#mcpPages.get(page);
     if (!mcpPage) {
-      mcpPage = new McpPage(page, this.#nextPageId++, {
+      mcpPage = new McpPage(page, nextPageId++, {
         locatorClass: this.#locatorClass,
         hasNetworkBlockOrAllowlist: this.#hasNetworkBlockOrAllowlist,
         isolatedContextName: this.#getBrowserContextToNameMap().get(

src/McpResponse.ts

@@ -42,7 +42,7 @@ import type {
   Extension,
   HTTPRequest,
 } from './third_party/index.js';
-import {handleDialog} from './tools/pages.js';
+import {handleDialog, listPages} from './tools/pages.js';
 import type {ToolGroups} from './tools/thirdPartyDeveloper.js';
 import type {
   DevToolsData,
@@ -269,6 +269,7 @@ export class McpResponse implements Response {
   #redactNetworkHeaders = true;
   #error?: Error;
   #attachedWaitForResult?: WaitForEventsResult;
+  #reconnectNotice = false;
 
   get #deviceScope(): DevTools.CrUXManager.DeviceScope {
     return this.#page?.viewport?.isMobile ? 'PHONE' : 'DESKTOP';
@@ -286,6 +287,14 @@ export class McpResponse implements Response {
     this.#redactNetworkHeaders = value;
   }
 
+  /**
+   * Surfaces a one-time note that the browser reconnected and page ids changed.
+   * Set by the tool handler when the context reports a pending reconnect notice.
+   */
+  setReconnectNotice(): void {
+    this.#reconnectNotice = true;
+  }
+
   attachDevToolsData(data: DevToolsData): void {
     this.#devToolsData = data;
   }
@@ -859,6 +868,7 @@ export class McpResponse implements Response {
       thirdPartyDeveloperTools?: object[];
       webmcpTools?: object[];
       message?: string;
+      reconnected?: boolean;
       networkConditions?: string;
       navigationTimeout?: number;
       viewport?: object;
@@ -921,6 +931,12 @@ export class McpResponse implements Response {
     }
 
     const response = [];
+    if (this.#reconnectNotice) {
+      structuredContent.reconnected = true;
+      response.push(
+        `Note: the browser was restarted or reconnected since the last call. Page ids have changed. Call ${listPages().name} to see open pages.`,
+      );
+    }
     if (this.#textResponseLines.length) {
       structuredContent.message = this.#textResponseLines.join('\n');
       response.push(...this.#textResponseLines);

src/ToolHandler.ts

@@ -219,6 +219,9 @@ export class ToolHandler {
         : new McpResponse(this.serverArgs);
 
       response.setRedactNetworkHeaders(this.serverArgs.redactNetworkHeaders);
+      if (context.consumeReconnectNotice()) {
+        response.setReconnectNotice();
+      }
       try {
         if (this.tool.verifyFilesSchema) {
           for (const key of this.tool.verifyFilesSchema) {

src/index.ts

@@ -154,6 +154,8 @@ export async function createMcpServer(
         allowList: allowlist,
         blocklist: blocklist,
         allowUnrestrictedPaths: serverArgs.allowUnrestrictedPaths,
+        // Surfaces a one-time note in the next response after a reconnect.
+        reconnected: context !== undefined,
       });
       await updateRoots();
     }

tests/McpContext.test.ts

@@ -11,14 +11,17 @@ import path from 'node:path';
 import {afterEach, describe, it} from 'node:test';
 import {pathToFileURL} from 'node:url';
 
+import logger from 'debug';
+import {Locator} from 'puppeteer';
 import sinon from 'sinon';
 
 import {NetworkFormatter} from '../src/formatters/NetworkFormatter.js';
+import {McpContext} from '../src/McpContext.js';
 import {TextSnapshot} from '../src/TextSnapshot.js';
 import type {HTTPResponse} from '../src/third_party/index.js';
 import type {TraceResult} from '../src/trace-processing/parse.js';
 
-import {getMockRequest, html, withMcpContext} from './utils.js';
+import {getMockRequest, html, withBrowser, withMcpContext} from './utils.js';
 
 describe('McpContext', () => {
   afterEach(() => {
@@ -222,6 +225,66 @@ describe('McpContext', () => {
     });
   });
 
+  it('continues page ids across contexts so stale ids do not resolve', async () => {
+    await withBrowser(async browser => {
+      const options = {
+        experimentalDevToolsDebugging: false,
+        performanceCrux: false,
+      };
+      const first = await McpContext.from(
+        browser,
+        logger('test'),
+        options,
+        Locator,
+      );
+      const idBeforeReconnect = (await first.newPage()).id;
+      first.dispose();
+
+      // A new context (as created after a browser reconnect) continues the
+      // shared id counter, so an id handed out before no longer resolves and
+      // the next page keeps counting up rather than colliding with it.
+      const second = await McpContext.from(
+        browser,
+        logger('test'),
+        options,
+        Locator,
+      );
+      try {
+        assert.throws(
+          () => second.getPageById(idBeforeReconnect),
+          /No page found/,
+        );
+        assert.ok(
+          (await second.newPage()).id > idBeforeReconnect,
+          'ids continue past the pre-reconnect ids',
+        );
+      } finally {
+        second.dispose();
+      }
+    });
+  });
+
+  it('reports the reconnect notice once', async () => {
+    await withBrowser(async browser => {
+      const context = await McpContext.from(
+        browser,
+        logger('test'),
+        {
+          experimentalDevToolsDebugging: false,
+          performanceCrux: false,
+          reconnected: true,
+        },
+        Locator,
+      );
+      try {
+        assert.ok(context.consumeReconnectNotice(), 'notice available once');
+        assert.ok(!context.consumeReconnectNotice(), 'notice does not repeat');
+      } finally {
+        context.dispose();
+      }
+    });
+  });
+
   it('should include network requests in structured content', async t => {
     await withMcpContext(async (response, context) => {
       const mockRequest = getMockRequest({

tests/McpResponse.test.ts

@@ -63,6 +63,32 @@ describe('McpResponse', () => {
     });
   });
 
+  it('includes a reconnect notice only when set', async () => {
+    await withMcpContext(async (response, context) => {
+      const before = await response.handle('test', context);
+      assert.ok(
+        !JSON.stringify(before.content).includes('Page ids have changed'),
+        'no reconnect notice by default',
+      );
+      assert.ok(
+        !(before.structuredContent as {reconnected?: boolean}).reconnected,
+        'structuredContent is not flagged reconnected by default',
+      );
+
+      response.setReconnectNotice();
+      const after = await response.handle('test', context);
+      assert.ok(
+        JSON.stringify(after.content).includes('Page ids have changed'),
+        'reconnect notice is included once set',
+      );
+      assert.strictEqual(
+        (after.structuredContent as {reconnected?: boolean}).reconnected,
+        true,
+        'structuredContent is flagged reconnected once set',
+      );
+    });
+  });
+
   it('allows response text lines to be added', async t => {
     await withMcpContext(async (response, context) => {
       response.appendResponseLine('Testing 1');

tests/utils.ts

@@ -127,6 +127,7 @@ export async function withMcpContext(
 ) {
   await withBrowser(async browser => {
     TextSnapshot.resetCounter();
+    McpContext.resetPageIdsForTesting();
     const response = new McpResponse(args as ParsedArguments);
     if (context) {
       context.dispose();