豆豆友情提示：这是一个非官方 GitHub 代理镜像，主要用于网络测试或访问加速。请勿在此进行登录、注册或处理任何敏感信息。进行这些操作请务必访问官方网站 github.com。 Raw 内容也通过此代理提供。
Skip to content

Commit a23c6ba

Browse files
OrKoNOrKoN
authored
refactor: change pageIdx to page ids (#741)
This allows for order-independent page IDs that are still easy to consume by LLMs (incremental integers).
1 parent 79ab800 commit a23c6ba

File tree

8 files changed

+48
-39
lines changed

8 files changed

+48
-39
lines changed

‎docs/tool-reference.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@
130130

131131
**Parameters:**
132132

133-
- **pageIdx** (number) **(required)**: The index of the page to close. Call [`list_pages`](#list_pages) to list pages.
133+
- **pageId** (number) **(required)**: The ID of the page to close. Call [`list_pages`](#list_pages) to list pages.
134134

135135
---
136136

@@ -172,7 +172,7 @@
172172

173173
**Parameters:**
174174

175-
- **pageIdx** (number) **(required)**: The index of the page to select. Call [`list_pages`](#list_pages) to get available pages.
175+
- **pageId** (number) **(required)**: The ID of the page to select. Call [`list_pages`](#list_pages) to get available pages.
176176
- **bringToFront** (boolean) _(optional)_: Whether to focus the page and bring it to the top.
177177

178178
---

‎src/McpContext.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ export class McpContext implements Context {
111111
#geolocationMap = new WeakMap<Page, GeolocationOptions>();
112112
#dialog?: Dialog;
113113

114+
#pageIdMap = new WeakMap<Page, number>();
115+
#nextPageId = 1;
116+
114117
#nextSnapshotId = 1;
115118
#traceResults: TraceResult[] = [];
116119

@@ -246,11 +249,11 @@ export class McpContext implements Context {
246249
this.#consoleCollector.addPage(page);
247250
return page;
248251
}
249-
async closePage(pageIdx: number): Promise<void> {
252+
async closePage(pageId: number): Promise<void> {
250253
if (this.#pages.length === 1) {
251254
throw new Error(CLOSE_PAGE_ERROR);
252255
}
253-
const page = this.getPageByIdx(pageIdx);
256+
const page = this.getPageById(pageId);
254257
await page.close({runBeforeUnload: false});
255258
}
256259

@@ -327,15 +330,18 @@ export class McpContext implements Context {
327330
return page;
328331
}
329332

330-
getPageByIdx(idx: number): Page {
331-
const pages = this.#pages;
332-
const page = pages[idx];
333+
getPageById(pageId: number): Page {
334+
const page = this.#pages.find(p => this.#pageIdMap.get(p) === pageId);
333335
if (!page) {
334336
throw new Error('No page found');
335337
}
336338
return page;
337339
}
338340

341+
getPageId(page: Page): number | undefined {
342+
return this.#pageIdMap.get(page);
343+
}
344+
339345
#dialogHandler = (dialog: Dialog): void => {
340346
this.#dialog = dialog;
341347
};
@@ -417,6 +423,12 @@ export class McpContext implements Context {
417423
this.#options.experimentalIncludeAllPages,
418424
);
419425

426+
for (const page of allPages) {
427+
if (!this.#pageIdMap.has(page)) {
428+
this.#pageIdMap.set(page, this.#nextPageId++);
429+
}
430+
}
431+
420432
this.#pages = allPages.filter(page => {
421433
// If we allow debugging DevTools windows, return all pages.
422434
// If we are in regular mode, the user should only see non-DevTools page.

‎src/McpResponse.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,12 +385,10 @@ Call ${handleDialog.name} to handle it before continuing.`);
385385

386386
if (this.#includePages) {
387387
const parts = [`## Pages`];
388-
let idx = 0;
389388
for (const page of context.getPages()) {
390389
parts.push(
391-
`${idx}: ${page.url()}${context.isPageSelected(page) ? ' [selected]' : ''}`,
390+
`${context.getPageId(page)}: ${page.url()}${context.isPageSelected(page) ? ' [selected]' : ''}`,
392391
);
393-
idx++;
394392
}
395393
response.push(...parts);
396394
}

‎src/tools/ToolDefinition.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,11 @@ export type Context = Readonly<{
8989
getSelectedPage(): Page;
9090
getDialog(): Dialog | undefined;
9191
clearDialog(): void;
92-
getPageByIdx(idx: number): Page;
92+
getPageById(pageId: number): Page;
93+
getPageId(page: Page): number | undefined;
9394
isPageSelected(page: Page): boolean;
9495
newPage(): Promise<Page>;
95-
closePage(pageIdx: number): Promise<void>;
96+
closePage(pageId: number): Promise<void>;
9697
selectPage(page: Page): void;
9798
getElementByUid(uid: string): Promise<ElementHandle<Element>>;
9899
getAXNodeByUid(uid: string): TextSnapshotNode | undefined;

‎src/tools/pages.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,18 @@ export const selectPage = defineTool({
3131
readOnlyHint: true,
3232
},
3333
schema: {
34-
pageIdx: zod
34+
pageId: zod
3535
.number()
3636
.describe(
37-
`The index of the page to select. Call ${listPages.name} to get available pages.`,
37+
`The ID of the page to select. Call ${listPages.name} to get available pages.`,
3838
),
3939
bringToFront: zod
4040
.boolean()
4141
.optional()
4242
.describe('Whether to focus the page and bring it to the top.'),
4343
},
4444
handler: async (request, response, context) => {
45-
const page = context.getPageByIdx(request.params.pageIdx);
45+
const page = context.getPageById(request.params.pageId);
4646
context.selectPage(page);
4747
response.setIncludePages(true);
4848
if (request.params.bringToFront) {
@@ -59,15 +59,13 @@ export const closePage = defineTool({
5959
readOnlyHint: false,
6060
},
6161
schema: {
62-
pageIdx: zod
62+
pageId: zod
6363
.number()
64-
.describe(
65-
'The index of the page to close. Call list_pages to list pages.',
66-
),
64+
.describe('The ID of the page to close. Call list_pages to list pages.'),
6765
},
6866
handler: async (request, response, context) => {
6967
try {
70-
await context.closePage(request.params.pageIdx);
68+
await context.closePage(request.params.pageId);
7169
} catch (err) {
7270
if (err.message === CLOSE_PAGE_ERROR) {
7371
response.appendResponseLine(err.message);

‎tests/McpResponse.test.js.snapshot‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ Testing 2
8383
exports[`McpResponse > list pages 1`] = `
8484
# test response
8585
## Pages
86-
0: about:blank [selected]
86+
1: about:blank [selected]
8787
`;
8888

8989
exports[`McpResponse > returns correctly formatted snapshot for a simple tree 1`] = `

‎tests/index.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('e2e', () => {
5151
content: [
5252
{
5353
type: 'text',
54-
text: '# list_pages response\n## Pages\n0: about:blank [selected]',
54+
text: '# list_pages response\n## Pages\n1: about:blank [selected]',
5555
},
5656
],
5757
});
@@ -72,7 +72,7 @@ describe('e2e', () => {
7272
content: [
7373
{
7474
type: 'text',
75-
text: '# list_pages response\n## Pages\n0: about:blank [selected]',
75+
text: '# list_pages response\n## Pages\n1: about:blank [selected]',
7676
},
7777
],
7878
});

‎tests/tools/pages.test.ts‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ describe('pages', () => {
3232
describe('new_page', () => {
3333
it('create a page', async () => {
3434
await withMcpContext(async (response, context) => {
35-
assert.strictEqual(context.getPageByIdx(0), context.getSelectedPage());
35+
assert.strictEqual(context.getPageById(1), context.getSelectedPage());
3636
await newPage.handler(
3737
{params: {url: 'about:blank'}},
3838
response,
3939
context,
4040
);
41-
assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
41+
assert.strictEqual(context.getPageById(2), context.getSelectedPage());
4242
assert.ok(response.includePages);
4343
});
4444
});
@@ -47,17 +47,17 @@ describe('pages', () => {
4747
it('closes a page', async () => {
4848
await withMcpContext(async (response, context) => {
4949
const page = await context.newPage();
50-
assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
51-
assert.strictEqual(context.getPageByIdx(1), page);
52-
await closePage.handler({params: {pageIdx: 1}}, response, context);
50+
assert.strictEqual(context.getPageById(2), context.getSelectedPage());
51+
assert.strictEqual(context.getPageById(2), page);
52+
await closePage.handler({params: {pageId: 2}}, response, context);
5353
assert.ok(page.isClosed());
5454
assert.ok(response.includePages);
5555
});
5656
});
5757
it('cannot close the last page', async () => {
5858
await withMcpContext(async (response, context) => {
5959
const page = context.getSelectedPage();
60-
await closePage.handler({params: {pageIdx: 0}}, response, context);
60+
await closePage.handler({params: {pageId: 1}}, response, context);
6161
assert.deepStrictEqual(
6262
response.responseLines[0],
6363
`The last open page cannot be closed. It is fine to keep it open.`,
@@ -71,24 +71,24 @@ describe('pages', () => {
7171
it('selects a page', async () => {
7272
await withMcpContext(async (response, context) => {
7373
await context.newPage();
74-
assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
75-
await selectPage.handler({params: {pageIdx: 0}}, response, context);
76-
assert.strictEqual(context.getPageByIdx(0), context.getSelectedPage());
74+
assert.strictEqual(context.getPageById(2), context.getSelectedPage());
75+
await selectPage.handler({params: {pageId: 1}}, response, context);
76+
assert.strictEqual(context.getPageById(1), context.getSelectedPage());
7777
assert.ok(response.includePages);
7878
});
7979
});
8080
it('selects a page and keeps it focused in the background', async () => {
8181
await withMcpContext(async (response, context) => {
8282
await context.newPage();
83-
assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
83+
assert.strictEqual(context.getPageById(2), context.getSelectedPage());
8484
assert.strictEqual(
85-
await context.getPageByIdx(0).evaluate(() => document.hasFocus()),
85+
await context.getPageById(1).evaluate(() => document.hasFocus()),
8686
false,
8787
);
88-
await selectPage.handler({params: {pageIdx: 0}}, response, context);
89-
assert.strictEqual(context.getPageByIdx(0), context.getSelectedPage());
88+
await selectPage.handler({params: {pageId: 1}}, response, context);
89+
assert.strictEqual(context.getPageById(1), context.getSelectedPage());
9090
assert.strictEqual(
91-
await context.getPageByIdx(0).evaluate(() => document.hasFocus()),
91+
await context.getPageById(1).evaluate(() => document.hasFocus()),
9292
true,
9393
);
9494
assert.ok(response.includePages);
@@ -115,8 +115,8 @@ describe('pages', () => {
115115
it('throws an error if the page was closed not by the MCP server', async () => {
116116
await withMcpContext(async (response, context) => {
117117
const page = await context.newPage();
118-
assert.strictEqual(context.getPageByIdx(1), context.getSelectedPage());
119-
assert.strictEqual(context.getPageByIdx(1), page);
118+
assert.strictEqual(context.getPageById(2), context.getSelectedPage());
119+
assert.strictEqual(context.getPageById(2), page);
120120

121121
await page.close();
122122

0 commit comments

Comments
 (0)