Releasing a Claim, and removing it when thirdshift closes the issue

CONTEXT.md

@@ -121,7 +121,7 @@ Writing the **User config** by answering a few questions, one per setting that m
 _Avoid_: init, onboarding, configure
 
 **Self-merge**:
-The step at the end of a **Merge run** in which thirdshift itself merges the pull request into the **Base branch** with a merge commit, once it is open, ready for review, mergeable and green on the head commit it merges. No human reviews it first. It ends with the **Issue branch** deleted and the issue closed by thirdshift, unless it is already closed.
+The step at the end of a **Merge run** in which thirdshift itself merges the pull request into the **Base branch** with a merge commit, once it is open, ready for review, mergeable and green on the head commit it merges. No human reviews it first. It ends with the **Issue branch** deleted, the issue closed by thirdshift, unless it is already closed, and the issue's **Claim** removed.
 
 **Policy refusal**:
 A merge the **Self-merge** tried that failed, where the round of the Repair loop that followed found nothing to fix: the **Base branch** unchanged, no conflict, CI green or absent, the pull request mergeable. The cause is a repository setting or rule, such as merge commits disallowed or a review required. thirdshift never reads GitHub's error text to decide it. The **Merge run** is a **Failed run** that leaves the pull request ready for review.

src/claim.rs

@@ -1,10 +1,14 @@
 //! The Claim: the mark that the factory has taken an issue, the label
 //! `in-progress` in place of `ready-for-agent`, so the issue list shows what a
-//! Run or a Spec run is working on.
+//! Run or a Spec run is working on. It is released when that run ends with
+//! nothing on origin to take over, and removed once its Self-merge has left
+//! the issue closed.
 
 use anyhow::{Context, Result};
 
+use crate::git::Git;
 use crate::github;
+use crate::interrupt;
 use crate::issue::IssueUrl;
 use crate::progress;
 use crate::spec_run::READY_FOR_AGENT;
@@ -16,16 +20,21 @@ const IN_PROGRESS: &str = "in-progress";
 /// if the repository lacks it.
 const IN_PROGRESS_DESCRIPTION: &str = "A Claim: thirdshift has taken this issue";
 
+/// The Claim a Run or a Spec run made on its issue, with what making it
+/// changed, which is what releasing it puts back.
+pub struct Claim<'a> {
+    issue: &'a IssueUrl,
+    /// Whether making it added `in-progress`: the issue was not Claimed
+    /// already.
+    added_in_progress: bool,
+    /// Whether making it took `ready-for-agent` off the issue.
+    removed_ready_for_agent: bool,
+}
+
 /// Whether an issue with `labels` carries a Claim, whatever else it is
 /// labelled.
 pub fn is_on(labels: &[String]) -> bool {
-    has(labels, IN_PROGRESS)
-}
-
-/// Whether `label` is one of `labels`, whatever its case: GitHub's label
-/// names are case-insensitive.
-fn has(labels: &[String], label: &str) -> bool {
-    labels.iter().any(|name| name.eq_ignore_ascii_case(label))
+    has_label(labels, IN_PROGRESS)
 }
 
 /// Make the Claim on `issue`: label it `in-progress`, in place of
@@ -34,17 +43,22 @@ fn has(labels: &[String], label: &str) -> bool {
 /// issue already Claimed, `in-progress` and not `ready-for-agent`, is left as
 /// it is, with no request made. A failure names the Claim as what could not
 /// be made.
-pub fn make(issue: &IssueUrl) -> Result<()> {
+pub fn make(issue: &IssueUrl) -> Result<Claim<'_>> {
     label_in_progress(issue)
         .with_context(|| format!("could not make the Claim on #{}", issue.number))
 }
 
 /// [`make`], its failure as `gh` gave it.
-fn label_in_progress(issue: &IssueUrl) -> Result<()> {
+fn label_in_progress(issue: &IssueUrl) -> Result<Claim<'_>> {
     let mut labels = github::issue_labels(issue)?;
-    let (claimed, ready) = (is_on(&labels), has(&labels, READY_FOR_AGENT));
+    let (claimed, ready) = (is_on(&labels), has_label(&labels, READY_FOR_AGENT));
+    let claim = Claim {
+        issue,
+        added_in_progress: !claimed,
+        removed_ready_for_agent: ready,
+    };
     if claimed && !ready {
-        return Ok(());
+        return Ok(claim);
     }
     if ready {
         progress::step(format_args!(
@@ -61,5 +75,125 @@ fn label_in_progress(issue: &IssueUrl) -> Result<()> {
         )?;
     }
     labels.retain(|name| !name.eq_ignore_ascii_case(READY_FOR_AGENT));
-    github::set_labels_adding(issue, &labels, &[IN_PROGRESS])
+    github::set_labels_adding(issue, &labels, &[IN_PROGRESS])?;
+    Ok(claim)
+}
+
+/// Whether `label` is one of `labels`, whatever its case: GitHub's label
+/// names are case-insensitive.
+fn has_label(labels: &[String], label: &str) -> bool {
+    labels.iter().any(|name| name.eq_ignore_ascii_case(label))
+}
+
+impl Claim<'_> {
+    /// Release the Claim if the Run or the Spec run that made it ended with
+    /// nothing on origin to take over: neither `branch`, its Issue branch or
+    /// Spec branch, which `launch` asks origin for, nor a pull request from
+    /// it. The issue's labels then go back as they were before the Claim:
+    /// `in-progress` comes off if the Claim added it, and `ready-for-agent`
+    /// goes back if the Claim took it off, in one request that keeps the
+    /// issue's other labels, those added since included. An issue that is no
+    /// longer `in-progress` is left as it is: someone took the Claim off
+    /// meanwhile. A Claim that changed no label has nothing to put back, and
+    /// makes no request.
+    ///
+    /// The run has ended as it has, so this never fails: a failure is a
+    /// warning naming what to run by hand, and an interrupt doesn't stop it.
+    pub fn release_if_nothing_on_origin(&self, launch: &Git, branch: &str) {
+        if !self.added_in_progress && !self.removed_ready_for_agent {
+            return;
+        }
+        if let Err(error) = interrupt::retry_if_interrupted(|| {
+            self.put_labels_back_unless_on_origin(launch, branch)
+        }) {
+            progress::warn(
+                &error,
+                format_args!(
+                    "could not release the Claim on #{}, so if nothing of the run is on origin, \
+                     release it by hand: {}",
+                    self.issue.number,
+                    self.release_by_hand()
+                ),
+            );
+        }
+    }
+
+    /// [`Claim::release_if_nothing_on_origin`], its failure as `git` or `gh`
+    /// gave it.
+    fn put_labels_back_unless_on_origin(&self, launch: &Git, branch: &str) -> Result<()> {
+        let issue = self.issue;
+        if launch.on_origin(branch)? || github::pull_request_for(issue, branch)?.is_some() {
+            return Ok(());
+        }
+        let mut labels = github::issue_labels(issue)?;
+        if !is_on(&labels) {
+            return Ok(());
+        }
+        let number = issue.number;
+        if !self.removed_ready_for_agent {
+            progress::step(format_args!(
+                "releasing the Claim on #{number}: removing {IN_PROGRESS}"
+            ));
+            return github::remove_label(issue, IN_PROGRESS);
+        }
+        if self.added_in_progress {
+            progress::step(format_args!(
+                "releasing the Claim on #{number}: labelling it {READY_FOR_AGENT}, \
+                 in place of {IN_PROGRESS}"
+            ));
+            labels.retain(|name| !name.eq_ignore_ascii_case(IN_PROGRESS));
+        } else {
+            progress::step(format_args!(
+                "releasing the Claim on #{number}: labelling it {READY_FOR_AGENT} again"
+            ));
+        }
+        github::set_labels_adding(issue, &labels, &[READY_FOR_AGENT])
+    }
+
+    /// Remove the Claim once the Self-merge of the Run or the Spec run that
+    /// made it has left its issue closed: `in-progress` comes off, in one
+    /// request that keeps the issue's other labels. An issue still open, as
+    /// when closing it failed, keeps it, and one no longer `in-progress` is
+    /// left as it is.
+    ///
+    /// Like the other steps after a merge, this never fails: a failure is a
+    /// warning naming the command to run by hand, and an interrupt doesn't
+    /// stop it.
+    pub fn remove_if_closed(&self) {
+        if let Err(error) = interrupt::retry_if_interrupted(|| self.unlabel_if_closed()) {
+            progress::warn(
+                &error,
+                format_args!(
+                    "could not remove {IN_PROGRESS} from issue #{}, so remove it by hand: {}",
+                    self.issue.number,
+                    github::remove_label_command(self.issue, IN_PROGRESS)
+                ),
+            );
+        }
+    }
+
+    /// [`Claim::remove_if_closed`], its failure as `gh` gave it.
+    fn unlabel_if_closed(&self) -> Result<()> {
+        let issue = github::issue(self.issue)?;
+        if issue.is_open || !is_on(&issue.labels) {
+            return Ok(());
+        }
+        progress::step(format_args!(
+            "removing {IN_PROGRESS} from issue #{}",
+            self.issue.number
+        ));
+        github::remove_label(self.issue, IN_PROGRESS)
+    }
+
+    /// The commands that release the Claim by hand.
+    fn release_by_hand(&self) -> String {
+        let mut commands = Vec::new();
+        if self.added_in_progress {
+            commands.push(github::remove_label_command(self.issue, IN_PROGRESS));
+        }
+        if self.removed_ready_for_agent {
+            commands.push(github::add_label_command(self.issue, READY_FOR_AGENT));
+        }
+        commands.join(" && ")
+    }
 }

src/git.rs

@@ -65,6 +65,13 @@ impl Git {
         Ok(file)
     }
 
+    /// Whether origin has the branch `branch`, as origin itself answers.
+    pub fn on_origin(&self, branch: &str) -> Result<bool> {
+        let reference = format!("refs/heads/{branch}");
+        let found = self.run(&["ls-remote", "--heads", "origin", &reference])?;
+        Ok(!found.is_empty())
+    }
+
     /// Run `git <args>` and report whether it exited zero, for commands whose
     /// exit status is the answer.
     pub fn succeeds(&self, args: &[&str]) -> Result<bool> {

src/github.rs

@@ -43,7 +43,7 @@ pub fn issue_title(issue: &IssueUrl) -> Result<String> {
 }
 
 /// An issue as an Architect run reads the plan its Architecture review
-/// published.
+/// published, and as a Claim reads its issue after a Self-merge.
 pub struct Issue {
     pub is_open: bool,
     pub labels: Vec<String>,
@@ -80,11 +80,16 @@ pub fn issue_labels(issue: &IssueUrl) -> Result<Vec<String>> {
     label_names(&issue_view(issue, "labels")?)
 }
 
+/// The REST API's path for the labels of `issue`.
+fn labels_path(issue: &IssueUrl) -> String {
+    format!("repos/{}/issues/{}/labels", issue.repo_slug(), issue.number)
+}
+
 /// Set `issue`'s labels to exactly `labels`, in one request, so a swap of
 /// one label for another can't stop halfway. Through the REST API: `gh issue
 /// edit` fails on the GitHub Projects (classic) sunset in older `gh`.
 fn set_labels(issue: &IssueUrl, labels: &[&str]) -> Result<()> {
-    let path = format!("repos/{}/issues/{}/labels", issue.repo_slug(), issue.number);
+    let path = labels_path(issue);
     let fields: Vec<String> = labels
         .iter()
         .map(|label| format!("labels[]={label}"))
@@ -110,6 +115,38 @@ pub fn set_labels_adding(issue: &IssueUrl, kept: &[String], added: &[&str]) -> R
     set_labels(issue, &labels)
 }
 
+/// The REST API's path for `label` on `issue`.
+fn label_path(issue: &IssueUrl, label: &str) -> String {
+    format!("{}/{label}", labels_path(issue))
+}
+
+/// Take `label` off `issue`, in one request that keeps its other labels.
+/// Through the REST API, as [`set_labels`] is. GitHub refuses it if the issue
+/// does not have the label.
+pub fn remove_label(issue: &IssueUrl, label: &str) -> Result<()> {
+    gh(&[
+        "api",
+        "--method",
+        "DELETE",
+        &label_path(issue, label),
+        "--silent",
+    ])
+}
+
+/// The command that does what [`remove_label`] does, to run by hand.
+pub fn remove_label_command(issue: &IssueUrl, label: &str) -> String {
+    format!("gh api --method DELETE {}", label_path(issue, label))
+}
+
+/// The command that adds `label` to `issue`, keeping its other labels, to
+/// run by hand: unlike [`set_labels`], it needs none of the others named.
+pub fn add_label_command(issue: &IssueUrl, label: &str) -> String {
+    format!(
+        "gh api --method POST {} -f 'labels[]={label}'",
+        labels_path(issue)
+    )
+}
+
 /// A Spec's sub-issue, as a Spec run reads it.
 pub struct Ticket {
     pub number: u64,

src/interrupt.rs

@@ -27,6 +27,12 @@ pub fn requested() -> bool {
         .is_some_and(|flag| flag.load(Ordering::SeqCst))
 }
 
+/// Run `step`, and once more if it failed with the Run interrupted: Ctrl-C in
+/// a terminal also kills the git or gh the step was running.
+pub fn retry_if_interrupted(step: impl Fn() -> Result<()>) -> Result<()> {
+    step().or_else(|error| if requested() { step() } else { Err(error) })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/main.rs

@@ -86,7 +86,11 @@ A Run or a Spec run makes the Claim on its issue once its checks pass, before an
 labels the issue in-progress, in place of ready-for-agent if it has that, keeping its other
 labels and creating the label if the repository lacks it. A Ticket's Run in a Spec run and a
 Base fix make none, and an issue already in-progress is left as it is. A Run whose Claim
-can't be made stops there, naming the cause.
+can't be made stops there, naming the cause. The Claim is released, the issue's labels put
+back as they were, when the Run or the Spec run fails with nothing on origin to take over:
+no Issue branch or Spec branch and no pull request. It is removed once a Self-merge has left
+the issue closed, and otherwise stays: a failure to release or remove it is a warning naming
+the command to run by hand.
 
 On a Spec, an issue with sub-issues, the Run is a Spec run: it takes every Ticket (sub-issue) it
 can reach, in the order their \"blocked by\" links allow, each merged into the Spec branch. Its Spec

src/progress.rs

@@ -24,6 +24,12 @@ pub fn step(message: impl Display) {
     write_line(Local::now().format(STAMP), message);
 }
 
+/// Report `error`, then a warning saying what to do about it by hand.
+pub fn warn(error: &anyhow::Error, warning: std::fmt::Arguments) {
+    step(format_args!("{error:#}"));
+    step(format_args!("warning: {warning}"));
+}
+
 /// Print `line`, from the stderr of a child thirdshift, under `label`,
 /// keeping the time the child stamped it with (or stamping it now, if it
 /// has none), and return its message: the line without prefix or time.