feat(cli): aoe session restart --all (#910)

* feat(cli): add --all flag scaffold to restart subcommand

Introduces RestartArgs with optional identifier, --all switch
(mutually exclusive with identifier), and --parallel concurrency
cap (default 3). Orchestrator is stubbed; only arg parsing is
wired up. Per njbrake's spec in #855.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* feat(cli): sequential restart-all orchestrator

Implements `aoe session restart --all`:
- pick_targets_for_restart_all() skips Deleting/Creating sessions
  (mid-transition; restarting them races their existing state machine).
- Orchestrator loads sessions, mutates Vec<Instance> in memory,
  saves once at end. No per-instance persistence inside the loop —
  this matters when we add concurrency next.
- Per-instance failures collected and surfaced; exit code reflects
  failure count.

Concurrency cap (`--parallel`) accepted but currently unused; sequential
loop is a known-good baseline before adding the semaphore-bounded
parallel path.

Tests cover the filter (Deleting/Creating excluded, all other states
included) and arg parsing (already in place).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* feat(cli): bound restart-all concurrency with semaphore + JoinSet

Replaces the sequential loop with a parallel pool gated by
tokio::sync::Semaphore (default 3, configurable via --parallel).
Each target is cloned into its worker; the worker calls
restart_with_size on a blocking thread (since restart_with_size
spawns processes and sleeps), and the mutated copy is written
back to instances[idx] when the worker returns.

The shared Vec<Instance> is never touched by workers, only by
the dispatcher loop, and sessions.json is still saved exactly
once after every worker has joined. This is what njbrake's #855
spec called out: don't persist per-instance inside a parallel
pool, or reads-and-writes will race.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs(cli): regenerate reference for restart --all

Auto-generated by `cargo xtask gen-docs` after adding --all and
--parallel flags to `aoe session restart`. CI enforces this is
in sync with clap help.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(cli): rehydrate source_profile before session restart

Instance::source_profile is #[serde(skip_serializing)], so storage-loaded
instances always come back with it blank. Every other load site
(start_session in this file, the TUI dashboard, the web server) repopulates
it from the active profile right after Storage::load_with_groups so that
start_with_size_opts can resolve sandbox.environment and on_launch hooks
from the correct profile. The single-session restart path was missed when
PR #812 fixed the analogous web bug, and the new restart_all_sessions
inherited that miss.

Without this, `aoe -p foo session restart [<id>|--all]` against a profile
that is not the user's globally configured default falls back to the
default profile's sandbox env vars and on-launch hooks (cf.
build_docker_env_args -> resolved_sandbox_config -> effective_profile,
plus Instance::effective_profile in start_with_size_opts). The
test_build_docker_env_args_uses_passed_profile_not_global_default
regression test in src/session/environment.rs documents the same class
of bug from the web flow ("personal profile's GH_TOKEN was ignored when
launching from the web app").

Fixes the latent miss in restart_session and the new bug in
restart_all_sessions in one place.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: njbrake <nathan@mozilla.ai>
Co-authored-by: Nathan Brake (Mozilla.ai) <33383515+njbrake@users.noreply.github.com>

Author: 3 people

docs/cli/reference.md

@@ -209,7 +209,7 @@ Manage session lifecycle (start, stop, attach, etc.)
 
 * `start` — Start a session's tmux process
 * `stop` — Stop session process
-* `restart` — Restart session
+* `restart` — Restart session (or all sessions with `--all`)
 * `attach` — Attach to session interactively
 * `show` — Show session details
 * `rename` — Rename a session
@@ -245,13 +245,20 @@ Stop session process
 
 ## `aoe session restart`
 
-Restart session
+Restart session (or all sessions with `--all`)
 
-**Usage:** `aoe session restart <IDENTIFIER>`
+**Usage:** `aoe session restart [OPTIONS] [IDENTIFIER]`
 
 ###### **Arguments:**
 
-* `<IDENTIFIER>` — Session ID or title
+* `<IDENTIFIER>` — Session ID or title (required unless `--all` is passed)
+
+###### **Options:**
+
+* `--all` — Restart every session in the active profile. Useful after `aoe update`, after editing `sandbox.environment`, after a Docker hiccup, or after changing a hook. Mutually exclusive with `identifier`
+* `--parallel <PARALLEL>` — Concurrency cap for `--all`. Restarting many sandboxed sessions in parallel pressures dockerd, so the default is intentionally modest. Ignored when `--all` is not set
+
+  Default value: `3`
 
 
 

src/cli/session.rs

@@ -14,8 +14,8 @@ pub enum SessionCommands {
     /// Stop session process
     Stop(SessionIdArgs),
 
-    /// Restart session
-    Restart(SessionIdArgs),
+    /// Restart session (or all sessions with `--all`)
+    Restart(RestartArgs),
 
     /// Attach to session interactively
     Attach(SessionIdArgs),
@@ -42,6 +42,25 @@ pub struct SessionIdArgs {
     identifier: String,
 }
 
+#[derive(Args)]
+pub struct RestartArgs {
+    /// Session ID or title (required unless `--all` is passed)
+    pub identifier: Option<String>,
+
+    /// Restart every session in the active profile. Useful after
+    /// `aoe update`, after editing `sandbox.environment`, after a
+    /// Docker hiccup, or after changing a hook. Mutually exclusive
+    /// with `identifier`.
+    #[arg(long, conflicts_with = "identifier")]
+    pub all: bool,
+
+    /// Concurrency cap for `--all`. Restarting many sandboxed
+    /// sessions in parallel pressures dockerd, so the default is
+    /// intentionally modest. Ignored when `--all` is not set.
+    #[arg(long, default_value_t = 3)]
+    pub parallel: usize,
+}
+
 #[derive(Args)]
 pub struct RenameArgs {
     /// Session ID or title (optional, auto-detects in tmux)
@@ -131,7 +150,7 @@ pub async fn run(profile: &str, command: SessionCommands) -> Result<()> {
     match command {
         SessionCommands::Start(args) => start_session(profile, args).await,
         SessionCommands::Stop(args) => stop_session(profile, args).await,
-        SessionCommands::Restart(args) => restart_session(profile, args).await,
+        SessionCommands::Restart(args) => restart_session_dispatch(profile, args).await,
         SessionCommands::Attach(args) => attach_session(profile, args).await,
         SessionCommands::Show(args) => show_session(profile, args).await,
         SessionCommands::Capture(args) => capture_session(profile, args).await,
@@ -205,6 +224,123 @@ async fn stop_session(profile: &str, args: SessionIdArgs) -> Result<()> {
     Ok(())
 }
 
+async fn restart_session_dispatch(profile: &str, args: RestartArgs) -> Result<()> {
+    if args.all {
+        return restart_all_sessions(profile, args.parallel).await;
+    }
+    let identifier = args
+        .identifier
+        .ok_or_else(|| anyhow::anyhow!("session identifier required (or pass --all)"))?;
+    restart_session(profile, SessionIdArgs { identifier }).await
+}
+
+async fn restart_all_sessions(profile: &str, parallel: usize) -> Result<()> {
+    let storage = Storage::new(profile)?;
+    let (mut instances, groups) = storage.load_with_groups()?;
+
+    let target_ids = pick_targets_for_restart_all(&instances);
+    if target_ids.is_empty() {
+        println!("No sessions to restart in profile '{}'.", profile);
+        return Ok(());
+    }
+
+    let total = target_ids.len();
+    let size = crate::terminal::get_size();
+    let parallel = parallel.max(1);
+
+    // Clone each target into its worker; we'll write the (mutated) copy back
+    // by index after the worker returns. Workers never touch the shared Vec.
+    // `source_profile` is runtime-only (skip_serializing) so storage-loaded
+    // instances always come back blank; rehydrate it from the storage profile
+    // so start-time config resolution honors the right profile's overrides
+    // (sandbox.environment, on_launch hooks, etc.).
+    let mut targets: Vec<(usize, crate::session::Instance)> = Vec::with_capacity(total);
+    for id in &target_ids {
+        if let Some(idx) = instances.iter().position(|i| &i.id == id) {
+            let mut clone = instances[idx].clone();
+            clone.source_profile = profile.to_string();
+            targets.push((idx, clone));
+        }
+    }
+
+    let semaphore = std::sync::Arc::new(tokio::sync::Semaphore::new(parallel));
+    let mut join_set: tokio::task::JoinSet<(
+        usize,
+        String,
+        Option<crate::session::Instance>,
+        Result<()>,
+    )> = tokio::task::JoinSet::new();
+
+    for (idx, mut inst) in targets {
+        let permit_sem = semaphore.clone();
+        join_set.spawn(async move {
+            let _permit = permit_sem
+                .acquire_owned()
+                .await
+                .expect("semaphore not closed");
+            let title = inst.title.clone();
+            let res = tokio::task::spawn_blocking(move || {
+                let result = inst.restart_with_size(size);
+                (inst, result)
+            })
+            .await;
+            match res {
+                Ok((inst, result)) => (idx, title, Some(inst), result),
+                Err(join_err) => (
+                    idx,
+                    title,
+                    None,
+                    Err(anyhow::anyhow!("worker panicked: {}", join_err)),
+                ),
+            }
+        });
+    }
+
+    let mut succeeded: Vec<String> = Vec::new();
+    let mut failed: Vec<(String, String)> = Vec::new();
+
+    while let Some(joined) = join_set.join_next().await {
+        let (idx, title, inst_opt, result) =
+            joined.expect("JoinSet shouldn't panic on join itself");
+        if let Some(inst) = inst_opt {
+            instances[idx] = inst;
+        }
+        match result {
+            Ok(()) => succeeded.push(title),
+            Err(e) => failed.push((title, e.to_string())),
+        }
+    }
+
+    let group_tree = GroupTree::new_with_groups(&instances, &groups);
+    storage.save_with_groups(&instances, &group_tree)?;
+
+    println!("✓ Restarted {}/{} sessions:", succeeded.len(), total);
+    for title in &succeeded {
+        println!("  · {}", title);
+    }
+    if !failed.is_empty() {
+        println!("✗ {} failed:", failed.len());
+        for (title, err) in &failed {
+            println!("  · {}: {}", title, err);
+        }
+        bail!("{} session(s) failed to restart", failed.len());
+    }
+
+    Ok(())
+}
+
+/// Sessions in `Deleting` or `Creating` are mid-transition; restarting them
+/// would race the deletion/boot path. Everything else is fair game; agents
+/// have their own resume-or-restart logic on the next start.
+fn pick_targets_for_restart_all(instances: &[crate::session::Instance]) -> Vec<String> {
+    use crate::session::Status;
+    instances
+        .iter()
+        .filter(|i| !matches!(i.status, Status::Deleting | Status::Creating))
+        .map(|i| i.id.clone())
+        .collect()
+}
+
 async fn restart_session(profile: &str, args: SessionIdArgs) -> Result<()> {
     let storage = Storage::new(profile)?;
     let (mut instances, groups) = storage.load_with_groups()?;
@@ -218,6 +354,10 @@ async fn restart_session(profile: &str, args: SessionIdArgs) -> Result<()> {
         })
         .ok_or_else(|| anyhow::anyhow!("Session not found: {}", args.identifier))?;
 
+    // `source_profile` is runtime-only (skip_serializing) so storage-loaded
+    // instances always come back blank; rehydrate it from the storage profile
+    // so restart-time config resolution honors the right profile's overrides.
+    instances[idx].source_profile = profile.to_string();
     instances[idx].restart_with_size(crate::terminal::get_size())?;
     let title = instances[idx].title.clone();
 
@@ -540,3 +680,110 @@ async fn set_session_id(profile: &str, args: SetSessionIdArgs) -> Result<()> {
     }
     Ok(())
 }
+
+#[cfg(test)]
+mod restart_args_tests {
+    use super::SessionCommands;
+    use clap::Parser;
+
+    #[derive(Parser)]
+    struct Cli {
+        #[command(subcommand)]
+        cmd: SessionCommands,
+    }
+
+    #[test]
+    fn restart_with_identifier_still_parses() {
+        let cli = Cli::try_parse_from(["aoe", "restart", "claude-3"])
+            .expect("identifier-only must parse");
+        match cli.cmd {
+            SessionCommands::Restart(args) => {
+                assert!(!args.all);
+                assert_eq!(args.identifier.as_deref(), Some("claude-3"));
+                assert_eq!(args.parallel, 3);
+            }
+            _ => panic!("wrong subcommand"),
+        }
+    }
+
+    #[test]
+    fn restart_all_alone_parses() {
+        let cli = Cli::try_parse_from(["aoe", "restart", "--all"]).expect("--all alone must parse");
+        match cli.cmd {
+            SessionCommands::Restart(args) => {
+                assert!(args.all);
+                assert!(args.identifier.is_none());
+                assert_eq!(args.parallel, 3);
+            }
+            _ => panic!("wrong subcommand"),
+        }
+    }
+
+    #[test]
+    fn restart_all_with_parallel_parses() {
+        let cli = Cli::try_parse_from(["aoe", "restart", "--all", "--parallel", "5"])
+            .expect("--all --parallel must parse");
+        match cli.cmd {
+            SessionCommands::Restart(args) => {
+                assert!(args.all);
+                assert_eq!(args.parallel, 5);
+            }
+            _ => panic!("wrong subcommand"),
+        }
+    }
+
+    #[test]
+    fn restart_identifier_and_all_conflicts() {
+        let result = Cli::try_parse_from(["aoe", "restart", "claude-3", "--all"]);
+        assert!(
+            result.is_err(),
+            "passing both identifier and --all should error"
+        );
+    }
+}
+
+#[cfg(test)]
+mod target_filter_tests {
+    use super::pick_targets_for_restart_all;
+    use crate::session::{Instance, Status};
+
+    fn instance_with_status(id: &str, status: Status) -> Instance {
+        let mut inst = Instance::new(id, "/tmp");
+        inst.id = id.to_string();
+        inst.status = status;
+        inst
+    }
+
+    #[test]
+    fn skips_deleting_and_creating() {
+        let instances = vec![
+            instance_with_status("running", Status::Running),
+            instance_with_status("idle", Status::Idle),
+            instance_with_status("stopped", Status::Stopped),
+            instance_with_status("error", Status::Error),
+            instance_with_status("waiting", Status::Waiting),
+            instance_with_status("starting", Status::Starting),
+            instance_with_status("unknown", Status::Unknown),
+            instance_with_status("deleting", Status::Deleting),
+            instance_with_status("creating", Status::Creating),
+        ];
+        let mut picked = pick_targets_for_restart_all(&instances);
+        picked.sort();
+        let mut expected = vec![
+            "error".to_string(),
+            "idle".to_string(),
+            "running".to_string(),
+            "starting".to_string(),
+            "stopped".to_string(),
+            "unknown".to_string(),
+            "waiting".to_string(),
+        ];
+        expected.sort();
+        assert_eq!(picked, expected);
+    }
+
+    #[test]
+    fn empty_input_yields_empty_targets() {
+        assert!(pick_targets_for_restart_all(&[]).is_empty());
+    }
+}