From 64af5f87631bdc30a3c9f3b77bcbfc904408a2ce Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Fri, 22 Aug 2025 10:02:09 -0700 Subject: [PATCH] chore(extension): do not show tab selector for browser_navigate (#923) --- extension/src/background.ts | 8 ++++-- extension/src/ui/connect.tsx | 35 +++++++++++++++++++----- extension/tests/extension.spec.ts | 3 +- src/browserContextFactory.ts | 2 +- src/browserServerBackend.ts | 4 +-- src/context.ts | 10 +++---- src/extension/cdpRelay.ts | 8 ++++-- src/extension/extensionContextFactory.ts | 8 +++--- 8 files changed, 52 insertions(+), 26 deletions(-) diff --git a/extension/src/background.ts b/extension/src/background.ts index 34623fa..e71e0df 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -23,8 +23,8 @@ type PageMessage = { type: 'getTabs'; } | { type: 'connectToTab'; - tabId: number; - windowId: number; + tabId?: number; + windowId?: number; mcpRelayUrl: string; } | { type: 'getConnectionStatus'; @@ -59,7 +59,9 @@ class TabShareExtension { (error: any) => sendResponse({ success: false, error: error.message })); return true; case 'connectToTab': - this._connectTab(sender.tab!.id!, message.tabId, message.windowId, message.mcpRelayUrl!).then( + const tabId = message.tabId || sender.tab?.id!; + const windowId = message.windowId || sender.tab?.windowId!; + this._connectTab(sender.tab!.id!, tabId, windowId, message.mcpRelayUrl!).then( () => sendResponse({ success: true }), (error: any) => sendResponse({ success: false, error: error.message })); return true; // Return true to indicate that the response will be sent asynchronously diff --git a/extension/src/ui/connect.tsx b/extension/src/ui/connect.tsx index 301596e..0f0f1f1 100644 --- a/extension/src/ui/connect.tsx +++ b/extension/src/ui/connect.tsx @@ -32,6 +32,7 @@ const ConnectApp: React.FC = () => { const [showTabList, setShowTabList] = useState(true); const [clientInfo, setClientInfo] = useState('unknown'); const [mcpRelayUrl, setMcpRelayUrl] = useState(''); + const [newTab, setNewTab] = useState(false); useEffect(() => { const params = new URLSearchParams(window.location.search); @@ -76,7 +77,14 @@ const ConnectApp: React.FC = () => { } void connectToMCPRelay(relayUrl); - void loadTabs(); + + // If this is a browser_navigate command, hide the tab list and show simple allow/reject + if (params.get('newTab') === 'true') { + setNewTab(true); + setShowTabList(false); + } else { + void loadTabs(); + } }, []); const handleReject = useCallback((message: string) => { @@ -100,7 +108,7 @@ const ConnectApp: React.FC = () => { setStatus({ type: 'error', message: 'Failed to load tabs: ' + response.error }); }, []); - const handleConnectToTab = useCallback(async (tab: TabInfo) => { + const handleConnectToTab = useCallback(async (tab?: TabInfo) => { setShowButtons(false); setShowTabList(false); @@ -108,8 +116,8 @@ const ConnectApp: React.FC = () => { const response = await chrome.runtime.sendMessage({ type: 'connectToTab', mcpRelayUrl, - tabId: tab.id, - windowId: tab.windowId, + tabId: tab?.id, + windowId: tab?.windowId, }); if (response?.success) { @@ -146,9 +154,22 @@ const ConnectApp: React.FC = () => {
{showButtons && ( - +
+ {newTab ? ( + <> + + + + ) : ( + + )} +
)}
)} diff --git a/extension/tests/extension.spec.ts b/extension/tests/extension.spec.ts index fe60fb4..174ad79 100644 --- a/extension/tests/extension.spec.ts +++ b/extension/tests/extension.spec.ts @@ -152,7 +152,8 @@ for (const [mode, startClientMethod] of [ }); const selectorPage = await confirmationPagePromise; - await selectorPage.locator('.tab-item', { hasText: 'Playwright MCP Extension' }).getByRole('button', { name: 'Connect' }).click(); + // For browser_navigate command, the UI shows Allow/Reject buttons instead of tab selector + await selectorPage.getByRole('button', { name: 'Allow' }).click(); expect(await navigateResponse).toHaveResponse({ pageState: expect.stringContaining(`- generic [active] [ref=e1]: Hello, world!`), diff --git a/src/browserContextFactory.ts b/src/browserContextFactory.ts index 4d39562..81072e1 100644 --- a/src/browserContextFactory.ts +++ b/src/browserContextFactory.ts @@ -42,7 +42,7 @@ export function contextFactory(config: FullConfig): BrowserContextFactory { export type ClientInfo = { name?: string, version?: string, rootPath?: string }; export interface BrowserContextFactory { - createContext(clientInfo: ClientInfo, abortSignal: AbortSignal): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise }>; + createContext(clientInfo: ClientInfo, abortSignal: AbortSignal, toolName: string | undefined): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise }>; } class BaseContextFactory implements BrowserContextFactory { diff --git a/src/browserServerBackend.ts b/src/browserServerBackend.ts index 5286e03..4d18fce 100644 --- a/src/browserServerBackend.ts +++ b/src/browserServerBackend.ts @@ -69,7 +69,7 @@ export class BrowserServerBackend implements ServerBackend { const parsedArguments = tool.schema.inputSchema.parse(rawArguments || {}); const context = this._context!; const response = new Response(context, name, parsedArguments); - context.setRunningTool(true); + context.setRunningTool(name); try { await tool.handle(context, parsedArguments, response); await response.finish(); @@ -77,7 +77,7 @@ export class BrowserServerBackend implements ServerBackend { } catch (error: any) { response.addError(String(error)); } finally { - context.setRunningTool(false); + context.setRunningTool(undefined); } return response.serialize(); } diff --git a/src/context.ts b/src/context.ts index 52ebb59..1890b3d 100644 --- a/src/context.ts +++ b/src/context.ts @@ -50,7 +50,7 @@ export class Context { private static _allContexts: Set = new Set(); private _closeBrowserContextPromise: Promise | undefined; - private _isRunningTool: boolean = false; + private _runningToolName: string | undefined; private _abortController = new AbortController(); constructor(options: ContextOptions) { @@ -145,11 +145,11 @@ export class Context { } isRunningTool() { - return this._isRunningTool; + return this._runningToolName !== undefined; } - setRunningTool(isRunningTool: boolean) { - this._isRunningTool = isRunningTool; + setRunningTool(name: string | undefined) { + this._runningToolName = name; } private async _closeBrowserContextImpl() { @@ -202,7 +202,7 @@ export class Context { if (this._closeBrowserContextPromise) throw new Error('Another browser context is being closed.'); // TODO: move to the browser context factory to make it based on isolation mode. - const result = await this._browserContextFactory.createContext(this._clientInfo, this._abortController.signal); + const result = await this._browserContextFactory.createContext(this._clientInfo, this._abortController.signal, this._runningToolName); const { browserContext } = result; await this._setupRequestInterception(browserContext); if (this.sessionLog) diff --git a/src/extension/cdpRelay.ts b/src/extension/cdpRelay.ts index c06a94b..4a85656 100644 --- a/src/extension/cdpRelay.ts +++ b/src/extension/cdpRelay.ts @@ -94,11 +94,11 @@ export class CDPRelayServer { return `${this._wsHost}${this._extensionPath}`; } - async ensureExtensionConnectionForMCPContext(clientInfo: ClientInfo, abortSignal: AbortSignal) { + async ensureExtensionConnectionForMCPContext(clientInfo: ClientInfo, abortSignal: AbortSignal, toolName: string | undefined) { debugLogger('Ensuring extension connection for MCP context'); if (this._extensionConnection) return; - this._connectBrowser(clientInfo); + this._connectBrowser(clientInfo, toolName); debugLogger('Waiting for incoming extension connection'); await Promise.race([ this._extensionConnectionPromise, @@ -110,7 +110,7 @@ export class CDPRelayServer { debugLogger('Extension connection established'); } - private _connectBrowser(clientInfo: ClientInfo) { + private _connectBrowser(clientInfo: ClientInfo, toolName: string | undefined) { const mcpRelayEndpoint = `${this._wsHost}${this._extensionPath}`; // Need to specify "key" in the manifest.json to make the id stable when loading from file. const url = new URL('chrome-extension://jakfalbnbhgkpmoaakfflhflbfpkailf/connect.html'); @@ -121,6 +121,8 @@ export class CDPRelayServer { }; url.searchParams.set('client', JSON.stringify(client)); url.searchParams.set('pwMcpVersion', packageJSON.version); + if (toolName) + url.searchParams.set('newTab', String(toolName === 'browser_navigate')); const href = url.toString(); const executableInfo = registry.findExecutable(this._browserChannel); if (!executableInfo) diff --git a/src/extension/extensionContextFactory.ts b/src/extension/extensionContextFactory.ts index 0346419..397aaa8 100644 --- a/src/extension/extensionContextFactory.ts +++ b/src/extension/extensionContextFactory.ts @@ -32,8 +32,8 @@ export class ExtensionContextFactory implements BrowserContextFactory { this._userDataDir = userDataDir; } - async createContext(clientInfo: ClientInfo, abortSignal: AbortSignal): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise }> { - const browser = await this._obtainBrowser(clientInfo, abortSignal); + async createContext(clientInfo: ClientInfo, abortSignal: AbortSignal, toolName: string | undefined): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise }> { + const browser = await this._obtainBrowser(clientInfo, abortSignal, toolName); return { browserContext: browser.contexts()[0], close: async () => { @@ -43,9 +43,9 @@ export class ExtensionContextFactory implements BrowserContextFactory { }; } - private async _obtainBrowser(clientInfo: ClientInfo, abortSignal: AbortSignal): Promise { + private async _obtainBrowser(clientInfo: ClientInfo, abortSignal: AbortSignal, toolName: string | undefined): Promise { const relay = await this._startRelay(abortSignal); - await relay.ensureExtensionConnectionForMCPContext(clientInfo, abortSignal); + await relay.ensureExtensionConnectionForMCPContext(clientInfo, abortSignal, toolName); return await playwright.chromium.connectOverCDP(relay.cdpEndpoint()); }