From 9e079ed2c5560db5ce839ccf48db9e266f5aa79b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 17 Mar 2026 14:12:26 -0400 Subject: [PATCH] Clean up fabro-hooks extraction Extract set_hook_node() helper to deduplicate the 5 call sites that populate node fields on HookContext. The helper lives in fabro-workflows (which has the fabro-graphviz dependency) rather than fabro-hooks. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-workflows/src/engine.rs | 19 ++++++++++--------- .../fabro-workflows/src/handler/parallel.rs | 9 +++------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs index 0d8506bec..5341be5cf 100644 --- a/lib/crates/fabro-workflows/src/engine.rs +++ b/lib/crates/fabro-workflows/src/engine.rs @@ -30,6 +30,13 @@ use fabro_graphviz::graph::{Edge, Graph, Node}; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner}; use fabro_interview::Interviewer; +/// Populate node-related fields on a `HookContext` from a graph `Node`. +pub(crate) fn set_hook_node(ctx: &mut HookContext, node: &Node) { + ctx.node_id = Some(node.id.clone()); + ctx.node_label = Some(node.label().to_string()); + ctx.handler_type = node.handler_type().map(String::from); +} + /// Classify the failure mode of a completed outcome. /// /// Returns `None` for `Success`, `PartialSuccess`, and `Skipped` outcomes. @@ -1857,9 +1864,7 @@ impl WorkflowRunEngine { let mut hook_ctx = HookContext::new(HookEvent::StageStart, run_id.clone(), graph.name.clone()); hook_ctx.cwd = hook_work_dir.as_ref().map(|p| p.display().to_string()); - 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); + set_hook_node(&mut hook_ctx, node); hook_ctx.attempt = Some(1); hook_ctx.max_attempts = Some(usize::try_from(retry_policy.max_attempts).unwrap_or(usize::MAX)); @@ -1995,9 +2000,7 @@ impl WorkflowRunEngine { run_id.clone(), graph.name.clone(), ); - 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); + set_hook_node(&mut hook_ctx, node); hook_ctx.status = Some("fail".into()); hook_ctx.failure_reason = outcome.failure_reason().map(String::from); let _ = self.run_hooks(&hook_ctx, hook_work_dir.as_deref()).await; @@ -2029,9 +2032,7 @@ impl WorkflowRunEngine { run_id.clone(), graph.name.clone(), ); - 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); + set_hook_node(&mut hook_ctx, node); hook_ctx.status = Some(outcome.status.to_string()); let _ = self.run_hooks(&hook_ctx, hook_work_dir.as_deref()).await; } diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs index 741934172..b1e73dc55 100644 --- a/lib/crates/fabro-workflows/src/handler/parallel.rs +++ b/lib/crates/fabro-workflows/src/handler/parallel.rs @@ -8,6 +8,7 @@ use tokio::sync::Semaphore; use crate::context::keys; use crate::context::Context; +use crate::engine::set_hook_node; use crate::error::FabroError; use crate::event::WorkflowRunEvent; use crate::millis_u64; @@ -306,9 +307,7 @@ impl Handler for ParallelHandler { context.run_id(), graph.name.clone(), ); - 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); + set_hook_node(&mut hook_ctx, node); let _ = services.run_hooks(&hook_ctx).await; } let max_parallel = node @@ -729,9 +728,7 @@ impl Handler for ParallelHandler { context.run_id(), graph.name.clone(), ); - 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); + set_hook_node(&mut hook_ctx, node); let _ = services.run_hooks(&hook_ctx).await; }