From c3a3e17abb60cfc9ad339b2a416019c018f049e3 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jakub@stacklok.com>
Date: Fri, 15 Dec 2023 15:43:59 +0100
Subject: [PATCH] Don't attempt to reopen a PR after updating a branch if it
 already exists

The PR remediation was checking if a PR with the same contents already
exist as a fast and cheap check to avoid triggering the remediation
again. But in case the profile changes or the contents change for
another reason, we want to push to the existing branch to update it, but
don't try to open a PR again as that would fail with 422.

To do that, this patch adds a check if a PR is already open from the
branch we just pushed to and if it is, just returns before opening the
PR again.
---
 .../remediate/pull_request/pull_request.go    | 37 +++++++++++++++++--
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/internal/engine/actions/remediate/pull_request/pull_request.go b/internal/engine/actions/remediate/pull_request/pull_request.go
index 3384dc0d64..6330aad4ef 100644
--- a/internal/engine/actions/remediate/pull_request/pull_request.go
+++ b/internal/engine/actions/remediate/pull_request/pull_request.go
@@ -207,7 +207,7 @@ func (r *Remediator) Do(
 	var remErr error
 	switch remAction {
 	case interfaces.ActionOptOn:
-		alreadyExists, err := prAlreadyExists(ctx, r.ghCli, repo, magicComment)
+		alreadyExists, err := prWithContentAlreadyExists(ctx, r.ghCli, repo, magicComment)
 		if err != nil {
 			return nil, fmt.Errorf("cannot check if PR already exists: %w", err)
 		}
@@ -319,6 +319,19 @@ func (r *Remediator) runGit(
 	}
 	zerolog.Ctx(ctx).Debug().Msgf("Push output: %s", b.String())
 
+	// if a PR from this branch already exists, don't create a new one
+	// this handles the case where the content changed (e.g. profile changed)
+	// but the PR was not closed
+	prAlreadyExists, err := prFromBranchAlreadyExists(ctx, r.ghCli, pbRepo, branchBaseName(title))
+	if err != nil {
+		return fmt.Errorf("cannot check if PR from branch already exists: %w", err)
+	}
+
+	if prAlreadyExists {
+		zerolog.Ctx(ctx).Info().Msg("PR from branch already exists, won't create a new one")
+		return nil
+	}
+
 	_, err = r.ghCli.CreatePullRequest(
 		ctx, pbRepo.GetOwner(), pbRepo.GetName(),
 		title, body,
@@ -434,13 +447,12 @@ func createReviewBody(prText, magicComment string) (string, error) {
 }
 
 // returns true if an open PR with the magic comment already exists
-func prAlreadyExists(
+func prWithContentAlreadyExists(
 	ctx context.Context,
 	cli provifv1.GitHub,
 	repo *pb.Repository,
 	magicComment string,
 ) (bool, error) {
-	// TODO(jakub): pagination
 	openPrs, err := cli.ListPullRequests(ctx, repo.GetOwner(), repo.GetName(), &github.PullRequestListOptions{})
 	if err != nil {
 		return false, fmt.Errorf("cannot list pull requests: %w", err)
@@ -453,3 +465,22 @@ func prAlreadyExists(
 	}
 	return false, nil
 }
+
+func prFromBranchAlreadyExists(
+	ctx context.Context,
+	cli provifv1.GitHub,
+	repo *pb.Repository,
+	branchName string,
+) (bool, error) {
+	// TODO(jakub): pagination
+	opts := &github.PullRequestListOptions{
+		Head: branchName,
+	}
+
+	openPrs, err := cli.ListPullRequests(ctx, repo.GetOwner(), repo.GetName(), opts)
+	if err != nil {
+		return false, fmt.Errorf("cannot list pull requests: %w", err)
+	}
+
+	return len(openPrs) > 0, nil
+}