From 8bd7cf2cdae4e2b4e0c77db25e86f35f73f3cb8f Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Mon, 23 Mar 2026 16:02:54 +0800 Subject: [PATCH 1/3] add singal abort for hooks --- packages/cli/src/ui/AppContainer.tsx | 7 +- .../cli/src/ui/commands/clearCommand.test.ts | 2 + packages/cli/src/ui/commands/clearCommand.ts | 2 + packages/cli/src/ui/hooks/useResumeCommand.ts | 2 + packages/core/src/config/config.ts | 21 +++ .../core/src/confirmation-bus/message-bus.ts | 19 +++ packages/core/src/confirmation-bus/types.ts | 2 + packages/core/src/core/client.ts | 24 +++- packages/core/src/core/toolHookTriggers.ts | 10 ++ .../core/src/hooks/hookEventHandler.test.ts | 1 + packages/core/src/hooks/hookEventHandler.ts | 134 +++++++++++++----- packages/core/src/hooks/hookRunner.ts | 92 ++++++++++-- packages/core/src/hooks/hookSystem.test.ts | 30 +++- packages/core/src/hooks/hookSystem.ts | 33 ++++- .../src/services/chatCompressionService.ts | 15 +- packages/core/src/tools/agent.ts | 2 + 16 files changed, 344 insertions(+), 52 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index b1918ebaae..8ba48a4c9e 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -41,6 +41,7 @@ import { Storage, SessionEndReason, SessionStartSource, + type PermissionMode, } from '@qwen-code/qwen-code-core'; import { buildResumedHistoryItems } from './utils/resumeHistoryUtils.js'; import { validateAuthMethod } from '../config/auth.js'; @@ -308,7 +309,11 @@ export const AppContainer = (props: AppContainerProps) => { if (hookSystem) { hookSystem - .fireSessionStartEvent(sessionStartSource, config.getModel() ?? '') + .fireSessionStartEvent( + sessionStartSource, + config.getModel() ?? '', + String(config.getApprovalMode()) as PermissionMode, + ) .then(() => { debugLogger.debug('SessionStart event completed successfully'); }) diff --git a/packages/cli/src/ui/commands/clearCommand.test.ts b/packages/cli/src/ui/commands/clearCommand.test.ts index 5887a80121..1eb4f4707d 100644 --- a/packages/cli/src/ui/commands/clearCommand.test.ts +++ b/packages/cli/src/ui/commands/clearCommand.test.ts @@ -59,6 +59,7 @@ describe('clearCommand', () => { }), getModel: () => 'test-model', getToolRegistry: () => undefined, + getApprovalMode: () => 'default', }, }, session: { @@ -108,6 +109,7 @@ describe('clearCommand', () => { expect(mockFireSessionStartEvent).toHaveBeenCalledWith( SessionStartSource.Clear, 'test-model', + expect.any(String), // permissionMode ); // SessionEnd should be called before SessionStart diff --git a/packages/cli/src/ui/commands/clearCommand.ts b/packages/cli/src/ui/commands/clearCommand.ts index 7de8192e2b..ce3b78066e 100644 --- a/packages/cli/src/ui/commands/clearCommand.ts +++ b/packages/cli/src/ui/commands/clearCommand.ts @@ -13,6 +13,7 @@ import { SessionStartSource, ToolNames, SkillTool, + type PermissionMode, } from '@qwen-code/qwen-code-core'; export const clearCommand: SlashCommand = { @@ -72,6 +73,7 @@ export const clearCommand: SlashCommand = { ?.fireSessionStartEvent( SessionStartSource.Clear, config.getModel() ?? '', + String(config.getApprovalMode()) as PermissionMode, ); } catch (err) { config.getDebugLogger().warn(`SessionStart hook failed: ${err}`); diff --git a/packages/cli/src/ui/hooks/useResumeCommand.ts b/packages/cli/src/ui/hooks/useResumeCommand.ts index 6a77ffdebd..04edc21eab 100644 --- a/packages/cli/src/ui/hooks/useResumeCommand.ts +++ b/packages/cli/src/ui/hooks/useResumeCommand.ts @@ -9,6 +9,7 @@ import { SessionService, type Config, SessionStartSource, + type PermissionMode, } from '@qwen-code/qwen-code-core'; import { buildResumedHistoryItems } from '../utils/resumeHistoryUtils.js'; import type { UseHistoryManagerReturn } from './useHistoryManager.js'; @@ -78,6 +79,7 @@ export function useResumeCommand( ?.fireSessionStartEvent( SessionStartSource.Resume, config.getModel() ?? '', + String(config.getApprovalMode()) as PermissionMode, ); } catch (err) { config.getDebugLogger().warn(`SessionStart hook failed: ${err}`); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 6ffd3ac3b8..a69e4d29b0 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -810,19 +810,33 @@ export class Config { return; } + // Check if request was aborted + if (request.signal?.aborted) { + this.messageBus?.publish({ + type: MessageBusType.HOOK_EXECUTION_RESPONSE, + correlationId: request.correlationId, + success: false, + error: new Error('Hook execution cancelled (aborted)'), + } as HookExecutionResponse); + return; + } + // Execute the appropriate hook based on eventName let result; const input = request.input || {}; + const signal = request.signal; switch (request.eventName) { case 'UserPromptSubmit': result = await hookSystem.fireUserPromptSubmitEvent( (input['prompt'] as string) || '', + signal, ); break; case 'Stop': result = await hookSystem.fireStopEvent( (input['stop_hook_active'] as boolean) || false, (input['last_assistant_message'] as string) || '', + signal, ); break; case 'PreToolUse': { @@ -832,6 +846,7 @@ export class Config { (input['tool_use_id'] as string) || '', (input['permission_mode'] as PermissionMode | undefined) ?? PermissionMode.Default, + signal, ); break; } @@ -842,6 +857,7 @@ export class Config { (input['tool_response'] as Record) || {}, (input['tool_use_id'] as string) || '', (input['permission_mode'] as PermissionMode) || 'default', + signal, ); break; case 'PostToolUseFailure': @@ -852,6 +868,7 @@ export class Config { (input['error'] as string) || '', input['is_interrupt'] as boolean | undefined, (input['permission_mode'] as PermissionMode) || 'default', + signal, ); break; case 'Notification': @@ -860,6 +877,7 @@ export class Config { (input['notification_type'] as NotificationType) || 'permission_prompt', (input['title'] as string) || undefined, + signal, ); break; case 'PermissionRequest': @@ -871,6 +889,7 @@ export class Config { (input['permission_suggestions'] as | PermissionSuggestion[] | undefined) || undefined, + signal, ); break; case 'SubagentStart': @@ -879,6 +898,7 @@ export class Config { (input['agent_type'] as string) || '', (input['permission_mode'] as PermissionMode) || PermissionMode.Default, + signal, ); break; case 'SubagentStop': @@ -890,6 +910,7 @@ export class Config { (input['stop_hook_active'] as boolean) || false, (input['permission_mode'] as PermissionMode) || PermissionMode.Default, + signal, ); break; default: diff --git a/packages/core/src/confirmation-bus/message-bus.ts b/packages/core/src/confirmation-bus/message-bus.ts index fcd2caab75..e8a737f82e 100644 --- a/packages/core/src/confirmation-bus/message-bus.ts +++ b/packages/core/src/confirmation-bus/message-bus.ts @@ -90,10 +90,17 @@ export class MessageBus extends EventEmitter { request: Omit, responseType: TResponse['type'], timeoutMs: number = 60000, + signal?: AbortSignal, ): Promise { const correlationId = randomUUID(); return new Promise((resolve, reject) => { + // Check if already aborted + if (signal?.aborted) { + reject(new Error('Request aborted')); + return; + } + const timeoutId = setTimeout(() => { cleanup(); reject(new Error(`Request timed out waiting for ${responseType}`)); @@ -102,8 +109,20 @@ export class MessageBus extends EventEmitter { const cleanup = () => { clearTimeout(timeoutId); this.unsubscribe(responseType, responseHandler); + if (signal) { + signal.removeEventListener('abort', abortHandler); + } + }; + + const abortHandler = () => { + cleanup(); + reject(new Error('Request aborted')); }; + if (signal) { + signal.addEventListener('abort', abortHandler); + } + const responseHandler = (response: TResponse) => { // Check if this response matches our request if ( diff --git a/packages/core/src/confirmation-bus/types.ts b/packages/core/src/confirmation-bus/types.ts index 7a699bacb3..819794b616 100644 --- a/packages/core/src/confirmation-bus/types.ts +++ b/packages/core/src/confirmation-bus/types.ts @@ -109,6 +109,8 @@ export interface HookExecutionRequest { eventName: string; input: Record; correlationId: string; + /** Optional AbortSignal to cancel hook execution */ + signal?: AbortSignal; } export interface HookExecutionResponse { diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 4a0de97460..d95efc844f 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -535,7 +535,7 @@ export class GeminiClient { return new Turn(this.getChat(), prompt_id); } - const compressed = await this.tryCompressChat(prompt_id, false); + const compressed = await this.tryCompressChat(prompt_id, false, signal); if (compressed.compressionStatus === CompressionStatus.COMPRESSED) { yield { type: GeminiEventType.ChatCompressed, value: compressed }; @@ -677,7 +677,13 @@ export class GeminiClient { } // Fire Stop hook through MessageBus (only if hooks are enabled) // This must be done before any early returns to ensure hooks are always triggered - if (hooksEnabled && messageBus && !turn.pendingToolCalls.length) { + if ( + hooksEnabled && + messageBus && + !turn.pendingToolCalls.length && + signal && + !signal.aborted + ) { // Get response text from the chat history const history = this.getHistory(); const lastModelMessage = history @@ -700,9 +706,16 @@ export class GeminiClient { stop_hook_active: true, last_assistant_message: responseText, }, + signal, }, MessageBusType.HOOK_EXECUTION_RESPONSE, ); + + // Check if aborted after hook execution + if (signal.aborted) { + return turn; + } + const hookOutput = response.output ? createHookOutput('Stop', response.output) : undefined; @@ -714,6 +727,11 @@ export class GeminiClient { stopOutput?.isBlockingDecision() || stopOutput?.shouldStopExecution() ) { + // Check if aborted before continuing + if (signal.aborted) { + return turn; + } + // Emit system message if provided (e.g., "🔄 Ralph iteration 5") if (stopOutput.systemMessage) { yield { @@ -844,6 +862,7 @@ export class GeminiClient { async tryCompressChat( prompt_id: string, force: boolean = false, + signal?: AbortSignal, ): Promise { const compressionService = new ChatCompressionService(); @@ -854,6 +873,7 @@ export class GeminiClient { this.config.getModel(), this.config, this.hasFailedCompressionAttempt, + signal, ); // Handle compression result diff --git a/packages/core/src/core/toolHookTriggers.ts b/packages/core/src/core/toolHookTriggers.ts index 1d62477e0f..73423d77d1 100644 --- a/packages/core/src/core/toolHookTriggers.ts +++ b/packages/core/src/core/toolHookTriggers.ts @@ -81,6 +81,7 @@ export async function firePreToolUseHook( toolInput: Record, toolUseId: string, permissionMode: string, + signal?: AbortSignal, ): Promise { if (!messageBus) { return { shouldProceed: true }; @@ -100,6 +101,7 @@ export async function firePreToolUseHook( tool_input: toolInput, tool_use_id: toolUseId, }, + signal, }, MessageBusType.HOOK_EXECUTION_RESPONSE, ); @@ -178,6 +180,7 @@ export async function firePostToolUseHook( toolResponse: Record, toolUseId: string, permissionMode: string, + signal?: AbortSignal, ): Promise { if (!messageBus) { return { shouldStop: false }; @@ -198,6 +201,7 @@ export async function firePostToolUseHook( tool_response: toolResponse, tool_use_id: toolUseId, }, + signal, }, MessageBusType.HOOK_EXECUTION_RESPONSE, ); @@ -255,6 +259,7 @@ export async function firePostToolUseFailureHook( errorMessage: string, isInterrupt?: boolean, permissionMode?: string, + signal?: AbortSignal, ): Promise { if (!messageBus) { return {}; @@ -276,6 +281,7 @@ export async function firePostToolUseFailureHook( error: errorMessage, is_interrupt: isInterrupt, }, + signal, }, MessageBusType.HOOK_EXECUTION_RESPONSE, ); @@ -319,6 +325,7 @@ export async function fireNotificationHook( message: string, notificationType: NotificationType, title?: string, + signal?: AbortSignal, ): Promise { if (!messageBus) { return {}; @@ -337,6 +344,7 @@ export async function fireNotificationHook( notification_type: notificationType, title, }, + signal, }, MessageBusType.HOOK_EXECUTION_RESPONSE, ); @@ -390,6 +398,7 @@ export async function firePermissionRequestHook( toolInput: Record, permissionMode: string, permissionSuggestions?: PermissionSuggestion[], + signal?: AbortSignal, ): Promise { if (!messageBus) { return { hasDecision: false }; @@ -409,6 +418,7 @@ export async function firePermissionRequestHook( permission_mode: permissionMode, permission_suggestions: permissionSuggestions, }, + signal, }, MessageBusType.HOOK_EXECUTION_RESPONSE, ); diff --git a/packages/core/src/hooks/hookEventHandler.test.ts b/packages/core/src/hooks/hookEventHandler.test.ts index 9bffed8bb9..f3c9b50d9d 100644 --- a/packages/core/src/hooks/hookEventHandler.test.ts +++ b/packages/core/src/hooks/hookEventHandler.test.ts @@ -712,6 +712,7 @@ describe('HookEventHandler', () => { expect.any(Object), // input object expect.any(Function), // onHookStart callback expect.any(Function), // onHookEnd callback + undefined, // signal ); }); diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index 16bc92b4a9..45eacf81b8 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -64,13 +64,19 @@ export class HookEventHandler { */ async fireUserPromptSubmitEvent( prompt: string, + signal?: AbortSignal, ): Promise { const input: UserPromptSubmitInput = { ...this.createBaseInput(HookEventName.UserPromptSubmit), prompt, }; - return this.executeHooks(HookEventName.UserPromptSubmit, input); + return this.executeHooks( + HookEventName.UserPromptSubmit, + input, + undefined, + signal, + ); } /** @@ -80,6 +86,7 @@ export class HookEventHandler { async fireStopEvent( stopHookActive: boolean = false, lastAssistantMessage: string = '', + signal?: AbortSignal, ): Promise { const input: StopInput = { ...this.createBaseInput(HookEventName.Stop), @@ -87,7 +94,7 @@ export class HookEventHandler { last_assistant_message: lastAssistantMessage, }; - return this.executeHooks(HookEventName.Stop, input); + return this.executeHooks(HookEventName.Stop, input, undefined, signal); } /** @@ -99,6 +106,7 @@ export class HookEventHandler { model: string, permissionMode?: PermissionMode, agentType?: AgentType, + signal?: AbortSignal, ): Promise { const input: SessionStartInput = { ...this.createBaseInput(HookEventName.SessionStart), @@ -109,9 +117,14 @@ export class HookEventHandler { }; // Pass source as context for matcher filtering - return this.executeHooks(HookEventName.SessionStart, input, { - trigger: source, - }); + return this.executeHooks( + HookEventName.SessionStart, + input, + { + trigger: source, + }, + signal, + ); } /** @@ -120,6 +133,7 @@ export class HookEventHandler { */ async fireSessionEndEvent( reason: SessionEndReason, + signal?: AbortSignal, ): Promise { const input: SessionEndInput = { ...this.createBaseInput(HookEventName.SessionEnd), @@ -127,9 +141,14 @@ export class HookEventHandler { }; // Pass reason as context for matcher filtering - return this.executeHooks(HookEventName.SessionEnd, input, { - trigger: reason, - }); + return this.executeHooks( + HookEventName.SessionEnd, + input, + { + trigger: reason, + }, + signal, + ); } /** @@ -141,6 +160,7 @@ export class HookEventHandler { toolInput: Record, toolUseId: string, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const input: PreToolUseInput = { ...this.createBaseInput(HookEventName.PreToolUse), @@ -151,9 +171,14 @@ export class HookEventHandler { }; // Pass tool name as context for matcher filtering - return this.executeHooks(HookEventName.PreToolUse, input, { - toolName, - }); + return this.executeHooks( + HookEventName.PreToolUse, + input, + { + toolName, + }, + signal, + ); } /** @@ -166,6 +191,7 @@ export class HookEventHandler { toolResponse: Record, toolUseId: string, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const input: PostToolUseInput = { ...this.createBaseInput(HookEventName.PostToolUse), @@ -177,9 +203,14 @@ export class HookEventHandler { }; // Pass tool name as context for matcher filtering - return this.executeHooks(HookEventName.PostToolUse, input, { - toolName, - }); + return this.executeHooks( + HookEventName.PostToolUse, + input, + { + toolName, + }, + signal, + ); } /** @@ -193,6 +224,7 @@ export class HookEventHandler { errorMessage: string, isInterrupt?: boolean, permissionMode?: PermissionMode, + signal?: AbortSignal, ): Promise { const input: PostToolUseFailureInput = { ...this.createBaseInput(HookEventName.PostToolUseFailure), @@ -205,9 +237,14 @@ export class HookEventHandler { }; // Pass tool name as context for matcher filtering - return this.executeHooks(HookEventName.PostToolUseFailure, input, { - toolName, - }); + return this.executeHooks( + HookEventName.PostToolUseFailure, + input, + { + toolName, + }, + signal, + ); } /** @@ -217,6 +254,7 @@ export class HookEventHandler { async firePreCompactEvent( trigger: PreCompactTrigger, customInstructions: string = '', + signal?: AbortSignal, ): Promise { const input: PreCompactInput = { ...this.createBaseInput(HookEventName.PreCompact), @@ -225,9 +263,14 @@ export class HookEventHandler { }; // Pass trigger as context for matcher filtering - return this.executeHooks(HookEventName.PreCompact, input, { - trigger, - }); + return this.executeHooks( + HookEventName.PreCompact, + input, + { + trigger, + }, + signal, + ); } /** @@ -237,6 +280,7 @@ export class HookEventHandler { message: string, notificationType: NotificationType, title?: string, + signal?: AbortSignal, ): Promise { const input: NotificationInput = { ...this.createBaseInput(HookEventName.Notification), @@ -246,9 +290,14 @@ export class HookEventHandler { }; // Pass notification_type as context for matcher filtering - return this.executeHooks(HookEventName.Notification, input, { - notificationType, - }); + return this.executeHooks( + HookEventName.Notification, + input, + { + notificationType, + }, + signal, + ); } /** @@ -260,6 +309,7 @@ export class HookEventHandler { toolInput: Record, permissionMode: PermissionMode, permissionSuggestions?: PermissionSuggestion[], + signal?: AbortSignal, ): Promise { const input: PermissionRequestInput = { ...this.createBaseInput(HookEventName.PermissionRequest), @@ -270,9 +320,14 @@ export class HookEventHandler { }; // Pass tool name as context for matcher filtering - return this.executeHooks(HookEventName.PermissionRequest, input, { - toolName, - }); + return this.executeHooks( + HookEventName.PermissionRequest, + input, + { + toolName, + }, + signal, + ); } /** @@ -283,6 +338,7 @@ export class HookEventHandler { agentId: string, agentType: AgentType | string, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const input: SubagentStartInput = { ...this.createBaseInput(HookEventName.SubagentStart), @@ -292,9 +348,14 @@ export class HookEventHandler { }; // Pass agentType as context for matcher filtering - return this.executeHooks(HookEventName.SubagentStart, input, { - agentType: String(agentType), - }); + return this.executeHooks( + HookEventName.SubagentStart, + input, + { + agentType: String(agentType), + }, + signal, + ); } /** @@ -308,6 +369,7 @@ export class HookEventHandler { lastAssistantMessage: string, stopHookActive: boolean, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const input: SubagentStopInput = { ...this.createBaseInput(HookEventName.SubagentStop), @@ -320,9 +382,14 @@ export class HookEventHandler { }; // Pass agentType as context for matcher filtering - return this.executeHooks(HookEventName.SubagentStop, input, { - agentType: String(agentType), - }); + return this.executeHooks( + HookEventName.SubagentStop, + input, + { + agentType: String(agentType), + }, + signal, + ); } /** @@ -333,6 +400,7 @@ export class HookEventHandler { eventName: HookEventName, input: HookInput, context?: HookEventContext, + signal?: AbortSignal, ): Promise { try { // Create execution plan @@ -363,6 +431,7 @@ export class HookEventHandler { input, onHookStart, onHookEnd, + signal, ) : await this.hookRunner.executeHooksParallel( plan.hookConfigs, @@ -370,6 +439,7 @@ export class HookEventHandler { input, onHookStart, onHookEnd, + signal, ); // Aggregate results diff --git a/packages/core/src/hooks/hookRunner.ts b/packages/core/src/hooks/hookRunner.ts index 26a09f3506..d7d11f17d5 100644 --- a/packages/core/src/hooks/hookRunner.ts +++ b/packages/core/src/hooks/hookRunner.ts @@ -46,20 +46,38 @@ const EXIT_CODE_NON_BLOCKING_ERROR = 1; export class HookRunner { /** * Execute a single hook + * @param hookConfig Hook configuration + * @param eventName Event name + * @param input Hook input + * @param signal Optional AbortSignal to cancel hook execution */ async executeHook( hookConfig: HookConfig, eventName: HookEventName, input: HookInput, + signal?: AbortSignal, ): Promise { const startTime = Date.now(); + // Check if already aborted before starting + if (signal?.aborted) { + const hookId = hookConfig.name || hookConfig.command || 'unknown'; + return { + hookConfig, + eventName, + success: false, + error: new Error(`Hook execution cancelled (aborted): ${hookId}`), + duration: 0, + }; + } + try { return await this.executeCommandHook( hookConfig, eventName, input, startTime, + signal, ); } catch (error) { const duration = Date.now() - startTime; @@ -79,6 +97,7 @@ export class HookRunner { /** * Execute multiple hooks in parallel + * @param signal Optional AbortSignal to cancel hook execution */ async executeHooksParallel( hookConfigs: HookConfig[], @@ -86,10 +105,11 @@ export class HookRunner { input: HookInput, onHookStart?: (config: HookConfig, index: number) => void, onHookEnd?: (config: HookConfig, result: HookExecutionResult) => void, + signal?: AbortSignal, ): Promise { const promises = hookConfigs.map(async (config, index) => { onHookStart?.(config, index); - const result = await this.executeHook(config, eventName, input); + const result = await this.executeHook(config, eventName, input, signal); onHookEnd?.(config, result); return result; }); @@ -99,6 +119,7 @@ export class HookRunner { /** * Execute multiple hooks sequentially + * @param signal Optional AbortSignal to cancel hook execution */ async executeHooksSequential( hookConfigs: HookConfig[], @@ -106,14 +127,24 @@ export class HookRunner { input: HookInput, onHookStart?: (config: HookConfig, index: number) => void, onHookEnd?: (config: HookConfig, result: HookExecutionResult) => void, + signal?: AbortSignal, ): Promise { const results: HookExecutionResult[] = []; let currentInput = input; for (let i = 0; i < hookConfigs.length; i++) { + // Check if aborted before each hook + if (signal?.aborted) { + break; + } const config = hookConfigs[i]; onHookStart?.(config, i); - const result = await this.executeHook(config, eventName, currentInput); + const result = await this.executeHook( + config, + eventName, + currentInput, + signal, + ); onHookEnd?.(config, result); results.push(result); @@ -184,12 +215,18 @@ export class HookRunner { /** * Execute a command hook + * @param hookConfig Hook configuration + * @param eventName Event name + * @param input Hook input + * @param startTime Start time for duration calculation + * @param signal Optional AbortSignal to cancel hook execution */ private async executeCommandHook( hookConfig: HookConfig, eventName: HookEventName, input: HookInput, startTime: number, + signal?: AbortSignal, ): Promise { const timeout = hookConfig.timeout ?? DEFAULT_HOOK_TIMEOUT; @@ -212,6 +249,7 @@ export class HookRunner { let stdout = ''; let stderr = ''; let timedOut = false; + let aborted = false; const shellConfig = getShellConfiguration(); const command = this.expandCommand( @@ -239,19 +277,36 @@ export class HookRunner { }, ); + // Helper to kill child process + const killChild = () => { + if (!child.killed) { + child.kill('SIGTERM'); + // Force kill after 2 seconds + setTimeout(() => { + if (!child.killed) { + child.kill('SIGKILL'); + } + }, 2000); + } + }; + // Set up timeout const timeoutHandle = setTimeout(() => { timedOut = true; - child.kill('SIGTERM'); - - // Force kill after 5 seconds - setTimeout(() => { - if (!child.killed) { - child.kill('SIGKILL'); - } - }, 5000); + killChild(); }, timeout); + // Set up abort handler + const abortHandler = () => { + aborted = true; + clearTimeout(timeoutHandle); + killChild(); + }; + + if (signal) { + signal.addEventListener('abort', abortHandler); + } + // Send input to stdin if (child.stdin) { child.stdin.on('error', (err: NodeJS.ErrnoException) => { @@ -303,8 +358,25 @@ export class HookRunner { // Handle process exit child.on('close', (exitCode) => { clearTimeout(timeoutHandle); + // Clean up abort listener + if (signal) { + signal.removeEventListener('abort', abortHandler); + } const duration = Date.now() - startTime; + if (aborted) { + resolve({ + hookConfig, + eventName, + success: false, + error: new Error('Hook execution cancelled (aborted)'), + stdout, + stderr, + duration, + }); + return; + } + if (timedOut) { resolve({ hookConfig, diff --git a/packages/core/src/hooks/hookSystem.test.ts b/packages/core/src/hooks/hookSystem.test.ts index b0741a8298..cc09289de7 100644 --- a/packages/core/src/hooks/hookSystem.test.ts +++ b/packages/core/src/hooks/hookSystem.test.ts @@ -207,6 +207,7 @@ describe('HookSystem', () => { expect(mockHookEventHandler.fireStopEvent).toHaveBeenCalledWith( true, 'last message', + undefined, ); expect(result).toBeDefined(); }); @@ -228,6 +229,7 @@ describe('HookSystem', () => { expect(mockHookEventHandler.fireStopEvent).toHaveBeenCalledWith( false, '', + undefined, ); }); @@ -269,7 +271,7 @@ describe('HookSystem', () => { expect( mockHookEventHandler.fireUserPromptSubmitEvent, - ).toHaveBeenCalledWith('test prompt'); + ).toHaveBeenCalledWith('test prompt', undefined); expect(result).toBeDefined(); }); @@ -291,7 +293,7 @@ describe('HookSystem', () => { expect( mockHookEventHandler.fireUserPromptSubmitEvent, - ).toHaveBeenCalledWith('my custom prompt'); + ).toHaveBeenCalledWith('my custom prompt', undefined); }); it('should return undefined when no final output', async () => { @@ -382,6 +384,7 @@ describe('HookSystem', () => { 'gpt-4', undefined, undefined, + undefined, ); expect(result).toBeDefined(); }); @@ -412,6 +415,7 @@ describe('HookSystem', () => { 'claude-3', PermissionMode.AutoEdit, AgentType.Custom, + undefined, ); }); @@ -458,6 +462,7 @@ describe('HookSystem', () => { expect(mockHookEventHandler.fireSessionEndEvent).toHaveBeenCalledWith( SessionEndReason.Other, + undefined, ); expect(result).toBeDefined(); }); @@ -480,6 +485,7 @@ describe('HookSystem', () => { expect(mockHookEventHandler.fireSessionEndEvent).toHaveBeenCalledWith( SessionEndReason.Other, + undefined, ); }); @@ -531,6 +537,7 @@ describe('HookSystem', () => { { command: 'ls' }, 'toolu_test123', PermissionMode.AutoEdit, + undefined, ); expect(result).toBeDefined(); }); @@ -561,6 +568,7 @@ describe('HookSystem', () => { { path: '/test.txt', content: 'test' }, 'toolu_test456', PermissionMode.Yolo, + undefined, ); }); @@ -674,6 +682,7 @@ describe('HookSystem', () => { { output: 'file1.txt\nfile2.txt' }, 'toolu_test123', PermissionMode.AutoEdit, + undefined, ); expect(result).toBeDefined(); }); @@ -706,6 +715,7 @@ describe('HookSystem', () => { { content: 'file content' }, 'toolu_test456', PermissionMode.Plan, + undefined, ); }); @@ -794,6 +804,7 @@ describe('HookSystem', () => { 'Command not found', false, PermissionMode.AutoEdit, + undefined, ); expect(result).toBeDefined(); }); @@ -830,6 +841,7 @@ describe('HookSystem', () => { 'Permission denied', true, PermissionMode.Yolo, + undefined, ); }); @@ -861,6 +873,7 @@ describe('HookSystem', () => { 'Error occurred', undefined, undefined, + undefined, ); }); @@ -941,6 +954,7 @@ describe('HookSystem', () => { expect(mockHookEventHandler.firePreCompactEvent).toHaveBeenCalledWith( PreCompactTrigger.Auto, '', + undefined, ); expect(result).toBeDefined(); }); @@ -964,6 +978,7 @@ describe('HookSystem', () => { expect(mockHookEventHandler.firePreCompactEvent).toHaveBeenCalledWith( PreCompactTrigger.Manual, '', + undefined, ); }); @@ -989,6 +1004,7 @@ describe('HookSystem', () => { expect(mockHookEventHandler.firePreCompactEvent).toHaveBeenCalledWith( PreCompactTrigger.Auto, 'Custom compression instructions', + undefined, ); }); @@ -1065,6 +1081,7 @@ describe('HookSystem', () => { 'Test notification message', NotificationType.PermissionPrompt, 'Permission needed', + undefined, ); expect(result).toBeDefined(); }); @@ -1093,6 +1110,7 @@ describe('HookSystem', () => { 'Qwen Code is waiting for your input', NotificationType.IdlePrompt, 'Waiting for input', + undefined, ); }); @@ -1119,6 +1137,7 @@ describe('HookSystem', () => { 'Authentication successful', NotificationType.AuthSuccess, undefined, + undefined, ); }); @@ -1194,6 +1213,7 @@ describe('HookSystem', () => { 'Dialog shown to user', NotificationType.ElicitationDialog, 'Dialog', + undefined, ); }); }); @@ -1226,6 +1246,7 @@ describe('HookSystem', () => { { command: 'ls -la' }, PermissionMode.Default, undefined, + undefined, ); expect(result).toBeDefined(); // Type assertion needed because getPermissionDecision is specific to PermissionRequestHookOutput @@ -1259,6 +1280,7 @@ describe('HookSystem', () => { { command: 'npm test' }, PermissionMode.Default, suggestions, + undefined, ); }); @@ -1354,6 +1376,7 @@ describe('HookSystem', () => { 'agent-123', 'code-reviewer', PermissionMode.Default, + undefined, ); expect(result).toBeDefined(); }); @@ -1382,6 +1405,7 @@ describe('HookSystem', () => { 'agent-456', AgentType.Bash, PermissionMode.Yolo, + undefined, ); }); @@ -1468,6 +1492,7 @@ describe('HookSystem', () => { 'Final output from subagent', false, PermissionMode.Default, + undefined, ); expect(result).toBeDefined(); }); @@ -1502,6 +1527,7 @@ describe('HookSystem', () => { 'last message from agent', true, PermissionMode.Plan, + undefined, ); }); diff --git a/packages/core/src/hooks/hookSystem.ts b/packages/core/src/hooks/hookSystem.ts index 4716a0c846..f37d5c7121 100644 --- a/packages/core/src/hooks/hookSystem.ts +++ b/packages/core/src/hooks/hookSystem.ts @@ -89,9 +89,12 @@ export class HookSystem { async fireUserPromptSubmitEvent( prompt: string, + signal?: AbortSignal, ): Promise { - const result = - await this.hookEventHandler.fireUserPromptSubmitEvent(prompt); + const result = await this.hookEventHandler.fireUserPromptSubmitEvent( + prompt, + signal, + ); return result.finalOutput ? createHookOutput('UserPromptSubmit', result.finalOutput) : undefined; @@ -100,10 +103,12 @@ export class HookSystem { async fireStopEvent( stopHookActive: boolean = false, lastAssistantMessage: string = '', + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.fireStopEvent( stopHookActive, lastAssistantMessage, + signal, ); return result.finalOutput ? createHookOutput('Stop', result.finalOutput) @@ -115,12 +120,14 @@ export class HookSystem { model: string, permissionMode?: PermissionMode, agentType?: AgentType, + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.fireSessionStartEvent( source, model, permissionMode, agentType, + signal, ); return result.finalOutput ? createHookOutput('SessionStart', result.finalOutput) @@ -129,8 +136,12 @@ export class HookSystem { async fireSessionEndEvent( reason: SessionEndReason, + signal?: AbortSignal, ): Promise { - const result = await this.hookEventHandler.fireSessionEndEvent(reason); + const result = await this.hookEventHandler.fireSessionEndEvent( + reason, + signal, + ); return result.finalOutput ? createHookOutput('SessionEnd', result.finalOutput) : undefined; @@ -144,12 +155,14 @@ export class HookSystem { toolInput: Record, toolUseId: string, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.firePreToolUseEvent( toolName, toolInput, toolUseId, permissionMode, + signal, ); return result.finalOutput ? createHookOutput('PreToolUse', result.finalOutput) @@ -165,6 +178,7 @@ export class HookSystem { toolResponse: Record, toolUseId: string, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.firePostToolUseEvent( toolName, @@ -172,6 +186,7 @@ export class HookSystem { toolResponse, toolUseId, permissionMode, + signal, ); return result.finalOutput ? createHookOutput('PostToolUse', result.finalOutput) @@ -188,6 +203,7 @@ export class HookSystem { errorMessage: string, isInterrupt?: boolean, permissionMode?: PermissionMode, + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.firePostToolUseFailureEvent( toolUseId, @@ -196,6 +212,7 @@ export class HookSystem { errorMessage, isInterrupt, permissionMode, + signal, ); return result.finalOutput ? createHookOutput('PostToolUseFailure', result.finalOutput) @@ -208,10 +225,12 @@ export class HookSystem { async firePreCompactEvent( trigger: PreCompactTrigger, customInstructions: string = '', + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.firePreCompactEvent( trigger, customInstructions, + signal, ); return result.finalOutput ? createHookOutput('PreCompact', result.finalOutput) @@ -225,11 +244,13 @@ export class HookSystem { message: string, notificationType: NotificationType, title?: string, + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.fireNotificationEvent( message, notificationType, title, + signal, ); return result.finalOutput ? createHookOutput('Notification', result.finalOutput) @@ -243,11 +264,13 @@ export class HookSystem { agentId: string, agentType: AgentType | string, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.fireSubagentStartEvent( agentId, agentType, permissionMode, + signal, ); return result.finalOutput ? createHookOutput('SubagentStart', result.finalOutput) @@ -264,6 +287,7 @@ export class HookSystem { lastAssistantMessage: string, stopHookActive: boolean, permissionMode: PermissionMode, + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.fireSubagentStopEvent( agentId, @@ -272,6 +296,7 @@ export class HookSystem { lastAssistantMessage, stopHookActive, permissionMode, + signal, ); return result.finalOutput ? createHookOutput('SubagentStop', result.finalOutput) @@ -286,12 +311,14 @@ export class HookSystem { toolInput: Record, permissionMode: PermissionMode, permissionSuggestions?: PermissionSuggestion[], + signal?: AbortSignal, ): Promise { const result = await this.hookEventHandler.firePermissionRequestEvent( toolName, toolInput, permissionMode, permissionSuggestions, + signal, ); return result.finalOutput ? createHookOutput('PermissionRequest', result.finalOutput) diff --git a/packages/core/src/services/chatCompressionService.ts b/packages/core/src/services/chatCompressionService.ts index 0829716716..610445fb42 100644 --- a/packages/core/src/services/chatCompressionService.ts +++ b/packages/core/src/services/chatCompressionService.ts @@ -14,6 +14,7 @@ import { getCompressionPrompt } from '../core/prompts.js'; 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'; /** @@ -84,6 +85,7 @@ export class ChatCompressionService { model: string, config: Config, hasFailedCompressionAttempt: boolean, + signal?: AbortSignal, ): Promise<{ newHistory: Content[] | null; info: ChatCompressionInfo }> { const curatedHistory = chat.getHistory(true); const threshold = @@ -130,7 +132,7 @@ export class ChatCompressionService { if (hookSystem) { const trigger = force ? PreCompactTrigger.Manual : PreCompactTrigger.Auto; try { - await hookSystem.firePreCompactEvent(trigger, ''); + await hookSystem.firePreCompactEvent(trigger, '', signal); } catch (err) { config.getDebugLogger().warn(`PreCompact hook failed: ${err}`); } @@ -276,9 +278,18 @@ export class ChatCompressionService { // Fire SessionStart event after successful compression try { + const permissionMode = String( + config.getApprovalMode(), + ) as PermissionMode; await config .getHookSystem() - ?.fireSessionStartEvent(SessionStartSource.Compact, model ?? ''); + ?.fireSessionStartEvent( + SessionStartSource.Compact, + model ?? '', + permissionMode, + undefined, + signal, + ); } catch (err) { config.getDebugLogger().warn(`SessionStart hook failed: ${err}`); } diff --git a/packages/core/src/tools/agent.ts b/packages/core/src/tools/agent.ts index 1b0c1c924c..77c1be4f02 100644 --- a/packages/core/src/tools/agent.ts +++ b/packages/core/src/tools/agent.ts @@ -525,6 +525,7 @@ class AgentToolInvocation extends BaseToolInvocation { agentId, agentType, PermissionMode.Default, + signal, ); // Inject additional context from hook output into subagent context @@ -572,6 +573,7 @@ class AgentToolInvocation extends BaseToolInvocation { subagent.getFinalText(), stopHookActive, PermissionMode.Default, + signal, ); const typedStopOutput = stopHookOutput as From b87c7314875726ace4645e3782183679f39328e6 Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Mon, 23 Mar 2026 16:42:46 +0800 Subject: [PATCH 2/3] fix test --- .../core/src/services/chatCompressionService.test.ts | 9 +++++++++ packages/core/src/tools/agent.test.ts | 3 +++ 2 files changed, 12 insertions(+) diff --git a/packages/core/src/services/chatCompressionService.test.ts b/packages/core/src/services/chatCompressionService.test.ts index 074f46461f..f3f490214d 100644 --- a/packages/core/src/services/chatCompressionService.test.ts +++ b/packages/core/src/services/chatCompressionService.test.ts @@ -126,6 +126,7 @@ describe('ChatCompressionService', () => { getContentGeneratorConfig: vi.fn().mockReturnValue({}), getHookSystem: mockGetHookSystem, getModel: () => 'test-model', + getApprovalMode: () => 'default', getDebugLogger: () => ({ warn: vi.fn(), }), @@ -290,6 +291,9 @@ describe('ChatCompressionService', () => { expect(mockFireSessionStartEvent).toHaveBeenCalledWith( SessionStartSource.Compact, mockModel, + 'default', + undefined, + undefined, ); }); @@ -337,6 +341,9 @@ describe('ChatCompressionService', () => { expect(mockFireSessionStartEvent).toHaveBeenCalledWith( SessionStartSource.Compact, mockModel, + 'default', + undefined, + undefined, ); }); @@ -650,6 +657,7 @@ describe('ChatCompressionService', () => { expect(mockFirePreCompactEvent).toHaveBeenCalledWith( PreCompactTrigger.Manual, '', + undefined, ); }); @@ -699,6 +707,7 @@ describe('ChatCompressionService', () => { expect(mockFirePreCompactEvent).toHaveBeenCalledWith( PreCompactTrigger.Auto, '', + undefined, ); }); diff --git a/packages/core/src/tools/agent.test.ts b/packages/core/src/tools/agent.test.ts index aae2f6373b..ac38139b54 100644 --- a/packages/core/src/tools/agent.test.ts +++ b/packages/core/src/tools/agent.test.ts @@ -617,6 +617,7 @@ describe('AgentTool', () => { expect.stringContaining('file-search-'), 'file-search', PermissionMode.Default, + undefined, ); }); @@ -798,6 +799,7 @@ describe('AgentTool', () => { 'Task completed successfully', false, PermissionMode.Default, + undefined, ); }); @@ -842,6 +844,7 @@ describe('AgentTool', () => { 'Task completed successfully', true, PermissionMode.Default, + undefined, ); }); From 7573e1b0bdbafd657a449e959a3fe9b735f3746e Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Mon, 23 Mar 2026 19:48:08 +0800 Subject: [PATCH 3/3] add systemMessage for jook --- packages/core/src/core/client.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index d95efc844f..dfbcc38eae 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -722,6 +722,14 @@ export class GeminiClient { const stopOutput = hookOutput as StopHookOutput | undefined; + // This should happen regardless of the hook's decision + if (stopOutput?.systemMessage) { + yield { + type: GeminiEventType.HookSystemMessage, + value: stopOutput.systemMessage, + }; + } + // For Stop hooks, blocking/stop execution should force continuation if ( stopOutput?.isBlockingDecision() || @@ -732,14 +740,6 @@ export class GeminiClient { return turn; } - // Emit system message if provided (e.g., "🔄 Ralph iteration 5") - if (stopOutput.systemMessage) { - yield { - type: GeminiEventType.HookSystemMessage, - value: stopOutput.systemMessage, - }; - } - const continueReason = stopOutput.getEffectiveReason(); const continueRequest = [{ text: continueReason }]; return yield* this.sendMessageStream(