diff --git a/packages/core/src/hooks/hookEventHandler.test.ts b/packages/core/src/hooks/hookEventHandler.test.ts index 3dbc4ae881..b9ae878e76 100644 --- a/packages/core/src/hooks/hookEventHandler.test.ts +++ b/packages/core/src/hooks/hookEventHandler.test.ts @@ -4,6 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type { + GenerateContentParameters, + GenerateContentResponse, +} from '@google/genai'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { HookEventHandler } from './hookEventHandler.js'; import type { Config } from '../config/config.js'; @@ -776,6 +780,68 @@ describe('HookEventHandler', () => { }); }); + describe('failure suppression', () => { + it('should suppress duplicate feedback for the same failing hook and request context', async () => { + const mockHook: HookConfig = { + type: HookType.Command, + command: './fail.sh', + name: 'failing-hook', + }; + const mockResults: HookExecutionResult[] = [ + { + success: false, + duration: 10, + hookConfig: mockHook, + eventName: HookEventName.AfterModel, + error: new Error('Failed'), + }, + ]; + const mockAggregated = { + success: false, + allOutputs: [], + errors: [new Error('Failed')], + totalDuration: 10, + }; + + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue({ + eventName: HookEventName.AfterModel, + hookConfigs: [mockHook], + sequential: false, + }); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue( + mockResults, + ); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + mockAggregated, + ); + + const llmRequest = { model: 'test', contents: [] }; + const llmResponse = { candidates: [] }; + + // First call - should emit feedback + await hookEventHandler.fireAfterModelEvent( + llmRequest as unknown as GenerateContentParameters, + llmResponse as unknown as GenerateContentResponse, + ); + expect(mockCoreEvents.emitFeedback).toHaveBeenCalledTimes(1); + + // Second call with SAME request - should NOT emit feedback + await hookEventHandler.fireAfterModelEvent( + llmRequest as unknown as GenerateContentParameters, + llmResponse as unknown as GenerateContentResponse, + ); + expect(mockCoreEvents.emitFeedback).toHaveBeenCalledTimes(1); + + // Third call with DIFFERENT request - should emit feedback again + const differentRequest = { model: 'different', contents: [] }; + await hookEventHandler.fireAfterModelEvent( + differentRequest as unknown as GenerateContentParameters, + llmResponse as unknown as GenerateContentResponse, + ); + expect(mockCoreEvents.emitFeedback).toHaveBeenCalledTimes(2); + }); + }); + describe('createBaseInput', () => { it('should create base input with correct fields', async () => { const mockPlan = [ diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index cae3c61625..3301ffb69d 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -49,6 +49,13 @@ 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, @@ -210,7 +217,12 @@ export class HookEventHandler { llm_request: defaultHookTranslator.toHookLLMRequest(llmRequest), }; - return this.executeHooks(HookEventName.BeforeModel, input); + return this.executeHooks( + HookEventName.BeforeModel, + input, + undefined, + llmRequest, + ); } /** @@ -227,7 +239,12 @@ export class HookEventHandler { llm_response: defaultHookTranslator.toHookLLMResponse(llmResponse), }; - return this.executeHooks(HookEventName.AfterModel, input); + return this.executeHooks( + HookEventName.AfterModel, + input, + undefined, + llmRequest, + ); } /** @@ -242,7 +259,12 @@ export class HookEventHandler { llm_request: defaultHookTranslator.toHookLLMRequest(llmRequest), }; - return this.executeHooks(HookEventName.BeforeToolSelection, input); + return this.executeHooks( + HookEventName.BeforeToolSelection, + input, + undefined, + llmRequest, + ); } /** @@ -253,6 +275,7 @@ export class HookEventHandler { eventName: HookEventName, input: HookInput, context?: HookEventContext, + requestContext?: object, ): Promise { try { // Create execution plan @@ -311,7 +334,13 @@ export class HookEventHandler { this.processCommonHookOutputFields(aggregated); // Log hook execution - this.logHookExecution(eventName, input, results, aggregated); + this.logHookExecution( + eventName, + input, + results, + aggregated, + requestContext, + ); return aggregated; } catch (error) { @@ -354,6 +383,7 @@ export class HookEventHandler { input: HookInput, results: HookExecutionResult[], aggregated: AggregatedHookResult, + requestContext?: object, ): void { const failedHooks = results.filter((r) => !r.success); const successCount = results.length - failedHooks.length; @@ -364,15 +394,33 @@ export class HookEventHandler { .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`, ); - coreEvents.emitFeedback( - 'warning', - `Hook(s) [${failedNames}] failed for event ${eventName}. Press F12 to see the debug drawer for more details.\n`, - ); + if (shouldEmit) { + coreEvents.emitFeedback( + 'warning', + `Hook(s) [${failedNames}] failed for event ${eventName}. Press F12 to see the debug drawer for more details.\n`, + ); + } } else { debugLogger.debug( `Hook execution for ${eventName}: ${successCount} hooks executed successfully, ` +