refactor: clean up page management (#2333)

- remove `#pages` and Puppeteer page getters
- rename methods for clarity
This commit is contained in:
Alex Rudenko
2026-07-09 13:42:11 +02:00
committed by GitHub
parent ca8d219ede
commit 16db01f79c
6 changed files with 56 additions and 64 deletions
+23 -30
View File
@@ -87,7 +87,6 @@ export class McpContext implements Context {
// Auto-generated name counter for when no name is provided.
#nextIsolatedContextId = 1;
#pages: Page[] = [];
#extensionServiceWorkers: ExtensionServiceWorker[] = [];
#mcpPages = new Map<Page, McpPage>();
@@ -374,7 +373,7 @@ export class McpContext implements Context {
return mcpPage;
}
async closePage(pageId: number): Promise<void> {
if (this.#pages.length === 1) {
if (this.getPages().length === 1) {
throw new Error(CLOSE_PAGE_ERROR);
}
const page = this.getPageById(pageId);
@@ -560,17 +559,15 @@ export class McpContext implements Context {
}
getPageById(pageId: number): McpPage {
const page = this.#mcpPages.values().find(mcpPage => mcpPage.id === pageId);
const page = Array.from(this.#mcpPages.values()).find(
mcpPage => mcpPage.id === pageId,
);
if (!page) {
throw new Error('No page found');
}
return page;
}
getPageId(page: Page): number | undefined {
return this.#mcpPages.get(page)?.id;
}
#getMcpPage(page: Page): McpPage {
const mcpPage = this.#mcpPages.get(page);
if (!mcpPage) {
@@ -687,7 +684,8 @@ export class McpContext implements Context {
}
async createPagesSnapshot(): Promise<Page[]> {
const {pages: allPages, isolatedContextNames} = await this.#getAllPages();
const {pages: allPages, isolatedContextNames} =
await this.#fetchBrowserPages();
for (const page of allPages) {
const mcpPage = this.#createMcpPage(page);
@@ -703,21 +701,16 @@ export class McpContext implements Context {
}
}
this.#pages = allPages.filter(page => {
return (
this.#options.experimentalDevToolsDebugging ||
!page.url().startsWith('devtools://')
);
});
const pages = this.getPages();
// Only fall back when the selected page is actually gone. Gating on
// `isClosed()` instead of `#pages` membership avoids silently swapping a
// `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.
// `devtools://` page, which is selectable but filtered out of `pages` above.
this.#selectedPageFallback = undefined;
if (
(!this.#selectedPage || this.#selectedPage.pptrPage.isClosed()) &&
this.#pages[0]
pages[0]
) {
// Record the automatic change so the response can surface it. Skipped on
// first connect, when there was no prior selection to replace.
@@ -726,15 +719,15 @@ export class McpContext implements Context {
wasClosed: this.#selectedPage.pptrPage.isClosed(),
};
}
this.selectPage(this.#getMcpPage(this.#pages[0]));
this.selectPage(pages[0]);
}
await this.detectOpenDevToolsWindows();
return this.#pages;
return pages.map(p => p.pptrPage);
}
async #getAllPages(): Promise<{
async #fetchBrowserPages(): Promise<{
pages: Page[];
isolatedContextNames: Map<Page, string>;
}> {
@@ -805,15 +798,10 @@ export class McpContext implements Context {
async detectOpenDevToolsWindows() {
this.logger?.('Detecting open DevTools windows');
const {pages} = await this.#getAllPages();
await Promise.all(
pages.map(async page => {
const mcpPage = this.#mcpPages.get(page);
if (!mcpPage) {
return;
}
Array.from(this.#mcpPages.values()).map(async mcpPage => {
const page = mcpPage.pptrPage;
// Prior to Chrome 144.0.7559.59, the command fails,
// Some Electron apps still use older version
// Fall back to not exposing DevTools at all.
@@ -840,8 +828,13 @@ export class McpContext implements Context {
return this.#extensionServiceWorkerMap.get(extensionServiceWorker.target);
}
getPages(): Page[] {
return this.#pages;
getPages(): McpPage[] {
return Array.from(this.#mcpPages.values()).filter(mcpPage => {
return (
this.#options.experimentalDevToolsDebugging ||
!mcpPage.pptrPage.url().startsWith('devtools://')
);
});
}
getIsolatedContextName(page: Page): string | undefined {
@@ -923,7 +916,7 @@ export class McpContext implements Context {
* We need to ignore favicon request as they make our test flaky
*/
async setUpNetworkCollectorForTesting() {
for (const mcpPage of this.#mcpPages.values()) {
for (const mcpPage of this.getPages()) {
mcpPage.networkCollector.dispose();
mcpPage.networkCollector = new NetworkCollector(
mcpPage.pptrPage,
+26 -23
View File
@@ -994,11 +994,14 @@ Call ${handleDialog.name} to handle it before continuing.`);
const allPages = context.getPages();
const {regularPages, extensionPages} = allPages.reduce(
(acc: {regularPages: Page[]; extensionPages: Page[]}, page: Page) => {
if (page.url().startsWith('chrome-extension://')) {
acc.extensionPages.push(page);
(
acc: {regularPages: McpPage[]; extensionPages: McpPage[]},
mcpPage: McpPage,
) => {
if (mcpPage.pptrPage.url().startsWith('chrome-extension://')) {
acc.extensionPages.push(mcpPage);
} else {
acc.regularPages.push(page);
acc.regularPages.push(mcpPage);
}
return acc;
},
@@ -1020,19 +1023,19 @@ Call ${handleDialog.name} to handle it before continuing.`);
if (regularPages.length) {
const parts = [`## Pages`];
const structuredPages = [];
for (const page of regularPages) {
const isolatedContextName = context.getIsolatedContextName(page);
for (const mcpPage of regularPages) {
const isolatedContextName = mcpPage.isolatedContextName;
const contextLabel = isolatedContextName
? ` isolatedContext=${isolatedContextName}`
: '';
const title = await fetchPageTitle(page);
const title = await fetchPageTitle(mcpPage.pptrPage);
const pageLabel = title
? `${truncateTitle(title)} (${page.url()})`
: page.url();
? `${truncateTitle(title)} (${mcpPage.pptrPage.url()})`
: mcpPage.pptrPage.url();
parts.push(
`${context.getPageId(page)}: ${pageLabel}${context.isPageSelected(page) ? ' [selected]' : ''}${contextLabel}`,
`${mcpPage.id}: ${pageLabel}${context.isPageSelected(mcpPage.pptrPage) ? ' [selected]' : ''}${contextLabel}`,
);
structuredPages.push(createStructuredPage(page, context, title));
structuredPages.push(createStructuredPage(mcpPage, context, title));
}
response.push(...parts);
structuredContent.pages = structuredPages;
@@ -1042,20 +1045,20 @@ Call ${handleDialog.name} to handle it before continuing.`);
if (extensionPages.length) {
response.push(`## Extension Pages`);
const structuredExtensionPages = [];
for (const page of extensionPages) {
const isolatedContextName = context.getIsolatedContextName(page);
for (const mcpPage of extensionPages) {
const isolatedContextName = mcpPage.isolatedContextName;
const contextLabel = isolatedContextName
? ` isolatedContext=${isolatedContextName}`
: '';
const title = await fetchPageTitle(page);
const title = await fetchPageTitle(mcpPage.pptrPage);
const pageLabel = title
? `${truncateTitle(title)} (${page.url()})`
: page.url();
? `${truncateTitle(title)} (${mcpPage.pptrPage.url()})`
: mcpPage.pptrPage.url();
response.push(
`${context.getPageId(page)}: ${pageLabel}${context.isPageSelected(page) ? ' [selected]' : ''}${contextLabel}`,
`${mcpPage.id}: ${pageLabel}${context.isPageSelected(mcpPage.pptrPage) ? ' [selected]' : ''}${contextLabel}`,
);
structuredExtensionPages.push(
createStructuredPage(page, context, title),
createStructuredPage(mcpPage, context, title),
);
}
structuredContent.extensionPages = structuredExtensionPages;
@@ -1496,11 +1499,11 @@ async function fetchPageTitle(page: Page): Promise<string> {
}
function createStructuredPage(
page: Page,
mcpPage: McpPage,
context: McpContext,
rawTitle: string,
) {
const isolatedContextName = context.getIsolatedContextName(page);
const isolatedContextName = mcpPage.isolatedContextName;
const title = truncateTitle(rawTitle);
const entry: {
id: number | undefined;
@@ -1509,10 +1512,10 @@ function createStructuredPage(
selected: boolean;
isolatedContext?: string;
} = {
id: context.getPageId(page),
url: page.url(),
id: mcpPage.id,
url: mcpPage.pptrPage.url(),
title,
selected: context.isPageSelected(page),
selected: context.isPageSelected(mcpPage.pptrPage),
};
if (isolatedContextName) {
entry.isolatedContext = isolatedContextName;
+1 -1
View File
@@ -145,7 +145,7 @@ describe('McpContext', () => {
const [firstPage] = context.getPages();
assert.ok(firstPage);
assert.ok(context.isPageSelected(firstPage));
assert.ok(context.isPageSelected(firstPage.pptrPage));
const fallback = context.getSelectedPageFallback();
assert.ok(fallback, 'fallback should be reported');
+3 -6
View File
@@ -1168,16 +1168,14 @@ describe('third-party developer tools', () => {
it('includes third-party developer tools in select_page response', async () => {
await testIncludesThirdPartyDeveloperTools(async (response, context) => {
const pageId =
context.getPageId(context.getSelectedMcpPage().pptrPage) ?? 1;
const pageId = context.getSelectedMcpPage().id;
await selectPage.handler({params: {pageId}}, response, context);
}, 'select_page');
});
it('includes third-party developer tools in close_page response', async () => {
await testIncludesThirdPartyDeveloperTools(async (response, context) => {
const pageId =
context.getPageId(context.getSelectedMcpPage().pptrPage) ?? 1;
const pageId = context.getSelectedMcpPage().id;
await closePage.handler({params: {pageId}}, response, context);
}, 'close_page');
});
@@ -1523,8 +1521,7 @@ describe('webmcp', () => {
t,
{categoryExperimentalWebmcp: true},
async (response, context) => {
const pageId =
context.getPageId(context.getSelectedMcpPage().pptrPage) ?? 1;
const pageId = context.getSelectedMcpPage().id;
await selectPage.handler({params: {pageId}}, response, context);
},
'select_page',
+2 -2
View File
@@ -372,7 +372,7 @@ describe('pages', () => {
context,
);
const page = context.getSelectedPptrPage();
const pageId = context.getPageId(page)!;
const pageId = context.getSelectedMcpPage().id;
assert.ok(!page.isClosed());
await closePage.handler({params: {pageId}}, response, context);
assert.ok(page.isClosed());
@@ -560,7 +560,7 @@ describe('pages', () => {
context,
);
const pageA = context.getSelectedPptrPage();
const pageAId = context.getPageId(pageA)!;
const pageAId = context.getSelectedMcpPage().id;
await newPage().handler(
{params: {url: 'about:blank', isolatedContext: 'ctx-b'}},
+1 -2
View File
@@ -34,8 +34,7 @@ describe('webmcp', () => {
it('list webmcp tools in select_page response', async () => {
await withMcpContext(async (response, context) => {
const pageId =
context.getPageId(context.getSelectedMcpPage().pptrPage) ?? 1;
const pageId = context.getSelectedMcpPage().id ?? 1;
await selectPage.handler({params: {pageId}}, response, context);
assert.ok(response.listWebMcpTools);
});