Merge PR #3049 from Hmbown: hooks v2 — JSON decision contract, glob matchers, project-local hooks
feat(hooks): JSON decision contract, glob matchers, project-local hooks
This commit is contained in:
Hunter Bown
@@ -1332,11 +1332,15 @@ impl Engine {
|
||||
let active_tools_at_batch_start = active_tool_names.clone();
|
||||
let mut deferred_tools_hydrated_this_batch: std::collections::HashSet<String> =
|
||||
std::collections::HashSet::new();
|
||||
// #3026: `additionalContext` strings from tool_call_before hooks,
|
||||
// keyed by tool id; appended to the tool result sent to the model.
|
||||
let mut hook_contexts: std::collections::HashMap<String, String> =
|
||||
std::collections::HashMap::new();
|
||||
let mut plans: Vec<ToolExecutionPlan> = Vec::with_capacity(tool_uses.len());
|
||||
for (index, tool) in tool_uses.iter_mut().enumerate() {
|
||||
let tool_id = tool.id.clone();
|
||||
let mut tool_name = tool.name.clone();
|
||||
let tool_input = tool.input.clone();
|
||||
let mut tool_input = tool.input.clone();
|
||||
let tool_caller = tool.caller.clone();
|
||||
crate::logging::info(format!(
|
||||
"Planning tool '{tool_name}' with input: {tool_input:?}"
|
||||
@@ -1362,6 +1366,10 @@ impl Engine {
|
||||
let mut read_only = false;
|
||||
let mut blocked_error: Option<ToolError> = None;
|
||||
let mut guard_result: Option<ToolResult> = None;
|
||||
// #3026: set by a hook `ask` decision; applied AFTER the
|
||||
// registry-based approval computation below so it cannot be
|
||||
// clobbered by it.
|
||||
let mut hook_requires_approval = false;
|
||||
|
||||
if mode == AppMode::Plan
|
||||
&& matches!(
|
||||
@@ -1456,29 +1464,25 @@ impl Engine {
|
||||
tracing::error!("Hook executor task panicked: {join_err}");
|
||||
Vec::new()
|
||||
});
|
||||
if let Some(denial) = hook_results
|
||||
.iter()
|
||||
.find(|result| result.exit_code == Some(2))
|
||||
{
|
||||
let reason = denial
|
||||
.stdout
|
||||
.trim()
|
||||
.lines()
|
||||
.next()
|
||||
.filter(|line| !line.is_empty())
|
||||
.or_else(|| {
|
||||
denial
|
||||
.stderr
|
||||
.trim()
|
||||
.lines()
|
||||
.next()
|
||||
.filter(|line| !line.is_empty())
|
||||
})
|
||||
.or(denial.error.as_deref())
|
||||
.unwrap_or("ToolCallBefore hook denied tool execution");
|
||||
// #3026: fold all foreground hook results into one
|
||||
// decision: deny (exit code 2 or JSON) > ask > allow;
|
||||
// last `updatedInput` writer wins; `additionalContext`
|
||||
// strings are concatenated.
|
||||
let fold = fold_tool_call_before_results(&hook_results);
|
||||
if let Some(reason) = fold.deny_reason {
|
||||
blocked_error = Some(ToolError::permission_denied(format!(
|
||||
"ToolCallBefore hook denied tool '{tool_name}': {reason}"
|
||||
)));
|
||||
} else {
|
||||
if fold.requires_approval {
|
||||
hook_requires_approval = true;
|
||||
}
|
||||
if let Some(updated) = fold.updated_input {
|
||||
tool_input = updated;
|
||||
}
|
||||
if let Some(context) = fold.additional_context {
|
||||
hook_contexts.insert(tool_id.clone(), context);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1514,6 +1518,14 @@ impl Engine {
|
||||
read_only = true;
|
||||
}
|
||||
|
||||
// #3026: a hook `ask` decision forces the approval prompt even
|
||||
// for tools the registry would auto-run. Must stay after the
|
||||
// registry-based computation above, which assigns rather than
|
||||
// ORs `approval_required`.
|
||||
if hook_requires_approval {
|
||||
approval_required = true;
|
||||
}
|
||||
|
||||
let should_emit_hydration_status =
|
||||
!deferred_tools_hydrated_this_batch.contains(&tool_name);
|
||||
if blocked_error.is_none()
|
||||
@@ -2139,6 +2151,15 @@ impl Engine {
|
||||
.await;
|
||||
}
|
||||
|
||||
// #3026: pipe `additionalContext` from tool_call_before
|
||||
// hooks back to the model alongside the tool result.
|
||||
let output_for_context = match hook_contexts.get(&outcome.id) {
|
||||
Some(context) => {
|
||||
format!("{output_for_context}\n\n[hook context] {context}")
|
||||
}
|
||||
None => output_for_context,
|
||||
};
|
||||
|
||||
self.add_session_message(Message {
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentBlock::ToolResult {
|
||||
@@ -2388,6 +2409,81 @@ pub(super) fn command_allows_tool(allowed_tools: Option<&[String]>, tool_name: &
|
||||
allowed_tools.contains(&tool_name.to_ascii_lowercase())
|
||||
}
|
||||
|
||||
/// Folded outcome of all `tool_call_before` hook results for one tool call
|
||||
/// (#3026). Precedence: deny (exit code 2 or JSON) > ask > allow;
|
||||
/// `updatedInput` is last-writer-wins; `additionalContext` is concatenated.
|
||||
#[derive(Debug, Default, PartialEq)]
|
||||
struct ToolCallHookFold {
|
||||
/// Denial reason from an exit-code-2 hook or a JSON `deny` decision.
|
||||
deny_reason: Option<String>,
|
||||
/// At least one hook returned a JSON `ask` decision.
|
||||
requires_approval: bool,
|
||||
/// Replacement tool input from the last hook that supplied one.
|
||||
updated_input: Option<serde_json::Value>,
|
||||
/// Concatenated `additionalContext` strings from all hooks.
|
||||
additional_context: Option<String>,
|
||||
}
|
||||
|
||||
fn fold_tool_call_before_results(results: &[crate::hooks::HookResult]) -> ToolCallHookFold {
|
||||
let mut fold = ToolCallHookFold::default();
|
||||
|
||||
// Legacy hard deny: exit code 2 wins regardless of stdout (backwards
|
||||
// compatible with pre-#3026 hooks).
|
||||
if let Some(denial) = results.iter().find(|result| result.exit_code == Some(2)) {
|
||||
let reason = denial
|
||||
.stdout
|
||||
.trim()
|
||||
.lines()
|
||||
.next()
|
||||
.filter(|line| !line.is_empty())
|
||||
.or_else(|| {
|
||||
denial
|
||||
.stderr
|
||||
.trim()
|
||||
.lines()
|
||||
.next()
|
||||
.filter(|line| !line.is_empty())
|
||||
})
|
||||
.or(denial.error.as_deref())
|
||||
.unwrap_or("ToolCallBefore hook denied tool execution");
|
||||
fold.deny_reason = Some(reason.to_string());
|
||||
return fold;
|
||||
}
|
||||
|
||||
for result in results {
|
||||
// Background hooks return immediately with no process result and
|
||||
// cannot steer (the caller warns about that configuration).
|
||||
if result.exit_code.is_none() {
|
||||
continue;
|
||||
}
|
||||
let parsed = crate::hooks::parse_tool_call_before_stdout(&result.stdout);
|
||||
match parsed.decision {
|
||||
Some(crate::hooks::ToolCallDecision::Deny) => {
|
||||
fold.deny_reason =
|
||||
Some(parsed.reason.unwrap_or_else(|| {
|
||||
"ToolCallBefore hook denied tool execution".to_string()
|
||||
}));
|
||||
return fold;
|
||||
}
|
||||
Some(crate::hooks::ToolCallDecision::Ask) => fold.requires_approval = true,
|
||||
Some(crate::hooks::ToolCallDecision::Allow) | None => {}
|
||||
}
|
||||
if let Some(updated) = parsed.updated_input {
|
||||
fold.updated_input = Some(updated);
|
||||
}
|
||||
if let Some(context) = parsed.additional_context {
|
||||
match &mut fold.additional_context {
|
||||
Some(existing) => {
|
||||
existing.push('\n');
|
||||
existing.push_str(&context);
|
||||
}
|
||||
None => fold.additional_context = Some(context),
|
||||
}
|
||||
}
|
||||
}
|
||||
fold
|
||||
}
|
||||
|
||||
/// Check whether `tool_name` is explicitly denied (#3027).
|
||||
/// Deny always wins over allow.
|
||||
pub(super) fn command_denies_tool(disallowed_tools: Option<&[String]>, tool_name: &str) -> bool {
|
||||
@@ -2880,4 +2976,149 @@ mod tests {
|
||||
assert_eq!(results[0].exit_code, Some(2));
|
||||
assert!(results[0].stdout.contains("security"));
|
||||
}
|
||||
|
||||
// ── #3026: JSON decision contract fold ─────────────────────────────────
|
||||
|
||||
fn hook_result(stdout: &str, exit_code: Option<i32>) -> crate::hooks::HookResult {
|
||||
crate::hooks::HookResult {
|
||||
name: None,
|
||||
success: exit_code == Some(0),
|
||||
exit_code,
|
||||
stdout: stdout.to_string(),
|
||||
stderr: String::new(),
|
||||
duration: Duration::from_millis(1),
|
||||
error: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_json_deny_blocks_with_reason() {
|
||||
let fold = fold_tool_call_before_results(&[hook_result(
|
||||
r#"{"decision":"deny","reason":"nope"}"#,
|
||||
Some(0),
|
||||
)]);
|
||||
assert_eq!(fold.deny_reason.as_deref(), Some("nope"));
|
||||
assert!(!fold.requires_approval);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_exit_code_2_denies_regardless_of_stdout() {
|
||||
let fold =
|
||||
fold_tool_call_before_results(&[hook_result(r#"{"decision":"allow"}"#, Some(2))]);
|
||||
assert!(
|
||||
fold.deny_reason.is_some(),
|
||||
"exit code 2 must hard-deny even when stdout says allow"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_deny_wins_over_ask_and_allow() {
|
||||
let fold = fold_tool_call_before_results(&[
|
||||
hook_result(r#"{"decision":"allow"}"#, Some(0)),
|
||||
hook_result(r#"{"decision":"ask"}"#, Some(0)),
|
||||
hook_result(r#"{"decision":"deny","reason":"policy"}"#, Some(0)),
|
||||
]);
|
||||
assert_eq!(fold.deny_reason.as_deref(), Some("policy"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_ask_requires_approval() {
|
||||
let fold = fold_tool_call_before_results(&[
|
||||
hook_result(r#"{"decision":"allow"}"#, Some(0)),
|
||||
hook_result(r#"{"decision":"ask"}"#, Some(0)),
|
||||
]);
|
||||
assert!(fold.deny_reason.is_none());
|
||||
assert!(fold.requires_approval);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_updated_input_last_writer_wins() {
|
||||
let fold = fold_tool_call_before_results(&[
|
||||
hook_result(r#"{"updatedInput":{"command":"first"}}"#, Some(0)),
|
||||
hook_result(r#"{"updatedInput":{"command":"second"}}"#, Some(0)),
|
||||
]);
|
||||
assert_eq!(
|
||||
fold.updated_input,
|
||||
Some(serde_json::json!({"command":"second"}))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_background_results_cannot_steer() {
|
||||
// Background hooks return exit_code: None immediately — their stdout
|
||||
// (if any were captured) must not deny, ask, or rewrite input.
|
||||
let fold = fold_tool_call_before_results(&[hook_result(
|
||||
r#"{"decision":"deny","reason":"too late"}"#,
|
||||
None,
|
||||
)]);
|
||||
assert_eq!(fold, ToolCallHookFold::default());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_concatenates_additional_context() {
|
||||
let fold = fold_tool_call_before_results(&[
|
||||
hook_result(r#"{"additionalContext":"one"}"#, Some(0)),
|
||||
hook_result(r#"{"additionalContext":"two"}"#, Some(0)),
|
||||
]);
|
||||
assert_eq!(fold.additional_context.as_deref(), Some("one\ntwo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_fold_legacy_stdout_is_passthrough() {
|
||||
let fold = fold_tool_call_before_results(&[
|
||||
hook_result("", Some(0)),
|
||||
hook_result("not json at all", Some(0)),
|
||||
hook_result(r#"{"status":"fine"}"#, Some(1)),
|
||||
]);
|
||||
assert_eq!(fold, ToolCallHookFold::default());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_gate_denies_with_json_decision_from_executor() {
|
||||
use crate::hooks::{Hook, HookContext, HookEvent, HookExecutor, HooksConfig};
|
||||
|
||||
let deny_cmd = if cfg!(windows) {
|
||||
r#"echo {"decision":"deny","reason":"blocked by project policy"}"#
|
||||
} else {
|
||||
r#"echo '{"decision":"deny","reason":"blocked by project policy"}'"#
|
||||
};
|
||||
let config = HooksConfig {
|
||||
enabled: true,
|
||||
hooks: vec![Hook::new(HookEvent::ToolCallBefore, deny_cmd)],
|
||||
..HooksConfig::default()
|
||||
};
|
||||
let executor = HookExecutor::new(config, std::path::PathBuf::from("."));
|
||||
let ctx = HookContext::new().with_tool_name("exec_shell");
|
||||
let results = executor.execute(HookEvent::ToolCallBefore, &ctx);
|
||||
|
||||
let fold = fold_tool_call_before_results(&results);
|
||||
assert_eq!(
|
||||
fold.deny_reason.as_deref(),
|
||||
Some("blocked by project policy"),
|
||||
"JSON deny with exit code 0 must block: {results:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_gate_ask_forces_approval_from_executor() {
|
||||
use crate::hooks::{Hook, HookContext, HookEvent, HookExecutor, HooksConfig};
|
||||
|
||||
let ask_cmd = if cfg!(windows) {
|
||||
r#"echo {"decision":"ask"}"#
|
||||
} else {
|
||||
r#"echo '{"decision":"ask"}'"#
|
||||
};
|
||||
let config = HooksConfig {
|
||||
enabled: true,
|
||||
hooks: vec![Hook::new(HookEvent::ToolCallBefore, ask_cmd)],
|
||||
..HooksConfig::default()
|
||||
};
|
||||
let executor = HookExecutor::new(config, std::path::PathBuf::from("."));
|
||||
let ctx = HookContext::new().with_tool_name("write_file");
|
||||
let results = executor.execute(HookEvent::ToolCallBefore, &ctx);
|
||||
|
||||
let fold = fold_tool_call_before_results(&results);
|
||||
assert!(fold.deny_reason.is_none());
|
||||
assert!(fold.requires_approval);
|
||||
}
|
||||
}
|
||||
|
||||
+342
-2
@@ -217,6 +217,30 @@ fn default_enabled() -> bool {
|
||||
}
|
||||
|
||||
impl HooksConfig {
|
||||
/// Load global hooks merged with project-local `.codewhale/hooks.toml` (#3026).
|
||||
///
|
||||
/// Project hooks are appended after global hooks. A malformed project file
|
||||
/// logs a warning and falls back to global-only.
|
||||
pub fn load_with_project(global: HooksConfig, workspace: &std::path::Path) -> HooksConfig {
|
||||
let project_path = workspace.join(".codewhale").join("hooks.toml");
|
||||
let Ok(contents) = std::fs::read_to_string(&project_path) else {
|
||||
return global;
|
||||
};
|
||||
let project: HooksConfig = match toml::from_str(&contents) {
|
||||
Ok(cfg) => cfg,
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
"Failed to parse project hooks at {}: {e}; falling back to global hooks only",
|
||||
project_path.display()
|
||||
);
|
||||
return global;
|
||||
}
|
||||
};
|
||||
let mut merged = global;
|
||||
merged.hooks.extend(project.hooks);
|
||||
merged
|
||||
}
|
||||
|
||||
/// Get hooks for a specific event
|
||||
pub fn hooks_for_event(&self, event: HookEvent) -> Vec<&Hook> {
|
||||
if !self.enabled {
|
||||
@@ -484,6 +508,90 @@ enum MessageSubmitStdout {
|
||||
Invalid(String),
|
||||
}
|
||||
|
||||
/// Parsed stdout from a `tool_call_before` hook (#3026).
|
||||
///
|
||||
/// Hooks may emit a JSON decision on stdout:
|
||||
/// `{"decision": "allow"|"deny"|"ask", "reason": "...",
|
||||
/// "updatedInput": {...}, "additionalContext": "..."}`
|
||||
/// Non-JSON or empty stdout → legacy passthrough (allow).
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ToolCallBeforeStdout {
|
||||
pub decision: Option<ToolCallDecision>,
|
||||
pub reason: Option<String>,
|
||||
pub updated_input: Option<serde_json::Value>,
|
||||
pub additional_context: Option<String>,
|
||||
}
|
||||
|
||||
/// Decision a hook can return for a tool call.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ToolCallDecision {
|
||||
Allow,
|
||||
Deny,
|
||||
Ask,
|
||||
}
|
||||
|
||||
pub(crate) fn parse_tool_call_before_stdout(stdout: &str) -> ToolCallBeforeStdout {
|
||||
let passthrough = ToolCallBeforeStdout {
|
||||
decision: None,
|
||||
reason: None,
|
||||
updated_input: None,
|
||||
additional_context: None,
|
||||
};
|
||||
let trimmed = stdout.trim();
|
||||
if trimmed.is_empty() {
|
||||
return passthrough;
|
||||
}
|
||||
let value: serde_json::Value = match serde_json::from_str(trimmed) {
|
||||
Ok(v) => v,
|
||||
// Non-JSON stdout → legacy passthrough (allow).
|
||||
Err(_) => return passthrough,
|
||||
};
|
||||
let Some(obj) = value.as_object() else {
|
||||
tracing::warn!(
|
||||
"tool_call_before hook stdout is JSON but not an object; \
|
||||
ignoring it (legacy passthrough)"
|
||||
);
|
||||
return passthrough;
|
||||
};
|
||||
let decision = obj
|
||||
.get("decision")
|
||||
.and_then(|v| v.as_str())
|
||||
.and_then(|s| match s {
|
||||
"allow" => Some(ToolCallDecision::Allow),
|
||||
"deny" => Some(ToolCallDecision::Deny),
|
||||
"ask" => Some(ToolCallDecision::Ask),
|
||||
other => {
|
||||
tracing::warn!(
|
||||
"tool_call_before hook returned unrecognized decision \
|
||||
'{other}' (expected allow|deny|ask); treating as allow"
|
||||
);
|
||||
None
|
||||
}
|
||||
});
|
||||
let reason = obj
|
||||
.get("reason")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(|s| s.to_string());
|
||||
let updated_input = obj.get("updatedInput").cloned().filter(|v| {
|
||||
if v.is_object() {
|
||||
true
|
||||
} else {
|
||||
tracing::warn!("tool_call_before hook updatedInput must be a JSON object; ignoring");
|
||||
false
|
||||
}
|
||||
});
|
||||
let additional_context = obj
|
||||
.get("additionalContext")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(|s| s.to_string());
|
||||
ToolCallBeforeStdout {
|
||||
decision,
|
||||
reason,
|
||||
updated_input,
|
||||
additional_context,
|
||||
}
|
||||
}
|
||||
|
||||
/// Post-turn accumulated totals included in the `turn_end` observer payload.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct TurnEndTotals {
|
||||
@@ -518,8 +626,11 @@ impl HookExecutor {
|
||||
fn build_shell_command(command: &str) -> Command {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
use std::os::windows::process::CommandExt as _;
|
||||
let mut cmd = Command::new("cmd");
|
||||
cmd.arg("/C").arg(command);
|
||||
// raw_arg: cmd.exe does not parse the CRT-style \" escapes that
|
||||
// Command::arg would insert, so pass the command line verbatim.
|
||||
cmd.arg("/C").raw_arg(command);
|
||||
cmd
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
@@ -862,13 +973,30 @@ impl HookExecutor {
|
||||
results
|
||||
}
|
||||
|
||||
/// Check whether a tool name matches a condition pattern with `*` glob support.
|
||||
fn tool_name_matches_condition(tool_name: &str, pattern: &str) -> bool {
|
||||
if !pattern.contains('*') {
|
||||
return tool_name == pattern;
|
||||
}
|
||||
// Escape regex metacharacters except `*`, which becomes `.*`.
|
||||
let escaped = regex::escape(pattern);
|
||||
let regex_pattern = escaped.replace(r"\*", ".*");
|
||||
let anchored = format!("^{regex_pattern}$");
|
||||
regex::Regex::new(&anchored).is_ok_and(|re| re.is_match(tool_name))
|
||||
}
|
||||
|
||||
/// Check if a hook's condition matches the context
|
||||
#[allow(clippy::only_used_in_recursion)]
|
||||
fn matches_condition(&self, hook: &Hook, context: &HookContext) -> bool {
|
||||
match &hook.condition {
|
||||
None | Some(HookCondition::Always) => true,
|
||||
Some(HookCondition::ToolName { name }) => {
|
||||
context.tool_name.as_ref().is_some_and(|n| n == name)
|
||||
// #3026: Support `*` globs in tool_name conditions so
|
||||
// `mcp__*` matches all MCP tools. Exact names keep working.
|
||||
context
|
||||
.tool_name
|
||||
.as_ref()
|
||||
.is_some_and(|n| Self::tool_name_matches_condition(n, name))
|
||||
}
|
||||
Some(HookCondition::ToolCategory { category }) => {
|
||||
// Map tool names to categories
|
||||
@@ -2147,4 +2275,216 @@ exit 7
|
||||
assert!(!executor.has_hooks_for_event(HookEvent::OnError));
|
||||
assert!(!executor.has_hooks_for_event(HookEvent::ModeChange));
|
||||
}
|
||||
|
||||
// ── #3026: tool_call_before stdout decision contract ──────────────────
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_parses_deny_with_reason() {
|
||||
let parsed =
|
||||
parse_tool_call_before_stdout(r#"{"decision":"deny","reason":"blocked by policy"}"#);
|
||||
assert_eq!(parsed.decision, Some(ToolCallDecision::Deny));
|
||||
assert_eq!(parsed.reason.as_deref(), Some("blocked by policy"));
|
||||
assert!(parsed.updated_input.is_none());
|
||||
assert!(parsed.additional_context.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_parses_ask_and_allow() {
|
||||
let ask = parse_tool_call_before_stdout(r#"{"decision":"ask"}"#);
|
||||
assert_eq!(ask.decision, Some(ToolCallDecision::Ask));
|
||||
|
||||
let allow = parse_tool_call_before_stdout(r#"{"decision":"allow"}"#);
|
||||
assert_eq!(allow.decision, Some(ToolCallDecision::Allow));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_parses_updated_input_object() {
|
||||
let parsed =
|
||||
parse_tool_call_before_stdout(r#"{"updatedInput":{"command":"ls -la","timeout":5}}"#);
|
||||
assert!(parsed.decision.is_none());
|
||||
assert_eq!(
|
||||
parsed.updated_input,
|
||||
Some(serde_json::json!({"command":"ls -la","timeout":5}))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_rejects_non_object_updated_input() {
|
||||
let parsed = parse_tool_call_before_stdout(r#"{"updatedInput":"rm -rf /"}"#);
|
||||
assert!(
|
||||
parsed.updated_input.is_none(),
|
||||
"updatedInput must be a JSON object"
|
||||
);
|
||||
let parsed = parse_tool_call_before_stdout(r#"{"updatedInput":[1,2]}"#);
|
||||
assert!(parsed.updated_input.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_parses_additional_context() {
|
||||
let parsed =
|
||||
parse_tool_call_before_stdout(r#"{"additionalContext":"remember the style guide"}"#);
|
||||
assert_eq!(
|
||||
parsed.additional_context.as_deref(),
|
||||
Some("remember the style guide")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_empty_and_non_json_are_passthrough() {
|
||||
for stdout in ["", " \n ", "ok, proceeding", "exit code zero"] {
|
||||
let parsed = parse_tool_call_before_stdout(stdout);
|
||||
assert!(parsed.decision.is_none(), "stdout {stdout:?}");
|
||||
assert!(parsed.reason.is_none());
|
||||
assert!(parsed.updated_input.is_none());
|
||||
assert!(parsed.additional_context.is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_json_without_decision_is_passthrough() {
|
||||
let parsed = parse_tool_call_before_stdout(r#"{"status":"fine"}"#);
|
||||
assert!(parsed.decision.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_non_object_json_is_passthrough() {
|
||||
for stdout in [r#""deny""#, "[1,2,3]", "42", "true"] {
|
||||
let parsed = parse_tool_call_before_stdout(stdout);
|
||||
assert!(parsed.decision.is_none(), "stdout {stdout:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_before_stdout_unknown_decision_treated_as_allow() {
|
||||
let parsed = parse_tool_call_before_stdout(r#"{"decision":"block"}"#);
|
||||
assert!(parsed.decision.is_none());
|
||||
}
|
||||
|
||||
// ── #3026: glob matchers for tool_name conditions ──────────────────────
|
||||
|
||||
#[test]
|
||||
fn tool_name_glob_matches_mcp_prefix() {
|
||||
assert!(HookExecutor::tool_name_matches_condition(
|
||||
"mcp__github__create_issue",
|
||||
"mcp__*"
|
||||
));
|
||||
assert!(!HookExecutor::tool_name_matches_condition(
|
||||
"read_file",
|
||||
"mcp__*"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_name_exact_match_still_works() {
|
||||
assert!(HookExecutor::tool_name_matches_condition(
|
||||
"read_file",
|
||||
"read_file"
|
||||
));
|
||||
assert!(!HookExecutor::tool_name_matches_condition(
|
||||
"read_files",
|
||||
"read_file"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_name_glob_escapes_regex_metacharacters() {
|
||||
// Without escaping, `.` would match any character.
|
||||
assert!(!HookExecutor::tool_name_matches_condition(
|
||||
"mcpXgithub",
|
||||
"mcp.git*"
|
||||
));
|
||||
assert!(HookExecutor::tool_name_matches_condition(
|
||||
"mcp.github",
|
||||
"mcp.git*"
|
||||
));
|
||||
// `+` and parens must be literal too.
|
||||
assert!(HookExecutor::tool_name_matches_condition(
|
||||
"weird+tool(name)",
|
||||
"weird+tool(*)"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_name_glob_supports_infix_and_suffix_positions() {
|
||||
assert!(HookExecutor::tool_name_matches_condition(
|
||||
"mcp__github__create_issue",
|
||||
"mcp__*__create_issue"
|
||||
));
|
||||
assert!(HookExecutor::tool_name_matches_condition(
|
||||
"task_shell_start",
|
||||
"*_shell_start"
|
||||
));
|
||||
assert!(!HookExecutor::tool_name_matches_condition(
|
||||
"task_shell_wait",
|
||||
"*_shell_start"
|
||||
));
|
||||
}
|
||||
|
||||
// ── #3026: project-local hooks ─────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn load_with_project_missing_file_keeps_global() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let global = HooksConfig {
|
||||
enabled: true,
|
||||
hooks: vec![Hook::new(HookEvent::ToolCallBefore, "echo global")],
|
||||
..HooksConfig::default()
|
||||
};
|
||||
|
||||
let merged = HooksConfig::load_with_project(global.clone(), dir.path());
|
||||
assert_eq!(merged.hooks.len(), 1);
|
||||
assert_eq!(merged.hooks[0].command, "echo global");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_with_project_appends_project_hooks_after_global() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let project_dir = dir.path().join(".codewhale");
|
||||
std::fs::create_dir_all(&project_dir).expect("mkdir .codewhale");
|
||||
std::fs::write(
|
||||
project_dir.join("hooks.toml"),
|
||||
r#"
|
||||
[[hooks]]
|
||||
event = "tool_call_before"
|
||||
command = "echo project"
|
||||
"#,
|
||||
)
|
||||
.expect("write hooks.toml");
|
||||
|
||||
let global = HooksConfig {
|
||||
enabled: true,
|
||||
hooks: vec![Hook::new(HookEvent::ToolCallBefore, "echo global")],
|
||||
..HooksConfig::default()
|
||||
};
|
||||
|
||||
let merged = HooksConfig::load_with_project(global, dir.path());
|
||||
assert_eq!(merged.hooks.len(), 2);
|
||||
assert_eq!(
|
||||
merged.hooks[0].command, "echo global",
|
||||
"global hooks run first"
|
||||
);
|
||||
assert_eq!(
|
||||
merged.hooks[1].command, "echo project",
|
||||
"project hooks are appended after global"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_with_project_malformed_file_falls_back_to_global() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let project_dir = dir.path().join(".codewhale");
|
||||
std::fs::create_dir_all(&project_dir).expect("mkdir .codewhale");
|
||||
std::fs::write(project_dir.join("hooks.toml"), "this is [ not toml")
|
||||
.expect("write hooks.toml");
|
||||
|
||||
let global = HooksConfig {
|
||||
enabled: true,
|
||||
hooks: vec![Hook::new(HookEvent::ToolCallBefore, "echo global")],
|
||||
..HooksConfig::default()
|
||||
};
|
||||
|
||||
let merged = HooksConfig::load_with_project(global, dir.path());
|
||||
assert_eq!(merged.hooks.len(), 1, "malformed project file is ignored");
|
||||
assert_eq!(merged.hooks[0].command, "echo global");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2047,8 +2047,10 @@ impl App {
|
||||
let allow_shell = allow_shell || initial_mode == AppMode::Yolo;
|
||||
let shell_manager = new_shared_shell_manager(workspace.clone());
|
||||
|
||||
// Initialize hooks executor from config
|
||||
let hooks_config = config.hooks_config();
|
||||
// Initialize hooks executor from config, merged with project-local
|
||||
// `.codewhale/hooks.toml` (#3026).
|
||||
let hooks_config =
|
||||
crate::hooks::HooksConfig::load_with_project(config.hooks_config(), &workspace);
|
||||
let hooks = HookExecutor::new(hooks_config, workspace.clone());
|
||||
|
||||
// Initialize plan state
|
||||
|
||||
@@ -6521,7 +6521,10 @@ fn spawn_external_url_command(mut command: Command) -> Result<()> {
|
||||
|
||||
fn apply_workspace_runtime_state(app: &mut App, config: &Config, workspace: PathBuf) {
|
||||
app.workspace = workspace.clone();
|
||||
app.hooks = HookExecutor::new(config.hooks_config(), workspace.clone());
|
||||
app.hooks = HookExecutor::new(
|
||||
crate::hooks::HooksConfig::load_with_project(config.hooks_config(), &workspace),
|
||||
workspace.clone(),
|
||||
);
|
||||
app.skills_dir = crate::tui::app::resolve_skills_dir(&workspace, &config.skills_dir(), config);
|
||||
app.refresh_skill_cache();
|
||||
app.workspace_context = None;
|
||||
|
||||
+88
-1
@@ -612,7 +612,94 @@ receives the text produced by the previous hook. Hooks marked
|
||||
`background = true` are observer-only and cannot transform or block
|
||||
the message. Existing environment variables remain available.
|
||||
`shell_env` hooks keep their existing `KEY=VALUE` stdout contract;
|
||||
the JSON stdout contract applies only to `message_submit`.
|
||||
JSON stdout contracts exist for `message_submit` (above) and
|
||||
`tool_call_before` (below).
|
||||
|
||||
### `tool_call_before` decision hooks
|
||||
|
||||
`tool_call_before` hooks run before each tool call executes. In
|
||||
addition to the legacy hard deny (exit code `2`, which always wins
|
||||
regardless of stdout), a foreground hook may print a JSON decision on
|
||||
stdout with exit code `0`:
|
||||
|
||||
```json
|
||||
{
|
||||
"decision": "allow" | "deny" | "ask",
|
||||
"reason": "human-readable explanation (used for deny)",
|
||||
"updatedInput": { "command": "ls -la" },
|
||||
"additionalContext": "text appended to the tool result for the model"
|
||||
}
|
||||
```
|
||||
|
||||
All fields are optional. Empty stdout, non-JSON stdout, and JSON
|
||||
without a `decision` field behave exactly as before (allow). An
|
||||
unrecognized `decision` string logs a warning and is treated as allow.
|
||||
|
||||
- `deny` blocks the tool; the model receives a permission-denied tool
|
||||
result containing `reason`.
|
||||
- `ask` forces the interactive approval prompt even for tools that
|
||||
would otherwise auto-run.
|
||||
- `updatedInput` must be a JSON object; it replaces the tool input
|
||||
before execution. When several hooks supply it, the last hook wins.
|
||||
- `additionalContext` is appended to the tool result sent back to the
|
||||
model as `[hook context] ...`. Multiple hooks' contexts are
|
||||
concatenated.
|
||||
|
||||
When multiple hooks match, precedence is deny > ask > allow. Hooks
|
||||
marked `background = true` cannot steer tool calls — they exit
|
||||
immediately without a captured result.
|
||||
|
||||
Example deny hook:
|
||||
|
||||
```toml
|
||||
[[hooks.hooks]]
|
||||
event = "tool_call_before"
|
||||
command = '''echo '{"decision":"deny","reason":"blocked by project policy"}' '''
|
||||
condition = { type = "tool_name", name = "exec_shell" }
|
||||
```
|
||||
|
||||
Example ask hook (force approval for every MCP tool):
|
||||
|
||||
```toml
|
||||
[[hooks.hooks]]
|
||||
event = "tool_call_before"
|
||||
command = '''echo '{"decision":"ask"}' '''
|
||||
condition = { type = "tool_name", name = "mcp__*" }
|
||||
```
|
||||
|
||||
Example input rewrite:
|
||||
|
||||
```toml
|
||||
[[hooks.hooks]]
|
||||
event = "tool_call_before"
|
||||
command = "~/.codewhale/hooks/clamp-shell-timeout.sh"
|
||||
condition = { type = "tool_name", name = "exec_shell" }
|
||||
```
|
||||
|
||||
where the script reads the hook context, then prints
|
||||
`{"updatedInput": {...}}` with the adjusted arguments.
|
||||
|
||||
`tool_name` conditions support `*` globs: `mcp__*` matches every MCP
|
||||
tool (e.g. `mcp__github__create_issue`) but not built-ins like
|
||||
`read_file`; exact names keep matching exactly. Other regex
|
||||
metacharacters in the pattern are matched literally.
|
||||
|
||||
### Project-local hooks
|
||||
|
||||
Repositories can ship policy in `<workspace>/.codewhale/hooks.toml`,
|
||||
using the same shape as the `[hooks]` table (top-level fields plus
|
||||
`[[hooks]]` entries). Project hooks are appended after global hooks
|
||||
from `config.toml`, so they run last and, for `updatedInput`, win
|
||||
ties. A malformed project file logs a warning and startup falls back
|
||||
to global hooks only.
|
||||
|
||||
```toml
|
||||
# .codewhale/hooks.toml
|
||||
[[hooks]]
|
||||
event = "tool_call_before"
|
||||
command = '''echo '{"decision":"deny","reason":"no shell in this repo"}' '''
|
||||
condition = { type = "tool_name", name = "exec_shell" }
|
||||
```
|
||||
|
||||
### Turn-end observer hooks
|
||||
|
||||
|
||||
Reference in New Issue
Block a user