Compare commits
15
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fb87a00906 | ||
|
|
d4dcfe7f00 | ||
|
|
6161cabe77 | ||
|
|
524a848fc6 | ||
|
|
187bcb01d6 | ||
|
|
003e3839bc | ||
|
|
d2079cac3a | ||
|
|
2083911baa | ||
|
|
b5450651d1 | ||
|
|
8aa007c73b | ||
|
|
f4d12fa02d | ||
|
|
dde8dd8cfc | ||
|
|
2fc19a886e | ||
|
|
0b95d11aa6 | ||
|
|
a85084d0d9 |
@@ -62,6 +62,11 @@ pub async fn dispatch(
|
|||||||
run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?;
|
run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?;
|
||||||
}
|
}
|
||||||
Command::Kill(args) => {
|
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?;
|
let outcome = run_kill(&daemon, &args).await?;
|
||||||
println!("killed {}", args.session_id);
|
println!("killed {}", args.session_id);
|
||||||
if let Some(w) = outcome.retained {
|
if let Some(w) = outcome.retained {
|
||||||
@@ -99,6 +104,25 @@ pub async fn dispatch(
|
|||||||
/// Default detach byte, matching `maestro attach --detach-key`'s `^q`.
|
/// Default detach byte, matching `maestro attach --detach-key`'s `^q`.
|
||||||
const CTRL_Q: u8 = 0x11;
|
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 —
|
/// HTTP client pointed at the daemon's API. That surface needs no auth —
|
||||||
/// reaching `bind_addr` is the authorization.
|
/// reaching `bind_addr` is the authorization.
|
||||||
#[allow(clippy::result_large_err)]
|
#[allow(clippy::result_large_err)]
|
||||||
|
|||||||
@@ -93,8 +93,8 @@ fn print_session_table(out: &mut impl Write, sessions: &[Session], info: &Daemon
|
|||||||
}
|
}
|
||||||
writeln!(
|
writeln!(
|
||||||
out,
|
out,
|
||||||
" {:<36} {:<8} {:<9} {:<28} {}",
|
" {:<36} {:<8} {:<9} {:<28} {:<40} {}",
|
||||||
"ID", "STATUS", "ACTIVITY", "DIR", "COMMAND"
|
"ID", "STATUS", "ACTIVITY", "DIR", "COMMAND", "BLOCKED"
|
||||||
)
|
)
|
||||||
.ok();
|
.ok();
|
||||||
for s in sorted {
|
for s in sorted {
|
||||||
@@ -115,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),
|
Some(fg) if fg != &s.command => format!("{} [{}]", s.command, fg),
|
||||||
_ => s.command.clone(),
|
_ => 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!(
|
writeln!(
|
||||||
out,
|
out,
|
||||||
" {:<36} {:<8} {:<9} {:<28} {command}",
|
" {:<36} {:<8} {:<9} {:<28} {:<40} {blocked}",
|
||||||
s.id, status, activity, dir
|
s.id, status, activity, dir, command
|
||||||
)
|
)
|
||||||
.ok();
|
.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
|
/// 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.
|
/// UI's abbreviation so CLI and browser show the same directory shorthand.
|
||||||
pub(crate) fn abbreviate_dir(path: &std::path::Path, home: &str) -> String {
|
pub(crate) fn abbreviate_dir(path: &std::path::Path, home: &str) -> String {
|
||||||
@@ -164,6 +177,7 @@ mod tests {
|
|||||||
activity: Default::default(),
|
activity: Default::default(),
|
||||||
last_activity_at: chrono::Utc::now(),
|
last_activity_at: chrono::Utc::now(),
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -224,4 +238,37 @@ mod tests {
|
|||||||
super::print_session_table(&mut buf, &[], &info(""));
|
super::print_session_table(&mut buf, &[], &info(""));
|
||||||
assert_eq!(String::from_utf8(buf).unwrap(), "no sessions\n");
|
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}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -33,6 +33,13 @@ pub const EVENTS_CAPACITY: usize = maestro_config::EVENTS_CAPACITY;
|
|||||||
/// How often the liveness watcher polls tmux for sessions that disappeared.
|
/// How often the liveness watcher polls tmux for sessions that disappeared.
|
||||||
pub const DEFAULT_LIVENESS_INTERVAL: Duration = Duration::from_secs(2);
|
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`.
|
/// Per-session daemon-local bookkeeping that complements the public `Session`.
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub struct SessionRecord {
|
pub struct SessionRecord {
|
||||||
@@ -344,6 +351,9 @@ impl SessionManager {
|
|||||||
last_activity_at: Utc::now(),
|
last_activity_at: Utc::now(),
|
||||||
|
|
||||||
current_cwd: None,
|
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) {
|
if let Err(e) = self.inner.sidecar.write(&session) {
|
||||||
@@ -441,21 +451,8 @@ impl SessionManager {
|
|||||||
),
|
),
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
// The probe runs before anything is torn down: both refusing (#79)
|
|
||||||
// and recording the retention (#81) must see the tree as it stands,
|
|
||||||
// and a refused kill leaves the session running as the operator left
|
|
||||||
// it.
|
|
||||||
let probe = match &worktree {
|
let probe = match &worktree {
|
||||||
Some((_, path)) => {
|
Some(wt) => Some(probe_worktree(wt).await?),
|
||||||
let path = path.clone();
|
|
||||||
Some(
|
|
||||||
tmux_blocking(move || crate::worktree_probe::in_the_way(&path))
|
|
||||||
.await
|
|
||||||
.map_err(|e| {
|
|
||||||
SessionError::Worktree(crate::worktree::WorktreeError::from(e))
|
|
||||||
})?,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
None => None,
|
None => None,
|
||||||
};
|
};
|
||||||
let guarded = probe.as_ref().is_some_and(|p| !p.is_clear());
|
let guarded = probe.as_ref().is_some_and(|p| !p.is_clear());
|
||||||
@@ -463,29 +460,15 @@ impl SessionManager {
|
|||||||
return Err(SessionError::KillGuarded(probe.expect("guarded ⇒ probed")));
|
return Err(SessionError::KillGuarded(probe.expect("guarded ⇒ probed")));
|
||||||
}
|
}
|
||||||
|
|
||||||
// Record the retention BEFORE tearing down: a failed store write then
|
|
||||||
// leaves the session running and every artifact untouched, instead of
|
|
||||||
// ending a session whose work outlives it unrecorded.
|
|
||||||
let mut retained = None;
|
let mut retained = None;
|
||||||
if guarded {
|
if guarded {
|
||||||
let store =
|
let entry = record_retention(
|
||||||
self.inner.retained.clone().ok_or_else(|| {
|
&self.inner,
|
||||||
SessionError::RetainFailed("no retained store is wired".into())
|
worktree.as_ref().expect("guarded ⇒ worktree"),
|
||||||
})?;
|
known_branch,
|
||||||
let (base_repo, path) = worktree.as_ref().expect("guarded ⇒ worktree");
|
RetainedReason::ForceKilled,
|
||||||
let branch = known_branch
|
)
|
||||||
.or_else(|| crate::worktree_probe::current_branch(path))
|
.await?;
|
||||||
.unwrap_or_default();
|
|
||||||
let entry = {
|
|
||||||
let store = store.clone();
|
|
||||||
let base_repo = base_repo.clone();
|
|
||||||
let path = path.clone();
|
|
||||||
tmux_blocking(move || {
|
|
||||||
store.retain(&path, &base_repo, &branch, RetainedReason::ForceKilled)
|
|
||||||
})
|
|
||||||
.await
|
|
||||||
.map_err(|e| SessionError::RetainFailed(e.to_string()))?
|
|
||||||
};
|
|
||||||
retained = Some((entry, probe.expect("guarded ⇒ probed")));
|
retained = Some((entry, probe.expect("guarded ⇒ probed")));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -516,6 +499,17 @@ impl SessionManager {
|
|||||||
/// `SessionManager` clone is dropped — call this once from the daemon
|
/// `SessionManager` clone is dropped — call this once from the daemon
|
||||||
/// startup path after [`Self::recover`] has populated the map.
|
/// startup path after [`Self::recover`] has populated the map.
|
||||||
pub fn start_liveness_watcher(&self, interval: Duration) {
|
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);
|
let weak = Arc::downgrade(&self.inner);
|
||||||
tokio::spawn(async move {
|
tokio::spawn(async move {
|
||||||
let mut ticker = tokio::time::interval(interval);
|
let mut ticker = tokio::time::interval(interval);
|
||||||
@@ -523,6 +517,9 @@ impl SessionManager {
|
|||||||
// first tick fires immediately — skip it so we don't reap before
|
// first tick fires immediately — skip it so we don't reap before
|
||||||
// the caller has a chance to create anything.
|
// the caller has a chance to create anything.
|
||||||
ticker.tick().await;
|
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 {
|
loop {
|
||||||
ticker.tick().await;
|
ticker.tick().await;
|
||||||
let Some(inner) = weak.upgrade() else {
|
let Some(inner) = weak.upgrade() else {
|
||||||
@@ -532,6 +529,11 @@ impl SessionManager {
|
|||||||
// Same tick, no new timer (NAP-340): refresh activity for the
|
// Same tick, no new timer (NAP-340): refresh activity for the
|
||||||
// survivors and emit SessionUpdated on any transition.
|
// survivors and emit SessionUpdated on any transition.
|
||||||
run_activity_pass(&inner).await;
|
run_activity_pass(&inner).await;
|
||||||
|
since_probe += interval;
|
||||||
|
if since_probe >= blocked_work_interval {
|
||||||
|
since_probe = Duration::ZERO;
|
||||||
|
run_blocked_work_pass(&inner).await;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -568,6 +570,7 @@ impl SessionManager {
|
|||||||
last_activity_at: Utc::now(),
|
last_activity_at: Utc::now(),
|
||||||
|
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
|
let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
|
||||||
@@ -671,6 +674,49 @@ where
|
|||||||
.expect("tmux blocking task panicked")
|
.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
|
/// 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.
|
/// the tmux-reported one, and reap anything we still track but tmux has lost.
|
||||||
async fn run_liveness_pass(inner: &Arc<Inner>) {
|
async fn run_liveness_pass(inner: &Arc<Inner>) {
|
||||||
@@ -793,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`]
|
/// 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
|
/// 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
|
/// still attach and read the replay buffer — the last thing a crashed agent
|
||||||
/// printed. Artifacts are only released when the operator dismisses the session
|
/// printed. The worktree, though, is settled right away: a pristine tree is
|
||||||
/// via [`SessionManager::kill`].
|
/// 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`]
|
/// Dead sessions do not survive a daemon restart: [`SessionManager::recover`]
|
||||||
/// adopts only what tmux still holds.
|
/// adopts only what tmux still holds.
|
||||||
async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
|
async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
|
||||||
let updated = {
|
let flipped = {
|
||||||
let mut sessions = inner.sessions.lock().await;
|
let mut sessions = inner.sessions.lock().await;
|
||||||
match sessions.get_mut(&id) {
|
match sessions.get_mut(&id) {
|
||||||
Some(record) if record.session.status == SessionStatus::Running => {
|
Some(record) if record.session.status == SessionStatus::Running => {
|
||||||
@@ -812,19 +919,114 @@ async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
|
|||||||
// Release anyone attached; the scrollback survives for a later
|
// Release anyone attached; the scrollback survives for a later
|
||||||
// attach.
|
// attach.
|
||||||
record.bridge.seal();
|
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,
|
_ => None,
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
let Some(session) = updated else {
|
let Some((mut session, worktree, branch)) = flipped else {
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
tracing::info!(%id, "session vanished from tmux — marking dead");
|
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.sidecar.write(&session);
|
||||||
let _ = inner.events.send(CatalogEvent::SessionUpdated { 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
|
/// Tear down a session's artifacts after it has already been removed from the
|
||||||
/// live map. Shared by [`SessionManager::kill`] and [`reap_dead_session`],
|
/// live map. Shared by [`SessionManager::kill`] and [`reap_dead_session`],
|
||||||
/// which previously hand-rolled the same five-step choreography and only
|
/// which previously hand-rolled the same five-step choreography and only
|
||||||
|
|||||||
@@ -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}"
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -8,7 +8,9 @@ use std::process::Command;
|
|||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use std::time::Duration;
|
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_daemon::SessionError;
|
||||||
use maestro_protocol::CreateSessionRequest;
|
use maestro_protocol::CreateSessionRequest;
|
||||||
use maestro_protocol::{CatalogEvent, SessionStatus, WorktreeSpec};
|
use maestro_protocol::{CatalogEvent, SessionStatus, WorktreeSpec};
|
||||||
@@ -365,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());
|
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
|
/// 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
|
/// that says "don't clean up" loses its worktree — the branch was pushed, so
|
||||||
/// nothing in the tree is unique and removal goes ahead.
|
/// nothing in the tree is unique and removal goes ahead.
|
||||||
@@ -662,6 +725,189 @@ async fn killing_an_ignored_only_worktree_succeeds() {
|
|||||||
assert!(!fx.manager.has(s.id).await);
|
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]) {
|
fn run_git_in(dir: &std::path::Path, args: &[&str]) {
|
||||||
let out = Command::new("git")
|
let out = Command::new("git")
|
||||||
.arg("-C")
|
.arg("-C")
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ fn sample(id: Uuid) -> Session {
|
|||||||
activity: Default::default(),
|
activity: Default::default(),
|
||||||
last_activity_at: chrono::Utc::now(),
|
last_activity_at: chrono::Utc::now(),
|
||||||
current_cwd: Some(PathBuf::from("/tmp/work/crates")),
|
current_cwd: Some(PathBuf::from("/tmp/work/crates")),
|
||||||
|
blocked_work: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -12,7 +12,8 @@ use std::time::Duration;
|
|||||||
|
|
||||||
use futures_util::{SinkExt, StreamExt};
|
use futures_util::{SinkExt, StreamExt};
|
||||||
use maestro_protocol::{
|
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::client::IntoClientRequest;
|
||||||
use tokio_tungstenite::tungstenite::Message as WsMessage;
|
use tokio_tungstenite::tungstenite::Message as WsMessage;
|
||||||
@@ -64,6 +65,20 @@ impl TestClient {
|
|||||||
Ok(self.daemon.kill_session(id, false).await.map(|_| ())?)
|
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> {
|
pub async fn list_projects(&self) -> Result<Vec<Project>, ClientError> {
|
||||||
Ok(self.daemon.list_projects().await?)
|
Ok(self.daemon.list_projects().await?)
|
||||||
}
|
}
|
||||||
|
|||||||
+522
-33
@@ -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`.
|
/// Spawn request carrying a worktree spec against `base`.
|
||||||
fn worktree_req(base: &std::path::Path, branch: &str, create_branch: bool) -> CreateSessionRequest {
|
fn worktree_req(base: &std::path::Path, branch: &str, create_branch: bool) -> CreateSessionRequest {
|
||||||
CreateSessionRequest {
|
CreateSessionRequest {
|
||||||
@@ -42,12 +53,7 @@ fn worktree_req(base: &std::path::Path, branch: &str, create_branch: bool) -> Cr
|
|||||||
command: "sleep 30".into(),
|
command: "sleep 30".into(),
|
||||||
env: HashMap::new(),
|
env: HashMap::new(),
|
||||||
metadata: HashMap::new(),
|
metadata: HashMap::new(),
|
||||||
worktree: Some(WorktreeSpec {
|
worktree: Some(worktree_spec(base, branch, create_branch)),
|
||||||
base_repo: base.to_path_buf(),
|
|
||||||
branch: branch.into(),
|
|
||||||
cleanup_on_kill: true,
|
|
||||||
create_branch,
|
|
||||||
}),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -141,12 +147,7 @@ async fn kill_removes_clean_worktree() {
|
|||||||
command: "sleep 30".into(),
|
command: "sleep 30".into(),
|
||||||
env: HashMap::new(),
|
env: HashMap::new(),
|
||||||
metadata: HashMap::new(),
|
metadata: HashMap::new(),
|
||||||
worktree: Some(WorktreeSpec {
|
worktree: Some(worktree_spec(base.path(), "feature/e2e", true)),
|
||||||
base_repo: base.path().to_path_buf(),
|
|
||||||
branch: "feature/e2e".into(),
|
|
||||||
cleanup_on_kill: true,
|
|
||||||
create_branch: true,
|
|
||||||
}),
|
|
||||||
};
|
};
|
||||||
|
|
||||||
let s = client.spawn_session(&req).await.expect("spawn worktree");
|
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:?}"
|
"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");
|
client.kill_session(s.id).await.expect("kill");
|
||||||
|
|
||||||
// Give the daemon a moment to remove the worktree, then verify.
|
// 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(),
|
command: "sleep 30".into(),
|
||||||
env: HashMap::new(),
|
env: HashMap::new(),
|
||||||
metadata: HashMap::new(),
|
metadata: HashMap::new(),
|
||||||
worktree: Some(WorktreeSpec {
|
worktree: Some(worktree_spec(base.path(), "feature/unsaved", true)),
|
||||||
base_repo: base.path().to_path_buf(),
|
|
||||||
branch: "feature/unsaved".into(),
|
|
||||||
cleanup_on_kill: true,
|
|
||||||
create_branch: true,
|
|
||||||
}),
|
|
||||||
};
|
};
|
||||||
|
|
||||||
let s = client.spawn_session(&req).await.expect("spawn worktree");
|
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(),
|
command: "sleep 30".into(),
|
||||||
env: HashMap::new(),
|
env: HashMap::new(),
|
||||||
metadata: HashMap::new(),
|
metadata: HashMap::new(),
|
||||||
worktree: Some(WorktreeSpec {
|
worktree: Some(worktree_spec(base.path(), "feature/existing", false)),
|
||||||
base_repo: base.path().to_path_buf(),
|
|
||||||
branch: "feature/existing".into(),
|
|
||||||
cleanup_on_kill: true,
|
|
||||||
create_branch: false,
|
|
||||||
}),
|
|
||||||
};
|
};
|
||||||
|
|
||||||
let s = client
|
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:?}"
|
"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");
|
client.kill_session(s.id).await.expect("kill");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -328,7 +319,7 @@ async fn spawn_new_branch_creates_worktree_and_runs_in_it() {
|
|||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
push_to_throwaway_origin(base.path());
|
let _origin = throwaway_origin(base.path());
|
||||||
client.kill_session(s.id).await.expect("kill");
|
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"
|
"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");
|
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"
|
"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");
|
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)
|
// GET /api/projects + /api/branches (#54/#55/#66)
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -788,10 +1250,12 @@ fn git_worktree_list(dir: &std::path::Path) -> Vec<String> {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Push every branch of `repo` to a throwaway bare remote, so its commits are
|
/// Bare remote registered as `origin` on `repo`, seeded with every current
|
||||||
/// reachable from remote-tracking refs. Without one the kill guard (#79)
|
/// branch, so existing commits are reachable from remote-tracking refs.
|
||||||
/// reads every commit as unpushed, and any worktree session refuses to die.
|
/// Without one the kill guard (#79) reads every commit as unpushed, and any
|
||||||
fn push_to_throwaway_origin(repo: &std::path::Path) {
|
/// 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();
|
let origin = tempfile::TempDir::new().unwrap();
|
||||||
run_git(
|
run_git(
|
||||||
&["init", "-q", "--bare", origin.path().to_str().unwrap()],
|
&["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()])
|
.args(["remote", "add", "origin", origin.path().to_str().unwrap()])
|
||||||
.output()
|
.output()
|
||||||
.expect("git remote add");
|
.expect("git remote add");
|
||||||
|
push_all(repo);
|
||||||
|
origin
|
||||||
|
}
|
||||||
|
|
||||||
|
fn push_all(repo: &std::path::Path) {
|
||||||
run_git(&["push", "-q", "--all", "origin"], repo);
|
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")]
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
async fn daemon_events_ws_streams_snapshot_then_session_added() {
|
async fn daemon_events_ws_streams_snapshot_then_session_added() {
|
||||||
// NAP-248: subscribe to the daemon's WS /api/events, expect an initial
|
// NAP-248: subscribe to the daemon's WS /api/events, expect an initial
|
||||||
|
|||||||
@@ -87,6 +87,13 @@ pub struct Session {
|
|||||||
/// from daemons that predate the field.
|
/// from daemons that predate the field.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub current_cwd: Option<PathBuf>,
|
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.
|
/// Default for [`Session::last_activity_at`] when an older payload omits it.
|
||||||
@@ -531,6 +538,7 @@ mod tests {
|
|||||||
.unwrap()
|
.unwrap()
|
||||||
.with_timezone(&Utc),
|
.with_timezone(&Utc),
|
||||||
current_cwd: Some(PathBuf::from("/home/naps62/projects/maestro/crates")),
|
current_cwd: Some(PathBuf::from("/home/naps62/projects/maestro/crates")),
|
||||||
|
blocked_work: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -550,6 +558,34 @@ mod tests {
|
|||||||
assert_eq!(s.activity, ActivityState::Idle);
|
assert_eq!(s.activity, ActivityState::Idle);
|
||||||
assert_eq!(s.last_activity_at, epoch());
|
assert_eq!(s.last_activity_at, epoch());
|
||||||
assert!(s.current_cwd.is_none());
|
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]
|
#[test]
|
||||||
|
|||||||
@@ -43,6 +43,7 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session {
|
|||||||
activity: Default::default(),
|
activity: Default::default(),
|
||||||
last_activity_at: Utc::now(),
|
last_activity_at: Utc::now(),
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1107,6 +1107,7 @@ mod tests {
|
|||||||
activity: Default::default(),
|
activity: Default::default(),
|
||||||
last_activity_at: Utc::now(),
|
last_activity_at: Utc::now(),
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
}],
|
}],
|
||||||
});
|
});
|
||||||
app.dispatch(Action::SidebarNext);
|
app.dispatch(Action::SidebarNext);
|
||||||
@@ -1614,6 +1615,7 @@ mod tests {
|
|||||||
last_activity_at: chrono::Utc::now(),
|
last_activity_at: chrono::Utc::now(),
|
||||||
|
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
}],
|
}],
|
||||||
});
|
});
|
||||||
app.dispatch(Action::KillHighlighted);
|
app.dispatch(Action::KillHighlighted);
|
||||||
|
|||||||
@@ -195,6 +195,7 @@ mod tests {
|
|||||||
activity: Default::default(),
|
activity: Default::default(),
|
||||||
last_activity_at: chrono::Utc::now(),
|
last_activity_at: chrono::Utc::now(),
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -14,23 +14,28 @@ use ratatui::Frame;
|
|||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
pub struct KillConfirm {
|
pub struct KillConfirm {
|
||||||
pub label: String,
|
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 {
|
impl KillConfirm {
|
||||||
pub fn render(&self, f: &mut Frame, parent: Rect) {
|
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);
|
f.render_widget(Clear, area);
|
||||||
let block = Block::default().title(" kill ").borders(Borders::ALL);
|
let block = Block::default().title(" kill ").borders(Borders::ALL);
|
||||||
let inner = block.inner(area);
|
let inner = block.inner(area);
|
||||||
f.render_widget(block, area);
|
f.render_widget(block, area);
|
||||||
f.render_widget(
|
let mut lines = vec![Line::from(format!("Kill session {}?", self.label))];
|
||||||
Paragraph::new(vec![
|
if let Some(blockers) = &self.blockers {
|
||||||
Line::from(format!("Kill session {}?", self.label)),
|
// Same phrasing the reclaim confirmation uses for its own guard.
|
||||||
Line::from(""),
|
lines.push(Line::from(format!("Currently in the way: {blockers}")));
|
||||||
Line::from("[y] yes [n] no"),
|
lines.push(Line::from("The kill will be refused unless forced."));
|
||||||
]),
|
}
|
||||||
inner,
|
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() {
|
fn kill_confirm_renders_prompt() {
|
||||||
let modal = KillConfirm {
|
let modal = KillConfirm {
|
||||||
label: "session-7".into(),
|
label: "session-7".into(),
|
||||||
|
blockers: None,
|
||||||
};
|
};
|
||||||
let rendered = render_to_string(60, 10, |f| modal.render(f, f.area()));
|
let rendered = render_to_string(60, 10, |f| modal.render(f, f.area()));
|
||||||
assert!(rendered.contains("Kill"));
|
assert!(rendered.contains("Kill"));
|
||||||
@@ -129,6 +135,22 @@ mod tests {
|
|||||||
assert!(rendered.contains("no"));
|
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]
|
#[test]
|
||||||
fn help_overlay_lists_lines() {
|
fn help_overlay_lists_lines() {
|
||||||
let help = HelpOverlay {
|
let help = HelpOverlay {
|
||||||
|
|||||||
@@ -146,8 +146,19 @@ pub fn draw(f: &mut Frame, inputs: RenderInputs<'_>) -> RegionMap {
|
|||||||
|
|
||||||
match inputs.state.overlay() {
|
match inputs.state.overlay() {
|
||||||
Some(OverlayKind::KillConfirm(id)) => {
|
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 {
|
let kc = KillConfirm {
|
||||||
label: id.to_string(),
|
label: id.to_string(),
|
||||||
|
blockers,
|
||||||
};
|
};
|
||||||
kc.render(f, area);
|
kc.render(f, area);
|
||||||
map.modal = Some(area);
|
map.modal = Some(area);
|
||||||
|
|||||||
@@ -52,6 +52,13 @@ impl Sidebar {
|
|||||||
(SessionStatus::Running, false) => "○",
|
(SessionStatus::Running, false) => "○",
|
||||||
};
|
};
|
||||||
let label = name_for(s);
|
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) {
|
let style = if self.highlight == Some(s.id) {
|
||||||
Style::default().add_modifier(Modifier::REVERSED)
|
Style::default().add_modifier(Modifier::REVERSED)
|
||||||
} else {
|
} else {
|
||||||
@@ -59,7 +66,7 @@ impl Sidebar {
|
|||||||
};
|
};
|
||||||
lines.push(Line::from(vec![
|
lines.push(Line::from(vec![
|
||||||
Span::raw(" "),
|
Span::raw(" "),
|
||||||
Span::styled(format!("{glyph} {label}"), style),
|
Span::styled(format!("{glyph} {label}{blocked}"), style),
|
||||||
]));
|
]));
|
||||||
}
|
}
|
||||||
f.render_widget(Paragraph::new(lines), inner);
|
f.render_widget(Paragraph::new(lines), inner);
|
||||||
@@ -99,6 +106,7 @@ mod tests {
|
|||||||
activity: Default::default(),
|
activity: Default::default(),
|
||||||
last_activity_at: chrono::Utc::now(),
|
last_activity_at: chrono::Utc::now(),
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -172,6 +180,40 @@ mod tests {
|
|||||||
assert!(line.contains('✗'), "expected dead glyph in: {line:?}");
|
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]
|
#[test]
|
||||||
fn falls_back_to_command_when_name_metadata_is_absent() {
|
fn falls_back_to_command_when_name_metadata_is_absent() {
|
||||||
let s = Session {
|
let s = Session {
|
||||||
@@ -188,6 +230,7 @@ mod tests {
|
|||||||
last_activity_at: chrono::Utc::now(),
|
last_activity_at: chrono::Utc::now(),
|
||||||
|
|
||||||
current_cwd: None,
|
current_cwd: None,
|
||||||
|
blocked_work: None,
|
||||||
};
|
};
|
||||||
let sb = Sidebar {
|
let sb = Sidebar {
|
||||||
sessions: vec![s],
|
sessions: vec![s],
|
||||||
|
|||||||
@@ -97,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
|
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 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
|
## The retained store
|
||||||
|
|
||||||
A `state_dir`-backed record of worktrees the daemon created that no longer have a
|
A `state_dir`-backed record of worktrees the daemon created that no longer have a
|
||||||
|
|||||||
@@ -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
@@ -3,7 +3,7 @@ import { getDaemonInfo, killSession } from "./api.js";
|
|||||||
import { HUD_TOGGLE_EVENT, isHudEnabled, toggleHud } from "./debug.js";
|
import { HUD_TOGGLE_EVENT, isHudEnabled, toggleHud } from "./debug.js";
|
||||||
import { CatalogStore } from "./events.js";
|
import { CatalogStore } from "./events.js";
|
||||||
import type { Session } from "./types.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 type { TreeNode } from "./tree.js";
|
||||||
import { buildTree } from "./tree.js";
|
import { buildTree } from "./tree.js";
|
||||||
import { createThemeToggle } from "./theme.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
|
// The directory is the enclosing folder, so only surface the command — and
|
||||||
// only when it isn't already what the name shows.
|
// only when it isn't already what the name shows.
|
||||||
const showCmd = s.command !== name;
|
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 = `
|
row.innerHTML = `
|
||||||
<span class="leaf-name">${escapeHtml(name)}</span>
|
<span class="leaf-name">${escapeHtml(name)}</span>
|
||||||
${showCmd ? `<span class="leaf-meta"><span class="leaf-cmd">${escapeHtml(s.command)}</span></span>` : ""}
|
${showCmd ? `<span class="leaf-meta"><span class="leaf-cmd">${escapeHtml(s.command)}</span></span>` : ""}
|
||||||
${running ? "" : `<span class="leaf-badge">exited</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">
|
<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-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>
|
<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) => {
|
killBtn.addEventListener("click", async (ev) => {
|
||||||
ev.preventDefault();
|
ev.preventDefault();
|
||||||
ev.stopPropagation();
|
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 {
|
try {
|
||||||
await killSession(s.id);
|
await killSession(s.id);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
|||||||
@@ -831,6 +831,13 @@ textarea {
|
|||||||
flex: 0 0 auto;
|
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 {
|
.leaf-actions {
|
||||||
position: absolute;
|
position: absolute;
|
||||||
right: var(--sp-1);
|
right: var(--sp-1);
|
||||||
|
|||||||
@@ -31,6 +31,11 @@ export interface Session {
|
|||||||
* observed it (`#{pane_current_path}`) — distinct from `cwd` (the spawn dir).
|
* observed it (`#{pane_current_path}`) — distinct from `cwd` (the spawn dir).
|
||||||
* Drives the directory-based session tree. Absent until first observed. */
|
* Drives the directory-based session tree. Absent until first observed. */
|
||||||
current_cwd?: string | null;
|
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). */
|
/** One directory's immediate subdirectories (NAP-329). */
|
||||||
@@ -180,6 +185,19 @@ export function reasonLabel(reason: RetainedReason): string {
|
|||||||
return reason;
|
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 {
|
export interface CreateSessionRequest {
|
||||||
cwd: string;
|
cwd: string;
|
||||||
command: string;
|
command: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user