fix(maestro): mark graceful ACP recovery

crates/maestro-acp/tests/fake_acp_agent.py

@@ -17,6 +17,8 @@ Environment knobs, all optional:
                             signal SIGTERM
   FAKE_ACP_ECHO_ENV       name of one env var to log at start
   FAKE_ACP_INIT_FAIL=1    answer `initialize` with a JSON-RPC error
+  FAKE_ACP_DELAY_SESSION_NEW_MS=n  wait n milliseconds before answering
+                             `session/new`
   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)
@@ -36,6 +38,7 @@ import json
 import os
 import signal
 import sys
+import time
 
 LOG = os.environ.get("FAKE_ACP_LOG")
 
@@ -113,6 +116,7 @@ for raw in sys.stdin:
             "authMethods": [],
         }})
     elif method == "session/new":
+        time.sleep(int(os.environ.get("FAKE_ACP_DELAY_SESSION_NEW_MS", "0")) / 1000)
         send({"jsonrpc": "2.0", "id": rid, "result": {"sessionId": session_id}})
         send(chunk("hello from fake"))
         if os.environ.get("FAKE_ACP_EXIT_AFTER_NEW") == "1":

crates/maestro-daemon/src/local_api.rs

@@ -170,6 +170,7 @@ fn session_error_status(code: &str) -> StatusCode {
         // The session outlived its pane: nothing to type into, and nothing
         // the daemon can do about it.
         "not_running" => StatusCode::CONFLICT,
+        "shutting_down" => StatusCode::SERVICE_UNAVAILABLE,
         _ => StatusCode::BAD_GATEWAY,
     }
 }

crates/maestro-daemon/src/session.rs

@@ -127,6 +127,8 @@ pub enum SessionError {
     /// a stray keystroke on an attach socket, which is dropped.
     #[error("session is no longer running: {0}")]
     NotRunning(Uuid),
+    #[error("daemon is shutting down")]
+    ShuttingDown,
     /// The kill was refused (#79): the session's worktree holds unsaved work.
     /// Nothing happened — the session keeps running and the tree is untouched,
     /// so it stays the operator's handle on their own unfinished work.
@@ -144,6 +146,7 @@ impl SessionError {
             SessionError::NotFound(_) => "not_found",
             SessionError::InvalidEnv(_) => "invalid_env",
             SessionError::NotRunning(_) => "not_running",
+            SessionError::ShuttingDown => "shutting_down",
             SessionError::KillGuarded(_) => "kill_guarded",
         };
         let unsaved_work = match self {
@@ -198,6 +201,9 @@ struct Inner {
     /// Once daemon shutdown starts, it owns structured-session finalization.
     /// Background passes must not classify those deliberate exits as natural.
     shutting_down: AtomicBool,
+    /// Serializes structured creation with shutdown so every adapter child is
+    /// either rejected or included in shutdown's final drain.
+    structured_lifecycle: Mutex<()>,
 }
 
 impl SessionManager {
@@ -241,6 +247,7 @@ impl SessionManager {
                 viewports: Mutex::new(Viewports::new()),
                 next_viewport: AtomicU64::new(0),
                 shutting_down: AtomicBool::new(false),
+                structured_lifecycle: Mutex::new(()),
             }),
         }
     }
@@ -356,6 +363,7 @@ impl SessionManager {
         if self.inner.shutting_down.swap(true, Ordering::AcqRel) {
             return;
         }
+        let _lifecycle = self.inner.structured_lifecycle.lock().await;
         let children: Vec<(Uuid, Arc<AcpSession>, Option<watch::Receiver<bool>>)> = {
             let sessions = self.inner.sessions.lock().await;
             sessions
@@ -465,6 +473,17 @@ impl SessionManager {
 
     /// Spawn a new session. On success returns the freshly-created `Session`.
     pub async fn create(&self, req: CreateSessionRequest) -> Result<Session, SessionError> {
+        if req.view == SessionView::Structured {
+            let _lifecycle = self.inner.structured_lifecycle.lock().await;
+            if self.inner.shutting_down.load(Ordering::Acquire) {
+                return Err(SessionError::ShuttingDown);
+            }
+            return self.create_inner(req).await;
+        }
+        self.create_inner(req).await
+    }
+
+    async fn create_inner(&self, req: CreateSessionRequest) -> Result<Session, SessionError> {
         let (effective_cwd, worktree_path, worktree_base_repo) = match &req.worktree {
             Some(spec) => {
                 let WorktreeSpec {
@@ -911,9 +930,9 @@ impl SessionManager {
             adopted += 1;
         }
 
-        // Structured sessions have no tmux process to discover. Their
-        // sidecars are the recovery inventory: when an ACP id exists, start a
-        // fresh adapter process and load that conversation into it.
+        // Structured sessions have no tmux process to discover. A durable
+        // marker written during graceful shutdown is the only evidence that a
+        // sidecar may be resumed; older and naturally-dead sidecars stay dead.
         for mut session in self
             .inner
             .sidecar
@@ -926,6 +945,7 @@ impl SessionManager {
             }
 
             let id = session.id;
+            let recoverable = self.inner.sidecar.take_recovery_marker(id)?;
             let mut session_env = sanitize_env(&session.env)?;
             let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
             let bridge =
@@ -941,40 +961,50 @@ impl SessionManager {
                 session_env.insert("MAESTRO_URL".into(), url);
             }
 
-            let (acp, failure) = match session.acp_session_id.clone() {
-                Some(acp_session_id) => {
-                    let config = SpawnConfig::new(
-                        &session.cwd,
-                        session_env
-                            .iter()
-                            .map(|(key, value)| (key.clone(), value.clone()))
-                            .collect(),
-                    )
-                    .load_session(acp_session_id);
-                    match maestro_acp::spawn(config).await {
-                        Ok(acp) => (Some(Arc::new(acp)), None),
-                        Err(error) => {
-                            tracing::warn!(%id, %error, "structured session: adapter failed to resume");
-                            (None, Some(error.to_string()))
+            let (acp, failure) = if recoverable {
+                match session.acp_session_id.clone() {
+                    Some(acp_session_id) => {
+                        let config = SpawnConfig::new(
+                            &session.cwd,
+                            session_env
+                                .iter()
+                                .map(|(key, value)| (key.clone(), value.clone()))
+                                .collect(),
+                        )
+                        .load_session(acp_session_id);
+                        match maestro_acp::spawn(config).await {
+                            Ok(acp) => (Some(Arc::new(acp)), None),
+                            Err(error) => {
+                                tracing::warn!(%id, %error, "structured session: adapter failed to resume");
+                                (None, Some(error.to_string()))
+                            }
                         }
                     }
+                    None => (
+                        None,
+                        Some("structured session has no stored ACP session id".into()),
+                    ),
                 }
-                None => (
-                    None,
-                    Some("structured session has no stored ACP session id".into()),
-                ),
+            } else {
+                (None, None)
             };
-            session.status = if failure.is_some() {
+            session.status = if recoverable {
+                if failure.is_some() {
+                    SessionStatus::Dead
+                } else {
+                    SessionStatus::Running
+                }
+            } else {
                 SessionStatus::Dead
-            } else {
-                SessionStatus::Running
             };
-            session.activity = if failure.is_some() {
-                ActivityState::Error
-            } else {
-                ActivityState::Active
-            };
-            session.activity_detail = failure;
+            if recoverable {
+                session.activity = if failure.is_some() {
+                    ActivityState::Error
+                } else {
+                    ActivityState::Active
+                };
+                session.activity_detail = failure;
+            }
 
             let acp_events = if let Some(acp) = &acp {
                 match self.inner.sidecar.open_events(id) {
@@ -1705,6 +1735,7 @@ async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
 /// 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;
+    let _ = inner.sidecar.mark_for_recovery(id);
 }
 
 async fn mark_session_dead_with_worktree_settlement(

crates/maestro-daemon/src/sidecar.rs

@@ -56,6 +56,28 @@ impl SidecarStore {
         self.dir.join(format!("{id}.events.jsonl"))
     }
 
+    /// Durable evidence that graceful daemon shutdown ended this structured
+    /// session and the next daemon may resume it.
+    pub fn recovery_path_for(&self, id: Uuid) -> PathBuf {
+        self.dir.join(format!("{id}.recover"))
+    }
+
+    /// Mark `id` as eligible for one structured-session recovery attempt.
+    pub fn mark_for_recovery(&self, id: Uuid) -> Result<(), SidecarError> {
+        std::fs::create_dir_all(&self.dir)?;
+        std::fs::write(self.recovery_path_for(id), [])?;
+        Ok(())
+    }
+
+    /// Consume the graceful-shutdown recovery marker for `id`.
+    pub fn take_recovery_marker(&self, id: Uuid) -> Result<bool, SidecarError> {
+        match std::fs::remove_file(self.recovery_path_for(id)) {
+            Ok(()) => Ok(true),
+            Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false),
+            Err(error) => Err(error.into()),
+        }
+    }
+
     /// Open the event log for `id` for appending, creating it (and the
     /// directory) when missing. One handle per live session: the caller
     /// keeps it for the session's lifetime rather than reopening per
@@ -164,10 +186,11 @@ impl SidecarStore {
         }
     }
 
-    /// Delete the sidecar and the event log for `id`. No-op for whichever
-    /// does not exist.
+    /// Delete the sidecar, event log, and recovery marker for `id`. No-op for
+    /// whichever does not exist.
     pub fn delete(&self, id: Uuid) -> Result<(), SidecarError> {
         remove_if_present(&self.events_path_for(id))?;
+        remove_if_present(&self.recovery_path_for(id))?;
         remove_if_present(&self.path_for(id))
     }
 

crates/maestro-daemon/tests/acp.rs

@@ -232,6 +232,12 @@ async fn recovery_loads_the_acp_session_without_duplicating_history() {
     let old_pid = fake.pid();
     fx.manager.shutdown().await;
     wait_for_exit(old_pid).await;
+    assert!(
+        fx.sidecar_dir()
+            .join(format!("{}.recover", session.id))
+            .exists(),
+        "graceful shutdown must record recovery provenance"
+    );
 
     let revived = SessionManager::new(
         Arc::new(fx.tmux.tmux.clone()),
@@ -498,6 +504,50 @@ async fn shutdown_reaps_every_adapter_child() {
     assert_eq!(sidecar.status, SessionStatus::Dead);
 }
 
+#[tokio::test]
+async fn shutdown_waits_for_structured_creation_and_rejects_new_creates() {
+    let fx = DaemonFixture::new();
+    let fake = Fake::new();
+    let cwd = TempDir::new().unwrap();
+    let mut env = fake.env();
+    env.insert("FAKE_ACP_DELAY_SESSION_NEW_MS".into(), "500".into());
+
+    let create_cwd = cwd.path().to_path_buf();
+    let manager = fx.manager.clone();
+    let create = tokio::spawn(async move { manager.create(structured(&create_cwd, env)).await });
+    wait_for_async(Duration::from_secs(5), || async {
+        fake.log().contains("recv session/new")
+    })
+    .await;
+
+    let manager = fx.manager.clone();
+    let shutdown = tokio::spawn(async move { manager.shutdown().await });
+    tokio::time::sleep(Duration::from_millis(50)).await;
+    assert!(
+        !shutdown.is_finished(),
+        "shutdown returned before create finished"
+    );
+
+    let session = create.await.expect("create task").expect("create");
+    let pid = fake.pid();
+    shutdown.await.expect("shutdown task");
+
+    wait_for_exit(pid).await;
+    assert!(fake.log().contains("signal SIGTERM"), "{}", fake.log());
+    assert_eq!(
+        fx.manager.status(session.id).await,
+        Some(SessionStatus::Dead)
+    );
+    assert!(matches!(
+        fx.manager.create(structured(cwd.path(), fake.env())).await,
+        Err(maestro_daemon::SessionError::ShuttingDown)
+    ));
+
+    // A repeated signal must not attempt to terminate the child again.
+    fx.manager.shutdown().await;
+    assert_eq!(fake.log().matches("signal SIGTERM").count(), 1);
+}
+
 #[tokio::test]
 async fn shutdown_drains_final_notifications_to_the_log() {
     let fx = DaemonFixture::new();
@@ -556,7 +606,27 @@ async fn adapter_exit_marks_the_session_dead_and_keeps_it_listed() {
     })
     .await;
     assert!(fx.manager.has(session.id).await);
+    assert!(
+        !fx.sidecar_dir()
+            .join(format!("{}.recover", session.id))
+            .exists(),
+        "a naturally exited adapter is never marked for recovery"
+    );
 
-    fx.manager.kill(session.id, false).await.expect("dismiss");
-    assert!(!fx.manager.has(session.id).await);
+    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::Dead));
+    assert!(
+        !fake.log().contains("recv session/load"),
+        "a naturally dead adapter must not be resumed: {}",
+        fake.log()
+    );
+
+    revived.kill(session.id, false).await.expect("dismiss");
+    assert!(!revived.has(session.id).await);
 }

crates/maestro-daemon/tests/sidecar.rs

@@ -68,15 +68,30 @@ fn delete_removes_file_and_is_idempotent() {
     let s = sample(id);
 
     store.write(&s).unwrap();
+    store.mark_for_recovery(id).unwrap();
     assert!(store.path_for(id).exists());
+    assert!(store.recovery_path_for(id).exists());
 
     store.delete(id).unwrap();
     assert!(!store.path_for(id).exists());
+    assert!(!store.recovery_path_for(id).exists());
 
     // second delete is a no-op
     store.delete(id).unwrap();
 }
 
+#[test]
+fn recovery_marker_is_consumed_once() {
+    let dir = TempDir::new().unwrap();
+    let store = SidecarStore::new(dir.path());
+    let id = Uuid::new_v4();
+
+    assert!(!store.take_recovery_marker(id).unwrap());
+    store.mark_for_recovery(id).unwrap();
+    assert!(store.take_recovery_marker(id).unwrap());
+    assert!(!store.take_recovery_marker(id).unwrap());
+}
+
 #[test]
 fn load_all_recovers_every_sidecar_and_skips_garbage() {
     let dir = TempDir::new().unwrap();