feat(M018/S04): Worktree correctness — error chains, orphan detection, cleanup_all MCP tool

crates/assay-cli/src/commands/worktree.rs

@@ -130,7 +130,7 @@ fn resolve_dirs(
     if !ad.is_dir() {
         bail!("No Assay project found. Run `assay init` first.");
     }
-    let config = assay_core::config::load(&root).map_err(|e| anyhow::anyhow!("{e}"))?;
+    let config = assay_core::config::load(&root)?;
     let worktree_dir =
         assay_core::worktree::resolve_worktree_dir(worktree_dir_override, &config, &root);
     let specs_dir = root.join(".assay").join(&config.specs_dir);
@@ -145,8 +145,7 @@ fn handle_worktree_create(
 ) -> anyhow::Result<i32> {
     let (root, worktree_dir, specs_dir) = resolve_dirs(worktree_dir_override)?;
 
-    let info = assay_core::worktree::create(&root, name, base, &worktree_dir, &specs_dir, None)
-        .map_err(|e| anyhow::anyhow!("{e}"))?;
+    let info = assay_core::worktree::create(&root, name, base, &worktree_dir, &specs_dir, None)?;
 
     if json {
         let output = serde_json::to_string_pretty(&info)?;
@@ -170,7 +169,7 @@ fn handle_worktree_create(
 fn handle_worktree_list(json: bool) -> anyhow::Result<i32> {
     let (root, _worktree_dir, _specs_dir) = resolve_dirs(None)?;
 
-    let result = assay_core::worktree::list(&root).map_err(|e| anyhow::anyhow!("{e}"))?;
+    let result = assay_core::worktree::list(&root)?;
     for warning in &result.warnings {
         tracing::warn!("{warning}");
     }
@@ -238,8 +237,9 @@ fn handle_worktree_list(json: bool) -> anyhow::Result<i32> {
         // ANSI overhead for colored spec column
         let extra = if color { super::ANSI_COLOR_OVERHEAD } else { 0 };
 
+        let orphan_marker = if entry.is_orphan { " [orphan]" } else { "" };
         println!(
-            "  {:<sw$}  {:<bw$}  {}",
+            "  {:<sw$}  {:<bw$}  {}{orphan_marker}",
             spec_display,
             entry.branch,
             entry.path.display(),
@@ -259,8 +259,7 @@ fn handle_worktree_status(
     let (_root, worktree_dir, _specs_dir) = resolve_dirs(worktree_dir_override)?;
     let worktree_path = worktree_dir.join(name);
 
-    let st =
-        assay_core::worktree::status(&worktree_path, name).map_err(|e| anyhow::anyhow!("{e}"))?;
+    let st = assay_core::worktree::status(&worktree_path, name)?;
 
     if json {
         let output = serde_json::to_string_pretty(&st)?;
@@ -324,7 +323,7 @@ fn handle_worktree_cleanup(
         let is_dirty = match assay_core::worktree::status(&worktree_path, spec_slug) {
             Ok(s) => s.dirty,
             Err(assay_core::error::AssayError::WorktreeNotFound { .. }) => false,
-            Err(e) => return Err(anyhow::anyhow!("{e}")),
+            Err(e) => return Err(e.into()),
         };
 
         if is_dirty {
@@ -349,8 +348,7 @@ fn handle_worktree_cleanup(
         true
     };
 
-    assay_core::worktree::cleanup(&root, &worktree_path, spec_slug, effective_force)
-        .map_err(|e| anyhow::anyhow!("{e}"))?;
+    assay_core::worktree::cleanup(&root, &worktree_path, spec_slug, effective_force)?;
 
     if json {
         let output = serde_json::json!({"removed": spec_slug});
@@ -367,7 +365,7 @@ fn handle_worktree_cleanup_all(
     force: bool,
     json: bool,
 ) -> anyhow::Result<i32> {
-    let result = assay_core::worktree::list(root).map_err(|e| anyhow::anyhow!("{e}"))?;
+    let result = assay_core::worktree::list(root)?;
     for warning in &result.warnings {
         tracing::warn!("{warning}");
     }

crates/assay-core/src/worktree.rs

@@ -360,6 +360,7 @@ pub fn create(
         path: worktree_path,
         branch: branch_name,
         base_branch: Some(base),
+        is_orphan: false,
     })
 }
 
@@ -388,11 +389,36 @@ pub fn list(project_root: &Path) -> Result<WorktreeListResult> {
                 path: wt.path,
                 branch: branch.to_string(),
                 base_branch,
+                is_orphan: false,
             })
         })
         .collect();
 
     entries.sort_by(|a, b| a.spec_slug.cmp(&b.spec_slug));
+
+    // Cross-reference with sessions to populate is_orphan.
+    // Reuses the same logic as detect_orphans() but inline to avoid recursion.
+    let assay_dir = project_root.join(".assay");
+    for entry in &mut entries {
+        let metadata = read_metadata(&entry.path);
+        entry.is_orphan = match metadata.and_then(|m| m.session_id) {
+            None => true, // No session_id — orphaned
+            Some(sid) => match crate::work_session::load_session(&assay_dir, &sid) {
+                Ok(session) => session.phase.is_terminal(), // Terminal phase — orphaned
+                Err(AssayError::WorkSessionNotFound { .. }) => true, // Session file missing — orphaned
+                Err(e) => {
+                    // I/O or parse error — conservatively not orphaned; warn so operator can investigate
+                    tracing::warn!(
+                        spec_slug = %entry.spec_slug,
+                        error = %e,
+                        "worktree list: could not load session to determine orphan status, assuming active"
+                    );
+                    false
+                }
+            },
+        };
+    }
+
     Ok(WorktreeListResult { entries, warnings })
 }
 
@@ -538,26 +564,14 @@ pub fn cleanup(
 /// - Its `session_id` points to a session in a terminal phase (Completed or Abandoned)
 ///
 /// Worktrees with an active (non-terminal) session are NOT orphaned.
-pub fn detect_orphans(project_root: &Path, assay_dir: &Path) -> Result<Vec<WorktreeInfo>> {
+pub fn detect_orphans(project_root: &Path, _assay_dir: &Path) -> Result<Vec<WorktreeInfo>> {
     let list_result = list(project_root)?;
-    let mut orphans = Vec::new();
-
-    for entry in list_result.entries {
-        let metadata = read_metadata(&entry.path);
-        let is_orphan = match metadata.and_then(|m| m.session_id) {
-            None => true, // No session_id — orphaned
-            Some(sid) => match crate::work_session::load_session(assay_dir, &sid) {
-                Err(_) => true, // Session doesn't exist on disk — orphaned
-                Ok(session) => session.phase.is_terminal(), // Terminal phase — orphaned
-            },
-        };
-
-        if is_orphan {
-            orphans.push(entry);
-        }
-    }
-
-    Ok(orphans)
+    // list() already populates is_orphan on every entry via session cross-reference.
+    Ok(list_result
+        .entries
+        .into_iter()
+        .filter(|e| e.is_orphan)
+        .collect())
 }
 
 /// Detect if the current working directory is inside a linked worktree.
@@ -1552,4 +1566,109 @@ cmd = "echo ok"
         result_mut.warnings.push("test warning".to_string());
         assert_eq!(result_mut.warnings.len(), 1);
     }
+
+    #[test]
+    fn test_list_marks_orphan_entries() {
+        let (_tmp, _wt_tmp, root, specs_dir) = setup_repo();
+        let worktree_base = _wt_tmp.path().join("worktrees");
+
+        // Create worktree with no session_id — should be orphaned
+        create(
+            &root,
+            "auth-flow",
+            Some("main"),
+            &worktree_base,
+            &specs_dir,
+            None,
+        )
+        .expect("create failed");
+
+        let result = list(&root).expect("list failed");
+        assert_eq!(result.entries.len(), 1);
+        assert!(
+            result.entries[0].is_orphan,
+            "worktree with no session_id should be marked as orphan"
+        );
+    }
+
+    #[test]
+    fn test_list_marks_non_orphan_entries() {
+        let (_tmp, _wt_tmp, root, specs_dir) = setup_repo();
+        let worktree_base = _wt_tmp.path().join("worktrees");
+        let assay_dir = root.join(".assay");
+
+        // Create an active session
+        let session = crate::work_session::start_session(
+            &assay_dir,
+            "auth-flow",
+            worktree_base.join("auth-flow"),
+            "claude",
+            None,
+        )
+        .expect("start_session failed");
+
+        // Create worktree linked to the active session
+        create(
+            &root,
+            "auth-flow",
+            Some("main"),
+            &worktree_base,
+            &specs_dir,
+            Some(&session.id),
+        )
+        .expect("create failed");
+
+        let result = list(&root).expect("list failed");
+        assert_eq!(result.entries.len(), 1);
+        assert!(
+            !result.entries[0].is_orphan,
+            "worktree with active session should NOT be marked as orphan"
+        );
+    }
+
+    #[test]
+    fn test_list_marks_terminal_session_as_orphan() {
+        let (_tmp, _wt_tmp, root, specs_dir) = setup_repo();
+        let worktree_base = _wt_tmp.path().join("worktrees");
+        let assay_dir = root.join(".assay");
+
+        // Create a session and complete it (terminal phase).
+        // complete_session requires prior record_gate_result to advance state.
+        let session = crate::work_session::start_session(
+            &assay_dir,
+            "auth-flow",
+            worktree_base.join("auth-flow"),
+            "claude",
+            None,
+        )
+        .expect("start_session failed");
+        crate::work_session::record_gate_result(
+            &assay_dir,
+            &session.id,
+            "run-001",
+            "gate_eval",
+            None,
+        )
+        .expect("record_gate_result failed");
+        crate::work_session::complete_session(&assay_dir, &session.id, None)
+            .expect("complete_session failed");
+
+        // Create worktree linked to the completed (terminal) session
+        create(
+            &root,
+            "auth-flow",
+            Some("main"),
+            &worktree_base,
+            &specs_dir,
+            Some(&session.id),
+        )
+        .expect("create failed");
+
+        let result = list(&root).expect("list failed");
+        assert_eq!(result.entries.len(), 1);
+        assert!(
+            result.entries[0].is_orphan,
+            "worktree linked to a terminal session should be marked as orphan"
+        );
+    }
 }