Continue an existing Issue branch

src/branch.rs

@@ -0,0 +1,150 @@
+//! Issue branch selection: start a fresh Issue branch, or continue the one
+//! already on origin (ADR-0002).
+
+use anyhow::{Context, Result, bail};
+
+use crate::git::Git;
+use crate::github::{self, PrState, PullRequest};
+use crate::issue::IssueUrl;
+
+pub enum Selection {
+    /// No Issue branch has been used yet: start `branch` from the Base branch.
+    Fresh { branch: String },
+    /// `branch` is on origin, with no PR or the open `pr`.
+    Continuation {
+        branch: String,
+        pr: Option<PullRequest>,
+    },
+}
+
+impl Selection {
+    pub fn branch(&self) -> &str {
+        match self {
+            Selection::Fresh { branch } | Selection::Continuation { branch, .. } => branch,
+        }
+    }
+
+    /// The Base branch: the open PR's base in a Continuation that has one,
+    /// otherwise `checked_out`, the branch checked out in the launch
+    /// directory (`None` on a detached HEAD). Says so on stderr when the PR's
+    /// base replaces a different checked-out branch.
+    pub fn base_branch(&self, checked_out: Option<&str>) -> Result<String> {
+        if let Selection::Continuation {
+            branch,
+            pr: Some(pr),
+        } = self
+        {
+            if let Some(checked_out) = checked_out.filter(|&c| c != pr.base) {
+                eprintln!(
+                    "thirdshift: continuing {branch} and its PR {}, so the Base branch is {}, not the checked-out {checked_out}",
+                    pr.url, pr.base
+                );
+            }
+            return Ok(pr.base.clone());
+        }
+        checked_out
+            .map(String::from)
+            .context("HEAD is detached; check out the branch the work should be based on")
+    }
+}
+
+/// Pick the Issue branch for `issue` from the Issue branches on origin (one
+/// `git ls-remote`) and their PRs in any state (one `gh pr list`). Fails if a
+/// local copy of the chosen branch in the launch repository differs from
+/// origin's: the Run replaces it and deletes it at cleanup.
+pub fn select(launch: &Git, issue: &IssueUrl) -> Result<Selection> {
+    let first_branch = branch_name(issue, 1);
+    let remote = launch.run(&[
+        "ls-remote",
+        "--heads",
+        "origin",
+        &format!("refs/heads/{first_branch}"),
+        &format!("refs/heads/{first_branch}-branch-*"),
+    ])?;
+    let on_origin: Vec<(u64, String)> = remote
+        .lines()
+        .filter_map(|line| {
+            let (sha, name) = line.split_once('\t')?;
+            let number = branch_number(issue, name.strip_prefix("refs/heads/")?)?;
+            Some((number, sha.to_string()))
+        })
+        .collect();
+    let prs: Vec<(u64, PullRequest)> =
+        github::pull_requests_with_head_prefix(issue, &first_branch)?
+            .into_iter()
+            .filter_map(|pr| Some((branch_number(issue, &pr.head)?, pr)))
+            .collect();
+
+    let highest = on_origin
+        .iter()
+        .map(|(number, _)| *number)
+        .chain(prs.iter().map(|(number, _)| *number))
+        .max();
+    let Some(highest) = highest else {
+        check_local_branch(launch, &first_branch, None)?;
+        return Ok(Selection::Fresh {
+            branch: first_branch,
+        });
+    };
+    let branch = branch_name(issue, highest);
+    // A branch can have had several PRs; the newest one decides.
+    let pr = prs
+        .into_iter()
+        .filter_map(|(number, pr)| (number == highest).then_some(pr))
+        .max_by_key(|pr| pr.number);
+    let origin_sha = on_origin
+        .into_iter()
+        .find_map(|(number, sha)| (number == highest).then_some(sha));
+    match (origin_sha, pr) {
+        (Some(origin_sha), pr) if pr.as_ref().is_none_or(|pr| pr.state == PrState::Open) => {
+            check_local_branch(launch, &branch, Some(&origin_sha))?;
+            Ok(Selection::Continuation { branch, pr })
+        }
+        (_, pr) => bail!(
+            "{branch} can't be continued ({}), and numbered Issue branches are not supported yet",
+            match pr {
+                Some(pr) => format!("its PR {} is {}", pr.url, pr.state),
+                None => "it is gone from origin".to_string(),
+            }
+        ),
+    }
+}
+
+/// 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(launch: &Git, branch: &str, origin_sha: Option<&str>) -> Result<()> {
+    let Ok(local_sha) = launch.run(&[
+        "rev-parse",
+        "--verify",
+        "--quiet",
+        &format!("refs/heads/{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))?;
+    if rest.is_empty() {
+        return Some(1);
+    }
+    let k: u64 = rest.strip_prefix("-branch-")?.parse().ok()?;
+    (k >= 2 && branch == branch_name(issue, k)).then_some(k)
+}
+
+fn branch_name(issue: &IssueUrl, number: u64) -> String {
+    match number {
+        1 => format!("issue-{}", issue.number),
+        k => format!("issue-{}-branch-{k}", issue.number),
+    }
+}

src/github.rs

@@ -1,5 +1,6 @@
 //! Asking GitHub, through `gh`, about issues and pull requests.
 
+use std::fmt;
 use std::process::Command;
 
 use anyhow::{Context, Result, bail};
@@ -17,46 +18,111 @@ pub fn issue_is_open(issue: &IssueUrl) -> Result<bool> {
         &issue.repo_slug(),
         "--json",
         "state",
-    ])?
-    .context("issue not found")?;
-    Ok(string_field(&json, "state")? == "OPEN")
+    ])?;
+    Ok(json["state"].as_str().context("gh output has no state")? == "OPEN")
+}
+
+const PR_FIELDS: &str = "number,url,state,headRefName,baseRefName,isDraft";
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum PrState {
+    Open,
+    Closed,
+    Merged,
+}
+
+impl fmt::Display for PrState {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(match self {
+            PrState::Open => "open",
+            PrState::Closed => "closed",
+            PrState::Merged => "merged",
+        })
+    }
 }
 
 pub struct PullRequest {
+    pub number: u64,
     pub url: String,
-    pub state: String,
+    pub state: PrState,
+    pub head: String,
     pub base: String,
     pub is_draft: bool,
 }
 
 impl PullRequest {
     pub fn is_open(&self) -> bool {
-        self.state == "OPEN"
+        self.state == PrState::Open
+    }
+
+    fn from_json(json: &Value) -> Result<Self> {
+        let field = |name: &str| -> Result<String> {
+            json[name]
+                .as_str()
+                .map(String::from)
+                .with_context(|| format!("gh output has no {name}"))
+        };
+        let state = match field("state")?.as_str() {
+            "OPEN" => PrState::Open,
+            "CLOSED" => PrState::Closed,
+            "MERGED" => PrState::Merged,
+            other => bail!("gh output has an unknown PR state {other}"),
+        };
+        Ok(PullRequest {
+            number: json["number"].as_u64().context("gh output has no number")?,
+            url: field("url")?,
+            state,
+            head: field("headRefName")?,
+            base: field("baseRefName")?,
+            is_draft: json["isDraft"]
+                .as_bool()
+                .context("gh output has no isDraft")?,
+        })
     }
 }
 
 /// The pull request whose head is `branch`, if `gh` finds one.
 pub fn pull_request_for(issue: &IssueUrl, branch: &str) -> Result<Option<PullRequest>> {
-    let json = gh_json(&[
+    let json = match gh_json(&[
         "pr",
         "view",
         branch,
         "--repo",
         &issue.repo_slug(),
         "--json",
-        "url,state,baseRefName,isDraft",
-    ])?;
-    let Some(json) = json else {
-        return Ok(None);
+        PR_FIELDS,
+    ]) {
+        Ok(json) => json,
+        Err(error) if format!("{error:#}").contains("no pull requests found") => return Ok(None),
+        Err(error) => return Err(error),
     };
-    Ok(Some(PullRequest {
-        url: string_field(&json, "url")?,
-        state: string_field(&json, "state")?,
-        base: string_field(&json, "baseRefName")?,
-        is_draft: json["isDraft"]
-            .as_bool()
-            .context("gh output has no isDraft")?,
-    }))
+    PullRequest::from_json(&json).map(Some)
+}
+
+/// Every pull request in this repository, in any state, whose head branch
+/// starts with `prefix`. PRs from forks are left out: their heads are not
+/// this repository's branches.
+pub fn pull_requests_with_head_prefix(issue: &IssueUrl, prefix: &str) -> Result<Vec<PullRequest>> {
+    let json = gh_json(&[
+        "pr",
+        "list",
+        "--repo",
+        &issue.repo_slug(),
+        "--state",
+        "all",
+        "--search",
+        &format!("head:{prefix}"),
+        "--limit",
+        "1000",
+        "--json",
+        &format!("{PR_FIELDS},isCrossRepository"),
+    ])?;
+    json.as_array()
+        .context("gh pr list did not return a list")?
+        .iter()
+        .filter(|pr| pr["isCrossRepository"] != Value::Bool(true))
+        .map(PullRequest::from_json)
+        .collect()
 }
 
 /// Mark the pull request whose head is `branch` ready for review.
@@ -92,29 +158,20 @@ fn gh(args: &[&str]) -> Result<()> {
     Ok(())
 }
 
-/// Run `gh <args>` and parse its JSON output, or `None` if `gh` found no
-/// pull request.
-fn gh_json(args: &[&str]) -> Result<Option<Value>> {
+/// Run `gh <args>` and parse its stdout as JSON, failing with gh's stderr if
+/// it exits non-zero.
+fn gh_json(args: &[&str]) -> Result<Value> {
+    let command = format!("gh {}", args[..2].join(" "));
     let output = Command::new("gh")
         .args(args)
         .output()
         .context("could not run gh")?;
-    let command = format!("gh {}", args.join(" "));
     if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr);
-        if stderr.contains("no pull requests found") {
-            return Ok(None);
-        }
-        bail!("{command} failed: {}", stderr.trim());
+        bail!(
+            "{command} failed: {}",
+            String::from_utf8_lossy(&output.stderr).trim()
+        );
     }
     serde_json::from_slice(&output.stdout)
-        .map(Some)
         .with_context(|| format!("{command} returned invalid JSON"))
 }
-
-fn string_field(json: &Value, name: &str) -> Result<String> {
-    json[name]
-        .as_str()
-        .map(String::from)
-        .with_context(|| format!("gh output has no {name}"))
-}

src/main.rs

@@ -1,3 +1,4 @@
+mod branch;
 mod failed_run;
 mod git;
 mod github;

src/preflight.rs

@@ -7,9 +7,10 @@ use crate::git::Git;
 use crate::github;
 use crate::issue::IssueUrl;
 
-/// Check that a Run on `issue` from the `launch` repository makes sense, and
-/// return its Base branch.
-pub fn check(launch: &Git, issue: &IssueUrl) -> Result<String> {
+/// Check that a Run on `issue` from the `launch` repository makes sense. The
+/// Base branch is checked separately, by [`check_base_branch`], once Issue
+/// branch selection has picked it.
+pub fn check(launch: &Git, issue: &IssueUrl) -> Result<()> {
     let origin = launch.run(&["config", "remote.origin.url"])?;
     if !issue.matches_origin(&origin) {
         bail!(
@@ -25,16 +26,12 @@ pub fn check(launch: &Git, issue: &IssueUrl) -> Result<String> {
             bail!("git {key} is not set; the agent needs it to commit");
         }
     }
-    base_branch(launch)
+    Ok(())
 }
 
-/// The checked-out branch, provided it exists on origin and has no commits
-/// origin lacks.
-fn base_branch(launch: &Git) -> Result<String> {
-    let base = launch.run(&["branch", "--show-current"])?;
-    if base.is_empty() {
-        bail!("HEAD is detached; check out the branch the work should be based on");
-    }
+/// Check that the Base branch `base` exists on origin and that the local
+/// `base`, if any, has no commits origin lacks.
+pub fn check_base_branch(launch: &Git, base: &str) -> Result<()> {
     let on_origin = launch.run(&[
         "ls-remote",
         "--heads",
@@ -44,10 +41,17 @@ fn base_branch(launch: &Git) -> Result<String> {
     if on_origin.is_empty() {
         bail!("base branch {base} does not exist on origin; push it first");
     }
-    launch.run(&["fetch", "origin", &base])?;
+    launch.run(&["fetch", "origin", base])?;
+    let local = format!("refs/heads/{base}");
+    if launch
+        .run(&["rev-parse", "--verify", "--quiet", &local])
+        .is_err()
+    {
+        return Ok(());
+    }
     let ahead = launch.run(&["rev-list", "--count", &format!("origin/{base}..{base}")])?;
     if ahead != "0" {
         bail!("local {base} is {ahead} commit(s) ahead of origin/{base}; push them first");
     }
-    Ok(base)
+    Ok(())
 }