From e97be9a47f6e403c9e463beea251a184f8659eda Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Thu, 30 Oct 2025 11:35:39 +0100 Subject: [PATCH] chore: improvements and fixes --- internal/agent/agent.go | 59 +++++++--- internal/agent/agent_tool.go | 17 +++ internal/agent/common_test.go | 2 +- internal/agent/coordinator.go | 4 +- internal/app/app.go | 10 +- internal/config/hooks.go | 26 ++-- internal/hooks/HOOKS.md | 157 ++++++++++++++++++++----- internal/hooks/hooks.go | 63 +++++++--- internal/hooks/hooks_test.go | 117 ++++++++++++++++++ internal/permission/permission.go | 31 +++-- internal/permission/permission_test.go | 10 +- schema.json | 4 +- 12 files changed, 402 insertions(+), 98 deletions(-) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 76503631fa9b80c32b8a95d9c2a6a54d8910e758..83b3238c3a7f0f916481838346255d88eaf56478 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -175,13 +175,15 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (*fantasy // Execute UserPromptSubmit hook if a.hooks != nil { - _ = a.hooks.Execute(ctx, hooks.HookContext{ + if err := a.hooks.Execute(ctx, hooks.HookContext{ EventType: config.UserPromptSubmit, SessionID: call.SessionID, UserPrompt: call.Prompt, Provider: a.largeModel.ModelCfg.Provider, Model: a.largeModel.ModelCfg.Model, - }) + }); err != nil { + slog.Debug("user_prompt_submit hook execution failed", "error", err) + } } // add the session to the context @@ -312,18 +314,19 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (*fantasy // Execute PreToolUse hook - blocks tool execution on error if a.hooks != nil { toolInput := make(map[string]any) - if err := json.Unmarshal([]byte(tc.Input), &toolInput); err == nil { - if err := a.hooks.Execute(genCtx, hooks.HookContext{ - EventType: config.PreToolUse, - SessionID: call.SessionID, - ToolName: tc.ToolName, - ToolInput: toolInput, - MessageID: currentAssistant.ID, - Provider: a.largeModel.ModelCfg.Provider, - Model: a.largeModel.ModelCfg.Model, - }); err != nil { - return fmt.Errorf("PreToolUse hook blocked tool execution: %w", err) - } + if err := json.Unmarshal([]byte(tc.Input), &toolInput); err != nil { + slog.Warn("Failed to unmarshal tool input for PreToolUse hook", "error", err, "tool", tc.ToolName) + } + if err := a.hooks.Execute(genCtx, hooks.HookContext{ + EventType: config.PreToolUse, + SessionID: call.SessionID, + ToolName: tc.ToolName, + ToolInput: toolInput, + MessageID: currentAssistant.ID, + Provider: a.largeModel.ModelCfg.Provider, + Model: a.largeModel.ModelCfg.Model, + }); err != nil { + return fmt.Errorf("PreToolUse hook blocked tool execution: %w", err) } } @@ -363,12 +366,14 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (*fantasy toolCalls := currentAssistant.ToolCalls() for _, tc := range toolCalls { if tc.ID == result.ToolCallID { - _ = json.Unmarshal([]byte(tc.Input), &toolInput) + if err := json.Unmarshal([]byte(tc.Input), &toolInput); err != nil { + slog.Debug("Failed to unmarshal tool input for PostToolUse hook", "error", err, "tool", result.ToolName) + } break } } - _ = a.hooks.Execute(genCtx, hooks.HookContext{ + if err := a.hooks.Execute(genCtx, hooks.HookContext{ EventType: config.PostToolUse, SessionID: call.SessionID, ToolName: result.ToolName, @@ -378,7 +383,9 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (*fantasy MessageID: currentAssistant.ID, Provider: a.largeModel.ModelCfg.Provider, Model: a.largeModel.ModelCfg.Model, - }) + }); err != nil { + slog.Debug("post_tool_use hook execution failed", "error", err) + } } toolResult := message.ToolResult{ @@ -529,7 +536,7 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (*fantasy inputTokens += step.Usage.InputTokens } - _ = a.hooks.Execute(ctx, hooks.HookContext{ + if err := a.hooks.Execute(ctx, hooks.HookContext{ EventType: config.Stop, SessionID: call.SessionID, MessageID: currentAssistant.ID, @@ -537,7 +544,9 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (*fantasy Model: a.largeModel.ModelCfg.Model, TokensUsed: totalTokens, TokensInput: inputTokens, - }) + }); err != nil { + slog.Debug("stop hook execution failed", "error", err) + } } if shouldSummarize { @@ -589,6 +598,18 @@ func (a *sessionAgent) Summarize(ctx context.Context, sessionID string, opts fan return nil } + // Execute PreCompact hook + if a.hooks != nil { + if err := a.hooks.Execute(ctx, hooks.HookContext{ + EventType: config.PreCompact, + SessionID: sessionID, + Provider: a.largeModel.ModelCfg.Provider, + Model: a.largeModel.ModelCfg.Model, + }); err != nil { + slog.Debug("pre_compact hook execution failed", "error", err) + } + } + aiMsgs, _ := a.preparePrompt(msgs) genCtx, cancel := context.WithCancel(ctx) diff --git a/internal/agent/agent_tool.go b/internal/agent/agent_tool.go index 3e93181325091a1a91cd8a3e661194f11d38ee3f..6628bc39dba74ac5a1030238b85f9cddd3a0cb63 100644 --- a/internal/agent/agent_tool.go +++ b/internal/agent/agent_tool.go @@ -6,12 +6,14 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "charm.land/fantasy" "github.com/charmbracelet/crush/internal/agent/prompt" "github.com/charmbracelet/crush/internal/agent/tools" "github.com/charmbracelet/crush/internal/config" + "github.com/charmbracelet/crush/internal/hooks" ) //go:embed templates/agent_tool.md @@ -104,6 +106,21 @@ func (c *coordinator) agentTool(ctx context.Context) (fantasy.AgentTool, error) if err != nil { return fantasy.ToolResponse{}, fmt.Errorf("error saving parent session: %s", err) } + + // Execute SubagentStop hook + if c.hooks != nil { + if err := c.hooks.Execute(ctx, hooks.HookContext{ + EventType: config.SubagentStop, + SessionID: sessionID, + ToolName: AgentToolName, + MessageID: agentMessageID, + Provider: model.ModelCfg.Provider, + Model: model.ModelCfg.Model, + }); err != nil { + slog.Debug("subagent_stop hook execution failed", "error", err) + } + } + return fantasy.NewTextResponse(result.Response.Content.Text()), nil }), nil } diff --git a/internal/agent/common_test.go b/internal/agent/common_test.go index 8495ca7a42a257753b6c4fafb33996a740074fc6..82e2bcd46d9055808d81dead48710c08bbf500b4 100644 --- a/internal/agent/common_test.go +++ b/internal/agent/common_test.go @@ -114,7 +114,7 @@ func testEnv(t *testing.T) env { sessions := session.NewService(q) messages := message.NewService(q) - permissions := permission.NewPermissionService(workingDir, true, []string{}) + permissions := permission.NewPermissionService(workingDir, true, []string{}, nil) history := history.NewService(q, conn) lspClients := csync.NewMap[string, *lsp.Client]() diff --git a/internal/agent/coordinator.go b/internal/agent/coordinator.go index f7cbaf5ea16bc60d4e9d33d3c7408773b505344b..d070046eefa594822d75bf72a30766befd10bb6a 100644 --- a/internal/agent/coordinator.go +++ b/internal/agent/coordinator.go @@ -75,10 +75,8 @@ func NewCoordinator( permissions permission.Service, history history.Service, lspClients *csync.Map[string, *lsp.Client], + hooksExecutor *hooks.Executor, ) (Coordinator, error) { - // Initialize hooks executor - hooksExecutor := hooks.NewExecutor(cfg.Hooks, cfg.WorkingDir()) - c := &coordinator{ cfg: cfg, sessions: sessions, diff --git a/internal/app/app.go b/internal/app/app.go index a801f70a5feccac655209ac5c36deaae7e38592b..3e8b7acb1ca33a481df3082c1e6a2923e76dac4a 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -18,6 +18,7 @@ import ( "github.com/charmbracelet/crush/internal/db" "github.com/charmbracelet/crush/internal/format" "github.com/charmbracelet/crush/internal/history" + "github.com/charmbracelet/crush/internal/hooks" "github.com/charmbracelet/crush/internal/log" "github.com/charmbracelet/crush/internal/lsp" "github.com/charmbracelet/crush/internal/message" @@ -38,6 +39,7 @@ type App struct { LSPClients *csync.Map[string, *lsp.Client] config *config.Config + hooks *hooks.Executor serviceEventsWG *sync.WaitGroup eventsCtx context.Context @@ -61,16 +63,20 @@ func New(ctx context.Context, conn *sql.DB, cfg *config.Config) (*App, error) { allowedTools = cfg.Permissions.AllowedTools } + // Initialize hooks executor + hooksExecutor := hooks.NewExecutor(cfg.Hooks, cfg.WorkingDir()) + app := &App{ Sessions: sessions, Messages: messages, History: files, - Permissions: permission.NewPermissionService(cfg.WorkingDir(), skipPermissionsRequests, allowedTools), + Permissions: permission.NewPermissionService(cfg.WorkingDir(), skipPermissionsRequests, allowedTools, hooksExecutor), LSPClients: csync.NewMap[string, *lsp.Client](), globalCtx: ctx, config: cfg, + hooks: hooksExecutor, events: make(chan tea.Msg, 100), serviceEventsWG: &sync.WaitGroup{}, @@ -267,6 +273,7 @@ func (app *App) InitCoderAgent(ctx context.Context) error { if coderAgentCfg.ID == "" { return fmt.Errorf("coder agent configuration is missing") } + var err error app.AgentCoordinator, err = agent.NewCoordinator( ctx, @@ -276,6 +283,7 @@ func (app *App) InitCoderAgent(ctx context.Context) error { app.Permissions, app.History, app.LSPClients, + app.hooks, ) if err != nil { slog.Error("Failed to create coder agent", "err", err) diff --git a/internal/config/hooks.go b/internal/config/hooks.go index 46939149b56c2107e6bd6a6b6efb064c41ba354d..1a61786c209077fe3dd0cc2b41f77ba2a7e32cf2 100644 --- a/internal/config/hooks.go +++ b/internal/config/hooks.go @@ -5,23 +5,19 @@ type HookEventType string const ( // PreToolUse runs before tool calls and can block them. - PreToolUse HookEventType = "PreToolUse" + PreToolUse HookEventType = "pre_tool_use" // PostToolUse runs after tool calls complete. - PostToolUse HookEventType = "PostToolUse" + PostToolUse HookEventType = "post_tool_use" // UserPromptSubmit runs when the user submits a prompt, before processing. - UserPromptSubmit HookEventType = "UserPromptSubmit" - // Notification runs when Crush sends notifications. - Notification HookEventType = "Notification" + UserPromptSubmit HookEventType = "user_prompt_submit" // Stop runs when Crush finishes responding. - Stop HookEventType = "Stop" + Stop HookEventType = "stop" // SubagentStop runs when subagent tasks complete. - SubagentStop HookEventType = "SubagentStop" + SubagentStop HookEventType = "subagent_stop" // PreCompact runs before running a compact operation. - PreCompact HookEventType = "PreCompact" - // SessionStart runs when a session starts or resumes. - SessionStart HookEventType = "SessionStart" - // SessionEnd runs when a session ends. - SessionEnd HookEventType = "SessionEnd" + PreCompact HookEventType = "pre_compact" + // PermissionRequested runs when a permission is requested from the user. + PermissionRequested HookEventType = "permission_requested" ) // Hook represents a single hook command configuration. @@ -29,7 +25,8 @@ type Hook struct { // Type is the hook type, currently only "command" is supported. Type string `json:"type" jsonschema:"description=Hook type,enum=command,default=command"` // Command is the shell command to execute. - Command string `json:"command" jsonschema:"required,description=Shell command to execute for this hook,example=echo 'Hook executed'"` + // WARNING: Hook commands execute with Crush's full permissions. Only use trusted commands. + Command string `json:"command" jsonschema:"required,description=Shell command to execute for this hook (executes with Crush's permissions),example=echo 'Hook executed'"` // Timeout is the maximum time in seconds to wait for the hook to complete. // Default is 30 seconds. Timeout *int `json:"timeout,omitempty" jsonschema:"description=Maximum time in seconds to wait for hook completion,default=30,minimum=1,maximum=300"` @@ -39,7 +36,8 @@ type Hook struct { type HookMatcher struct { // Matcher is the tool name or pattern to match (for tool events). // For non-tool events, this can be empty or "*" to match all. - Matcher string `json:"matcher,omitempty" jsonschema:"description=Tool name or pattern to match (e.g. 'bash' or '*' for all),example=bash,example=edit,example=*"` + // Supports pipe-separated tool names like "edit|write|multiedit". + Matcher string `json:"matcher,omitempty" jsonschema:"description=Tool name or pattern to match (e.g. 'bash' 'edit|write' for multiple or '*' for all),example=bash,example=edit|write|multiedit,example=*"` // Hooks is the list of hooks to execute when the matcher matches. Hooks []Hook `json:"hooks" jsonschema:"required,description=List of hooks to execute when matcher matches"` } diff --git a/internal/hooks/HOOKS.md b/internal/hooks/HOOKS.md index e58e3c8432b5433989394011f6c4303d04033da3..505601e45367a7b19983bb08ad05b77819c92f8f 100644 --- a/internal/hooks/HOOKS.md +++ b/internal/hooks/HOOKS.md @@ -9,19 +9,17 @@ Hooks are user-defined shell commands that execute at various points in Crush's Crush provides several lifecycle events where hooks can run: ### Tool Events -- **`PreToolUse`**: Runs before tool calls. If a hook fails (non-zero exit code), the tool execution is blocked. -- **`PostToolUse`**: Runs after tool calls complete, can be used to process results or trigger actions. +- **`pre_tool_use`**: Runs before tool calls. If a hook fails (non-zero exit code), the tool execution is blocked. +- **`post_tool_use`**: Runs after tool calls complete, can be used to process results or trigger actions. ### Session Events -- **`UserPromptSubmit`**: Runs when the user submits a prompt, before processing -- **`Stop`**: Runs when Crush finishes responding to a prompt -- **`SubagentStop`**: Runs when subagent tasks complete (e.g., fetch tool, agent tool) -- **`SessionStart`**: Runs when a session starts or resumes -- **`SessionEnd`**: Runs when a session ends +- **`user_prompt_submit`**: Runs when the user submits a prompt, before processing +- **`stop`**: Runs when Crush finishes responding to a prompt +- **`subagent_stop`**: Runs when subagent tasks complete (e.g., fetch tool, agent tool) ### Other Events -- **`Notification`**: Runs when Crush sends notifications -- **`PreCompact`**: Runs before running a compact operation +- **`pre_compact`**: Runs before running a compact operation +- **`permission_requested`**: Runs when a permission is requested from the user ## Configuration Format @@ -30,7 +28,7 @@ Hooks are configured in your Crush configuration file (e.g., `crush.json` or `~/ ```json { "hooks": { - "PreToolUse": [ + "pre_tool_use": [ { "matcher": "bash", "hooks": [ @@ -42,7 +40,7 @@ Hooks are configured in your Crush configuration file (e.g., `crush.json` or `~/ ] } ], - "PostToolUse": [ + "post_tool_use": [ { "matcher": "*", "hooks": [ @@ -53,7 +51,7 @@ Hooks are configured in your Crush configuration file (e.g., `crush.json` or `~/ ] } ], - "Stop": [ + "stop": [ { "hooks": [ { @@ -73,7 +71,7 @@ Each hook receives a JSON context object via stdin containing information about ```json { - "event_type": "PreToolUse", + "event_type": "pre_tool_use", "session_id": "abc123", "tool_name": "bash", "tool_input": { @@ -97,10 +95,10 @@ Each hook receives a JSON context object via stdin containing information about Different events include different fields: -- **PreToolUse**: `event_type`, `session_id`, `tool_name`, `tool_input`, `message_id`, `provider`, `model` -- **PostToolUse**: `event_type`, `session_id`, `tool_name`, `tool_input`, `tool_result`, `tool_error`, `message_id`, `provider`, `model` -- **UserPromptSubmit**: `event_type`, `session_id`, `user_prompt`, `provider`, `model` -- **Stop**: `event_type`, `session_id`, `message_id`, `provider`, `model`, `tokens_used`, `tokens_input` +- **pre_tool_use**: `event_type`, `session_id`, `tool_name`, `tool_input`, `message_id`, `provider`, `model` +- **post_tool_use**: `event_type`, `session_id`, `tool_name`, `tool_input`, `tool_result`, `tool_error`, `message_id`, `provider`, `model` +- **user_prompt_submit**: `event_type`, `session_id`, `user_prompt`, `provider`, `model` +- **stop**: `event_type`, `session_id`, `message_id`, `provider`, `model`, `tokens_used`, `tokens_input` All events include: `event_type`, `timestamp`, `working_dir` @@ -117,20 +115,27 @@ Hooks also receive environment variables: ### Matchers -For tool events (`PreToolUse`, `PostToolUse`), you can specify matchers to target specific tools: +For tool events (`pre_tool_use`, `post_tool_use`), you can specify matchers to target specific tools: - `"bash"` - Only matches the bash tool - `"edit"` - Only matches the edit tool +- `"edit|write|multiedit"` - Matches any of the specified tools (pipe-separated) - `"*"` or `""` - Matches all tools For non-tool events, leave the matcher empty or use `"*"`. -### Hook Properties +### Hook Command +Each hook has these properties: - `type`: Currently only `"command"` is supported -- `command`: The shell command to execute +- `command`: Shell command to execute. Receives JSON context via stdin - `timeout`: (optional) Maximum execution time in seconds (default: 30, max: 300) +**Important**: When processing JSON with `jq`, be aware that `tool_result` fields can contain large content or special characters that may cause parse errors. For reliability: +- Use `cat` instead of `jq` to output raw JSON: `cat >> hooks.log` +- Extract only specific fields: `jq -r '.tool_name, .session_id'` +- For `post_tool_use` hooks, tool results can be very large (e.g., entire file contents) + ## Examples ### Log All Bash Commands @@ -138,7 +143,7 @@ For non-tool events, leave the matcher empty or use `"*"`. ```json { "hooks": { - "PreToolUse": [ + "pre_tool_use": [ { "matcher": "bash", "hooks": [ @@ -158,9 +163,9 @@ For non-tool events, leave the matcher empty or use `"*"`. ```json { "hooks": { - "PostToolUse": [ + "post_tool_use": [ { - "matcher": "edit", + "matcher": "edit|write|multiedit", "hooks": [ { "type": "command", @@ -175,10 +180,12 @@ For non-tool events, leave the matcher empty or use `"*"`. ### Notify on Completion +Note: Examples use macOS-specific tools. For cross-platform alternatives, use `notify-send` (Linux) or custom scripts. + ```json { "hooks": { - "Stop": [ + "stop": [ { "hooks": [ { @@ -197,7 +204,7 @@ For non-tool events, leave the matcher empty or use `"*"`. ```json { "hooks": { - "Stop": [ + "stop": [ { "hooks": [ { @@ -216,7 +223,7 @@ For non-tool events, leave the matcher empty or use `"*"`. ```json { "hooks": { - "PreToolUse": [ + "pre_tool_use": [ { "matcher": "bash", "hooks": [ @@ -238,7 +245,7 @@ You can execute multiple hooks for the same event: ```json { "hooks": { - "PostToolUse": [ + "post_tool_use": [ { "matcher": "*", "hooks": [ @@ -257,6 +264,98 @@ You can execute multiple hooks for the same event: } ``` +### Debug: Log All Hook Events + +For debugging or monitoring, log complete JSON for all events: + +```json +{ + "hooks": { + "pre_tool_use": [ + { + "matcher": "*", + "hooks": [ + { + "type": "command", + "command": "(echo \"[$(date '+%Y-%m-%d %H:%M:%S')] pre_tool_use:\" && cat && echo \"\") >> hooks.log" + } + ] + } + ], + "post_tool_use": [ + { + "matcher": "*", + "hooks": [ + { + "type": "command", + "command": "(echo \"[$(date '+%Y-%m-%d %H:%M:%S')] post_tool_use:\" && cat && echo \"\") >> hooks.log" + } + ] + } + ] + } +} +``` + +Note: Using `cat` avoids potential jq parsing errors with large or complex tool results. + +### Track Subagent Completion + +```json +{ + "hooks": { + "subagent_stop": [ + { + "hooks": [ + { + "type": "command", + "command": "echo \"Subagent task completed: $(jq -r .tool_name)\" | tee -a ~/.crush/subagent-log.txt" + } + ] + } + ] + } +} +``` + +### Pre-Compact Notification + +```json +{ + "hooks": { + "pre_compact": [ + { + "hooks": [ + { + "type": "command", + "command": "osascript -e 'display notification \"Compacting conversation...\" with title \"Crush\"'" + } + ] + } + ] + } +} +``` + +### Permission Requested Notification + +```json +{ + "hooks": { + "permission_requested": [ + { + "hooks": [ + { + "type": "command", + "command": "jq -r '\"Permission requested: \\(.tool_name) \\(.permission_action) \\(.permission_path)\"' | tee -a ~/.crush/permissions-log.txt" + } + ] + } + ] + } +} +``` + ## Best Practices 1. **Keep hooks fast**: Hooks run synchronously and can slow down Crush if they take too long. @@ -275,7 +374,7 @@ Hooks log errors and warnings to Crush's log output. To see hook execution: 2. Check the logs for hook-related messages 3. Test hook shell commands manually: ```bash - echo '{"event_type":"PreToolUse","tool_name":"bash"}' | jq -r '.tool_name' + echo '{"event_type":"pre_tool_use","tool_name":"bash"}' | jq -r '.tool_name' ``` ## Limitations @@ -283,5 +382,5 @@ Hooks log errors and warnings to Crush's log output. To see hook execution: - Hooks must complete within their timeout (default 30 seconds) - Hooks run in a shell environment and require shell utilities (bash, jq, etc.) - Hooks cannot modify Crush's internal state -- Hook errors are logged but don't stop Crush execution (except for PreToolUse) +- Hook errors are logged but don't stop Crush execution (except for pre_tool_use) - Interactive hooks are not supported diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index 6c2d4fb9460c80acca0d5ae1fb6ef1174645e1b3..d9ffff92323869575f3d522723be661742b6640c 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "log/slog" + "strings" "time" "github.com/charmbracelet/crush/internal/config" @@ -15,20 +16,24 @@ const DefaultHookTimeout = 30 * time.Second // HookContext contains context information passed to hooks. type HookContext struct { - EventType config.HookEventType `json:"event_type"` - SessionID string `json:"session_id,omitempty"` - ToolName string `json:"tool_name,omitempty"` - ToolInput map[string]any `json:"tool_input,omitempty"` - ToolResult string `json:"tool_result,omitempty"` - ToolError bool `json:"tool_error,omitempty"` - UserPrompt string `json:"user_prompt,omitempty"` - Timestamp time.Time `json:"timestamp"` - WorkingDir string `json:"working_dir,omitempty"` - MessageID string `json:"message_id,omitempty"` - Provider string `json:"provider,omitempty"` - Model string `json:"model,omitempty"` - TokensUsed int64 `json:"tokens_used,omitempty"` - TokensInput int64 `json:"tokens_input,omitempty"` + EventType config.HookEventType `json:"event_type"` + SessionID string `json:"session_id,omitempty"` + ToolName string `json:"tool_name,omitempty"` + ToolInput map[string]any `json:"tool_input,omitempty"` + ToolResult string `json:"tool_result,omitempty"` + ToolError bool `json:"tool_error,omitempty"` + UserPrompt string `json:"user_prompt,omitempty"` + Timestamp time.Time `json:"timestamp"` + WorkingDir string `json:"working_dir,omitempty"` + MessageID string `json:"message_id,omitempty"` + Provider string `json:"provider,omitempty"` + Model string `json:"model,omitempty"` + TokensUsed int64 `json:"tokens_used,omitempty"` + TokensInput int64 `json:"tokens_input,omitempty"` + PermissionAction string `json:"permission_action,omitempty"` + PermissionPath string `json:"permission_path,omitempty"` + PermissionParams any `json:"permission_params,omitempty"` + PermissionToolCall string `json:"permission_tool_call,omitempty"` } // Executor executes hooks based on configuration. @@ -96,12 +101,40 @@ func (e *Executor) matcherApplies(matcher config.HookMatcher, ctx HookContext) b } if ctx.EventType == config.PreToolUse || ctx.EventType == config.PostToolUse { - return matcher.Matcher == ctx.ToolName + return matchesToolName(matcher.Matcher, ctx.ToolName) } + // For non-tool events, only empty or wildcard matchers apply return matcher.Matcher == "" || matcher.Matcher == "*" } +// matchesToolName supports pipe-separated patterns like "edit|write|multiedit". +func matchesToolName(pattern, toolName string) bool { + if pattern == "" || pattern == "*" { + return true + } + + // Check for exact match first + if pattern == toolName { + return true + } + + // Check if pattern contains pipes (multiple tool names) + if !strings.Contains(pattern, "|") { + return false + } + + // Split by pipe and check each tool name + for tool := range strings.SplitSeq(pattern, "|") { + tool = strings.TrimSpace(tool) + if tool == toolName { + return true + } + } + + return false +} + // executeHook executes a single hook command. func (e *Executor) executeHook(ctx context.Context, hook config.Hook, hookCtx HookContext) error { if hook.Type != "command" { diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go index 2855cfe9e3f63d0181e2a920cf7956e3ef5d3c17..3d854f9b86c2d1835a42d98bc68b1b55cb4c1b6b 100644 --- a/internal/hooks/hooks_test.go +++ b/internal/hooks/hooks_test.go @@ -222,6 +222,61 @@ func TestHookExecutor_MatcherApplies(t *testing.T) { }, want: false, }, + { + name: "pipe-separated matcher matches first tool", + matcher: config.HookMatcher{ + Matcher: "edit|write|multiedit", + }, + ctx: HookContext{ + EventType: config.PreToolUse, + ToolName: "edit", + }, + want: true, + }, + { + name: "pipe-separated matcher matches middle tool", + matcher: config.HookMatcher{ + Matcher: "edit|write|multiedit", + }, + ctx: HookContext{ + EventType: config.PreToolUse, + ToolName: "write", + }, + want: true, + }, + { + name: "pipe-separated matcher matches last tool", + matcher: config.HookMatcher{ + Matcher: "edit|write|multiedit", + }, + ctx: HookContext{ + EventType: config.PreToolUse, + ToolName: "multiedit", + }, + want: true, + }, + { + name: "pipe-separated matcher doesn't match different tool", + matcher: config.HookMatcher{ + Matcher: "edit|write|multiedit", + }, + ctx: HookContext{ + EventType: config.PreToolUse, + ToolName: "bash", + }, + want: false, + }, + { + name: "pipe-separated matcher with spaces", + matcher: config.HookMatcher{ + Matcher: "edit | write | multiedit", + }, + ctx: HookContext{ + EventType: config.PreToolUse, + ToolName: "write", + }, + want: true, + }, { name: "non-tool event matches empty matcher", matcher: config.HookMatcher{ @@ -325,6 +380,68 @@ func TestHookExecutor_MultipleHooks(t *testing.T) { require.Equal(t, "hook3", lines[2]) } +func TestHookExecutor_PipeSeparatedMatcher(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + logFile := filepath.Join(tempDir, "pipe-matcher-log.txt") + + hookConfig := config.HookConfig{ + config.PostToolUse: []config.HookMatcher{ + { + Matcher: "edit|write|multiedit", + Hooks: []config.Hook{ + { + Type: "command", + Command: `jq -r '.tool_name' >> ` + logFile, + }, + }, + }, + }, + } + + executor := NewExecutor(hookConfig, tempDir) + ctx := context.Background() + + // Test that edit triggers the hook + err := executor.Execute(ctx, HookContext{ + EventType: config.PostToolUse, + ToolName: "edit", + }) + require.NoError(t, err) + + // Test that write triggers the hook + err = executor.Execute(ctx, HookContext{ + EventType: config.PostToolUse, + ToolName: "write", + }) + require.NoError(t, err) + + // Test that multiedit triggers the hook + err = executor.Execute(ctx, HookContext{ + EventType: config.PostToolUse, + ToolName: "multiedit", + }) + require.NoError(t, err) + + // Test that bash does NOT trigger the hook + err = executor.Execute(ctx, HookContext{ + EventType: config.PostToolUse, + ToolName: "bash", + }) + require.NoError(t, err) + + // Verify only the matching tools were logged + content, err := os.ReadFile(logFile) + require.NoError(t, err) + + lines := strings.Split(strings.TrimSpace(string(content)), "\n") + require.Len(t, lines, 3) + require.Equal(t, "edit", lines[0]) + require.Equal(t, "write", lines[1]) + require.Equal(t, "multiedit", lines[2]) +} + func TestHookExecutor_ContextCancellation(t *testing.T) { t.Parallel() diff --git a/internal/permission/permission.go b/internal/permission/permission.go index 77b2526a592d0d194f75fb71af05477ae75df80b..e7d2d1242ff78f4961d9a69a0633bb1729f992ba 100644 --- a/internal/permission/permission.go +++ b/internal/permission/permission.go @@ -3,12 +3,15 @@ package permission import ( "context" "errors" + "log/slog" "os" "path/filepath" "slices" "sync" + "github.com/charmbracelet/crush/internal/config" "github.com/charmbracelet/crush/internal/csync" + "github.com/charmbracelet/crush/internal/hooks" "github.com/charmbracelet/crush/internal/pubsub" "github.com/google/uuid" ) @@ -66,6 +69,7 @@ type permissionService struct { autoApproveSessionsMu sync.RWMutex skip bool allowedTools []string + hooks *hooks.Executor // used to make sure we only process one request at a time requestMu sync.Mutex @@ -181,16 +185,24 @@ func (s *permissionService) Request(opts CreatePermissionRequest) bool { } s.sessionPermissionsMu.RUnlock() - s.sessionPermissionsMu.RLock() - for _, p := range s.sessionPermissions { - if p.ToolName == permission.ToolName && p.Action == permission.Action && p.SessionID == permission.SessionID && p.Path == permission.Path { - s.sessionPermissionsMu.RUnlock() - return true + s.activeRequest = &permission + + // Execute PermissionRequested hook. + // Uses context.Background() since Request() is called synchronously and hooks should + // run even if the calling operation is cancelled. Hooks have their own timeout. + if s.hooks != nil { + if err := s.hooks.Execute(context.Background(), hooks.HookContext{ + EventType: config.PermissionRequested, + SessionID: permission.SessionID, + ToolName: permission.ToolName, + PermissionAction: permission.Action, + PermissionPath: permission.Path, + PermissionParams: permission.Params, + PermissionToolCall: permission.ToolCallID, + }); err != nil { + slog.Debug("permission_requested hook execution failed", "error", err) } } - s.sessionPermissionsMu.RUnlock() - - s.activeRequest = &permission respCh := make(chan bool, 1) s.pendingRequests.Set(permission.ID, respCh) @@ -220,7 +232,7 @@ func (s *permissionService) SkipRequests() bool { return s.skip } -func NewPermissionService(workingDir string, skip bool, allowedTools []string) Service { +func NewPermissionService(workingDir string, skip bool, allowedTools []string, hooksExecutor *hooks.Executor) Service { return &permissionService{ Broker: pubsub.NewBroker[PermissionRequest](), notificationBroker: pubsub.NewBroker[PermissionNotification](), @@ -230,5 +242,6 @@ func NewPermissionService(workingDir string, skip bool, allowedTools []string) S skip: skip, allowedTools: allowedTools, pendingRequests: csync.NewMap[string, chan bool](), + hooks: hooksExecutor, } } diff --git a/internal/permission/permission_test.go b/internal/permission/permission_test.go index d1ccd286836768f1bc1119966568941f7494affd..c977f4ffe64c056cbb38c5fa6af672a148b1c64f 100644 --- a/internal/permission/permission_test.go +++ b/internal/permission/permission_test.go @@ -54,7 +54,7 @@ func TestPermissionService_AllowedCommands(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - service := NewPermissionService("/tmp", false, tt.allowedTools) + service := NewPermissionService("/tmp", false, tt.allowedTools, nil) // Create a channel to capture the permission request // Since we're testing the allowlist logic, we need to simulate the request @@ -79,7 +79,7 @@ func TestPermissionService_AllowedCommands(t *testing.T) { } func TestPermissionService_SkipMode(t *testing.T) { - service := NewPermissionService("/tmp", true, []string{}) + service := NewPermissionService("/tmp", true, []string{}, nil) result := service.Request(CreatePermissionRequest{ SessionID: "test-session", @@ -96,7 +96,7 @@ func TestPermissionService_SkipMode(t *testing.T) { func TestPermissionService_SequentialProperties(t *testing.T) { t.Run("Sequential permission requests with persistent grants", func(t *testing.T) { - service := NewPermissionService("/tmp", false, []string{}) + service := NewPermissionService("/tmp", false, []string{}, nil) req1 := CreatePermissionRequest{ SessionID: "session1", @@ -140,7 +140,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) { assert.True(t, result2, "Second request should be auto-approved") }) t.Run("Sequential requests with temporary grants", func(t *testing.T) { - service := NewPermissionService("/tmp", false, []string{}) + service := NewPermissionService("/tmp", false, []string{}, nil) req := CreatePermissionRequest{ SessionID: "session2", @@ -180,7 +180,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) { assert.False(t, result2, "Second request should be denied") }) t.Run("Concurrent requests with different outcomes", func(t *testing.T) { - service := NewPermissionService("/tmp", false, []string{}) + service := NewPermissionService("/tmp", false, []string{}, nil) events := service.Subscribe(t.Context()) diff --git a/schema.json b/schema.json index b5292ea955945e4638b739816180da3e7783e0d5..ddfd500eb092c1eac59c73b9e8f20e0c555cb21a 100644 --- a/schema.json +++ b/schema.json @@ -136,10 +136,10 @@ "properties": { "matcher": { "type": "string", - "description": "Tool name or pattern to match (e.g. 'bash' or '*' for all)", + "description": "Tool name or pattern to match (e.g. 'bash' 'edit|write' for multiple or '*' for all)", "examples": [ "bash", - "edit", + "edit|write|multiedit", "*" ] },