From 7249fbaf54b1e5219f683e96fb21ebdf2475bdb5 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Sun, 15 Mar 2026 14:50:42 -0400
Subject: [PATCH] Deduplicate CheckpointSaved hook, use idiomatic bsha.clone()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Move the identical CheckpointSaved hook block from both git and non-git
checkpoint branches to a single block after the if/else. Replace
bsha.to_string() with bsha.clone() for &String → String conversion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 lib/crates/fabro-workflows/src/engine.rs      | 32 +++++++------------
 .../fabro-workflows/src/handler/parallel.rs   |  8 ++---
 2 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs
index 87b80d1a8..515287708 100644
--- a/lib/crates/fabro-workflows/src/engine.rs
+++ b/lib/crates/fabro-workflows/src/engine.rs
@@ -1947,17 +1947,6 @@ impl WorkflowRunEngine {
                                 git_commit_sha: Some(sha.clone()),
                             });
 
-                        // CheckpointSaved hook (non-blocking)
-                        {
-                            let mut hook_ctx = HookContext::new(
-                                HookEvent::CheckpointSaved,
-                                run_id.clone(),
-                                graph.name.clone(),
-                            );
-                            hook_ctx.node_id = Some(node.id.clone());
-                            let _ = self.run_hooks(&hook_ctx, hook_work_dir.as_deref()).await;
-                        }
-
                         self.services.emitter.emit(&WorkflowRunEvent::GitCommit {
                             node_id: Some(node.id.clone()),
                             sha: sha.clone(),
@@ -2057,17 +2046,18 @@ impl WorkflowRunEngine {
                         status: outcome.status.to_string(),
                         git_commit_sha: None,
                     });
+            }
 
-                // CheckpointSaved hook (non-blocking)
-                {
-                    let mut hook_ctx = HookContext::new(
-                        HookEvent::CheckpointSaved,
-                        run_id.clone(),
-                        graph.name.clone(),
-                    );
-                    hook_ctx.node_id = Some(node.id.clone());
-                    let _ = self.run_hooks(&hook_ctx, hook_work_dir.as_deref()).await;
-                }
+            // CheckpointSaved hook (non-blocking) — fires for both git and non-git paths.
+            // The Err arm above returns early, so this only runs on success.
+            {
+                let mut hook_ctx = HookContext::new(
+                    HookEvent::CheckpointSaved,
+                    run_id.clone(),
+                    graph.name.clone(),
+                );
+                hook_ctx.node_id = Some(node.id.clone());
+                let _ = self.run_hooks(&hook_ctx, hook_work_dir.as_deref()).await;
             }
 
             // Step 7: Follow selected edge (or direct jump)
diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs
index 818d5f1f7..486978bcb 100644
--- a/lib/crates/fabro-workflows/src/handler/parallel.rs
+++ b/lib/crates/fabro-workflows/src/handler/parallel.rs
@@ -331,7 +331,7 @@ impl Handler for ParallelHandler {
                 }
                 services.emitter.emit(&WorkflowRunEvent::GitBranch {
                     branch: branch_name.clone(),
-                    sha: bsha.to_string(),
+                    sha: bsha.clone(),
                 });
                 if !crate::engine::git_replace_worktree(
                     &*services.sandbox,
@@ -358,9 +358,9 @@ impl Handler for ParallelHandler {
                         "failed to reset worktree {wt_path_str}"
                     )));
                 }
-                services.emitter.emit(&WorkflowRunEvent::GitReset {
-                    sha: bsha.to_string(),
-                });
+                services
+                    .emitter
+                    .emit(&WorkflowRunEvent::GitReset { sha: bsha.clone() });
 
                 branch_context.set(keys::INTERNAL_WORK_DIR, serde_json::json!(&wt_path_str));