From 06bec7628958280febb5974fe48a6757405eb795 Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Thu, 16 Apr 2026 12:12:31 +0000 Subject: [PATCH 1/9] handle complex inPage tool responses extract creating extra snapshot nodes into separate function format address comments move to McpPage --- src/McpContext.ts | 191 +++++++++++++++++++++++- src/McpPage.ts | 195 +++++++++++++++++++++++- src/tools/ToolDefinition.ts | 16 ++ src/tools/inPage.ts | 55 +------ tests/McpContext.test.ts | 135 +++++++++++++++++ tests/tools/inPage.test.ts | 289 +++++++++++++++++++++++++++++++++++- 6 files changed, 818 insertions(+), 63 deletions(-) diff --git a/src/McpContext.ts b/src/McpContext.ts index 241636978..ffa4d5990 100644 --- a/src/McpContext.ts +++ b/src/McpContext.ts @@ -31,8 +31,8 @@ import type { Target, Extension, } from './third_party/index.js'; -import type {DevTools} from './third_party/index.js'; -import {Locator} from './third_party/index.js'; +import type {DevTools, Protocol} from './third_party/index.js'; +import {Locator, type ElementHandle} from './third_party/index.js'; import {PredefinedNetworkConditions} from './third_party/index.js'; import {listPages} from './tools/pages.js'; import {CLOSE_PAGE_ERROR} from './tools/ToolDefinition.js'; @@ -691,6 +691,7 @@ export class McpContext implements Context { page: McpPage, verbose = false, devtoolsData: DevToolsData | undefined = undefined, + extraHandles: ElementHandle[] = [], ): Promise { const rootNode = await page.pptrPage.accessibility.snapshot({ includeIframes: true, @@ -708,10 +709,13 @@ export class McpContext implements Context { let idCounter = 0; const idToNode = new Map(); const seenUniqueIds = new Set(); + const seenBackendNodeIds = new Set(); const assignIds = (node: SerializedAXNode): TextSnapshotNode => { let id = ''; - // @ts-expect-error untyped loaderId & backendNodeId. - const uniqueBackendId = `${node.loaderId}_${node.backendNodeId}`; + // @ts-expect-error untyped backendNodeId. + const backendNodeId: number = node.backendNodeId; + // @ts-expect-error untyped loaderId. + const uniqueBackendId = `${node.loaderId}_${backendNodeId}`; if (uniqueBackendNodeIdToMcpId.has(uniqueBackendId)) { // Re-use MCP exposed ID if the uniqueId is the same. id = uniqueBackendNodeIdToMcpId.get(uniqueBackendId)!; @@ -721,6 +725,7 @@ export class McpContext implements Context { uniqueBackendNodeIdToMcpId.set(uniqueBackendId, id); } seenUniqueIds.add(uniqueBackendId); + seenBackendNodeIds.add(backendNodeId); const nodeWithId: TextSnapshotNode = { ...node, @@ -744,6 +749,18 @@ export class McpContext implements Context { }; const rootNodeWithId = assignIds(rootNode); + + await this.#insertExtraNodes( + page, + idToNode, + seenUniqueIds, + snapshotId, + idCounter, + rootNodeWithId, + seenBackendNodeIds, + extraHandles, + ); + const snapshot: TextSnapshot = { root: rootNodeWithId, snapshotId: String(snapshotId), @@ -768,6 +785,172 @@ export class McpContext implements Context { } } + // ExtraHandles represent DOM nodes which might not be part of the accessibility tree, e.g. DOM nodes + // returned by in-page tools. We insert them into the tree by finding the closest ancestor in the + // tree and inserting the node as a child. The ancestor's child nodes are re-parented if necessary. + async #insertExtraNodes( + page: McpPage, + idToNode: Map, + seenUniqueIds: Set, + snapshotId: number, + idCounter: number, + rootNodeWithId: TextSnapshotNode, + seenBackendNodeIds: Set, + extraHandles: ElementHandle[], + ): Promise { + const {uniqueBackendNodeIdToMcpId} = page; + + const createExtraNode = async ( + handle: ElementHandle, + ): Promise => { + const backendNodeId = await handle.backendNodeId(); + if (!backendNodeId || seenBackendNodeIds.has(backendNodeId)) { + return null; + } + const uniqueBackendId = `custom_${backendNodeId}`; + if (seenUniqueIds.has(uniqueBackendId)) { + return null; + } + seenBackendNodeIds.add(backendNodeId); + + let id = ''; + const mcpId = uniqueBackendNodeIdToMcpId.get(uniqueBackendId); + if (mcpId !== undefined) { + id = mcpId; + } else { + id = `${snapshotId}_${idCounter++}`; + uniqueBackendNodeIdToMcpId.set(uniqueBackendId, id); + } + seenUniqueIds.add(uniqueBackendId); + + const tagHandle = await handle.getProperty('localName'); + const tagValue = await tagHandle.jsonValue(); + const extraNode: TextSnapshotNode = { + role: tagValue, + id, + backendNodeId, + children: [], + elementHandle: async () => handle, + }; + return extraNode; + }; + + const findAncestorNode = async ( + handle: ElementHandle, + ): Promise => { + let ancestorHandle = await handle.evaluateHandle(el => el.parentElement); + + while (ancestorHandle) { + const ancestorElement = ancestorHandle.asElement(); + if (!ancestorElement) { + await ancestorHandle.dispose(); + return null; + } + + const ancestorBackendId = await ancestorElement.backendNodeId(); + if (ancestorBackendId) { + const ancestorNode = idToNode + .values() + .find(node => node.backendNodeId === ancestorBackendId); + if (ancestorNode) { + await ancestorHandle.dispose(); + return ancestorNode; + } + } + + const nextHandle = await ancestorElement.evaluateHandle( + el => el.parentElement, + ); + await ancestorHandle.dispose(); + ancestorHandle = nextHandle; + } + return null; + }; + + const findDescendantNodes = async ( + backendNodeId: number, + ): Promise> => { + const descendantIds = new Set(); + try { + // @ts-expect-error internal API + const client = page.pptrPage._client(); + if (client) { + const {node}: {node: Protocol.DOM.Node} = await client.send( + 'DOM.describeNode', + { + backendNodeId, + depth: -1, + pierce: true, + }, + ); + const collect = (node: Protocol.DOM.Node) => { + if (node.backendNodeId && node.backendNodeId !== backendNodeId) { + descendantIds.add(node.backendNodeId); + } + if (node.children) { + for (const child of node.children) { + collect(child); + } + } + }; + collect(node); + } + } catch (e) { + this.logger( + `Failed to collect descendants for backend node ${backendNodeId}`, + e, + ); + } + return descendantIds; + }; + + const moveChildNodes = ( + attachTarget: TextSnapshotNode, + extraNode: TextSnapshotNode, + descendantIds: Set, + ): number => { + let firstMovedIndex = -1; + if (descendantIds.size > 0 && attachTarget.children) { + const remainingChildren: TextSnapshotNode[] = []; + for (const child of attachTarget.children) { + if (child.backendNodeId && descendantIds.has(child.backendNodeId)) { + if (firstMovedIndex === -1) { + firstMovedIndex = remainingChildren.length; + } + extraNode.children.push(child); + } else { + remainingChildren.push(child); + } + } + attachTarget.children = remainingChildren; + } + return firstMovedIndex !== -1 + ? firstMovedIndex + : attachTarget.children + ? attachTarget.children.length + : 0; + }; + + if (extraHandles.length) { + page.extraHandles = extraHandles; + } + for (const handle of page.extraHandles) { + const extraNode = await createExtraNode(handle); + if (!extraNode) { + continue; + } + idToNode.set(extraNode.id, extraNode); + const attachTarget = (await findAncestorNode(handle)) || rootNodeWithId; + if (extraNode.backendNodeId !== undefined) { + const descendantIds = await findDescendantNodes( + extraNode.backendNodeId, + ); + const index = moveChildNodes(attachTarget, extraNode, descendantIds); + attachTarget.children.splice(index, 0, extraNode); + } + } + } + async saveTemporaryFile( data: Uint8Array, filename: string, diff --git a/src/McpPage.ts b/src/McpPage.ts index 35898d326..e024ea58d 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -14,7 +14,7 @@ import type { } from './third_party/index.js'; import type {ToolGroup, ToolDefinition} from './tools/inPage.js'; import {takeSnapshot} from './tools/snapshot.js'; -import type {ContextPage} from './tools/ToolDefinition.js'; +import type {ContextPage, Context, Response} from './tools/ToolDefinition.js'; import type { EmulationSettings, GeolocationOptions, @@ -41,6 +41,7 @@ export class McpPage implements ContextPage { // Snapshot textSnapshot: TextSnapshot | null = null; uniqueBackendNodeIdToMcpId = new Map(); + extraHandles: ElementHandle[] = []; // Emulation emulationSettings: EmulationSettings = {}; @@ -131,6 +132,198 @@ export class McpPage implements ContextPage { this.pptrPage.off('dialog', this.#dialogHandler); } + async executeInPageTool( + toolName: string, + params: Record, + response: Response, + context: Context, + ): Promise { + // Creates array of ElementHandles from the UIDs in the params. + // We do not replace the uids with the ElementsHandles yet, because + // the `evaluate` function only turns them into DOM elements if they + // are passed as non-nested arguments. + const handles: ElementHandle[] = []; + for (const value of Object.values(params)) { + if ( + value instanceof Object && + 'uid' in value && + typeof value.uid === 'string' && + Object.keys(value).length === 1 + ) { + handles.push(await this.getElementByUid(value.uid)); + } + } + + const result = await this.pptrPage.evaluate( + async (name, args, ...elements) => { + // Replace the UIDs with DOM elements. + for (const [key, value] of Object.entries(args)) { + if ( + value instanceof Object && + 'uid' in value && + typeof value.uid === 'string' && + Object.keys(value).length === 1 + ) { + args[key] = elements.shift(); + } + } + + if (!window.__dtmcp?.executeTool) { + throw new Error('No tools found on the page'); + } + const toolResult = await window.__dtmcp.executeTool(name, args); + + const stashDOMElement = (el: Element) => { + if (!window.__dtmcp) { + window.__dtmcp = {}; + } + if (window.__dtmcp.stashedElements === undefined) { + window.__dtmcp.stashedElements = []; + } + window.__dtmcp.stashedElements.push(el); + return { + stashedId: `stashed-${window.__dtmcp.stashedElements.length - 1}`, + }; + }; + + const ancestors: unknown[] = []; + // Recursively walks the tool result: + // - Replaces DOM elements with an ID and stashes the DOM element on the window object + // - Replaces non-plain objects with a string representation of the object + // - Replaces circular references with the string '' + // - Replaces functions with the string '' + const processToolResult = ( + data: unknown, + parentEl?: unknown, + ): unknown => { + // 1. Handle DOM Elements + if (data instanceof Element) { + return stashDOMElement(data); + } + + // 2. Handle Arrays + if (Array.isArray(data)) { + return data.map((item: unknown) => + processToolResult(item, parentEl), + ); + } + + // 3. Handle Objects + if (data !== null && typeof data === 'object') { + while (ancestors.length > 0 && ancestors.at(-1) !== parentEl) { + ancestors.pop(); + } + if (ancestors.includes(data)) { + return ''; + } + ancestors.push(data); + + // If not a plain object, return a string representation of the object + if (Object.getPrototypeOf(data) !== Object.prototype) { + return `<${data.constructor.name} instance>`; + } + + const processedObj: Record = {}; + for (const [key, value] of Object.entries(data)) { + processedObj[key] = processToolResult(value, data); + } + return processedObj; + } + + // 4. Handle Functions + if (typeof data === 'function') { + return ''; + } + + // 5. Return primitives (strings, numbers, booleans) as-is + return data; + }; + + return { + result: processToolResult(toolResult), + stashed: window.__dtmcp?.stashedElements?.length ?? 0, + }; + }, + toolName, + params, + ...handles, + ); + + const elementHandles: ElementHandle[] = []; + for (let i = 0; i < (result.stashed ?? 0); i++) { + const elementHandle = await this.pptrPage.evaluateHandle(index => { + const el = window.__dtmcp?.stashedElements?.[index]; + if (!el) { + throw new Error(`Stashed element at index ${index} not found`); + } + return el; + }, i); + elementHandles.push(elementHandle); + } + const resultWithStashedElements = result.result; + + let isPageSnapshotUpdated = false; + + const stashedToUid = async (index: number) => { + const backendNodeId = await elementHandles[index].backendNodeId(); + if (!backendNodeId) { + logger(`No backendNodeId for stashed DOM element with index ${index}`); + return {uid: `stashed-${index}`}; + } + let cdpElementId = context.resolveCdpElementId(this, backendNodeId); + if (!cdpElementId) { + await context.createTextSnapshot( + this, + false, + undefined, + elementHandles, + ); + isPageSnapshotUpdated = true; + cdpElementId = context.resolveCdpElementId(this, backendNodeId); + } + if (!cdpElementId) { + logger(`Could not get cdpElementId for backend node ${backendNodeId}`); + return {uid: `stashed-${index}`}; + } + return {uid: cdpElementId}; + }; + + const recursivelyReplaceStashedElements = async ( + node: unknown, + ): Promise => { + if (Array.isArray(node)) { + return await Promise.all( + node.map(async x => await recursivelyReplaceStashedElements(x)), + ); + } + if (node !== null && typeof node === 'object') { + if ( + 'stashedId' in node && + typeof node.stashedId === 'string' && + node.stashedId.startsWith('stashed-') && + Object.keys(node).length === 1 + ) { + const index = parseInt(node.stashedId.split('-')[1]); + return stashedToUid(index); + } + const resultObj: Record = {}; + for (const [key, value] of Object.entries(node)) { + resultObj[key] = await recursivelyReplaceStashedElements(value); + } + return resultObj; + } + return node; + }; + + const resultWithUids = await recursivelyReplaceStashedElements( + resultWithStashedElements, + ); + response.appendResponseLine(JSON.stringify(resultWithUids, null, 2)); + if (isPageSnapshotUpdated) { + response.includeSnapshot(); + } + } + async getElementByUid(uid: string): Promise> { if (!this.textSnapshot) { throw new Error( diff --git a/src/tools/ToolDefinition.ts b/src/tools/ToolDefinition.ts index 1c42915d5..1ff7b13dc 100644 --- a/src/tools/ToolDefinition.ts +++ b/src/tools/ToolDefinition.ts @@ -221,6 +221,16 @@ export type Context = Readonly<{ triggerExtensionAction(id: string): Promise; listExtensions(): Promise>; getExtension(id: string): Promise; + resolveCdpElementId( + page: ContextPage, + cdpBackendNodeId: number, + ): string | undefined; + createTextSnapshot( + page: ContextPage, + verbose: boolean, + devtoolsData: DevToolsData | undefined, + extraHandles?: ElementHandle[], + ): Promise; getSelectedMcpPage(): McpPage; getExtensionServiceWorkers(): ExtensionServiceWorker[]; getExtensionServiceWorkerId( @@ -249,6 +259,12 @@ export type ContextPage = Readonly<{ options?: {timeout?: number; handleDialog?: 'accept' | 'dismiss' | string}, ): Promise; getInPageTools(): ToolGroup | undefined; + executeInPageTool( + toolName: string, + params: Record, + response: Response, + context: Context, + ): Promise; }>; export function defineTool( diff --git a/src/tools/inPage.ts b/src/tools/inPage.ts index 0cb1b6ba7..61a76b052 100644 --- a/src/tools/inPage.ts +++ b/src/tools/inPage.ts @@ -4,12 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - zod, - ajv, - type JSONSchema7, - type ElementHandle, -} from '../third_party/index.js'; +import {zod, ajv, type JSONSchema7} from '../third_party/index.js'; import {ToolCategory} from './categories.js'; import {definePageTool} from './ToolDefinition.js'; @@ -32,6 +27,7 @@ declare global { toolGroup?: ToolGroup< ToolDefinition & {execute: (args: Record) => unknown} >; + stashedElements?: Element[]; executeTool?: ( toolName: string, args: Record, @@ -75,7 +71,7 @@ export const executeInPageTool = definePageTool({ .optional() .describe('The JSON-stringified parameters to pass to the tool'), }, - handler: async (request, response) => { + handler: async (request, response, context) => { const toolName = request.params.toolName; let params: Record = {}; if (request.params.params) { @@ -92,22 +88,6 @@ export const executeInPageTool = definePageTool({ } } - // Creates array of ElementHandles from the UIDs in the params. - // We do not replace the uids with the ElementsHandles yet, because - // the `evaluate` function only turns them into DOM elements if they - // are passed as non-nested arguments. - const handles: ElementHandle[] = []; - for (const value of Object.values(params)) { - if ( - value instanceof Object && - 'uid' in value && - typeof value.uid === 'string' && - Object.keys(value).length === 1 - ) { - handles.push(await request.page.getElementByUid(value.uid)); - } - } - const toolGroup = request.page.getInPageTools(); const tool = toolGroup?.tools.find(t => t.name === toolName); if (!tool) { @@ -122,33 +102,6 @@ export const executeInPageTool = definePageTool({ ); } - const result = await request.page.pptrPage.evaluate( - async (name, args, ...elements) => { - // Replace the UIDs with DOM elements. - for (const [key, value] of Object.entries(args)) { - if ( - value instanceof Object && - 'uid' in value && - typeof value.uid === 'string' && - Object.keys(value).length === 1 - ) { - args[key] = elements.shift(); - } - } - - if (!window.__dtmcp?.executeTool) { - throw new Error('No tools found on the page'); - } - const toolResult = await window.__dtmcp.executeTool(name, args); - - return { - result: toolResult, - }; - }, - toolName, - params, - ...handles, - ); - response.appendResponseLine(JSON.stringify(result, null, 2)); + await request.page.executeInPageTool(toolName, params, response, context); }, }); diff --git a/tests/McpContext.test.ts b/tests/McpContext.test.ts index 31a6c88b3..908da9cad 100644 --- a/tests/McpContext.test.ts +++ b/tests/McpContext.test.ts @@ -12,6 +12,7 @@ import sinon from 'sinon'; import {NetworkFormatter} from '../src/formatters/NetworkFormatter.js'; import type {HTTPResponse} from '../src/third_party/index.js'; import type {TraceResult} from '../src/trace-processing/parse.js'; +import type {TextSnapshotNode} from '../src/types.js'; import {getMockRequest, html, withMcpContext} from './utils.js'; @@ -208,4 +209,138 @@ describe('McpContext', () => { fromStub.restore(); }); }); + + it('inserts extraHandles into the snapshot correctly', async () => { + await withMcpContext(async (_response, context) => { + const page = context.getSelectedMcpPage(); + await page.pptrPage.setContent(html` +
+
+ +
+
+ `); + + const middleHandle = await page.pptrPage.$('#middle'); + if (!middleHandle) { + throw new Error('middle element not found'); + } + + const backendNodeId = await middleHandle.backendNodeId(); + if (!backendNodeId) { + throw new Error('Failed to get backendNodeId'); + } + + // Verify it is not in the snapshot by default (due to role="none") + await context.createTextSnapshot(page, false, undefined, []); + const snapshotBefore = page.textSnapshot; + if (!snapshotBefore) { + throw new Error('Snapshot not created'); + } + + let foundMiddleBefore = false; + for (const node of snapshotBefore.idToNode.values()) { + if (node.backendNodeId === backendNodeId) { + foundMiddleBefore = true; + break; + } + } + assert.ok( + !foundMiddleBefore, + 'Middle element should NOT be in the snapshot when not passed as extra handle', + ); + + // Now take snapshot with extra handle + await context.createTextSnapshot(page, false, undefined, [middleHandle]); + + const snapshot = page.textSnapshot; + if (!snapshot) { + throw new Error('Snapshot not created'); + } + + // Find the extra node in idToNode + let extraNode: TextSnapshotNode | undefined; + for (const node of snapshot.idToNode.values()) { + if (node.backendNodeId === backendNodeId) { + extraNode = node; + break; + } + } + + assert.ok(extraNode, 'Extra node should be in the snapshot'); + assert.strictEqual( + extraNode.role, + 'div', + 'Extra node should have role "div"', + ); + + // Check if the child was moved to extraNode + const childHandle = await page.pptrPage.$('#child'); + if (!childHandle) { + throw new Error('child element not found'); + } + const childBackendNodeId = await childHandle.backendNodeId(); + + let foundChild = false; + for (const child of extraNode.children) { + if (child.backendNodeId === childBackendNodeId) { + foundChild = true; + break; + } + } + assert.ok( + foundChild, + 'Child node should be moved to extra node children', + ); + + // Find parent node in snapshot + const parentHandle = await page.pptrPage.$('#parent'); + if (!parentHandle) { + throw new Error('parent element not found'); + } + const parentBackendId = await parentHandle.backendNodeId(); + + let parentNode: TextSnapshotNode | undefined; + for (const node of snapshot.idToNode.values()) { + if (node.backendNodeId === parentBackendId) { + parentNode = node; + break; + } + } + + assert.ok(parentNode, 'Parent node should be in snapshot'); + + // Check that child is NOT a child of parent anymore + let foundChildInParent = false; + for (const child of parentNode.children) { + if (child.backendNodeId === childBackendNodeId) { + foundChildInParent = true; + break; + } + } + assert.ok( + !foundChildInParent, + 'Child node should NOT be in parent children', + ); + + // Check that middle IS a child of parent + let foundMiddleInParent = false; + for (const child of parentNode.children) { + if (child.backendNodeId === backendNodeId) { + foundMiddleInParent = true; + break; + } + } + assert.ok( + foundMiddleInParent, + 'Middle node should be in parent children', + ); + }); + }); }); diff --git a/tests/tools/inPage.test.ts b/tests/tools/inPage.test.ts index d87bbea83..225fba934 100644 --- a/tests/tools/inPage.test.ts +++ b/tests/tools/inPage.test.ts @@ -7,6 +7,8 @@ import assert from 'node:assert'; import {describe, it} from 'node:test'; +import sinon from 'sinon'; + import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js'; import type {McpContext} from '../../src/McpContext.js'; import type {McpResponse} from '../../src/McpResponse.js'; @@ -208,7 +210,7 @@ describe('inPage', () => { ); assert.strictEqual( response.responseLines[0], - JSON.stringify({result: 'result'}, null, 2), + JSON.stringify('result', null, 2), ); }, undefined, @@ -340,7 +342,7 @@ describe('inPage', () => { ); assert.strictEqual( response.responseLines[0], - JSON.stringify({result: {foo: 'bar'}}, null, 2), + JSON.stringify({foo: 'bar'}, null, 2), ); }, undefined, @@ -428,11 +430,9 @@ describe('inPage', () => { response.responseLines[0], JSON.stringify( { - result: { - isElement: true, - tagName: 'DIV', - id: 'test-id', - }, + isElement: true, + tagName: 'DIV', + id: 'test-id', }, null, 2, @@ -440,5 +440,280 @@ describe('inPage', () => { ); }); }); + + it('processToolResult replaces functions with ""', async () => { + await withMcpContext( + async (response, context) => { + await setupInPageTools(response, context, () => { + window.__dtmcp = { + toolGroup: { + name: 'test-group', + description: 'test description', + tools: [ + { + name: 'test-tool', + description: 'test tool description', + inputSchema: {}, + execute: () => ({ + foo: 'bar', + func: () => undefined, + }), + }, + ], + }, + }; + window.addEventListener('devtoolstooldiscovery', (e: Event) => { + // @ts-expect-error Event has `respondWith` + e.respondWith(window.__dtmcp?.toolGroup); + }); + }); + + await executeInPageTool.handler( + { + params: { + toolName: 'test-tool', + params: JSON.stringify({}), + }, + page: context.getSelectedMcpPage(), + }, + response, + context, + ); + assert.strictEqual( + response.responseLines[0], + JSON.stringify({foo: 'bar', func: ''}, null, 2), + ); + }, + undefined, + {categoryInPageTools: true} as ParsedArguments, + ); + }); + + it('processToolResult replaces circular references with ""', async () => { + await withMcpContext( + async (response, context) => { + await setupInPageTools(response, context, () => { + window.__dtmcp = { + toolGroup: { + name: 'test-group', + description: 'test description', + tools: [ + { + name: 'test-tool', + description: 'test tool description', + inputSchema: {}, + execute: () => { + const obj: Record = {foo: 'bar'}; + obj.self = obj; + return obj; + }, + }, + ], + }, + }; + window.addEventListener('devtoolstooldiscovery', (e: Event) => { + // @ts-expect-error Event has `respondWith` + e.respondWith(window.__dtmcp?.toolGroup); + }); + }); + + await executeInPageTool.handler( + { + params: { + toolName: 'test-tool', + params: JSON.stringify({}), + }, + page: context.getSelectedMcpPage(), + }, + response, + context, + ); + assert.strictEqual( + response.responseLines[0], + JSON.stringify({foo: 'bar', self: ''}, null, 2), + ); + }, + undefined, + {categoryInPageTools: true} as ParsedArguments, + ); + }); + + it('processToolResult replaces non-plain objects with ""', async () => { + await withMcpContext( + async (response, context) => { + await setupInPageTools(response, context, () => { + class CustomClass { + val = 'value'; + } + window.__dtmcp = { + toolGroup: { + name: 'test-group', + description: 'test description', + tools: [ + { + name: 'test-tool', + description: 'test tool description', + inputSchema: {}, + execute: () => ({ + foo: 'bar', + custom: new CustomClass(), + }), + }, + ], + }, + }; + window.addEventListener('devtoolstooldiscovery', (e: Event) => { + // @ts-expect-error Event has `respondWith` + e.respondWith(window.__dtmcp?.toolGroup); + }); + }); + + await executeInPageTool.handler( + { + params: { + toolName: 'test-tool', + params: JSON.stringify({}), + }, + page: context.getSelectedMcpPage(), + }, + response, + context, + ); + assert.strictEqual( + response.responseLines[0], + JSON.stringify( + {foo: 'bar', custom: ''}, + null, + 2, + ), + ); + }, + undefined, + {categoryInPageTools: true} as ParsedArguments, + ); + }); + + it('stashDOMElement stashes elements and returns UID', async () => { + await withMcpContext( + async (response, context) => { + const page = await context.newPage(); + response.setPage(page); + + page.inPageTools = { + name: 'test-group', + description: 'test description', + tools: [ + { + name: 'test-tool', + description: 'test tool description', + inputSchema: {}, + }, + ], + }; + + await page.pptrPage.evaluate(() => { + window.__dtmcp = { + executeTool: async () => { + const div = document.createElement('div'); + div.id = 'test-element'; + document.body.appendChild(div); + return div; + }, + }; + }); + + const stub = sinon + .stub(context, 'resolveCdpElementId') + .returns('mock-uid'); + + await executeInPageTool.handler( + { + params: { + toolName: 'test-tool', + params: JSON.stringify({}), + }, + page: page, + }, + response, + context, + ); + + assert.strictEqual( + response.responseLines[0], + JSON.stringify({uid: 'mock-uid'}, null, 2), + ); + + stub.restore(); + }, + undefined, + {categoryInPageTools: true} as ParsedArguments, + ); + }); + + it('creates a new snapshot if the stashed ID cannot be mapped to a UID initially', async () => { + await withMcpContext( + async (response, context) => { + const page = await context.newPage(); + response.setPage(page); + + page.inPageTools = { + name: 'test-group', + description: 'test description', + tools: [ + { + name: 'test-tool', + description: 'test tool description', + inputSchema: {}, + }, + ], + }; + + await page.pptrPage.evaluate(() => { + window.__dtmcp = { + executeTool: async () => { + const div = document.createElement('div'); + div.id = 'test-element'; + document.body.appendChild(div); + return div; + }, + }; + }); + + const stubResolve = sinon.stub(context, 'resolveCdpElementId'); + stubResolve.onFirstCall().returns(undefined); + stubResolve.onSecondCall().returns('mock-uid'); + + const stubSnapshot = sinon + .stub(context, 'createTextSnapshot') + .resolves(); + + await executeInPageTool.handler( + { + params: { + toolName: 'test-tool', + params: JSON.stringify({}), + }, + page: page, + }, + response, + context, + ); + + assert.ok( + stubSnapshot.calledOnce, + 'Expected createTextSnapshot to be called', + ); + assert.strictEqual( + response.responseLines[0], + JSON.stringify({uid: 'mock-uid'}, null, 2), + ); + + stubResolve.restore(); + stubSnapshot.restore(); + }, + undefined, + {categoryInPageTools: true} as ParsedArguments, + ); + }); }); }); From 1e990e321c9852ab64daefd4b77168af4dcaf471 Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Tue, 21 Apr 2026 13:17:12 +0000 Subject: [PATCH 2/9] refactor --- src/McpPage.ts | 59 +++++++---------- tests/tools/inPage.test.ts | 130 ++++++++++++++++++------------------- 2 files changed, 90 insertions(+), 99 deletions(-) diff --git a/src/McpPage.ts b/src/McpPage.ts index e024ea58d..f99f8f00f 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -250,6 +250,7 @@ export class McpPage implements ContextPage { ); const elementHandles: ElementHandle[] = []; + const cdpElementIds: string[] = []; for (let i = 0; i < (result.stashed ?? 0); i++) { const elementHandle = await this.pptrPage.evaluateHandle(index => { const el = window.__dtmcp?.stashedElements?.[index]; @@ -259,42 +260,35 @@ export class McpPage implements ContextPage { return el; }, i); elementHandles.push(elementHandle); - } - const resultWithStashedElements = result.result; - let isPageSnapshotUpdated = false; - - const stashedToUid = async (index: number) => { - const backendNodeId = await elementHandles[index].backendNodeId(); + const backendNodeId = await elementHandle.backendNodeId(); if (!backendNodeId) { - logger(`No backendNodeId for stashed DOM element with index ${index}`); - return {uid: `stashed-${index}`}; - } - let cdpElementId = context.resolveCdpElementId(this, backendNodeId); - if (!cdpElementId) { - await context.createTextSnapshot( - this, - false, - undefined, - elementHandles, - ); - isPageSnapshotUpdated = true; - cdpElementId = context.resolveCdpElementId(this, backendNodeId); + logger(`No backendNodeId for stashed DOM element with index ${i}`); + cdpElementIds.push(`stashed-${i}`); + continue; } + const cdpElementId = context.resolveCdpElementId(this, backendNodeId); if (!cdpElementId) { logger(`Could not get cdpElementId for backend node ${backendNodeId}`); - return {uid: `stashed-${index}`}; + cdpElementIds.push(`stashed-${i}`); + continue; } - return {uid: cdpElementId}; - }; + cdpElementIds.push(cdpElementId); + } + const resultWithStashedElements = result.result; + if (elementHandles.length) { + await context.createTextSnapshot( + this, + false, + undefined, + elementHandles, + ); + response.includeSnapshot(); + } - const recursivelyReplaceStashedElements = async ( - node: unknown, - ): Promise => { + const recursivelyReplaceStashedElements = (node: unknown): unknown => { if (Array.isArray(node)) { - return await Promise.all( - node.map(async x => await recursivelyReplaceStashedElements(x)), - ); + return node.map(x => recursivelyReplaceStashedElements(x)); } if (node !== null && typeof node === 'object') { if ( @@ -304,24 +298,21 @@ export class McpPage implements ContextPage { Object.keys(node).length === 1 ) { const index = parseInt(node.stashedId.split('-')[1]); - return stashedToUid(index); + return {uid: cdpElementIds[index]}; } const resultObj: Record = {}; for (const [key, value] of Object.entries(node)) { - resultObj[key] = await recursivelyReplaceStashedElements(value); + resultObj[key] = recursivelyReplaceStashedElements(value); } return resultObj; } return node; }; - const resultWithUids = await recursivelyReplaceStashedElements( + const resultWithUids = recursivelyReplaceStashedElements( resultWithStashedElements, ); response.appendResponseLine(JSON.stringify(resultWithUids, null, 2)); - if (isPageSnapshotUpdated) { - response.includeSnapshot(); - } } async getElementByUid(uid: string): Promise> { diff --git a/tests/tools/inPage.test.ts b/tests/tools/inPage.test.ts index 225fba934..70944a78b 100644 --- a/tests/tools/inPage.test.ts +++ b/tests/tools/inPage.test.ts @@ -650,70 +650,70 @@ describe('inPage', () => { ); }); - it('creates a new snapshot if the stashed ID cannot be mapped to a UID initially', async () => { - await withMcpContext( - async (response, context) => { - const page = await context.newPage(); - response.setPage(page); - - page.inPageTools = { - name: 'test-group', - description: 'test description', - tools: [ - { - name: 'test-tool', - description: 'test tool description', - inputSchema: {}, - }, - ], - }; - - await page.pptrPage.evaluate(() => { - window.__dtmcp = { - executeTool: async () => { - const div = document.createElement('div'); - div.id = 'test-element'; - document.body.appendChild(div); - return div; - }, - }; - }); - - const stubResolve = sinon.stub(context, 'resolveCdpElementId'); - stubResolve.onFirstCall().returns(undefined); - stubResolve.onSecondCall().returns('mock-uid'); - - const stubSnapshot = sinon - .stub(context, 'createTextSnapshot') - .resolves(); - - await executeInPageTool.handler( - { - params: { - toolName: 'test-tool', - params: JSON.stringify({}), - }, - page: page, - }, - response, - context, - ); - - assert.ok( - stubSnapshot.calledOnce, - 'Expected createTextSnapshot to be called', - ); - assert.strictEqual( - response.responseLines[0], - JSON.stringify({uid: 'mock-uid'}, null, 2), - ); - - stubResolve.restore(); - stubSnapshot.restore(); - }, - undefined, - {categoryInPageTools: true} as ParsedArguments, - ); - }); + // it('creates a new snapshot if the stashed ID cannot be mapped to a UID initially', async () => { + // await withMcpContext( + // async (response, context) => { + // const page = await context.newPage(); + // response.setPage(page); + + // page.inPageTools = { + // name: 'test-group', + // description: 'test description', + // tools: [ + // { + // name: 'test-tool', + // description: 'test tool description', + // inputSchema: {}, + // }, + // ], + // }; + + // await page.pptrPage.evaluate(() => { + // window.__dtmcp = { + // executeTool: async () => { + // const div = document.createElement('div'); + // div.id = 'test-element'; + // document.body.appendChild(div); + // return div; + // }, + // }; + // }); + + // const stubResolve = sinon.stub(context, 'resolveCdpElementId'); + // stubResolve.onFirstCall().returns(undefined); + // stubResolve.onSecondCall().returns('mock-uid'); + + // const stubSnapshot = sinon + // .stub(context, 'createTextSnapshot') + // .resolves(); + + // await executeInPageTool.handler( + // { + // params: { + // toolName: 'test-tool', + // params: JSON.stringify({}), + // }, + // page: page, + // }, + // response, + // context, + // ); + + // assert.ok( + // stubSnapshot.calledOnce, + // 'Expected createTextSnapshot to be called', + // ); + // assert.strictEqual( + // response.responseLines[0], + // JSON.stringify({uid: 'mock-uid'}, null, 2), + // ); + + // stubResolve.restore(); + // stubSnapshot.restore(); + // }, + // undefined, + // {categoryInPageTools: true} as ParsedArguments, + // ); + // }); }); }); From 0bbe14beaaf443a70b54641e5dfdf104a69a4aba Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Tue, 21 Apr 2026 14:58:35 +0000 Subject: [PATCH 3/9] add TextSnapshot.ts --- src/McpContext.ts | 271 +----------------- src/McpPage.ts | 11 +- src/McpResponse.ts | 10 +- src/TextSnapshot.ts | 318 +++++++++++++++++++++ src/formatters/SnapshotFormatter.ts | 3 +- src/tools/ToolDefinition.ts | 7 +- src/types.ts | 11 - tests/McpContext.test.ts | 23 +- tests/formatters/snapshotFormatter.test.ts | 3 +- tests/tools/console.test.ts | 5 +- tests/tools/input.test.ts | 116 ++++++-- tests/tools/screenshot.test.ts | 6 +- tests/tools/script.test.ts | 16 +- tests/utils.ts | 2 + 14 files changed, 467 insertions(+), 335 deletions(-) create mode 100644 src/TextSnapshot.ts diff --git a/src/McpContext.ts b/src/McpContext.ts index ffa4d5990..cd9a8f47d 100644 --- a/src/McpContext.ts +++ b/src/McpContext.ts @@ -26,13 +26,12 @@ import type { HTTPRequest, Page, ScreenRecorder, - SerializedAXNode, Viewport, Target, Extension, } from './third_party/index.js'; -import type {DevTools, Protocol} from './third_party/index.js'; -import {Locator, type ElementHandle} from './third_party/index.js'; +import type {DevTools} from './third_party/index.js'; +import {Locator} from './third_party/index.js'; import {PredefinedNetworkConditions} from './third_party/index.js'; import {listPages} from './tools/pages.js'; import {CLOSE_PAGE_ERROR} from './tools/ToolDefinition.js'; @@ -45,8 +44,6 @@ import type {TraceResult} from './trace-processing/parse.js'; import type { EmulationSettings, GeolocationOptions, - TextSnapshot, - TextSnapshotNode, ExtensionServiceWorker, } from './types.js'; import {ensureExtension, saveTemporaryFile} from './utils/files.js'; @@ -92,7 +89,6 @@ export class McpContext implements Context { #extensionServiceWorkerMap = new WeakMap(); #nextExtensionServiceWorkerId = 1; - #nextSnapshotId = 1; #traceResults: TraceResult[] = []; #locatorClass: typeof Locator; @@ -687,269 +683,6 @@ export class McpContext implements Context { /** * Creates a text snapshot of a page. */ - async createTextSnapshot( - page: McpPage, - verbose = false, - devtoolsData: DevToolsData | undefined = undefined, - extraHandles: ElementHandle[] = [], - ): Promise { - const rootNode = await page.pptrPage.accessibility.snapshot({ - includeIframes: true, - interestingOnly: !verbose, - }); - if (!rootNode) { - return; - } - - const {uniqueBackendNodeIdToMcpId} = page; - - const snapshotId = this.#nextSnapshotId++; - // Iterate through the whole accessibility node tree and assign node ids that - // will be used for the tree serialization and mapping ids back to nodes. - let idCounter = 0; - const idToNode = new Map(); - const seenUniqueIds = new Set(); - const seenBackendNodeIds = new Set(); - const assignIds = (node: SerializedAXNode): TextSnapshotNode => { - let id = ''; - // @ts-expect-error untyped backendNodeId. - const backendNodeId: number = node.backendNodeId; - // @ts-expect-error untyped loaderId. - const uniqueBackendId = `${node.loaderId}_${backendNodeId}`; - if (uniqueBackendNodeIdToMcpId.has(uniqueBackendId)) { - // Re-use MCP exposed ID if the uniqueId is the same. - id = uniqueBackendNodeIdToMcpId.get(uniqueBackendId)!; - } else { - // Only generate a new ID if we have not seen the node before. - id = `${snapshotId}_${idCounter++}`; - uniqueBackendNodeIdToMcpId.set(uniqueBackendId, id); - } - seenUniqueIds.add(uniqueBackendId); - seenBackendNodeIds.add(backendNodeId); - - const nodeWithId: TextSnapshotNode = { - ...node, - id, - children: node.children - ? node.children.map(child => assignIds(child)) - : [], - }; - - // The AXNode for an option doesn't contain its `value`. - // Therefore, set text content of the option as value. - if (node.role === 'option') { - const optionText = node.name; - if (optionText) { - nodeWithId.value = optionText.toString(); - } - } - - idToNode.set(nodeWithId.id, nodeWithId); - return nodeWithId; - }; - - const rootNodeWithId = assignIds(rootNode); - - await this.#insertExtraNodes( - page, - idToNode, - seenUniqueIds, - snapshotId, - idCounter, - rootNodeWithId, - seenBackendNodeIds, - extraHandles, - ); - - const snapshot: TextSnapshot = { - root: rootNodeWithId, - snapshotId: String(snapshotId), - idToNode, - hasSelectedElement: false, - verbose, - }; - page.textSnapshot = snapshot; - const data = devtoolsData ?? (await this.getDevToolsData(page)); - if (data?.cdpBackendNodeId) { - snapshot.hasSelectedElement = true; - snapshot.selectedElementUid = page.resolveCdpElementId( - data?.cdpBackendNodeId, - ); - } - - // Clean up unique IDs that we did not see anymore. - for (const key of uniqueBackendNodeIdToMcpId.keys()) { - if (!seenUniqueIds.has(key)) { - uniqueBackendNodeIdToMcpId.delete(key); - } - } - } - - // ExtraHandles represent DOM nodes which might not be part of the accessibility tree, e.g. DOM nodes - // returned by in-page tools. We insert them into the tree by finding the closest ancestor in the - // tree and inserting the node as a child. The ancestor's child nodes are re-parented if necessary. - async #insertExtraNodes( - page: McpPage, - idToNode: Map, - seenUniqueIds: Set, - snapshotId: number, - idCounter: number, - rootNodeWithId: TextSnapshotNode, - seenBackendNodeIds: Set, - extraHandles: ElementHandle[], - ): Promise { - const {uniqueBackendNodeIdToMcpId} = page; - - const createExtraNode = async ( - handle: ElementHandle, - ): Promise => { - const backendNodeId = await handle.backendNodeId(); - if (!backendNodeId || seenBackendNodeIds.has(backendNodeId)) { - return null; - } - const uniqueBackendId = `custom_${backendNodeId}`; - if (seenUniqueIds.has(uniqueBackendId)) { - return null; - } - seenBackendNodeIds.add(backendNodeId); - - let id = ''; - const mcpId = uniqueBackendNodeIdToMcpId.get(uniqueBackendId); - if (mcpId !== undefined) { - id = mcpId; - } else { - id = `${snapshotId}_${idCounter++}`; - uniqueBackendNodeIdToMcpId.set(uniqueBackendId, id); - } - seenUniqueIds.add(uniqueBackendId); - - const tagHandle = await handle.getProperty('localName'); - const tagValue = await tagHandle.jsonValue(); - const extraNode: TextSnapshotNode = { - role: tagValue, - id, - backendNodeId, - children: [], - elementHandle: async () => handle, - }; - return extraNode; - }; - - const findAncestorNode = async ( - handle: ElementHandle, - ): Promise => { - let ancestorHandle = await handle.evaluateHandle(el => el.parentElement); - - while (ancestorHandle) { - const ancestorElement = ancestorHandle.asElement(); - if (!ancestorElement) { - await ancestorHandle.dispose(); - return null; - } - - const ancestorBackendId = await ancestorElement.backendNodeId(); - if (ancestorBackendId) { - const ancestorNode = idToNode - .values() - .find(node => node.backendNodeId === ancestorBackendId); - if (ancestorNode) { - await ancestorHandle.dispose(); - return ancestorNode; - } - } - - const nextHandle = await ancestorElement.evaluateHandle( - el => el.parentElement, - ); - await ancestorHandle.dispose(); - ancestorHandle = nextHandle; - } - return null; - }; - - const findDescendantNodes = async ( - backendNodeId: number, - ): Promise> => { - const descendantIds = new Set(); - try { - // @ts-expect-error internal API - const client = page.pptrPage._client(); - if (client) { - const {node}: {node: Protocol.DOM.Node} = await client.send( - 'DOM.describeNode', - { - backendNodeId, - depth: -1, - pierce: true, - }, - ); - const collect = (node: Protocol.DOM.Node) => { - if (node.backendNodeId && node.backendNodeId !== backendNodeId) { - descendantIds.add(node.backendNodeId); - } - if (node.children) { - for (const child of node.children) { - collect(child); - } - } - }; - collect(node); - } - } catch (e) { - this.logger( - `Failed to collect descendants for backend node ${backendNodeId}`, - e, - ); - } - return descendantIds; - }; - - const moveChildNodes = ( - attachTarget: TextSnapshotNode, - extraNode: TextSnapshotNode, - descendantIds: Set, - ): number => { - let firstMovedIndex = -1; - if (descendantIds.size > 0 && attachTarget.children) { - const remainingChildren: TextSnapshotNode[] = []; - for (const child of attachTarget.children) { - if (child.backendNodeId && descendantIds.has(child.backendNodeId)) { - if (firstMovedIndex === -1) { - firstMovedIndex = remainingChildren.length; - } - extraNode.children.push(child); - } else { - remainingChildren.push(child); - } - } - attachTarget.children = remainingChildren; - } - return firstMovedIndex !== -1 - ? firstMovedIndex - : attachTarget.children - ? attachTarget.children.length - : 0; - }; - - if (extraHandles.length) { - page.extraHandles = extraHandles; - } - for (const handle of page.extraHandles) { - const extraNode = await createExtraNode(handle); - if (!extraNode) { - continue; - } - idToNode.set(extraNode.id, extraNode); - const attachTarget = (await findAncestorNode(handle)) || rootNodeWithId; - if (extraNode.backendNodeId !== undefined) { - const descendantIds = await findDescendantNodes( - extraNode.backendNodeId, - ); - const index = moveChildNodes(attachTarget, extraNode, descendantIds); - attachTarget.children.splice(index, 0, extraNode); - } - } - } async saveTemporaryFile( data: Uint8Array, diff --git a/src/McpPage.ts b/src/McpPage.ts index f99f8f00f..3546b72b2 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -5,6 +5,7 @@ */ import {logger} from './logger.js'; +import {TextSnapshot} from './TextSnapshot.js'; import type { Dialog, ElementHandle, @@ -18,7 +19,6 @@ import type {ContextPage, Context, Response} from './tools/ToolDefinition.js'; import type { EmulationSettings, GeolocationOptions, - TextSnapshot, TextSnapshotNode, } from './types.js'; import { @@ -277,12 +277,9 @@ export class McpPage implements ContextPage { } const resultWithStashedElements = result.result; if (elementHandles.length) { - await context.createTextSnapshot( - this, - false, - undefined, - elementHandles, - ); + this.textSnapshot = await TextSnapshot.create(this, context, { + extraHandles: elementHandles, + }); response.includeSnapshot(); } diff --git a/src/McpResponse.ts b/src/McpResponse.ts index 45e587354..9605fca3e 100644 --- a/src/McpResponse.ts +++ b/src/McpResponse.ts @@ -15,6 +15,7 @@ import {SnapshotFormatter} from './formatters/SnapshotFormatter.js'; import type {McpContext} from './McpContext.js'; import type {McpPage} from './McpPage.js'; import {UncaughtError} from './PageCollector.js'; +import {TextSnapshot} from './TextSnapshot.js'; import {DevTools, type Protocol} from './third_party/index.js'; import type { ConsoleMessage, @@ -443,11 +444,10 @@ export class McpResponse implements Response { if (!this.#page) { throw new Error('Response must have a page'); } - await context.createTextSnapshot( - this.#page, - this.#snapshotParams.verbose, - this.#devToolsData, - ); + this.#page.textSnapshot = await TextSnapshot.create(this.#page, context, { + verbose: this.#snapshotParams.verbose, + devtoolsData: this.#devToolsData, + }); const textSnapshot = this.#page.textSnapshot; if (textSnapshot) { const formatter = new SnapshotFormatter(textSnapshot); diff --git a/src/TextSnapshot.ts b/src/TextSnapshot.ts new file mode 100644 index 000000000..b6a728c5c --- /dev/null +++ b/src/TextSnapshot.ts @@ -0,0 +1,318 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {logger} from './logger.js'; +import type {McpPage} from './McpPage.js'; +import type { + Protocol, + SerializedAXNode, + ElementHandle, +} from './third_party/index.js'; +import type {Context, DevToolsData} from './tools/ToolDefinition.js'; +import type {TextSnapshotNode} from './types.js'; + +export class TextSnapshot { + static nextSnapshotId = 1; + + static resetCounter() { + TextSnapshot.nextSnapshotId = 1; + } + + root: TextSnapshotNode; + idToNode: Map; + snapshotId: string; + selectedElementUid?: string; + hasSelectedElement: boolean; + verbose: boolean; + + constructor(data: { + root: TextSnapshotNode; + idToNode: Map; + snapshotId: string; + selectedElementUid?: string; + hasSelectedElement: boolean; + verbose: boolean; + }) { + this.root = data.root; + this.idToNode = data.idToNode; + this.snapshotId = data.snapshotId; + this.selectedElementUid = data.selectedElementUid; + this.hasSelectedElement = data.hasSelectedElement; + this.verbose = data.verbose; + } + + static async create( + page: McpPage, + context: Context, + options: { + verbose?: boolean; + devtoolsData?: DevToolsData; + extraHandles?: ElementHandle[]; + } = {}, + ): Promise { + const verbose = options.verbose ?? false; + const rootNode = await page.pptrPage.accessibility.snapshot({ + includeIframes: true, + interestingOnly: !verbose, + }); + if (!rootNode) { + throw new Error('Failed to create accessibility snapshot'); + } + + const {uniqueBackendNodeIdToMcpId} = page; + const snapshotId = TextSnapshot.nextSnapshotId++; + // Iterate through the whole accessibility node tree and assign node ids that + // will be used for the tree serialization and mapping ids back to nodes. + let idCounter = 0; + const idToNode = new Map(); + const seenUniqueIds = new Set(); + const seenBackendNodeIds = new Set(); + + const assignIds = (node: SerializedAXNode): TextSnapshotNode => { + let id = ''; + // @ts-expect-error untyped backendNodeId. + const backendNodeId: number = node.backendNodeId; + // @ts-expect-error untyped loaderId. + const uniqueBackendId = `${node.loaderId}_${backendNodeId}`; + const existingMcpId = uniqueBackendNodeIdToMcpId.get(uniqueBackendId); + if (existingMcpId !== undefined) { + // Re-use MCP exposed ID if the uniqueId is the same. + id = existingMcpId; + } else { + // Only generate a new ID if we have not seen the node before. + id = `${snapshotId}_${idCounter++}`; + uniqueBackendNodeIdToMcpId.set(uniqueBackendId, id); + } + seenUniqueIds.add(uniqueBackendId); + seenBackendNodeIds.add(backendNodeId); + + const nodeWithId: TextSnapshotNode = { + ...node, + id, + children: node.children + ? node.children.map(child => assignIds(child)) + : [], + }; + + // The AXNode for an option doesn't contain its `value`. + // Therefore, set text content of the option as value. + if (node.role === 'option') { + const optionText = node.name; + if (optionText) { + nodeWithId.value = optionText.toString(); + } + } + + idToNode.set(nodeWithId.id, nodeWithId); + return nodeWithId; + }; + + const rootNodeWithId = assignIds(rootNode); + + await TextSnapshot.insertExtraNodes( + page, + idToNode, + seenUniqueIds, + snapshotId, + idCounter, + rootNodeWithId, + seenBackendNodeIds, + options.extraHandles ?? [], + ); + + const snapshot = new TextSnapshot({ + root: rootNodeWithId, + snapshotId: String(snapshotId), + idToNode, + hasSelectedElement: false, + verbose, + }); + + const data = options.devtoolsData ?? (await context.getDevToolsData(page)); + if (data?.cdpBackendNodeId) { + snapshot.hasSelectedElement = true; + snapshot.selectedElementUid = context.resolveCdpElementId( + page, + data.cdpBackendNodeId, + ); + } + + // Clean up unique IDs that we did not see anymore. + for (const key of uniqueBackendNodeIdToMcpId.keys()) { + if (!seenUniqueIds.has(key)) { + uniqueBackendNodeIdToMcpId.delete(key); + } + } + + return snapshot; + } + + // ExtraHandles represent DOM nodes which might not be part of the accessibility tree, e.g. DOM nodes + // returned by in-page tools. We insert them into the tree by finding the closest ancestor in the + // tree and inserting the node as a child. The ancestor's child nodes are re-parented if necessary. + private static async insertExtraNodes( + page: McpPage, + idToNode: Map, + seenUniqueIds: Set, + snapshotId: number, + idCounter: number, + rootNodeWithId: TextSnapshotNode, + seenBackendNodeIds: Set, + extraHandles: ElementHandle[], + ): Promise { + const {uniqueBackendNodeIdToMcpId} = page; + + const createExtraNode = async ( + handle: ElementHandle, + ): Promise => { + const backendNodeId = await handle.backendNodeId(); + if (!backendNodeId || seenBackendNodeIds.has(backendNodeId)) { + return null; + } + const uniqueBackendId = `custom_${backendNodeId}`; + if (seenUniqueIds.has(uniqueBackendId)) { + return null; + } + seenBackendNodeIds.add(backendNodeId); + + let id = ''; + const mcpId = uniqueBackendNodeIdToMcpId.get(uniqueBackendId); + if (mcpId !== undefined) { + id = mcpId; + } else { + id = `${snapshotId}_${idCounter++}`; + uniqueBackendNodeIdToMcpId.set(uniqueBackendId, id); + } + seenUniqueIds.add(uniqueBackendId); + + const tagHandle = await handle.getProperty('localName'); + const tagValue = await tagHandle.jsonValue(); + const extraNode: TextSnapshotNode = { + role: tagValue, + id, + backendNodeId, + children: [], + elementHandle: async () => handle, + }; + return extraNode; + }; + + const findAncestorNode = async ( + handle: ElementHandle, + ): Promise => { + let ancestorHandle = await handle.evaluateHandle(el => el.parentElement); + + while (ancestorHandle) { + const ancestorElement = ancestorHandle.asElement(); + if (!ancestorElement) { + await ancestorHandle.dispose(); + return null; + } + + const ancestorBackendId = await ancestorElement.backendNodeId(); + if (ancestorBackendId) { + const ancestorNode = idToNode + .values() + .find(node => node.backendNodeId === ancestorBackendId); + if (ancestorNode) { + await ancestorHandle.dispose(); + return ancestorNode; + } + } + + const nextHandle = await ancestorElement.evaluateHandle( + el => el.parentElement, + ); + await ancestorHandle.dispose(); + ancestorHandle = nextHandle; + } + return null; + }; + + const findDescendantNodes = async ( + backendNodeId: number, + ): Promise> => { + const descendantIds = new Set(); + try { + // @ts-expect-error internal API + const client = page.pptrPage._client(); + if (client) { + const {node}: {node: Protocol.DOM.Node} = await client.send( + 'DOM.describeNode', + { + backendNodeId, + depth: -1, + pierce: true, + }, + ); + const collect = (node: Protocol.DOM.Node) => { + if (node.backendNodeId && node.backendNodeId !== backendNodeId) { + descendantIds.add(node.backendNodeId); + } + if (node.children) { + for (const child of node.children) { + collect(child); + } + } + }; + collect(node); + } + } catch (e) { + logger( + `Failed to collect descendants for backend node ${backendNodeId}`, + e, + ); + } + return descendantIds; + }; + + const moveChildNodes = ( + attachTarget: TextSnapshotNode, + extraNode: TextSnapshotNode, + descendantIds: Set, + ): number => { + let firstMovedIndex = -1; + if (descendantIds.size > 0 && attachTarget.children) { + const remainingChildren: TextSnapshotNode[] = []; + for (const child of attachTarget.children) { + if (child.backendNodeId && descendantIds.has(child.backendNodeId)) { + if (firstMovedIndex === -1) { + firstMovedIndex = remainingChildren.length; + } + extraNode.children.push(child); + } else { + remainingChildren.push(child); + } + } + attachTarget.children = remainingChildren; + } + return firstMovedIndex !== -1 + ? firstMovedIndex + : attachTarget.children + ? attachTarget.children.length + : 0; + }; + + if (extraHandles.length) { + page.extraHandles = extraHandles; + } + for (const handle of page.extraHandles) { + const extraNode = await createExtraNode(handle); + if (!extraNode) { + continue; + } + idToNode.set(extraNode.id, extraNode); + const attachTarget = (await findAncestorNode(handle)) || rootNodeWithId; + if (extraNode.backendNodeId !== undefined) { + const descendantIds = await findDescendantNodes( + extraNode.backendNodeId, + ); + const index = moveChildNodes(attachTarget, extraNode, descendantIds); + attachTarget.children.splice(index, 0, extraNode); + } + } + } +} diff --git a/src/formatters/SnapshotFormatter.ts b/src/formatters/SnapshotFormatter.ts index d118fc171..bc13f641b 100644 --- a/src/formatters/SnapshotFormatter.ts +++ b/src/formatters/SnapshotFormatter.ts @@ -4,7 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type {TextSnapshot, TextSnapshotNode} from '../types.js'; +import type {TextSnapshot} from '../TextSnapshot.js'; +import type {TextSnapshotNode} from '../types.js'; export class SnapshotFormatter { #snapshot: TextSnapshot; diff --git a/src/tools/ToolDefinition.ts b/src/tools/ToolDefinition.ts index 1ff7b13dc..5321e411b 100644 --- a/src/tools/ToolDefinition.ts +++ b/src/tools/ToolDefinition.ts @@ -225,12 +225,7 @@ export type Context = Readonly<{ page: ContextPage, cdpBackendNodeId: number, ): string | undefined; - createTextSnapshot( - page: ContextPage, - verbose: boolean, - devtoolsData: DevToolsData | undefined, - extraHandles?: ElementHandle[], - ): Promise; + getSelectedMcpPage(): McpPage; getExtensionServiceWorkers(): ExtensionServiceWorker[]; getExtensionServiceWorkerId( diff --git a/src/types.ts b/src/types.ts index 4efd8dc93..13e64f58d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -24,17 +24,6 @@ export interface GeolocationOptions { longitude: number; } -export interface TextSnapshot { - root: TextSnapshotNode; - idToNode: Map; - snapshotId: string; - selectedElementUid?: string; - // It might happen that there is a selected element, but it is not part of the - // snapshot. This flag indicates if there is any selected element. - hasSelectedElement: boolean; - verbose: boolean; -} - export interface EmulationSettings { networkConditions?: string; cpuThrottlingRate?: number; diff --git a/tests/McpContext.test.ts b/tests/McpContext.test.ts index 908da9cad..c3f295799 100644 --- a/tests/McpContext.test.ts +++ b/tests/McpContext.test.ts @@ -10,6 +10,7 @@ import {afterEach, describe, it} from 'node:test'; import sinon from 'sinon'; import {NetworkFormatter} from '../src/formatters/NetworkFormatter.js'; +import {TextSnapshot} from '../src/TextSnapshot.js'; import type {HTTPResponse} from '../src/third_party/index.js'; import type {TraceResult} from '../src/trace-processing/parse.js'; import type {TextSnapshotNode} from '../src/types.js'; @@ -31,9 +32,9 @@ describe('McpContext', () => { value="Input" />`, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + page.textSnapshot = await TextSnapshot.create(page, context); assert.ok(await page.getElementByUid('1_1')); - await context.createTextSnapshot(context.getSelectedMcpPage()); + page.textSnapshot = await TextSnapshot.create(page, context); await page.getElementByUid('1_1'); }); }); @@ -103,7 +104,9 @@ describe('McpContext', () => { // Page 1: set content and snapshot const page1 = context.getSelectedMcpPage(); await page1.pptrPage.setContent(html``); - await context.createTextSnapshot(page1, false, undefined); + page1.textSnapshot = await TextSnapshot.create(page1, context, { + verbose: false, + }); // Capture a uid from page1's snapshot (snapshotId=1, button is node 1) const page1Uid = '1_1'; @@ -114,7 +117,9 @@ describe('McpContext', () => { const page2 = await context.newPage(); context.selectPage(page2); await page2.pptrPage.setContent(html``); - await context.createTextSnapshot(page2, false, undefined); + page2.textSnapshot = await TextSnapshot.create(page2, context, { + verbose: false, + }); // Page 2 is now selected. Page 1's uid should still resolve. const node = context.getAXNodeByUid(page1Uid); @@ -238,7 +243,10 @@ describe('McpContext', () => { } // Verify it is not in the snapshot by default (due to role="none") - await context.createTextSnapshot(page, false, undefined, []); + page.textSnapshot = await TextSnapshot.create(page, context, { + verbose: false, + extraHandles: [], + }); const snapshotBefore = page.textSnapshot; if (!snapshotBefore) { throw new Error('Snapshot not created'); @@ -257,7 +265,10 @@ describe('McpContext', () => { ); // Now take snapshot with extra handle - await context.createTextSnapshot(page, false, undefined, [middleHandle]); + page.textSnapshot = await TextSnapshot.create(page, context, { + verbose: false, + extraHandles: [middleHandle], + }); const snapshot = page.textSnapshot; if (!snapshot) { diff --git a/tests/formatters/snapshotFormatter.test.ts b/tests/formatters/snapshotFormatter.test.ts index 90b1f07fe..39385286e 100644 --- a/tests/formatters/snapshotFormatter.test.ts +++ b/tests/formatters/snapshotFormatter.test.ts @@ -10,7 +10,8 @@ import {describe, it} from 'node:test'; import type {ElementHandle} from 'puppeteer-core'; import {SnapshotFormatter} from '../../src/formatters/SnapshotFormatter.js'; -import type {TextSnapshot, TextSnapshotNode} from '../../src/types.js'; +import type {TextSnapshot} from '../../src/TextSnapshot.js'; +import type {TextSnapshotNode} from '../../src/types.js'; describe('snapshotFormatter', () => { it('formats a snapshot with value properties', () => { diff --git a/tests/tools/console.test.ts b/tests/tools/console.test.ts index 82f27784b..0d7eae148 100644 --- a/tests/tools/console.test.ts +++ b/tests/tools/console.test.ts @@ -10,6 +10,7 @@ import {before, describe, it} from 'node:test'; import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js'; import {loadIssueDescriptions} from '../../src/issue-descriptions.js'; import {McpResponse} from '../../src/McpResponse.js'; +import {TextSnapshot} from '../../src/TextSnapshot.js'; import {DevTools} from '../../src/third_party/index.js'; import { getConsoleMessage, @@ -205,7 +206,7 @@ describe('console', () => { await page.pptrPage.setContent( '', ); - await context.createTextSnapshot(page); + page.textSnapshot = await TextSnapshot.create(page, context); await issuePromise; await listConsoleMessages().handler( {params: {}, page: context.getSelectedMcpPage()}, @@ -250,7 +251,7 @@ describe('console', () => { }); `); - await context.createTextSnapshot(page); + page.textSnapshot = await TextSnapshot.create(page, context); await issuePromise; const messages = context.getConsoleData(page); let issueMsg; diff --git a/tests/tools/input.test.ts b/tests/tools/input.test.ts index 5590e88c1..a65fe563d 100644 --- a/tests/tools/input.test.ts +++ b/tests/tools/input.test.ts @@ -11,6 +11,7 @@ import {describe, it} from 'node:test'; import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js'; import {McpResponse} from '../../src/McpResponse.js'; +import {TextSnapshot} from '../../src/TextSnapshot.js'; import { click, hover, @@ -36,7 +37,10 @@ describe('input', () => { await page.setContent( html``, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await click.handler( { params: { @@ -63,7 +67,10 @@ describe('input', () => { >test`, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await click.handler( { params: { @@ -98,7 +105,10 @@ describe('input', () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPptrPage(); await page.goto(server.getRoute('/link')); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); const clickPromise = click.handler( { params: { @@ -141,7 +151,10 @@ describe('input', () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPptrPage(); await page.goto(server.getRoute('/unstable')); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); const handlerResolveTime = await click .handler( { @@ -169,7 +182,10 @@ describe('input', () => { await page.setContent( html``, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await click.handler( { params: { @@ -194,7 +210,10 @@ describe('input', () => { await page.setContent( html``, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await click.handler( { params: { @@ -222,7 +241,10 @@ describe('input', () => { await page.setContent( html``, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await hover.handler( { params: { @@ -253,7 +275,10 @@ describe('input', () => { onclick="this.innerText = 'clicked'" >`, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await clickAt.handler( { params: { @@ -283,7 +308,10 @@ describe('input', () => { ondblclick="this.innerText = 'dblclicked'" >`, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await clickAt.handler( { params: { @@ -311,7 +339,10 @@ describe('input', () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPptrPage(); await page.setContent(html``); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await fill.handler( { params: { @@ -341,7 +372,10 @@ describe('input', () => { >`, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await fill.handler( { params: { @@ -369,7 +403,10 @@ describe('input', () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPptrPage(); await page.setContent(html``); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await fill.handler( { params: { @@ -398,7 +435,10 @@ describe('input', () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPptrPage(); await page.setContent(html``); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); page.setDefaultTimeout(1000); await fill.handler( { @@ -431,7 +471,10 @@ describe('input', () => { const page = context.getSelectedPptrPage(); await page.setContent(html``); await page.click('textarea'); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await typeText.handler( { params: { @@ -457,7 +500,10 @@ describe('input', () => { const page = context.getSelectedPptrPage(); await page.setContent(html``); await page.click('textarea'); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await typeText.handler( { params: { @@ -494,7 +540,10 @@ describe('input', () => { const page = context.getSelectedPptrPage(); await page.setContent(html``); await page.click('textarea'); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); try { await typeText.handler( { @@ -528,7 +577,10 @@ describe('input', () => { /> `, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); // Fill email const response1 = new McpResponse({} as ParsedArguments); @@ -622,7 +674,10 @@ describe('input', () => { }); `, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await drag.handler( { params: { @@ -666,7 +721,10 @@ describe('input', () => { /> `, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await fillForm.handler( { params: { @@ -721,7 +779,10 @@ describe('input', () => { /> `, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await uploadFile.handler( { params: { @@ -764,7 +825,10 @@ describe('input', () => { }); `, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await uploadFile.handler( { params: { @@ -798,7 +862,10 @@ describe('input', () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPptrPage(); await page.setContent(html`
Not a file input
`); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await assert.rejects( uploadFile.handler( @@ -864,7 +931,10 @@ describe('input', () => { document.addEventListener('keyup', e => logs.push('u' + e.key)); `, ); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await pressKey.handler( { diff --git a/tests/tools/screenshot.test.ts b/tests/tools/screenshot.test.ts index 9d177f9bb..65193bfd4 100644 --- a/tests/tools/screenshot.test.ts +++ b/tests/tools/screenshot.test.ts @@ -10,6 +10,7 @@ import {tmpdir} from 'node:os'; import {join} from 'node:path'; import {describe, it} from 'node:test'; +import {TextSnapshot} from '../../src/TextSnapshot.js'; import {screenshot} from '../../src/tools/screenshot.js'; import {screenshots} from '../snapshot.js'; import {html, withMcpContext} from '../utils.js'; @@ -155,7 +156,10 @@ describe('screenshot', () => { const page = context.getSelectedPptrPage(); await page.setContent(fixture.html); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await screenshot.handler( { params: { diff --git a/tests/tools/script.test.ts b/tests/tools/script.test.ts index b0d57678a..b6bb63cea 100644 --- a/tests/tools/script.test.ts +++ b/tests/tools/script.test.ts @@ -9,6 +9,7 @@ import path from 'node:path'; import {describe, it} from 'node:test'; import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js'; +import {TextSnapshot} from '../../src/TextSnapshot.js'; import {installExtension} from '../../src/tools/extensions.js'; import {evaluateScript} from '../../src/tools/script.js'; import {serverHooks} from '../server.js'; @@ -201,7 +202,10 @@ describe('script', () => { await page.setContent(html``); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await evaluateScript().handler( { @@ -226,7 +230,10 @@ describe('script', () => { await page.setContent(html``); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await evaluateScript().handler( { @@ -255,7 +262,10 @@ describe('script', () => { await withMcpContext(async (response, context) => { const page = context.getSelectedPptrPage(); await page.goto(server.getRoute('/main')); - await context.createTextSnapshot(context.getSelectedMcpPage()); + context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( + context.getSelectedMcpPage(), + context, + ); await evaluateScript().handler( { params: { diff --git a/tests/utils.ts b/tests/utils.ts index 811f164e1..c8bc1a204 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -25,6 +25,7 @@ import sinon from 'sinon'; import type {ParsedArguments} from '../src/bin/chrome-devtools-mcp-cli-options.js'; import {McpContext} from '../src/McpContext.js'; import {McpResponse} from '../src/McpResponse.js'; +import {TextSnapshot} from '../src/TextSnapshot.js'; import {DevTools} from '../src/third_party/index.js'; import {stableIdSymbol} from '../src/utils/id.js'; @@ -118,6 +119,7 @@ export async function withMcpContext( args: ParsedArguments = {} as ParsedArguments, ) { await withBrowser(async browser => { + TextSnapshot.resetCounter(); const response = new McpResponse(args); if (context) { context.dispose(); From b5f1a04a35302afc9293deb3b24597bb234b8aae Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Tue, 21 Apr 2026 16:02:29 +0000 Subject: [PATCH 4/9] resolveCdpElementId fixes --- src/McpPage.ts | 2 +- src/TextSnapshot.ts | 5 +---- src/tools/ToolDefinition.ts | 5 ----- tests/tools/inPage.test.ts | 2 +- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/McpPage.ts b/src/McpPage.ts index 3546b72b2..685177083 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -267,7 +267,7 @@ export class McpPage implements ContextPage { cdpElementIds.push(`stashed-${i}`); continue; } - const cdpElementId = context.resolveCdpElementId(this, backendNodeId); + const cdpElementId = this.resolveCdpElementId(backendNodeId); if (!cdpElementId) { logger(`Could not get cdpElementId for backend node ${backendNodeId}`); cdpElementIds.push(`stashed-${i}`); diff --git a/src/TextSnapshot.ts b/src/TextSnapshot.ts index b6a728c5c..e39e5cdfb 100644 --- a/src/TextSnapshot.ts +++ b/src/TextSnapshot.ts @@ -134,10 +134,7 @@ export class TextSnapshot { const data = options.devtoolsData ?? (await context.getDevToolsData(page)); if (data?.cdpBackendNodeId) { snapshot.hasSelectedElement = true; - snapshot.selectedElementUid = context.resolveCdpElementId( - page, - data.cdpBackendNodeId, - ); + snapshot.selectedElementUid = page.resolveCdpElementId(data.cdpBackendNodeId); } // Clean up unique IDs that we did not see anymore. diff --git a/src/tools/ToolDefinition.ts b/src/tools/ToolDefinition.ts index 5321e411b..db995b735 100644 --- a/src/tools/ToolDefinition.ts +++ b/src/tools/ToolDefinition.ts @@ -221,11 +221,6 @@ export type Context = Readonly<{ triggerExtensionAction(id: string): Promise; listExtensions(): Promise>; getExtension(id: string): Promise; - resolveCdpElementId( - page: ContextPage, - cdpBackendNodeId: number, - ): string | undefined; - getSelectedMcpPage(): McpPage; getExtensionServiceWorkers(): ExtensionServiceWorker[]; getExtensionServiceWorkerId( diff --git a/tests/tools/inPage.test.ts b/tests/tools/inPage.test.ts index 70944a78b..592f47c7d 100644 --- a/tests/tools/inPage.test.ts +++ b/tests/tools/inPage.test.ts @@ -623,7 +623,7 @@ describe('inPage', () => { }); const stub = sinon - .stub(context, 'resolveCdpElementId') + .stub(page, 'resolveCdpElementId') .returns('mock-uid'); await executeInPageTool.handler( From 1c6f5f18906c18e61f1344641aa2010ad4ea324e Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Tue, 21 Apr 2026 17:27:15 +0000 Subject: [PATCH 5/9] move getDevToolsData --- src/McpContext.ts | 47 +--------------------------------- src/McpPage.ts | 42 +++++++++++++++++++++++++++--- src/McpResponse.ts | 2 +- src/TextSnapshot.ts | 9 ++++--- src/tools/ToolDefinition.ts | 3 +-- src/tools/inPage.ts | 4 +-- src/tools/network.ts | 4 +-- tests/McpContext.test.ts | 14 +++++----- tests/tools/console.test.ts | 4 +-- tests/tools/input.test.ts | 23 ----------------- tests/tools/screenshot.test.ts | 1 - tests/tools/script.test.ts | 3 --- 12 files changed, 60 insertions(+), 96 deletions(-) diff --git a/src/McpContext.ts b/src/McpContext.ts index cd9a8f47d..f10a68de2 100644 --- a/src/McpContext.ts +++ b/src/McpContext.ts @@ -35,11 +35,7 @@ import {Locator} from './third_party/index.js'; import {PredefinedNetworkConditions} from './third_party/index.js'; import {listPages} from './tools/pages.js'; import {CLOSE_PAGE_ERROR} from './tools/ToolDefinition.js'; -import type { - Context, - DevToolsData, - SupportedExtensions, -} from './tools/ToolDefinition.js'; +import type {Context, SupportedExtensions} from './tools/ToolDefinition.js'; import type {TraceResult} from './trace-processing/parse.js'; import type { EmulationSettings, @@ -643,47 +639,6 @@ export class McpContext implements Context { return this.#mcpPages.get(page)?.isolatedContextName; } - getDevToolsPage(page: Page): Page | undefined { - return this.#mcpPages.get(page)?.devToolsPage; - } - - async getDevToolsData(page: McpPage): Promise { - try { - this.logger('Getting DevTools UI data'); - const devtoolsPage = this.getDevToolsPage(page.pptrPage); - if (!devtoolsPage) { - this.logger('No DevTools page detected'); - return {}; - } - const {cdpRequestId, cdpBackendNodeId} = await devtoolsPage.evaluate( - async () => { - // @ts-expect-error no types - const UI = await import('/bundled/ui/legacy/legacy.js'); - // @ts-expect-error no types - const SDK = await import('/bundled/core/sdk/sdk.js'); - const request = UI.Context.Context.instance().flavor( - SDK.NetworkRequest.NetworkRequest, - ); - const node = UI.Context.Context.instance().flavor( - SDK.DOMModel.DOMNode, - ); - return { - cdpRequestId: request?.requestId(), - cdpBackendNodeId: node?.backendNodeId(), - }; - }, - ); - return {cdpBackendNodeId, cdpRequestId}; - } catch (err) { - this.logger('error getting devtools data', err); - } - return {}; - } - - /** - * Creates a text snapshot of a page. - */ - async saveTemporaryFile( data: Uint8Array, filename: string, diff --git a/src/McpPage.ts b/src/McpPage.ts index 685177083..1b8fec9f6 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -15,7 +15,11 @@ import type { } from './third_party/index.js'; import type {ToolGroup, ToolDefinition} from './tools/inPage.js'; import {takeSnapshot} from './tools/snapshot.js'; -import type {ContextPage, Context, Response} from './tools/ToolDefinition.js'; +import type { + ContextPage, + DevToolsData, + Response, +} from './tools/ToolDefinition.js'; import type { EmulationSettings, GeolocationOptions, @@ -136,7 +140,6 @@ export class McpPage implements ContextPage { toolName: string, params: Record, response: Response, - context: Context, ): Promise { // Creates array of ElementHandles from the UIDs in the params. // We do not replace the uids with the ElementsHandles yet, because @@ -277,7 +280,7 @@ export class McpPage implements ContextPage { } const resultWithStashedElements = result.result; if (elementHandles.length) { - this.textSnapshot = await TextSnapshot.create(this, context, { + this.textSnapshot = await TextSnapshot.create(this, { extraHandles: elementHandles, }); response.includeSnapshot(); @@ -370,4 +373,37 @@ export class McpPage implements ContextPage { } return; } + + async getDevToolsData(): Promise { + try { + logger('Getting DevTools UI data'); + const devtoolsPage = this.devToolsPage; + if (!devtoolsPage) { + logger('No DevTools page detected'); + return {}; + } + const {cdpRequestId, cdpBackendNodeId} = await devtoolsPage.evaluate( + async () => { + // @ts-expect-error no types + const UI = await import('/bundled/ui/legacy/legacy.js'); + // @ts-expect-error no types + const SDK = await import('/bundled/core/sdk/sdk.js'); + const request = UI.Context.Context.instance().flavor( + SDK.NetworkRequest.NetworkRequest, + ); + const node = UI.Context.Context.instance().flavor( + SDK.DOMModel.DOMNode, + ); + return { + cdpRequestId: request?.requestId(), + cdpBackendNodeId: node?.backendNodeId(), + }; + }, + ); + return {cdpBackendNodeId, cdpRequestId}; + } catch (err) { + logger('error getting devtools data', err); + } + return {}; + } } diff --git a/src/McpResponse.ts b/src/McpResponse.ts index 9605fca3e..fa487047d 100644 --- a/src/McpResponse.ts +++ b/src/McpResponse.ts @@ -444,7 +444,7 @@ export class McpResponse implements Response { if (!this.#page) { throw new Error('Response must have a page'); } - this.#page.textSnapshot = await TextSnapshot.create(this.#page, context, { + this.#page.textSnapshot = await TextSnapshot.create(this.#page, { verbose: this.#snapshotParams.verbose, devtoolsData: this.#devToolsData, }); diff --git a/src/TextSnapshot.ts b/src/TextSnapshot.ts index e39e5cdfb..4f084e8e4 100644 --- a/src/TextSnapshot.ts +++ b/src/TextSnapshot.ts @@ -11,7 +11,7 @@ import type { SerializedAXNode, ElementHandle, } from './third_party/index.js'; -import type {Context, DevToolsData} from './tools/ToolDefinition.js'; +import type {DevToolsData} from './tools/ToolDefinition.js'; import type {TextSnapshotNode} from './types.js'; export class TextSnapshot { @@ -46,7 +46,6 @@ export class TextSnapshot { static async create( page: McpPage, - context: Context, options: { verbose?: boolean; devtoolsData?: DevToolsData; @@ -131,10 +130,12 @@ export class TextSnapshot { verbose, }); - const data = options.devtoolsData ?? (await context.getDevToolsData(page)); + const data = options.devtoolsData ?? (await page.getDevToolsData()); if (data?.cdpBackendNodeId) { snapshot.hasSelectedElement = true; - snapshot.selectedElementUid = page.resolveCdpElementId(data.cdpBackendNodeId); + snapshot.selectedElementUid = page.resolveCdpElementId( + data.cdpBackendNodeId, + ); } // Clean up unique IDs that we did not see anymore. diff --git a/src/tools/ToolDefinition.ts b/src/tools/ToolDefinition.ts index db995b735..762c800c4 100644 --- a/src/tools/ToolDefinition.ts +++ b/src/tools/ToolDefinition.ts @@ -204,7 +204,6 @@ export type Context = Readonly<{ timeout?: number, page?: Page, ): Promise; - getDevToolsData(page: ContextPage): Promise; /** * Returns a reqid for a cdpRequestId. */ @@ -253,8 +252,8 @@ export type ContextPage = Readonly<{ toolName: string, params: Record, response: Response, - context: Context, ): Promise; + getDevToolsData(): Promise; }>; export function defineTool( diff --git a/src/tools/inPage.ts b/src/tools/inPage.ts index 61a76b052..5c3398723 100644 --- a/src/tools/inPage.ts +++ b/src/tools/inPage.ts @@ -71,7 +71,7 @@ export const executeInPageTool = definePageTool({ .optional() .describe('The JSON-stringified parameters to pass to the tool'), }, - handler: async (request, response, context) => { + handler: async (request, response) => { const toolName = request.params.toolName; let params: Record = {}; if (request.params.params) { @@ -102,6 +102,6 @@ export const executeInPageTool = definePageTool({ ); } - await request.page.executeInPageTool(toolName, params, response, context); + await request.page.executeInPageTool(toolName, params, response); }, }); diff --git a/src/tools/network.ts b/src/tools/network.ts index 596afa546..181a2ed26 100644 --- a/src/tools/network.ts +++ b/src/tools/network.ts @@ -71,7 +71,7 @@ export const listNetworkRequests = definePageTool({ ), }, handler: async (request, response, context) => { - const data = await context.getDevToolsData(request.page); + const data = await request.page.getDevToolsData(); response.attachDevToolsData(data); const reqid = data?.cdpRequestId ? context.resolveCdpRequestId(request.page, data.cdpRequestId) @@ -120,7 +120,7 @@ export const getNetworkRequest = definePageTool({ responseFilePath: request.params.responseFilePath, }); } else { - const data = await context.getDevToolsData(request.page); + const data = await request.page.getDevToolsData(); response.attachDevToolsData(data); const reqid = data?.cdpRequestId ? context.resolveCdpRequestId(request.page, data.cdpRequestId) diff --git a/tests/McpContext.test.ts b/tests/McpContext.test.ts index c3f295799..bccac0ce0 100644 --- a/tests/McpContext.test.ts +++ b/tests/McpContext.test.ts @@ -32,9 +32,9 @@ describe('McpContext', () => { value="Input" />`, ); - page.textSnapshot = await TextSnapshot.create(page, context); + page.textSnapshot = await TextSnapshot.create(page); assert.ok(await page.getElementByUid('1_1')); - page.textSnapshot = await TextSnapshot.create(page, context); + page.textSnapshot = await TextSnapshot.create(page); await page.getElementByUid('1_1'); }); }); @@ -92,7 +92,7 @@ describe('McpContext', () => { async (_response, context) => { const page = await context.newPage(); await context.createPagesSnapshot(); - assert.ok(context.getDevToolsPage(page.pptrPage)); + assert.ok(page.devToolsPage); }, { autoOpenDevTools: true, @@ -104,7 +104,7 @@ describe('McpContext', () => { // Page 1: set content and snapshot const page1 = context.getSelectedMcpPage(); await page1.pptrPage.setContent(html``); - page1.textSnapshot = await TextSnapshot.create(page1, context, { + page1.textSnapshot = await TextSnapshot.create(page1, { verbose: false, }); @@ -117,7 +117,7 @@ describe('McpContext', () => { const page2 = await context.newPage(); context.selectPage(page2); await page2.pptrPage.setContent(html``); - page2.textSnapshot = await TextSnapshot.create(page2, context, { + page2.textSnapshot = await TextSnapshot.create(page2, { verbose: false, }); @@ -243,7 +243,7 @@ describe('McpContext', () => { } // Verify it is not in the snapshot by default (due to role="none") - page.textSnapshot = await TextSnapshot.create(page, context, { + page.textSnapshot = await TextSnapshot.create(page, { verbose: false, extraHandles: [], }); @@ -265,7 +265,7 @@ describe('McpContext', () => { ); // Now take snapshot with extra handle - page.textSnapshot = await TextSnapshot.create(page, context, { + page.textSnapshot = await TextSnapshot.create(page, { verbose: false, extraHandles: [middleHandle], }); diff --git a/tests/tools/console.test.ts b/tests/tools/console.test.ts index 0d7eae148..90c23de23 100644 --- a/tests/tools/console.test.ts +++ b/tests/tools/console.test.ts @@ -206,7 +206,7 @@ describe('console', () => { await page.pptrPage.setContent( '', ); - page.textSnapshot = await TextSnapshot.create(page, context); + page.textSnapshot = await TextSnapshot.create(page); await issuePromise; await listConsoleMessages().handler( {params: {}, page: context.getSelectedMcpPage()}, @@ -251,7 +251,7 @@ describe('console', () => { }); `); - page.textSnapshot = await TextSnapshot.create(page, context); + page.textSnapshot = await TextSnapshot.create(page); await issuePromise; const messages = context.getConsoleData(page); let issueMsg; diff --git a/tests/tools/input.test.ts b/tests/tools/input.test.ts index a65fe563d..b0033ec4d 100644 --- a/tests/tools/input.test.ts +++ b/tests/tools/input.test.ts @@ -39,7 +39,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await click.handler( { @@ -69,7 +68,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await click.handler( { @@ -107,7 +105,6 @@ describe('input', () => { await page.goto(server.getRoute('/link')); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); const clickPromise = click.handler( { @@ -153,7 +150,6 @@ describe('input', () => { await page.goto(server.getRoute('/unstable')); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); const handlerResolveTime = await click .handler( @@ -184,7 +180,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await click.handler( { @@ -212,7 +207,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await click.handler( { @@ -243,7 +237,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await hover.handler( { @@ -277,7 +270,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await clickAt.handler( { @@ -310,7 +302,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await clickAt.handler( { @@ -341,7 +332,6 @@ describe('input', () => { await page.setContent(html``); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await fill.handler( { @@ -374,7 +364,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await fill.handler( { @@ -405,7 +394,6 @@ describe('input', () => { await page.setContent(html``); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await fill.handler( { @@ -437,7 +425,6 @@ describe('input', () => { await page.setContent(html``); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); page.setDefaultTimeout(1000); await fill.handler( @@ -473,7 +460,6 @@ describe('input', () => { await page.click('textarea'); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await typeText.handler( { @@ -502,7 +488,6 @@ describe('input', () => { await page.click('textarea'); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await typeText.handler( { @@ -542,7 +527,6 @@ describe('input', () => { await page.click('textarea'); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); try { await typeText.handler( @@ -579,7 +563,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); // Fill email @@ -676,7 +659,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await drag.handler( { @@ -723,7 +705,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await fillForm.handler( { @@ -781,7 +762,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await uploadFile.handler( { @@ -827,7 +807,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await uploadFile.handler( { @@ -864,7 +843,6 @@ describe('input', () => { await page.setContent(html`
Not a file input
`); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await assert.rejects( @@ -933,7 +911,6 @@ describe('input', () => { ); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await pressKey.handler( diff --git a/tests/tools/screenshot.test.ts b/tests/tools/screenshot.test.ts index 65193bfd4..b6aee504d 100644 --- a/tests/tools/screenshot.test.ts +++ b/tests/tools/screenshot.test.ts @@ -158,7 +158,6 @@ describe('screenshot', () => { await page.setContent(fixture.html); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await screenshot.handler( { diff --git a/tests/tools/script.test.ts b/tests/tools/script.test.ts index b6bb63cea..c87048fbe 100644 --- a/tests/tools/script.test.ts +++ b/tests/tools/script.test.ts @@ -204,7 +204,6 @@ describe('script', () => { context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await evaluateScript().handler( @@ -232,7 +231,6 @@ describe('script', () => { context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await evaluateScript().handler( @@ -264,7 +262,6 @@ describe('script', () => { await page.goto(server.getRoute('/main')); context.getSelectedMcpPage().textSnapshot = await TextSnapshot.create( context.getSelectedMcpPage(), - context, ); await evaluateScript().handler( { From 52731f857ccdfed3aa58ebbb32e6bf781cbab360 Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Wed, 22 Apr 2026 11:26:54 +0000 Subject: [PATCH 6/9] fix refactor by taking snapshot earlier --- src/McpPage.ts | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/McpPage.ts b/src/McpPage.ts index 1b8fec9f6..4edd6a05e 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -253,7 +253,6 @@ export class McpPage implements ContextPage { ); const elementHandles: ElementHandle[] = []; - const cdpElementIds: string[] = []; for (let i = 0; i < (result.stashed ?? 0); i++) { const elementHandle = await this.pptrPage.evaluateHandle(index => { const el = window.__dtmcp?.stashedElements?.[index]; @@ -263,28 +262,32 @@ export class McpPage implements ContextPage { return el; }, i); elementHandles.push(elementHandle); + } + + if (elementHandles.length) { + this.textSnapshot = await TextSnapshot.create(this, { + extraHandles: elementHandles, + }); + response.includeSnapshot(); + } + const cdpElementIds: string[] = []; + // can this be mapped? + for (const [index, elementHandle] of elementHandles.entries()) { const backendNodeId = await elementHandle.backendNodeId(); if (!backendNodeId) { - logger(`No backendNodeId for stashed DOM element with index ${i}`); - cdpElementIds.push(`stashed-${i}`); + logger(`No backendNodeId for stashed DOM element with index ${index}`); + cdpElementIds.push(`stashed-${index}`); continue; } const cdpElementId = this.resolveCdpElementId(backendNodeId); if (!cdpElementId) { logger(`Could not get cdpElementId for backend node ${backendNodeId}`); - cdpElementIds.push(`stashed-${i}`); + cdpElementIds.push(`stashed-${index}`); continue; } cdpElementIds.push(cdpElementId); } - const resultWithStashedElements = result.result; - if (elementHandles.length) { - this.textSnapshot = await TextSnapshot.create(this, { - extraHandles: elementHandles, - }); - response.includeSnapshot(); - } const recursivelyReplaceStashedElements = (node: unknown): unknown => { if (Array.isArray(node)) { @@ -309,9 +312,7 @@ export class McpPage implements ContextPage { return node; }; - const resultWithUids = recursivelyReplaceStashedElements( - resultWithStashedElements, - ); + const resultWithUids = recursivelyReplaceStashedElements(result.result); response.appendResponseLine(JSON.stringify(resultWithUids, null, 2)); } From 9bcd5b94288c30c214f77f374a8626cb575abd6f Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Wed, 22 Apr 2026 13:11:09 +0000 Subject: [PATCH 7/9] add/move tests --- tests/McpContext.test.ts | 141 --------------------------- tests/TextSnapshot.test.ts | 175 +++++++++++++++++++++++++++++++++ tests/tools/inPage.test.ts | 191 ++++++++++++++++++++++++------------- 3 files changed, 300 insertions(+), 207 deletions(-) create mode 100644 tests/TextSnapshot.test.ts diff --git a/tests/McpContext.test.ts b/tests/McpContext.test.ts index bccac0ce0..a33dc71e5 100644 --- a/tests/McpContext.test.ts +++ b/tests/McpContext.test.ts @@ -13,7 +13,6 @@ import {NetworkFormatter} from '../src/formatters/NetworkFormatter.js'; import {TextSnapshot} from '../src/TextSnapshot.js'; import type {HTTPResponse} from '../src/third_party/index.js'; import type {TraceResult} from '../src/trace-processing/parse.js'; -import type {TextSnapshotNode} from '../src/types.js'; import {getMockRequest, html, withMcpContext} from './utils.js'; @@ -214,144 +213,4 @@ describe('McpContext', () => { fromStub.restore(); }); }); - - it('inserts extraHandles into the snapshot correctly', async () => { - await withMcpContext(async (_response, context) => { - const page = context.getSelectedMcpPage(); - await page.pptrPage.setContent(html` -
-
- -
-
- `); - - const middleHandle = await page.pptrPage.$('#middle'); - if (!middleHandle) { - throw new Error('middle element not found'); - } - - const backendNodeId = await middleHandle.backendNodeId(); - if (!backendNodeId) { - throw new Error('Failed to get backendNodeId'); - } - - // Verify it is not in the snapshot by default (due to role="none") - page.textSnapshot = await TextSnapshot.create(page, { - verbose: false, - extraHandles: [], - }); - const snapshotBefore = page.textSnapshot; - if (!snapshotBefore) { - throw new Error('Snapshot not created'); - } - - let foundMiddleBefore = false; - for (const node of snapshotBefore.idToNode.values()) { - if (node.backendNodeId === backendNodeId) { - foundMiddleBefore = true; - break; - } - } - assert.ok( - !foundMiddleBefore, - 'Middle element should NOT be in the snapshot when not passed as extra handle', - ); - - // Now take snapshot with extra handle - page.textSnapshot = await TextSnapshot.create(page, { - verbose: false, - extraHandles: [middleHandle], - }); - - const snapshot = page.textSnapshot; - if (!snapshot) { - throw new Error('Snapshot not created'); - } - - // Find the extra node in idToNode - let extraNode: TextSnapshotNode | undefined; - for (const node of snapshot.idToNode.values()) { - if (node.backendNodeId === backendNodeId) { - extraNode = node; - break; - } - } - - assert.ok(extraNode, 'Extra node should be in the snapshot'); - assert.strictEqual( - extraNode.role, - 'div', - 'Extra node should have role "div"', - ); - - // Check if the child was moved to extraNode - const childHandle = await page.pptrPage.$('#child'); - if (!childHandle) { - throw new Error('child element not found'); - } - const childBackendNodeId = await childHandle.backendNodeId(); - - let foundChild = false; - for (const child of extraNode.children) { - if (child.backendNodeId === childBackendNodeId) { - foundChild = true; - break; - } - } - assert.ok( - foundChild, - 'Child node should be moved to extra node children', - ); - - // Find parent node in snapshot - const parentHandle = await page.pptrPage.$('#parent'); - if (!parentHandle) { - throw new Error('parent element not found'); - } - const parentBackendId = await parentHandle.backendNodeId(); - - let parentNode: TextSnapshotNode | undefined; - for (const node of snapshot.idToNode.values()) { - if (node.backendNodeId === parentBackendId) { - parentNode = node; - break; - } - } - - assert.ok(parentNode, 'Parent node should be in snapshot'); - - // Check that child is NOT a child of parent anymore - let foundChildInParent = false; - for (const child of parentNode.children) { - if (child.backendNodeId === childBackendNodeId) { - foundChildInParent = true; - break; - } - } - assert.ok( - !foundChildInParent, - 'Child node should NOT be in parent children', - ); - - // Check that middle IS a child of parent - let foundMiddleInParent = false; - for (const child of parentNode.children) { - if (child.backendNodeId === backendNodeId) { - foundMiddleInParent = true; - break; - } - } - assert.ok( - foundMiddleInParent, - 'Middle node should be in parent children', - ); - }); - }); }); diff --git a/tests/TextSnapshot.test.ts b/tests/TextSnapshot.test.ts new file mode 100644 index 000000000..7627ea9c1 --- /dev/null +++ b/tests/TextSnapshot.test.ts @@ -0,0 +1,175 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'node:assert'; +import {afterEach, describe, it} from 'node:test'; + +import sinon from 'sinon'; + +import {TextSnapshot} from '../src/TextSnapshot.js'; +import type {TextSnapshotNode} from '../src/types.js'; + +import {html, withMcpContext} from './utils.js'; + +describe('TextSnapshot', () => { + afterEach(() => { + sinon.restore(); + TextSnapshot.resetCounter(); + }); + + it('creates a snapshot', async () => { + await withMcpContext(async (_response, context) => { + const page = context.getSelectedMcpPage(); + await page.pptrPage.setContent(html``); + + const snapshot = await TextSnapshot.create(page); + + assert.ok(snapshot); + assert.strictEqual(snapshot.snapshotId, '1'); + assert.ok(snapshot.root); + + let foundButton = false; + for (const node of snapshot.idToNode.values()) { + if (node.role === 'button' && node.name === 'Click me') { + foundButton = true; + break; + } + } + assert.ok(foundButton, 'Button should be in the snapshot'); + }); + }); + + it('inserts extraHandles into the snapshot correctly', async () => { + await withMcpContext(async (_response, context) => { + const page = context.getSelectedMcpPage(); + await page.pptrPage.setContent(html` +
+
+ +
+
+ `); + + const middleHandle = await page.pptrPage.$('#middle'); + if (!middleHandle) { + throw new Error('middle element not found'); + } + + const backendNodeId = await middleHandle.backendNodeId(); + if (!backendNodeId) { + throw new Error('Failed to get backendNodeId'); + } + + // Verify it is not in the snapshot by default (due to role="none") + const snapshotBefore = await TextSnapshot.create(page, { + verbose: false, + extraHandles: [], + }); + + let foundMiddleBefore = false; + for (const node of snapshotBefore.idToNode.values()) { + if (node.backendNodeId === backendNodeId) { + foundMiddleBefore = true; + break; + } + } + assert.ok( + !foundMiddleBefore, + 'Middle element should NOT be in the snapshot when not passed as extra handle', + ); + + // Now take snapshot with extra handle + const snapshot = await TextSnapshot.create(page, { + verbose: false, + extraHandles: [middleHandle], + }); + + // Find the extra node in idToNode + let extraNode: TextSnapshotNode | undefined; + for (const node of snapshot.idToNode.values()) { + if (node.backendNodeId === backendNodeId) { + extraNode = node; + break; + } + } + + assert.ok(extraNode, 'Extra node should be in the snapshot'); + assert.strictEqual( + extraNode.role, + 'div', + 'Extra node should have role "div"', + ); + + // Check if the child was moved to extraNode + const childHandle = await page.pptrPage.$('#child'); + if (!childHandle) { + throw new Error('child element not found'); + } + const childBackendNodeId = await childHandle.backendNodeId(); + + let foundChild = false; + for (const child of extraNode.children) { + if (child.backendNodeId === childBackendNodeId) { + foundChild = true; + break; + } + } + assert.ok( + foundChild, + 'Child node should be moved to extra node children', + ); + + // Find parent node in snapshot + const parentHandle = await page.pptrPage.$('#parent'); + if (!parentHandle) { + throw new Error('parent element not found'); + } + const parentBackendId = await parentHandle.backendNodeId(); + + let parentNode: TextSnapshotNode | undefined; + for (const node of snapshot.idToNode.values()) { + if (node.backendNodeId === parentBackendId) { + parentNode = node; + break; + } + } + + assert.ok(parentNode, 'Parent node should be in snapshot'); + + // Check that child is NOT a child of parent anymore + let foundChildInParent = false; + for (const child of parentNode.children) { + if (child.backendNodeId === childBackendNodeId) { + foundChildInParent = true; + break; + } + } + assert.ok( + !foundChildInParent, + 'Child node should NOT be in parent children', + ); + + // Check that middle IS a child of parent + let foundMiddleInParent = false; + for (const child of parentNode.children) { + if (child.backendNodeId === backendNodeId) { + foundMiddleInParent = true; + break; + } + } + assert.ok( + foundMiddleInParent, + 'Middle node should be in parent children', + ); + }); + }); +}); diff --git a/tests/tools/inPage.test.ts b/tests/tools/inPage.test.ts index 592f47c7d..413efe396 100644 --- a/tests/tools/inPage.test.ts +++ b/tests/tools/inPage.test.ts @@ -12,8 +12,9 @@ import sinon from 'sinon'; import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js'; import type {McpContext} from '../../src/McpContext.js'; import type {McpResponse} from '../../src/McpResponse.js'; -import type {ToolGroup, ToolDefinition} from '../../src/tools/inPage.js'; +import {TextSnapshot} from '../../src/TextSnapshot.js'; import {executeInPageTool, listInPageTools} from '../../src/tools/inPage.js'; +import type {ToolGroup, ToolDefinition} from '../../src/tools/inPage.js'; import {withMcpContext} from '../utils.js'; describe('inPage', () => { @@ -650,70 +651,128 @@ describe('inPage', () => { ); }); - // it('creates a new snapshot if the stashed ID cannot be mapped to a UID initially', async () => { - // await withMcpContext( - // async (response, context) => { - // const page = await context.newPage(); - // response.setPage(page); - - // page.inPageTools = { - // name: 'test-group', - // description: 'test description', - // tools: [ - // { - // name: 'test-tool', - // description: 'test tool description', - // inputSchema: {}, - // }, - // ], - // }; - - // await page.pptrPage.evaluate(() => { - // window.__dtmcp = { - // executeTool: async () => { - // const div = document.createElement('div'); - // div.id = 'test-element'; - // document.body.appendChild(div); - // return div; - // }, - // }; - // }); - - // const stubResolve = sinon.stub(context, 'resolveCdpElementId'); - // stubResolve.onFirstCall().returns(undefined); - // stubResolve.onSecondCall().returns('mock-uid'); - - // const stubSnapshot = sinon - // .stub(context, 'createTextSnapshot') - // .resolves(); - - // await executeInPageTool.handler( - // { - // params: { - // toolName: 'test-tool', - // params: JSON.stringify({}), - // }, - // page: page, - // }, - // response, - // context, - // ); - - // assert.ok( - // stubSnapshot.calledOnce, - // 'Expected createTextSnapshot to be called', - // ); - // assert.strictEqual( - // response.responseLines[0], - // JSON.stringify({uid: 'mock-uid'}, null, 2), - // ); - - // stubResolve.restore(); - // stubSnapshot.restore(); - // }, - // undefined, - // {categoryInPageTools: true} as ParsedArguments, - // ); - // }); + it('creates a new snapshot if the in-page tool response contains a DOM element', async () => { + await withMcpContext( + async (response, context) => { + const page = await context.newPage(); + response.setPage(page); + + page.inPageTools = { + name: 'test-group', + description: 'test description', + tools: [ + { + name: 'test-tool', + description: 'test tool description', + inputSchema: {}, + }, + ], + }; + + await page.pptrPage.evaluate(() => { + window.__dtmcp = { + executeTool: async () => { + const div = document.createElement('div'); + div.id = 'test-element'; + document.body.appendChild(div); + return div; + }, + }; + }); + + const stubSnapshot = sinon + .stub(TextSnapshot, 'create') + .resolves({} as TextSnapshot); + + const stubResolve = sinon + .stub(page, 'resolveCdpElementId') + .returns('mock-uid'); + + await executeInPageTool.handler( + { + params: { + toolName: 'test-tool', + params: JSON.stringify({}), + }, + page: page, + }, + response, + context, + ); + + assert.ok( + stubSnapshot.calledOnce, + 'Expected TextSnapshot.create to be called', + ); + assert.strictEqual( + response.responseLines[0], + JSON.stringify({uid: 'mock-uid'}, null, 2), + ); + + stubResolve.restore(); + stubSnapshot.restore(); + }, + undefined, + {categoryInPageTools: true} as ParsedArguments, + ); + }); + + it('does not create a new snapshot if the in-page tool response does not contain a DOM element', async () => { + await withMcpContext( + async (response, context) => { + const page = await context.newPage(); + response.setPage(page); + + page.inPageTools = { + name: 'test-group', + description: 'test description', + tools: [ + { + name: 'test-tool', + description: 'test tool description', + inputSchema: {}, + }, + ], + }; + + await page.pptrPage.evaluate(() => { + window.__dtmcp = { + executeTool: async () => { + return 'simple-result'; + }, + }; + }); + + const stubSnapshot = sinon + .stub(TextSnapshot, 'create') + .resolves({} as TextSnapshot); + + await executeInPageTool.handler( + { + params: { + toolName: 'test-tool', + params: JSON.stringify({}), + }, + page: page, + }, + response, + context, + ); + + assert.ok( + stubSnapshot.notCalled, + 'Expected TextSnapshot.create not to be called', + ); + assert.strictEqual( + response.responseLines[0], + JSON.stringify('simple-result', null, 2), + ); + + stubSnapshot.restore(); + }, + undefined, + {categoryInPageTools: true} as ParsedArguments, + ); + }); }); }); From e963f172280559b1a2c0aacc35ae881ecbb7ce1d Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Wed, 22 Apr 2026 14:14:31 +0000 Subject: [PATCH 8/9] dispose handles --- src/McpPage.ts | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/McpPage.ts b/src/McpPage.ts index 4edd6a05e..c2c749b73 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -265,30 +265,33 @@ export class McpPage implements ContextPage { } if (elementHandles.length) { + const oldHandles = [...this.extraHandles]; this.textSnapshot = await TextSnapshot.create(this, { extraHandles: elementHandles, }); response.includeSnapshot(); - } - const cdpElementIds: string[] = []; - // can this be mapped? - for (const [index, elementHandle] of elementHandles.entries()) { - const backendNodeId = await elementHandle.backendNodeId(); - if (!backendNodeId) { - logger(`No backendNodeId for stashed DOM element with index ${index}`); - cdpElementIds.push(`stashed-${index}`); - continue; - } - const cdpElementId = this.resolveCdpElementId(backendNodeId); - if (!cdpElementId) { - logger(`Could not get cdpElementId for backend node ${backendNodeId}`); - cdpElementIds.push(`stashed-${index}`); - continue; + for (const handle of oldHandles) { + await handle.dispose().catch(e => logger('Failed to dispose old handle', e)); } - cdpElementIds.push(cdpElementId); } + const cdpElementIds = await Promise.all( + elementHandles.map(async (elementHandle, index) => { + const backendNodeId = await elementHandle.backendNodeId(); + if (!backendNodeId) { + logger(`No backendNodeId for stashed DOM element with index ${index}`); + return `stashed-${index}`; + } + const cdpElementId = this.resolveCdpElementId(backendNodeId); + if (!cdpElementId) { + logger(`Could not get cdpElementId for backend node ${backendNodeId}`); + return `stashed-${index}`; + } + return cdpElementId; + }) + ); + const recursivelyReplaceStashedElements = (node: unknown): unknown => { if (Array.isArray(node)) { return node.map(x => recursivelyReplaceStashedElements(x)); From 66924494b0a292a2037bc9a194a462bdc5ddf66a Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Wed, 22 Apr 2026 14:40:24 +0000 Subject: [PATCH 9/9] format --- src/McpPage.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/McpPage.ts b/src/McpPage.ts index c2c749b73..485e43006 100644 --- a/src/McpPage.ts +++ b/src/McpPage.ts @@ -272,7 +272,9 @@ export class McpPage implements ContextPage { response.includeSnapshot(); for (const handle of oldHandles) { - await handle.dispose().catch(e => logger('Failed to dispose old handle', e)); + await handle + .dispose() + .catch(e => logger('Failed to dispose old handle', e)); } } @@ -280,16 +282,20 @@ export class McpPage implements ContextPage { elementHandles.map(async (elementHandle, index) => { const backendNodeId = await elementHandle.backendNodeId(); if (!backendNodeId) { - logger(`No backendNodeId for stashed DOM element with index ${index}`); + logger( + `No backendNodeId for stashed DOM element with index ${index}`, + ); return `stashed-${index}`; } const cdpElementId = this.resolveCdpElementId(backendNodeId); if (!cdpElementId) { - logger(`Could not get cdpElementId for backend node ${backendNodeId}`); + logger( + `Could not get cdpElementId for backend node ${backendNodeId}`, + ); return `stashed-${index}`; } return cdpElementId; - }) + }), ); const recursivelyReplaceStashedElements = (node: unknown): unknown => {