Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a575b8bef1 | ||
|
|
898c3cc763 | ||
|
|
50a9d35f62 | ||
|
|
a9ef3ff97b | ||
|
|
132b606830 |
@@ -103,6 +103,11 @@ impl DaemonConfig {
|
||||
pub fn spawn_history_path(&self) -> PathBuf {
|
||||
self.state_dir.join("spawn-history.jsonl")
|
||||
}
|
||||
|
||||
/// Retained-worktree store directory (#78): `<state_dir>/retained-worktrees/`.
|
||||
pub fn retained_worktree_dir(&self) -> PathBuf {
|
||||
self.state_dir.join("retained-worktrees")
|
||||
}
|
||||
}
|
||||
|
||||
/// Env-var overrides applied by [`load_config`]. Tests pass explicit
|
||||
|
||||
@@ -111,6 +111,7 @@ fn io_error(dir: &Path, e: &std::io::Error) -> ErrorInfo {
|
||||
ErrorInfo {
|
||||
code: code.into(),
|
||||
message: format!("{message}: {}", dir.display()),
|
||||
unsaved_work: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ pub mod panes;
|
||||
pub mod proc;
|
||||
pub mod projects;
|
||||
pub mod pty;
|
||||
pub mod retained_store;
|
||||
pub mod session;
|
||||
pub mod sidecar;
|
||||
pub mod spawn_history;
|
||||
|
||||
@@ -94,6 +94,8 @@ fn session_error_status(code: &str) -> StatusCode {
|
||||
match code {
|
||||
"not_found" => StatusCode::NOT_FOUND,
|
||||
"invalid_env" => StatusCode::BAD_REQUEST,
|
||||
// A guard refusal is the daemon doing its job, not a failure (#79).
|
||||
"kill_guarded" => StatusCode::CONFLICT,
|
||||
_ => StatusCode::BAD_GATEWAY,
|
||||
}
|
||||
}
|
||||
@@ -168,16 +170,17 @@ async fn create_session(
|
||||
/// `kill` already reports a missing session as `SessionError::NotFound`, which
|
||||
/// maps to 404. Checking first would only add a window in which the liveness
|
||||
/// watcher reaps the session between the two awaits, turning a 404 into a 502.
|
||||
///
|
||||
/// 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(),
|
||||
Err(e) => {
|
||||
let info = e.to_error_info();
|
||||
error_response(
|
||||
session_error_status(&info.code),
|
||||
info.code.as_str(),
|
||||
info.message.as_str(),
|
||||
)
|
||||
let status = session_error_status(&info.code);
|
||||
(status, Json(ApiError { error: info })).into_response()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,319 @@
|
||||
//! Store of retained worktrees (#78): worktrees the daemon created whose
|
||||
//! session is gone.
|
||||
//!
|
||||
//! One JSON file per entry under `<state_dir>/retained-worktrees/`, written
|
||||
//! the way `sidecar.rs` writes session records — atomic tmp+rename replace,
|
||||
//! missing or malformed files tolerated on read. The store survives a daemon
|
||||
//! restart; it is the recovery path for everything else in the worktree
|
||||
//! lifecycle.
|
||||
//!
|
||||
//! It is not an ownership record: it must never be consulted to decide whether
|
||||
//! a path may be adopted. Its only job is to remember what the daemon is
|
||||
//! holding so it can be listed and reclaimed.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use chrono::{DateTime, Utc};
|
||||
use uuid::Uuid;
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum RetainedStoreError {
|
||||
#[error("io: {0}")]
|
||||
Io(#[from] std::io::Error),
|
||||
#[error("json: {0}")]
|
||||
Json(#[from] serde_json::Error),
|
||||
}
|
||||
|
||||
/// Why a worktree outlived its session. Kebab-case on the wire; new variants
|
||||
/// are additive but clients should render unknown reasons generically.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
|
||||
#[serde(rename_all = "kebab-case")]
|
||||
pub enum RetainedReason {
|
||||
/// The session was killed with `--force` while its worktree held unsaved
|
||||
/// work.
|
||||
ForceKilled,
|
||||
/// The session ended by itself and its worktree held unsaved work.
|
||||
ExitedWithUnsavedWork,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)]
|
||||
pub struct RetainedWorktree {
|
||||
pub id: Uuid,
|
||||
pub path: PathBuf,
|
||||
pub base_repo: PathBuf,
|
||||
pub branch: String,
|
||||
pub reason: RetainedReason,
|
||||
pub retained_at: DateTime<Utc>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct RetainedStore {
|
||||
dir: PathBuf,
|
||||
}
|
||||
|
||||
impl RetainedStore {
|
||||
pub fn new(dir: impl Into<PathBuf>) -> Self {
|
||||
Self { dir: dir.into() }
|
||||
}
|
||||
|
||||
pub fn dir(&self) -> &Path {
|
||||
&self.dir
|
||||
}
|
||||
|
||||
fn path_for(&self, id: Uuid) -> PathBuf {
|
||||
self.dir.join(format!("{id}.json"))
|
||||
}
|
||||
|
||||
/// Record a retained worktree and return the entry that was written.
|
||||
pub fn retain(
|
||||
&self,
|
||||
path: &Path,
|
||||
base_repo: &Path,
|
||||
branch: &str,
|
||||
reason: RetainedReason,
|
||||
) -> Result<RetainedWorktree, RetainedStoreError> {
|
||||
let entry = RetainedWorktree {
|
||||
id: Uuid::new_v4(),
|
||||
path: path.to_path_buf(),
|
||||
base_repo: base_repo.to_path_buf(),
|
||||
branch: branch.to_string(),
|
||||
reason,
|
||||
retained_at: Utc::now(),
|
||||
};
|
||||
self.overwrite(&entry)?;
|
||||
Ok(entry)
|
||||
}
|
||||
|
||||
/// Atomically write `entry`. Creates the directory if it does not exist.
|
||||
pub fn overwrite(&self, entry: &RetainedWorktree) -> Result<(), RetainedStoreError> {
|
||||
std::fs::create_dir_all(&self.dir)?;
|
||||
let final_path = self.path_for(entry.id);
|
||||
let tmp = self.dir.join(format!("{}.json.tmp", entry.id));
|
||||
let json = serde_json::to_vec_pretty(entry)?;
|
||||
std::fs::write(&tmp, json)?;
|
||||
std::fs::rename(&tmp, &final_path)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Drop one entry. No-op when the id is unknown — reclaiming an entry the
|
||||
/// store already lost (e.g. pruned) must not fail.
|
||||
pub fn remove(&self, id: Uuid) -> Result<(), RetainedStoreError> {
|
||||
match std::fs::remove_file(self.path_for(id)) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
|
||||
Err(e) => Err(e.into()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Read every entry. Files that are missing or malformed are skipped
|
||||
/// silently: a corrupt record must not hide the others, and an empty or
|
||||
/// absent directory simply means nothing is retained.
|
||||
pub fn load_all(&self) -> Result<Vec<RetainedWorktree>, RetainedStoreError> {
|
||||
let mut out = Vec::new();
|
||||
let entries = match std::fs::read_dir(&self.dir) {
|
||||
Ok(e) => e,
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(out),
|
||||
Err(e) => return Err(e.into()),
|
||||
};
|
||||
for entry in entries {
|
||||
let entry = entry?;
|
||||
let path = entry.path();
|
||||
if path.extension().and_then(|s| s.to_str()) != Some("json") {
|
||||
continue;
|
||||
}
|
||||
let Ok(bytes) = std::fs::read(&path) else {
|
||||
continue;
|
||||
};
|
||||
if let Ok(worktree) = serde_json::from_slice::<RetainedWorktree>(&bytes) {
|
||||
out.push(worktree);
|
||||
}
|
||||
}
|
||||
out.sort_by_key(|w| w.retained_at);
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
/// Drop entries whose worktree directory no longer exists — somebody
|
||||
/// removed it by hand. Returns the ids that were pruned.
|
||||
pub fn prune_missing(&self) -> Result<Vec<Uuid>, RetainedStoreError> {
|
||||
let mut pruned = Vec::new();
|
||||
for entry in self.load_all()? {
|
||||
if !entry.path.exists() {
|
||||
self.remove(entry.id)?;
|
||||
pruned.push(entry.id);
|
||||
}
|
||||
}
|
||||
Ok(pruned)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use chrono::Duration;
|
||||
use std::path::PathBuf;
|
||||
|
||||
fn tmp_dir() -> tempfile::TempDir {
|
||||
tempfile::tempdir().expect("tempdir")
|
||||
}
|
||||
|
||||
fn sample_path(root: &std::path::Path, name: &str) -> PathBuf {
|
||||
let p = root.join(name);
|
||||
std::fs::create_dir_all(&p).expect("mkdir");
|
||||
p
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retain_then_load_round_trips_every_field() {
|
||||
let dir = tmp_dir();
|
||||
let store = RetainedStore::new(dir.path());
|
||||
let wt = sample_path(dir.path(), "wt");
|
||||
let repo = sample_path(dir.path(), "repo");
|
||||
|
||||
let entry = store
|
||||
.retain(&wt, &repo, "feature/login", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
|
||||
let all = store.load_all().expect("load_all");
|
||||
assert_eq!(all, vec![entry.clone()]);
|
||||
assert_eq!(entry.path, wt);
|
||||
assert_eq!(entry.base_repo, repo);
|
||||
assert_eq!(entry.branch, "feature/login");
|
||||
assert_eq!(entry.reason, RetainedReason::ForceKilled);
|
||||
let age = Utc::now() - entry.retained_at;
|
||||
assert!(age >= Duration::zero() && age < Duration::minutes(1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reason_serializes_kebab_case() {
|
||||
let json =
|
||||
serde_json::to_string(&RetainedReason::ExitedWithUnsavedWork).expect("serialize");
|
||||
assert_eq!(json, r#""exited-with-unsaved-work""#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn entries_survive_a_restart() {
|
||||
let dir = tmp_dir();
|
||||
let wt = sample_path(dir.path(), "wt");
|
||||
|
||||
let first = RetainedStore::new(dir.path());
|
||||
let entry = first
|
||||
.retain(&wt, dir.path(), "main", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
|
||||
// A fresh store over the same state_dir — what a daemon restart sees.
|
||||
let second = RetainedStore::new(dir.path());
|
||||
assert_eq!(second.load_all().expect("load_all"), vec![entry]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_store_reads_as_empty() {
|
||||
let dir = tmp_dir();
|
||||
let store = RetainedStore::new(dir.path().join("does-not-exist"));
|
||||
assert_eq!(store.load_all().expect("load_all"), Vec::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn malformed_entries_are_skipped_silently() {
|
||||
let dir = tmp_dir();
|
||||
let store = RetainedStore::new(dir.path());
|
||||
let wt = sample_path(dir.path(), "wt");
|
||||
store
|
||||
.retain(&wt, dir.path(), "kept", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
std::fs::write(dir.path().join("not-a-uuid.json"), b"{oops").expect("write junk");
|
||||
|
||||
let all = store.load_all().expect("load_all");
|
||||
assert_eq!(all.len(), 1);
|
||||
assert_eq!(all[0].branch, "kept");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ids_are_unique() {
|
||||
let dir = tmp_dir();
|
||||
let store = RetainedStore::new(dir.path());
|
||||
let a = store
|
||||
.retain(dir.path(), dir.path(), "a", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
let b = store
|
||||
.retain(dir.path(), dir.path(), "b", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
assert_ne!(a.id, b.id);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_drops_the_entry() {
|
||||
let dir = tmp_dir();
|
||||
let store = RetainedStore::new(dir.path());
|
||||
let wt = sample_path(dir.path(), "wt");
|
||||
let entry = store
|
||||
.retain(&wt, dir.path(), "main", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
|
||||
store.remove(entry.id).expect("remove");
|
||||
assert_eq!(store.load_all().expect("load_all"), Vec::new());
|
||||
|
||||
// Removing an unknown id is a no-op, not an error.
|
||||
store.remove(entry.id).expect("remove again");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_all_is_sorted_by_time() {
|
||||
let dir = tmp_dir();
|
||||
let store = RetainedStore::new(dir.path());
|
||||
let mut first = store
|
||||
.retain(dir.path(), dir.path(), "first", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
first.retained_at -= Duration::hours(1);
|
||||
store.overwrite(&first).expect("rewrite");
|
||||
|
||||
let second = store
|
||||
.retain(
|
||||
dir.path(),
|
||||
dir.path(),
|
||||
"second",
|
||||
RetainedReason::ForceKilled,
|
||||
)
|
||||
.expect("retain");
|
||||
|
||||
let branches: Vec<_> = store
|
||||
.load_all()
|
||||
.expect("load_all")
|
||||
.into_iter()
|
||||
.map(|e| e.branch)
|
||||
.collect();
|
||||
assert_eq!(branches, vec!["first", "second"]);
|
||||
let _ = second;
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prune_missing_drops_gone_directories_only() {
|
||||
let dir = tmp_dir();
|
||||
let store = RetainedStore::new(dir.path());
|
||||
let alive = sample_path(dir.path(), "alive");
|
||||
let gone = sample_path(dir.path(), "gone");
|
||||
let keep = store
|
||||
.retain(&alive, dir.path(), "keep", RetainedReason::ForceKilled)
|
||||
.expect("retain");
|
||||
let drop = store
|
||||
.retain(
|
||||
&gone,
|
||||
dir.path(),
|
||||
"drop",
|
||||
RetainedReason::ExitedWithUnsavedWork,
|
||||
)
|
||||
.expect("retain");
|
||||
std::fs::remove_dir_all(&gone).expect("rm");
|
||||
|
||||
let pruned = store.prune_missing().expect("prune_missing");
|
||||
assert_eq!(pruned, vec![drop.id]);
|
||||
|
||||
let branches: Vec<_> = store
|
||||
.load_all()
|
||||
.expect("load_all")
|
||||
.into_iter()
|
||||
.map(|e| e.branch)
|
||||
.collect();
|
||||
assert_eq!(branches, vec!["keep"]);
|
||||
let _ = keep;
|
||||
}
|
||||
}
|
||||
@@ -38,8 +38,8 @@ pub const DEFAULT_LIVENESS_INTERVAL: Duration = Duration::from_secs(2);
|
||||
pub struct SessionRecord {
|
||||
pub session: Session,
|
||||
pub fifo_path: PathBuf,
|
||||
/// Worktree we created at spawn time; removed on kill unless the
|
||||
/// dirty-work guard refuses (#56).
|
||||
/// Worktree we created at spawn time; removed on kill, but only after the
|
||||
/// kill guard (#79) found nothing worth keeping in it.
|
||||
pub worktree_path: Option<PathBuf>,
|
||||
/// `base_repo` from the original `WorktreeSpec`. Needed because `git
|
||||
/// worktree remove` is invoked from the base repo.
|
||||
@@ -61,6 +61,11 @@ pub enum SessionError {
|
||||
NotFound(Uuid),
|
||||
#[error("invalid environment variable name: {0:?}")]
|
||||
InvalidEnv(String),
|
||||
/// The kill was refused (#79): the session's worktree holds unsaved work.
|
||||
/// Nothing happened — the session keeps running and the tree is untouched,
|
||||
/// 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::UnsavedWork),
|
||||
}
|
||||
|
||||
impl SessionError {
|
||||
@@ -72,10 +77,16 @@ impl SessionError {
|
||||
SessionError::Io(_) => "io_failed",
|
||||
SessionError::NotFound(_) => "not_found",
|
||||
SessionError::InvalidEnv(_) => "invalid_env",
|
||||
SessionError::KillGuarded(_) => "kill_guarded",
|
||||
};
|
||||
let unsaved_work = match self {
|
||||
SessionError::KillGuarded(b) => Some(*b),
|
||||
_ => None,
|
||||
};
|
||||
ErrorInfo {
|
||||
code: code.into(),
|
||||
message: self.to_string(),
|
||||
unsaved_work,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -370,20 +381,48 @@ impl SessionManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Kill a session and remove its on-disk artifacts. The worktree is
|
||||
/// always pruned -- unless it holds uncommitted changes or commits not on
|
||||
/// any other ref, in which case the guard leaves it on disk (#56).
|
||||
/// Kill a session and remove its on-disk artifacts.
|
||||
///
|
||||
/// Refused (#79) when the session's worktree holds unsaved work —
|
||||
/// uncommitted tracked changes, untracked non-ignored files, or commits
|
||||
/// reachable from no remote-tracking ref. Nothing is killed and nothing
|
||||
/// is removed; the error names what blocked it and how much of it there
|
||||
/// is. The session stays as the operator's handle on their own unfinished
|
||||
/// work, so ending a session that still matters becomes a deliberate
|
||||
/// `--force`, which retains rather than destroys (WORKTREES.md).
|
||||
///
|
||||
/// 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) = {
|
||||
let sessions = self.inner.sessions.lock().await;
|
||||
match sessions.get(&id) {
|
||||
None => return Err(SessionError::NotFound(id)),
|
||||
Some(record) => (
|
||||
record
|
||||
.worktree_base_repo
|
||||
.clone()
|
||||
.zip(record.worktree_path.clone()),
|
||||
record.session.status == SessionStatus::Running,
|
||||
),
|
||||
}
|
||||
};
|
||||
// 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::unsaved_work(&path))
|
||||
.await
|
||||
.map_err(SessionError::Worktree)?;
|
||||
if !probe.is_empty() {
|
||||
return Err(SessionError::KillGuarded(probe));
|
||||
}
|
||||
}
|
||||
let removed = {
|
||||
let mut sessions = self.inner.sessions.lock().await;
|
||||
sessions.remove(&id)
|
||||
};
|
||||
let record = removed.ok_or(SessionError::NotFound(id))?;
|
||||
let alive = record.session.status == SessionStatus::Running;
|
||||
cleanup_record(&self.inner, id, record, alive).await;
|
||||
Ok(())
|
||||
}
|
||||
@@ -720,10 +759,9 @@ 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");
|
||||
}
|
||||
// Cleanup is unconditional now (#56): `cleanup_on_kill` is no longer a
|
||||
// user-facing choice. The manager's guard is what protects unsaved work --
|
||||
// when it refuses, leave the tree on disk and say where it is so the
|
||||
// operator can find it.
|
||||
// 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) {
|
||||
tracing::warn!(
|
||||
|
||||
@@ -10,7 +10,9 @@ pub enum WorktreeError {
|
||||
Spawn(#[from] std::io::Error),
|
||||
#[error("git exited with code {code}: {stderr}")]
|
||||
NonZero { code: i32, stderr: String },
|
||||
/// The worktree holds unsaved work; removal was refused (#56).
|
||||
/// A registered worktree lives under the tree about to be removed; git
|
||||
/// hides its contents from `git status`, so removal would delete its
|
||||
/// uncommitted work unseen (#54 review).
|
||||
#[error("left {} on disk: {}", .path.display(), .reason)]
|
||||
Guarded { path: PathBuf, reason: String },
|
||||
}
|
||||
@@ -126,12 +128,13 @@ impl WorktreeManager {
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
/// `git -C <base_repo> worktree remove <path>` — but only when the
|
||||
/// worktree holds nothing unique (#56). A worktree with uncommitted
|
||||
/// changes, or with commits reachable from no other ref, is refused and
|
||||
/// left on disk so the operator can rescue it; cleanup is unconditional,
|
||||
/// which makes this guard the only thing standing between a stray kill
|
||||
/// and destroyed work.
|
||||
/// `git -C <base_repo> worktree remove <path>`.
|
||||
///
|
||||
/// Unconditional (#79): the guard moved off the worktree and onto the
|
||||
/// kill, so by the time removal runs the caller has already chosen to
|
||||
/// destroy the tree — the kill only gets here after
|
||||
/// [`unsaved_work`] came back empty. Still refused for a tree holding a
|
||||
/// nested registered worktree, which `git status` cannot see.
|
||||
pub fn remove(&self, base_repo: &Path, path: &Path) -> Result<(), WorktreeError> {
|
||||
// Gone from disk already (operator `rm -rf`, or a create that failed
|
||||
// half-way). Probing it with git would fail and be misread as unsaved
|
||||
@@ -147,12 +150,6 @@ impl WorktreeManager {
|
||||
reason: format!("holds a nested worktree at {}", inner.display()),
|
||||
});
|
||||
}
|
||||
if let Some(reason) = unsafe_to_remove(path)? {
|
||||
return Err(WorktreeError::Guarded {
|
||||
path: path.to_path_buf(),
|
||||
reason,
|
||||
});
|
||||
}
|
||||
let out = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(base_repo)
|
||||
@@ -251,22 +248,49 @@ fn branch_relpath(branch: &str) -> Option<PathBuf> {
|
||||
Some(candidate.to_path_buf())
|
||||
}
|
||||
|
||||
/// `Some(reason)` when deleting `path` would destroy work.
|
||||
/// The unsaved-work probe (#79): what would be lost if `path` were deleted.
|
||||
///
|
||||
/// Only an unclean working tree qualifies. `git worktree remove` deletes the
|
||||
/// checkout but keeps `refs/heads/<branch>`, so committed work — pushed or
|
||||
/// not — stays reachable and is never at risk. Guarding on "commits on no
|
||||
/// other ref" instead would refuse every ordinary unpushed feature branch,
|
||||
/// and because the path is deterministic that branch could then never be
|
||||
/// spawned again (#56 review).
|
||||
fn unsafe_to_remove(path: &Path) -> Result<Option<String>, WorktreeError> {
|
||||
// `--ignored=no` keeps build output out of it; untracked sources count,
|
||||
// since removal would delete them.
|
||||
let status = run_git(path, ["status", "--porcelain", "--ignored=no"])?;
|
||||
if !status.stdout.is_empty() {
|
||||
return Ok(Some("has uncommitted changes".into()));
|
||||
/// One function, three answers, shared by every caller that must make the
|
||||
/// same decision — the kill guard refuses on any non-empty result and the
|
||||
/// natural-exit classifier (#82) retains on it — so the two paths cannot
|
||||
/// drift. A worktree whose directory is already gone holds nothing to save.
|
||||
pub fn unsaved_work(path: &Path) -> Result<maestro_protocol::UnsavedWork, WorktreeError> {
|
||||
if std::fs::symlink_metadata(path).is_err() {
|
||||
return Ok(maestro_protocol::UnsavedWork::default());
|
||||
}
|
||||
Ok(None)
|
||||
// `--ignored=no` keeps build output out of it; ignored files never block
|
||||
// (#79), or no Rust checkout would ever be killable.
|
||||
let out = run_git(path, &["status", "--porcelain", "--ignored=no"])?;
|
||||
let status = String::from_utf8_lossy(&out.stdout);
|
||||
let mut dirty = 0u32;
|
||||
let mut untracked = 0u32;
|
||||
for line in status.lines() {
|
||||
if line.starts_with("??") {
|
||||
untracked += 1;
|
||||
} else {
|
||||
dirty += 1;
|
||||
}
|
||||
}
|
||||
|
||||
// Detached HEAD has no branch; the tip itself still works as a starting
|
||||
// point for the reachability query below.
|
||||
let head = run_git(path, &["rev-parse", "--abbrev-ref", "HEAD"])?;
|
||||
let branch = String::from_utf8_lossy(&head.stdout).trim().to_string();
|
||||
let tip: &str = if branch == "HEAD" { "HEAD" } else { &branch };
|
||||
let count = run_git(path, &["rev-list", "--count", tip, "--not", "--remotes"])?;
|
||||
let unpushed: u32 = String::from_utf8_lossy(&count.stdout)
|
||||
.trim()
|
||||
.parse()
|
||||
.map_err(|e| WorktreeError::NonZero {
|
||||
code: 0,
|
||||
stderr: format!("unparseable rev-list count: {e}"),
|
||||
})?;
|
||||
|
||||
Ok(maestro_protocol::UnsavedWork {
|
||||
dirty_tracked_files: dirty,
|
||||
untracked_files: untracked,
|
||||
unpushed_commits: unpushed,
|
||||
})
|
||||
}
|
||||
|
||||
/// `git -C <base_repo> worktree prune` — drops registrations whose directory
|
||||
@@ -280,10 +304,7 @@ fn prune(base_repo: &Path) -> Result<(), WorktreeError> {
|
||||
check(out)
|
||||
}
|
||||
|
||||
fn run_git(
|
||||
dir: &Path,
|
||||
args: impl IntoIterator<Item = &'static str>,
|
||||
) -> Result<Output, WorktreeError> {
|
||||
fn run_git(dir: &Path, args: &[&str]) -> Result<Output, WorktreeError> {
|
||||
let mut cmd = Command::new("git");
|
||||
cmd.arg("-C").arg(dir).args(args);
|
||||
let out = cmd.output()?;
|
||||
|
||||
@@ -93,6 +93,38 @@ fn run(cmd: &mut Command) {
|
||||
);
|
||||
}
|
||||
|
||||
/// An empty bare repository at `dir`, usable as a push target (#79).
|
||||
pub fn init_bare_repo(dir: &Path) {
|
||||
run(Command::new("git")
|
||||
.arg("init")
|
||||
.arg("-q")
|
||||
.arg("--bare")
|
||||
.arg(dir.to_str().expect("non-utf8 bare repo path")));
|
||||
}
|
||||
|
||||
/// Push every branch of `repo` to `origin`, creating remote-tracking refs.
|
||||
///
|
||||
/// The unsaved-work probe (#79) counts commits reachable from no
|
||||
/// remote-tracking ref as unpushed, so tests that want a killable worktree
|
||||
/// must give its branch one. Safe to call again: re-adding an existing
|
||||
/// remote is ignored.
|
||||
pub fn push_all_to_origin(repo: &Path, origin: &Path) {
|
||||
let _ = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo)
|
||||
.args([
|
||||
"remote",
|
||||
"add",
|
||||
"origin",
|
||||
origin.to_str().expect("non-utf8 origin path"),
|
||||
])
|
||||
.status();
|
||||
run(Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo)
|
||||
.args(["push", "-q", "--all", "origin"]));
|
||||
}
|
||||
|
||||
/// Async polling sibling of [`wait_for`] for tokio tests.
|
||||
pub async fn wait_for_async<F, Fut>(timeout: Duration, mut cond: F)
|
||||
where
|
||||
|
||||
@@ -9,7 +9,7 @@ use std::time::Duration;
|
||||
use common::DaemonFixture;
|
||||
use futures_util::{SinkExt, StreamExt};
|
||||
use maestro_daemon::local_api;
|
||||
use maestro_protocol::{CreateSessionRequest, Session};
|
||||
use maestro_protocol::{ApiError, CreateSessionRequest, Session, WorktreeSpec};
|
||||
use tokio::net::TcpListener;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio_tungstenite::tungstenite::Message as WsMessage;
|
||||
@@ -338,6 +338,53 @@ async fn delete_owned_session_kills_it() {
|
||||
assert!(!h.manager.has(session.id).await);
|
||||
}
|
||||
|
||||
/// The kill guard on the wire (#79): `DELETE /api/sessions/:id` answers 409
|
||||
/// with the structured breakdown, and the session keeps running.
|
||||
#[tokio::test]
|
||||
async fn delete_a_session_whose_worktree_holds_unsaved_work_returns_409_with_the_breakdown() {
|
||||
let h = spawn("naps-laptop").await;
|
||||
let repo = tempfile::TempDir::new().unwrap();
|
||||
common::init_repo(repo.path());
|
||||
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(
|
||||
repo.path()
|
||||
.join("worktrees")
|
||||
.join("feat")
|
||||
.join("guarded")
|
||||
.join("README"),
|
||||
b"unsaved edits\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let resp = reqwest::Client::new()
|
||||
.delete(url(h.addr, &format!("/api/sessions/{}", session.id)))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), reqwest::StatusCode::CONFLICT);
|
||||
let body: ApiError = resp.json().await.unwrap();
|
||||
assert_eq!(body.error.code, "kill_guarded");
|
||||
let breakdown = body.error.unsaved_work.expect("breakdown on the wire");
|
||||
assert_eq!(breakdown.dirty_tracked_files, 1);
|
||||
|
||||
assert!(h.manager.has(session.id).await, "the session keeps running");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attach_ws_streams_pty_bytes() {
|
||||
let h = spawn("naps-laptop").await;
|
||||
|
||||
@@ -4,10 +4,12 @@ mod common;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use common::{init_repo, wait_for_async, DaemonFixture};
|
||||
use common::{init_bare_repo, init_repo, push_all_to_origin, wait_for_async, DaemonFixture};
|
||||
use maestro_daemon::SessionError;
|
||||
use maestro_protocol::CreateSessionRequest;
|
||||
use maestro_protocol::{CatalogEvent, SessionStatus, WorktreeSpec};
|
||||
use tempfile::TempDir;
|
||||
@@ -363,8 +365,9 @@ async fn liveness_watcher_marks_dead_sessions_and_keeps_them_until_dismissed() {
|
||||
assert!(!fx.fifo_dir().join(format!("{}.fifo", s.id)).exists());
|
||||
}
|
||||
|
||||
/// Cleanup is unconditional (#56): even a spec that says "don't clean up"
|
||||
/// loses its worktree on kill — the guard, not the flag, protects work.
|
||||
/// Cleanup on kill is unconditional once the guard passes (#79): even a spec
|
||||
/// that says "don't clean up" loses its worktree — the branch was pushed, so
|
||||
/// nothing in the tree is unique and removal goes ahead.
|
||||
#[tokio::test]
|
||||
async fn kill_removes_worktree_even_when_spec_says_keep() {
|
||||
let fx = DaemonFixture::new();
|
||||
@@ -390,14 +393,21 @@ async fn kill_removes_worktree_even_when_spec_says_keep() {
|
||||
.expect("create");
|
||||
assert!(wt_path.exists());
|
||||
|
||||
// Push the branch so its commits are reachable from a remote-tracking ref.
|
||||
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");
|
||||
assert!(!wt_path.exists(), "worktree must be removed on kill");
|
||||
}
|
||||
|
||||
/// The guard (#56): a worktree holding uncommitted changes survives the kill,
|
||||
/// left on disk where the operator can find it.
|
||||
/// The kill guard (#79): a worktree holding uncommitted changes refuses the
|
||||
/// kill outright. Nothing happens — the session keeps running as the
|
||||
/// operator's handle on their unfinished work, and the tree is untouched.
|
||||
/// The refusal names what blocked it and how much of it there is.
|
||||
#[tokio::test]
|
||||
async fn kill_leaves_worktree_with_unsaved_work_on_disk() {
|
||||
async fn killing_refuses_a_worktree_with_unsaved_work_and_keeps_the_session() {
|
||||
let fx = DaemonFixture::new();
|
||||
let repo = TempDir::new().unwrap();
|
||||
init_repo(repo.path());
|
||||
@@ -420,13 +430,138 @@ async fn kill_leaves_worktree_with_unsaved_work_on_disk() {
|
||||
.await
|
||||
.expect("create");
|
||||
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
|
||||
std::fs::write(wt_path.join("notes.txt"), b"untracked\n").unwrap();
|
||||
|
||||
fx.manager.kill(s.id).await.expect("kill");
|
||||
let err = fx.manager.kill(s.id).await.expect_err("kill must refuse");
|
||||
let SessionError::KillGuarded(breakdown) = err else {
|
||||
panic!("expected KillGuarded, got {err:?}");
|
||||
};
|
||||
assert_eq!(breakdown.dirty_tracked_files, 1);
|
||||
assert_eq!(breakdown.untracked_files, 1);
|
||||
assert!(
|
||||
wt_path.exists(),
|
||||
"dirty worktree must be left on disk, not destroyed"
|
||||
breakdown.unpushed_commits >= 1,
|
||||
"a fresh branch has no remote-tracking ref"
|
||||
);
|
||||
let message = breakdown.to_string();
|
||||
assert!(message.contains("1 uncommitted file"), "{message}");
|
||||
assert!(message.contains("1 untracked file"), "{message}");
|
||||
|
||||
// Nothing happened: the session survives and so does the tree.
|
||||
assert!(fx.manager.has(s.id).await, "the session must keep running");
|
||||
assert!(wt_path.exists(), "the worktree must be untouched");
|
||||
|
||||
// A second kill after the work is saved goes through.
|
||||
run_git_in(&wt_path, &["add", "."]);
|
||||
run_git_in(&wt_path, &["commit", "-q", "-m", "saved"]);
|
||||
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");
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
}
|
||||
|
||||
/// Unpushed commits alone block the kill (#79): a feature branch almost
|
||||
/// always has them, which is why `--force` (which retains) is the ordinary
|
||||
/// way to end such a session.
|
||||
#[tokio::test]
|
||||
async fn killing_refuses_until_the_branch_is_pushed() {
|
||||
let fx = DaemonFixture::new();
|
||||
let repo = TempDir::new().unwrap();
|
||||
init_repo(repo.path());
|
||||
let wt_path = repo.path().join("worktrees").join("feat").join("pushed");
|
||||
|
||||
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/pushed".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
})
|
||||
.await
|
||||
.expect("create");
|
||||
|
||||
let err = fx.manager.kill(s.id).await.expect_err("unpushed blocks");
|
||||
let SessionError::KillGuarded(breakdown) = err else {
|
||||
panic!("expected KillGuarded, got {err:?}");
|
||||
};
|
||||
assert_eq!(breakdown.dirty_tracked_files, 0);
|
||||
assert_eq!(breakdown.untracked_files, 0);
|
||||
assert!(breakdown.unpushed_commits >= 1);
|
||||
|
||||
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("pushed branch kills");
|
||||
assert!(!wt_path.exists());
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
}
|
||||
|
||||
/// Ignored files NEVER block (#79): build output like `target/` exists in
|
||||
/// every Rust checkout, and an ignored-only worktree stays killable.
|
||||
#[tokio::test]
|
||||
async fn killing_an_ignored_only_worktree_succeeds() {
|
||||
let fx = DaemonFixture::new();
|
||||
let repo = TempDir::new().unwrap();
|
||||
init_repo(repo.path());
|
||||
// .gitignore must be committed before the worktree branches off, or it
|
||||
// would itself read as an untracked file.
|
||||
std::fs::write(repo.path().join(".gitignore"), b"/target\n").unwrap();
|
||||
run_git_in(repo.path(), &["add", ".gitignore"]);
|
||||
run_git_in(repo.path(), &["commit", "-q", "-m", "ignore target"]);
|
||||
let origin = TempDir::new().unwrap();
|
||||
init_bare_repo(origin.path());
|
||||
|
||||
let wt_path = repo.path().join("worktrees").join("feat").join("rusty");
|
||||
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/rusty".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
})
|
||||
.await
|
||||
.expect("create");
|
||||
std::fs::create_dir_all(wt_path.join("target")).unwrap();
|
||||
std::fs::write(wt_path.join("target").join("app.o"), b"binary\n").unwrap();
|
||||
|
||||
// Branch pushed; the only dirt is ignored output.
|
||||
push_all_to_origin(repo.path(), origin.path());
|
||||
|
||||
fx.manager
|
||||
.kill(s.id)
|
||||
.await
|
||||
.expect("ignored files never block");
|
||||
assert!(!wt_path.exists());
|
||||
assert!(!fx.manager.has(s.id).await);
|
||||
}
|
||||
|
||||
fn run_git_in(dir: &std::path::Path, args: &[&str]) {
|
||||
let out = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(dir)
|
||||
.args(args)
|
||||
.output()
|
||||
.expect("git spawn");
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"git {args:?} failed: {}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
assert!(fx.manager.snapshot().await.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -6,8 +6,8 @@ mod common;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
|
||||
use common::init_repo;
|
||||
use maestro_daemon::worktree::WorktreeManager;
|
||||
use common::{init_bare_repo, init_repo, push_all_to_origin};
|
||||
use maestro_daemon::worktree::{unsaved_work, WorktreeManager};
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn worktree_branches(repo: &PathBuf) -> Vec<String> {
|
||||
@@ -134,6 +134,123 @@ fn add_twice_generates_distinct_paths() {
|
||||
assert!(p2.exists());
|
||||
}
|
||||
|
||||
/// The unsaved-work probe (#79) counts uncommitted tracked changes.
|
||||
#[test]
|
||||
fn the_probe_counts_uncommitted_tracked_changes() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo = repo_dir.path().to_path_buf();
|
||||
init_repo(&repo);
|
||||
|
||||
let mgr = WorktreeManager::new(None);
|
||||
let path = mgr.add_new(&repo, "feat/dirty").expect("add");
|
||||
std::fs::write(path.join("README"), b"unsaved edits\n").unwrap();
|
||||
|
||||
let w = unsaved_work(&path).expect("probe");
|
||||
assert_eq!(w.dirty_tracked_files, 1);
|
||||
assert_eq!(w.untracked_files, 0);
|
||||
}
|
||||
|
||||
/// Untracked sources count; a staged new file is also unsaved tracked work.
|
||||
#[test]
|
||||
fn the_probe_counts_untracked_files() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo = repo_dir.path().to_path_buf();
|
||||
init_repo(&repo);
|
||||
|
||||
let mgr = WorktreeManager::new(None);
|
||||
let path = mgr.add_new(&repo, "feat/untracked").expect("add");
|
||||
std::fs::write(path.join("notes.txt"), b"wip\n").unwrap();
|
||||
std::fs::write(path.join("more.txt"), b"also wip\n").unwrap();
|
||||
|
||||
let w = unsaved_work(&path).expect("probe");
|
||||
assert_eq!(w.untracked_files, 2);
|
||||
assert_eq!(w.dirty_tracked_files, 0);
|
||||
|
||||
run_git(&path, &["add", "notes.txt"]);
|
||||
let w = unsaved_work(&path).expect("probe");
|
||||
assert_eq!(w.dirty_tracked_files, 1, "staged counts as uncommitted");
|
||||
assert_eq!(w.untracked_files, 1);
|
||||
}
|
||||
|
||||
/// Ignored files NEVER block: every Rust checkout has a `target/`, and
|
||||
/// counting ignored output would leave no session killable (#79).
|
||||
#[test]
|
||||
fn the_probe_ignores_ignored_files() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo = repo_dir.path().to_path_buf();
|
||||
init_repo(&repo);
|
||||
|
||||
// .gitignore must be committed, or it would itself read as untracked.
|
||||
std::fs::write(repo.join(".gitignore"), b"/target\n").unwrap();
|
||||
run_git(&repo, &["add", ".gitignore"]);
|
||||
run_git(&repo, &["commit", "-q", "-m", "ignore target"]);
|
||||
|
||||
let mgr = WorktreeManager::new(None);
|
||||
let path = mgr.add_new(&repo, "feat/builds").expect("add");
|
||||
std::fs::create_dir_all(path.join("target")).unwrap();
|
||||
std::fs::write(path.join("target").join("app.o"), b"binary\n").unwrap();
|
||||
|
||||
let w = unsaved_work(&path).expect("probe");
|
||||
assert_eq!(w.dirty_tracked_files, 0);
|
||||
assert_eq!(w.untracked_files, 0);
|
||||
|
||||
// With the branch pushed too, an ignored-only tree reads fully clean.
|
||||
let origin_dir = TempDir::new().unwrap();
|
||||
init_bare_repo(origin_dir.path());
|
||||
push_all_to_origin(&repo, origin_dir.path());
|
||||
let w = unsaved_work(&path).expect("probe after push");
|
||||
assert!(w.is_empty(), "ignored-only worktree must read clean: {w:?}");
|
||||
}
|
||||
|
||||
/// Commits reachable from no remote-tracking ref block; pushing the branch
|
||||
/// clears them. With no remote at all every commit is unreachable from one,
|
||||
/// which is exactly why feature branches almost always refuse (#79).
|
||||
#[test]
|
||||
fn the_probe_counts_unpushed_commits_until_the_branch_is_pushed() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo = repo_dir.path().to_path_buf();
|
||||
init_repo(&repo);
|
||||
|
||||
let mgr = WorktreeManager::new(None);
|
||||
let path = mgr.add_new(&repo, "feat/branch").expect("add");
|
||||
|
||||
let w = unsaved_work(&path).expect("probe");
|
||||
assert!(
|
||||
w.unpushed_commits >= 1,
|
||||
"a fresh branch has no remote-tracking ref: {w:?}"
|
||||
);
|
||||
|
||||
let origin_dir = TempDir::new().unwrap();
|
||||
init_bare_repo(origin_dir.path());
|
||||
push_all_to_origin(&repo, origin_dir.path());
|
||||
|
||||
let w = unsaved_work(&path).expect("probe after push");
|
||||
assert_eq!(w.unpushed_commits, 0, "{w:?}");
|
||||
assert!(w.is_empty(), "pushed and clean must be empty: {w:?}");
|
||||
}
|
||||
|
||||
/// The full breakdown names each blocker with its count.
|
||||
#[test]
|
||||
fn the_probe_reports_every_blocker_at_once() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo = repo_dir.path().to_path_buf();
|
||||
init_repo(&repo);
|
||||
|
||||
let mgr = WorktreeManager::new(None);
|
||||
let path = mgr.add_new(&repo, "feat/mixed").expect("add");
|
||||
std::fs::write(path.join("README"), b"edited\n").unwrap();
|
||||
std::fs::write(path.join("scratch.txt"), b"untracked\n").unwrap();
|
||||
|
||||
let w = unsaved_work(&path).expect("probe");
|
||||
assert_eq!(w.dirty_tracked_files, 1);
|
||||
assert_eq!(w.untracked_files, 1);
|
||||
assert!(w.unpushed_commits >= 1);
|
||||
let summary = w.to_string();
|
||||
assert!(summary.contains("1 uncommitted file"), "{summary}");
|
||||
assert!(summary.contains("1 untracked file"), "{summary}");
|
||||
assert!(summary.contains("unpushed commit"), "{summary}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_prunes_worktree() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
@@ -211,8 +328,14 @@ fn add_existing_checks_out_branch_without_creating_it() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Removal is guarded upstream now (#79): the kill refuses unsaved work
|
||||
/// before anything is torn down, so by the time `remove` runs the tree is
|
||||
/// clean. Git still refuses a dirty tree on its own — this asserts that last
|
||||
/// line of defense — and committed-but-unpushed work does not block removal,
|
||||
/// because `worktree remove` keeps `refs/heads/<branch>` and the commits stay
|
||||
/// reachable.
|
||||
#[test]
|
||||
fn remove_refuses_worktree_with_uncommitted_changes() {
|
||||
fn remove_still_relies_on_git_refusing_a_dirty_tree() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo = repo_dir.path().to_path_buf();
|
||||
init_repo(&repo);
|
||||
@@ -225,10 +348,6 @@ fn remove_refuses_worktree_with_uncommitted_changes() {
|
||||
.expect_err("dirty worktree must be refused");
|
||||
assert!(path.exists(), "guarded worktree must stay on disk");
|
||||
|
||||
// Committing it makes removal safe: `worktree remove` keeps
|
||||
// `refs/heads/feat/dirty`, so the work is still reachable. Refusing here
|
||||
// instead would make every unpushed branch a one-shot, since the path is
|
||||
// deterministic and the leftover tree blocks the next spawn (#56 review).
|
||||
run_git(&path, &["add", "."]);
|
||||
run_git(&path, &["commit", "-q", "-m", "saved"]);
|
||||
|
||||
@@ -410,7 +529,8 @@ fn a_guarded_leftover_is_never_adopted() {
|
||||
let first = mgr.add_new(&repo, "feat/resume").expect("add");
|
||||
std::fs::write(first.join("notes.txt"), b"unsaved\n").unwrap();
|
||||
|
||||
// The guard refuses, leaving it on disk.
|
||||
// Removal is refused — git itself refuses a dirty tree — leaving it on
|
||||
// disk; the daemon-side guard lives on the kill (#79).
|
||||
mgr.remove(&repo, &first)
|
||||
.expect_err("dirty tree is guarded");
|
||||
|
||||
|
||||
@@ -160,6 +160,7 @@ async fn kill_removes_clean_worktree() {
|
||||
"expected a worktree for feature/e2e under <repo>/worktrees, got: {worktrees:?}"
|
||||
);
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
|
||||
// Give the daemon a moment to remove the worktree, then verify.
|
||||
@@ -177,11 +178,12 @@ async fn kill_removes_clean_worktree() {
|
||||
);
|
||||
}
|
||||
|
||||
/// The guard (#56): killing a session whose worktree holds uncommitted
|
||||
/// changes must leave the tree on disk — cleanup is unconditional, so this
|
||||
/// guard is what keeps a stray kill from destroying work.
|
||||
/// The kill guard (#79): killing a session whose worktree holds uncommitted
|
||||
/// changes is refused outright. Nothing happens — the session keeps running
|
||||
/// as the operator's handle on their unfinished work, the tree stays on disk,
|
||||
/// and the refusal names what blocked it and how much.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn kill_spares_worktree_with_uncommitted_changes() {
|
||||
async fn kill_refuses_worktree_with_uncommitted_changes() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
@@ -209,17 +211,25 @@ async fn kill_spares_worktree_with_uncommitted_changes() {
|
||||
assert!(wt_path.exists());
|
||||
std::fs::write(wt_path.join("README"), b"unsaved edits\n").unwrap();
|
||||
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
|
||||
// The session is gone but the dirty worktree stays findable on disk.
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
while Instant::now() < deadline {
|
||||
let sessions = client.list_sessions().await.unwrap_or_default();
|
||||
if sessions.is_empty() {
|
||||
break;
|
||||
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.dirty_tracked_files, 1);
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(100)).await;
|
||||
other => panic!("expected a kill_guarded refusal, got {other:?}"),
|
||||
}
|
||||
|
||||
// The session survives and the dirty worktree stays findable on disk.
|
||||
let sessions = client.list_sessions().await.unwrap_or_default();
|
||||
assert!(
|
||||
sessions.iter().any(|x| x.id == s.id),
|
||||
"a refused kill must keep the session alive"
|
||||
);
|
||||
assert!(
|
||||
wt_path.exists(),
|
||||
"guarded worktree must be left on disk after kill"
|
||||
@@ -270,6 +280,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());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
@@ -317,6 +328,7 @@ async fn spawn_new_branch_creates_worktree_and_runs_in_it() {
|
||||
)
|
||||
.await;
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
@@ -364,6 +376,7 @@ async fn second_spawn_on_same_branch_is_rejected() {
|
||||
"original worktree must be untouched"
|
||||
);
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
client.kill_session(first.id).await.expect("kill");
|
||||
}
|
||||
|
||||
@@ -406,6 +419,7 @@ async fn spawn_from_linked_worktree_places_in_main_checkout() {
|
||||
"worktrees must never nest inside another worktree"
|
||||
);
|
||||
|
||||
push_to_throwaway_origin(base.path());
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
@@ -774,6 +788,24 @@ 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) {
|
||||
let origin = tempfile::TempDir::new().unwrap();
|
||||
run_git(
|
||||
&["init", "-q", "--bare", origin.path().to_str().unwrap()],
|
||||
repo,
|
||||
);
|
||||
let _ = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo)
|
||||
.args(["remote", "add", "origin", origin.path().to_str().unwrap()])
|
||||
.output()
|
||||
.expect("git remote add");
|
||||
run_git(&["push", "-q", "--all", "origin"], repo);
|
||||
}
|
||||
|
||||
#[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
|
||||
|
||||
@@ -143,6 +143,62 @@ pub struct SessionWorktree {
|
||||
pub base_repo: PathBuf,
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Unsaved-work breakdown (#79)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// What a worktree holds that would be lost if its directory were destroyed.
|
||||
///
|
||||
/// The kill guard refuses `DELETE /api/sessions/:id` with this breakdown, and
|
||||
/// natural exits (#82) classify with the same daemon-side probe, so the two
|
||||
/// paths cannot drift. Ignored files never count: every Rust checkout has a
|
||||
/// `target/`, and counting those would leave no session killable.
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct UnsavedWork {
|
||||
/// Tracked files with uncommitted modifications or staged changes.
|
||||
#[serde(default)]
|
||||
pub dirty_tracked_files: u32,
|
||||
/// Untracked files that are not ignored.
|
||||
#[serde(default)]
|
||||
pub untracked_files: u32,
|
||||
/// Commits on the branch reachable from no remote-tracking ref.
|
||||
#[serde(default)]
|
||||
pub unpushed_commits: u32,
|
||||
}
|
||||
|
||||
impl UnsavedWork {
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.dirty_tracked_files == 0 && self.untracked_files == 0 && self.unpushed_commits == 0
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for UnsavedWork {
|
||||
/// "3 uncommitted files, 2 untracked files, 2 unpushed commits" — zero
|
||||
/// parts are omitted; all-zero renders "no unsaved work".
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
let parts = [
|
||||
(self.dirty_tracked_files, "uncommitted file"),
|
||||
(self.untracked_files, "untracked file"),
|
||||
(self.unpushed_commits, "unpushed commit"),
|
||||
];
|
||||
let said: Vec<String> = parts
|
||||
.into_iter()
|
||||
.filter(|(n, _)| *n > 0)
|
||||
.map(|(n, unit)| {
|
||||
if n == 1 {
|
||||
format!("1 {unit}")
|
||||
} else {
|
||||
format!("{n} {unit}s")
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
if said.is_empty() {
|
||||
return write!(f, "no unsaved work");
|
||||
}
|
||||
write!(f, "{}", said.join(", "))
|
||||
}
|
||||
}
|
||||
|
||||
/// HTTP `POST /api/sessions` body.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct CreateSessionRequest {
|
||||
@@ -248,6 +304,10 @@ pub struct BranchInfo {
|
||||
pub struct ErrorInfo {
|
||||
pub code: String,
|
||||
pub message: String,
|
||||
/// Set only by the kill guard (#79): the unsaved work that blocked
|
||||
/// `DELETE /api/sessions/:id`, so a client can render counts directly.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub unsaved_work: Option<UnsavedWork>,
|
||||
}
|
||||
|
||||
/// The envelope every `/api/*` failure is wrapped in: `{"error": {…}}`.
|
||||
@@ -268,6 +328,7 @@ impl ApiError {
|
||||
error: ErrorInfo {
|
||||
code: code.into(),
|
||||
message: message.into(),
|
||||
unsaved_work: None,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -578,6 +639,7 @@ mod tests {
|
||||
ClientControl::Error(ErrorInfo {
|
||||
code: "bad_frame".into(),
|
||||
message: "decode failed".into(),
|
||||
unsaved_work: None,
|
||||
}),
|
||||
resize,
|
||||
] {
|
||||
|
||||
Reference in New Issue
Block a user