From cc2b33bb6b321813ba4d38109925a48b63c0a21a Mon Sep 17 00:00:00 2001 From: jnjpng Date: Fri, 23 Jan 2026 15:28:18 -0800 Subject: [PATCH] feat: add stop hook continuation on blocking and example hooks (#657) --- hooks/fix-on-changes.sh | 21 +++ hooks/typecheck-on-changes.sh | 21 +++ src/cli/App.tsx | 45 +++++- src/cli/components/HooksManager.tsx | 234 ++++++++++++++++++---------- src/hooks/loader.ts | 113 ++++++++++---- src/hooks/types.ts | 48 +++++- src/hooks/writer.ts | 208 ++++++++++++++++++++----- src/tests/hooks/loader.test.ts | 60 ++++--- src/tests/settings-manager.test.ts | 22 +-- 9 files changed, 572 insertions(+), 200 deletions(-) create mode 100755 hooks/fix-on-changes.sh create mode 100755 hooks/typecheck-on-changes.sh diff --git a/hooks/fix-on-changes.sh b/hooks/fix-on-changes.sh new file mode 100755 index 0000000..4234d16 --- /dev/null +++ b/hooks/fix-on-changes.sh @@ -0,0 +1,21 @@ +#!/bin/bash +# Hook script: Run bun run fix if there are uncommitted changes +# Triggered on: Stop event + +# Check if there are any uncommitted changes (staged or unstaged) +if git diff --quiet HEAD 2>/dev/null; then + echo "No changes, skipping." + exit 0 +fi + +# Run fix - capture output and send to stderr on failure +output=$(bun run fix 2>&1) +exit_code=$? + +if [ $exit_code -eq 0 ]; then + echo "$output" + exit 0 +else + echo "$output" >&2 + exit 2 +fi diff --git a/hooks/typecheck-on-changes.sh b/hooks/typecheck-on-changes.sh new file mode 100755 index 0000000..f29e50e --- /dev/null +++ b/hooks/typecheck-on-changes.sh @@ -0,0 +1,21 @@ +#!/bin/bash +# Hook script: Run typecheck if there are uncommitted changes +# Triggered on: Stop event + +# Check if there are any uncommitted changes (staged or unstaged) +if git diff --quiet HEAD 2>/dev/null; then + echo "No changes, skipping." + exit 0 +fi + +# Run typecheck - capture output and send to stderr on failure +output=$(tsc --noEmit --pretty 2>&1) +exit_code=$? + +if [ $exit_code -eq 0 ]; then + echo "$output" + exit 0 +else + echo "$output" >&2 + exit 2 +fi diff --git a/src/cli/App.tsx b/src/cli/App.tsx index cc45e87..4e9ad6b 100644 --- a/src/cli/App.tsx +++ b/src/cli/App.tsx @@ -2359,16 +2359,51 @@ export default function App({ conversationBusyRetriesRef.current = 0; lastDequeuedMessageRef.current = null; // Clear - message was processed successfully - // Run Stop hooks (fire-and-forget) - runStopHooks( + // Run Stop hooks - if blocked/errored, continue the conversation with feedback + const stopHookResult = await runStopHooks( stopReasonToHandle, buffersRef.current.order.length, Array.from(buffersRef.current.byId.values()).filter( (item) => item.kind === "tool_call", ).length, - ).catch(() => { - // Silently ignore hook errors - }); + ); + + // If hook blocked (exit 2), inject stderr feedback and continue conversation + if (stopHookResult.blocked) { + const stderrOutput = stopHookResult.results + .map((r) => r.stderr) + .filter(Boolean) + .join("\n"); + const feedback = stderrOutput || "Stop hook blocked"; + const hookMessage = `\n${feedback}\n`; + + // Add status to transcript so user sees what's happening + const statusId = uid("status"); + buffersRef.current.byId.set(statusId, { + kind: "status", + id: statusId, + lines: [ + "Stop hook encountered blocking error, continuing loop with stderr feedback.", + ], + }); + buffersRef.current.order.push(statusId); + refreshDerived(); + + // Continue conversation with the hook feedback + setTimeout(() => { + processConversation( + [ + { + type: "message", + role: "user", + content: hookMessage, + }, + ], + { allowReentry: true }, + ); + }, 0); + return; + } // Disable eager approval check after first successful message (LET-7101) // Any new approvals from here on are from our own turn, not orphaned diff --git a/src/cli/components/HooksManager.tsx b/src/cli/components/HooksManager.tsx index d6c76a9..73bbed4 100644 --- a/src/cli/components/HooksManager.tsx +++ b/src/cli/components/HooksManager.tsx @@ -4,14 +4,24 @@ import { Box, Text, useInput } from "ink"; import TextInput from "ink-text-input"; import { memo, useCallback, useEffect, useState } from "react"; -import type { HookEvent, HookMatcher } from "../../hooks/types"; +import { + type HookEvent, + type HookMatcher, + isToolEvent, + type SimpleHookEvent, + type SimpleHookMatcher, + type ToolHookEvent, +} from "../../hooks/types"; import { addHookMatcher, + addSimpleHookMatcher, countHooksForEvent, countTotalHooks, type HookMatcherWithSource, - loadHooksWithSource, - removeHookMatcher, + type HookWithSource, + loadMatchersWithSource, + loadSimpleMatchersWithSource, + removeHook, type SaveLocation, } from "../../hooks/writer"; import { useTerminalWidth } from "../hooks/useTerminalWidth"; @@ -31,8 +41,8 @@ interface HooksManagerProps { type Screen = | "events" - | "matchers" - | "add-matcher" + | "hooks-list" // Was "matchers" - now handles both matchers and commands + | "add-matcher" // For tool events only | "add-command" | "save-location" | "delete-confirm"; @@ -127,7 +137,8 @@ export const HooksManager = memo(function HooksManager({ const [screen, setScreen] = useState("events"); const [selectedIndex, setSelectedIndex] = useState(0); const [selectedEvent, setSelectedEvent] = useState(null); - const [matchers, setMatchers] = useState([]); + // For tool events: HookMatcherWithSource[], for simple events: HookCommandWithSource[] + const [hooks, setHooks] = useState([]); const [totalHooks, setTotalHooks] = useState(0); // New hook state @@ -136,9 +147,12 @@ export const HooksManager = memo(function HooksManager({ const [selectedLocation, setSelectedLocation] = useState(0); // Delete confirmation - const [deleteMatcherIndex, setDeleteMatcherIndex] = useState(-1); + const [deleteHookIndex, setDeleteHookIndex] = useState(-1); const [deleteConfirmIndex, setDeleteConfirmIndex] = useState(1); // Default to No + // Helper to check if current event is a tool event + const isCurrentToolEvent = selectedEvent ? isToolEvent(selectedEvent) : false; + // Refresh counts - called when hooks change const refreshCounts = useCallback(() => { setTotalHooks(countTotalHooks()); @@ -151,10 +165,13 @@ export const HooksManager = memo(function HooksManager({ } }, [screen, refreshCounts]); - // Load matchers when event is selected - const loadMatchers = useCallback((event: HookEvent) => { - const loaded = loadHooksWithSource(event); - setMatchers(loaded); + // Load hooks when event is selected (matchers for both tool and simple events) + const loadHooks = useCallback((event: HookEvent) => { + if (isToolEvent(event)) { + setHooks(loadMatchersWithSource(event as ToolHookEvent)); + } else { + setHooks(loadSimpleMatchersWithSource(event as SimpleHookEvent)); + } }, []); // Handle adding a hook @@ -164,52 +181,59 @@ export const HooksManager = memo(function HooksManager({ const location = SAVE_LOCATIONS[selectedLocation]?.location; if (!location) return; - const matcher: HookMatcher = { - matcher: newMatcher.trim() || "*", - hooks: [{ type: "command", command: newCommand.trim() }], - }; + if (isToolEvent(selectedEvent)) { + // Tool events use HookMatcher with matcher pattern + const matcher: HookMatcher = { + matcher: newMatcher.trim() || "*", + hooks: [{ type: "command", command: newCommand.trim() }], + }; + await addHookMatcher(selectedEvent as ToolHookEvent, matcher, location); + } else { + // Simple events use SimpleHookMatcher (same structure, just no matcher field) + const matcher: SimpleHookMatcher = { + hooks: [{ type: "command", command: newCommand.trim() }], + }; + await addSimpleHookMatcher( + selectedEvent as SimpleHookEvent, + matcher, + location, + ); + } - await addHookMatcher(selectedEvent, matcher, location); - loadMatchers(selectedEvent); + loadHooks(selectedEvent); refreshCounts(); - // Reset and go back to matchers + // Reset and go back to hooks list setNewMatcher(""); setNewCommand(""); setSelectedLocation(0); - setScreen("matchers"); + setScreen("hooks-list"); setSelectedIndex(0); }, [ selectedEvent, newMatcher, newCommand, selectedLocation, - loadMatchers, + loadHooks, refreshCounts, ]); // Handle deleting a hook const handleDeleteHook = useCallback(async () => { - if (deleteMatcherIndex < 0 || !selectedEvent) return; + if (deleteHookIndex < 0 || !selectedEvent) return; - const matcher = matchers[deleteMatcherIndex]; - if (!matcher) return; + const hook = hooks[deleteHookIndex]; + if (!hook) return; - await removeHookMatcher(selectedEvent, matcher.sourceIndex, matcher.source); - loadMatchers(selectedEvent); + await removeHook(selectedEvent, hook.sourceIndex, hook.source); + loadHooks(selectedEvent); refreshCounts(); - // Reset and go back to matchers - setDeleteMatcherIndex(-1); - setScreen("matchers"); + // Reset and go back to hooks list + setDeleteHookIndex(-1); + setScreen("hooks-list"); setSelectedIndex(0); - }, [ - deleteMatcherIndex, - selectedEvent, - matchers, - loadMatchers, - refreshCounts, - ]); + }, [deleteHookIndex, selectedEvent, hooks, loadHooks, refreshCounts]); useInput((input, key) => { // CTRL-C: immediately cancel @@ -228,16 +252,16 @@ export const HooksManager = memo(function HooksManager({ const selected = HOOK_EVENTS[selectedIndex]; if (selected) { setSelectedEvent(selected.event); - loadMatchers(selected.event); - setScreen("matchers"); + loadHooks(selected.event); + setScreen("hooks-list"); setSelectedIndex(0); } } else if (key.escape) { onClose(); } - } else if (screen === "matchers") { - // Items: [+ Add new matcher] + existing matchers - const itemCount = matchers.length + 1; + } else if (screen === "hooks-list") { + // Items: [+ Add new hook] + existing hooks + const itemCount = hooks.length + 1; if (key.upArrow) { setSelectedIndex((prev) => Math.max(0, prev - 1)); @@ -245,15 +269,20 @@ export const HooksManager = memo(function HooksManager({ setSelectedIndex((prev) => Math.min(itemCount - 1, prev + 1)); } else if (key.return) { if (selectedIndex === 0) { - // Add new matcher - setScreen("add-matcher"); - setNewMatcher(""); + // Add new hook - for tool events, go to matcher screen; for simple, go to command + if (isCurrentToolEvent) { + setScreen("add-matcher"); + setNewMatcher(""); + } else { + setScreen("add-command"); + setNewCommand(""); + } } else { // Could add edit functionality here } } else if ((input === "d" || input === "D") && selectedIndex > 0) { - // Delete selected matcher - setDeleteMatcherIndex(selectedIndex - 1); + // Delete selected hook + setDeleteHookIndex(selectedIndex - 1); setDeleteConfirmIndex(1); // Default to No setScreen("delete-confirm"); } else if (key.escape) { @@ -262,12 +291,12 @@ export const HooksManager = memo(function HooksManager({ setSelectedEvent(null); } } else if (screen === "add-matcher") { - // Text input handles most keys + // Text input handles most keys (tool events only) if (key.return && !key.shift) { setScreen("add-command"); setNewCommand(""); } else if (key.escape) { - setScreen("matchers"); + setScreen("hooks-list"); setSelectedIndex(0); setNewMatcher(""); } @@ -276,7 +305,13 @@ export const HooksManager = memo(function HooksManager({ setScreen("save-location"); setSelectedLocation(0); } else if (key.escape) { - setScreen("add-matcher"); + // Go back to matcher screen for tool events, or hooks list for simple + if (isCurrentToolEvent) { + setScreen("add-matcher"); + } else { + setScreen("hooks-list"); + setSelectedIndex(0); + } } } else if (screen === "save-location") { if (key.upArrow) { @@ -297,10 +332,10 @@ export const HooksManager = memo(function HooksManager({ if (deleteConfirmIndex === 0) { handleDeleteHook(); } else { - setScreen("matchers"); + setScreen("hooks-list"); } } else if (key.escape) { - setScreen("matchers"); + setScreen("hooks-list"); } } }); @@ -342,49 +377,76 @@ export const HooksManager = memo(function HooksManager({ ); } - // Render Matchers List - if (screen === "matchers" && selectedEvent) { + // Render Hooks List (matchers for tool events, commands for simple events) + if (screen === "hooks-list" && selectedEvent) { + const title = isCurrentToolEvent + ? ` ${selectedEvent} - Tool Matchers ` + : ` ${selectedEvent} - Hooks `; + const addLabel = isCurrentToolEvent + ? "+ Add new matcher..." + : "+ Add new hook..."; + return ( {boxTop(boxWidth)} - {boxLine(` ${selectedEvent} - Tool Matchers `, boxWidth)} + {boxLine(title, boxWidth)} {boxBottom(boxWidth)} - Input to command is JSON of tool call arguments. - Exit code 0 - stdout/stderr not shown - - Exit code 2 - show stderr to model and block tool call - - - Other exit codes - show stderr to user only but continue - + {isCurrentToolEvent ? ( + <> + + Input to command is JSON of tool call arguments. + + Exit code 0 - stdout/stderr not shown + + Exit code 2 - show stderr to model and block tool call + + + Other exit codes - show stderr to user only but continue + + + ) : ( + <> + Exit code 0 - success, continue + Exit code 2 - show stderr to model and block + Other exit codes - show stderr to user only + + )} - {/* Add new matcher option */} + {/* Add new hook option */} {selectedIndex === 0 ? "❯" : " "} 1.{" "} - + Add new matcher... + {addLabel} - {/* Existing matchers */} - {matchers.map((matcher, index) => { + {/* Existing hooks */} + {hooks.map((hook, index) => { const isSelected = index + 1 === selectedIndex; const prefix = isSelected ? "❯" : " "; - const sourceLabel = `[${getSourceLabel(matcher.source)}]`; - const matcherPattern = matcher.matcher || "*"; - const command = matcher.hooks[0]?.command || ""; + const sourceLabel = `[${getSourceLabel(hook.source)}]`; + + // Handle both tool matchers (with matcher field) and simple matchers (without) + const isToolMatcher = "matcher" in hook; + const matcherPattern = isToolMatcher + ? (hook as HookMatcherWithSource).matcher || "*" + : null; + // Both types have hooks array + const command = "hooks" in hook ? hook.hooks[0]?.command || "" : ""; const truncatedCommand = command.length > 30 ? `${command.slice(0, 27)}...` : command; return ( - + {prefix} {index + 2}.{" "} {sourceLabel} - {matcherPattern.padEnd(12)} + {matcherPattern !== null && ( + {matcherPattern.padEnd(12)} + )} {truncatedCommand} ); @@ -440,18 +502,20 @@ export const HooksManager = memo(function HooksManager({ ); } - // Render Add Matcher - Command Input + // Render Add Command Input if (screen === "add-command" && selectedEvent) { + const title = isCurrentToolEvent + ? ` Add new matcher for ${selectedEvent} ` + : ` Add new hook for ${selectedEvent} `; + return ( {boxTop(boxWidth)} - - {boxLine(` Add new matcher for ${selectedEvent} `, boxWidth)} - + {boxLine(title, boxWidth)} {boxBottom(boxWidth)} - Matcher: {newMatcher || "*"} - + {isCurrentToolEvent && Matcher: {newMatcher || "*"}} + {isCurrentToolEvent && } Command: {boxTop(boxWidth - 2)} @@ -481,7 +545,7 @@ export const HooksManager = memo(function HooksManager({ Event: {selectedEvent} - Matcher: {newMatcher || "*"} + {isCurrentToolEvent && Matcher: {newMatcher || "*"}} Command: {newCommand} @@ -509,8 +573,14 @@ export const HooksManager = memo(function HooksManager({ } // Render Delete Confirmation - if (screen === "delete-confirm" && deleteMatcherIndex >= 0) { - const matcher = matchers[deleteMatcherIndex]; + if (screen === "delete-confirm" && deleteHookIndex >= 0) { + const hook = hooks[deleteHookIndex]; + const isToolMatcher = hook && "matcher" in hook; + const matcherPattern = isToolMatcher + ? (hook as HookMatcherWithSource).matcher || "*" + : null; + // Both types have hooks array + const command = hook && "hooks" in hook ? hook.hooks[0]?.command : ""; return ( @@ -519,9 +589,9 @@ export const HooksManager = memo(function HooksManager({ {boxBottom(boxWidth)} - Matcher: {matcher?.matcher || "*"} - Command: {matcher?.hooks[0]?.command} - Source: {matcher ? getSourceLabel(matcher.source) : ""} + {matcherPattern !== null && Matcher: {matcherPattern}} + Command: {command} + Source: {hook ? getSourceLabel(hook.source) : ""} diff --git a/src/hooks/loader.ts b/src/hooks/loader.ts index 315354c..a1bb85d 100644 --- a/src/hooks/loader.ts +++ b/src/hooks/loader.ts @@ -2,7 +2,16 @@ // Loads and matches hooks from settings-manager import { settingsManager } from "../settings-manager"; -import type { HookCommand, HookEvent, HooksConfig } from "./types"; +import { + type HookCommand, + type HookEvent, + type HookMatcher, + type HooksConfig, + isToolEvent, + type SimpleHookEvent, + type SimpleHookMatcher, + type ToolHookEvent, +} from "./types"; /** * Clear hooks cache - kept for API compatibility with existing callers. @@ -71,7 +80,7 @@ export async function loadProjectLocalHooks( /** * Merge hooks configurations * Priority order: project-local > project > global - * For each event, matchers are ordered by priority (local first, global last) + * For each event, hooks are ordered by priority (local first, global last) */ export function mergeHooksConfigs( global: HooksConfig, @@ -86,15 +95,34 @@ export function mergeHooksConfigs( ]) as Set; for (const event of allEvents) { - const globalMatchers = global[event] || []; - const projectMatchers = project[event] || []; - const projectLocalMatchers = projectLocal[event] || []; - // Project-local matchers run first, then project, then global - merged[event] = [ - ...projectLocalMatchers, - ...projectMatchers, - ...globalMatchers, - ]; + if (isToolEvent(event)) { + // Tool events use HookMatcher[] + const toolEvent = event as ToolHookEvent; + const globalMatchers = (global[toolEvent] || []) as HookMatcher[]; + const projectMatchers = (project[toolEvent] || []) as HookMatcher[]; + const projectLocalMatchers = (projectLocal[toolEvent] || + []) as HookMatcher[]; + // Project-local runs first, then project, then global + (merged as Record)[toolEvent] = [ + ...projectLocalMatchers, + ...projectMatchers, + ...globalMatchers, + ]; + } else { + // Simple events use SimpleHookMatcher[] (same as HookMatcher but without matcher field) + const simpleEvent = event as SimpleHookEvent; + const globalMatchers = (global[simpleEvent] || []) as SimpleHookMatcher[]; + const projectMatchers = (project[simpleEvent] || + []) as SimpleHookMatcher[]; + const projectLocalMatchers = (projectLocal[simpleEvent] || + []) as SimpleHookMatcher[]; + // Project-local runs first, then project, then global + (merged as Record)[simpleEvent] = [ + ...projectLocalMatchers, + ...projectMatchers, + ...globalMatchers, + ]; + } } return merged; @@ -146,22 +174,37 @@ export function getMatchingHooks( event: HookEvent, toolName?: string, ): HookCommand[] { - const matchers = config[event]; - if (!matchers || matchers.length === 0) { - return []; - } + if (isToolEvent(event)) { + // Tool events use HookMatcher[] - need to match against tool name + const matchers = config[event as ToolHookEvent] as + | HookMatcher[] + | undefined; + if (!matchers || matchers.length === 0) { + return []; + } - const hooks: HookCommand[] = []; + const hooks: HookCommand[] = []; + for (const matcher of matchers) { + if (!toolName || matchesTool(matcher.matcher, toolName)) { + hooks.push(...matcher.hooks); + } + } + return hooks; + } else { + // Simple events use SimpleHookMatcher[] - extract hooks from each matcher + const matchers = config[event as SimpleHookEvent] as + | SimpleHookMatcher[] + | undefined; + if (!matchers || matchers.length === 0) { + return []; + } - for (const matcher of matchers) { - // For non-tool events, matcher is usually empty/"*" - // For tool events, check if the tool matches - if (!toolName || matchesTool(matcher.matcher, toolName)) { + const hooks: HookCommand[] = []; + for (const matcher of matchers) { hooks.push(...matcher.hooks); } + return hooks; } - - return hooks; } /** @@ -171,13 +214,27 @@ export function hasHooksForEvent( config: HooksConfig, event: HookEvent, ): boolean { - const matchers = config[event]; - if (!matchers || matchers.length === 0) { - return false; + if (isToolEvent(event)) { + // Tool events use HookMatcher[] + const matchers = config[event as ToolHookEvent] as + | HookMatcher[] + | undefined; + if (!matchers || matchers.length === 0) { + return false; + } + // Check if any matcher has hooks + return matchers.some((m) => m.hooks && m.hooks.length > 0); + } else { + // Simple events use SimpleHookMatcher[] + const matchers = config[event as SimpleHookEvent] as + | SimpleHookMatcher[] + | undefined; + if (!matchers || matchers.length === 0) { + return false; + } + // Check if any matcher has hooks + return matchers.some((m) => m.hooks && m.hooks.length > 0); } - - // Check if any matcher has hooks - return matchers.some((m) => m.hooks && m.hooks.length > 0); } /** diff --git a/src/hooks/types.ts b/src/hooks/types.ts index e8ad623..c45fdb9 100644 --- a/src/hooks/types.ts +++ b/src/hooks/types.ts @@ -2,12 +2,17 @@ // Types for Letta Code hooks system (Claude Code-compatible) /** - * Hook event types that can trigger hooks + * Tool-related hook events that require matchers to specify which tools to match */ -export type HookEvent = +export type ToolHookEvent = | "PreToolUse" // Runs before tool calls (can block them) | "PostToolUse" // Runs after tool calls complete (cannot block) - | "PermissionRequest" // Runs when a permission dialog is shown (can allow or deny) + | "PermissionRequest"; // Runs when a permission dialog is shown (can allow or deny) + +/** + * Simple hook events that don't require matchers + */ +export type SimpleHookEvent = | "UserPromptSubmit" // Runs when the user submits a prompt (can block) | "Notification" // Runs when a notification is sent (cannot block) | "Stop" // Runs when the agent finishes responding (can block) @@ -17,6 +22,11 @@ export type HookEvent = | "SessionStart" // Runs when a new session starts or is resumed | "SessionEnd"; // Runs when session ends (cannot block) +/** + * All hook event types + */ +export type HookEvent = ToolHookEvent | SimpleHookEvent; + /** * Individual hook command configuration */ @@ -30,7 +40,7 @@ export interface HookCommand { } /** - * Hook matcher configuration - matches hooks to specific tools/events + * Hook matcher configuration for tool events - matches hooks to specific tools */ export interface HookMatcher { /** @@ -44,13 +54,41 @@ export interface HookMatcher { hooks: HookCommand[]; } +/** + * Simple hook matcher for non-tool events - no matcher needed, just hooks + */ +export interface SimpleHookMatcher { + /** List of hooks to run */ + hooks: HookCommand[]; +} + /** * Full hooks configuration stored in settings + * - Tool events (PreToolUse, PostToolUse, PermissionRequest) use HookMatcher[] with matcher patterns + * - Simple events use SimpleHookMatcher[] (same structure, just no matcher field) */ export type HooksConfig = { - [K in HookEvent]?: HookMatcher[]; + [K in ToolHookEvent]?: HookMatcher[]; +} & { + [K in SimpleHookEvent]?: SimpleHookMatcher[]; }; +/** + * Set of tool events that require matchers + */ +export const TOOL_EVENTS: Set = new Set([ + "PreToolUse", + "PostToolUse", + "PermissionRequest", +]); + +/** + * Type guard to check if an event is a tool event + */ +export function isToolEvent(event: HookEvent): event is ToolHookEvent { + return TOOL_EVENTS.has(event); +} + /** * Exit codes from hook execution */ diff --git a/src/hooks/writer.ts b/src/hooks/writer.ts index 4c3c70b..f1cb67e 100644 --- a/src/hooks/writer.ts +++ b/src/hooks/writer.ts @@ -2,7 +2,15 @@ // Functions to write hooks to settings files via settings-manager import { settingsManager } from "../settings-manager"; -import type { HookEvent, HookMatcher, HooksConfig } from "./types"; +import { + type HookEvent, + type HookMatcher, + type HooksConfig, + isToolEvent, + type SimpleHookEvent, + type SimpleHookMatcher, + type ToolHookEvent, +} from "./types"; /** * Save location for hooks @@ -71,10 +79,10 @@ export async function saveHooksToLocation( } /** - * Add a new hook matcher to an event + * Add a new hook matcher to a tool event (PreToolUse, PostToolUse, PermissionRequest) */ export async function addHookMatcher( - event: HookEvent, + event: ToolHookEvent, matcher: HookMatcher, location: SaveLocation, workingDirectory: string = process.cwd(), @@ -83,61 +91,91 @@ export async function addHookMatcher( // Initialize event array if needed if (!hooks[event]) { - hooks[event] = []; + (hooks as Record)[event] = []; } // Add the new matcher - const eventMatchers = hooks[event]; - if (eventMatchers) { - eventMatchers.push(matcher); + const eventMatchers = hooks[event] as HookMatcher[]; + eventMatchers.push(matcher); + + await saveHooksToLocation(hooks, location, workingDirectory); +} + +/** + * Add a new hook matcher to a simple event (non-tool events) + * Simple events use the same structure as tool events but without the matcher field + */ +export async function addSimpleHookMatcher( + event: SimpleHookEvent, + matcher: SimpleHookMatcher, + location: SaveLocation, + workingDirectory: string = process.cwd(), +): Promise { + const hooks = loadHooksFromLocation(location, workingDirectory); + + // Initialize event array if needed + if (!hooks[event]) { + (hooks as Record)[event] = []; } + // Add the new matcher + const eventMatchers = hooks[event] as SimpleHookMatcher[]; + eventMatchers.push(matcher); + await saveHooksToLocation(hooks, location, workingDirectory); } /** * Remove a hook matcher from an event by index + * Works for both tool events (HookMatcher) and simple events (SimpleHookMatcher) */ -export async function removeHookMatcher( +export async function removeHook( event: HookEvent, - matcherIndex: number, + index: number, location: SaveLocation, workingDirectory: string = process.cwd(), ): Promise { const hooks = loadHooksFromLocation(location, workingDirectory); - const eventMatchers = hooks[event]; - if ( - !eventMatchers || - matcherIndex < 0 || - matcherIndex >= eventMatchers.length - ) { - throw new Error(`Invalid matcher index ${matcherIndex} for event ${event}`); - } - - // Remove the matcher at the given index - eventMatchers.splice(matcherIndex, 1); - - // Clean up empty arrays - if (eventMatchers.length === 0) { - delete hooks[event]; + if (isToolEvent(event)) { + const eventMatchers = hooks[event as ToolHookEvent] as + | HookMatcher[] + | undefined; + if (!eventMatchers || index < 0 || index >= eventMatchers.length) { + throw new Error(`Invalid matcher index ${index} for event ${event}`); + } + eventMatchers.splice(index, 1); + if (eventMatchers.length === 0) { + delete hooks[event as ToolHookEvent]; + } + } else { + const eventMatchers = hooks[event as SimpleHookEvent] as + | SimpleHookMatcher[] + | undefined; + if (!eventMatchers || index < 0 || index >= eventMatchers.length) { + throw new Error(`Invalid matcher index ${index} for event ${event}`); + } + eventMatchers.splice(index, 1); + if (eventMatchers.length === 0) { + delete hooks[event as SimpleHookEvent]; + } } await saveHooksToLocation(hooks, location, workingDirectory); } /** - * Update a hook matcher at a specific index + * Update a hook matcher at a specific index (tool events only) */ export async function updateHookMatcher( - event: HookEvent, + event: ToolHookEvent, matcherIndex: number, matcher: HookMatcher, location: SaveLocation, workingDirectory: string = process.cwd(), ): Promise { const hooks = loadHooksFromLocation(location, workingDirectory); - const eventMatchers = hooks[event]; + const eventMatchers = hooks[event] as HookMatcher[] | undefined; if ( !eventMatchers || @@ -147,14 +185,39 @@ export async function updateHookMatcher( throw new Error(`Invalid matcher index ${matcherIndex} for event ${event}`); } - // Update the matcher at the given index eventMatchers[matcherIndex] = matcher; await saveHooksToLocation(hooks, location, workingDirectory); } /** - * Hook matcher with source tracking for display + * Update a hook matcher at a specific index (simple events only) + */ +export async function updateSimpleHookMatcher( + event: SimpleHookEvent, + matcherIndex: number, + matcher: SimpleHookMatcher, + location: SaveLocation, + workingDirectory: string = process.cwd(), +): Promise { + const hooks = loadHooksFromLocation(location, workingDirectory); + const eventMatchers = hooks[event] as SimpleHookMatcher[] | undefined; + + if ( + !eventMatchers || + matcherIndex < 0 || + matcherIndex >= eventMatchers.length + ) { + throw new Error(`Invalid matcher index ${matcherIndex} for event ${event}`); + } + + eventMatchers[matcherIndex] = matcher; + + await saveHooksToLocation(hooks, location, workingDirectory); +} + +/** + * Hook matcher with source tracking for display (tool events) */ export interface HookMatcherWithSource extends HookMatcher { source: SaveLocation; @@ -162,21 +225,62 @@ export interface HookMatcherWithSource extends HookMatcher { } /** - * Load all hooks for an event with source tracking - * Returns matchers tagged with their source location + * Simple hook matcher with source tracking for display (simple events) */ -export function loadHooksWithSource( - event: HookEvent, +export interface SimpleHookMatcherWithSource extends SimpleHookMatcher { + source: SaveLocation; + sourceIndex: number; // Index within that source file +} + +/** + * Union type for hooks with source tracking + */ +export type HookWithSource = + | HookMatcherWithSource + | SimpleHookMatcherWithSource; + +/** + * Load all hook matchers for a tool event with source tracking + */ +export function loadMatchersWithSource( + event: ToolHookEvent, workingDirectory: string = process.cwd(), ): HookMatcherWithSource[] { const result: HookMatcherWithSource[] = []; - - // Load from each location and tag with source const locations: SaveLocation[] = ["project-local", "project", "user"]; for (const location of locations) { const hooks = loadHooksFromLocation(location, workingDirectory); - const matchers = hooks[event] || []; + const matchers = (hooks[event] || []) as HookMatcher[]; + + for (let i = 0; i < matchers.length; i++) { + const matcher = matchers[i]; + if (matcher) { + result.push({ + ...matcher, + source: location, + sourceIndex: i, + }); + } + } + } + + return result; +} + +/** + * Load all hook matchers for a simple event with source tracking + */ +export function loadSimpleMatchersWithSource( + event: SimpleHookEvent, + workingDirectory: string = process.cwd(), +): SimpleHookMatcherWithSource[] { + const result: SimpleHookMatcherWithSource[] = []; + const locations: SaveLocation[] = ["project-local", "project", "user"]; + + for (const location of locations) { + const hooks = loadHooksFromLocation(location, workingDirectory); + const matchers = (hooks[event] || []) as SimpleHookMatcher[]; for (let i = 0; i < matchers.length; i++) { const matcher = matchers[i]; @@ -206,9 +310,19 @@ export function countTotalHooks( for (const location of locations) { const hooks = loadHooksFromLocation(location, workingDirectory); for (const event of Object.keys(hooks) as HookEvent[]) { - const matchers = hooks[event] || []; - for (const matcher of matchers) { - count += matcher.hooks.length; + if (isToolEvent(event)) { + // Tool events have HookMatcher[] with nested hooks + const matchers = (hooks[event as ToolHookEvent] || []) as HookMatcher[]; + for (const matcher of matchers) { + count += matcher.hooks.length; + } + } else { + // Simple events have SimpleHookMatcher[] with nested hooks + const matchers = (hooks[event as SimpleHookEvent] || + []) as SimpleHookMatcher[]; + for (const matcher of matchers) { + count += matcher.hooks.length; + } } } } @@ -229,9 +343,19 @@ export function countHooksForEvent( for (const location of locations) { const hooks = loadHooksFromLocation(location, workingDirectory); - const matchers = hooks[event] || []; - for (const matcher of matchers) { - count += matcher.hooks.length; + if (isToolEvent(event)) { + // Tool events have HookMatcher[] with nested hooks + const matchers = (hooks[event as ToolHookEvent] || []) as HookMatcher[]; + for (const matcher of matchers) { + count += matcher.hooks.length; + } + } else { + // Simple events have SimpleHookMatcher[] with nested hooks + const matchers = (hooks[event as SimpleHookEvent] || + []) as SimpleHookMatcher[]; + for (const matcher of matchers) { + count += matcher.hooks.length; + } } } diff --git a/src/tests/hooks/loader.test.ts b/src/tests/hooks/loader.test.ts index 35eb875..dfbbaf6 100644 --- a/src/tests/hooks/loader.test.ts +++ b/src/tests/hooks/loader.test.ts @@ -11,7 +11,13 @@ import { matchesTool, mergeHooksConfigs, } from "../../hooks/loader"; -import type { HookEvent, HooksConfig } from "../../hooks/types"; +import { + type HookEvent, + type HooksConfig, + isToolEvent, + type SimpleHookEvent, + type ToolHookEvent, +} from "../../hooks/types"; import { settingsManager } from "../../settings-manager"; describe("Hooks Loader", () => { @@ -251,11 +257,9 @@ describe("Hooks Loader", () => { test("handles undefined tool name (for non-tool events)", () => { const config: HooksConfig = { + // Simple events use SimpleHookMatcher[] (hooks wrapper, no matcher) SessionStart: [ - { - matcher: "*", - hooks: [{ type: "command", command: "session hook" }], - }, + { hooks: [{ type: "command", command: "session hook" }] }, ], }; @@ -367,12 +371,20 @@ describe("Hooks Loader", () => { test("config can have all 11 event types", () => { const config: HooksConfig = {}; for (const event of allEvents) { - config[event] = [ - { - matcher: "*", - hooks: [{ type: "command", command: `echo ${event}` }], - }, - ]; + if (isToolEvent(event)) { + // Tool events use HookMatcher[] + (config as Record)[event as ToolHookEvent] = [ + { + matcher: "*", + hooks: [{ type: "command", command: `echo ${event}` }], + }, + ]; + } else { + // Simple events use SimpleHookMatcher[] (hooks wrapper) + (config as Record)[ + event as SimpleHookEvent + ] = [{ hooks: [{ type: "command", command: `echo ${event}` }] }]; + } } for (const event of allEvents) { @@ -384,21 +396,21 @@ describe("Hooks Loader", () => { test("merging preserves all event types", () => { const global: HooksConfig = { + // Tool events use HookMatcher[] PreToolUse: [ { matcher: "*", hooks: [{ type: "command", command: "g1" }] }, ], - SessionStart: [ - { matcher: "*", hooks: [{ type: "command", command: "g2" }] }, - ], + // Simple events use SimpleHookMatcher[] (hooks wrapper) + SessionStart: [{ hooks: [{ type: "command", command: "g2" }] }], }; const project: HooksConfig = { + // Tool events use HookMatcher[] PostToolUse: [ { matcher: "*", hooks: [{ type: "command", command: "p1" }] }, ], - SessionEnd: [ - { matcher: "*", hooks: [{ type: "command", command: "p2" }] }, - ], + // Simple events use SimpleHookMatcher[] (hooks wrapper) + SessionEnd: [{ hooks: [{ type: "command", command: "p2" }] }], }; const merged = mergeHooksConfigs(global, project); @@ -503,14 +515,15 @@ describe("Hooks Loader", () => { test("all three levels merge correctly", () => { const global: HooksConfig = { + // Tool event with HookMatcher[] PreToolUse: [ { matcher: "*", hooks: [{ type: "command", command: "global" }] }, ], - SessionEnd: [ - { matcher: "*", hooks: [{ type: "command", command: "global-end" }] }, - ], + // Simple event with SimpleHookMatcher[] + SessionEnd: [{ hooks: [{ type: "command", command: "global-end" }] }], }; const project: HooksConfig = { + // Tool event with HookMatcher[] PreToolUse: [ { matcher: "*", hooks: [{ type: "command", command: "project" }] }, ], @@ -522,14 +535,13 @@ describe("Hooks Loader", () => { ], }; const projectLocal: HooksConfig = { + // Tool event with HookMatcher[] PreToolUse: [ { matcher: "*", hooks: [{ type: "command", command: "local" }] }, ], + // Simple event with SimpleHookMatcher[] SessionStart: [ - { - matcher: "*", - hooks: [{ type: "command", command: "local-start" }], - }, + { hooks: [{ type: "command", command: "local-start" }] }, ], }; diff --git a/src/tests/settings-manager.test.ts b/src/tests/settings-manager.test.ts index bc74bd0..a23d530 100644 --- a/src/tests/settings-manager.test.ts +++ b/src/tests/settings-manager.test.ts @@ -535,17 +535,16 @@ describe("Settings Manager - Hooks", () => { test("Hooks configuration persists to disk", async () => { settingsManager.updateSettings({ hooks: { + // Tool event with HookMatcher[] PreToolUse: [ { matcher: "*", hooks: [{ type: "command", command: "echo persisted" }], }, ], + // Simple event with SimpleHookMatcher[] SessionStart: [ - { - matcher: "*", - hooks: [{ type: "command", command: "echo session" }], - }, + { hooks: [{ type: "command", command: "echo session" }] }, ], }, }); @@ -594,11 +593,9 @@ describe("Settings Manager - Hooks", () => { settingsManager.updateLocalProjectSettings( { hooks: { + // Simple event uses SimpleHookMatcher[] (hooks wrapper) UserPromptSubmit: [ - { - matcher: "*", - hooks: [{ type: "command", command: "echo local-hook" }], - }, + { hooks: [{ type: "command", command: "echo local-hook" }] }, ], }, }, @@ -616,12 +613,8 @@ describe("Settings Manager - Hooks", () => { settingsManager.updateLocalProjectSettings( { hooks: { - Stop: [ - { - matcher: "*", - hooks: [{ type: "command", command: "echo stop-hook" }], - }, - ], + // Simple event uses SimpleHookMatcher[] (hooks wrapper) + Stop: [{ hooks: [{ type: "command", command: "echo stop-hook" }] }], }, }, testProjectDir, @@ -637,6 +630,7 @@ describe("Settings Manager - Hooks", () => { await settingsManager.loadLocalProjectSettings(testProjectDir); expect(reloaded.hooks?.Stop).toHaveLength(1); + // Simple event hooks are in SimpleHookMatcher format with hooks array expect(reloaded.hooks?.Stop?.[0]?.hooks[0]?.command).toBe("echo stop-hook"); });