fix(maestro): always probe tmux during recovery
#164 skipped the tmux probe whenever the stored boot id differed from the current one, assuming a changed boot id proves the tmux server is gone. It does not: a restored state directory, a VM or container snapshot rollback, or a truncated boot_id file all change it while tmux keeps running with live sessions. Every one of those sessions was then registered Dormant and stranded — resume respawns `maestro-<id>`, which collides with the name tmux still holds, so it failed 502 forever. Recovery now always runs `tmux list-sessions`, the single source of truth for liveness the rest of the daemon assumes. The boot id keeps its other job: telling a reboot from a restart in recovery's log. The #164 test asserted the old behaviour with tmux still alive, so it is replaced by one asserting the session is adopted Running, plus one covering a real reboot — boot id changed and the tmux server dead — still going dormant. Refs #174
This commit is contained in:
@@ -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;
|
||||
|
||||
|
||||
@@ -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<usize, SessionError> {
|
||||
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,
|
||||
|
||||
@@ -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]
|
||||
|
||||
+7
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user