From 0c135aa06503523828e801f326c5f86973b352f6 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 11:34:15 +0100 Subject: [PATCH] feat(maestro): maestro kill --force MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flag overrides the kill refusal (#81): the session ends, the guarded worktree is retained, and the CLI prints where the work went — path, branch, force-killed — instead of leaving the operator to wonder. WORKTREES.md: write down that a forced kill of a tree whose guard is clear removes it as usual — force changes what happens to blocked work, not ordinary cleanup. --- crates/maestro-cli/src/cli.rs | 31 +++++++++++++++++++ crates/maestro-cli/src/dispatch.rs | 9 +++++- crates/maestro-cli/src/sessions.rs | 11 +++++-- .../maestro-cli/tests/against_mock_daemon.rs | 7 +++-- crates/maestro-e2e/src/client.rs | 2 +- crates/maestro-tui/src/app.rs | 10 ++++-- docs/WORKTREES.md | 13 +++++--- 7 files changed, 70 insertions(+), 13 deletions(-) diff --git a/crates/maestro-cli/src/cli.rs b/crates/maestro-cli/src/cli.rs index 877d344..e34ffbb 100644 --- a/crates/maestro-cli/src/cli.rs +++ b/crates/maestro-cli/src/cli.rs @@ -95,6 +95,11 @@ pub struct AttachArgs { pub struct KillArgs { /// Session ID to kill. pub session_id: Uuid, + /// End the session even when its worktree holds unsaved work (#81). The + /// worktree is retained — never removed — and enters the retained store; + /// the output says where the work went. + #[arg(long)] + pub force: bool, } fn parse_kv(s: &str) -> Result<(String, String), String> { @@ -226,6 +231,32 @@ mod tests { } } + #[test] + fn parses_kill_with_force() { + let id = "01234567-89ab-cdef-0123-456789abcdef"; + let cli = Cli::try_parse_from(["maestro", "kill", "--force", id]).unwrap(); + match cli.command { + Command::Kill(a) => { + assert!(a.force); + assert_eq!(a.session_id.to_string(), id); + } + _ => panic!("wrong subcommand"), + } + } + + #[test] + fn kill_force_defaults_false() { + let id = "01234567-89ab-cdef-0123-456789abcdef"; + let cli = Cli::try_parse_from(["maestro", "kill", id]).unwrap(); + match cli.command { + Command::Kill(a) => { + assert!(!a.force); + assert_eq!(a.session_id.to_string(), id); + } + _ => panic!("wrong subcommand"), + } + } + #[test] fn rejects_env_without_equals() { let err = Cli::try_parse_from([ diff --git a/crates/maestro-cli/src/dispatch.rs b/crates/maestro-cli/src/dispatch.rs index b1a6fed..f96675e 100644 --- a/crates/maestro-cli/src/dispatch.rs +++ b/crates/maestro-cli/src/dispatch.rs @@ -61,8 +61,15 @@ pub async fn dispatch( run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?; } Command::Kill(args) => { - run_kill(&daemon, &args).await?; + let outcome = run_kill(&daemon, &args).await?; println!("killed {}", args.session_id); + if let Some(w) = outcome.retained { + println!( + "worktree retained at {} (branch {}, force-killed)", + w.path.display(), + w.branch + ); + } } } Ok(()) diff --git a/crates/maestro-cli/src/sessions.rs b/crates/maestro-cli/src/sessions.rs index 902a2de..64db192 100644 --- a/crates/maestro-cli/src/sessions.rs +++ b/crates/maestro-cli/src/sessions.rs @@ -59,9 +59,14 @@ pub async fn run_spawn(client: &DaemonClient, args: SpawnArgs) -> Result Result<(), ClientError> { - client.kill_session(args.session_id).await +/// `maestro kill`. A forced kill (#81) retains the guarded worktree instead +/// of removing it; the outcome names what was kept so dispatch can say where +/// the work went. +pub async fn run_kill( + client: &DaemonClient, + args: &KillArgs, +) -> Result { + client.kill_session(args.session_id, args.force).await } #[allow(clippy::write_literal)] diff --git a/crates/maestro-cli/tests/against_mock_daemon.rs b/crates/maestro-cli/tests/against_mock_daemon.rs index 955cc8b..064daaf 100644 --- a/crates/maestro-cli/tests/against_mock_daemon.rs +++ b/crates/maestro-cli/tests/against_mock_daemon.rs @@ -76,7 +76,7 @@ async fn killing_an_unknown_session_reports_the_daemon_reason() { let err = mock .client() - .kill_session(Uuid::new_v4()) + .kill_session(Uuid::new_v4(), false) .await .expect_err("should fail"); @@ -102,6 +102,9 @@ async fn spawn_round_trips_through_the_daemon() { assert_eq!(created.command, "bash"); assert_eq!(mock.session_count(), 1); - mock.client().kill_session(created.id).await.expect("kill"); + mock.client() + .kill_session(created.id, false) + .await + .expect("kill"); assert_eq!(mock.session_count(), 0); } diff --git a/crates/maestro-e2e/src/client.rs b/crates/maestro-e2e/src/client.rs index 8de967e..f89cd2c 100644 --- a/crates/maestro-e2e/src/client.rs +++ b/crates/maestro-e2e/src/client.rs @@ -61,7 +61,7 @@ impl TestClient { } pub async fn kill_session(&self, id: Uuid) -> Result<(), ClientError> { - Ok(self.daemon.kill_session(id).await?) + Ok(self.daemon.kill_session(id, false).await.map(|_| ())?) } pub async fn list_projects(&self) -> Result, ClientError> { diff --git a/crates/maestro-tui/src/app.rs b/crates/maestro-tui/src/app.rs index 96b5375..5cdf662 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -667,8 +667,14 @@ impl App { } async fn submit_kill(&mut self, sid: Uuid) { - match self.daemon.kill_session(sid).await { - Ok(()) => self.release_session(sid), + match self.daemon.kill_session(sid, false).await { + Ok(outcome) => { + if let Some(w) = outcome.retained { + self.state + .toast(format!("worktree retained: {}", w.path.display())); + } + self.release_session(sid) + } Err(e) => self.state.toast(format!("kill failed: {e}")), } } diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index fca3566..2c5bbd8 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -75,11 +75,16 @@ That is only safe because force retains. `--force` overrides the refusal and nothing else. It ends the session and **retains** the worktree, which enters the retained store with reason -`force-killed`. Force never removes a tree that would have blocked. +`force-killed`. Force never removes a tree that would have blocked. A tree +whose guard is clear never needed the override, so a forced kill removes it as +usual — force changes what happens to blocked work, not ordinary cleanup. -Ending a session and destroying its work are two separate destructive acts. Each -is chosen on its own, and the second one — reclaim — is chosen while looking at -exactly what would be lost. +Ending a session and destroying its work are two separate destructive acts. +Each is chosen on its own, and the second one — reclaim — is chosen while +looking at exactly what would be lost. + +A forced kill answers with what was retained and why, so a client can say where +the work went rather than leaving the operator to wonder. ### Exiting on its own