Compare commits
12
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2083911baa | ||
|
|
b5450651d1 | ||
|
|
dde8dd8cfc | ||
|
|
2fc19a886e | ||
|
|
0b95d11aa6 | ||
|
|
a85084d0d9 | ||
|
|
5282ba3411 | ||
|
|
2d796a3556 | ||
|
|
f67803acf5 | ||
|
|
0c135aa065 | ||
|
|
b96f18c7cb | ||
|
|
bb2f439302 |
@@ -125,6 +125,11 @@ pub struct AttachArgs {
|
||||
pub struct KillArgs {
|
||||
/// Session ID to kill.
|
||||
pub session_id: Uuid,
|
||||
/// End the session even when its worktree holds unsaved work (#81). The
|
||||
/// worktree is retained — never removed — and enters the retained store;
|
||||
/// the output says where the work went.
|
||||
#[arg(long)]
|
||||
pub force: bool,
|
||||
}
|
||||
|
||||
fn parse_kv(s: &str) -> Result<(String, String), String> {
|
||||
@@ -256,6 +261,32 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_kill_with_force() {
|
||||
let id = "01234567-89ab-cdef-0123-456789abcdef";
|
||||
let cli = Cli::try_parse_from(["maestro", "kill", "--force", id]).unwrap();
|
||||
match cli.command {
|
||||
Command::Kill(a) => {
|
||||
assert!(a.force);
|
||||
assert_eq!(a.session_id.to_string(), id);
|
||||
}
|
||||
_ => panic!("wrong subcommand"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kill_force_defaults_false() {
|
||||
let id = "01234567-89ab-cdef-0123-456789abcdef";
|
||||
let cli = Cli::try_parse_from(["maestro", "kill", id]).unwrap();
|
||||
match cli.command {
|
||||
Command::Kill(a) => {
|
||||
assert!(!a.force);
|
||||
assert_eq!(a.session_id.to_string(), id);
|
||||
}
|
||||
_ => panic!("wrong subcommand"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_env_without_equals() {
|
||||
let err = Cli::try_parse_from([
|
||||
|
||||
@@ -62,8 +62,15 @@ pub async fn dispatch(
|
||||
run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?;
|
||||
}
|
||||
Command::Kill(args) => {
|
||||
run_kill(&daemon, &args).await?;
|
||||
let outcome = run_kill(&daemon, &args).await?;
|
||||
println!("killed {}", args.session_id);
|
||||
if let Some(w) = outcome.retained {
|
||||
println!(
|
||||
"worktree retained at {} (branch {}, force-killed)",
|
||||
w.path.display(),
|
||||
w.branch
|
||||
);
|
||||
}
|
||||
}
|
||||
Command::Worktrees(args) => match args.command {
|
||||
WorktreesCommand::List(list_args) => {
|
||||
|
||||
@@ -59,9 +59,14 @@ pub async fn run_spawn(client: &DaemonClient, args: SpawnArgs) -> Result<Session
|
||||
client.create_session(&body).await
|
||||
}
|
||||
|
||||
/// `maestro kill`
|
||||
pub async fn run_kill(client: &DaemonClient, args: &KillArgs) -> Result<(), ClientError> {
|
||||
client.kill_session(args.session_id).await
|
||||
/// `maestro kill`. A forced kill (#81) retains the guarded worktree instead
|
||||
/// of removing it; the outcome names what was kept so dispatch can say where
|
||||
/// the work went.
|
||||
pub async fn run_kill(
|
||||
client: &DaemonClient,
|
||||
args: &KillArgs,
|
||||
) -> Result<maestro_protocol::KillOutcome, ClientError> {
|
||||
client.kill_session(args.session_id, args.force).await
|
||||
}
|
||||
|
||||
#[allow(clippy::write_literal)]
|
||||
|
||||
@@ -76,7 +76,7 @@ async fn killing_an_unknown_session_reports_the_daemon_reason() {
|
||||
|
||||
let err = mock
|
||||
.client()
|
||||
.kill_session(Uuid::new_v4())
|
||||
.kill_session(Uuid::new_v4(), false)
|
||||
.await
|
||||
.expect_err("should fail");
|
||||
|
||||
@@ -102,6 +102,9 @@ async fn spawn_round_trips_through_the_daemon() {
|
||||
assert_eq!(created.command, "bash");
|
||||
assert_eq!(mock.session_count(), 1);
|
||||
|
||||
mock.client().kill_session(created.id).await.expect("kill");
|
||||
mock.client()
|
||||
.kill_session(created.id, false)
|
||||
.await
|
||||
.expect("kill");
|
||||
assert_eq!(mock.session_count(), 0);
|
||||
}
|
||||
|
||||
@@ -56,6 +56,9 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
|
||||
Some(p) => Tmux::new().with_tmpdir(p),
|
||||
None => Tmux::new(),
|
||||
};
|
||||
let retained = std::sync::Arc::new(crate::retained_store::RetainedStore::new(
|
||||
config.retained_worktree_dir(),
|
||||
));
|
||||
let manager = SessionManager::new_with_replay(
|
||||
std::sync::Arc::new(tmux),
|
||||
sidecar,
|
||||
@@ -66,7 +69,10 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
|
||||
.with_spawn_history(crate::spawn_history::SpawnHistory::new(
|
||||
config.spawn_history_path(),
|
||||
config.spawn_history_days,
|
||||
));
|
||||
))
|
||||
// One store instance shared with the API handlers: forced kills (#81)
|
||||
// record here, `GET /api/worktrees` (#80) reads back from here.
|
||||
.with_retained_store((*retained).clone());
|
||||
|
||||
let recovered = manager.recover().await?;
|
||||
tracing::info!(recovered, "adopted existing tmux sessions");
|
||||
@@ -85,9 +91,7 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
|
||||
config.home_dir.clone(),
|
||||
config.spawn_presets.clone(),
|
||||
projects,
|
||||
std::sync::Arc::new(crate::retained_store::RetainedStore::new(
|
||||
config.retained_worktree_dir(),
|
||||
)),
|
||||
retained,
|
||||
));
|
||||
|
||||
let listener = TcpListener::bind(config.bind_addr).await?;
|
||||
|
||||
@@ -101,6 +101,8 @@ fn session_error_status(code: &str) -> StatusCode {
|
||||
"invalid_env" => StatusCode::BAD_REQUEST,
|
||||
// A guard refusal is the daemon doing its job, not a failure (#79).
|
||||
"kill_guarded" => StatusCode::CONFLICT,
|
||||
// The retention record failed before anything was torn down (#81).
|
||||
"retain_failed" => StatusCode::INTERNAL_SERVER_ERROR,
|
||||
_ => StatusCode::BAD_GATEWAY,
|
||||
}
|
||||
}
|
||||
@@ -179,9 +181,29 @@ async fn create_session(
|
||||
/// The full error info is passed through rather than rebuilt from
|
||||
/// code + message: a `kill_guarded` refusal (#79) carries the unsaved-work
|
||||
/// breakdown clients render as "3 uncommitted files, 2 unpushed commits".
|
||||
async fn kill_session(State(s): State<LocalApiState>, Path(id): Path<Uuid>) -> Response {
|
||||
match s.manager.kill(id).await {
|
||||
Ok(()) => StatusCode::NO_CONTENT.into_response(),
|
||||
///
|
||||
/// With `force=true` (#81) the refusal is overridden and the worktree
|
||||
/// retained; the answer is `200` with a [`KillOutcome`] naming what was kept.
|
||||
/// Without it the historical `204` stands.
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct KillQuery {
|
||||
#[serde(default)]
|
||||
force: bool,
|
||||
}
|
||||
|
||||
async fn kill_session(
|
||||
State(s): State<LocalApiState>,
|
||||
Path(id): Path<Uuid>,
|
||||
Query(q): Query<KillQuery>,
|
||||
) -> Response {
|
||||
match s.manager.kill(id, q.force).await {
|
||||
Ok(outcome) => {
|
||||
if q.force {
|
||||
(StatusCode::OK, Json(outcome)).into_response()
|
||||
} else {
|
||||
StatusCode::NO_CONTENT.into_response()
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
let info = e.to_error_info();
|
||||
let status = session_error_status(&info.code);
|
||||
|
||||
@@ -12,8 +12,8 @@ use std::time::Duration;
|
||||
|
||||
use chrono::Utc;
|
||||
use maestro_protocol::{
|
||||
ActivityState, CatalogEvent, CreateSessionRequest, ErrorInfo, Session, SessionStatus,
|
||||
WorktreeSpec,
|
||||
ActivityState, CatalogEvent, CreateSessionRequest, ErrorInfo, KillOutcome, RetainedReason,
|
||||
RetainedWorktree, Session, SessionStatus, WorktreeSpec,
|
||||
};
|
||||
use tokio::sync::{broadcast, Mutex};
|
||||
use uuid::Uuid;
|
||||
@@ -44,6 +44,11 @@ pub struct SessionRecord {
|
||||
/// `base_repo` from the original `WorktreeSpec`. Needed because `git
|
||||
/// worktree remove` is invoked from the base repo.
|
||||
pub worktree_base_repo: Option<PathBuf>,
|
||||
/// Branch the worktree was created for, recorded at spawn time so a
|
||||
/// forced kill (#81) can name it in the retained store without asking
|
||||
/// git. Sessions adopted from a sidecar have no branch on record and
|
||||
/// fall back to a probe.
|
||||
pub worktree_branch: Option<String>,
|
||||
pub bridge: PtyBridge,
|
||||
}
|
||||
|
||||
@@ -66,6 +71,11 @@ pub enum SessionError {
|
||||
/// so it stays the operator's handle on their own unfinished work.
|
||||
#[error("kill refused: the session's worktree holds unsaved work ({0})")]
|
||||
KillGuarded(maestro_protocol::InTheWay),
|
||||
/// A forced kill (#81) records the retained worktree *before* tearing
|
||||
/// anything down; if that write fails nothing was destroyed, and the
|
||||
/// error says so rather than silently dropping unrecorded work.
|
||||
#[error("could not record the retained worktree: {0}")]
|
||||
RetainFailed(String),
|
||||
}
|
||||
|
||||
impl SessionError {
|
||||
@@ -78,6 +88,7 @@ impl SessionError {
|
||||
SessionError::NotFound(_) => "not_found",
|
||||
SessionError::InvalidEnv(_) => "invalid_env",
|
||||
SessionError::KillGuarded(_) => "kill_guarded",
|
||||
SessionError::RetainFailed(_) => "retain_failed",
|
||||
};
|
||||
let unsaved_work = match self {
|
||||
SessionError::KillGuarded(b) => Some(*b),
|
||||
@@ -110,6 +121,9 @@ struct Inner {
|
||||
/// `None` when not wired (tests, or before `with_spawn_history`); a present
|
||||
/// but disabled (`0`-day) history no-ops internally.
|
||||
spawn_history: Option<SpawnHistory>,
|
||||
/// Retained-worktree store (#78). `None` when not wired (tests); a forced
|
||||
/// kill (#81) without one fails rather than destroying unrecorded work.
|
||||
retained: Option<crate::retained_store::RetainedStore>,
|
||||
sessions: Mutex<HashMap<Uuid, SessionRecord>>,
|
||||
events: broadcast::Sender<CatalogEvent>,
|
||||
}
|
||||
@@ -148,6 +162,7 @@ impl SessionManager {
|
||||
fifo_dir,
|
||||
replay_bytes,
|
||||
spawn_history: None,
|
||||
retained: None,
|
||||
sessions: Mutex::new(HashMap::new()),
|
||||
events,
|
||||
}),
|
||||
@@ -170,6 +185,16 @@ impl SessionManager {
|
||||
self
|
||||
}
|
||||
|
||||
/// Attach the retained-worktree store (#78). A forced kill (#81) needs it
|
||||
/// to record what it retained; without one, forcing a guarded kill fails
|
||||
/// instead of destroying unrecorded work.
|
||||
pub fn with_retained_store(mut self, store: crate::retained_store::RetainedStore) -> Self {
|
||||
let inner = Arc::get_mut(&mut self.inner)
|
||||
.expect("with_retained_store must be called before the SessionManager is cloned");
|
||||
inner.retained = Some(store);
|
||||
self
|
||||
}
|
||||
|
||||
/// Recent working dirs for the spawn form's picker (#24), newest-first and
|
||||
/// deduped by dir. Empty when no history is wired or it's disabled.
|
||||
pub fn recent_dirs(&self) -> Vec<RecentDir> {
|
||||
@@ -340,6 +365,7 @@ impl SessionManager {
|
||||
fifo_path,
|
||||
worktree_path,
|
||||
worktree_base_repo,
|
||||
worktree_branch: req.worktree.as_ref().map(|w| w.branch.clone()),
|
||||
bridge,
|
||||
},
|
||||
);
|
||||
@@ -391,11 +417,17 @@ impl SessionManager {
|
||||
/// work, so ending a session that still matters becomes a deliberate
|
||||
/// `--force`, which retains rather than destroys (WORKTREES.md).
|
||||
///
|
||||
/// With `force` (#81) the refusal — and nothing else — is overridden:
|
||||
/// the session ends and the guarded worktree is retained into the store
|
||||
/// with reason `force-killed`, never removed. The outcome names what was
|
||||
/// kept so a client can say where the work went. A tree whose guard is
|
||||
/// clear never needed the override; force removes it as usual.
|
||||
///
|
||||
/// This is also how an already-dead session is dismissed. tmux is only
|
||||
/// asked to kill something it still has — calling `kill-session` on a
|
||||
/// session tmux already lost fails, and that failure is not interesting.
|
||||
pub async fn kill(&self, id: Uuid) -> Result<(), SessionError> {
|
||||
let (worktree, alive) = {
|
||||
pub async fn kill(&self, id: Uuid, force: bool) -> Result<KillOutcome, SessionError> {
|
||||
let (worktree, alive, known_branch) = {
|
||||
let sessions = self.inner.sessions.lock().await;
|
||||
match sessions.get(&id) {
|
||||
None => return Err(SessionError::NotFound(id)),
|
||||
@@ -405,26 +437,48 @@ impl SessionManager {
|
||||
.clone()
|
||||
.zip(record.worktree_path.clone()),
|
||||
record.session.status == SessionStatus::Running,
|
||||
record.worktree_branch.clone(),
|
||||
),
|
||||
}
|
||||
};
|
||||
// The guard runs before anything is torn down: refusing must leave the
|
||||
// session running, exactly as the operator left it.
|
||||
if let Some((_, path)) = worktree {
|
||||
let probe = tmux_blocking(move || crate::worktree_probe::in_the_way(&path))
|
||||
.await
|
||||
.map_err(|e| SessionError::Worktree(crate::worktree::WorktreeError::from(e)))?;
|
||||
if !probe.is_clear() {
|
||||
return Err(SessionError::KillGuarded(probe));
|
||||
}
|
||||
let probe = match &worktree {
|
||||
Some(wt) => Some(probe_worktree(wt).await?),
|
||||
None => None,
|
||||
};
|
||||
let guarded = probe.as_ref().is_some_and(|p| !p.is_clear());
|
||||
if guarded && !force {
|
||||
return Err(SessionError::KillGuarded(probe.expect("guarded ⇒ probed")));
|
||||
}
|
||||
|
||||
let mut retained = None;
|
||||
if guarded {
|
||||
let entry = record_retention(
|
||||
&self.inner,
|
||||
worktree.as_ref().expect("guarded ⇒ worktree"),
|
||||
known_branch,
|
||||
RetainedReason::ForceKilled,
|
||||
)
|
||||
.await?;
|
||||
retained = Some((entry, probe.expect("guarded ⇒ probed")));
|
||||
}
|
||||
|
||||
let removed = {
|
||||
let mut sessions = self.inner.sessions.lock().await;
|
||||
sessions.remove(&id)
|
||||
};
|
||||
let record = removed.ok_or(SessionError::NotFound(id))?;
|
||||
cleanup_record(&self.inner, id, record, alive).await;
|
||||
Ok(())
|
||||
cleanup_record(&self.inner, id, record, alive, retained.is_some()).await;
|
||||
Ok(KillOutcome {
|
||||
retained: retained.map(|(entry, in_the_way)| RetainedWorktree {
|
||||
id: entry.id,
|
||||
path: entry.path,
|
||||
base_repo: entry.base_repo,
|
||||
branch: entry.branch,
|
||||
reason: entry.reason,
|
||||
retained_at: entry.retained_at,
|
||||
in_the_way,
|
||||
}),
|
||||
})
|
||||
}
|
||||
|
||||
/// Spawn a background task that polls tmux every `interval` and reaps any
|
||||
@@ -511,6 +565,9 @@ impl SessionManager {
|
||||
// directory blocks that branch forever (#54 review).
|
||||
worktree_path: session.worktree.as_ref().map(|w| w.path.clone()),
|
||||
worktree_base_repo: session.worktree.as_ref().map(|w| w.base_repo.clone()),
|
||||
// The sidecar carries no branch; a forced kill of a
|
||||
// recovered session probes git for it at retain time.
|
||||
worktree_branch: None,
|
||||
bridge,
|
||||
},
|
||||
);
|
||||
@@ -587,6 +644,49 @@ where
|
||||
.expect("tmux blocking task panicked")
|
||||
}
|
||||
|
||||
/// What would be lost if this session's worktree were removed right now.
|
||||
///
|
||||
/// The single [`crate::worktree_probe::in_the_way`] call site behind both
|
||||
/// endings of a session — the kill guard (#79) and the natural-exit
|
||||
/// classifier (#82) — so neither can grow its own answer. Runs on the
|
||||
/// blocking pool; callers invoke it before anything is torn down, because
|
||||
/// refusing (#79), recording a retention (#81) and classifying an exit (#82)
|
||||
/// must all see the tree as it stands.
|
||||
async fn probe_worktree(
|
||||
(_, path): &(PathBuf, PathBuf),
|
||||
) -> Result<maestro_protocol::InTheWay, SessionError> {
|
||||
let path = path.clone();
|
||||
tmux_blocking(move || crate::worktree_probe::in_the_way(&path))
|
||||
.await
|
||||
.map_err(|e| SessionError::Worktree(crate::worktree::WorktreeError::from(e)))
|
||||
}
|
||||
|
||||
/// Record a guarded worktree into the retained store with `reason`, BEFORE
|
||||
/// the caller tears anything down: a failed write then leaves every artifact
|
||||
/// untouched instead of destroying work nothing records anymore. Shared by
|
||||
/// the forced kill (`ForceKilled`, #81) and the natural exit
|
||||
/// (`ExitedWithUnsavedWork`, #82) so the two paths cannot drift apart.
|
||||
async fn record_retention(
|
||||
inner: &Arc<Inner>,
|
||||
(base_repo, path): &(PathBuf, PathBuf),
|
||||
known_branch: Option<String>,
|
||||
reason: RetainedReason,
|
||||
) -> Result<crate::retained_store::RetainedWorktreeRecord, SessionError> {
|
||||
let store = inner
|
||||
.retained
|
||||
.clone()
|
||||
.ok_or_else(|| SessionError::RetainFailed("no retained store is wired".into()))?;
|
||||
// Sessions adopted from a sidecar carry no branch; ask the tree itself.
|
||||
let branch = known_branch
|
||||
.or_else(|| crate::worktree_probe::current_branch(path))
|
||||
.unwrap_or_default();
|
||||
let base_repo = base_repo.clone();
|
||||
let path = path.clone();
|
||||
tmux_blocking(move || store.retain(&path, &base_repo, &branch, reason))
|
||||
.await
|
||||
.map_err(|e| SessionError::RetainFailed(e.to_string()))
|
||||
}
|
||||
|
||||
/// One pass of the liveness watcher: diff the in-memory session set against
|
||||
/// the tmux-reported one, and reap anything we still track but tmux has lost.
|
||||
async fn run_liveness_pass(inner: &Arc<Inner>) {
|
||||
@@ -710,17 +810,23 @@ async fn run_activity_pass(inner: &Arc<Inner>) {
|
||||
}
|
||||
|
||||
/// Mark a session whose tmux session has disappeared as [`SessionStatus::Dead`]
|
||||
/// and announce it, keeping everything else intact.
|
||||
/// and settle its worktree per the natural-exit rule (#82).
|
||||
///
|
||||
/// The record, its [`PtyBridge`] and its sidecar all survive, so a client can
|
||||
/// still attach and read the replay buffer — the last thing a crashed agent
|
||||
/// printed. Artifacts are only released when the operator dismisses the session
|
||||
/// via [`SessionManager::kill`].
|
||||
/// printed. The worktree, though, is settled right away: a pristine tree is
|
||||
/// removed, anything else is retained into the store with reason
|
||||
/// `exited-with-unsaved-work` (#82) — nobody is waiting for an answer here,
|
||||
/// so there is nothing to refuse. Once settled, the session drops its
|
||||
/// worktree reference so a later dismissal neither re-probes nor retains
|
||||
/// twice. When settling is impossible (probe or store failure) nothing is
|
||||
/// destroyed: the tree stays attached to the dead record for a later forced
|
||||
/// dismissal (#81).
|
||||
///
|
||||
/// Dead sessions do not survive a daemon restart: [`SessionManager::recover`]
|
||||
/// adopts only what tmux still holds.
|
||||
async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
|
||||
let updated = {
|
||||
let flipped = {
|
||||
let mut sessions = inner.sessions.lock().await;
|
||||
match sessions.get_mut(&id) {
|
||||
Some(record) if record.session.status == SessionStatus::Running => {
|
||||
@@ -728,19 +834,111 @@ async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
|
||||
// Release anyone attached; the scrollback survives for a later
|
||||
// attach.
|
||||
record.bridge.seal();
|
||||
Some(record.session.clone())
|
||||
let session = record.session.clone();
|
||||
let worktree = record
|
||||
.worktree_base_repo
|
||||
.clone()
|
||||
.zip(record.worktree_path.clone());
|
||||
let branch = record.worktree_branch.clone();
|
||||
Some((session, worktree, branch))
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
};
|
||||
let Some(session) = updated else {
|
||||
let Some((mut session, worktree, branch)) = flipped else {
|
||||
return;
|
||||
};
|
||||
tracing::info!(%id, "session vanished from tmux — marking dead");
|
||||
|
||||
if let Some(wt) = &worktree {
|
||||
if settle_worktree_on_exit(inner, id, wt, branch).await {
|
||||
session.worktree = None;
|
||||
let mut sessions = inner.sessions.lock().await;
|
||||
if let Some(record) = sessions.get_mut(&id) {
|
||||
record.session.worktree = None;
|
||||
record.worktree_path = None;
|
||||
record.worktree_base_repo = None;
|
||||
record.worktree_branch = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let _ = inner.sidecar.write(&session);
|
||||
let _ = inner.events.send(CatalogEvent::SessionUpdated { session });
|
||||
}
|
||||
|
||||
/// Apply the exit rule (#82) to a vanished session's worktree: probe what
|
||||
/// would be lost through the kill's own predicate, then remove a pristine
|
||||
/// tree or retain anything else under `exited-with-unsaved-work` — recorded
|
||||
/// before anything is torn down, exactly as the forced kill does (#81). A
|
||||
/// removal that fails falls back to retaining, since whatever blocked it is
|
||||
/// by definition something worth keeping.
|
||||
///
|
||||
/// Returns whether the worktree was fully accounted for. When it wasn't,
|
||||
/// nothing was destroyed and the caller keeps the reference on the record.
|
||||
async fn settle_worktree_on_exit(
|
||||
inner: &Arc<Inner>,
|
||||
id: Uuid,
|
||||
(base_repo, path): &(PathBuf, PathBuf),
|
||||
known_branch: Option<String>,
|
||||
) -> bool {
|
||||
let probe = match probe_worktree(&(base_repo.clone(), path.clone())).await {
|
||||
Ok(probe) => probe,
|
||||
Err(e) => {
|
||||
tracing::error!(
|
||||
%id,
|
||||
path = %path.display(),
|
||||
error = ?e,
|
||||
"natural exit: could not probe the worktree; leaving it untouched"
|
||||
);
|
||||
return false;
|
||||
}
|
||||
};
|
||||
if probe.is_clear() {
|
||||
match inner.worktree_mgr.remove(base_repo, path) {
|
||||
Ok(()) => {
|
||||
tracing::info!(%id, path = %path.display(), "natural exit: pristine worktree removed");
|
||||
return true;
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
%id,
|
||||
path = %path.display(),
|
||||
error = ?e,
|
||||
"natural exit: removal failed; falling back to retention"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
match record_retention(
|
||||
inner,
|
||||
&(base_repo.clone(), path.clone()),
|
||||
known_branch,
|
||||
RetainedReason::ExitedWithUnsavedWork,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(entry) => {
|
||||
tracing::info!(
|
||||
%id,
|
||||
path = %path.display(),
|
||||
entry = %entry.id,
|
||||
"natural exit: worktree retained with unsaved work"
|
||||
);
|
||||
true
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::error!(
|
||||
%id,
|
||||
path = %path.display(),
|
||||
error = ?e,
|
||||
"natural exit: could not record the retained worktree; leaving it untouched"
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Tear down a session's artifacts after it has already been removed from the
|
||||
/// live map. Shared by [`SessionManager::kill`] and [`reap_dead_session`],
|
||||
/// which previously hand-rolled the same five-step choreography and only
|
||||
@@ -750,7 +948,17 @@ async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
|
||||
/// gone from the authoritative in-memory map, so a stray FIFO or sidecar can't
|
||||
/// resurrect it. `kill_tmux` is `true` for an explicit kill and `false` for the
|
||||
/// liveness reaper (tmux already lost the session).
|
||||
async fn cleanup_record(inner: &Arc<Inner>, id: Uuid, record: SessionRecord, kill_tmux: bool) {
|
||||
///
|
||||
/// `retain_worktree` is `true` for a forced kill (#81) that retained the
|
||||
/// worktree instead of removing it: the store entry is already written, so
|
||||
/// only the removal is skipped.
|
||||
async fn cleanup_record(
|
||||
inner: &Arc<Inner>,
|
||||
id: Uuid,
|
||||
record: SessionRecord,
|
||||
kill_tmux: bool,
|
||||
retain_worktree: bool,
|
||||
) {
|
||||
if kill_tmux {
|
||||
let tmux = inner.tmux.clone();
|
||||
let _ = tmux_blocking(move || tmux.kill_session(id)).await;
|
||||
@@ -759,11 +967,18 @@ async fn cleanup_record(inner: &Arc<Inner>, id: Uuid, record: SessionRecord, kil
|
||||
if let Err(e) = inner.sidecar.delete(id) {
|
||||
tracing::warn!(error = ?e, %id, "cleanup: failed to delete sidecar");
|
||||
}
|
||||
// The kill already passed the unsaved-work guard (#79), so this removal
|
||||
// is unconditional; the manager's own nested-worktree refusal is all that
|
||||
// can still leave a tree on disk, and it says where.
|
||||
if let (Some(base), Some(path)) = (&record.worktree_base_repo, &record.worktree_path) {
|
||||
if let Err(e) = inner.worktree_mgr.remove(base, path) {
|
||||
if retain_worktree {
|
||||
tracing::info!(
|
||||
%id,
|
||||
path = %path.display(),
|
||||
"forced kill: worktree retained in the store"
|
||||
);
|
||||
} else if let Err(e) = inner.worktree_mgr.remove(base, path) {
|
||||
// The kill already passed the unsaved-work guard (#79), so this
|
||||
// removal is unconditional; the manager's own nested-worktree
|
||||
// refusal is all that can still leave a tree on disk, and it
|
||||
// says where.
|
||||
tracing::warn!(
|
||||
%id,
|
||||
path = %path.display(),
|
||||
|
||||
@@ -90,6 +90,14 @@ pub fn reclaim(base_repo: &Path, path: &Path) -> Result<(), ProbeError> {
|
||||
.map(|_| ())
|
||||
}
|
||||
|
||||
/// Branch checked out at `path`. Sessions adopted from a sidecar carry no
|
||||
/// branch on record, so a forced kill (#81) asks the tree itself when it
|
||||
/// needs one for the retained store. `None` when git cannot answer.
|
||||
pub fn current_branch(path: &Path) -> Option<String> {
|
||||
let out = run_git(path, ["rev-parse", "--abbrev-ref", "HEAD"]).ok()?;
|
||||
Some(String::from_utf8_lossy(&out.stdout).trim().to_string())
|
||||
}
|
||||
|
||||
fn run_git<I, S>(dir: &Path, args: I) -> Result<std::process::Output, ProbeError>
|
||||
where
|
||||
I: IntoIterator<Item = S>,
|
||||
|
||||
@@ -122,7 +122,7 @@ async fn pty_round_trip_through_real_tmux() {
|
||||
"marker not seen in {} bytes",
|
||||
got.len()
|
||||
);
|
||||
let _ = h.manager.kill(id).await;
|
||||
let _ = h.manager.kill(id, false).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -170,7 +170,7 @@ async fn multi_attach_broadcasts_to_every_client() {
|
||||
"client B missed marker"
|
||||
);
|
||||
|
||||
let _ = h.manager.kill(id).await;
|
||||
let _ = h.manager.kill(id, false).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -219,7 +219,7 @@ async fn sends_close_frame_when_session_is_killed() {
|
||||
// give the broadcast subscriber a moment to register before killing.
|
||||
tokio::time::sleep(Duration::from_millis(100)).await;
|
||||
|
||||
h.manager.kill(id).await.unwrap();
|
||||
h.manager.kill(id, false).await.unwrap();
|
||||
|
||||
let mut saw_close = false;
|
||||
let deadline = std::time::Instant::now() + Duration::from_secs(2);
|
||||
@@ -267,7 +267,7 @@ async fn ping_text_frame_gets_pong() {
|
||||
}
|
||||
}
|
||||
assert!(saw_pong, "no Pong frame received");
|
||||
let _ = h.manager.kill(id).await;
|
||||
let _ = h.manager.kill(id, false).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -176,6 +176,11 @@ impl DaemonFixture {
|
||||
manager =
|
||||
manager.with_spawn_history(SpawnHistory::new(Self::history_path(&state_dir), days));
|
||||
}
|
||||
// Same wiring as production: the manager records retained worktrees
|
||||
// into the same dir the API reads back from (#78/#81).
|
||||
let retained =
|
||||
maestro_daemon::retained_store::RetainedStore::new(Self::retained_path(&state_dir));
|
||||
manager = manager.with_retained_store(retained.clone());
|
||||
Self {
|
||||
tmux,
|
||||
manager,
|
||||
@@ -187,6 +192,10 @@ impl DaemonFixture {
|
||||
state_dir.path().join("spawn-history.jsonl")
|
||||
}
|
||||
|
||||
fn retained_path(state_dir: &TempDir) -> PathBuf {
|
||||
state_dir.path().join("retained-worktrees")
|
||||
}
|
||||
|
||||
pub fn spawn_history_path(&self) -> PathBuf {
|
||||
Self::history_path(&self.state_dir)
|
||||
}
|
||||
|
||||
@@ -25,6 +25,13 @@ struct Harness {
|
||||
_fx: DaemonFixture,
|
||||
}
|
||||
|
||||
impl Harness {
|
||||
/// Store over the same dir the manager records into.
|
||||
fn fx_retained(&self) -> maestro_daemon::retained_store::RetainedStore {
|
||||
self._fx.retained_store()
|
||||
}
|
||||
}
|
||||
|
||||
async fn spawn(hostname: &str) -> Harness {
|
||||
spawn_with(
|
||||
hostname,
|
||||
@@ -386,6 +393,62 @@ async fn delete_a_session_whose_worktree_holds_unsaved_work_returns_409_with_the
|
||||
assert!(h.manager.has(session.id).await, "the session keeps running");
|
||||
}
|
||||
|
||||
/// `?force=true` on the wire (#81): the refusal is overridden, the session
|
||||
/// ends, and the answer is 200 with the retained worktree named — path,
|
||||
/// branch, reason — while the directory survives on disk.
|
||||
#[tokio::test]
|
||||
async fn delete_with_force_ends_the_session_and_answers_what_was_retained() {
|
||||
let h = spawn("naps-laptop").await;
|
||||
let repo = tempfile::TempDir::new().unwrap();
|
||||
common::init_repo(repo.path());
|
||||
let wt_path = repo
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feat")
|
||||
.join("guarded")
|
||||
.to_path_buf();
|
||||
let session = h
|
||||
.manager
|
||||
.create(CreateSessionRequest {
|
||||
cwd: repo.path().to_path_buf(),
|
||||
command: "sleep 60".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: repo.path().to_path_buf(),
|
||||
branch: "feat/guarded".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
|
||||
|
||||
let resp = reqwest::Client::new()
|
||||
.delete(url(h.addr, &format!("/api/sessions/{}", session.id)))
|
||||
.query(&[("force", "true")])
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), reqwest::StatusCode::OK);
|
||||
let body: maestro_protocol::KillOutcome = resp.json().await.unwrap();
|
||||
let retained = body.retained.expect("force names what was kept");
|
||||
assert_eq!(retained.path, wt_path);
|
||||
assert_eq!(retained.branch, "feat/guarded");
|
||||
assert_eq!(
|
||||
retained.reason,
|
||||
maestro_protocol::RetainedReason::ForceKilled
|
||||
);
|
||||
|
||||
// The tree would have blocked; force must not have removed it.
|
||||
assert!(wt_path.exists(), "a forced kill never removes the worktree");
|
||||
// And the store entry exists, so reclaim can find it (#80).
|
||||
let all = h.fx_retained().load_all().unwrap();
|
||||
assert_eq!(all.len(), 1);
|
||||
assert_eq!(all[0].id, retained.id);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attach_ws_streams_pty_bytes() {
|
||||
let h = spawn("naps-laptop").await;
|
||||
@@ -476,7 +539,7 @@ async fn events_ws_sends_a_snapshot_then_streams_deltas() {
|
||||
other => panic!("expected SessionAdded, got {other:?}"),
|
||||
}
|
||||
|
||||
h.manager.kill(created.id).await.expect("kill");
|
||||
h.manager.kill(created.id, false).await.expect("kill");
|
||||
|
||||
// Activity ticks may interleave; the removal is what we are waiting for.
|
||||
let deadline = tokio::time::Instant::now() + Duration::from_secs(5);
|
||||
|
||||
@@ -211,7 +211,7 @@ async fn killing_a_dead_session_dismisses_it_without_touching_tmux() {
|
||||
fx.manager.start_liveness_watcher(Duration::from_millis(25));
|
||||
wait_for_status(&fx.manager, s.id, SessionStatus::Dead).await;
|
||||
|
||||
fx.manager.kill(s.id).await.expect("dismiss");
|
||||
fx.manager.kill(s.id, false).await.expect("dismiss");
|
||||
|
||||
assert!(fx.manager.snapshot().await.is_empty());
|
||||
assert!(
|
||||
@@ -371,7 +371,7 @@ async fn killing_an_unknown_session_is_reported_as_not_found() {
|
||||
let fx = Fx::new();
|
||||
let err = fx
|
||||
.manager
|
||||
.kill(uuid::Uuid::new_v4())
|
||||
.kill(uuid::Uuid::new_v4(), false)
|
||||
.await
|
||||
.expect_err("should fail");
|
||||
assert_eq!(err.to_error_info().code, "not_found");
|
||||
|
||||
@@ -8,7 +8,9 @@ use std::process::Command;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use common::{init_bare_repo, init_repo, push_all_to_origin, wait_for_async, DaemonFixture};
|
||||
use common::{
|
||||
init_bare_repo, init_repo, push_all_to_origin, wait_for_async, DaemonFixture, TmuxFixture,
|
||||
};
|
||||
use maestro_daemon::SessionError;
|
||||
use maestro_protocol::CreateSessionRequest;
|
||||
use maestro_protocol::{CatalogEvent, SessionStatus, WorktreeSpec};
|
||||
@@ -44,7 +46,7 @@ async fn create_then_kill_round_trip() {
|
||||
// FIFO should also exist
|
||||
assert!(fx.fifo_dir().join(format!("{}.fifo", session.id)).exists());
|
||||
|
||||
fx.manager.kill(session.id).await.expect("kill");
|
||||
fx.manager.kill(session.id, false).await.expect("kill");
|
||||
assert!(!fx.manager.has(session.id).await);
|
||||
assert!(!fx
|
||||
.sidecar_dir()
|
||||
@@ -79,7 +81,7 @@ async fn create_appends_to_spawn_history() {
|
||||
assert_eq!(recents[0].cwd, cwd.path());
|
||||
assert_eq!(recents[0].last_command, "sleep 30");
|
||||
|
||||
fx.manager.kill(session.id).await.expect("kill");
|
||||
fx.manager.kill(session.id, false).await.expect("kill");
|
||||
// History is durable — it survives the kill that deletes the sidecar.
|
||||
assert_eq!(fx.manager.recent_dirs().len(), 1);
|
||||
}
|
||||
@@ -185,7 +187,7 @@ async fn create_captures_initial_output_in_replay_buffer() {
|
||||
"first-line marker never reached the replay buffer — pipe-pane raced past the child's initial output",
|
||||
);
|
||||
|
||||
fx.manager.kill(s.id).await.unwrap();
|
||||
fx.manager.kill(s.id, false).await.unwrap();
|
||||
}
|
||||
|
||||
/// Regression for NAP-228: after `recover`, PTY output must reach new
|
||||
@@ -251,7 +253,7 @@ async fn recover_re_pipes_pane_so_output_reaches_subscribers() {
|
||||
String::from_utf8_lossy(&buf),
|
||||
);
|
||||
|
||||
revived.kill(s.id).await.unwrap();
|
||||
revived.kill(s.id, false).await.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -300,7 +302,7 @@ async fn pty_broadcast_delivers_bytes_to_subscribers() {
|
||||
assert!(buf_a.windows(want.len()).any(|w| w == want), "A missed");
|
||||
assert!(buf_b.windows(want.len()).any(|w| w == want), "B missed");
|
||||
|
||||
fx.manager.kill(s.id).await.unwrap();
|
||||
fx.manager.kill(s.id, false).await.unwrap();
|
||||
let _ = (cwd, PathBuf::from("/"));
|
||||
}
|
||||
|
||||
@@ -359,7 +361,7 @@ async fn liveness_watcher_marks_dead_sessions_and_keeps_them_until_dismissed() {
|
||||
);
|
||||
|
||||
// Dismissing it is what finally releases the artifacts.
|
||||
fx.manager.kill(s.id).await.expect("dismiss");
|
||||
fx.manager.kill(s.id, false).await.expect("dismiss");
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
assert!(!fx.sidecar_dir().join(format!("{}.json", s.id)).exists());
|
||||
assert!(!fx.fifo_dir().join(format!("{}.fifo", s.id)).exists());
|
||||
@@ -398,7 +400,7 @@ async fn kill_removes_worktree_even_when_spec_says_keep() {
|
||||
init_bare_repo(origin.path());
|
||||
push_all_to_origin(repo.path(), origin.path());
|
||||
|
||||
fx.manager.kill(s.id).await.expect("kill");
|
||||
fx.manager.kill(s.id, false).await.expect("kill");
|
||||
assert!(!wt_path.exists(), "worktree must be removed on kill");
|
||||
}
|
||||
|
||||
@@ -432,7 +434,11 @@ async fn killing_refuses_a_worktree_with_unsaved_work_and_keeps_the_session() {
|
||||
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
|
||||
std::fs::write(wt_path.join("notes.txt"), b"untracked\n").unwrap();
|
||||
|
||||
let err = fx.manager.kill(s.id).await.expect_err("kill must refuse");
|
||||
let err = fx
|
||||
.manager
|
||||
.kill(s.id, false)
|
||||
.await
|
||||
.expect_err("kill must refuse");
|
||||
let SessionError::KillGuarded(breakdown) = err else {
|
||||
panic!("expected KillGuarded, got {err:?}");
|
||||
};
|
||||
@@ -456,7 +462,10 @@ async fn killing_refuses_a_worktree_with_unsaved_work_and_keeps_the_session() {
|
||||
let origin = TempDir::new().unwrap();
|
||||
init_bare_repo(origin.path());
|
||||
push_all_to_origin(repo.path(), origin.path());
|
||||
fx.manager.kill(s.id).await.expect("kill after saving");
|
||||
fx.manager
|
||||
.kill(s.id, false)
|
||||
.await
|
||||
.expect("kill after saving");
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
}
|
||||
|
||||
@@ -487,7 +496,11 @@ async fn killing_refuses_until_the_branch_is_pushed() {
|
||||
.await
|
||||
.expect("create");
|
||||
|
||||
let err = fx.manager.kill(s.id).await.expect_err("unpushed blocks");
|
||||
let err = fx
|
||||
.manager
|
||||
.kill(s.id, false)
|
||||
.await
|
||||
.expect_err("unpushed blocks");
|
||||
let SessionError::KillGuarded(breakdown) = err else {
|
||||
panic!("expected KillGuarded, got {err:?}");
|
||||
};
|
||||
@@ -499,11 +512,118 @@ async fn killing_refuses_until_the_branch_is_pushed() {
|
||||
init_bare_repo(origin.path());
|
||||
push_all_to_origin(repo.path(), origin.path());
|
||||
|
||||
fx.manager.kill(s.id).await.expect("pushed branch kills");
|
||||
fx.manager
|
||||
.kill(s.id, false)
|
||||
.await
|
||||
.expect("pushed branch kills");
|
||||
assert!(!wt_path.exists());
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
}
|
||||
|
||||
/// `--force` overrides the refusal and nothing else (#81): the session ends,
|
||||
/// but the guarded worktree is RETAINED — never removed — and enters the
|
||||
/// retained store with reason `force-killed`. The outcome names what was
|
||||
/// kept, so a client can say where the work went.
|
||||
#[tokio::test]
|
||||
async fn force_killing_a_guarded_worktree_retains_it_in_the_store() {
|
||||
let fx = DaemonFixture::new();
|
||||
let repo = TempDir::new().unwrap();
|
||||
init_repo(repo.path());
|
||||
let wt_path = repo.path().join("worktrees").join("feat").join("forced");
|
||||
|
||||
let s = fx
|
||||
.manager
|
||||
.create(CreateSessionRequest {
|
||||
cwd: repo.path().to_path_buf(),
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: repo.path().to_path_buf(),
|
||||
branch: "feat/forced".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
})
|
||||
.await
|
||||
.expect("create");
|
||||
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
|
||||
|
||||
// Without force the same kill is refused (#79).
|
||||
assert!(fx.manager.kill(s.id, false).await.is_err());
|
||||
|
||||
let outcome = fx
|
||||
.manager
|
||||
.kill(s.id, true)
|
||||
.await
|
||||
.expect("force overrides the refusal");
|
||||
|
||||
// The tree would have blocked, so force must NOT have removed it...
|
||||
assert!(
|
||||
wt_path.exists(),
|
||||
"a forced kill never removes a tree that would have blocked"
|
||||
);
|
||||
// ...but the session itself still ends.
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
assert!(!fx.sidecar_dir().join(format!("{}.json", s.id)).exists());
|
||||
|
||||
let retained = outcome.retained.expect("the outcome names what was kept");
|
||||
assert_eq!(retained.path, wt_path);
|
||||
assert_eq!(retained.branch, "feat/forced");
|
||||
assert_eq!(
|
||||
retained.reason,
|
||||
maestro_protocol::RetainedReason::ForceKilled
|
||||
);
|
||||
assert!(
|
||||
retained.in_the_way.uncommitted_files >= 1,
|
||||
"the outcome carries what blocked the unforced kill"
|
||||
);
|
||||
|
||||
// The store holds the same entry, so reclaim can find it later (#80).
|
||||
let all = fx.retained_store().load_all().expect("load_all");
|
||||
assert_eq!(all.len(), 1);
|
||||
assert_eq!(all[0].id, retained.id);
|
||||
assert_eq!(all[0].path, wt_path);
|
||||
assert_eq!(all[0].branch, "feat/forced");
|
||||
}
|
||||
|
||||
/// Force overrides the *refusal*, not ordinary cleanup (#81): a tree whose
|
||||
/// guard is clear never needed the override, so a forced kill removes it as
|
||||
/// usual and retains nothing.
|
||||
#[tokio::test]
|
||||
async fn force_on_a_clear_worktree_removes_it_as_usual() {
|
||||
let fx = DaemonFixture::new();
|
||||
let repo = TempDir::new().unwrap();
|
||||
init_repo(repo.path());
|
||||
let wt_path = repo.path().join("worktrees").join("feat").join("clear");
|
||||
|
||||
let s = fx
|
||||
.manager
|
||||
.create(CreateSessionRequest {
|
||||
cwd: repo.path().to_path_buf(),
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: repo.path().to_path_buf(),
|
||||
branch: "feat/clear".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
})
|
||||
.await
|
||||
.expect("create");
|
||||
|
||||
let origin = TempDir::new().unwrap();
|
||||
init_bare_repo(origin.path());
|
||||
push_all_to_origin(repo.path(), origin.path());
|
||||
|
||||
let outcome = fx.manager.kill(s.id, true).await.expect("kill");
|
||||
assert!(!wt_path.exists(), "a clear tree needs no override");
|
||||
assert_eq!(outcome.retained, None);
|
||||
assert!(fx.retained_store().load_all().unwrap().is_empty());
|
||||
}
|
||||
|
||||
/// Ignored files NEVER block (#79): build output like `target/` exists in
|
||||
/// every Rust checkout, and an ignored-only worktree stays killable.
|
||||
#[tokio::test]
|
||||
@@ -543,13 +663,198 @@ async fn killing_an_ignored_only_worktree_succeeds() {
|
||||
push_all_to_origin(repo.path(), origin.path());
|
||||
|
||||
fx.manager
|
||||
.kill(s.id)
|
||||
.kill(s.id, false)
|
||||
.await
|
||||
.expect("ignored files never block");
|
||||
assert!(!wt_path.exists());
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
}
|
||||
|
||||
/// Natural exit (#82): a session that ends on its own never reaches the kill
|
||||
/// path, so the watcher applies the kill's rule itself. A worktree holding
|
||||
/// unsaved work is RETAINED into the store with reason
|
||||
/// `exited-with-unsaved-work`; the session stays in the catalog marked dead
|
||||
/// until dismissed — and dismissing it afterwards neither refuses nor
|
||||
/// retains a second time.
|
||||
#[tokio::test]
|
||||
async fn natural_exit_of_a_guarded_worktree_retains_it_in_the_store() {
|
||||
let fx = DaemonFixture::new();
|
||||
let repo = TempDir::new().unwrap();
|
||||
init_repo(repo.path());
|
||||
let wt_path = repo.path().join("worktrees").join("feat").join("exited");
|
||||
|
||||
let s = fx
|
||||
.manager
|
||||
.create(CreateSessionRequest {
|
||||
cwd: repo.path().to_path_buf(),
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: repo.path().to_path_buf(),
|
||||
branch: "feat/exited".into(),
|
||||
cleanup_on_kill: true,
|
||||
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 {
|
||||
fx.manager.snapshot().await[0].worktree.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(),
|
||||
cleanup_on_kill: true,
|
||||
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(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
})
|
||||
.await
|
||||
.expect("create");
|
||||
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
|
||||
|
||||
tmux.tmux.kill_session(s.id).expect("kill tmux directly");
|
||||
manager.start_liveness_watcher(Duration::from_millis(50));
|
||||
wait_for_async(Duration::from_secs(3), || async {
|
||||
manager.status(s.id).await == Some(SessionStatus::Dead)
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
wt_path.exists(),
|
||||
"nothing may be destroyed when it cannot be recorded"
|
||||
);
|
||||
let snap = manager.snapshot().await;
|
||||
assert!(
|
||||
snap[0].worktree.is_some(),
|
||||
"the tree must stay attached to the record so a later forced kill can retain it"
|
||||
);
|
||||
}
|
||||
|
||||
fn run_git_in(dir: &std::path::Path, args: &[&str]) {
|
||||
let out = Command::new("git")
|
||||
.arg("-C")
|
||||
@@ -587,5 +892,5 @@ async fn liveness_watcher_leaves_running_sessions_alone() {
|
||||
tokio::time::sleep(Duration::from_millis(400)).await;
|
||||
assert!(fx.manager.has(s.id).await, "live session got reaped");
|
||||
|
||||
fx.manager.kill(s.id).await.unwrap();
|
||||
fx.manager.kill(s.id, false).await.unwrap();
|
||||
}
|
||||
|
||||
@@ -12,7 +12,8 @@ use std::time::Duration;
|
||||
|
||||
use futures_util::{SinkExt, StreamExt};
|
||||
use maestro_protocol::{
|
||||
BranchInfo, CatalogEvent, CreateSessionRequest, DaemonClient, Project, Session,
|
||||
BranchInfo, CatalogEvent, CreateSessionRequest, DaemonClient, KillOutcome, Project,
|
||||
RetainedWorktree, Session,
|
||||
};
|
||||
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
|
||||
use tokio_tungstenite::tungstenite::Message as WsMessage;
|
||||
@@ -61,7 +62,21 @@ impl TestClient {
|
||||
}
|
||||
|
||||
pub async fn kill_session(&self, id: Uuid) -> Result<(), ClientError> {
|
||||
Ok(self.daemon.kill_session(id).await?)
|
||||
Ok(self.daemon.kill_session(id, false).await.map(|_| ())?)
|
||||
}
|
||||
|
||||
/// Kill with `force=true`: overrides a kill refusal and retains the
|
||||
/// guarded worktree (#81). Answers with what was kept.
|
||||
pub async fn kill_session_forced(&self, id: Uuid) -> Result<KillOutcome, ClientError> {
|
||||
Ok(self.daemon.kill_session(id, true).await?)
|
||||
}
|
||||
|
||||
pub async fn list_worktrees(&self) -> Result<Vec<RetainedWorktree>, ClientError> {
|
||||
Ok(self.daemon.list_worktrees().await?)
|
||||
}
|
||||
|
||||
pub async fn reclaim_worktree(&self, id: Uuid) -> Result<(), ClientError> {
|
||||
Ok(self.daemon.reclaim_worktree(id).await?)
|
||||
}
|
||||
|
||||
pub async fn list_projects(&self) -> Result<Vec<Project>, ClientError> {
|
||||
|
||||
+523
-33
@@ -35,6 +35,18 @@ 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 {
|
||||
serde_json::from_value(serde_json::json!({
|
||||
"base_repo": base_repo,
|
||||
"branch": branch,
|
||||
"create_branch": create_branch,
|
||||
}))
|
||||
.expect("worktree spec")
|
||||
}
|
||||
|
||||
/// Spawn request carrying a worktree spec against `base`.
|
||||
fn worktree_req(base: &std::path::Path, branch: &str, create_branch: bool) -> CreateSessionRequest {
|
||||
CreateSessionRequest {
|
||||
@@ -42,12 +54,7 @@ fn worktree_req(base: &std::path::Path, branch: &str, create_branch: bool) -> Cr
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: base.to_path_buf(),
|
||||
branch: branch.into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch,
|
||||
}),
|
||||
worktree: Some(worktree_spec(base, branch, create_branch)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -141,12 +148,7 @@ async fn kill_removes_clean_worktree() {
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: base.path().to_path_buf(),
|
||||
branch: "feature/e2e".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
worktree: Some(worktree_spec(base.path(), "feature/e2e", true)),
|
||||
};
|
||||
|
||||
let s = client.spawn_session(&req).await.expect("spawn worktree");
|
||||
@@ -160,7 +162,7 @@ async fn kill_removes_clean_worktree() {
|
||||
"expected a worktree for feature/e2e under <repo>/worktrees, got: {worktrees:?}"
|
||||
);
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
|
||||
// Give the daemon a moment to remove the worktree, then verify.
|
||||
@@ -199,12 +201,7 @@ async fn kill_refuses_worktree_with_uncommitted_changes() {
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: base.path().to_path_buf(),
|
||||
branch: "feature/unsaved".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
worktree: Some(worktree_spec(base.path(), "feature/unsaved", true)),
|
||||
};
|
||||
|
||||
let s = client.spawn_session(&req).await.expect("spawn worktree");
|
||||
@@ -262,12 +259,7 @@ async fn spawn_on_existing_branch_checks_it_out() {
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: base.path().to_path_buf(),
|
||||
branch: "feature/existing".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: false,
|
||||
}),
|
||||
worktree: Some(worktree_spec(base.path(), "feature/existing", false)),
|
||||
};
|
||||
|
||||
let s = client
|
||||
@@ -280,7 +272,7 @@ async fn spawn_on_existing_branch_checks_it_out() {
|
||||
"expected a worktree checked out at feature/existing, got: {worktrees:?}"
|
||||
);
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
@@ -328,7 +320,7 @@ async fn spawn_new_branch_creates_worktree_and_runs_in_it() {
|
||||
)
|
||||
.await;
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
@@ -376,7 +368,7 @@ async fn second_spawn_on_same_branch_is_rejected() {
|
||||
"original worktree must be untouched"
|
||||
);
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(first.id).await.expect("kill");
|
||||
}
|
||||
|
||||
@@ -419,10 +411,481 @@ async fn spawn_from_linked_worktree_places_in_main_checkout() {
|
||||
"worktrees must never nest inside another worktree"
|
||||
);
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Worktree lifecycle: collisions, retention, reclaim (#88)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// #77/#88: the target path `<repo>/worktrees/<branch>` is occupied by a real
|
||||
/// registered worktree the operator made by hand. The daemon sidesteps to the
|
||||
/// suffixed directory and never touches the occupant — and both trees keep
|
||||
/// working: two registered worktrees, two sessions, each running in its own.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn occupied_path_gets_suffixed_directory_and_both_worktrees_work() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
init_repo(base.path());
|
||||
|
||||
// An operator-made worktree sits exactly where `feature/collide` would go.
|
||||
// Its branch is a different one, so the requested branch stays creatable.
|
||||
let squatter = base
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feature")
|
||||
.join("collide");
|
||||
run_git(
|
||||
&[
|
||||
"worktree",
|
||||
"add",
|
||||
"-q",
|
||||
"-b",
|
||||
"operator/manual",
|
||||
&squatter.to_string_lossy(),
|
||||
],
|
||||
base.path(),
|
||||
);
|
||||
|
||||
let s = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/collide", true))
|
||||
.await
|
||||
.expect("spawn onto occupied path");
|
||||
|
||||
let suffixed = base
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feature")
|
||||
.join("collide-2");
|
||||
assert_eq!(
|
||||
s.cwd, suffixed,
|
||||
"the daemon must take the suffixed directory, not the occupied one"
|
||||
);
|
||||
assert!(suffixed.is_dir(), "suffixed worktree must exist on disk");
|
||||
|
||||
// Both trees are registered, distinct, and the squatter is byte-intact.
|
||||
let registrations = git_worktree_list(base.path());
|
||||
assert!(
|
||||
registrations
|
||||
.iter()
|
||||
.any(|l| l == &format!("worktree {}", squatter.canonicalize().unwrap().display())),
|
||||
"operator's worktree must stay registered: {registrations:?}"
|
||||
);
|
||||
assert!(
|
||||
registrations.iter().any(|l| l.contains("collide-2")),
|
||||
"suffixed worktree must be registered too: {registrations:?}"
|
||||
);
|
||||
|
||||
// Both sessions run inside their own tree at the same time.
|
||||
let sessions = client.list_sessions().await.expect("list");
|
||||
assert!(
|
||||
sessions.iter().any(|x| x.id == s.id && x.cwd == suffixed),
|
||||
"session must keep running in its suffixed tree"
|
||||
);
|
||||
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
|
||||
// Only the daemon-made trees go; the operator's manual one survives.
|
||||
let after = git_worktree_list(base.path());
|
||||
assert!(
|
||||
after
|
||||
.iter()
|
||||
.any(|l| l == &format!("worktree {}", squatter.canonicalize().unwrap().display())),
|
||||
"operator's worktree must outlive the sessions: {after:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// #77/#88: the target path holds a plain directory the daemon never made and
|
||||
/// git has never heard of. The daemon neither adopts it nor errors — it builds
|
||||
/// beside it and leaves every byte of it alone.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn foreign_directory_is_sidestepped_never_adopted() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
init_repo(base.path());
|
||||
|
||||
let squatter = base
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feature")
|
||||
.join("foreign");
|
||||
std::fs::create_dir_all(&squatter).unwrap();
|
||||
std::fs::write(squatter.join("operator-notes.txt"), b"mine\n").unwrap();
|
||||
|
||||
let s = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/foreign", true))
|
||||
.await
|
||||
.expect("spawn must sidestep, not adopt or fail");
|
||||
|
||||
let suffixed = base
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feature")
|
||||
.join("foreign-2");
|
||||
assert_eq!(s.cwd, suffixed, "session must run in the suffixed tree");
|
||||
assert_eq!(
|
||||
std::fs::read(squatter.join("operator-notes.txt")).unwrap(),
|
||||
b"mine\n",
|
||||
"the foreign directory must be untouched"
|
||||
);
|
||||
let registrations = git_worktree_list(base.path());
|
||||
let registered_paths: Vec<&str> = registrations
|
||||
.iter()
|
||||
.filter_map(|l| l.strip_prefix("worktree "))
|
||||
.collect();
|
||||
assert!(
|
||||
!registered_paths
|
||||
.iter()
|
||||
.any(|p| *p == squatter.canonicalize().unwrap().display().to_string()),
|
||||
"a plain directory must never be adopted as a worktree: {registered_paths:?}"
|
||||
);
|
||||
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
assert!(squatter.join("operator-notes.txt").exists());
|
||||
}
|
||||
|
||||
/// #79/#88: killing with uncommitted changes is refused; committing and
|
||||
/// pushing that work clears the guard and the very same kill then succeeds
|
||||
/// and removes the tree. The session is the handle on the work: while it is
|
||||
/// unsaved the session stays, once it is saved the tree goes.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn guarded_kill_then_commit_and_push_makes_the_same_kill_succeed() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
init_repo(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
|
||||
let wt = base.path().join("worktrees").join("feature").join("guard");
|
||||
let s = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/guard", true))
|
||||
.await
|
||||
.expect("spawn");
|
||||
std::fs::write(wt.join("README"), b"work in progress\n").unwrap();
|
||||
|
||||
// Refused: the edit is uncommitted.
|
||||
let err = client
|
||||
.kill_session(s.id)
|
||||
.await
|
||||
.expect_err("kill must refuse");
|
||||
match err {
|
||||
maestro_e2e::ClientError::Api(maestro_protocol::ClientError::Api { info, .. }) => {
|
||||
assert_eq!(info.code, "kill_guarded", "{info:?}");
|
||||
let breakdown = info.unsaved_work.expect("breakdown on the wire");
|
||||
assert_eq!(breakdown.uncommitted_files, 1);
|
||||
assert_eq!(breakdown.unpushed_commits, 0);
|
||||
}
|
||||
other => panic!("expected a kill_guarded refusal, got {other:?}"),
|
||||
}
|
||||
assert!(
|
||||
client
|
||||
.list_sessions()
|
||||
.await
|
||||
.unwrap()
|
||||
.iter()
|
||||
.any(|x| x.id == s.id),
|
||||
"a refused kill keeps the session running"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read(wt.join("README")).unwrap(),
|
||||
b"work in progress\n",
|
||||
"the worktree is untouched by the refusal"
|
||||
);
|
||||
|
||||
// The operator does exactly what the refusal asked for: save and push.
|
||||
commit_all(&wt, "wip");
|
||||
push_all(base.path());
|
||||
|
||||
client.kill_session(s.id).await.expect("kill after saving");
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
while Instant::now() < deadline {
|
||||
if !wt.exists() {
|
||||
break;
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(100)).await;
|
||||
}
|
||||
assert!(
|
||||
!wt.exists(),
|
||||
"a cleared worktree must be removed after kill"
|
||||
);
|
||||
let registrations = git_worktree_list(base.path());
|
||||
assert!(
|
||||
!registrations.iter().any(|l| l.contains("feature/guard")),
|
||||
"removal must deregister the tree: {registrations:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// #81/#88: commits that no remote has refuse the kill; `--force` ends the
|
||||
/// session anyway but RETAINS the worktree — force never destroys blocked
|
||||
/// work, it parks it in the retained store with reason `force-killed`.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn unpushed_commits_block_kill_and_force_retains_the_tree() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
init_repo(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
|
||||
let wt = base.path().join("worktrees").join("feature").join("local");
|
||||
let s = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/local", true))
|
||||
.await
|
||||
.expect("spawn");
|
||||
std::fs::write(wt.join("README"), b"local only\n").unwrap();
|
||||
commit_all(&wt, "not pushed yet");
|
||||
|
||||
// Refused: the commit is on no remote-tracking ref.
|
||||
let err = client
|
||||
.kill_session(s.id)
|
||||
.await
|
||||
.expect_err("kill must refuse");
|
||||
match err {
|
||||
maestro_e2e::ClientError::Api(maestro_protocol::ClientError::Api { info, .. }) => {
|
||||
assert_eq!(info.code, "kill_guarded", "{info:?}");
|
||||
let breakdown = info.unsaved_work.expect("breakdown on the wire");
|
||||
assert_eq!(breakdown.unpushed_commits, 1);
|
||||
assert_eq!(breakdown.uncommitted_files, 0);
|
||||
}
|
||||
other => panic!("expected a kill_guarded refusal, got {other:?}"),
|
||||
}
|
||||
|
||||
// Force overrides the refusal and nothing else: session gone, tree kept.
|
||||
let outcome = client.kill_session_forced(s.id).await.expect("forced kill");
|
||||
let retained = outcome
|
||||
.retained
|
||||
.expect("force must retain the guarded tree");
|
||||
assert_eq!(
|
||||
retained.reason,
|
||||
maestro_protocol::RetainedReason::ForceKilled
|
||||
);
|
||||
assert_eq!(retained.path, wt);
|
||||
assert_eq!(retained.branch, "feature/local");
|
||||
assert_eq!(retained.in_the_way.unpushed_commits, 1);
|
||||
|
||||
assert!(
|
||||
!client
|
||||
.list_sessions()
|
||||
.await
|
||||
.unwrap()
|
||||
.iter()
|
||||
.any(|x| x.id == s.id),
|
||||
"the forced kill ends the session"
|
||||
);
|
||||
assert!(wt.is_dir(), "a force-retained worktree must stay on disk");
|
||||
assert!(
|
||||
git_worktree_list(base.path())
|
||||
.iter()
|
||||
.any(|l| l.contains("feature/local")),
|
||||
"the retained tree stays registered in git"
|
||||
);
|
||||
}
|
||||
|
||||
/// #82/#88: a session that exits on its own never reaches the kill path, but
|
||||
/// its worktree follows the same rule. Dirty → retained with reason
|
||||
/// `exited-with-unsaved-work`; pristine → removed, leaving nothing behind.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn natural_exit_settles_dirty_and_pristine_worktrees_differently() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
// Two repos so the two cases cannot see each other's state.
|
||||
let dirty_repo = TempDir::new().unwrap();
|
||||
init_repo(dirty_repo.path());
|
||||
let _origin = throwaway_origin(dirty_repo.path());
|
||||
let pristine_repo = TempDir::new().unwrap();
|
||||
init_repo(pristine_repo.path());
|
||||
let _origin_pristine = throwaway_origin(pristine_repo.path());
|
||||
|
||||
let dirty_wt = dirty_repo
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feature")
|
||||
.join("messy");
|
||||
let pristine_wt = pristine_repo
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feature")
|
||||
.join("tidy");
|
||||
|
||||
let _dirty = client
|
||||
.spawn_session(&CreateSessionRequest {
|
||||
cwd: dirty_repo.path().to_path_buf(),
|
||||
command: "sh -c 'echo unsaved > scratch.txt'".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(worktree_spec(dirty_repo.path(), "feature/messy", true)),
|
||||
})
|
||||
.await
|
||||
.expect("spawn dirty");
|
||||
let _pristine = client
|
||||
.spawn_session(&create_req_cmd(
|
||||
pristine_repo.path(),
|
||||
"true",
|
||||
"feature/tidy",
|
||||
))
|
||||
.await
|
||||
.expect("spawn pristine");
|
||||
|
||||
assert!(dirty_wt.is_dir() && pristine_wt.is_dir());
|
||||
|
||||
// The watcher reaps dead tmux sessions every couple of ticks and settles
|
||||
// each worktree accordingly. Poll for both outcomes.
|
||||
wait_for_async(
|
||||
"dirty exit retains its worktree",
|
||||
Duration::from_secs(20),
|
||||
|| {
|
||||
let client = client.clone();
|
||||
let path = dirty_wt.clone();
|
||||
async move {
|
||||
client
|
||||
.list_worktrees()
|
||||
.await
|
||||
.ok()
|
||||
.map(|wts| {
|
||||
wts.iter().any(|w| {
|
||||
w.path == path
|
||||
&& w.reason
|
||||
== maestro_protocol::RetainedReason::ExitedWithUnsavedWork
|
||||
})
|
||||
})
|
||||
.unwrap_or(false)
|
||||
}
|
||||
},
|
||||
)
|
||||
.await;
|
||||
wait_for_async(
|
||||
"pristine exit removes its worktree entirely",
|
||||
Duration::from_secs(20),
|
||||
|| {
|
||||
let client = client.clone();
|
||||
let repo = pristine_repo.path().to_path_buf();
|
||||
let path = pristine_wt.clone();
|
||||
async move {
|
||||
if path.exists() {
|
||||
return false;
|
||||
}
|
||||
let listed = client.list_worktrees().await.ok();
|
||||
let listed_clear = listed
|
||||
.map(|wts| !wts.iter().any(|w| w.path == path))
|
||||
.unwrap_or(false);
|
||||
let registered = git_worktree_list(&repo);
|
||||
!registered.iter().any(|l| l.contains("feature/tidy")) && listed_clear
|
||||
}
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
// The dirty tree is still fully there, file included.
|
||||
assert!(dirty_wt.is_dir());
|
||||
assert_eq!(
|
||||
std::fs::read(dirty_wt.join("scratch.txt")).unwrap(),
|
||||
b"unsaved\n"
|
||||
);
|
||||
}
|
||||
|
||||
fn create_req_cmd(cwd: &std::path::Path, command: &str, branch: &str) -> CreateSessionRequest {
|
||||
CreateSessionRequest {
|
||||
cwd: cwd.to_path_buf(),
|
||||
command: command.into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(worktree_spec(cwd, branch, true)),
|
||||
}
|
||||
}
|
||||
|
||||
/// #80/#88: `GET /api/worktrees` lists what the daemon is holding, with the
|
||||
/// reason and a live in-the-way breakdown; reclaim removes the directory and
|
||||
/// the store entry but NEVER the branch — its commits outlive the reclaim.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn worktrees_endpoint_lists_reasons_and_reclaim_keeps_the_branch() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
init_repo(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
|
||||
let wt = base.path().join("worktrees").join("feature").join("parked");
|
||||
let s = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/parked", true))
|
||||
.await
|
||||
.expect("spawn");
|
||||
std::fs::write(wt.join("README"), b"worth keeping\n").unwrap();
|
||||
commit_all(&wt, "worth keeping");
|
||||
let sha = branch_sha(&wt, "feature/parked");
|
||||
|
||||
let outcome = client.kill_session_forced(s.id).await.expect("force kill");
|
||||
assert!(outcome.retained.is_some());
|
||||
|
||||
let listed = client.list_worktrees().await.expect("list worktrees");
|
||||
assert_eq!(listed.len(), 1, "exactly one retained entry: {listed:?}");
|
||||
let entry = &listed[0];
|
||||
assert_eq!(entry.path, wt);
|
||||
assert_eq!(entry.branch, "feature/parked");
|
||||
assert_eq!(entry.reason, maestro_protocol::RetainedReason::ForceKilled);
|
||||
assert_eq!(entry.in_the_way.unpushed_commits, 1);
|
||||
|
||||
client.reclaim_worktree(entry.id).await.expect("reclaim");
|
||||
assert!(!wt.exists(), "reclaim removes the directory");
|
||||
assert!(
|
||||
client.list_worktrees().await.unwrap().is_empty(),
|
||||
"reclaim drops the store entry"
|
||||
);
|
||||
|
||||
// The whole point of reclaim: the branch and its commits survive.
|
||||
assert_eq!(
|
||||
branch_sha(base.path(), "feature/parked"),
|
||||
sha,
|
||||
"the branch must keep pointing at the same commit after reclaim"
|
||||
);
|
||||
}
|
||||
|
||||
/// The retained store is state-dir backed (#80): a daemon restart must not
|
||||
/// orphan the trees it is holding — they are still on disk with entries
|
||||
/// pointing at them, and still reclaimable afterwards.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn retained_store_survives_a_daemon_restart() {
|
||||
let (mut daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
init_repo(base.path());
|
||||
let _origin = throwaway_origin(base.path());
|
||||
|
||||
let wt = base
|
||||
.path()
|
||||
.join("worktrees")
|
||||
.join("feature")
|
||||
.join("durable");
|
||||
let s = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/durable", true))
|
||||
.await
|
||||
.expect("spawn");
|
||||
std::fs::write(wt.join("README"), b"survives restarts\n").unwrap();
|
||||
commit_all(&wt, "survives restarts");
|
||||
client.kill_session_forced(s.id).await.expect("force kill");
|
||||
|
||||
daemon.restart().await;
|
||||
|
||||
let listed = client.list_worktrees().await.expect("list after restart");
|
||||
assert_eq!(listed.len(), 1, "store must survive restart: {listed:?}");
|
||||
assert_eq!(listed[0].path, wt);
|
||||
assert_eq!(
|
||||
listed[0].reason,
|
||||
maestro_protocol::RetainedReason::ForceKilled
|
||||
);
|
||||
assert!(wt.is_dir(), "the tree itself must survive the restart too");
|
||||
|
||||
client
|
||||
.reclaim_worktree(listed[0].id)
|
||||
.await
|
||||
.expect("reclaim");
|
||||
assert!(!wt.exists());
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// GET /api/projects + /api/branches (#54/#55/#66)
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -788,10 +1251,12 @@ fn git_worktree_list(dir: &std::path::Path) -> Vec<String> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Push every branch of `repo` to a throwaway bare remote, so its commits are
|
||||
/// reachable from remote-tracking refs. Without one the kill guard (#79)
|
||||
/// reads every commit as unpushed, and any worktree session refuses to die.
|
||||
fn push_to_throwaway_origin(repo: &std::path::Path) {
|
||||
/// Bare remote registered as `origin` on `repo`, seeded with every current
|
||||
/// branch, so existing commits are reachable from remote-tracking refs.
|
||||
/// Without one the kill guard (#79) reads every commit as unpushed, and any
|
||||
/// worktree session refuses to die. The caller must keep the `TempDir` alive
|
||||
/// while the remote is still needed; later pushes go through [`push_all`].
|
||||
fn throwaway_origin(repo: &std::path::Path) -> TempDir {
|
||||
let origin = tempfile::TempDir::new().unwrap();
|
||||
run_git(
|
||||
&["init", "-q", "--bare", origin.path().to_str().unwrap()],
|
||||
@@ -803,9 +1268,34 @@ fn push_to_throwaway_origin(repo: &std::path::Path) {
|
||||
.args(["remote", "add", "origin", origin.path().to_str().unwrap()])
|
||||
.output()
|
||||
.expect("git remote add");
|
||||
push_all(repo);
|
||||
origin
|
||||
}
|
||||
|
||||
fn push_all(repo: &std::path::Path) {
|
||||
run_git(&["push", "-q", "--all", "origin"], repo);
|
||||
}
|
||||
|
||||
fn commit_all(dir: &std::path::Path, message: &str) {
|
||||
run_git(&["add", "-A"], dir);
|
||||
run_git(&["commit", "-q", "-m", message], dir);
|
||||
}
|
||||
|
||||
fn branch_sha(repo: &std::path::Path, branch: &str) -> String {
|
||||
let out = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo)
|
||||
.args(["rev-parse", "--verify", &format!("refs/heads/{branch}")])
|
||||
.output()
|
||||
.expect("git rev-parse");
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"branch {branch} missing: {}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
String::from_utf8_lossy(&out.stdout).trim().to_string()
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn daemon_events_ws_streams_snapshot_then_session_added() {
|
||||
// NAP-248: subscribe to the daemon's WS /api/events, expect an initial
|
||||
|
||||
@@ -16,7 +16,7 @@ use uuid::Uuid;
|
||||
use crate::routes::{self, UrlError};
|
||||
use crate::{
|
||||
ApiError, BranchInfo, CreateSessionRequest, DaemonInfo, DirListing, ErrorInfo, GitRoot,
|
||||
Project, RecentDirs, RetainedWorktree, Session,
|
||||
KillOutcome, Project, RecentDirs, RetainedWorktree, Session,
|
||||
};
|
||||
|
||||
/// Time budget for establishing a TCP connection to the daemon.
|
||||
@@ -114,12 +114,24 @@ impl DaemonClient {
|
||||
self.post(routes::SESSIONS, req).await
|
||||
}
|
||||
|
||||
pub async fn kill_session(&self, id: Uuid) -> Result<(), ClientError> {
|
||||
/// Kill a session. With `force`, the daemon overrides the kill refusal
|
||||
/// (#79) and retains the worktree instead of removing it (#81); the
|
||||
/// outcome then names what was kept and where.
|
||||
pub async fn kill_session(&self, id: Uuid, force: bool) -> Result<KillOutcome, ClientError> {
|
||||
let query: &[(&str, &str)] = if force {
|
||||
&[(routes::FORCE_QUERY, "true")]
|
||||
} else {
|
||||
&[]
|
||||
};
|
||||
let resp = self
|
||||
.send(Method::DELETE, &routes::session(id), &[], None::<&()>)
|
||||
.send(Method::DELETE, &routes::session(id), query, None::<&()>)
|
||||
.await?;
|
||||
check(resp).await?;
|
||||
Ok(())
|
||||
let resp = check(resp).await?;
|
||||
// An unforced kill keeps its historical `204 No Content`.
|
||||
if resp.status() == reqwest::StatusCode::NO_CONTENT {
|
||||
return Ok(KillOutcome { retained: None });
|
||||
}
|
||||
json(resp).await
|
||||
}
|
||||
|
||||
pub async fn daemon_info(&self) -> Result<DaemonInfo, ClientError> {
|
||||
|
||||
@@ -195,6 +195,19 @@ pub struct RetainedWorktree {
|
||||
pub in_the_way: InTheWay,
|
||||
}
|
||||
|
||||
/// Body of a successful forced `DELETE /api/sessions/:id?force=true` (#81).
|
||||
/// A forced kill ends the session and retains its worktree instead of
|
||||
/// removing it; this names what was kept, where, and why, so a client can say
|
||||
/// where the work went rather than leaving the operator to wonder.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct KillOutcome {
|
||||
/// Set when the worktree was retained with reason `force-killed`; `None`
|
||||
/// when there was nothing to retain — no worktree at all, or one whose
|
||||
/// guard was clear, which a forced kill removes as usual.
|
||||
#[serde(default)]
|
||||
pub retained: Option<RetainedWorktree>,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for InTheWay {
|
||||
/// "3 uncommitted files, 2 unpushed commits" — zero parts are omitted;
|
||||
/// all-zero renders "no unsaved work". This is the phrasing every client
|
||||
@@ -679,6 +692,22 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kill_outcome_round_trips_a_retained_worktree() {
|
||||
let outcome = KillOutcome {
|
||||
retained: Some(sample_retained_worktree()),
|
||||
};
|
||||
assert_eq!(outcome, round_trip(&outcome));
|
||||
}
|
||||
|
||||
/// A kill with nothing to retain — no worktree, or a clean one removed as
|
||||
/// usual — decodes from an absent field.
|
||||
#[test]
|
||||
fn kill_outcome_defaults_to_nothing_retained() {
|
||||
let outcome: KillOutcome = serde_json::from_value(serde_json::json!({})).unwrap();
|
||||
assert_eq!(outcome.retained, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_event_round_trips_and_tags() {
|
||||
let session = sample_session();
|
||||
|
||||
@@ -38,6 +38,10 @@ pub const PATH_QUERY: &str = "path";
|
||||
/// Name of the query parameter [`NAMES`] takes.
|
||||
pub const COUNT_QUERY: &str = "count";
|
||||
|
||||
/// Name of the boolean query parameter `DELETE /api/sessions/:id` takes (#81).
|
||||
/// `force=true` overrides the kill refusal and retains the worktree.
|
||||
pub const FORCE_QUERY: &str = "force";
|
||||
|
||||
/// `DELETE /api/sessions/:id`.
|
||||
pub fn session(id: Uuid) -> String {
|
||||
format!("{SESSIONS}/{id}")
|
||||
|
||||
@@ -733,8 +733,14 @@ impl App {
|
||||
}
|
||||
|
||||
async fn submit_kill(&mut self, sid: Uuid) {
|
||||
match self.daemon.kill_session(sid).await {
|
||||
Ok(()) => self.release_session(sid),
|
||||
match self.daemon.kill_session(sid, false).await {
|
||||
Ok(outcome) => {
|
||||
if let Some(w) = outcome.retained {
|
||||
self.state
|
||||
.toast(format!("worktree retained: {}", w.path.display()));
|
||||
}
|
||||
self.release_session(sid)
|
||||
}
|
||||
Err(e) => self.state.toast(format!("kill failed: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
+20
-4
@@ -75,11 +75,16 @@ That is only safe because force retains.
|
||||
|
||||
`--force` overrides the refusal and nothing else. It ends the session and
|
||||
**retains** the worktree, which enters the retained store with reason
|
||||
`force-killed`. Force never removes a tree that would have blocked.
|
||||
`force-killed`. Force never removes a tree that would have blocked. A tree
|
||||
whose guard is clear never needed the override, so a forced kill removes it as
|
||||
usual — force changes what happens to blocked work, not ordinary cleanup.
|
||||
|
||||
Ending a session and destroying its work are two separate destructive acts. Each
|
||||
is chosen on its own, and the second one — reclaim — is chosen while looking at
|
||||
exactly what would be lost.
|
||||
Ending a session and destroying its work are two separate destructive acts.
|
||||
Each is chosen on its own, and the second one — reclaim — is chosen while
|
||||
looking at exactly what would be lost.
|
||||
|
||||
A forced kill answers with what was retained and why, so a client can say where
|
||||
the work went rather than leaving the operator to wonder.
|
||||
|
||||
### Exiting on its own
|
||||
|
||||
@@ -92,6 +97,17 @@ Nobody is waiting for an answer here, so there is nothing to refuse; the choice
|
||||
only between removing and retaining. It uses the same predicate as the kill, from
|
||||
the same place, so the two paths cannot drift apart.
|
||||
|
||||
The session itself stays in the catalog marked dead until it is dismissed — the
|
||||
replay buffer of a crashed agent outlives it — but its worktree is settled at
|
||||
once. Once settled, the session drops its worktree reference, so dismissing it
|
||||
afterwards neither refuses (there is no guard left to trip) nor records a second
|
||||
store entry.
|
||||
|
||||
If settling is impossible — the probe fails, or the store cannot take the entry —
|
||||
nothing is destroyed. The tree stays on disk *and* stays attached to the dead
|
||||
record, where dismissing it later still goes through `--force`'s
|
||||
retain-before-teardown path. Losing the store must never mean losing the work.
|
||||
|
||||
## The retained store
|
||||
|
||||
A `state_dir`-backed record of worktrees the daemon created that no longer have a
|
||||
|
||||
Reference in New Issue
Block a user