test(maestro): cover signal shutdown drain

Cargo.lock

@@ -1558,6 +1558,7 @@ version = "0.1.0"
 dependencies = [
  "chrono",
  "futures-util",
+ "libc",
  "maestro-protocol",
  "maestro-testing",
  "maestro-tui",

crates/maestro-daemon/src/cli.rs

@@ -122,21 +122,23 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
     // shutdown, and both must reach the adapter children below (#134). The
     // unit's `KillMode=process` signals only the daemon, so nothing else
     // would end them.
-    let shutdown = async {
+    let shutdown_manager = manager.clone();
+    let shutdown = async move {
         let mut sigterm = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())
             .expect("install SIGTERM handler");
         tokio::select! {
             _ = tokio::signal::ctrl_c() => tracing::info!("ctrl-c received — shutting down"),
             _ = sigterm.recv() => tracing::info!("SIGTERM received — shutting down"),
         }
+        // Axum waits for upgraded WebSockets to drain. Structured children
+        // must be settled before that wait, while the signal path still owns
+        // the shutdown order (#179).
+        shutdown_manager.shutdown().await;
     };
 
     axum::serve(listener, router)
         .tcp_nodelay(true)
         .with_graceful_shutdown(shutdown)
         .await?;
-    // tmux sessions outlive the daemon by design; structured sessions'
-    // adapter children do not (#134).
-    manager.shutdown().await;
     Ok(())
 }

crates/maestro-daemon/src/session.rs

@@ -7,7 +7,7 @@
 
 use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
-use std::sync::atomic::{AtomicU64, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
 use std::sync::Arc;
 use std::time::Duration;
 
@@ -195,6 +195,9 @@ struct Inner {
     /// client that reconnects never collides with the registration its
     /// previous socket has not finished releasing.
     next_viewport: AtomicU64,
+    /// Once daemon shutdown starts, it owns structured-session finalization.
+    /// Background passes must not classify those deliberate exits as natural.
+    shutting_down: AtomicBool,
 }
 
 impl SessionManager {
@@ -237,6 +240,7 @@ impl SessionManager {
                 events,
                 viewports: Mutex::new(Viewports::new()),
                 next_viewport: AtomicU64::new(0),
+                shutting_down: AtomicBool::new(false),
             }),
         }
     }
@@ -349,13 +353,17 @@ impl SessionManager {
     /// is on disk after the restart is truthful. tmux sessions are left
     /// alone — surviving a restart is what they are for.
     pub async fn shutdown(&self) {
+        if self.inner.shutting_down.swap(true, Ordering::AcqRel) {
+            return;
+        }
         let children: Vec<(Uuid, Arc<AcpSession>, Option<watch::Receiver<bool>>)> = {
             let sessions = self.inner.sessions.lock().await;
             sessions
                 .iter()
                 .filter_map(|(id, r)| {
-                    r.acp
-                        .clone()
+                    (r.session.status == SessionStatus::Running)
+                        .then_some(())
+                        .and_then(|()| r.acp.clone())
                         .map(|acp| (*id, acp, r.acp_event_pump.clone()))
                 })
                 .collect()
@@ -374,7 +382,7 @@ impl SessionManager {
             .map(wait_for_event_pump);
         futures_util::future::join_all(drains).await;
         for (id, _, _) in children {
-            mark_session_dead(&self.inner, id).await;
+            mark_session_dead_after_shutdown(&self.inner, id).await;
         }
     }
 
@@ -1438,6 +1446,9 @@ async fn probe_worktree(
 /// One pass of the liveness watcher: diff the in-memory session set against
 /// the tmux-reported one, and reap anything we still track but tmux has lost.
 async fn run_liveness_pass(inner: &Arc<Inner>) {
+    if inner.shutting_down.load(Ordering::Acquire) {
+        return;
+    }
     // Snapshot the in-memory ids first; if we own no sessions there is
     // nothing to reap and no reason to spawn a tmux subprocess (this also
     // avoids racing with the very first `new-session` that would otherwise
@@ -1497,6 +1508,9 @@ async fn run_liveness_pass(inner: &Arc<Inner>) {
 /// `SessionUpdated` only when `activity` or `foreground` actually changes — so a
 /// steady state produces no diffs. Runs on the existing liveness tick.
 async fn run_activity_pass(inner: &Arc<Inner>) {
+    if inner.shutting_down.load(Ordering::Acquire) {
+        return;
+    }
     let ids: Vec<Uuid> = {
         let sessions = inner.sessions.lock().await;
         if sessions.is_empty() {
@@ -1539,6 +1553,9 @@ async fn run_activity_pass(inner: &Arc<Inner>) {
 
         let updated = {
             let mut sessions = inner.sessions.lock().await;
+            if inner.shutting_down.load(Ordering::Acquire) {
+                return;
+            }
             let Some(rec) = sessions.get_mut(&id) else {
                 continue;
             };
@@ -1613,6 +1630,9 @@ async fn run_activity_pass(inner: &Arc<Inner>) {
 /// error must not read as "nothing blocks". Like the activity pass, this only
 /// emits `SessionUpdated` when the value actually changed.
 async fn run_blocked_work_pass(inner: &Arc<Inner>) {
+    if inner.shutting_down.load(Ordering::Acquire) {
+        return;
+    }
     let targets: Vec<(Uuid, (PathBuf, PathBuf))> = {
         let sessions = inner.sessions.lock().await;
         sessions
@@ -1640,6 +1660,9 @@ async fn run_blocked_work_pass(inner: &Arc<Inner>) {
         };
         let updated = {
             let mut sessions = inner.sessions.lock().await;
+            if inner.shutting_down.load(Ordering::Acquire) {
+                return;
+            }
             let Some(rec) = sessions.get_mut(&id) else {
                 continue;
             };
@@ -1656,7 +1679,7 @@ async fn run_blocked_work_pass(inner: &Arc<Inner>) {
     }
 }
 
-/// Mark a session whose tmux session has disappeared as [`SessionStatus::Dead`]
+/// Mark a session whose backing process has disappeared as [`SessionStatus::Dead`]
 /// and settle its worktree per the natural-exit rule (#82).
 ///
 /// The record, its [`PtyBridge`] and its sidecar all survive, so a client can
@@ -1668,9 +1691,27 @@ async fn run_blocked_work_pass(inner: &Arc<Inner>) {
 /// alone and stays attached to the dead record for a later forced dismissal
 /// (#81), which leaves it on disk too.
 ///
-/// Dead sessions do not survive a daemon restart: [`SessionManager::recover`]
-/// adopts only what tmux still holds.
+/// Terminal sessions that die naturally do not survive a daemon restart:
+/// [`SessionManager::recover`] adopts only what tmux still holds. Structured
+/// sessions stopped for daemon shutdown retain their sidecars for ACP recovery.
 async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
+    if inner.shutting_down.load(Ordering::Acquire) {
+        return;
+    }
+    mark_session_dead_with_worktree_settlement(inner, id, true).await;
+}
+
+/// A daemon restart terminates ACP children, but it is not a natural adapter
+/// exit: the sidecar is the recovery inventory, including its worktree.
+async fn mark_session_dead_after_shutdown(inner: &Arc<Inner>, id: Uuid) {
+    mark_session_dead_with_worktree_settlement(inner, id, false).await;
+}
+
+async fn mark_session_dead_with_worktree_settlement(
+    inner: &Arc<Inner>,
+    id: Uuid,
+    settle_worktree: bool,
+) {
     let flipped = {
         let mut sessions = inner.sessions.lock().await;
         match sessions.get_mut(&id) {
@@ -1694,17 +1735,19 @@ async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
     };
     tracing::info!(%id, "session process gone — marking dead");
 
-    if let Some(wt) = &worktree {
-        if settle_worktree_on_exit(inner, id, wt).await {
-            // The tree is settled (removed); a stale "in the way" would
-            // describe something the session no longer holds.
-            session.blocked_work = None;
-            let mut sessions = inner.sessions.lock().await;
-            if let Some(record) = sessions.get_mut(&id) {
-                record.session.worktree = None;
-                record.session.blocked_work = None;
-                record.worktree_path = None;
-                record.worktree_base_repo = None;
+    if settle_worktree {
+        if let Some(wt) = &worktree {
+            if settle_worktree_on_exit(inner, id, wt).await {
+                // The tree is settled (removed); a stale "in the way" would
+                // describe something the session no longer holds.
+                session.blocked_work = None;
+                let mut sessions = inner.sessions.lock().await;
+                if let Some(record) = sessions.get_mut(&id) {
+                    record.session.worktree = None;
+                    record.session.blocked_work = None;
+                    record.worktree_path = None;
+                    record.worktree_base_repo = None;
+                }
             }
         }
     }

crates/maestro-daemon/tests/acp.rs

@@ -9,13 +9,15 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
-use common::{wait_for_async, DaemonFixture};
+use common::{init_bare_repo, init_repo, push_all_to_origin, wait_for_async, DaemonFixture};
 use maestro_acp::schema::{ContentBlock, SessionUpdate};
 use maestro_acp::{ADAPTER_BIN, INSTALL_COMMAND};
 use maestro_daemon::sidecar::{EventCursor, SidecarStore};
 use maestro_daemon::worktree::WorktreeManager;
 use maestro_daemon::SessionManager;
-use maestro_protocol::{ActivityState, CreateSessionRequest, SessionStatus, SessionView};
+use maestro_protocol::{
+    ActivityState, CreateSessionRequest, SessionStatus, SessionView, WorktreeSpec,
+};
 use tempfile::TempDir;
 
 fn fake_agent() -> PathBuf {
@@ -285,6 +287,76 @@ async fn recovery_loads_the_acp_session_without_duplicating_history() {
     revived.kill(session.id, false).await.expect("kill");
 }
 
+#[tokio::test]
+async fn shutdown_preserves_a_pristine_structured_worktree_for_recovery() {
+    let fx = DaemonFixture::new();
+    let fake = Fake::new();
+    let repo = TempDir::new().unwrap();
+    init_repo(repo.path());
+    let origin = TempDir::new().unwrap();
+    init_bare_repo(origin.path());
+
+    let branch = "feature/restart";
+    let worktree = repo
+        .path()
+        .join("worktrees")
+        .join("feature")
+        .join("restart");
+    let session = fx
+        .manager
+        .create(CreateSessionRequest {
+            view: SessionView::Structured,
+            cwd: repo.path().to_path_buf(),
+            command: "claude".into(),
+            env: fake.env(),
+            metadata: HashMap::new(),
+            worktree: Some(WorktreeSpec {
+                base_repo: repo.path().to_path_buf(),
+                branch: branch.into(),
+                create_branch: true,
+            }),
+        })
+        .await
+        .expect("create");
+    assert_eq!(session.cwd, worktree);
+    push_all_to_origin(repo.path(), origin.path());
+
+    fx.manager.shutdown().await;
+
+    assert!(
+        worktree.is_dir(),
+        "shutdown must not settle a pristine worktree"
+    );
+    let sidecar: maestro_protocol::Session = serde_json::from_slice(
+        &std::fs::read(fx.sidecar_dir().join(format!("{}.json", session.id))).unwrap(),
+    )
+    .unwrap();
+    assert_eq!(
+        sidecar.worktree.as_ref().map(|wt| &wt.path),
+        Some(&worktree)
+    );
+
+    let revived = SessionManager::new(
+        Arc::new(fx.tmux.tmux.clone()),
+        SidecarStore::new(fx.sidecar_dir()),
+        WorktreeManager::new(None),
+        fx.fifo_dir(),
+    );
+    assert_eq!(revived.recover().await.expect("recover"), 1);
+    assert_eq!(
+        revived.status(session.id).await,
+        Some(SessionStatus::Running)
+    );
+    assert!(fake.log().contains("recv session/load"), "{}", fake.log());
+    assert!(worktree.is_dir(), "recovery must retain the worktree");
+
+    revived.kill(session.id, false).await.expect("dismiss");
+    assert!(
+        !worktree.exists(),
+        "operator dismissal still cleans a pristine tree"
+    );
+}
+
 #[tokio::test]
 async fn failed_recovery_cleans_its_fifo() {
     let fx = DaemonFixture::new();

crates/maestro-e2e/Cargo.toml

@@ -13,6 +13,7 @@ path = "src/lib.rs"
 [dependencies]
 chrono.workspace = true
 futures-util.workspace = true
+libc.workspace = true
 maestro-protocol = { workspace = true, features = ["client"] }
 maestro-testing.workspace = true
 serde.workspace = true

crates/maestro-e2e/src/daemon.rs

@@ -87,6 +87,14 @@ impl Daemon {
         }
     }
 
+    /// Send the daemon the same SIGTERM systemd uses, without waiting for its
+    /// graceful HTTP shutdown to finish.
+    pub fn send_sigterm(&self) {
+        let child = self.child.as_ref().expect("daemon is running");
+        let result = unsafe { libc::kill(child.id() as libc::pid_t, libc::SIGTERM) };
+        assert_eq!(result, 0, "send SIGTERM to daemon");
+    }
+
     /// Re-spawn the daemon on the same port + state. The previous tmux
     /// server (under `TMUX_TMPDIR`) is left running so `recover()` can adopt
     /// its sessions.

crates/maestro-e2e/tests/e2e.rs

@@ -15,7 +15,7 @@ use std::time::{Duration, Instant};
 use futures_util::{SinkExt, StreamExt};
 use maestro_e2e::util::wait_for_async;
 use maestro_e2e::{Daemon, TestClient};
-use maestro_protocol::{CreateSessionRequest, WorktreeSpec};
+use maestro_protocol::{CreateSessionRequest, SessionStatus, SessionView, WorktreeSpec};
 use tempfile::TempDir;
 use tokio_tungstenite::tungstenite::Message as WsMessage;
 
@@ -36,6 +36,61 @@ fn create_req(cwd: &std::path::Path, command: &str) -> CreateSessionRequest {
     }
 }
 
+struct FakeAcp {
+    _dir: TempDir,
+    env: HashMap<String, String>,
+}
+
+fn fake_acp(log: &std::path::Path) -> FakeAcp {
+    let dir = tempfile::tempdir().expect("fake adapter dir");
+    let adapter = dir.path().join("claude-code-acp");
+    std::fs::copy(
+        concat!(
+            env!("CARGO_MANIFEST_DIR"),
+            "/../maestro-acp/tests/fake_acp_agent.py"
+        ),
+        &adapter,
+    )
+    .expect("copy fake adapter");
+    let mut permissions = std::fs::metadata(&adapter)
+        .expect("adapter metadata")
+        .permissions();
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        permissions.set_mode(0o755);
+    }
+    std::fs::set_permissions(&adapter, permissions).expect("make fake adapter executable");
+    let env = HashMap::from([
+        (
+            "PATH".into(),
+            format!(
+                "{}:{}",
+                dir.path().display(),
+                std::env::var("PATH").unwrap_or_default()
+            ),
+        ),
+        ("FAKE_ACP_LOG".into(), log.to_string_lossy().into_owned()),
+    ]);
+    FakeAcp { _dir: dir, env }
+}
+
+fn fake_acp_pid(log: &std::path::Path) -> Option<u32> {
+    std::fs::read_to_string(log).ok()?.lines().find_map(|line| {
+        line.strip_prefix("start pid=")?
+            .split(' ')
+            .next()?
+            .parse()
+            .ok()
+    })
+}
+
+fn pid_alive(pid: u32) -> bool {
+    std::fs::read_to_string(format!("/proc/{pid}/stat"))
+        .map(|stat| !stat.contains(") Z "))
+        .unwrap_or(false)
+}
+
 /// A worktree spec built through its wire shape. The daemon and clients ship
 /// together (#87 dropped `cleanup_on_kill` from the struct), so the JSON
 /// object carries only the fields the lifecycle actually honours.
@@ -995,6 +1050,66 @@ async fn daemon_restart_recovers_session() {
     client.kill_session(s.id).await.expect("kill");
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn sigterm_stops_structured_child_before_events_ws_drains() {
+    let (mut daemon, client) = bring_up().await;
+    let cwd = TempDir::new().unwrap();
+    let log = cwd.path().join("fake-acp.log");
+    let fake = fake_acp(&log);
+    let session = client
+        .spawn_session(&CreateSessionRequest {
+            view: SessionView::Structured,
+            cwd: cwd.path().to_path_buf(),
+            command: "claude".into(),
+            env: fake.env.clone(),
+            metadata: HashMap::new(),
+            worktree: None,
+        })
+        .await
+        .expect("spawn structured session");
+    let ws_url = daemon.client().events_url().expect("events URL");
+    let (_events_ws, _) = tokio_tungstenite::connect_async(ws_url)
+        .await
+        .expect("events WS handshake");
+    wait_for_async("fake ACP starts", Duration::from_secs(5), || {
+        let log = log.clone();
+        async move { fake_acp_pid(&log).is_some() }
+    })
+    .await;
+    let pid = fake_acp_pid(&log).expect("fake ACP pid");
+
+    daemon.send_sigterm();
+
+    wait_for_async(
+        "fake ACP exits after SIGTERM",
+        Duration::from_secs(5),
+        || async { !pid_alive(pid) },
+    )
+    .await;
+    let sidecar_path = daemon
+        .state_dir()
+        .join("sessions")
+        .join(format!("{}.json", session.id));
+    wait_for_async(
+        "dead structured session sidecar",
+        Duration::from_secs(5),
+        || {
+            let sidecar_path = sidecar_path.clone();
+            async move {
+                std::fs::read(&sidecar_path)
+                    .ok()
+                    .and_then(|json| {
+                        serde_json::from_slice::<maestro_protocol::Session>(&json).ok()
+                    })
+                    .is_some_and(|saved| saved.status == SessionStatus::Dead)
+            }
+        },
+    )
+    .await;
+
+    daemon.stop();
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn concurrent_spawns_each_get_unique_ids() {
     let (_daemon, client) = bring_up().await;

docs/INSTALL.md

@@ -208,8 +208,10 @@ When the adapter is missing the spawn still creates the session, marked dead
 with the install command above as its activity detail, so the sidebar says
 what to run. The adapter is a child of the daemon: it ends with a `DELETE
 /api/sessions/:id` (`session/cancel`, then `SIGTERM`) and with the daemon
-itself. Terminal sessions keep surviving restarts through tmux; structured
-ones do not yet.
+itself. A daemon restart ends an in-flight structured turn, but preserves its
+worktree, session record, and event-log transcript. The replacement daemon
+starts a new adapter, resumes it with `session/load`, and replays the
+transcript. Terminal sessions keep surviving restarts through tmux.
 
 ## The browser UI