refactor: clean up page management (#2333)
- remove `#pages` and Puppeteer page getters - rename methods for clarity
This commit is contained in:
+23
-30
@@ -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
@@ -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;
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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'}},
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user