diff --git a/crates/maestro-config/src/lib.rs b/crates/maestro-config/src/lib.rs index f7817c7..c8f2835 100644 --- a/crates/maestro-config/src/lib.rs +++ b/crates/maestro-config/src/lib.rs @@ -129,6 +129,80 @@ pub struct DaemonSection { /// `Some(0)` disables history entirely (no log written, no recents read). #[serde(default)] pub spawn_history_days: Option, + /// What recovery does with sessions that outlived their tmux server — + /// after a host reboot, or any restart that finds a sidecar with no pane + /// (#167). Default [`ResumeOnBoot::Dormant`]. + #[serde(default, deserialize_with = "deserialize_resume_on_boot")] + pub resume_on_boot: Option, +} + +impl DaemonSection { + /// The configured `[daemon].resume_on_boot`, or the safe default: + /// recovered-but-dead sessions wait for the operator. + pub fn resume_on_boot(&self) -> ResumeOnBoot { + self.resume_on_boot.unwrap_or_default() + } +} + +/// `[daemon].resume_on_boot` — what recovery does with a session whose tmux +/// pane is gone but whose sidecar is still on disk (#167). +/// +/// Resuming agents unattended seconds after a reboot is a foot-gun: they run +/// with whatever flags they were spawned with, with nobody watching. So the +/// default waits for a human, and `auto` is opted into per host. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum ResumeOnBoot { + /// Register each one `Dormant` and wait for the operator to resume it. + #[default] + Dormant, + /// Resume each one during recovery, on the same path a client's + /// `POST /api/sessions/:id/resume` takes. + Auto, + /// Prune the sidecars instead: a reboot is a clean slate. + Never, +} + +/// Every `[daemon].resume_on_boot` name and its variant, in documentation +/// order. +pub const RESUME_ON_BOOT_POLICIES: &[(&str, ResumeOnBoot)] = &[ + ("dormant", ResumeOnBoot::Dormant), + ("auto", ResumeOnBoot::Auto), + ("never", ResumeOnBoot::Never), +]; + +impl ResumeOnBoot { + /// The name this policy is spelled with in the config file and in logs. + pub fn as_str(&self) -> &'static str { + match self { + ResumeOnBoot::Dormant => "dormant", + ResumeOnBoot::Auto => "auto", + ResumeOnBoot::Never => "never", + } + } +} + +fn deserialize_resume_on_boot<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let raw = Option::::deserialize(deserializer)?; + raw.map(|name| { + RESUME_ON_BOOT_POLICIES + .iter() + .find(|(n, _)| *n == name) + .map(|(_, policy)| *policy) + .ok_or_else(|| { + let known = RESUME_ON_BOOT_POLICIES + .iter() + .map(|(n, _)| format!("{n:?}")) + .collect::>() + .join(", "); + D::Error::custom(format!( + "invalid [daemon].resume_on_boot {name:?}; expected one of {known}" + )) + }) + }) + .transpose() } /// `[tui]` — `maestro tui` settings (NAP-275 epic, phase 2 NAP-277). @@ -775,4 +849,27 @@ bind_addr = "127.0.0.1:9090" ); assert_eq!(cfg.resolve_daemon_url(None), "http://explicit:1"); } + + #[test] + fn resume_on_boot_defaults_to_dormant_and_parses_every_policy() { + assert_eq!( + MaestroConfigFile::default().daemon.resume_on_boot(), + ResumeOnBoot::Dormant, + "an absent policy is the safe one" + ); + for (name, expected) in RESUME_ON_BOOT_POLICIES { + let cfg = file_with(&format!("[daemon]\nresume_on_boot = \"{name}\"\n")); + assert_eq!(cfg.daemon.resume_on_boot(), *expected, "{name}"); + assert_eq!(expected.as_str(), *name); + } + } + + #[test] + fn resume_on_boot_rejects_an_unknown_policy() { + let err = toml::from_str::("[daemon]\nresume_on_boot = \"maybe\"\n") + .expect_err("unknown policy must not silently fall back"); + let msg = err.to_string(); + assert!(msg.contains("resume_on_boot"), "{msg}"); + assert!(msg.contains("\"auto\""), "{msg}"); + } } diff --git a/crates/maestro-daemon/src/cli.rs b/crates/maestro-daemon/src/cli.rs index 4f6e13e..35fb078 100644 --- a/crates/maestro-daemon/src/cli.rs +++ b/crates/maestro-daemon/src/cli.rs @@ -87,7 +87,10 @@ async fn run(config_override: Option) -> Result<(), Box> { .with_boot_id( config.boot_id_path(), std::sync::Arc::new(crate::boot_id::ProcBootId), - ); + ) + // #167: recovery needs the presets to resolve a resumed session's command, + // so `auto` runs the same resume a client's POST would. + .with_resume_policy(config.resume_on_boot, config.spawn_presets.clone()); // Spawned sessions learn where to report back (#103). Derived from the // configured bind address; set before any spawn can happen. manager.set_daemon_url(format!("http://{}", config.bind_addr)); diff --git a/crates/maestro-daemon/src/config.rs b/crates/maestro-daemon/src/config.rs index e78e483..ffd849a 100644 --- a/crates/maestro-daemon/src/config.rs +++ b/crates/maestro-daemon/src/config.rs @@ -97,6 +97,10 @@ pub struct DaemonConfig { /// never binds it itself; it serves it so the web client and the TUI read /// one knob. pub leader: String, + /// What recovery does with sessions whose tmux pane is gone but whose + /// sidecar survives (#167): `[daemon].resume_on_boot`, default + /// [`maestro_config::ResumeOnBoot::Dormant`]. + pub resume_on_boot: maestro_config::ResumeOnBoot, } impl DaemonConfig { @@ -149,6 +153,8 @@ pub fn load_config(path: &Path, env: EnvOverrides) -> Result Result, + /// What recovery does with a session whose pane is gone (#167), and the + /// presets an `auto` resume resolves its command from. Defaults to + /// [`ResumeOnBoot::Dormant`] with no presets: register and wait. + resume_policy: ResumeOnBoot, + resume_presets: Vec, } /// Where recovery persists the boot id it last saw, and how it reads the @@ -253,6 +259,8 @@ impl SessionManager { viewports: Mutex::new(Viewports::new()), next_viewport: AtomicU64::new(0), boot_id: None, + resume_policy: ResumeOnBoot::default(), + resume_presets: Vec::new(), }), } } @@ -297,6 +305,26 @@ impl SessionManager { self } + /// Wire the `[daemon].resume_on_boot` policy (#167) and the spawn presets + /// an `auto` resume resolves each session's command from. Builder, called + /// once right after construction like the other `with_*` methods. Without + /// it, recovery behaves as [`ResumeOnBoot::Dormant`]. + /// + /// # Panics + /// + /// If the handle has already been cloned. + pub fn with_resume_policy( + mut self, + policy: ResumeOnBoot, + presets: Vec, + ) -> Self { + let inner = Arc::get_mut(&mut self.inner) + .expect("with_resume_policy must be called before the SessionManager is cloned"); + inner.resume_policy = policy; + inner.resume_presets = presets; + self + } + /// Set the daemon's own HTTP base URL (#103), exported to every session /// spawned afterwards as `MAESTRO_URL`. Unlike the `with_*` builders this /// is a plain setter: the daemon runner learns its bound address after @@ -970,6 +998,7 @@ impl SessionManager { }; let mut registered = Vec::new(); let mut pruned = 0usize; + let mut discarded = 0usize; { let mut sessions = self.inner.sessions.lock().await; for mut session in stored { @@ -984,6 +1013,16 @@ impl SessionManager { pruned += 1; continue; } + if self.inner.resume_policy == ResumeOnBoot::Never { + // #167: this host wants a reboot to be a clean slate, so + // the sidecar goes the way #163's does — nothing is + // registered and the next recovery finds nothing. + if let Err(e) = self.inner.sidecar.delete(session.id) { + tracing::warn!(error = ?e, id = %session.id, "recover: failed to prune sidecar under resume_on_boot=never"); + } + discarded += 1; + continue; + } let id = session.id; session.status = SessionStatus::Dormant; session.foreground = None; @@ -1025,12 +1064,45 @@ impl SessionManager { if pruned > 0 { tracing::info!(pruned, "pruned sidecars whose cwd no longer exists"); } + let resumable: Vec = registered.iter().map(|s| s.id).collect(); for session in registered { let _ = self .inner .events .send(CatalogEvent::SessionAdded { session }); } + + // #167: say which policy ran and how many sessions it acted on, so a + // host that resumes agents unattended says so in its own log. The + // match is exhaustive on purpose: a fourth policy must decide what it + // does here rather than inherit whichever arm happened to be last. + match self.inner.resume_policy { + ResumeOnBoot::Dormant => tracing::info!( + policy = ResumeOnBoot::Dormant.as_str(), + sessions = resumable.len(), + "resume_on_boot: sessions left dormant for the operator" + ), + ResumeOnBoot::Never => tracing::info!( + policy = ResumeOnBoot::Never.as_str(), + sessions = discarded, + "resume_on_boot: pruned the sidecars of sessions with no live pane" + ), + ResumeOnBoot::Auto => { + tracing::info!( + policy = ResumeOnBoot::Auto.as_str(), + sessions = resumable.len(), + "resume_on_boot: resuming sessions with no live pane" + ); + let presets = self.inner.resume_presets.clone(); + for id in resumable { + // The same call the client's POST /api/sessions/:id/resume + // makes — one resume path, not two. + if let Err(e) = self.resume(id, &presets).await { + tracing::warn!(error = %e, id = %id, "resume_on_boot=auto: resume failed; session stays dormant"); + } + } + } + } } /// Forward keystrokes to a session's pane. A dead or dormant session is diff --git a/crates/maestro-daemon/tests/resume_policy.rs b/crates/maestro-daemon/tests/resume_policy.rs new file mode 100644 index 0000000..9ced0e4 --- /dev/null +++ b/crates/maestro-daemon/tests/resume_policy.rs @@ -0,0 +1,161 @@ +//! `[daemon].resume_on_boot` (#167): what recovery does with sessions whose +//! tmux pane is gone but whose sidecar survived, against real tmux. +//! +//! The default is deliberately the timid one. Resuming agents unattended +//! seconds after a reboot is a foot-gun, so `auto` is opted into per host and +//! `never` throws the sidecars away instead. + +mod common; + +use std::collections::HashMap; +use std::sync::Arc; +use std::time::Duration; + +use common::{wait_for_async, DaemonFixture}; +use maestro_config::ResumeOnBoot; +use maestro_daemon::SessionManager; +use maestro_protocol::{CreateSessionRequest, Session, SessionStatus, SpawnPreset}; +use tempfile::TempDir; + +/// A fresh manager over the same tmux server, sidecars and fifos — what the +/// daemon builds after a restart — carrying `policy`. +fn revive(fx: &DaemonFixture, policy: ResumeOnBoot, presets: Vec) -> SessionManager { + SessionManager::new( + Arc::new(fx.tmux.tmux.clone()), + maestro_daemon::sidecar::SidecarStore::new(fx.sidecar_dir()), + maestro_daemon::worktree::WorktreeManager::new(None), + fx.fifo_dir(), + ) + .with_resume_policy(policy, presets) +} + +/// A preset whose `resume_command` touches a marker file, so a test can see +/// that the resumed session ran the resume command rather than the original. +fn marker_preset(command: &str, marker_dir: &TempDir) -> Vec { + vec![SpawnPreset { + id: "agent".into(), + label: "Agent".into(), + command: command.into(), + resume_command: Some(format!( + "sh -c 'touch {}/resumed; sleep 30'", + marker_dir.path().display() + )), + }] +} + +async fn create(fx: &DaemonFixture, cwd: &TempDir, command: &str) -> Session { + fx.manager + .create(CreateSessionRequest { + cwd: cwd.path().to_path_buf(), + command: command.into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create") +} + +/// The default: a reboot leaves the session listed but not running, waiting +/// for the operator. +#[tokio::test] +async fn dormant_is_the_default_and_leaves_the_session_waiting() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + let session = create(&fx, &cwd, "sleep 30").await; + fx.tmux.tmux.kill_server().expect("kill tmux server"); + + let revived = revive(&fx, ResumeOnBoot::Dormant, Vec::new()); + revived.recover().await.expect("recover"); + + assert_eq!( + revived.status(session.id).await, + Some(SessionStatus::Dormant) + ); + assert!(fx + .sidecar_dir() + .join(format!("{}.json", session.id)) + .exists()); + assert!( + fx.tmux.tmux.list_sessions().expect("list").is_empty(), + "nothing was respawned" + ); +} + +/// `auto` brings each one back itself, down the same path a client's resume +/// takes: same uuid, same sidecar, the preset's `resume_command`. +#[tokio::test] +async fn auto_resumes_every_dormant_session_during_recovery() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + let markers = TempDir::new().unwrap(); + let session = create(&fx, &cwd, "sleep 30").await; + fx.tmux.tmux.kill_server().expect("kill tmux server"); + + let revived = revive(&fx, ResumeOnBoot::Auto, marker_preset("sleep 30", &markers)); + revived.recover().await.expect("recover"); + + assert_eq!( + revived.status(session.id).await, + Some(SessionStatus::Running), + "auto resumed it" + ); + assert_eq!( + fx.tmux.tmux.list_sessions().expect("list"), + vec![session.id], + "back under the same uuid" + ); + let stored = maestro_daemon::sidecar::SidecarStore::new(fx.sidecar_dir()) + .read(session.id) + .expect("read sidecar") + .expect("same sidecar"); + assert_eq!(stored.created_at, session.created_at); + assert_eq!(stored.command, "sleep 30", "the original command is kept"); + + // It really ran the preset's resume command, not the spawn command. + let marker = markers.path().join("resumed"); + wait_for_async(Duration::from_secs(5), || { + let marker = marker.clone(); + async move { marker.exists() } + }) + .await; + assert!(marker.exists(), "resume_command ran"); +} + +/// `never` makes a reboot a clean slate: the sidecars go, and the session is +/// not listed at all. +#[tokio::test] +async fn never_prunes_the_sidecars_instead() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + let session = create(&fx, &cwd, "sleep 30").await; + fx.tmux.tmux.kill_server().expect("kill tmux server"); + + let revived = revive(&fx, ResumeOnBoot::Never, Vec::new()); + revived.recover().await.expect("recover"); + + assert!(!revived.has(session.id).await, "not listed"); + assert!( + !fx.sidecar_dir() + .join(format!("{}.json", session.id)) + .exists(), + "sidecar pruned" + ); + assert!(fx.tmux.tmux.list_sessions().expect("list").is_empty()); +} + +/// The policy only decides what happens to sessions that lost their pane. A +/// session tmux still holds is adopted running by the first pass under every +/// policy — including `never`, which must not delete a live session's sidecar. +#[tokio::test] +async fn the_policy_leaves_sessions_tmux_still_holds_alone() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + let live = create(&fx, &cwd, "sleep 30").await; + + let revived = revive(&fx, ResumeOnBoot::Never, Vec::new()); + assert_eq!(revived.recover().await.expect("recover"), 1); + + assert_eq!(revived.status(live.id).await, Some(SessionStatus::Running)); + assert!(fx.sidecar_dir().join(format!("{}.json", live.id)).exists()); +} diff --git a/docs/INSTALL.md b/docs/INSTALL.md index 803cae4..7e79980 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -243,6 +243,26 @@ Pick `/tmp` if the daemon must outlive your login session and you can't run tmux sockets with it). The env var `MAESTRO_DAEMON_TMUX_TMPDIR` takes precedence over the file. +## What happens to sessions after a reboot + +A host reboot kills the tmux server, so every session the daemon knew is dead +while its record on disk survives. What recovery does with those is +`[daemon].resume_on_boot`: + +```toml +[daemon] +resume_on_boot = "dormant" # "dormant" (default) | "auto" | "never" +``` + +- `dormant` — the sessions are listed but not running, and you resume the ones + you want from the web UI or the TUI. +- `auto` — the daemon resumes them all itself during startup. Only pick this + on a host where restarting every agent unattended is safe: they come back on + the flags they were spawned with, with nobody watching. +- `never` — the records are deleted and a reboot is a clean slate. + +See `docs/RESTART.md` for the full contract. + ## What the systemd unit looks like The wizard writes a *user* unit (`systemctl --user`), since `maestro daemon` diff --git a/docs/RESTART.md b/docs/RESTART.md index d9026d6..2d8aa8f 100644 --- a/docs/RESTART.md +++ b/docs/RESTART.md @@ -96,6 +96,38 @@ neither is something a respawn could fix. The other thing to do with a dormant session is dismiss it, which removes the sidecar so the next recovery does not bring it back. +## What recovery does with them: `resume_on_boot` + +Whether a recovered-but-dead session is left dormant at all is the operator's +call, per host, via `[daemon].resume_on_boot` (#167): + +| value | what recovery does | +| ----------- | --------------------------------------------------------------------- | +| `"dormant"` | **Default.** Register each one dormant and wait for the operator. | +| `"auto"` | Resume each one during recovery, itself. | +| `"never"` | Prune the sidecars instead — a reboot is a clean slate. | + +`dormant` is the default because resuming agents unattended is a foot-gun: +they come back on whatever flags they were spawned with, seconds after a +reboot, with nobody watching. `auto` exists for hosts where that is known to +be safe, and it is opted into rather than inherited. + +`auto` calls the same `resume` the client's `POST /api/sessions/:id/resume` +does — same uuid, same sidecar, the preset's `resume_command` — so there is +one resume path and not two. A session that fails to resume is logged and +stays dormant; the rest still come back. + +`never` prunes the same way #163 does for a missing `cwd`: the sidecar is +deleted and nothing is registered, so the session is not listed and the next +recovery does not find it. Whatever worktree it owned is left on disk. + +The policy only decides the fate of sessions with no live pane. A session +tmux still holds is adopted `Running` by the first pass under every policy, +including `never`. + +Recovery logs the policy that ran and the number of sessions it acted on, so +a host that resumes agents unattended says so in its own journal. + Attaching to a session that has no bridge answers `409 not_attachable`, not `404 unknown_session`: the session exists, and a client that cannot tell "gone" from "not running" cannot offer the resume.