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 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
26 changed files with 617 additions and 84 deletions
-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,
@@ -168,7 +163,6 @@ mod tests {
"/repos/foo",
"--branch",
"feat/x",
"--cleanup-on-kill",
"-e",
"FOO=bar",
"-e",
@@ -179,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()),]
@@ -189,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.
+24
View File
@@ -62,6 +62,11 @@ pub async fn dispatch(
run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?;
}
Command::Kill(args) => {
// #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 {
@@ -99,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)]
+51 -5
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,
@@ -94,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 {
@@ -116,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 {
@@ -165,6 +177,7 @@ mod tests {
activity: Default::default(),
last_activity_at: chrono::Utc::now(),
current_cwd: None,
blocked_work: None,
}
}
@@ -225,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}");
}
}
}
}
+89 -1
View File
@@ -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 {
@@ -344,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) {
@@ -489,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);
@@ -496,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 {
@@ -505,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;
}
}
});
}
@@ -541,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"));
@@ -809,6 +839,61 @@ 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 settle its worktree per the natural-exit rule (#82).
///
@@ -852,10 +937,13 @@ async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
if let Some(wt) = &worktree {
if settle_worktree_on_exit(inner, id, wt, branch).await {
session.worktree = None;
// 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;
@@ -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}"
);
}
-2
View File
@@ -363,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,
}),
})
@@ -417,7 +416,6 @@ async fn delete_with_force_ends_the_session_and_answers_what_was_retained() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/guarded".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
+63 -10
View File
@@ -367,6 +367,67 @@ async fn liveness_watcher_marks_dead_sessions_and_keeps_them_until_dismissed() {
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.
@@ -387,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,
}),
})
@@ -425,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,
}),
})
@@ -489,7 +548,6 @@ 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,
}),
})
@@ -541,7 +599,6 @@ async fn force_killing_a_guarded_worktree_retains_it_in_the_store() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/forced".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
@@ -607,7 +664,6 @@ async fn force_on_a_clear_worktree_removes_it_as_usual() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/clear".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
@@ -650,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,
}),
})
@@ -693,7 +748,6 @@ async fn natural_exit_of_a_guarded_worktree_retains_it_in_the_store() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/exited".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
@@ -728,7 +782,8 @@ async fn natural_exit_of_a_guarded_worktree_retains_it_in_the_store() {
// 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 {
fx.manager.snapshot().await[0].worktree.is_none()
let s = &fx.manager.snapshot().await[0];
s.worktree.is_none() && s.blocked_work.is_none()
})
.await;
let snap = fx.manager.snapshot().await;
@@ -767,7 +822,6 @@ async fn natural_exit_of_a_pristine_worktree_removes_it() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/clean".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
@@ -829,7 +883,6 @@ async fn natural_exit_without_a_retained_store_destroys_nothing() {
worktree: Some(WorktreeSpec {
base_repo: repo.path().to_path_buf(),
branch: "feat/nostore".into(),
cleanup_on_kill: true,
create_branch: true,
}),
})
+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,
}
}
+5 -6
View File
@@ -39,12 +39,11 @@ fn create_req(cwd: &std::path::Path, command: &str) -> CreateSessionRequest {
/// 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 {
serde_json::from_value(serde_json::json!({
"base_repo": base_repo,
"branch": branch,
"create_branch": create_branch,
}))
.expect("worktree spec")
WorktreeSpec {
base_repo: base_repo.to_path_buf(),
branch: branch.into(),
create_branch,
}
}
/// Spawn request carrying a worktree spec against `base`.
+36 -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`.
@@ -536,6 +538,7 @@ mod tests {
.unwrap()
.with_timezone(&Utc),
current_cwd: Some(PathBuf::from("/home/naps62/projects/maestro/crates")),
blocked_work: None,
}
}
@@ -555,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]
@@ -594,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,
}),
};
@@ -624,7 +654,6 @@ mod tests {
});
let spec: WorktreeSpec = serde_json::from_value(json).unwrap();
assert!(spec.create_branch);
assert!(spec.cleanup_on_kill);
}
#[test]
@@ -632,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));
+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,
}
}
+2
View File
@@ -1107,6 +1107,7 @@ mod tests {
activity: Default::default(),
last_activity_at: Utc::now(),
current_cwd: None,
blocked_work: None,
}],
});
app.dispatch(Action::SidebarNext);
@@ -1614,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],
+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;