mind-body: brake hold stops her running units after writing the brake held

SystemdLauncher::stop_all runs one synchronous systemctl stop over the
instance's mind-self and mind-persona units (stop_args, built from the same
templates as start and alive). cli::run gains a stop_units parameter that only
brake hold calls, after the write whatever its outcome. The window probe uses
dial set instead of brake hold, since a real hold now runs systemctl.

Builds not yet verified on the stopgap at this commit.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/mind-body/src/cli.rs

@@ -4,10 +4,14 @@
 //!
 //! ```text
 //! mind-control [--root <dir>] --instance <slug> show
-//! mind-control --instance <slug> brake hold
+//! mind-control --instance <slug> brake hold        (also stops her running units)
 //! mind-control --instance <slug> brake release
 //! mind-control --instance <slug> dial set --heat H --base-cooldown-s S --base-run-usd U
 //! ```
+//!
+//! `brake hold` writes the brake held and then, whatever the write's outcome,
+//! stops the instance's running units through the `stop_units` parameter. It
+//! fails if either step failed. No other command stops anything.
 
 use std::io::Write;
 use std::path::Path;
@@ -26,18 +30,22 @@ const USAGE: &str =
 /// Who made the change: `SUDO_USER`, else `USER`, else `unknown`. An empty
 /// variable counts as unset.
 pub fn operator_name(sudo_user: Option<String>, user: Option<String>) -> String {
-    [sudo_user, user]
-        .into_iter()
-        .flatten()
-        .find(|name| !name.is_empty())
-        .unwrap_or_else(|| "unknown".to_string())
+    [sudo_user, user].into_iter().flatten().find(|name| !name.is_empty()).unwrap_or_else(|| "unknown".to_string())
 }
 
 /// Runs one command against `<root>/<instance>/control.cc`. `root` is the
 /// default; a leading `--root <dir>` argument replaces it (argv, not environment,
-/// because `sudo -E` carries the environment). The operator's name, the clock and the output are parameters so a test drives
-/// the whole command without touching `/etc`.
-pub fn run(args: &[String], root: &Path, set_by: &str, now: DateTime<Utc>, out: &mut impl Write) -> Result<()> {
+/// because `sudo -E` carries the environment). The operator's name, the clock, the output and the stopper are parameters so a test drives
+/// the whole command without touching `/etc` or systemd. `stop_units` is
+/// called only by `brake hold`, after the write, whatever the write's outcome.
+pub fn run(
+    args: &[String],
+    root: &Path,
+    set_by: &str,
+    now: DateTime<Utc>,
+    out: &mut impl Write,
+    stop_units: &mut dyn FnMut(&Slug) -> Result<()>,
+) -> Result<()> {
     let (root, args) = match args {
         [flag, dir, rest @ ..] if flag == "--root" => (Path::new(dir), rest),
         _ => (root, args),
@@ -50,7 +58,19 @@ pub fn run(args: &[String], root: &Path, set_by: &str, now: DateTime<Utc>, out:
     let words: Vec<&str> = command.iter().map(String::as_str).collect();
     match words.as_slice() {
         ["show"] => show(root, &instance, out),
-        ["brake", "hold"] => ControlWriter::open(&path)?.set_brake(false, now, set_by),
+        ["brake", "hold"] => {
+            let written = ControlWriter::open(&path).and_then(|mut writer| writer.set_brake(false, now, set_by));
+            let stopped = stop_units(&instance);
+            match (written, stopped) {
+                (Ok(()), Ok(())) => {
+                    writeln!(out, "units: stopped mind-self@{0}:* mind-persona@{0}:*", instance.0)?;
+                    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:#})")),
+            }
+        }
         ["brake", "release"] => ControlWriter::open(&path)?.set_brake(true, now, set_by),
         ["dial", "set", flags @ ..] => {
             let dial = parse_dial(flags, now, set_by)?;
@@ -77,9 +97,7 @@ fn parse_dial(flags: &[&str], now: DateTime<Utc>, set_by: &str) -> Result<BurnRa
     }
     Ok(BurnRate {
         heat: Decimal::from_str(need(heat, "--heat")?).context("--heat is not a decimal")?,
-        base_cooldown_s: need(cooldown, "--base-cooldown-s")?
-            .parse()
-            .context("--base-cooldown-s is not a whole number of seconds")?,
+        base_cooldown_s: need(cooldown, "--base-cooldown-s")?.parse().context("--base-cooldown-s is not a whole number of seconds")?,
         base_run_usd: Decimal::from_str(need(run_usd, "--base-run-usd")?).context("--base-run-usd is not a decimal")?,
         set_at: now,
         set_by: set_by.to_string(),

crates/mind-body/src/launch.rs

@@ -14,8 +14,8 @@
 //! the instance it was read for: a Grant of another instance is refused. A
 //! Persona turn's run opens with no claims, so a request that carries any is
 //! refused. The primitive
-//! owns no file and stops no unit: stopping her units is the operator's brake
-//! hold, which is another cut's.
+//! owns no file and stops no unit through the `Launcher` port: stopping her
+//! units is the operator's brake hold, through `SystemdLauncher::stop_all`.
 
 use anyhow::{Result, anyhow};
 use chrono::{DateTime, Duration, Utc};
@@ -55,9 +55,11 @@ impl Clock for SystemClock {
     }
 }
 
-/// What `systemctl` answered: whether it succeeded, and what it printed.
+/// What `systemctl` answered: whether it succeeded, its exit code (none when a
+/// signal ended it), and what it printed.
 pub struct SystemctlReply {
     pub success: bool,
+    pub code: Option<i32>,
     pub stdout: String,
 }
 
@@ -71,7 +73,11 @@ struct RealSystemctl;
 impl Systemctl for RealSystemctl {
     fn run(&self, args: &[&str]) -> Result<SystemctlReply> {
         let output = std::process::Command::new("systemctl").args(args).output().map_err(|_| anyhow!("systemctl could not be run"))?;
-        Ok(SystemctlReply { success: output.status.success(), stdout: String::from_utf8_lossy(&output.stdout).into_owned() })
+        Ok(SystemctlReply {
+            success: output.status.success(),
+            code: output.status.code(),
+            stdout: String::from_utf8_lossy(&output.stdout).into_owned(),
+        })
     }
 }
 
@@ -101,16 +107,50 @@ impl Default for SystemdLauncher {
     }
 }
 
+/// The unit template of one turn. Start, alive and stop all name units from
+/// these two.
+fn template(turn: RunTurn) -> &'static str {
+    match turn {
+        RunTurn::PersonaTurn => "mind-persona",
+        RunTurn::SelfRun => "mind-self",
+    }
+}
+
 /// The unit of one turn: the template by turn, then the instance, held to the
 /// leaf's grammar first so it cannot escape the name. The error names nothing
 /// of the value. Start and alive both name the unit here.
 pub(crate) fn unit(instance: &Slug, turn: RunTurn) -> Result<String> {
     instance.validate_slug().map_err(|_| anyhow!("the instance is not a unit name"))?;
-    let template = match turn {
-        RunTurn::PersonaTurn => "mind-persona",
-        RunTurn::SelfRun => "mind-self",
-    };
-    Ok(format!("{template}@{}.service", instance.0))
+    Ok(format!("{}@{}.service", template(turn), instance.0))
+}
+
+/// The arguments of the one synchronous stop: every unit of both turns of this
+/// instance, matched by glob. The `:` after the instance keeps `eureka` from
+/// matching `eureka-two`. No `--no-block`: `systemctl stop` returns when the
+/// units are down. The instance is not validated here; `stop_all` does.
+pub(crate) fn stop_args(instance: &Slug) -> Vec<String> {
+    let mut args = vec!["stop".to_string(), "--".to_string()];
+    args.extend([RunTurn::SelfRun, RunTurn::PersonaTurn].map(|turn| format!("{}@{}:*.service", template(turn), instance.0)));
+    args
+}
+
+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 glob that matches no loaded unit is a success.
+    pub fn stop_all(&self, instance: &Slug) -> Result<()> {
+        instance.validate_slug().map_err(|_| anyhow!("the instance is not a unit name"))?;
+        let args = stop_args(instance);
+        let args: Vec<&str> = args.iter().map(String::as_str).collect();
+        let reply = self.systemctl.run(&args)?;
+        if reply.success {
+            return Ok(());
+        }
+        match reply.code {
+            Some(code) => Err(anyhow!("systemctl stop failed with exit status {code}")),
+            None => Err(anyhow!("systemctl stop was ended by a signal")),
+        }
+    }
 }
 
 impl Launcher for SystemdLauncher {
@@ -1082,18 +1122,20 @@ mod tests {
     struct Spy {
         calls: RefCell<Vec<Vec<String>>>,
         success: Cell<bool>,
+        code: Cell<Option<i32>>,
         stdout: RefCell<String>,
     }
 
     impl Systemctl for &'static Spy {
         fn run(&self, args: &[&str]) -> Result<SystemctlReply> {
             self.calls.borrow_mut().push(args.iter().map(|arg| arg.to_string()).collect());
-            Ok(SystemctlReply { success: self.success.get(), stdout: self.stdout.borrow().clone() })
+            Ok(SystemctlReply { success: self.success.get(), code: self.code.get(), stdout: 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), stdout: RefCell::default() }));
+        let spy: &'static Spy =
+            Box::leak(Box::new(Spy { calls: RefCell::default(), success: Cell::new(true), code: Cell::new(Some(0)), stdout: RefCell::default() }));
         (SystemdLauncher::over(Box::new(spy)), spy)
     }
 
@@ -1126,6 +1168,45 @@ mod tests {
         assert_eq!(spy.calls.borrow()[0], vec!["is-active", "mind-self@eureka.service"]);
     }
 
+    #[test]
+    fn stop_args_is_one_synchronous_stop_scoped_to_one_instance() {
+        let args = stop_args(&eureka());
+        assert_eq!(args, ["stop", "--", "mind-self@eureka:*.service", "mind-persona@eureka:*.service"]);
+        // The text before each glob star must not be a prefix of another instance's unit.
+        for (pattern, other) in [(&args[2], "mind-self@eureka-two:x.service"), (&args[3], "mind-persona@eureka-two:x.service")] {
+            assert!(!other.starts_with(pattern.split('*').next().unwrap()), "{pattern}");
+        }
+    }
+
+    #[test]
+    fn stop_all_runs_the_one_stop_and_waits() {
+        let (launcher, spy) = spied();
+        launcher.stop_all(&eureka()).unwrap();
+        assert_eq!(*spy.calls.borrow(), vec![stop_args(&eureka())]);
+    }
+
+    #[test]
+    fn stop_all_with_an_invalid_instance_is_an_error_and_runs_no_systemctl() {
+        let (launcher, spy) = spied();
+        for bad in ["canary-../evil-7f3a91", "canary instance", "", "canary*"] {
+            let error = launcher.stop_all(&Slug(bad.into())).unwrap_err();
+            assert!(!format!("{error:#}").contains("canary"), "{error:#}");
+        }
+        assert!(spy.calls.borrow().is_empty());
+    }
+
+    #[test]
+    fn a_failed_stop_is_an_error_naming_the_exit_status_and_no_instance() {
+        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.code.set(None);
+        let shown = format!("{:#}", launcher.stop_all(&eureka()).unwrap_err());
+        assert!(shown.contains("signal"), "{shown}");
+    }
+
     #[test]
     fn a_failed_start_names_no_instance() {
         let (launcher, spy) = spied();

crates/mind-body/src/main.rs

@@ -32,6 +32,7 @@ fn main() {
         &set_by,
         chrono::Utc::now(),
         &mut std::io::stdout(),
+        &mut |instance| mind_body::launch::SystemdLauncher::new().stop_all(instance),
     );
     if let Err(error) = result {
         eprintln!("mind-control: {error:#}");

crates/mind-body/tests/cli.rs

@@ -3,14 +3,32 @@
 //! status. Nothing touches `/etc`.
 
 use chrono::{TimeZone, Utc};
+use eureka_pipeline::Slug;
 use mind_body::cli::{operator_name, run};
 
 fn go(root: &std::path::Path, args: &[&str], set_by: &str, secs: i64) -> (anyhow::Result<()>, String) {
+    let (result, out, _) = go_stopping(root, args, set_by, secs, |_| Ok(()));
+    (result, out)
+}
+
+/// Runs one command with a stopper that records the instance of every call and
+/// answers with `stop`. `stop` also sees the call before it returns.
+fn go_stopping(
+    root: &std::path::Path,
+    args: &[&str],
+    set_by: &str,
+    secs: i64,
+    mut stop: impl FnMut(&Slug) -> anyhow::Result<()>,
+) -> (anyhow::Result<()>, String, Vec<String>) {
     let args: Vec<String> = args.iter().map(|a| a.to_string()).collect();
     let mut out = Vec::new();
+    let mut calls = Vec::new();
     let now = Utc.timestamp_opt(1_800_000_000 + secs, 0).unwrap();
-    let result = run(&args, root, set_by, now, &mut out);
-    (result, String::from_utf8(out).unwrap())
+    let result = run(&args, root, set_by, now, &mut out, &mut |instance| {
+        calls.push(instance.0.clone());
+        stop(instance)
+    });
+    (result, String::from_utf8(out).unwrap(), calls)
 }
 
 #[test]
@@ -49,13 +67,12 @@ fn instances_do_not_share_a_store() {
 fn a_dial_set_outside_bounds_refuses_and_leaves_the_store_unchanged() {
     let root = tempfile::tempdir().unwrap();
     go(root.path(), &["--instance", "eureka", "brake", "release"], "op", 0).0.unwrap();
-    go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 1)
-        .0
-        .unwrap();
+    go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 1).0.unwrap();
     let path = root.path().join("eureka").join("control.cc");
     let before = std::fs::read(&path).unwrap();
     for heat in ["2.0001", "0.0499", "0", "-1"] {
-        let (r, _) = go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", heat, "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 9);
+        let (r, _) =
+            go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", heat, "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 9);
         assert!(r.is_err(), "heat {heat} must be refused");
         assert_eq!(std::fs::read(&path).unwrap(), before, "heat {heat} changed the store");
     }
@@ -134,3 +151,75 @@ fn a_root_flag_without_a_directory_or_an_instance_is_refused_and_writes_nothing(
     assert_eq!(std::fs::read_dir(default.path()).unwrap().count(), 0);
     assert!(!std::path::Path::new("/nonexistent-root-for-test").exists());
 }
+
+fn shown(root: &std::path::Path) -> String {
+    go(root, &["--instance", "eureka", "show"], "op", 0).1
+}
+
+#[test]
+fn brake_hold_stops_once_with_the_instance_and_the_store_already_held() {
+    let root = tempfile::tempdir().unwrap();
+    go(root.path(), &["--instance", "eureka", "brake", "release"], "op", 0).0.unwrap();
+    let mut seen_at_stop = String::new();
+    let (result, out, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "alice", 1, |_| {
+        seen_at_stop = shown(root.path());
+        Ok(())
+    });
+    result.unwrap();
+    assert_eq!(calls, ["eureka"]);
+    assert!(seen_at_stop.contains("BrakeHeld"), "the write comes before the stop: {seen_at_stop}");
+    assert!(out.contains("units: stopped mind-self@eureka:* mind-persona@eureka:*"), "{out}");
+}
+
+#[test]
+fn only_brake_hold_calls_the_stopper() {
+    let root = tempfile::tempdir().unwrap();
+    let dial = ["--instance", "eureka", "dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"];
+    let commands: &[&[&str]] = &[
+        &["--instance", "eureka", "brake", "release"],
+        &dial,
+        &["--instance", "eureka", "show"],
+        &["--instance", "eureka", "brake"],
+        &["--instance", "eureka", "brake", "toggle"],
+        &["--instance", "eureka", "dial", "set", "--heat", "99", "--base-cooldown-s", "60", "--base-run-usd", "1"],
+        &["--instance", "../evil", "brake", "release"],
+        &["--instance", "eureka", "brake", "hold", "now"],
+        &[],
+    ];
+    for args in commands {
+        let (_, _, calls) = go_stopping(root.path(), args, "op", 0, |_| Ok(()));
+        assert!(calls.is_empty(), "{args:?} called the stopper");
+    }
+    let (_, _, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 1, |_| Ok(()));
+    assert_eq!(calls.len(), 1);
+}
+
+#[test]
+fn a_failing_stopper_fails_the_hold_and_leaves_the_brake_held() {
+    let root = tempfile::tempdir().unwrap();
+    go(root.path(), &["--instance", "eureka", "brake", "release"], "op", 0).0.unwrap();
+    let (result, out, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 1, |_| {
+        Err(anyhow::anyhow!("systemctl stop failed with exit status 5"))
+    });
+    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!(!out.contains("stopped"), "a failed stop printed success: {out}");
+    assert!(shown(root.path()).contains("BrakeHeld"));
+}
+
+#[test]
+fn a_failing_write_still_stops_and_the_hold_fails() {
+    let root = tempfile::tempdir().unwrap();
+    std::fs::create_dir_all(root.path().join("eureka").join("control.cc")).unwrap();
+    let (result, out, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 1, |_| Ok(()));
+    let error = format!("{:#}", result.unwrap_err());
+    assert_eq!(calls, ["eureka"], "stopping does not wait on the write");
+    assert!(error.contains("brake was not written"), "{error}");
+    assert!(!out.contains("stopped"), "{out}");
+
+    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}");
+}

crates/mind-body/tests/control.rs

@@ -328,9 +328,14 @@ mod unsafe_installs {
             .unwrap();
         let bin = env!("CARGO_BIN_EXE_mind-control");
         for round in 0..100 {
-            let verb = if round % 2 == 0 { "release" } else { "hold" };
+            let verb: &[&str] = if round % 2 == 0 {
+                &["brake", "release"]
+            } else {
+                &["dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"]
+            };
             let status = Command::new("sh")
-                .args(["-c", "umask 000; exec \"$@\"", "sh", bin, "--root", minds.to_str().unwrap(), "--instance", &instance, "brake", verb])
+                .args(["-c", "umask 000; exec \"$@\"", "sh", bin, "--root", minds.to_str().unwrap(), "--instance", &instance])
+                .args(verb)
                 .status()
                 .unwrap();
             assert!(status.success());