From a0af4bf3ae711b062fbc31d1655f76af870817c1 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 14 Aug 2026 11:28:15 -0700 Subject: [PATCH 1/3] fix(net): pass a generous autoSelectFamilyAttemptTimeout (#42255) --- packages/playwright-core/src/server/fetch.ts | 7 ++++--- .../playwright-core/src/server/transport.ts | 4 ++-- .../src/server/webkit/webview/wvBrowser.ts | 4 ++-- packages/utils/network.ts | 17 ++++++++++++++--- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 1a9718ef3867c..3a393b610e8f2 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -25,7 +25,7 @@ import { assert } from '@isomorphic/assert'; import { constructURLBasedOnBaseURL } from '@isomorphic/urlMatch'; import { eventsHelper } from '@utils/eventsHelper'; import { monotonicTime } from '@isomorphic/time'; -import { createProxyAgent, dualStackLookup, flattenAggregateError } from '@utils/network'; +import { createProxyAgent, flattenAggregateError, happyEyeballsOptions } from '@utils/network'; import { getUserAgent } from './userAgent'; import { BrowserContext, findMatchingHttpCredentials, verifyClientCertificates } from './browserContext'; import { Cookie, CookieStore, domainMatches, parseRawCookie } from './cookieStore'; @@ -343,8 +343,9 @@ export abstract class APIRequestContext extends SdkObject { = (url.protocol === 'https:' ? https : http).request; // Without an explicit proxy agent, the default global agent is used, which // has keep-alive enabled and connects with Happy Eyeballs (autoSelectFamily). - const requestOptions = { ...options }; - requestOptions.lookup = options.__testHookLookup ? lookupWithTestHook(options.__testHookLookup) : dualStackLookup; + const requestOptions = { ...options, ...happyEyeballsOptions }; + if (options.__testHookLookup) + requestOptions.lookup = lookupWithTestHook(options.__testHookLookup); const startAt = monotonicTime(); const startAtWallTime = Date.now(); diff --git a/packages/playwright-core/src/server/transport.ts b/packages/playwright-core/src/server/transport.ts index d333834efc86a..028da8bd59254 100644 --- a/packages/playwright-core/src/server/transport.ts +++ b/packages/playwright-core/src/server/transport.ts @@ -16,7 +16,7 @@ */ import ws from 'ws'; -import { dualStackLookup, flattenAggregateError } from '@utils/network'; +import { flattenAggregateError, happyEyeballsOptions } from '@utils/network'; import { makeWaitForNextTask } from '@utils/task'; import type { WebSocket } from 'ws'; import type { Progress } from './progress'; @@ -138,7 +138,7 @@ export class WebSocketTransport implements ConnectionTransport { maxPayload: 256 * 1024 * 1024, // 256Mb, headers: options.headers, followRedirects: options.followRedirects, - lookup: dualStackLookup, + ...happyEyeballsOptions, perMessageDeflate, }); this._ws.on('upgrade', response => { diff --git a/packages/playwright-core/src/server/webkit/webview/wvBrowser.ts b/packages/playwright-core/src/server/webkit/webview/wvBrowser.ts index b2664ad68917a..1ff676dd1a627 100644 --- a/packages/playwright-core/src/server/webkit/webview/wvBrowser.ts +++ b/packages/playwright-core/src/server/webkit/webview/wvBrowser.ts @@ -21,7 +21,7 @@ import path from 'path'; import ws from 'ws'; import { debugLogger, RecentLogsCollector } from '@utils/debugLogger'; import { removeFolders } from '@utils/fileUtils'; -import { dualStackLookup } from '@utils/network'; +import { happyEyeballsOptions } from '@utils/network'; import { headersArrayToObject } from '@isomorphic/headers'; import { Browser } from '../../browser'; import { helper } from '../../helper'; @@ -96,7 +96,7 @@ class DeferredWebSocketTransport implements ConnectOverCDPTransport { maxPayload: 256 * 1024 * 1024, headers: this._headers, followRedirects: true, - lookup: dualStackLookup, + ...happyEyeballsOptions, perMessageDeflate, allowSynchronousEvents: false, }); diff --git a/packages/utils/network.ts b/packages/utils/network.ts index 9205f225fe806..383edc0f25d33 100644 --- a/packages/utils/network.ts +++ b/packages/utils/network.ts @@ -50,7 +50,7 @@ export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.Inco const options: https.RequestOptions = { method: params.method || 'GET', headers: params.headers, - lookup: dualStackLookup, + ...happyEyeballsOptions, }; if (params.rejectUnauthorized !== undefined) options.rejectUnauthorized = params.rejectUnauthorized; @@ -160,7 +160,7 @@ export function createProxyAgent(proxy?: ProxySettings, forUrl?: URL) { // of a family that is not listed in /etc/hosts — e.g. resolving to only 127.0.0.1 even though // ::1 is served. Separate family: 4 and family: 6 lookups do not have these problems. Native // Happy Eyeballs (autoSelectFamily) then races connection attempts across the families. -export const dualStackLookup: net.LookupFunction = (hostname, options, callback) => { +const dualStackLookup: net.LookupFunction = (hostname, options, callback) => { const families = options.family === 4 || options.family === 6 ? [options.family] : [6, 4]; void Promise.allSettled(families.map(family => dns.promises.lookup(hostname, { all: true, family }))).then(results => { const perFamily = results.map(result => result.status === 'fulfilled' ? result.value : []); @@ -184,6 +184,17 @@ export const dualStackLookup: net.LookupFunction = (hostname, options, callback) }); }; +// Node.js aborts every connection attempt but the last one after autoSelectFamilyAttemptTimeout, +// and its 250ms default is too short for a TCP handshake over a slow network, failing connections +// that would have succeeded: https://github.com/nodejs/node/issues/54359. Floor it at 5s to +// survive two SYN retransmits, honoring a higher process-wide default when the user set one. +// Revisit when attempts run in parallel: https://github.com/nodejs/node/issues/48145. +export const happyEyeballsOptions = { + lookup: dualStackLookup, + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: Math.max(5000, net.getDefaultAutoSelectFamilyAttemptTimeout()), +}; + // When every raced connection attempt fails, Node.js reports an AggregateError with an // empty message and the individual failures in the `errors` property. Surface those instead. export function flattenAggregateError(error: Error): Error { @@ -195,7 +206,7 @@ export function flattenAggregateError(error: Error): Error { export async function createSocket(host: string, port: number): Promise { return new Promise((resolve, reject) => { - const socket = net.createConnection({ host, port, lookup: dualStackLookup }); + const socket = net.createConnection({ host, port, ...happyEyeballsOptions }); socket.on('connect', () => resolve(socket)); socket.on('error', error => reject(flattenAggregateError(error))); }); From 8dfd42cdf1ab8e48a9791b52eba2bdf2c54f12bf Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 14 Aug 2026 17:03:43 -0700 Subject: [PATCH 2/3] fix(storageState): close IndexedDB connections opened by collect and restore (#42260) --- packages/injected/src/storageScript.ts | 129 +++++++++--------- .../browsercontext-storage-state.spec.ts | 35 +++++ 2 files changed, 103 insertions(+), 61 deletions(-) diff --git a/packages/injected/src/storageScript.ts b/packages/injected/src/storageScript.ts index 2583b2c3c3e63..e373cd8a00f84 100644 --- a/packages/injected/src/storageScript.ts +++ b/packages/injected/src/storageScript.ts @@ -76,61 +76,65 @@ export class StorageScript { throw new Error('Database version is unset'); const db = await this._idbRequestToPromise(indexedDB.open(dbInfo.name)); - if (db.objectStoreNames.length === 0) - return { name: dbInfo.name, version: dbInfo.version, stores: [] }; - - const transaction = db.transaction(db.objectStoreNames, 'readonly'); - const stores = await Promise.all([...db.objectStoreNames].map(async storeName => { - const objectStore = transaction.objectStore(storeName); - - const keys = await this._idbRequestToPromise(objectStore.getAllKeys()); - const records = await Promise.all(keys.map(async key => { - const record: IndexedDBDatabase['stores'][0]['records'][0] = {}; - - if (objectStore.keyPath === null) { - const { encoded, trivial } = this._trySerialize(key); + try { + if (db.objectStoreNames.length === 0) + return { name: dbInfo.name, version: dbInfo.version, stores: [] }; + + const transaction = db.transaction(db.objectStoreNames, 'readonly'); + const stores = await Promise.all([...db.objectStoreNames].map(async storeName => { + const objectStore = transaction.objectStore(storeName); + + const keys = await this._idbRequestToPromise(objectStore.getAllKeys()); + const records = await Promise.all(keys.map(async key => { + const record: IndexedDBDatabase['stores'][0]['records'][0] = {}; + + if (objectStore.keyPath === null) { + const { encoded, trivial } = this._trySerialize(key); + if (trivial) + record.key = trivial; + else + record.keyEncoded = encoded; + } + + const value = await this._idbRequestToPromise(objectStore.get(key)); + const { encoded, trivial } = this._trySerialize(value); if (trivial) - record.key = trivial; + record.value = trivial; else - record.keyEncoded = encoded; - } + record.valueEncoded = encoded; + + return record; + })); + + const indexes = [...objectStore.indexNames].map(indexName => { + const index = objectStore.index(indexName); + return { + name: index.name, + keyPath: typeof index.keyPath === 'string' ? index.keyPath : undefined, + keyPathArray: Array.isArray(index.keyPath) ? index.keyPath : undefined, + multiEntry: index.multiEntry, + unique: index.unique, + }; + }); - const value = await this._idbRequestToPromise(objectStore.get(key)); - const { encoded, trivial } = this._trySerialize(value); - if (trivial) - record.value = trivial; - else - record.valueEncoded = encoded; - - return record; - })); - - const indexes = [...objectStore.indexNames].map(indexName => { - const index = objectStore.index(indexName); return { - name: index.name, - keyPath: typeof index.keyPath === 'string' ? index.keyPath : undefined, - keyPathArray: Array.isArray(index.keyPath) ? index.keyPath : undefined, - multiEntry: index.multiEntry, - unique: index.unique, + name: storeName, + records: records, + indexes, + autoIncrement: objectStore.autoIncrement, + keyPath: typeof objectStore.keyPath === 'string' ? objectStore.keyPath : undefined, + keyPathArray: Array.isArray(objectStore.keyPath) ? objectStore.keyPath : undefined, }; - }); + })); return { - name: storeName, - records: records, - indexes, - autoIncrement: objectStore.autoIncrement, - keyPath: typeof objectStore.keyPath === 'string' ? objectStore.keyPath : undefined, - keyPathArray: Array.isArray(objectStore.keyPath) ? objectStore.keyPath : undefined, + name: dbInfo.name, + version: dbInfo.version, + stores, }; - })); - - return { - name: dbInfo.name, - version: dbInfo.version, - stores, - }; + } finally { + db.close(); + } } async collect(recordIndexedDB: boolean): Promise { @@ -159,21 +163,24 @@ export class StorageScript { // after `upgradeneeded` finishes, `success` event is fired. const db = await this._idbRequestToPromise(openRequest); - - if (db.objectStoreNames.length === 0) - return; - const transaction = db.transaction(db.objectStoreNames, 'readwrite'); - await Promise.all(dbInfo.stores.map(async store => { - const objectStore = transaction.objectStore(store.name); - await Promise.all(store.records.map(async record => { - await this._idbRequestToPromise( - objectStore.add( - record.value ?? parseEvaluationResultValue(record.valueEncoded), - record.key ?? parseEvaluationResultValue(record.keyEncoded), - ) - ); + try { + if (db.objectStoreNames.length === 0) + return; + const transaction = db.transaction(db.objectStoreNames, 'readwrite'); + await Promise.all(dbInfo.stores.map(async store => { + const objectStore = transaction.objectStore(store.name); + await Promise.all(store.records.map(async record => { + await this._idbRequestToPromise( + objectStore.add( + record.value ?? parseEvaluationResultValue(record.valueEncoded), + record.key ?? parseEvaluationResultValue(record.keyEncoded), + ) + ); + })); })); - })); + } finally { + db.close(); + } } async restore(originState: SetOriginStorage | undefined) { diff --git a/tests/library/browsercontext-storage-state.spec.ts b/tests/library/browsercontext-storage-state.spec.ts index 488e9b959a5d9..b25995217f537 100644 --- a/tests/library/browsercontext-storage-state.spec.ts +++ b/tests/library/browsercontext-storage-state.spec.ts @@ -540,6 +540,41 @@ it('should support empty indexedDB', { annotation: { type: 'issue', description: expect(await context.storageState({ indexedDB: true })).toEqual(storageState); }); +it('should not leave IndexedDB connections open', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42258' } }, async ({ contextFactory, server }) => { + const context = await contextFactory(); + const page = await context.newPage(); + await page.goto(server.EMPTY_PAGE); + await page.evaluate(async () => { + const openRequest = indexedDB.open('db', 1); + openRequest.onupgradeneeded = () => openRequest.result.createObjectStore('store'); + await new Promise((resolve, reject) => { + openRequest.onsuccess = () => { + const db = openRequest.result; + const transaction = db.transaction('store', 'readwrite'); + transaction.objectStore('store').put('value', 'key'); + transaction.oncomplete = () => { + db.close(); + resolve(); + }; + transaction.onerror = () => reject(transaction.error); + }; + openRequest.onerror = () => reject(openRequest.error); + }); + }); + + const state = await context.storageState({ indexedDB: true }); + + await page.evaluate(() => new Promise((resolve, reject) => { + const request = indexedDB.deleteDatabase('db'); + request.onsuccess = () => resolve(); + request.onerror = () => reject(request.error); + request.onblocked = () => reject(new Error('deleteDatabase was blocked')); + })); + + await context.setStorageState(state); + expect(await context.storageState({ indexedDB: true })).toEqual(state); +}); + it('should round-trip WebAuthn credentials with storageState', async ({ contextFactory, server }) => { const context = await contextFactory(); const credential = await context.credentials.create(server.HOSTNAME); From d5a185a894ab3ab17ff77a44e116a1339c6bdaed Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Fri, 14 Aug 2026 17:19:06 -0700 Subject: [PATCH 3/3] feat(extension): support multiple simultaneous client connections (#42259) --- packages/extension/README.md | 4 + packages/extension/src/background.ts | 84 ++++---- packages/extension/src/connectedTabGroup.ts | 106 +++++++--- packages/extension/src/pendingConnection.ts | 5 + packages/extension/src/ui/connect.css | 7 + packages/extension/src/ui/status.tsx | 82 ++++---- tests/extension/multi-connection.spec.ts | 206 ++++++++++++++++++++ tests/extension/tab-grouping.spec.ts | 4 +- 8 files changed, 397 insertions(+), 101 deletions(-) create mode 100644 tests/extension/multi-connection.spec.ts diff --git a/packages/extension/README.md b/packages/extension/README.md index 6713f28660cba..521dd1be5d514 100644 --- a/packages/extension/README.md +++ b/packages/extension/README.md @@ -38,6 +38,10 @@ Configure Playwright MCP server to connect to the browser using the extension by When the LLM interacts with the browser for the first time, it will load a page where you can select which browser tab the LLM will connect to. This allows you to control which specific page the AI assistant will interact with during the session. +### Multiple Clients + +Several clients can be connected at the same time. Each one gets its own tab group, named after the client and colored apart from the other groups, and only sees the tabs in that group — a tab can belong to a single client at a time. Drag tabs in and out of a group to change what a client can reach, and use the extension's status page to see every connection and disconnect them individually. + ### Bypassing the Connection Approval Dialog By default, you'll need to approve each connection when the MCP server tries to connect to your browser. To bypass this approval dialog and allow automatic connections, you can use an authentication token. diff --git a/packages/extension/src/background.ts b/packages/extension/src/background.ts index da94041c7213c..801f69084c5a4 100644 --- a/packages/extension/src/background.ts +++ b/packages/extension/src/background.ts @@ -16,7 +16,7 @@ import { debugLog } from './relayConnection'; import { PendingConnections } from './pendingConnection'; -import { ConnectedTabGroup, cleanupStalePlaywrightGroups, isNonDebuggableUrl } from './connectedTabGroup'; +import { ConnectedTabGroup, cleanupStalePlaywrightGroups, isNonDebuggableUrl, ungroupTabs, uniqueGroupStyle } from './connectedTabGroup'; type PageMessage = { type: 'connectionRequested'; @@ -33,13 +33,14 @@ type PageMessage = { type: 'getConnectionStatus'; } | { type: 'disconnect'; + connectionId: number; } | { type: 'keepalive'; }; class PlaywrightExtension { - private _activeGroup: ConnectedTabGroup | undefined; - private _activeClientName: string | undefined; + private _connections = new Map(); + private _lastConnectionId = 0; private _pendingConnections = new PendingConnections(); // Service worker restarts lose all connection state, so any existing // Playwright groups are stale. Connections wait on this before reconciling. @@ -54,12 +55,16 @@ class PlaywrightExtension { // Promise-based message handling is not supported in Chrome: https://issues.chromium.org/issues/40753031 private _onMessage(message: PageMessage, sender: chrome.runtime.MessageSender, sendResponse: (response: any) => void) { switch (message.type) { - case 'connectionRequested': - this._pendingConnections.create(sender.tab!.id!, message.mcpRelayUrl); - sendResponse({ success: true }); - return false; + case 'connectionRequested': { + const selectorTabId = sender.tab!.id!; + this._releaseConnectPage(selectorTabId).then(() => { + this._pendingConnections.create(selectorTabId, message.mcpRelayUrl); + sendResponse({ success: true }); + }); + return true; + } case 'getTabs': - this._getTabs().then( + this._getTabs(sender.tab?.id).then( tabs => sendResponse({ success: true, tabs, currentTabId: sender.tab?.id }), (error: any) => sendResponse({ success: false, error: error.message })); return true; @@ -76,18 +81,17 @@ class PlaywrightExtension { } case 'getConnectionStatus': sendResponse({ - connectedTabIds: this._activeGroup?.connectedTabIds() ?? [], - clientName: this._activeClientName, + connections: [...this._connections].map(([id, group]) => ({ + id, + clientName: group.clientName, + connectedTabIds: group.connectedTabIds(), + })), }); return false; case 'disconnect': - try { - this._disconnect('User disconnected'); - sendResponse({ success: true }); - } catch (error: any) { - sendResponse({ success: false, error: error.message }); - } - return true; + this._connections.get(message.connectionId)?.close('User disconnected'); + sendResponse({ success: true }); + return false; case 'keepalive': // Connect page pings us every ~20s so receiving this message resets // the MV3 service worker idle timer and keeps the relay WebSocket alive. @@ -98,21 +102,19 @@ class PlaywrightExtension { private async _connectTab(selectorTabId: number, tab: chrome.tabs.Tab & { id: number }, clientName: string | undefined): Promise { try { await this._cleanupPromise; - this._disconnect('Another connection is requested'); + this._releaseTab(selectorTabId); + if (tab.id !== selectorTabId && this._connectedTabIds().has(tab.id)) + throw new Error('This tab is already connected to another client'); const connection = await this._pendingConnections.take(selectorTabId); if (!connection) throw new Error('Pending client connection closed'); - const group = new ConnectedTabGroup(connection, tab); - group.onclose = () => { - if (this._activeGroup === group) { - this._activeGroup = undefined; - this._activeClientName = undefined; - } - }; - this._activeGroup = group; - this._activeClientName = clientName; + const id = ++this._lastConnectionId; + const taken = [...this._connections.values()].map(group => group.groupStyle); + const group = new ConnectedTabGroup(connection, tab, clientName, uniqueGroupStyle(clientName, taken), tabId => this._pendingConnections.has(tabId)); + group.onclose = () => this._connections.delete(id); + this._connections.set(id, group); await Promise.all([ chrome.tabs.update(tab.id, { active: true }), @@ -127,9 +129,25 @@ class PlaywrightExtension { } } - private async _getTabs(): Promise { + // Chrome may create the connect page inside the active client's group. + private async _releaseConnectPage(tabId: number): Promise { + this._releaseTab(tabId); + await ungroupTabs([tabId]); + } + + private _releaseTab(tabId: number): void { + for (const group of this._connections.values()) + group.releaseTab(tabId); + } + + private async _getTabs(selectorTabId: number | undefined): Promise { const tabs = await chrome.tabs.query({}); - return tabs.filter(tab => !isNonDebuggableUrl(tab.url)); + const connectedTabIds = this._connectedTabIds(); + return tabs.filter(tab => !isNonDebuggableUrl(tab.url) && (tab.id === selectorTabId || !connectedTabIds.has(tab.id!))); + } + + private _connectedTabIds(): Set { + return new Set([...this._connections.values()].flatMap(group => group.connectedTabIds())); } private async _onActionClicked(): Promise { @@ -138,14 +156,6 @@ class PlaywrightExtension { active: true }); } - - // Closes the active group's connection if any. ConnectedTabGroup's onclose - // handles state cleanup (connectedTabIds, badges, reconcile). - private _disconnect(reason: string) { - this._activeGroup?.close(reason); - this._activeGroup = undefined; - this._activeClientName = undefined; - } } new PlaywrightExtension(); diff --git a/packages/extension/src/connectedTabGroup.ts b/packages/extension/src/connectedTabGroup.ts index dd08c72668f57..023e084e33cb2 100644 --- a/packages/extension/src/connectedTabGroup.ts +++ b/packages/extension/src/connectedTabGroup.ts @@ -17,7 +17,9 @@ import { RelayConnection, debugLog } from './relayConnection'; const PLAYWRIGHT_GROUP_TITLE = 'Playwright'; -const PLAYWRIGHT_GROUP_COLOR = 'green'; +const PLAYWRIGHT_GROUP_TITLE_PREFIX = `${PLAYWRIGHT_GROUP_TITLE} · `; +// Green first, so a lone connection keeps the familiar look. +const PLAYWRIGHT_GROUP_COLORS: GroupColor[] = ['green', 'blue', 'purple', 'orange', 'pink', 'cyan', 'yellow', 'red']; const NON_DEBUGGABLE_SCHEMES = ['chrome:', 'edge:', 'devtools:']; const CONNECTED_BADGE = { text: '✓', color: '#4CAF50', title: 'Connected to Playwright client' }; @@ -25,14 +27,35 @@ export function isNonDebuggableUrl(url: string | undefined): boolean { return !!url && NON_DEBUGGABLE_SCHEMES.some(s => url.startsWith(s)); } +type GroupColor = `${chrome.tabGroups.Color}`; + +export type GroupStyle = { + title: string; + color: GroupColor; +}; + +export function uniqueGroupStyle(clientName: string | undefined, taken: readonly GroupStyle[]): GroupStyle { + const titles = new Set(taken.map(style => style.title)); + const base = PLAYWRIGHT_GROUP_TITLE_PREFIX + (clientName || 'unknown'); + let title = base; + for (let i = 2; titles.has(title); i++) + title = `${base} (${i})`; + + const colors = new Set(taken.map(style => style.color)); + const color = PLAYWRIGHT_GROUP_COLORS.find(candidate => !colors.has(candidate)) ?? PLAYWRIGHT_GROUP_COLORS[0]; + return { title, color }; +} + // Ungroups any Playwright-titled groups left behind by a prior service worker. export async function cleanupStalePlaywrightGroups(): Promise { try { - const groups = await chrome.tabGroups.query({ title: PLAYWRIGHT_GROUP_TITLE }); - const tabsPerGroup = await Promise.all(groups.map(g => chrome.tabs.query({ groupId: g.id }))); + const groups = await chrome.tabGroups.query({}); + // The bare title comes from versions that predate per-client groups. + const stale = groups.filter(g => g.title === PLAYWRIGHT_GROUP_TITLE || g.title?.startsWith(PLAYWRIGHT_GROUP_TITLE_PREFIX)); + const tabsPerGroup = await Promise.all(stale.map(g => chrome.tabs.query({ groupId: g.id }))); const tabIds = tabsPerGroup.flat().map(t => t.id).filter((id): id is number => id !== undefined); if (tabIds.length) - await chrome.tabs.ungroup(tabIds); + await ungroupTabs(tabIds); } catch (error: any) { debugLog('Error cleaning up stale groups:', error); } @@ -47,7 +70,10 @@ export async function cleanupStalePlaywrightGroups(): Promise { // `_groupTabIds` caches group membership from Chrome events so hot-path checks // in `_onTabUpdated` stay synchronous. export class ConnectedTabGroup { + readonly clientName: string | undefined; + readonly groupStyle: GroupStyle; private _connection: RelayConnection; + private _isTabReserved: (tabId: number) => boolean; private _groupId: number | null = null; private _groupTabIds: Set = new Set(); private _onTabUpdatedListener: (tabId: number, changeInfo: chrome.tabs.TabChangeInfo, tab: chrome.tabs.Tab) => void; @@ -55,7 +81,10 @@ export class ConnectedTabGroup { onclose?: () => void; - constructor(connection: RelayConnection, selectedTab: chrome.tabs.Tab) { + constructor(connection: RelayConnection, selectedTab: chrome.tabs.Tab, clientName: string | undefined, groupStyle: GroupStyle, isTabReserved: (tabId: number) => boolean) { + this.clientName = clientName; + this.groupStyle = groupStyle; + this._isTabReserved = isTabReserved; this._connection = connection; this._connection.onclose = () => this._onConnectionClose(); this._connection.ontabattached = (tabId: number) => this._onTabAttached(tabId); @@ -80,6 +109,13 @@ export class ConnectedTabGroup { this._connection.close(reason); } + releaseTab(tabId: number): void { + if (!this._groupTabIds.has(tabId)) + return; + this._groupTabIds.delete(tabId); + this._connection.detachTab(tabId); + } + private _onTabUpdated(tabId: number, changeInfo: chrome.tabs.TabChangeInfo, tab: chrome.tabs.Tab): void { if (changeInfo.groupId !== undefined) this._onTabGroupChanged(tabId, tab); @@ -102,6 +138,12 @@ export class ConnectedTabGroup { if (inOurGroup === wasInGroup) return; if (inOurGroup) { + // Chrome may drop the connect page of a client that is still connecting + // into our group; that tab is spoken for. + if (this._isTabReserved(tabId)) { + void ungroupTabs([tabId]); + return; + } this._groupTabIds.add(tabId); if (!isNonDebuggableUrl(tab.url)) this._connection.attachTab(tab); @@ -133,11 +175,8 @@ export class ConnectedTabGroup { chrome.tabs.onRemoved.removeListener(this._onTabRemovedListener); const groupTabs = [...this._groupTabIds]; this._groupTabIds.clear(); - if (groupTabs.length) { - this._retryOnDrag(() => chrome.tabs.ungroup(groupTabs)).catch(error => { - debugLog('Error ungrouping tabs on close:', error); - }); - } + if (groupTabs.length) + void ungroupTabs(groupTabs); this.onclose?.(); } @@ -161,10 +200,10 @@ export class ConnectedTabGroup { if (this._groupTabIds.has(tabId)) return; try { - await this._retryOnDrag(async () => { + await retryOnDrag(async () => { if (this._groupId === null) { this._groupId = await chrome.tabs.group({ tabIds: [tabId] }); - await chrome.tabGroups.update(this._groupId, { color: PLAYWRIGHT_GROUP_COLOR, title: PLAYWRIGHT_GROUP_TITLE }); + await chrome.tabGroups.update(this._groupId, this.groupStyle); } else { await chrome.tabs.group({ groupId: this._groupId, tabIds: [tabId] }); } @@ -175,23 +214,32 @@ export class ConnectedTabGroup { } } - // Chrome throws "user may be dragging a tab" while a drag is in progress. - // Retry with backoff until it clears (or we give up). - private async _retryOnDrag(fn: () => Promise): Promise { - const delays = [0, 100, 200, 400, 800]; - let lastError: unknown; - for (const delay of delays) { - if (delay) - await new Promise(resolve => setTimeout(resolve, delay)); - try { - await fn(); - return; - } catch (error: any) { - if (!error?.message?.includes('user may be dragging a tab')) - throw error; - lastError = error; - } +} + +export async function ungroupTabs(tabIds: number[]): Promise { + try { + await retryOnDrag(() => chrome.tabs.ungroup(tabIds)); + } catch (error: any) { + debugLog('Error ungrouping tabs:', error); + } +} + +// Chrome throws "user may be dragging a tab" while a drag is in progress. +// Retry with backoff until it clears (or we give up). +async function retryOnDrag(fn: () => Promise): Promise { + const delays = [0, 100, 200, 400, 800]; + let lastError: unknown; + for (const delay of delays) { + if (delay) + await new Promise(resolve => setTimeout(resolve, delay)); + try { + await fn(); + return; + } catch (error: any) { + if (!error?.message?.includes('user may be dragging a tab')) + throw error; + lastError = error; } - throw lastError; } + throw lastError; } diff --git a/packages/extension/src/pendingConnection.ts b/packages/extension/src/pendingConnection.ts index 38a339e7e1805..43fbbb85854ea 100644 --- a/packages/extension/src/pendingConnection.ts +++ b/packages/extension/src/pendingConnection.ts @@ -29,6 +29,11 @@ export class PendingConnections { this._map.set(selectorTabId, mcpRelayUrl); } + // A connect page awaiting approval; no connection may claim its tab. + has(selectorTabId: number): boolean { + return this._map.has(selectorTabId); + } + async take(selectorTabId: number): Promise { const mcpRelayUrl = this._map.get(selectorTabId); if (mcpRelayUrl === undefined) diff --git a/packages/extension/src/ui/connect.css b/packages/extension/src/ui/connect.css index f73b23a3a53f2..d29ae186acf5d 100644 --- a/packages/extension/src/ui/connect.css +++ b/packages/extension/src/ui/connect.css @@ -140,6 +140,13 @@ body { border-color: #c73836; } +/* Connections */ +.connection + .connection { + margin-top: 16px; + padding-top: 16px; + border-top: 1px solid #d1d9e0; +} + /* Connection header */ .connection-header { display: flex; diff --git a/packages/extension/src/ui/status.tsx b/packages/extension/src/ui/status.tsx index 4869b0b94c1c1..2e270d6f71604 100644 --- a/packages/extension/src/ui/status.tsx +++ b/packages/extension/src/ui/status.tsx @@ -19,57 +19,73 @@ import { createRoot } from 'react-dom/client'; import { Button, TabItem } from './tabItem'; import { AuthTokenSection } from './authToken'; +type ConnectionStatus = { + id: number; + clientName?: string; + connectedTabIds: number[]; +}; + +type Connection = { + id: number; + clientName?: string; + tabs: chrome.tabs.Tab[]; +}; + const StatusApp: React.FC = () => { - const [connectedTabs, setConnectedTabs] = useState([]); - const [clientName, setClientName] = useState(undefined); + const [connections, setConnections] = useState([]); + + const loadStatus = async () => { + const response = await chrome.runtime.sendMessage({ type: 'getConnectionStatus' }); + const statuses = (response.connections ?? []) as ConnectionStatus[]; + const loaded = await Promise.all(statuses.map(async ({ id, clientName, connectedTabIds }) => { + const tabs = await Promise.all(connectedTabIds.map(tabId => chrome.tabs.get(tabId).catch(() => undefined))); + return { id, clientName, tabs: tabs.filter(tab => !!tab) }; + })); + setConnections(loaded); + }; useEffect(() => { void loadStatus(); }, []); - const loadStatus = async () => { - const { connectedTabIds, clientName } = await chrome.runtime.sendMessage({ type: 'getConnectionStatus' }); - const tabs = await Promise.all((connectedTabIds as number[] ?? []).map(tabId => chrome.tabs.get(tabId))); - setConnectedTabs(tabs); - setClientName(clientName); - }; - const openTab = async (tabId: number) => { await chrome.tabs.update(tabId, { active: true }); window.close(); }; - const disconnect = async () => { - await chrome.runtime.sendMessage({ type: 'disconnect' }); - window.close(); + const disconnect = async (connectionId: number) => { + await chrome.runtime.sendMessage({ type: 'disconnect', connectionId }); + await loadStatus(); }; return (
- {connectedTabs.length > 0 ? ( -
-
-
- Connected to "{clientName || 'unknown'}" + {connections.length > 0 ? ( + connections.map(connection => ( +
+
+
+ Connected to "{connection.clientName || 'unknown'}" +
+ +
+
+ {connection.tabs.length === 1 ? 'Accessible page:' : 'Accessible pages:'} +
+
+ {connection.tabs.map(tab => ( + openTab(tab.id!)} + /> + ))}
- -
-
- {connectedTabs.length === 1 ? 'Accessible page:' : 'Accessible pages:'} -
-
- {connectedTabs.map(tab => ( - openTab(tab.id!)} - /> - ))}
-
+ )) ) : (
No clients are currently connected. You can connect from the Playwright CLI or MCP server by passing the --extension flag. diff --git a/tests/extension/multi-connection.spec.ts b/tests/extension/multi-connection.spec.ts new file mode 100644 index 0000000000000..476cdc870a2bb --- /dev/null +++ b/tests/extension/multi-connection.spec.ts @@ -0,0 +1,206 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test, expect, extensionId, clickAllowAndSelect, readExtensionToken, startWithExtensionFlag } from './extension-fixtures'; + +import type { BrowserWithExtension } from './extension-fixtures'; +import type { StartClient } from '../mcp/fixtures'; +import type { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import type { BrowserContext } from 'playwright'; + +// Connects without the approval dialog, so a test can start several clients +// and tell them apart by name. +async function connectWithName(browserWithExtension: BrowserWithExtension, startClient: StartClient, token: string, clientName: string): Promise { + const { client } = await startClient({ + clientName, + args: ['--extension'], + env: { + PLAYWRIGHT_MCP_EXTENSION_TOKEN: token, + PWTEST_EXTENSION_USER_DATA_DIR: browserWithExtension.userDataDir, + }, + }); + return client; +} + +// Starts a client and hands back its connect page, before any tab is picked. +async function beginConnect(browserContext: BrowserContext, browserWithExtension: BrowserWithExtension, startClient: StartClient, url: string) { + const client = await startWithExtensionFlag(browserWithExtension, startClient); + const connectPagePromise = browserContext.waitForEvent('page', page => + page.url().startsWith(`chrome-extension://${extensionId}/connect.html`) + ); + const navigatePromise = client.callTool({ name: 'browser_navigate', arguments: { url } }); + const connectPage = await connectPagePromise; + return { client, connectPage, navigatePromise }; +} + +async function playwrightGroups(browserContext: BrowserContext): Promise<{ title: string, color: string }[]> { + const [sw] = browserContext.serviceWorkers(); + const groups = await sw.evaluate(async () => { + const chrome = (globalThis as any).chrome; + return await chrome.tabGroups.query({}); + }); + return groups + .map((group: any) => ({ title: group.title, color: group.color })) + .sort((a: any, b: any) => a.title.localeCompare(b.title)); +} + +async function tabList(client: Client): Promise { + const response = await client.callTool({ name: 'browser_tabs', arguments: { action: 'list' } }) as any; + return response.content?.[0]?.text ?? ''; +} + +test(`two clients connect at the same time, each in its own tab group`, { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41838' }, +}, async ({ browserWithExtension, startClient, server }) => { + server.setContent('/second', 'SecondSecond page', 'text/html'); + + const browserContext = await browserWithExtension.launch(); + const token = await readExtensionToken(browserContext); + + const clientA = await connectWithName(browserWithExtension, startClient, token, 'client-a'); + expect(await clientA.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + })).toHaveResponse({ snapshot: expect.stringContaining('Hello, world!') }); + + const clientB = await connectWithName(browserWithExtension, startClient, token, 'client-b'); + expect(await clientB.callTool({ + name: 'browser_navigate', + arguments: { url: server.PREFIX + '/second' }, + })).toHaveResponse({ snapshot: expect.stringContaining('Second page') }); + + // Neither connection took the other over. + expect(await tabList(clientA)).toContain('Title'); + expect(await tabList(clientA)).not.toContain('Second'); + expect(await tabList(clientB)).toContain('Second'); + expect(await tabList(clientB)).not.toContain('Title'); + + // Each connection takes the next unused color. + await expect.poll(() => playwrightGroups(browserContext)).toEqual([ + { title: 'Playwright · client-a', color: 'green' }, + { title: 'Playwright · client-b', color: 'blue' }, + ]); +}); + +test(`second client cannot pick a tab owned by the first one`, { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41838' }, +}, async ({ browserWithExtension, startClient, server }) => { + server.setContent('/second', 'SecondSecond page', 'text/html'); + + const browserContext = await browserWithExtension.launch(); + + const first = await browserContext.newPage(); + await first.goto(server.HELLO_WORLD); + const second = await browserContext.newPage(); + await second.goto(server.PREFIX + '/second'); + + const a = await beginConnect(browserContext, browserWithExtension, startClient, server.HELLO_WORLD); + await clickAllowAndSelect(a.connectPage, 'Title'); + await a.navigatePromise; + + const b = await beginConnect(browserContext, browserWithExtension, startClient, server.PREFIX + '/second'); + // A tab the first client controls is not offered. + await expect(b.connectPage.locator('.tab-item', { hasText: 'Second' })).toBeVisible(); + await expect(b.connectPage.locator('.tab-item', { hasText: 'Title' })).toHaveCount(0); + + await clickAllowAndSelect(b.connectPage, 'Second'); + await b.navigatePromise; + + // Same client name, so the second group gets a suffix. + expect(await tabList(a.client)).toContain('Title'); + expect(await tabList(b.client)).toContain('Second'); + await expect.poll(() => playwrightGroups(browserContext)).toEqual([ + { title: 'Playwright · test', color: 'green' }, + { title: 'Playwright · test (2)', color: 'blue' }, + ]); +}); + +test(`connect page opened inside another client's group is released`, { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41838' }, +}, async ({ browserWithExtension, startClient, server }) => { + const browserContext = await browserWithExtension.launch(); + + const page = await browserContext.newPage(); + await page.goto(server.HELLO_WORLD); + + const a = await beginConnect(browserContext, browserWithExtension, startClient, server.HELLO_WORLD); + await clickAllowAndSelect(a.connectPage, 'Title'); + await a.navigatePromise; + + const b = await beginConnect(browserContext, browserWithExtension, startClient, server.HELLO_WORLD); + await expect(b.connectPage.locator('.tab-item').first()).toBeVisible(); + + // Reproduce a connect page created inside the first client's group, where + // no group-change event ever fires. + const [sw] = browserContext.serviceWorkers(); + await sw.evaluate(async () => { + const chrome = (globalThis as any).chrome; + const [connected] = await chrome.tabs.query({ title: 'Title' }); + const [connectTab] = await chrome.tabs.query({ url: 'chrome-extension://*/connect.html*' }); + await chrome.tabs.group({ groupId: connected.groupId, tabIds: [connectTab.id] }); + }); + await b.connectPage.reload(); + + // The connect page leaves the group on its own. + await expect.poll(() => b.connectPage.evaluate(async () => { + const tab = await (window as any).chrome.tabs.getCurrent(); + return tab?.groupId ?? -1; + })).toBe(-1); + await expect(b.connectPage.locator('.tab-item').first()).toBeVisible(); + + await clickAllowAndSelect(b.connectPage, 'Welcome'); + await b.navigatePromise; + + expect(await tabList(a.client)).toContain('Title'); + await expect.poll(() => playwrightGroups(browserContext)).toEqual([ + { title: 'Playwright · test', color: 'green' }, + { title: 'Playwright · test (2)', color: 'blue' }, + ]); +}); + +test(`status page disconnects a single client`, { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41838' }, +}, async ({ browserWithExtension, startClient, server }) => { + server.setContent('/second', 'SecondSecond page', 'text/html'); + + const browserContext = await browserWithExtension.launch(); + const token = await readExtensionToken(browserContext); + + const connect = async (clientName: string, url: string) => { + const client = await connectWithName(browserWithExtension, startClient, token, clientName); + await client.callTool({ name: 'browser_navigate', arguments: { url } }); + return client; + }; + + await connect('client-a', server.HELLO_WORLD); + const clientB = await connect('client-b', server.PREFIX + '/second'); + + const statusPage = await browserContext.newPage(); + await statusPage.goto(`chrome-extension://${extensionId}/status.html`); + await expect(statusPage.locator('.client-info')).toHaveText([ + 'Connected to "client-a"', + 'Connected to "client-b"', + ]); + + await statusPage.locator('.connection', { hasText: 'client-a' }).getByRole('button', { name: 'Disconnect' }).click(); + + await expect(statusPage.locator('.client-info')).toHaveText(['Connected to "client-b"']); + // The surviving connection keeps the color it started with. + expect(await tabList(clientB)).toContain('Second'); + await expect.poll(() => playwrightGroups(browserContext)).toEqual([ + { title: 'Playwright · client-b', color: 'blue' }, + ]); +}); diff --git a/tests/extension/tab-grouping.spec.ts b/tests/extension/tab-grouping.spec.ts index 7a80d52c0de34..c0b2d93624a8f 100644 --- a/tests/extension/tab-grouping.spec.ts +++ b/tests/extension/tab-grouping.spec.ts @@ -74,7 +74,7 @@ test('connected tab is in green Playwright group, connect page is closed', async const g = await chrome.tabGroups.get(connectedTab.groupId); return { color: g.color, title: g.title }; }); - }).toEqual({ color: 'green', title: 'Playwright' }); + }).toEqual({ color: 'green', title: 'Playwright · test' }); }); test('tab added to group gets auto-attached', async ({ browserWithExtension, startClient, server }) => { @@ -337,5 +337,5 @@ test('tab is re-added to Playwright group after reconnecting', async ({ browserW const g = await chrome.tabGroups.get(tab.groupId); return { color: g.color, title: g.title }; }); - }).toEqual({ color: 'green', title: 'Playwright' }); + }).toEqual({ color: 'green', title: 'Playwright · test' }); });