Don't attempt to reopen a PR after updating a branch if it already ex…

…ists

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.

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
+}

internal/engine/actions/remediate/pull_request/pull_request_test.go

@@ -202,6 +202,8 @@ func happyPathMockSetup(mockGitHub *mock_ghclient.MockGitHub) {
 	}, nil)
 	mockGitHub.EXPECT().
 		GetToken().Return("token")
+	mockGitHub.EXPECT().
+		ListPullRequests(gomock.Any(), repoOwner, repoName, gomock.Any()).Return([]*github.PullRequest{}, nil)
 }
 
 func resolveActionMockSetup(t *testing.T, mockGitHub *mock_ghclient.MockGitHub, url, ref string) {
@@ -443,6 +445,38 @@ func TestPullRequestRemediate(t *testing.T) {
 					}, nil)
 			},
 		},
+		{
+			name: "A branch for this PR already exists, shouldn't open a new PR, but only update the branch",
+			newRemArgs: &newPullRequestRemediateArgs{
+				prRem:      dependabotPrRem(),
+				pbuild:     testGithubProviderBuilder(),
+				actionType: TestActionTypeValid,
+			},
+			remArgs:   createTestRemArgs(),
+			repoSetup: defaultMockRepoSetup,
+			mockSetup: func(_ *testing.T, mockGitHub *mock_ghclient.MockGitHub) {
+				// no pull requst so far
+				mockGitHub.EXPECT().
+					ListPullRequests(gomock.Any(), repoOwner, repoName, gomock.Any()).Return([]*github.PullRequest{}, nil)
+				// we need to get the user information and update the branch
+				mockGitHub.EXPECT().
+					GetAuthenticatedUser(gomock.Any()).Return(&github.User{
+					Email: github.String("[email protected]"),
+					Login: github.String("stacklok-bot"),
+				}, nil)
+				mockGitHub.EXPECT().
+					GetToken().Return("token")
+				// this is the last call we expect to make. It returns existing PRs from this branch, so we
+				// stop after having updated the branch
+				mockGitHub.EXPECT().
+					ListPullRequests(gomock.Any(), repoOwner, repoName, gomock.Any()).Return([]*github.PullRequest{
+					// it doesn't matter what we return here, we just need to return a non-empty list
+					{
+						Number: github.Int(1),
+					},
+				}, nil)
+			},
+		},
 		{
 			name: "resolve tags using frizbee",
 			newRemArgs: &newPullRequestRemediateArgs{