chore(extension): do not show tab selector for browser_navigate (#923)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<boolean>(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 = () => {
|
||||
<div className='status-container'>
|
||||
<StatusBanner status={status} />
|
||||
{showButtons && (
|
||||
<Button variant='reject' onClick={() => handleReject('Connection rejected. This tab can be closed.')}>
|
||||
Reject
|
||||
</Button>
|
||||
<div className='button-container'>
|
||||
{newTab ? (
|
||||
<>
|
||||
<Button variant='primary' onClick={() => handleConnectToTab()}>
|
||||
Allow
|
||||
</Button>
|
||||
<Button variant='reject' onClick={() => handleReject('Connection rejected. This tab can be closed.')}>
|
||||
Reject
|
||||
</Button>
|
||||
</>
|
||||
) : (
|
||||
<Button variant='reject' onClick={() => handleReject('Connection rejected. This tab can be closed.')}>
|
||||
Reject
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -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!`),
|
||||
|
||||
@@ -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<void> }>;
|
||||
createContext(clientInfo: ClientInfo, abortSignal: AbortSignal, toolName: string | undefined): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise<void> }>;
|
||||
}
|
||||
|
||||
class BaseContextFactory implements BrowserContextFactory {
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -50,7 +50,7 @@ export class Context {
|
||||
|
||||
private static _allContexts: Set<Context> = new Set();
|
||||
private _closeBrowserContextPromise: Promise<void> | 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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -32,8 +32,8 @@ export class ExtensionContextFactory implements BrowserContextFactory {
|
||||
this._userDataDir = userDataDir;
|
||||
}
|
||||
|
||||
async createContext(clientInfo: ClientInfo, abortSignal: AbortSignal): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise<void> }> {
|
||||
const browser = await this._obtainBrowser(clientInfo, abortSignal);
|
||||
async createContext(clientInfo: ClientInfo, abortSignal: AbortSignal, toolName: string | undefined): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise<void> }> {
|
||||
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<playwright.Browser> {
|
||||
private async _obtainBrowser(clientInfo: ClientInfo, abortSignal: AbortSignal, toolName: string | undefined): Promise<playwright.Browser> {
|
||||
const relay = await this._startRelay(abortSignal);
|
||||
await relay.ensureExtensionConnectionForMCPContext(clientInfo, abortSignal);
|
||||
await relay.ensureExtensionConnectionForMCPContext(clientInfo, abortSignal, toolName);
|
||||
return await playwright.chromium.connectOverCDP(relay.cdpEndpoint());
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user