diff --git a/packages/core/src/agents/browser/browserManager.test.ts b/packages/core/src/agents/browser/browserManager.test.ts index c38457e4aa..a326164c43 100644 --- a/packages/core/src/agents/browser/browserManager.test.ts +++ b/packages/core/src/agents/browser/browserManager.test.ts @@ -9,6 +9,7 @@ import { BrowserManager } from './browserManager.js'; import { makeFakeConfig } from '../../test-utils/config.js'; import type { Config } from '../../config/config.js'; import { injectAutomationOverlay } from './automationOverlay.js'; +import { injectInputBlocker } from './inputBlocker.js'; import { coreEvents } from '../../utils/events.js'; // Mock the MCP SDK @@ -54,6 +55,13 @@ vi.mock('./automationOverlay.js', () => ({ injectAutomationOverlay: vi.fn().mockResolvedValue(undefined), })); +vi.mock('./inputBlocker.js', () => ({ + injectInputBlocker: vi.fn().mockResolvedValue(undefined), + removeInputBlocker: vi.fn().mockResolvedValue(undefined), + suspendInputBlocker: vi.fn().mockResolvedValue(undefined), + resumeInputBlocker: vi.fn().mockResolvedValue(undefined), +})); + vi.mock('node:fs', async (importOriginal) => { const actual = await importOriginal(); return { @@ -78,6 +86,7 @@ describe('BrowserManager', () => { beforeEach(() => { vi.resetAllMocks(); vi.mocked(injectAutomationOverlay).mockClear(); + vi.mocked(injectInputBlocker).mockClear(); vi.spyOn(coreEvents, 'emitFeedback').mockImplementation(() => {}); // Re-establish consent mock after resetAllMocks @@ -692,21 +701,66 @@ describe('BrowserManager', () => { }); describe('overlay re-injection in callTool', () => { - it('should re-inject overlay after click in non-headless mode', async () => { + it('should re-inject overlay and input blocker after click in non-headless mode when input disabling is enabled', async () => { + // Enable input disabling in config + mockConfig = makeFakeConfig({ + agents: { + overrides: { + browser_agent: { + enabled: true, + }, + }, + browser: { + headless: false, + disableUserInput: true, + }, + }, + }); + const manager = new BrowserManager(mockConfig); await manager.callTool('click', { uid: '1_2' }); expect(injectAutomationOverlay).toHaveBeenCalledWith(manager, undefined); + expect(injectInputBlocker).toHaveBeenCalledWith(manager, undefined); }); - it('should re-inject overlay after navigate_page in non-headless mode', async () => { + it('should re-inject overlay and input blocker after navigate_page in non-headless mode when input disabling is enabled', async () => { + mockConfig = makeFakeConfig({ + agents: { + overrides: { + browser_agent: { + enabled: true, + }, + }, + browser: { + headless: false, + disableUserInput: true, + }, + }, + }); + const manager = new BrowserManager(mockConfig); await manager.callTool('navigate_page', { url: 'https://example.com' }); expect(injectAutomationOverlay).toHaveBeenCalledWith(manager, undefined); + expect(injectInputBlocker).toHaveBeenCalledWith(manager, undefined); }); - it('should re-inject overlay after click_at, new_page, press_key, handle_dialog', async () => { + it('should re-inject overlay and input blocker after click_at, new_page, press_key, handle_dialog when input disabling is enabled', async () => { + mockConfig = makeFakeConfig({ + agents: { + overrides: { + browser_agent: { + enabled: true, + }, + }, + browser: { + headless: false, + disableUserInput: true, + }, + }, + }); + const manager = new BrowserManager(mockConfig); for (const tool of [ 'click_at', @@ -715,12 +769,15 @@ describe('BrowserManager', () => { 'handle_dialog', ]) { vi.mocked(injectAutomationOverlay).mockClear(); + vi.mocked(injectInputBlocker).mockClear(); await manager.callTool(tool, {}); expect(injectAutomationOverlay).toHaveBeenCalledTimes(1); + expect(injectInputBlocker).toHaveBeenCalledTimes(1); + expect(injectInputBlocker).toHaveBeenCalledWith(manager, undefined); } }); - it('should NOT re-inject overlay after read-only tools', async () => { + it('should NOT re-inject overlay or input blocker after read-only tools', async () => { const manager = new BrowserManager(mockConfig); for (const tool of [ 'take_snapshot', @@ -729,8 +786,10 @@ describe('BrowserManager', () => { 'fill', ]) { vi.mocked(injectAutomationOverlay).mockClear(); + vi.mocked(injectInputBlocker).mockClear(); await manager.callTool(tool, {}); expect(injectAutomationOverlay).not.toHaveBeenCalled(); + expect(injectInputBlocker).not.toHaveBeenCalled(); } }); diff --git a/packages/core/src/agents/browser/browserManager.ts b/packages/core/src/agents/browser/browserManager.ts index 4eb9c2b19c..90de6b99fc 100644 --- a/packages/core/src/agents/browser/browserManager.ts +++ b/packages/core/src/agents/browser/browserManager.ts @@ -215,6 +215,10 @@ export class BrowserManager { // Re-inject the automation overlay and input blocker after tools that // can cause a full-page navigation. chrome-devtools-mcp emits no MCP // notifications, so callTool() is the only interception point. + // + // The input blocker injection is idempotent: the injected function + // reuses the existing DOM element when present and only recreates + // it when navigation has actually replaced the page DOM. if ( !result.isError && POTENTIALLY_NAVIGATING_TOOLS.has(toolName) && @@ -224,17 +228,8 @@ export class BrowserManager { if (this.shouldInjectOverlay) { await injectAutomationOverlay(this, signal); } - // Only re-inject the input blocker for tools that *reliably* - // replace the page DOM (navigate_page, new_page, select_page). - // click/click_at are handled by pointer-events suspend/resume - // in mcpToolWrapper — no full re-inject roundtrip needed. - // press_key/handle_dialog only sometimes navigate. - const reliableNavigation = - toolName === 'navigate_page' || - toolName === 'new_page' || - toolName === 'select_page'; - if (this.shouldDisableInput && reliableNavigation) { - await injectInputBlocker(this); + if (this.shouldDisableInput) { + await injectInputBlocker(this, signal); } } catch { // Never let overlay/blocker failures interrupt the tool result diff --git a/packages/core/src/agents/browser/inputBlocker.test.ts b/packages/core/src/agents/browser/inputBlocker.test.ts index 5d77aac079..abccac70c3 100644 --- a/packages/core/src/agents/browser/inputBlocker.test.ts +++ b/packages/core/src/agents/browser/inputBlocker.test.ts @@ -5,7 +5,12 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { injectInputBlocker, removeInputBlocker } from './inputBlocker.js'; +import { + injectInputBlocker, + removeInputBlocker, + suspendInputBlocker, + resumeInputBlocker, +} from './inputBlocker.js'; import type { BrowserManager } from './browserManager.js'; describe('inputBlocker', () => { @@ -28,6 +33,7 @@ describe('inputBlocker', () => { { function: expect.stringContaining('__gemini_input_blocker'), }, + undefined, ); }); @@ -77,6 +83,29 @@ describe('inputBlocker', () => { injectInputBlocker(mockBrowserManager), ).resolves.toBeUndefined(); }); + + it('should be safe to call multiple times (idempotent injection)', async () => { + await injectInputBlocker(mockBrowserManager); + await injectInputBlocker(mockBrowserManager); + + expect(mockBrowserManager.callTool).toHaveBeenCalledTimes(2); + expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith( + 1, + 'evaluate_script', + expect.objectContaining({ + function: expect.stringContaining('__gemini_input_blocker'), + }), + undefined, + ); + expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith( + 2, + 'evaluate_script', + expect.objectContaining({ + function: expect.stringContaining('__gemini_input_blocker'), + }), + undefined, + ); + }); }); describe('removeInputBlocker', () => { @@ -88,6 +117,7 @@ describe('inputBlocker', () => { { function: expect.stringContaining('__gemini_input_blocker'), }, + undefined, ); }); @@ -110,4 +140,38 @@ describe('inputBlocker', () => { ).resolves.toBeUndefined(); }); }); + + describe('suspendInputBlocker and resumeInputBlocker', () => { + it('should not throw when blocker element is missing', async () => { + // Simulate evaluate_script resolving successfully even if the DOM element is absent. + mockBrowserManager.callTool = vi.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'Script ran on page and returned:' }], + }); + + await expect( + suspendInputBlocker(mockBrowserManager), + ).resolves.toBeUndefined(); + await expect( + resumeInputBlocker(mockBrowserManager), + ).resolves.toBeUndefined(); + + expect(mockBrowserManager.callTool).toHaveBeenCalledTimes(2); + expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith( + 1, + 'evaluate_script', + expect.objectContaining({ + function: expect.stringContaining('__gemini_input_blocker'), + }), + undefined, + ); + expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith( + 2, + 'evaluate_script', + expect.objectContaining({ + function: expect.stringContaining('__gemini_input_blocker'), + }), + undefined, + ); + }); + }); }); diff --git a/packages/core/src/agents/browser/inputBlocker.ts b/packages/core/src/agents/browser/inputBlocker.ts index ea6a797271..0d6b9610cf 100644 --- a/packages/core/src/agents/browser/inputBlocker.ts +++ b/packages/core/src/agents/browser/inputBlocker.ts @@ -198,11 +198,14 @@ const RESUME_BLOCKER_FUNCTION = `() => { */ export async function injectInputBlocker( browserManager: BrowserManager, + signal?: AbortSignal, ): Promise { try { - await browserManager.callTool('evaluate_script', { - function: INPUT_BLOCKER_FUNCTION, - }); + await browserManager.callTool( + 'evaluate_script', + { function: INPUT_BLOCKER_FUNCTION }, + signal, + ); debugLogger.log('Input blocker injected successfully'); } catch (error) { // Log but don't throw - input blocker is a UX enhancement, not critical functionality @@ -222,11 +225,14 @@ export async function injectInputBlocker( */ export async function removeInputBlocker( browserManager: BrowserManager, + signal?: AbortSignal, ): Promise { try { - await browserManager.callTool('evaluate_script', { - function: REMOVE_BLOCKER_FUNCTION, - }); + await browserManager.callTool( + 'evaluate_script', + { function: REMOVE_BLOCKER_FUNCTION }, + signal, + ); debugLogger.log('Input blocker removed successfully'); } catch (error) { // Log but don't throw - removal failure is not critical @@ -244,11 +250,14 @@ export async function removeInputBlocker( */ export async function suspendInputBlocker( browserManager: BrowserManager, + signal?: AbortSignal, ): Promise { try { - await browserManager.callTool('evaluate_script', { - function: SUSPEND_BLOCKER_FUNCTION, - }); + await browserManager.callTool( + 'evaluate_script', + { function: SUSPEND_BLOCKER_FUNCTION }, + signal, + ); } catch { // Non-critical — tool call will still attempt to proceed } @@ -260,11 +269,14 @@ export async function suspendInputBlocker( */ export async function resumeInputBlocker( browserManager: BrowserManager, + signal?: AbortSignal, ): Promise { try { - await browserManager.callTool('evaluate_script', { - function: RESUME_BLOCKER_FUNCTION, - }); + await browserManager.callTool( + 'evaluate_script', + { function: RESUME_BLOCKER_FUNCTION }, + signal, + ); } catch { // Non-critical } diff --git a/packages/core/src/agents/browser/mcpToolWrapper.test.ts b/packages/core/src/agents/browser/mcpToolWrapper.test.ts index 3a4d5cfe38..fa9aa228a5 100644 --- a/packages/core/src/agents/browser/mcpToolWrapper.test.ts +++ b/packages/core/src/agents/browser/mcpToolWrapper.test.ts @@ -224,6 +224,7 @@ describe('mcpToolWrapper', () => { expect.objectContaining({ function: expect.stringContaining('__gemini_input_blocker'), }), + expect.any(AbortSignal), ); // Second call: click @@ -241,6 +242,7 @@ describe('mcpToolWrapper', () => { expect.objectContaining({ function: expect.stringContaining('__gemini_input_blocker'), }), + expect.any(AbortSignal), ); }); diff --git a/packages/core/src/agents/browser/mcpToolWrapper.ts b/packages/core/src/agents/browser/mcpToolWrapper.ts index b57a7af7f0..cab493dff7 100644 --- a/packages/core/src/agents/browser/mcpToolWrapper.ts +++ b/packages/core/src/agents/browser/mcpToolWrapper.ts @@ -129,7 +129,7 @@ class McpToolInvocation extends BaseToolInvocation< // chrome-devtools-mcp's interactability checks pass. // Only toggles pointer-events CSS — no DOM change, no flicker. if (this.needsBlockerSuspend) { - await suspendInputBlocker(this.browserManager); + await suspendInputBlocker(this.browserManager, signal); } const result: McpToolCallResult = await this.browserManager.callTool( @@ -155,7 +155,7 @@ class McpToolInvocation extends BaseToolInvocation< // Resume input blocker after interactive tool completes. if (this.needsBlockerSuspend) { - await resumeInputBlocker(this.browserManager); + await resumeInputBlocker(this.browserManager, signal); } if (result.isError) { @@ -181,7 +181,7 @@ class McpToolInvocation extends BaseToolInvocation< // Resume on error path too so the blocker is always restored if (this.needsBlockerSuspend) { - await resumeInputBlocker(this.browserManager).catch(() => {}); + await resumeInputBlocker(this.browserManager, signal).catch(() => {}); } debugLogger.error(`MCP tool ${this.toolName} failed: ${errorMsg}`);