diff --git a/integration-tests/hooks-system.after-agent.responses b/integration-tests/hooks-system.after-agent.responses new file mode 100644 index 0000000000..1475070c3d --- /dev/null +++ b/integration-tests/hooks-system.after-agent.responses @@ -0,0 +1,2 @@ +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"Hi there!"}],"role":"model"},"finishReason":"STOP","index":0}]}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"Clarification: I am a bot."}],"role":"model"},"finishReason":"STOP","index":0}]}]} diff --git a/integration-tests/hooks-system.before-tool-stop.responses b/integration-tests/hooks-system.before-tool-stop.responses new file mode 100644 index 0000000000..3a45bc756e --- /dev/null +++ b/integration-tests/hooks-system.before-tool-stop.responses @@ -0,0 +1,2 @@ +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"write_file","args":{"file_path":"test.txt","content":"hello"}}}],"role":"model"},"finishReason":"STOP","index":0}]}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"Okay, stopping."}],"role":"model"},"finishReason":"STOP","index":0}]}]} \ No newline at end of file diff --git a/integration-tests/hooks-system.input-modification.responses b/integration-tests/hooks-system.input-modification.responses new file mode 100644 index 0000000000..aa97444663 --- /dev/null +++ b/integration-tests/hooks-system.input-modification.responses @@ -0,0 +1,2 @@ +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"write_file","args":{"content":"original content","file_path":"original.txt"}}}],"role":"model"},"finishReason":"STOP","index":0}]}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"I have created the file."}],"role":"model"},"finishReason":"STOP","index":0}]}]} diff --git a/integration-tests/hooks-system.test.ts b/integration-tests/hooks-system.test.ts index 6dd838be58..6ad81bad2d 100644 --- a/integration-tests/hooks-system.test.ts +++ b/integration-tests/hooks-system.test.ts @@ -1459,4 +1459,170 @@ console.log(JSON.stringify({decision: "block", systemMessage: "Disabled hook sho expect(disabledHookCalls.length).toBe(0); }); }); + + describe('BeforeTool Hooks - Input Override', () => { + it('should override tool input parameters via BeforeTool hook', async () => { + // 1. First setup to get the test directory and prepare the hook script + await rig.setup( + 'should override tool input parameters via BeforeTool hook', + ); + + // Create a hook script that overrides the tool input + const hookOutput = { + decision: 'allow', + hookSpecificOutput: { + hookEventName: 'BeforeTool', + tool_input: { + file_path: 'modified.txt', + content: 'modified content', + }, + }, + }; + + const hookScript = `process.stdout.write(JSON.stringify(${JSON.stringify( + hookOutput, + )}));`; + + const scriptPath = join(rig.testDir!, 'input_override_hook.js'); + writeFileSync(scriptPath, hookScript); + + // Ensure path is properly escaped for command line usage on all platforms + // On Windows, backslashes in the command string need to be handled carefully + // Using forward slashes works well with Node.js on all platforms + const commandPath = scriptPath.replace(/\\/g, '/'); + + // 2. Full setup with settings and fake responses + await rig.setup( + 'should override tool input parameters via BeforeTool hook', + { + fakeResponsesPath: join( + import.meta.dirname, + 'hooks-system.input-modification.responses', + ), + settings: { + tools: { + enableHooks: true, + }, + hooks: { + BeforeTool: [ + { + matcher: 'write_file', + hooks: [ + { + type: 'command', + command: `node "${commandPath}"`, + timeout: 5000, + }, + ], + }, + ], + }, + }, + }, + ); + + // Run the agent. The fake response will attempt to call write_file with + // file_path="original.txt" and content="original content" + await rig.run({ + args: 'Create a file called original.txt with content "original content"', + }); + + // 1. Verify that 'modified.txt' was created with 'modified content' (Override successful) + const modifiedContent = rig.readFile('modified.txt'); + expect(modifiedContent).toBe('modified content'); + + // 2. Verify that 'original.txt' was NOT created (Override replaced original) + let originalExists = false; + try { + rig.readFile('original.txt'); + originalExists = true; + } catch { + originalExists = false; + } + expect(originalExists).toBe(false); + + // 3. Verify hook telemetry + const hookTelemetryFound = await rig.waitForTelemetryEvent('hook_call'); + expect(hookTelemetryFound).toBeTruthy(); + + const hookLogs = rig.readHookLogs(); + expect(hookLogs.length).toBe(1); + expect(hookLogs[0].hookCall.hook_name).toContain( + 'input_override_hook.js', + ); + + // 4. Verify that the agent didn't try to work-around the hook input change + const toolLogs = rig.readToolLogs(); + expect(toolLogs.length).toBe(1); + expect(toolLogs[0].toolRequest.name).toBe('write_file'); + expect(JSON.parse(toolLogs[0].toolRequest.args).file_path).toBe( + 'modified.txt', + ); + }); + }); + + describe('BeforeTool Hooks - Stop Execution', () => { + it('should stop agent execution via BeforeTool hook', async () => { + // Create a hook script that stops execution + const hookOutput = { + continue: false, + reason: 'Emergency Stop triggered by hook', + hookSpecificOutput: { + hookEventName: 'BeforeTool', + }, + }; + + const hookScript = `console.log(JSON.stringify(${JSON.stringify( + hookOutput, + )}));`; + + await rig.setup('should stop agent execution via BeforeTool hook'); + const scriptPath = join(rig.testDir!, 'before_tool_stop_hook.js'); + writeFileSync(scriptPath, hookScript); + const commandPath = scriptPath.replace(/\\/g, '/'); + + await rig.setup('should stop agent execution via BeforeTool hook', { + fakeResponsesPath: join( + import.meta.dirname, + 'hooks-system.before-tool-stop.responses', + ), + settings: { + tools: { + enableHooks: true, + }, + hooks: { + BeforeTool: [ + { + matcher: 'write_file', + hooks: [ + { + type: 'command', + command: `node "${commandPath}"`, + timeout: 5000, + }, + ], + }, + ], + }, + }, + }); + + const result = await rig.run({ + args: 'Run tool', + }); + + // The hook should have stopped execution message (returned from tool) + expect(result).toContain( + 'Agent execution stopped: Emergency Stop triggered by hook', + ); + + // Tool should NOT be called successfully (it was blocked/stopped) + const toolLogs = rig.readToolLogs(); + const writeFileCalls = toolLogs.filter( + (t) => + t.toolRequest.name === 'write_file' && t.toolRequest.success === true, + ); + expect(writeFileCalls).toHaveLength(0); + }); + }); }); diff --git a/integration-tests/test-helper.ts b/integration-tests/test-helper.ts index 7ff1aac4ad..e197c724a5 100644 --- a/integration-tests/test-helper.ts +++ b/integration-tests/test-helper.ts @@ -285,7 +285,9 @@ export class TestRig { ) { this.testName = testName; const sanitizedName = sanitizeTestName(testName); - this.testDir = join(env['INTEGRATION_TEST_FILE_DIR']!, sanitizedName); + const testFileDir = + env['INTEGRATION_TEST_FILE_DIR'] || join(os.tmpdir(), 'gemini-cli-tests'); + this.testDir = join(testFileDir, sanitizedName); mkdirSync(this.testDir, { recursive: true }); if (options.fakeResponsesPath) { this.fakeResponsesPath = join(this.testDir, 'fake-responses.json'); diff --git a/packages/core/src/core/coreToolHookTriggers.test.ts b/packages/core/src/core/coreToolHookTriggers.test.ts new file mode 100644 index 0000000000..e49b48e4ae --- /dev/null +++ b/packages/core/src/core/coreToolHookTriggers.test.ts @@ -0,0 +1,131 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { executeToolWithHooks } from './coreToolHookTriggers.js'; +import { + BaseToolInvocation, + type ToolResult, + type AnyDeclarativeTool, +} from '../tools/tools.js'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import { + MessageBusType, + type HookExecutionResponse, +} from '../confirmation-bus/types.js'; + +class MockInvocation extends BaseToolInvocation<{ key: string }, ToolResult> { + constructor(params: { key: string }) { + super(params); + } + getDescription() { + return 'mock'; + } + async execute() { + return { + llmContent: `key: ${this.params.key}`, + returnDisplay: `key: ${this.params.key}`, + }; + } +} + +describe('executeToolWithHooks', () => { + let messageBus: MessageBus; + let mockTool: AnyDeclarativeTool; + + beforeEach(() => { + messageBus = { + request: vi.fn(), + } as unknown as MessageBus; + mockTool = { + build: vi.fn().mockImplementation((params) => new MockInvocation(params)), + } as unknown as AnyDeclarativeTool; + }); + + it('should apply modified tool input from BeforeTool hook', async () => { + const params = { key: 'original' }; + const invocation = new MockInvocation(params); + const toolName = 'test-tool'; + const abortSignal = new AbortController().signal; + + // Capture arguments to verify what was passed before modification + const requestSpy = vi.fn().mockImplementation(async (request) => { + if (request.eventName === 'BeforeTool') { + // Verify input is original before we return modification instruction + expect(request.input.tool_input.key).toBe('original'); + return { + type: MessageBusType.HOOK_EXECUTION_RESPONSE, + correlationId: 'test-id', + success: true, + output: { + hookSpecificOutput: { + hookEventName: 'BeforeTool', + tool_input: { key: 'modified' }, + }, + }, + } as HookExecutionResponse; + } + return { + type: MessageBusType.HOOK_EXECUTION_RESPONSE, + correlationId: 'test-id', + success: true, + output: {}, + } as HookExecutionResponse; + }); + messageBus.request = requestSpy; + + const result = await executeToolWithHooks( + invocation, + toolName, + abortSignal, + messageBus, + true, // hooksEnabled + mockTool, + ); + + // Verify result reflects modified input + expect(result.llmContent).toBe( + 'key: modified\n\n[System] Tool input parameters (key) were modified by a hook before execution.', + ); + // Verify params object was modified in place + expect(invocation.params.key).toBe('modified'); + + expect(requestSpy).toHaveBeenCalled(); + expect(mockTool.build).toHaveBeenCalledWith({ key: 'modified' }); + }); + + it('should not modify input if hook does not provide tool_input', async () => { + const params = { key: 'original' }; + const invocation = new MockInvocation(params); + const toolName = 'test-tool'; + const abortSignal = new AbortController().signal; + + vi.mocked(messageBus.request).mockResolvedValue({ + type: MessageBusType.HOOK_EXECUTION_RESPONSE, + correlationId: 'test-id', + success: true, + output: { + hookSpecificOutput: { + hookEventName: 'BeforeTool', + // No tool_input + }, + }, + } as HookExecutionResponse); + + const result = await executeToolWithHooks( + invocation, + toolName, + abortSignal, + messageBus, + true, // hooksEnabled + mockTool, + ); + + expect(result.llmContent).toBe('key: original'); + expect(invocation.params.key).toBe('original'); + expect(mockTool.build).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/core/coreToolHookTriggers.ts b/packages/core/src/core/coreToolHookTriggers.ts index 37ca499d33..24fd113ee6 100644 --- a/packages/core/src/core/coreToolHookTriggers.ts +++ b/packages/core/src/core/coreToolHookTriggers.ts @@ -14,10 +14,12 @@ import { createHookOutput, NotificationType, type DefaultHookOutput, + BeforeToolHookOutput, } from '../hooks/types.js'; import type { ToolCallConfirmationDetails, ToolResult, + AnyDeclarativeTool, } from '../tools/tools.js'; import { ToolErrorType } from '../tools/tool-error.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -254,11 +256,14 @@ export async function executeToolWithHooks( signal: AbortSignal, messageBus: MessageBus | undefined, hooksEnabled: boolean, + tool: AnyDeclarativeTool, liveOutputCallback?: (outputChunk: string | AnsiOutput) => void, shellExecutionConfig?: ShellExecutionConfig, setPidCallback?: (pid: number) => void, ): Promise { const toolInput = (invocation.params || {}) as Record; + let inputWasModified = false; + let modifiedKeys: string[] = []; // Fire BeforeTool hook through MessageBus (only if hooks are enabled) if (hooksEnabled && messageBus) { @@ -293,6 +298,38 @@ export async function executeToolWithHooks( }, }; } + + // Check if hook requested to update tool input + if (beforeOutput instanceof BeforeToolHookOutput) { + const modifiedInput = beforeOutput.getModifiedToolInput(); + if (modifiedInput) { + // We modify the toolInput object in-place, which should be the same reference as invocation.params + // We use Object.assign to update properties + Object.assign(invocation.params, modifiedInput); + debugLogger.debug(`Tool input modified by hook for ${toolName}`); + inputWasModified = true; + modifiedKeys = Object.keys(modifiedInput); + + // Recreate the invocation with the new parameters + // to ensure any derived state (like resolvedPath in ReadFileTool) is updated. + try { + // We use the tool's build method to validate and create the invocation + // This ensures consistent behavior with the initial creation + invocation = tool.build(invocation.params); + } catch (error) { + return { + llmContent: `Tool parameter modification by hook failed validation: ${ + error instanceof Error ? error.message : String(error) + }`, + returnDisplay: `Tool parameter modification by hook failed validation.`, + error: { + type: ToolErrorType.INVALID_TOOL_PARAMS, + message: String(error), + }, + }; + } + } + } } // Execute the actual tool @@ -312,6 +349,24 @@ export async function executeToolWithHooks( ); } + // Append notification if parameters were modified + if (inputWasModified) { + const modificationMsg = `\n\n[System] Tool input parameters (${modifiedKeys.join( + ', ', + )}) were modified by a hook before execution.`; + if (typeof toolResult.llmContent === 'string') { + toolResult.llmContent += modificationMsg; + } else if (Array.isArray(toolResult.llmContent)) { + toolResult.llmContent.push({ text: modificationMsg }); + } else if (toolResult.llmContent) { + // Handle single Part case by converting to an array + toolResult.llmContent = [ + toolResult.llmContent, + { text: modificationMsg }, + ]; + } + } + // Fire AfterTool hook through MessageBus (only if hooks are enabled) if (hooksEnabled && messageBus) { const afterOutput = await fireAfterToolHook( diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index ae064a148d..cace893f4b 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -900,6 +900,7 @@ export class CoreToolScheduler { signal, messageBus, hooksEnabled, + toolCall.tool, liveOutputCallback, shellExecutionConfig, setPidCallback, @@ -911,6 +912,7 @@ export class CoreToolScheduler { signal, messageBus, hooksEnabled, + toolCall.tool, liveOutputCallback, shellExecutionConfig, ); diff --git a/packages/core/src/hooks/hookAggregator.ts b/packages/core/src/hooks/hookAggregator.ts index 16176d7cfc..0163f21856 100644 --- a/packages/core/src/hooks/hookAggregator.ts +++ b/packages/core/src/hooks/hookAggregator.ts @@ -158,6 +158,14 @@ export class HookAggregator { merged.suppressOutput = true; } + // Merge hookSpecificOutput + if (output.hookSpecificOutput) { + merged.hookSpecificOutput = { + ...(merged.hookSpecificOutput || {}), + ...output.hookSpecificOutput, + }; + } + // Collect additional context from hook-specific outputs this.extractAdditionalContext(output, additionalContexts); } diff --git a/packages/core/src/hooks/hookRunner.ts b/packages/core/src/hooks/hookRunner.ts index 0f7b903521..79aa4a1fd6 100644 --- a/packages/core/src/hooks/hookRunner.ts +++ b/packages/core/src/hooks/hookRunner.ts @@ -15,6 +15,7 @@ import type { BeforeAgentInput, BeforeModelInput, BeforeModelOutput, + BeforeToolInput, } from './types.js'; import type { LLMRequest } from './hookTranslator.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -190,6 +191,20 @@ export class HookRunner { } break; + case HookEventName.BeforeTool: + if ('tool_input' in hookOutput.hookSpecificOutput) { + const newToolInput = hookOutput.hookSpecificOutput[ + 'tool_input' + ] as Record; + if (newToolInput && 'tool_input' in modifiedInput) { + (modifiedInput as BeforeToolInput).tool_input = { + ...(modifiedInput as BeforeToolInput).tool_input, + ...newToolInput, + }; + } + } + break; + default: // For other events, no special input modification is needed break; diff --git a/packages/core/src/hooks/types.test.ts b/packages/core/src/hooks/types.test.ts index d8fbb79488..18a18fe121 100644 --- a/packages/core/src/hooks/types.test.ts +++ b/packages/core/src/hooks/types.test.ts @@ -13,6 +13,7 @@ import { AfterModelHookOutput, HookEventName, HookType, + BeforeToolHookOutput, } from './types.js'; import { defaultHookTranslator } from './hookTranslator.js'; import type { @@ -92,6 +93,11 @@ describe('Hook Output Classes', () => { const output = createHookOutput(HookEventName.BeforeToolSelection, {}); expect(output).toBeInstanceOf(BeforeToolSelectionHookOutput); }); + + it('should return BeforeToolHookOutput for BeforeTool event', () => { + const output = createHookOutput(HookEventName.BeforeTool, {}); + expect(output).toBeInstanceOf(BeforeToolHookOutput); + }); }); describe('DefaultHookOutput', () => { diff --git a/packages/core/src/hooks/types.ts b/packages/core/src/hooks/types.ts index 1f6691966f..08fec0c5dc 100644 --- a/packages/core/src/hooks/types.ts +++ b/packages/core/src/hooks/types.ts @@ -133,6 +133,8 @@ export function createHookOutput( return new AfterModelHookOutput(data); case 'BeforeToolSelection': return new BeforeToolSelectionHookOutput(data); + case 'BeforeTool': + return new BeforeToolHookOutput(data); default: return new DefaultHookOutput(data); } @@ -236,7 +238,24 @@ export class DefaultHookOutput implements HookOutput { /** * Specific hook output class for BeforeTool events. */ -export class BeforeToolHookOutput extends DefaultHookOutput {} +export class BeforeToolHookOutput extends DefaultHookOutput { + /** + * Get modified tool input if provided by hook + */ + getModifiedToolInput(): Record | undefined { + if (this.hookSpecificOutput && 'tool_input' in this.hookSpecificOutput) { + const input = this.hookSpecificOutput['tool_input']; + if ( + typeof input === 'object' && + input !== null && + !Array.isArray(input) + ) { + return input as Record; + } + } + return undefined; + } +} /** * Specific hook output class for BeforeModel events @@ -368,6 +387,7 @@ export interface BeforeToolInput extends HookInput { export interface BeforeToolOutput extends HookOutput { hookSpecificOutput?: { hookEventName: 'BeforeTool'; + tool_input?: Record; }; }