From d427b0e268e099922856ea1dfad59cd6398cce4d Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Thu, 22 Oct 2026 17:37:08 +0200 Subject: [PATCH] mind-body: a partial brake stop names each unit's state When systemctl stop fails, stop_all asks is-active for each of the two units and the error names each state (active, inactive, failed, unknown) beside the exit status; brake hold says the brake is held and the stop failed, not that the units were not stopped. Exit 2 and the held brake are unchanged. Follow-up brake-partial-stop-message. Not yet built; verified at the head of the cut. Co-Authored-By: Claude Sonnet 5.5 --- crates/mind-body/src/cli.rs | 4 +-- crates/mind-body/src/launch.rs | 59 ++++++++++++++++++++++++++++------ crates/mind-body/tests/cli.rs | 40 +++++++++++++++-------- 3 files changed, 79 insertions(+), 24 deletions(-) diff --git a/crates/mind-body/src/cli.rs b/crates/mind-body/src/cli.rs index edd59aa..92a9895 100644 --- a/crates/mind-body/src/cli.rs +++ b/crates/mind-body/src/cli.rs @@ -73,8 +73,8 @@ pub fn run( Ok(()) } (Err(write), Ok(())) => Err(write.context("the brake was not written; the units were stopped")), - (Ok(()), Err(stop)) => Err(stop.context("the brake is held; the units were not stopped")), - (Err(write), Err(stop)) => Err(anyhow!("the brake was not written ({write:#}) and the units were not stopped ({stop:#})")), + (Ok(()), Err(stop)) => Err(stop.context("the brake is held; the stop failed")), + (Err(write), Err(stop)) => Err(anyhow!("the brake was not written ({write:#}) and the stop failed ({stop:#})")), } } ["brake", "release"] => ControlWriter::open(&path)?.set_brake(true, now, set_by), diff --git a/crates/mind-body/src/launch.rs b/crates/mind-body/src/launch.rs index e135c2c..b3f32bd 100644 --- a/crates/mind-body/src/launch.rs +++ b/crates/mind-body/src/launch.rs @@ -142,7 +142,8 @@ impl SystemdLauncher { /// Stops every unit of both turns of one instance and waits for them. An /// inherent method, not on `Launcher`: only the operator's brake hold /// calls it. A unit that is not installed is systemctl's exit 5, and the - /// others still stop. + /// others still stop; when the stop fails the error names each unit's + /// state, because systemd may have stopped one and not the other. pub fn stop_all(&self, instance: &Slug) -> Result<()> { let args = stop_args(instance)?; let args: Vec<&str> = args.iter().map(String::as_str).collect(); @@ -150,9 +151,33 @@ impl SystemdLauncher { if reply.success { return Ok(()); } + let states = [RunTurn::SelfRun, RunTurn::PersonaTurn] + .into_iter() + .map(|turn| { + let unit = unit(instance, turn)?; + let state = self.unit_state(&unit); + Ok(format!("{unit} is {state}")) + }) + .collect::>>()? + .join(", "); match reply.code { - Some(code) => Err(anyhow!("systemctl stop failed with exit status {code}")), - None => Err(anyhow!("systemctl stop was ended by a signal")), + Some(code) => Err(anyhow!("systemctl stop failed with exit status {code}; {states}")), + None => Err(anyhow!("systemctl stop was ended by a signal; {states}")), + } + } + + /// One unit's state after a failed stop: systemd's own word when it is one + /// of the three a failed stop can leave, else `unknown`. Nothing else the query + /// prints is carried. + fn unit_state(&self, unit: &str) -> &'static str { + let Ok(reply) = self.systemctl.run(&["is-active", unit]) else { + return "unknown"; + }; + match reply.stdout.trim() { + "active" => "active", + "inactive" => "inactive", + "failed" => "failed", + _ => "unknown", } } } @@ -1128,18 +1153,24 @@ mod tests { success: Cell, code: Cell>, stdout: RefCell, + /// Per-unit answers to `is-active`; a unit not listed gets `stdout`. + states: RefCell>, } impl Systemctl for &'static Spy { fn run(&self, args: &[&str]) -> Result { self.calls.borrow_mut().push(args.iter().map(|arg| arg.to_string()).collect()); - Ok(SystemctlReply { success: self.success.get(), code: self.code.get(), stdout: self.stdout.borrow().clone() }) + let said = match args { + ["is-active", unit] => self.states.borrow().iter().find(|(named, _)| named == unit).map(|(_, state)| state.to_string()), + _ => None, + }; + Ok(SystemctlReply { success: self.success.get(), code: self.code.get(), stdout: said.unwrap_or_else(|| self.stdout.borrow().clone()) }) } } fn spied() -> (SystemdLauncher, &'static Spy) { let spy: &'static Spy = - Box::leak(Box::new(Spy { calls: RefCell::default(), success: Cell::new(true), code: Cell::new(Some(0)), stdout: RefCell::default() })); + Box::leak(Box::new(Spy { calls: RefCell::default(), success: Cell::new(true), code: Cell::new(Some(0)), stdout: RefCell::default(), states: RefCell::default() })); (SystemdLauncher::over(Box::new(spy)), spy) } @@ -1208,15 +1239,25 @@ mod tests { } #[test] - fn a_failed_stop_is_an_error_naming_the_exit_status_and_no_instance() { + fn a_failed_stop_names_the_exit_status_and_each_units_state_and_carries_no_other_output() { let (launcher, spy) = spied(); spy.success.set(false); spy.code.set(Some(5)); - let shown = format!("{:#}", launcher.stop_all(&Slug("canary-instance-7f3a91".into())).unwrap_err()); - assert!(shown.contains("exit status 5") && !shown.contains("canary"), "{shown}"); + spy.states.borrow_mut().extend([("mind-self@eureka.service", "inactive\n"), ("mind-persona@eureka.service", "active\n")]); + let shown = format!("{:#}", launcher.stop_all(&eureka()).unwrap_err()); + assert!(shown.contains("exit status 5"), "{shown}"); + assert!(shown.contains("mind-self@eureka.service is inactive") && shown.contains("mind-persona@eureka.service is active"), "{shown}"); + let asked: Vec> = spy.calls.borrow().iter().skip(1).cloned().collect(); + assert_eq!(asked, [["is-active", "mind-self@eureka.service"], ["is-active", "mind-persona@eureka.service"]], "one query per unit, after the one stop"); + // A state word outside the three, and anything systemctl prints with it, is not carried. + spy.states.borrow_mut().clear(); + *spy.stdout.borrow_mut() = "canary-output-7f3a91\n".to_string(); spy.code.set(None); let shown = format!("{:#}", launcher.stop_all(&eureka()).unwrap_err()); - assert!(shown.contains("signal"), "{shown}"); + assert!(shown.contains("signal") && shown.contains("is unknown") && !shown.contains("canary"), "{shown}"); + spy.states.borrow_mut().push(("mind-self@eureka.service", "failed\n")); + let shown = format!("{:#}", launcher.stop_all(&eureka()).unwrap_err()); + assert!(shown.contains("mind-self@eureka.service is failed"), "{shown}"); } #[test] diff --git a/crates/mind-body/tests/cli.rs b/crates/mind-body/tests/cli.rs index 159efc0..6d489d5 100644 --- a/crates/mind-body/tests/cli.rs +++ b/crates/mind-body/tests/cli.rs @@ -314,7 +314,8 @@ fn a_failing_stopper_fails_the_hold_and_leaves_the_brake_held() { }); let error = format!("{:#}", result.unwrap_err()); assert_eq!(calls.len(), 1); - assert!(error.contains("units were not stopped") && error.contains("exit status 5"), "{error}"); + assert!(error.contains("the brake is held") && error.contains("exit status 5"), "{error}"); + assert!(!error.contains("were not stopped"), "{error}"); assert!(!out.contains("stopped"), "a failed stop printed success: {out}"); assert!(shown(root.path()).contains("BrakeHeld")); } @@ -332,20 +333,27 @@ fn a_failing_write_still_stops_and_the_hold_fails() { let (result, _, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 2, |_| Err(anyhow::anyhow!("stop failed"))); let error = format!("{:#}", result.unwrap_err()); assert_eq!(calls.len(), 1); - assert!(error.contains("brake was not written") && error.contains("units were not stopped"), "{error}"); + assert!(error.contains("brake was not written") && error.contains("the stop failed") && !error.contains("were not stopped"), "{error}"); } /// Runs the real binary's `brake hold` with a `systemctl` of ours first on -/// PATH, so the stopper main.rs wires is the one under test. The fake records -/// its arguments, one per line, and exits with `exit`. +/// PATH, so the stopper main.rs wires is the one under test. The fake appends +/// its arguments to a record, one per line; `stop` exits with `exit`, and +/// `is-active` answers inactive for the Self unit and active for the Persona +/// unit, as systemd does when it stopped one and failed the other. The root is +/// returned so the store can be read afterwards. #[cfg(unix)] -fn hold_through_the_binary(exit: i32) -> (std::process::Output, Vec) { +fn hold_through_the_binary(exit: i32) -> (std::process::Output, Vec, tempfile::TempDir) { use std::os::unix::fs::PermissionsExt; let bin = tempfile::tempdir().unwrap(); let root = tempfile::tempdir().unwrap(); let record = bin.path().join("recorded"); let script = bin.path().join("systemctl"); - std::fs::write(&script, format!("#!/bin/sh\nprintf '%s\n' \"$@\" > '{}'\nexit {exit}\n", record.display())).unwrap(); + let body = format!( + "#!/bin/sh\nprintf '%s\n' \"$@\" >> '{}'\ncase \"$1\" in\n is-active) case \"$2\" in mind-self@eureka.service) echo inactive; exit 3;; *) echo active; exit 0;; esac;;\n *) exit {exit};;\nesac\n", + record.display() + ); + std::fs::write(&script, body).unwrap(); std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); let output = std::process::Command::new(env!("CARGO_BIN_EXE_mind-control")) .env("PATH", bin.path()) @@ -353,25 +361,31 @@ fn hold_through_the_binary(exit: i32) -> (std::process::Output, Vec) { .output() .unwrap(); let recorded = std::fs::read_to_string(&record).map(|text| text.lines().map(str::to_string).collect()).unwrap_or_default(); - (output, recorded) + (output, recorded, root) } #[cfg(unix)] #[test] fn the_binary_hold_runs_systemctl_stop_on_exactly_the_units_it_starts() { - let (output, recorded) = hold_through_the_binary(0); + let (output, recorded, _root) = hold_through_the_binary(0); assert_eq!(output.status.code(), Some(0), "{}", String::from_utf8_lossy(&output.stderr)); - assert_eq!(recorded, ["stop", "--", "mind-self@eureka.service", "mind-persona@eureka.service"]); + assert_eq!(recorded, ["stop", "--", "mind-self@eureka.service", "mind-persona@eureka.service"], "a stop that succeeds asks no state"); assert!(String::from_utf8_lossy(&output.stdout).contains("units: stopped mind-self@eureka.service mind-persona@eureka.service")); } #[cfg(unix)] #[test] -fn the_binary_hold_fails_with_status_2_when_systemctl_does() { - let (output, recorded) = hold_through_the_binary(5); +fn a_partial_stop_names_each_units_state_and_keeps_the_brake_held_with_exit_2() { + let (output, recorded, root) = hold_through_the_binary(5); assert_eq!(output.status.code(), Some(2)); - assert_eq!(recorded.len(), 4, "systemctl was run: {recorded:?}"); + assert_eq!( + recorded, + ["stop", "--", "mind-self@eureka.service", "mind-persona@eureka.service", "is-active", "mind-self@eureka.service", "is-active", "mind-persona@eureka.service"] + ); let stderr = String::from_utf8_lossy(&output.stderr); - assert!(stderr.contains("units were not stopped") && stderr.contains("exit status 5"), "{stderr}"); + assert!(stderr.contains("exit status 5") && stderr.contains("the brake is held"), "{stderr}"); + assert!(stderr.contains("mind-self@eureka.service is inactive") && stderr.contains("mind-persona@eureka.service is active"), "{stderr}"); + assert!(!stderr.contains("were not stopped"), "{stderr}"); assert!(!String::from_utf8_lossy(&output.stdout).contains("stopped")); + assert!(shown(root.path()).contains("BrakeHeld")); }