fix: avoid no page selected errors (#537)

Uses object identity to track the selected page. This helps to be in
sync with the browser state better.

Closes #492

---------

Co-authored-by: Nikolay Vitkov <[email protected]>

Author: OrKoN

src/McpContext.ts

@@ -88,7 +88,7 @@ export class McpContext implements Context {
   // The most recent page state.
   #pages: Page[] = [];
   #pageToDevToolsPage = new Map<Page, Page>();
-  #selectedPageIdx = 0;
+  #selectedPage?: Page;
   // The most recent snapshot.
   #textSnapshot: TextSnapshot | null = null;
   #networkCollector: NetworkCollector;
@@ -146,7 +146,6 @@ export class McpContext implements Context {
 
   async #init() {
     await this.createPagesSnapshot();
-    this.setSelectedPageIdx(0);
     await this.#networkCollector.init();
     await this.#consoleCollector.init();
   }
@@ -221,8 +220,8 @@ export class McpContext implements Context {
 
   async newPage(): Promise<Page> {
     const page = await this.browser.newPage();
-    const pages = await this.createPagesSnapshot();
-    this.setSelectedPageIdx(pages.indexOf(page));
+    await this.createPagesSnapshot();
+    this.selectPage(page);
     this.#networkCollector.addPage(page);
     this.#consoleCollector.addPage(page);
     return page;
@@ -232,7 +231,6 @@ export class McpContext implements Context {
       throw new Error(CLOSE_PAGE_ERROR);
     }
     const page = this.getPageByIdx(pageIdx);
-    this.setSelectedPageIdx(0);
     await page.close({runBeforeUnload: false});
   }
 
@@ -283,7 +281,7 @@ export class McpContext implements Context {
   }
 
   getSelectedPage(): Page {
-    const page = this.#pages[this.#selectedPageIdx];
+    const page = this.#selectedPage;
     if (!page) {
       throw new Error('No page selected');
     }
@@ -304,19 +302,20 @@ export class McpContext implements Context {
     return page;
   }
 
-  getSelectedPageIdx(): number {
-    return this.#selectedPageIdx;
-  }
-
   #dialogHandler = (dialog: Dialog): void => {
     this.#dialog = dialog;
   };
 
-  setSelectedPageIdx(idx: number): void {
-    const oldPage = this.getSelectedPage();
-    oldPage.off('dialog', this.#dialogHandler);
-    this.#selectedPageIdx = idx;
-    const newPage = this.getSelectedPage();
+  isPageSelected(page: Page): boolean {
+    return this.#selectedPage === page;
+  }
+
+  selectPage(newPage: Page): void {
+    const oldPage = this.#selectedPage;
+    if (oldPage) {
+      oldPage.off('dialog', this.#dialogHandler);
+    }
+    this.#selectedPage = newPage;
     newPage.on('dialog', this.#dialogHandler);
     this.#updateSelectedPageTimeouts();
   }
@@ -387,6 +386,10 @@ export class McpContext implements Context {
       );
     });
 
+    if (!this.#selectedPage || this.#pages.indexOf(this.#selectedPage) === -1) {
+      this.selectPage(this.#pages[0]);
+    }
+
     await this.detectOpenDevToolsWindows();
 
     return this.#pages;

src/McpResponse.ts

@@ -362,7 +362,7 @@ Call ${handleDialog.name} to handle it before continuing.`);
       let idx = 0;
       for (const page of context.getPages()) {
         parts.push(
-          `${idx}: ${page.url()}${idx === context.getSelectedPageIdx() ? ' [selected]' : ''}`,
+          `${idx}: ${page.url()}${context.isPageSelected(page) ? ' [selected]' : ''}`,
         );
         idx++;
       }

src/tools/ToolDefinition.ts

@@ -90,9 +90,10 @@ export type Context = Readonly<{
   getDialog(): Dialog | undefined;
   clearDialog(): void;
   getPageByIdx(idx: number): Page;
+  isPageSelected(page: Page): boolean;
   newPage(): Promise<Page>;
   closePage(pageIdx: number): Promise<void>;
-  setSelectedPageIdx(idx: number): void;
+  selectPage(page: Page): void;
   getElementByUid(uid: string): Promise<ElementHandle<Element>>;
   getAXNodeByUid(uid: string): TextSnapshotNode | undefined;
   setNetworkConditions(conditions: string | null): void;

src/tools/pages.ts

@@ -40,7 +40,7 @@ export const selectPage = defineTool({
   handler: async (request, response, context) => {
     const page = context.getPageByIdx(request.params.pageIdx);
     await page.bringToFront();
-    context.setSelectedPageIdx(request.params.pageIdx);
+    context.selectPage(page);
     response.setIncludePages(true);
   },
 });

tests/tools/emulation.test.ts

@@ -76,7 +76,6 @@ describe('emulation', () => {
 
     it('report correctly for the currently selected page', async () => {
       await withBrowser(async (response, context) => {
-        await context.newPage();
         await emulate.handler(
           {
             params: {
@@ -89,7 +88,8 @@ describe('emulation', () => {
 
         assert.strictEqual(context.getNetworkConditions(), 'Slow 3G');
 
-        context.setSelectedPageIdx(0);
+        const page = await context.newPage();
+        context.selectPage(page);
 
         assert.strictEqual(context.getNetworkConditions(), null);
       });
@@ -132,7 +132,6 @@ describe('emulation', () => {
 
     it('report correctly for the currently selected page', async () => {
       await withBrowser(async (response, context) => {
-        await context.newPage();
         await emulate.handler(
           {
             params: {
@@ -145,7 +144,8 @@ describe('emulation', () => {
 
         assert.strictEqual(context.getCpuThrottlingRate(), 4);
 
-        context.setSelectedPageIdx(0);
+        const page = await context.newPage();
+        context.selectPage(page);
 
         assert.strictEqual(context.getCpuThrottlingRate(), 1);
       });

tests/tools/pages.test.ts

@@ -31,13 +31,13 @@ describe('pages', () => {
   describe('new_page', () => {
     it('create a page', async () => {
       await withBrowser(async (response, context) => {
-        assert.strictEqual(context.getSelectedPageIdx(), 0);
+        assert.strictEqual(context.getPageByIdx(0), context.getSelectedPage());
         await newPage.handler(
           {params: {url: 'about:blank'}},
           response,
           context,
         );
-        assert.strictEqual(context.getSelectedPageIdx(), 1);
+        assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
         assert.ok(response.includePages);
       });
     });
@@ -46,7 +46,7 @@ describe('pages', () => {
     it('closes a page', async () => {
       await withBrowser(async (response, context) => {
         const page = await context.newPage();
-        assert.strictEqual(context.getSelectedPageIdx(), 1);
+        assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
         assert.strictEqual(context.getPageByIdx(1), page);
         await closePage.handler({params: {pageIdx: 1}}, response, context);
         assert.ok(page.isClosed());
@@ -70,9 +70,9 @@ describe('pages', () => {
     it('selects a page', async () => {
       await withBrowser(async (response, context) => {
         await context.newPage();
-        assert.strictEqual(context.getSelectedPageIdx(), 1);
+        assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
         await selectPage.handler({params: {pageIdx: 0}}, response, context);
-        assert.strictEqual(context.getSelectedPageIdx(), 0);
+        assert.strictEqual(context.getPageByIdx(0), context.getSelectedPage());
         assert.ok(response.includePages);
       });
     });
@@ -97,7 +97,7 @@ describe('pages', () => {
     it('throws an error if the page was closed not by the MCP server', async () => {
       await withBrowser(async (response, context) => {
         const page = await context.newPage();
-        assert.strictEqual(context.getSelectedPageIdx(), 1);
+        assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
         assert.strictEqual(context.getPageByIdx(1), page);
 
         await page.close();
@@ -197,7 +197,6 @@ describe('pages', () => {
   describe('resize', () => {
     it('create a page', async () => {
       await withBrowser(async (response, context) => {
-        assert.strictEqual(context.getSelectedPageIdx(), 0);
         const page = context.getSelectedPage();
         const resizePromise = page.evaluate(() => {
           return new Promise(resolve => {