Compare commits

..
Author SHA1 Message Date
Miguel Palhas a575b8bef1 feat(maestro): refuse a kill when the worktree holds unsaved work (#79)
The guard moves off the worktree and onto the kill. DELETE
/api/sessions/:id now runs the shared unsaved_work probe before tearing
anything down and answers 409 kill_guarded with the breakdown on the
wire, so a client can say "3 uncommitted files, 2 unpushed commits".
The session keeps running and the tree is untouched.

WorktreeManager::remove is unconditional now — git itself still refuses
a dirty tree as a last line of defense.
2026-08-24 11:03:43 +01:00
Miguel Palhas 898c3cc763 feat(maestro): shared unsaved-work probe for worktrees (#79)
One function, unsaved_work(), returns the structured breakdown —
uncommitted tracked changes, untracked non-ignored files, unpushed
commits — so the kill guard and the natural-exit classifier (#82)
answer from the same place. Ignored files never count: every Rust
checkout has a target/, and counting those would leave no session
killable.

WorktreeManager::remove drops its own dirty-tree guard; git itself
still refuses a dirty tree, and the real guard moves onto the kill.
2026-08-24 10:59:08 +01:00
Miguel Palhas 50a9d35f62 merge: never adopt, sidestep collisions with a suffix
Closes #77
2026-08-24 10:41:17 +01:00
Miguel Palhas a9ef3ff97b merge: retained-worktree store
Closes #78
2026-08-24 10:39:08 +01:00
Miguel Palhas 0457a61a61 refactor(maestro): sidestep worktree path collisions with a numeric suffix
Anything occupying <repo>/worktrees/<branch> — one of ours, one the
operator made, an unrelated directory — is now bypassed by appending
-2, -3, ... to the directory; the branch stays as requested. The same
suffix applies under the [daemon].worktree_root override.

The daemon no longer asks whether an occupant is "ours": delete
registered_for(), the adoption branch in run_add and the
SessionManager::create live-session check, plus WorktreeError::PathExists.
A branch already checked out elsewhere is git's refusal to surface,
already marked by the branch picker's is_head/worktree flags.
2026-08-24 10:38:32 +01:00
Miguel Palhas 132b606830 feat(maestro): retained-worktree store in state_dir (#78)
Per-entry JSON files under <state_dir>/retained-worktrees/, written like
sidecar records: atomic tmp+rename, missing/malformed files tolerated on
read. Entries carry path, base repo, branch, reason (force-killed,
exited-with-unsaved-work), timestamp; they leave on reclaim or when
prune_missing notices the directory was removed by hand.
2026-08-24 10:35:26 +01:00
Miguel Palhas 35c75f9238 docs(maestro): write the worktree lifecycle contract
Closes #89
2026-08-24 10:29:42 +01:00
Miguel Palhas bbbc24a4aa fix(maestro): fifth-review findings, incl. the guarded-worktree dead end
The cleanup guard plus deterministic paths had created a trap: one
untracked file an agent left behind — a log, a .env, a stray artifact —
made the guard refuse removal, the directory stayed, and every later
spawn of that branch hit PathExists. The branch was burned permanently
and the product offered no way to clear it. A leftover tree registered
for the same branch is now adopted, so the operator lands back in their
own unsaved work. A tree a *live* session holds is still rejected, since
two sessions must not share a checkout.

Also:

- Fast-forward still awaited /api/info, /api/projects and /api/names
  inline, the exact freeze the draft loaders were restructured to avoid.
  It posts to the same channel now.
- Draft updates were routed by tab index, which shifts when a tab closes:
  an in-flight load could be dropped or land on the wrong pane. Drafts
  carry an id and are found by it.
- extend_names left the name it had just displayed at the head of the
  queue, so the next reroll produced it again and the key looked dead.
- The fast-forward fallback discarded info.home_dir, so its draft showed
  unabbreviated paths and seeded "Other directory…" with / — disagreeing
  with the normal draft entry point.
- Web fast-forward mounted via openSession, which replaces the focused
  pane: pressing it while attached evicted that session from the layout.
  It reuses an empty pane or splits, as the TUI does.
- branchEdited was inferred from the seeded branch value, but in "new"
  mode that value is the auto-synced session name — so any form rebuild
  froze the branch against later rerolls.
- resolveBaseRepo ignored the worktree checkbox, rebuilding the fieldset
  expanded under an unticked box and probing gitroot for a worktree the
  operator had turned off.
- short_hash used DefaultHasher, whose algorithm is not stable across
  Rust releases, in a directory name — a toolchain bump would orphan
  every worktree under the override root. Now FNV-1a.
2026-08-24 00:28:50 +01:00
Miguel Palhas e9368bb242 fix(maestro): fourth-review findings across daemon, tui and web
The existing-branch picker could never succeed in either client: both
treated the main checkout's HEAD as available, and since that is usually
`main` it became the default pick — git then refuses the second checkout.
A TUI test had baked the bug in by asserting the selection lands on the
`is_head` entry. Both pickers now treat "checked out anywhere" as held.

Worktree paths were still not injective. Flattening `/` and `.` to `-`
made `feat/x`, `feat.x` and `feat-x` share one directory, and the
override root keyed on the repo basename alone, so `~/a/maestro` and
`~/b/maestro` collided. A branch name is already a valid relative path —
git stores it as refs/heads/feat/example — so it is used verbatim and
nests under `worktrees/feat/`; the override root gains a digest of the
full repo path. Git forbids holding both `feat` and `feat/x`, so the
nesting cannot collide for valid refs.

Also:

- Recovered sessions dropped their worktree association, so after a
  restart a kill cleaned up nothing and the leftover directory blocked
  that branch forever. It is persisted on Session now.
- The nesting guard compared git's resolved paths against an unresolved
  one, so it silently passed on any symlinked repo path — and that guard
  is what stands between a kill and an inner tree's uncommitted work.
- Draft loads ran on the TUI event loop: /api/projects on a cold cache
  walks all of $HOME, freezing keys, frames and attached panes for up to
  the request timeout. They are detached tasks posting to a channel now.
- Fast-forward in the web client created the session and never mounted
  it, so `N` looked like a dead key while the TUI attached.
- The fast-forward fallback seed forced the worktree toggle off, against
  both a normal draft and the TUI.
- The projects background refresh honoured the TTL, so two of every three
  ticks were no-ops and requests paid for the rescan instead.
2026-08-24 00:10:35 +01:00
Miguel Palhas 918609574e merge: fast-forward spawn in both clients (#67) 2026-08-23 23:45:18 +01:00
29 changed files with 1708 additions and 290 deletions
+4
View File
@@ -62,6 +62,10 @@ nix/ # Home Manager module (`services.maestro`)
See `docs/INSTALL.md` for end-user install instructions.
`docs/WORKTREES.md` is the contract for session worktrees — placement,
collisions, what blocks a kill, what `--force` retains, and what reclaim
touches. Read it before changing anything that creates or removes one.
`nix run .#dev` (or `just dev`) brings up the whole local stack under
process-compose: a debug daemon on 127.0.0.1:7071, then Vite on :5173 once the
daemon answers `/api/info`. The SPA proxies `/api/*` back to the daemon, so only
+1
View File
@@ -153,6 +153,7 @@ mod tests {
command: command.into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: None,
status: SessionStatus::Running,
created_at: Utc.with_ymd_and_hms(2026, 5, 25, 12, 0, 0).unwrap(),
foreground: None,
+5
View File
@@ -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
+1
View File
@@ -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,
}
}
+1
View File
@@ -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;
+8 -5
View File
@@ -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()
}
}
}
+13 -5
View File
@@ -262,10 +262,16 @@ impl ProjectScanner {
/// Cached snapshot, rescanning (off-thread) when the TTL has lapsed.
pub async fn repos(&self) -> std::sync::Arc<Vec<DiscoveredRepo>> {
self.repos_inner(false).await
}
async fn repos_inner(&self, force: bool) -> std::sync::Arc<Vec<DiscoveredRepo>> {
let mut cache = self.cache.lock().await;
if let Some((at, repos)) = cache.as_ref() {
if at.elapsed() < self.ttl {
return repos.clone();
if !force {
if let Some((at, repos)) = cache.as_ref() {
if at.elapsed() < self.ttl {
return repos.clone();
}
}
}
let roots = self.roots.clone();
@@ -281,9 +287,11 @@ impl ProjectScanner {
repos
}
/// Rescan now, ignoring the TTL. Called by the background timer.
/// Rescan now, ignoring the TTL. Called by the background timer — going
/// through `repos()` would hit the cache and make most ticks no-ops,
/// leaving requests to pay for the rescan instead (#54 review).
async fn refresh(&self) {
self.repos().await;
self.repos_inner(true).await;
}
/// Spawn the slow background refresh task. Drop the returned handle to
+319
View File
@@ -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;
}
}
+63 -12
View File
@@ -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,
}
}
}
@@ -226,6 +237,9 @@ impl SessionManager {
let WorktreeSpec {
base_repo, branch, ..
} = spec;
// A path collision is sidestepped inside `run_add` (#77), and
// a branch already checked out elsewhere is git's refusal,
// not ours.
let path = if spec.create_branch {
self.inner.worktree_mgr.add_new(base_repo, branch)
} else {
@@ -290,6 +304,12 @@ impl SessionManager {
command: req.command,
env: req.env,
metadata: req.metadata,
worktree: worktree_path.as_ref().zip(worktree_base_repo.as_ref()).map(
|(path, base_repo)| maestro_protocol::SessionWorktree {
path: path.clone(),
base_repo: base_repo.clone(),
},
),
status: SessionStatus::Running,
created_at: Utc::now(),
// A freshly-spawned session is producing its initial output; the
@@ -361,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(())
}
@@ -430,6 +478,7 @@ impl SessionManager {
command: String::new(),
env: Default::default(),
metadata: Default::default(),
worktree: None,
status: SessionStatus::Running,
created_at: Utc::now(),
// Activity is unknown on adoption; the watcher fills it in.
@@ -457,8 +506,11 @@ impl SessionManager {
SessionRecord {
session: session.clone(),
fifo_path,
worktree_path: None,
worktree_base_repo: None,
// Restored from the sidecar: without it the recovered
// session cleans up nothing on kill, and the leftover
// 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()),
bridge,
},
);
@@ -707,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!(
+132 -62
View File
@@ -10,11 +10,9 @@ pub enum WorktreeError {
Spawn(#[from] std::io::Error),
#[error("git exited with code {code}: {stderr}")]
NonZero { code: i32, stderr: String },
/// A worktree already occupies the target path. The caller decides what
/// to do; the daemon never relocates to a different directory (#56).
#[error("worktree path already exists: {}", .path.display())]
PathExists { path: PathBuf },
/// 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 },
}
@@ -39,9 +37,9 @@ impl WorktreeManager {
}
/// Where a worktree for `branch` will be created: the override root, or
/// `<main checkout>/worktrees/<sanitized branch>` by default. The path is
/// predictable (no UUID suffix), so an existing directory at the target
/// is an error rather than a silently different location.
/// `<main checkout>/worktrees/<sanitized branch>` by default. If the path
/// is occupied, [`Self::add_new`] / [`Self::add_existing`] sidestep to
/// `<path>-2`, `<path>-3`, ... instead.
///
/// `base_repo` is whatever the client picked, which the project list makes
/// easy to point at a *linked worktree* — placing the new tree inside it
@@ -51,23 +49,36 @@ impl WorktreeManager {
/// uncommitted work (#54 review). Always place against the main checkout.
///
/// The override root gets the repo's basename too: without it two repos
/// spawning the same branch name target one directory and the second
/// fails with `PathExists`.
/// spawning the same branch name target one directory.
fn target_path(&self, base_repo: &Path, branch: &str) -> PathBuf {
let slug = sanitize_branch_for_path(branch);
// An unusable branch name falls back to a digest rather than escaping
// the root; `run_add` surfaces git's own error for it anyway.
let slug = branch_relpath(branch).unwrap_or_else(|| PathBuf::from(short_hash(branch)));
let main = main_checkout(base_repo);
match &self.root_override {
Some(root) => {
let repo = main
// Basename alone collides for two checkouts of the same name
// (`~/a/maestro` and `~/b/maestro`), so disambiguate with a
// digest of the full path (#54 review).
let base = main
.file_name()
.map(|n| n.to_string_lossy().into_owned())
.unwrap_or_else(|| "repo".into());
let repo = format!("{base}-{}", short_hash(&main.to_string_lossy()));
root.join(repo).join(slug)
}
None => main.join("worktrees").join(slug),
}
}
/// [`Self::target_path`] with `-n` appended to the final component, the
/// collision suffix (#77).
fn target_path_suffixed(&self, base_repo: &Path, branch: &str, n: u32) -> PathBuf {
let mut name = self.target_path(base_repo, branch).into_os_string();
name.push(format!("-{n}"));
PathBuf::from(name)
}
/// `git -C <base_repo> worktree add -b <branch> <path>` — creates the
/// branch. Returns the path of the new worktree on success.
pub fn add_new(&self, base_repo: &Path, branch: &str) -> Result<PathBuf, WorktreeError> {
@@ -86,12 +97,18 @@ impl WorktreeManager {
branch: &str,
create_branch: bool,
) -> Result<PathBuf, WorktreeError> {
let path = self.target_path(base_repo, branch);
// Clear registrations whose directory is gone before testing the
// path, so a previously-leaked entry doesn't block this branch.
// Clear registrations whose directory is gone first, so a
// previously-leaked entry doesn't push this branch to a suffix.
prune(base_repo)?;
if path.symlink_metadata().is_ok() {
return Err(WorktreeError::PathExists { path });
// Anything occupying the target — a tree we left behind, one the
// operator made, an unrelated directory — is sidestepped with a
// numeric suffix on the directory; the branch stays whatever was
// requested (#77). The daemon never adopts what it finds.
let mut path = self.target_path(base_repo, branch);
let mut n = 1;
while path.symlink_metadata().is_ok() {
n += 1;
path = self.target_path_suffixed(base_repo, branch, n);
}
// The parent (`<repo>/worktrees` or the override root) may not exist
// yet; the target itself must not.
@@ -111,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
@@ -132,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)
@@ -180,44 +192,105 @@ fn nested_worktree(path: &Path) -> Option<PathBuf> {
.args(["worktree", "list", "--porcelain"])
.output()
.ok()?;
// `worktree list` prints fully-resolved paths while ours comes from
// `--git-common-dir` and may still hold symlinks. Comparing the two forms
// directly makes the guard silently pass on any symlinked repo path — and
// this guard is what stands between a kill and deleting an inner
// worktree's uncommitted work (#54 review).
let needle = path.canonicalize().unwrap_or_else(|_| path.to_path_buf());
let text = String::from_utf8_lossy(&out.stdout);
text.lines()
.filter_map(|l| l.strip_prefix("worktree "))
.map(PathBuf::from)
.find(|p| p != path && p.starts_with(path))
.find(|p| {
let real = p.canonicalize().unwrap_or_else(|_| p.clone());
real != needle && real.starts_with(&needle)
})
}
fn sanitize_branch_for_path(branch: &str) -> String {
let mut out = String::with_capacity(branch.len());
for c in branch.chars() {
if c.is_ascii_alphanumeric() || c == '-' || c == '_' {
out.push(c);
} else if c == '/' || c == '.' {
out.push('-');
/// Short digest used to disambiguate override roots.
///
/// Hand-rolled FNV-1a rather than `DefaultHasher`: this value becomes a
/// directory name, and `DefaultHasher`'s algorithm is explicitly not
/// guaranteed stable across Rust releases — a toolchain bump would remap every
/// repo to a new directory and orphan the worktrees under the old one
/// (#67 review).
fn short_hash(s: &str) -> String {
let mut h: u32 = 0x811c_9dc5;
for b in s.as_bytes() {
h ^= u32::from(*b);
h = h.wrapping_mul(0x0100_0193);
}
format!("{h:08x}")
}
/// The relative directory a branch's worktree lives in.
///
/// A git branch name is already a valid relative path — git stores it as
/// `.git/refs/heads/feat/example` — so it is used verbatim and `feat/example`
/// nests under `worktrees/feat/`. That keeps the path readable *and* keeps the
/// mapping injective: flattening `/` and `.` to `-` made `feat/x`, `feat.x`
/// and `feat-x` share one directory (#54 review).
///
/// Git already forbids holding both `feat` and `feat/x`, so the directory
/// collision that nesting could cause cannot arise from valid refs. Only
/// traversal and absolute components are rejected.
fn branch_relpath(branch: &str) -> Option<PathBuf> {
use std::path::Component;
let candidate = Path::new(branch);
if branch.is_empty()
|| !candidate
.components()
.all(|c| matches!(c, Component::Normal(_)))
{
return None;
}
Some(candidate.to_path_buf())
}
/// The unsaved-work probe (#79): what would be lost if `path` were deleted.
///
/// 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());
}
// `--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;
}
}
if out.is_empty() {
out.push_str("wt");
}
out
}
/// `Some(reason)` when deleting `path` would destroy work.
///
/// 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()));
}
Ok(None)
// 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
@@ -231,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()?;
+32
View File
@@ -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
+48 -1
View File
@@ -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;
+147 -12
View File
@@ -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,14 +365,15 @@ 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();
let repo = TempDir::new().unwrap();
init_repo(repo.path());
let wt_path = repo.path().join("worktrees").join("feat-keep");
let wt_path = repo.path().join("worktrees").join("feat").join("keep");
let s = fx
.manager
@@ -390,18 +393,25 @@ 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());
let wt_path = repo.path().join("worktrees").join("feat-guarded");
let wt_path = repo.path().join("worktrees").join("feat").join("guarded");
let s = fx
.manager
@@ -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]
+1
View File
@@ -19,6 +19,7 @@ fn sample(id: Uuid) -> Session {
command: "bash".into(),
env: HashMap::from([("FOO".into(), "bar".into())]),
metadata: HashMap::from([("name".into(), Value::String("maestro-impl".into()))]),
worktree: None,
status: SessionStatus::Running,
created_at: DateTime::parse_from_rfc3339("2026-05-24T12:00:00Z")
.unwrap()
+232 -26
View File
@@ -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> {
@@ -53,7 +53,7 @@ fn add_places_worktree_inside_the_base_repo_by_default() {
let mgr = WorktreeManager::new(None);
let path = mgr.add_new(&repo, "feat/example").expect("add worktree");
assert_eq!(path, repo.join("worktrees").join("feat-example"));
assert_eq!(path, repo.join("worktrees").join("feat").join("example"));
assert!(path.exists());
assert!(path.join("README").exists());
@@ -74,31 +74,47 @@ fn worktree_root_override_redirects_placement() {
// repos spawning the same branch name don't target one directory.
let repo_name = repo.file_name().unwrap();
let path = mgr.add_new(&repo, "feat/example").expect("add worktree");
assert_eq!(path, wt_root.path().join(repo_name).join("feat-example"));
// The override root disambiguates same-named checkouts with a digest of
// the full repo path, and the branch keeps its own shape.
// <root>/<repo basename>-<digest>/feat/example
let prefix = format!("{}-", repo_name.to_string_lossy());
let repo_dir_name = path
.strip_prefix(wt_root.path())
.unwrap()
.components()
.next()
.unwrap();
assert!(
repo_dir_name
.as_os_str()
.to_string_lossy()
.starts_with(&prefix),
"{path:?}"
);
assert!(path.ends_with("feat/example"), "{path:?}");
assert!(path.exists());
}
/// Anything at all occupying the target path — ours, the operator's, or an
/// unrelated directory — is sidestepped, not adopted and not reported (#77).
#[test]
fn existing_directory_at_target_path_is_an_error_carrying_the_path() {
fn an_occupied_target_path_gets_a_numeric_suffix() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
// A predictable path means collisions are possible; they must be
// reported, never silently relocated.
std::fs::create_dir_all(repo.join("worktrees").join("feat-collide")).unwrap();
// A non-git directory sits exactly where the worktree would go.
let squatter = repo.join("worktrees").join("feat").join("collide");
std::fs::create_dir_all(&squatter).unwrap();
std::fs::write(squatter.join("operator-notes.txt"), b"mine\n").unwrap();
let mgr = WorktreeManager::new(None);
let err = mgr.add_new(&repo, "feat/collide").unwrap_err();
let msg = format!("{err}");
let path = mgr.add_new(&repo, "feat/collide").expect("sidesteps");
assert_eq!(path, repo.join("worktrees").join("feat").join("collide-2"));
assert!(path.exists());
assert!(
msg.contains(
repo.join("worktrees")
.join("feat-collide")
.to_str()
.unwrap()
),
"{msg}"
squatter.join("operator-notes.txt").exists(),
"the occupant is untouched"
);
}
@@ -118,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();
@@ -195,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);
@@ -209,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"]);
@@ -312,10 +447,9 @@ fn placement_resolves_a_linked_worktree_to_the_main_checkout() {
!inner.starts_with(&outer),
"must not nest: {inner:?} inside {outer:?}"
);
assert_eq!(
inner.parent().and_then(|p| p.parent()),
Some(repo.canonicalize().unwrap().as_path()),
"placed against the main checkout"
assert!(
inner.starts_with(repo.canonicalize().unwrap().join("worktrees")),
"placed against the main checkout: {inner:?}"
);
}
@@ -363,3 +497,75 @@ fn override_root_keeps_repos_apart() {
let b = mgr.add_new(b_dir.path(), "feat/shared").expect("repo b");
assert_ne!(a, b, "same branch name in two repos must not collide");
}
/// Flattening `/` and `.` to `-` made these three share one directory, so the
/// losers could never get a worktree (#54 review).
#[test]
fn similar_branch_names_get_distinct_directories() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
let mgr = WorktreeManager::new(None);
let a = mgr.add_new(&repo, "feat/x").expect("feat/x");
let b = mgr.add_new(&repo, "feat.x").expect("feat.x");
let c = mgr.add_new(&repo, "feat-x").expect("feat-x");
assert_ne!(a, b);
assert_ne!(b, c);
assert_ne!(a, c);
assert!(a.ends_with("feat/x"), "{a:?}");
}
/// A guarded leftover stays on disk untouched, and is never adopted (#77).
/// Re-spawning its branch is refused by git itself — the branch is still
/// checked out inside the leftover — which the caller surfaces.
#[test]
fn a_guarded_leftover_is_never_adopted() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
let mgr = WorktreeManager::new(None);
let first = mgr.add_new(&repo, "feat/resume").expect("add");
std::fs::write(first.join("notes.txt"), b"unsaved\n").unwrap();
// 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");
let err = mgr
.add_existing(&repo, "feat/resume")
.expect_err("a branch already checked out cannot be spawned twice");
assert!(format!("{err}").contains("git exited"), "{err}");
assert!(
first.join("notes.txt").exists(),
"the leftover's unsaved work is untouched"
);
}
/// The `worktree_root` override gets the same collision suffix (#77).
#[test]
fn the_override_root_suffixes_collisions_too() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
let wt_root = TempDir::new().unwrap();
let mgr = WorktreeManager::new(Some(wt_root.path().to_path_buf()));
// First add settles `<root>/<repo basename>-<digest>/` so the squatter
// can be created at the exact target.
mgr.add_new(&repo, "feat/first").expect("add first");
let repo_dir_in_root = std::fs::read_dir(wt_root.path())
.unwrap()
.next()
.unwrap()
.unwrap()
.path();
let squatter = repo_dir_in_root.join("feat").join("collide");
std::fs::create_dir_all(&squatter).unwrap();
let path = mgr.add_new(&repo, "feat/collide").expect("sidesteps");
assert_eq!(path, squatter.with_file_name("collide-2"), "{path:?}");
}
+62 -19
View File
@@ -156,10 +156,11 @@ async fn kill_removes_clean_worktree() {
assert!(
worktrees
.iter()
.any(|w| w.contains("worktrees/feature-e2e")),
.any(|w| w.contains("worktrees/feature/e2e")),
"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,16 +178,21 @@ 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();
init_repo(base.path());
let wt_path = base.path().join("worktrees").join("feature-unsaved");
let wt_path = base
.path()
.join("worktrees")
.join("feature")
.join("unsaved");
let req = CreateSessionRequest {
cwd: base.path().to_path_buf(),
@@ -205,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"
@@ -266,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");
}
@@ -278,7 +293,7 @@ async fn spawn_new_branch_creates_worktree_and_runs_in_it() {
let base = TempDir::new().unwrap();
init_repo(base.path());
let wt_path = base.path().join("worktrees").join("feature-fresh");
let wt_path = base.path().join("worktrees").join("feature").join("fresh");
let s = client
.spawn_session(&worktree_req(base.path(), "feature/fresh", true))
@@ -313,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");
}
@@ -343,16 +359,24 @@ async fn second_spawn_on_same_branch_is_rejected() {
}
// Exactly one registration, one directory, still usable.
// Count branch records only: the worktree path now contains the branch
// name too (`worktrees/feature/dup`), so a substring match over every line
// would count the same tree twice.
let holders = git_worktree_list(base.path())
.iter()
.filter(|w| w.contains("feature/dup"))
.filter(|w| w.as_str() == "branch refs/heads/feature/dup")
.count();
assert_eq!(holders, 1, "duplicate spawn must not register another tree");
assert!(
base.path().join("worktrees").join("feature-dup").is_dir(),
base.path()
.join("worktrees")
.join("feature")
.join("dup")
.is_dir(),
"original worktree must be untouched"
);
push_to_throwaway_origin(base.path());
client.kill_session(first.id).await.expect("kill");
}
@@ -384,7 +408,7 @@ async fn spawn_from_linked_worktree_places_in_main_checkout() {
.await
.expect("spawn from linked worktree");
let want = base.path().join("worktrees").join("feature-nested");
let want = base.path().join("worktrees").join("feature").join("nested");
assert_eq!(
s.cwd, want,
"worktree must be placed under the main checkout"
@@ -395,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");
}
@@ -763,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
+79
View File
@@ -53,6 +53,14 @@ pub struct Session {
pub env: HashMap<String, String>,
#[serde(default)]
pub metadata: HashMap<String, Value>,
/// The worktree this session owns, when it was spawned with one.
///
/// Persisted rather than kept in memory only: after a daemon restart the
/// recovered session must still know what to clean up, and with
/// deterministic worktree paths a forgotten association leaves a directory
/// that blocks that branch from ever being spawned again (#54 review).
#[serde(default, skip_serializing_if = "Option::is_none")]
pub worktree: Option<SessionWorktree>,
pub status: SessionStatus,
pub created_at: DateTime<Utc>,
/// Current foreground command in the session's pane (e.g. `claude`,
@@ -127,6 +135,70 @@ fn default_true() -> bool {
true
}
/// The worktree a running session owns, carried on [`Session`] so it survives
/// a daemon restart.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct SessionWorktree {
pub path: PathBuf,
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 {
@@ -232,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": {…}}`.
@@ -252,6 +328,7 @@ impl ApiError {
error: ErrorInfo {
code: code.into(),
message: message.into(),
unsaved_work: None,
},
}
}
@@ -410,6 +487,7 @@ mod tests {
command: "claude".into(),
env: HashMap::from([("FOO".into(), "bar".into())]),
metadata: HashMap::from([("name".into(), Value::String("maestro-impl".into()))]),
worktree: None,
status: SessionStatus::Running,
created_at: DateTime::parse_from_rfc3339("2026-05-24T12:00:00Z")
.unwrap()
@@ -561,6 +639,7 @@ mod tests {
ClientControl::Error(ErrorInfo {
code: "bad_frame".into(),
message: "decode failed".into(),
unsaved_work: None,
}),
resize,
] {
+1
View File
@@ -36,6 +36,7 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session {
command: command.into(),
env: HashMap::new(),
metadata: HashMap::new(),
worktree: None,
status: SessionStatus::Running,
created_at: Utc::now(),
foreground: None,
+312 -128
View File
@@ -55,6 +55,14 @@ pub struct App {
help: HelpOverlay,
chord: ChordState,
daemon: DaemonClient,
/// Results of draft data loads, delivered back to the event loop.
///
/// These are network calls — `/api/projects` on a cold cache walks all of
/// `$HOME` — and awaiting them inside `perform` froze the whole TUI: no
/// keys, no frames, attached panes stalled, for up to the request timeout
/// (#67 review). They run as detached tasks and post here instead.
draft_tx: tokio::sync::mpsc::UnboundedSender<DraftUpdate>,
draft_rx: tokio::sync::mpsc::UnboundedReceiver<DraftUpdate>,
/// Direct HTTP for the one endpoint [`DaemonClient`] doesn't cover yet:
/// `GET /api/names` (#64).
http: reqwest::Client,
@@ -105,6 +113,7 @@ pub enum Effect {
impl App {
pub fn new(cfg: TuiConfig, catalog: CatalogStore, daemon: DaemonClient) -> Self {
let (draft_tx, draft_rx) = tokio::sync::mpsc::unbounded_channel();
let chord_timeout = cfg.chord_timeout;
let help = HelpOverlay {
lines: render_help_lines(&cfg),
@@ -123,6 +132,8 @@ impl App {
chord: ChordState::new(chord_timeout),
daemon,
http,
draft_tx,
draft_rx,
region: RegionMap::default(),
last_sent_size: HashMap::new(),
clicks: ClickTracker::default(),
@@ -144,6 +155,9 @@ impl App {
return Ok(());
}
}
Some(update) = self.draft_rx.recv() => {
self.apply_draft_update(update).await;
}
_ = self.catalog.changed() => {
// Render on next loop iteration.
}
@@ -204,13 +218,13 @@ impl App {
Effect::Attach(id) => self.attach(id),
Effect::Spawn(req) => self.submit_spawn(req).await,
Effect::Kill(id) => self.submit_kill(id).await,
Effect::LoadDraft(tab_idx) => self.load_draft_data(tab_idx).await,
Effect::FastForward => self.fast_forward_spawn().await,
Effect::RefillNames(tab_idx) => self.refill_names(tab_idx).await,
Effect::LoadDraft(tab_idx) => self.spawn_load_draft(tab_idx),
Effect::FastForward => self.spawn_fast_forward(),
Effect::RefillNames(tab_idx) => self.spawn_refill_names(tab_idx),
Effect::ProbeGitRoot { tab, seq, path } => {
self.probe_git_root(tab, seq, path).await
self.spawn_probe_git_root(tab, seq, path)
}
Effect::LoadBranches(tab) => self.load_branches(tab).await,
Effect::LoadBranches(tab) => self.spawn_load_branches(tab),
Effect::Forward { id, bytes } => {
if let Some(handle) = self.emulators.get(id) {
let _ = handle.tx.send(SessionInput::Bytes(bytes)).await;
@@ -381,38 +395,136 @@ impl App {
effects
}
/// One `/api/names` batch. [`DaemonClient`] has no typed call for this
/// route yet, so it goes through plain reqwest against the same base URL
/// and query contract (`routes::NAMES` + `routes::COUNT_QUERY`).
async fn fetch_names(&self, count: usize) -> Vec<String> {
let url = format!(
"{}{}",
maestro_protocol::routes::normalize_base(self.daemon.base()),
routes::NAMES
);
match self
.http
.get(url)
.query(&[(routes::COUNT_QUERY, count)])
.send()
.await
{
Ok(resp) => resp
.json::<NameBatch>()
.await
.map(|batch| batch.names)
.unwrap_or_default(),
Err(_) => Vec::new(),
}
}
/// Populate a freshly opened draft: presets from `/api/info` (falling back
/// to the built-ins already in place), projects from `/api/projects`, and
/// a first name batch. Applies only if that tab still shows the draft —
/// a slow response must never clobber what replaced it.
async fn load_draft_data(&mut self, tab_idx: usize) {
let (info, projects) = tokio::join!(self.daemon.daemon_info(), self.daemon.list_projects());
let names = self.fetch_names(crate::draft::NAME_BATCH_SIZE).await;
/// Kick off the draft's initial data load without blocking the loop.
fn spawn_load_draft(&self, tab: usize) {
let Some(draft) = self.draft_id(tab) else {
return;
};
let (daemon, http, base, tx) = self.loader_handles();
tokio::spawn(async move {
let (info, projects) = tokio::join!(daemon.daemon_info(), daemon.list_projects());
let names = fetch_names(&http, &base, crate::draft::NAME_BATCH_SIZE).await;
let _ = tx.send(DraftUpdate::Data {
draft,
info: info.ok(),
projects: projects.ok(),
names,
});
});
}
fn spawn_refill_names(&self, tab: usize) {
let Some(draft) = self.draft_id(tab) else {
return;
};
let (_, http, base, tx) = self.loader_handles();
tokio::spawn(async move {
let names = fetch_names(&http, &base, crate::draft::NAME_BATCH_SIZE).await;
let _ = tx.send(DraftUpdate::Names { draft, names });
});
}
fn spawn_probe_git_root(&self, tab: usize, seq: u32, path: PathBuf) {
let Some(draft) = self.draft_id(tab) else {
return;
};
let (daemon, _, _, tx) = self.loader_handles();
tokio::spawn(async move {
let root = daemon
.git_root(&path.to_string_lossy())
.await
.ok()
.and_then(|g| g.root);
let _ = tx.send(DraftUpdate::GitRoot { draft, seq, root });
});
}
fn spawn_load_branches(&self, tab: usize) {
let Some(draft) = self.draft_id(tab) else {
return;
};
let Some(cwd) = self
.state
.tabs
.get(tab)
.and_then(|t| t.pane.as_ref())
.and_then(|p| match p {
PaneRef::Draft(d) => d.cwd(),
_ => None,
})
else {
return;
};
let (_, http, base, tx) = self.loader_handles();
tokio::spawn(async move {
let branches = fetch_branches(&http, &base, &cwd).await;
let _ = tx.send(DraftUpdate::Branches {
draft,
cwd,
branches,
});
});
}
/// The draft pane living in `tab`, by identity.
fn draft_id(&self, tab: usize) -> Option<u64> {
match self.state.tabs.get(tab).and_then(|t| t.pane.as_ref()) {
Some(PaneRef::Draft(d)) => Some(d.id),
_ => None,
}
}
fn loader_handles(
&self,
) -> (
DaemonClient,
reqwest::Client,
String,
tokio::sync::mpsc::UnboundedSender<DraftUpdate>,
) {
(
self.daemon.clone(),
self.http.clone(),
self.daemon.base().to_string(),
self.draft_tx.clone(),
)
}
/// Fold a finished background load into the draft that asked for it.
async fn apply_draft_update(&mut self, update: DraftUpdate) {
if let DraftUpdate::FastForward {
info,
projects,
names,
} = update
{
self.fast_forward_spawn(info, projects, names).await;
return;
}
let want = match &update {
DraftUpdate::Data { draft, .. }
| DraftUpdate::Names { draft, .. }
| DraftUpdate::GitRoot { draft, .. }
| DraftUpdate::Branches { draft, .. } => *draft,
// Handled above; it targets no particular draft.
DraftUpdate::FastForward { .. } => unreachable!(),
};
// Find it by identity: closing a tab shifts every index to its right,
// so an in-flight load addressed by index would be dropped or land on
// whatever pane inherited the slot (#67 review).
let Some(tab_idx) = self
.state
.tabs
.iter()
.position(|t| matches!(&t.pane, Some(PaneRef::Draft(d)) if d.id == want))
else {
// The draft went away while the request was in flight.
return;
};
let Some(PaneRef::Draft(draft)) = self
.state
.tabs
@@ -421,17 +533,43 @@ impl App {
else {
return;
};
if let Ok(info) = info {
draft.home_dir.clone_from(&info.home_dir);
if !info.presets.is_empty() {
draft.set_presets(info.presets);
let mut follow_up_branches = false;
match update {
DraftUpdate::Data {
info,
projects,
names,
..
} => {
if let Some(info) = info {
draft.home_dir.clone_from(&info.home_dir);
if !info.presets.is_empty() {
draft.set_presets(info.presets);
}
}
if let Some(projects) = projects {
draft.set_projects(projects, Utc::now());
}
if !names.is_empty() {
draft.seed_names(names);
}
}
DraftUpdate::Names { names, .. } => {
if !names.is_empty() {
draft.extend_names(names);
}
}
DraftUpdate::GitRoot { seq, root, .. } => {
draft.resolve_gitroot(seq, root);
follow_up_branches = draft.take_branch_load().is_some();
}
DraftUpdate::Branches { cwd, branches, .. } => {
draft.set_branches(&cwd, branches);
}
DraftUpdate::FastForward { .. } => unreachable!(),
}
if let Ok(projects) = projects {
draft.set_projects(projects, Utc::now());
}
if !names.is_empty() {
draft.seed_names(names);
if follow_up_branches {
self.spawn_load_branches(tab_idx);
}
}
@@ -441,11 +579,38 @@ impl App {
/// branch named after that name. Anything unresolvable — no projects
/// known yet, no command, no name — downgrades to a draft pane seeded
/// with whatever did resolve.
async fn fast_forward_spawn(&mut self) {
let (info, projects) = tokio::join!(self.daemon.daemon_info(), self.daemon.list_projects());
let names = self.fetch_names(crate::draft::NAME_BATCH_SIZE).await;
/// Load what the fast path needs without blocking the loop — the same
/// reason the draft loaders were moved off it (#67 review): a cold
/// `/api/projects` walks all of `$HOME`, and pressing the key must not
/// freeze keys, frames and attached panes until it returns.
fn spawn_fast_forward(&self) {
let (daemon, http, base, tx) = self.loader_handles();
tokio::spawn(async move {
let (info, projects) = tokio::join!(daemon.daemon_info(), daemon.list_projects());
let names = fetch_names(&http, &base, crate::draft::NAME_BATCH_SIZE).await;
let _ = tx.send(DraftUpdate::FastForward {
info: info.ok(),
projects: projects.ok(),
names,
});
});
}
async fn fast_forward_spawn(
&mut self,
info: Option<maestro_protocol::DaemonInfo>,
projects: Option<Vec<maestro_protocol::Project>>,
names: Vec<String>,
) {
// Keep the home dir: without it the pre-filled draft renders
// unabbreviated paths and "Other directory…" seeds `/` instead of
// home, so the two draft entry points disagreed (#67 review).
let home_dir = info
.as_ref()
.map(|i| i.home_dir.clone())
.unwrap_or_default();
let presets = match info {
Ok(info) if !info.presets.is_empty() => info.presets,
Some(info) if !info.presets.is_empty() => info.presets,
_ => Vec::new(),
};
let discovered = projects.unwrap_or_default();
@@ -465,6 +630,7 @@ impl App {
let Some(PaneRef::Draft(draft)) = self.state.active_tab_mut().pane.as_mut() else {
return;
};
draft.home_dir = home_dir;
if !presets.is_empty() {
draft.set_presets(presets);
}
@@ -475,88 +641,6 @@ impl App {
draft.seed_names(names);
}
/// Fetch a fresh name batch after a reroll drained the local queue.
async fn refill_names(&mut self, tab_idx: usize) {
let names = self.fetch_names(crate::draft::NAME_BATCH_SIZE).await;
let Some(PaneRef::Draft(draft)) = self
.state
.tabs
.get_mut(tab_idx)
.and_then(|t| t.pane.as_mut())
else {
return;
};
if !names.is_empty() {
draft.extend_names(names);
}
}
/// Resolve the draft target's enclosing repository via `/api/gitroot`
/// (#65). A miss unticks the worktree toggle inside `resolve_gitroot`;
/// a hit may schedule a branch load (Existing mode), served inline.
async fn probe_git_root(&mut self, tab_idx: usize, seq: u32, path: PathBuf) {
let root = self
.daemon
.git_root(&path.to_string_lossy())
.await
.ok()
.and_then(|g| g.root);
let Some(PaneRef::Draft(draft)) = self
.state
.tabs
.get_mut(tab_idx)
.and_then(|t| t.pane.as_mut())
else {
return;
};
draft.resolve_gitroot(seq, root);
if draft.take_branch_load().is_some() {
self.load_branches(tab_idx).await;
}
}
/// One `/api/branches` batch for the draft's current cwd (#65). Same
/// reqwest route as [`Self::fetch_names`]: no typed client call exists.
async fn load_branches(&mut self, tab_idx: usize) {
let Some(path) = self
.state
.tabs
.get(tab_idx)
.and_then(|t| t.pane.as_ref())
.and_then(|p| match p {
PaneRef::Draft(d) => d.cwd(),
_ => None,
})
else {
return;
};
let url = format!(
"{}{}",
maestro_protocol::routes::normalize_base(self.daemon.base()),
routes::BRANCHES
);
let resp = self
.http
.get(url)
.query(&[(routes::PATH_QUERY, path.to_string_lossy().as_ref())])
.send()
.await;
// On failure the picker stays empty; "new" mode still works.
let branches = match resp {
Ok(resp) => resp.json::<Vec<BranchInfo>>().await.unwrap_or_default(),
Err(_) => return,
};
let Some(PaneRef::Draft(draft)) = self
.state
.tabs
.get_mut(tab_idx)
.and_then(|t| t.pane.as_mut())
else {
return;
};
draft.set_branches(&path, branches);
}
async fn submit_spawn(&mut self, req: CreateSessionRequest) {
match self.daemon.create_session(&req).await {
Ok(session) => {
@@ -744,6 +828,82 @@ fn render_help_lines(cfg: &TuiConfig) -> Vec<String> {
lines
}
/// A completed background load, applied on the event loop.
#[derive(Debug)]
enum DraftUpdate {
Data {
draft: u64,
info: Option<maestro_protocol::DaemonInfo>,
projects: Option<Vec<maestro_protocol::Project>>,
names: Vec<String>,
},
Names {
draft: u64,
names: Vec<String>,
},
GitRoot {
draft: u64,
seq: u32,
root: Option<PathBuf>,
},
Branches {
draft: u64,
cwd: PathBuf,
branches: Vec<BranchInfo>,
},
/// A fast-forward's data arrived; resolution and spawn happen on the loop.
FastForward {
info: Option<maestro_protocol::DaemonInfo>,
projects: Option<Vec<maestro_protocol::Project>>,
names: Vec<String>,
},
}
/// `GET /api/names?count=N`. Free function: the spawned loaders have no `self`.
async fn fetch_names(http: &reqwest::Client, base: &str, count: usize) -> Vec<String> {
let url = format!(
"{}{}",
maestro_protocol::routes::normalize_base(base),
routes::NAMES
);
match http
.get(url)
.query(&[(routes::COUNT_QUERY, count)])
.send()
.await
{
Ok(resp) => resp
.json::<NameBatch>()
.await
.map(|batch| batch.names)
.unwrap_or_default(),
Err(_) => Vec::new(),
}
}
/// `GET /api/branches?path=`. On failure the picker stays empty and "new
/// branch" mode still works.
async fn fetch_branches(
http: &reqwest::Client,
base: &str,
path: &std::path::Path,
) -> Vec<BranchInfo> {
let url = format!(
"{}{}",
maestro_protocol::routes::normalize_base(base),
routes::BRANCHES
);
match http
.get(url)
.query(&[(routes::PATH_QUERY, path.to_string_lossy().as_ref())])
.send()
.await
{
Ok(resp) => resp.json::<Vec<BranchInfo>>().await.unwrap_or_default(),
Err(_) => Vec::new(),
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -830,6 +990,7 @@ mod tests {
cwd: std::path::PathBuf::from("/"),
env: Default::default(),
metadata: Default::default(),
worktree: None,
status: SessionStatus::Running,
created_at: Utc::now(),
foreground: None,
@@ -957,7 +1118,14 @@ mod tests {
let mut app = make_app_at(&base);
let effects = app.dispatch(Action::FastForwardSpawn);
// The fast path loads off the event loop now; the run loop drains the
// channel, so do that by hand.
app.perform(effects).await;
let update = tokio::time::timeout(Duration::from_secs(5), app.draft_rx.recv())
.await
.expect("load completed")
.expect("channel open");
app.apply_draft_update(update).await;
let posts = posted.lock().unwrap();
assert_eq!(posts.len(), 1, "one immediate POST, no form shown");
@@ -1001,7 +1169,14 @@ mod tests {
let mut app = make_app_at(&base);
let effects = app.dispatch(Action::FastForwardSpawn);
// The fast path loads off the event loop now; the run loop drains the
// channel, so do that by hand.
app.perform(effects).await;
let update = tokio::time::timeout(Duration::from_secs(5), app.draft_rx.recv())
.await
.expect("load completed")
.expect("channel open");
app.apply_draft_update(update).await;
assert!(
matches!(app.state.active_tab().pane, Some(PaneRef::Draft(_))),
@@ -1220,7 +1395,15 @@ mod tests {
let mut app = make_app_at(&base);
let effects = app.dispatch(Action::NewDraftPane);
assert!(matches!(effects.as_slice(), [Effect::LoadDraft(0)]));
// `perform` only spawns the load — it must not block the event loop on
// the network (#67 review). The result arrives on the draft channel,
// which the run loop drains; do that by hand here.
app.perform(effects).await;
let update = tokio::time::timeout(Duration::from_secs(5), app.draft_rx.recv())
.await
.expect("load completed")
.expect("channel open");
app.apply_draft_update(update).await;
let PaneRef::Draft(draft) = app.state.active_tab().pane.as_ref().unwrap() else {
panic!("expected a draft pane");
@@ -1313,6 +1496,7 @@ mod tests {
cwd: std::path::PathBuf::from("/"),
env: Default::default(),
metadata: Default::default(),
worktree: None,
status: SessionStatus::Running,
created_at: Utc::now(),
foreground: None,
+1
View File
@@ -188,6 +188,7 @@ mod tests {
cwd: std::path::PathBuf::from("/tmp"),
env: Default::default(),
metadata: Default::default(),
worktree: None,
status: SessionStatus::Running,
created_at: Utc::now(),
foreground: None,
+35 -10
View File
@@ -114,6 +114,11 @@ pub enum DraftInputResult {
#[derive(Debug, Clone, PartialEq)]
pub struct DraftPane {
/// Identity for routing async load results back to *this* draft.
///
/// Tab indices shift when a tab closes, so an in-flight load addressed by
/// index could be dropped or land on the wrong pane (#67 review).
pub id: u64,
/// Includes the implicit custom tile (empty `command`) last.
pub presets: Vec<SpawnPreset>,
pub selected_preset: usize,
@@ -164,6 +169,9 @@ pub struct DraftPane {
pub home_dir: String,
}
/// Source of [`DraftPane::id`].
static NEXT_DRAFT_ID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
impl Default for DraftPane {
fn default() -> Self {
// Built-ins until `/api/info` answers — the form must be usable even
@@ -171,6 +179,7 @@ impl Default for DraftPane {
let mut presets = builtin_presets();
presets.push(custom_preset());
Self {
id: NEXT_DRAFT_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
presets,
selected_preset: 0,
custom_command: crate::text_input::TextInput::default(),
@@ -256,10 +265,16 @@ impl DraftPane {
/// Append a refill batch; the first name replaces the field (this only
/// happens after an explicit reroll on an empty batch).
pub fn extend_names(&mut self, names: Vec<String>) {
if let Some(first) = names.first() {
self.set_name(first.clone());
///
/// The shown name is *consumed*, not left at the head — queueing it too
/// made the next reroll produce the same name and the key look dead
/// (#67 review).
pub fn extend_names(&mut self, mut names: Vec<String>) {
if names.is_empty() {
return;
}
let first = names.remove(0);
self.set_name(first);
self.name_queue.extend(names);
}
@@ -382,7 +397,7 @@ impl DraftPane {
self.branches_for = Some(cwd.to_path_buf());
self.branches = branches;
self.selected_branch = (0..self.branches.len())
.find(|&i| self.branches[i].worktree.is_none())
.find(|&i| Self::branch_is_available(&self.branches[i]))
.unwrap_or(0);
}
@@ -424,8 +439,17 @@ impl DraftPane {
self.schedule_branch_load();
}
/// A branch is pickable only when nothing has it checked out. That means
/// no linked worktree *and* not the main checkout's HEAD — git refuses a
/// second checkout either way, and HEAD is commonly `main`, so treating it
/// as available made it the default pick and every spawn failed with
/// "already used by worktree" (#54 review).
fn branch_is_available(b: &BranchInfo) -> bool {
b.worktree.is_none() && !b.is_head
}
fn available_branch(&self, i: usize) -> bool {
self.branches.get(i).is_some_and(|b| b.worktree.is_none())
self.branches.get(i).is_some_and(Self::branch_is_available)
}
/// Move the existing-branch highlight, skipping branches another worktree
@@ -445,11 +469,11 @@ impl DraftPane {
self.selected_branch = available[((cur as i32) + delta).rem_euclid(len) as usize];
}
/// The highlighted branch, only when no other worktree holds it.
/// The highlighted branch, only when nothing has it checked out.
pub fn picked_branch(&self) -> Option<&BranchInfo> {
self.branches
.get(self.selected_branch)
.filter(|b| b.worktree.is_none())
.filter(|b| Self::branch_is_available(b))
}
pub fn is_custom(&self) -> bool {
@@ -1781,15 +1805,16 @@ mod tests {
],
);
assert_eq!(
draft.branches[draft.selected_branch].name, "main",
"selection lands on an available branch"
draft.branches[draft.selected_branch].name, "feature",
"selection skips `main`: the main checkout has it, so git would \
refuse a second checkout"
);
draft.focused_field = DraftField::BranchExisting;
draft.on_input(&InputKey::Down);
assert_eq!(
draft.branches[draft.selected_branch].name, "feature",
"moving skips branches another worktree holds"
"the only pickable branch stays selected"
);
let wt = draft.build_request().unwrap().worktree.unwrap();
+2
View File
@@ -92,6 +92,7 @@ mod tests {
cwd: std::path::PathBuf::from("/tmp"),
env: Default::default(),
metadata,
worktree: None,
status: SessionStatus::Running,
created_at: Utc::now(),
foreground: None,
@@ -179,6 +180,7 @@ mod tests {
cwd: std::path::PathBuf::from("/tmp"),
env: Default::default(),
metadata: HashMap::new(),
worktree: None,
status: SessionStatus::Running,
created_at: Utc::now(),
foreground: None,
+127
View File
@@ -0,0 +1,127 @@
# Worktrees
Maestro can create a git worktree for a session and is responsible for it from
that moment until the worktree is removed. This document is the contract for
that lifetime: where a worktree goes, what ends a session, what survives, and
what the operator has to do by hand.
It exists because the rules here are not guessable from the code. Every rule
below was decided once, re-litigated in review, and decided the same way again.
The reasoning is written down alongside each rule so the next reader does not
have to reconstruct it.
## Nothing is ever adopted
The daemon only ever removes a worktree it created in this session's lifetime or
recorded in its retained store. It never takes ownership of a directory it finds
on disk, whatever that directory looks like.
There is no reliable way to ask "is this worktree mine?". A git worktree carries
no marker distinguishing one a daemon made from one a human made ten minutes
earlier, and any heuristic that answers the question will eventually answer it
wrong — in the direction of deleting somebody's work.
So the daemon does not ask. If the target path is occupied by anything at all, it
picks a different path.
## Placement
A worktree for branch `<branch>` of repository `<repo>` goes to
`<repo>/worktrees/<branch>`, where `<repo>` is the main checkout — not whichever
worktree the request happened to come from.
The branch name is used verbatim as a relative path, so `feature/login` becomes
`worktrees/feature/login`. A branch name that cannot be a relative path falls
back to a short hash of the branch.
If that path exists, the daemon appends `-2`, then `-3`, and so on until it finds
a free one. The suffix names the **directory only**. The branch is whatever was
requested; a second worktree of the same branch is not what happens here, because
git refuses to check the same branch out twice and the branch picker already
marks branches that are checked out elsewhere.
### The `worktree_root` override
`[daemon].worktree_root` moves every worktree out of the repository and under one
root. The per-repository directory is the repository's basename plus a short
digest of its full path — the digest is what keeps two repositories with the same
basename apart. Inside it the layout and the collision suffix are the same.
## What ends a session
`DELETE /api/sessions/:id` is **refused** when the session's worktree holds
unsaved work, meaning any of:
- uncommitted changes to tracked files
- untracked files that are not ignored
- commits on the branch reachable from no remote-tracking ref
Ignored files never block. Every Rust worktree has a `target/`; if ignored files
counted, no session would ever be killable.
The refusal is a structured error naming what blocked it and how much of it there
is, so a client can say "3 uncommitted files, 2 unpushed commits". The session
keeps running and the worktree is untouched.
The point of refusing rather than cleaning up is that the session stays as the
operator's handle on their own unfinished work. A session that is killed while
its worktree still matters leaves a directory nobody is looking at.
A consequence, accepted deliberately: a feature branch almost always has unpushed
commits, so `--force` is the ordinary way to end a session, not an exception.
That is only safe because force retains.
### `--force`
`--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.
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.
### Exiting on its own
A session that ends by itself — the command finished, the operator typed `exit` —
never reaches the kill path. When the watcher notices the tmux session is gone it
applies the same rule: a pristine worktree is removed, anything else is retained
with reason `exited-with-unsaved-work`.
Nobody is waiting for an answer here, so there is nothing to refuse; the choice is
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 retained store
A `state_dir`-backed record of worktrees the daemon created that no longer have a
session. For each one: the path, the base repository, the branch, why it was
retained, and when.
It survives a daemon restart. It is the recovery path for everything above — a
store that is lost on restart is worse than no store, because the worktrees are
still on disk with nothing pointing at them.
Entries leave when the worktree is reclaimed, and when the daemon notices the
directory is gone because somebody removed it by hand.
**It is not an ownership record.** It must never be consulted to decide whether a
path may be adopted — see the first rule. Its only job is to remember what the
daemon is holding so that it can be listed and reclaimed.
## Reclaim
`GET /api/worktrees` lists the store: path, base repository, branch, reason, when,
and what is currently in the way. The in-the-way breakdown is recomputed on read,
not stored, so a tree whose work has since been pushed says so.
`DELETE /api/worktrees/<id>` reclaims one. It removes the directory and drops the
store entry. It does **not** consult the guard — reclaim is the deliberate
destructive act, and the operator chose it while looking at what was in the way.
It never deletes the branch. `git worktree remove` leaves the ref, and reclaiming
a tree must not throw away commits on it. This is worth saying out loud in every
client, because "reclaim" otherwise sounds like it takes the commits too.
Nothing is reclaimed automatically. There is no age-based expiry and no bulk
reclaim.
+5 -2
View File
@@ -85,9 +85,12 @@ describe("resolveFastForward (#67)", () => {
});
});
it("seeds worktree off when not even a project resolved", () => {
it("leaves the worktree default alone when not even a project resolved", () => {
// Not `false`: the form defaults the toggle on and a false would stick,
// so the fallback draft would disagree with a normally-opened draft and
// with the TUI's fast path.
const seed = fastForwardSeed(resolveFastForward([], presets));
expect(seed.worktreeEnabled).toBe(false);
expect(seed.worktreeEnabled).toBeUndefined();
expect(seed.cwd).toBe("");
});
});
+5 -1
View File
@@ -55,7 +55,11 @@ export function fastForwardSeed(res: FastForwardResolution) {
cwd: res.project?.path ?? "",
command: res.command ?? "",
name: res.name ?? "",
worktreeEnabled: Boolean(res.project),
// Left undefined rather than false when nothing resolved: the form
// defaults the toggle on, and `false` would stick, so the fallback draft
// would disagree with both a normally-opened draft and the TUI's fast path
// (#67 review).
worktreeEnabled: res.project ? true : undefined,
worktreeBranch:
res.project && res.name ? res.name : undefined,
};
+6 -1
View File
@@ -303,7 +303,12 @@ async function requestFastForward(): Promise<void> {
const req = buildFastForwardRequest(resolved);
if (req) {
try {
await createSession(req);
const session = await createSession(req);
// Mount it, the way the TUI's fast path attaches. Without this the
// keystroke creates a session and leaves the UI untouched, so it reads
// as a dead key (#67 review).
mv.openSessionBeside(session.id);
updateShellRoute();
return;
} catch {
// A failed POST degrades to the same pre-filled draft as an
+35
View File
@@ -49,6 +49,7 @@ export interface MultiView {
/** Bring a session into the layout: focus it if already open, else fill the
* focused pane (or open the first pane in an empty tab). */
openSession: (sessionId: string) => void;
openSessionBeside: (sessionId: string) => void;
/** Drop the pane showing `sessionId` (the session was disconnected/killed). */
closeSession: (sessionId: string) => void;
/** Every session currently shown in some pane, across all tabs. */
@@ -378,6 +379,39 @@ export function createMultiView(opts: MultiViewOptions): MultiView {
render();
}
/// Mount a session without evicting one.
///
/// `openSession` deliberately replaces the focused pane (VS Code style) for
/// a sidebar click, but the fast path must never do that: pressing the key
/// while attached would drop the running session out of the layout
/// entirely. Follows `openDraft`'s rule instead — reuse an empty pane, else
/// split beside the focused one (#67 review).
function openSessionBeside(sessionId: string): void {
const loc = locateSession(layout, sessionId);
if (loc) {
layout.activeTabId = loc.tab.id;
loc.tab.focusedLeafId = loc.leaf.id;
render();
return;
}
const tab = activeTab(layout);
if (!tab.root || !tab.focusedLeafId || !findLeaf(tab.root, tab.focusedLeafId)) {
tab.focusedLeafId = splitLeaf(tab, "", sessionId, "row", true);
render();
opts.onOpenSessionsChange?.();
return;
}
const target = findLeaf(tab.root, tab.focusedLeafId)!;
if (!target.sessionId) {
target.sessionId = sessionId;
target.draft = null;
} else {
tab.focusedLeafId = splitLeaf(tab, target.id, sessionId, "row", true);
}
render();
opts.onOpenSessionsChange?.();
}
function openDraft(seed?: Partial<Draft>): void {
const tab = activeTab(layout);
const draft = (): Draft => ({ command: "", cwd: "", name: "", ...seed });
@@ -693,6 +727,7 @@ export function createMultiView(opts: MultiViewOptions): MultiView {
return {
el,
openSession,
openSessionBeside,
openDraft,
closeSession,
openSessions: () => openSessions(layout),
+1
View File
@@ -9,6 +9,7 @@ const base: SpawnFormValues = {
worktreeEnabled: false,
worktreeBranch: "",
worktreeBranchMode: "new",
worktreeBranchEdited: false,
};
describe("buildSpawnRequest", () => {
+30 -6
View File
@@ -25,6 +25,10 @@ export interface SpawnFormSeed {
name?: string;
worktreeEnabled?: boolean;
worktreeBranch?: string;
/** Whether the operator typed the branch themselves, rather than it having
* been auto-synced from the session name. Carried separately because the
* value alone cannot distinguish the two. */
worktreeBranchEdited?: boolean;
worktreeBranchMode?: WorktreeBranchMode;
cwdRawVisible?: boolean;
}
@@ -37,6 +41,7 @@ export interface SpawnFormValues {
name: string;
worktreeEnabled: boolean;
worktreeBranch: string;
worktreeBranchEdited: boolean;
/** "new" creates the branch; "existing" checks out a picked one (#61). */
worktreeBranchMode: WorktreeBranchMode;
/** Whether the cwd lives in the raw text input (vs a picked recent row). */
@@ -230,6 +235,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
name: nameInput.value,
worktreeEnabled: wtEnabled.checked,
worktreeBranch: branchValue(),
worktreeBranchEdited: branchEdited,
worktreeBranchMode: branchMode,
cwdRawVisible: !cwdRawWrap.hidden,
});
@@ -239,7 +245,11 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
// The worktree branch defaults to the session name — including a clone's
// prefilled name — and tracks it on reroll/edit until the user types their
// own branch. Only in "new" mode: an existing-branch pick is not a name.
let branchEdited = Boolean(prefill.worktreeBranch);
// True only once the operator types their own branch. It cannot be inferred
// from the seed: in "new" mode `emit()` publishes the auto-synced session
// name, so seeding from it froze the branch after any form rebuild
// (#67 review). `worktreeBranchEdited` is carried explicitly instead.
let branchEdited = Boolean(prefill.worktreeBranchEdited);
const syncBranchToName = () => {
if (!branchEdited && branchMode === "new") branchInput.value = nameInput.value;
};
@@ -540,6 +550,16 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
/** Decide how the worktree section describes the current target. Called
* once the initial load has settled `cwd`. */
function resolveBaseRepo(): void {
// Nothing to resolve when the operator turned the worktree off — the
// section would otherwise rebuild expanded under an unchecked box, and
// fire a `/api/gitroot` probe for a worktree nobody asked for
// (#67 review).
if (!wtEnabled.checked) {
wtBody.hidden = true;
wtHint.hidden = true;
inferredBaseRepo = null;
return;
}
if (!cwd) {
inferredBaseRepo = null;
wtBody.hidden = true;
@@ -634,11 +654,14 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
return;
}
for (const b of branches) {
// A branch another worktree holds stays visible but unpickable — git
// would refuse to check it out twice; the path says who holds it.
const o = opt(b.worktree ? `${b.name} — held by ${b.worktree}` : b.name, b.name);
o.disabled = Boolean(b.worktree);
o.title = b.worktree ?? "";
// A branch that is already checked out anywhere stays visible but
// unpickable — git refuses a second checkout. That includes the main
// checkout's own HEAD, which is commonly `main` and would otherwise be
// the default pick, making every existing-branch spawn fail (#54 review).
const held = b.worktree ?? (b.is_head ? "the main checkout" : null);
const o = opt(held ? `${b.name} — held by ${held}` : b.name, b.name);
o.disabled = held !== null;
o.title = held ?? "";
branchSelect.appendChild(o);
}
}
@@ -685,6 +708,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
name: nameInput.value,
worktreeEnabled: wtEnabled.checked,
worktreeBranch: branchValue(),
worktreeBranchEdited: branchEdited,
worktreeBranchMode: branchMode,
},
inferredBaseRepo,