fix: keep a still-open selected page instead of falling back to the first page (#2328)

Follow-up to #2304.

`createPagesSnapshot()` reselects `#pages[0]` whenever the selected page
is missing from `#pages`, even when that page is still open. So a live
selection can be silently swapped, not only a closed one.

The case that hit us is a `devtools://` page:

- the server connects with `handleDevToolsAsPage: true`
(`src/browser.ts`), so DevTools frontends are pages and land in
`#mcpPages`
- `#pages` filters `devtools://` out (`src/McpContext.ts`, unless
`experimentalDevToolsDebugging` is set)
- `getPageById()` (used by `select_page`) reads `#mcpPages`, not
`#pages`, so a devtools page can be selected even though `list_pages`
never lists it
- the next snapshot finds it missing from `#pages` and, because the
fallback keys on `#pages` membership rather than `isClosed()`, reselects
`#pages[0]` with the page still open (`isClosed() === false`)

This reproduces on a normal Chrome (details and a repro in #2304).

This change gates the fallback on `isClosed()`, as proposed in #2304. A
still-open selection is kept; a genuinely closed page still
auto-advances, so the browser-like behavior for closed tabs is
unchanged.

One consequence: the "is no longer listed" wording added in #2308
becomes unreachable, since a live page missing from the list no longer
triggers a fallback. I left it in place to keep this diff focused, but
I'm happy to simplify it here or in a follow-up.

The unit test that covered the missing-but-open case now asserts the
selection is retained.

Refs: #2304
This commit is contained in:
Thomas Bachem
2026-07-09 08:09:11 +01:00
committed by GitHub
parent ed7e95d3e0
commit c645eee875
2 changed files with 13 additions and 7 deletions
+5 -2
View File
@@ -695,10 +695,13 @@ export class McpContext implements Context {
);
});
// Only fall back when the selected page is actually gone. Gating on
// `isClosed()` instead of `#pages` membership avoids silently swapping a
// live page that is momentarily missing from the snapshot, e.g., a
// `devtools://` page, which is selectable but filtered out of `#pages` above.
this.#selectedPageFallback = undefined;
if (
(!this.#selectedPage ||
this.#pages.indexOf(this.#selectedPage.pptrPage) === -1) &&
(!this.#selectedPage || this.#selectedPage.pptrPage.isClosed()) &&
this.#pages[0]
) {
// Record the automatic change so the response can surface it. Skipped on
+8 -5
View File
@@ -175,12 +175,13 @@ describe('McpContext', () => {
});
});
it('reports the fallback when the selected page is missing from the list', async () => {
it('keeps a still-open selected page that is missing from the list', async () => {
await withMcpContext(async (_response, context) => {
const page = await context.newPage();
assert.ok(context.isPageSelected(page.pptrPage));
// A live page that is temporarily missing from the pages list.
// A live page that is temporarily missing from the pages list must keep
// its selection — only a genuinely closed page is replaced.
const pages = await context.browser.pages();
const stub = sinon
.stub(context.browser, 'pages')
@@ -191,9 +192,11 @@ describe('McpContext', () => {
stub.restore();
}
const fallback = context.getSelectedPageFallback();
assert.ok(fallback, 'fallback should be reported');
assert.strictEqual(fallback.wasClosed, false);
assert.ok(
context.isPageSelected(page.pptrPage),
'a still-open page should keep its selection',
);
assert.strictEqual(context.getSelectedPageFallback(), undefined);
});
});