From b96f18c7cbe21bd352ad09b74ce460ebac7f5205 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 11:29:45 +0100 Subject: [PATCH] feat(maestro): forced kill ends the session and retains the worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --force overrides the kill refusal and nothing else (#81). The guarded worktree is recorded in the retained store with reason force-killed — never removed — and the outcome names what was kept so a client can say where the work went. A tree whose guard is clear never needed the override; force removes it as usual. The store write happens before teardown, so a failed retention leaves the session running and every artifact intact. DELETE /api/sessions/:id takes ?force=true and answers 200 with KillOutcome; the unforced kill keeps its 204. --- crates/maestro-daemon/src/cli.rs | 12 +- crates/maestro-daemon/src/local_api.rs | 28 +++- crates/maestro-daemon/src/session.rs | 139 ++++++++++++++--- crates/maestro-daemon/src/worktree_probe.rs | 8 + crates/maestro-daemon/tests/attach.rs | 8 +- crates/maestro-daemon/tests/common/mod.rs | 9 ++ crates/maestro-daemon/tests/local_api.rs | 65 +++++++- .../maestro-daemon/tests/session_decisions.rs | 4 +- .../maestro-daemon/tests/session_manager.rs | 144 ++++++++++++++++-- 9 files changed, 371 insertions(+), 46 deletions(-) diff --git a/crates/maestro-daemon/src/cli.rs b/crates/maestro-daemon/src/cli.rs index c3781a5..cef210d 100644 --- a/crates/maestro-daemon/src/cli.rs +++ b/crates/maestro-daemon/src/cli.rs @@ -56,6 +56,9 @@ async fn run(config_override: Option) -> Result<(), Box> { Some(p) => Tmux::new().with_tmpdir(p), None => Tmux::new(), }; + let retained = std::sync::Arc::new(crate::retained_store::RetainedStore::new( + config.retained_worktree_dir(), + )); let manager = SessionManager::new_with_replay( std::sync::Arc::new(tmux), sidecar, @@ -66,7 +69,10 @@ async fn run(config_override: Option) -> Result<(), Box> { .with_spawn_history(crate::spawn_history::SpawnHistory::new( config.spawn_history_path(), config.spawn_history_days, - )); + )) + // One store instance shared with the API handlers: forced kills (#81) + // record here, `GET /api/worktrees` (#80) reads back from here. + .with_retained_store((*retained).clone()); let recovered = manager.recover().await?; tracing::info!(recovered, "adopted existing tmux sessions"); @@ -85,9 +91,7 @@ async fn run(config_override: Option) -> Result<(), Box> { config.home_dir.clone(), config.spawn_presets.clone(), projects, - std::sync::Arc::new(crate::retained_store::RetainedStore::new( - config.retained_worktree_dir(), - )), + retained, )); let listener = TcpListener::bind(config.bind_addr).await?; diff --git a/crates/maestro-daemon/src/local_api.rs b/crates/maestro-daemon/src/local_api.rs index 106f73a..6ae1c3a 100644 --- a/crates/maestro-daemon/src/local_api.rs +++ b/crates/maestro-daemon/src/local_api.rs @@ -101,6 +101,8 @@ fn session_error_status(code: &str) -> StatusCode { "invalid_env" => StatusCode::BAD_REQUEST, // A guard refusal is the daemon doing its job, not a failure (#79). "kill_guarded" => StatusCode::CONFLICT, + // The retention record failed before anything was torn down (#81). + "retain_failed" => StatusCode::INTERNAL_SERVER_ERROR, _ => StatusCode::BAD_GATEWAY, } } @@ -179,9 +181,29 @@ async fn create_session( /// The full error info is passed through rather than rebuilt from /// code + message: a `kill_guarded` refusal (#79) carries the unsaved-work /// breakdown clients render as "3 uncommitted files, 2 unpushed commits". -async fn kill_session(State(s): State, Path(id): Path) -> Response { - match s.manager.kill(id).await { - Ok(()) => StatusCode::NO_CONTENT.into_response(), +/// +/// With `force=true` (#81) the refusal is overridden and the worktree +/// retained; the answer is `200` with a [`KillOutcome`] naming what was kept. +/// Without it the historical `204` stands. +#[derive(Debug, Deserialize)] +struct KillQuery { + #[serde(default)] + force: bool, +} + +async fn kill_session( + State(s): State, + Path(id): Path, + Query(q): Query, +) -> Response { + match s.manager.kill(id, q.force).await { + Ok(outcome) => { + if q.force { + (StatusCode::OK, Json(outcome)).into_response() + } else { + StatusCode::NO_CONTENT.into_response() + } + } Err(e) => { let info = e.to_error_info(); let status = session_error_status(&info.code); diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 2c269d7..9edd43c 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -12,8 +12,8 @@ use std::time::Duration; use chrono::Utc; use maestro_protocol::{ - ActivityState, CatalogEvent, CreateSessionRequest, ErrorInfo, Session, SessionStatus, - WorktreeSpec, + ActivityState, CatalogEvent, CreateSessionRequest, ErrorInfo, KillOutcome, RetainedReason, + RetainedWorktree, Session, SessionStatus, WorktreeSpec, }; use tokio::sync::{broadcast, Mutex}; use uuid::Uuid; @@ -44,6 +44,11 @@ pub struct SessionRecord { /// `base_repo` from the original `WorktreeSpec`. Needed because `git /// worktree remove` is invoked from the base repo. pub worktree_base_repo: Option, + /// Branch the worktree was created for, recorded at spawn time so a + /// forced kill (#81) can name it in the retained store without asking + /// git. Sessions adopted from a sidecar have no branch on record and + /// fall back to a probe. + pub worktree_branch: Option, pub bridge: PtyBridge, } @@ -66,6 +71,11 @@ pub enum SessionError { /// so it stays the operator's handle on their own unfinished work. #[error("kill refused: the session's worktree holds unsaved work ({0})")] KillGuarded(maestro_protocol::InTheWay), + /// A forced kill (#81) records the retained worktree *before* tearing + /// anything down; if that write fails nothing was destroyed, and the + /// error says so rather than silently dropping unrecorded work. + #[error("could not record the retained worktree: {0}")] + RetainFailed(String), } impl SessionError { @@ -78,6 +88,7 @@ impl SessionError { SessionError::NotFound(_) => "not_found", SessionError::InvalidEnv(_) => "invalid_env", SessionError::KillGuarded(_) => "kill_guarded", + SessionError::RetainFailed(_) => "retain_failed", }; let unsaved_work = match self { SessionError::KillGuarded(b) => Some(*b), @@ -110,6 +121,9 @@ struct Inner { /// `None` when not wired (tests, or before `with_spawn_history`); a present /// but disabled (`0`-day) history no-ops internally. spawn_history: Option, + /// Retained-worktree store (#78). `None` when not wired (tests); a forced + /// kill (#81) without one fails rather than destroying unrecorded work. + retained: Option, sessions: Mutex>, events: broadcast::Sender, } @@ -148,6 +162,7 @@ impl SessionManager { fifo_dir, replay_bytes, spawn_history: None, + retained: None, sessions: Mutex::new(HashMap::new()), events, }), @@ -170,6 +185,16 @@ impl SessionManager { self } + /// Attach the retained-worktree store (#78). A forced kill (#81) needs it + /// to record what it retained; without one, forcing a guarded kill fails + /// instead of destroying unrecorded work. + pub fn with_retained_store(mut self, store: crate::retained_store::RetainedStore) -> Self { + let inner = Arc::get_mut(&mut self.inner) + .expect("with_retained_store must be called before the SessionManager is cloned"); + inner.retained = Some(store); + self + } + /// Recent working dirs for the spawn form's picker (#24), newest-first and /// deduped by dir. Empty when no history is wired or it's disabled. pub fn recent_dirs(&self) -> Vec { @@ -340,6 +365,7 @@ impl SessionManager { fifo_path, worktree_path, worktree_base_repo, + worktree_branch: req.worktree.as_ref().map(|w| w.branch.clone()), bridge, }, ); @@ -391,11 +417,17 @@ impl SessionManager { /// work, so ending a session that still matters becomes a deliberate /// `--force`, which retains rather than destroys (WORKTREES.md). /// + /// With `force` (#81) the refusal — and nothing else — is overridden: + /// the session ends and the guarded worktree is retained into the store + /// with reason `force-killed`, never removed. The outcome names what was + /// kept so a client can say where the work went. A tree whose guard is + /// clear never needed the override; force removes it as usual. + /// /// This is also how an already-dead session is dismissed. tmux is only /// asked to kill something it still has — calling `kill-session` on a /// session tmux already lost fails, and that failure is not interesting. - pub async fn kill(&self, id: Uuid) -> Result<(), SessionError> { - let (worktree, alive) = { + pub async fn kill(&self, id: Uuid, force: bool) -> Result { + let (worktree, alive, known_branch) = { let sessions = self.inner.sessions.lock().await; match sessions.get(&id) { None => return Err(SessionError::NotFound(id)), @@ -405,26 +437,75 @@ impl SessionManager { .clone() .zip(record.worktree_path.clone()), record.session.status == SessionStatus::Running, + record.worktree_branch.clone(), ), } }; - // The guard runs before anything is torn down: refusing must leave the - // session running, exactly as the operator left it. - if let Some((_, path)) = worktree { - let probe = tmux_blocking(move || crate::worktree_probe::in_the_way(&path)) - .await - .map_err(|e| SessionError::Worktree(crate::worktree::WorktreeError::from(e)))?; - if !probe.is_clear() { - return Err(SessionError::KillGuarded(probe)); + // The probe runs before anything is torn down: both refusing (#79) + // and recording the retention (#81) must see the tree as it stands, + // and a refused kill leaves the session running as the operator left + // it. + let probe = match &worktree { + Some((_, path)) => { + let path = path.clone(); + Some( + tmux_blocking(move || crate::worktree_probe::in_the_way(&path)) + .await + .map_err(|e| { + SessionError::Worktree(crate::worktree::WorktreeError::from(e)) + })?, + ) } + None => None, + }; + let guarded = probe.as_ref().is_some_and(|p| !p.is_clear()); + if guarded && !force { + return Err(SessionError::KillGuarded(probe.expect("guarded ⇒ probed"))); } + + // Record the retention BEFORE tearing down: a failed store write then + // leaves the session running and every artifact untouched, instead of + // ending a session whose work outlives it unrecorded. + let mut retained = None; + if guarded { + let store = + self.inner.retained.clone().ok_or_else(|| { + SessionError::RetainFailed("no retained store is wired".into()) + })?; + let (base_repo, path) = worktree.as_ref().expect("guarded ⇒ worktree"); + let branch = known_branch + .or_else(|| crate::worktree_probe::current_branch(path)) + .unwrap_or_default(); + let entry = { + let store = store.clone(); + let base_repo = base_repo.clone(); + let path = path.clone(); + tmux_blocking(move || { + store.retain(&path, &base_repo, &branch, RetainedReason::ForceKilled) + }) + .await + .map_err(|e| SessionError::RetainFailed(e.to_string()))? + }; + retained = Some((entry, probe.expect("guarded ⇒ probed"))); + } + let removed = { let mut sessions = self.inner.sessions.lock().await; sessions.remove(&id) }; let record = removed.ok_or(SessionError::NotFound(id))?; - cleanup_record(&self.inner, id, record, alive).await; - Ok(()) + cleanup_record(&self.inner, id, record, alive, retained.is_some()).await; + Ok(KillOutcome { + retained: retained.map(|(entry, in_the_way)| RetainedWorktree { + id: entry.id, + path: entry.path, + base_repo: entry.base_repo, + branch: entry.branch, + reason: entry.reason, + retained_at: entry.retained_at, + in_the_way, + }), + }) } /// Spawn a background task that polls tmux every `interval` and reaps any @@ -511,6 +592,9 @@ impl SessionManager { // directory blocks that branch forever (#54 review). worktree_path: session.worktree.as_ref().map(|w| w.path.clone()), worktree_base_repo: session.worktree.as_ref().map(|w| w.base_repo.clone()), + // The sidecar carries no branch; a forced kill of a + // recovered session probes git for it at retain time. + worktree_branch: None, bridge, }, ); @@ -750,7 +834,17 @@ async fn mark_session_dead(inner: &Arc, id: Uuid) { /// gone from the authoritative in-memory map, so a stray FIFO or sidecar can't /// resurrect it. `kill_tmux` is `true` for an explicit kill and `false` for the /// liveness reaper (tmux already lost the session). -async fn cleanup_record(inner: &Arc, id: Uuid, record: SessionRecord, kill_tmux: bool) { +/// +/// `retain_worktree` is `true` for a forced kill (#81) that retained the +/// worktree instead of removing it: the store entry is already written, so +/// only the removal is skipped. +async fn cleanup_record( + inner: &Arc, + id: Uuid, + record: SessionRecord, + kill_tmux: bool, + retain_worktree: bool, +) { if kill_tmux { let tmux = inner.tmux.clone(); let _ = tmux_blocking(move || tmux.kill_session(id)).await; @@ -759,11 +853,18 @@ async fn cleanup_record(inner: &Arc, id: Uuid, record: SessionRecord, kil if let Err(e) = inner.sidecar.delete(id) { tracing::warn!(error = ?e, %id, "cleanup: failed to delete sidecar"); } - // The kill already passed the unsaved-work guard (#79), so this removal - // is unconditional; the manager's own nested-worktree refusal is all that - // can still leave a tree on disk, and it says where. if let (Some(base), Some(path)) = (&record.worktree_base_repo, &record.worktree_path) { - if let Err(e) = inner.worktree_mgr.remove(base, path) { + if retain_worktree { + tracing::info!( + %id, + path = %path.display(), + "forced kill: worktree retained in the store" + ); + } else if let Err(e) = inner.worktree_mgr.remove(base, path) { + // The kill already passed the unsaved-work guard (#79), so this + // removal is unconditional; the manager's own nested-worktree + // refusal is all that can still leave a tree on disk, and it + // says where. tracing::warn!( %id, path = %path.display(), diff --git a/crates/maestro-daemon/src/worktree_probe.rs b/crates/maestro-daemon/src/worktree_probe.rs index 24f81cb..f1720e7 100644 --- a/crates/maestro-daemon/src/worktree_probe.rs +++ b/crates/maestro-daemon/src/worktree_probe.rs @@ -90,6 +90,14 @@ pub fn reclaim(base_repo: &Path, path: &Path) -> Result<(), ProbeError> { .map(|_| ()) } +/// Branch checked out at `path`. Sessions adopted from a sidecar carry no +/// branch on record, so a forced kill (#81) asks the tree itself when it +/// needs one for the retained store. `None` when git cannot answer. +pub fn current_branch(path: &Path) -> Option { + let out = run_git(path, ["rev-parse", "--abbrev-ref", "HEAD"]).ok()?; + Some(String::from_utf8_lossy(&out.stdout).trim().to_string()) +} + fn run_git(dir: &Path, args: I) -> Result where I: IntoIterator, diff --git a/crates/maestro-daemon/tests/attach.rs b/crates/maestro-daemon/tests/attach.rs index 09f1d32..6aa0e31 100644 --- a/crates/maestro-daemon/tests/attach.rs +++ b/crates/maestro-daemon/tests/attach.rs @@ -122,7 +122,7 @@ async fn pty_round_trip_through_real_tmux() { "marker not seen in {} bytes", got.len() ); - let _ = h.manager.kill(id).await; + let _ = h.manager.kill(id, false).await; } #[tokio::test] @@ -170,7 +170,7 @@ async fn multi_attach_broadcasts_to_every_client() { "client B missed marker" ); - let _ = h.manager.kill(id).await; + let _ = h.manager.kill(id, false).await; } #[tokio::test] @@ -219,7 +219,7 @@ async fn sends_close_frame_when_session_is_killed() { // give the broadcast subscriber a moment to register before killing. tokio::time::sleep(Duration::from_millis(100)).await; - h.manager.kill(id).await.unwrap(); + h.manager.kill(id, false).await.unwrap(); let mut saw_close = false; let deadline = std::time::Instant::now() + Duration::from_secs(2); @@ -267,7 +267,7 @@ async fn ping_text_frame_gets_pong() { } } assert!(saw_pong, "no Pong frame received"); - let _ = h.manager.kill(id).await; + let _ = h.manager.kill(id, false).await; } #[tokio::test] diff --git a/crates/maestro-daemon/tests/common/mod.rs b/crates/maestro-daemon/tests/common/mod.rs index 3603e33..a079660 100644 --- a/crates/maestro-daemon/tests/common/mod.rs +++ b/crates/maestro-daemon/tests/common/mod.rs @@ -176,6 +176,11 @@ impl DaemonFixture { manager = manager.with_spawn_history(SpawnHistory::new(Self::history_path(&state_dir), days)); } + // Same wiring as production: the manager records retained worktrees + // into the same dir the API reads back from (#78/#81). + let retained = + maestro_daemon::retained_store::RetainedStore::new(Self::retained_path(&state_dir)); + manager = manager.with_retained_store(retained.clone()); Self { tmux, manager, @@ -187,6 +192,10 @@ impl DaemonFixture { state_dir.path().join("spawn-history.jsonl") } + fn retained_path(state_dir: &TempDir) -> PathBuf { + state_dir.path().join("retained-worktrees") + } + pub fn spawn_history_path(&self) -> PathBuf { Self::history_path(&self.state_dir) } diff --git a/crates/maestro-daemon/tests/local_api.rs b/crates/maestro-daemon/tests/local_api.rs index 10f08bb..ce6e071 100644 --- a/crates/maestro-daemon/tests/local_api.rs +++ b/crates/maestro-daemon/tests/local_api.rs @@ -25,6 +25,13 @@ struct Harness { _fx: DaemonFixture, } +impl Harness { + /// Store over the same dir the manager records into. + fn fx_retained(&self) -> maestro_daemon::retained_store::RetainedStore { + self._fx.retained_store() + } +} + async fn spawn(hostname: &str) -> Harness { spawn_with( hostname, @@ -386,6 +393,62 @@ async fn delete_a_session_whose_worktree_holds_unsaved_work_returns_409_with_the assert!(h.manager.has(session.id).await, "the session keeps running"); } +/// `?force=true` on the wire (#81): the refusal is overridden, the session +/// ends, and the answer is 200 with the retained worktree named — path, +/// branch, reason — while the directory survives on disk. +#[tokio::test] +async fn delete_with_force_ends_the_session_and_answers_what_was_retained() { + let h = spawn("naps-laptop").await; + let repo = tempfile::TempDir::new().unwrap(); + common::init_repo(repo.path()); + let wt_path = repo + .path() + .join("worktrees") + .join("feat") + .join("guarded") + .to_path_buf(); + let session = h + .manager + .create(CreateSessionRequest { + cwd: repo.path().to_path_buf(), + command: "sleep 60".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: Some(WorktreeSpec { + base_repo: repo.path().to_path_buf(), + branch: "feat/guarded".into(), + cleanup_on_kill: true, + create_branch: true, + }), + }) + .await + .unwrap(); + std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap(); + + let resp = reqwest::Client::new() + .delete(url(h.addr, &format!("/api/sessions/{}", session.id))) + .query(&[("force", "true")]) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), reqwest::StatusCode::OK); + let body: maestro_protocol::KillOutcome = resp.json().await.unwrap(); + let retained = body.retained.expect("force names what was kept"); + assert_eq!(retained.path, wt_path); + assert_eq!(retained.branch, "feat/guarded"); + assert_eq!( + retained.reason, + maestro_protocol::RetainedReason::ForceKilled + ); + + // The tree would have blocked; force must not have removed it. + assert!(wt_path.exists(), "a forced kill never removes the worktree"); + // And the store entry exists, so reclaim can find it (#80). + let all = h.fx_retained().load_all().unwrap(); + assert_eq!(all.len(), 1); + assert_eq!(all[0].id, retained.id); +} + #[tokio::test] async fn attach_ws_streams_pty_bytes() { let h = spawn("naps-laptop").await; @@ -476,7 +539,7 @@ async fn events_ws_sends_a_snapshot_then_streams_deltas() { other => panic!("expected SessionAdded, got {other:?}"), } - h.manager.kill(created.id).await.expect("kill"); + h.manager.kill(created.id, false).await.expect("kill"); // Activity ticks may interleave; the removal is what we are waiting for. let deadline = tokio::time::Instant::now() + Duration::from_secs(5); diff --git a/crates/maestro-daemon/tests/session_decisions.rs b/crates/maestro-daemon/tests/session_decisions.rs index fb48862..aae5475 100644 --- a/crates/maestro-daemon/tests/session_decisions.rs +++ b/crates/maestro-daemon/tests/session_decisions.rs @@ -211,7 +211,7 @@ async fn killing_a_dead_session_dismisses_it_without_touching_tmux() { fx.manager.start_liveness_watcher(Duration::from_millis(25)); wait_for_status(&fx.manager, s.id, SessionStatus::Dead).await; - fx.manager.kill(s.id).await.expect("dismiss"); + fx.manager.kill(s.id, false).await.expect("dismiss"); assert!(fx.manager.snapshot().await.is_empty()); assert!( @@ -371,7 +371,7 @@ async fn killing_an_unknown_session_is_reported_as_not_found() { let fx = Fx::new(); let err = fx .manager - .kill(uuid::Uuid::new_v4()) + .kill(uuid::Uuid::new_v4(), false) .await .expect_err("should fail"); assert_eq!(err.to_error_info().code, "not_found"); diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 4b1f25f..234421c 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -44,7 +44,7 @@ async fn create_then_kill_round_trip() { // FIFO should also exist assert!(fx.fifo_dir().join(format!("{}.fifo", session.id)).exists()); - fx.manager.kill(session.id).await.expect("kill"); + fx.manager.kill(session.id, false).await.expect("kill"); assert!(!fx.manager.has(session.id).await); assert!(!fx .sidecar_dir() @@ -79,7 +79,7 @@ async fn create_appends_to_spawn_history() { assert_eq!(recents[0].cwd, cwd.path()); assert_eq!(recents[0].last_command, "sleep 30"); - fx.manager.kill(session.id).await.expect("kill"); + fx.manager.kill(session.id, false).await.expect("kill"); // History is durable — it survives the kill that deletes the sidecar. assert_eq!(fx.manager.recent_dirs().len(), 1); } @@ -185,7 +185,7 @@ async fn create_captures_initial_output_in_replay_buffer() { "first-line marker never reached the replay buffer — pipe-pane raced past the child's initial output", ); - fx.manager.kill(s.id).await.unwrap(); + fx.manager.kill(s.id, false).await.unwrap(); } /// Regression for NAP-228: after `recover`, PTY output must reach new @@ -251,7 +251,7 @@ async fn recover_re_pipes_pane_so_output_reaches_subscribers() { String::from_utf8_lossy(&buf), ); - revived.kill(s.id).await.unwrap(); + revived.kill(s.id, false).await.unwrap(); } #[tokio::test] @@ -300,7 +300,7 @@ async fn pty_broadcast_delivers_bytes_to_subscribers() { assert!(buf_a.windows(want.len()).any(|w| w == want), "A missed"); assert!(buf_b.windows(want.len()).any(|w| w == want), "B missed"); - fx.manager.kill(s.id).await.unwrap(); + fx.manager.kill(s.id, false).await.unwrap(); let _ = (cwd, PathBuf::from("/")); } @@ -359,7 +359,7 @@ async fn liveness_watcher_marks_dead_sessions_and_keeps_them_until_dismissed() { ); // Dismissing it is what finally releases the artifacts. - fx.manager.kill(s.id).await.expect("dismiss"); + fx.manager.kill(s.id, false).await.expect("dismiss"); assert!(!fx.manager.has(s.id).await); assert!(!fx.sidecar_dir().join(format!("{}.json", s.id)).exists()); assert!(!fx.fifo_dir().join(format!("{}.fifo", s.id)).exists()); @@ -398,7 +398,7 @@ async fn kill_removes_worktree_even_when_spec_says_keep() { init_bare_repo(origin.path()); push_all_to_origin(repo.path(), origin.path()); - fx.manager.kill(s.id).await.expect("kill"); + fx.manager.kill(s.id, false).await.expect("kill"); assert!(!wt_path.exists(), "worktree must be removed on kill"); } @@ -432,7 +432,11 @@ async fn killing_refuses_a_worktree_with_unsaved_work_and_keeps_the_session() { std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap(); std::fs::write(wt_path.join("notes.txt"), b"untracked\n").unwrap(); - let err = fx.manager.kill(s.id).await.expect_err("kill must refuse"); + let err = fx + .manager + .kill(s.id, false) + .await + .expect_err("kill must refuse"); let SessionError::KillGuarded(breakdown) = err else { panic!("expected KillGuarded, got {err:?}"); }; @@ -456,7 +460,10 @@ async fn killing_refuses_a_worktree_with_unsaved_work_and_keeps_the_session() { let origin = TempDir::new().unwrap(); init_bare_repo(origin.path()); push_all_to_origin(repo.path(), origin.path()); - fx.manager.kill(s.id).await.expect("kill after saving"); + fx.manager + .kill(s.id, false) + .await + .expect("kill after saving"); assert!(!fx.manager.has(s.id).await); } @@ -487,7 +494,11 @@ async fn killing_refuses_until_the_branch_is_pushed() { .await .expect("create"); - let err = fx.manager.kill(s.id).await.expect_err("unpushed blocks"); + let err = fx + .manager + .kill(s.id, false) + .await + .expect_err("unpushed blocks"); let SessionError::KillGuarded(breakdown) = err else { panic!("expected KillGuarded, got {err:?}"); }; @@ -499,11 +510,118 @@ async fn killing_refuses_until_the_branch_is_pushed() { init_bare_repo(origin.path()); push_all_to_origin(repo.path(), origin.path()); - fx.manager.kill(s.id).await.expect("pushed branch kills"); + fx.manager + .kill(s.id, false) + .await + .expect("pushed branch kills"); assert!(!wt_path.exists()); assert!(!fx.manager.has(s.id).await); } +/// `--force` overrides the refusal and nothing else (#81): the session ends, +/// but the guarded worktree is RETAINED — never removed — and enters the +/// retained store with reason `force-killed`. The outcome names what was +/// kept, so a client can say where the work went. +#[tokio::test] +async fn force_killing_a_guarded_worktree_retains_it_in_the_store() { + let fx = DaemonFixture::new(); + let repo = TempDir::new().unwrap(); + init_repo(repo.path()); + let wt_path = repo.path().join("worktrees").join("feat").join("forced"); + + let s = fx + .manager + .create(CreateSessionRequest { + cwd: repo.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: Some(WorktreeSpec { + base_repo: repo.path().to_path_buf(), + branch: "feat/forced".into(), + cleanup_on_kill: true, + create_branch: true, + }), + }) + .await + .expect("create"); + std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap(); + + // Without force the same kill is refused (#79). + assert!(fx.manager.kill(s.id, false).await.is_err()); + + let outcome = fx + .manager + .kill(s.id, true) + .await + .expect("force overrides the refusal"); + + // The tree would have blocked, so force must NOT have removed it... + assert!( + wt_path.exists(), + "a forced kill never removes a tree that would have blocked" + ); + // ...but the session itself still ends. + assert!(!fx.manager.has(s.id).await); + assert!(!fx.sidecar_dir().join(format!("{}.json", s.id)).exists()); + + let retained = outcome.retained.expect("the outcome names what was kept"); + assert_eq!(retained.path, wt_path); + assert_eq!(retained.branch, "feat/forced"); + assert_eq!( + retained.reason, + maestro_protocol::RetainedReason::ForceKilled + ); + assert!( + retained.in_the_way.uncommitted_files >= 1, + "the outcome carries what blocked the unforced kill" + ); + + // The store holds the same entry, so reclaim can find it later (#80). + let all = fx.retained_store().load_all().expect("load_all"); + assert_eq!(all.len(), 1); + assert_eq!(all[0].id, retained.id); + assert_eq!(all[0].path, wt_path); + assert_eq!(all[0].branch, "feat/forced"); +} + +/// Force overrides the *refusal*, not ordinary cleanup (#81): a tree whose +/// guard is clear never needed the override, so a forced kill removes it as +/// usual and retains nothing. +#[tokio::test] +async fn force_on_a_clear_worktree_removes_it_as_usual() { + let fx = DaemonFixture::new(); + let repo = TempDir::new().unwrap(); + init_repo(repo.path()); + let wt_path = repo.path().join("worktrees").join("feat").join("clear"); + + let s = fx + .manager + .create(CreateSessionRequest { + cwd: repo.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: Some(WorktreeSpec { + base_repo: repo.path().to_path_buf(), + branch: "feat/clear".into(), + cleanup_on_kill: true, + create_branch: true, + }), + }) + .await + .expect("create"); + + let origin = TempDir::new().unwrap(); + init_bare_repo(origin.path()); + push_all_to_origin(repo.path(), origin.path()); + + let outcome = fx.manager.kill(s.id, true).await.expect("kill"); + assert!(!wt_path.exists(), "a clear tree needs no override"); + assert_eq!(outcome.retained, None); + assert!(fx.retained_store().load_all().unwrap().is_empty()); +} + /// Ignored files NEVER block (#79): build output like `target/` exists in /// every Rust checkout, and an ignored-only worktree stays killable. #[tokio::test] @@ -543,7 +661,7 @@ async fn killing_an_ignored_only_worktree_succeeds() { push_all_to_origin(repo.path(), origin.path()); fx.manager - .kill(s.id) + .kill(s.id, false) .await .expect("ignored files never block"); assert!(!wt_path.exists()); @@ -587,5 +705,5 @@ async fn liveness_watcher_leaves_running_sessions_alone() { tokio::time::sleep(Duration::from_millis(400)).await; assert!(fx.manager.has(s.id).await, "live session got reaped"); - fx.manager.kill(s.id).await.unwrap(); + fx.manager.kill(s.id, false).await.unwrap(); }