diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 8672347a58..a7f2fd5adb 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -7,6 +7,8 @@ import { HookEventType as HookEventTypeSchema, type HookEventType, type HookUpdateData, + eventSupportsMatchers, + isValidMatcherForEvent, } from "../../services/hooks/types" import { copyHookConfig } from "../../services/hooks/HookConfigWriter" import { getRooDirectoriesForCwd } from "../../services/roo-config/index.js" @@ -3689,6 +3691,22 @@ export const webviewMessageHandler = async ( : undefined, } + // Validate matcher against event type. + // Note: For backwards compatibility, the UI may send a single event as `events: [event]`. + // We also accept legacy/multi-event hooks without rejecting them; validation is warning/correction only. + if (hookUpdates.matcher && hookUpdates.events && hookUpdates.events.length > 0) { + const event = hookUpdates.events[0] + if (!eventSupportsMatchers(event)) { + console.warn(`[Hooks] Event "${event}" does not support matchers, clearing matcher`) + hookUpdates.matcher = undefined + } else if (!isValidMatcherForEvent(event, hookUpdates.matcher)) { + console.warn( + `[Hooks] Matcher "${hookUpdates.matcher}" is not valid for event "${event}", clearing matcher`, + ) + hookUpdates.matcher = undefined + } + } + await hookManager.updateHook(message.filePath, message.hookId, hookUpdates) await hookManager.reloadHooksConfig() await provider.postStateToWebview() diff --git a/src/services/hooks/HookConfigLoader.ts b/src/services/hooks/HookConfigLoader.ts index 1d5093d727..b9c2404a22 100644 --- a/src/services/hooks/HookConfigLoader.ts +++ b/src/services/hooks/HookConfigLoader.ts @@ -22,6 +22,10 @@ import { ResolvedHook, HookSource, HookEventType, + eventSupportsMatchers, + getValidMatchersForEvent, + isToolEvent, + isValidMatcherForEvent, } from "./types" import { getGlobalRooDirectory, getProjectRooDirectoryForCwd } from "../roo-config" @@ -100,7 +104,52 @@ function validateConfig( return { success: false, errors } } -function toHooksByEventMap(data: z.infer): Map { +/** + * Validate matcher against event type and log warnings for invalid combinations. + * Returns true if valid, false if invalid (but doesn't reject - for backward compat) + */ +function validateMatcherForEvent( + event: HookEventType, + matcher: string | undefined, + hookId: string, + filePath: string, +): boolean { + // If no matcher specified, always valid + if (!matcher) { + return true + } + + // If event doesn't support matchers, warn + if (!eventSupportsMatchers(event)) { + console.warn( + `[Hooks] Hook "${hookId}" in ${filePath}: Event "${event}" does not support matchers, ` + + `but matcher "${matcher}" was specified. Matcher will be ignored.`, + ) + return false + } + + // For tool events, any non-empty matcher is potentially valid (could be regex/glob) + if (isToolEvent(event)) { + return true + } + + // For other events with matchers, validate against known values + if (!isValidMatcherForEvent(event, matcher)) { + const validMatchers = getValidMatchersForEvent(event) + console.warn( + `[Hooks] Hook "${hookId}" in ${filePath}: Matcher "${matcher}" is not valid for event "${event}". ` + + `Valid matchers are: ${validMatchers?.join(", ")}`, + ) + return false + } + + return true +} + +function toHooksByEventMap( + data: z.infer, + filePath: string, +): Map { const hooks = new Map() // New format: hooks: HookDefinitionWithEvents[] @@ -108,6 +157,7 @@ function toHooksByEventMap(data: z.infer): Map) { const events: HookEventType[] = Array.isArray(def?.events) ? def.events : [] for (const event of events) { + validateMatcherForEvent(event, def.matcher, def.id, filePath) const hookDef: HookDefinition = { id: def.id, matcher: def.matcher, @@ -133,6 +183,9 @@ function toHooksByEventMap(data: z.infer): Map 0) { + for (const def of definitions) { + validateMatcherForEvent(event, def.matcher, def.id, filePath) + } hooks.set(event, definitions) } } @@ -171,7 +224,7 @@ async function loadConfigFile(filePath: string, source: HookSource): Promise { + it("TOOL_EVENTS contains all tool-related events", () => { + expect(TOOL_EVENTS).toContain("PreToolUse") + expect(TOOL_EVENTS).toContain("PostToolUse") + expect(TOOL_EVENTS).toContain("PostToolUseFailure") + expect(TOOL_EVENTS).toContain("PermissionRequest") + expect(TOOL_EVENTS).toHaveLength(4) + }) + + it("LIFECYCLE_EVENTS_WITH_MATCHERS contains events that have matchers", () => { + expect(LIFECYCLE_EVENTS_WITH_MATCHERS).toContain("SessionStart") + expect(LIFECYCLE_EVENTS_WITH_MATCHERS).toContain("Notification") + expect(LIFECYCLE_EVENTS_WITH_MATCHERS).toContain("PreCompact") + expect(LIFECYCLE_EVENTS_WITH_MATCHERS).toHaveLength(3) + }) + + it("LIFECYCLE_EVENTS_WITHOUT_MATCHERS contains events without matchers", () => { + expect(LIFECYCLE_EVENTS_WITHOUT_MATCHERS).toContain("Stop") + expect(LIFECYCLE_EVENTS_WITHOUT_MATCHERS).toContain("SubagentStart") + expect(LIFECYCLE_EVENTS_WITHOUT_MATCHERS).toContain("SubagentStop") + expect(LIFECYCLE_EVENTS_WITHOUT_MATCHERS).toContain("SessionEnd") + expect(LIFECYCLE_EVENTS_WITHOUT_MATCHERS).toContain("UserPromptSubmit") + expect(LIFECYCLE_EVENTS_WITHOUT_MATCHERS).toHaveLength(5) + }) +}) + +describe("Matcher Constants", () => { + it("TOOL_MATCHERS contains all tool groups", () => { + expect(TOOL_MATCHERS).toContain("read") + expect(TOOL_MATCHERS).toContain("edit") + expect(TOOL_MATCHERS).toContain("browser") + expect(TOOL_MATCHERS).toContain("command") + expect(TOOL_MATCHERS).toContain("mcp") + expect(TOOL_MATCHERS).toContain("modes") + expect(TOOL_MATCHERS).toHaveLength(6) + }) + + it("SESSION_START_MATCHERS contains session start triggers", () => { + expect(SESSION_START_MATCHERS).toContain("startup") + expect(SESSION_START_MATCHERS).toContain("resume") + expect(SESSION_START_MATCHERS).toContain("clear") + expect(SESSION_START_MATCHERS).toContain("compact") + expect(SESSION_START_MATCHERS).toHaveLength(4) + }) + + it("NOTIFICATION_MATCHERS contains notification types", () => { + expect(NOTIFICATION_MATCHERS).toContain("permission_prompt") + expect(NOTIFICATION_MATCHERS).toContain("idle_prompt") + expect(NOTIFICATION_MATCHERS).toContain("auth_success") + expect(NOTIFICATION_MATCHERS).toContain("elicitation_dialog") + expect(NOTIFICATION_MATCHERS).toHaveLength(4) + }) + + it("PRE_COMPACT_MATCHERS contains compaction triggers", () => { + expect(PRE_COMPACT_MATCHERS).toContain("manual") + expect(PRE_COMPACT_MATCHERS).toContain("auto") + expect(PRE_COMPACT_MATCHERS).toHaveLength(2) + }) +}) + +describe("isToolEvent", () => { + it("returns true for tool events", () => { + expect(isToolEvent("PreToolUse")).toBe(true) + expect(isToolEvent("PostToolUse")).toBe(true) + expect(isToolEvent("PostToolUseFailure")).toBe(true) + expect(isToolEvent("PermissionRequest")).toBe(true) + }) + + it("returns false for lifecycle events", () => { + expect(isToolEvent("SessionStart")).toBe(false) + expect(isToolEvent("SessionEnd")).toBe(false) + expect(isToolEvent("Stop")).toBe(false) + expect(isToolEvent("SubagentStart")).toBe(false) + expect(isToolEvent("SubagentStop")).toBe(false) + expect(isToolEvent("Notification")).toBe(false) + expect(isToolEvent("PreCompact")).toBe(false) + expect(isToolEvent("UserPromptSubmit")).toBe(false) + }) +}) + +describe("eventSupportsMatchers", () => { + it("returns true for tool events", () => { + expect(eventSupportsMatchers("PreToolUse")).toBe(true) + expect(eventSupportsMatchers("PostToolUse")).toBe(true) + expect(eventSupportsMatchers("PostToolUseFailure")).toBe(true) + expect(eventSupportsMatchers("PermissionRequest")).toBe(true) + }) + + it("returns true for lifecycle events with matchers", () => { + expect(eventSupportsMatchers("SessionStart")).toBe(true) + expect(eventSupportsMatchers("Notification")).toBe(true) + expect(eventSupportsMatchers("PreCompact")).toBe(true) + }) + + it("returns false for lifecycle events without matchers", () => { + expect(eventSupportsMatchers("Stop")).toBe(false) + expect(eventSupportsMatchers("SubagentStart")).toBe(false) + expect(eventSupportsMatchers("SubagentStop")).toBe(false) + expect(eventSupportsMatchers("SessionEnd")).toBe(false) + expect(eventSupportsMatchers("UserPromptSubmit")).toBe(false) + }) +}) + +describe("getValidMatchersForEvent", () => { + it("returns TOOL_MATCHERS for tool events", () => { + expect(getValidMatchersForEvent("PreToolUse")).toEqual(TOOL_MATCHERS) + expect(getValidMatchersForEvent("PostToolUse")).toEqual(TOOL_MATCHERS) + expect(getValidMatchersForEvent("PostToolUseFailure")).toEqual(TOOL_MATCHERS) + expect(getValidMatchersForEvent("PermissionRequest")).toEqual(TOOL_MATCHERS) + }) + + it("returns SESSION_START_MATCHERS for SessionStart", () => { + expect(getValidMatchersForEvent("SessionStart")).toEqual(SESSION_START_MATCHERS) + }) + + it("returns NOTIFICATION_MATCHERS for Notification", () => { + expect(getValidMatchersForEvent("Notification")).toEqual(NOTIFICATION_MATCHERS) + }) + + it("returns PRE_COMPACT_MATCHERS for PreCompact", () => { + expect(getValidMatchersForEvent("PreCompact")).toEqual(PRE_COMPACT_MATCHERS) + }) + + it("returns null for events without matchers", () => { + expect(getValidMatchersForEvent("Stop")).toBeNull() + expect(getValidMatchersForEvent("SubagentStart")).toBeNull() + expect(getValidMatchersForEvent("SubagentStop")).toBeNull() + expect(getValidMatchersForEvent("SessionEnd")).toBeNull() + expect(getValidMatchersForEvent("UserPromptSubmit")).toBeNull() + }) +}) + +describe("isValidMatcherForEvent", () => { + describe("for tool events", () => { + it("returns true for any non-empty string (supports regex/glob)", () => { + expect(isValidMatcherForEvent("PreToolUse", "read")).toBe(true) + expect(isValidMatcherForEvent("PreToolUse", "edit|read")).toBe(true) + expect(isValidMatcherForEvent("PreToolUse", "Write|Edit")).toBe(true) + expect(isValidMatcherForEvent("PreToolUse", "mcp__memory__.*")).toBe(true) + expect(isValidMatcherForEvent("PostToolUse", "custom-pattern")).toBe(true) + }) + + it("returns false for empty string", () => { + expect(isValidMatcherForEvent("PreToolUse", "")).toBe(false) + }) + }) + + describe("for SessionStart", () => { + it("returns true for valid matchers", () => { + expect(isValidMatcherForEvent("SessionStart", "startup")).toBe(true) + expect(isValidMatcherForEvent("SessionStart", "resume")).toBe(true) + expect(isValidMatcherForEvent("SessionStart", "startup|resume")).toBe(true) + }) + + it("returns false for invalid matchers", () => { + expect(isValidMatcherForEvent("SessionStart", "invalid")).toBe(false) + expect(isValidMatcherForEvent("SessionStart", "read")).toBe(false) + }) + }) + + describe("for Notification", () => { + it("returns true for valid matchers", () => { + expect(isValidMatcherForEvent("Notification", "permission_prompt")).toBe(true) + expect(isValidMatcherForEvent("Notification", "idle_prompt|auth_success")).toBe(true) + }) + + it("returns false for invalid matchers", () => { + expect(isValidMatcherForEvent("Notification", "invalid")).toBe(false) + expect(isValidMatcherForEvent("Notification", "read")).toBe(false) + }) + }) + + describe("for PreCompact", () => { + it("returns true for valid matchers", () => { + expect(isValidMatcherForEvent("PreCompact", "manual")).toBe(true) + expect(isValidMatcherForEvent("PreCompact", "auto")).toBe(true) + expect(isValidMatcherForEvent("PreCompact", "manual|auto")).toBe(true) + }) + + it("returns false for invalid matchers", () => { + expect(isValidMatcherForEvent("PreCompact", "invalid")).toBe(false) + }) + }) + + describe("for events without matchers", () => { + it("returns false for any matcher", () => { + expect(isValidMatcherForEvent("Stop", "anything")).toBe(false) + expect(isValidMatcherForEvent("SessionEnd", "read")).toBe(false) + expect(isValidMatcherForEvent("SubagentStart", "")).toBe(false) + }) + }) +}) + +describe("EVENT_MATCHER_MAP", () => { + it("has correct mapping for all 12 event types", () => { + // Tool events + expect(EVENT_MATCHER_MAP.PreToolUse).toEqual(TOOL_MATCHERS) + expect(EVENT_MATCHER_MAP.PostToolUse).toEqual(TOOL_MATCHERS) + expect(EVENT_MATCHER_MAP.PostToolUseFailure).toEqual(TOOL_MATCHERS) + expect(EVENT_MATCHER_MAP.PermissionRequest).toEqual(TOOL_MATCHERS) + + // Lifecycle with matchers + expect(EVENT_MATCHER_MAP.SessionStart).toEqual(SESSION_START_MATCHERS) + expect(EVENT_MATCHER_MAP.Notification).toEqual(NOTIFICATION_MATCHERS) + expect(EVENT_MATCHER_MAP.PreCompact).toEqual(PRE_COMPACT_MATCHERS) + + // Lifecycle without matchers + expect(EVENT_MATCHER_MAP.Stop).toBeNull() + expect(EVENT_MATCHER_MAP.SubagentStart).toBeNull() + expect(EVENT_MATCHER_MAP.SubagentStop).toBeNull() + expect(EVENT_MATCHER_MAP.SessionEnd).toBeNull() + expect(EVENT_MATCHER_MAP.UserPromptSubmit).toBeNull() + }) +}) + +// Ensure the exported HookEventType stays aligned with our hardcoded expectations in this spec. +// This is a no-op at runtime, but TypeScript will verify the union type. +const _eventTypeSmoke: HookEventType[] = [ + "PreToolUse", + "PostToolUse", + "PostToolUseFailure", + "PermissionRequest", + "SessionStart", + "Notification", + "PreCompact", + "Stop", + "SubagentStart", + "SubagentStop", + "SessionEnd", + "UserPromptSubmit", +] + +void _eventTypeSmoke diff --git a/src/services/hooks/types.ts b/src/services/hooks/types.ts index 46c6636098..55c577cbdd 100644 --- a/src/services/hooks/types.ts +++ b/src/services/hooks/types.ts @@ -52,6 +52,112 @@ export function isBlockingEvent(event: HookEventType): boolean { return BLOCKING_EVENTS.has(event) } +// ========================================================================== +// Event Categories and Matchers +// ========================================================================== + +// Event categories based on matcher semantics +export const TOOL_EVENTS = ["PreToolUse", "PostToolUse", "PostToolUseFailure", "PermissionRequest"] as const +export type ToolEvent = (typeof TOOL_EVENTS)[number] + +export const LIFECYCLE_EVENTS_WITH_MATCHERS = ["SessionStart", "Notification", "PreCompact"] as const +export type LifecycleEventWithMatcher = (typeof LIFECYCLE_EVENTS_WITH_MATCHERS)[number] + +export const LIFECYCLE_EVENTS_WITHOUT_MATCHERS = [ + "Stop", + "SubagentStart", + "SubagentStop", + "SessionEnd", + "UserPromptSubmit", +] as const +export type LifecycleEventWithoutMatcher = (typeof LIFECYCLE_EVENTS_WITHOUT_MATCHERS)[number] + +// Tool matchers (for tool events) +export const TOOL_MATCHERS = ["read", "edit", "browser", "command", "mcp", "modes"] as const +export type ToolMatcher = (typeof TOOL_MATCHERS)[number] + +// Session start matchers +export const SESSION_START_MATCHERS = ["startup", "resume", "clear", "compact"] as const +export type SessionStartMatcher = (typeof SESSION_START_MATCHERS)[number] + +// Notification matchers +export const NOTIFICATION_MATCHERS = ["permission_prompt", "idle_prompt", "auth_success", "elicitation_dialog"] as const +export type NotificationMatcher = (typeof NOTIFICATION_MATCHERS)[number] + +// PreCompact matchers +export const PRE_COMPACT_MATCHERS = ["manual", "auto"] as const +export type PreCompactMatcher = (typeof PRE_COMPACT_MATCHERS)[number] + +// Union of all event-specific matchers +export type EventMatcher = ToolMatcher | SessionStartMatcher | NotificationMatcher | PreCompactMatcher + +// Map from event type to valid matcher values +export const EVENT_MATCHER_MAP: Record = { + // Tool events use tool matchers + PreToolUse: TOOL_MATCHERS, + PostToolUse: TOOL_MATCHERS, + PostToolUseFailure: TOOL_MATCHERS, + PermissionRequest: TOOL_MATCHERS, + // Lifecycle events with specific matchers + SessionStart: SESSION_START_MATCHERS, + Notification: NOTIFICATION_MATCHERS, + PreCompact: PRE_COMPACT_MATCHERS, + // Lifecycle events without matchers + Stop: null, + SubagentStart: null, + SubagentStop: null, + SessionEnd: null, + UserPromptSubmit: null, +} + +/** + * Check if an event type is a tool event (uses tool matchers) + */ +export function isToolEvent(event: HookEventType): event is ToolEvent { + return (TOOL_EVENTS as readonly string[]).includes(event) +} + +/** + * Check if an event type supports matchers + */ +export function eventSupportsMatchers(event: HookEventType): boolean { + return EVENT_MATCHER_MAP[event] !== null +} + +/** + * Get valid matcher values for a given event type + * Returns null for events that don't support matchers + */ +export function getValidMatchersForEvent(event: HookEventType): readonly string[] | null { + return EVENT_MATCHER_MAP[event] +} + +/** + * Validate if a matcher value is valid for a given event type + * For tool events, also accepts custom patterns (regex/glob) + */ +export function isValidMatcherForEvent(event: HookEventType, matcher: string): boolean { + const validMatchers = EVENT_MATCHER_MAP[event] + + // Events without matchers should not have any matcher + if (validMatchers === null) { + return false + } + + // For tool events, allow custom patterns (any non-empty string is valid as it could be regex/glob) + if (isToolEvent(event)) { + return matcher.length > 0 + } + + // For other events with matchers, check against valid values (can be | separated) + const matcherParts = matcher + .split("|") + .map((m) => m.trim()) + .filter((m) => m.length > 0) + + return matcherParts.every((part) => (validMatchers as readonly string[]).includes(part)) +} + // ============================================================================ // Hook Definition Schema // ============================================================================ diff --git a/webview-ui/src/components/settings/HooksSettings.tsx b/webview-ui/src/components/settings/HooksSettings.tsx index 2feada5326..a15917461c 100644 --- a/webview-ui/src/components/settings/HooksSettings.tsx +++ b/webview-ui/src/components/settings/HooksSettings.tsx @@ -10,30 +10,105 @@ import { import { useAppTranslation } from "@src/i18n/TranslationContext" import { useExtensionState } from "@src/context/ExtensionStateContext" import { vscode } from "@src/utils/vscode" -import { Button, StandardTooltip, ToggleSwitch } from "@src/components/ui" +import { + Button, + SearchableSelect, + StandardTooltip, + ToggleSwitch, + type SearchableSelectOption, +} from "@src/components/ui" import type { HookInfo, HookExecutionRecord, HookExecutionStatusPayload } from "@roo-code/types" import { SectionHeader } from "./SectionHeader" import { Section } from "./Section" -const HOOK_EVENT_OPTIONS = [ - "PreToolUse", - "PostToolUse", - "PostToolUseFailure", - "PermissionRequest", - "UserPromptSubmit", - "Stop", - "SubagentStop", - "SubagentStart", - "SessionStart", - "SessionEnd", - "Notification", - "PreCompact", -] as const +import { + TOOL_EVENTS, + LIFECYCLE_EVENTS_WITH_MATCHERS, + LIFECYCLE_EVENTS_WITHOUT_MATCHERS, + TOOL_MATCHERS, + SESSION_START_MATCHERS, + NOTIFICATION_MATCHERS, + PRE_COMPACT_MATCHERS, + isToolEvent, + eventSupportsMatchers, + getValidMatchersForEvent, + type HookEventType, +} from "../../../../src/services/hooks/types" -type HookEventOption = (typeof HOOK_EVENT_OPTIONS)[number] +const TOOL_EVENT_DESCRIPTIONS: Record<(typeof TOOL_EVENTS)[number], string> = { + PreToolUse: "Before a tool is executed", + PostToolUse: "After a tool executes successfully", + PostToolUseFailure: "After a tool execution fails", + PermissionRequest: "When user is shown a permission dialog", +} -const TOOL_GROUPS = ["read", "edit", "browser", "command", "mcp", "modes"] as const -type ToolGroup = (typeof TOOL_GROUPS)[number] +const LIFECYCLE_EVENT_DESCRIPTIONS_WITH_MATCHERS: Record<(typeof LIFECYCLE_EVENTS_WITH_MATCHERS)[number], string> = { + SessionStart: "When a session begins", + Notification: "When a notification is sent", + PreCompact: "Before context compaction", +} + +const LIFECYCLE_EVENT_DESCRIPTIONS_WITHOUT_MATCHERS: Record< + (typeof LIFECYCLE_EVENTS_WITHOUT_MATCHERS)[number], + string +> = { + SessionEnd: "When a session ends", + Stop: "When the main agent stops", + SubagentStart: "When a subagent starts", + SubagentStop: "When a subagent stops", + UserPromptSubmit: "When user submits a prompt", +} + +// Event options with descriptions for the dropdown +const EVENT_OPTIONS: { value: HookEventType; label: string; description: string; category: string }[] = [ + ...TOOL_EVENTS.map((event) => ({ + value: event, + label: event, + description: TOOL_EVENT_DESCRIPTIONS[event], + category: "Tool Events", + })), + ...LIFECYCLE_EVENTS_WITH_MATCHERS.map((event) => ({ + value: event, + label: event, + description: LIFECYCLE_EVENT_DESCRIPTIONS_WITH_MATCHERS[event], + category: "Lifecycle Events", + })), + ...LIFECYCLE_EVENTS_WITHOUT_MATCHERS.map((event) => ({ + value: event, + label: event, + description: LIFECYCLE_EVENT_DESCRIPTIONS_WITHOUT_MATCHERS[event], + category: "Lifecycle Events", + })), +] + +// Matcher display labels +const TOOL_MATCHER_LABELS: Record = { + read: "Read (file reading)", + edit: "Edit (file writing)", + browser: "Browser (web tools)", + command: "Command (shell/bash)", + mcp: "MCP (protocol tools)", + modes: "Modes (mode tools)", +} + +const SESSION_START_MATCHER_LABELS: Record = { + startup: "Startup (new session)", + resume: "Resume (existing session)", + clear: "Clear (conversation cleared)", + compact: "Compact (context compacted)", +} + +const NOTIFICATION_MATCHER_LABELS: Record = { + permission_prompt: "Permission Prompt", + idle_prompt: "Idle Prompt", + auth_success: "Auth Success", + elicitation_dialog: "Elicitation Dialog", +} + +const PRE_COMPACT_MATCHER_LABELS: Record = { + manual: "Manual (user triggered)", + auto: "Auto (automatic)", +} const TIMEOUT_OPTIONS: Array<{ label: string; seconds: number }> = [ { label: "15 seconds", seconds: 15 }, @@ -312,90 +387,45 @@ const HookItem: React.FC = ({ hook, onToggle, autoExpandHookId, o const canEditConfig = Boolean(hook.filePath) - const hooksForId = useMemo(() => { - const enabledHooks = hooks?.enabledHooks ?? [] - return enabledHooks.filter((h) => h.id === hook.id) - }, [hooks?.enabledHooks, hook.id]) - - const selectedEvents = useMemo(() => { - // Prefer the aggregated `hook.events` when present (newer extension state). - // Fallback to the legacy state shape where the same ID appeared once per event. - const rawEvents = (hook.events && hook.events.length > 0 ? hook.events : hooksForId.map((h) => h.event)).filter( - Boolean, - ) - // Preserve stable order based on HOOK_EVENT_OPTIONS - return HOOK_EVENT_OPTIONS.filter((e) => rawEvents.includes(e)) - }, [hooksForId, hook.events]) - - const eventTooltipText = useCallback( - (event: HookEventOption) => { - // Keep translation fallback-friendly: if keys don't exist yet, show English. - const fallbackMap: Record = { - PreToolUse: "Before a tool is executed. Can block execution.", - PostToolUse: "After a tool completes successfully.", - PostToolUseFailure: "After a tool fails. Can perform cleanup.", - PermissionRequest: "When a permission dialog is shown. Can auto-approve/deny.", - UserPromptSubmit: "When user submits a prompt. Can modify or block.", - Stop: "When task completes or stops. Can perform cleanup.", - SubagentStop: "When a subtask completes.", - SubagentStart: "When a subtask begins.", - SessionStart: "When a new task session starts.", - SessionEnd: "When task session fully ends.", - Notification: "When status notifications are sent.", - PreCompact: "Before context compaction occurs.", - } - - return (t(`settings:hooks.eventTooltips.${event}`) as string) || fallbackMap[event] || event - }, - [t], - ) - - const matcherRaw = hook.matcher ?? "" - const { matcherGroups, matcherCustom } = useMemo(() => { - const parts = matcherRaw - .split("|") - .map((p) => p.trim()) - .filter(Boolean) - - const groups = parts.filter((p): p is ToolGroup => - (TOOL_GROUPS as readonly string[]).includes(p), - ) as ToolGroup[] - const customParts = parts.filter((p) => !(TOOL_GROUPS as readonly string[]).includes(p)) - - return { - matcherGroups: groups, - matcherCustom: customParts.join("|"), + const selectedEvent = useMemo(() => { + // Prefer events array if present, take first event + if (hook.events && hook.events.length > 0) { + return hook.events[0] as HookEventType } - }, [matcherRaw]) + // Fall back to legacy event field + if (hook.event) { + return hook.event as HookEventType + } + return "" + }, [hook.events, hook.event]) - const customMatcherTooltip = useMemo(() => { - return ( -
-
Regex / matcher tips
-
    -
  • - This field matches tool names. Use regex alternation with |{" "} - (e.g. fetch_instructions|search_files). -
    • -
  • - If you select tool groups and also add a custom matcher, they are combined with{" "} - |. -
    • -
  • - Available tool groups: read,{" "} - edit, browser,{" "} - command, mcp,{" "} - modes. -
    • -
-
- ) + const eventOptions = useMemo(() => { + return EVENT_OPTIONS.map((opt) => ({ value: opt.value, label: opt.label })) }, []) - const [customMatcher, setCustomMatcher] = useState(matcherCustom) + const { matcherGroups, matcherCustom: initialMatcherCustom } = useMemo(() => { + const matcher = hook.matcher || "" + if (!selectedEvent || !eventSupportsMatchers(selectedEvent)) { + return { matcherGroups: [] as string[], matcherCustom: "" } + } + + const validMatchers = getValidMatchersForEvent(selectedEvent) || [] + const parts = matcher + .split("|") + .map((p) => p.trim()) + .filter((p) => p.length > 0) + + const groups = parts.filter((p) => (validMatchers as readonly string[]).includes(p)) + const custom = parts.filter((p) => !(validMatchers as readonly string[]).includes(p)).join("|") + + return { matcherGroups: groups, matcherCustom: custom } + }, [hook.matcher, selectedEvent]) + + const [matcherCustom, setMatcherCustom] = useState(initialMatcherCustom) + useEffect(() => { - setCustomMatcher(matcherCustom) - }, [matcherCustom]) + setMatcherCustom(initialMatcherCustom) + }, [initialMatcherCustom]) useEffect(() => { setHookIdDraft(hook.id) @@ -414,8 +444,8 @@ const HookItem: React.FC = ({ hook, onToggle, autoExpandHookId, o const postHookUpdate = useCallback( (updates: { - events?: HookEventOption[] - matcher?: string + events?: HookEventType[] + matcher?: string | undefined timeout?: number id?: string command?: string @@ -433,27 +463,50 @@ const HookItem: React.FC = ({ hook, onToggle, autoExpandHookId, o [hook.filePath, hook.id], ) - const selectedEventsSet = useMemo(() => new Set(selectedEvents), [selectedEvents]) + const buildMatcherString = useCallback((groups: string[], custom: string): string => { + const parts = [...groups] + if (custom.trim()) { + parts.push(custom.trim()) + } + return parts.join("|") + }, []) - const handleEventToggle = useCallback( - (event: HookEventOption, checked: boolean) => { - // Base events should come from the current resolved view, but we use a Set to - // ensure we merge cleanly and avoid duplicates before sending to the backend. - const nextSet = new Set(selectedEventsSet) - if (checked) { - nextSet.add(event) - } else { - nextSet.delete(event) + const handleEventChange = useCallback( + (newEvent: HookEventType) => { + let newMatcher = hook.matcher + + if (!eventSupportsMatchers(newEvent)) { + newMatcher = undefined + } else if (selectedEvent && isToolEvent(selectedEvent) !== isToolEvent(newEvent)) { + newMatcher = undefined + setMatcherCustom("") } - const next = HOOK_EVENT_OPTIONS.filter((e) => nextSet.has(e)) - if (next.length === 0) { - return - } - postHookUpdate({ events: next }) + + postHookUpdate({ + events: [newEvent], + matcher: newMatcher, + }) }, - [postHookUpdate, selectedEventsSet], + [hook.matcher, postHookUpdate, selectedEvent], ) + const handleMatcherGroupToggle = useCallback( + (matcher: string) => { + const newGroups = matcherGroups.includes(matcher) + ? matcherGroups.filter((g) => g !== matcher) + : [...matcherGroups, matcher] + + const newMatcher = buildMatcherString(newGroups, matcherCustom) + postHookUpdate({ matcher: newMatcher || undefined }) + }, + [buildMatcherString, matcherCustom, matcherGroups, postHookUpdate], + ) + + const handleMatcherSave = useCallback(() => { + const newMatcher = buildMatcherString(matcherGroups, matcherCustom) + postHookUpdate({ matcher: newMatcher || undefined }) + }, [buildMatcherString, matcherCustom, matcherGroups, postHookUpdate]) + const validateHookIdDraft = useCallback( (nextId: string): string | null => { const trimmed = nextId.trim() @@ -518,15 +571,6 @@ const HookItem: React.FC = ({ hook, onToggle, autoExpandHookId, o [commandDraft], ) - const buildMatcherString = useCallback((nextGroups: ToolGroup[], nextCustom: string) => { - const parts: string[] = [...nextGroups] - const trimmedCustom = nextCustom.trim() - if (trimmedCustom.length > 0) { - parts.push(trimmedCustom) - } - return parts.join("|") - }, []) - // Filter execution history for this specific hook useEffect(() => { const history = hooks?.executionHistory || [] @@ -613,6 +657,19 @@ const HookItem: React.FC = ({ hook, onToggle, autoExpandHookId, o
{hook.id} +
+ ยท +
+ + {selectedEvent || "No event"} + + {selectedEvent && eventSupportsMatchers(selectedEvent) && hook.matcher && ( + + [{hook.matcher}] + + )} +
+
= ({ hook, onToggle, autoExpandHookId, o
- - {t("settings:hooks.event")} - -
- {HOOK_EVENT_OPTIONS.map((event) => ( - - ))} + {/* Event Type Dropdown */} +
+ + handleEventChange(value as HookEventType)} + options={eventOptions} + placeholder="Select event type..." + searchPlaceholder="Search events..." + emptyMessage="No matching events found." + className="min-w-[200px]" + data-testid={`hook-event-select-${hook.id}`} + /> + {selectedEvent && ( + + {EVENT_OPTIONS.find((o) => o.value === selectedEvent)?.description} + + )}
+ + {/* Dynamic Matcher Section */} + {selectedEvent && ( +
+ + + {!eventSupportsMatchers(selectedEvent) ? ( +
+ + + This event type does not use matchers - the hook will fire for + all occurrences. + +
+ ) : isToolEvent(selectedEvent) ? ( +
+
+ {TOOL_MATCHERS.map((matcher) => ( + + ))} +
+
+ + setMatcherCustom(e.target.value)} + onBlur={handleMatcherSave} + placeholder="e.g., Write|Edit or mcp__memory__.*" + className="bg-vscode-input-background text-vscode-input-foreground border border-vscode-input-border rounded px-2 py-1 text-xs" + /> +
+
+ ) : selectedEvent === "SessionStart" ? ( +
+ {SESSION_START_MATCHERS.map((matcher) => ( + + ))} +
+ ) : selectedEvent === "Notification" ? ( +
+ {NOTIFICATION_MATCHERS.map((matcher) => ( + + ))} +
+ ) : selectedEvent === "PreCompact" ? ( +
+ {PRE_COMPACT_MATCHERS.map((matcher) => ( + + ))} +
+ ) : null} +
+ )} + {!canEditConfig && (
{t("settings:hooks.openHookFileUnavailableTooltip")} @@ -757,65 +914,6 @@ const HookItem: React.FC = ({ hook, onToggle, autoExpandHookId, o )}
-
- - {t("settings:hooks.matcher")} - -
- {TOOL_GROUPS.map((group) => ( - - ))} -
-
-
- - - - -
- setCustomMatcher(e.target.value)} - onBlur={() => { - const next = buildMatcherString(matcherGroups, customMatcher) - postHookUpdate({ matcher: next }) - }} - placeholder="fetch_instructions|search_files" - /> -
-
-
{t("settings:hooks.timeout")} diff --git a/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx b/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx index 348e83c21d..b6f9cfab50 100644 --- a/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx @@ -74,6 +74,18 @@ vi.mock("@src/components/ui", () => ({ {children} ), + SearchableSelect: ({ value, onValueChange, options, placeholder, "data-testid": dataTestId, ...props }: any) => ( +
+ +
+ ), ToggleSwitch: ({ checked, onChange, ...props }: any) => (
), @@ -167,9 +179,10 @@ describe("HooksSettings", () => { render() - // Hook details should not be visible initially - expect(screen.queryByText(mockHook.event)).not.toBeInTheDocument() - expect(screen.queryByText(mockHook.matcher!)).not.toBeInTheDocument() + // Collapsed header should show the selected event (event-centric header) + expect(screen.getByText(mockHook.event)).toBeInTheDocument() + // Tool events support matchers; show matcher summary in header + expect(screen.getByText(`[${mockHook.matcher}]`)).toBeInTheDocument() // Click to expand const hookHeader = screen.getByText(mockHook.id).closest("div") @@ -180,10 +193,12 @@ describe("HooksSettings", () => { expect(screen.getByTestId("tab-command")).toBeInTheDocument() expect(screen.getByTestId("tab-logs")).toBeInTheDocument() - // Check Config tab content (visible by default usually or we can check panels exist) - expect(screen.getByText(mockHook.event)).toBeInTheDocument() - // Matcher is split into tool-group checkboxes + custom matcher input - const customMatcherInput = screen.getByLabelText("Custom matcher") as HTMLInputElement + // Check Config tab content: event dropdown should have the selected event + expect(screen.getByDisplayValue(mockHook.event)).toBeInTheDocument() + // Tool event matcher custom pattern input should contain the matcher string + const customMatcherInput = screen.getByPlaceholderText( + "e.g., Write|Edit or mcp__memory__.*", + ) as HTMLInputElement expect(customMatcherInput).toHaveValue(mockHook.matcher) // Command textarea should be present @@ -192,8 +207,8 @@ describe("HooksSettings", () => { // Click to collapse fireEvent.click(hookHeader!) - // Hook details should be hidden again - expect(screen.queryByText(mockHook.event)).not.toBeInTheDocument() + // Hook details should be hidden again (tabs/content not rendered) + expect(screen.queryByTestId("tab-config")).not.toBeInTheDocument() }) it("shows per-hook logs in Logs tab", () => { @@ -507,13 +522,13 @@ describe("HooksSettings", () => { render() - // Hook should be collapsed initially - expect(screen.queryByText(mockHook.event)).not.toBeInTheDocument() + // Hook should be collapsed initially (no tabs/content) + expect(screen.queryByTestId("tab-config")).not.toBeInTheDocument() fireEvent.click(screen.getByTestId("hook-enabled-toggle-hook-1")) // Hook should still be collapsed after toggling - expect(screen.queryByText(mockHook.event)).not.toBeInTheDocument() + expect(screen.queryByTestId("tab-config")).not.toBeInTheDocument() // Toggle message should have been sent expect(vscode.postMessage).toHaveBeenCalledWith({