Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/eval_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
};
Expand Down
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
25 changes: 18 additions & 7 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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)),
}
Expand Down
80 changes: 71 additions & 9 deletions tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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"),
Expand All @@ -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]
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand All @@ -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");
Expand Down Expand Up @@ -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")));
Expand Down Expand Up @@ -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 {
Expand All @@ -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]
Expand All @@ -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
Expand All @@ -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();
Expand All @@ -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);
}

Expand Down
Loading