From 3a68c1dd2f0e24a40b8a8dbcd8f55a340b0da103 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Sat, 5 Sep 2026 00:54:46 +0100 Subject: [PATCH] fix(maestro): preserve dismissed recovery cleanup --- crates/maestro-daemon/src/session.rs | 10 +++-- crates/maestro-daemon/tests/acp.rs | 56 ++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index cad62de..2d087fc 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -1752,15 +1752,16 @@ async fn mark_session_dead(inner: &Arc, id: Uuid) { /// 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, id: Uuid) { - mark_session_dead_with_worktree_settlement(inner, id, false).await; - let _ = inner.sidecar.mark_for_recovery(id); + if 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( inner: &Arc, id: Uuid, settle_worktree: bool, -) { +) -> bool { let flipped = { let mut sessions = inner.sessions.lock().await; match sessions.get_mut(&id) { @@ -1780,7 +1781,7 @@ async fn mark_session_dead_with_worktree_settlement( } }; let Some((mut session, worktree)) = flipped else { - return; + return false; }; tracing::info!(%id, "session process gone — marking dead"); @@ -1811,6 +1812,7 @@ async fn mark_session_dead_with_worktree_settlement( let _ = inner.sidecar.write(&session); let _ = inner.events.send(CatalogEvent::SessionUpdated { session }); + true } /// Apply the exit rule (#82) to a vanished session's worktree: probe what diff --git a/crates/maestro-daemon/tests/acp.rs b/crates/maestro-daemon/tests/acp.rs index 9cd364f..47314c8 100644 --- a/crates/maestro-daemon/tests/acp.rs +++ b/crates/maestro-daemon/tests/acp.rs @@ -504,6 +504,62 @@ async fn shutdown_reaps_every_adapter_child() { assert_eq!(sidecar.status, SessionStatus::Dead); } +#[tokio::test] +async fn dismissal_during_shutdown_does_not_restore_recovery_artifacts() { + let fx = DaemonFixture::new(); + let dismissed = Fake::new(); + let blocker = Fake::new(); + let cwd = TempDir::new().unwrap(); + let mut env = dismissed.env(); + env.insert("FAKE_ACP_EXIT_ON_CANCEL".into(), "1".into()); + + let session = fx + .manager + .create(structured(cwd.path(), env)) + .await + .expect("create"); + fx.manager + .create(structured(cwd.path(), blocker.env())) + .await + .expect("create blocker"); + + let manager = fx.manager.clone(); + let shutdown = tokio::spawn(async move { manager.shutdown().await }); + wait_for_async(Duration::from_secs(5), || async { + dismissed.log().contains("recv session/cancel") + }) + .await; + + fx.manager.kill(session.id, false).await.expect("dismiss"); + assert!( + !fx.sidecar_dir() + .join(format!("{}.json", session.id)) + .exists(), + "dismissal must finish before shutdown settles its stale snapshot" + ); + shutdown.await.expect("shutdown task"); + + assert!(!fx.manager.has(session.id).await); + assert!( + !fx.sidecar_dir() + .join(format!("{}.recover", session.id)) + .exists(), + "dismissal must leave no recovery marker" + ); + assert!( + !fx.sidecar_dir() + .join(format!("{}.json", session.id)) + .exists(), + "dismissal must leave no sidecar" + ); + assert!( + !fx.sidecar_dir() + .join(format!("{}.events.jsonl", session.id)) + .exists(), + "dismissal must leave no event log" + ); +} + #[tokio::test] async fn shutdown_does_not_recover_an_adapter_that_already_exited() { let fx = DaemonFixture::new();