feat(hooks): add tool group matchers for simplified hook configuration

- Add getToolsForGroup() helper to expand group names to tool lists
- Update HookMatcher to support group names like "edit", "read", "browser"
- Group names are case-insensitive and expand to all tools in the group
- Unknown group names log a warning but don't break the hook
- Add comprehensive tests for group expansion functionality

Supported groups: read, edit, browser, command, mcp, modes
This commit is contained in:
Toray Altas 2026-01-17 17:14:37 -05:00
parent fe0e61f232
commit 0f00dc7a46
5 changed files with 232 additions and 14 deletions

8
.roo/hooks/example.yaml Normal file
View file

@ -0,0 +1,8 @@
version: "1"
hooks:
PreToolUse:
- id: edit-verification
matcher: "edit"
enabled: true
command: 'echo "Edit verification triggered for $ROO_TOOL_NAME"'
timeout: 5

View file

@ -2,10 +2,40 @@
* Hook Matcher
*
* Provides pattern matching for hooks against tool names.
* Supports exact match, regex patterns, glob patterns, and match-all.
* Supports exact match, regex patterns, glob patterns, group names, and match-all.
*/
import { ResolvedHook } from "./types"
import { getToolsForGroup } from "../../shared/tools"
/**
* Expand group patterns in a matcher pattern.
* Only expands groups if the pattern is a simple group name or alternation of group names.
* For complex regex patterns, groups are not expanded to avoid breaking existing behavior.
*/
function expandGroupPatterns(pattern: string): string {
// Don't expand groups in patterns that contain | (to preserve existing regex alternation behavior)
if (pattern.includes("|")) {
return pattern
}
// Don't expand groups in complex patterns that contain regex metacharacters
const regexMetaChars = /[*^$+.()[\]{}\\]/
if (regexMetaChars.test(pattern)) {
return pattern // Keep complex patterns as-is
}
// Single group name
const tools = getToolsForGroup(pattern)
if (tools) {
// It's a known group, expand to all tools in the group
return tools.join("|")
} else {
// Not a group, keep as-is, but warn if it looks like it was intended as a group
console.warn(`Unknown tool group "${pattern}". Treating as literal tool name.`)
return pattern
}
}
/**
* Result of compiling a matcher pattern.
@ -46,17 +76,20 @@ export function compileMatcher(pattern: string | undefined): CompiledMatcher {
}
}
// Check if pattern looks like a regex (contains regex metacharacters except * and ?)
const regexMetaChars = /[|^$+.()[\]{}\\]/
const isRegexPattern = regexMetaChars.test(pattern)
// Expand group patterns before processing
const expandedPattern = expandGroupPatterns(pattern)
// Check if pattern looks like a glob (contains * or ?)
const isGlobPattern = /[*?]/.test(pattern) && !isRegexPattern
// Check if expanded pattern looks like a regex (contains regex metacharacters except * and ?)
const regexMetaChars = /[|^$+.()[\]{}\\]/
const isRegexPattern = regexMetaChars.test(expandedPattern)
// Check if expanded pattern looks like a glob (contains * or ?)
const isGlobPattern = /[*?]/.test(expandedPattern) && !isRegexPattern
if (isRegexPattern) {
// Treat as regex pattern
try {
const regex = new RegExp(`^(?:${pattern})$`, "i")
const regex = new RegExp(`^(?:${expandedPattern})$`, "i")
return {
pattern,
type: "regex",
@ -69,7 +102,7 @@ export function compileMatcher(pattern: string | undefined): CompiledMatcher {
return {
pattern,
type: "exact",
matches: (toolName: string) => toolName.toLowerCase() === pattern.toLowerCase(),
matches: (toolName: string) => toolName.toLowerCase() === expandedPattern.toLowerCase(),
}
}
}
@ -77,7 +110,7 @@ export function compileMatcher(pattern: string | undefined): CompiledMatcher {
if (isGlobPattern) {
// Convert glob to regex
// * matches any characters, ? matches single character
const regexPattern = pattern
const regexPattern = expandedPattern
.replace(/[.+^${}()|[\]\\]/g, "\\$&") // Escape regex special chars except * and ?
.replace(/\*/g, ".*") // * -> .*
.replace(/\?/g, ".") // ? -> .
@ -96,7 +129,7 @@ export function compileMatcher(pattern: string | undefined): CompiledMatcher {
return {
pattern,
type: "exact",
matches: (toolName: string) => toolName.toLowerCase() === pattern.toLowerCase(),
matches: (toolName: string) => toolName.toLowerCase() === expandedPattern.toLowerCase(),
}
}
}
@ -105,7 +138,7 @@ export function compileMatcher(pattern: string | undefined): CompiledMatcher {
return {
pattern,
type: "exact",
matches: (toolName: string) => toolName.toLowerCase() === pattern.toLowerCase(),
matches: (toolName: string) => toolName.toLowerCase() === expandedPattern.toLowerCase(),
}
}

View file

@ -9,6 +9,7 @@
* - Cache behavior
*/
import { vi } from "vitest"
import { compileMatcher, getMatcher, clearMatcherCache, filterMatchingHooks, hookMatchesTool } from "../HookMatcher"
import type { ResolvedHook } from "../types"
@ -77,9 +78,9 @@ describe("HookMatcher", () => {
})
it("should be case-insensitive", () => {
const matcher = compileMatcher("edit|write")
expect(matcher.matches("Edit")).toBe(true)
expect(matcher.matches("WRITE")).toBe(true)
const matcher = compileMatcher("foo|bar")
expect(matcher.matches("FOO")).toBe(true)
expect(matcher.matches("BAR")).toBe(true)
})
it("should fall back to exact match on invalid regex", () => {
@ -124,6 +125,105 @@ describe("HookMatcher", () => {
expect(matcher.matches("MCP__TOOL")).toBe(true)
})
})
describe("group expansion", () => {
it('should expand "edit" group to all edit tools', () => {
const matcher = compileMatcher("edit")
expect(matcher.type).toBe("regex")
expect(matcher.matches("apply_diff")).toBe(true)
expect(matcher.matches("write_to_file")).toBe(true)
expect(matcher.matches("generate_image")).toBe(true)
expect(matcher.matches("search_and_replace")).toBe(true)
expect(matcher.matches("search_replace")).toBe(true)
expect(matcher.matches("edit_file")).toBe(true)
expect(matcher.matches("apply_patch")).toBe(true)
expect(matcher.matches("read_file")).toBe(false)
})
it('should expand "read" group to all read tools', () => {
const matcher = compileMatcher("read")
expect(matcher.matches("read_file")).toBe(true)
expect(matcher.matches("fetch_instructions")).toBe(true)
expect(matcher.matches("search_files")).toBe(true)
expect(matcher.matches("list_files")).toBe(true)
expect(matcher.matches("codebase_search")).toBe(true)
expect(matcher.matches("write_to_file")).toBe(false)
})
it('should expand "browser" group', () => {
const matcher = compileMatcher("browser")
expect(matcher.matches("browser_action")).toBe(true)
expect(matcher.matches("read_file")).toBe(false)
})
it('should expand "command" group', () => {
const matcher = compileMatcher("command")
expect(matcher.matches("execute_command")).toBe(true)
expect(matcher.matches("read_file")).toBe(false)
})
it('should expand "mcp" group', () => {
const matcher = compileMatcher("mcp")
expect(matcher.matches("use_mcp_tool")).toBe(true)
expect(matcher.matches("access_mcp_resource")).toBe(true)
expect(matcher.matches("read_file")).toBe(false)
})
it('should expand "modes" group', () => {
const matcher = compileMatcher("modes")
expect(matcher.matches("switch_mode")).toBe(true)
expect(matcher.matches("new_task")).toBe(true)
expect(matcher.matches("read_file")).toBe(false)
})
it("should be case insensitive for groups", () => {
const matcher = compileMatcher("EDIT")
expect(matcher.matches("apply_diff")).toBe(true)
expect(matcher.matches("write_to_file")).toBe(true)
})
it("should still work with individual tool names", () => {
const matcher = compileMatcher("read_file")
expect(matcher.matches("read_file")).toBe(true)
expect(matcher.matches("write_to_file")).toBe(false)
})
it("should expand single group name", () => {
const matcher = compileMatcher("edit")
expect(matcher.matches("apply_diff")).toBe(true)
expect(matcher.matches("write_to_file")).toBe(true)
expect(matcher.matches("read_file")).toBe(false)
})
it("should treat unknown groups as literal tool names", () => {
// Mock console.warn to capture warnings
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
const matcher = compileMatcher("unknown_group")
expect(matcher.matches("unknown_group")).toBe(true)
expect(matcher.matches("read_file")).toBe(false)
// Should have warned about unknown group
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Unknown tool group "unknown_group"'),
)
consoleWarnSpy.mockRestore()
})
it("should not warn for glob patterns that look like groups", () => {
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
const matcher = compileMatcher("edit*")
expect(matcher.type).toBe("glob")
expect(matcher.matches("edit_file")).toBe(true)
// Should not warn because it contains *
expect(consoleWarnSpy).not.toHaveBeenCalled()
consoleWarnSpy.mockRestore()
})
})
})
describe("getMatcher (caching)", () => {

View file

@ -0,0 +1,64 @@
import { getToolsForGroup, TOOL_GROUPS } from "../tools"
describe("getToolsForGroup", () => {
test("should return tools for 'read' group", () => {
const result = getToolsForGroup("read")
expect(result).toEqual(["read_file", "fetch_instructions", "search_files", "list_files", "codebase_search"])
})
test("should return tools for 'edit' group (tools and customTools combined)", () => {
const result = getToolsForGroup("edit")
expect(result).toEqual([
"apply_diff",
"write_to_file",
"generate_image",
"search_and_replace",
"search_replace",
"edit_file",
"apply_patch",
])
})
test("should return tools for 'browser' group", () => {
const result = getToolsForGroup("browser")
expect(result).toEqual(["browser_action"])
})
test("should return tools for 'command' group", () => {
const result = getToolsForGroup("command")
expect(result).toEqual(["execute_command"])
})
test("should return tools for 'mcp' group", () => {
const result = getToolsForGroup("mcp")
expect(result).toEqual(["use_mcp_tool", "access_mcp_resource"])
})
test("should return tools for 'modes' group", () => {
const result = getToolsForGroup("modes")
expect(result).toEqual(["switch_mode", "new_task"])
})
test("should be case insensitive", () => {
const result = getToolsForGroup("EDIT")
expect(result).toEqual([
"apply_diff",
"write_to_file",
"generate_image",
"search_and_replace",
"search_replace",
"edit_file",
"apply_patch",
])
})
test("should return undefined for unknown groups", () => {
const result = getToolsForGroup("unknown")
expect(result).toBeUndefined()
})
test("should return undefined for empty string", () => {
const result = getToolsForGroup("")
expect(result).toBeUndefined()
})
})

View file

@ -293,6 +293,19 @@ export const TOOL_GROUPS: Record<ToolGroup, ToolGroupConfig> = {
},
}
/**
* Gets all tools for a given group name (case-insensitive).
* Returns all tools in both `tools` and `customTools` arrays combined.
* Returns undefined if the group doesn't exist.
*/
export function getToolsForGroup(groupName: string): string[] | undefined {
const group = TOOL_GROUPS[groupName.toLowerCase() as ToolGroup]
if (!group) {
return undefined
}
return [...(group.tools || []), ...(group.customTools || [])]
}
// Tools that are always available to all modes.
export const ALWAYS_AVAILABLE_TOOLS: ToolName[] = [
"ask_followup_question",