Skip to content

Commit 5ae8622

Browse files
wolfibwolfib
committed
address comments
1 parent 532add2 commit 5ae8622

8 files changed

Lines changed: 106 additions & 50 deletions

File tree

‎src/McpResponse.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import type {
2020
ResourceType,
2121
TextContent,
2222
} from './third_party/index.js';
23-
import type {ToolGroup} from './tools/inPage.js';
23+
import type {ToolGroup, ToolDefinition} from './tools/inPage.js';
2424
import {handleDialog} from './tools/pages.js';
2525
import type {
2626
DevToolsData,
@@ -41,7 +41,7 @@ interface TraceInsightData {
4141
insightName: InsightName;
4242
}
4343

44-
async function getToolGroup(page: McpPage): Promise<ToolGroup | undefined> {
44+
async function getToolGroup(page: McpPage): Promise<ToolGroup<ToolDefinition> | undefined> {
4545
// Check if there is a `devtoolstooldiscovery` event listener
4646
const windowHandle = await page.pptrPage.evaluateHandle(() => window);
4747
// @ts-expect-error internal API
@@ -55,7 +55,7 @@ async function getToolGroup(page: McpPage): Promise<ToolGroup | undefined> {
5555
}
5656

5757
const toolGroup = await page.pptrPage.evaluate(() => {
58-
return new Promise<ToolGroup | undefined>(resolve => {
58+
return new Promise<ToolGroup<ToolDefinition> | undefined>(resolve => {
5959
const event = new CustomEvent('devtoolstooldiscovery');
6060
// @ts-expect-error Adding custom property
6161
event.respondWith = (toolGroup: ToolGroup) => {
@@ -83,7 +83,7 @@ async function getToolGroup(page: McpPage): Promise<ToolGroup | undefined> {
8383
resolve(toolGroup);
8484
};
8585
window.dispatchEvent(event);
86-
// If the page does not call `event.respondWith`, return instead of timing out
86+
// If the page does not synchronously call `event.respondWith`, return instead of timing out
8787
setTimeout(() => {
8888
resolve(undefined);
8989
}, 0);
@@ -417,7 +417,7 @@ export class McpResponse implements Response {
417417
extensions = context.listExtensions();
418418
}
419419

420-
let inPageTools: ToolGroup | undefined;
420+
let inPageTools: ToolGroup<ToolDefinition> | undefined;
421421
if (this.#listInPageTools) {
422422
inPageTools = await getToolGroup(context.getSelectedMcpPage());
423423
}
@@ -541,7 +541,7 @@ export class McpResponse implements Response {
541541
traceInsight?: TraceInsightData;
542542
extensions?: InstalledExtension[];
543543
lighthouseResult?: LighthouseData;
544-
inPageTools?: ToolGroup;
544+
inPageTools?: ToolGroup<ToolDefinition>;
545545
},
546546
): {content: Array<TextContent | ImageContent>; structuredContent: object} {
547547
const structuredContent: {
@@ -797,7 +797,7 @@ Call ${handleDialog.name} to handle it before continuing.`);
797797
if (this.#listInPageTools) {
798798
structuredContent.inPageTools = data.inPageTools ?? undefined;
799799
response.push('## In-page tools');
800-
if (!data.inPageTools) {
800+
if (!data.inPageTools || !data.inPageTools.tools) {
801801
response.push('No in-page tools available.');
802802
} else {
803803
const toolGroup = data.inPageTools;

‎src/bin/chrome-devtools-mcp-cli-options.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ export const cliOptions = {
218218
},
219219
categoryInPageTools: {
220220
type: 'boolean',
221-
default: false,
221+
hidden: true,
222222
describe:
223223
'Set to true to enable tools exposed by the inspected page itself',
224224
},

‎src/tools/inPage.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,18 @@ export interface ToolDefinition {
1313
name: string;
1414
description: string;
1515
inputSchema: JSONSchema7;
16-
execute: (input: Record<string, unknown>) => unknown;
1716
}
1817

19-
export interface ToolGroup {
18+
export interface ToolGroup<T extends ToolDefinition> {
2019
name: string;
2120
description: string;
22-
tools: ToolDefinition[];
21+
tools: T[];
2322
}
2423

2524
declare global {
2625
interface Window {
2726
__dtmcp?: {
28-
toolGroup?: ToolGroup;
27+
toolGroup?: ToolGroup<ToolDefinition & {execute: (args: Record<string, unknown>) => unknown}>;
2928
executeTool?: (
3029
toolName: string,
3130
args: Record<string, unknown>,
@@ -37,11 +36,12 @@ declare global {
3736
export const listInPageTools = definePageTool({
3837
name: 'list_in_page_tools',
3938
description: `Lists all in-page-tools the page exposes for providing runtime information.
40-
In-page-tools are exposed on the page via the 'window.__dtmcp.executeTool(toolName, params)'
41-
function where they can be called by 'evaluate_script'.`,
39+
To call 'list_in_page_tools', call 'evaluate_script' with
40+
'window.__dtmcp.executeTool("list_in_page_tools", {})'.`,
4241
annotations: {
4342
category: ToolCategory.IN_PAGE,
4443
readOnlyHint: true,
44+
conditions: ['inPageTools'],
4545
},
4646
schema: {},
4747
handler: async (_request, response, _context) => {

‎tests/McpResponse.test.js.snapshot‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,14 +1206,14 @@ exports[`extensions > lists extensions 2`] = `
12061206
}
12071207
`;
12081208

1209-
exports[`in-page tools > lists in-page tools 1`] = `
1209+
exports[`inPage tools > lists in-page tools 1`] = `
12101210
## In-page tools
12111211
My Tool Group: A group of tools
12121212
Available tools:
12131213
name="myTool", description="Does something", inputSchema={"type":"object","properties":{"foo":{"type":"string"}}}
12141214
`;
12151215

1216-
exports[`in-page tools > lists in-page tools 2`] = `
1216+
exports[`inPage tools > lists in-page tools 2`] = `
12171217
{
12181218
"inPageTools": {
12191219
"name": "My Tool Group",

‎tests/McpResponse.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ describe('lighthouse', () => {
10171017
});
10181018
});
10191019

1020-
describe('in-page tools', () => {
1020+
describe('inPage tools', () => {
10211021
function stubToolDiscovery(page: object) {
10221022
// @ts-expect-error Internal API
10231023
const client = page._client();

‎tests/cli.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ describe('cli args parsing', () => {
2323
performanceCrux: true,
2424
'usage-statistics': true,
2525
usageStatistics: true,
26-
'category-in-page-tools': false,
27-
categoryInPageTools: false,
2826
};
2927

3028
it('parses with default args', async () => {

‎tests/index.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ describe('e2e', () => {
116116
definedNames.sort();
117117
assert.deepStrictEqual(exposedNames, definedNames);
118118
},
119+
);
120+
});
121+
122+
it('has experimental in-Page tools', async () => {
123+
await withClient(
124+
async client => {
125+
const {tools} = await client.listTools();
126+
const listInPageTools = tools.find(t => t.name === 'list_in_page_tools');
127+
assert.ok(listInPageTools);
128+
},
119129
['--category-in-page-tools'],
120130
);
121131
});
@@ -124,8 +134,8 @@ describe('e2e', () => {
124134
await withClient(
125135
async client => {
126136
const {tools} = await client.listTools();
127-
const clickAt = tools.find(t => t.name === 'install_extension');
128-
assert.ok(clickAt);
137+
const installExtension = tools.find(t => t.name === 'install_extension');
138+
assert.ok(installExtension);
129139
},
130140
['--category-extensions'],
131141
);

‎tests/tools/inPage.test.ts‎

Lines changed: 77 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,7 @@
77
import assert from 'node:assert';
88
import {describe, it} from 'node:test';
99

10-
import sinon from 'sinon';
11-
1210
import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js';
13-
import type {ToolGroup} from '../../src/tools/inPage.js';
1411
import {listInPageTools} from '../../src/tools/inPage.js';
1512
import {withMcpContext} from '../utils.js';
1613

@@ -20,60 +17,111 @@ describe('inPage', () => {
2017
await withMcpContext(
2118
async (response, context) => {
2219
const page = await context.newPage();
23-
const toolGroup: ToolGroup = {
24-
name: 'test-group',
25-
description: 'test description',
26-
tools: [
27-
{
28-
name: 'test-tool',
29-
description: 'test tool description',
30-
inputSchema: {
31-
type: 'object',
32-
properties: {
33-
arg: {type: 'string'},
20+
21+
await page.pptrPage.evaluate(() => {
22+
window.__dtmcp = {
23+
toolGroup: {
24+
name: 'test-group',
25+
description: 'test description',
26+
tools: [
27+
{
28+
name: 'test-tool',
29+
description: 'test tool description',
30+
inputSchema: {
31+
type: 'object',
32+
properties: {
33+
arg: {type: 'string'},
34+
},
35+
},
36+
execute: () => 'result',
3437
},
35-
},
36-
execute: () => 'result',
38+
],
3739
},
38-
],
39-
};
40+
};
41+
window.addEventListener('devtoolstooldiscovery', (e: Event) => {
42+
// @ts-expect-error Event has `respondWith`
43+
e.respondWith(window.__dtmcp?.toolGroup);
44+
});
45+
});
4046

47+
await listInPageTools.handler({params: {}, page}, response, context);
48+
49+
const result = await response.handle('list_in_page_tools', context);
50+
// @ts-expect-error `structuredContent` has `inPageTools`
51+
const actualGroup = result.structuredContent.inPageTools;
52+
assert.strictEqual(actualGroup.name, 'test-group');
53+
assert.strictEqual(actualGroup.description, 'test description');
54+
assert.strictEqual(actualGroup.tools.length, 1);
55+
assert.strictEqual(actualGroup.tools[0].name, 'test-tool');
56+
assert.strictEqual(
57+
actualGroup.tools[0].description,
58+
'test tool description',
59+
);
60+
assert.deepEqual(actualGroup.tools[0].inputSchema, {
61+
type: 'object',
62+
properties: {
63+
arg: {type: 'string'},
64+
},
65+
});
66+
},
67+
undefined,
68+
{categoryInPageTools: true} as ParsedArguments,
69+
);
70+
});
71+
72+
it('handles empty response', async () => {
73+
await withMcpContext(
74+
async (response, context) => {
75+
const page = await context.newPage();
4176
await page.pptrPage.evaluate(() => {
42-
window.addEventListener('devtoolstooldiscovery', () => {
43-
// No-op
77+
window.addEventListener('devtoolstooldiscovery', (e: Event) => {
78+
// @ts-expect-error Event has `respondWith`
79+
e.respondWith({});
4480
});
4581
});
4682

47-
const evaluateStub = sinon.stub(page.pptrPage, 'evaluate');
48-
evaluateStub.resolves(toolGroup);
49-
5083
await listInPageTools.handler({params: {}, page}, response, context);
5184

5285
const result = await response.handle('list_in_page_tools', context);
5386
assert.ok('inPageTools' in result.structuredContent);
5487
assert.deepEqual(
55-
(result.structuredContent as {inPageTools: ToolGroup}).inPageTools,
56-
toolGroup,
88+
(result.structuredContent as {inPageTools: undefined}).inPageTools,
89+
{},
5790
);
5891
},
5992
undefined,
6093
{categoryInPageTools: true} as ParsedArguments,
6194
);
6295
});
6396

64-
it('handles no tools', async () => {
97+
it('handles no response', async () => {
6598
await withMcpContext(
6699
async (response, context) => {
67100
const page = await context.newPage();
68101
await page.pptrPage.evaluate(() => {
69102
window.addEventListener('devtoolstooldiscovery', () => {
70-
// No-op
103+
// do nothing
71104
});
72105
});
73106

74-
const evaluateStub = sinon.stub(page.pptrPage, 'evaluate');
75-
evaluateStub.resolves(undefined);
107+
await listInPageTools.handler({params: {}, page}, response, context);
76108

109+
const result = await response.handle('list_in_page_tools', context);
110+
assert.ok('inPageTools' in result.structuredContent);
111+
assert.strictEqual(
112+
(result.structuredContent as {inPageTools: undefined}).inPageTools,
113+
undefined,
114+
);
115+
},
116+
undefined,
117+
{categoryInPageTools: true} as ParsedArguments,
118+
);
119+
});
120+
121+
it('handles no eventListener', async () => {
122+
await withMcpContext(
123+
async (response, context) => {
124+
const page = await context.newPage();
77125
await listInPageTools.handler({params: {}, page}, response, context);
78126

79127
const result = await response.handle('list_in_page_tools', context);

0 commit comments

Comments
 (0)