refactor: clean up page snapshot generation (#2348)

- remove obsolete devtools page detection step
- move isolated context processing out of the browser pages fetching
- move filtering of pages out of getPages(). That should just be a
getter.
This commit is contained in:
Alex Rudenko
2026-07-13 10:08:59 +02:00
committed by GitHub
parent c53c1ec4c9
commit 68cfce2bcb
7 changed files with 104 additions and 124 deletions
+58 -103
View File
@@ -145,7 +145,7 @@ export class McpContext implements Context {
if (!page) {
return;
}
this.#createMcpPage(page);
void this.#createMcpPage(page);
} catch (err) {
this.logger?.('Error handling targetcreated', err);
}
@@ -298,13 +298,13 @@ export class McpContext implements Context {
} else {
page = await this.browser.newPage({background});
}
const mcpPage = this.#createMcpPage(page);
const mcpPage = await this.#createMcpPage(page);
await this.createPagesSnapshot();
this.selectPage(mcpPage);
return mcpPage;
}
async closePage(pageId: number): Promise<void> {
if (this.getPages().length === 1) {
if (this.#mcpPages.size === 1) {
throw new Error(CLOSE_PAGE_ERROR);
}
const page = this.getPageById(pageId);
@@ -341,6 +341,10 @@ export class McpContext implements Context {
return this.#options.performanceCrux;
}
getPages(): McpPage[] {
return Array.from(this.#mcpPages.values());
}
getSelectedMcpPage(): McpPage {
const page = this.#selectedPage;
if (!page) {
@@ -355,11 +359,7 @@ export class McpContext implements Context {
}
getPageById(pageId: number): McpPage {
// Resolve only among listed pages (`getPages()`), so an id that
// `list_pages` never showed cannot be targeted (e.g. a `devtools://`
// frontend, which the listing excludes unless `experimentalDevToolsDebugging`
// is set).
const page = this.getPages().find(mcpPage => mcpPage.id === pageId);
const page = this.#mcpPages.values().find(mcpPage => mcpPage.id === pageId);
if (!page) {
throw new Error('No page found');
}
@@ -426,27 +426,46 @@ export class McpContext implements Context {
return this.#serviceWorkerConsoleCollector.getData(extensionId);
}
#createMcpPage(page: Page): McpPage {
#getBrowserContextToNameMap(): Map<BrowserContext, string> {
// Build a reverse lookup from BrowserContext instance → name.
const contextToName = new Map<BrowserContext, string>();
for (const [name, ctx] of this.#isolatedContexts) {
contextToName.set(ctx, name);
}
const defaultCtx = this.browser.defaultBrowserContext();
// Auto-discover BrowserContexts not in our mapping (e.g., externally
// created incognito contexts) and assign generated names.
const knownContexts = new Set(this.#isolatedContexts.values());
for (const ctx of this.browser.browserContexts()) {
if (ctx !== defaultCtx && !ctx.closed && !knownContexts.has(ctx)) {
const name = `isolated-context-${this.#nextIsolatedContextId++}`;
this.#isolatedContexts.set(name, ctx);
contextToName.set(ctx, name);
}
}
return contextToName;
}
async #createMcpPage(page: Page): Promise<McpPage> {
let mcpPage = this.#mcpPages.get(page);
if (!mcpPage) {
mcpPage = new McpPage(page, this.#nextPageId++, {
locatorClass: this.#locatorClass,
hasNetworkBlockOrAllowlist: this.#hasNetworkBlockOrAllowlist,
isolatedContextName: this.#getBrowserContextToNameMap().get(
page.browserContext(),
),
});
this.#mcpPages.set(page, mcpPage);
void mcpPage.init();
await mcpPage.init();
}
return mcpPage;
}
async createPagesSnapshot(): Promise<Page[]> {
const {pages: allPages, isolatedContextNames} =
await this.#fetchBrowserPages();
const allPages = await this.#fetchBrowserPages();
for (const page of allPages) {
const mcpPage = this.#createMcpPage(page);
mcpPage.isolatedContextName = isolatedContextNames.get(page);
}
await Promise.allSettled(allPages.map(page => this.#createMcpPage(page)));
// Prune orphaned #mcpPages entries (pages that no longer exist).
const currentPages = new Set(allPages);
@@ -457,7 +476,7 @@ export class McpContext implements Context {
}
}
const pages = this.getPages();
const pages = Array.from(this.#mcpPages.values());
// Only fall back when the selected page is actually gone. Gating on
// `isClosed()` instead of `pages` membership avoids silently swapping a
@@ -477,19 +496,18 @@ export class McpContext implements Context {
this.selectPage(pages[0]);
}
await this.detectOpenDevToolsWindows();
return pages.map(p => p.pptrPage);
}
async #fetchBrowserPages(): Promise<{
pages: Page[];
isolatedContextNames: Map<Page, string>;
}> {
const defaultCtx = this.browser.defaultBrowserContext();
const allPages = await this.browser.pages(
this.#options.experimentalIncludeAllPages,
);
async #fetchBrowserPages(): Promise<Page[]> {
const allPages = (
await this.browser.pages(this.#options.experimentalIncludeAllPages)
).filter(page => {
return (
this.#options.experimentalDevToolsDebugging ||
!page.url().startsWith('devtools://')
);
});
const allTargets = this.browser.targets();
const extensionTargets = allTargets.filter(target => {
@@ -499,78 +517,24 @@ export class McpContext implements Context {
);
});
for (const target of extensionTargets) {
// Right now target.page() returns null for popup and side panel pages.
let page = await target.page();
if (!page) {
// We need to cache pages instances for targets because target.asPage()
// returns a new page instance every time.
page = this.#extensionPages.get(target) ?? null;
if (!page) {
try {
page = await target.asPage();
this.#extensionPages.set(target, page);
} catch (e) {
this.logger?.('Failed to get page for extension target', e);
}
}
}
if (page && !allPages.includes(page)) {
allPages.push(page);
}
}
// Build a reverse lookup from BrowserContext instance → name.
const contextToName = new Map<BrowserContext, string>();
for (const [name, ctx] of this.#isolatedContexts) {
contextToName.set(ctx, name);
}
// Auto-discover BrowserContexts not in our mapping (e.g., externally
// created incognito contexts) and assign generated names.
const knownContexts = new Set(this.#isolatedContexts.values());
for (const ctx of this.browser.browserContexts()) {
if (ctx !== defaultCtx && !ctx.closed && !knownContexts.has(ctx)) {
const name = `isolated-context-${this.#nextIsolatedContextId++}`;
this.#isolatedContexts.set(name, ctx);
contextToName.set(ctx, name);
}
}
// Map each page to its isolated context name (if any).
const isolatedContextNames = new Map<Page, string>();
for (const page of allPages) {
const ctx = page.browserContext();
const name = contextToName.get(ctx);
if (name) {
isolatedContextNames.set(page, name);
}
}
return {pages: allPages, isolatedContextNames};
}
async detectOpenDevToolsWindows() {
this.logger?.('Detecting open DevTools windows');
await Promise.all(
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.
await Promise.allSettled(
extensionTargets.map(async target => {
try {
if (await page.hasDevTools()) {
mcpPage.devToolsPage = await page.openDevTools();
} else {
mcpPage.devToolsPage = undefined;
let page = await target.page();
if (!page) {
page = await target.asPage();
}
} catch {
mcpPage.devToolsPage = undefined;
this.#extensionPages.set(target, page);
if (page && !allPages.includes(page)) {
allPages.push(page);
}
} catch (e) {
this.logger?.('Failed to get page for extension target', e);
}
}),
);
return allPages;
}
getExtensionServiceWorkers(): ExtensionServiceWorker[] {
@@ -583,15 +547,6 @@ export class McpContext implements Context {
return this.#extensionServiceWorkerMap.get(extensionServiceWorker.target);
}
getPages(): McpPage[] {
return Array.from(this.#mcpPages.values()).filter(mcpPage => {
return (
this.#options.experimentalDevToolsDebugging ||
!mcpPage.pptrPage.url().startsWith('devtools://')
);
});
}
async saveTemporaryFile(
data: Uint8Array<ArrayBufferLike>,
filename: string,
+35 -10
View File
@@ -72,7 +72,6 @@ export class McpPage implements ContextPage {
// Metadata
isolatedContextName?: string;
devToolsPage?: Page;
#devtoolsUniverse?: TargetUniverse;
// Dialog
@@ -93,12 +92,14 @@ export class McpPage implements ContextPage {
options: {
hasNetworkBlockOrAllowlist: boolean;
locatorClass: typeof Locator;
isolatedContextName?: string;
},
) {
this.#hasNetworkBlockOrAllowlist = options.hasNetworkBlockOrAllowlist;
this.#locatorClass = options.locatorClass;
this.pptrPage = page;
this.id = id;
this.isolatedContextName = options.isolatedContextName;
this.#dialogHandler = (dialog: Dialog): void => {
this.#dialog = dialog;
};
@@ -121,21 +122,31 @@ export class McpPage implements ContextPage {
}
async init(): Promise<void> {
if (this.#devtoolsUniverse) {
return;
}
try {
this.#devtoolsUniverse = await createTargetUniverse(this.pptrPage);
} catch (e) {
logger?.('Failed to initialize DevTools universe', e);
}
await Promise.allSettled([
this.#initDevToolsUniverseNoThrow(),
this.#initFocusEmulationNoThrow(),
]);
}
async #initFocusEmulationNoThrow(): Promise<void> {
// We emulate a focused page for all pages to support multi-agent workflows.
void this.pptrPage.emulateFocusedPage(true).catch(error => {
logger?.('Error turning on focused page emulation', error);
});
}
async #initDevToolsUniverseNoThrow(): Promise<void> {
if (this.#devtoolsUniverse) {
return undefined;
}
try {
const session = await this.pptrPage.createCDPSession();
this.#devtoolsUniverse = await createTargetUniverse(session);
} catch (e) {
logger?.('Failed to initialize DevTools universe', e);
}
}
get devtoolsUniverse(): TargetUniverse | undefined {
return this.#devtoolsUniverse;
}
@@ -184,6 +195,20 @@ export class McpPage implements ContextPage {
return this.networkCollector.getData(includePreservedRequests);
}
async getDevToolsPage(): Promise<Page | undefined> {
try {
if (await this.pptrPage.hasDevTools()) {
return await this.pptrPage.openDevTools();
}
return undefined;
} catch {
// 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.
return undefined;
}
}
getConsoleData(
includePreservedMessages?: boolean,
): Array<ConsoleMessage | Error | DevTools.AggregatedIssue | UncaughtError> {
@@ -481,7 +506,7 @@ export class McpPage implements ContextPage {
async getDevToolsData(): Promise<DevToolsData> {
try {
logger?.('Getting DevTools UI data');
const devtoolsPage = this.devToolsPage;
const devtoolsPage = await this.getDevToolsPage();
if (!devtoolsPage) {
logger?.('No DevTools page detected');
return {};
-1
View File
@@ -214,7 +214,6 @@ export class ToolHandler {
);
const context = await this.getContext();
logger?.(`${this.tool.name} context: resolved`);
await context.detectOpenDevToolsWindows();
const response = this.serverArgs.slim
? new SlimMcpResponse(this.serverArgs)
: new McpResponse(this.serverArgs);
+1 -3
View File
@@ -8,7 +8,6 @@ import {DevTools} from '../third_party/index.js';
import type {
CDPSession,
ConsoleMessage,
Page,
Protocol,
} from '../third_party/index.js';
@@ -134,7 +133,7 @@ export interface TargetUniverse {
}
export async function createTargetUniverse(
page: Page,
session: CDPSession,
): Promise<TargetUniverse> {
const settingStorage = new DevTools.Common.Settings.SettingsStorage({});
const universe = new DevTools.Foundation.Universe.Universe({
@@ -148,7 +147,6 @@ export async function createTargetUniverse(
overrideAutoStartModels: new Set([DevTools.DebuggerModel]),
});
const session = await page.createCDPSession();
const connection = new PuppeteerDevToolsConnection(session);
const targetManager = universe.context.get(DevTools.TargetManager);
+1 -1
View File
@@ -97,7 +97,7 @@ describe('McpContext', () => {
async (_response, context) => {
const page = await context.newPage();
await context.createPagesSnapshot();
assert.ok(page.devToolsPage);
assert.ok(await page.getDevToolsPage());
// A devtools page is tracked but excluded from the listing, so its id
// must not resolve through `getPageById()` (which backs `select_page`
-3
View File
@@ -47,7 +47,6 @@ describe('ToolHandler', () => {
const mockContext = sinon.createStubInstance(McpContext);
const mockPage = sinon.createStubInstance(McpPage);
mockContext.getSelectedMcpPage.returns(mockPage);
mockContext.detectOpenDevToolsWindows.resolves();
const toolMutex = new Mutex();
const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
@@ -86,7 +85,6 @@ describe('ToolHandler', () => {
};
const mockContext = sinon.createStubInstance(McpContext);
mockContext.detectOpenDevToolsWindows.resolves();
const toolMutex = new Mutex();
const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
@@ -129,7 +127,6 @@ describe('ToolHandler', () => {
};
const mockContext = sinon.createStubInstance(McpContext);
mockContext.detectOpenDevToolsWindows.resolves();
const toolMutex = new Mutex();
const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
+9 -3
View File
@@ -23,7 +23,9 @@ describe('createTargetUniverse', () => {
it('works with a real browser', async () => {
await withBrowser(async (browser, page) => {
const targetUniverse = await createTargetUniverse(page);
const targetUniverse = await createTargetUniverse(
await page.createCDPSession(),
);
assert.notStrictEqual(targetUniverse, null);
});
@@ -31,7 +33,9 @@ describe('createTargetUniverse', () => {
it('ignores pauses', async () => {
await withBrowser(async (browser, page) => {
const targetUniverse = await createTargetUniverse(page);
const targetUniverse = await createTargetUniverse(
await page.createCDPSession(),
);
assert.ok(targetUniverse);
const model = targetUniverse.target.model(DevTools.DebuggerModel);
assert.ok(model);
@@ -50,7 +54,9 @@ describe('createTargetUniverse', () => {
server.addHtmlRoute('/test', html`<div>Test</div>`);
await withBrowser(async (browser, page) => {
const targetUniverse = await createTargetUniverse(page);
const targetUniverse = await createTargetUniverse(
await page.createCDPSession(),
);
assert.ok(targetUniverse);
const networkManager = targetUniverse.target.model(