diff --git a/docs/users/features/hooks.md b/docs/users/features/hooks.md index 99b9d6370..84aa8ce04 100644 --- a/docs/users/features/hooks.md +++ b/docs/users/features/hooks.md @@ -57,9 +57,11 @@ The following table lists all available hook events in Qwen Code: | `UserPromptSubmit` | Fired when user submits a prompt | Input processing, validation, context injection | | `SessionStart` | Fired when a new session starts | Initialization, context setup | | `Stop` | Fired before Qwen concludes its response | Finalization, cleanup | +| `StopFailure` | Fired when turn ends due to API error | Error logging, alerting, rate limit handling | | `SubagentStart` | Fired when a subagent starts | Subagent initialization | | `SubagentStop` | Fired when a subagent stops | Subagent finalization | | `PreCompact` | Fired before conversation compaction | Pre-compaction processing | +| `PostCompact` | Fired after conversation compaction | Summary archiving, usage statistics | | `SessionEnd` | Fired when a session ends | Cleanup, reporting | | `PermissionRequest` | Fired when permission dialogs are displayed | Permission automation, policy enforcement | @@ -299,6 +301,60 @@ Event-specific fields are added based on the hook type. Below are the event-spec } ``` +#### StopFailure + +**Purpose**: Executed when the turn ends due to an API error (instead of Stop). This is a **fire-and-forget** event - hook output and exit codes are ignored. + +**Event-specific fields**: + +```json +{ + "error": "rate_limit | authentication_failed | billing_error | invalid_request | server_error | max_output_tokens | unknown", + "error_details": "detailed error message (optional)", + "last_assistant_message": "the last message from the assistant before the error (optional)" +} +``` + +**Matcher**: Matches against the `error` field. For example, `"matcher": "rate_limit"` will only trigger for rate limit errors. + +**Output Options**: + +- **None** - StopFailure is fire-and-forget. All hook output and exit codes are ignored. + +**Exit Code Handling**: + +| Exit Code | Behavior | +| --------- | ------------------------- | +| Any | Ignored (fire-and-forget) | + +**Example Configuration**: + +```json +{ + "hooks": { + "StopFailure": [ + { + "matcher": "rate_limit", + "hooks": [ + { + "type": "command", + "command": "/path/to/rate-limit-alert.sh", + "name": "rate-limit-alerter" + } + ] + } + ] + } +} +``` + +**Use Cases**: + +- Rate limit monitoring and alerting +- Authentication failure logging +- Billing error notifications +- Error statistics collection + #### SubagentStart **Purpose**: Executed when a subagent (like the Task tool) is started to set up context or permissions. @@ -387,6 +443,63 @@ Event-specific fields are added based on the hook type. Below are the event-spec } ``` +#### PostCompact + +**Purpose**: Executed after conversation compaction completes to archive summaries or track usage. + +**Event-specific fields**: + +```json +{ + "trigger": "manual | auto", + "compact_summary": "the summary generated by the compaction process" +} +``` + +**Matcher**: Matches against the `trigger` field. For example, `"matcher": "manual"` will only trigger for manual compaction via `/compact` command. + +**Output Options**: + +- `hookSpecificOutput.additionalContext`: additional context (for logging only) +- Standard hook output fields (for logging only) + +**Note**: PostCompact is **not** in the official decision mode supported events list. The `decision` field and other control fields do not produce any control effects - they are only used for logging purposes. + +**Exit Code Handling**: + +| Exit Code | Behavior | +| --------- | --------------------------------------------------------- | +| 0 | Success - stdout shown to user in verbose mode | +| Other | Non-blocking error - stderr shown to user in verbose mode | + +**Example Configuration**: + +```json +{ + "hooks": { + "PostCompact": [ + { + "matcher": "manual", + "hooks": [ + { + "type": "command", + "command": "/path/to/save-compact-summary.sh", + "name": "save-summary" + } + ] + } + ] + } +} +``` + +**Use Cases**: + +- Summary archiving to files or databases +- Usage statistics tracking +- Context change monitoring +- Audit logging for compaction operations + #### Notification **Purpose**: Executed when notifications are sent to customize or intercept them. diff --git a/packages/cli/src/ui/components/hooks/constants.test.ts b/packages/cli/src/ui/components/hooks/constants.test.ts index e9bbc705a..0b49c7136 100644 --- a/packages/cli/src/ui/components/hooks/constants.test.ts +++ b/packages/cli/src/ui/components/hooks/constants.test.ts @@ -163,6 +163,7 @@ describe('hooks constants', () => { describe('DISPLAY_HOOK_EVENTS', () => { it('should contain all expected hook events', () => { expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.Stop); + expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.StopFailure); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PreToolUse); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PostToolUse); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PostToolUseFailure); @@ -173,11 +174,12 @@ describe('hooks constants', () => { expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.SubagentStart); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.SubagentStop); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PreCompact); + expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PostCompact); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PermissionRequest); }); - it('should have 12 events', () => { - expect(DISPLAY_HOOK_EVENTS).toHaveLength(12); + it('should have 14 events', () => { + expect(DISPLAY_HOOK_EVENTS).toHaveLength(14); }); }); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index 9c0c54ee1..498c1d049 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -8,7 +8,7 @@ import type { Mock, MockInstance } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderHook, act, waitFor } from '@testing-library/react'; -import { useGeminiStream } from './useGeminiStream.js'; +import { useGeminiStream, classifyApiError } from './useGeminiStream.js'; import * as atCommandProcessor from './atCommandProcessor.js'; import type { TrackedToolCall, @@ -3512,3 +3512,115 @@ describe('useGeminiStream', () => { }); }); }); + +describe('classifyApiError', () => { + it('should classify rate limit errors by status code 429', () => { + expect(classifyApiError({ message: 'error', status: 429 })).toBe( + 'rate_limit', + ); + }); + + it('should classify rate limit errors by message', () => { + expect(classifyApiError({ message: 'Rate limit exceeded' })).toBe( + 'rate_limit', + ); + }); + + it('should classify authentication errors by status code 401', () => { + expect(classifyApiError({ message: 'error', status: 401 })).toBe( + 'authentication_failed', + ); + }); + + it('should classify authentication errors by message', () => { + expect(classifyApiError({ message: 'Unauthorized access' })).toBe( + 'authentication_failed', + ); + }); + + it('should classify billing errors by status code 402', () => { + expect(classifyApiError({ message: 'error', status: 402 })).toBe( + 'billing_error', + ); + }); + + it('should classify billing errors by status code 403', () => { + expect(classifyApiError({ message: 'error', status: 403 })).toBe( + 'billing_error', + ); + }); + + it('should classify billing errors by message containing billing', () => { + expect(classifyApiError({ message: 'Billing issue detected' })).toBe( + 'billing_error', + ); + }); + + it('should classify billing errors by message containing quota', () => { + expect(classifyApiError({ message: 'Quota exceeded' })).toBe( + 'billing_error', + ); + }); + + it('should classify invalid request errors by status code 400', () => { + expect(classifyApiError({ message: 'error', status: 400 })).toBe( + 'invalid_request', + ); + }); + + it('should classify invalid request errors by message', () => { + expect(classifyApiError({ message: 'Invalid request format' })).toBe( + 'invalid_request', + ); + }); + + it('should classify server errors by status code 500', () => { + expect(classifyApiError({ message: 'error', status: 500 })).toBe( + 'server_error', + ); + }); + + it('should classify server errors by status code 502', () => { + expect(classifyApiError({ message: 'error', status: 502 })).toBe( + 'server_error', + ); + }); + + it('should classify server errors by status code 503', () => { + expect(classifyApiError({ message: 'error', status: 503 })).toBe( + 'server_error', + ); + }); + + it('should classify max output tokens errors by message', () => { + expect(classifyApiError({ message: 'max_tokens limit reached' })).toBe( + 'max_output_tokens', + ); + }); + + it('should classify token limit errors by message', () => { + expect(classifyApiError({ message: 'Token limit exceeded' })).toBe( + 'max_output_tokens', + ); + }); + + it('should return unknown for unrecognized errors', () => { + expect(classifyApiError({ message: 'Some random error' })).toBe('unknown'); + }); + + it('should return unknown for empty message', () => { + expect(classifyApiError({ message: '' })).toBe('unknown'); + }); + + it('should handle case insensitive matching', () => { + expect(classifyApiError({ message: 'RATE LIMIT exceeded' })).toBe( + 'rate_limit', + ); + expect(classifyApiError({ message: 'UNAUTHORIZED' })).toBe( + 'authentication_failed', + ); + expect(classifyApiError({ message: 'BILLING error' })).toBe( + 'billing_error', + ); + }); +}); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index ce0537a92..b4e757dd9 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -17,6 +17,7 @@ import type { ThoughtSummary, ToolCallRequestInfo, GeminiErrorEventValue, + StopFailureErrorType, } from '@qwen-code/qwen-code-core'; import { GeminiEventType as ServerGeminiEventType, @@ -78,6 +79,43 @@ import { t } from '../../i18n/index.js'; const debugLogger = createDebugLogger('GEMINI_STREAM'); +/** + * Classify API error to StopFailureErrorType + * @internal Exported for testing purposes + */ +export function classifyApiError(error: { + message: string; + status?: number; +}): StopFailureErrorType { + const status = error.status; + const message = error.message?.toLowerCase() ?? ''; + + if (status === 429 || message.includes('rate limit')) { + return 'rate_limit'; + } + if (status === 401 || message.includes('unauthorized')) { + return 'authentication_failed'; + } + if ( + status === 402 || + status === 403 || + message.includes('billing') || + message.includes('quota') + ) { + return 'billing_error'; + } + if (status === 400 || message.includes('invalid')) { + return 'invalid_request'; + } + if (status !== undefined && status >= 500) { + return 'server_error'; + } + if (message.includes('max_tokens') || message.includes('token limit')) { + return 'max_output_tokens'; + } + return 'unknown'; +} + /** * Checks if image parts have supported formats and returns unsupported ones */ @@ -795,6 +833,12 @@ export const useGeminiStream = ( // (auto-retry countdown is shown when retryCountdownTimerRef is active) const isShowingAutoRetry = retryCountdownTimerRef.current !== null; clearRetryCountdown(); + + const formattedErrorText = parseAndFormatApiError( + eventValue.error, + config.getContentGeneratorConfig()?.authType, + ); + if (!isShowingAutoRetry) { const retryHint = t('Press Ctrl+Y to retry'); // Store error with hint as a pending item (not in history). @@ -802,14 +846,24 @@ export const useGeminiStream = ( // since pending items are in the dynamic rendering area (not ). setPendingRetryErrorItem({ type: 'error' as const, - text: parseAndFormatApiError( - eventValue.error, - config.getContentGeneratorConfig()?.authType, - ), + text: formattedErrorText, hint: retryHint, }); } setThought(null); // Reset thought when there's an error + + // Fire StopFailure hook (fire-and-forget, replaces Stop event for API errors) + const errorType = classifyApiError(eventValue.error); + config + .getHookSystem() + ?.fireStopFailureEvent( + errorType, + eventValue.error.message, + formattedErrorText, + ) + .catch((err) => { + debugLogger.warn(`StopFailure hook failed: ${err}`); + }); }, [ addItem, diff --git a/packages/core/src/hooks/hookAggregator.test.ts b/packages/core/src/hooks/hookAggregator.test.ts index 5667d5654..f270a845b 100644 --- a/packages/core/src/hooks/hookAggregator.test.ts +++ b/packages/core/src/hooks/hookAggregator.test.ts @@ -794,4 +794,179 @@ describe('HookAggregator', () => { expect(hookOutput.isBlockingDecision()).toBe(false); }); }); + + describe('StopFailure - fire-and-forget special handling', () => { + it('should always return success true for StopFailure', () => { + const results: HookExecutionResult[] = [ + { + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.StopFailure, + success: false, + error: new Error('Hook failed'), + duration: 100, + }, + ]; + + const result = aggregator.aggregateResults( + results, + HookEventName.StopFailure, + ); + expect(result.success).toBe(true); + }); + + it('should ignore all outputs for StopFailure', () => { + const outputs: HookOutput[] = [ + { decision: 'block', reason: 'should be ignored' }, + { continue: false, stopReason: 'also ignored' }, + ]; + + const results: HookExecutionResult[] = outputs.map((output) => ({ + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.StopFailure, + success: true, + output, + duration: 100, + })); + + const result = aggregator.aggregateResults( + results, + HookEventName.StopFailure, + ); + expect(result.allOutputs).toEqual([]); + expect(result.finalOutput).toBeUndefined(); + }); + + it('should ignore all errors for StopFailure', () => { + const results: HookExecutionResult[] = [ + { + hookConfig: { type: HookType.Command, command: 'hook1.sh' }, + eventName: HookEventName.StopFailure, + success: false, + error: new Error('First error'), + duration: 50, + }, + { + hookConfig: { type: HookType.Command, command: 'hook2.sh' }, + eventName: HookEventName.StopFailure, + success: false, + error: new Error('Second error'), + duration: 75, + }, + ]; + + const result = aggregator.aggregateResults( + results, + HookEventName.StopFailure, + ); + expect(result.success).toBe(true); + expect(result.errors).toEqual([]); + }); + + it('should calculate total duration for StopFailure', () => { + const results: HookExecutionResult[] = [ + { + hookConfig: { type: HookType.Command, command: 'hook1.sh' }, + eventName: HookEventName.StopFailure, + success: true, + duration: 100, + }, + { + hookConfig: { type: HookType.Command, command: 'hook2.sh' }, + eventName: HookEventName.StopFailure, + success: true, + duration: 200, + }, + ]; + + const result = aggregator.aggregateResults( + results, + HookEventName.StopFailure, + ); + expect(result.totalDuration).toBe(300); + }); + + it('should return empty result for StopFailure with no hooks', () => { + const result = aggregator.aggregateResults([], HookEventName.StopFailure); + expect(result.success).toBe(true); + expect(result.allOutputs).toEqual([]); + expect(result.errors).toEqual([]); + expect(result.totalDuration).toBe(0); + expect(result.finalOutput).toBeUndefined(); + }); + }); + + describe('PostCompact - mergeSimple', () => { + it('should use mergeSimple for PostCompact event', () => { + const outputs: HookOutput[] = [ + { reason: 'first', continue: true }, + { reason: 'second', continue: false }, + ]; + + const results: HookExecutionResult[] = outputs.map((output) => ({ + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.PostCompact, + success: true, + output, + duration: 100, + })); + + const result = aggregator.aggregateResults( + results, + HookEventName.PostCompact, + ); + // mergeSimple uses later values for simple fields + expect(result.finalOutput?.reason).toBe('second'); + expect(result.finalOutput?.continue).toBe(false); + }); + + it('should concatenate additionalContext for PostCompact', () => { + const outputs: HookOutput[] = [ + { hookSpecificOutput: { additionalContext: 'context 1' } }, + { hookSpecificOutput: { additionalContext: 'context 2' } }, + ]; + + const results: HookExecutionResult[] = outputs.map((output) => ({ + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.PostCompact, + success: true, + output, + duration: 100, + })); + + const result = aggregator.aggregateResults( + results, + HookEventName.PostCompact, + ); + expect( + result.finalOutput?.hookSpecificOutput?.['additionalContext'], + ).toBe('context 1\ncontext 2'); + }); + + it('should handle single output for PostCompact', () => { + const output: HookOutput = { + hookSpecificOutput: { + hookEventName: 'PostCompact', + additionalContext: 'single context', + }, + }; + const results: HookExecutionResult[] = [ + { + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.PostCompact, + success: true, + output, + duration: 100, + }, + ]; + + const result = aggregator.aggregateResults( + results, + HookEventName.PostCompact, + ); + expect(result.finalOutput).toBeDefined(); + expect( + result.finalOutput?.hookSpecificOutput?.['additionalContext'], + ).toBe('single context'); + }); + }); }); diff --git a/packages/core/src/hooks/hookAggregator.ts b/packages/core/src/hooks/hookAggregator.ts index 32da197cb..544b65180 100644 --- a/packages/core/src/hooks/hookAggregator.ts +++ b/packages/core/src/hooks/hookAggregator.ts @@ -40,6 +40,21 @@ export class HookAggregator { results: HookExecutionResult[], eventName: HookEventName, ): AggregatedHookResult { + // StopFailure special handling: fire-and-forget, ignore all outputs and errors + if (eventName === HookEventName.StopFailure) { + let totalDuration = 0; + for (const result of results) { + totalDuration += result.duration; + } + return { + success: true, // Always return success + allOutputs: [], + errors: [], // Ignore errors + totalDuration, + finalOutput: undefined, + }; + } + const allOutputs: HookOutput[] = []; const errors: Error[] = []; let totalDuration = 0; diff --git a/packages/core/src/hooks/hookEventHandler.test.ts b/packages/core/src/hooks/hookEventHandler.test.ts index bf22f4cc9..996c683c0 100644 --- a/packages/core/src/hooks/hookEventHandler.test.ts +++ b/packages/core/src/hooks/hookEventHandler.test.ts @@ -15,8 +15,10 @@ import { PermissionMode, AgentType, PreCompactTrigger, + PostCompactTrigger, NotificationType, } from './types.js'; +import type { StopFailureErrorType } from './types.js'; import type { Config } from '../config/config.js'; import type { HookPlanner, @@ -2254,6 +2256,340 @@ describe('HookEventHandler', () => { }); }); + describe('fireStopFailureEvent', () => { + it('should execute hooks for StopFailure 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.fireStopFailureEvent( + 'rate_limit', + '429 Too Many Requests', + 'API Error: Rate limit reached', + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.StopFailure, + { error: 'rate_limit' }, + ); + expect(result.success).toBe(true); + }); + + it('should include all parameters 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.fireStopFailureEvent( + 'authentication_failed', + '401 Unauthorized', + 'Please check your API key', + ); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { + error: StopFailureErrorType; + error_details: string; + last_assistant_message: string; + hook_event_name: string; + }; + + expect(input.error).toBe('authentication_failed'); + expect(input.error_details).toBe('401 Unauthorized'); + expect(input.last_assistant_message).toBe('Please check your API key'); + expect(input.hook_event_name).toBe(HookEventName.StopFailure); + }); + + it('should pass error type as context for matcher filtering', 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, + ); + + await hookEventHandler.fireStopFailureEvent('server_error'); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.StopFailure, + { error: 'server_error' }, + ); + }); + + it('should handle all error types correctly', 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), + ); + + const errorTypes: StopFailureErrorType[] = [ + 'rate_limit', + 'authentication_failed', + 'billing_error', + 'invalid_request', + 'server_error', + 'max_output_tokens', + 'unknown', + ]; + + for (const errorType of errorTypes) { + await hookEventHandler.fireStopFailureEvent(errorType); + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[mockCalls.length - 1][2] as { + error: StopFailureErrorType; + }; + expect(input.error).toBe(errorType); + } + }); + + it('should handle optional parameters', 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), + ); + + // Call with only required parameter + await hookEventHandler.fireStopFailureEvent('unknown'); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { + error: StopFailureErrorType; + error_details?: string; + last_assistant_message?: string; + }; + + expect(input.error).toBe('unknown'); + expect(input.error_details).toBeUndefined(); + expect(input.last_assistant_message).toBeUndefined(); + }); + + it('should handle errors gracefully', async () => { + vi.mocked(mockHookPlanner.createExecutionPlan).mockImplementation(() => { + throw new Error('StopFailure planner error'); + }); + + const result = await hookEventHandler.fireStopFailureEvent('rate_limit'); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('StopFailure planner error'); + }); + }); + + describe('firePostCompactEvent', () => { + it('should execute hooks for PostCompact event with manual trigger', 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.firePostCompactEvent( + PostCompactTrigger.Manual, + 'Summary of compacted conversation', + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.PostCompact, + { trigger: PostCompactTrigger.Manual }, + ); + expect(result.success).toBe(true); + }); + + it('should execute hooks for PostCompact event with auto trigger', 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.firePostCompactEvent( + PostCompactTrigger.Auto, + 'Auto-generated summary', + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.PostCompact, + { trigger: PostCompactTrigger.Auto }, + ); + expect(result.success).toBe(true); + }); + + it('should include all parameters 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), + ); + + const summary = 'The user requested to implement a new feature...'; + await hookEventHandler.firePostCompactEvent( + PostCompactTrigger.Manual, + summary, + ); + + const mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock + .calls; + const input = mockCalls[0][2] as { + trigger: PostCompactTrigger; + compact_summary: string; + hook_event_name: string; + }; + + expect(input.trigger).toBe(PostCompactTrigger.Manual); + expect(input.compact_summary).toBe(summary); + expect(input.hook_event_name).toBe(HookEventName.PostCompact); + }); + + it('should pass trigger as context for matcher filtering', 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, + ); + + await hookEventHandler.firePostCompactEvent( + PostCompactTrigger.Auto, + 'summary', + ); + + expect(mockHookPlanner.createExecutionPlan).toHaveBeenCalledWith( + HookEventName.PostCompact, + { trigger: PostCompactTrigger.Auto }, + ); + }); + + 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.firePostCompactEvent( + PostCompactTrigger.Manual, + 'summary', + ); + + expect(mockHookRunner.executeHooksSequential).toHaveBeenCalled(); + expect(mockHookRunner.executeHooksParallel).not.toHaveBeenCalled(); + }); + + it('should handle errors gracefully', async () => { + vi.mocked(mockHookPlanner.createExecutionPlan).mockImplementation(() => { + throw new Error('PostCompact planner error'); + }); + + const result = await hookEventHandler.firePostCompactEvent( + PostCompactTrigger.Auto, + 'summary', + ); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('PostCompact planner error'); + }); + + it('should handle both trigger types correctly', 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), + ); + + // Test manual trigger + await hookEventHandler.firePostCompactEvent( + PostCompactTrigger.Manual, + 'manual summary', + ); + let mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock.calls; + let input = mockCalls[mockCalls.length - 1][2] as { + trigger: PostCompactTrigger; + }; + expect(input.trigger).toBe(PostCompactTrigger.Manual); + + // Test auto trigger + await hookEventHandler.firePostCompactEvent( + PostCompactTrigger.Auto, + 'auto summary', + ); + mockCalls = (mockHookRunner.executeHooksParallel as Mock).mock.calls; + input = mockCalls[mockCalls.length - 1][2] as { + trigger: PostCompactTrigger; + }; + expect(input.trigger).toBe(PostCompactTrigger.Auto); + }); + }); + describe('telemetry', () => { const createMockHookExecutionResult = ( success: boolean, diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index ae3602ab9..97baf23f6 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -25,12 +25,16 @@ import type { PostToolUseFailureInput, PreCompactInput, PreCompactTrigger, + PostCompactInput, + PostCompactTrigger, NotificationInput, NotificationType, PermissionRequestInput, PermissionSuggestion, SubagentStartInput, SubagentStopInput, + StopFailureInput, + StopFailureErrorType, } from './types.js'; import { PermissionMode } from './types.js'; import { createDebugLogger } from '../utils/debugLogger.js'; @@ -394,6 +398,57 @@ export class HookEventHandler { ); } + /** + * Fire a StopFailure event + * Called when an API error ends the turn (instead of Stop) + * Fire-and-forget: output and exit codes are ignored + */ + async fireStopFailureEvent( + error: StopFailureErrorType, + errorDetails?: string, + lastAssistantMessage?: string, + signal?: AbortSignal, + ): Promise { + const input: StopFailureInput = { + ...this.createBaseInput(HookEventName.StopFailure), + error, + error_details: errorDetails, + last_assistant_message: lastAssistantMessage, + }; + + // Pass error type as context for matcher filtering (fieldToMatch: 'error') + return this.executeHooks( + HookEventName.StopFailure, + input, + { error }, + signal, + ); + } + + /** + * Fire a PostCompact event + * Called after conversation compaction completes + */ + async firePostCompactEvent( + trigger: PostCompactTrigger, + compactSummary: string, + signal?: AbortSignal, + ): Promise { + const input: PostCompactInput = { + ...this.createBaseInput(HookEventName.PostCompact), + trigger, + compact_summary: compactSummary, + }; + + // Pass trigger as context for matcher filtering + return this.executeHooks( + HookEventName.PostCompact, + input, + { trigger }, + signal, + ); + } + /** * Execute hooks for a specific event (direct execution without MessageBus) * Used as fallback when MessageBus is not available diff --git a/packages/core/src/hooks/hookPlanner.test.ts b/packages/core/src/hooks/hookPlanner.test.ts index 85b1aae56..ae9334068 100644 --- a/packages/core/src/hooks/hookPlanner.test.ts +++ b/packages/core/src/hooks/hookPlanner.test.ts @@ -713,5 +713,158 @@ describe('HookPlanner', () => { expect(result).not.toBeNull(); }); + + // StopFailure matcher tests + it('should match error type with exact string for StopFailure', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.StopFailure, + matcher: 'rate_limit', + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.StopFailure, { + error: 'rate_limit', + }); + + expect(result).not.toBeNull(); + }); + + it('should not match error type with different string for StopFailure', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.StopFailure, + matcher: 'rate_limit', + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.StopFailure, { + error: 'authentication_failed', + }); + + expect(result).toBeNull(); + }); + + it('should match all error types when no matcher for StopFailure', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.StopFailure, + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.StopFailure, { + error: 'server_error', + }); + + expect(result).not.toBeNull(); + }); + + it('should match all error types when matcher is wildcard for StopFailure', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.StopFailure, + matcher: '*', + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.StopFailure, { + error: 'billing_error', + }); + + expect(result).not.toBeNull(); + }); + + // PostCompact matcher tests + it('should match trigger with exact string for PostCompact', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.PostCompact, + matcher: 'manual', + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.PostCompact, { + trigger: 'manual', + }); + + expect(result).not.toBeNull(); + }); + + it('should not match trigger with different string for PostCompact', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.PostCompact, + matcher: 'manual', + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.PostCompact, { + trigger: 'auto', + }); + + expect(result).toBeNull(); + }); + + it('should match all triggers when no matcher for PostCompact', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.PostCompact, + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.PostCompact, { + trigger: 'auto', + }); + + expect(result).not.toBeNull(); + }); + + it('should match all triggers when matcher is wildcard for PostCompact', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.PostCompact, + matcher: '*', + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.PostCompact, { + trigger: 'manual', + }); + + expect(result).not.toBeNull(); + }); + + it('should match auto trigger for PostCompact', () => { + const entry: HookRegistryEntry = { + config: { type: HookType.Command, command: 'echo test' }, + source: HooksConfigSource.Project, + eventName: HookEventName.PostCompact, + matcher: 'auto', + enabled: true, + }; + vi.mocked(mockRegistry.getHooksForEvent).mockReturnValue([entry]); + + const result = planner.createExecutionPlan(HookEventName.PostCompact, { + trigger: 'auto', + }); + + expect(result).not.toBeNull(); + }); }); }); diff --git a/packages/core/src/hooks/hookPlanner.ts b/packages/core/src/hooks/hookPlanner.ts index 23628c712..da550aabd 100644 --- a/packages/core/src/hooks/hookPlanner.ts +++ b/packages/core/src/hooks/hookPlanner.ts @@ -108,6 +108,18 @@ export class HookPlanner { ? this.matchesTrigger(matcher, context.trigger) : true; + // PostCompact: match against trigger + case HookEventName.PostCompact: + return context.trigger + ? this.matchesTrigger(matcher, context.trigger) + : true; + + // StopFailure: match against error type (fieldToMatch: 'error') + case HookEventName.StopFailure: + return context.error + ? this.matchesTrigger(matcher, context.error) + : true; + // Notification: match against notification type case HookEventName.Notification: return context.notificationType @@ -229,4 +241,6 @@ export interface HookEventContext { notificationType?: string; /** Agent type for SubagentStart/SubagentStop matcher filtering */ agentType?: string; + /** Error type for StopFailure matcher filtering (fieldToMatch: 'error') */ + error?: string; } diff --git a/packages/core/src/hooks/hookSystem.ts b/packages/core/src/hooks/hookSystem.ts index cb346b42c..2408d446f 100644 --- a/packages/core/src/hooks/hookSystem.ts +++ b/packages/core/src/hooks/hookSystem.ts @@ -20,9 +20,11 @@ import type { AgentType, PermissionMode, PreCompactTrigger, + PostCompactTrigger, NotificationType, PermissionSuggestion, HookEventName, + StopFailureErrorType, } from './types.js'; const debugLogger = createDebugLogger('TRUSTED_HOOKS'); @@ -312,6 +314,42 @@ export class HookSystem { : undefined; } + /** + * Fire a StopFailure event - called when an API error ends the turn + * Fire-and-forget: output and exit codes are ignored + */ + async fireStopFailureEvent( + error: StopFailureErrorType, + errorDetails?: string, + lastAssistantMessage?: string, + signal?: AbortSignal, + ): Promise { + return this.hookEventHandler.fireStopFailureEvent( + error, + errorDetails, + lastAssistantMessage, + signal, + ); + } + + /** + * Fire a PostCompact event - called after conversation compaction completes + */ + async firePostCompactEvent( + trigger: PostCompactTrigger, + compactSummary: string, + signal?: AbortSignal, + ): Promise { + const result = await this.hookEventHandler.firePostCompactEvent( + trigger, + compactSummary, + signal, + ); + return result.finalOutput + ? createHookOutput('PostCompact', result.finalOutput) + : undefined; + } + /** * Fire a PermissionRequest event */ diff --git a/packages/core/src/hooks/types.ts b/packages/core/src/hooks/types.ts index e07e1087c..7a5f15f90 100644 --- a/packages/core/src/hooks/types.ts +++ b/packages/core/src/hooks/types.ts @@ -38,10 +38,14 @@ export enum HookEventName { SubagentStop = 'SubagentStop', // PreCompact - Before conversation compaction PreCompact = 'PreCompact', + // PostCompact - After conversation compaction + PostCompact = 'PostCompact', // SessionEnd - When a session is ending SessionEnd = 'SessionEnd', // When a permission dialog is displayed PermissionRequest = 'PermissionRequest', + // StopFailure - When the turn ends due to an API error (instead of Stop) + StopFailure = 'StopFailure', } /** @@ -673,6 +677,36 @@ export interface PreCompactOutput extends HookOutput { }; } +/** + * PostCompact trigger types + */ +export enum PostCompactTrigger { + Manual = 'manual', + Auto = 'auto', +} + +/** + * PostCompact hook input + * Fired after conversation compaction completes + */ +export interface PostCompactInput extends HookInput { + trigger: PostCompactTrigger; + compact_summary: string; +} + +/** + * PostCompact hook output + * Note: PostCompact is not in the official decision mode supported events list, + * so hookSpecificOutput / additionalContext do not produce any control effects + */ +export interface PostCompactOutput extends HookOutput { + // All returned JSON is ignored for control purposes + hookSpecificOutput?: { + hookEventName: 'PostCompact'; + additionalContext?: string; + }; +} + export enum AgentType { Bash = 'Bash', Explorer = 'Explorer', @@ -724,6 +758,36 @@ export interface SubagentStopOutput extends HookOutput { }; } +/** + * StopFailure error types + * Fires instead of Stop when an API error ended the turn + */ +export type StopFailureErrorType = + | 'rate_limit' + | 'authentication_failed' + | 'billing_error' + | 'invalid_request' + | 'server_error' + | 'max_output_tokens' + | 'unknown'; + +/** + * StopFailure hook input + * Fired when the turn ends due to an API error (instead of Stop) + */ +export interface StopFailureInput extends HookInput { + error: StopFailureErrorType; + error_details?: string; + last_assistant_message?: string; +} + +/** + * StopFailure hook output + * Fire-and-forget: hook output and exit codes are ignored + * This type alias is used instead of an empty interface to satisfy ESLint rules + */ +export type StopFailureOutput = HookOutput; + /** * Hook execution result */ diff --git a/packages/core/src/services/chatCompressionService.test.ts b/packages/core/src/services/chatCompressionService.test.ts index 55a1e2723..417988331 100644 --- a/packages/core/src/services/chatCompressionService.test.ts +++ b/packages/core/src/services/chatCompressionService.test.ts @@ -16,7 +16,11 @@ import { tokenLimit } from '../core/tokenLimits.js'; import type { GeminiChat } from '../core/geminiChat.js'; import type { Config } from '../config/config.js'; import type { ContentGenerator } from '../core/contentGenerator.js'; -import { SessionStartSource, PreCompactTrigger } from '../hooks/types.js'; +import { + SessionStartSource, + PreCompactTrigger, + PostCompactTrigger, +} from '../hooks/types.js'; vi.mock('../telemetry/uiTelemetry.js'); vi.mock('../core/tokenLimits.js'); @@ -813,12 +817,15 @@ describe('ChatCompressionService', () => { describe('PreCompact hook', () => { let mockFirePreCompactEvent: ReturnType; + let mockFirePostCompactEvent: ReturnType; beforeEach(() => { mockFirePreCompactEvent = vi.fn().mockResolvedValue(undefined); + mockFirePostCompactEvent = vi.fn().mockResolvedValue(undefined); mockGetHookSystem.mockReturnValue({ fireSessionStartEvent: mockFireSessionStartEvent, firePreCompactEvent: mockFirePreCompactEvent, + firePostCompactEvent: mockFirePostCompactEvent, }); }); @@ -1143,6 +1150,327 @@ describe('ChatCompressionService', () => { }); }); + describe('PostCompact hook', () => { + let mockFirePreCompactEvent: ReturnType; + let mockFirePostCompactEvent: ReturnType; + + beforeEach(() => { + mockFirePreCompactEvent = vi.fn().mockResolvedValue(undefined); + mockFirePostCompactEvent = vi.fn().mockResolvedValue(undefined); + mockGetHookSystem.mockReturnValue({ + fireSessionStartEvent: mockFireSessionStartEvent, + firePreCompactEvent: mockFirePreCompactEvent, + firePostCompactEvent: mockFirePostCompactEvent, + }); + }); + + it('should fire PostCompact hook with Manual trigger when force=true', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'msg1' }] }, + { role: 'model', parts: [{ text: 'msg2' }] }, + { role: 'user', parts: [{ text: 'msg3' }] }, + { role: 'model', parts: [{ text: 'msg4' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(uiTelemetryService.getLastPromptTokenCount).mockReturnValue( + 100, + ); + vi.mocked(tokenLimit).mockReturnValue(1000); + + const mockGenerateContent = vi.fn().mockResolvedValue({ + candidates: [ + { + content: { + parts: [{ text: 'Summary' }], + }, + }, + ], + usageMetadata: { + promptTokenCount: 1100, + candidatesTokenCount: 50, + totalTokenCount: 1150, + }, + } as unknown as GenerateContentResponse); + vi.mocked(mockConfig.getContentGenerator).mockReturnValue({ + generateContent: mockGenerateContent, + } as unknown as ContentGenerator); + + await service.compress( + mockChat, + mockPromptId, + true, // force = true -> Manual trigger + mockModel, + mockConfig, + false, + ); + + expect(mockFirePostCompactEvent).toHaveBeenCalledWith( + PostCompactTrigger.Manual, + 'Summary', + undefined, + ); + }); + + it('should fire PostCompact hook with Auto trigger when force=false', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'msg1' }] }, + { role: 'model', parts: [{ text: 'msg2' }] }, + { role: 'user', parts: [{ text: 'msg3' }] }, + { role: 'model', parts: [{ text: 'msg4' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(uiTelemetryService.getLastPromptTokenCount).mockReturnValue( + 800, + ); + vi.mocked(mockConfig.getContentGeneratorConfig).mockReturnValue({ + model: 'gemini-pro', + contextWindowSize: 1000, + } as unknown as ReturnType); + + const mockGenerateContent = vi.fn().mockResolvedValue({ + candidates: [ + { + content: { + parts: [{ text: 'Auto Summary' }], + }, + }, + ], + usageMetadata: { + promptTokenCount: 1600, + candidatesTokenCount: 50, + totalTokenCount: 1650, + }, + } as unknown as GenerateContentResponse); + vi.mocked(mockConfig.getContentGenerator).mockReturnValue({ + generateContent: mockGenerateContent, + } as unknown as ContentGenerator); + + await service.compress( + mockChat, + mockPromptId, + false, // force = false -> Auto trigger + mockModel, + mockConfig, + false, + ); + + expect(mockFirePostCompactEvent).toHaveBeenCalledWith( + PostCompactTrigger.Auto, + 'Auto Summary', + undefined, + ); + }); + + it('should not fire PostCompact hook when compression fails with empty summary', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'msg1' }] }, + { role: 'model', parts: [{ text: 'msg2' }] }, + { role: 'user', parts: [{ text: 'msg3' }] }, + { role: 'model', parts: [{ text: 'msg4' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(uiTelemetryService.getLastPromptTokenCount).mockReturnValue( + 100, + ); + vi.mocked(tokenLimit).mockReturnValue(1000); + + const mockGenerateContent = vi.fn().mockResolvedValue({ + candidates: [ + { + content: { + parts: [{ text: '' }], // Empty summary + }, + }, + ], + usageMetadata: { + promptTokenCount: 1100, + candidatesTokenCount: 0, + totalTokenCount: 1100, + }, + } as unknown as GenerateContentResponse); + vi.mocked(mockConfig.getContentGenerator).mockReturnValue({ + generateContent: mockGenerateContent, + } as unknown as ContentGenerator); + + const result = await service.compress( + mockChat, + mockPromptId, + true, + mockModel, + mockConfig, + false, + ); + + expect(result.info.compressionStatus).toBe( + CompressionStatus.COMPRESSION_FAILED_EMPTY_SUMMARY, + ); + expect(mockFirePostCompactEvent).not.toHaveBeenCalled(); + }); + + it('should handle PostCompact hook errors gracefully', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'msg1' }] }, + { role: 'model', parts: [{ text: 'msg2' }] }, + { role: 'user', parts: [{ text: 'msg3' }] }, + { role: 'model', parts: [{ text: 'msg4' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(uiTelemetryService.getLastPromptTokenCount).mockReturnValue( + 800, + ); + vi.mocked(mockConfig.getContentGeneratorConfig).mockReturnValue({ + model: 'gemini-pro', + contextWindowSize: 1000, + } as unknown as ReturnType); + + mockFirePostCompactEvent.mockRejectedValue( + new Error('PostCompact hook failed'), + ); + + const mockGenerateContent = vi.fn().mockResolvedValue({ + candidates: [ + { + content: { + parts: [{ text: 'Summary' }], + }, + }, + ], + usageMetadata: { + promptTokenCount: 1600, + candidatesTokenCount: 50, + totalTokenCount: 1650, + }, + } as unknown as GenerateContentResponse); + vi.mocked(mockConfig.getContentGenerator).mockReturnValue({ + generateContent: mockGenerateContent, + } as unknown as ContentGenerator); + + const result = await service.compress( + mockChat, + mockPromptId, + false, + mockModel, + mockConfig, + false, + ); + + // Should still complete compression despite hook error + expect(result.info.compressionStatus).toBe(CompressionStatus.COMPRESSED); + expect(result.newHistory).not.toBeNull(); + expect(mockFirePostCompactEvent).toHaveBeenCalled(); + }); + + it('should fire hooks in correct order: PreCompact -> SessionStart -> PostCompact', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'msg1' }] }, + { role: 'model', parts: [{ text: 'msg2' }] }, + { role: 'user', parts: [{ text: 'msg3' }] }, + { role: 'model', parts: [{ text: 'msg4' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(uiTelemetryService.getLastPromptTokenCount).mockReturnValue( + 800, + ); + vi.mocked(mockConfig.getContentGeneratorConfig).mockReturnValue({ + model: 'gemini-pro', + contextWindowSize: 1000, + } as unknown as ReturnType); + + const callOrder: string[] = []; + mockFirePreCompactEvent.mockImplementation(async () => { + callOrder.push('PreCompact'); + }); + mockFireSessionStartEvent.mockImplementation(async () => { + callOrder.push('SessionStart'); + }); + mockFirePostCompactEvent.mockImplementation(async () => { + callOrder.push('PostCompact'); + }); + + const mockGenerateContent = vi.fn().mockResolvedValue({ + candidates: [ + { + content: { + parts: [{ text: 'Summary' }], + }, + }, + ], + usageMetadata: { + promptTokenCount: 1600, + candidatesTokenCount: 50, + totalTokenCount: 1650, + }, + } as unknown as GenerateContentResponse); + vi.mocked(mockConfig.getContentGenerator).mockReturnValue({ + generateContent: mockGenerateContent, + } as unknown as ContentGenerator); + + await service.compress( + mockChat, + mockPromptId, + false, + mockModel, + mockConfig, + false, + ); + + // Hooks should be called in order: PreCompact -> SessionStart -> PostCompact + expect(callOrder).toEqual(['PreCompact', 'SessionStart', 'PostCompact']); + }); + + it('should not fire PostCompact hook when hookSystem is null', async () => { + mockGetHookSystem.mockReturnValue(null); + + const history: Content[] = [ + { role: 'user', parts: [{ text: 'msg1' }] }, + { role: 'model', parts: [{ text: 'msg2' }] }, + { role: 'user', parts: [{ text: 'msg3' }] }, + { role: 'model', parts: [{ text: 'msg4' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(uiTelemetryService.getLastPromptTokenCount).mockReturnValue( + 800, + ); + vi.mocked(mockConfig.getContentGeneratorConfig).mockReturnValue({ + model: 'gemini-pro', + contextWindowSize: 1000, + } as unknown as ReturnType); + + const mockGenerateContent = vi.fn().mockResolvedValue({ + candidates: [ + { + content: { + parts: [{ text: 'Summary' }], + }, + }, + ], + usageMetadata: { + promptTokenCount: 1600, + candidatesTokenCount: 50, + totalTokenCount: 1650, + }, + } as unknown as GenerateContentResponse); + vi.mocked(mockConfig.getContentGenerator).mockReturnValue({ + generateContent: mockGenerateContent, + } as unknown as ContentGenerator); + + const result = await service.compress( + mockChat, + mockPromptId, + false, + mockModel, + mockConfig, + false, + ); + + // Should still complete compression without hook + expect(result.info.compressionStatus).toBe(CompressionStatus.COMPRESSED); + expect(result.newHistory).not.toBeNull(); + // mockFirePostCompactEvent should not be called since hookSystem is null + expect(mockFirePostCompactEvent).not.toHaveBeenCalled(); + }); + }); + describe('orphaned trailing funcCall handling', () => { it('should compress everything when force=true and last message is an orphaned funcCall', async () => { // Issue #2647: tool-heavy conversation interrupted/crashed while a tool diff --git a/packages/core/src/services/chatCompressionService.ts b/packages/core/src/services/chatCompressionService.ts index 528a57b44..876d051ac 100644 --- a/packages/core/src/services/chatCompressionService.ts +++ b/packages/core/src/services/chatCompressionService.ts @@ -15,7 +15,11 @@ import { getResponseText } from '../utils/partUtils.js'; import { logChatCompression } from '../telemetry/loggers.js'; import { makeChatCompressionEvent } from '../telemetry/types.js'; import type { PermissionMode } from '../hooks/types.js'; -import { SessionStartSource, PreCompactTrigger } from '../hooks/types.js'; +import { + SessionStartSource, + PreCompactTrigger, + PostCompactTrigger, +} from '../hooks/types.js'; /** * Threshold for compression token count as a fraction of the model's token limit. @@ -355,6 +359,18 @@ export class ChatCompressionService { config.getDebugLogger().warn(`SessionStart hook failed: ${err}`); } + // Fire PostCompact event after successful compression + try { + const postCompactTrigger = force + ? PostCompactTrigger.Manual + : PostCompactTrigger.Auto; + await config + .getHookSystem() + ?.firePostCompactEvent(postCompactTrigger, summary, signal); + } catch (err) { + config.getDebugLogger().warn(`PostCompact hook failed: ${err}`); + } + return { newHistory: extraHistory, info: {