diff --git a/crates/maestro-daemon/src/boot_id.rs b/crates/maestro-daemon/src/boot_id.rs new file mode 100644 index 0000000..9106815 --- /dev/null +++ b/crates/maestro-daemon/src/boot_id.rs @@ -0,0 +1,62 @@ +//! Tells a daemon restart from a host reboot (#164). +//! +//! `/proc/sys/kernel/random/boot_id` is fixed for the life of the kernel and +//! changes on every boot. [`SessionManager::recover`] compares it against the +//! value it stored on the previous run: a difference means every session from +//! before is provably dead, so the tmux probe can be skipped and those +//! sessions registered dormant directly instead of relying on tmux reporting +//! an empty list. + +use std::path::Path; + +/// Where the daemon reads the running kernel's boot id. Behind a trait so +/// tests can simulate a changed (or unreadable) boot id without rebooting the +/// machine. +pub trait BootIdSource: std::fmt::Debug + Send + Sync + 'static { + /// Current boot id, or `None` if it can't be determined (non-Linux, + /// restricted `/proc`, missing or empty file). + fn current(&self) -> Option; +} + +/// Production source: `/proc/sys/kernel/random/boot_id`. +#[derive(Debug, Default)] +pub struct ProcBootId; + +impl BootIdSource for ProcBootId { + fn current(&self) -> Option { + read_boot_id(Path::new("/proc/sys/kernel/random/boot_id")) + } +} + +fn read_boot_id(path: &Path) -> Option { + let contents = std::fs::read_to_string(path).ok()?; + let trimmed = contents.trim(); + (!trimmed.is_empty()).then(|| trimmed.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reads_trimmed_contents() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("boot_id"); + std::fs::write(&path, "abc-123\n").unwrap(); + assert_eq!(read_boot_id(&path).as_deref(), Some("abc-123")); + } + + #[test] + fn missing_file_returns_none() { + let dir = tempfile::tempdir().unwrap(); + assert_eq!(read_boot_id(&dir.path().join("missing")), None); + } + + #[test] + fn empty_file_returns_none() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("boot_id"); + std::fs::write(&path, "\n").unwrap(); + assert_eq!(read_boot_id(&path), None); + } +} diff --git a/crates/maestro-daemon/src/cli.rs b/crates/maestro-daemon/src/cli.rs index c61a5e9..4f6e13e 100644 --- a/crates/maestro-daemon/src/cli.rs +++ b/crates/maestro-daemon/src/cli.rs @@ -83,7 +83,11 @@ async fn run(config_override: Option) -> Result<(), Box> { .with_spawn_history(crate::spawn_history::SpawnHistory::new( config.spawn_history_path(), config.spawn_history_days, - )); + )) + .with_boot_id( + config.boot_id_path(), + std::sync::Arc::new(crate::boot_id::ProcBootId), + ); // 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 ef70e73..e78e483 100644 --- a/crates/maestro-daemon/src/config.rs +++ b/crates/maestro-daemon/src/config.rs @@ -112,6 +112,12 @@ impl DaemonConfig { pub fn spawn_history_path(&self) -> PathBuf { self.state_dir.join("spawn-history.jsonl") } + + /// Where recovery records the kernel boot id it saw last (#164): + /// `/boot_id`. + pub fn boot_id_path(&self) -> PathBuf { + self.state_dir.join("boot_id") + } } /// Env-var overrides applied by [`load_config`]. Tests pass explicit diff --git a/crates/maestro-daemon/src/lib.rs b/crates/maestro-daemon/src/lib.rs index 6d67cea..c4f8031 100644 --- a/crates/maestro-daemon/src/lib.rs +++ b/crates/maestro-daemon/src/lib.rs @@ -12,6 +12,7 @@ pub mod attach; /// Naming a session after its first prompt (#131). #[cfg(feature = "haiku-autoname")] pub mod autoname; +pub mod boot_id; pub mod branches; pub mod cli; pub mod config; diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 2cf7a0b..cc8492b 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -19,6 +19,7 @@ use maestro_protocol::{ use tokio::sync::{broadcast, Mutex}; use uuid::Uuid; +use crate::boot_id::BootIdSource; use crate::panes::Panes; use crate::pty::{pipe_pane_command, PtyBridge}; use crate::sidecar::{SidecarError, SidecarStore}; @@ -179,6 +180,25 @@ struct Inner { /// client that reconnects never collides with the registration its /// previous socket has not finished releasing. next_viewport: AtomicU64, + /// Boot-id tracking for `recover` (#164). `None` — the default — means + /// reboot detection is unwired, which behaves exactly like a boot id + /// that could never be read: always fall back to probing tmux. + boot_id: Option, +} + +/// Where recovery persists the boot id it last saw, and how it reads the +/// current one. See [`crate::boot_id`]. +struct BootIdConfig { + path: PathBuf, + source: Arc, +} + +impl std::fmt::Debug for BootIdConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("BootIdConfig") + .field("path", &self.path) + .finish_non_exhaustive() + } } impl SessionManager { @@ -221,6 +241,7 @@ impl SessionManager { events, viewports: Mutex::new(Viewports::new()), next_viewport: AtomicU64::new(0), + boot_id: None, }), } } @@ -250,6 +271,21 @@ impl SessionManager { self } + /// Wire boot-id tracking for `recover` (#164). Builder, called once right + /// after construction like the other `with_*` methods. Without it, + /// `recover` always falls back to probing tmux — the same as a boot id + /// that could never be read. + /// + /// # Panics + /// + /// If the handle has already been cloned. + pub fn with_boot_id(mut self, path: PathBuf, source: Arc) -> Self { + let inner = Arc::get_mut(&mut self.inner) + .expect("with_boot_id must be called before the SessionManager is cloned"); + inner.boot_id = Some(BootIdConfig { path, source }); + 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 @@ -665,9 +701,21 @@ impl SessionManager { /// and start a `PtyBridge`. Sessions whose sidecar references a `cwd` /// that no longer exists are still adopted — the spec does not require /// us to repair them. + /// + /// #164: when the kernel's boot id has changed since the last run, the + /// tmux probe below is skipped entirely — a reboot kills the tmux server, + /// so every previous session is already dead and there is nothing tmux + /// could truthfully adopt. [`Self::register_dormant`] then registers all + /// of them directly. pub async fn recover(&self) -> Result { tokio::fs::create_dir_all(&self.inner.fifo_dir).await?; - let live = { + let rebooted = self.check_boot_id().await; + let live = if rebooted { + // #164: every session from the previous boot is provably dead — + // no tmux server survives a reboot — so there is nothing to gain + // from asking it and no session it could honestly report back. + Vec::new() + } else { let tmux = self.inner.tmux.clone(); tmux_blocking(move || tmux.list_sessions()).await? }; @@ -737,10 +785,39 @@ impl SessionManager { .send(CatalogEvent::SessionAdded { session }); adopted += 1; } - self.register_dormant().await; + self.register_dormant(rebooted).await; Ok(adopted) } + /// Compare the running kernel's boot id against the one recovery stored + /// last time (#164), then persist the current one for next time. Returns + /// `true` only when both are known and differ — a host reboot. + /// + /// A missing stored value (first run) or an unreadable current one + /// (non-Linux, restricted `/proc`, unwired in this build) both read as + /// "no reboot", which is today's behaviour: fall back to probing tmux. + async fn check_boot_id(&self) -> bool { + let Some(cfg) = &self.inner.boot_id else { + return false; + }; + let Some(current) = cfg.source.current() else { + return false; + }; + let stored = tokio::fs::read_to_string(&cfg.path) + .await + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()); + let rebooted = stored.is_some_and(|s| s != current); + if let Some(parent) = cfg.path.parent() { + let _ = tokio::fs::create_dir_all(parent).await; + } + if let Err(e) = tokio::fs::write(&cfg.path, ¤t).await { + tracing::warn!(error = ?e, path = %cfg.path.display(), "recover: failed to persist boot id"); + } + rebooted + } + /// Second recovery pass (#162): register every sidecar tmux had nothing to /// adopt as a [`SessionStatus::Dormant`] session. /// @@ -761,7 +838,12 @@ impl SessionManager { /// nothing can be resumed in a directory that is gone, so the honest move /// is to delete the sidecar rather than register a record no operator can /// act on. A sidecar whose `cwd` still exists is kept regardless of age. - async fn register_dormant(&self) { + /// + /// `rebooted` (#164) only changes what gets logged: a detected reboot + /// names the count even when it is zero, so the operator sees that + /// recovery told a reboot from a restart instead of a log that stays + /// silent either way. + async fn register_dormant(&self, rebooted: bool) { let stored = match self.inner.sidecar.load_all() { Ok(stored) => stored, Err(e) => { @@ -812,7 +894,12 @@ impl SessionManager { registered.push(session); } } - if !registered.is_empty() { + if rebooted { + tracing::info!( + dormant = registered.len(), + "host rebooted, sessions dormant" + ); + } else if !registered.is_empty() { tracing::info!( dormant = registered.len(), "registered sessions with no live tmux as dormant" diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index e5d8c58..08db711 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -297,6 +297,159 @@ async fn recover_keeps_dormant_sidecar_when_cwd_exists() { assert_eq!(snap[0].status, SessionStatus::Dormant); } +/// #164: a fake [`maestro_daemon::boot_id::BootIdSource`] lets tests simulate +/// a changed (or unreadable) boot id without rebooting the machine. +#[derive(Debug)] +struct FakeBootId(Option); + +impl maestro_daemon::boot_id::BootIdSource for FakeBootId { + fn current(&self) -> Option { + self.0.clone() + } +} + +fn revive_with_boot_id( + fx: &DaemonFixture, + boot_id_path: PathBuf, + current: Option<&str>, +) -> maestro_daemon::SessionManager { + maestro_daemon::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_boot_id( + boot_id_path, + Arc::new(FakeBootId(current.map(String::from))), + ) +} + +/// #164: a changed boot id means every session from the previous boot is +/// already dead, so recovery must not trust tmux to say so — it skips the +/// probe outright. Proven by leaving the tmux session alive: if the probe +/// ran, it would come back `Running`, not `Dormant`. +#[tokio::test] +async fn recover_skips_tmux_probe_when_boot_id_changed() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + let s = fx + .manager + .create(CreateSessionRequest { + cwd: cwd.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create"); + + let boot_id_path = fx.state_dir.path().join("boot_id"); + std::fs::write(&boot_id_path, "previous-boot").unwrap(); + + // tmux session is still alive — a probe would find it if one ran. + let revived = revive_with_boot_id(&fx, boot_id_path.clone(), Some("current-boot")); + let adopted = revived.recover().await.expect("recover"); + assert_eq!(adopted, 0, "reboot detected: nothing adopted live"); + + let snap = revived.snapshot().await; + assert_eq!(snap.len(), 1); + assert_eq!(snap[0].id, s.id); + assert_eq!(snap[0].status, SessionStatus::Dormant); + + // The current boot id is now the stored one, for the next comparison. + assert_eq!( + std::fs::read_to_string(&boot_id_path).unwrap().trim(), + "current-boot" + ); +} + +/// #164: an unchanged boot id is a plain restart — recovery probes tmux as +/// before and adopts the still-live session `Running`. +#[tokio::test] +async fn recover_probes_tmux_when_boot_id_unchanged() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + let s = fx + .manager + .create(CreateSessionRequest { + cwd: cwd.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create"); + + let boot_id_path = fx.state_dir.path().join("boot_id"); + std::fs::write(&boot_id_path, "same-boot").unwrap(); + + let revived = revive_with_boot_id(&fx, boot_id_path, Some("same-boot")); + let adopted = revived.recover().await.expect("recover"); + assert_eq!(adopted, 1); + let snap = revived.snapshot().await; + assert_eq!(snap[0].id, s.id); + assert_eq!(snap[0].status, SessionStatus::Running); +} + +/// #164: no stored boot id (first run ever, or an older build that never +/// wrote one) means there is nothing to compare against — falls back to +/// today's probe-based recovery rather than assuming a reboot. +#[tokio::test] +async fn recover_falls_back_to_probe_when_no_stored_boot_id() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + fx.manager + .create(CreateSessionRequest { + cwd: cwd.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create"); + + let boot_id_path = fx.state_dir.path().join("boot_id"); + assert!(!boot_id_path.exists()); + + let revived = revive_with_boot_id(&fx, boot_id_path.clone(), Some("current-boot")); + let adopted = revived.recover().await.expect("recover"); + assert_eq!(adopted, 1, "no stored boot id; falls back to probing tmux"); + assert_eq!( + std::fs::read_to_string(&boot_id_path).unwrap().trim(), + "current-boot" + ); +} + +/// #164: an unreadable current boot id (non-Linux, restricted `/proc`) falls +/// back to today's behaviour rather than failing startup or assuming a +/// reboot. +#[tokio::test] +async fn recover_falls_back_to_probe_when_boot_id_unreadable() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + fx.manager + .create(CreateSessionRequest { + cwd: cwd.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create"); + + let boot_id_path = fx.state_dir.path().join("boot_id"); + std::fs::write(&boot_id_path, "stored-boot").unwrap(); + + let revived = revive_with_boot_id(&fx, boot_id_path, None); + let adopted = revived.recover().await.expect("recover"); + assert_eq!(adopted, 1, "boot id unreadable; falls back to probing tmux"); +} + /// #162: dismissing a dormant session is the one thing that can be done to it /// — no tmux call to fail, and the sidecar goes with it so the next recovery /// does not resurrect it.