diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 8121007e9..6a6e269ec 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -748,9 +748,8 @@ export class Config { break; case 'Stop': result = await hookSystem.fireStopEvent( - (input['prompt'] as string) || '', - (input['prompt_response'] as string) || '', (input['stop_hook_active'] as boolean) || false, + (input['last_assistant_message'] as string) || '', ); break; default: diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 0c41bde64..1d50349b5 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -590,10 +590,7 @@ export class GeminiClient { const stopOutput = hookOutput as StopHookOutput | undefined; - // For AfterAgent hooks, blocking/stop execution should force continuation (like Stop Hook) - // This enables Ralph Loop functionality where the hook can: - // 1. Return {"decision": "block", "reason": ""} to continue with a new prompt - // 2. Optionally include "systemMessage" to display a status message + // For Stop hooks, blocking/stop execution should force continuation if ( stopOutput?.isBlockingDecision() || stopOutput?.shouldStopExecution() diff --git a/packages/core/src/core/clientHookTriggers.ts b/packages/core/src/core/clientHookTriggers.ts index 8535d3ab9..9f8936c9a 100644 --- a/packages/core/src/core/clientHookTriggers.ts +++ b/packages/core/src/core/clientHookTriggers.ts @@ -22,7 +22,7 @@ const debugLogger = createDebugLogger('HOOK_TRIGGERS'); * This should be called before processing a user prompt. * * The caller can use the returned DefaultHookOutput methods: - * - isBlockingDecision() / shouldStopExecution() to check if blocked + * - isBlockingDecision() to check if the request is blocked * - getEffectiveReason() to get the blocking reason * - getAdditionalContext() to get additional context to add * @@ -65,8 +65,9 @@ export async function fireUserPromptSubmitHook( * This should be called after the agent has generated a response. * * The caller can use the returned DefaultHookOutput methods: - * - isBlockingDecision() / shouldStopExecution() to check if continuation is requested - * - getEffectiveReason() to get the continuation reason + * - isBlockingDecision() to check if the request is blocked + * - shouldStopExecution() to check if execution should be stopped + * - getEffectiveReason() to get the stop/blocking reason * * @param messageBus The message bus to use for hook communication * @param request The original user's request (prompt) @@ -79,8 +80,6 @@ export async function fireStopHook( responseText: string, ): Promise { try { - const promptText = partToString(request); - const response = await messageBus.request< HookExecutionRequest, HookExecutionResponse @@ -89,9 +88,8 @@ export async function fireStopHook( type: MessageBusType.HOOK_EXECUTION_REQUEST, eventName: 'Stop', input: { - prompt: promptText, - prompt_response: responseText, stop_hook_active: true, + last_assistant_message: responseText, }, }, MessageBusType.HOOK_EXECUTION_RESPONSE, diff --git a/packages/core/src/hooks/hookEventHandler.test.ts b/packages/core/src/hooks/hookEventHandler.test.ts new file mode 100644 index 000000000..0b032e547 --- /dev/null +++ b/packages/core/src/hooks/hookEventHandler.test.ts @@ -0,0 +1,693 @@ +/** + * @license + * Copyright 2026 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { HookEventHandler } from './hookEventHandler.js'; +import { + HookEventName, + HookType, + HooksConfigSource, + NotificationType, + SessionStartSource, + SessionEndReason, + PreCompactTrigger, +} from './types.js'; +import type { Config } from '../config/config.js'; +import type { + HookPlanner, + HookRunner, + HookAggregator, + AggregatedHookResult, +} from './index.js'; +import type { HookConfig, HookExecutionResult, HookOutput } from './types.js'; + +describe('HookEventHandler', () => { + let mockConfig: Config; + let mockHookPlanner: HookPlanner; + let mockHookRunner: HookRunner; + let mockHookAggregator: HookAggregator; + let hookEventHandler: HookEventHandler; + + beforeEach(() => { + mockConfig = { + getSessionId: vi.fn().mockReturnValue('test-session-id'), + getTranscriptPath: vi.fn().mockReturnValue('/test/transcript'), + getWorkingDir: vi.fn().mockReturnValue('/test/cwd'), + } as unknown as Config; + + mockHookPlanner = { + createExecutionPlan: vi.fn(), + } as unknown as HookPlanner; + + mockHookRunner = { + executeHooksSequential: vi.fn(), + executeHooksParallel: vi.fn(), + } as unknown as HookRunner; + + mockHookAggregator = { + aggregateResults: vi.fn(), + } as unknown as HookAggregator; + + hookEventHandler = new HookEventHandler( + mockConfig, + mockHookPlanner, + mockHookRunner, + mockHookAggregator, + ); + }); + + const createMockExecutionPlan = ( + hookConfigs: HookConfig[] = [], + sequential: boolean = false, + ) => ({ + hookConfigs, + sequential, + eventName: HookEventName.PreToolUse, + }); + + const createMockExecutionResult = ( + success: boolean = true, + output?: HookOutput, + ): HookExecutionResult => ({ + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.PreToolUse, + success, + output, + duration: 100, + }); + + const createMockAggregatedResult = ( + success: boolean = true, + finalOutput?: HookOutput, + ): AggregatedHookResult => ({ + success, + allOutputs: [], + errors: [], + totalDuration: 100, + finalOutput, + }); + + describe('firePreToolUseEvent', () => { + it('should execute hooks for PreToolUse event', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + const mockResults = [ + createMockExecutionResult(true, { decision: 'allow' }), + ]; + const mockAggregated = createMockAggregatedResult(true, { + decision: 'allow', + }); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue( + mockResults, + ); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = await hookEventHandler.firePreToolUseEvent('Read', { + path: '/test/file.txt', + }); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.PreToolUse, + { toolName: 'Read' }, + ); + expect(mockHookRunner.executeHooksParallel).toHaveBeenCalled(); + expect(result.success).toBe(true); + }); + + it('should include tool name and input in the hook input', async () => { + // Need to provide at least one hook config so the runner is called + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.firePreToolUseEvent('Edit', { file: '/test.txt' }); + + // Verify the mock was called + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalled(); + expect(mockHookRunner.executeHooksParallel).toHaveBeenCalled(); + + // Get the input parameter (3rd argument, index 2) + const inputArg = (mockHookRunner.executeHooksParallel as Mock).mock + .calls[0][2]; + expect(inputArg.tool_name).toBe('Edit'); + expect(inputArg.tool_input).toEqual({ file: '/test.txt' }); + }); + + it('should include mcp_context when provided', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + const mcpContext = { + server_name: 'test-server', + tool_name: 'mcp-tool', + command: 'npx', + }; + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.firePreToolUseEvent('Bash', {}, mcpContext); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { mcp_context?: typeof mcpContext }; + expect(input.mcp_context).toEqual(mcpContext); + }); + + it('should return empty result when no hooks are configured', async () => { + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(null); + + const result = await hookEventHandler.firePreToolUseEvent('Read', {}); + + expect(result.success).toBe(true); + expect(result.allOutputs).toEqual([]); + }); + }); + + describe('firePostToolUseEvent', () => { + it('should execute hooks for PostToolUse event', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + const mockResults = [ + createMockExecutionResult(true, { decision: 'allow' }), + ]; + const mockAggregated = createMockAggregatedResult(true, { + decision: 'allow', + }); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue( + mockResults, + ); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = await hookEventHandler.firePostToolUseEvent( + 'Read', + { path: '/test/file.txt' }, + { content: 'file content' }, + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.PostToolUse, + { toolName: 'Read' }, + ); + expect(result.success).toBe(true); + }); + + it('should include tool_response in the hook input', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.firePostToolUseEvent( + 'Read', + { path: '/test.txt' }, + { content: 'hello' }, + ); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { + tool_response: Record; + }; + expect(input.tool_response).toEqual({ content: 'hello' }); + }); + }); + + describe('fireUserPromptSubmitEvent', () => { + it('should execute hooks for UserPromptSubmit event', async () => { + const mockPlan = createMockExecutionPlan([]); + const mockAggregated = createMockAggregatedResult(true); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = + await hookEventHandler.fireUserPromptSubmitEvent('test prompt'); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.UserPromptSubmit, + undefined, + ); + expect(result.success).toBe(true); + }); + + it('should include prompt in the hook input', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.fireUserPromptSubmitEvent('my test prompt'); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { prompt: string }; + expect(input.prompt).toBe('my test prompt'); + }); + }); + + describe('fireNotificationEvent', () => { + it('should execute hooks for Notification event', async () => { + const mockPlan = createMockExecutionPlan([]); + const mockAggregated = createMockAggregatedResult(true); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = await hookEventHandler.fireNotificationEvent( + NotificationType.ToolPermission, + 'Test message', + { key: 'value' }, + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.Notification, + undefined, + ); + expect(result.success).toBe(true); + }); + }); + + describe('fireStopEvent', () => { + it('should execute hooks for Stop event', async () => { + const mockPlan = createMockExecutionPlan([]); + const mockAggregated = createMockAggregatedResult(true); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = await hookEventHandler.fireStopEvent(true, 'last message'); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.Stop, + undefined, + ); + expect(result.success).toBe(true); + }); + + it('should include stop parameters in hook input', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.fireStopEvent(true, 'last assistant message'); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { + stop_hook_active: boolean; + last_assistant_message: string; + }; + expect(input.stop_hook_active).toBe(true); + expect(input.last_assistant_message).toBe('last assistant message'); + }); + }); + + describe('fireSessionStartEvent', () => { + it('should execute hooks for SessionStart event', async () => { + const mockPlan = createMockExecutionPlan([]); + const mockAggregated = createMockAggregatedResult(true); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = await hookEventHandler.fireSessionStartEvent( + SessionStartSource.Startup, + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.SessionStart, + { trigger: SessionStartSource.Startup }, + ); + expect(result.success).toBe(true); + }); + + it('should include source in hook input', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.fireSessionStartEvent(SessionStartSource.Resume); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { source: string }; + expect(input.source).toBe(SessionStartSource.Resume); + }); + }); + + describe('fireSessionEndEvent', () => { + it('should execute hooks for SessionEnd event', async () => { + const mockPlan = createMockExecutionPlan([]); + const mockAggregated = createMockAggregatedResult(true); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = await hookEventHandler.fireSessionEndEvent( + SessionEndReason.Clear, + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.SessionEnd, + { trigger: SessionEndReason.Clear }, + ); + expect(result.success).toBe(true); + }); + + it('should include reason in hook input', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.fireSessionEndEvent(SessionEndReason.Logout); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { reason: string }; + expect(input.reason).toBe(SessionEndReason.Logout); + }); + }); + + describe('firePreCompactEvent', () => { + it('should execute hooks for PreCompact event', async () => { + const mockPlan = createMockExecutionPlan([]); + const mockAggregated = createMockAggregatedResult(true); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const result = await hookEventHandler.firePreCompactEvent( + PreCompactTrigger.Manual, + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.PreCompact, + { trigger: PreCompactTrigger.Manual }, + ); + expect(result.success).toBe(true); + }); + + it('should include trigger in hook input', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.firePreCompactEvent(PreCompactTrigger.Auto); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { trigger: string }; + expect(input.trigger).toBe(PreCompactTrigger.Auto); + }); + }); + + describe('base input creation', () => { + it('should include common fields in all hook inputs', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.firePreToolUseEvent('Read', {}); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { + session_id: string; + transcript_path: string; + cwd: string; + hook_event_name: string; + timestamp: string; + }; + + expect(input.session_id).toBe('test-session-id'); + expect(input.transcript_path).toBe('/test/transcript'); + expect(input.cwd).toBe('/test/cwd'); + expect(input.hook_event_name).toBe(HookEventName.PreToolUse); + expect(input.timestamp).toBeDefined(); + }); + }); + + describe('sequential vs parallel execution', () => { + it('should execute hooks sequentially when plan.sequential is true', async () => { + const mockPlan = createMockExecutionPlan( + [ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ], + true, + ); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksSequential).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.firePreToolUseEvent('Read', {}); + + expect(mockHookRunner.executeHooksSequential).toHaveBeenCalled(); + expect(mockHookRunner.executeHooksParallel).not.toHaveBeenCalled(); + }); + + it('should execute hooks in parallel when plan.sequential is false', async () => { + const mockPlan = createMockExecutionPlan( + [ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ], + false, + ); + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.firePreToolUseEvent('Read', {}); + + expect(mockHookRunner.executeHooksParallel).toHaveBeenCalled(); + expect(mockHookRunner.executeHooksSequential).not.toHaveBeenCalled(); + }); + }); + + describe('error handling', () => { + it('should return error result when hook execution throws', async () => { + vi.mocked(mockHookPlanner.createExecutionPlan).mockImplementation(() => { + throw new Error('Planner error'); + }); + + const result = await hookEventHandler.firePreToolUseEvent('Read', {}); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('Planner error'); + }); + + it('should return error result when hook runner throws', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockRejectedValue( + new Error('Runner error'), + ); + + const result = await hookEventHandler.firePreToolUseEvent('Read', {}); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('Runner error'); + }); + }); + + describe('processCommonHookOutputFields', () => { + it('should handle systemMessage in final output', async () => { + const mockPlan = createMockExecutionPlan([]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true, { + systemMessage: 'test system message', + }), + ); + + await hookEventHandler.firePreToolUseEvent('Read', {}); + + // The method processes the output - we just verify it doesn't throw + expect(true).toBe(true); + }); + + it('should handle continue=false in final output', async () => { + const mockPlan = createMockExecutionPlan([]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true, { + continue: false, + stopReason: 'test stop', + }), + ); + + await hookEventHandler.fireStopEvent(); + + // The method processes the output - we just verify it doesn't throw + expect(true).toBe(true); + }); + + it('should handle suppressOutput in final output', async () => { + const mockPlan = createMockExecutionPlan([]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true, { suppressOutput: true }), + ); + + await hookEventHandler.firePreToolUseEvent('Read', {}); + + // The method processes the output - we just verify it doesn't throw + expect(true).toBe(true); + }); + + it('should handle missing finalOutput gracefully', async () => { + const mockPlan = createMockExecutionPlan([]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true, undefined), + ); + + const result = await hookEventHandler.firePreToolUseEvent('Read', {}); + + expect(result.success).toBe(true); + expect(result.finalOutput).toBeUndefined(); + }); + }); +}); diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index a0100537f..b29d9f0aa 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -40,13 +40,6 @@ export class HookEventHandler { private readonly hookRunner: HookRunner; private readonly hookAggregator: HookAggregator; - /** - * Track reported failures to suppress duplicate warnings during streaming. - * Uses a WeakMap with the original request object as a key to ensure - * failures are only reported once per logical model interaction. - */ - private readonly reportedFailures = new WeakMap>(); - constructor( config: Config, hookPlanner: HookPlanner, @@ -139,15 +132,13 @@ export class HookEventHandler { * Called by handleHookExecutionRequest - executes hooks directly */ async fireStopEvent( - prompt: string, - promptResponse: string, stopHookActive: boolean = false, + lastAssistantMessage: string = '', ): Promise { const input: StopInput = { ...this.createBaseInput(HookEventName.Stop), - prompt, - prompt_response: promptResponse, stop_hook_active: stopHookActive, + last_assistant_message: lastAssistantMessage, }; return this.executeHooks(HookEventName.Stop, input); @@ -206,7 +197,6 @@ export class HookEventHandler { eventName: HookEventName, input: HookInput, context?: HookEventContext, - requestContext?: object, ): Promise { try { // Create execution plan @@ -255,15 +245,6 @@ export class HookEventHandler { // Process common hook output fields centrally this.processCommonHookOutputFields(aggregated); - // Log hook execution - this.logHookExecution( - eventName, - input, - results, - aggregated, - requestContext, - ); - return aggregated; } catch (error) { debugLogger.error(`Hook event bus error for ${eventName}: ${error}`); @@ -293,64 +274,6 @@ export class HookEventHandler { }; } - /** - * Log hook execution for observability - */ - private logHookExecution( - eventName: HookEventName, - input: HookInput, - results: HookExecutionResult[], - aggregated: AggregatedHookResult, - requestContext?: object, - ): void { - const failedHooks = results.filter((r) => !r.success); - const successCount = results.length - failedHooks.length; - const errorCount = failedHooks.length; - - if (errorCount > 0) { - const failedNames = failedHooks - .map((r) => this.getHookNameFromResult(r)) - .join(', '); - - let shouldEmit = true; - if (requestContext) { - let reportedSet = this.reportedFailures.get(requestContext); - if (!reportedSet) { - reportedSet = new Set(); - this.reportedFailures.set(requestContext, reportedSet); - } - - const failureKey = `${eventName}:${failedNames}`; - if (reportedSet.has(failureKey)) { - shouldEmit = false; - } else { - reportedSet.add(failureKey); - } - } - - debugLogger.warn( - `Hook execution for ${eventName}: ${successCount} succeeded, ${errorCount} failed (${failedNames}), ` + - `total duration: ${aggregated.totalDuration}ms`, - ); - - if (shouldEmit) { - debugLogger.warn( - `Hook(s) [${failedNames}] failed for event ${eventName}. Check debug logs for more details.`, - ); - } - } else { - debugLogger.debug( - `Hook execution for ${eventName}: ${successCount} hooks executed successfully, ` + - `total duration: ${aggregated.totalDuration}ms`, - ); - } - - // Log individual errors - for (const error of aggregated.errors) { - debugLogger.warn(`Hook execution error: ${error.message}`); - } - } - /** * Process common hook output fields centrally */ @@ -381,21 +304,5 @@ export class HookEventHandler { // as they need to interpret this signal in the context of their specific workflow // This is just logging the request centrally } - - // Other common fields like decision/reason are handled by specific hook output classes - } - - /** - * Get hook name from config for display or telemetry - */ - private getHookName(config: HookConfig): string { - return config.name || config.command || 'unknown-command'; - } - - /** - * Get hook name from execution result for telemetry - */ - private getHookNameFromResult(result: HookExecutionResult): string { - return this.getHookName(result.hookConfig); } } diff --git a/packages/core/src/hooks/hookSystem.test.ts b/packages/core/src/hooks/hookSystem.test.ts new file mode 100644 index 000000000..d2558c591 --- /dev/null +++ b/packages/core/src/hooks/hookSystem.test.ts @@ -0,0 +1,628 @@ +/** + * @license + * Copyright 2026 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { HookSystem } from './hookSystem.js'; +import { HookRegistry } from './hookRegistry.js'; +import { HookRunner } from './hookRunner.js'; +import { HookAggregator } from './hookAggregator.js'; +import { HookPlanner } from './hookPlanner.js'; +import { HookEventHandler } from './hookEventHandler.js'; +import { + SessionStartSource, + SessionEndReason, + PreCompactTrigger, + HookType, + HooksConfigSource, + NotificationType, +} from './types.js'; +import type { Config } from '../config/config.js'; + +vi.mock('./hookRegistry.js'); +vi.mock('./hookRunner.js'); +vi.mock('./hookAggregator.js'); +vi.mock('./hookPlanner.js'); +vi.mock('./hookEventHandler.js'); + +describe('HookSystem', () => { + let mockConfig: Config; + let mockHookRegistry: HookRegistry; + let mockHookRunner: HookRunner; + let mockHookAggregator: HookAggregator; + let mockHookPlanner: HookPlanner; + let mockHookEventHandler: HookEventHandler; + let hookSystem: HookSystem; + + beforeEach(() => { + mockConfig = { + getSessionId: vi.fn().mockReturnValue('test-session-id'), + getTranscriptPath: vi.fn().mockReturnValue('/test/transcript'), + getWorkingDir: vi.fn().mockReturnValue('/test/cwd'), + } as unknown as Config; + + mockHookRegistry = { + initialize: vi.fn().mockResolvedValue(undefined), + setHookEnabled: vi.fn(), + getAllHooks: vi.fn().mockReturnValue([]), + } as unknown as HookRegistry; + + mockHookRunner = { + executeHooksSequential: vi.fn(), + executeHooksParallel: vi.fn(), + } as unknown as HookRunner; + + mockHookAggregator = { + aggregateResults: vi.fn(), + } as unknown as HookAggregator; + + mockHookPlanner = { + createExecutionPlan: vi.fn(), + } as unknown as HookPlanner; + + mockHookEventHandler = { + fireSessionStartEvent: vi.fn(), + fireSessionEndEvent: vi.fn(), + firePreCompactEvent: vi.fn(), + fireUserPromptSubmitEvent: vi.fn(), + fireStopEvent: vi.fn(), + firePreToolUseEvent: vi.fn(), + firePostToolUseEvent: vi.fn(), + fireNotificationEvent: vi.fn(), + } as unknown as HookEventHandler; + + vi.mocked(HookRegistry).mockImplementation(() => mockHookRegistry); + vi.mocked(HookRunner).mockImplementation(() => mockHookRunner); + vi.mocked(HookAggregator).mockImplementation(() => mockHookAggregator); + vi.mocked(HookPlanner).mockImplementation(() => mockHookPlanner); + vi.mocked(HookEventHandler).mockImplementation(() => mockHookEventHandler); + + hookSystem = new HookSystem(mockConfig); + }); + + describe('constructor', () => { + it('should create instance with all dependencies', () => { + expect(HookRegistry).toHaveBeenCalledWith(mockConfig); + expect(HookRunner).toHaveBeenCalled(); + expect(HookAggregator).toHaveBeenCalled(); + expect(HookPlanner).toHaveBeenCalledWith(mockHookRegistry); + expect(HookEventHandler).toHaveBeenCalledWith( + mockConfig, + mockHookPlanner, + mockHookRunner, + mockHookAggregator, + ); + }); + }); + + describe('initialize', () => { + it('should initialize hook registry', async () => { + await hookSystem.initialize(); + + expect(mockHookRegistry.initialize).toHaveBeenCalled(); + }); + }); + + describe('getEventHandler', () => { + it('should return the hook event handler', () => { + const eventHandler = hookSystem.getEventHandler(); + + expect(eventHandler).toBe(mockHookEventHandler); + }); + }); + + describe('getRegistry', () => { + it('should return the hook registry', () => { + const registry = hookSystem.getRegistry(); + + expect(registry).toBe(mockHookRegistry); + }); + }); + + describe('setHookEnabled', () => { + it('should enable a hook', () => { + hookSystem.setHookEnabled('test-hook', true); + + expect(mockHookRegistry.setHookEnabled).toHaveBeenCalledWith( + 'test-hook', + true, + ); + }); + + it('should disable a hook', () => { + hookSystem.setHookEnabled('test-hook', false); + + expect(mockHookRegistry.setHookEnabled).toHaveBeenCalledWith( + 'test-hook', + false, + ); + }); + }); + + describe('getAllHooks', () => { + it('should return all registered hooks', () => { + const mockHooks = [ + { + name: 'hook1', + config: { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + enabled: true, + }, + ]; + vi.mocked(mockHookRegistry.getAllHooks).mockReturnValue(mockHooks); + + const hooks = hookSystem.getAllHooks(); + + expect(hooks).toEqual(mockHooks); + expect(mockHookRegistry.getAllHooks).toHaveBeenCalled(); + }); + }); + + describe('fireSessionStartEvent', () => { + it('should fire session start event and return output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 100, + finalOutput: { + continue: true, + }, + }; + vi.mocked(mockHookEventHandler.fireSessionStartEvent).mockResolvedValue( + mockResult, + ); + + const result = await hookSystem.fireSessionStartEvent( + SessionStartSource.Startup, + ); + + expect(mockHookEventHandler.fireSessionStartEvent).toHaveBeenCalledWith( + SessionStartSource.Startup, + ); + expect(result).toBeDefined(); + }); + + it('should return undefined when no final output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + finalOutput: undefined, + }; + vi.mocked(mockHookEventHandler.fireSessionStartEvent).mockResolvedValue( + mockResult, + ); + + const result = await hookSystem.fireSessionStartEvent( + SessionStartSource.Resume, + ); + + expect(result).toBeUndefined(); + }); + }); + + describe('fireSessionEndEvent', () => { + it('should fire session end event', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 100, + }; + vi.mocked(mockHookEventHandler.fireSessionEndEvent).mockResolvedValue( + mockResult, + ); + + const result = await hookSystem.fireSessionEndEvent( + SessionEndReason.Clear, + ); + + expect(mockHookEventHandler.fireSessionEndEvent).toHaveBeenCalledWith( + SessionEndReason.Clear, + ); + expect(result).toEqual(mockResult); + }); + }); + + describe('firePreCompactEvent', () => { + it('should fire pre compact event', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 100, + }; + vi.mocked(mockHookEventHandler.firePreCompactEvent).mockResolvedValue( + mockResult, + ); + + const result = await hookSystem.firePreCompactEvent( + PreCompactTrigger.Manual, + ); + + expect(mockHookEventHandler.firePreCompactEvent).toHaveBeenCalledWith( + PreCompactTrigger.Manual, + ); + expect(result).toEqual(mockResult); + }); + }); + + describe('fireUserPromptSubmitEvent', () => { + it('should fire user prompt submit event and return output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 50, + finalOutput: { + continue: true, + }, + }; + vi.mocked( + mockHookEventHandler.fireUserPromptSubmitEvent, + ).mockResolvedValue(mockResult); + + const result = await hookSystem.fireUserPromptSubmitEvent('test prompt'); + + expect( + mockHookEventHandler.fireUserPromptSubmitEvent, + ).toHaveBeenCalledWith('test prompt'); + expect(result).toBeDefined(); + }); + + it('should return undefined when no final output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + finalOutput: undefined, + }; + vi.mocked( + mockHookEventHandler.fireUserPromptSubmitEvent, + ).mockResolvedValue(mockResult); + + const result = await hookSystem.fireUserPromptSubmitEvent('test prompt'); + + expect(result).toBeUndefined(); + }); + }); + + describe('fireStopEvent', () => { + it('should fire stop event and return output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 50, + finalOutput: { + continue: false, + stopReason: 'user_stop', + }, + }; + vi.mocked(mockHookEventHandler.fireStopEvent).mockResolvedValue( + mockResult, + ); + + const result = await hookSystem.fireStopEvent(true, 'last message'); + + expect(mockHookEventHandler.fireStopEvent).toHaveBeenCalledWith( + true, + 'last message', + ); + expect(result).toBeDefined(); + }); + + it('should use default parameters when not provided', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + finalOutput: undefined, + }; + vi.mocked(mockHookEventHandler.fireStopEvent).mockResolvedValue( + mockResult, + ); + + await hookSystem.fireStopEvent(); + + expect(mockHookEventHandler.fireStopEvent).toHaveBeenCalledWith( + false, + '', + ); + }); + }); + + describe('firePreToolUseEvent', () => { + it('should fire pre tool use event and return output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 100, + finalOutput: { + decision: 'allow', + }, + }; + vi.mocked(mockHookEventHandler.firePreToolUseEvent).mockResolvedValue( + mockResult, + ); + + const result = await hookSystem.firePreToolUseEvent('Read', { + path: '/test.txt', + }); + + expect(mockHookEventHandler.firePreToolUseEvent).toHaveBeenCalledWith( + 'Read', + { path: '/test.txt' }, + undefined, + ); + expect(result).toBeDefined(); + }); + + it('should include mcpContext when provided', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 100, + finalOutput: { + decision: 'allow', + }, + }; + const mcpContext = { + server_name: 'test-server', + tool_name: 'mcp-tool', + command: 'npx', + }; + vi.mocked(mockHookEventHandler.firePreToolUseEvent).mockResolvedValue( + mockResult, + ); + + await hookSystem.firePreToolUseEvent( + 'Bash', + { command: 'ls' }, + mcpContext, + ); + + expect(mockHookEventHandler.firePreToolUseEvent).toHaveBeenCalledWith( + 'Bash', + { command: 'ls' }, + mcpContext, + ); + }); + + it('should return undefined when error occurs', async () => { + vi.mocked(mockHookEventHandler.firePreToolUseEvent).mockRejectedValue( + new Error('Hook error'), + ); + + const result = await hookSystem.firePreToolUseEvent('Read', { + path: '/test.txt', + }); + + expect(result).toBeUndefined(); + }); + + it('should return undefined when no final output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + finalOutput: undefined, + }; + vi.mocked(mockHookEventHandler.firePreToolUseEvent).mockResolvedValue( + mockResult, + ); + + const result = await hookSystem.firePreToolUseEvent('Read', {}); + + expect(result).toBeUndefined(); + }); + }); + + describe('firePostToolUseEvent', () => { + it('should fire post tool use event and return output', async () => { + const mockResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 100, + finalOutput: { + decision: 'allow', + }, + }; + vi.mocked(mockHookEventHandler.firePostToolUseEvent).mockResolvedValue( + mockResult, + ); + + const toolResponse = { + llmContent: 'file content', + returnDisplay: true, + error: null, + }; + + const result = await hookSystem.firePostToolUseEvent( + 'Read', + { path: '/test.txt' }, + toolResponse, + ); + + expect(mockHookEventHandler.firePostToolUseEvent).toHaveBeenCalledWith( + 'Read', + { path: '/test.txt' }, + toolResponse, + undefined, + ); + expect(result).toBeDefined(); + }); + + it('should return undefined when error occurs', async () => { + vi.mocked(mockHookEventHandler.firePostToolUseEvent).mockRejectedValue( + new Error('Hook error'), + ); + + const result = await hookSystem.firePostToolUseEvent( + 'Read', + {}, + { llmContent: null, returnDisplay: false, error: null }, + ); + + expect(result).toBeUndefined(); + }); + }); + + describe('fireToolNotificationEvent', () => { + it('should fire notification event for edit type', async () => { + vi.mocked(mockHookEventHandler.fireNotificationEvent).mockResolvedValue({ + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + }); + + const confirmationDetails = { + type: 'edit' as const, + title: 'Edit File', + fileName: 'test.txt', + filePath: '/test/test.txt', + fileDiff: 'diff', + originalContent: 'old', + newContent: 'new', + isModifying: true, + }; + + await hookSystem.fireToolNotificationEvent(confirmationDetails); + + expect(mockHookEventHandler.fireNotificationEvent).toHaveBeenCalledWith( + NotificationType.ToolPermission, + 'Tool Edit File requires editing', + { + type: 'edit', + title: 'Edit File', + fileName: 'test.txt', + filePath: '/test/test.txt', + fileDiff: 'diff', + originalContent: 'old', + newContent: 'new', + isModifying: true, + }, + ); + }); + + it('should fire notification event for exec type', async () => { + vi.mocked(mockHookEventHandler.fireNotificationEvent).mockResolvedValue({ + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + }); + + const confirmationDetails = { + type: 'exec' as const, + title: 'Run Command', + command: 'ls -la', + rootCommand: 'ls', + }; + + await hookSystem.fireToolNotificationEvent(confirmationDetails); + + expect(mockHookEventHandler.fireNotificationEvent).toHaveBeenCalledWith( + NotificationType.ToolPermission, + 'Tool Run Command requires execution', + { + type: 'exec', + title: 'Run Command', + command: 'ls -la', + rootCommand: 'ls', + }, + ); + }); + + it('should fire notification event for mcp type', async () => { + vi.mocked(mockHookEventHandler.fireNotificationEvent).mockResolvedValue({ + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + }); + + const confirmationDetails = { + type: 'mcp' as const, + title: 'MCP Tool', + serverName: 'test-server', + toolName: 'mcp-tool', + toolDisplayName: 'MCP Tool', + }; + + await hookSystem.fireToolNotificationEvent(confirmationDetails); + + expect(mockHookEventHandler.fireNotificationEvent).toHaveBeenCalledWith( + NotificationType.ToolPermission, + 'Tool MCP Tool requires MCP', + { + type: 'mcp', + title: 'MCP Tool', + serverName: 'test-server', + toolName: 'mcp-tool', + toolDisplayName: 'MCP Tool', + }, + ); + }); + + it('should fire notification event for info type', async () => { + vi.mocked(mockHookEventHandler.fireNotificationEvent).mockResolvedValue({ + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + }); + + const confirmationDetails = { + type: 'info' as const, + title: 'Info Tool', + prompt: 'Some prompt', + urls: ['https://example.com'], + }; + + await hookSystem.fireToolNotificationEvent(confirmationDetails); + + expect(mockHookEventHandler.fireNotificationEvent).toHaveBeenCalledWith( + NotificationType.ToolPermission, + 'Tool Info Tool requires information', + { + type: 'info', + title: 'Info Tool', + prompt: 'Some prompt', + urls: ['https://example.com'], + }, + ); + }); + + it('should handle error gracefully', async () => { + vi.mocked(mockHookEventHandler.fireNotificationEvent).mockRejectedValue( + new Error('Notification error'), + ); + + const confirmationDetails = { + type: 'info' as const, + title: 'Info Tool', + prompt: 'Some prompt', + urls: [], + }; + + await expect( + hookSystem.fireToolNotificationEvent(confirmationDetails), + ).resolves.not.toThrow(); + }); + }); +}); diff --git a/packages/core/src/hooks/hookSystem.ts b/packages/core/src/hooks/hookSystem.ts index 4dea42753..22d5dfa58 100644 --- a/packages/core/src/hooks/hookSystem.ts +++ b/packages/core/src/hooks/hookSystem.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2025 Google LLC + * Copyright 2026 Qwen * SPDX-License-Identifier: Apache-2.0 */ @@ -188,14 +188,12 @@ export class HookSystem { } async fireStopEvent( - prompt: string, - response: string, stopHookActive: boolean = false, + lastAssistantMessage: string = '', ): Promise { const result = await this.hookEventHandler.fireStopEvent( - prompt, - response, stopHookActive, + lastAssistantMessage, ); return result.finalOutput ? createHookOutput('Stop', result.finalOutput) diff --git a/packages/core/src/hooks/types.ts b/packages/core/src/hooks/types.ts index 573401b9b..66510d86b 100644 --- a/packages/core/src/hooks/types.ts +++ b/packages/core/src/hooks/types.ts @@ -497,9 +497,8 @@ export interface NotificationOutput extends HookOutput { * Stop hook input */ export interface StopInput extends HookInput { - prompt: string; - prompt_response: string; stop_hook_active: boolean; + last_assistant_message: string; } /**