From d29c1d817eddca4175be066fb3226badb0232f55 Mon Sep 17 00:00:00 2001
From: "brynary-fabro[bot]"
<265161896+brynary-fabro[bot]@users.noreply.github.com>
Date: Mon, 16 Mar 2026 21:38:53 -0400
Subject: [PATCH] Wire up missing hook invocations (#19)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This PR wires up three `HookEvent` variants—`StageRetrying`,
`ParallelStart`, and `ParallelComplete`—that were defined in the enum
and documented but never actually invoked by the engine. `StageRetrying`
hooks now fire at both retry sites in `execute_with_retry` (error-retry
and explicit-Retry-status paths) immediately before the backoff sleep.
`ParallelStart` and `ParallelComplete` hooks fire in the parallel
handler after their corresponding event emissions, using a new
`EngineServices::run_hooks()` convenience method since the handler
doesn't have direct access to the engine's hook method.
The two remaining unwired events, `SandboxReady` and `SandboxCleanup`,
are marked as reserved with doc comments on the enum variants and
annotated in the docs table, since wiring them requires sandbox
lifecycle changes outside the engine's scope. A small
`HookContext::set_node()` helper is introduced to reduce repeated field
assignment across all hook call sites, and existing
`StageStart`/`StageComplete` hook calls are refactored to use it.
### Fabro Details
Ran 10 stages in 25m 6s for $5.60
| Stage | Duration | Cost | Retries |
|---|---|---|---|
| start | 0s | – | 0 |
| toolchain | 0s | – | 0 |
| preflight_compile | 0s | – | 0 |
| preflight_lint | 0s | – | 0 |
| implement | 0s | $1.04 | 0 |
| simplify_opus | 0s | $1.51 | 0 |
| simplify_gemini | 0s | $1.44 | 0 |
| simplify_gpt | 0s | $1.62 | 0 |
| verify | 0s | – | 0 |
| fmt | 0s | – | 0 |
| **Total** | **25m 6s** | **$5.60** | **0** |
Ran ImplementAndSimplify.fabro (13 nodes and 16
edges)
```dot
digraph ImplementAndSimplify {
graph [
goal="Implement and simplify",
model_stylesheet="
* { backend: api; model: claude-opus-4-6;}
"
]
rankdir=LR
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0]
preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0]
preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo clippy -q --workspace -- -D warnings 2>&1", max_retries=0]
fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3]
implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD."]
simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"]
simplify_gemini [label="Simplify (Gemini)", prompt="@prompts/simplify.md", model="gemini-3.1-pro-preview-customtools"]
simplify_gpt [label="Simplify (GPT-54)", prompt="@prompts/simplify.md", model="gpt-54"]
verify [label="Verify", shape=parallelogram, script="cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1", goal_gate=true, retry_target="fixup"]
fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all clippy lint warnings and test failures.", max_visits=3]
fmt [label="Format", shape=parallelogram, script="cargo fmt --all 2>&1", goal_gate=true, max_retries=0]
start -> toolchain
toolchain -> preflight_compile [condition="outcome=success"]
toolchain -> exit
preflight_compile -> preflight_lint [condition="outcome=success"]
preflight_compile -> exit
preflight_lint -> implement [condition="outcome=success"]
preflight_lint -> fix_lints
fix_lints -> preflight_lint
implement -> simplify_opus -> simplify_gemini -> simplify_gpt -> verify
verify -> fmt [condition="outcome=success"]
verify -> fixup
fixup -> verify
fmt -> exit
}
```
⚒️ Generated with [Fabro](https://fabro.sh)
---------
Co-authored-by: Fabro
---
docs/agents/hooks.mdx | 4 +--
lib/crates/fabro-workflows/src/engine.rs | 36 ++++++++++++++-----
lib/crates/fabro-workflows/src/handler/mod.rs | 11 +++++-
.../fabro-workflows/src/handler/parallel.rs | 19 ++++++++++
lib/crates/fabro-workflows/src/hook/types.rs | 9 +++++
5 files changed, 67 insertions(+), 12 deletions(-)
diff --git a/docs/agents/hooks.mdx b/docs/agents/hooks.mdx
index 25cf86082..5310dba6d 100644
--- a/docs/agents/hooks.mdx
+++ b/docs/agents/hooks.mdx
@@ -94,8 +94,8 @@ Each hook fires on a specific lifecycle event:
| `edge_selected` | After an edge is chosen for traversal | Yes |
| `parallel_start` | Before parallel branches fan out | No |
| `parallel_complete` | After parallel branches merge | No |
-| `sandbox_ready` | After the sandbox environment is created | No |
-| `sandbox_cleanup` | Before the sandbox is torn down | No |
+| `sandbox_ready` | After the sandbox environment is created (reserved — not yet wired) | No |
+| `sandbox_cleanup` | Before the sandbox is torn down (reserved — not yet wired) | No |
| `checkpoint_saved` | After a checkpoint is written to disk | No |
| `pre_tool_use` | Before an agent tool call executes | Yes |
| `post_tool_use` | After an agent tool call succeeds | No |
diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs
index 3a31fe590..95a91cf63 100644
--- a/lib/crates/fabro-workflows/src/engine.rs
+++ b/lib/crates/fabro-workflows/src/engine.rs
@@ -980,6 +980,26 @@ impl WorkflowRunEngine {
let _ = self.run_hooks(&hook_ctx, work_dir).await;
}
+ /// Fire a non-blocking StageRetrying hook.
+ async fn stage_retrying_hook(
+ &self,
+ node: &Node,
+ context: &Context,
+ graph: &Graph,
+ attempt: u32,
+ policy: &RetryPolicy,
+ ) {
+ let mut hook_ctx = HookContext::new(
+ HookEvent::StageRetrying,
+ context.run_id(),
+ graph.name.clone(),
+ );
+ hook_ctx.set_node(node);
+ hook_ctx.attempt = Some(usize::try_from(attempt).unwrap_or(usize::MAX));
+ hook_ctx.max_attempts = Some(usize::try_from(policy.max_attempts).unwrap_or(usize::MAX));
+ let _ = self.run_hooks(&hook_ctx, None).await;
+ }
+
/// Mirror graph-level attributes into the context.
fn mirror_graph_attributes(graph: &Graph, context: &Context) {
if !graph.goal().is_empty() {
@@ -1129,6 +1149,8 @@ impl WorkflowRunEngine {
.unwrap_or(usize::MAX),
delay_ms: millis_u64(delay),
});
+ self.stage_retrying_hook(node, context, graph, attempt, policy)
+ .await;
tokio::time::sleep(delay).await;
continue;
}
@@ -1157,6 +1179,8 @@ impl WorkflowRunEngine {
.unwrap_or(usize::MAX),
delay_ms: millis_u64(delay),
});
+ self.stage_retrying_hook(node, context, graph, attempt, policy)
+ .await;
tokio::time::sleep(delay).await;
continue;
}
@@ -1633,9 +1657,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);
+ hook_ctx.set_node(node);
hook_ctx.attempt = Some(1);
hook_ctx.max_attempts =
Some(usize::try_from(retry_policy.max_attempts).unwrap_or(usize::MAX));
@@ -1771,9 +1793,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);
+ hook_ctx.set_node(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;
@@ -1805,9 +1825,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);
+ hook_ctx.set_node(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/mod.rs b/lib/crates/fabro-workflows/src/handler/mod.rs
index 29f0b30ac..953306880 100644
--- a/lib/crates/fabro-workflows/src/handler/mod.rs
+++ b/lib/crates/fabro-workflows/src/handler/mod.rs
@@ -22,7 +22,7 @@ use crate::engine::GitState;
use crate::error::FabroError;
use crate::event::EventEmitter;
use crate::graph::{shape_to_handler_type, Graph, Node};
-use crate::hook::HookRunner;
+use crate::hook::{HookContext, HookDecision, HookRunner};
use crate::interviewer::Interviewer;
use crate::outcome::Outcome;
@@ -53,6 +53,15 @@ impl EngineServices {
*self.git_state.write().unwrap() = state;
}
+ /// Run lifecycle hooks and return the merged decision.
+ /// Returns `Proceed` if no hook runner is configured.
+ pub async fn run_hooks(&self, hook_context: &HookContext) -> HookDecision {
+ let Some(ref runner) = self.hook_runner else {
+ return HookDecision::Proceed;
+ };
+ runner.run(hook_context, self.sandbox.clone(), None).await
+ }
+
/// Test-only default: empty registry, no hooks, local sandbox at cwd.
#[cfg(test)]
pub fn test_default() -> Self {
diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs
index 41b83f9bb..6da3a82db 100644
--- a/lib/crates/fabro-workflows/src/handler/parallel.rs
+++ b/lib/crates/fabro-workflows/src/handler/parallel.rs
@@ -11,6 +11,7 @@ use crate::context::Context;
use crate::error::FabroError;
use crate::event::WorkflowRunEvent;
use crate::graph::{Graph, Node};
+use crate::hook::{HookContext, HookEvent};
use crate::millis_u64;
use crate::outcome::{Outcome, StageStatus};
use fabro_agent::LocalSandbox;
@@ -299,6 +300,15 @@ impl Handler for ParallelHandler {
join_policy: join_policy.to_string(),
error_policy: error_policy.to_string(),
});
+ {
+ let mut hook_ctx = HookContext::new(
+ HookEvent::ParallelStart,
+ context.run_id(),
+ graph.name.clone(),
+ );
+ hook_ctx.set_node(node);
+ let _ = services.run_hooks(&hook_ctx).await;
+ }
let max_parallel = node
.attrs
.get("max_parallel")
@@ -711,6 +721,15 @@ impl Handler for ParallelHandler {
success_count,
failure_count: fail_count,
});
+ {
+ let mut hook_ctx = HookContext::new(
+ HookEvent::ParallelComplete,
+ context.run_id(),
+ graph.name.clone(),
+ );
+ hook_ctx.set_node(node);
+ let _ = services.run_hooks(&hook_ctx).await;
+ }
// Evaluate join policy
let status = match join_policy {
diff --git a/lib/crates/fabro-workflows/src/hook/types.rs b/lib/crates/fabro-workflows/src/hook/types.rs
index 2a299b860..5712a1b17 100644
--- a/lib/crates/fabro-workflows/src/hook/types.rs
+++ b/lib/crates/fabro-workflows/src/hook/types.rs
@@ -14,7 +14,9 @@ pub enum HookEvent {
EdgeSelected,
ParallelStart,
ParallelComplete,
+ /// Reserved: hooks for this event are not yet invoked by the engine.
SandboxReady,
+ /// Reserved: hooks for this event are not yet invoked by the engine.
SandboxCleanup,
CheckpointSaved,
PreToolUse,
@@ -97,6 +99,13 @@ pub struct HookContext {
}
impl HookContext {
+ /// Populate node-related fields from a graph `Node`.
+ pub fn set_node(&mut self, node: &crate::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 {