fix: report dead-task respawns as restarts (#71)

* fix: report dead-task respawns as restarts

* fix(reporting): preserve restart taxonomy on current main

Author: myobie

src/eval_run.rs

@@ -1192,7 +1192,10 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos
                         ),
                     };
                     if !report.launched.is_empty() {
-                        eval_log!("== supervise: respawned {:?} from spec ==", report.launched);
+                        eval_log!("== supervise: launched {:?} from spec ==", report.launched);
+                    }
+                    if !report.restarted.is_empty() {
+                        eval_log!("== supervise: restarted {:?} from spec ==", report.restarted);
                     }
                 }
             };

src/main.rs

@@ -1959,6 +1959,7 @@ fn up(
 
 fn print_report(report: &UpReport) {
     report_line("launched", &report.launched);
+    report_line("restarted", &report.restarted);
     report_line("torn down", &report.torn_down);
     report_line("gc", &report.gc);
     report_line("held", &report.held);

src/run.rs

@@ -779,11 +779,15 @@ pub struct UpReport {
     /// The pass could not obtain an authoritative session snapshot, so it deliberately performed no
     /// reconciliation. Long-running supervisors retry; a one-shot caller must exit unsuccessfully.
     pub skipped: bool,
-    /// pty ids spawned this pass.
+    /// Task IDs that st2 started without a restart reap in this pass.
     pub launched: Vec<String>,
+    /// Task IDs that st2 restarted successfully in this pass. st2 reaped a dead active record
+    /// before it spawned the replacement. These IDs are not first launches or final garbage
+    /// collection.
+    pub restarted: Vec<String>,
     /// pty ids torn down (retired agents) this pass.
     pub torn_down: Vec<String>,
-    /// pty ids garbage-collected (dead, non-`keep`) this pass.
+    /// Task IDs in final garbage collection. st2 did not spawn replacements.
     pub gc: Vec<String>,
     /// pty ids whose GC/relaunch was DEFERRED this pass by the liveness debounce — a task that read
     /// not-alive but was alive within the grace window, i.e. a transient `pty list` flicker under load,
@@ -811,6 +815,7 @@ impl UpReport {
     fn absorb(&mut self, mut other: UpReport) {
         self.skipped |= other.skipped;
         self.launched.append(&mut other.launched);
+        self.restarted.append(&mut other.restarted);
         self.torn_down.append(&mut other.torn_down);
         self.gc.append(&mut other.gc);
         self.deferred.append(&mut other.deferred);
@@ -828,6 +833,7 @@ impl UpReport {
     pub fn is_noteworthy(&self) -> bool {
         self.skipped
             || !self.launched.is_empty()
+            || !self.restarted.is_empty()
             || !self.torn_down.is_empty()
             || !self.gc.is_empty()
             || !self.flapping.is_empty()
@@ -888,11 +894,12 @@ pub fn execute(
                 crate::flapping::RestartDecision::Delaying
                 | crate::flapping::RestartDecision::RateLimited => continue,
             }
-            // Reap the corpse first (a dead session blocks respawn), preserving any backend-owned
-            // bounded diagnostics, then respawn.
-            if gc_set.contains(target.pty_id.as_str()) {
+            // Reap the dead record before st2 starts a replacement. A dead record blocks the
+            // replacement. The backend preserves its bounded diagnostics.
+            let restarting = gc_set.contains(target.pty_id.as_str());
+            if restarting {
                 match runner.reap_for_restart(&target.pty_id) {
-                    Ok(()) => report.gc.push(target.pty_id.clone()),
+                    Ok(()) => {}
                     Err(e) => {
                         report
                             .errors
@@ -904,7 +911,11 @@ pub fn execute(
             match runner.spawn(target, spec_dir) {
                 Ok(()) => {
                     cap.record(&target.pty_id, now);
-                    report.launched.push(target.pty_id.clone());
+                    if restarting {
+                        report.restarted.push(target.pty_id.clone());
+                    } else {
+                        report.launched.push(target.pty_id.clone());
+                    }
                 }
                 Err(e) => report.errors.push(format!("spawn {}: {e}", target.pty_id)),
             }

tests/run.rs

@@ -103,17 +103,23 @@ fn selected_catalog_two_agent_kdl_recording_runner_matrix() {
                 assert_eq!(runner.spawned.borrow().as_slice(), ["host.owner.work"]);
                 assert!(runner.reaped.borrow().is_empty());
                 assert_eq!(report.launched, ["host.owner.work"]);
+                assert!(report.restarted.is_empty());
+                assert!(report.gc.is_empty());
             }
             Actual::Live => {
                 assert!(runner.spawned.borrow().is_empty());
                 assert!(runner.reaped.borrow().is_empty());
                 assert_eq!(report.adopted, ["owner"]);
+                assert!(report.launched.is_empty());
+                assert!(report.restarted.is_empty());
+                assert!(report.gc.is_empty());
             }
             Actual::Dead => {
                 assert_eq!(runner.reaped.borrow().as_slice(), ["host.owner.work"]);
                 assert_eq!(runner.spawned.borrow().as_slice(), ["host.owner.work"]);
-                assert_eq!(report.gc, ["host.owner.work"]);
-                assert_eq!(report.launched, ["host.owner.work"]);
+                assert!(report.launched.is_empty());
+                assert_eq!(report.restarted, ["host.owner.work"]);
+                assert!(report.gc.is_empty());
             }
         }
         assert!(
@@ -349,7 +355,7 @@ fn selected_one_shot_live_adopts_without_actions() {
 }
 
 #[test]
-fn selected_one_shot_dead_reaps_and_relaunches_only_selected() {
+fn selected_one_shot_reports_a_dead_task_only_as_restarted() {
     let runner = FakeRunner {
         sessions: vec![
             dead("host.agent.work"),
@@ -375,8 +381,9 @@ fn selected_one_shot_dead_reaps_and_relaunches_only_selected() {
     assert_eq!(runner.spawned.borrow().as_slice(), ["host.agent.work"]);
     assert!(runner.killed.borrow().is_empty());
     assert!(runner.removed.borrow().is_empty());
-    assert_eq!(report.gc, ["host.agent.work"]);
-    assert_eq!(report.launched, ["host.agent.work"]);
+    assert!(report.launched.is_empty());
+    assert_eq!(report.restarted, ["host.agent.work"]);
+    assert!(report.gc.is_empty());
 }
 
 #[test]
@@ -444,6 +451,7 @@ struct FakeRunner {
     killed: RefCell<Vec<String>>,
     reaped: RefCell<Vec<String>>,
     removed: RefCell<Vec<String>>,
+    ops: RefCell<Vec<String>>,
 }
 
 impl Runner for FakeRunner {
@@ -458,6 +466,9 @@ impl Runner for FakeRunner {
         if self.fail_spawn.as_deref() == Some(target.pty_id.as_str()) {
             anyhow::bail!("simulated spawn failure");
         }
+        self.ops
+            .borrow_mut()
+            .push(format!("spawn:{}", target.pty_id));
         self.spawned.borrow_mut().push(target.pty_id.clone());
         self.spawn_dirs
             .borrow_mut()
@@ -469,6 +480,7 @@ impl Runner for FakeRunner {
         Ok(())
     }
     fn reap_for_restart(&self, pty_id: &str) -> anyhow::Result<()> {
+        self.ops.borrow_mut().push(format!("reap:{pty_id}"));
         self.reaped.borrow_mut().push(pty_id.to_string());
         if self.fail_reap.as_deref() == Some(pty_id) {
             anyhow::bail!("reap broke");
@@ -525,6 +537,8 @@ fn up_once_launches_all_tasks_of_a_fresh_agent() {
     let mut launched = report.launched.clone();
     launched.sort();
     assert_eq!(launched, vec!["hetz.demo-claude", "hetz.demo.ding"]);
+    assert!(report.restarted.is_empty());
+    assert!(report.gc.is_empty());
     assert!(report.errors.is_empty());
     let dirs = runner.spawn_dirs.borrow();
     assert!(dirs.iter().all(|(_, d)| d.ends_with("agents/hetz/demo")));
@@ -614,7 +628,7 @@ fn up_once_collects_spawn_errors_without_aborting() {
 }
 
 #[test]
-fn up_once_reaps_dead_nonkeep_then_respawns() {
+fn up_once_reports_a_successful_replacement_only_as_restarted() {
     let tmp = tempfile::tempdir().unwrap();
     write(tmp.path(), "agents/hetz/demo/agent.toml", AGENT);
     let runner = FakeRunner {
@@ -627,9 +641,26 @@ fn up_once_reaps_dead_nonkeep_then_respawns() {
     assert_eq!(reaped, vec!["hetz.demo-claude", "hetz.demo.ding"]);
     assert!(
         runner.removed.borrow().is_empty(),
-        "a crash restart is not final retirement cleanup"
+        "a restart must not remove final retirement state"
+    );
+    assert!(report.launched.is_empty());
+    let mut restarted = report.restarted.clone();
+    restarted.sort();
+    assert_eq!(restarted, vec!["hetz.demo-claude", "hetz.demo.ding"]);
+    assert!(
+        report.gc.is_empty(),
+        "a successful restart must not be reported as final garbage collection"
+    );
+    assert_eq!(
+        runner.ops.borrow().as_slice(),
+        [
+            "reap:hetz.demo-claude",
+            "spawn:hetz.demo-claude",
+            "reap:hetz.demo.ding",
+            "spawn:hetz.demo.ding",
+        ],
+        "st2 must reap each dead record before it starts the replacement"
     );
-    assert_eq!(report.launched.len(), 2);
 }
 
 #[test]
@@ -644,7 +675,9 @@ fn up_once_does_not_restart_a_task_when_diagnostic_reap_fails() {
 
     let report = up_once(tmp.path(), "hetz", &runner).unwrap();
 
-    assert_eq!(report.launched, vec!["hetz.demo.ding"]);
+    assert!(report.launched.is_empty());
+    assert_eq!(report.restarted, vec!["hetz.demo.ding"]);
+    assert!(report.gc.is_empty());
     assert_eq!(runner.spawned.borrow().as_slice(), ["hetz.demo.ding"]);
     assert!(
         report
@@ -654,6 +687,34 @@ fn up_once_does_not_restart_a_task_when_diagnostic_reap_fails() {
     );
 }
 
+#[test]
+fn up_once_does_not_report_failed_replacement_as_restarted() {
+    let tmp = tempfile::tempdir().unwrap();
+    write(tmp.path(), "agents/hetz/demo/agent.toml", AGENT);
+    let runner = FakeRunner {
+        sessions: vec![dead("hetz.demo-claude"), live("hetz.demo.ding")],
+        fail_spawn: Some("hetz.demo-claude".into()),
+        ..Default::default()
+    };
+
+    let report = up_once(tmp.path(), "hetz", &runner).unwrap();
+
+    assert_eq!(
+        runner.reaped.borrow().as_slice(),
+        ["hetz.demo-claude"],
+        "st2 must reap the stale record before it starts a replacement"
+    );
+    assert!(report.launched.is_empty());
+    assert!(report.restarted.is_empty());
+    assert!(report.gc.is_empty());
+    assert!(
+        report
+            .errors
+            .iter()
+            .any(|error| error == "spawn hetz.demo-claude: simulated spawn failure")
+    );
+}
+
 #[test]
 fn up_once_finally_removes_dead_retired_tasks_without_restarting_them() {
     let tmp = tempfile::tempdir().unwrap();
@@ -675,6 +736,7 @@ fn up_once_finally_removes_dead_retired_tasks_without_restarting_them() {
     assert_eq!(removed, vec!["hetz.demo-claude", "hetz.demo.ding"]);
     assert!(runner.reaped.borrow().is_empty());
     assert!(report.launched.is_empty());
+    assert!(report.restarted.is_empty());
     assert_eq!(report.gc.len(), 2);
 }