feat(M018/S04): Worktree correctness — error chains, orphan detection, cleanup_all MCP tool by wollax · Pull Request #213 · wollax/assay

wollax · 2026-04-01T18:40:15Z

S04: Worktree correctness

Closes M018/S04. Fixes four worktree pain points for parallel runs.

What's in this PR

T01 — CLI error chain preservation (R104)

Replaced 7 .map_err(|e| anyhow::anyhow!("{e}")) with ?/.into() in assay-cli/src/commands/worktree.rs
Full AssayError source chain now surfaces in CLI error output

T01 — WorktreeInfo.is_orphan field (R105)

Added #[serde(default)] pub is_orphan: bool to WorktreeInfo
worktree::list() cross-references metadata session_id against work_session::load_session() inline to populate orphan status
Inline implementation avoids recursion with detect_orphans() (which calls list())
Schema snapshot updated

T02 — CLI orphan display + unit tests (R105)

[orphan] marker shown in non-JSON worktree list output
2 new integration tests: test_list_marks_orphan_entries, test_list_marks_non_orphan_entries

R106 — Collision prevention (pre-existing, verified)

3 pre-existing collision tests pass; no new code needed

T03 — worktree_cleanup_all MCP tool (R107)

New worktree_cleanup_all tool: orphans_only: bool (default true), force: bool (default true)
Returns { "removed": N, "orphans_only": bool }
Individual failures warned, not fatal
Removes the TODO(M002) placeholder
2 handler tests added

Verification

just ready  # all pass

All 4 requirements validated: R104, R105, R106, R107.

…pdate schema snapshot - Replace 7 .map_err(|e| anyhow::anyhow!("{e}")) with ? in CLI worktree handlers (R104) - Add is_orphan: bool to WorktreeInfo with #[serde(default)] (R105) - Wire orphan cross-reference into list() using work_session phase check - Update worktree-info schema snapshot with new field - Update schema_roundtrip tests with is_orphan field

…ist unit tests - Append [orphan] marker to non-JSON worktree list output for orphaned entries - Add test_list_marks_orphan_entries proving no-session worktrees are flagged - Add test_list_marks_non_orphan_entries proving active-session worktrees are clean

…ests - Add worktree_cleanup_all MCP tool (R107) with orphans_only + force params - Default orphans_only=true, force=true for safe non-interactive bulk cleanup - Returns { removed: N, orphans_only: bool } JSON response - Individual failures warned, not fatal to batch - Remove TODO(M002) comment - Add test_worktree_cleanup_all_appears_in_tools + test_worktree_cleanup_all_empty_list - just ready green

Copilot

Pull request overview

This PR improves Assay’s parallel worktree workflow correctness by preserving CLI error chains, adding orphan detection to worktree listings, and introducing an MCP tool to clean up worktrees in bulk.

Changes:

Add is_orphan: bool to WorktreeInfo and populate it in assay_core::worktree::list() by cross-referencing persisted WorkSessions.
Update CLI worktree list to display an [orphan] marker and improve CLI error-chain preservation by removing lossy map_err(anyhow!("{e}")) wrappers.
Add MCP tool worktree_cleanup_all with handler + tests, and update schemas/tests for the new field.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
crates/assay-types/src/worktree.rs	Adds `WorktreeInfo.is_orphan` with serde defaulting.
crates/assay-core/src/worktree.rs	Populates `is_orphan` in `worktree::list()` and adds unit tests for orphan detection.
crates/assay-cli/src/commands/worktree.rs	Preserves error chains via `?`/`.into()` and shows `[orphan]` in `worktree list` output.
crates/assay-mcp/src/server.rs	Introduces `worktree_cleanup_all` MCP tool plus handler tests.
crates/assay-types/tests/schema_roundtrip.rs	Updates schema validation roundtrip to include `is_orphan`.
crates/assay-types/tests/snapshots/schema_snapshots__worktree-info-schema.snap	Updates schema snapshot for the new `is_orphan` field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T18:45:48Z

+        let worktree_dir = assay_core::worktree::resolve_worktree_dir(None, &config, &cwd);
+        for entry in &entries {
+            let worktree_path = if entry.path.is_absolute() {
+                entry.path.clone()
+            } else {
+                worktree_dir.join(&entry.spec_slug)


In worktree_cleanup_all, relative worktree paths are reconstructed as worktree_dir/spec_slug, which can diverge from the actual path reported by git worktree list (and could target the wrong directory). Prefer using entry.path directly, and if it’s not absolute, resolve it relative to cwd (the directory used for worktree::list).

Suggested change

let worktree_dir = assay_core::worktree::resolve_worktree_dir(None, &config, &cwd);

for entry in &entries {

let worktree_path = if entry.path.is_absolute() {

entry.path.clone()

} else {

worktree_dir.join(&entry.spec_slug)

for entry in &entries {

let worktree_path = if entry.path.is_absolute() {

entry.path.clone()

} else {

cwd.join(&entry.path)

Copilot · 2026-04-01T18:45:49Z

+            .expect("git commit failed");
+
+        std::env::set_current_dir(dir.path()).unwrap();
+
+        let server = AssayServer::new();


This test changes the process working directory via std::env::set_current_dir(...) but never restores it. Because CWD is global for the whole test process, this can leak into other concurrently-running tests and cause flakes. Use an RAII guard (save current_dir then restore in Drop) or refactor the server helpers to accept an explicit root path so tests don’t need to mutate global state.

Copilot · 2026-04-01T18:45:49Z

+        std::process::Command::new("git")
+            .args(["init", "-b", "main"])
+            .current_dir(dir.path())
+            .output()
+            .expect("git init failed");
+        std::process::Command::new("git")
+            .args(["config", "user.email", "test@example.com"])
+            .current_dir(dir.path())
+            .output()
+            .expect("git config failed");
+        std::process::Command::new("git")
+            .args(["config", "user.name", "Test"])
+            .current_dir(dir.path())
+            .output()
+            .expect("git config failed");
+        std::process::Command::new("git")
+            .args(["add", "."])
+            .current_dir(dir.path())
+            .output()
+            .expect("git add failed");
+        std::process::Command::new("git")
+            .args(["commit", "-m", "init"])
+            .current_dir(dir.path())
+            .output()
+            .expect("git commit failed");


The git setup commands use .output().expect(...) but don’t assert status.success(). If a command fails (e.g., git not available, commit rejected), the test may proceed and fail later with a less actionable error. Capture the Output and assert success, including stderr/stdout in the failure message.

Suggested change

std::process::Command::new("git")

.args(["init", "-b", "main"])

.current_dir(dir.path())

.output()

.expect("git init failed");

std::process::Command::new("git")

.args(["config", "user.email", "test@example.com"])

.current_dir(dir.path())

.output()

.expect("git config failed");

std::process::Command::new("git")

.args(["config", "user.name", "Test"])

.current_dir(dir.path())

.output()

.expect("git config failed");

std::process::Command::new("git")

.args(["add", "."])

.current_dir(dir.path())

.output()

.expect("git add failed");

std::process::Command::new("git")

.args(["commit", "-m", "init"])

.current_dir(dir.path())

.output()

.expect("git commit failed");

let output = std::process::Command::new("git")

.args(["init", "-b", "main"])

.current_dir(dir.path())

.output()

.expect("failed to execute git init");

assert!(

output.status.success(),

"git init failed: stdout: {}, stderr: {}",

String::from_utf8_lossy(&output.stdout),

String::from_utf8_lossy(&output.stderr),

);

let output = std::process::Command::new("git")

.args(["config", "user.email", "test@example.com"])

.current_dir(dir.path())

.output()

.expect("failed to execute git config user.email");

assert!(

output.status.success(),

"git config user.email failed: stdout: {}, stderr: {}",

String::from_utf8_lossy(&output.stdout),

String::from_utf8_lossy(&output.stderr),

);

let output = std::process::Command::new("git")

.args(["config", "user.name", "Test"])

.current_dir(dir.path())

.output()

.expect("failed to execute git config user.name");

assert!(

output.status.success(),

"git config user.name failed: stdout: {}, stderr: {}",

String::from_utf8_lossy(&output.stdout),

String::from_utf8_lossy(&output.stderr),

);

let output = std::process::Command::new("git")

.args(["add", "."])

.current_dir(dir.path())

.output()

.expect("failed to execute git add");

assert!(

output.status.success(),

"git add failed: stdout: {}, stderr: {}",

String::from_utf8_lossy(&output.stdout),

String::from_utf8_lossy(&output.stderr),

);

let output = std::process::Command::new("git")

.args(["commit", "-m", "init"])

.current_dir(dir.path())

.output()

.expect("failed to execute git commit");

assert!(

output.status.success(),

"git commit failed: stdout: {}, stderr: {}",

String::from_utf8_lossy(&output.stdout),

String::from_utf8_lossy(&output.stderr),

);

Copilot · 2026-04-01T18:45:49Z

    let path_width = entries
        .iter()
        .map(|e| e.path.display().to_string().len())
        .max()
        .unwrap_or(4)
        .max(4);

    // Header
    println!(
        "  {:<sw$}  {:<bw$}  {:<pw$}",
        "Spec",
        "Branch",
        "Path",
        sw = spec_width,
        bw = branch_width,
        pw = path_width,
    );
    println!(
        "  {:<sw$}  {:<bw$}  {:<pw$}",
        "\u{2500}".repeat(spec_width),
        "\u{2500}".repeat(branch_width),
        "\u{2500}".repeat(path_width),
        sw = spec_width,
        bw = branch_width,
        pw = path_width,
    );

    for entry in &entries {
        let spec_display = if color {
            colorize(&entry.spec_slug, "\x1b[36m", true)
        } else {
            entry.spec_slug.clone()
        };
        // 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}",


path_width is computed from entry.path only, but the row output appends " [orphan]" for orphaned entries. This makes the header/underline width inconsistent with the longest rendered row. Consider including the marker length in path_width (or drop pw entirely since the last column isn’t padded).

- Fix false-orphan on I/O errors: distinguish WorkSessionNotFound (orphaned) from real I/O errors (warn + conservatively treat as active) in list() - Simplify detect_orphans() to delegate to list() is_orphan field - Add test_list_marks_terminal_session_as_orphan covering the terminal case

Test added 3 commits April 1, 2026 12:33

Copilot AI review requested due to automatic review settings April 1, 2026 18:40

Copilot started reviewing on behalf of wollax April 1, 2026 18:40 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

wollax merged commit d4ed214 into main Apr 1, 2026
1 of 2 checks passed

wollax deleted the kata/root/M018/S04 branch April 1, 2026 18:47

wollax restored the kata/root/M018/S04 branch April 2, 2026 02:55

wollax deleted the kata/root/M018/S04 branch April 9, 2026 21:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

feat(M018/S04): Worktree correctness — error chains, orphan detection, cleanup_all MCP tool#213
wollax merged 4 commits into
mainfrom
kata/root/M018/S04

wollax commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

wollax commented Apr 1, 2026

S04: Worktree correctness

What's in this PR

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants