From 02c2c4b0f5ce2a05e87a081ccfac8e2e5bc39706 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Wed, 29 Jul 2026 23:38:36 +0200 Subject: [PATCH 1/2] fix: report dead-task respawns as restarts --- src/eval_run.rs | 5 +++- src/main.rs | 1 + src/run.rs | 19 +++++++++++---- tests/run.rs | 61 ++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/src/eval_run.rs b/src/eval_run.rs index 7a61fb0..c4cdd51 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -824,7 +824,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); } } }; diff --git a/src/main.rs b/src/main.rs index 7cc8208..696d605 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1845,6 +1845,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("flapping", &report.flapping); diff --git a/src/run.rs b/src/run.rs index 7a868d0..5becc7c 100644 --- a/src/run.rs +++ b/src/run.rs @@ -527,11 +527,14 @@ 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 first spawned this pass (no prior runtime record was reaped). pub launched: Vec, + /// Task ids successfully restarted this pass: a dead active record was reaped, then its + /// replacement was spawned. Kept distinct from both first launch and final garbage collection. + pub restarted: Vec, /// pty ids torn down (retired agents) this pass. pub torn_down: Vec, - /// pty ids garbage-collected (dead, non-`keep`) this pass. + /// Task ids finally garbage-collected this pass, with no replacement spawned. pub gc: Vec, /// 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, @@ -559,6 +562,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() @@ -621,9 +625,10 @@ pub fn execute( } // 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()) { + 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 @@ -635,7 +640,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)), } diff --git a/tests/run.rs b/tests/run.rs index 21f5c4c..30eabee 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -21,6 +21,7 @@ struct FakeRunner { killed: RefCell>, reaped: RefCell>, removed: RefCell>, + ops: RefCell>, } impl Runner for FakeRunner { @@ -34,6 +35,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() @@ -45,6 +49,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"); @@ -93,6 +98,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"))); @@ -168,7 +175,7 @@ fn up_once_collects_spawn_errors_without_aborting() { } #[test] -fn up_once_reaps_dead_nonkeep_then_respawns() { +fn up_once_reports_dead_active_reap_spawn_as_restart_not_gc_or_launch() { let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "agents/hetz/demo/agent.toml", AGENT); let runner = FakeRunner { @@ -183,7 +190,24 @@ fn up_once_reaps_dead_nonkeep_then_respawns() { runner.removed.borrow().is_empty(), "a crash restart is not final retirement cleanup" ); - assert_eq!(report.launched.len(), 2); + 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 GC" + ); + assert_eq!( + runner.ops.borrow().as_slice(), + [ + "reap:hetz.demo-claude", + "spawn:hetz.demo-claude", + "reap:hetz.demo.ding", + "spawn:hetz.demo.ding", + ], + "report taxonomy must not change reap-before-spawn execution ordering" + ); } #[test] @@ -198,7 +222,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 @@ -208,6 +234,34 @@ fn up_once_does_not_restart_a_task_when_diagnostic_reap_fails() { ); } +#[test] +fn up_once_does_not_report_a_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"], + "the stale record is still reaped before the replacement attempt" + ); + 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(); @@ -229,6 +283,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); } From ac653955bf2d6830cc9ae84ed415313fa96a2133 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Fri, 31 Jul 2026 16:24:11 +0200 Subject: [PATCH 2/2] fix(reporting): preserve restart taxonomy on current main --- src/run.rs | 14 ++++++++------ tests/run.rs | 29 ++++++++++++++++++----------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/run.rs b/src/run.rs index 6b6858a..fb81a12 100644 --- a/src/run.rs +++ b/src/run.rs @@ -779,14 +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, - /// Task ids first spawned this pass (no prior runtime record was reaped). + /// Task IDs that st2 started without a restart reap in this pass. pub launched: Vec, - /// Task ids successfully restarted this pass: a dead active record was reaped, then its - /// replacement was spawned. Kept distinct from both first launch and final garbage collection. + /// 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, /// pty ids torn down (retired agents) this pass. pub torn_down: Vec, - /// Task ids finally garbage-collected this pass, with no replacement spawned. + /// Task IDs in final garbage collection. st2 did not spawn replacements. pub gc: Vec, /// 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, @@ -814,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); @@ -892,8 +894,8 @@ 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. + // 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) { diff --git a/tests/run.rs b/tests/run.rs index ec30fee..a477a34 100644 --- a/tests/run.rs +++ b/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] @@ -621,7 +628,7 @@ fn up_once_collects_spawn_errors_without_aborting() { } #[test] -fn up_once_reports_dead_active_reap_spawn_as_restart_not_gc_or_launch() { +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 { @@ -634,7 +641,7 @@ fn up_once_reports_dead_active_reap_spawn_as_restart_not_gc_or_launch() { 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(); @@ -642,7 +649,7 @@ fn up_once_reports_dead_active_reap_spawn_as_restart_not_gc_or_launch() { assert_eq!(restarted, vec!["hetz.demo-claude", "hetz.demo.ding"]); assert!( report.gc.is_empty(), - "a successful restart must not be reported as final GC" + "a successful restart must not be reported as final garbage collection" ); assert_eq!( runner.ops.borrow().as_slice(), @@ -652,7 +659,7 @@ fn up_once_reports_dead_active_reap_spawn_as_restart_not_gc_or_launch() { "reap:hetz.demo.ding", "spawn:hetz.demo.ding", ], - "report taxonomy must not change reap-before-spawn execution ordering" + "st2 must reap each dead record before it starts the replacement" ); } @@ -681,7 +688,7 @@ fn up_once_does_not_restart_a_task_when_diagnostic_reap_fails() { } #[test] -fn up_once_does_not_report_a_failed_replacement_as_restarted() { +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 { @@ -695,7 +702,7 @@ fn up_once_does_not_report_a_failed_replacement_as_restarted() { assert_eq!( runner.reaped.borrow().as_slice(), ["hetz.demo-claude"], - "the stale record is still reaped before the replacement attempt" + "st2 must reap the stale record before it starts a replacement" ); assert!(report.launched.is_empty()); assert!(report.restarted.is_empty());