From 24cb5c955f7bb777e8d984b71c7ec4a5c6f574ee Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Fri, 11 Sep 2026 09:26:46 -0400 Subject: [PATCH 1/3] fix(pr): wait until auto-merge is allowed --- cmd/wait-for-github/pr.go | 10 ++++++ cmd/wait-for-github/pr_test.go | 45 +++++++++++++++++++++++---- internal/github/github.go | 18 +++++++++++ internal/github/github_test.go | 57 ++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 6 deletions(-) diff --git a/cmd/wait-for-github/pr.go b/cmd/wait-for-github/pr.go index 9ab13bb1..916a3b92 100644 --- a/cmd/wait-for-github/pr.go +++ b/cmd/wait-for-github/pr.go @@ -154,6 +154,7 @@ type commitInfo struct { type checkMergedAndOverallCI interface { github.CheckPRMerged github.GetPRHeadSHA + github.CheckPRMergeable github.CheckOverallCIStatus github.RerunFailedWorkflows github.MergePR @@ -227,6 +228,15 @@ func (pr *prCheck) Check(ctx context.Context) error { } if pr.autoMerge && status == github.CIStatusPassed { + mergeable, mergeableState, err := pr.githubClient.IsPRMergeable(ctx, pr.owner, pr.repo, pr.pr) + if err != nil { + return err + } + if !mergeable { + pr.logger.InfoContext(ctx, "CI passed but PR is not mergeable yet", "mergeable_state", mergeableState) + return nil + } + pr.logger.InfoContext(ctx, "CI passed and auto-merge is enabled, merging PR", "method", pr.autoMergeMethod) // Pass sha to prevent merging a different commit than the one CI ran on. // Merge failures (conflicts, branch protection) are retried on each poll; diff --git a/cmd/wait-for-github/pr_test.go b/cmd/wait-for-github/pr_test.go index 05a101f2..1895b3e1 100644 --- a/cmd/wait-for-github/pr_test.go +++ b/cmd/wait-for-github/pr_test.go @@ -38,11 +38,15 @@ type fakeGithubClientPRCheck struct { isPRMergedError error getPRHeadSHAError error + checkPRMergeableError error getCIStatusError error rerunFailedWorkflowsError error mergePRError error CIStatus github.CIStatus + Mergeable bool + MergeableState string + MergeableCalledCount int RerunCount int HasRunsInProgress bool RerunCalledCount int @@ -62,6 +66,11 @@ func (fg *fakeGithubClientPRCheck) GetPRHeadSHA(ctx context.Context, owner, repo return fg.MergedCommit, fg.getPRHeadSHAError } +func (fg *fakeGithubClientPRCheck) IsPRMergeable(ctx context.Context, owner, repo string, pr int) (bool, string, error) { + fg.MergeableCalledCount++ + return fg.Mergeable, fg.MergeableState, fg.checkPRMergeableError +} + func (fg *fakeGithubClientPRCheck) GetCIStatus(ctx context.Context, owner, repo string, commitHash string, excludes []string) (github.CIStatus, error) { return fg.CIStatus, fg.getCIStatusError } @@ -188,8 +197,9 @@ func TestPRCheck(t *testing.T) { { name: "CI passed with auto-merge squash, merge succeeds", fakeClient: fakeGithubClientPRCheck{ - HeadSHA: "abc123", - CIStatus: github.CIStatusPassed, + HeadSHA: "abc123", + CIStatus: github.CIStatusPassed, + Mergeable: true, }, autoMerge: true, autoMergeMethod: "squash", @@ -201,8 +211,9 @@ func TestPRCheck(t *testing.T) { { name: "CI passed with auto-merge rebase, merge succeeds", fakeClient: fakeGithubClientPRCheck{ - HeadSHA: "abc123", - CIStatus: github.CIStatusPassed, + HeadSHA: "abc123", + CIStatus: github.CIStatusPassed, + Mergeable: true, }, autoMerge: true, autoMergeMethod: "rebase", @@ -213,8 +224,9 @@ func TestPRCheck(t *testing.T) { { name: "CI passed with auto-merge, default method", fakeClient: fakeGithubClientPRCheck{ - HeadSHA: "abc123", - CIStatus: github.CIStatusPassed, + HeadSHA: "abc123", + CIStatus: github.CIStatusPassed, + Mergeable: true, }, autoMerge: true, autoMergeMethod: "merge", @@ -227,6 +239,7 @@ func TestPRCheck(t *testing.T) { fakeClient: fakeGithubClientPRCheck{ HeadSHA: "abc123", CIStatus: github.CIStatusPassed, + Mergeable: true, mergePRError: fmt.Errorf("merge failed"), }, autoMerge: true, @@ -236,6 +249,26 @@ func TestPRCheck(t *testing.T) { expectMergeMethod: "squash", // No exit code - continues polling and will retry }, + { + name: "CI passed with auto-merge, PR is blocked", + fakeClient: fakeGithubClientPRCheck{ + HeadSHA: "abc123", + CIStatus: github.CIStatusPassed, + MergeableState: "blocked", + }, + autoMerge: true, + autoMergeMethod: "merge", + }, + { + name: "CI passed with auto-merge, mergeability check fails", + fakeClient: fakeGithubClientPRCheck{ + HeadSHA: "abc123", + CIStatus: github.CIStatusPassed, + checkPRMergeableError: fmt.Errorf("mergeability check failed"), + }, + autoMerge: true, + autoMergeMethod: "merge", + }, { name: "CI pending with auto-merge, no merge attempt", fakeClient: fakeGithubClientPRCheck{ diff --git a/internal/github/github.go b/internal/github/github.go index 21f372b8..8adaf8b0 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -53,6 +53,10 @@ type GetPRHeadSHA interface { GetPRHeadSHA(ctx context.Context, owner, repo string, pr int) (string, error) } +type CheckPRMergeable interface { + IsPRMergeable(ctx context.Context, owner, repo string, pr int) (bool, string, error) +} + type CheckOverallCIStatus interface { GetCIStatus(ctx context.Context, owner, repo string, commitHash string, excludes []string) (CIStatus, error) } @@ -438,6 +442,20 @@ func (c GHClient) GetPRHeadSHA(ctx context.Context, owner, repo string, prNumber return pr.GetHead().GetSHA(), nil } +func (c GHClient) IsPRMergeable(ctx context.Context, owner, repo string, prNumber int) (bool, string, error) { + pr, resp, err := c.client.PullRequests.Get(ctx, owner, repo, prNumber) + if err != nil { + return false, "", fmt.Errorf("failed to query GitHub for PR mergeability: %w", err) + } + + if respErr := c.handleResponseError(resp, "GetPullRequest", owner, repo); respErr != nil { + return false, "", respErr + } + + mergeableState := pr.GetMergeableState() + return pr.GetMergeable() && mergeableState == "clean", mergeableState, nil +} + func (c GHClient) MergePR(ctx context.Context, owner, repo string, prNumber int, sha, mergeMethod string) error { result, resp, err := c.client.PullRequests.Merge(ctx, owner, repo, prNumber, "", &github.PullRequestOptions{ SHA: sha, diff --git a/internal/github/github_test.go b/internal/github/github_test.go index dc49fc5d..93eda073 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -343,6 +343,63 @@ func TestIsPRMergedOrClosed_Error(t *testing.T) { require.Error(t, err) } +func TestIsPRMergeable(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + githubMergeable bool + mergeableState string + expectedMergeable bool + }{ + { + name: "clean", + githubMergeable: true, + mergeableState: "clean", + expectedMergeable: true, + }, + { + name: "blocked by branch protection", + githubMergeable: true, + mergeableState: "blocked", + }, + { + name: "merge conflict", + githubMergeable: false, + mergeableState: "dirty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockedHTTPClient := mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposPullsByOwnerByRepoByPullNumber, + github.PullRequest{ + Mergeable: github.Bool(tt.githubMergeable), + MergeableState: github.String(tt.mergeableState), + }, + ), + ) + + ghClient := newClientFromMock(t, mockedHTTPClient, "") + mergeable, state, err := ghClient.IsPRMergeable(context.Background(), "owner", "repo", 1) + + require.NoError(t, err) + require.Equal(t, tt.expectedMergeable, mergeable) + require.Equal(t, tt.mergeableState, state) + }) + } +} + +func TestIsPRMergeable_Error(t *testing.T) { + t.Parallel() + + ghClient := newErrorReturningClient(t) + _, _, err := ghClient.IsPRMergeable(context.Background(), "owner", "repo", 1) + require.Error(t, err) +} + func TestGetCIStatus(t *testing.T) { tests := []struct { name string From 912cc6e1a3e99d00756e1c5bc74cdf34b445165b Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Fri, 11 Sep 2026 09:30:53 -0400 Subject: [PATCH 2/3] fix: satisfy inline pointer lint --- internal/github/github_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 93eda073..8cf6e412 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -376,8 +376,8 @@ func TestIsPRMergeable(t *testing.T) { mock.WithRequestMatch( mock.GetReposPullsByOwnerByRepoByPullNumber, github.PullRequest{ - Mergeable: github.Bool(tt.githubMergeable), - MergeableState: github.String(tt.mergeableState), + Mergeable: &tt.githubMergeable, + MergeableState: &tt.mergeableState, }, ), ) From 342b16fd6cf9be5a8398486facbaa1a17249bc9c Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Thu, 8 Oct 2026 13:32:26 -0400 Subject: [PATCH 3/3] fix(test): remove duplicate mergeable state field --- cmd/wait-for-github/pr_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/wait-for-github/pr_test.go b/cmd/wait-for-github/pr_test.go index 93608dce..e5933d8d 100644 --- a/cmd/wait-for-github/pr_test.go +++ b/cmd/wait-for-github/pr_test.go @@ -48,7 +48,6 @@ type fakeGithubClientPRCheck struct { CIStatus github.CIStatus Mergeable bool - MergeableState string MergeableCalledCount int RerunCount int HasRunsInProgress bool