From b5e1d1fa74d613ba0a9646e2a4a93595012b1564 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 17 Mar 2026 14:08:02 -0400 Subject: [PATCH] Extract fabro-hooks crate from fabro-workflows Move the self-contained hooks module (~2900 LOC) into its own crate to clarify the dependency graph and make the hook system independently reusable. The set_node convenience method is inlined at its two call sites in parallel.rs since it depends on fabro-graphviz types. Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 21 +++ lib/crates/fabro-api/Cargo.toml | 1 + lib/crates/fabro-api/src/server.rs | 8 +- .../fabro-api/tests/openapi_conformance.rs | 2 +- lib/crates/fabro-config/Cargo.toml | 1 + lib/crates/fabro-config/src/server.rs | 4 +- lib/crates/fabro-hooks/Cargo.toml | 26 +++ .../src/hook => fabro-hooks/src}/bridge.rs | 10 +- .../src/hook => fabro-hooks/src}/config.rs | 2 +- .../src/hook => fabro-hooks/src}/executor.rs | 8 +- .../hook/mod.rs => fabro-hooks/src/lib.rs} | 0 .../src/hook => fabro-hooks/src}/runner.rs | 10 +- .../src/hook => fabro-hooks/src}/types.rs | 7 - lib/crates/fabro-workflows/Cargo.toml | 1 + .../fabro-workflows/src/cli/project_config.rs | 2 +- lib/crates/fabro-workflows/src/cli/run.rs | 4 +- .../fabro-workflows/src/cli/run_config.rs | 26 +-- lib/crates/fabro-workflows/src/engine.rs | 2 +- .../fabro-workflows/src/handler/agent.rs | 2 +- lib/crates/fabro-workflows/src/handler/mod.rs | 2 +- .../fabro-workflows/src/handler/parallel.rs | 10 +- lib/crates/fabro-workflows/src/lib.rs | 1 - .../fabro-workflows/tests/integration.rs | 153 +++++++----------- 23 files changed, 158 insertions(+), 145 deletions(-) create mode 100644 lib/crates/fabro-hooks/Cargo.toml rename lib/crates/{fabro-workflows/src/hook => fabro-hooks/src}/bridge.rs (97%) rename lib/crates/{fabro-workflows/src/hook => fabro-hooks/src}/config.rs (99%) rename lib/crates/{fabro-workflows/src/hook => fabro-hooks/src}/executor.rs (99%) rename lib/crates/{fabro-workflows/src/hook/mod.rs => fabro-hooks/src/lib.rs} (100%) rename lib/crates/{fabro-workflows/src/hook => fabro-hooks/src}/runner.rs (98%) rename lib/crates/{fabro-workflows/src/hook => fabro-hooks/src}/types.rs (97%) diff --git a/Cargo.lock b/Cargo.lock index 24e66986f..937e9d42e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1286,6 +1286,7 @@ dependencies = [ "fabro-exe", "fabro-github", "fabro-graphviz", + "fabro-hooks", "fabro-interview", "fabro-llm", "fabro-retro", @@ -1391,6 +1392,7 @@ dependencies = [ "anyhow", "dirs", "fabro-agent", + "fabro-hooks", "fabro-mcp", "fabro-util", "fabro-workflows", @@ -1505,6 +1507,24 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "fabro-hooks" +version = "0.174.0" +dependencies = [ + "async-trait", + "fabro-agent", + "fabro-llm", + "mockito", + "regex", + "reqwest 0.12.28", + "serde", + "serde_json", + "tokio", + "tokio-util", + "toml", + "tracing", +] + [[package]] name = "fabro-interview" version = "0.174.0" @@ -1745,6 +1765,7 @@ dependencies = [ "fabro-git-storage", "fabro-github", "fabro-graphviz", + "fabro-hooks", "fabro-interview", "fabro-llm", "fabro-mcp", diff --git a/lib/crates/fabro-api/Cargo.toml b/lib/crates/fabro-api/Cargo.toml index 3c3a54478..4e361d46f 100644 --- a/lib/crates/fabro-api/Cargo.toml +++ b/lib/crates/fabro-api/Cargo.toml @@ -11,6 +11,7 @@ doctest = false [dependencies] fabro-config = { path = "../fabro-config" } fabro-graphviz = { path = "../fabro-graphviz" } +fabro-hooks = { path = "../fabro-hooks" } fabro-interview = { path = "../fabro-interview" } fabro-workflows = { path = "../fabro-workflows", features = ["exedev"] } fabro-daytona = { path = "../fabro-daytona" } diff --git a/lib/crates/fabro-api/src/server.rs b/lib/crates/fabro-api/src/server.rs index fca334a7c..e1e13d393 100644 --- a/lib/crates/fabro-api/src/server.rs +++ b/lib/crates/fabro-api/src/server.rs @@ -104,7 +104,7 @@ pub struct AppState { pub db: sqlx::SqlitePool, max_concurrent_runs: usize, scheduler_notify: tokio::sync::Notify, - pub hooks: Vec, + pub hooks: Vec, git_author: fabro_workflows::git::GitAuthor, pub sessions: crate::sessions::SessionStore, llm_client: tokio::sync::OnceCell, @@ -404,7 +404,7 @@ pub fn create_app_state_with_options( dry_run: bool, max_concurrent_runs: usize, git_author: fabro_workflows::git::GitAuthor, - hooks: Vec, + hooks: Vec, ) -> Arc { Arc::new(AppState { runs: Mutex::new(HashMap::new()), @@ -586,10 +586,10 @@ async fn execute_run(state: Arc, run_id: String) { // Wire up hook runner from server config if !state.hooks.is_empty() { - let hook_config = fabro_workflows::hook::HookConfig { + let hook_config = fabro_hooks::HookConfig { hooks: state.hooks.clone(), }; - let runner = fabro_workflows::hook::HookRunner::new(hook_config); + let runner = fabro_hooks::HookRunner::new(hook_config); engine.set_hook_runner(std::sync::Arc::new(runner)); } diff --git a/lib/crates/fabro-api/tests/openapi_conformance.rs b/lib/crates/fabro-api/tests/openapi_conformance.rs index 0c132f13b..5d8b41881 100644 --- a/lib/crates/fabro-api/tests/openapi_conformance.rs +++ b/lib/crates/fabro-api/tests/openapi_conformance.rs @@ -9,12 +9,12 @@ use fabro_api::jwt_auth::AuthMode; use fabro_api::server::{build_router, create_app_state}; use fabro_api::server_config::*; use fabro_daytona::*; +use fabro_hooks::*; use fabro_interview::Interviewer; use fabro_workflows::cli::run_config::*; use fabro_workflows::handler::exit::ExitHandler; use fabro_workflows::handler::start::StartHandler; use fabro_workflows::handler::HandlerRegistry; -use fabro_workflows::hook::*; use tower::ServiceExt; fn test_registry(_interviewer: Arc) -> HandlerRegistry { diff --git a/lib/crates/fabro-config/Cargo.toml b/lib/crates/fabro-config/Cargo.toml index 3debef3f4..afb29a3c3 100644 --- a/lib/crates/fabro-config/Cargo.toml +++ b/lib/crates/fabro-config/Cargo.toml @@ -15,6 +15,7 @@ exedev = ["fabro-workflows/exedev"] [dependencies] anyhow.workspace = true fabro-agent = { path = "../fabro-agent" } +fabro-hooks = { path = "../fabro-hooks" } fabro-mcp = { path = "../fabro-mcp" } fabro-workflows = { path = "../fabro-workflows" } fabro-util = { path = "../fabro-util" } diff --git a/lib/crates/fabro-config/src/server.rs b/lib/crates/fabro-config/src/server.rs index 717fb6d46..41cf77902 100644 --- a/lib/crates/fabro-config/src/server.rs +++ b/lib/crates/fabro-config/src/server.rs @@ -465,7 +465,7 @@ matcher = "agent_loop" assert_eq!(config.run_defaults.hooks.len(), 2); assert_eq!( config.run_defaults.hooks[0].event, - fabro_workflows::hook::HookEvent::RunStart + fabro_hooks::HookEvent::RunStart ); assert_eq!( config.run_defaults.hooks[0].command.as_deref(), @@ -473,7 +473,7 @@ matcher = "agent_loop" ); assert_eq!( config.run_defaults.hooks[1].event, - fabro_workflows::hook::HookEvent::StageComplete + fabro_hooks::HookEvent::StageComplete ); assert_eq!( config.run_defaults.hooks[1].matcher.as_deref(), diff --git a/lib/crates/fabro-hooks/Cargo.toml b/lib/crates/fabro-hooks/Cargo.toml new file mode 100644 index 000000000..fc38f9613 --- /dev/null +++ b/lib/crates/fabro-hooks/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "fabro-hooks" +edition.workspace = true +version.workspace = true +license.workspace = true +description = "User-defined lifecycle hooks for Fabro workflows" + +[lib] +doctest = false + +[dependencies] +fabro-agent = { path = "../fabro-agent" } +fabro-llm = { path = "../fabro-llm" } +serde.workspace = true +serde_json.workspace = true +tokio.workspace = true +async-trait.workspace = true +regex.workspace = true +reqwest.workspace = true +tracing.workspace = true +tokio-util.workspace = true + +[dev-dependencies] +mockito = "1" +tokio = { workspace = true, features = ["test-util", "macros"] } +toml.workspace = true diff --git a/lib/crates/fabro-workflows/src/hook/bridge.rs b/lib/crates/fabro-hooks/src/bridge.rs similarity index 97% rename from lib/crates/fabro-workflows/src/hook/bridge.rs rename to lib/crates/fabro-hooks/src/bridge.rs index ff25ca9e7..a0f0526a6 100644 --- a/lib/crates/fabro-workflows/src/hook/bridge.rs +++ b/lib/crates/fabro-hooks/src/bridge.rs @@ -3,8 +3,8 @@ use std::sync::Arc; use fabro_agent::{Sandbox, ToolHookCallback, ToolHookDecision}; -use super::runner::HookRunner; -use super::types::{HookContext, HookDecision, HookEvent}; +use crate::runner::HookRunner; +use crate::types::{HookContext, HookDecision, HookEvent}; /// Bridge between the workflow hook system and the agent tool-hook callback. /// @@ -72,9 +72,9 @@ impl ToolHookCallback for WorkflowToolHookCallback { #[cfg(test)] mod tests { use super::*; - use crate::hook::config::{HookConfig, HookDefinition}; - use crate::hook::executor::HookExecutor; - use crate::hook::types::{HookContext, HookResult}; + use crate::config::{HookConfig, HookDefinition}; + use crate::executor::HookExecutor; + use crate::types::{HookContext, HookResult}; use std::path::Path; use std::sync::Mutex; diff --git a/lib/crates/fabro-workflows/src/hook/config.rs b/lib/crates/fabro-hooks/src/config.rs similarity index 99% rename from lib/crates/fabro-workflows/src/hook/config.rs rename to lib/crates/fabro-hooks/src/config.rs index f2f2de546..523bb44f7 100644 --- a/lib/crates/fabro-workflows/src/hook/config.rs +++ b/lib/crates/fabro-hooks/src/config.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use serde::{Deserialize, Serialize}; -use super::types::HookEvent; +use crate::types::HookEvent; /// TLS verification mode for HTTP hooks. #[derive(Debug, Clone, Copy, Deserialize, PartialEq, Eq, Default, Serialize)] diff --git a/lib/crates/fabro-workflows/src/hook/executor.rs b/lib/crates/fabro-hooks/src/executor.rs similarity index 99% rename from lib/crates/fabro-workflows/src/hook/executor.rs rename to lib/crates/fabro-hooks/src/executor.rs index 2910aec08..64beac36d 100644 --- a/lib/crates/fabro-workflows/src/hook/executor.rs +++ b/lib/crates/fabro-hooks/src/executor.rs @@ -8,8 +8,8 @@ use async_trait::async_trait; use fabro_agent::Sandbox; -use super::config::{HookDefinition, HookType, TlsMode}; -use super::types::{HookContext, HookDecision, HookResult, PromptHookResponse}; +use crate::config::{HookDefinition, HookType, TlsMode}; +use crate::types::{HookContext, HookDecision, HookResult, PromptHookResponse}; const HOOK_EVALUATOR_SYSTEM_PROMPT: &str = "You are a hook evaluator for a workflow engine. Given context about a workflow event, evaluate the condition."; @@ -599,8 +599,8 @@ impl HookExecutor for HookExecutorImpl { #[cfg(test)] mod tests { use super::*; - use crate::hook::config::HookType; - use crate::hook::types::HookEvent; + use crate::config::HookType; + use crate::types::HookEvent; fn make_context() -> HookContext { HookContext::new(HookEvent::StageStart, "run-1".into(), "test-wf".into()) diff --git a/lib/crates/fabro-workflows/src/hook/mod.rs b/lib/crates/fabro-hooks/src/lib.rs similarity index 100% rename from lib/crates/fabro-workflows/src/hook/mod.rs rename to lib/crates/fabro-hooks/src/lib.rs diff --git a/lib/crates/fabro-workflows/src/hook/runner.rs b/lib/crates/fabro-hooks/src/runner.rs similarity index 98% rename from lib/crates/fabro-workflows/src/hook/runner.rs rename to lib/crates/fabro-hooks/src/runner.rs index 945bbc659..b52b42b1b 100644 --- a/lib/crates/fabro-workflows/src/hook/runner.rs +++ b/lib/crates/fabro-hooks/src/runner.rs @@ -4,9 +4,9 @@ use std::sync::Arc; use fabro_agent::Sandbox; -use super::config::{HookConfig, HookDefinition}; -use super::executor::{HookExecutor, HookExecutorImpl}; -use super::types::{HookContext, HookDecision}; +use crate::config::{HookConfig, HookDefinition}; +use crate::executor::{HookExecutor, HookExecutorImpl}; +use crate::types::{HookContext, HookDecision}; /// Central orchestrator: filters matching hooks, executes them, merges decisions. pub struct HookRunner { @@ -210,8 +210,8 @@ impl HookRunner { #[cfg(test)] mod tests { use super::*; - use crate::hook::config::HookConfig; - use crate::hook::types::{HookContext, HookEvent, HookResult}; + use crate::config::HookConfig; + use crate::types::{HookContext, HookEvent, HookResult}; struct MockExecutor { decision: HookDecision, diff --git a/lib/crates/fabro-workflows/src/hook/types.rs b/lib/crates/fabro-hooks/src/types.rs similarity index 97% rename from lib/crates/fabro-workflows/src/hook/types.rs rename to lib/crates/fabro-hooks/src/types.rs index 09baa287a..b20aeaab0 100644 --- a/lib/crates/fabro-workflows/src/hook/types.rs +++ b/lib/crates/fabro-hooks/src/types.rs @@ -103,13 +103,6 @@ pub struct HookContext { } impl HookContext { - /// Populate node-related fields from a graph `Node`. - pub fn set_node(&mut self, node: &fabro_graphviz::graph::Node) { - self.node_id = Some(node.id.clone()); - self.node_label = Some(node.label().to_string()); - self.handler_type = node.handler_type().map(String::from); - } - #[must_use] pub fn new(event: HookEvent, run_id: String, workflow_name: String) -> Self { Self { diff --git a/lib/crates/fabro-workflows/Cargo.toml b/lib/crates/fabro-workflows/Cargo.toml index 69add2c90..4f5f2cc13 100644 --- a/lib/crates/fabro-workflows/Cargo.toml +++ b/lib/crates/fabro-workflows/Cargo.toml @@ -22,6 +22,7 @@ anyhow.workspace = true dotenvy.workspace = true fabro-agent = { path = "../fabro-agent" } fabro-graphviz = { path = "../fabro-graphviz" } +fabro-hooks = { path = "../fabro-hooks" } fabro-validate = { path = "../fabro-validate" } fabro-devcontainer = { path = "../fabro-devcontainer" } fabro-exe = { path = "../fabro-exe", optional = true } diff --git a/lib/crates/fabro-workflows/src/cli/project_config.rs b/lib/crates/fabro-workflows/src/cli/project_config.rs index 91a678ce5..31b960631 100644 --- a/lib/crates/fabro-workflows/src/cli/project_config.rs +++ b/lib/crates/fabro-workflows/src/cli/project_config.rs @@ -8,7 +8,7 @@ use super::run_config::{ AssetsConfig, CheckpointConfig, GitHubConfig, LlmConfig, McpServerEntry, PullRequestConfig, RunDefaults, SandboxConfig, SetupConfig, }; -use crate::hook::HookDefinition; +use fabro_hooks::HookDefinition; const CONFIG_FILENAME: &str = "fabro.toml"; diff --git a/lib/crates/fabro-workflows/src/cli/run.rs b/lib/crates/fabro-workflows/src/cli/run.rs index ee7ba4229..8fcfaf348 100644 --- a/lib/crates/fabro-workflows/src/cli/run.rs +++ b/lib/crates/fabro-workflows/src/cli/run.rs @@ -1103,10 +1103,10 @@ pub async fn run_command( .map(|c| &c.hooks) .unwrap_or(&run_defaults.hooks); if !hooks.is_empty() { - let hook_config = crate::hook::HookConfig { + let hook_config = fabro_hooks::HookConfig { hooks: hooks.clone(), }; - let runner = crate::hook::HookRunner::new(hook_config); + let runner = fabro_hooks::HookRunner::new(hook_config); engine.set_hook_runner(Arc::new(runner)); } } diff --git a/lib/crates/fabro-workflows/src/cli/run_config.rs b/lib/crates/fabro-workflows/src/cli/run_config.rs index 7547545de..07d739fe8 100644 --- a/lib/crates/fabro-workflows/src/cli/run_config.rs +++ b/lib/crates/fabro-workflows/src/cli/run_config.rs @@ -73,7 +73,7 @@ pub struct WorkflowRunConfig { pub sandbox: Option, pub vars: Option>, #[serde(default)] - pub hooks: Vec, + pub hooks: Vec, #[serde(default)] pub checkpoint: CheckpointConfig, pub pull_request: Option, @@ -164,7 +164,7 @@ pub struct RunDefaults { pub pull_request: Option, pub assets: Option, #[serde(default)] - pub hooks: Vec, + pub hooks: Vec, #[serde(default)] pub mcp_servers: HashMap, pub github: Option, @@ -291,10 +291,10 @@ impl WorkflowRunConfig { // Merge hooks: defaults as base, workflow overrides by name if !defaults.hooks.is_empty() { - let base = crate::hook::HookConfig { + let base = fabro_hooks::HookConfig { hooks: defaults.hooks.clone(), }; - let overlay = crate::hook::HookConfig { + let overlay = fabro_hooks::HookConfig { hooks: std::mem::take(&mut self.hooks), }; self.hooks = base.merge(overlay).hooks; @@ -427,10 +427,10 @@ impl RunDefaults { } if !overlay.hooks.is_empty() { - let base = crate::hook::HookConfig { + let base = fabro_hooks::HookConfig { hooks: std::mem::take(&mut self.hooks), }; - let over = crate::hook::HookConfig { + let over = fabro_hooks::HookConfig { hooks: overlay.hooks, }; self.hooks = base.merge(over).hooks; @@ -1711,14 +1711,14 @@ command = "echo done" "#; let cfg: WorkflowRunConfig = toml::from_str(toml).unwrap(); assert_eq!(cfg.hooks.len(), 2); - assert_eq!(cfg.hooks[0].event, crate::hook::HookEvent::StageStart); + assert_eq!(cfg.hooks[0].event, fabro_hooks::HookEvent::StageStart); assert_eq!( cfg.hooks[0].command.as_deref(), Some("./scripts/pre-check.sh") ); assert_eq!(cfg.hooks[0].blocking, Some(true)); assert_eq!(cfg.hooks[0].sandbox, Some(false)); - assert_eq!(cfg.hooks[1].event, crate::hook::HookEvent::RunComplete); + assert_eq!(cfg.hooks[1].event, fabro_hooks::HookEvent::RunComplete); } #[test] @@ -2399,9 +2399,9 @@ command = "echo done" ) .unwrap(); let defaults = RunDefaults { - hooks: vec![crate::hook::HookDefinition { + hooks: vec![fabro_hooks::HookDefinition { name: Some("default-hook".into()), - event: crate::hook::HookEvent::RunStart, + event: fabro_hooks::HookEvent::RunStart, command: Some("echo start".into()), hook_type: None, matcher: None, @@ -2433,9 +2433,9 @@ command = "echo from-workflow" ) .unwrap(); let defaults = RunDefaults { - hooks: vec![crate::hook::HookDefinition { + hooks: vec![fabro_hooks::HookDefinition { name: Some("shared".into()), - event: crate::hook::HookEvent::RunStart, + event: fabro_hooks::HookEvent::RunStart, command: Some("echo from-default".into()), hook_type: None, matcher: None, @@ -2447,7 +2447,7 @@ command = "echo from-workflow" }; cfg.apply_defaults(&defaults); assert_eq!(cfg.hooks.len(), 1); - assert_eq!(cfg.hooks[0].event, crate::hook::HookEvent::RunComplete); + assert_eq!(cfg.hooks[0].event, fabro_hooks::HookEvent::RunComplete); } #[test] diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs index 901f68423..0d8506bec 100644 --- a/lib/crates/fabro-workflows/src/engine.rs +++ b/lib/crates/fabro-workflows/src/engine.rs @@ -23,11 +23,11 @@ use crate::context::Context; use crate::error::{FabroError, FailureClass, FailureSignature, Result}; use crate::event::{EventEmitter, WorkflowRunEvent}; use crate::handler::{EngineServices, HandlerRegistry}; -use crate::hook::{HookContext, HookDecision, HookEvent, HookRunner}; use crate::millis_u64; use crate::outcome::{Outcome, StageStatus}; use crate::preamble::build_preamble; use fabro_graphviz::graph::{Edge, Graph, Node}; +use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner}; use fabro_interview::Interviewer; /// Classify the failure mode of a completed outcome. diff --git a/lib/crates/fabro-workflows/src/handler/agent.rs b/lib/crates/fabro-workflows/src/handler/agent.rs index 71afe66e5..b0648306d 100644 --- a/lib/crates/fabro-workflows/src/handler/agent.rs +++ b/lib/crates/fabro-workflows/src/handler/agent.rs @@ -259,7 +259,7 @@ impl Handler for AgentHandler { let thread_id = context.thread_id(); let tool_hooks: Option> = services.hook_runner.as_ref().map(|hr| { - Arc::new(crate::hook::bridge::WorkflowToolHookCallback { + Arc::new(fabro_hooks::WorkflowToolHookCallback { hook_runner: Arc::clone(hr), sandbox: Arc::clone(&services.sandbox), run_id: context.run_id(), diff --git a/lib/crates/fabro-workflows/src/handler/mod.rs b/lib/crates/fabro-workflows/src/handler/mod.rs index 5d05b7b3d..7f1b503dd 100644 --- a/lib/crates/fabro-workflows/src/handler/mod.rs +++ b/lib/crates/fabro-workflows/src/handler/mod.rs @@ -21,9 +21,9 @@ use crate::context::Context; use crate::engine::GitState; use crate::error::FabroError; use crate::event::EventEmitter; -use crate::hook::{HookContext, HookDecision, HookRunner}; use crate::outcome::Outcome; use fabro_graphviz::graph::{shape_to_handler_type, Graph, Node}; +use fabro_hooks::{HookContext, HookDecision, HookRunner}; use fabro_interview::Interviewer; /// Shared services available to all handlers during execution. diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs index a40b6f4b6..741934172 100644 --- a/lib/crates/fabro-workflows/src/handler/parallel.rs +++ b/lib/crates/fabro-workflows/src/handler/parallel.rs @@ -10,11 +10,11 @@ use crate::context::keys; use crate::context::Context; use crate::error::FabroError; use crate::event::WorkflowRunEvent; -use crate::hook::{HookContext, HookEvent}; use crate::millis_u64; use crate::outcome::{Outcome, StageStatus}; use fabro_agent::LocalSandbox; use fabro_graphviz::graph::{Graph, Node}; +use fabro_hooks::{HookContext, HookEvent}; use super::{EngineServices, Handler}; @@ -306,7 +306,9 @@ impl Handler for ParallelHandler { context.run_id(), graph.name.clone(), ); - hook_ctx.set_node(node); + hook_ctx.node_id = Some(node.id.clone()); + hook_ctx.node_label = Some(node.label().to_string()); + hook_ctx.handler_type = node.handler_type().map(String::from); let _ = services.run_hooks(&hook_ctx).await; } let max_parallel = node @@ -727,7 +729,9 @@ impl Handler for ParallelHandler { context.run_id(), graph.name.clone(), ); - hook_ctx.set_node(node); + hook_ctx.node_id = Some(node.id.clone()); + hook_ctx.node_label = Some(node.label().to_string()); + hook_ctx.handler_type = node.handler_type().map(String::from); let _ = services.run_hooks(&hook_ctx).await; } diff --git a/lib/crates/fabro-workflows/src/lib.rs b/lib/crates/fabro-workflows/src/lib.rs index af924ce2c..1af969ff8 100644 --- a/lib/crates/fabro-workflows/src/lib.rs +++ b/lib/crates/fabro-workflows/src/lib.rs @@ -106,7 +106,6 @@ pub mod error; pub mod event; pub mod git; pub mod handler; -pub mod hook; pub mod manifest; pub mod outcome; pub mod preamble; diff --git a/lib/crates/fabro-workflows/tests/integration.rs b/lib/crates/fabro-workflows/tests/integration.rs index 894997e94..44a8b0390 100644 --- a/lib/crates/fabro-workflows/tests/integration.rs +++ b/lib/crates/fabro-workflows/tests/integration.rs @@ -7404,14 +7404,14 @@ fn subgraph_without_label_no_class_derived() { // --------------------------------------------------------------------------- /// Helper: create a WorkflowRunEngine with hooks configured from HookDefinitions. -fn engine_with_hooks(hooks: Vec) -> WorkflowRunEngine { +fn engine_with_hooks(hooks: Vec) -> WorkflowRunEngine { let registry = make_linear_registry(); let emitter = Arc::new(EventEmitter::new()); let sandbox = local_env(); let mut engine = WorkflowRunEngine::new(registry, emitter, sandbox); if !hooks.is_empty() { - let config = fabro_workflows::hook::HookConfig { hooks }; - let runner = fabro_workflows::hook::HookRunner::new(config); + let config = fabro_hooks::HookConfig { hooks }; + let runner = fabro_hooks::HookRunner::new(config); engine.set_hook_runner(Arc::new(runner)); } engine @@ -7419,7 +7419,7 @@ fn engine_with_hooks(hooks: Vec) -> Workf /// Helper: create a WorkflowRunEngine with hooks and event capture. fn engine_with_hooks_and_events( - hooks: Vec, + hooks: Vec, ) -> ( WorkflowRunEngine, Arc>>, @@ -7430,8 +7430,8 @@ fn engine_with_hooks_and_events( let sandbox = local_env(); let mut engine = WorkflowRunEngine::new(registry, Arc::new(emitter), sandbox); if !hooks.is_empty() { - let config = fabro_workflows::hook::HookConfig { hooks }; - let runner = fabro_workflows::hook::HookRunner::new(config); + let config = fabro_hooks::HookConfig { hooks }; + let runner = fabro_hooks::HookRunner::new(config); engine.set_hook_runner(Arc::new(runner)); } (engine, events) @@ -7459,11 +7459,8 @@ fn make_run_config(dir: &std::path::Path) -> RunConfig { } } -fn make_hook( - event: fabro_workflows::hook::HookEvent, - command: &str, -) -> fabro_workflows::hook::HookDefinition { - fabro_workflows::hook::HookDefinition { +fn make_hook(event: fabro_hooks::HookEvent, command: &str) -> fabro_hooks::HookDefinition { + fabro_hooks::HookDefinition { name: None, event, command: Some(command.into()), @@ -7516,10 +7513,7 @@ fn branching_dot() -> &'static str { #[tokio::test] async fn hook_run_start_proceed_allows_run() { - let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::RunStart, - "exit 0", - )]; + let hooks = vec![make_hook(fabro_hooks::HookEvent::RunStart, "exit 0")]; let engine = engine_with_hooks(hooks); let graph = parse(simple_linear_dot()).unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -7531,10 +7525,7 @@ async fn hook_run_start_proceed_allows_run() { #[tokio::test] async fn hook_run_start_block_prevents_run() { - let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::RunStart, - "exit 1", - )]; + let hooks = vec![make_hook(fabro_hooks::HookEvent::RunStart, "exit 1")]; let (engine, events) = engine_with_hooks_and_events(hooks); let graph = parse(simple_linear_dot()).unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -7570,7 +7561,7 @@ async fn hook_run_start_block_prevents_run() { async fn hook_run_start_block_with_json_reason() { // Hook that outputs JSON with a reason let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::RunStart, + fabro_hooks::HookEvent::RunStart, r#"echo '{"decision":"block","reason":"policy violation"}'; exit 2"#, )]; let engine = engine_with_hooks(hooks); @@ -7591,10 +7582,7 @@ async fn hook_run_start_block_with_json_reason() { #[tokio::test] async fn hook_stage_start_proceed_allows_execution() { - let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::StageStart, - "exit 0", - )]; + let hooks = vec![make_hook(fabro_hooks::HookEvent::StageStart, "exit 0")]; let engine = engine_with_hooks(hooks); let graph = parse(simple_linear_dot()).unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -7618,7 +7606,7 @@ async fn hook_stage_start_proceed_allows_execution() { async fn hook_stage_start_skip_bypasses_node() { // Hook that outputs skip decision as JSON let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::StageStart, + fabro_hooks::HookEvent::StageStart, r#"echo '{"decision":"skip","reason":"not needed"}'; exit 0"#, )]; let (engine, events) = engine_with_hooks_and_events(hooks); @@ -7657,10 +7645,7 @@ async fn hook_stage_start_skip_bypasses_node() { #[tokio::test] async fn hook_stage_start_block_aborts_run() { - let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::StageStart, - "exit 1", - )]; + let hooks = vec![make_hook(fabro_hooks::HookEvent::StageStart, "exit 1")]; let engine = engine_with_hooks(hooks); let graph = parse(simple_linear_dot()).unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -7674,7 +7659,7 @@ async fn hook_stage_start_block_aborts_run() { async fn hook_stage_start_matcher_filters_by_node_id() { // Hook that only matches nodes with "step2" in their ID let mut hook = make_hook( - fabro_workflows::hook::HookEvent::StageStart, + fabro_hooks::HookEvent::StageStart, r#"echo '{"decision":"skip","reason":"filtered"}'"#, ); hook.matcher = Some("step2".into()); @@ -7713,7 +7698,7 @@ async fn hook_stage_start_matcher_filters_by_node_id() { #[tokio::test] async fn hook_stage_start_matcher_no_match_proceeds() { // Hook with matcher that matches nothing - let mut hook = make_hook(fabro_workflows::hook::HookEvent::StageStart, "exit 1"); + let mut hook = make_hook(fabro_hooks::HookEvent::StageStart, "exit 1"); hook.matcher = Some("nonexistent_node".into()); let hooks = vec![hook]; @@ -7734,7 +7719,7 @@ async fn hook_stage_complete_fires_after_success() { let marker = dir.path().join("stage_complete_marker.txt"); let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::StageComplete, + fabro_hooks::HookEvent::StageComplete, &format!("echo $FABRO_NODE_ID >> {}", marker.display()), )]; let engine = engine_with_hooks(hooks); @@ -7764,10 +7749,7 @@ async fn hook_stage_complete_fires_after_success() { #[tokio::test] async fn hook_stage_complete_failure_does_not_block_pipeline() { // Non-blocking hook that fails should not affect the pipeline - let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::StageComplete, - "exit 1", - )]; + let hooks = vec![make_hook(fabro_hooks::HookEvent::StageComplete, "exit 1")]; let engine = engine_with_hooks(hooks); let graph = parse(simple_linear_dot()).unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -7789,7 +7771,7 @@ async fn hook_run_complete_fires_on_success() { let marker = dir.path().join("run_complete_marker.txt"); let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::RunComplete, + fabro_hooks::HookEvent::RunComplete, &format!("echo done > {}", marker.display()), )]; let engine = engine_with_hooks(hooks); @@ -7814,11 +7796,11 @@ async fn hook_run_complete_does_not_fire_on_blocked_run() { let hooks = vec![ make_hook( - fabro_workflows::hook::HookEvent::RunStart, + fabro_hooks::HookEvent::RunStart, "exit 1", // block the run ), make_hook( - fabro_workflows::hook::HookEvent::RunComplete, + fabro_hooks::HookEvent::RunComplete, &format!("echo done > {}", marker.display()), ), ]; @@ -7843,11 +7825,11 @@ async fn hook_run_failed_fires_on_stage_block() { let hooks = vec![ make_hook( - fabro_workflows::hook::HookEvent::StageStart, + fabro_hooks::HookEvent::StageStart, "exit 1", // block during stage ), make_hook( - fabro_workflows::hook::HookEvent::RunFailed, + fabro_hooks::HookEvent::RunFailed, &format!("echo failed > {}", marker.display()), ), ]; @@ -7870,7 +7852,7 @@ async fn hook_receives_env_vars() { let env_file = dir.path().join("hook_env.txt"); let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::StageComplete, + fabro_hooks::HookEvent::StageComplete, &format!( "echo \"event=$FABRO_EVENT run=$FABRO_RUN_ID wf=$FABRO_WORKFLOW node=$FABRO_NODE_ID\" >> {}", env_file.display() @@ -7917,11 +7899,11 @@ async fn multiple_hooks_same_event_all_fire() { let hooks = vec![ make_hook( - fabro_workflows::hook::HookEvent::StageComplete, + fabro_hooks::HookEvent::StageComplete, &format!("echo hook1 > {}", marker1.display()), ), make_hook( - fabro_workflows::hook::HookEvent::StageComplete, + fabro_hooks::HookEvent::StageComplete, &format!("echo hook2 > {}", marker2.display()), ), ]; @@ -7954,7 +7936,7 @@ async fn no_hooks_configured_runs_normally() { async fn hook_edge_selected_override_redirects_routing() { // Hook that overrides edge routing to pathB when it would go to pathA let mut hook = make_hook( - fabro_workflows::hook::HookEvent::EdgeSelected, + fabro_hooks::HookEvent::EdgeSelected, // Override routing to pathB r#"echo '{"decision":"override","edge_to":"pathB"}'"#, ); @@ -7987,7 +7969,7 @@ async fn hook_edge_selected_override_redirects_routing() { #[tokio::test] async fn hook_edge_selected_block_aborts_run() { - let mut hook = make_hook(fabro_workflows::hook::HookEvent::EdgeSelected, "exit 1"); + let mut hook = make_hook(fabro_hooks::HookEvent::EdgeSelected, "exit 1"); hook.matcher = Some("^plan$".into()); let hooks = vec![hook]; @@ -8008,7 +7990,7 @@ async fn hook_checkpoint_saved_fires() { let marker = dir.path().join("checkpoint_marker.txt"); let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::CheckpointSaved, + fabro_hooks::HookEvent::CheckpointSaved, &format!("echo $FABRO_NODE_ID >> {}", marker.display()), )]; let engine = engine_with_hooks(hooks); @@ -8031,10 +8013,7 @@ async fn hook_checkpoint_saved_fires() { #[tokio::test] async fn hook_stage_start_exit_2_blocks() { - let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::StageStart, - "exit 2", - )]; + let hooks = vec![make_hook(fabro_hooks::HookEvent::StageStart, "exit 2")]; let engine = engine_with_hooks(hooks); let graph = parse(simple_linear_dot()).unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -8049,7 +8028,7 @@ async fn hook_stage_start_exit_2_blocks() { #[tokio::test] async fn hook_config_merge_concatenates() { - use fabro_workflows::hook::{HookConfig, HookDefinition, HookEvent}; + use fabro_hooks::{HookConfig, HookDefinition, HookEvent}; let server_hooks = HookConfig { hooks: vec![HookDefinition { @@ -8084,7 +8063,7 @@ async fn hook_config_merge_concatenates() { #[tokio::test] async fn hook_config_merge_run_overrides_by_name() { - use fabro_workflows::hook::{HookConfig, HookDefinition, HookEvent}; + use fabro_hooks::{HookConfig, HookDefinition, HookEvent}; let server_hooks = HookConfig { hooks: vec![HookDefinition { @@ -8150,10 +8129,7 @@ command = "echo done" let cfg: fabro_workflows::cli::run_config::WorkflowRunConfig = toml::from_str(toml).unwrap(); assert_eq!(cfg.hooks.len(), 2); - assert_eq!( - cfg.hooks[0].event, - fabro_workflows::hook::HookEvent::StageStart - ); + assert_eq!(cfg.hooks[0].event, fabro_hooks::HookEvent::StageStart); assert_eq!(cfg.hooks[0].matcher.as_deref(), Some("agent_loop")); assert!(cfg.hooks[0].is_blocking()); assert!(!cfg.hooks[0].runs_in_sandbox()); @@ -8161,10 +8137,7 @@ command = "echo done" cfg.hooks[0].timeout(), std::time::Duration::from_millis(30000) ); - assert_eq!( - cfg.hooks[1].event, - fabro_workflows::hook::HookEvent::RunComplete - ); + assert_eq!(cfg.hooks[1].event, fabro_hooks::HookEvent::RunComplete); assert!(!cfg.hooks[1].is_blocking()); // RunComplete non-blocking by default } @@ -8173,7 +8146,7 @@ command = "echo done" #[tokio::test] async fn hook_blocking_override_makes_non_blocking_event_blocking() { // StageComplete is non-blocking by default, but force it to blocking - let mut hook = make_hook(fabro_workflows::hook::HookEvent::StageComplete, "exit 1"); + let mut hook = make_hook(fabro_hooks::HookEvent::StageComplete, "exit 1"); hook.blocking = Some(true); let hooks = vec![hook]; @@ -8195,7 +8168,7 @@ async fn hook_blocking_override_makes_non_blocking_event_blocking() { #[tokio::test] async fn hook_non_blocking_override_on_blocking_event() { // RunStart is blocking by default, but force it to non-blocking - let mut hook = make_hook(fabro_workflows::hook::HookEvent::RunStart, "exit 1"); + let mut hook = make_hook(fabro_hooks::HookEvent::RunStart, "exit 1"); hook.blocking = Some(false); let hooks = vec![hook]; @@ -8216,7 +8189,7 @@ async fn hook_non_blocking_override_on_blocking_event() { async fn hook_matcher_regex_pattern() { // Hook matches any node starting with "step" let mut hook = make_hook( - fabro_workflows::hook::HookEvent::StageStart, + fabro_hooks::HookEvent::StageStart, r#"echo '{"decision":"skip","reason":"regex match"}'"#, ); hook.matcher = Some("^step".into()); @@ -8255,7 +8228,7 @@ async fn hook_matcher_regex_pattern() { #[tokio::test] async fn hook_json_proceed_explicit() { let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::RunStart, + fabro_hooks::HookEvent::RunStart, r#"echo '{"decision":"proceed"}'"#, )]; let engine = engine_with_hooks(hooks); @@ -8270,7 +8243,7 @@ async fn hook_json_proceed_explicit() { #[tokio::test] async fn hook_json_block_with_reason() { let hooks = vec![make_hook( - fabro_workflows::hook::HookEvent::RunStart, + fabro_hooks::HookEvent::RunStart, r#"echo '{"decision":"block","reason":"forbidden by policy"}'; exit 2"#, )]; let engine = engine_with_hooks(hooks); @@ -8294,7 +8267,7 @@ async fn hook_sandbox_false_runs_on_host() { let marker = dir.path().join("host_hook.txt"); let mut hook = make_hook( - fabro_workflows::hook::HookEvent::RunComplete, + fabro_hooks::HookEvent::RunComplete, &format!("echo host > {}", marker.display()), ); hook.sandbox = Some(false); @@ -8338,13 +8311,10 @@ timeout_ms = 120000 assert_eq!(cfg.hooks.len(), 2); // Prompt hook - assert_eq!( - cfg.hooks[0].event, - fabro_workflows::hook::HookEvent::StageStart - ); + assert_eq!(cfg.hooks[0].event, fabro_hooks::HookEvent::StageStart); assert!(matches!( cfg.hooks[0].resolved_hook_type().as_deref(), - Some(fabro_workflows::hook::HookType::Prompt { prompt, model }) + Some(fabro_hooks::HookType::Prompt { prompt, model }) if prompt == "Should this stage proceed?" && *model == Some("haiku".into()) )); assert_eq!( @@ -8353,13 +8323,10 @@ timeout_ms = 120000 ); // Agent hook - assert_eq!( - cfg.hooks[1].event, - fabro_workflows::hook::HookEvent::RunComplete - ); + assert_eq!(cfg.hooks[1].event, fabro_hooks::HookEvent::RunComplete); assert!(matches!( cfg.hooks[1].resolved_hook_type().as_deref(), - Some(fabro_workflows::hook::HookType::Agent { prompt, model, max_tool_rounds }) + Some(fabro_hooks::HookType::Agent { prompt, model, max_tool_rounds }) if prompt == "Verify all tests pass." && *model == Some("sonnet".into()) && *max_tool_rounds == Some(10) @@ -8377,11 +8344,11 @@ timeout_ms = 120000 async fn hook_prompt_proceed_allows_run() { dotenvy::dotenv().ok(); - let hooks = vec![fabro_workflows::hook::HookDefinition { + let hooks = vec![fabro_hooks::HookDefinition { name: Some("prompt-proceed".into()), - event: fabro_workflows::hook::HookEvent::RunStart, + event: fabro_hooks::HookEvent::RunStart, command: None, - hook_type: Some(fabro_workflows::hook::HookType::Prompt { + hook_type: Some(fabro_hooks::HookType::Prompt { prompt: "A workflow is starting. Always approve. Respond with {\"ok\": true}.".into(), model: Some("haiku".into()), }), @@ -8405,11 +8372,11 @@ async fn hook_prompt_block_prevents_run() { dotenvy::dotenv().ok(); // Use a factual question that evaluates to false: "Is 2+2=5?" - let hooks = vec![fabro_workflows::hook::HookDefinition { + let hooks = vec![fabro_hooks::HookDefinition { name: Some("prompt-block".into()), - event: fabro_workflows::hook::HookEvent::RunStart, + event: fabro_hooks::HookEvent::RunStart, command: None, - hook_type: Some(fabro_workflows::hook::HookType::Prompt { + hook_type: Some(fabro_hooks::HookType::Prompt { prompt: "Check: is 2+2 equal to 5? If the statement is true, respond {\"ok\": true}. If false, respond {\"ok\": false, \"reason\": \"math check failed\"}.".into(), model: Some("haiku".into()), }), @@ -8435,11 +8402,11 @@ async fn hook_prompt_block_prevents_run() { async fn hook_agent_proceed_allows_run() { dotenvy::dotenv().ok(); - let hooks = vec![fabro_workflows::hook::HookDefinition { + let hooks = vec![fabro_hooks::HookDefinition { name: Some("agent-proceed".into()), - event: fabro_workflows::hook::HookEvent::RunStart, + event: fabro_hooks::HookEvent::RunStart, command: None, - hook_type: Some(fabro_workflows::hook::HookType::Agent { + hook_type: Some(fabro_hooks::HookType::Agent { prompt: "A workflow is starting. Always approve. Respond with {\"ok\": true}. Do not use any tools.".into(), model: Some("haiku".into()), max_tool_rounds: Some(1), @@ -8467,11 +8434,11 @@ async fn hook_agent_with_tool_use() { let marker = dir.path().join("hook_check.txt"); std::fs::write(&marker, "READY").unwrap(); - let hooks = vec![fabro_workflows::hook::HookDefinition { + let hooks = vec![fabro_hooks::HookDefinition { name: Some("agent-tools".into()), - event: fabro_workflows::hook::HookEvent::RunStart, + event: fabro_hooks::HookEvent::RunStart, command: None, - hook_type: Some(fabro_workflows::hook::HookType::Agent { + hook_type: Some(fabro_hooks::HookType::Agent { prompt: format!( "Read the file at {} using the read_file tool. If it contains 'READY', respond with {{\"ok\": true}}. Otherwise respond with {{\"ok\": false, \"reason\": \"not ready\"}}.", marker.display() @@ -8497,10 +8464,10 @@ async fn hook_agent_with_tool_use() { #[tokio::test] async fn hooks_do_not_duplicate_workflow_events() { let hooks = vec![ - make_hook(fabro_workflows::hook::HookEvent::RunStart, "exit 0"), - make_hook(fabro_workflows::hook::HookEvent::StageStart, "exit 0"), - make_hook(fabro_workflows::hook::HookEvent::StageComplete, "exit 0"), - make_hook(fabro_workflows::hook::HookEvent::RunComplete, "exit 0"), + make_hook(fabro_hooks::HookEvent::RunStart, "exit 0"), + make_hook(fabro_hooks::HookEvent::StageStart, "exit 0"), + make_hook(fabro_hooks::HookEvent::StageComplete, "exit 0"), + make_hook(fabro_hooks::HookEvent::RunComplete, "exit 0"), ]; let (engine, events) = engine_with_hooks_and_events(hooks); let graph = parse(simple_linear_dot()).unwrap();