From 0c4d0d777c177a6f16e582b37ebc127ba3960e5a Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Tue, 15 Sep 2026 19:01:36 -0600 Subject: [PATCH] feat(TASK-034): hook dispatch engine in src/hooks.rs - HookEvent (22 dotted variants) + owned ResolvedHook snapshots - RequestContext::resolved_hooks: global (gated by agent global_hooks) -> role (additive) -> agent precedence; per-scope cwds; unknown whitelist entries inert + debug-logged with agent name - fire/fire_resolved: fire-and-forget detached dispatch via platform shell (sh -c / cmd /C raw_arg), null stdio, fresh unix process group, reap-only await, no-runtime guard (skip + debug, never panic) - tool.* payload files: crypto-random nonce, create_new + 0600, verbatim JSON, best-effort delete after reap, hook still runs on write failure - base env (COYOTE_EVENT/HOOK_NAME/CONFIG_DIR/EVENT_TIMESTAMP RFC3339 UTC, SESSION_ID/AGENT_NAME when present) + per-event extras; every value truncated engine-side to 2048 bytes (UTF-8-safe; payload path documented exempt) - drain_pending(timeout): opt-in exit-path seam with spawn-ack semantics (Drop-guard counter; never waits for child completion) - #[cfg(test)] synchronous capture sink (records instead of spawning) + hooks-scoped debug capture in src/testing.rs (poison-proof, boundary-aware target prefix) - Agent::hooks()/global_hooks() accessors; Role::hooks() dead_code marker removed Engine compiles unused by design; call sites land in TASK-035/036/037. --- src/config/agent.rs | 8 + src/config/role.rs | 1 - src/hooks.rs | 1520 +++++++++++++++++++++++++++++++++++++++++++ src/testing.rs | 65 +- 4 files changed, 1580 insertions(+), 14 deletions(-) diff --git a/src/config/agent.rs b/src/config/agent.rs index 6db3e90..f82e272 100644 --- a/src/config/agent.rs +++ b/src/config/agent.rs @@ -640,6 +640,14 @@ impl Agent { self.config.compression_model.as_deref() } + pub fn hooks(&self) -> &HooksMap { + &self.config.hooks + } + + pub fn global_hooks(&self) -> &[String] { + &self.config.global_hooks + } + pub fn is_dynamic_instructions(&self) -> bool { self.config.dynamic_instructions } diff --git a/src/config/role.rs b/src/config/role.rs index 01423c0..18d9220 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -388,7 +388,6 @@ impl Role { self.memory } - #[allow(dead_code)] pub fn hooks(&self) -> Option<&HooksMap> { self.hooks.as_ref() } diff --git a/src/hooks.rs b/src/hooks.rs index 7b47320..3f1d7b1 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -1,5 +1,16 @@ +use crate::config::{RequestContext, paths}; + +use chrono::{SecondsFormat, Utc}; use indexmap::IndexMap; +use rand::distr::{Alphanumeric, SampleString}; use serde::{Deserialize, Serialize}; +use std::path::{Path, PathBuf}; +use std::process::Stdio; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::Duration; +use tokio::io::AsyncWriteExt; +use tokio::process::Command; +use tokio::sync::Notify; /// A single named hook: an external command to run when its event fires. #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] @@ -10,3 +21,1512 @@ pub struct HookDef { /// Ordered mapping of event name -> hook definitions, shared by every config scope. pub type HooksMap = IndexMap>; + +/// Every event hooks can be attached to. `as_str` yields the dotted name used +/// as the key in `hooks:` maps and in `global_hooks` whitelist entries. +#[allow(dead_code)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum HookEvent { + TurnStarted, + TurnCompleted, + TurnInterrupted, + TurnFailed, + SessionStarted, + SessionEnded, + SessionCompressed, + ToolStarted, + ToolCompleted, + ToolFailed, + LlmRequestStarted, + LlmRequestCompleted, + LlmRequestFailed, + AgentStarted, + AgentCompleted, + AgentFailed, + GraphNodeStarted, + GraphNodeCompleted, + GraphNodeFailed, + JobStarted, + JobCompleted, + JobFailed, +} + +impl HookEvent { + pub fn as_str(self) -> &'static str { + match self { + HookEvent::TurnStarted => "turn.started", + HookEvent::TurnCompleted => "turn.completed", + HookEvent::TurnInterrupted => "turn.interrupted", + HookEvent::TurnFailed => "turn.failed", + HookEvent::SessionStarted => "session.started", + HookEvent::SessionEnded => "session.ended", + HookEvent::SessionCompressed => "session.compressed", + HookEvent::ToolStarted => "tool.started", + HookEvent::ToolCompleted => "tool.completed", + HookEvent::ToolFailed => "tool.failed", + HookEvent::LlmRequestStarted => "llm.request.started", + HookEvent::LlmRequestCompleted => "llm.request.completed", + HookEvent::LlmRequestFailed => "llm.request.failed", + HookEvent::AgentStarted => "agent.started", + HookEvent::AgentCompleted => "agent.completed", + HookEvent::AgentFailed => "agent.failed", + HookEvent::GraphNodeStarted => "graph.node.started", + HookEvent::GraphNodeCompleted => "graph.node.completed", + HookEvent::GraphNodeFailed => "graph.node.failed", + HookEvent::JobStarted => "job.started", + HookEvent::JobCompleted => "job.completed", + HookEvent::JobFailed => "job.failed", + } + } +} + +/// An owned snapshot of one hook to run. Detached dispatch tasks, job +/// snapshots, and forked graph-branch contexts carry it without borrowing the +/// context it was resolved from. +#[derive(Debug, Clone)] +pub struct ResolvedHook { + pub name: String, + /// Whitelist form `.`, e.g. `tool.started.notify`. + pub full_name: String, + pub command: String, + pub cwd: PathBuf, +} + +impl RequestContext { + /// Hooks that apply to this context for `event`, in firing order: global + /// (whitelisted through `global_hooks` when an agent is active), then + /// role, then agent. Duplicate names across scopes all fire. + /// + /// Role hooks resolve only while the role is held directly on the context; + /// a role held by an active session is not consulted, because sessions do + /// not carry hooks. Resolution through sessions is a call-site concern. + #[allow(dead_code)] + pub fn resolved_hooks(&self, event: HookEvent) -> Vec { + resolve_hooks( + event, + &self.app.config.hooks, + self.agent + .as_ref() + .map(|agent| (agent.global_hooks(), agent.name())), + self.role.as_ref().and_then(|role| role.hooks()), + self.agent + .as_ref() + .map(|agent| (agent.hooks(), agent.name())), + ) + } +} + +fn resolve_hooks( + event: HookEvent, + global_hooks: &HooksMap, + agent_gate: Option<(&[String], &str)>, + role_hooks: Option<&HooksMap>, + agent_hooks: Option<(&HooksMap, &str)>, +) -> Vec { + let event_name = event.as_str(); + let mut resolved = Vec::new(); + + if let Some(defs) = global_hooks.get(event_name) { + let cwd = paths::config_dir(); + for def in defs { + let full_name = format!("{event_name}.{}", def.name); + let admitted = match agent_gate { + None => true, + Some((gate, _)) => gate.contains(&full_name), + }; + if admitted { + resolved.push(ResolvedHook { + name: def.name.clone(), + full_name, + command: def.command.clone(), + cwd: cwd.clone(), + }); + } + } + } + if let Some((gate, agent_name)) = agent_gate + && !global_hooks.is_empty() + { + let prefix = format!("{event_name}."); + for entry in gate { + if let Some(name) = entry.strip_prefix(&prefix) { + let known = global_hooks + .get(event_name) + .is_some_and(|defs| defs.iter().any(|def| def.name == name)); + if !known { + debug!( + "Ignoring unknown global hook whitelist entry '{entry}' for agent '{agent_name}'" + ); + } + } + } + } + + if let Some(defs) = role_hooks.and_then(|hooks| hooks.get(event_name)) { + push_defs(event_name, defs, &paths::roles_dir(), &mut resolved); + } + + if let Some((hooks, agent_name)) = agent_hooks + && let Some(defs) = hooks.get(event_name) + { + push_defs( + event_name, + defs, + &paths::agent_data_dir(agent_name), + &mut resolved, + ); + } + + resolved +} + +fn push_defs(event_name: &str, defs: &[HookDef], cwd: &Path, out: &mut Vec) { + for def in defs { + out.push(ResolvedHook { + name: def.name.clone(), + full_name: format!("{event_name}.{}", def.name), + command: def.command.clone(), + cwd: cwd.to_path_buf(), + }); + } +} + +/// Fires every hook configured for `event` in `ctx`. Fire-and-forget: each +/// hook runs in its own detached task and nothing about its outcome — exit +/// status, stdout, stderr — is awaited or read. `payload` carries the tool +/// arguments JSON for `tool.*` events; pass `None` otherwise. +#[allow(dead_code)] +pub fn fire( + event: HookEvent, + ctx: &RequestContext, + extras: &[(&str, String)], + payload: Option, +) { + let resolved = ctx.resolved_hooks(event); + if resolved.is_empty() { + return; + } + fire_resolved(event, resolved, base_envs(event, ctx), extras, payload); +} + +/// Dispatches already-resolved hooks, for call sites that outlive the context +/// borrow they resolved from. Same fire-and-forget semantics as [`fire`]. +#[allow(dead_code)] +pub fn fire_resolved( + event: HookEvent, + resolved: Vec, + base_envs: Vec<(String, String)>, + extras: &[(&str, String)], + payload: Option, +) { + let runtime = tokio::runtime::Handle::try_current(); + for hook in resolved { + if hook.command.trim().is_empty() { + debug!( + "Skipping hook '{}' (cwd '{}'): empty command", + hook.full_name, + hook.cwd.display() + ); + continue; + } + + let mut envs = base_envs.clone(); + envs.push(("COYOTE_HOOK_NAME".to_string(), hook.name.clone())); + envs.extend( + extras + .iter() + .map(|(key, value)| (key.to_string(), value.clone())), + ); + let envs: Vec<(String, String)> = envs + .into_iter() + .map(|(key, mut value)| { + // `end` is a char boundary by construction, so `truncate` cannot panic. + let end = truncate_env_value(&value).len(); + value.truncate(end); + (key, value) + }) + .collect(); + + #[cfg(test)] + if test_sink::record(event, &hook, &envs) { + continue; + } + + let Ok(runtime) = &runtime else { + debug!( + "Skipping hook '{}': no Tokio runtime on this thread", + hook.full_name + ); + continue; + }; + + let payload = payload.clone(); + drop(runtime.spawn(run_hook( + event, + hook, + envs, + payload, + SpawnAckGuard::register(), + ))); + } +} + +static PENDING_SPAWNS: AtomicUsize = AtomicUsize::new(0); +static SPAWN_NOTIFY: Notify = Notify::const_new(); + +/// Marks one dispatch as pending until its spawn attempt resolves. The +/// decrement lives in `Drop` so it also fires on panic or when the dispatch +/// task is dropped before it was ever polled. +struct SpawnAckGuard; + +impl SpawnAckGuard { + fn register() -> Self { + PENDING_SPAWNS.fetch_add(1, Ordering::SeqCst); + SpawnAckGuard + } +} + +impl Drop for SpawnAckGuard { + fn drop(&mut self) { + PENDING_SPAWNS.fetch_sub(1, Ordering::SeqCst); + SPAWN_NOTIFY.notify_waiters(); + } +} + +/// Waits until every in-flight dispatch has finished its spawn attempt — +/// payload file written and `Command::spawn` returned, successfully or not — +/// or until `timeout` elapses. For exit-path call sites only (session end / +/// final turn): this waits for child spawn, never completion. Hook children +/// deliberately outlive coyote as orphans. +#[allow(dead_code)] +pub async fn drain_pending(timeout: Duration) { + let deadline = tokio::time::Instant::now() + timeout; + while PENDING_SPAWNS.load(Ordering::SeqCst) > 0 { + let mut notified = std::pin::pin!(SPAWN_NOTIFY.notified()); + notified.as_mut().enable(); + if PENDING_SPAWNS.load(Ordering::SeqCst) == 0 { + return; + } + if tokio::time::timeout_at(deadline, notified).await.is_err() { + debug!( + "Gave up waiting for {} pending hook dispatch(es) after {timeout:?}", + PENDING_SPAWNS.load(Ordering::SeqCst) + ); + return; + } + } +} + +fn base_envs(event: HookEvent, ctx: &RequestContext) -> Vec<(String, String)> { + base_envs_parts( + event, + ctx.session.as_ref().map(|session| session.name()), + ctx.agent.as_ref().map(|agent| agent.name()), + ) +} + +fn base_envs_parts( + event: HookEvent, + session_name: Option<&str>, + agent_name: Option<&str>, +) -> Vec<(String, String)> { + let mut envs = vec![ + ("COYOTE_EVENT".to_string(), event.as_str().to_string()), + ( + "COYOTE_CONFIG_DIR".to_string(), + paths::config_dir().display().to_string(), + ), + ( + "COYOTE_EVENT_TIMESTAMP".to_string(), + Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true), + ), + ]; + if let Some(name) = session_name { + envs.push(("COYOTE_SESSION_ID".to_string(), name.to_string())); + } + if let Some(name) = agent_name { + envs.push(("COYOTE_AGENT_NAME".to_string(), name.to_string())); + } + envs +} + +/// Creates the payload file fresh (`create_new`) with owner-only permissions, +/// so a pre-existing path — symlink or otherwise — is an error. On failure +/// the hook still runs, just without `COYOTE_HOOK_PAYLOAD_FILE`. +async fn write_payload_file(path: &Path, json: &str) -> std::io::Result<()> { + let mut options = tokio::fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + options.mode(0o600); + let mut file = options.open(path).await?; + file.write_all(json.as_bytes()).await?; + file.flush().await +} + +#[cfg(test)] +static PAYLOAD_DIR_OVERRIDE: std::sync::Mutex> = std::sync::Mutex::new(None); + +/// Directory payload files are written to: `std::env::temp_dir()`, except in +/// tests, where `PAYLOAD_DIR_OVERRIDE` redirects it without touching +/// process-global state like `TMPDIR`. +fn payload_dir() -> PathBuf { + #[cfg(test)] + if let Some(dir) = PAYLOAD_DIR_OVERRIDE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone() + { + return dir; + } + std::env::temp_dir() +} + +async fn run_hook( + event: HookEvent, + hook: ResolvedHook, + mut envs: Vec<(String, String)>, + payload: Option, + ack: SpawnAckGuard, +) { + let payload_file = match payload { + Some(json) => { + let nonce = Alphanumeric.sample_string(&mut rand::rng(), 16); + let path = payload_dir().join(format!("coyote-hook-{}-{nonce}.json", event.as_str())); + match write_payload_file(&path, &json).await { + Ok(()) => Some(path), + Err(err) => { + debug!( + "Failed to write payload file for hook '{}': {err}", + hook.full_name + ); + None + } + } + } + None => None, + }; + if let Some(path) = &payload_file { + envs.push(( + "COYOTE_HOOK_PAYLOAD_FILE".to_string(), + path.display().to_string(), + )); + } + + #[cfg(not(windows))] + let mut command = { + let mut command = Command::new("sh"); + command.arg("-c").arg(&hook.command); + command + }; + // cmd.exe does not parse CommandLineToArgvW quoting, so the command line + // must be passed verbatim rather than through arg()'s quoting rules. + #[cfg(windows)] + let mut command = { + use std::os::windows::process::CommandExt; + let mut command = Command::new("cmd"); + command.arg("/C"); + command.as_std_mut().raw_arg(&hook.command); + command + }; + + command + .envs(envs) + .current_dir(&hook.cwd) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + // A fresh process group keeps a terminal Ctrl-C aimed at coyote from + // signalling hook children, which deliberately outlive coyote. + #[cfg(unix)] + command.process_group(0); + match command.spawn() { + Ok(mut child) => { + drop(ack); + let _ = child.wait().await; + } + Err(err) => { + debug!( + "Failed to spawn hook '{}' in '{}': {err}", + hook.full_name, + hook.cwd.display() + ); + drop(ack); + } + } + + if let Some(path) = payload_file { + let _ = tokio::fs::remove_file(path).await; + } +} + +/// Truncates an env value to 2048 bytes without splitting a UTF-8 character. +/// `fire_resolved` applies it to every env value it dispatches, with one +/// exemption: `COYOTE_HOOK_PAYLOAD_FILE` is pushed by `run_hook` after the +/// truncation pass, because that value is engine-generated, inherently short, +/// and a truncated path would be corrupt rather than merely trimmed. Exported +/// for call sites that need the bound before dispatch. +#[allow(dead_code)] +pub fn truncate_env_value(value: &str) -> &str { + const MAX_ENV_VALUE_BYTES: usize = 2048; + if value.len() <= MAX_ENV_VALUE_BYTES { + return value; + } + let mut end = MAX_ENV_VALUE_BYTES; + while !value.is_char_boundary(end) { + end -= 1; + } + &value[..end] +} + +#[cfg(test)] +pub(crate) mod test_sink { + use super::{HookEvent, ResolvedHook}; + use std::collections::HashMap; + use std::sync::Mutex; + use std::sync::atomic::{AtomicUsize, Ordering}; + + static INSTALLED: AtomicUsize = AtomicUsize::new(0); + // A test that panics while holding the buffer poisons its mutex; every + // lock site recovers via `into_inner` so later tests keep working. + pub(super) static CAPTURES: Mutex> = Mutex::new(Vec::new()); + + /// One dispatch the sink recorded instead of spawning. + #[derive(Debug, Clone)] + pub struct Capture { + pub event: HookEvent, + pub hook_name: String, + pub envs: HashMap, + } + + /// Suppresses process spawning and records every dispatch synchronously + /// until the returned guard drops. The capture buffer is process-global + /// and shared by all tests, so count assertions must either use hook + /// names unique to the test fixture and filter captures, or serialize + /// with `#[serial_test::serial]`. Tests that rely on real spawning must + /// also serialize against tests that install the sink. + #[must_use] + pub fn install() -> SinkGuard { + INSTALLED.fetch_add(1, Ordering::SeqCst); + SinkGuard + } + + pub struct SinkGuard; + + impl Drop for SinkGuard { + fn drop(&mut self) { + INSTALLED.fetch_sub(1, Ordering::SeqCst); + } + } + + pub(super) fn record(event: HookEvent, hook: &ResolvedHook, envs: &[(String, String)]) -> bool { + if INSTALLED.load(Ordering::SeqCst) == 0 { + return false; + } + CAPTURES + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .push(Capture { + event, + hook_name: hook.name.clone(), + envs: envs.iter().cloned().collect(), + }); + true + } + + /// Removes and returns everything recorded so far. + pub fn drain() -> Vec { + std::mem::take( + &mut *CAPTURES + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()), + ) + } + + /// A copy of everything recorded so far, for tests that filter by a hook + /// name unique to their fixture. + #[allow(dead_code)] + pub fn snapshot() -> Vec { + CAPTURES + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{AppConfig, AppState, Role, WorkingMode}; + use serial_test::serial; + use std::env; + use std::sync::Arc; + + fn hooks_map(event: &str, defs: &[(&str, &str)]) -> HooksMap { + let defs = defs + .iter() + .map(|(name, command)| HookDef { + name: name.to_string(), + command: command.to_string(), + }) + .collect(); + IndexMap::from([(event.to_string(), defs)]) + } + + fn names(resolved: &[ResolvedHook]) -> Vec<&str> { + resolved.iter().map(|hook| hook.name.as_str()).collect() + } + + fn ctx_with_global_hooks(hooks: HooksMap) -> RequestContext { + let mut app = AppState::test_default(); + app.config = Arc::new(AppConfig { + hooks, + ..Default::default() + }); + RequestContext::new(Arc::new(app), WorkingMode::Cmd) + } + + #[test] + fn hook_event_names_are_dotted() { + let cases = [ + (HookEvent::TurnStarted, "turn.started"), + (HookEvent::TurnCompleted, "turn.completed"), + (HookEvent::TurnInterrupted, "turn.interrupted"), + (HookEvent::TurnFailed, "turn.failed"), + (HookEvent::SessionStarted, "session.started"), + (HookEvent::SessionEnded, "session.ended"), + (HookEvent::SessionCompressed, "session.compressed"), + (HookEvent::ToolStarted, "tool.started"), + (HookEvent::ToolCompleted, "tool.completed"), + (HookEvent::ToolFailed, "tool.failed"), + (HookEvent::LlmRequestStarted, "llm.request.started"), + (HookEvent::LlmRequestCompleted, "llm.request.completed"), + (HookEvent::LlmRequestFailed, "llm.request.failed"), + (HookEvent::AgentStarted, "agent.started"), + (HookEvent::AgentCompleted, "agent.completed"), + (HookEvent::AgentFailed, "agent.failed"), + (HookEvent::GraphNodeStarted, "graph.node.started"), + (HookEvent::GraphNodeCompleted, "graph.node.completed"), + (HookEvent::GraphNodeFailed, "graph.node.failed"), + (HookEvent::JobStarted, "job.started"), + (HookEvent::JobCompleted, "job.completed"), + (HookEvent::JobFailed, "job.failed"), + ]; + for (event, name) in cases { + assert_eq!(event.as_str(), name); + } + } + + #[test] + fn root_context_gets_all_global_hooks() { + let mut global = hooks_map("turn.started", &[("a", "cmd-a"), ("b", "cmd-b")]); + global.extend(hooks_map("turn.completed", &[("other", "cmd-other")])); + + let resolved = resolve_hooks(HookEvent::TurnStarted, &global, None, None, None); + + assert_eq!(names(&resolved), ["a", "b"]); + assert_eq!(resolved[0].full_name, "turn.started.a"); + } + + #[test] + fn agent_without_whitelist_gets_no_global_hooks() { + let global = hooks_map("turn.started", &[("a", "cmd-a")]); + let agent = hooks_map("turn.started", &[("own", "agent-cmd")]); + + let resolved = resolve_hooks( + HookEvent::TurnStarted, + &global, + Some((&[], "plain-agent")), + None, + Some((&agent, "plain-agent")), + ); + + assert_eq!(names(&resolved), ["own"]); + } + + #[test] + fn whitelist_admits_only_exact_global_entries() { + let global = hooks_map( + "turn.started", + &[("a", "cmd-a"), ("b", "cmd-b"), ("c", "cmd-c")], + ); + let empty = HooksMap::default(); + let gate = vec![ + "turn.started.b".to_string(), + "turn.started.missing".to_string(), + "other.event.x".to_string(), + ]; + + let resolved = resolve_hooks( + HookEvent::TurnStarted, + &global, + Some((&gate, "gated-agent")), + None, + Some((&empty, "gated-agent")), + ); + + assert_eq!(names(&resolved), ["b"]); + } + + #[test] + fn unknown_whitelist_entries_log_debug_and_stay_inert() { + crate::testing::install_warn_collector(); + let global = hooks_map("turn.started", &[("a", "cmd-a")]); + let gate = vec![ + "turn.started.a".to_string(), + "turn.started.nonexistent-marker-xyz".to_string(), + ]; + + let resolved = resolve_hooks( + HookEvent::TurnStarted, + &global, + Some((&gate, "gate-agent-xyz")), + None, + None, + ); + + assert_eq!(names(&resolved), ["a"]); + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!(debugs.iter().any(|message| { + message.contains("turn.started.nonexistent-marker-xyz") + && message.contains("for agent 'gate-agent-xyz'") + })); + } + + #[test] + fn empty_global_map_short_circuits_whitelist_diagnostics() { + crate::testing::install_warn_collector(); + let global = HooksMap::default(); + let role = hooks_map("turn.started", &[("role-only", "role-cmd")]); + let gate = vec!["turn.started.x-shortcircuit-marker-p3q".to_string()]; + + let resolved = resolve_hooks( + HookEvent::TurnStarted, + &global, + Some((&gate, "short-circuit-agent")), + Some(&role), + None, + ); + + assert_eq!(names(&resolved), ["role-only"]); + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!( + debugs + .iter() + .all(|message| !message.contains("shortcircuit-marker-p3q")) + ); + } + + #[test] + fn role_hooks_are_always_additive() { + let global = HooksMap::default(); + let role = hooks_map("session.ended", &[("archive", "role-cmd")]); + let empty = HooksMap::default(); + + let gated = resolve_hooks( + HookEvent::SessionEnded, + &global, + Some((&[], "gated-agent")), + Some(&role), + Some((&empty, "gated-agent")), + ); + assert_eq!(names(&gated), ["archive"]); + + let root = resolve_hooks(HookEvent::SessionEnded, &global, None, Some(&role), None); + assert_eq!(names(&root), ["archive"]); + } + + #[test] + #[serial] + fn scopes_resolve_in_global_role_agent_order() { + let global = hooks_map("tool.completed", &[("notify", "global-cmd")]); + let role = hooks_map("tool.completed", &[("notify", "role-cmd")]); + let agent = hooks_map("tool.completed", &[("notify", "agent-cmd")]); + let gate = vec!["tool.completed.notify".to_string()]; + + let resolved = resolve_hooks( + HookEvent::ToolCompleted, + &global, + Some((&gate, "order-agent")), + Some(&role), + Some((&agent, "order-agent")), + ); + + let commands: Vec<&str> = resolved.iter().map(|hook| hook.command.as_str()).collect(); + assert_eq!(commands, ["global-cmd", "role-cmd", "agent-cmd"]); + assert!( + resolved + .iter() + .all(|hook| hook.full_name == "tool.completed.notify") + ); + assert_eq!(resolved[0].cwd, paths::config_dir()); + assert_eq!(resolved[1].cwd, paths::roles_dir()); + assert_eq!(resolved[2].cwd, paths::agent_data_dir("order-agent")); + } + + #[test] + fn resolved_hooks_reads_global_config_from_context() { + let ctx = ctx_with_global_hooks(hooks_map("session.started", &[("smoke", "true")])); + + let resolved = ctx.resolved_hooks(HookEvent::SessionStarted); + + assert_eq!(names(&resolved), ["smoke"]); + assert_eq!(resolved[0].full_name, "session.started.smoke"); + assert!(ctx.resolved_hooks(HookEvent::SessionEnded).is_empty()); + } + + #[test] + #[serial] + fn empty_command_is_skipped_without_error() { + let _guard = test_sink::install(); + let cwd = env::temp_dir(); + let hooks = vec![ + ResolvedHook { + name: "gap".to_string(), + full_name: "turn.failed.gap".to_string(), + command: " ".to_string(), + cwd: cwd.clone(), + }, + ResolvedHook { + name: "after-gap".to_string(), + full_name: "turn.failed.after-gap".to_string(), + command: "true".to_string(), + cwd, + }, + ]; + + fire_resolved(HookEvent::TurnFailed, hooks, Vec::new(), &[], None); + + let captures = test_sink::drain(); + assert!(captures.iter().all(|capture| capture.hook_name != "gap")); + assert!( + captures + .iter() + .any(|capture| capture.hook_name == "after-gap") + ); + } + + #[test] + #[serial] + fn sink_captures_synchronously_and_suppresses_spawn() { + let marker = + env::temp_dir().join(format!("coyote-hook-sink-marker-{}", std::process::id())); + let _ = std::fs::remove_file(&marker); + let touch = format!("touch \"{}\"", marker.display()); + let ctx = ctx_with_global_hooks(hooks_map("job.completed", &[("sink-only", &touch)])); + + let _guard = test_sink::install(); + fire( + HookEvent::JobCompleted, + &ctx, + &[("HOOK_EXTRA", "extra-value".to_string())], + None, + ); + + let captures = test_sink::drain(); + let capture = captures + .iter() + .find(|capture| capture.hook_name == "sink-only") + .expect("sink must record synchronously before fire returns"); + assert_eq!(capture.event, HookEvent::JobCompleted); + assert_eq!( + capture.envs.get("COYOTE_EVENT").map(String::as_str), + Some("job.completed") + ); + assert_eq!( + capture.envs.get("COYOTE_HOOK_NAME").map(String::as_str), + Some("sink-only") + ); + assert!(capture.envs.contains_key("COYOTE_CONFIG_DIR")); + assert!(capture.envs.contains_key("COYOTE_EVENT_TIMESTAMP")); + assert_eq!( + capture.envs.get("HOOK_EXTRA").map(String::as_str), + Some("extra-value") + ); + assert!( + !marker.exists(), + "no process may spawn while the sink is installed" + ); + } + + #[test] + #[serial] + fn fire_resolved_truncates_every_env_value() { + let _guard = test_sink::install(); + let oversized = "x".repeat(3000); + let hook = ResolvedHook { + name: "truncate-probe".to_string(), + full_name: "turn.failed.truncate-probe".to_string(), + command: "true".to_string(), + cwd: env::temp_dir(), + }; + + fire_resolved( + HookEvent::TurnFailed, + vec![hook], + vec![("COYOTE_BIG_BASE".to_string(), oversized.clone())], + &[("COYOTE_BIG_EXTRA", oversized)], + None, + ); + + let captures = test_sink::drain(); + let capture = captures + .iter() + .find(|capture| capture.hook_name == "truncate-probe") + .expect("sink must record the dispatch"); + assert_eq!(capture.envs.get("COYOTE_BIG_BASE").unwrap().len(), 2048); + assert_eq!(capture.envs.get("COYOTE_BIG_EXTRA").unwrap().len(), 2048); + } + + #[test] + #[serial] + fn sink_recovers_after_capture_buffer_poisoning() { + let _guard = test_sink::install(); + test_sink::drain(); + std::thread::spawn(|| { + let _held = test_sink::CAPTURES.lock().unwrap(); + panic!("poison the capture buffer"); + }) + .join() + .unwrap_err(); + + let hook = ResolvedHook { + name: "poison-recovery-marker-j8r".to_string(), + full_name: "turn.failed.poison-recovery-marker-j8r".to_string(), + command: "true".to_string(), + cwd: env::temp_dir(), + }; + assert!(test_sink::record(HookEvent::TurnFailed, &hook, &[])); + + let captures = test_sink::drain(); + assert!( + captures + .iter() + .any(|capture| capture.hook_name == "poison-recovery-marker-j8r") + ); + } + + #[test] + fn base_envs_parts_reflects_session_and_agent() { + let envs = base_envs_parts(HookEvent::TurnStarted, Some("sess-1"), Some("helper")); + let find = |key: &str| { + envs.iter() + .find(|(k, _)| k == key) + .map(|(_, value)| value.as_str()) + }; + assert_eq!(find("COYOTE_SESSION_ID"), Some("sess-1")); + assert_eq!(find("COYOTE_AGENT_NAME"), Some("helper")); + let timestamp = find("COYOTE_EVENT_TIMESTAMP").expect("timestamp env"); + let parsed = chrono::DateTime::parse_from_rfc3339(timestamp).unwrap(); + assert_eq!(parsed.offset().local_minus_utc(), 0); + + let envs = base_envs_parts(HookEvent::TurnStarted, None, None); + assert!( + envs.iter() + .all(|(key, _)| key != "COYOTE_SESSION_ID" && key != "COYOTE_AGENT_NAME") + ); + } + + #[tokio::test] + #[serial] + async fn drain_pending_returns_immediately_when_idle() { + let start = std::time::Instant::now(); + drain_pending(Duration::from_secs(5)).await; + assert!(start.elapsed() < Duration::from_secs(1)); + } + + #[tokio::test(start_paused = true)] + #[serial] + async fn drain_pending_logs_when_the_timeout_expires() { + crate::testing::install_warn_collector(); + let pending = SpawnAckGuard::register(); + + drain_pending(Duration::from_millis(50)).await; + + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!( + debugs + .iter() + .any(|message| message.contains("Gave up waiting")) + ); + drop(pending); + } + + #[test] + fn truncate_env_value_keeps_short_values() { + assert_eq!(truncate_env_value("short"), "short"); + let exact = "a".repeat(2048); + assert_eq!(truncate_env_value(&exact), exact); + } + + #[test] + fn truncate_env_value_respects_char_boundaries() { + let value = format!("{}€", "a".repeat(2047)); + let truncated = truncate_env_value(&value); + assert_eq!(truncated.len(), 2047); + assert!(truncated.chars().all(|c| c == 'a')); + } + + #[test] + #[serial] + fn fire_resolved_without_runtime_skips_instead_of_panicking() { + crate::testing::install_warn_collector(); + let hook = ResolvedHook { + name: "no-runtime-marker-a7c".to_string(), + full_name: "turn.failed.no-runtime-marker-a7c".to_string(), + command: "true".to_string(), + cwd: env::temp_dir(), + }; + + fire_resolved(HookEvent::TurnFailed, vec![hook], Vec::new(), &[], None); + + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!(debugs.iter().any(|message| { + message.contains("no-runtime-marker-a7c") + && message.contains("no Tokio runtime on this thread") + })); + } + + #[test] + #[serial] + fn fire_without_runtime_skips_instead_of_panicking() { + crate::testing::install_warn_collector(); + let ctx = ctx_with_global_hooks(hooks_map( + "turn.failed", + &[("fire-no-runtime-marker-b8d", "true")], + )); + + fire(HookEvent::TurnFailed, &ctx, &[], None); + + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!(debugs.iter().any(|message| { + message.contains("fire-no-runtime-marker-b8d") + && message.contains("no Tokio runtime on this thread") + })); + } + + #[test] + fn debug_capture_excludes_non_hooks_targets() { + crate::testing::install_warn_collector(); + let hook = ResolvedHook { + name: "included-marker-k2v".to_string(), + full_name: "turn.failed.included-marker-k2v".to_string(), + command: " ".to_string(), + cwd: env::temp_dir(), + }; + + log::debug!( + target: concat!(env!("CARGO_CRATE_NAME"), "::not_hooks_marker_k2v"), + "excluded-marker-k2v" + ); + log::debug!( + target: concat!(env!("CARGO_CRATE_NAME"), "::hooksbogus_marker_k2v"), + "boundary-excluded-marker-k2v" + ); + fire_resolved(HookEvent::TurnFailed, vec![hook], Vec::new(), &[], None); + + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!( + debugs + .iter() + .all(|message| !message.contains("excluded-marker-k2v")) + ); + assert!( + debugs + .iter() + .all(|message| !message.contains("boundary-excluded-marker-k2v")) + ); + assert!( + debugs + .iter() + .any(|message| message.contains("included-marker-k2v")) + ); + } + + #[test] + #[serial] + fn debug_capture_recovers_after_buffer_poisoning() { + crate::testing::install_warn_collector(); + std::thread::spawn(|| { + let _held = crate::testing::debug_messages().lock().unwrap(); + panic!("poison the debug buffer"); + }) + .join() + .unwrap_err(); + + let hook = ResolvedHook { + name: "poisoned-buffer-marker-w4t".to_string(), + full_name: "turn.failed.poisoned-buffer-marker-w4t".to_string(), + command: " ".to_string(), + cwd: env::temp_dir(), + }; + fire_resolved(HookEvent::TurnFailed, vec![hook], Vec::new(), &[], None); + + let mut debugs = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + assert!(debugs.iter().any(|message| { + message.contains("poisoned-buffer-marker-w4t") && message.contains("empty command") + })); + debugs.retain(|message| !message.contains("poisoned-buffer-marker-w4t")); + } + + #[test] + #[serial] + fn resolved_hooks_adds_role_hooks_through_context() { + let mut ctx = ctx_with_global_hooks(hooks_map( + "turn.completed", + &[("global-probe", "global-cmd")], + )); + let content = "---\nhooks:\n turn.completed:\n - name: role-probe\n command: role-cmd\n---\nPrompt"; + ctx.role = Some(Role::new("test", content)); + + let resolved = ctx.resolved_hooks(HookEvent::TurnCompleted); + + assert_eq!(names(&resolved), ["global-probe", "role-probe"]); + assert_eq!(resolved[1].command, "role-cmd"); + assert_eq!(resolved[1].cwd, paths::roles_dir()); + } + + #[cfg(unix)] + mod dispatch { + use super::*; + use crate::utils::get_env_name; + use std::fs::{create_dir_all, remove_dir_all}; + use std::path::PathBuf; + use std::time::{Duration, SystemTime, UNIX_EPOCH}; + + struct TestConfigDirGuard { + key: String, + previous: Option, + path: PathBuf, + } + + impl TestConfigDirGuard { + fn new() -> Self { + let key = get_env_name("config_dir"); + let previous = env::var_os(&key); + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let path = env::temp_dir().join(format!("coyote-hooks-tests-{unique}")); + create_dir_all(&path).unwrap(); + unsafe { + env::set_var(&key, &path); + } + Self { + key, + previous, + path, + } + } + } + + impl Drop for TestConfigDirGuard { + fn drop(&mut self) { + if let Some(previous) = &self.previous { + unsafe { + env::set_var(&self.key, previous); + } + } else { + unsafe { + env::remove_var(&self.key); + } + } + let _ = remove_dir_all(&self.path); + } + } + + /// Points `run_hook`'s payload directory at `path` for the guard's + /// lifetime, so payload writes can be aimed at a nonexistent + /// directory without touching the process-global `TMPDIR`. + struct PayloadDirOverrideGuard; + + impl PayloadDirOverrideGuard { + fn new(path: PathBuf) -> Self { + *PAYLOAD_DIR_OVERRIDE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = Some(path); + Self + } + } + + impl Drop for PayloadDirOverrideGuard { + fn drop(&mut self) { + *PAYLOAD_DIR_OVERRIDE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = None; + } + } + + async fn wait_for(what: &str, cond: impl Fn() -> bool) { + for _ in 0..400 { + if cond() { + return; + } + tokio::time::sleep(Duration::from_millis(25)).await; + } + panic!("timed out waiting for {what}"); + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn fire_spawns_detached_with_envs() { + let guard = TestConfigDirGuard::new(); + let out = guard.path.join("env-out"); + let gate = guard.path.join("gate"); + let done = guard.path.join("done"); + let command = r#"env > "$HOOK_OUT.tmp" && mv "$HOOK_OUT.tmp" "$HOOK_OUT"; until [ -e "$HOOK_GATE" ]; do sleep 0.05; done; : > "$HOOK_DONE""#; + let ctx = ctx_with_global_hooks(hooks_map("turn.completed", &[("envdump", command)])); + + fire( + HookEvent::TurnCompleted, + &ctx, + &[ + ("HOOK_OUT", out.display().to_string()), + ("HOOK_GATE", gate.display().to_string()), + ("HOOK_DONE", done.display().to_string()), + ], + None, + ); + + wait_for("hook env dump", || out.exists()).await; + assert!( + !done.exists(), + "dispatch must return before the hook completes" + ); + + let env_dump = std::fs::read_to_string(&out).unwrap(); + let lines: Vec<&str> = env_dump.lines().collect(); + assert!(lines.contains(&"COYOTE_EVENT=turn.completed")); + assert!(lines.contains(&"COYOTE_HOOK_NAME=envdump")); + let config_dir_line = format!("COYOTE_CONFIG_DIR={}", guard.path.display()); + assert!(lines.contains(&config_dir_line.as_str())); + assert!( + lines + .iter() + .any(|line| line.starts_with("COYOTE_EVENT_TIMESTAMP=")) + ); + let gate_line = format!("HOOK_GATE={}", gate.display()); + assert!(lines.contains(&gate_line.as_str())); + + std::fs::write(&gate, "").unwrap(); + wait_for("hook completion", || done.exists()).await; + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn hook_stdin_is_null_not_inherited() { + let guard = TestConfigDirGuard::new(); + let out = guard.path.join("stdin-out"); + let command = r#"if read -t 5 line; then echo open > "$HOOK_OUT.tmp"; else echo eof > "$HOOK_OUT.tmp"; fi; mv "$HOOK_OUT.tmp" "$HOOK_OUT""#; + let ctx = ctx_with_global_hooks(hooks_map("turn.started", &[("stdin-probe", command)])); + let start = std::time::Instant::now(); + + fire( + HookEvent::TurnStarted, + &ctx, + &[("HOOK_OUT", out.display().to_string())], + None, + ); + + wait_for("stdin probe output", || out.exists()).await; + assert_eq!(std::fs::read_to_string(&out).unwrap().trim(), "eof"); + assert!( + start.elapsed() < Duration::from_secs(5), + "read must hit EOF immediately, not time out" + ); + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn payload_file_is_written_and_removed() { + let guard = TestConfigDirGuard::new(); + let out = guard.path.join("payload-out"); + let path_out = guard.path.join("payload-path"); + let command = r#"cp "$COYOTE_HOOK_PAYLOAD_FILE" "$HOOK_OUT.tmp" && mv "$HOOK_OUT.tmp" "$HOOK_OUT"; printf '%s' "$COYOTE_HOOK_PAYLOAD_FILE" > "$HOOK_PATH.tmp" && mv "$HOOK_PATH.tmp" "$HOOK_PATH""#; + let ctx = ctx_with_global_hooks(hooks_map("tool.started", &[("payload", command)])); + let payload = r#"{"path":"/tmp/target","recursive":true}"#; + + fire( + HookEvent::ToolStarted, + &ctx, + &[ + ("HOOK_OUT", out.display().to_string()), + ("HOOK_PATH", path_out.display().to_string()), + ], + Some(payload.to_string()), + ); + + wait_for("payload copy", || out.exists() && path_out.exists()).await; + assert_eq!(std::fs::read_to_string(&out).unwrap(), payload); + + let payload_file = PathBuf::from(std::fs::read_to_string(&path_out).unwrap()); + let file_name = payload_file.file_name().unwrap().to_string_lossy(); + assert!(file_name.starts_with("coyote-hook-tool.started-")); + wait_for("payload file removal", || !payload_file.exists()).await; + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn drain_pending_waits_for_spawn_not_completion() { + let guard = TestConfigDirGuard::new(); + let out = guard.path.join("spawned"); + let gate = guard.path.join("gate"); + let done = guard.path.join("done"); + let command = r#": > "$HOOK_OUT"; until [ -e "$HOOK_GATE" ]; do sleep 0.05; done; : > "$HOOK_DONE""#; + let ctx = + ctx_with_global_hooks(hooks_map("session.ended", &[("drain-probe", command)])); + + fire( + HookEvent::SessionEnded, + &ctx, + &[ + ("HOOK_OUT", out.display().to_string()), + ("HOOK_GATE", gate.display().to_string()), + ("HOOK_DONE", done.display().to_string()), + ], + None, + ); + + drain_pending(Duration::from_secs(10)).await; + + assert!(!done.exists(), "drain must not wait for hook completion"); + wait_for("hook start marker", || out.exists()).await; + assert!(!done.exists()); + + std::fs::write(&gate, "").unwrap(); + wait_for("hook completion", || done.exists()).await; + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn error_paths_log_debug_only() { + crate::testing::install_warn_collector(); + let empty_cwd = env::temp_dir(); + let hooks = vec![ + ResolvedHook { + name: "empty-marker-f5b".to_string(), + full_name: "turn.failed.empty-marker-f5b".to_string(), + command: " ".to_string(), + cwd: empty_cwd.clone(), + }, + ResolvedHook { + name: "badcwd-marker-f5b".to_string(), + full_name: "turn.failed.badcwd-marker-f5b".to_string(), + command: "true".to_string(), + cwd: PathBuf::from("/nonexistent/coyote-badcwd-marker-f5b"), + }, + ]; + + fire_resolved(HookEvent::TurnFailed, hooks, Vec::new(), &[], None); + drain_pending(Duration::from_secs(10)).await; + + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + let empty_cwd_display = empty_cwd.display().to_string(); + assert!(debugs.iter().any(|message| { + message.contains("empty-marker-f5b") + && message.contains("empty command") + && message.contains(empty_cwd_display.as_str()) + })); + assert!(debugs.iter().any(|message| { + message.contains("Failed to spawn hook 'turn.failed.badcwd-marker-f5b'") + && message.contains("/nonexistent/coyote-badcwd-marker-f5b") + })); + let warns: Vec = crate::testing::warn_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!(warns.iter().all(|message| !message.contains("marker-f5b"))); + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn failing_hook_never_disturbs_the_engine() { + crate::testing::install_warn_collector(); + let guard = TestConfigDirGuard::new(); + let first = guard.path.join("first-out"); + let second = guard.path.join("second-out"); + let command = r#"echo err >&2; : > "$HOOK_OUT"; exit 1"#; + let ctx = ctx_with_global_hooks(hooks_map( + "tool.failed", + &[("stderr-exit1-marker", command)], + )); + + fire( + HookEvent::ToolFailed, + &ctx, + &[("HOOK_OUT", first.display().to_string())], + None, + ); + wait_for("first hook output", || first.exists()).await; + + fire( + HookEvent::ToolFailed, + &ctx, + &[("HOOK_OUT", second.display().to_string())], + None, + ); + wait_for("second hook output", || second.exists()).await; + + let warns: Vec = crate::testing::warn_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!( + warns + .iter() + .all(|message| !message.contains("stderr-exit1-marker")) + ); + } + + #[tokio::test] + #[serial] + async fn payload_file_is_created_with_owner_only_mode() { + use std::os::unix::fs::PermissionsExt; + let guard = TestConfigDirGuard::new(); + let path = guard.path.join("payload.json"); + + write_payload_file(&path, r#"{"probe":true}"#) + .await + .unwrap(); + + let mode = std::fs::metadata(&path).unwrap().permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "payload file mode was {mode:o}"); + } + + #[tokio::test] + #[serial] + async fn payload_file_write_refuses_existing_paths() { + let guard = TestConfigDirGuard::new(); + + let plain = guard.path.join("existing.json"); + std::fs::write(&plain, "original").unwrap(); + assert!(write_payload_file(&plain, "{}").await.is_err()); + assert_eq!(std::fs::read_to_string(&plain).unwrap(), "original"); + + let target = guard.path.join("symlink-target"); + std::fs::write(&target, "original").unwrap(); + let link = guard.path.join("existing-link.json"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + assert!(write_payload_file(&link, "{}").await.is_err()); + assert_eq!(std::fs::read_to_string(&target).unwrap(), "original"); + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn payload_write_failure_still_runs_hook_without_payload_env() { + crate::testing::install_warn_collector(); + let guard = TestConfigDirGuard::new(); + let out = guard.path.join("env-out"); + let _payload_dir = PayloadDirOverrideGuard::new(guard.path.join("missing-payload-dir")); + let command = r#"env > "$HOOK_OUT.tmp" && mv "$HOOK_OUT.tmp" "$HOOK_OUT""#; + let ctx = ctx_with_global_hooks(hooks_map( + "tool.started", + &[("payload-fallback-marker", command)], + )); + + fire( + HookEvent::ToolStarted, + &ctx, + &[("HOOK_OUT", out.display().to_string())], + Some(r#"{"probe":true}"#.to_string()), + ); + + wait_for("hook env dump", || out.exists()).await; + let env_dump = std::fs::read_to_string(&out).unwrap(); + assert!( + env_dump + .lines() + .all(|line| !line.starts_with("COYOTE_HOOK_PAYLOAD_FILE=")) + ); + let debugs: Vec = crate::testing::debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone(); + assert!(debugs.iter().any(|message| message.contains( + "Failed to write payload file for hook 'tool.started.payload-fallback-marker'" + ))); + } + + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn payload_paths_differ_across_dispatches() { + let guard = TestConfigDirGuard::new(); + let first = guard.path.join("first-path"); + let second = guard.path.join("second-path"); + let command = r#"printf '%s' "$COYOTE_HOOK_PAYLOAD_FILE" > "$HOOK_PATH.tmp" && mv "$HOOK_PATH.tmp" "$HOOK_PATH""#; + let ctx = ctx_with_global_hooks(hooks_map("tool.started", &[("nonce-probe", command)])); + + fire( + HookEvent::ToolStarted, + &ctx, + &[("HOOK_PATH", first.display().to_string())], + Some("{}".to_string()), + ); + wait_for("first payload path", || first.exists()).await; + + fire( + HookEvent::ToolStarted, + &ctx, + &[("HOOK_PATH", second.display().to_string())], + Some("{}".to_string()), + ); + wait_for("second payload path", || second.exists()).await; + + let first_path = std::fs::read_to_string(&first).unwrap(); + let second_path = std::fs::read_to_string(&second).unwrap(); + assert!(!first_path.is_empty()); + assert_ne!(first_path, second_path); + } + + #[tokio::test] + #[serial] + async fn resolved_hooks_gates_globals_and_adds_agent_hooks_through_context() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = + ctx_with_global_hooks(hooks_map("tool.started", &[("notify", "global-cmd")])); + let app = ctx.app.config.clone(); + let agent_name = "hooks-gate-probe"; + let agent_dir = paths::agent_data_dir(agent_name); + create_dir_all(&agent_dir).unwrap(); + std::fs::write( + agent_dir.join("config.yaml"), + format!( + "name: {agent_name}\ninstructions: hi\nhooks:\n tool.started:\n - name: own\n command: agent-cmd\nglobal_hooks: []\n" + ), + ) + .unwrap(); + + ctx.use_agent(&app, agent_name, None, crate::utils::create_abort_signal()) + .await + .unwrap(); + + let resolved = ctx.resolved_hooks(HookEvent::ToolStarted); + assert_eq!(names(&resolved), ["own"]); + assert_eq!(resolved[0].command, "agent-cmd"); + assert_eq!(resolved[0].cwd, paths::agent_data_dir(agent_name)); + } + } +} diff --git a/src/testing.rs b/src/testing.rs index 6ff02ea..a41d69c 100644 --- a/src/testing.rs +++ b/src/testing.rs @@ -3,27 +3,64 @@ use log::{Level, LevelFilter, Log, Metadata, Record}; use std::sync::{Mutex, Once, OnceLock}; -struct WarnCollector; +struct TestLogCollector; static WARN_MESSAGES: OnceLock>> = OnceLock::new(); +static DEBUG_MESSAGES: OnceLock>> = OnceLock::new(); -/// Every warn-level message captured since the collector was installed. The -/// buffer is process-global and shared by all tests, so assert by filtering -/// for a marker unique to the test rather than on the whole buffer. +/// Debug capture is restricted to the hooks module so the buffer is not +/// flooded by debug output from the rest of the crate. Matching is +/// module-boundary-aware: only this exact module or its `::` submodules +/// qualify, so a sibling like `hooks_registry` would not. +const DEBUG_TARGET_PREFIX: &str = concat!(env!("CARGO_CRATE_NAME"), "::hooks"); + +fn captures_warn(metadata: &Metadata) -> bool { + metadata.level() <= Level::Warn +} + +fn captures_hooks_debug(metadata: &Metadata) -> bool { + if metadata.level() != Level::Debug { + return false; + } + let target = metadata.target(); + target == DEBUG_TARGET_PREFIX + || target + .strip_prefix(DEBUG_TARGET_PREFIX) + .is_some_and(|rest| rest.starts_with("::")) +} + +/// Every warn- or error-level message captured since the collector was +/// installed. The buffer is process-global and shared by all tests, so assert +/// by filtering for a marker unique to the test rather than on the whole +/// buffer. pub(crate) fn warn_messages() -> &'static Mutex> { WARN_MESSAGES.get_or_init(Mutex::default) } -impl Log for WarnCollector { +/// Every debug-level message from the hooks module captured since the +/// collector was installed. Same marker-filtering discipline as +/// [`warn_messages`]. +pub(crate) fn debug_messages() -> &'static Mutex> { + DEBUG_MESSAGES.get_or_init(Mutex::default) +} + +impl Log for TestLogCollector { fn enabled(&self, metadata: &Metadata) -> bool { - metadata.level() <= Level::Warn + captures_warn(metadata) || captures_hooks_debug(metadata) } + // A test that panics while holding a buffer poisons its mutex; the logger + // recovers via `into_inner` so every later test can still log. fn log(&self, record: &Record) { - if self.enabled(record.metadata()) { + if captures_warn(record.metadata()) { warn_messages() .lock() - .unwrap() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .push(record.args().to_string()); + } else if captures_hooks_debug(record.metadata()) { + debug_messages() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) .push(record.args().to_string()); } } @@ -31,13 +68,15 @@ impl Log for WarnCollector { fn flush(&self) {} } -/// Installs the process-wide warn collector. `log::set_logger` accepts one -/// logger per process, so every test that captures warns must install through -/// this shared entry point. +/// Installs the process-wide log collector. `log::set_logger` accepts one +/// logger per process, so every test that captures warns or hooks debug +/// output must install through this shared entry point. The max level is +/// Debug so the hooks debug capture sees its records; warn capture is +/// unaffected. pub(crate) fn install_warn_collector() { static INSTALL: Once = Once::new(); INSTALL.call_once(|| { - log::set_logger(&WarnCollector).expect("no other logger should be installed"); - log::set_max_level(LevelFilter::Warn); + log::set_logger(&TestLogCollector).expect("no other logger should be installed"); + log::set_max_level(LevelFilter::Debug); }); }