Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/McpContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import {
} from './third_party/index.js';
import {listPages} from './tools/pages.js';
import {CLOSE_PAGE_ERROR} from './tools/ToolDefinition.js';
import type {Context, SupportedExtensions} from './tools/ToolDefinition.js';
import type {
Context,
DevToolsData,
SupportedExtensions,
} from './tools/ToolDefinition.js';
import type {TraceResult} from './trace-processing/parse.js';
import type {Logger} from './types.js';
import type {ExtensionServiceWorker} from './types.js';
Expand Down Expand Up @@ -373,6 +377,25 @@ export class McpContext implements Context {
return page;
}

async getDevToolsData(page?: McpPage): Promise<DevToolsData | undefined> {
const targetPage = page ?? this.#selectedPage;
if (!targetPage) {
return undefined;
}
let timeoutId: ReturnType<typeof setTimeout>;
const timeoutPromise = new Promise<undefined>(resolve => {
timeoutId = setTimeout(() => resolve(undefined), 500);
});
const dataPromise = targetPage.getDevToolsData();
try {
return await Promise.race([dataPromise, timeoutPromise]);
} catch {
return undefined;
} finally {
clearTimeout(timeoutId!);
}
}

/**
* Returns true once if this context was created by reconnecting after the
* previous browser connection was lost, so the next response can surface a
Expand Down
16 changes: 14 additions & 2 deletions src/ToolHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import type {parseArguments} from './bin/chrome-devtools-mcp-cli-options.js';
import type {McpContext} from './McpContext.js';
import type {McpPage} from './McpPage.js';
import type {DataFormat} from './McpResponse.js';
import {McpResponse} from './McpResponse.js';
import {SlimMcpResponse} from './SlimMcpResponse.js';
Expand All @@ -15,7 +16,11 @@ import type {CallToolResult} from './third_party/index.js';
import {zod} from './third_party/index.js';
import type {ToolCategory} from './tools/categories.js';
import {labels, OFF_BY_DEFAULT_CATEGORIES} from './tools/categories.js';
import type {DefinedPageTool, ToolDefinition} from './tools/ToolDefinition.js';
import type {
DefinedPageTool,
DevToolsData,
ToolDefinition,
} from './tools/ToolDefinition.js';
import {pageIdSchema} from './tools/ToolDefinition.js';
import {logger} from './utils/logger.js';
import type {Mutex} from './utils/Mutex.js';
Expand Down Expand Up @@ -208,6 +213,7 @@ export class ToolHandler {
const guard = await this.toolMutex.acquire();
const startTime = Date.now();
let success = false;
let devToolsData: DevToolsData | undefined;
try {
logger?.(
`${this.tool.name} request: ${JSON.stringify(params, null, ' ')}`,
Expand All @@ -222,6 +228,7 @@ export class ToolHandler {
if (context.consumeReconnectNotice()) {
response.setReconnectNotice();
}
let page: McpPage | undefined;
try {
if (this.tool.verifyFilesSchema) {
for (const key of this.tool.verifyFilesSchema) {
Expand All @@ -232,7 +239,7 @@ export class ToolHandler {
if (isPageScopedTool(this.tool)) {
const pageId =
typeof params.pageId === 'number' ? params.pageId : undefined;
const page =
page =
this.serverArgs.experimentalPageIdRouting &&
pageId !== undefined &&
!this.serverArgs.slim
Expand Down Expand Up @@ -262,6 +269,7 @@ export class ToolHandler {
} catch (err) {
response.setError(err);
}
devToolsData = await context.getDevToolsData(page);
// Resolve data format: --experimentalDataFormat takes precedence, fall back to legacy --experimentalToonFormat
let dataFormat: DataFormat = 'default';
if (this.serverArgs.experimentalDataFormat) {
Expand Down Expand Up @@ -303,12 +311,16 @@ export class ToolHandler {
isError: true,
};
} finally {
const isDevToolsOpen = devToolsData
? Object.keys(devToolsData).length > 0
: undefined;
void ClearcutLogger.get()?.logToolInvocation({
toolName: this.tool.name,
params,
schema: this.inputSchema,
success,
latencyMs: bucketizeLatency(Date.now() - startTime),
isDevToolsOpen,
});
guard.dispose();
}
Expand Down
6 changes: 6 additions & 0 deletions src/telemetry/ClearcutLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,19 @@ export class ClearcutLogger {
schema: zod.ZodRawShape;
success: boolean;
latencyMs: number;
isDevToolsOpen?: boolean;
}): Promise<void> {
const sanitizedToolName = stripUnderscoreBeforeNumber(args.toolName);
const tool_invocation: ToolInvocation = {
tool_name: sanitizedToolName,
success: args.success,
latency_ms: args.latencyMs,
};
if (args.isDevToolsOpen !== undefined) {
tool_invocation.context = {
is_devtools_open: args.isDevToolsOpen,
};
}
if (Object.keys(args.params).length > 0) {
tool_invocation.tool_params = {
[`${sanitizedToolName}_params`]: sanitizeParams(
Expand Down
5 changes: 5 additions & 0 deletions src/telemetry/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,16 @@ export interface ServerError {

export type ServerShutdown = Record<string, never>;

export interface ToolInvocationContext {
is_devtools_open?: boolean;
}

export interface ToolInvocation {
tool_name: string;
success: boolean;
latency_ms: number;
tool_params?: object;
context?: ToolInvocationContext;
}

export interface ServerStart {
Expand Down
32 changes: 32 additions & 0 deletions tests/McpContext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import sinon from 'sinon';

import {NetworkFormatter} from '../src/formatters/NetworkFormatter.js';
import {McpContext} from '../src/McpContext.js';
import {McpPage} from '../src/McpPage.js';
import {TextSnapshot} from '../src/TextSnapshot.js';
import {type HTTPResponse} from '../src/third_party/index.js';
import type {TraceResult} from '../src/trace-processing/parse.js';
Expand Down Expand Up @@ -591,5 +592,36 @@ describe('McpContext', () => {
);
});
});

describe('getDevToolsData', () => {
it('returns devtools data from passed page', async () => {
await withMcpContext(async (_response, context) => {
const mockPage = sinon.createStubInstance(McpPage);
mockPage.getDevToolsData.resolves({cdpBackendNodeId: 42});
const result = await context.getDevToolsData(mockPage);
assert.deepStrictEqual(result, {cdpBackendNodeId: 42});
});
});

it('returns undefined when getDevToolsData times out', async () => {
await withMcpContext(async (_response, context) => {
const mockPage = sinon.createStubInstance(McpPage);
mockPage.getDevToolsData.returns(
new Promise(resolve => {
setTimeout(() => resolve({cdpBackendNodeId: 100}), 600);
}),
);
const result = await context.getDevToolsData(mockPage);
assert.strictEqual(result, undefined);
});
});

it('returns empty object from selected page when devtools is closed', async () => {
await withMcpContext(async (_response, context) => {
const result = await context.getDevToolsData();
assert.deepStrictEqual(result, {});
});
});
});
});
});
49 changes: 48 additions & 1 deletion tests/ToolHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import sinon from 'sinon';
import {parseArguments} from '../src/bin/chrome-devtools-mcp-cli-options.js';
import {McpContext} from '../src/McpContext.js';
import {McpPage} from '../src/McpPage.js';
import {ClearcutLogger} from '../src/telemetry/ClearcutLogger.js';
import {zod} from '../src/third_party/index.js';
import {ToolHandler} from '../src/ToolHandler.js';
import {ToolCategory} from '../src/tools/categories.js';
Expand All @@ -24,6 +25,7 @@ import {Mutex} from '../src/utils/Mutex.js';
describe('ToolHandler', () => {
afterEach(() => {
sinon.restore();
ClearcutLogger.resetForTesting();
});

it('calls page getter for page scoped tools', async () => {
Expand Down Expand Up @@ -67,7 +69,7 @@ describe('ToolHandler', () => {
assert.strictEqual(handlerCalled, true);
});

it('does not call page getter for non-page scoped tools', async () => {
it('does not pass page to handler for non-page scoped tools', async () => {
let handlerCalled = false;
const tool: ToolDefinition = {
name: 'global_tool',
Expand Down Expand Up @@ -101,12 +103,57 @@ describe('ToolHandler', () => {
assert.strictEqual(toolHandler.shouldRegister, true);
const result = await toolHandler.handle({});

assert.strictEqual(mockContext.getDevToolsData.calledOnce, true);
assert.strictEqual(mockContext.getSelectedMcpPage.called, false);
assert.strictEqual(mockContext.getPageById.called, false);
assert.strictEqual(handlerCalled, true);
assert.strictEqual(result.isError, undefined);
});

it('logs isDevToolsOpen telemetry based on getDevToolsData', async () => {
let handlerCalled = false;
const tool: ToolDefinition = {
name: 'global_tool',
description: 'A global tool',
annotations: {
category: ToolCategory.NAVIGATION,
readOnlyHint: true,
},
schema: {},
blockedByDialog: false,
verifyFilesSchema: [],
handler: async () => {
handlerCalled = true;
},
};

const mockContext = sinon.createStubInstance(McpContext);
mockContext.getDevToolsData.resolves({cdpBackendNodeId: 1});

const logSpy = sinon.spy();
sinon.stub(ClearcutLogger, 'get').returns({
logToolInvocation: logSpy,
} as unknown as ClearcutLogger);

const toolMutex = new Mutex();
const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
});

const toolHandler = new ToolHandler(
tool,
serverArgs,
async () => mockContext,
toolMutex,
);

await toolHandler.handle({});

assert.strictEqual(logSpy.calledOnce, true);
assert.strictEqual(logSpy.firstCall.args[0].isDevToolsOpen, true);
assert.strictEqual(handlerCalled, true);
});

it('reports unknown registered tool arguments clearly', async () => {
let handlerCalled = false;
const tool: ToolDefinition = {
Expand Down
22 changes: 22 additions & 0 deletions tests/telemetry/ClearcutLogger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,28 @@ describe('ClearcutLogger', () => {
assert.strictEqual(msg.payload.tool_invocation?.success, true);
assert.strictEqual(msg.payload.tool_invocation?.latency_ms, 123);
});
it('sends context with is_devtools_open', async () => {
const logger = ClearcutLogger.initialize({
persistence: mockPersistence,
appVersion: '1.0.0',
watchdogClient: mockWatchdogClient,
});
await logger.logToolInvocation({
toolName: 'test_tool',
params: {},
schema: {},
success: true,
latencyMs: 123,
isDevToolsOpen: true,
});

assert(mockWatchdogClient.send.calledOnce);
const msg = mockWatchdogClient.send.firstCall.args[0];
assert.strictEqual(msg.type, WatchdogMessageType.LOG_EVENT);
assert.deepStrictEqual(msg.payload.tool_invocation?.context, {
is_devtools_open: true,
});
});
it('sends sanitized params', async () => {
const logger = ClearcutLogger.initialize({
persistence: mockPersistence,
Expand Down