diff --git a/packages/core/src/agents/browser/browserManager.test.ts b/packages/core/src/agents/browser/browserManager.test.ts index d24e2c0a7b..389e7da70d 100644 --- a/packages/core/src/agents/browser/browserManager.test.ts +++ b/packages/core/src/agents/browser/browserManager.test.ts @@ -138,7 +138,7 @@ describe('BrowserManager', () => { }); describe('MCP connection', () => { - it('should spawn npx chrome-devtools-mcp with --isolated and --experimental-vision', async () => { + it('should spawn npx chrome-devtools-mcp with --experimental-vision (persistent mode by default)', async () => { const manager = new BrowserManager(mockConfig); await manager.ensureConnection(); @@ -148,10 +148,13 @@ describe('BrowserManager', () => { args: expect.arrayContaining([ '-y', expect.stringMatching(/chrome-devtools-mcp@/), - '--isolated', '--experimental-vision', ]), }); + // Persistent mode should NOT include --isolated or --autoConnect + const args = vi.mocked(StdioClientTransport).mock.calls[0]?.[0]?.args as string[]; + expect(args).not.toContain('--isolated'); + expect(args).not.toContain('--autoConnect'); }); it('should pass headless flag when configured', async () => { @@ -177,7 +180,7 @@ describe('BrowserManager', () => { }); }); - it('should pass chromeProfilePath when configured', async () => { + it('should pass chromeProfilePath as --userDataDir when configured', async () => { const profileConfig = makeFakeConfig({ agents: { overrides: { @@ -196,9 +199,90 @@ describe('BrowserManager', () => { expect(StdioClientTransport).toHaveBeenCalledWith({ command: 'npx', - args: expect.arrayContaining(['--profile-path', '/path/to/profile']), + args: expect.arrayContaining(['--userDataDir', '/path/to/profile']), }); }); + + it('should pass --isolated when sessionMode is isolated', async () => { + const isolatedConfig = makeFakeConfig({ + agents: { + overrides: { + browser_agent: { + enabled: true, + customConfig: { + sessionMode: 'isolated', + }, + }, + }, + }, + }); + + const manager = new BrowserManager(isolatedConfig); + await manager.ensureConnection(); + + const args = vi.mocked(StdioClientTransport).mock.calls[0]?.[0]?.args as string[]; + expect(args).toContain('--isolated'); + expect(args).not.toContain('--autoConnect'); + }); + + it('should pass --autoConnect when sessionMode is existing', async () => { + const existingConfig = makeFakeConfig({ + agents: { + overrides: { + browser_agent: { + enabled: true, + customConfig: { + sessionMode: 'existing', + }, + }, + }, + }, + }); + + const manager = new BrowserManager(existingConfig); + await manager.ensureConnection(); + + const args = vi.mocked(StdioClientTransport).mock.calls[0]?.[0]?.args as string[]; + expect(args).toContain('--autoConnect'); + expect(args).not.toContain('--isolated'); + }); + + it('should throw actionable error when existing mode connection fails', async () => { + // Make the Client mock's connect method reject + vi.mocked(Client).mockImplementation( + () => + ({ + connect: vi + .fn() + .mockRejectedValue(new Error('Connection refused')), + close: vi.fn().mockResolvedValue(undefined), + listTools: vi.fn(), + callTool: vi.fn(), + }) as unknown as InstanceType, + ); + + const existingConfig = makeFakeConfig({ + agents: { + overrides: { + browser_agent: { + enabled: true, + customConfig: { + sessionMode: 'existing', + }, + }, + }, + }, + }); + + const manager = new BrowserManager(existingConfig); + + await expect(manager.ensureConnection()).rejects.toThrow( + /Failed to connect to existing Chrome instance/, + ); + await expect(manager.ensureConnection()).rejects.toThrow( + /chrome:\/\/inspect\/#remote-debugging/, + ); + }); }); describe('MCP isolation', () => { diff --git a/packages/core/src/agents/browser/browserManager.ts b/packages/core/src/agents/browser/browserManager.ts index 835ba300c2..a79f6d3b19 100644 --- a/packages/core/src/agents/browser/browserManager.ts +++ b/packages/core/src/agents/browser/browserManager.ts @@ -237,22 +237,32 @@ export class BrowserManager { // Build args for chrome-devtools-mcp const browserConfig = this.config.getBrowserAgentConfig(); - const sessionMode = browserConfig.customConfig.sessionMode ?? 'isolated'; + const sessionMode = browserConfig.customConfig.sessionMode ?? 'persistent'; const mcpArgs = [ '-y', `chrome-devtools-mcp@${CHROME_DEVTOOLS_MCP_VERSION}`, - sessionMode === 'existing' ? '--existing' : '--isolated', '--experimental-vision', ]; + // Session mode determines how the browser is managed: + // - "isolated": Temp profile, cleaned up after session (--isolated) + // - "persistent": Persistent profile at ~/.cache/chrome-devtools-mcp/ (default) + // - "existing": Connect to already-running Chrome (--autoConnect, requires + // remote debugging enabled at chrome://inspect/#remote-debugging) + if (sessionMode === 'isolated') { + mcpArgs.push('--isolated'); + } else if (sessionMode === 'existing') { + mcpArgs.push('--autoConnect'); + } + // Add optional settings from config if (browserConfig.customConfig.headless) { mcpArgs.push('--headless'); } if (browserConfig.customConfig.chromeProfilePath) { mcpArgs.push( - '--profile-path', + '--userDataDir', browserConfig.customConfig.chromeProfilePath, ); } @@ -267,12 +277,50 @@ export class BrowserManager { args: mcpArgs, }); - // Connect to MCP server - await this.rawMcpClient.connect(this.mcpTransport); - debugLogger.log('MCP client connected to chrome-devtools-mcp'); + // Connect to MCP server — use a shorter timeout for 'existing' mode + // since it should connect quickly if remote debugging is enabled. + const connectTimeoutMs = + sessionMode === 'existing' ? 15_000 : MCP_TIMEOUT_MS; - // Discover tools from the MCP server - await this.discoverTools(); + let timeoutId: ReturnType | undefined; + try { + await Promise.race([ + (async () => { + await this.rawMcpClient!.connect(this.mcpTransport!); + debugLogger.log('MCP client connected to chrome-devtools-mcp'); + await this.discoverTools(); + })(), + new Promise((_, reject) => { + timeoutId = setTimeout( + () => + reject( + new Error( + `Timed out connecting to chrome-devtools-mcp (${connectTimeoutMs}ms)`, + ), + ), + connectTimeoutMs, + ); + }), + ]); + } catch (error) { + // Provide actionable error for 'existing' mode failures + if (sessionMode === 'existing') { + const message = error instanceof Error ? error.message : String(error); + throw new Error( + `Failed to connect to existing Chrome instance: ${message}\n\n` + + `To use sessionMode "existing", you must:\n` + + ` 1. Open Chrome (version 144+)\n` + + ` 2. Navigate to chrome://inspect/#remote-debugging\n` + + ` 3. Enable remote debugging\n\n` + + `Alternatively, use sessionMode "persistent" (default) to launch a dedicated browser.`, + ); + } + throw error; + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + } } /** diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 9f466d443f..74329b6263 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -1331,7 +1331,7 @@ describe('Server Config (config.ts)', () => { expect(browserConfig.enabled).toBe(false); expect(browserConfig.model).toBeUndefined(); - expect(browserConfig.customConfig.sessionMode).toBe('isolated'); + expect(browserConfig.customConfig.sessionMode).toBe('persistent'); expect(browserConfig.customConfig.headless).toBe(false); expect(browserConfig.customConfig.chromeProfilePath).toBeUndefined(); expect(browserConfig.customConfig.visualModel).toBeUndefined(); @@ -1390,7 +1390,7 @@ describe('Server Config (config.ts)', () => { expect(browserConfig.enabled).toBe(true); expect(browserConfig.customConfig.headless).toBe(true); // Defaults for unspecified fields - expect(browserConfig.customConfig.sessionMode).toBe('isolated'); + expect(browserConfig.customConfig.sessionMode).toBe('persistent'); }); }); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index aa7ce13f64..d8ca51e9e6 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -270,8 +270,14 @@ export interface CustomTheme { * Used in agents.overrides.browser_agent.customConfig */ export interface BrowserAgentCustomConfig { - /** Session mode: 'isolated' (launch new browser) or 'existing' (attach to Chrome M144+). Default: 'isolated' */ - sessionMode?: 'isolated' | 'existing'; + /** + * Session mode: + * - 'persistent': Launch Chrome with a persistent profile at ~/.cache/chrome-devtools-mcp/ (default) + * - 'isolated': Launch Chrome with a temporary profile, cleaned up after session + * - 'existing': Attach to an already-running Chrome instance (requires remote debugging + * enabled at chrome://inspect/#remote-debugging) + */ + sessionMode?: 'isolated' | 'persistent' | 'existing'; /** Run browser in headless mode. Default: false */ headless?: boolean; /** Path to Chrome profile directory for session persistence. */ @@ -2539,7 +2545,7 @@ export class Config { enabled: override?.enabled ?? false, model: override?.modelConfig?.model, customConfig: { - sessionMode: customConfig.sessionMode ?? 'isolated', + sessionMode: customConfig.sessionMode ?? 'persistent', headless: customConfig.headless ?? false, chromeProfilePath: customConfig.chromeProfilePath, visualModel: customConfig.visualModel,