diff --git a/crates/maestro-daemon/src/boot_id.rs b/crates/maestro-daemon/src/boot_id.rs index 9106815..4d47690 100644 --- a/crates/maestro-daemon/src/boot_id.rs +++ b/crates/maestro-daemon/src/boot_id.rs @@ -2,10 +2,13 @@ //! //! `/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. +//! value it stored on the previous run and logs which of the two it was. +//! +//! It does not decide liveness (#174). A boot id changes without the tmux +//! server dying whenever the state directory is restored, a VM or container +//! snapshot is rolled back, or the `boot_id` file is truncated — so recovery +//! always probes `tmux list-sessions` and the boot id only says whether a +//! server that did not answer is coming back. use std::path::Path; diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index e4fbb25..f66b72d 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -847,20 +847,20 @@ impl SessionManager { /// 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. + /// #164/#174: the kernel's boot id says whether the host rebooted since + /// the last run, but it never decides which sessions are alive — + /// `tmux list-sessions` does, always, which is what the rest of the + /// daemon assumes. A boot id can change without the tmux server dying + /// (a restored state directory, a VM or container snapshot restore, a + /// truncated `boot_id` file); skipping the probe then registered every + /// live session `Dormant`, and resume could never bring one back because + /// the tmux name was still taken. The boot id is now only a signal that a + /// tmux server which did not answer is not coming back, and it names + /// itself in recovery's log. pub async fn recover(&self) -> Result { tokio::fs::create_dir_all(&self.inner.fifo_dir).await?; 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 live = { let tmux = self.inner.tmux.clone(); tmux_blocking(move || tmux.list_sessions()).await? }; @@ -940,7 +940,9 @@ impl SessionManager { /// /// 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. + /// "no reboot". Either way the tmux probe runs (#174): the answer only + /// tells recovery whether a tmux server that did not answer is coming + /// back, never which sessions are alive. async fn check_boot_id(&self) -> bool { let Some(cfg) = &self.inner.boot_id else { return false; @@ -987,7 +989,8 @@ impl SessionManager { /// `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. + /// silent either way. It never decides membership — everything here got + /// here by tmux not holding it (#174). async fn register_dormant(&self, rebooted: bool) { let stored = match self.inner.sidecar.load_all() { Ok(stored) => stored, diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 4cb39a4..8cde9e8 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -326,12 +326,14 @@ fn revive_with_boot_id( ) } -/// #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`. +/// #174: a boot id can change without the tmux server dying — a restored +/// state directory, a VM or container snapshot restore, a truncated +/// `boot_id` file. `tmux list-sessions` stays the single source of truth for +/// liveness, so a session tmux still holds is adopted `Running` no matter +/// what the boot id says. Registering it dormant instead stranded it: resume +/// would collide with the tmux name still in use and fail forever. #[tokio::test] -async fn recover_skips_tmux_probe_when_boot_id_changed() { +async fn recover_adopts_live_session_when_boot_id_changed() { let fx = DaemonFixture::new(); let cwd = TempDir::new().unwrap(); let s = fx @@ -349,15 +351,15 @@ async fn recover_skips_tmux_probe_when_boot_id_changed() { 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. + // tmux session is still alive: the probe runs and finds it. 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"); + assert_eq!(adopted, 1, "tmux still holds the session; adopt it"); let snap = revived.snapshot().await; assert_eq!(snap.len(), 1); assert_eq!(snap[0].id, s.id); - assert_eq!(snap[0].status, SessionStatus::Dormant); + assert_eq!(snap[0].status, SessionStatus::Running); // The current boot id is now the stored one, for the next comparison. assert_eq!( @@ -366,6 +368,38 @@ async fn recover_skips_tmux_probe_when_boot_id_changed() { ); } +/// #164, still true after #174: a real reboot takes the tmux server with it, +/// so the probe comes back empty and every sidecar is registered dormant. +#[tokio::test] +async fn recover_registers_dormant_when_boot_id_changed_and_tmux_gone() { + 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(); + fx.tmux.tmux.kill_server().expect("kill tmux server"); + + let revived = revive_with_boot_id(&fx, boot_id_path, Some("current-boot")); + let adopted = revived.recover().await.expect("recover"); + assert_eq!(adopted, 0, "no tmux server survived the reboot"); + + let snap = revived.snapshot().await; + assert_eq!(snap.len(), 1); + assert_eq!(snap[0].id, s.id); + assert_eq!(snap[0].status, SessionStatus::Dormant); +} + /// #164: an unchanged boot id is a plain restart — recovery probes tmux as /// before and adopts the still-live session `Running`. #[tokio::test] diff --git a/docs/RESTART.md b/docs/RESTART.md index 2d8aa8f..60e46e5 100644 --- a/docs/RESTART.md +++ b/docs/RESTART.md @@ -75,7 +75,13 @@ A daemon restart leaves tmux alone, so recovery re-adopts every beyond the reconnect above. A **host reboot** is different: it kills the tmux server, so there is nothing -to adopt while every session's sidecar is still on disk. Recovery makes a +to adopt while every session's sidecar is still on disk. Recovery always asks +tmux — `tmux list-sessions` is the single source of truth for liveness, and the +kernel's `boot_id` only tells a reboot from a restart in the log (#164, #174). +A boot id changes without the tmux server dying whenever the state directory is +restored, a VM or container snapshot is rolled back, or the `boot_id` file is +truncated; a session tmux still holds is adopted `Running` in every one of +those cases. Recovery makes a second pass over those sidecars and registers each as a `Dormant` session (#162) — id, name, cwd, command, worktree and `created_at` intact. A dormant session has no pane and no PTY bridge: it cannot be attached to, typed into or