From c9336cd9670d09563c045dab11810eb4c5811012 Mon Sep 17 00:00:00 2001 From: sean-park-funda Date: Fri, 25 Sep 2026 10:42:46 +0900 Subject: [PATCH] Carry merge_commit_sha through to MinimalPullRequest GetPullRequest fetches the full pull request and returns convertToMinimalPullRequest(pr). MinimalPullRequest carries merged, merged_at, merged_by, head and base, but had no merge_commit_sha field, so the value was dropped during conversion. For a merged pull request that commit is otherwise unreachable from the pull request itself: callers fall back to listing commits and matching on merge time or on the pull request number in the commit message, both of which are unreliable on an active branch. The field is omitted when empty, so an open pull request does not present the API's test-merge commit as a result. Fixes #3235 Co-Authored-By: Claude Opus 5 --- pkg/github/minimal_types.go | 7 ++++++ pkg/github/pullrequests_test.go | 42 +++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 2eba9a1628..5e25bdf104 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -724,6 +724,7 @@ type MinimalPullRequest struct { Assignees []string `json:"assignees,omitempty"` RequestedReviewers []string `json:"requested_reviewers,omitempty"` MergedBy string `json:"merged_by,omitempty"` + MergeCommitSHA string `json:"merge_commit_sha,omitempty"` Head *MinimalPRBranch `json:"head,omitempty"` Base *MinimalPRBranch `json:"base,omitempty"` Additions int `json:"additions,omitempty"` @@ -1134,6 +1135,12 @@ func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest { m.MergedBy = mergedBy.GetLogin() } + // For a merged pull request this is the commit the merge produced, which is + // otherwise unreachable from the pull request without a second call. For an + // open one the API reports a test-merge commit instead, so the field is + // omitted when empty rather than presented as a result. + m.MergeCommitSHA = pr.GetMergeCommitSHA() + if head := pr.Head; head != nil { m.Head = convertToMinimalPRBranch(head) } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index c0e392aea6..2b51ef295c 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -4877,3 +4877,45 @@ func TestResolveReviewThread(t *testing.T) { }) } } + +func Test_convertToMinimalPullRequest_MergeCommitSHA(t *testing.T) { + t.Run("merged pull request carries the merge commit", func(t *testing.T) { + mergedAt := time.Date(2026, 9, 6, 12, 0, 0, 0, time.UTC) + pr := &github.PullRequest{ + Number: github.Ptr(42), + Title: github.Ptr("Test PR"), + State: github.Ptr("closed"), + Merged: github.Ptr(true), + MergedAt: &github.Timestamp{Time: mergedAt}, + MergeCommitSHA: github.Ptr("5b1d8e0c6f4a3b2c1d0e9f8a7b6c5d4e3f2a1b0c"), + } + + minimal := convertToMinimalPullRequest(pr) + assert.Equal(t, "5b1d8e0c6f4a3b2c1d0e9f8a7b6c5d4e3f2a1b0c", minimal.MergeCommitSHA) + + // The field has to survive serialisation, since that is what the caller reads. + raw, err := json.Marshal(minimal) + require.NoError(t, err) + var decoded map[string]any + require.NoError(t, json.Unmarshal(raw, &decoded)) + assert.Equal(t, "5b1d8e0c6f4a3b2c1d0e9f8a7b6c5d4e3f2a1b0c", decoded["merge_commit_sha"]) + }) + + t.Run("pull request without a merge commit omits the field", func(t *testing.T) { + pr := &github.PullRequest{ + Number: github.Ptr(43), + Title: github.Ptr("Open PR"), + State: github.Ptr("open"), + Merged: github.Ptr(false), + } + + minimal := convertToMinimalPullRequest(pr) + assert.Empty(t, minimal.MergeCommitSHA) + + raw, err := json.Marshal(minimal) + require.NoError(t, err) + var decoded map[string]any + require.NoError(t, json.Unmarshal(raw, &decoded)) + assert.NotContains(t, decoded, "merge_commit_sha") + }) +}