Compare commits

..
Author SHA1 Message Date
Miguel Palhas 31702e03e3 refactor(maestro): drop cleanup_on_kill from the wire and the CLI
Nothing has honoured WorktreeSpec.cleanup_on_kill since #56, and after
#79 and #81 survival is decided by what is in a worktree, not by what
the spawn asked for. A field that lies is worse than a missing one.

Removes the field from the protocol, the --cleanup-on-kill flag from
the CLI, and the tests asserting its default.
2026-08-24 12:08:41 +01:00
Miguel Palhas 5282ba3411 merge: web retained-worktrees view with reclaim
Closes #84
2026-08-24 11:45:34 +01:00
Miguel Palhas 2d796a3556 merge: kill --force ends the session and retains the worktree
Closes #81
2026-08-24 11:39:28 +01:00
Miguel Palhas f67803acf5 Merge remote-tracking branch 'origin/blitz/worktree-lifecycle' into wl/81-force-retains
# Conflicts:
#	crates/maestro-protocol/src/client.rs
2026-08-24 11:35:08 +01:00
Miguel Palhas 0c135aa065 feat(maestro): maestro kill --force
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.
2026-08-24 11:34:15 +01:00
Miguel Palhas b96f18c7cb feat(maestro): forced kill ends the session and retains the worktree
--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.
2026-08-24 11:29:45 +01:00
Miguel Palhas bb2f439302 feat(maestro): kill outcome payload and force query in protocol
DELETE /api/sessions/:id gains a force query param (#81). A forced kill
answers 200 with KillOutcome, naming what was retained and why; the
unforced kill keeps its 204. DaemonClient::kill_session takes the flag.
2026-08-24 11:20:00 +01:00
26 changed files with 488 additions and 118 deletions
+31 -28
View File
@@ -102,11 +102,6 @@ pub struct SpawnArgs {
/// there.
#[arg(long)]
pub existing_branch: bool,
/// Remove the worktree when the session is killed. Kept for wire
/// compatibility (#56): the daemon now always cleans up on kill, guarded
/// against unsaved work.
#[arg(long, default_value_t = true)]
pub cleanup_on_kill: bool,
/// After spawning, attach immediately.
#[arg(long, short = 'a')]
pub attach: bool,
@@ -125,6 +120,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> {
@@ -163,7 +163,6 @@ mod tests {
"/repos/foo",
"--branch",
"feat/x",
"--cleanup-on-kill",
"-e",
"FOO=bar",
"-e",
@@ -174,7 +173,6 @@ mod tests {
Command::Spawn(a) => {
assert_eq!(a.command, "bash");
assert_eq!(a.branch.as_deref(), Some("feat/x"));
assert!(a.cleanup_on_kill);
assert_eq!(
a.env,
vec![("FOO".into(), "bar".into()), ("BAZ".into(), "qux".into()),]
@@ -184,27 +182,6 @@ mod tests {
}
}
#[test]
fn cleanup_on_kill_defaults_true_for_the_wire() {
let cli = Cli::try_parse_from([
"maestro",
"spawn",
"--cwd",
"/tmp",
"--command",
"bash",
"--worktree-from",
"/repos/foo",
"--branch",
"feat/x",
])
.unwrap();
match cli.command {
Command::Spawn(a) => assert!(a.cleanup_on_kill),
_ => panic!("wrong subcommand"),
}
}
/// Review fix: without a flag the CLI always sent `create_branch: true`,
/// so the daemon's existing-branch path was unreachable and spawning onto
/// a branch that already existed just failed.
@@ -256,6 +233,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([
+8 -1
View File
@@ -62,8 +62,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
);
}
}
Command::Worktrees(args) => match args.command {
WorktreesCommand::List(list_args) => {
+8 -4
View File
@@ -43,7 +43,6 @@ pub async fn run_spawn(client: &DaemonClient, args: SpawnArgs) -> Result<Session
(Some(repo), Some(branch)) => Some(WorktreeSpec {
base_repo: repo.clone(),
branch: branch.clone(),
cleanup_on_kill: args.cleanup_on_kill,
create_branch: !args.existing_branch,
}),
_ => None,
@@ -59,9 +58,14 @@ pub async fn run_spawn(client: &DaemonClient, args: SpawnArgs) -> Result<Session
client.create_session(&body).await
}
/// `maestro kill`
pub async fn run_kill(client: &DaemonClient, args: &KillArgs) -> 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<maestro_protocol::KillOutcome, ClientError> {
client.kill_session(args.session_id, args.force).await
}
#[allow(clippy::write_literal)]
@@ -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);
}
+8 -4
View File
@@ -56,6 +56,9 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
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<PathBuf>) -> Result<(), Box<dyn Error>> {
.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<PathBuf>) -> Result<(), Box<dyn Error>> {
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?;
+25 -3
View File
@@ -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<LocalApiState>, Path(id): Path<Uuid>) -> 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<LocalApiState>,
Path(id): Path<Uuid>,
Query(q): Query<KillQuery>,
) -> 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);
+120 -19
View File
@@ -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<PathBuf>,
/// 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<String>,
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<SpawnHistory>,
/// Retained-worktree store (#78). `None` when not wired (tests); a forced
/// kill (#81) without one fails rather than destroying unrecorded work.
retained: Option<crate::retained_store::RetainedStore>,
sessions: Mutex<HashMap<Uuid, SessionRecord>>,
events: broadcast::Sender<CatalogEvent>,
}
@@ -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<RecentDir> {
@@ -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<KillOutcome, SessionError> {
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<Inner>, 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<Inner>, 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<Inner>,
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<Inner>, 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(),
@@ -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<String> {
let out = run_git(path, ["rev-parse", "--abbrev-ref", "HEAD"]).ok()?;
Some(String::from_utf8_lossy(&out.stdout).trim().to_string())
}
fn run_git<I, S>(dir: &Path, args: I) -> Result<std::process::Output, ProbeError>
where
I: IntoIterator<Item = S>,
+4 -4
View File
@@ -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]
@@ -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)
}
+63 -2
View File
@@ -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,
@@ -356,7 +363,6 @@ async fn delete_a_session_whose_worktree_holds_unsaved_work_returns_409_with_the
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/guarded".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
@@ -386,6 +392,61 @@ 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(),
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 +537,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);
@@ -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");
+129 -17
View File
@@ -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());
@@ -385,7 +385,6 @@ async fn kill_removes_worktree_even_when_spec_says_keep() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/keep".into(),
cleanup_on_kill: false,
create_branch: true,
}),
})
@@ -398,7 +397,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");
}
@@ -423,7 +422,6 @@ async fn killing_refuses_a_worktree_with_unsaved_work_and_keeps_the_session() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/guarded".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
@@ -432,7 +430,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 +458,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);
}
@@ -480,14 +485,17 @@ async fn killing_refuses_until_the_branch_is_pushed() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/pushed".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
.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 +507,116 @@ 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(),
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(),
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]
@@ -530,7 +643,6 @@ async fn killing_an_ignored_only_worktree_succeeds() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/rusty".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
@@ -543,7 +655,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 +699,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();
}
+1 -1
View File
@@ -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<Vec<Project>, ClientError> {
+17 -5
View File
@@ -16,7 +16,7 @@ use uuid::Uuid;
use crate::routes::{self, UrlError};
use crate::{
ApiError, BranchInfo, CreateSessionRequest, DaemonInfo, DirListing, ErrorInfo, GitRoot,
Project, RecentDirs, RetainedWorktree, Session,
KillOutcome, Project, RecentDirs, RetainedWorktree, Session,
};
/// Time budget for establishing a TCP connection to the daemon.
@@ -114,12 +114,24 @@ impl DaemonClient {
self.post(routes::SESSIONS, req).await
}
pub async fn kill_session(&self, id: Uuid) -> Result<(), ClientError> {
/// Kill a session. With `force`, the daemon overrides the kill refusal
/// (#79) and retains the worktree instead of removing it (#81); the
/// outcome then names what was kept and where.
pub async fn kill_session(&self, id: Uuid, force: bool) -> Result<KillOutcome, ClientError> {
let query: &[(&str, &str)] = if force {
&[(routes::FORCE_QUERY, "true")]
} else {
&[]
};
let resp = self
.send(Method::DELETE, &routes::session(id), &[], None::<&()>)
.send(Method::DELETE, &routes::session(id), query, None::<&()>)
.await?;
check(resp).await?;
Ok(())
let resp = check(resp).await?;
// An unforced kill keeps its historical `204 No Content`.
if resp.status() == reqwest::StatusCode::NO_CONTENT {
return Ok(KillOutcome { retained: None });
}
json(resp).await
}
pub async fn daemon_info(&self) -> Result<DaemonInfo, ClientError> {
+29 -8
View File
@@ -119,11 +119,6 @@ pub enum ActivityState {
pub struct WorktreeSpec {
pub base_repo: PathBuf,
pub branch: String,
/// Kept on the wire for CLI compatibility (#56) but no longer a
/// user-facing choice: the daemon always cleans up on kill, guarded
/// against unsaved work. Defaults to `true`.
#[serde(default = "default_true")]
pub cleanup_on_kill: bool,
/// When false, `branch` must already exist and is checked out as-is
/// (#56); when true (the default, and what every payload meant before
/// the field existed) the daemon creates it with `worktree add -b`.
@@ -195,6 +190,19 @@ pub struct RetainedWorktree {
pub in_the_way: InTheWay,
}
/// Body of a successful forced `DELETE /api/sessions/:id?force=true` (#81).
/// A forced kill ends the session and retains its worktree instead of
/// removing it; this names what was kept, where, and why, so a client can say
/// where the work went rather than leaving the operator to wonder.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct KillOutcome {
/// Set when the worktree was retained with reason `force-killed`; `None`
/// when there was nothing to retain — no worktree at all, or one whose
/// guard was clear, which a forced kill removes as usual.
#[serde(default)]
pub retained: Option<RetainedWorktree>,
}
impl std::fmt::Display for InTheWay {
/// "3 uncommitted files, 2 unpushed commits" — zero parts are omitted;
/// all-zero renders "no unsaved work". This is the phrasing every client
@@ -581,7 +589,6 @@ mod tests {
worktree: Some(WorktreeSpec {
base_repo: PathBuf::from("/home/naps62/projects/maestro"),
branch: "feat/foo".into(),
cleanup_on_kill: true,
create_branch: true,
}),
};
@@ -611,7 +618,6 @@ mod tests {
});
let spec: WorktreeSpec = serde_json::from_value(json).unwrap();
assert!(spec.create_branch);
assert!(spec.cleanup_on_kill);
}
#[test]
@@ -619,7 +625,6 @@ mod tests {
let spec = WorktreeSpec {
base_repo: PathBuf::from("/work"),
branch: "feat/foo".into(),
cleanup_on_kill: true,
create_branch: false,
};
assert_eq!(spec, round_trip(&spec));
@@ -679,6 +684,22 @@ mod tests {
);
}
#[test]
fn kill_outcome_round_trips_a_retained_worktree() {
let outcome = KillOutcome {
retained: Some(sample_retained_worktree()),
};
assert_eq!(outcome, round_trip(&outcome));
}
/// A kill with nothing to retain — no worktree, or a clean one removed as
/// usual — decodes from an absent field.
#[test]
fn kill_outcome_defaults_to_nothing_retained() {
let outcome: KillOutcome = serde_json::from_value(serde_json::json!({})).unwrap();
assert_eq!(outcome.retained, None);
}
#[test]
fn catalog_event_round_trips_and_tags() {
let session = sample_session();
+4
View File
@@ -38,6 +38,10 @@ pub const PATH_QUERY: &str = "path";
/// Name of the query parameter [`NAMES`] takes.
pub const COUNT_QUERY: &str = "count";
/// Name of the boolean query parameter `DELETE /api/sessions/:id` takes (#81).
/// `force=true` overrides the kill refusal and retains the worktree.
pub const FORCE_QUERY: &str = "force";
/// `DELETE /api/sessions/:id`.
pub fn session(id: Uuid) -> String {
format!("{SESSIONS}/{id}")
+8 -2
View File
@@ -733,8 +733,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}")),
}
}
-3
View File
@@ -549,7 +549,6 @@ impl DraftPane {
WorktreeSpec {
base_repo,
branch: branch.to_string(),
cleanup_on_kill: true,
create_branch: true,
}
}
@@ -562,7 +561,6 @@ impl DraftPane {
WorktreeSpec {
base_repo,
branch: name,
cleanup_on_kill: true,
create_branch: false,
}
}
@@ -1689,7 +1687,6 @@ mod tests {
assert_eq!(wt.base_repo, PathBuf::from("/repos/maestro"));
assert_eq!(wt.branch, "crimson-otter", "branch defaults to the name");
assert!(wt.create_branch);
assert!(wt.cleanup_on_kill);
}
#[test]
-1
View File
@@ -80,7 +80,6 @@ impl FastForwardDefaults {
.clone()
.unwrap_or_else(|| project.path.clone()),
branch: name.to_string(),
cleanup_on_kill: true,
create_branch: true,
}),
})
+9 -4
View File
@@ -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
-1
View File
@@ -43,7 +43,6 @@ describe("resolveFastForward (#67)", () => {
expect(req.worktree).toEqual({
base_repo: "/repos/fresh",
branch: "amber-falcon",
cleanup_on_kill: true,
create_branch: true,
});
});
-1
View File
@@ -42,7 +42,6 @@ export function buildFastForwardRequest(
worktree: {
base_repo: project.base_repo ?? project.path,
branch: name,
cleanup_on_kill: true,
create_branch: true,
},
};
-2
View File
@@ -38,7 +38,6 @@ describe("buildSpawnRequest", () => {
worktree: {
base_repo: "/home/user/project",
branch: "feat/foo",
cleanup_on_kill: true,
create_branch: true,
},
},
@@ -63,7 +62,6 @@ describe("buildSpawnRequest", () => {
worktree: {
base_repo: "/home/user/project",
branch: "spawn-v2",
cleanup_on_kill: true,
create_branch: false,
},
},
-3
View File
@@ -75,9 +75,6 @@ export function buildSpawnRequest(
req.worktree = {
base_repo: baseRepo,
branch,
// The daemon always cleans up, guarded against unsaved work (#56);
// the wire field stays for older daemons.
cleanup_on_kill: true,
// Existing-mode picks must check out as-is; the daemon creates
// anything else with `worktree add -b` (#56/#61).
create_branch: mode !== "existing",
-1
View File
@@ -113,7 +113,6 @@ export type Projects = Project[];
export interface WorktreeSpec {
base_repo: string;
branch: string;
cleanup_on_kill?: boolean;
/** When false, `branch` already exists and is checked out as-is (#61).
* Defaults to true (create a new branch) on older daemons. */
create_branch?: boolean;