mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-07-09 01:27:41 -07:00
Feat/browser agent metrics (#24210)
Co-authored-by: Gaurav Ghosh <gaghosh@google.com>
This commit is contained in:
@@ -7,6 +7,7 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import {
|
||||
createBrowserAgentDefinition,
|
||||
cleanupBrowserAgent,
|
||||
resetBrowserSession,
|
||||
} from './browserAgentFactory.js';
|
||||
import { injectAutomationOverlay } from './automationOverlay.js';
|
||||
@@ -15,6 +16,12 @@ import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../../policy/types.js';
|
||||
import type { Config } from '../../config/config.js';
|
||||
import type { MessageBus } from '../../confirmation-bus/message-bus.js';
|
||||
import type { PolicyEngine } from '../../policy/policy-engine.js';
|
||||
import type { BrowserManager } from './browserManager.js';
|
||||
import {
|
||||
recordBrowserAgentToolDiscovery,
|
||||
recordBrowserAgentVisionStatus,
|
||||
recordBrowserAgentCleanup,
|
||||
} from '../../telemetry/metrics.js';
|
||||
|
||||
// Create mock browser manager
|
||||
const mockBrowserManager = {
|
||||
@@ -58,6 +65,12 @@ vi.mock('../../utils/debugLogger.js', () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../../telemetry/metrics.js', () => ({
|
||||
recordBrowserAgentToolDiscovery: vi.fn(),
|
||||
recordBrowserAgentVisionStatus: vi.fn(),
|
||||
recordBrowserAgentCleanup: vi.fn(),
|
||||
}));
|
||||
|
||||
import {
|
||||
buildBrowserSystemPrompt,
|
||||
BROWSER_AGENT_NAME,
|
||||
@@ -224,6 +237,11 @@ describe('browserAgentFactory', () => {
|
||||
const systemPrompt = definition.promptConfig?.systemPrompt ?? '';
|
||||
expect(systemPrompt).toContain('analyze_screenshot');
|
||||
expect(systemPrompt).toContain('VISUAL IDENTIFICATION');
|
||||
|
||||
expect(recordBrowserAgentVisionStatus).toHaveBeenCalledWith(
|
||||
configWithVision,
|
||||
{ enabled: true, disabled_reason: undefined },
|
||||
);
|
||||
});
|
||||
|
||||
it('should include analyze_screenshot tool when visualModel is configured', async () => {
|
||||
@@ -314,6 +332,47 @@ describe('browserAgentFactory', () => {
|
||||
// Total: 9 MCP + 1 type_text (no analyze_screenshot without visualModel)
|
||||
expect(definition.toolConfig?.tools).toHaveLength(10);
|
||||
});
|
||||
|
||||
it('should trigger telemetry recording for tool discovery', async () => {
|
||||
const configWithVision = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: { browser_agent: { enabled: true } },
|
||||
browser: { headless: false, visualModel: 'gemini-2.5-flash-preview' },
|
||||
},
|
||||
});
|
||||
|
||||
await createBrowserAgentDefinition(configWithVision, mockMessageBus);
|
||||
|
||||
expect(recordBrowserAgentToolDiscovery).toHaveBeenCalledWith(
|
||||
configWithVision,
|
||||
6, // 6 mock tools from getDiscoveredTools
|
||||
[], // Empty because all required semantic tools present
|
||||
'persistent',
|
||||
);
|
||||
});
|
||||
|
||||
it('should trigger telemetry recording for missing semantic tools', async () => {
|
||||
mockBrowserManager.getDiscoveredTools.mockResolvedValueOnce([
|
||||
{ name: 'take_snapshot', description: 'Take snapshot' },
|
||||
// 'click', 'fill', 'navigate_page' are missing
|
||||
]);
|
||||
|
||||
const configWithVision = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: { browser_agent: { enabled: true } },
|
||||
browser: { headless: false, visualModel: 'gemini-2.5-flash-preview' },
|
||||
},
|
||||
});
|
||||
|
||||
await createBrowserAgentDefinition(configWithVision, mockMessageBus);
|
||||
|
||||
expect(recordBrowserAgentToolDiscovery).toHaveBeenCalledWith(
|
||||
configWithVision,
|
||||
1, // 1 mock tool from getDiscoveredTools
|
||||
['click', 'fill', 'navigate_page'],
|
||||
'persistent',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resetBrowserSession', () => {
|
||||
@@ -452,6 +511,72 @@ describe('browserAgentFactory', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('cleanupBrowserAgent', () => {
|
||||
it('should call close on browser manager', async () => {
|
||||
const mockConfig = makeFakeConfig({});
|
||||
await cleanupBrowserAgent(
|
||||
mockBrowserManager as unknown as BrowserManager,
|
||||
mockConfig,
|
||||
'persistent',
|
||||
);
|
||||
|
||||
expect(mockBrowserManager.close).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle errors during cleanup gracefully', async () => {
|
||||
const errorManager = {
|
||||
close: vi.fn().mockRejectedValue(new Error('Close failed')),
|
||||
} as unknown as BrowserManager;
|
||||
const mockConfig = makeFakeConfig({});
|
||||
|
||||
// Should not throw
|
||||
await expect(
|
||||
cleanupBrowserAgent(errorManager, mockConfig, 'persistent'),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('should record successful cleanup metrics', async () => {
|
||||
const mockConfig = makeFakeConfig({});
|
||||
await cleanupBrowserAgent(
|
||||
mockBrowserManager as unknown as BrowserManager,
|
||||
mockConfig,
|
||||
'isolated',
|
||||
);
|
||||
|
||||
expect(mockBrowserManager.close).toHaveBeenCalled();
|
||||
expect(recordBrowserAgentCleanup).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'isolated',
|
||||
success: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should record failed cleanup metrics when browserManager.close() throws', async () => {
|
||||
const mockConfig = makeFakeConfig({});
|
||||
mockBrowserManager.close.mockRejectedValueOnce(
|
||||
new Error('Failed to close'),
|
||||
);
|
||||
|
||||
await cleanupBrowserAgent(
|
||||
mockBrowserManager as unknown as BrowserManager,
|
||||
mockConfig,
|
||||
'existing',
|
||||
);
|
||||
|
||||
expect(recordBrowserAgentCleanup).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'existing',
|
||||
success: false,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildBrowserSystemPrompt', () => {
|
||||
|
||||
@@ -32,12 +32,27 @@ import { createAnalyzeScreenshotTool } from './analyzeScreenshot.js';
|
||||
import { injectAutomationOverlay } from './automationOverlay.js';
|
||||
import { injectInputBlocker } from './inputBlocker.js';
|
||||
import { debugLogger } from '../../utils/debugLogger.js';
|
||||
import {
|
||||
recordBrowserAgentToolDiscovery,
|
||||
recordBrowserAgentVisionStatus,
|
||||
recordBrowserAgentCleanup,
|
||||
} from '../../telemetry/metrics.js';
|
||||
import {
|
||||
PolicyDecision,
|
||||
PRIORITY_SUBAGENT_TOOL,
|
||||
type PolicyRule,
|
||||
} from '../../policy/types.js';
|
||||
|
||||
/**
|
||||
* Structured return type for vision disabled reasons.
|
||||
* Separates the condition code from the human-readable message.
|
||||
*/
|
||||
type VisionDisabledReason =
|
||||
| { code: 'no_visual_model'; message: string }
|
||||
| { code: 'missing_visual_tools'; message: string }
|
||||
| { code: 'blocked_auth_type'; message: string }
|
||||
| undefined;
|
||||
|
||||
/**
|
||||
* Creates a browser agent definition with MCP tools configured.
|
||||
*
|
||||
@@ -57,6 +72,8 @@ export async function createBrowserAgentDefinition(
|
||||
): Promise<{
|
||||
definition: LocalAgentDefinition<typeof BrowserTaskResultSchema>;
|
||||
browserManager: BrowserManager;
|
||||
visionEnabled: boolean;
|
||||
sessionMode: 'persistent' | 'isolated' | 'existing';
|
||||
}> {
|
||||
debugLogger.log(
|
||||
'Creating browser agent definition with isolated MCP tools...',
|
||||
@@ -169,6 +186,20 @@ export async function createBrowserAgentDefinition(
|
||||
const missingSemanticTools = requiredSemanticTools.filter(
|
||||
(t) => !availableToolNames.includes(t),
|
||||
);
|
||||
|
||||
const rawSessionMode = browserConfig?.customConfig?.sessionMode;
|
||||
const sessionMode =
|
||||
rawSessionMode === 'isolated' || rawSessionMode === 'existing'
|
||||
? rawSessionMode
|
||||
: 'persistent';
|
||||
|
||||
recordBrowserAgentToolDiscovery(
|
||||
config,
|
||||
mcpTools.length,
|
||||
missingSemanticTools,
|
||||
sessionMode,
|
||||
);
|
||||
|
||||
if (missingSemanticTools.length > 0) {
|
||||
debugLogger.warn(
|
||||
`Semantic tools missing (${missingSemanticTools.join(', ')}). ` +
|
||||
@@ -182,17 +213,22 @@ export async function createBrowserAgentDefinition(
|
||||
(t) => !availableToolNames.includes(t),
|
||||
);
|
||||
|
||||
// Check whether vision can be enabled; returns undefined if all gates pass.
|
||||
function getVisionDisabledReason(): string | undefined {
|
||||
// Check whether vision can be enabled; returns structured type with code and message.
|
||||
function getVisionDisabledReason(): VisionDisabledReason {
|
||||
const browserConfig = config.getBrowserAgentConfig();
|
||||
if (!browserConfig.customConfig.visualModel) {
|
||||
return 'No visualModel configured.';
|
||||
return {
|
||||
code: 'no_visual_model',
|
||||
message: 'No visualModel configured.',
|
||||
};
|
||||
}
|
||||
if (missingVisualTools.length > 0) {
|
||||
return (
|
||||
`Visual tools missing (${missingVisualTools.join(', ')}). ` +
|
||||
`The installed chrome-devtools-mcp version may be too old.`
|
||||
);
|
||||
return {
|
||||
code: 'missing_visual_tools',
|
||||
message:
|
||||
`Visual tools missing (${missingVisualTools.join(', ')}). ` +
|
||||
`The installed chrome-devtools-mcp version may be too old.`,
|
||||
};
|
||||
}
|
||||
const authType = config.getContentGeneratorConfig()?.authType;
|
||||
const blockedAuthTypes = new Set([
|
||||
@@ -201,7 +237,10 @@ export async function createBrowserAgentDefinition(
|
||||
AuthType.COMPUTE_ADC,
|
||||
]);
|
||||
if (authType && blockedAuthTypes.has(authType)) {
|
||||
return 'Visual agent model not available for current auth type.';
|
||||
return {
|
||||
code: 'blocked_auth_type',
|
||||
message: 'Visual agent model not available for current auth type.',
|
||||
};
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
@@ -209,8 +248,13 @@ export async function createBrowserAgentDefinition(
|
||||
const allTools: AnyDeclarativeTool[] = [...mcpTools];
|
||||
const visionDisabledReason = getVisionDisabledReason();
|
||||
|
||||
recordBrowserAgentVisionStatus(config, {
|
||||
enabled: !visionDisabledReason,
|
||||
disabled_reason: visionDisabledReason?.code,
|
||||
});
|
||||
|
||||
if (visionDisabledReason) {
|
||||
debugLogger.log(`Vision disabled: ${visionDisabledReason}`);
|
||||
debugLogger.log(`Vision disabled: ${visionDisabledReason.message}`);
|
||||
} else {
|
||||
allTools.push(
|
||||
createAnalyzeScreenshotTool(browserManager, config, messageBus),
|
||||
@@ -232,12 +276,46 @@ export async function createBrowserAgentDefinition(
|
||||
},
|
||||
};
|
||||
|
||||
return { definition, browserManager };
|
||||
return {
|
||||
definition,
|
||||
browserManager,
|
||||
visionEnabled: !visionDisabledReason,
|
||||
sessionMode,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Closes all persistent browser sessions and cleans up resources.
|
||||
*
|
||||
* @param browserManager The browser manager to clean up
|
||||
* @param config Runtime configuration
|
||||
* @param sessionMode The browser session mode
|
||||
*/
|
||||
export async function cleanupBrowserAgent(
|
||||
browserManager: BrowserManager,
|
||||
config: Config,
|
||||
sessionMode: 'persistent' | 'isolated' | 'existing',
|
||||
): Promise<void> {
|
||||
const startMs = Date.now();
|
||||
try {
|
||||
await browserManager.close();
|
||||
recordBrowserAgentCleanup(config, Date.now() - startMs, {
|
||||
session_mode: sessionMode,
|
||||
success: true,
|
||||
});
|
||||
debugLogger.log('Browser agent cleanup complete');
|
||||
} catch (error) {
|
||||
recordBrowserAgentCleanup(config, Date.now() - startMs, {
|
||||
session_mode: sessionMode,
|
||||
success: false,
|
||||
});
|
||||
debugLogger.error(
|
||||
`Error during browser cleanup: ${error instanceof Error ? error.message : String(error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Call this on /clear commands and CLI exit to reset browser state.
|
||||
*/
|
||||
export async function resetBrowserSession(): Promise<void> {
|
||||
|
||||
@@ -26,6 +26,7 @@ vi.mock('../../utils/debugLogger.js', () => ({
|
||||
|
||||
vi.mock('./browserAgentFactory.js', () => ({
|
||||
createBrowserAgentDefinition: vi.fn(),
|
||||
cleanupBrowserAgent: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('./inputBlocker.js', () => ({
|
||||
@@ -36,16 +37,24 @@ vi.mock('./automationOverlay.js', () => ({
|
||||
removeAutomationOverlay: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../../telemetry/metrics.js', () => ({
|
||||
recordBrowserAgentTaskOutcome: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../local-executor.js', () => ({
|
||||
LocalAgentExecutor: {
|
||||
create: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import { createBrowserAgentDefinition } from './browserAgentFactory.js';
|
||||
import {
|
||||
createBrowserAgentDefinition,
|
||||
cleanupBrowserAgent,
|
||||
} from './browserAgentFactory.js';
|
||||
import { removeInputBlocker } from './inputBlocker.js';
|
||||
import { removeAutomationOverlay } from './automationOverlay.js';
|
||||
import { LocalAgentExecutor } from '../local-executor.js';
|
||||
import { recordBrowserAgentTaskOutcome } from '../../telemetry/metrics.js';
|
||||
import type { ToolLiveOutput } from '../../tools/tools.js';
|
||||
|
||||
describe('BrowserAgentInvocation', () => {
|
||||
@@ -184,6 +193,8 @@ describe('BrowserAgentInvocation', () => {
|
||||
toolConfig: { tools: ['analyze_screenshot', 'click'] },
|
||||
},
|
||||
browserManager: {} as never,
|
||||
visionEnabled: true,
|
||||
sessionMode: 'persistent',
|
||||
});
|
||||
|
||||
mockExecutor = {
|
||||
@@ -669,11 +680,12 @@ describe('BrowserAgentInvocation', () => {
|
||||
.map((c) => c[0] as SubagentProgress)
|
||||
.filter((p) => p.isSubagentProgress);
|
||||
|
||||
const allItems = progressCalls.flatMap((p) => p.recentActivity);
|
||||
const toolA = allItems.find(
|
||||
const finalActivity =
|
||||
progressCalls[progressCalls.length - 1].recentActivity;
|
||||
const toolA = finalActivity.find(
|
||||
(a) => a.type === 'tool_call' && a.content === 'tool_a',
|
||||
);
|
||||
const toolB = allItems.find(
|
||||
const toolB = finalActivity.find(
|
||||
(a) => a.type === 'tool_call' && a.content === 'tool_b',
|
||||
);
|
||||
|
||||
@@ -681,6 +693,69 @@ describe('BrowserAgentInvocation', () => {
|
||||
expect(toolA?.status).toBe('error');
|
||||
expect(toolB?.status).toBe('error');
|
||||
});
|
||||
|
||||
it('should record successful task outcome metrics', async () => {
|
||||
const invocation = new BrowserAgentInvocation(
|
||||
mockConfig,
|
||||
mockParams,
|
||||
mockMessageBus,
|
||||
);
|
||||
await invocation.execute(new AbortController().signal, vi.fn());
|
||||
|
||||
expect(recordBrowserAgentTaskOutcome).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.objectContaining({
|
||||
success: true,
|
||||
session_mode: 'persistent',
|
||||
vision_enabled: true,
|
||||
headless: false,
|
||||
duration_ms: expect.any(Number),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should record failed task outcome metrics', async () => {
|
||||
vi.mocked(LocalAgentExecutor.create).mockResolvedValue({
|
||||
run: vi.fn().mockResolvedValue({
|
||||
result: JSON.stringify({ success: false, foo: 'bar' }),
|
||||
}),
|
||||
} as never);
|
||||
|
||||
const updateOutput = vi.fn();
|
||||
const invocation = new BrowserAgentInvocation(
|
||||
mockConfig,
|
||||
mockParams,
|
||||
mockMessageBus,
|
||||
);
|
||||
|
||||
await invocation.execute(new AbortController().signal, updateOutput);
|
||||
|
||||
expect(recordBrowserAgentTaskOutcome).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.objectContaining({
|
||||
success: false,
|
||||
session_mode: 'persistent',
|
||||
vision_enabled: true,
|
||||
headless: false,
|
||||
duration_ms: expect.any(Number),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should call cleanupBrowserAgent with correct params', async () => {
|
||||
const invocation = new BrowserAgentInvocation(
|
||||
mockConfig,
|
||||
mockParams,
|
||||
mockMessageBus,
|
||||
);
|
||||
await invocation.execute(new AbortController().signal, vi.fn());
|
||||
|
||||
expect(cleanupBrowserAgent).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
mockConfig,
|
||||
'persistent',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('cleanup', () => {
|
||||
@@ -711,6 +786,8 @@ describe('BrowserAgentInvocation', () => {
|
||||
toolConfig: { tools: [] },
|
||||
},
|
||||
browserManager: mockBrowserManager as never,
|
||||
visionEnabled: true,
|
||||
sessionMode: 'persistent',
|
||||
});
|
||||
|
||||
const mockExecutor = {
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
*/
|
||||
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { debugLogger } from '../../utils/debugLogger.js';
|
||||
import type { Config } from '../../config/config.js';
|
||||
import { type AgentLoopContext } from '../../config/agent-loop-context.js';
|
||||
import { LocalAgentExecutor } from '../local-executor.js';
|
||||
@@ -33,8 +34,12 @@ import {
|
||||
isToolActivityError,
|
||||
} from '../types.js';
|
||||
import type { MessageBus } from '../../confirmation-bus/message-bus.js';
|
||||
import { createBrowserAgentDefinition } from './browserAgentFactory.js';
|
||||
import {
|
||||
createBrowserAgentDefinition,
|
||||
cleanupBrowserAgent,
|
||||
} from './browserAgentFactory.js';
|
||||
import { removeInputBlocker } from './inputBlocker.js';
|
||||
import { recordBrowserAgentTaskOutcome } from '../../telemetry/metrics.js';
|
||||
import {
|
||||
sanitizeThoughtContent,
|
||||
sanitizeToolArgs,
|
||||
@@ -109,8 +114,12 @@ export class BrowserAgentInvocation extends BaseToolInvocation<
|
||||
signal: AbortSignal,
|
||||
updateOutput?: (output: ToolLiveOutput) => void,
|
||||
): Promise<ToolResult> {
|
||||
const invocationStartMs = Date.now();
|
||||
let browserManager;
|
||||
let recentActivity: SubagentActivityItem[] = [];
|
||||
let sessionMode: 'persistent' | 'isolated' | 'existing' = 'persistent';
|
||||
let visionEnabled = false;
|
||||
let taskSuccess = false;
|
||||
|
||||
try {
|
||||
if (updateOutput) {
|
||||
@@ -154,6 +163,8 @@ export class BrowserAgentInvocation extends BaseToolInvocation<
|
||||
);
|
||||
const { definition } = result;
|
||||
browserManager = result.browserManager;
|
||||
visionEnabled = result.visionEnabled;
|
||||
sessionMode = result.sessionMode;
|
||||
|
||||
// Create activity callback for streaming output
|
||||
const onActivity = (activity: SubagentActivityEvent): void => {
|
||||
@@ -302,6 +313,19 @@ export class BrowserAgentInvocation extends BaseToolInvocation<
|
||||
|
||||
const output = await executor.run(this.params, signal);
|
||||
|
||||
try {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
const parsed = JSON.parse(output.result);
|
||||
|
||||
taskSuccess = parsed?.success === true;
|
||||
} catch (parseError) {
|
||||
// non-JSON result -> treat as unknown, default false
|
||||
debugLogger.log(
|
||||
'Failed to parse browser agent output as JSON:',
|
||||
parseError,
|
||||
);
|
||||
}
|
||||
|
||||
const resultContent = `Browser agent finished.
|
||||
Termination Reason: ${output.terminate_reason}
|
||||
Result:
|
||||
@@ -376,6 +400,14 @@ ${output.result}`;
|
||||
},
|
||||
};
|
||||
} finally {
|
||||
recordBrowserAgentTaskOutcome(this.config, {
|
||||
success: taskSuccess,
|
||||
session_mode: sessionMode,
|
||||
vision_enabled: visionEnabled,
|
||||
headless: !!this.config.getBrowserAgentConfig().customConfig.headless,
|
||||
duration_ms: Date.now() - invocationStartMs,
|
||||
});
|
||||
|
||||
// Clean up input blocker, but keep browserManager alive for persistent sessions
|
||||
if (browserManager) {
|
||||
await removeInputBlocker(browserManager, signal);
|
||||
@@ -412,6 +444,7 @@ ${output.result}`;
|
||||
} catch {
|
||||
// Ignore errors for removing the overlays.
|
||||
}
|
||||
await cleanupBrowserAgent(browserManager, this.config, sessionMode);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,6 +46,10 @@ vi.mock('../../utils/debugLogger.js', () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../../telemetry/metrics.js', () => ({
|
||||
recordBrowserAgentConnection: vi.fn(),
|
||||
}));
|
||||
|
||||
// Mock browser consent to always grant consent by default
|
||||
vi.mock('../../utils/browserConsent.js', () => ({
|
||||
getBrowserConsentIfNeeded: vi.fn().mockResolvedValue(true),
|
||||
@@ -78,6 +82,7 @@ vi.mock('node:fs', async (importOriginal) => {
|
||||
import * as fs from 'node:fs';
|
||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
|
||||
import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js';
|
||||
import { recordBrowserAgentConnection } from '../../telemetry/metrics.js';
|
||||
import { getBrowserConsentIfNeeded } from '../../utils/browserConsent.js';
|
||||
import { debugLogger } from '../../utils/debugLogger.js';
|
||||
|
||||
@@ -355,6 +360,21 @@ describe('BrowserManager', () => {
|
||||
});
|
||||
|
||||
describe('MCP connection', () => {
|
||||
it('should record connection success metrics', async () => {
|
||||
const manager = new BrowserManager(mockConfig);
|
||||
await manager.ensureConnection();
|
||||
|
||||
expect(recordBrowserAgentConnection).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'persistent',
|
||||
headless: false,
|
||||
success: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should spawn npx chrome-devtools-mcp with --experimental-vision (persistent mode by default)', async () => {
|
||||
const manager = new BrowserManager(mockConfig);
|
||||
await manager.ensureConnection();
|
||||
@@ -546,6 +566,18 @@ describe('BrowserManager', () => {
|
||||
await expect(manager.ensureConnection()).rejects.toThrow(
|
||||
/Failed to connect to existing Chrome instance/,
|
||||
);
|
||||
|
||||
expect(recordBrowserAgentConnection).toHaveBeenCalledWith(
|
||||
existingConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'existing',
|
||||
headless: false,
|
||||
success: false,
|
||||
error_type: 'connection_refused',
|
||||
},
|
||||
);
|
||||
|
||||
// Create a fresh manager to verify the error message includes remediation steps
|
||||
const manager2 = new BrowserManager(existingConfig);
|
||||
await expect(manager2.ensureConnection()).rejects.toThrow(
|
||||
@@ -576,6 +608,18 @@ describe('BrowserManager', () => {
|
||||
await expect(manager.ensureConnection()).rejects.toThrow(
|
||||
/Close all Chrome windows using this profile/,
|
||||
);
|
||||
|
||||
expect(recordBrowserAgentConnection).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'persistent',
|
||||
headless: false,
|
||||
success: false,
|
||||
error_type: 'profile_locked',
|
||||
},
|
||||
);
|
||||
|
||||
const manager2 = new BrowserManager(mockConfig);
|
||||
await expect(manager2.ensureConnection()).rejects.toThrow(
|
||||
/Set sessionMode to "isolated"/,
|
||||
@@ -602,6 +646,17 @@ describe('BrowserManager', () => {
|
||||
await expect(manager.ensureConnection()).rejects.toThrow(
|
||||
/Chrome is not installed/,
|
||||
);
|
||||
|
||||
expect(recordBrowserAgentConnection).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'persistent',
|
||||
headless: false,
|
||||
success: false,
|
||||
error_type: 'timeout',
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should include sessionMode in generic fallback error', async () => {
|
||||
@@ -622,6 +677,61 @@ describe('BrowserManager', () => {
|
||||
await expect(manager.ensureConnection()).rejects.toThrow(
|
||||
/sessionMode: persistent/,
|
||||
);
|
||||
|
||||
expect(recordBrowserAgentConnection).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'persistent',
|
||||
headless: false,
|
||||
success: false,
|
||||
error_type: 'unknown',
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should classify non-connection-refused errors in existing mode as unknown', async () => {
|
||||
vi.mocked(Client).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
connect: vi
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('Some unexpected error')),
|
||||
close: vi.fn().mockResolvedValue(undefined),
|
||||
listTools: vi.fn(),
|
||||
callTool: vi.fn(),
|
||||
}) as unknown as InstanceType<typeof Client>,
|
||||
);
|
||||
|
||||
const existingConfig = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: {
|
||||
browser_agent: {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
browser: {
|
||||
sessionMode: 'existing',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const manager = new BrowserManager(existingConfig);
|
||||
|
||||
await expect(manager.ensureConnection()).rejects.toThrow(
|
||||
/Failed to connect to existing Chrome instance/,
|
||||
);
|
||||
|
||||
expect(recordBrowserAgentConnection).toHaveBeenCalledWith(
|
||||
existingConfig,
|
||||
expect.any(Number),
|
||||
{
|
||||
session_mode: 'existing',
|
||||
headless: false,
|
||||
success: false,
|
||||
error_type: 'unknown',
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should pass --no-usage-statistics and --no-performance-crux when privacy is disabled', async () => {
|
||||
|
||||
@@ -30,6 +30,7 @@ import * as path from 'node:path';
|
||||
import * as fs from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { injectAutomationOverlay } from './automationOverlay.js';
|
||||
import { recordBrowserAgentConnection } from '../../telemetry/metrics.js';
|
||||
|
||||
const __filename = fileURLToPath(import.meta.url);
|
||||
const __dirname = path.dirname(__filename);
|
||||
@@ -486,7 +487,11 @@ export class BrowserManager {
|
||||
|
||||
// Build args for chrome-devtools-mcp
|
||||
const browserConfig = this.config.getBrowserAgentConfig();
|
||||
let sessionMode = browserConfig.customConfig.sessionMode ?? 'persistent';
|
||||
const rawSessionMode = browserConfig.customConfig.sessionMode;
|
||||
let sessionMode: 'persistent' | 'isolated' | 'existing' =
|
||||
rawSessionMode === 'isolated' || rawSessionMode === 'existing'
|
||||
? rawSessionMode
|
||||
: 'persistent';
|
||||
|
||||
// Detect sandbox environment.
|
||||
// SANDBOX env var is set to 'sandbox-exec' (seatbelt) or the container
|
||||
@@ -652,6 +657,7 @@ export class BrowserManager {
|
||||
sessionMode === 'existing' ? 15_000 : MCP_TIMEOUT_MS;
|
||||
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
const connectStartMs = Date.now();
|
||||
try {
|
||||
await Promise.race([
|
||||
(async () => {
|
||||
@@ -660,6 +666,16 @@ export class BrowserManager {
|
||||
await this.discoverTools();
|
||||
// clear the action counter for each connection
|
||||
this.actionCounter = 0;
|
||||
|
||||
recordBrowserAgentConnection(
|
||||
this.config,
|
||||
Date.now() - connectStartMs,
|
||||
{
|
||||
session_mode: sessionMode,
|
||||
headless: !!browserConfig.customConfig.headless,
|
||||
success: true,
|
||||
},
|
||||
);
|
||||
})(),
|
||||
new Promise<never>((_, reject) => {
|
||||
timeoutId = setTimeout(
|
||||
@@ -676,11 +692,19 @@ export class BrowserManager {
|
||||
} catch (error) {
|
||||
await this.close();
|
||||
|
||||
const rawErrorMessage =
|
||||
error instanceof Error ? error.message : String(error);
|
||||
const errorType = BrowserManager.classifyConnectionError(rawErrorMessage);
|
||||
|
||||
recordBrowserAgentConnection(this.config, Date.now() - connectStartMs, {
|
||||
session_mode: sessionMode,
|
||||
headless: !!browserConfig.customConfig.headless,
|
||||
success: false,
|
||||
error_type: errorType,
|
||||
});
|
||||
|
||||
// Provide error-specific, session-mode-aware remediation
|
||||
throw this.createConnectionError(
|
||||
error instanceof Error ? error.message : String(error),
|
||||
sessionMode,
|
||||
);
|
||||
throw this.createConnectionError(rawErrorMessage, sessionMode);
|
||||
} finally {
|
||||
if (timeoutId !== undefined) {
|
||||
clearTimeout(timeoutId);
|
||||
@@ -688,15 +712,34 @@ export class BrowserManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Classifies a connection error message into a known error type.
|
||||
* Shared between connectMcp error recording and createConnectionError
|
||||
* to ensure consistent error categorization across the browser agent.
|
||||
*/
|
||||
private static classifyConnectionError(
|
||||
message: string,
|
||||
): 'profile_locked' | 'timeout' | 'connection_refused' | 'unknown' {
|
||||
const lowerMessage = message.toLowerCase();
|
||||
if (lowerMessage.includes('already running')) {
|
||||
return 'profile_locked';
|
||||
} else if (lowerMessage.includes('timed out')) {
|
||||
return 'timeout';
|
||||
} else if (lowerMessage.includes('connection refused')) {
|
||||
return 'connection_refused';
|
||||
}
|
||||
return 'unknown';
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates an Error with context-specific remediation based on the actual
|
||||
* error message and the current sessionMode.
|
||||
*/
|
||||
private createConnectionError(message: string, sessionMode: string): Error {
|
||||
const lowerMessage = message.toLowerCase();
|
||||
const errorType = BrowserManager.classifyConnectionError(message);
|
||||
|
||||
// "already running for the current profile" — persistent mode profile lock
|
||||
if (lowerMessage.includes('already running')) {
|
||||
if (errorType === 'profile_locked') {
|
||||
if (sessionMode === 'persistent' || sessionMode === 'isolated') {
|
||||
return new Error(
|
||||
`Could not connect to Chrome: ${message}\n\n` +
|
||||
@@ -716,7 +759,7 @@ export class BrowserManager {
|
||||
}
|
||||
|
||||
// Timeout errors
|
||||
if (lowerMessage.includes('timed out')) {
|
||||
if (errorType === 'timeout') {
|
||||
if (sessionMode === 'existing') {
|
||||
return new Error(
|
||||
`Timed out connecting to Chrome: ${message}\n\n` +
|
||||
|
||||
Reference in New Issue
Block a user