diff --git a/packages/core/src/agents/browser/automationOverlay.ts b/packages/core/src/agents/browser/automationOverlay.ts index e87a70b6da..11d821c95c 100644 --- a/packages/core/src/agents/browser/automationOverlay.ts +++ b/packages/core/src/agents/browser/automationOverlay.ts @@ -88,8 +88,6 @@ export async function injectAutomationOverlay( signal?: AbortSignal, ): Promise { try { - debugLogger.log('Injecting automation overlay...'); - const result = await browserManager.callTool( 'evaluate_script', { function: buildInjectionScript() }, @@ -99,8 +97,6 @@ export async function injectAutomationOverlay( if (result.isError) { debugLogger.warn('Failed to inject automation overlay:', result); - } else { - debugLogger.log('Automation overlay injected successfully'); } } catch (error) { debugLogger.warn('Error injecting automation overlay:', error); @@ -115,8 +111,6 @@ export async function removeAutomationOverlay( signal?: AbortSignal, ): Promise { try { - debugLogger.log('Removing automation overlay...'); - const result = await browserManager.callTool( 'evaluate_script', { function: buildRemovalScript() }, @@ -126,8 +120,6 @@ export async function removeAutomationOverlay( if (result.isError) { debugLogger.warn('Failed to remove automation overlay:', result); - } else { - debugLogger.log('Automation overlay removed successfully'); } } catch (error) { debugLogger.warn('Error removing automation overlay:', error); diff --git a/packages/core/src/agents/browser/browserAgentInvocation.test.ts b/packages/core/src/agents/browser/browserAgentInvocation.test.ts index 200f04e67b..d8dbc69b43 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.test.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.test.ts @@ -32,6 +32,10 @@ vi.mock('./inputBlocker.js', () => ({ removeInputBlocker: vi.fn(), })); +vi.mock('./automationOverlay.js', () => ({ + removeAutomationOverlay: vi.fn(), +})); + vi.mock('../local-executor.js', () => ({ LocalAgentExecutor: { create: vi.fn(), @@ -40,6 +44,7 @@ vi.mock('../local-executor.js', () => ({ import { createBrowserAgentDefinition } from './browserAgentFactory.js'; import { removeInputBlocker } from './inputBlocker.js'; +import { removeAutomationOverlay } from './automationOverlay.js'; import { LocalAgentExecutor } from '../local-executor.js'; import type { ToolLiveOutput } from '../../tools/tools.js'; @@ -677,4 +682,81 @@ describe('BrowserAgentInvocation', () => { expect(toolB?.status).toBe('error'); }); }); + + describe('cleanup', () => { + it('should clean up all pages on finally', async () => { + const mockBrowserManager = { + callTool: vi.fn().mockImplementation(async (toolName: string) => { + if (toolName === 'list_pages') { + return { + content: [{ type: 'text', text: '0: Page 1\n1: Page 2\n' }], + isError: false, + }; + } + return { isError: false }; + }), + }; + + vi.mocked(createBrowserAgentDefinition).mockResolvedValue({ + definition: { + name: 'browser_agent', + description: 'mock definition', + kind: 'local', + inputConfig: {} as never, + outputConfig: {} as never, + processOutput: () => '', + modelConfig: { model: 'test' }, + runConfig: {}, + promptConfig: { query: '', systemPrompt: '' }, + toolConfig: { tools: [] }, + }, + browserManager: mockBrowserManager as never, + }); + + const mockExecutor = { + run: vi.fn().mockResolvedValue({ + result: JSON.stringify({ success: true }), + terminate_reason: 'GOAL', + }), + }; + + vi.mocked(LocalAgentExecutor.create).mockResolvedValue( + mockExecutor as never, + ); + + const invocation = new BrowserAgentInvocation( + mockConfig, + { task: 'test' }, + mockMessageBus, + ); + + await invocation.execute(new AbortController().signal); + + // Verify list_pages was called + expect(mockBrowserManager.callTool).toHaveBeenCalledWith( + 'list_pages', + expect.anything(), + expect.anything(), + true, + ); + + // Verify select_page was called for each page + expect(mockBrowserManager.callTool).toHaveBeenCalledWith( + 'select_page', + { pageId: 0, bringToFront: false }, + expect.anything(), + true, + ); + expect(mockBrowserManager.callTool).toHaveBeenCalledWith( + 'select_page', + { pageId: 1, bringToFront: false }, + expect.anything(), + true, + ); + + // Verify removeInputBlocker and removeAutomationOverlay were called for each page + initial cleanup + expect(removeInputBlocker).toHaveBeenCalledTimes(3); + expect(removeAutomationOverlay).toHaveBeenCalledTimes(3); + }); + }); }); diff --git a/packages/core/src/agents/browser/browserAgentInvocation.ts b/packages/core/src/agents/browser/browserAgentInvocation.ts index 86191c7cc3..17912e5354 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.ts @@ -40,6 +40,7 @@ import { sanitizeToolArgs, sanitizeErrorMessage, } from '../../utils/agent-sanitization-utils.js'; +import { removeAutomationOverlay } from './automationOverlay.js'; const INPUT_PREVIEW_MAX_LENGTH = 50; const DESCRIPTION_MAX_LENGTH = 200; @@ -377,7 +378,40 @@ ${output.result}`; } finally { // Clean up input blocker, but keep browserManager alive for persistent sessions if (browserManager) { - await removeInputBlocker(browserManager); + await removeInputBlocker(browserManager, signal); + await removeAutomationOverlay(browserManager, signal); + + // try cleaning up overlays in previous opened pages if any + try { + const listResult = await browserManager.callTool( + 'list_pages', + {}, + signal, + true, + ); + const pagesText = + listResult.content?.find((c) => c.type === 'text')?.text || ''; + const pageMatches = Array.from(pagesText.matchAll(/^(\d+):/gm)); + const pageIds = pageMatches.map((m) => parseInt(m[1], 10)); + if (pageIds.length > 1) { + for (const pageId of pageIds) { + try { + await browserManager.callTool( + 'select_page', + { pageId, bringToFront: false }, + signal, + true, + ); + await removeInputBlocker(browserManager, signal); + await removeAutomationOverlay(browserManager, signal); + } catch (_err) { + // Ignore errors for individual pages + } + } + } + } catch (_) { + // Ignore errors for removing the overlays. + } } } } diff --git a/packages/core/src/agents/browser/browserManager.test.ts b/packages/core/src/agents/browser/browserManager.test.ts index 0be3d6da82..0b3571eea2 100644 --- a/packages/core/src/agents/browser/browserManager.test.ts +++ b/packages/core/src/agents/browser/browserManager.test.ts @@ -996,6 +996,28 @@ describe('BrowserManager', () => { const manager = new BrowserManager(mockConfig); await manager.callTool('click', { uid: 'bad' }); }); + + it('should NOT re-inject overlay if select_page is called with bringToFront: false', async () => { + mockConfig = makeFakeConfig({ + agents: { + overrides: { + browser_agent: { + enabled: true, + }, + }, + browser: { + headless: false, + disableUserInput: true, + }, + }, + }); + + const manager = new BrowserManager(mockConfig); + await manager.callTool('select_page', { pageId: 1, bringToFront: false }); + + expect(injectAutomationOverlay).not.toHaveBeenCalled(); + expect(injectInputBlocker).not.toHaveBeenCalled(); + }); }); describe('Rate limiting', () => { diff --git a/packages/core/src/agents/browser/browserManager.ts b/packages/core/src/agents/browser/browserManager.ts index d95327a40d..904905cf75 100644 --- a/packages/core/src/agents/browser/browserManager.ts +++ b/packages/core/src/agents/browser/browserManager.ts @@ -302,6 +302,11 @@ export class BrowserManager { POTENTIALLY_NAVIGATING_TOOLS.has(toolName) && !signal?.aborted ) { + // Don't re-inject if explicitly switching to a page in the background + if (toolName === 'select_page' && args['bringToFront'] === false) { + return result; + } + try { if (this.shouldInjectOverlay) { await injectAutomationOverlay(this, signal); @@ -601,7 +606,6 @@ export class BrowserManager { await this.rawMcpClient!.connect(this.mcpTransport!); debugLogger.log('MCP client connected to chrome-devtools-mcp'); await this.discoverTools(); - this.registerInputBlockerHandler(); // clear the action counter for each connection this.actionCounter = 0; })(), @@ -805,45 +809,4 @@ export class BrowserManager { // If none matched, then deny return false; } - - /** - * Registers a fallback notification handler on the MCP client to - * automatically re-inject the input blocker after any server-side - * notification (e.g. page navigation, resource updates). - * - * This covers ALL navigation types (link clicks, form submissions, - * history navigation) — not just explicit navigate_page tool calls. - */ - private registerInputBlockerHandler(): void { - if (!this.rawMcpClient) { - return; - } - - if (!this.config.shouldDisableBrowserUserInput()) { - return; - } - - const existingHandler = this.rawMcpClient.fallbackNotificationHandler; - this.rawMcpClient.fallbackNotificationHandler = async (notification: { - method: string; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - params?: any; - }) => { - // Chain with any existing handler first. - if (existingHandler) { - await existingHandler(notification); - } - - // Only re-inject on resource update notifications which indicate - // page content has changed (navigation, new page, etc.) - if (notification.method === 'notifications/resources/updated') { - debugLogger.log('Page content changed, re-injecting input blocker...'); - void injectInputBlocker(this); - } - }; - - debugLogger.log( - 'Registered global notification handler for input blocker re-injection', - ); - } } diff --git a/packages/core/src/agents/browser/inputBlocker.ts b/packages/core/src/agents/browser/inputBlocker.ts index d7c6d8ce16..28fc8328f4 100644 --- a/packages/core/src/agents/browser/inputBlocker.ts +++ b/packages/core/src/agents/browser/inputBlocker.ts @@ -207,7 +207,6 @@ export async function injectInputBlocker( signal, true, ); - debugLogger.log('Input blocker injected successfully'); } catch (error) { // Log but don't throw - input blocker is a UX enhancement, not critical functionality debugLogger.warn( @@ -235,7 +234,6 @@ export async function removeInputBlocker( signal, true, ); - debugLogger.log('Input blocker removed successfully'); } catch (error) { // Log but don't throw - removal failure is not critical debugLogger.warn(