Skip to content

Commit 0ca2434

Browse files
authored
fix: forbid closing the last page (#90)
McpContext currently assumes that there is a page. Eventually we might support closing the browser by closing the last page but right now we do not really have responses that indicate that the browser was closed. So this PR helps the LLM understand that it is okay to keep the last page running (and there are platform-specific differences as to what happens if the last page is closed). Ref #87
1 parent 2ee30bf commit 0ca2434

File tree

5 files changed

+29
-5
lines changed

5 files changed

+29
-5
lines changed

docs/tool-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@
116116

117117
### `close_page`
118118

119-
**Description:** Closes the page by its index.
119+
**Description:** Closes the page by its index. The last open page cannot be closed.
120120

121121
**Parameters:**
122122

src/McpContext.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,16 @@ export class McpContext implements Context {
131131
this.#consoleCollector.addPage(page);
132132
return page;
133133
}
134+
async closePage(pageIdx: number): Promise<void> {
135+
if (this.#pages.length === 1) {
136+
throw new Error(
137+
'Unable to close the last page in the browser. It is fine to keep the last page open.',
138+
);
139+
}
140+
const page = this.getPageByIdx(pageIdx);
141+
this.setSelectedPageIdx(0);
142+
await page.close({runBeforeUnload: false});
143+
}
134144

135145
getNetworkRequestByUrl(url: string): HTTPRequest {
136146
const requests = this.getNetworkRequests();

src/tools/ToolDefinition.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export type Context = Readonly<{
6262
clearDialog(): void;
6363
getPageByIdx(idx: number): Page;
6464
newPage(): Promise<Page>;
65+
closePage(pageIdx: number): Promise<void>;
6566
setSelectedPageIdx(idx: number): void;
6667
getElementByUid(uid: string): Promise<ElementHandle<Element>>;
6768
setNetworkConditions(conditions: string | null): void;

src/tools/pages.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export const selectPage = defineTool({
4545

4646
export const closePage = defineTool({
4747
name: 'close_page',
48-
description: `Closes the page by its index.`,
48+
description: `Closes the page by its index. The last open page cannot be closed.`,
4949
annotations: {
5050
category: ToolCategories.NAVIGATION_AUTOMATION,
5151
readOnlyHint: false,
@@ -58,9 +58,7 @@ export const closePage = defineTool({
5858
),
5959
},
6060
handler: async (request, response, context) => {
61-
const page = context.getPageByIdx(request.params.pageIdx);
62-
context.setSelectedPageIdx(0);
63-
await page.close({runBeforeUnload: false});
61+
await context.closePage(request.params.pageIdx);
6462
response.setIncludePages(true);
6563
},
6664
});

tests/tools/pages.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,21 @@ describe('pages', () => {
5353
assert.ok(response.includePages);
5454
});
5555
});
56+
it('cannot close the last page', async () => {
57+
await withBrowser(async (response, context) => {
58+
const page = context.getSelectedPage();
59+
try {
60+
await closePage.handler({params: {pageIdx: 0}}, response, context);
61+
assert.fail('not reached');
62+
} catch (err) {
63+
assert.strictEqual(
64+
err.message,
65+
'Unable to close the last page in the browser. It is fine to keep the last page open.',
66+
);
67+
}
68+
assert.ok(!page.isClosed());
69+
});
70+
});
5671
});
5772
describe('browser_select_page', () => {
5873
it('selects a page', async () => {

0 commit comments

Comments
 (0)