Give Worktree ownership of safe Issue branch acquisition

src/branch.rs

@@ -6,6 +6,7 @@ use anyhow::{Result, bail};
 use crate::git::Git;
 use crate::github::{GitHub, PrState, PullRequest};
 use crate::issue::IssueUrl;
+use crate::worktree;
 
 pub enum Selection {
     /// No Issue branch has been used yet, or the highest-numbered one's PR is
@@ -64,8 +65,9 @@ impl Selection {
 
 /// Pick the Issue branch for `issue` from the Issue branches on origin and
 /// their PRs in any state: the highest number seen on either decides. Fails
-/// if a local copy of the chosen branch in the Launch directory differs from
-/// origin's: the Run replaces it and deletes it at cleanup.
+/// early if Worktree's advisory preflight refuses the sampled local head,
+/// before the Run makes its Claim. Worktree independently checks again
+/// during acquisition and owns normal cleanup of the acquired branch.
 pub fn select(launch: &Git, github: &GitHub, issue: &IssueUrl) -> Result<Selection> {
     select_through(&GitHubAndOrigin { launch, github }, issue)
 }
@@ -141,12 +143,7 @@ impl Reads for GitHubAndOrigin<'_> {
     }
 
     fn local_head(&self, branch: &str) -> Result<Option<String>> {
-        self.launch.run_optional(&[
-            "rev-parse",
-            "--verify",
-            "--quiet",
-            &format!("refs/heads/{branch}"),
-        ])
+        worktree::local_head(self.launch, branch)
     }
 }
 
@@ -161,7 +158,11 @@ fn select_through(reads: &impl Reads, issue: &IssueUrl) -> Result<Selection> {
         .chain(prs.iter().map(|(number, _)| *number))
         .max();
     let Some(highest) = highest else {
-        check_local_branch(reads, &first_branch, None)?;
+        worktree::check_local_branch(
+            &first_branch,
+            reads.local_head(&first_branch)?.as_deref(),
+            None,
+        )?;
         return Ok(Selection::Fresh {
             branch: first_branch,
         });
@@ -179,11 +180,15 @@ fn select_through(reads: &impl Reads, issue: &IssueUrl) -> Result<Selection> {
         // A merged or closed PR uses its number up, even if its branch was deleted.
         (_, Some(pr)) if pr.state != PrState::Open => {
             let next = branch_name(issue, highest + 1);
-            check_local_branch(reads, &next, None)?;
+            worktree::check_local_branch(&next, reads.local_head(&next)?.as_deref(), None)?;
             Ok(Selection::Fresh { branch: next })
         }
         (Some(origin_sha), pr) => {
-            check_local_branch(reads, &branch, Some(&origin_sha))?;
+            worktree::check_local_branch(
+                &branch,
+                reads.local_head(&branch)?.as_deref(),
+                Some(&origin_sha),
+            )?;
             Ok(Selection::Continuation { branch, pr })
         }
         (None, _) => bail!("{branch} has an open PR but is gone from origin"),
@@ -227,23 +232,6 @@ fn used(reads: &impl Reads, issue: &IssueUrl) -> Result<Used> {
     Ok(Used { on_origin, prs })
 }
 
-/// A local `branch` in the launch repository must be at `origin_sha`, or not
-/// exist when origin has no copy, so no local-only commits are destroyed.
-fn check_local_branch(reads: &impl Reads, branch: &str, origin_sha: Option<&str>) -> Result<()> {
-    let Some(local_sha) = reads.local_head(branch)? else {
-        return Ok(());
-    };
-    match origin_sha {
-        Some(origin_sha) if origin_sha == local_sha => Ok(()),
-        Some(_) => bail!(
-            "the local branch {branch} differs from origin/{branch}; push, reset or delete it first"
-        ),
-        None => {
-            bail!("the local branch {branch} is not on origin; push, rename or delete it first")
-        }
-    }
-}
-
 /// `issue-<n>` is branch 1, `issue-<n>-branch-<k>` is branch k for k ≥ 2.
 fn branch_number(issue: &IssueUrl, branch: &str) -> Option<u64> {
     let rest = branch.strip_prefix(&branch_name(issue, 1))?;
@@ -545,16 +533,7 @@ mod tests {
     }
 
     #[test]
-    fn a_local_copy_of_a_fresh_branch_is_refused() {
-        let reads = InMemory {
-            local: vec![("issue-5", "abc")],
-            ..InMemory::default()
-        };
-        assert_eq!(
-            refusal(&reads),
-            "the local branch issue-5 is not on origin; push, rename or delete it first"
-        );
-
+    fn advisory_preflight_refuses_the_chosen_next_numbered_branch() {
         let reads = InMemory {
             prs: vec![pr(1, "issue-5", PrState::Merged)],
             local: vec![("issue-5-branch-2", "abc")],
@@ -566,31 +545,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn a_local_copy_of_a_continued_branch_at_another_head_is_refused() {
-        let reads = InMemory {
-            origin: vec![("issue-5", "abc")],
-            local: vec![("issue-5", "def")],
-            ..InMemory::default()
-        };
-
-        assert_eq!(
-            refusal(&reads),
-            "the local branch issue-5 differs from origin/issue-5; push, reset or delete it first"
-        );
-    }
-
-    #[test]
-    fn a_local_copy_at_origins_head_is_continued() {
-        let reads = InMemory {
-            origin: vec![("issue-5", "abc")],
-            local: vec![("issue-5", "abc")],
-            ..InMemory::default()
-        };
-
-        assert_eq!(picked(&reads), continued("issue-5", None));
-    }
-
     #[test]
     fn selection_lists_origin_and_the_prs_once_whatever_the_history() {
         let reads = InMemory {

src/worktree.rs

@@ -13,6 +13,8 @@ use crate::git::Git;
 use crate::host;
 use crate::progress;
 
+#[cfg(test)]
+mod acquisition_tests;
 #[cfg(test)]
 mod preservation_tests;
 
@@ -45,33 +47,36 @@ pub struct Worktree {
 
 impl Worktree {
     /// Create `branch` fresh from `origin/<base>` in a new worktree next to the
-    /// launch repository's root, named `<repo>-<branch>`.
+    /// launch repository's root, named `<repo>-<branch>`. Under the worktree
+    /// lock, pin the fetched Base branch commit, refuse any existing local
+    /// Issue branch and create without force. No selection preflight is required.
     pub fn create_fresh(launch: &Git, repo: &str, branch: &str, base: &str) -> Result<Self> {
         let _lock = lock_launch(launch)?;
         launch.run(&["fetch", "origin", base])?;
-        Self::add(
-            launch,
-            repo,
-            branch,
-            &["-b", branch],
-            &format!("origin/{base}"),
-        )
+        let source = format!("origin/{base}");
+        let commit = launch.run(&["rev-parse", &format!("refs/remotes/{source}")])?;
+        check_local_branch(branch, local_head(launch, branch)?.as_deref(), None)?;
+        Self::add(launch, repo, branch, &["-b", branch], &commit, &source)
     }
 
     /// Check out the existing `branch` from origin in a new worktree next to
     /// the launch repository's root, named `<repo>-<branch>`. `origin/<base>`
-    /// is fetched too, for the review fixed point. A local `branch`, if any, is
-    /// reset to origin's: `branch::select` has checked they already match.
+    /// is fetched too, for the review fixed point. Under the worktree lock,
+    /// pin the fetched head and refuse any different local head. An equal
+    /// branch is attached without resetting it; an absent one is created
+    /// without force at the pinned commit. No selection preflight is required.
     pub fn continue_existing(launch: &Git, repo: &str, branch: &str, base: &str) -> Result<Self> {
         let _lock = lock_launch(launch)?;
         launch.run(&["fetch", "origin", base, branch])?;
-        Self::add(
-            launch,
-            repo,
-            branch,
-            &["-B", branch],
-            &format!("origin/{branch}"),
-        )
+        let source = format!("origin/{branch}");
+        let commit = launch.run(&["rev-parse", &format!("refs/remotes/{source}")])?;
+        let local = local_head(launch, branch)?;
+        check_local_branch(branch, local.as_deref(), Some(&commit))?;
+        if local.is_some() {
+            Self::add(launch, repo, branch, &[], branch, &source)
+        } else {
+            Self::add(launch, repo, branch, &["-b", branch], &commit, &source)
+        }
     }
 
     fn add(
@@ -80,10 +85,11 @@ impl Worktree {
         branch: &str,
         branch_args: &[&str],
         start: &str,
+        source: &str,
     ) -> Result<Self> {
         let (root, path) = sibling(launch, &format!("{repo}-{branch}"))?;
         progress::step(format_args!(
-            "creating worktree {} on {branch} from {start}",
+            "creating worktree {} on {branch} from {source}",
             path.display()
         ));
         add_worktree(launch, branch_args, &path, start)?;
@@ -263,6 +269,39 @@ impl Worktree {
     }
 }
 
+/// The local Issue branch head sampled for acquisition or advisory preflight.
+pub(crate) fn local_head(launch: &Git, branch: &str) -> Result<Option<String>> {
+    launch.run_optional(&[
+        "rev-parse",
+        "--verify",
+        "--quiet",
+        &format!("refs/heads/{branch}"),
+    ])
+}
+
+/// Refuse local work acquisition would replace. A fresh checkout requires
+/// absence; a Continuation permits absence or exact equality with its
+/// sampled origin head. Selection shares this rule for advisory preflight;
+/// acquisition checks it again under the worktree lock after fetching.
+pub(crate) fn check_local_branch(
+    branch: &str,
+    local_head: Option<&str>,
+    origin_head: Option<&str>,
+) -> Result<()> {
+    let Some(local_head) = local_head else {
+        return Ok(());
+    };
+    match origin_head {
+        Some(origin_head) if origin_head == local_head => Ok(()),
+        Some(_) => bail!(
+            "the local branch {branch} differs from origin/{branch}; push, reset or delete it first"
+        ),
+        None => {
+            bail!("the local branch {branch} is not on origin; push, rename or delete it first")
+        }
+    }
+}
+
 impl Drop for Worktree {
     fn drop(&mut self) {
         let path = self.path().to_string_lossy().into_owned();