diff --git a/src/eval_run.rs b/src/eval_run.rs index bd18f91..2f7689f 100644 --- a/src/eval_run.rs +++ b/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); } } }; diff --git a/src/main.rs b/src/main.rs index 6237892..026ce54 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1943,6 +1943,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); diff --git a/src/run.rs b/src/run.rs index a2460f1..fb81a12 100644 --- a/src/run.rs +++ b/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, + /// 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, - /// pty ids garbage-collected (dead, non-`keep`) this pass. + /// 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, @@ -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)), } diff --git a/tests/run.rs b/tests/run.rs index 43603a6..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] @@ -444,6 +451,7 @@ struct FakeRunner { killed: RefCell>, reaped: RefCell>, removed: RefCell>, + ops: RefCell>, } 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); }