mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-20 10:10:56 -07:00
feat(hooks): add mcp_context to BeforeTool and AfterTool hook inputs (#15656)
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
This commit is contained in:
@@ -258,6 +258,128 @@ describe('HookEventHandler', () => {
|
||||
expect.stringContaining('F12'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should fire BeforeTool event with MCP context when provided', async () => {
|
||||
const mockPlan = [
|
||||
{
|
||||
hookConfig: {
|
||||
type: HookType.Command,
|
||||
command: './test.sh',
|
||||
} as unknown as HookConfig,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
},
|
||||
];
|
||||
const mockResults: HookExecutionResult[] = [
|
||||
{
|
||||
success: true,
|
||||
duration: 100,
|
||||
hookConfig: {
|
||||
type: HookType.Command,
|
||||
command: './test.sh',
|
||||
timeout: 30000,
|
||||
},
|
||||
eventName: HookEventName.BeforeTool,
|
||||
},
|
||||
];
|
||||
const mockAggregated = {
|
||||
success: true,
|
||||
allOutputs: [],
|
||||
errors: [],
|
||||
totalDuration: 100,
|
||||
};
|
||||
|
||||
vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue({
|
||||
eventName: HookEventName.BeforeTool,
|
||||
hookConfigs: mockPlan.map((p) => p.hookConfig),
|
||||
sequential: false,
|
||||
});
|
||||
vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue(
|
||||
mockResults,
|
||||
);
|
||||
vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue(
|
||||
mockAggregated,
|
||||
);
|
||||
|
||||
const mcpContext = {
|
||||
server_name: 'my-mcp-server',
|
||||
tool_name: 'read_file',
|
||||
command: 'npx',
|
||||
args: ['-y', '@my-org/mcp-server'],
|
||||
};
|
||||
|
||||
const result = await hookEventHandler.fireBeforeToolEvent(
|
||||
'my-mcp-server__read_file',
|
||||
{ path: '/etc/passwd' },
|
||||
mcpContext,
|
||||
);
|
||||
|
||||
expect(mockHookRunner.executeHooksParallel).toHaveBeenCalledWith(
|
||||
[mockPlan[0].hookConfig],
|
||||
HookEventName.BeforeTool,
|
||||
expect.objectContaining({
|
||||
session_id: 'test-session',
|
||||
cwd: '/test/project',
|
||||
hook_event_name: 'BeforeTool',
|
||||
tool_name: 'my-mcp-server__read_file',
|
||||
tool_input: { path: '/etc/passwd' },
|
||||
mcp_context: mcpContext,
|
||||
}),
|
||||
expect.any(Function),
|
||||
expect.any(Function),
|
||||
);
|
||||
|
||||
expect(result).toBe(mockAggregated);
|
||||
});
|
||||
|
||||
it('should not include mcp_context when not provided', async () => {
|
||||
const mockPlan = [
|
||||
{
|
||||
hookConfig: {
|
||||
type: HookType.Command,
|
||||
command: './test.sh',
|
||||
} as unknown as HookConfig,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
},
|
||||
];
|
||||
const mockResults: HookExecutionResult[] = [
|
||||
{
|
||||
success: true,
|
||||
duration: 100,
|
||||
hookConfig: {
|
||||
type: HookType.Command,
|
||||
command: './test.sh',
|
||||
timeout: 30000,
|
||||
},
|
||||
eventName: HookEventName.BeforeTool,
|
||||
},
|
||||
];
|
||||
const mockAggregated = {
|
||||
success: true,
|
||||
allOutputs: [],
|
||||
errors: [],
|
||||
totalDuration: 100,
|
||||
};
|
||||
|
||||
vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue({
|
||||
eventName: HookEventName.BeforeTool,
|
||||
hookConfigs: mockPlan.map((p) => p.hookConfig),
|
||||
sequential: false,
|
||||
});
|
||||
vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue(
|
||||
mockResults,
|
||||
);
|
||||
vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue(
|
||||
mockAggregated,
|
||||
);
|
||||
|
||||
await hookEventHandler.fireBeforeToolEvent('EditTool', {
|
||||
file: 'test.txt',
|
||||
});
|
||||
|
||||
const callArgs = vi.mocked(mockHookRunner.executeHooksParallel).mock
|
||||
.calls[0][2];
|
||||
expect(callArgs).not.toHaveProperty('mcp_context');
|
||||
});
|
||||
});
|
||||
|
||||
describe('fireAfterToolEvent', () => {
|
||||
@@ -325,6 +447,78 @@ describe('HookEventHandler', () => {
|
||||
|
||||
expect(result).toBe(mockAggregated);
|
||||
});
|
||||
|
||||
it('should fire AfterTool event with MCP context when provided', async () => {
|
||||
const mockPlan = [
|
||||
{
|
||||
hookConfig: {
|
||||
type: HookType.Command,
|
||||
command: './after.sh',
|
||||
} as unknown as HookConfig,
|
||||
eventName: HookEventName.AfterTool,
|
||||
},
|
||||
];
|
||||
const mockResults: HookExecutionResult[] = [
|
||||
{
|
||||
success: true,
|
||||
duration: 100,
|
||||
hookConfig: {
|
||||
type: HookType.Command,
|
||||
command: './after.sh',
|
||||
timeout: 30000,
|
||||
},
|
||||
eventName: HookEventName.AfterTool,
|
||||
},
|
||||
];
|
||||
const mockAggregated = {
|
||||
success: true,
|
||||
allOutputs: [],
|
||||
errors: [],
|
||||
totalDuration: 100,
|
||||
};
|
||||
|
||||
vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue({
|
||||
eventName: HookEventName.AfterTool,
|
||||
hookConfigs: mockPlan.map((p) => p.hookConfig),
|
||||
sequential: false,
|
||||
});
|
||||
vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue(
|
||||
mockResults,
|
||||
);
|
||||
vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue(
|
||||
mockAggregated,
|
||||
);
|
||||
|
||||
const toolInput = { path: '/etc/passwd' };
|
||||
const toolResponse = { success: true, content: 'File content' };
|
||||
const mcpContext = {
|
||||
server_name: 'my-mcp-server',
|
||||
tool_name: 'read_file',
|
||||
url: 'https://mcp.example.com',
|
||||
};
|
||||
|
||||
const result = await hookEventHandler.fireAfterToolEvent(
|
||||
'my-mcp-server__read_file',
|
||||
toolInput,
|
||||
toolResponse,
|
||||
mcpContext,
|
||||
);
|
||||
|
||||
expect(mockHookRunner.executeHooksParallel).toHaveBeenCalledWith(
|
||||
[mockPlan[0].hookConfig],
|
||||
HookEventName.AfterTool,
|
||||
expect.objectContaining({
|
||||
tool_name: 'my-mcp-server__read_file',
|
||||
tool_input: toolInput,
|
||||
tool_response: toolResponse,
|
||||
mcp_context: mcpContext,
|
||||
}),
|
||||
expect.any(Function),
|
||||
expect.any(Function),
|
||||
);
|
||||
|
||||
expect(result).toBe(mockAggregated);
|
||||
});
|
||||
});
|
||||
|
||||
describe('fireBeforeAgentEvent', () => {
|
||||
|
||||
@@ -29,6 +29,7 @@ import type {
|
||||
SessionEndReason,
|
||||
PreCompressTrigger,
|
||||
HookExecutionResult,
|
||||
McpToolContext,
|
||||
} from './types.js';
|
||||
import { defaultHookTranslator } from './hookTranslator.js';
|
||||
import type {
|
||||
@@ -58,9 +59,11 @@ function isObject(value: unknown): value is Record<string, unknown> {
|
||||
function validateBeforeToolInput(input: Record<string, unknown>): {
|
||||
toolName: string;
|
||||
toolInput: Record<string, unknown>;
|
||||
mcpContext?: McpToolContext;
|
||||
} {
|
||||
const toolName = input['tool_name'];
|
||||
const toolInput = input['tool_input'];
|
||||
const mcpContext = input['mcp_context'];
|
||||
if (typeof toolName !== 'string') {
|
||||
throw new Error(
|
||||
'Invalid input for BeforeTool hook event: tool_name must be a string',
|
||||
@@ -71,7 +74,16 @@ function validateBeforeToolInput(input: Record<string, unknown>): {
|
||||
'Invalid input for BeforeTool hook event: tool_input must be an object',
|
||||
);
|
||||
}
|
||||
return { toolName, toolInput };
|
||||
if (mcpContext !== undefined && !isObject(mcpContext)) {
|
||||
throw new Error(
|
||||
'Invalid input for BeforeTool hook event: mcp_context must be an object',
|
||||
);
|
||||
}
|
||||
return {
|
||||
toolName,
|
||||
toolInput,
|
||||
mcpContext: mcpContext as McpToolContext | undefined,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -81,10 +93,12 @@ function validateAfterToolInput(input: Record<string, unknown>): {
|
||||
toolName: string;
|
||||
toolInput: Record<string, unknown>;
|
||||
toolResponse: Record<string, unknown>;
|
||||
mcpContext?: McpToolContext;
|
||||
} {
|
||||
const toolName = input['tool_name'];
|
||||
const toolInput = input['tool_input'];
|
||||
const toolResponse = input['tool_response'];
|
||||
const mcpContext = input['mcp_context'];
|
||||
if (typeof toolName !== 'string') {
|
||||
throw new Error(
|
||||
'Invalid input for AfterTool hook event: tool_name must be a string',
|
||||
@@ -100,7 +114,17 @@ function validateAfterToolInput(input: Record<string, unknown>): {
|
||||
'Invalid input for AfterTool hook event: tool_response must be an object',
|
||||
);
|
||||
}
|
||||
return { toolName, toolInput, toolResponse };
|
||||
if (mcpContext !== undefined && !isObject(mcpContext)) {
|
||||
throw new Error(
|
||||
'Invalid input for AfterTool hook event: mcp_context must be an object',
|
||||
);
|
||||
}
|
||||
return {
|
||||
toolName,
|
||||
toolInput,
|
||||
toolResponse,
|
||||
mcpContext: mcpContext as McpToolContext | undefined,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -313,11 +337,13 @@ export class HookEventHandler {
|
||||
async fireBeforeToolEvent(
|
||||
toolName: string,
|
||||
toolInput: Record<string, unknown>,
|
||||
mcpContext?: McpToolContext,
|
||||
): Promise<AggregatedHookResult> {
|
||||
const input: BeforeToolInput = {
|
||||
...this.createBaseInput(HookEventName.BeforeTool),
|
||||
tool_name: toolName,
|
||||
tool_input: toolInput,
|
||||
...(mcpContext && { mcp_context: mcpContext }),
|
||||
};
|
||||
|
||||
const context: HookEventContext = { toolName };
|
||||
@@ -332,12 +358,14 @@ export class HookEventHandler {
|
||||
toolName: string,
|
||||
toolInput: Record<string, unknown>,
|
||||
toolResponse: Record<string, unknown>,
|
||||
mcpContext?: McpToolContext,
|
||||
): Promise<AggregatedHookResult> {
|
||||
const input: AfterToolInput = {
|
||||
...this.createBaseInput(HookEventName.AfterTool),
|
||||
tool_name: toolName,
|
||||
tool_input: toolInput,
|
||||
tool_response: toolResponse,
|
||||
...(mcpContext && { mcp_context: mcpContext }),
|
||||
};
|
||||
|
||||
const context: HookEventContext = { toolName };
|
||||
@@ -725,18 +753,23 @@ export class HookEventHandler {
|
||||
// Route to appropriate event handler based on eventName
|
||||
switch (request.eventName) {
|
||||
case HookEventName.BeforeTool: {
|
||||
const { toolName, toolInput } =
|
||||
const { toolName, toolInput, mcpContext } =
|
||||
validateBeforeToolInput(enrichedInput);
|
||||
result = await this.fireBeforeToolEvent(toolName, toolInput);
|
||||
result = await this.fireBeforeToolEvent(
|
||||
toolName,
|
||||
toolInput,
|
||||
mcpContext,
|
||||
);
|
||||
break;
|
||||
}
|
||||
case HookEventName.AfterTool: {
|
||||
const { toolName, toolInput, toolResponse } =
|
||||
const { toolName, toolInput, toolResponse, mcpContext } =
|
||||
validateAfterToolInput(enrichedInput);
|
||||
result = await this.fireAfterToolEvent(
|
||||
toolName,
|
||||
toolInput,
|
||||
toolResponse,
|
||||
mcpContext,
|
||||
);
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -373,12 +373,37 @@ export class AfterModelHookOutput extends DefaultHookOutput {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Context for MCP tool executions.
|
||||
* Contains non-sensitive connection information about the MCP server
|
||||
* identity. Since server_name is user controlled and arbitrary, we
|
||||
* also include connection information (e.g., command or url) to
|
||||
* help identify the MCP server.
|
||||
*
|
||||
* NOTE: In the future, consider defining a shared sanitized interface
|
||||
* from MCPServerConfig to avoid duplication and ensure consistency.
|
||||
*/
|
||||
export interface McpToolContext {
|
||||
server_name: string;
|
||||
tool_name: string; // Original tool name from the MCP server
|
||||
|
||||
// Connection info (mutually exclusive based on transport type)
|
||||
command?: string; // For stdio transport
|
||||
args?: string[]; // For stdio transport
|
||||
cwd?: string; // For stdio transport
|
||||
|
||||
url?: string; // For SSE/HTTP transport
|
||||
|
||||
tcp?: string; // For WebSocket transport
|
||||
}
|
||||
|
||||
/**
|
||||
* BeforeTool hook input
|
||||
*/
|
||||
export interface BeforeToolInput extends HookInput {
|
||||
tool_name: string;
|
||||
tool_input: Record<string, unknown>;
|
||||
mcp_context?: McpToolContext; // Only present for MCP tools
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -398,6 +423,7 @@ export interface AfterToolInput extends HookInput {
|
||||
tool_name: string;
|
||||
tool_input: Record<string, unknown>;
|
||||
tool_response: Record<string, unknown>;
|
||||
mcp_context?: McpToolContext; // Only present for MCP tools
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user