From 581737583e977f32824e47d1a50f3203b2906097 Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Fri, 17 Jul 2026 01:19:35 +0200
Subject: [PATCH] fix(#2500): group_terminal must require the group node itself
 terminal
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

group_terminal checked only that every child was terminal, never the group
node's own state — so an empty group whose node is still Running returned
true (empty .all()), making a running node that has yet to append its
subgraph look already-finished. Now it requires the group node itself
terminal AND every child recursively terminal. Deciding when to settle a
group node to terminal once its children are done stays a scheduler concern;
this answers the dependents' question — is the whole group, node included,
finished. Adds group-node-pending-with-child-done + empty-running-group tests.
---
 hive-jobq/src/lib.rs | 45 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/hive-jobq/src/lib.rs b/hive-jobq/src/lib.rs
index 73c31a75..b63e125c 100644
--- a/hive-jobq/src/lib.rs
+++ b/hive-jobq/src/lib.rs
@@ -210,12 +210,18 @@ impl<N> Graph<N> {
         self.nodes.iter().filter(move |n| n.parent == Some(id))
     }
 
-    /// A group is terminal once every node inside it (recursively) is terminal.
-    /// An empty group is terminal.
+    /// A group is terminal once the group node itself is terminal *and* every
+    /// node inside it (recursively) is terminal. The node's own state matters:
+    /// a group node still `Pending`/`Running` is not terminal even with no
+    /// children yet, since a running node may still append some. (Deciding when
+    /// to *settle* a group node to terminal once its children are all done is a
+    /// separate concern the scheduler owns.) An unknown id is not terminal.
     #[must_use]
     pub fn group_terminal(&self, id: NodeId) -> bool {
-        self.children(id)
-            .all(|child| child.state.is_terminal() && self.group_terminal(child.id))
+        let Some(node) = self.node(id) else {
+            return false;
+        };
+        node.state.is_terminal() && self.children(id).all(|child| self.group_terminal(child.id))
     }
 }
 
@@ -242,16 +248,37 @@ mod tests {
         assert_eq!(g.node(a).unwrap().parent, None);
     }
 
+    fn set_state<N>(g: &mut Graph<N>, id: NodeId, state: State) {
+        let idx = g.nodes.iter().position(|n| n.id == id).unwrap();
+        g.nodes[idx].state = state;
+    }
+
     #[test]
-    fn group_is_terminal_only_when_all_children_terminal() {
+    fn group_terminal_requires_the_group_node_and_all_children_terminal() {
         let mut g: Graph<&str> = Graph::new();
         let group = g.insert("group", vec![], None);
         let child = g.insert("child", vec![], Some(group));
-        // Empty-below or pending child → not terminal.
+        // Both pending → not terminal.
         assert!(!g.group_terminal(group));
-        // Mark the child done.
-        let idx = g.nodes.iter().position(|n| n.id == child).unwrap();
-        g.nodes[idx].state = State::Done;
+        // Child done, but the group node itself is still pending → NOT terminal:
+        // the group node's own state is load-bearing, not just its children.
+        set_state(&mut g, child, State::Done);
+        assert!(!g.group_terminal(group));
+        // Group node terminal too → the whole group is terminal.
+        set_state(&mut g, group, State::Done);
+        assert!(g.group_terminal(group));
+    }
+
+    #[test]
+    fn empty_running_group_is_not_terminal() {
+        // A running node with no children yet may still append some, so it must
+        // not read as terminal just because its child set is currently empty.
+        let mut g: Graph<&str> = Graph::new();
+        let group = g.insert("group", vec![], None);
+        set_state(&mut g, group, State::Running);
+        assert!(!g.group_terminal(group));
+        // Once it finishes (having grown no children), it is terminal.
+        set_state(&mut g, group, State::Done);
         assert!(g.group_terminal(group));
     }