fix(git-shim): worktree residual polish — teardown messaging + gh merge collisions

crates/flare-git-core/src/classify.rs

@@ -112,12 +112,45 @@ pub fn agent_invocation_detected() -> bool {
         || std::env::var_os("AGENTFLARE_AGENT").is_some_and(|s| !s.is_empty())
 }
 
+/// `true` if `subcommand`/`args` is a branch-*creating* form: `git checkout
+/// -b/-B/--orphan <name>` and `git switch -c/-C/--create/--force-create/
+/// --orphan <name>` -- including the attached short-option spellings
+/// (`-bname`, `-Cname`, ...) and `--long=name`. These never detach HEAD
+/// (the new branch is checked out instead) but they DO move the canonical
+/// checkout off its current branch onto feature-branch work -- so the shim
+/// keeps blocking them there, with an accurate reason rather than the
+/// misleading "would detach HEAD" message (item #441 / vent #395). Scanning
+/// stops at a bare `--` -- anything after it is a pathspec, not an option.
+#[must_use]
+pub fn is_branch_create(subcommand: &str, args: &[String]) -> bool {
+    let (short_flags, long_flags): (&[&str], &[&str]) = match subcommand {
+        "checkout" => (&["-b", "-B"], &["--orphan"]),
+        "switch" => (&["-c", "-C"], &["--create", "--force-create", "--orphan"]),
+        _ => return false,
+    };
+    for a in args {
+        if a == "--" {
+            break;
+        }
+        let is_short = short_flags.iter().any(|f| a.starts_with(f));
+        let is_long = long_flags
+            .iter()
+            .any(|f| a == f || a.starts_with(&format!("{f}=")));
+        if is_short || is_long {
+            return true;
+        }
+    }
+    false
+}
+
 /// `true` if `subcommand`/`args` would detach HEAD -- `git checkout
 /// <target>` implicitly detaches when `target` isn't an existing local
 /// branch (no `--detach` flag required for that form); `git switch` never
 /// silently detaches, only `switch --detach`/`-d` does. `git checkout --
 /// <pathspec>` (and any form with `--` before the target) restores files
-/// and never touches HEAD at all.
+/// and never touches HEAD at all. Branch-creating forms (`-b`/`-B`/`-c`/
+/// `-C`) check out the new branch and never detach -- handled by
+/// `is_branch_create` instead.
 #[must_use]
 pub fn would_detach_head(repo_root: &Path, subcommand: &str, args: &[String]) -> bool {
     match subcommand {
@@ -128,6 +161,9 @@ pub fn would_detach_head(repo_root: &Path, subcommand: &str, args: &[String]) ->
             if args.iter().any(|a| a == "--detach") {
                 return true;
             }
+            if is_branch_create(subcommand, args) {
+                return false; // `checkout -b <name>` checks out the new branch
+            }
             let Some(target) = args.iter().find(|a| !a.starts_with('-')) else {
                 return false; // e.g. bare `git checkout` -- doesn't move HEAD
             };
@@ -340,9 +376,19 @@ pub fn classify_pure(
             if is_read_only {
                 Disposition::Passthrough
             } else {
-                Disposition::Deny {
-                    reason: "'git worktree' is orchestrator-managed by agentflare — call `item(action=\"claim\", id=<item>)` to provision one. (Not the standalone `claim`/`mcp__flare__claim` tool -- that only takes a scope lock and does not create a worktree.)".to_string(),
-                }
+                // Distinguish provisioning (`add`) from teardown (`remove`/
+                // `prune`): an agent denied mid-teardown needs the exact
+                // tool+action that owns cleanup, not the provisioning call.
+                let teardown = matches!(
+                    args.first().map(String::as_str),
+                    Some("remove") | Some("prune")
+                );
+                let reason = if teardown {
+                    "'git worktree remove/prune' is orchestrator-managed by agentflare — to tear down an item's worktree call `item(action=\"check_merge\", id=<item>)` once its PR merges, or `item(action=\"release\", id=<item>)`; to prune stale worktrees run `agentflare git worktree audit --prune`.".to_string()
+                } else {
+                    "'git worktree' is orchestrator-managed by agentflare — call `item(action=\"claim\", id=<item>)` to provision one. (Not the standalone `claim`/`mcp__flare__claim` tool -- that only takes a scope lock and does not create a worktree.)".to_string()
+                };
+                Disposition::Deny { reason }
             }
         }
         // Fail-open: anything not explicitly matched above is allowed through
@@ -860,6 +906,45 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn worktree_teardown_deny_points_at_the_cleanup_tool() {
+        // Item #441 / vent #350: an agent denied mid-teardown needs the
+        // exact cleanup action, not the provisioning call.
+        let policy = ResolvedGitShimPolicy::baseline();
+        for sub in ["remove", "prune"] {
+            let d = classify_pure(
+                "worktree",
+                &args(&[sub, "../x"]),
+                "master",
+                &TrustRootTouch::Clean,
+                false,
+                &policy,
+            );
+            let Disposition::Deny { reason } = d else {
+                panic!("expected deny for worktree {sub}");
+            };
+            assert!(reason.contains("check_merge"), "{reason}");
+            assert!(reason.contains("audit --prune"), "{reason}");
+        }
+    }
+
+    #[test]
+    fn worktree_provision_deny_points_at_the_claim_tool() {
+        let policy = ResolvedGitShimPolicy::baseline();
+        let d = classify_pure(
+            "worktree",
+            &args(&["add", "../x"]),
+            "master",
+            &TrustRootTouch::Clean,
+            false,
+            &policy,
+        );
+        let Disposition::Deny { reason } = d else {
+            panic!("expected deny for worktree add");
+        };
+        assert!(reason.contains("item(action=\"claim\""), "{reason}");
+    }
+
     #[test]
     fn worktree_list_is_passthrough() {
         let policy = ResolvedGitShimPolicy::baseline();
@@ -1156,6 +1241,61 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn branch_create_forms_are_recognized() {
+        assert!(is_branch_create("checkout", &args(&["-b", "feature/x"])));
+        assert!(is_branch_create("checkout", &args(&["-B", "feature/x"])));
+        assert!(is_branch_create("switch", &args(&["-c", "feature/x"])));
+        assert!(is_branch_create("switch", &args(&["-C", "feature/x"])));
+        assert!(!is_branch_create("checkout", &args(&["feature/x"])));
+        assert!(!is_branch_create(
+            "checkout",
+            &args(&["--detach", "feature/x"])
+        ));
+        assert!(!is_branch_create("push", &args(&["origin", "master"])));
+    }
+
+    #[test]
+    fn branch_create_recognizes_attached_and_long_forms() {
+        // Attached short-option spellings (`-bname`, not `-b name`).
+        assert!(is_branch_create("checkout", &args(&["-bfeature/x"])));
+        assert!(is_branch_create("checkout", &args(&["-Bfeature/x"])));
+        assert!(is_branch_create("switch", &args(&["-cfeature/x"])));
+        assert!(is_branch_create("switch", &args(&["-Cfeature/x"])));
+        // `--orphan` on both subcommands.
+        assert!(is_branch_create("checkout", &args(&["--orphan", "root"])));
+        assert!(is_branch_create("switch", &args(&["--orphan", "root"])));
+        // `switch`'s long forms of -c/-C, bare and `=name`.
+        assert!(is_branch_create(
+            "switch",
+            &args(&["--create", "feature/x"])
+        ));
+        assert!(is_branch_create("switch", &args(&["--create=feature/x"])));
+        assert!(is_branch_create(
+            "switch",
+            &args(&["--force-create", "feature/x"])
+        ));
+        // Scanning stops at `--`: nothing after it is an option.
+        assert!(!is_branch_create("checkout", &args(&["--", "-b"])));
+    }
+
+    #[test]
+    fn would_detach_head_false_for_branch_creating_checkout() {
+        let repo = crate::shell::test_support::init_repo_with_branch("master");
+        // `checkout -b <name>` creates the branch and checks it out -- HEAD
+        // is never detached, even though the target branch doesn't exist yet.
+        assert!(!would_detach_head(
+            &repo.path,
+            "checkout",
+            &args(&["-b", "feature/x"])
+        ));
+        assert!(!would_detach_head(
+            &repo.path,
+            "switch",
+            &args(&["-c", "feature/x"])
+        ));
+    }
+
     #[test]
     fn would_detach_head_true_for_explicit_detach_flag() {
         let repo = crate::shell::test_support::init_repo_with_branch("master");

crates/flare-git-core/src/worktree.rs

@@ -564,6 +564,11 @@ pub struct OrphanWorktree {
     pub sequence_id: Option<i64>,
     pub size_bytes: u64,
     pub has_broken_gitdir: bool,
+    /// The worktree's gitdir is intact but it sits on the repo's default
+    /// branch (a task worktree stranded there) -- the gh merge-collision
+    /// case. Mutually exclusive-ish with `has_broken_gitdir`: at least one
+    /// of the two is always true for an orphan.
+    pub on_default_branch: bool,
 }
 
 /// Scan `.worktrees/task/*` for orphaned worktree directories.
@@ -585,6 +590,7 @@ pub fn audit_orphans(
     if !task_dir.exists() {
         return Vec::new();
     }
+    let default_branch = crate::branch::resolve_default_branch(repo_root);
     let mut orphans = Vec::new();
     // filter_map skips unreadable entries individually rather than failing
     // the whole scan on the first error -- a single permission-denied or
@@ -622,9 +628,33 @@ pub fn audit_orphans(
         {
             continue;
         }
-        // Only consider as orphan when the gitdir pointer is broken
+        // Only consider as orphan when the gitdir pointer is broken, OR the
+        // worktree is sitting on the repo's default branch. The second case
+        // is the stranded-worktree root cause behind gh pr merge --delete-
+        // branch / post-merge local-sync failures (item #441, vents #351/
+        // #394/#423): a task worktree should only ever be on its own
+        // task/<N> branch, so one that has been switched to the default
+        // branch is abandoned junk -- it holds the default branch checked
+        // out, which blocks both deleting it and gh's merge flow.
+        let mut on_default_branch = false;
         if !has_broken_gitdir {
-            continue;
+            on_default_branch = crate::branch::current_branch(path)
+                .map(|b| !b.is_empty() && b == default_branch)
+                .unwrap_or(false);
+            if !on_default_branch {
+                continue;
+            }
+            // Stranded-but-dirty is still real work -- same guard
+            // `cleanup_item_worktree` applies before its own `gc_orphans`
+            // call; fail closed (preserve) on a status-check error too.
+            let clean = matches!(run_git_in(path, &["status", "--porcelain"]), Ok(o) if o.trim().is_empty());
+            if !clean {
+                eprintln!(
+                    "worktree: preserving {} -- uncommitted changes",
+                    path.display()
+                );
+                continue;
+            }
         }
         let sequence_id = dir_name.parse::<i64>().ok();
         let size_bytes = dir_size(path);
@@ -634,6 +664,7 @@ pub fn audit_orphans(
             sequence_id,
             size_bytes,
             has_broken_gitdir,
+            on_default_branch,
         });
     }
     orphans
@@ -1168,6 +1199,74 @@ mod tests {
         assert!(!cleanup_item_worktree(&item, &repo.path));
     }
 
+    #[test]
+    fn audit_orphans_flags_a_worktree_stranded_on_the_default_branch() {
+        let repo = init_repo();
+        let item = test_item(7);
+        let target = resolve_default_branch(&repo.path);
+        let worktree_path = create_worktree(&item, &repo.path, &target, None).unwrap();
+        assert!(worktree_path.exists());
+        // Free the default branch from the canonical checkout first -- git
+        // refuses to check out the same branch in two worktrees (this exact
+        // refusal is the gh merge-collision from item #441).
+        assert!(crate::shell::run_in_ok(
+            &repo.path,
+            &["switch", "-c", "canonical/other"]
+        ));
+        // The stranded-worktree failure mode: a task worktree that has been
+        // switched onto the default branch (intact gitdir, not broken).
+        crate::shell::run_in(&worktree_path, &["switch", "master"]).unwrap();
+
+        let claimed: std::collections::HashSet<String> =
+            std::collections::HashSet::from(["7".to_string()]);
+        // Claimed -> not an orphan.
+        assert!(audit_orphans(&repo.path, Some(&claimed)).is_empty());
+        // Not claimed -> orphan, flagged as on the default branch, with an
+        // intact gitdir.
+        let orphans = audit_orphans(&repo.path, Some(&std::collections::HashSet::new()));
+        assert_eq!(orphans.len(), 1);
+        assert!(!orphans[0].has_broken_gitdir);
+        assert!(orphans[0].on_default_branch);
+    }
+
+    #[test]
+    fn audit_orphans_preserves_a_dirty_worktree_stranded_on_the_default_branch() {
+        let repo = init_repo();
+        let item = test_item(9);
+        let target = resolve_default_branch(&repo.path);
+        let worktree_path = create_worktree(&item, &repo.path, &target, None).unwrap();
+        assert!(worktree_path.exists());
+        assert!(crate::shell::run_in_ok(
+            &repo.path,
+            &["switch", "-c", "canonical/other"]
+        ));
+        crate::shell::run_in(&worktree_path, &["switch", "master"]).unwrap();
+        // Uncommitted changes -- real work that must survive an audit/prune
+        // sweep even though the worktree is stranded on the default branch.
+        std::fs::write(worktree_path.join("dirty.txt"), "not committed").unwrap();
+
+        let orphans = audit_orphans(&repo.path, Some(&std::collections::HashSet::new()));
+        assert!(
+            orphans.is_empty(),
+            "dirty stranded worktree must not be listed as prunable: {} found",
+            orphans.len()
+        );
+    }
+
+    #[test]
+    fn audit_orphans_ignores_a_claimed_or_own_branch_worktree() {
+        let repo = init_repo();
+        let item = test_item(8);
+        let target = resolve_default_branch(&repo.path);
+        let worktree_path = create_worktree(&item, &repo.path, &target, None).unwrap();
+        assert!(worktree_path.exists());
+
+        // Own task/<N> branch, intact gitdir, unclaimed-but-live -- not an
+        // orphan (no broken gitdir, not on the default branch).
+        let orphans = audit_orphans(&repo.path, Some(&std::collections::HashSet::new()));
+        assert!(orphans.is_empty());
+    }
+
     #[test]
     fn commit_uncommitted_commits_a_dirty_worktree() {
         let repo = init_repo();

crates/flare-git-shim/src/main.rs

@@ -137,11 +137,13 @@ fn snapshots_enabled() -> bool {
     }
 }
 
-/// Deny reason for the canonical-repo HEAD-detach guard, or `None` to let
-/// the op through. Scoped tightly on purpose: agent-invoked (self-reported
-/// via env markers, same as `agentflare-shim`'s own gate) AND an agentflare-
-/// tracked project AND the canonical (non-worktree) checkout AND the command
-/// would actually detach HEAD. Interactive human use, any use inside an
+/// Deny reason for canonical-checkout mutation guards (HEAD detach AND
+/// branch-creation), or `None` to let the op through. Scoped tightly on
+/// purpose: agent-invoked (self-reported via env markers, same as
+/// `agentflare-shim`'s own gate) AND an agentflare-tracked project AND the
+/// canonical (non-worktree) checkout AND the command would actually move
+/// HEAD off its current branch (either by detaching, or by creating a new
+/// branch and checking it out). Interactive human use, any use inside an
 /// isolated worktree, and any project agentflare doesn't track, are all
 /// completely unaffected.
 fn deny_canonical_detach_reason(
@@ -161,6 +163,11 @@ fn deny_canonical_detach_reason(
     if branch::is_linked_worktree(repo_root) {
         return None; // agent worktrees are exactly where this is expected
     }
+    if classify::is_branch_create(subcommand, args) {
+        return Some(format!(
+            "this would create a new branch in the canonical checkout (not an isolated worktree) while agent-invoked -- feature work belongs in a worktree. Call `item(action=\"claim\", id=<item>)` to provision one, or set {ALLOW_CANONICAL_MUTATE_ENV}=1 to override."
+        ));
+    }
     if !classify::would_detach_head(repo_root, subcommand, args) {
         return None;
     }
@@ -358,8 +365,19 @@ fn main() {
         let _ = audit::log_event(&audit_path, &event);
     }
 
+    // Stranded-canonical-checkout recovery (item #441 / vent #386 residual):
+    // a merged worktree leaves the canonical checkout unable to `git switch`
+    // back to the default branch -- that's denied by classify_pure's
+    // protected-branch guard. ALLOW_CANONICAL_MUTATE is the escape hatch for
+    // exactly this class of canonical-checkout mutation, so when it's set in
+    // the canonical checkout, downgrade a checkout/switch deny to passthrough.
+    // Still audited (the original event was already logged above).
+    let canonical_recovery = agentflare_shim::is_set(ALLOW_CANONICAL_MUTATE_ENV)
+        && !branch::is_linked_worktree(&repo_root)
+        && matches!(subcommand.as_str(), "checkout" | "switch");
+
     match &event.disposition {
-        classify::Disposition::Deny { reason } => {
+        classify::Disposition::Deny { reason } if !canonical_recovery => {
             eprintln!("agentflare git shim: denied — {reason}");
             exit(1);
         }
@@ -373,7 +391,9 @@ fn main() {
             );
             exit(1);
         }
-        classify::Disposition::Passthrough | classify::Disposition::SilentExempt => {
+        classify::Disposition::Deny { .. }
+        | classify::Disposition::Passthrough
+        | classify::Disposition::SilentExempt => {
             if matches!(subcommand.as_str(), "commit" | "push")
                 && let Some(reason) = scope_check_deny_reason(&subcommand)
             {