fix(retest): acknowledge successful reruns and simplify head matching and test fixtures

Author: ayagmar

README.adoc

@@ -252,6 +252,7 @@ features: [ RETEST_PULL_REQUEST_WORKFLOWS ]
 ----
 
 Only failed jobs from the latest run of each workflow are retriggered. If the latest run is still in progress, the bot asks you to retry once it completes.
+When reruns are started, the bot posts a confirmation comment linking to the workflow runs.
 
 This feature requires:
 

src/main/java/io/quarkus/bot/ApproveWorkflow.java

@@ -21,6 +21,7 @@
 import io.quarkus.bot.config.Feature;
 import io.quarkus.bot.config.QuarkusGitHubBotConfig;
 import io.quarkus.bot.config.QuarkusGitHubBotConfigFile;
+import io.quarkus.bot.util.GHPullRequests;
 import io.quarkus.bot.util.PullRequestFilesMatcher;
 import io.quarkus.cache.CacheKey;
 import io.quarkus.cache.CacheResult;
@@ -90,36 +91,19 @@ private void checkUser(GHEventPayload.WorkflowRun workflowPayload, QuarkusGitHub
     }
 
     private void checkFiles(QuarkusGitHubBotConfigFile quarkusBotConfigFile, GHWorkflowRun workflowRun,
-            ApprovalStatus approval) {
+            ApprovalStatus approval) throws IOException {
         String sha = workflowRun.getHeadSha();
 
-        // Now we want to get the pull request we're supposed to be checking.
-        // It would be nice to use commit.listPullRequests() but that only returns something if the
-        // base and head of the PR are from the same repository, which rules out most scenarios where we would want to do an approval
-
-        String fullyQualifiedBranchName = workflowRun.getHeadRepository().getOwnerName() + ":" + workflowRun.getHeadBranch();
-
-        PagedIterable<GHPullRequest> pullRequestsForThisBranch = workflowRun.getRepository().queryPullRequests()
-                .head(fullyQualifiedBranchName)
-                .list();
-
-        // The number of PRs with matching branch name should be exactly one, but if the PR
-        // has been closed it sometimes disappears from the list; also, if two branch names
-        // start with the same string, both will turn up in the query.
-        for (GHPullRequest pullRequest : pullRequestsForThisBranch) {
-
-            // Only look at PRs whose commit sha matches
-            if (sha.equals(pullRequest.getHead().getSha())) {
-
-                for (QuarkusGitHubBotConfigFile.WorkflowApprovalRule rule : quarkusBotConfigFile.workflows.rules) {
-                    // We allow if the files or directories match the allow rule ...
-                    if (matchRuleFromChangedFiles(pullRequest, rule.allow)) {
-                        approval.shouldApprove = true;
-                    }
-                    // ... unless we also match the unless rule
-                    if (matchRuleFromChangedFiles(pullRequest, rule.unless)) {
-                        approval.shouldNotApprove = true;
-                    }
+        for (GHPullRequest pullRequest : GHPullRequests.matchingHeadPullRequests(workflowRun.getRepository(),
+                workflowRun.getHeadRepository(), workflowRun.getHeadBranch(), sha)) {
+            for (QuarkusGitHubBotConfigFile.WorkflowApprovalRule rule : quarkusBotConfigFile.workflows.rules) {
+                // We allow if the files or directories match the allow rule ...
+                if (matchRuleFromChangedFiles(pullRequest, rule.allow)) {
+                    approval.shouldApprove = true;
+                }
+                // ... unless we also match the unless rule
+                if (matchRuleFromChangedFiles(pullRequest, rule.unless)) {
+                    approval.shouldNotApprove = true;
                 }
             }
         }

src/main/java/io/quarkus/bot/retest/RetestCommand.java

@@ -1,8 +1,10 @@
 package io.quarkus.bot.retest;
 
 import java.io.IOException;
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import jakarta.inject.Inject;
 
@@ -23,6 +25,7 @@
 import io.quarkus.bot.config.Feature;
 import io.quarkus.bot.config.QuarkusGitHubBotConfig;
 import io.quarkus.bot.config.QuarkusGitHubBotConfigFile;
+import io.quarkus.bot.service.GHIssueCommentService;
 
 /**
  * Handles {@code @quarkusbot retest} comments on pull requests.
@@ -43,6 +46,9 @@ class RetestCommand implements RetestCommandHandler {
     @Inject
     FailedJobsRerunner failedJobsRerunner;
 
+    @Inject
+    GHIssueCommentService issueCommentService;
+
     @Override
     public void run(QuarkusGitHubBotConfigFile quarkusBotConfigFile, GHEventPayload.IssueComment issueCommentPayload) {
         if (!Feature.RETEST_PULL_REQUEST_WORKFLOWS.isEnabled(quarkusBotConfigFile)) {
@@ -60,7 +66,7 @@ public void run(QuarkusGitHubBotConfigFile quarkusBotConfigFile, GHEventPayload.
             throw RetestCommandException.noEligibleWorkflowRuns(workflowSelection.noEligibleReason());
         }
 
-        List<Long> startedWorkflowRunIds = new ArrayList<>();
+        List<GHWorkflowRun> startedWorkflowRuns = new ArrayList<>();
         for (GHWorkflowRun workflowRun : workflowSelection.eligibleRuns()) {
             if (quarkusBotConfig.isDryRun()) {
                 LOG.infof("Pull request #%d - Retest failed jobs for workflow run #%d (dry-run)",
@@ -70,15 +76,21 @@ public void run(QuarkusGitHubBotConfigFile quarkusBotConfigFile, GHEventPayload.
 
             try {
                 failedJobsRerunner.rerunFailedJobs(issueCommentPayload, workflowRun);
-                startedWorkflowRunIds.add(workflowRun.getId());
+                startedWorkflowRuns.add(workflowRun);
             } catch (RuntimeException e) {
-                if (startedWorkflowRunIds.isEmpty()) {
+                if (startedWorkflowRuns.isEmpty()) {
                     throw e;
                 }
 
-                throw RetestCommandException.partialRerunFailure(startedWorkflowRunIds, workflowRun.getId(), e);
+                throw RetestCommandException.partialRerunFailure(
+                        startedWorkflowRuns.stream().map(GHWorkflowRun::getId).toList(), workflowRun.getId(), e);
             }
         }
+
+        if (!startedWorkflowRuns.isEmpty()) {
+            issueCommentService.addComment(issueCommentPayload.getIssue(), successMessage(startedWorkflowRuns), false,
+                    quarkusBotConfig.isDryRun());
+        }
     }
 
     private static GHPullRequest getPullRequest(GHEventPayload.IssueComment issueCommentPayload) {
@@ -97,4 +109,27 @@ private RetestWorkflowSelection getWorkflowSelection(GHPullRequest pullRequest)
             throw RetestCommandException.unableToInspectWorkflowRuns(e);
         }
     }
+
+    private static String successMessage(List<GHWorkflowRun> startedWorkflowRuns) {
+        String workflowRunLinks = startedWorkflowRuns.stream()
+                .map(RetestCommand::workflowRunReference)
+                .collect(Collectors.joining(", "));
+        String label = startedWorkflowRuns.size() == 1 ? "workflow run " : "workflow runs ";
+        return ":arrows_counterclockwise: Retest started for failed jobs in " + label + workflowRunLinks + ".";
+    }
+
+    private static String workflowRunReference(GHWorkflowRun workflowRun) {
+        String label = "#" + workflowRun.getId();
+        URL htmlUrl;
+        try {
+            htmlUrl = workflowRun.getHtmlUrl();
+        } catch (IOException e) {
+            return label;
+        }
+        if (htmlUrl == null) {
+            return label;
+        }
+
+        return "[" + label + "](" + htmlUrl + ")";
+    }
 }

src/main/java/io/quarkus/bot/retest/RetestExecutionErrorHandler.java

@@ -7,8 +7,8 @@
 import org.kohsuke.github.GHEventPayload;
 
 import io.quarkiverse.githubapp.command.airline.ExecutionErrorHandler;
-import io.quarkus.bot.GHIssueCommentService;
 import io.quarkus.bot.config.QuarkusGitHubBotConfig;
+import io.quarkus.bot.service.GHIssueCommentService;
 
 /**
  * Formats command failures as pull-request comments.

src/main/java/io/quarkus/bot/retest/RetestParseErrorHandler.java

@@ -18,10 +18,10 @@
 import io.quarkiverse.githubapp.ConfigFile;
 import io.quarkiverse.githubapp.GitHubConfigFileProvider;
 import io.quarkiverse.githubapp.command.airline.ParseErrorHandler;
-import io.quarkus.bot.GHIssueCommentService;
 import io.quarkus.bot.config.Feature;
 import io.quarkus.bot.config.QuarkusGitHubBotConfig;
 import io.quarkus.bot.config.QuarkusGitHubBotConfigFile;
+import io.quarkus.bot.service.GHIssueCommentService;
 
 /**
  * Dry-run-aware parse error handler for retest commands.

src/main/java/io/quarkus/bot/retest/RetestWorkflowRunSelector.java

@@ -20,6 +20,8 @@
 import org.kohsuke.github.GHWorkflowRun;
 import org.kohsuke.github.PagedIterable;
 
+import io.quarkus.bot.util.GHPullRequests;
+
 /**
  * Selects the latest rerunnable workflow runs for a pull request head.
  */
@@ -48,8 +50,9 @@ RetestWorkflowSelection selectWorkflowRuns(GHPullRequest pullRequest) throws IOE
     private static PullRequestHeadContext pullRequestHeadContext(GHPullRequest pullRequest) throws IOException {
         GHCommitPointer pullRequestHead = pullRequest.getHead();
         GHRepository pullRequestHeadRepository = pullRequestHead.getRepository();
-        boolean ambiguousPullRequestHead = matchingPullRequestsForHead(pullRequest, pullRequestHead,
-                pullRequestHeadRepository).size() > 1;
+        boolean ambiguousPullRequestHead = pullRequestHeadRepository != null
+                && GHPullRequests.matchingHeadPullRequests(pullRequest.getRepository(), pullRequestHeadRepository,
+                        pullRequestHead.getRef(), pullRequestHead.getSha(), GHIssueState.ALL).size() > 1;
         return new PullRequestHeadContext(pullRequestHead, pullRequestHeadRepository, ambiguousPullRequestHead);
     }
 
@@ -127,7 +130,7 @@ private static boolean matchesPullRequestHead(GHWorkflowRun workflowRun, PullReq
         if (!headContext.pullRequestHead().getRef().equals(workflowRun.getHeadBranch())) {
             return false;
         }
-        return isSameRepository(headContext.pullRequestHeadRepository(), workflowRun.getHeadRepository());
+        return GHPullRequests.isSameRepository(headContext.pullRequestHeadRepository(), workflowRun.getHeadRepository());
     }
 
     private static boolean isAssociatedWithCurrentPullRequest(List<GHPullRequest> associatedPullRequests,
@@ -221,50 +224,6 @@ private static boolean hasFailedLatestJob(GHWorkflowRun workflowRun) {
         return false;
     }
 
-    private static boolean isSameRepository(GHRepository left, GHRepository right) {
-        if (left == null || right == null) {
-            return false;
-        }
-
-        String leftFullName = left.getFullName();
-        String rightFullName = right.getFullName();
-        if (leftFullName == null || rightFullName == null) {
-            return false;
-        }
-
-        return leftFullName.equals(rightFullName);
-    }
-
-    private static List<GHPullRequest> matchingPullRequestsForHead(GHPullRequest pullRequest,
-            GHCommitPointer pullRequestHead,
-            GHRepository pullRequestHeadRepository) throws IOException {
-        if (pullRequestHeadRepository == null) {
-            return List.of(pullRequest);
-        }
-
-        String fullyQualifiedBranchName = pullRequestHeadRepository.getOwnerName() + ":" + pullRequestHead.getRef();
-        List<GHPullRequest> matchingPullRequests = new ArrayList<>();
-
-        for (GHPullRequest candidatePullRequest : pullRequest.getRepository().queryPullRequests()
-                .state(GHIssueState.ALL)
-                .head(fullyQualifiedBranchName)
-                .list()) {
-            if (!pullRequestHead.getRef().equals(candidatePullRequest.getHead().getRef())) {
-                continue;
-            }
-            if (!pullRequestHead.getSha().equals(candidatePullRequest.getHead().getSha())) {
-                continue;
-            }
-            if (!isSameRepository(pullRequestHeadRepository, candidatePullRequest.getHead().getRepository())) {
-                continue;
-            }
-
-            matchingPullRequests.add(candidatePullRequest);
-        }
-
-        return matchingPullRequests;
-    }
-
     private static String workflowIdentity(GHWorkflowRun workflowRun, String event) {
         if (workflowRun.getWorkflowId() > 0) {
             return "workflow-id:" + workflowRun.getWorkflowId() + ":event:" + event;
@@ -275,7 +234,8 @@ private static String workflowIdentity(GHWorkflowRun workflowRun, String event)
             return "workflow-url:" + workflowUrl + ":event:" + event;
         }
 
-        return event + "::" + workflowRun.getName() + "::" + workflowRun.getRunNumber();
+        throw RetestCommandException.unableToInspectWorkflowRuns(
+                new IllegalStateException("Workflow run #" + workflowRun.getId() + " is missing workflow identity."));
     }
 
     private record PullRequestHeadContext(GHCommitPointer pullRequestHead, GHRepository pullRequestHeadRepository,

src/main/java/io/quarkus/bot/util/GHPullRequests.java

@@ -1,9 +1,14 @@
 package io.quarkus.bot.util;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.regex.Pattern;
 
+import org.kohsuke.github.GHIssueState;
 import org.kohsuke.github.GHLabel;
 import org.kohsuke.github.GHPullRequest;
+import org.kohsuke.github.GHRepository;
 
 public final class GHPullRequests {
 
@@ -18,6 +23,29 @@ public static boolean hasLabel(GHPullRequest pullRequest, String labelName) {
         return false;
     }
 
+    public static List<GHPullRequest> matchingHeadPullRequests(GHRepository repository, GHRepository headRepository,
+            String headRef, String headSha) throws IOException {
+        if (repository == null || headRepository == null || headRef == null || headSha == null) {
+            return List.of();
+        }
+
+        return matchingHeadPullRequests(repository.queryPullRequests()
+                .head(headRepository.getOwnerName() + ":" + headRef)
+                .list(), headRepository, headRef, headSha);
+    }
+
+    public static List<GHPullRequest> matchingHeadPullRequests(GHRepository repository, GHRepository headRepository,
+            String headRef, String headSha, GHIssueState state) throws IOException {
+        if (repository == null || headRepository == null || headRef == null || headSha == null) {
+            return List.of();
+        }
+
+        return matchingHeadPullRequests(repository.queryPullRequests()
+                .state(state)
+                .head(headRepository.getOwnerName() + ":" + headRef)
+                .list(), headRepository, headRef, headSha);
+    }
+
     public static String dropVersionSuffix(String title, String branch) {
         if (title == null || title.isBlank()) {
             return title;
@@ -39,4 +67,47 @@ public static String normalizeTitle(String title, String branch) {
 
         return "[" + branch + "] " + dropVersionSuffix(title, branch);
     }
+
+    private static List<GHPullRequest> matchingHeadPullRequests(Iterable<GHPullRequest> pullRequests,
+            GHRepository headRepository, String headRef, String headSha) {
+        List<GHPullRequest> matchingPullRequests = new ArrayList<>();
+
+        for (GHPullRequest pullRequest : pullRequests) {
+            if (!headRef.equals(pullRequest.getHead().getRef())) {
+                continue;
+            }
+            if (!headSha.equals(pullRequest.getHead().getSha())) {
+                continue;
+            }
+            if (!isSameRepository(headRepository, pullRequest.getHead().getRepository())) {
+                continue;
+            }
+
+            matchingPullRequests.add(pullRequest);
+        }
+
+        return matchingPullRequests;
+    }
+
+    public static boolean isSameRepository(GHRepository left, GHRepository right) {
+        if (left == null || right == null) {
+            return false;
+        }
+
+        String leftFullName = left.getFullName();
+        String rightFullName = right.getFullName();
+        if (leftFullName != null && rightFullName != null) {
+            return leftFullName.equals(rightFullName);
+        }
+
+        String leftOwner = left.getOwnerName();
+        String rightOwner = right.getOwnerName();
+        String leftName = left.getName();
+        String rightName = right.getName();
+        if (leftOwner == null || rightOwner == null || leftName == null || rightName == null) {
+            return false;
+        }
+
+        return leftOwner.equals(rightOwner) && leftName.equals(rightName);
+    }
 }