Stop a failed fork PR switch from deleting the user's branch

[max-sixty]

The bug

wt switch pr:N / mr:N against a fork PR/MR took the CreationMethod::ForkRef
path, which built the branch, its tracking config, and its worktree as four separate
git calls and compensated for that decomposition on any error:

if let Err(e) = setup_result {
    // Cleanup: try to delete the branch if it was created
    let _ = repo.run_command(&["branch", "-D", "--", &branch]);
    return Err(e);
}

The rollback was only ever valid if the first call — git branch -- <b> FETCH_HEAD —
had succeeded. Its likeliest failure is git refusing a name that is already taken, and
in that case wt force-deleted a branch it had not created, discarding the result of
the deletion.

The reachability window

The branch name comes from the PR's head ref. Whether it is free is decided in
resolve_fork_ref, which runs during resolve_ref_shortcut_target — before the
pre-switch hooks and before plan_switch. The branch is created later, inside
execute_switch. Anything that claims the name in between selects the fork path and
then fails it: a pre-switch hook, or another session in another worktree of the same
repo.

When that happened, git branch -D dropped the branch, and with it any commits only
that branch held — the reflog goes too, so recovery is git fsck --lost-found. The
error printed immediately afterwards was:

✗ Failed to create local branch feature-fix from PR #42
  fatal: a branch named 'feature-fix' already exists

which reads as though nothing happened, and the obvious response — re-run — succeeds
the second time, erasing the evidence.

The fix: remove the rollback, don't guard it

The rollback existed only to compensate for a decomposition that shouldn't exist. The
sibling CreationMethod::Regular arm already builds a single
git worktree add -b <branch> -- <path> <ref> and has no rollback at all, because git
owns the atomicity. ForkRef now uses the same shape:

git worktree add -b <branch> -- <path> FETCH_HEAD

Git writes nothing when the name is taken (verified: the branch keeps its SHA and no
directory is left behind), so on the one failure that lands on a branch wt did not
create there is nothing to roll back. Guarding the delete with "only if we created it"
would have kept a destructive path alive to fix a case where the destructive path was
never needed.

This is not a claim that git worktree add -b is atomic in general — with the
destination path occupied it creates the branch and then dies on the path (verified on
git 2.55). That leftover is a stray branch at the PR head, which the Regular arm
already accepts, and it is not someone's work. It does carry the same follow-on cost as
a failed tracking write below — no config written, so the next wt switch pr:N takes
the prefixed-branch path or errors — and the docstring now says so in both places. The
old code rolled back cleanly in this one case, its own branch being the only thing it
could have deleted, but distinguishing it needs the very did-we-create-it reasoning that
produced the bug.

Tracking config (branch.<b>.remote / .merge / .pushRemote) now follows the
worktree instead of preceding it, and the ordering is load-bearing: writing tracking
first would rewrite the existing branch's upstream on exactly the collision above —
the same class of bug as the rollback. If a tracking write fails, the user is left with
a branch plus worktree at the PR head and incomplete config. Nothing is lost, and a
re-run is non-destructive: branch_tracks_ref reads the branch as tracking something
else, so the next wt switch pr:N takes the prefixed-branch path (GitHub/Gitea) or
reports BranchTracksDifferentRef (GitLab/Azure DevOps). That is strictly better than
deleting their branch. This matches the repo's stated rules: prefer failure over silent
loss, no implicit destructive side effects, and one canonical path for worktree
creation.

branch.autoSetupMerge is inert on this path — git sets up tracking only where the
start point resolves under refs/heads/ or refs/remotes/, and FETCH_HEAD resolves
as itself, so no value of it (true, always, inherit, simple, false) writes
tracking config here. The -c branch.autoSetupMerge=simple the Regular arm needs
would be redundant.
git worktree add -b also has a known DWIM footgun where a bare remote-only start point
creates the remote's branch name instead of -b's; FETCH_HEAD is not a
remote-tracking ref, and the new test covers the case rather than assuming it.

Test

test_switch_pr_fork_failure_keeps_existing_branch pins the safety property: a
pre-existing branch survives a failed pr:N switch. It uses the existing mock-gh
fork-PR fixture, plus a pre-switch hook that renames a never-pushed local branch onto
the PR's branch name — occupying it after the forge answered (which is what selects the
fork path) and before the worktree is created, standing in for the concurrent session
that does the same in real use. The test then asserts the switch failed and that
refs/heads/feature-fix still points at the unpushed commit.

Against the old code it fails at exactly that assertion, with the branch gone.

The same failure mapping as Regular

Creating the branch with git worktree add -b means the fork path now hits the one
failure the Regular arm already translates: git stores refs as file paths, so a fork
PR whose head ref is feature-fix can't create a branch in a repo that already has
feature-fix/nested, and git says fatal: cannot lock ref 'refs/heads/feature-fix'.
The arm routes that through detect_branch_namespace_conflict, exactly as Regular
does, so the user is told which local branch is in the way. It matters more here than
there: a PR's head ref isn't the user's to rename, so the only actionable move is to
rename the local branch, and the raw git text doesn't name it.

test_switch_pr_fork_namespace_conflict covers it — same mock-gh fixture, with
feature-fix/nested created before the switch. It asserts
BranchNamespaceConflict's own wording, collides with existing branch, rather than
just the conflicting branch name: git's raw cannot lock ref text names that branch
too, so the looser assertion passed against the unmapped error. Verified by reverting
the mapping — the test fails.

Validation

• cargo run -- hook pre-merge --yes — 4,715 tests passed, 1 skipped; clippy, fmt,
  doc-sync, snapshot and lockfile checks green
• No help text or after_long_help changed, so no doc regeneration was needed
• The FAQ's "What can Worktrunk delete?" never listed this branch -D, so no doc update is
  needed — and its absence is more evidence the deletion was never intended behaviour

This was written by Claude Code on behalf of max-sixty

[worktrunk-bot]

Reviewed the fix; the reasoning holds and I independently confirmed the two load-bearing claims on git 2.55 rather than taking the PR body's word for them:

• git worktree add -b <taken> -- <path> FETCH_HEAD against an occupied branch name fails at Preparing worktree, leaves the existing branch at its original SHA, creates no directory, and registers no worktree — so there is genuinely nothing for a rollback to undo on the one failure the old branch -D was mishandling.
• No value of branch.autoSetupMerge (true, always, inherit, simple, false) writes branch.<b>.remote / .merge from a FETCH_HEAD start point, so omitting the -c the Regular arm needs is correct and the set_config calls below stay the only writers.

I also checked that CreationMethod::ForkRef is only ever selected when the branch does not exist (resolve_fork_ref returns Regular for both the tracking and prefixed-tracking hits), so -b can't collide with an adoption path, and that validate_worktree_creation deliberately skips branch-existence checks for ForkRef — which is what makes the new test exercise the real collision at git worktree add rather than a plan-time guard.

Not approving. This repo's reviewer guidance — .claude/skills/running-tend/references/review-pr.md, "Data-Loss Surface: Hold for Human Review" — routes any diff touching git branch -D to a human, and this one edits exactly that path in src/commands/worktree/switch.rs. It removes the destructive call rather than adding or widening one, and I'd have approved it on the merits otherwise; I'm following the rule rather than deciding the exception applies. The rule's next step is "request review from @max-sixty", which is moot here since you're the author.

One optional note inline about a small asymmetry the docstring's "exactly as the CreationMethod::Regular arm does" glosses over.