fix(browser): reset action counter for each agent session and let it ignore internal actions (#24228)

This commit is contained in:
cynthialong0-0
2026-03-31 08:23:19 -07:00
committed by GitHub
parent e293424bb4
commit 3982a252bb
6 changed files with 51 additions and 7 deletions
@@ -94,6 +94,7 @@ export async function injectAutomationOverlay(
'evaluate_script', 'evaluate_script',
{ function: buildInjectionScript() }, { function: buildInjectionScript() },
signal, signal,
true,
); );
if (result.isError) { if (result.isError) {
@@ -120,6 +121,7 @@ export async function removeAutomationOverlay(
'evaluate_script', 'evaluate_script',
{ function: buildRemovalScript() }, { function: buildRemovalScript() },
signal, signal,
true,
); );
if (result.isError) { if (result.isError) {
@@ -980,5 +980,29 @@ describe('BrowserManager', () => {
/maximum action limit \(3\)/, /maximum action limit \(3\)/,
); );
}); });
it('should NOT increment action counter when shouldCount is false', async () => {
const limitedConfig = makeFakeConfig({
agents: {
browser: {
maxActionsPerTask: 1,
},
},
});
const manager = new BrowserManager(limitedConfig);
// Multiple calls with isInternal: true should NOT exhaust the limit
await manager.callTool('evaluate_script', {}, undefined, true);
await manager.callTool('evaluate_script', {}, undefined, true);
await manager.callTool('evaluate_script', {}, undefined, true);
// This should still work
await manager.callTool('take_snapshot', {});
// Next one should throw (limit 1 allows exactly 1 call with >= check)
await expect(manager.callTool('take_snapshot', {})).rejects.toThrow(
/maximum action limit \(1\)/,
);
});
}); });
}); });
@@ -215,26 +215,30 @@ export class BrowserManager {
* @param toolName The name of the tool to call * @param toolName The name of the tool to call
* @param args Arguments to pass to the tool * @param args Arguments to pass to the tool
* @param signal Optional AbortSignal to cancel the call * @param signal Optional AbortSignal to cancel the call
* @param isInternal Determine if the tool is for internal execution
* @returns The result from the MCP server * @returns The result from the MCP server
*/ */
async callTool( async callTool(
toolName: string, toolName: string,
args: Record<string, unknown>, args: Record<string, unknown>,
signal?: AbortSignal, signal?: AbortSignal,
isInternal: boolean = false,
): Promise<McpToolCallResult> { ): Promise<McpToolCallResult> {
if (signal?.aborted) { if (signal?.aborted) {
throw signal.reason ?? new Error('Operation cancelled'); throw signal.reason ?? new Error('Operation cancelled');
} }
// Hard enforcement of per-action rate limit // Hard enforcement of per-action rate limit
if (this.actionCounter > this.maxActionsPerTask) { if (!isInternal) {
const error = new Error( if (this.actionCounter >= this.maxActionsPerTask) {
`Browser agent reached maximum action limit (${this.maxActionsPerTask}). ` + const error = new Error(
`Task terminated to prevent runaway execution. To config the limit, use maxActionsPerTask in the settings.`, `Browser agent reached maximum action limit (${this.maxActionsPerTask}). ` +
); `Task terminated to prevent runaway execution. To config the limit, use maxActionsPerTask in the settings.`,
throw error; );
throw error;
}
this.actionCounter++;
} }
this.actionCounter++;
const errorMessage = this.checkNavigationRestrictions(toolName, args); const errorMessage = this.checkNavigationRestrictions(toolName, args);
if (errorMessage) { if (errorMessage) {
@@ -588,6 +592,8 @@ export class BrowserManager {
debugLogger.log('MCP client connected to chrome-devtools-mcp'); debugLogger.log('MCP client connected to chrome-devtools-mcp');
await this.discoverTools(); await this.discoverTools();
this.registerInputBlockerHandler(); this.registerInputBlockerHandler();
// clear the action counter for each connection
this.actionCounter = 0;
})(), })(),
new Promise<never>((_, reject) => { new Promise<never>((_, reject) => {
timeoutId = setTimeout( timeoutId = setTimeout(
@@ -34,6 +34,7 @@ describe('inputBlocker', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}, },
undefined, undefined,
true,
); );
}); });
@@ -96,6 +97,7 @@ describe('inputBlocker', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}), }),
undefined, undefined,
true,
); );
expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith( expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith(
2, 2,
@@ -104,6 +106,7 @@ describe('inputBlocker', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}), }),
undefined, undefined,
true,
); );
}); });
}); });
@@ -118,6 +121,7 @@ describe('inputBlocker', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}, },
undefined, undefined,
true,
); );
}); });
@@ -163,6 +167,7 @@ describe('inputBlocker', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}), }),
undefined, undefined,
true,
); );
expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith( expect(mockBrowserManager.callTool).toHaveBeenNthCalledWith(
2, 2,
@@ -171,6 +176,7 @@ describe('inputBlocker', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}), }),
undefined, undefined,
true,
); );
}); });
}); });
@@ -205,6 +205,7 @@ export async function injectInputBlocker(
'evaluate_script', 'evaluate_script',
{ function: INPUT_BLOCKER_FUNCTION }, { function: INPUT_BLOCKER_FUNCTION },
signal, signal,
true,
); );
debugLogger.log('Input blocker injected successfully'); debugLogger.log('Input blocker injected successfully');
} catch (error) { } catch (error) {
@@ -232,6 +233,7 @@ export async function removeInputBlocker(
'evaluate_script', 'evaluate_script',
{ function: REMOVE_BLOCKER_FUNCTION }, { function: REMOVE_BLOCKER_FUNCTION },
signal, signal,
true,
); );
debugLogger.log('Input blocker removed successfully'); debugLogger.log('Input blocker removed successfully');
} catch (error) { } catch (error) {
@@ -257,6 +259,7 @@ export async function suspendInputBlocker(
'evaluate_script', 'evaluate_script',
{ function: SUSPEND_BLOCKER_FUNCTION }, { function: SUSPEND_BLOCKER_FUNCTION },
signal, signal,
true,
); );
} catch { } catch {
// Non-critical — tool call will still attempt to proceed // Non-critical — tool call will still attempt to proceed
@@ -276,6 +279,7 @@ export async function resumeInputBlocker(
'evaluate_script', 'evaluate_script',
{ function: RESUME_BLOCKER_FUNCTION }, { function: RESUME_BLOCKER_FUNCTION },
signal, signal,
true,
); );
} catch { } catch {
// Non-critical // Non-critical
@@ -225,6 +225,7 @@ describe('mcpToolWrapper', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}), }),
expect.any(AbortSignal), expect.any(AbortSignal),
true,
); );
// Second call: click // Second call: click
@@ -243,6 +244,7 @@ describe('mcpToolWrapper', () => {
function: expect.stringContaining('__gemini_input_blocker'), function: expect.stringContaining('__gemini_input_blocker'),
}), }),
expect.any(AbortSignal), expect.any(AbortSignal),
true,
); );
}); });