refactor: clear issue aggregator on page navigation

src/McpContext.ts

@@ -155,6 +155,11 @@ export class McpContext implements Context {
     await this.#consoleCollector.init();
   }
 
+  dispose() {
+    this.#networkCollector.dispose();
+    this.#consoleCollector.dispose();
+  }
+
   static async from(
     browser: Browser,
     logger: Debugger,

src/PageCollector.ts

@@ -4,8 +4,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import type {
+  AggregatedIssue,
+  Common,
+} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
 import {
-  type AggregatedIssue,
   IssueAggregatorEvents,
   IssuesManagerEvents,
   createIssuesFromProtocolIssue,
@@ -14,7 +17,12 @@ import {
 
 import {FakeIssuesManager} from './DevtoolsUtils.js';
 import {logger} from './logger.js';
-import type {CDPSession, ConsoleMessage} from './third_party/index.js';
+import type {
+  CDPSession,
+  ConsoleMessage,
+  Protocol,
+  Target,
+} from './third_party/index.js';
 import {
   type Browser,
   type Frame,
@@ -79,22 +87,31 @@ export class PageCollector<T> {
       this.addPage(page);
     }
 
-    this.#browser.on('targetcreated', async target => {
-      const page = await target.page();
-      if (!page) {
-        return;
-      }
-      this.addPage(page);
-    });
-    this.#browser.on('targetdestroyed', async target => {
-      const page = await target.page();
-      if (!page) {
-        return;
-      }
-      this.cleanupPageDestroyed(page);
-    });
+    this.#browser.on('targetcreated', this.#onTargetCreated);
+    this.#browser.on('targetdestroyed', this.#onTargetDestroyed);
+  }
+
+  dispose() {
+    this.#browser.off('targetcreated', this.#onTargetCreated);
+    this.#browser.off('targetdestroyed', this.#onTargetDestroyed);
   }
 
+  #onTargetCreated = async (target: Target) => {
+    const page = await target.page();
+    if (!page) {
+      return;
+    }
+    this.addPage(page);
+  };
+
+  #onTargetDestroyed = async (target: Target) => {
+    const page = await target.page();
+    if (!page) {
+      return;
+    }
+    this.cleanupPageDestroyed(page);
+  };
+
   public addPage(page: Page) {
     this.#initializePage(page);
   }
@@ -210,68 +227,130 @@ export class PageCollector<T> {
 export class ConsoleCollector extends PageCollector<
   ConsoleMessage | Error | AggregatedIssue
 > {
+  #subscribedPages = new WeakMap<Page, PageIssueSubscriber>();
+
   override addPage(page: Page): void {
-    const subscribed = this.storage.has(page);
     super.addPage(page);
-    if (!subscribed) {
-      void this.subscribeForIssues(page);
+    if (!this.#subscribedPages.has(page)) {
+      const subscriber = new PageIssueSubscriber(page);
+      this.#subscribedPages.set(page, subscriber);
+      void subscriber.subscribe();
     }
   }
 
-  async subscribeForIssues(page: Page) {
-    const seenKeys = new Set<string>();
-    const mockManager = new FakeIssuesManager();
-    const aggregator = new IssueAggregator(mockManager);
-    aggregator.addEventListener(
+  protected override cleanupPageDestroyed(page: Page): void {
+    super.cleanupPageDestroyed(page);
+    this.#subscribedPages.get(page)?.unsubscribe();
+    this.#subscribedPages.delete(page);
+  }
+}
+
+class PageIssueSubscriber {
+  #issueManager = new FakeIssuesManager();
+  #issueAggregator = new IssueAggregator(this.#issueManager);
+  #seenKeys = new Set<string>();
+  #seenIssues = new Set<AggregatedIssue>();
+  #page: Page;
+  #session: CDPSession;
+
+  constructor(page: Page) {
+    this.#page = page;
+    // @ts-expect-error use existing CDP client (internal Puppeteer API).
+    this.#session = this.#page._client() as CDPSession;
+  }
+
+  #resetIssueAggregator() {
+    this.#issueManager = new FakeIssuesManager();
+    if (this.#issueAggregator) {
+      this.#issueAggregator.removeEventListener(
+        IssueAggregatorEvents.AGGREGATED_ISSUE_UPDATED,
+        this.#onAggregatedissue,
+      );
+    }
+    this.#issueAggregator = new IssueAggregator(this.#issueManager);
+
+    this.#issueAggregator.addEventListener(
       IssueAggregatorEvents.AGGREGATED_ISSUE_UPDATED,
-      event => {
-        const withId = event.data as WithSymbolId<AggregatedIssue>;
-        // Emit aggregated issue only if it's a new one
-        if (withId[stableIdSymbol]) {
-          return;
-        }
-        page.emit('issue', event.data);
-      },
+      this.#onAggregatedissue,
     );
+  }
 
+  async subscribe() {
+    this.#resetIssueAggregator();
+    this.#page.on('framenavigated', this.#onFrameNavigated);
+    this.#session.on('Audits.issueAdded', this.#onIssueAdded);
     try {
-      // @ts-expect-error use existing CDP client (internal Puppeteer API).
-      const session = page._client() as CDPSession;
-      session.on('Audits.issueAdded', data => {
-        try {
-          const inspectorIssue = data.issue;
-          // @ts-expect-error Types of protocol from Puppeteer and CDP are
-          // incomparable for InspectorIssueCode, one is union, other is enum.
-          const issue = createIssuesFromProtocolIssue(null, inspectorIssue)[0];
-          if (!issue) {
-            logger('No issue mapping for for the issue: ', inspectorIssue.code);
-            return;
-          }
-
-          const primaryKey = issue.primaryKey();
-          if (seenKeys.has(primaryKey)) {
-            return;
-          }
-          seenKeys.add(primaryKey);
-
-          mockManager.dispatchEventToListeners(
-            IssuesManagerEvents.ISSUE_ADDED,
-            {
-              issue,
-              // @ts-expect-error We don't care that issues model is null
-              issuesModel: null,
-            },
-          );
-        } catch (error) {
-          logger('Error creating a new issue', error);
-        }
-      });
-
-      await session.send('Audits.enable');
+      await this.#session.send('Audits.enable');
     } catch (error) {
       logger('Error subscribing to issues', error);
     }
   }
+
+  unsubscribe() {
+    this.#seenKeys.clear();
+    this.#seenIssues.clear();
+    this.#page.off('framenavigated', this.#onFrameNavigated);
+    this.#session.off('Audits.issueAdded', this.#onIssueAdded);
+    if (this.#issueAggregator) {
+      this.#issueAggregator.removeEventListener(
+        IssueAggregatorEvents.AGGREGATED_ISSUE_UPDATED,
+        this.#onAggregatedissue,
+      );
+    }
+    void this.#session.send('Audits.disable').catch(() => {
+      // might fail.
+    });
+  }
+
+  #onAggregatedissue = (
+    event: Common.EventTarget.EventTargetEvent<AggregatedIssue>,
+  ) => {
+    if (this.#seenIssues.has(event.data)) {
+      return;
+    }
+    this.#seenIssues.add(event.data);
+    this.#page.emit('issue', event.data);
+  };
+
+  // On navigation, we reset issue aggregation.
+  #onFrameNavigated = (frame: Frame) => {
+    // Only split the storage on main frame navigation
+    if (frame !== frame.page().mainFrame()) {
+      return;
+    }
+    this.#seenKeys.clear();
+    this.#seenIssues.clear();
+    this.#resetIssueAggregator();
+  };
+
+  #onIssueAdded = (data: Protocol.Audits.IssueAddedEvent) => {
+    try {
+      const inspectorIssue = data.issue;
+      // @ts-expect-error Types of protocol from Puppeteer and CDP are
+      // incomparable for InspectorIssueCode, one is union, other is enum.
+      const issue = createIssuesFromProtocolIssue(null, inspectorIssue)[0];
+      if (!issue) {
+        logger('No issue mapping for for the issue: ', inspectorIssue.code);
+        return;
+      }
+
+      const primaryKey = issue.primaryKey();
+      if (this.#seenKeys.has(primaryKey)) {
+        return;
+      }
+      this.#seenKeys.add(primaryKey);
+      this.#issueManager.dispatchEventToListeners(
+        IssuesManagerEvents.ISSUE_ADDED,
+        {
+          issue,
+          // @ts-expect-error We don't care that issues model is null
+          issuesModel: null,
+        },
+      );
+    } catch (error) {
+      logger('Error creating a new issue', error);
+    }
+  };
 }
 
 export class NetworkCollector extends PageCollector<HTTPRequest> {

tests/PageCollector.test.ts

@@ -12,7 +12,6 @@ import type {
   HTTPRequest,
   Page,
   Target,
-  CDPSession,
   Protocol,
 } from 'puppeteer-core';
 import sinon from 'sinon';
@@ -60,9 +59,6 @@ function getMockPage(): Page {
     mainFrame() {
       return mainFrame;
     },
-    createCDPSession() {
-      return Promise.resolve(cdpSession as unknown as CDPSession);
-    },
     ...mockListener(),
     // @ts-expect-error internal API.
     _client() {

tests/tools/console.test.ts

@@ -41,15 +41,14 @@ describe('console', () => {
       });
     });
 
-    it('lists issues messages', async () => {
+    it('lists issues', async () => {
       await withBrowser(async (response, context) => {
         const page = await context.newPage();
         const issuePromise = new Promise<void>(resolve => {
-          page.on('issue', () => {
+          page.once('issue', () => {
             resolve();
           });
         });
-
         await page.setContent('<input type="text" name="username" />');
         await issuePromise;
         await listConsoleMessages.handler({params: {}}, response, context);
@@ -63,6 +62,48 @@ describe('console', () => {
       });
     });
 
+    it('lists issues after a page reload', async () => {
+      await withBrowser(async (response, context) => {
+        const page = await context.newPage();
+        const issuePromise = new Promise<void>(resolve => {
+          page.once('issue', () => {
+            resolve();
+          });
+        });
+
+        await page.setContent('<input type="text" name="username" />');
+        await issuePromise;
+        await listConsoleMessages.handler({params: {}}, response, context);
+        {
+          const formattedResponse = await response.handle('test', context);
+          const textContent = formattedResponse[0] as {text: string};
+          assert.ok(
+            textContent.text.includes(
+              `msgid=1 [issue] An element doesn't have an autocomplete attribute (count: 1)`,
+            ),
+          );
+        }
+
+        const anotherIssuePromise = new Promise<void>(resolve => {
+          page.once('issue', () => {
+            resolve();
+          });
+        });
+        await page.reload();
+        await page.setContent('<input type="text" name="username" />');
+        await anotherIssuePromise;
+        {
+          const formattedResponse = await response.handle('test', context);
+          const textContent = formattedResponse[0] as {text: string};
+          assert.ok(
+            textContent.text.includes(
+              `msgid=2 [issue] An element doesn't have an autocomplete attribute (count: 1)`,
+            ),
+          );
+        }
+      });
+    });
+
     it('work with primitive unhandled errors', async () => {
       await withBrowser(async (response, context) => {
         const page = await context.newPage();

tests/utils.ts

@@ -18,6 +18,7 @@ import {McpResponse} from '../src/McpResponse.js';
 import {stableIdSymbol} from '../src/PageCollector.js';
 
 const browsers = new Map<string, Browser>();
+let context: McpContext | undefined;
 
 export async function withBrowser(
   cb: (response: McpResponse, context: McpContext) => Promise<void>,
@@ -48,7 +49,10 @@ export async function withBrowser(
     }),
   );
   const response = new McpResponse();
-  const context = await McpContext.from(
+  if (context) {
+    context.dispose();
+  }
+  context = await McpContext.from(
     browser,
     logger('test'),
     {