docs(maestro): document structured restart recovery

crates/maestro-acp/src/lib.rs

@@ -156,6 +156,10 @@ enum Command {
     Close,
 }
 
+enum ChildCommand {
+    Signal(libc::c_int),
+}
+
 /// Why the handshake did not produce a session id.
 enum ReadyError {
     /// The adapter answered with an error.
@@ -180,6 +184,7 @@ pub struct AcpSession {
     session_id: String,
     pid: u32,
     commands: mpsc::UnboundedSender<Command>,
+    child_commands: mpsc::UnboundedSender<ChildCommand>,
     updates: Mutex<Option<mpsc::UnboundedReceiver<SessionNotification>>>,
     exit: watch::Receiver<Option<ExitStatus>>,
 }
@@ -221,9 +226,6 @@ impl AcpSession {
     /// End the child: `session/cancel`, wait `cancel_grace`, `SIGTERM`, wait
     /// `term_grace`, `SIGKILL`. Returns once the child has been reaped.
     pub async fn terminate(&self, cancel_grace: Duration, term_grace: Duration) {
-        if self.has_exited() {
-            return;
-        }
         self.cancel();
         if tokio::time::timeout(cancel_grace, self.wait())
             .await
@@ -245,16 +247,14 @@ impl AcpSession {
     }
 
     fn signal(&self, sig: libc::c_int) {
-        signal_pid(self.pid, sig);
+        let _ = self.child_commands.send(ChildCommand::Signal(sig));
     }
 }
 
 impl Drop for AcpSession {
     fn drop(&mut self) {
+        self.signal(libc::SIGTERM);
         let _ = self.commands.send(Command::Close);
-        if !self.has_exited() {
-            self.signal(libc::SIGTERM);
-        }
     }
 }
 
@@ -301,11 +301,8 @@ pub async fn spawn(config: SpawnConfig) -> Result<AcpSession, AcpError> {
     tokio::spawn(drain_stderr(stderr, pid, stderr_tail.clone()));
 
     let (exit_tx, exit_rx) = watch::channel(None);
-    tokio::spawn(async move {
-        let status = child.wait().await.ok();
-        tracing::debug!(pid, ?status, "{ADAPTER_BIN} exited");
-        let _ = exit_tx.send(status);
-    });
+    let (child_commands_tx, child_commands_rx) = mpsc::unbounded_channel();
+    tokio::spawn(supervise_child(child, pid, child_commands_rx, exit_tx));
 
     let (updates_tx, updates_rx) = mpsc::unbounded_channel();
     let (commands_tx, commands_rx) = mpsc::unbounded_channel();
@@ -327,6 +324,7 @@ pub async fn spawn(config: SpawnConfig) -> Result<AcpSession, AcpError> {
                 session_id,
                 pid,
                 commands: commands_tx,
+                child_commands: child_commands_tx,
                 updates: Mutex::new(Some(updates_rx)),
                 exit: exit_rx,
             });
@@ -356,12 +354,12 @@ pub async fn spawn(config: SpawnConfig) -> Result<AcpSession, AcpError> {
     };
     // No half-started adapter survives a failed spawn.
     let _ = commands_tx.send(Command::Close);
-    signal_pid(pid, libc::SIGTERM);
+    let _ = child_commands_tx.send(ChildCommand::Signal(libc::SIGTERM));
     if tokio::time::timeout(TERM_GRACE, wait_exit(&exit_rx))
         .await
         .is_err()
     {
-        signal_pid(pid, libc::SIGKILL);
+        let _ = child_commands_tx.send(ChildCommand::Signal(libc::SIGKILL));
         wait_exit(&exit_rx).await;
     }
     Err(failure)
@@ -373,11 +371,39 @@ async fn wait_exit(exit: &watch::Receiver<Option<ExitStatus>>) -> Option<ExitSta
     *status
 }
 
-fn signal_pid(pid: u32, sig: libc::c_int) {
-    // ESRCH just means the child is already gone.
-    // SAFETY: plain syscall on a pid we spawned and have not reaped.
-    unsafe {
-        libc::kill(pid as libc::pid_t, sig);
+async fn supervise_child(
+    mut child: tokio::process::Child,
+    pid: u32,
+    mut commands: mpsc::UnboundedReceiver<ChildCommand>,
+    exit_tx: watch::Sender<Option<ExitStatus>>,
+) {
+    loop {
+        tokio::select! {
+            status = child.wait() => {
+                let status = status.ok();
+                tracing::debug!(pid, ?status, "{ADAPTER_BIN} exited");
+                let _ = exit_tx.send(status);
+                return;
+            }
+            command = commands.recv() => match command {
+                Some(ChildCommand::Signal(sig)) => {
+                    // This task alone owns `child`. It cannot reap the child
+                    // between receiving this command and sending the signal,
+                    // so its PID cannot be reused in that window.
+                    unsafe { libc::kill(pid as libc::pid_t, sig) };
+                }
+                None => {
+                    // The session vanished without a command receiver. This
+                    // task still owns the unreaped child, so this cannot hit
+                    // a reused PID.
+                    unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) };
+                    let status = child.wait().await.ok();
+                    tracing::debug!(pid, ?status, "{ADAPTER_BIN} exited");
+                    let _ = exit_tx.send(status);
+                    return;
+                }
+            },
+        }
     }
 }
 

crates/maestro-acp/tests/fake_acp_agent.py

@@ -20,6 +20,7 @@ Environment knobs, all optional:
   FAKE_ACP_EXIT_AFTER_NEW=1  exit 0 right after answering `session/new`
   FAKE_ACP_EXIT_ON_CANCEL=1  exit 0 on `session/cancel` (default: keep running
                              so the SIGTERM path is exercised)
+  FAKE_ACP_IGNORE_EOF=1      keep running after stdin closes, until signalled
   FAKE_ACP_EMIT_ON_CANCEL=1  emit a final notification before handling cancel
   FAKE_ACP_NO_LOAD=1      advertise `loadSession: false`
   FAKE_ACP_LOAD_REPLAY=n  on `session/load`, replay n `agent_message_chunk`
@@ -135,3 +136,7 @@ for raw in sys.stdin:
     elif rid is not None:
         send({"jsonrpc": "2.0", "id": rid,
               "error": {"code": -32601, "message": f"method not found: {method}"}})
+
+if os.environ.get("FAKE_ACP_IGNORE_EOF") == "1":
+    while True:
+        signal.pause()

crates/maestro-acp/tests/spawn.rs

@@ -69,6 +69,24 @@ impl Fake {
         }
         panic!("log never contained {needle:?}:\n{}", self.log());
     }
+
+    fn pid(&self) -> u32 {
+        self.log()
+            .lines()
+            .find_map(|line| line.strip_prefix("start pid="))
+            .and_then(|rest| rest.split(' ').next())
+            .and_then(|pid| pid.parse().ok())
+            .expect("pid in log")
+    }
+
+    async fn wait_for_exit(&self) {
+        let pid = self.pid();
+        let deadline = Instant::now() + Duration::from_secs(5);
+        while pid_alive(pid) && Instant::now() < deadline {
+            tokio::time::sleep(Duration::from_millis(20)).await;
+        }
+        assert!(!pid_alive(pid), "child {pid} still alive");
+    }
 }
 
 fn pid_alive(pid: u32) -> bool {
@@ -274,6 +292,20 @@ async fn child_exit_before_handshake_reports_exited_early() {
     }
 }
 
+#[tokio::test]
+async fn drop_terminates_and_reaps_the_child() {
+    let fake = Fake::new();
+    let cwd = TempDir::new().unwrap();
+    let mut config = fake.config(cwd.path());
+    config.env.insert("FAKE_ACP_IGNORE_EOF".into(), "1".into());
+    let session = maestro_acp::spawn(config).await.expect("spawn");
+
+    drop(session);
+
+    fake.wait_for_log("signal SIGTERM").await;
+    fake.wait_for_exit().await;
+}
+
 fn chunk_text(n: &maestro_acp::schema::SessionNotification) -> String {
     match &n.update {
         SessionUpdate::AgentMessageChunk(chunk) => match &chunk.content {

crates/maestro-daemon/src/session.rs

@@ -374,7 +374,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;
         }
     }
 
@@ -1656,7 +1656,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 +1668,24 @@ 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) {
+    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 +1709,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();

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