Compare commits

..
Author SHA1 Message Date
Miguel Palhas fb87a00906 merge: surface blocked work before the kill is attempted
Closes #83
2026-08-24 13:22:50 +01:00
Miguel Palhas d4dcfe7f00 feat(maestro): show blocked work on session rows and warn on kill in the web UI (#83) 2026-08-24 13:19:12 +01:00
Miguel Palhas 6161cabe77 feat(maestro): warn on kill and mark blocked sessions in the TUI (#83) 2026-08-24 13:08:13 +01:00
Miguel Palhas 524a848fc6 feat(maestro): show blocked work in the CLI table and warn on kill (#83) 2026-08-24 13:06:43 +01:00
Miguel Palhas 187bcb01d6 feat(maestro): refresh the blocked-work indicator in the watcher (#83)
run_blocked_work_pass rides the liveness tick but probes on a slower
cadence (DEFAULT_BLOCKED_WORK_INTERVAL, 15s): each probe is three git
subprocesses per session. Sessions without a worktree are filtered out
before any probe, so their indicator costs nothing. A failed probe keeps
the previous value; settled exits clear it.
2026-08-24 13:04:30 +01:00
Miguel Palhas 003e3839bc feat(maestro): carry a blocked-work indicator on Session (#83)
Session.blocked_work is Option<InTheWay>, refreshed by the daemon's
watcher; None for worktree-less sessions and until first probed.
Defaults on the wire so older payloads stay forward-compatible.
2026-08-24 12:58:16 +01:00
Miguel Palhas d2079cac3a merge: e2e coverage for the whole worktree lifecycle
Resolves the e2e conflict in favour of #88's helper, rewritten as a
struct literal now that cleanup_on_kill is gone — #88 had built the
spec through serde_json to dodge the field, which cost compile-time
checking for no reason once the field no longer exists.

Closes #88
2026-08-24 12:37:41 +01:00
Miguel Palhas 2083911baa test(maestro): cover the whole worktree lifecycle end to end (#88)
Every scenario from the contract, against a real daemon, tmux and git:
collision suffixes with both trees working, foreign directories
sidestepped without adoption, the guarded kill and its refusal
breakdown, save-then-kill succeeding, --force retaining the tree,
natural exits settling dirty and pristine worktrees differently, the
retained list with reasons, reclaim keeping the branch, and the store
surviving a daemon restart.

Worktree specs are built through their wire shape so the tests carry no
reference to WorktreeSpec.cleanup_on_kill, which #87 is removing.
2026-08-24 12:33:39 +01:00
Miguel Palhas b5450651d1 test(maestro): expose forced kill and retained-worktree calls in the e2e harness 2026-08-24 12:33:39 +01:00
Miguel Palhas 8aa007c73b fix(maestro): drop the cleanup_on_kill call sites #87 could not reach
Three in session_manager.rs arrived with #82 after #87 branched; four in
maestro-e2e were fenced off because #88 owns that crate. Removing the
field without them left the workspace uncompilable.
2026-08-24 12:12:02 +01:00
Miguel Palhas f4d12fa02d merge: drop cleanup_on_kill from the wire and the CLI
Closes #87
2026-08-24 12:11:31 +01:00
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 dde8dd8cfc merge: apply the worktree rule when a session exits on its own
Closes #82
2026-08-24 11:57:51 +01:00
Miguel Palhas 2fc19a886e feat(maestro): apply the kill's worktree rule when a session exits on its own
When the liveness watcher finds the tmux session gone, it now settles
the worktree through the same probe_worktree/record_retention helpers
the kill uses: pristine trees are removed, anything else is retained as
exited-with-unsaved-work before anything is torn down. The dead session
drops its worktree reference once settled, so dismissing it neither
refuses nor retains twice. If the probe or store write fails, nothing
is destroyed and the tree stays attached to the record.

WORKTREES.md: written down the settle-then-drop and nothing-destroyed-
when-unrecordable rules alongside the existing exit rule.
2026-08-24 11:54:43 +01:00
Miguel Palhas 0b95d11aa6 Merge remote-tracking branch 'origin/blitz/worktree-lifecycle' into wl/82-natural-exit 2026-08-24 11:54:11 +01:00
Miguel Palhas a85084d0d9 refactor(maestro): extract shared probe and retain helpers from the kill path
probe_worktree and record_retention will be reused by the natural-exit
path (#82); kill's behavior is unchanged.
2026-08-24 11:49:36 +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
37 changed files with 1953 additions and 176 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([
+32 -1
View File
@@ -62,8 +62,20 @@ pub async fn dispatch(
run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?;
}
Command::Kill(args) => {
run_kill(&daemon, &args).await?;
// #83: warn BEFORE the attempt, so the refusal is not a surprise.
// Best-effort: an unreachable list must not block the kill.
if !args.force {
warn_if_blocked(&daemon, args.session_id).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) => {
@@ -92,6 +104,25 @@ pub async fn dispatch(
/// Default detach byte, matching `maestro attach --detach-key`'s `^q`.
const CTRL_Q: u8 = 0x11;
/// #83: say "this will be refused" before the operator commits to it, in the
/// same words [`maestro_protocol::InTheWay`] uses everywhere else.
async fn warn_if_blocked(daemon: &DaemonClient, id: uuid::Uuid) {
let Ok(sessions) = daemon.list_sessions().await else {
return;
};
if let Some(b) = sessions
.iter()
.find(|s| s.id == id)
.and_then(|s| s.blocked_work)
{
if !b.is_clear() {
eprintln!(
"warning: this kill will be refused — the session's worktree holds unsaved work ({b}); use --force to retain it"
);
}
}
}
/// HTTP client pointed at the daemon's API. That surface needs no auth —
/// reaching `bind_addr` is the authorization.
#[allow(clippy::result_large_err)]
+59 -8
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)]
@@ -89,8 +93,8 @@ fn print_session_table(out: &mut impl Write, sessions: &[Session], info: &Daemon
}
writeln!(
out,
" {:<36} {:<8} {:<9} {:<28} {}",
"ID", "STATUS", "ACTIVITY", "DIR", "COMMAND"
" {:<36} {:<8} {:<9} {:<28} {:<40} {}",
"ID", "STATUS", "ACTIVITY", "DIR", "COMMAND", "BLOCKED"
)
.ok();
for s in sorted {
@@ -111,15 +115,28 @@ fn print_session_table(out: &mut impl Write, sessions: &[Session], info: &Daemon
Some(fg) if fg != &s.command => format!("{} [{}]", s.command, fg),
_ => s.command.clone(),
};
// #83: what an unforced kill would be refused for, in InTheWay's own
// words; blank while nothing blocks (or hasn't been probed yet).
let blocked = blocked_label(s);
writeln!(
out,
" {:<36} {:<8} {:<9} {:<28} {command}",
s.id, status, activity, dir
" {:<36} {:<8} {:<9} {:<28} {:<40} {blocked}",
s.id, status, activity, dir, command
)
.ok();
}
}
/// The blocked-work indicator (#83), phrased by [`InTheWay`]'s `Display`.
/// Empty unless something actually blocks — "no unsaved work" would just be
/// noise on every row.
pub(crate) fn blocked_label(s: &Session) -> String {
s.blocked_work
.filter(|b| !b.is_clear())
.map(|b| b.to_string())
.unwrap_or_default()
}
/// Render a path with the daemon's home dir collapsed to `~`. Mirrors the web
/// UI's abbreviation so CLI and browser show the same directory shorthand.
pub(crate) fn abbreviate_dir(path: &std::path::Path, home: &str) -> String {
@@ -160,6 +177,7 @@ mod tests {
activity: Default::default(),
last_activity_at: chrono::Utc::now(),
current_cwd: None,
blocked_work: None,
}
}
@@ -220,4 +238,37 @@ mod tests {
super::print_session_table(&mut buf, &[], &info(""));
assert_eq!(String::from_utf8(buf).unwrap(), "no sessions\n");
}
/// #83: a blocked session shows what an unforced kill would be refused
/// for, in `InTheWay`'s own words; a clear or unprobed one shows nothing.
#[test]
fn shows_the_blocked_work_summary_only_when_something_blocks() {
use maestro_protocol::InTheWay;
let mut blocked = session("/tmp", "claude", 1);
blocked.blocked_work = Some(InTheWay {
uncommitted_files: 3,
unpushed_commits: 2,
..InTheWay::default()
});
let mut clear = session("/tmp", "bash", 2);
clear.blocked_work = Some(InTheWay::default());
let unprobed = session("/tmp", "zsh", 3);
let mut buf = Vec::new();
super::print_session_table(&mut buf, &[blocked, clear, unprobed], &info(""));
let out = String::from_utf8(buf).unwrap();
assert!(
out.contains("3 uncommitted files, 2 unpushed commits"),
"{out}"
);
// Clear and unprobed rows stay blank in the BLOCKED column.
let lines: Vec<&str> = out.lines().collect();
let header = lines.iter().position(|l| l.contains("BLOCKED")).unwrap();
for line in &lines[header + 1..] {
if !line.contains("uncommitted files") {
assert!(!line.contains("unsaved work"), "{line}");
}
}
}
}
@@ -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);
+329 -26
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;
@@ -33,6 +33,13 @@ pub const EVENTS_CAPACITY: usize = maestro_config::EVENTS_CAPACITY;
/// How often the liveness watcher polls tmux for sessions that disappeared.
pub const DEFAULT_LIVENESS_INTERVAL: Duration = Duration::from_secs(2);
/// How often the liveness watcher re-probes worktrees for the blocked-work
/// indicator (#83). Each probe costs three git subprocesses per session, so
/// the pass rides the same tick as everything else but runs on a slower
/// cadence: an indicator a few seconds stale still warns before the kill
/// attempt, which is all #83 needs.
pub const DEFAULT_BLOCKED_WORK_INTERVAL: Duration = Duration::from_secs(15);
/// Per-session daemon-local bookkeeping that complements the public `Session`.
#[derive(Debug)]
pub struct SessionRecord {
@@ -44,6 +51,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 +78,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 +95,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 +128,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 +169,7 @@ impl SessionManager {
fifo_dir,
replay_bytes,
spawn_history: None,
retained: None,
sessions: Mutex::new(HashMap::new()),
events,
}),
@@ -170,6 +192,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> {
@@ -319,6 +351,9 @@ impl SessionManager {
last_activity_at: Utc::now(),
current_cwd: None,
// The watcher fills this in on its first blocked-work pass; a
// fresh tree has nothing in the way anyway.
blocked_work: None,
};
if let Err(e) = self.inner.sidecar.write(&session) {
@@ -340,6 +375,7 @@ impl SessionManager {
fifo_path,
worktree_path,
worktree_base_repo,
worktree_branch: req.worktree.as_ref().map(|w| w.branch.clone()),
bridge,
},
);
@@ -391,11 +427,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 +447,48 @@ 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));
}
let probe = match &worktree {
Some(wt) => Some(probe_worktree(wt).await?),
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")));
}
let mut retained = None;
if guarded {
let entry = record_retention(
&self.inner,
worktree.as_ref().expect("guarded ⇒ worktree"),
known_branch,
RetainedReason::ForceKilled,
)
.await?;
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
@@ -435,6 +499,17 @@ impl SessionManager {
/// `SessionManager` clone is dropped — call this once from the daemon
/// startup path after [`Self::recover`] has populated the map.
pub fn start_liveness_watcher(&self, interval: Duration) {
self.start_liveness_watcher_with_blocked_work(interval, DEFAULT_BLOCKED_WORK_INTERVAL);
}
/// [`Self::start_liveness_watcher`] with the blocked-work cadence (#83)
/// made explicit. Tests pass a small `blocked_work_interval` to watch the
/// probe actually happen.
pub fn start_liveness_watcher_with_blocked_work(
&self,
interval: Duration,
blocked_work_interval: Duration,
) {
let weak = Arc::downgrade(&self.inner);
tokio::spawn(async move {
let mut ticker = tokio::time::interval(interval);
@@ -442,6 +517,9 @@ impl SessionManager {
// first tick fires immediately — skip it so we don't reap before
// the caller has a chance to create anything.
ticker.tick().await;
// Start "due" so the first live tick probes: a recovered daemon
// should not wait a full interval to show what blocks.
let mut since_probe = blocked_work_interval;
loop {
ticker.tick().await;
let Some(inner) = weak.upgrade() else {
@@ -451,6 +529,11 @@ impl SessionManager {
// Same tick, no new timer (NAP-340): refresh activity for the
// survivors and emit SessionUpdated on any transition.
run_activity_pass(&inner).await;
since_probe += interval;
if since_probe >= blocked_work_interval {
since_probe = Duration::ZERO;
run_blocked_work_pass(&inner).await;
}
}
});
}
@@ -487,6 +570,7 @@ impl SessionManager {
last_activity_at: Utc::now(),
current_cwd: None,
blocked_work: None,
},
};
let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
@@ -511,6 +595,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,
},
);
@@ -587,6 +674,49 @@ where
.expect("tmux blocking task panicked")
}
/// What would be lost if this session's worktree were removed right now.
///
/// The single [`crate::worktree_probe::in_the_way`] call site behind both
/// endings of a session — the kill guard (#79) and the natural-exit
/// classifier (#82) — so neither can grow its own answer. Runs on the
/// blocking pool; callers invoke it before anything is torn down, because
/// refusing (#79), recording a retention (#81) and classifying an exit (#82)
/// must all see the tree as it stands.
async fn probe_worktree(
(_, path): &(PathBuf, PathBuf),
) -> Result<maestro_protocol::InTheWay, SessionError> {
let path = path.clone();
tmux_blocking(move || crate::worktree_probe::in_the_way(&path))
.await
.map_err(|e| SessionError::Worktree(crate::worktree::WorktreeError::from(e)))
}
/// Record a guarded worktree into the retained store with `reason`, BEFORE
/// the caller tears anything down: a failed write then leaves every artifact
/// untouched instead of destroying work nothing records anymore. Shared by
/// the forced kill (`ForceKilled`, #81) and the natural exit
/// (`ExitedWithUnsavedWork`, #82) so the two paths cannot drift apart.
async fn record_retention(
inner: &Arc<Inner>,
(base_repo, path): &(PathBuf, PathBuf),
known_branch: Option<String>,
reason: RetainedReason,
) -> Result<crate::retained_store::RetainedWorktreeRecord, SessionError> {
let store = inner
.retained
.clone()
.ok_or_else(|| SessionError::RetainFailed("no retained store is wired".into()))?;
// Sessions adopted from a sidecar carry no branch; ask the tree itself.
let branch = known_branch
.or_else(|| crate::worktree_probe::current_branch(path))
.unwrap_or_default();
let base_repo = base_repo.clone();
let path = path.clone();
tmux_blocking(move || store.retain(&path, &base_repo, &branch, reason))
.await
.map_err(|e| SessionError::RetainFailed(e.to_string()))
}
/// One pass of the liveness watcher: diff the in-memory session set against
/// the tmux-reported one, and reap anything we still track but tmux has lost.
async fn run_liveness_pass(inner: &Arc<Inner>) {
@@ -709,18 +839,79 @@ async fn run_activity_pass(inner: &Arc<Inner>) {
}
}
/// Refresh each running session's blocked-work indicator (#83): what its
/// worktree holds right now that would block an unforced kill. Rides the
/// liveness tick on its own (slower) cadence — see
/// [`DEFAULT_BLOCKED_WORK_INTERVAL`] — because every probe is three git
/// subprocesses per session.
///
/// Sessions without a worktree are filtered out BEFORE any probe: they can
/// never block a kill, so their indicator must cost nothing at all. A failed
/// probe keeps the previous value rather than clearing it — a transient git
/// error must not read as "nothing blocks". Like the activity pass, this only
/// emits `SessionUpdated` when the value actually changed.
async fn run_blocked_work_pass(inner: &Arc<Inner>) {
let targets: Vec<(Uuid, (PathBuf, PathBuf))> = {
let sessions = inner.sessions.lock().await;
sessions
.iter()
.filter(|(_, r)| r.session.status == SessionStatus::Running)
.filter_map(|(id, r)| {
let wt = r.worktree_base_repo.clone().zip(r.worktree_path.clone())?;
Some((*id, wt))
})
.collect()
};
if targets.is_empty() {
return;
}
for (id, wt) in targets {
// The kill guard's own probe (#79), so the indicator can never
// disagree with what a kill attempt would find.
let breakdown = match probe_worktree(&wt).await {
Ok(breakdown) => breakdown,
Err(e) => {
tracing::debug!(%id, error = ?e, "blocked-work: probe failed; keeping previous value");
continue;
}
};
let updated = {
let mut sessions = inner.sessions.lock().await;
let Some(rec) = sessions.get_mut(&id) else {
continue;
};
if rec.session.blocked_work == Some(breakdown) {
None
} else {
rec.session.blocked_work = Some(breakdown);
Some(rec.session.clone())
}
};
if let Some(session) = updated {
let _ = inner.events.send(CatalogEvent::SessionUpdated { session });
}
}
}
/// Mark a session whose tmux session has disappeared as [`SessionStatus::Dead`]
/// and announce it, keeping everything else intact.
/// and settle its worktree per the natural-exit rule (#82).
///
/// The record, its [`PtyBridge`] and its sidecar all survive, so a client can
/// still attach and read the replay buffer — the last thing a crashed agent
/// printed. Artifacts are only released when the operator dismisses the session
/// via [`SessionManager::kill`].
/// printed. The worktree, though, is settled right away: a pristine tree is
/// removed, anything else is retained into the store with reason
/// `exited-with-unsaved-work` (#82) — nobody is waiting for an answer here,
/// so there is nothing to refuse. Once settled, the session drops its
/// worktree reference so a later dismissal neither re-probes nor retains
/// twice. When settling is impossible (probe or store failure) nothing is
/// destroyed: the tree stays attached to the dead record for a later forced
/// dismissal (#81).
///
/// Dead sessions do not survive a daemon restart: [`SessionManager::recover`]
/// adopts only what tmux still holds.
async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
let updated = {
let flipped = {
let mut sessions = inner.sessions.lock().await;
match sessions.get_mut(&id) {
Some(record) if record.session.status == SessionStatus::Running => {
@@ -728,19 +919,114 @@ async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
// Release anyone attached; the scrollback survives for a later
// attach.
record.bridge.seal();
Some(record.session.clone())
let session = record.session.clone();
let worktree = record
.worktree_base_repo
.clone()
.zip(record.worktree_path.clone());
let branch = record.worktree_branch.clone();
Some((session, worktree, branch))
}
_ => None,
}
};
let Some(session) = updated else {
let Some((mut session, worktree, branch)) = flipped else {
return;
};
tracing::info!(%id, "session vanished from tmux — marking dead");
if let Some(wt) = &worktree {
if settle_worktree_on_exit(inner, id, wt, branch).await {
// The tree is settled (removed or retained); a stale "in the
// way" would describe something the session no longer holds.
session.blocked_work = None;
let mut sessions = inner.sessions.lock().await;
if let Some(record) = sessions.get_mut(&id) {
record.session.worktree = None;
record.session.blocked_work = None;
record.worktree_path = None;
record.worktree_base_repo = None;
record.worktree_branch = None;
}
}
}
let _ = inner.sidecar.write(&session);
let _ = inner.events.send(CatalogEvent::SessionUpdated { session });
}
/// Apply the exit rule (#82) to a vanished session's worktree: probe what
/// would be lost through the kill's own predicate, then remove a pristine
/// tree or retain anything else under `exited-with-unsaved-work` — recorded
/// before anything is torn down, exactly as the forced kill does (#81). A
/// removal that fails falls back to retaining, since whatever blocked it is
/// by definition something worth keeping.
///
/// Returns whether the worktree was fully accounted for. When it wasn't,
/// nothing was destroyed and the caller keeps the reference on the record.
async fn settle_worktree_on_exit(
inner: &Arc<Inner>,
id: Uuid,
(base_repo, path): &(PathBuf, PathBuf),
known_branch: Option<String>,
) -> bool {
let probe = match probe_worktree(&(base_repo.clone(), path.clone())).await {
Ok(probe) => probe,
Err(e) => {
tracing::error!(
%id,
path = %path.display(),
error = ?e,
"natural exit: could not probe the worktree; leaving it untouched"
);
return false;
}
};
if probe.is_clear() {
match inner.worktree_mgr.remove(base_repo, path) {
Ok(()) => {
tracing::info!(%id, path = %path.display(), "natural exit: pristine worktree removed");
return true;
}
Err(e) => {
tracing::warn!(
%id,
path = %path.display(),
error = ?e,
"natural exit: removal failed; falling back to retention"
);
}
}
}
match record_retention(
inner,
&(base_repo.clone(), path.clone()),
known_branch,
RetainedReason::ExitedWithUnsavedWork,
)
.await
{
Ok(entry) => {
tracing::info!(
%id,
path = %path.display(),
entry = %entry.id,
"natural exit: worktree retained with unsaved work"
);
true
}
Err(e) => {
tracing::error!(
%id,
path = %path.display(),
error = ?e,
"natural exit: could not record the retained worktree; leaving it untouched"
);
false
}
}
}
/// Tear down a session's artifacts after it has already been removed from the
/// live map. Shared by [`SessionManager::kill`] and [`reap_dead_session`],
/// which previously hand-rolled the same five-step choreography and only
@@ -750,7 +1036,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 +1055,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]
@@ -0,0 +1,116 @@
//! The performance contract of #83: the blocked-work pass must cost NO git
//! probe at all for a session without a worktree, while a session with one is
//! still probed.
//!
//! `git` itself is shimmed through PATH with a wrapper that logs every
//! invocation before exec'ing the real binary, so the assertion counts actual
//! probes rather than inferring them from state. This lives in its own test
//! binary because it mutates the process environment.
mod common;
use std::collections::HashMap;
use std::process::Command;
use std::time::Duration;
use common::{init_bare_repo, init_repo, push_all_to_origin, wait_for_async, DaemonFixture};
use maestro_protocol::{CreateSessionRequest, InTheWay, WorktreeSpec};
use tempfile::TempDir;
fn request(cwd: &std::path::Path, repo: &std::path::Path, branch: &str) -> CreateSessionRequest {
CreateSessionRequest {
cwd: cwd.to_path_buf(),
command: "sleep 30".into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: Some(WorktreeSpec {
base_repo: repo.to_path_buf(),
branch: branch.into(),
create_branch: true,
}),
}
}
#[tokio::test]
async fn a_session_without_a_worktree_costs_no_probe() {
let fx = DaemonFixture::new();
let log = TempDir::new().unwrap();
let log_path = log.path().join("git-calls.log");
// Session A owns a real (pristine, pushed) worktree; session B has none.
let repo_a = TempDir::new().unwrap();
init_repo(repo_a.path());
init_bare_repo(&repo_a.path().join("origin.git"));
push_all_to_origin(repo_a.path(), &repo_a.path().join("origin.git"));
fx.manager
.create(request(repo_a.path(), repo_a.path(), "feat/probed"))
.await
.expect("create A");
let cwd_b = TempDir::new().unwrap();
let mut req_b = request(cwd_b.path(), cwd_b.path(), "feat/unused");
req_b.worktree = None;
fx.manager.create(req_b).await.expect("create B");
// Shim `git` AFTER setup so only watcher-driven probes get logged.
let real_git = String::from_utf8(
Command::new("sh")
.arg("-c")
.arg("command -v git")
.output()
.expect("locate git")
.stdout,
)
.unwrap()
.trim()
.to_string();
let shim_dir = TempDir::new().unwrap();
let script = format!(
"#!/bin/sh\nprintf '%s\\n' \"$*\" >> {}\nexec {} \"$@\"\n",
log_path.display(),
real_git,
);
std::fs::write(shim_dir.path().join("git"), script).unwrap();
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(
shim_dir.path().join("git"),
std::fs::Permissions::from_mode(0o755),
)
.unwrap();
}
let old_path = std::env::var("PATH").unwrap_or_default();
std::env::set_var(
"PATH",
format!("{}:{}", shim_dir.path().display(), old_path),
);
fx.manager.start_liveness_watcher_with_blocked_work(
Duration::from_millis(50),
Duration::from_millis(100),
);
// The pass demonstrably runs and probed A's tree...
wait_for_async(Duration::from_secs(5), || async {
fx.manager
.snapshot()
.await
.iter()
.any(|s| s.worktree.is_some() && s.blocked_work == Some(InTheWay::default()))
})
.await;
std::env::set_var("PATH", &old_path);
let calls = std::fs::read_to_string(&log_path).unwrap_or_default();
assert!(
!calls.is_empty(),
"the pass never probed anything — the shim saw no git call at all"
);
// ...while B — no worktree, so nothing to probe — was never touched.
assert!(
!calls.contains(cwd_b.path().to_str().unwrap()),
"a worktree-less session was probed:\n{calls}"
);
}
@@ -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");
+376 -18
View File
@@ -8,7 +8,9 @@ use std::process::Command;
use std::sync::Arc;
use std::time::Duration;
use common::{init_bare_repo, init_repo, push_all_to_origin, wait_for_async, DaemonFixture};
use common::{
init_bare_repo, init_repo, push_all_to_origin, wait_for_async, DaemonFixture, TmuxFixture,
};
use maestro_daemon::SessionError;
use maestro_protocol::CreateSessionRequest;
use maestro_protocol::{CatalogEvent, SessionStatus, WorktreeSpec};
@@ -44,7 +46,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 +81,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 +187,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 +253,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 +302,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,12 +361,73 @@ 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());
}
/// #83: the watcher refreshes `Session.blocked_work` on its own cadence —
/// first "nothing blocks" for a pristine tree, then the breakdown once the
/// tree picks up unsaved work, so clients can warn before the kill attempt.
#[tokio::test]
async fn watcher_refreshes_the_blocked_work_indicator() {
let fx = DaemonFixture::new();
let repo = TempDir::new().unwrap();
init_repo(repo.path());
let wt_path = repo.path().join("worktrees").join("feat").join("indicator");
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/indicator".into(),
create_branch: true,
}),
})
.await
.expect("create");
// Pushed and clean: nothing in the way, and the indicator must say so.
let origin = TempDir::new().unwrap();
init_bare_repo(origin.path());
push_all_to_origin(repo.path(), origin.path());
fx.manager.start_liveness_watcher_with_blocked_work(
Duration::from_millis(50),
Duration::from_millis(100),
);
wait_for_async(Duration::from_secs(5), || async {
fx.manager.snapshot().await[0].blocked_work == Some(maestro_protocol::InTheWay::default())
})
.await;
// Unsaved work appears; the next pass must surface it.
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
std::fs::write(wt_path.join("notes.txt"), b"untracked\n").unwrap();
wait_for_async(Duration::from_secs(5), || async {
matches!(
&fx.manager.snapshot().await[0].blocked_work,
Some(b) if b.uncommitted_files >= 1 && b.untracked_files >= 1
)
})
.await;
// A steady state must not re-emit every tick (#83 change-detection).
let mut events = fx.manager.events();
while events.try_recv().is_ok() {}
tokio::time::sleep(Duration::from_millis(400)).await;
assert!(
!matches!(events.try_recv(), Ok(CatalogEvent::SessionUpdated { .. })),
"an unchanged worktree must not re-emit SessionUpdated"
);
}
/// Cleanup on kill is unconditional once the guard passes (#79): even a spec
/// that says "don't clean up" loses its worktree — the branch was pushed, so
/// nothing in the tree is unique and removal goes ahead.
@@ -385,7 +448,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 +460,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 +485,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 +493,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 +521,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 +548,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 +570,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 +706,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,13 +718,196 @@ 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());
assert!(!fx.manager.has(s.id).await);
}
/// Natural exit (#82): a session that ends on its own never reaches the kill
/// path, so the watcher applies the kill's rule itself. A worktree holding
/// unsaved work is RETAINED into the store with reason
/// `exited-with-unsaved-work`; the session stays in the catalog marked dead
/// until dismissed — and dismissing it afterwards neither refuses nor
/// retains a second time.
#[tokio::test]
async fn natural_exit_of_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("exited");
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/exited".into(),
create_branch: true,
}),
})
.await
.expect("create");
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
// The command finishes / the operator types exit: tmux loses the session.
fx.tmux.tmux.kill_session(s.id).expect("kill tmux directly");
fx.manager.start_liveness_watcher(Duration::from_millis(50));
wait_for_async(Duration::from_secs(3), || async {
fx.manager.status(s.id).await == Some(SessionStatus::Dead)
})
.await;
// The tree would have blocked a kill, so the exit kept it...
assert!(
wt_path.exists(),
"a dirty worktree must be retained on natural exit"
);
// ...in the store, under the exit reason.
let all = fx.retained_store().load_all().expect("load_all");
assert_eq!(all.len(), 1);
assert_eq!(all[0].path, wt_path);
assert_eq!(all[0].branch, "feat/exited");
assert_eq!(
all[0].reason,
maestro_protocol::RetainedReason::ExitedWithUnsavedWork
);
// The session itself stays until dismissed; its worktree reference is
// settled, so dismissal is neither refused nor retained twice.
// (`Dead` is observable a moment before the settle step finishes.)
wait_for_async(Duration::from_secs(3), || async {
let s = &fx.manager.snapshot().await[0];
s.worktree.is_none() && s.blocked_work.is_none()
})
.await;
let snap = fx.manager.snapshot().await;
assert_eq!(snap.len(), 1);
assert_eq!(snap[0].worktree, None);
fx.manager.kill(s.id, false).await.expect("dismiss");
assert!(!fx.manager.has(s.id).await);
assert!(
wt_path.exists(),
"dismissal must not remove what was retained"
);
assert_eq!(
fx.retained_store().load_all().unwrap().len(),
1,
"dismissal must not record a second entry for the same tree"
);
}
/// Natural exit (#82), pristine side of the same rule: nothing in the tree,
/// nothing to keep — the worktree is removed and no store entry appears.
#[tokio::test]
async fn natural_exit_of_a_pristine_worktree_removes_it() {
let fx = DaemonFixture::new();
let repo = TempDir::new().unwrap();
init_repo(repo.path());
let wt_path = repo.path().join("worktrees").join("feat").join("clean");
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/clean".into(),
create_branch: true,
}),
})
.await
.expect("create");
// Push everything so the branch has no unpushed commits: fully pristine.
let origin = TempDir::new().unwrap();
init_bare_repo(origin.path());
push_all_to_origin(repo.path(), origin.path());
fx.tmux.tmux.kill_session(s.id).expect("kill tmux directly");
fx.manager.start_liveness_watcher(Duration::from_millis(50));
wait_for_async(Duration::from_secs(3), || async {
fx.manager.status(s.id).await == Some(SessionStatus::Dead)
})
.await;
assert!(
!wt_path.exists(),
"a pristine worktree leaves nothing behind"
);
assert!(
fx.retained_store().load_all().unwrap().is_empty(),
"nothing was retained"
);
assert!(
fx.manager.has(s.id).await,
"the session stays until dismissed"
);
}
/// When nothing can be recorded — here: no retained store wired — the exit
/// destroys nothing. The tree stays on disk AND stays attached to the dead
/// record, where dismissing it later still goes through #81's forced-kill
/// retention. Losing the store must not lose the work.
#[tokio::test]
async fn natural_exit_without_a_retained_store_destroys_nothing() {
let tmux = TmuxFixture::new();
let state_dir = TempDir::new().unwrap();
let manager = maestro_daemon::SessionManager::new(
Arc::new(tmux.tmux.clone()),
maestro_daemon::sidecar::SidecarStore::new(state_dir.path().join("sessions")),
maestro_daemon::worktree::WorktreeManager::new(None),
state_dir.path().join("fifos"),
);
// Deliberately NO with_retained_store.
let repo = TempDir::new().unwrap();
init_repo(repo.path());
let wt_path = repo.path().join("worktrees").join("feat").join("nostore");
let s = 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/nostore".into(),
create_branch: true,
}),
})
.await
.expect("create");
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
tmux.tmux.kill_session(s.id).expect("kill tmux directly");
manager.start_liveness_watcher(Duration::from_millis(50));
wait_for_async(Duration::from_secs(3), || async {
manager.status(s.id).await == Some(SessionStatus::Dead)
})
.await;
assert!(
wt_path.exists(),
"nothing may be destroyed when it cannot be recorded"
);
let snap = manager.snapshot().await;
assert!(
snap[0].worktree.is_some(),
"the tree must stay attached to the record so a later forced kill can retain it"
);
}
fn run_git_in(dir: &std::path::Path, args: &[&str]) {
let out = Command::new("git")
.arg("-C")
@@ -587,5 +945,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
View File
@@ -28,6 +28,7 @@ fn sample(id: Uuid) -> Session {
activity: Default::default(),
last_activity_at: chrono::Utc::now(),
current_cwd: Some(PathBuf::from("/tmp/work/crates")),
blocked_work: None,
}
}
+17 -2
View File
@@ -12,7 +12,8 @@ use std::time::Duration;
use futures_util::{SinkExt, StreamExt};
use maestro_protocol::{
BranchInfo, CatalogEvent, CreateSessionRequest, DaemonClient, Project, Session,
BranchInfo, CatalogEvent, CreateSessionRequest, DaemonClient, KillOutcome, Project,
RetainedWorktree, Session,
};
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
use tokio_tungstenite::tungstenite::Message as WsMessage;
@@ -61,7 +62,21 @@ 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(|_| ())?)
}
/// Kill with `force=true`: overrides a kill refusal and retains the
/// guarded worktree (#81). Answers with what was kept.
pub async fn kill_session_forced(&self, id: Uuid) -> Result<KillOutcome, ClientError> {
Ok(self.daemon.kill_session(id, true).await?)
}
pub async fn list_worktrees(&self) -> Result<Vec<RetainedWorktree>, ClientError> {
Ok(self.daemon.list_worktrees().await?)
}
pub async fn reclaim_worktree(&self, id: Uuid) -> Result<(), ClientError> {
Ok(self.daemon.reclaim_worktree(id).await?)
}
pub async fn list_projects(&self) -> Result<Vec<Project>, ClientError> {
+522 -33
View File
@@ -35,6 +35,17 @@ fn create_req(cwd: &std::path::Path, command: &str) -> CreateSessionRequest {
}
}
/// A worktree spec built through its wire shape. The daemon and clients ship
/// together (#87 dropped `cleanup_on_kill` from the struct), so the JSON
/// object carries only the fields the lifecycle actually honours.
fn worktree_spec(base_repo: &std::path::Path, branch: &str, create_branch: bool) -> WorktreeSpec {
WorktreeSpec {
base_repo: base_repo.to_path_buf(),
branch: branch.into(),
create_branch,
}
}
/// Spawn request carrying a worktree spec against `base`.
fn worktree_req(base: &std::path::Path, branch: &str, create_branch: bool) -> CreateSessionRequest {
CreateSessionRequest {
@@ -42,12 +53,7 @@ fn worktree_req(base: &std::path::Path, branch: &str, create_branch: bool) -> Cr
command: "sleep 30".into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: Some(WorktreeSpec {
base_repo: base.to_path_buf(),
branch: branch.into(),
cleanup_on_kill: true,
create_branch,
}),
worktree: Some(worktree_spec(base, branch, create_branch)),
}
}
@@ -141,12 +147,7 @@ async fn kill_removes_clean_worktree() {
command: "sleep 30".into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: Some(WorktreeSpec {
base_repo: base.path().to_path_buf(),
branch: "feature/e2e".into(),
cleanup_on_kill: true,
create_branch: true,
}),
worktree: Some(worktree_spec(base.path(), "feature/e2e", true)),
};
let s = client.spawn_session(&req).await.expect("spawn worktree");
@@ -160,7 +161,7 @@ async fn kill_removes_clean_worktree() {
"expected a worktree for feature/e2e under <repo>/worktrees, got: {worktrees:?}"
);
push_to_throwaway_origin(base.path());
let _origin = throwaway_origin(base.path());
client.kill_session(s.id).await.expect("kill");
// Give the daemon a moment to remove the worktree, then verify.
@@ -199,12 +200,7 @@ async fn kill_refuses_worktree_with_uncommitted_changes() {
command: "sleep 30".into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: Some(WorktreeSpec {
base_repo: base.path().to_path_buf(),
branch: "feature/unsaved".into(),
cleanup_on_kill: true,
create_branch: true,
}),
worktree: Some(worktree_spec(base.path(), "feature/unsaved", true)),
};
let s = client.spawn_session(&req).await.expect("spawn worktree");
@@ -262,12 +258,7 @@ async fn spawn_on_existing_branch_checks_it_out() {
command: "sleep 30".into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: Some(WorktreeSpec {
base_repo: base.path().to_path_buf(),
branch: "feature/existing".into(),
cleanup_on_kill: true,
create_branch: false,
}),
worktree: Some(worktree_spec(base.path(), "feature/existing", false)),
};
let s = client
@@ -280,7 +271,7 @@ async fn spawn_on_existing_branch_checks_it_out() {
"expected a worktree checked out at feature/existing, got: {worktrees:?}"
);
push_to_throwaway_origin(base.path());
let _origin = throwaway_origin(base.path());
client.kill_session(s.id).await.expect("kill");
}
@@ -328,7 +319,7 @@ async fn spawn_new_branch_creates_worktree_and_runs_in_it() {
)
.await;
push_to_throwaway_origin(base.path());
let _origin = throwaway_origin(base.path());
client.kill_session(s.id).await.expect("kill");
}
@@ -376,7 +367,7 @@ async fn second_spawn_on_same_branch_is_rejected() {
"original worktree must be untouched"
);
push_to_throwaway_origin(base.path());
let _origin = throwaway_origin(base.path());
client.kill_session(first.id).await.expect("kill");
}
@@ -419,10 +410,481 @@ async fn spawn_from_linked_worktree_places_in_main_checkout() {
"worktrees must never nest inside another worktree"
);
push_to_throwaway_origin(base.path());
let _origin = throwaway_origin(base.path());
client.kill_session(s.id).await.expect("kill");
}
// ---------------------------------------------------------------------------
// Worktree lifecycle: collisions, retention, reclaim (#88)
// ---------------------------------------------------------------------------
/// #77/#88: the target path `<repo>/worktrees/<branch>` is occupied by a real
/// registered worktree the operator made by hand. The daemon sidesteps to the
/// suffixed directory and never touches the occupant — and both trees keep
/// working: two registered worktrees, two sessions, each running in its own.
#[tokio::test(flavor = "multi_thread")]
async fn occupied_path_gets_suffixed_directory_and_both_worktrees_work() {
let (_daemon, client) = bring_up().await;
let base = TempDir::new().unwrap();
init_repo(base.path());
// An operator-made worktree sits exactly where `feature/collide` would go.
// Its branch is a different one, so the requested branch stays creatable.
let squatter = base
.path()
.join("worktrees")
.join("feature")
.join("collide");
run_git(
&[
"worktree",
"add",
"-q",
"-b",
"operator/manual",
&squatter.to_string_lossy(),
],
base.path(),
);
let s = client
.spawn_session(&worktree_req(base.path(), "feature/collide", true))
.await
.expect("spawn onto occupied path");
let suffixed = base
.path()
.join("worktrees")
.join("feature")
.join("collide-2");
assert_eq!(
s.cwd, suffixed,
"the daemon must take the suffixed directory, not the occupied one"
);
assert!(suffixed.is_dir(), "suffixed worktree must exist on disk");
// Both trees are registered, distinct, and the squatter is byte-intact.
let registrations = git_worktree_list(base.path());
assert!(
registrations
.iter()
.any(|l| l == &format!("worktree {}", squatter.canonicalize().unwrap().display())),
"operator's worktree must stay registered: {registrations:?}"
);
assert!(
registrations.iter().any(|l| l.contains("collide-2")),
"suffixed worktree must be registered too: {registrations:?}"
);
// Both sessions run inside their own tree at the same time.
let sessions = client.list_sessions().await.expect("list");
assert!(
sessions.iter().any(|x| x.id == s.id && x.cwd == suffixed),
"session must keep running in its suffixed tree"
);
let _origin = throwaway_origin(base.path());
client.kill_session(s.id).await.expect("kill");
// Only the daemon-made trees go; the operator's manual one survives.
let after = git_worktree_list(base.path());
assert!(
after
.iter()
.any(|l| l == &format!("worktree {}", squatter.canonicalize().unwrap().display())),
"operator's worktree must outlive the sessions: {after:?}"
);
}
/// #77/#88: the target path holds a plain directory the daemon never made and
/// git has never heard of. The daemon neither adopts it nor errors — it builds
/// beside it and leaves every byte of it alone.
#[tokio::test(flavor = "multi_thread")]
async fn foreign_directory_is_sidestepped_never_adopted() {
let (_daemon, client) = bring_up().await;
let base = TempDir::new().unwrap();
init_repo(base.path());
let squatter = base
.path()
.join("worktrees")
.join("feature")
.join("foreign");
std::fs::create_dir_all(&squatter).unwrap();
std::fs::write(squatter.join("operator-notes.txt"), b"mine\n").unwrap();
let s = client
.spawn_session(&worktree_req(base.path(), "feature/foreign", true))
.await
.expect("spawn must sidestep, not adopt or fail");
let suffixed = base
.path()
.join("worktrees")
.join("feature")
.join("foreign-2");
assert_eq!(s.cwd, suffixed, "session must run in the suffixed tree");
assert_eq!(
std::fs::read(squatter.join("operator-notes.txt")).unwrap(),
b"mine\n",
"the foreign directory must be untouched"
);
let registrations = git_worktree_list(base.path());
let registered_paths: Vec<&str> = registrations
.iter()
.filter_map(|l| l.strip_prefix("worktree "))
.collect();
assert!(
!registered_paths
.iter()
.any(|p| *p == squatter.canonicalize().unwrap().display().to_string()),
"a plain directory must never be adopted as a worktree: {registered_paths:?}"
);
let _origin = throwaway_origin(base.path());
client.kill_session(s.id).await.expect("kill");
assert!(squatter.join("operator-notes.txt").exists());
}
/// #79/#88: killing with uncommitted changes is refused; committing and
/// pushing that work clears the guard and the very same kill then succeeds
/// and removes the tree. The session is the handle on the work: while it is
/// unsaved the session stays, once it is saved the tree goes.
#[tokio::test(flavor = "multi_thread")]
async fn guarded_kill_then_commit_and_push_makes_the_same_kill_succeed() {
let (_daemon, client) = bring_up().await;
let base = TempDir::new().unwrap();
init_repo(base.path());
let _origin = throwaway_origin(base.path());
let wt = base.path().join("worktrees").join("feature").join("guard");
let s = client
.spawn_session(&worktree_req(base.path(), "feature/guard", true))
.await
.expect("spawn");
std::fs::write(wt.join("README"), b"work in progress\n").unwrap();
// Refused: the edit is uncommitted.
let err = client
.kill_session(s.id)
.await
.expect_err("kill must refuse");
match err {
maestro_e2e::ClientError::Api(maestro_protocol::ClientError::Api { info, .. }) => {
assert_eq!(info.code, "kill_guarded", "{info:?}");
let breakdown = info.unsaved_work.expect("breakdown on the wire");
assert_eq!(breakdown.uncommitted_files, 1);
assert_eq!(breakdown.unpushed_commits, 0);
}
other => panic!("expected a kill_guarded refusal, got {other:?}"),
}
assert!(
client
.list_sessions()
.await
.unwrap()
.iter()
.any(|x| x.id == s.id),
"a refused kill keeps the session running"
);
assert_eq!(
std::fs::read(wt.join("README")).unwrap(),
b"work in progress\n",
"the worktree is untouched by the refusal"
);
// The operator does exactly what the refusal asked for: save and push.
commit_all(&wt, "wip");
push_all(base.path());
client.kill_session(s.id).await.expect("kill after saving");
let deadline = Instant::now() + Duration::from_secs(5);
while Instant::now() < deadline {
if !wt.exists() {
break;
}
tokio::time::sleep(Duration::from_millis(100)).await;
}
assert!(
!wt.exists(),
"a cleared worktree must be removed after kill"
);
let registrations = git_worktree_list(base.path());
assert!(
!registrations.iter().any(|l| l.contains("feature/guard")),
"removal must deregister the tree: {registrations:?}"
);
}
/// #81/#88: commits that no remote has refuse the kill; `--force` ends the
/// session anyway but RETAINS the worktree — force never destroys blocked
/// work, it parks it in the retained store with reason `force-killed`.
#[tokio::test(flavor = "multi_thread")]
async fn unpushed_commits_block_kill_and_force_retains_the_tree() {
let (_daemon, client) = bring_up().await;
let base = TempDir::new().unwrap();
init_repo(base.path());
let _origin = throwaway_origin(base.path());
let wt = base.path().join("worktrees").join("feature").join("local");
let s = client
.spawn_session(&worktree_req(base.path(), "feature/local", true))
.await
.expect("spawn");
std::fs::write(wt.join("README"), b"local only\n").unwrap();
commit_all(&wt, "not pushed yet");
// Refused: the commit is on no remote-tracking ref.
let err = client
.kill_session(s.id)
.await
.expect_err("kill must refuse");
match err {
maestro_e2e::ClientError::Api(maestro_protocol::ClientError::Api { info, .. }) => {
assert_eq!(info.code, "kill_guarded", "{info:?}");
let breakdown = info.unsaved_work.expect("breakdown on the wire");
assert_eq!(breakdown.unpushed_commits, 1);
assert_eq!(breakdown.uncommitted_files, 0);
}
other => panic!("expected a kill_guarded refusal, got {other:?}"),
}
// Force overrides the refusal and nothing else: session gone, tree kept.
let outcome = client.kill_session_forced(s.id).await.expect("forced kill");
let retained = outcome
.retained
.expect("force must retain the guarded tree");
assert_eq!(
retained.reason,
maestro_protocol::RetainedReason::ForceKilled
);
assert_eq!(retained.path, wt);
assert_eq!(retained.branch, "feature/local");
assert_eq!(retained.in_the_way.unpushed_commits, 1);
assert!(
!client
.list_sessions()
.await
.unwrap()
.iter()
.any(|x| x.id == s.id),
"the forced kill ends the session"
);
assert!(wt.is_dir(), "a force-retained worktree must stay on disk");
assert!(
git_worktree_list(base.path())
.iter()
.any(|l| l.contains("feature/local")),
"the retained tree stays registered in git"
);
}
/// #82/#88: a session that exits on its own never reaches the kill path, but
/// its worktree follows the same rule. Dirty → retained with reason
/// `exited-with-unsaved-work`; pristine → removed, leaving nothing behind.
#[tokio::test(flavor = "multi_thread")]
async fn natural_exit_settles_dirty_and_pristine_worktrees_differently() {
let (_daemon, client) = bring_up().await;
// Two repos so the two cases cannot see each other's state.
let dirty_repo = TempDir::new().unwrap();
init_repo(dirty_repo.path());
let _origin = throwaway_origin(dirty_repo.path());
let pristine_repo = TempDir::new().unwrap();
init_repo(pristine_repo.path());
let _origin_pristine = throwaway_origin(pristine_repo.path());
let dirty_wt = dirty_repo
.path()
.join("worktrees")
.join("feature")
.join("messy");
let pristine_wt = pristine_repo
.path()
.join("worktrees")
.join("feature")
.join("tidy");
let _dirty = client
.spawn_session(&CreateSessionRequest {
cwd: dirty_repo.path().to_path_buf(),
command: "sh -c 'echo unsaved > scratch.txt'".into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: Some(worktree_spec(dirty_repo.path(), "feature/messy", true)),
})
.await
.expect("spawn dirty");
let _pristine = client
.spawn_session(&create_req_cmd(
pristine_repo.path(),
"true",
"feature/tidy",
))
.await
.expect("spawn pristine");
assert!(dirty_wt.is_dir() && pristine_wt.is_dir());
// The watcher reaps dead tmux sessions every couple of ticks and settles
// each worktree accordingly. Poll for both outcomes.
wait_for_async(
"dirty exit retains its worktree",
Duration::from_secs(20),
|| {
let client = client.clone();
let path = dirty_wt.clone();
async move {
client
.list_worktrees()
.await
.ok()
.map(|wts| {
wts.iter().any(|w| {
w.path == path
&& w.reason
== maestro_protocol::RetainedReason::ExitedWithUnsavedWork
})
})
.unwrap_or(false)
}
},
)
.await;
wait_for_async(
"pristine exit removes its worktree entirely",
Duration::from_secs(20),
|| {
let client = client.clone();
let repo = pristine_repo.path().to_path_buf();
let path = pristine_wt.clone();
async move {
if path.exists() {
return false;
}
let listed = client.list_worktrees().await.ok();
let listed_clear = listed
.map(|wts| !wts.iter().any(|w| w.path == path))
.unwrap_or(false);
let registered = git_worktree_list(&repo);
!registered.iter().any(|l| l.contains("feature/tidy")) && listed_clear
}
},
)
.await;
// The dirty tree is still fully there, file included.
assert!(dirty_wt.is_dir());
assert_eq!(
std::fs::read(dirty_wt.join("scratch.txt")).unwrap(),
b"unsaved\n"
);
}
fn create_req_cmd(cwd: &std::path::Path, command: &str, branch: &str) -> CreateSessionRequest {
CreateSessionRequest {
cwd: cwd.to_path_buf(),
command: command.into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: Some(worktree_spec(cwd, branch, true)),
}
}
/// #80/#88: `GET /api/worktrees` lists what the daemon is holding, with the
/// reason and a live in-the-way breakdown; reclaim removes the directory and
/// the store entry but NEVER the branch — its commits outlive the reclaim.
#[tokio::test(flavor = "multi_thread")]
async fn worktrees_endpoint_lists_reasons_and_reclaim_keeps_the_branch() {
let (_daemon, client) = bring_up().await;
let base = TempDir::new().unwrap();
init_repo(base.path());
let _origin = throwaway_origin(base.path());
let wt = base.path().join("worktrees").join("feature").join("parked");
let s = client
.spawn_session(&worktree_req(base.path(), "feature/parked", true))
.await
.expect("spawn");
std::fs::write(wt.join("README"), b"worth keeping\n").unwrap();
commit_all(&wt, "worth keeping");
let sha = branch_sha(&wt, "feature/parked");
let outcome = client.kill_session_forced(s.id).await.expect("force kill");
assert!(outcome.retained.is_some());
let listed = client.list_worktrees().await.expect("list worktrees");
assert_eq!(listed.len(), 1, "exactly one retained entry: {listed:?}");
let entry = &listed[0];
assert_eq!(entry.path, wt);
assert_eq!(entry.branch, "feature/parked");
assert_eq!(entry.reason, maestro_protocol::RetainedReason::ForceKilled);
assert_eq!(entry.in_the_way.unpushed_commits, 1);
client.reclaim_worktree(entry.id).await.expect("reclaim");
assert!(!wt.exists(), "reclaim removes the directory");
assert!(
client.list_worktrees().await.unwrap().is_empty(),
"reclaim drops the store entry"
);
// The whole point of reclaim: the branch and its commits survive.
assert_eq!(
branch_sha(base.path(), "feature/parked"),
sha,
"the branch must keep pointing at the same commit after reclaim"
);
}
/// The retained store is state-dir backed (#80): a daemon restart must not
/// orphan the trees it is holding — they are still on disk with entries
/// pointing at them, and still reclaimable afterwards.
#[tokio::test(flavor = "multi_thread")]
async fn retained_store_survives_a_daemon_restart() {
let (mut daemon, client) = bring_up().await;
let base = TempDir::new().unwrap();
init_repo(base.path());
let _origin = throwaway_origin(base.path());
let wt = base
.path()
.join("worktrees")
.join("feature")
.join("durable");
let s = client
.spawn_session(&worktree_req(base.path(), "feature/durable", true))
.await
.expect("spawn");
std::fs::write(wt.join("README"), b"survives restarts\n").unwrap();
commit_all(&wt, "survives restarts");
client.kill_session_forced(s.id).await.expect("force kill");
daemon.restart().await;
let listed = client.list_worktrees().await.expect("list after restart");
assert_eq!(listed.len(), 1, "store must survive restart: {listed:?}");
assert_eq!(listed[0].path, wt);
assert_eq!(
listed[0].reason,
maestro_protocol::RetainedReason::ForceKilled
);
assert!(wt.is_dir(), "the tree itself must survive the restart too");
client
.reclaim_worktree(listed[0].id)
.await
.expect("reclaim");
assert!(!wt.exists());
}
// ---------------------------------------------------------------------------
// GET /api/projects + /api/branches (#54/#55/#66)
// ---------------------------------------------------------------------------
@@ -788,10 +1250,12 @@ fn git_worktree_list(dir: &std::path::Path) -> Vec<String> {
.collect()
}
/// Push every branch of `repo` to a throwaway bare remote, so its commits are
/// reachable from remote-tracking refs. Without one the kill guard (#79)
/// reads every commit as unpushed, and any worktree session refuses to die.
fn push_to_throwaway_origin(repo: &std::path::Path) {
/// Bare remote registered as `origin` on `repo`, seeded with every current
/// branch, so existing commits are reachable from remote-tracking refs.
/// Without one the kill guard (#79) reads every commit as unpushed, and any
/// worktree session refuses to die. The caller must keep the `TempDir` alive
/// while the remote is still needed; later pushes go through [`push_all`].
fn throwaway_origin(repo: &std::path::Path) -> TempDir {
let origin = tempfile::TempDir::new().unwrap();
run_git(
&["init", "-q", "--bare", origin.path().to_str().unwrap()],
@@ -803,9 +1267,34 @@ fn push_to_throwaway_origin(repo: &std::path::Path) {
.args(["remote", "add", "origin", origin.path().to_str().unwrap()])
.output()
.expect("git remote add");
push_all(repo);
origin
}
fn push_all(repo: &std::path::Path) {
run_git(&["push", "-q", "--all", "origin"], repo);
}
fn commit_all(dir: &std::path::Path, message: &str) {
run_git(&["add", "-A"], dir);
run_git(&["commit", "-q", "-m", message], dir);
}
fn branch_sha(repo: &std::path::Path, branch: &str) -> String {
let out = Command::new("git")
.arg("-C")
.arg(repo)
.args(["rev-parse", "--verify", &format!("refs/heads/{branch}")])
.output()
.expect("git rev-parse");
assert!(
out.status.success(),
"branch {branch} missing: {}",
String::from_utf8_lossy(&out.stderr)
);
String::from_utf8_lossy(&out.stdout).trim().to_string()
}
#[tokio::test(flavor = "multi_thread")]
async fn daemon_events_ws_streams_snapshot_then_session_added() {
// NAP-248: subscribe to the daemon's WS /api/events, expect an initial
+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> {
+65 -8
View File
@@ -87,6 +87,13 @@ pub struct Session {
/// from daemons that predate the field.
#[serde(default)]
pub current_cwd: Option<PathBuf>,
/// Whether the session's worktree currently holds anything that would
/// block an unforced kill (#83), summarised by [`InTheWay`]'s `Display`.
/// Refreshed by the daemon's watcher; `None` for sessions without a
/// worktree and until the first probe. Defaults on the wire so payloads
/// from daemons that predate the field stay forward-compatible.
#[serde(default)]
pub blocked_work: Option<InTheWay>,
}
/// Default for [`Session::last_activity_at`] when an older payload omits it.
@@ -119,11 +126,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 +197,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
@@ -523,6 +538,7 @@ mod tests {
.unwrap()
.with_timezone(&Utc),
current_cwd: Some(PathBuf::from("/home/naps62/projects/maestro/crates")),
blocked_work: None,
}
}
@@ -542,6 +558,34 @@ mod tests {
assert_eq!(s.activity, ActivityState::Idle);
assert_eq!(s.last_activity_at, epoch());
assert!(s.current_cwd.is_none());
assert!(s.blocked_work.is_none());
}
#[test]
fn blocked_work_defaults_to_none_on_legacy_payload() {
// A pre-#83 daemon omits the indicator; it must deserialize to `None`
// rather than failing, and a present value round-trips.
let json = serde_json::json!({
"id": Uuid::from_u128(1),
"cwd": "/tmp",
"command": "bash",
"status": "Running",
"created_at": "2026-05-24T12:00:00Z"
});
let s: Session = serde_json::from_value(json).unwrap();
assert_eq!(s.blocked_work, None);
let mut with_blocker = sample_session();
with_blocker.blocked_work = Some(InTheWay {
uncommitted_files: 3,
..InTheWay::default()
});
assert_eq!(
with_blocker.clone().blocked_work,
serde_json::from_value::<Session>(serde_json::to_value(&with_blocker).unwrap())
.unwrap()
.blocked_work
);
}
#[test]
@@ -581,7 +625,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 +654,6 @@ mod tests {
});
let spec: WorktreeSpec = serde_json::from_value(json).unwrap();
assert!(spec.create_branch);
assert!(spec.cleanup_on_kill);
}
#[test]
@@ -619,7 +661,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 +720,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}")
+1
View File
@@ -43,6 +43,7 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session {
activity: Default::default(),
last_activity_at: Utc::now(),
current_cwd: None,
blocked_work: None,
}
}
+10 -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}")),
}
}
@@ -1101,6 +1107,7 @@ mod tests {
activity: Default::default(),
last_activity_at: Utc::now(),
current_cwd: None,
blocked_work: None,
}],
});
app.dispatch(Action::SidebarNext);
@@ -1608,6 +1615,7 @@ mod tests {
last_activity_at: chrono::Utc::now(),
current_cwd: None,
blocked_work: None,
}],
});
app.dispatch(Action::KillHighlighted);
+1
View File
@@ -195,6 +195,7 @@ mod tests {
activity: Default::default(),
last_activity_at: chrono::Utc::now(),
current_cwd: None,
blocked_work: None,
}
}
-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,
}),
})
+31 -9
View File
@@ -14,23 +14,28 @@ use ratatui::Frame;
#[derive(Debug, Clone)]
pub struct KillConfirm {
pub label: String,
/// #83: what the daemon's watcher last saw in the worktree, phrased by
/// [`maestro_protocol::InTheWay`]'s Display — so "this will be refused"
/// is visible before the attempt. `None` while nothing blocks.
pub blockers: Option<String>,
}
impl KillConfirm {
pub fn render(&self, f: &mut Frame, parent: Rect) {
let area = centered(parent, 50, 5);
let area = centered(parent, 60, if self.blockers.is_some() { 6 } else { 5 });
f.render_widget(Clear, area);
let block = Block::default().title(" kill ").borders(Borders::ALL);
let inner = block.inner(area);
f.render_widget(block, area);
f.render_widget(
Paragraph::new(vec![
Line::from(format!("Kill session {}?", self.label)),
Line::from(""),
Line::from("[y] yes [n] no"),
]),
inner,
);
let mut lines = vec![Line::from(format!("Kill session {}?", self.label))];
if let Some(blockers) = &self.blockers {
// Same phrasing the reclaim confirmation uses for its own guard.
lines.push(Line::from(format!("Currently in the way: {blockers}")));
lines.push(Line::from("The kill will be refused unless forced."));
}
lines.push(Line::from(""));
lines.push(Line::from("[y] yes [n] no"));
f.render_widget(Paragraph::new(lines), inner);
}
}
@@ -121,6 +126,7 @@ mod tests {
fn kill_confirm_renders_prompt() {
let modal = KillConfirm {
label: "session-7".into(),
blockers: None,
};
let rendered = render_to_string(60, 10, |f| modal.render(f, f.area()));
assert!(rendered.contains("Kill"));
@@ -129,6 +135,22 @@ mod tests {
assert!(rendered.contains("no"));
}
/// #83: when the worktree blocks, the confirm names it before the
/// operator commits — in `InTheWay`'s own words.
#[test]
fn kill_confirm_names_what_blocks() {
let modal = KillConfirm {
label: "session-7".into(),
blockers: Some("2 uncommitted files".into()),
};
let rendered = render_to_string(70, 10, |f| modal.render(f, f.area()));
assert!(
rendered.contains("Currently in the way: 2 uncommitted files"),
"{rendered}"
);
assert!(rendered.contains("refused"), "{rendered}");
}
#[test]
fn help_overlay_lists_lines() {
let help = HelpOverlay {
+11
View File
@@ -146,8 +146,19 @@ pub fn draw(f: &mut Frame, inputs: RenderInputs<'_>) -> RegionMap {
match inputs.state.overlay() {
Some(OverlayKind::KillConfirm(id)) => {
// #83: surface what the watcher last saw in the worktree, in
// InTheWay's own words, before the operator commits to the kill.
let blockers = inputs
.sidebar
.sessions
.iter()
.find(|s| &s.id == id)
.and_then(|s| s.blocked_work)
.filter(|b| !b.is_clear())
.map(|b| b.to_string());
let kc = KillConfirm {
label: id.to_string(),
blockers,
};
kc.render(f, area);
map.modal = Some(area);
+44 -1
View File
@@ -52,6 +52,13 @@ impl Sidebar {
(SessionStatus::Running, false) => "○",
};
let label = name_for(s);
// #83: a session whose worktree blocks a kill gets a warning
// glyph; the kill confirm names the specifics.
let blocked = s
.blocked_work
.filter(|b| !b.is_clear())
.map(|_| " ⚠")
.unwrap_or_default();
let style = if self.highlight == Some(s.id) {
Style::default().add_modifier(Modifier::REVERSED)
} else {
@@ -59,7 +66,7 @@ impl Sidebar {
};
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled(format!("{glyph} {label}"), style),
Span::styled(format!("{glyph} {label}{blocked}"), style),
]));
}
f.render_widget(Paragraph::new(lines), inner);
@@ -99,6 +106,7 @@ mod tests {
activity: Default::default(),
last_activity_at: chrono::Utc::now(),
current_cwd: None,
blocked_work: None,
}
}
@@ -172,6 +180,40 @@ mod tests {
assert!(line.contains('✗'), "expected dead glyph in: {line:?}");
}
/// #83: a session whose worktree blocks a kill carries the warning glyph;
/// a clear or unprobed one does not.
#[test]
fn blocked_session_gets_a_warning_glyph() {
use maestro_protocol::InTheWay;
let mut blocked = session_named(1, "claude");
blocked.blocked_work = Some(InTheWay {
unpushed_commits: 4,
..InTheWay::default()
});
let mut clear = session_named(2, "bash");
clear.blocked_work = Some(InTheWay::default());
let unprobed = session_named(3, "vim");
let sb = Sidebar {
sessions: vec![blocked, clear, unprobed],
highlight: None,
attached: Default::default(),
};
let rendered = render_to_string(&sb, 40, 8);
let line = rendered
.lines()
.find(|l| l.contains("claude"))
.expect("blocked row");
assert!(line.contains('⚠'), "expected warning glyph in: {line:?}");
for name in ["bash", "vim"] {
let line = rendered
.lines()
.find(|l| l.contains(name))
.expect("clear row");
assert!(!line.contains('⚠'), "{name} must not warn: {line:?}");
}
}
#[test]
fn falls_back_to_command_when_name_metadata_is_absent() {
let s = Session {
@@ -188,6 +230,7 @@ mod tests {
last_activity_at: chrono::Utc::now(),
current_cwd: None,
blocked_work: None,
};
let sb = Sidebar {
sessions: vec![s],
+20 -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
@@ -92,6 +97,17 @@ Nobody is waiting for an answer here, so there is nothing to refuse; the choice
only between removing and retaining. It uses the same predicate as the kill, from
the same place, so the two paths cannot drift apart.
The session itself stays in the catalog marked dead until it is dismissed — the
replay buffer of a crashed agent outlives it — but its worktree is settled at
once. Once settled, the session drops its worktree reference, so dismissing it
afterwards neither refuses (there is no guard left to trip) nor records a second
store entry.
If settling is impossible — the probe fails, or the store cannot take the entry —
nothing is destroyed. The tree stays on disk *and* stays attached to the dead
record, where dismissing it later still goes through `--force`'s
retain-before-teardown path. Losing the store must never mean losing the work.
## The retained store
A `state_dir`-backed record of worktrees the daemon created that no longer have a
+106
View File
@@ -0,0 +1,106 @@
// @vitest-environment happy-dom
import { beforeAll, describe, expect, it } from "vitest";
import { renderSidebar } from "./catalog.js";
import type { CatalogListener, CatalogStore } from "./events.js";
import type { InTheWay, Session } from "./types.js";
// happy-dom runs without a backing store; renderSidebar reads the persisted
// sidebar width on mount.
beforeAll(() => {
Object.defineProperty(window, "localStorage", {
value: { getItem: () => null, setItem: () => {} },
});
// No daemon here; renderSidebar treats a failed info fetch as "absolute
// paths are a fine fallback".
window.fetch = () => Promise.reject(new Error("offline"));
});
function session(overrides: Partial<Session>): Session {
return {
id: "11111111-2222-3333-4444-555555555555",
cwd: "/home/naps/tea/maestro",
command: "claude",
env: {},
metadata: {},
status: "Running",
created_at: "2026-08-24T10:00:00Z",
...overrides,
};
}
function storeWith(sessions: Session[]): CatalogStore {
const state = { sessions: new Map(sessions.map((s) => [s.id, s])) };
return {
snapshot: () => state,
// Like the real store, deliver an immediate snapshot so the sidebar
// renders its rows during construction.
subscribe: (fn: CatalogListener) => {
fn(state);
return () => {};
},
} as unknown as CatalogStore;
}
function rowFor(el: HTMLElement, id: string): HTMLElement | null {
return el.querySelector<HTMLElement>(`[data-session-id="${id}"]`);
}
describe("blocked-work indicator (#83)", () => {
it("marks the row of a blocked session with the shared phrasing", () => {
const blocked = session({
id: "aaaaaaaa-0000-0000-0000-000000000001",
blocked_work: { uncommitted_files: 3, unpushed_commits: 2 },
});
const el = renderSidebar(storeWith([blocked])).el;
const badge = rowFor(el, blocked.id)?.querySelector(".leaf-blocked");
expect(badge?.textContent).toContain("⚠");
expect(badge?.textContent).toContain("3 uncommitted files, 2 unpushed commits");
});
it("does not mark clear or unprobed sessions", () => {
const clear = session({
id: "aaaaaaaa-0000-0000-0000-000000000002",
blocked_work: {} as InTheWay,
});
const unprobed = session({
id: "aaaaaaaa-0000-0000-0000-000000000003",
});
const el = renderSidebar(storeWith([clear, unprobed])).el;
expect(rowFor(el, clear.id)?.querySelector(".leaf-blocked")).toBeNull();
expect(rowFor(el, unprobed.id)?.querySelector(".leaf-blocked")).toBeNull();
});
it("restates the warning at the kill confirm", async () => {
const confirmations: string[] = [];
window.confirm = (msg?: string) => {
confirmations.push(msg ?? "");
return false;
};
const blocked = session({
id: "aaaaaaaa-0000-0000-0000-000000000004",
command: "bash",
blocked_work: { untracked_files: 1 },
});
const el = renderSidebar(storeWith([blocked])).el;
const kill = rowFor(el, blocked.id)?.querySelector<HTMLButtonElement>(".leaf-kill");
kill?.click();
await Promise.resolve();
expect(confirmations).toHaveLength(1);
expect(confirmations[0]).toContain("1 untracked file");
expect(confirmations[0]).toContain("refused unless forced");
});
it("confirms plainly when nothing blocks", async () => {
const confirmations: string[] = [];
window.confirm = (msg?: string) => {
confirmations.push(msg ?? "");
return false;
};
const plain = session({ id: "aaaaaaaa-0000-0000-0000-000000000005" });
const el = renderSidebar(storeWith([plain])).el;
rowFor(el, plain.id)?.querySelector<HTMLButtonElement>(".leaf-kill")?.click();
await Promise.resolve();
expect(confirmations).toEqual(['Kill session "claude"?']);
});
});
+11 -2
View File
@@ -3,7 +3,7 @@ import { getDaemonInfo, killSession } from "./api.js";
import { HUD_TOGGLE_EVENT, isHudEnabled, toggleHud } from "./debug.js";
import { CatalogStore } from "./events.js";
import type { Session } from "./types.js";
import { abbreviateFolderName, sessionName } from "./types.js";
import { abbreviateFolderName, formatInTheWay, sessionBlockers, sessionName } from "./types.js";
import type { TreeNode } from "./tree.js";
import { buildTree } from "./tree.js";
import { createThemeToggle } from "./theme.js";
@@ -64,10 +64,14 @@ function renderNode(node: TreeNode, depth: number): HTMLElement {
// The directory is the enclosing folder, so only surface the command — and
// only when it isn't already what the name shows.
const showCmd = s.command !== name;
// #83: warn on the row itself, in `formatInTheWay`'s shared phrasing, so
// "this will be refused" is visible before the kill is attempted.
const blockers = sessionBlockers(s);
row.innerHTML = `
<span class="leaf-name">${escapeHtml(name)}</span>
${showCmd ? `<span class="leaf-meta"><span class="leaf-cmd">${escapeHtml(s.command)}</span></span>` : ""}
${running ? "" : `<span class="leaf-badge">exited</span>`}
${blockers ? `<span class="leaf-badge leaf-blocked" title="Kill will be refused unless forced">⚠ ${escapeHtml(formatInTheWay(blockers))}</span>` : ""}
<span class="leaf-actions">
<button class="leaf-act leaf-clone" type="button" title="Spawn similar" aria-label="Spawn similar to ${escapeHtml(name)}">⧉</button>
<button class="leaf-act leaf-kill" type="button" title="Kill session" aria-label="Kill ${escapeHtml(name)}">✕</button>
@@ -83,7 +87,12 @@ function renderNode(node: TreeNode, depth: number): HTMLElement {
killBtn.addEventListener("click", async (ev) => {
ev.preventDefault();
ev.stopPropagation();
if (!confirm(`Kill session "${sessionName(s)}"?`)) return;
// #83: the confirm restates the warning at the moment of decision.
const blockers = sessionBlockers(s);
const question = blockers
? `Kill session "${name}"? Its worktree still has ${formatInTheWay(blockers)} — the kill will be refused unless forced.`
: `Kill session "${name}"?`;
if (!confirm(question)) return;
try {
await killSession(s.id);
} catch (e) {
-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",
+7
View File
@@ -831,6 +831,13 @@ textarea {
flex: 0 0 auto;
}
/* #83: the session row's blocked-work warning — an unforced kill will be
refused, and the operator should see that before pressing the button. */
.leaf-badge.leaf-blocked {
color: var(--warn, #b45309);
border-color: var(--warn, #b45309);
}
.leaf-actions {
position: absolute;
right: var(--sp-1);
+18 -1
View File
@@ -31,6 +31,11 @@ export interface Session {
* observed it (`#{pane_current_path}`) — distinct from `cwd` (the spawn dir).
* Drives the directory-based session tree. Absent until first observed. */
current_cwd?: string | null;
/** What currently blocks an unforced kill of this session (#83), as the
* daemon's watcher last probed it. Absent until probed and for sessions
* without a worktree; all counts default to 0 on the wire for older
* daemons. */
blocked_work?: InTheWay | null;
}
/** One directory's immediate subdirectories (NAP-329). */
@@ -113,7 +118,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;
@@ -181,6 +185,19 @@ export function reasonLabel(reason: RetainedReason): string {
return reason;
}
/** #83: this session's blocked-work breakdown when something actually blocks
* an unforced kill; `null` while clear or not yet probed. Render it with
* {@link formatInTheWay} so every client phrases it identically. */
export function sessionBlockers(s: Session): InTheWay | null {
const w = s.blocked_work;
if (!w) return null;
const blocked =
(w.uncommitted_files ?? 0) > 0 ||
(w.untracked_files ?? 0) > 0 ||
(w.unpushed_commits ?? 0) > 0;
return blocked ? w : null;
}
export interface CreateSessionRequest {
cwd: string;
command: string;