From 4a872633ce3e580ac090f3198d00c686f601ff8b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 10 Apr 2026 12:01:46 -0400 Subject: [PATCH] fix(core): prevent infinite loop when goal-gate retry target is terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip retry when get_retry_target points at a terminal node — retrying into a terminal re-triggers the same goal-gate failure endlessly. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-core/src/executor.rs | 68 +++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/lib/crates/fabro-core/src/executor.rs b/lib/crates/fabro-core/src/executor.rs index 6dd4fea76..005eb4979 100644 --- a/lib/crates/fabro-core/src/executor.rs +++ b/lib/crates/fabro-core/src/executor.rs @@ -126,14 +126,19 @@ impl Executor { .await; // Check if there's a retry target for goal gate failure if let Some(retry_target) = graph.get_retry_target(&failed_node_id) { - tracing::debug!( - node = %node.id(), - retry_target = %retry_target, - failed_node = %failed_node_id, - "Goal gate unsatisfied, retrying" - ); - state.advance(&retry_target); - continue; + if graph + .get_node(&retry_target) + .is_some_and(|retry_node| !retry_node.is_terminal()) + { + tracing::debug!( + node = %node.id(), + retry_target = %retry_target, + failed_node = %failed_node_id, + "Goal gate unsatisfied, retrying" + ); + state.advance(&retry_target); + continue; + } } let outcome = Outcome::fail(&format!( "goal gate unsatisfied for node {failed_node_id} and no retry target" @@ -1926,6 +1931,53 @@ mod tests { assert_eq!(handler.calls(), 2); } + #[tokio::test] + async fn executor_goal_gate_retry_target_to_terminal_fails_without_looping() { + let terminal_visits = Arc::new(AtomicU32::new(0)); + + struct SingleTerminalVisit(Arc); + #[async_trait] + impl RunLifecycle for SingleTerminalVisit { + async fn on_terminal_reached( + &self, + _node: &TestNode, + _goal_gates_passed: bool, + _s: &ExecutionState, + ) { + let visits = self.0.fetch_add(1, Ordering::SeqCst); + assert_eq!(visits, 0, "terminal node reached more than once"); + } + } + + let g = TestGraph::new( + vec![ + TestNode::new("work"), + TestNode::terminal("end").with_goal_gate("work", StageStatus::Success), + ], + vec![TestEdge::new("work", "end")], + "work", + ) + .with_retry_target("work", "end"); + + let state = ExecutionState::new(&g).unwrap(); + let executor = ExecutorBuilder::new( + Arc::new(AlwaysFailHandler::new("boom")) as Arc> + ) + .lifecycle(Box::new(SingleTerminalVisit(terminal_visits.clone()))) + .build(); + + let (result, _) = executor.run(&g, state).await.unwrap(); + assert_eq!(result.status, StageStatus::Fail); + assert_eq!( + result + .failure + .as_ref() + .map(|failure| failure.message.as_str()), + Some("goal gate unsatisfied for node work and no retry target") + ); + assert_eq!(terminal_visits.load(Ordering::SeqCst), 1); + } + #[tokio::test] async fn executor_stall_token_interrupts_handler() { // stall token cancelled during handler execution returns StallTimeout