From e30746ec947feae2590c4714ba880a29700794ca Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 17 Feb 2026 18:16:36 +0000 Subject: [PATCH] fix: separate hook approval from tool approval and move PreToolUse to central location - Added new "hook" ClineAsk type to differentiate hook approval from tool approval - Moved PreToolUse hooks from inside askApproval callback to central location before switch(block.name), so hooks fire for ALL tools including read_file, ask_followup_question, and read_command_output - Added askHookApproval function using cline.ask("hook", ...) for hook-specific approval - Updated PostToolUse and Stop hooks to also use the new hook approval flow --- packages/types/src/message.ts | 3 + .../presentAssistantMessage.ts | 117 +++++++++++------- src/core/tools/AttemptCompletionTool.ts | 20 +-- 3 files changed, 84 insertions(+), 56 deletions(-) diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index ef3a831442..d6af9ffe3f 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -23,6 +23,7 @@ import { z } from "zod" * - `mistake_limit_reached`: Too many errors encountered, needs user guidance on how to proceed * - `use_mcp_server`: Permission to use Model Context Protocol (MCP) server functionality * - `auto_approval_max_req_reached`: Auto-approval limit has been reached, manual approval required + * - `hook`: Approval to execute a prompt-based hook at an agent lifecycle event */ export const clineAsks = [ "followup", @@ -36,6 +37,7 @@ export const clineAsks = [ "mistake_limit_reached", "use_mcp_server", "auto_approval_max_req_reached", + "hook", ] as const export const clineAskSchema = z.enum(clineAsks) @@ -53,6 +55,7 @@ export const idleAsks = [ "resume_completed_task", "mistake_limit_reached", "auto_approval_max_req_reached", + "hook", ] as const satisfies readonly ClineAsk[] export type IdleAsk = (typeof idleAsks)[number] diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 0907b994a6..c6fffdace8 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -527,28 +527,6 @@ export async function presentAssistantMessage(cline: Task) { approvalFeedback = { text, images } } - // === PreToolUse Hook (fires after user approves to avoid wasting API calls on rejected tools) === - try { - const hooksManager = cline.providerRef.deref()?.getHooksManager() - if (hooksManager?.hasHooksForEvent("PreToolUse")) { - const matchingHooks = hooksManager.getMatchingHooks("PreToolUse", block.name) - if (matchingHooks.length > 0) { - const hookContext: HookContext = { - event: "PreToolUse", - toolName: block.name, - toolInput: block.nativeArgs || block.params, - } - const hookResults = await executeHooks(matchingHooks, hookContext, cline.apiConfiguration) - const hookOutput = formatHookResults(hookResults) - if (hookOutput) { - await cline.say("hook_output", `PreToolUse hook for ${block.name}:\n${hookOutput}`) - } - } - } - } catch (hookError) { - console.warn(`[presentAssistantMessage] PreToolUse hook error:`, hookError) - } - return true } @@ -577,6 +555,12 @@ export async function presentAssistantMessage(cline: Task) { pushToolResult(formatResponse.toolError(errorString)) } + // === askHookApproval: separate approval flow for prompt-based hooks === + const askHookApproval = async (hookEvent: string, hookDescription: string): Promise => { + const { response } = await cline.ask("hook", JSON.stringify({ hookEvent, hookDescription })) + return response === "yesButtonClicked" + } + if (!block.partial) { // Check if this is a custom tool - if so, record as "custom_tool" (like MCP tools) const isCustomTool = stateExperiments?.customTools && customToolRegistry.has(block.name) @@ -699,6 +683,37 @@ export async function presentAssistantMessage(cline: Task) { } } + // === PreToolUse Hook (fires for ALL tools at the central location) === + if (!block.partial) { + try { + const hooksManager = cline.providerRef.deref()?.getHooksManager() + if (hooksManager?.hasHooksForEvent("PreToolUse")) { + const matchingHooks = hooksManager.getMatchingHooks("PreToolUse", block.name) + if (matchingHooks.length > 0) { + const hookDescription = `PreToolUse hook for ${block.name} (${matchingHooks.length} hook(s) will run)` + const approved = await askHookApproval("PreToolUse", hookDescription) + if (approved) { + const hookContext: HookContext = { + event: "PreToolUse", + toolName: block.name, + toolInput: block.nativeArgs || block.params, + } + const hookResults = await executeHooks( + matchingHooks, + hookContext, + cline.apiConfiguration, + ) + const hookOutput = formatHookResults(hookResults) + if (hookOutput) { + await cline.say("hook_output", `PreToolUse hook for ${block.name}:\n${hookOutput}`) + } + } + } + } + } catch (hookError) { + console.warn(`[presentAssistantMessage] PreToolUse hook error:`, hookError) + } + } switch (block.name) { case "write_to_file": await checkpointSaveAndMark(cline) @@ -948,33 +963,39 @@ export async function presentAssistantMessage(cline: Task) { if (hooksManager?.hasHooksForEvent("PostToolUse")) { const matchingHooks = hooksManager.getMatchingHooks("PostToolUse", block.name) if (matchingHooks.length > 0) { - // Get the last tool result text for context - const lastResult = cline.userMessageContent - .filter( - (b): b is import("@anthropic-ai/sdk").Anthropic.ToolResultBlockParam => - b.type === "tool_result", - ) - .pop() - const resultText = - typeof lastResult?.content === "string" - ? lastResult.content - : Array.isArray(lastResult?.content) + const hookDescription = `PostToolUse hook for ${block.name} (${matchingHooks.length} hook(s) will run)` + const approved = await askHookApproval("PostToolUse", hookDescription) + if (approved) { + // Get the last tool result text for context + const lastResult = cline.userMessageContent + .filter( + (b): b is import("@anthropic-ai/sdk").Anthropic.ToolResultBlockParam => + b.type === "tool_result", + ) + .pop() + const resultText = + typeof lastResult?.content === "string" ? lastResult.content - .filter( - (b): b is Anthropic.TextBlockParam => b.type === "text", - ) - .map((b) => b.text) - .join("\n") || "" - : "" - const hookContext: HookContext = { - event: "PostToolUse", - toolName: block.name, - toolResult: resultText.slice(0, 2000), - } - const hookResults = await executeHooks(matchingHooks, hookContext, cline.apiConfiguration) - const hookOutput = formatHookResults(hookResults) - if (hookOutput) { - await cline.say("hook_output", `PostToolUse hook for ${block.name}:\n${hookOutput}`) + : Array.isArray(lastResult?.content) + ? lastResult.content + .filter((b): b is Anthropic.TextBlockParam => b.type === "text") + .map((b) => b.text) + .join("\n") || "" + : "" + const hookContext: HookContext = { + event: "PostToolUse", + toolName: block.name, + toolResult: resultText.slice(0, 2000), + } + const hookResults = await executeHooks( + matchingHooks, + hookContext, + cline.apiConfiguration, + ) + const hookOutput = formatHookResults(hookResults) + if (hookOutput) { + await cline.say("hook_output", `PostToolUse hook for ${block.name}:\n${hookOutput}`) + } } } } diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index ec500ff36f..d6b04b397c 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -85,14 +85,18 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { const hooksManager = task.providerRef.deref()?.getHooksManager() if (hooksManager?.hasHooksForEvent("Stop")) { const stopHooks = hooksManager.getHooksForEvent("Stop") - const hookContext: HookContext = { - event: "Stop", - completionResult: result, - } - const hookResults = await executeHooks(stopHooks, hookContext, task.apiConfiguration) - const hookOutput = formatHookResults(hookResults) - if (hookOutput) { - await task.say("hook_output", `Stop hook:\n${hookOutput}`) + const hookDescription = `Stop hook (${stopHooks.length} hook(s) will run)` + const { response } = await task.ask("hook", JSON.stringify({ hookEvent: "Stop", hookDescription })) + if (response === "yesButtonClicked") { + const hookContext: HookContext = { + event: "Stop", + completionResult: result, + } + const hookResults = await executeHooks(stopHooks, hookContext, task.apiConfiguration) + const hookOutput = formatHookResults(hookResults) + if (hookOutput) { + await task.say("hook_output", `Stop hook:\n${hookOutput}`) + } } } } catch (hookError) {