feat(forge): carry PR head SHA, mergeable state and review commit id (RIG-5182)

go/internal/board/issue_prs_pgtest_test.go

@@ -143,6 +143,45 @@ func TestPullRequestBeforeIssueAttachesLater(t *testing.T) {
 	}
 }
 
+func TestPullRequestFieldsSurviveStoredPRRoundTrip(t *testing.T) {
+	p, _, _ := newIssueBoard(t)
+	mustPublishIssue(t, p, 1)
+
+	current := ingestedPR(10, 0, time.Hour, 1)
+	current.PR.HeadSha = "head-sha"
+	current.PR.MergeableState = "blocked"
+	current.PR.Reviews = []*compassv1.Review{{CommitId: "review-sha"}}
+	mustPublishPR(t, p, current)
+
+	legacy := ingestedPR(11, time.Hour, 2*time.Hour, 1)
+	mustPublishPR(t, p, legacy)
+
+	prs := cachedIssue1(t, p).GetPrs()
+	if len(prs) != 2 {
+		t.Fatalf("stored pull requests = %d, want 2", len(prs))
+	}
+	byNumber := make(map[uint32]*compassv1.PullRequest, len(prs))
+	for _, pr := range prs {
+		byNumber[pr.GetNumber()] = pr
+	}
+	currentPR := byNumber[10]
+	if currentPR == nil {
+		t.Fatal("stored current pull request is missing")
+	}
+	if currentPR.GetHeadSha() != "head-sha" || currentPR.GetMergeableState() != "blocked" {
+		t.Errorf("stored PR head/mergeable = %q/%q, want head-sha/blocked", currentPR.GetHeadSha(), currentPR.GetMergeableState())
+	}
+	if len(currentPR.GetReviews()) != 1 || currentPR.GetReviews()[0].GetCommitId() != "review-sha" {
+		t.Errorf("stored PR reviews = %v, want one review on review-sha", currentPR.GetReviews())
+	}
+	legacyPR := byNumber[11]
+	if legacyPR == nil {
+		t.Fatal("stored legacy pull request is missing")
+	}
+	if legacyPR.GetHeadSha() != "" || legacyPR.GetMergeableState() != "" || len(legacyPR.GetReviews()) != 0 {
+		t.Errorf("legacy stored PR fields = %q/%q/%v, want empty values", legacyPR.GetHeadSha(), legacyPR.GetMergeableState(), legacyPR.GetReviews())
+	}
+}
 func TestFallbackMovesWhenExplicitTargetArrives(t *testing.T) {
 	ctx := context.Background()
 	p, bus, st := newIssueBoard(t)

go/internal/forge/github.go

@@ -567,19 +567,20 @@ func (g *GitHub) GetIssue(ctx context.Context, repo string, number uint64) (Issu
 // open|closed plus a separate merged bool; toPullRequest folds them into the
 // forge domain's open|closed|merged.
 type ghPullDetail struct {
-	Number       uint64 `json:"number"`
-	Title        string `json:"title"`
-	Body         string `json:"body"`
-	State        string `json:"state"`
-	HTMLURL      string `json:"html_url"`
-	Draft        bool   `json:"draft"`
-	Additions    uint32 `json:"additions"`
-	Deletions    uint32 `json:"deletions"`
-	ChangedFiles uint32 `json:"changed_files"`
-	Merged       bool   `json:"merged"`
-	CreatedAt    string `json:"created_at"`
-	UpdatedAt    string `json:"updated_at"`
-	Head         struct {
+	Number         uint64 `json:"number"`
+	Title          string `json:"title"`
+	Body           string `json:"body"`
+	State          string `json:"state"`
+	HTMLURL        string `json:"html_url"`
+	Draft          bool   `json:"draft"`
+	Additions      uint32 `json:"additions"`
+	Deletions      uint32 `json:"deletions"`
+	ChangedFiles   uint32 `json:"changed_files"`
+	Merged         bool   `json:"merged"`
+	MergeableState string `json:"mergeable_state"`
+	CreatedAt      string `json:"created_at"`
+	UpdatedAt      string `json:"updated_at"`
+	Head           struct {
 		Ref string `json:"ref"`
 		SHA string `json:"sha"`
 	} `json:"head"`
@@ -601,15 +602,17 @@ func (r ghPullDetail) toPullRequest() PullRequest {
 		state = prStateMerged
 	}
 	return PullRequest{
-		Number:       r.Number,
-		Title:        r.Title,
-		Body:         r.Body,
-		State:        state,
-		URL:          r.HTMLURL,
-		HeadRef:      r.Head.Ref,
-		BaseRef:      r.Base.Ref,
-		ForgeAccount: r.User.Login,
-		Draft:        r.Draft,
+		Number:         r.Number,
+		Title:          r.Title,
+		Body:           r.Body,
+		State:          state,
+		URL:            r.HTMLURL,
+		HeadRef:        r.Head.Ref,
+		HeadSHA:        r.Head.SHA,
+		MergeableState: r.MergeableState,
+		BaseRef:        r.Base.Ref,
+		ForgeAccount:   r.User.Login,
+		Draft:          r.Draft,
 		Changed: ChangedStats{
 			Files:     r.ChangedFiles,
 			Additions: r.Additions,
@@ -623,9 +626,10 @@ func (r ghPullDetail) toPullRequest() PullRequest {
 // ghReviewRow is the wire shape of one entry in the pull reviews list. Only the
 // fields forge.Review needs are decoded; the user type discriminates a bot.
 type ghReviewRow struct {
-	Body  string `json:"body"`
-	State string `json:"state"`
-	User  struct {
+	Body     string `json:"body"`
+	State    string `json:"state"`
+	CommitID string `json:"commit_id"`
+	User     struct {
 		Login string `json:"login"`
 		Type  string `json:"type"`
 	} `json:"user"`
@@ -656,10 +660,11 @@ func (g *GitHub) GetPullRequest(ctx context.Context, repo string, number uint64)
 	pr.Reviews = make([]Review, 0, len(reviews))
 	for _, rv := range reviews {
 		pr.Reviews = append(pr.Reviews, Review{
-			Author:  rv.User.Login,
-			IsBot:   rv.User.Type == "Bot",
-			Verdict: strings.ToLower(rv.State),
-			Body:    rv.Body,
+			Author:   rv.User.Login,
+			IsBot:    rv.User.Type == "Bot",
+			Verdict:  strings.ToLower(rv.State),
+			Body:     rv.Body,
+			CommitID: rv.CommitID,
 		})
 	}
 

go/internal/forge/github_test.go

@@ -1461,13 +1461,14 @@ func TestGetPullRequestHappy(t *testing.T) {
 		"deletions": 30,
 		"changed_files": 4,
 		"merged": true,
+		"mergeable_state": "blocked",
 		"head": {"ref": "feature", "sha": "abc123"},
 		"base": {"ref": "main"},
 		"user": {"login": "octocat"}
 	}`
 	const reviewsBody = `[
-		{"body": "lgtm", "state": "APPROVED", "user": {"login": "botly", "type": "Bot"}},
-		{"body": "needs work", "state": "CHANGES_REQUESTED", "user": {"login": "carol", "type": "User"}}
+		{"body": "lgtm", "state": "APPROVED", "commit_id": "review-sha", "user": {"login": "botly", "type": "Bot"}},
+		{"body": "needs work", "state": "CHANGES_REQUESTED", "commit_id": "old-sha", "user": {"login": "carol", "type": "User"}}
 	]`
 	const checkRunsBody = `{"check_runs": [
 		{"name": "build", "status": "completed", "conclusion": "success", "html_url": "https://ci/build"}
@@ -1537,12 +1538,15 @@ func TestGetPullRequestHappy(t *testing.T) {
 	if len(got.Reviews) != 2 {
 		t.Fatalf("Reviews len = %d, want 2", len(got.Reviews))
 	}
-	if got.Reviews[0] != (Review{Author: "botly", IsBot: true, Verdict: "approved", Body: "lgtm"}) {
+	if got.Reviews[0] != (Review{Author: "botly", IsBot: true, Verdict: "approved", Body: "lgtm", CommitID: "review-sha"}) {
 		t.Errorf("Reviews[0] = %+v", got.Reviews[0])
 	}
-	if got.Reviews[1] != (Review{Author: "carol", IsBot: false, Verdict: "changes_requested", Body: "needs work"}) {
+	if got.Reviews[1] != (Review{Author: "carol", IsBot: false, Verdict: "changes_requested", Body: "needs work", CommitID: "old-sha"}) {
 		t.Errorf("Reviews[1] = %+v", got.Reviews[1])
 	}
+	if got.HeadSHA != "abc123" || got.MergeableState != "blocked" {
+		t.Errorf("HeadSHA/MergeableState = %q/%q, want abc123/blocked", got.HeadSHA, got.MergeableState)
+	}
 
 	// Embedded checks populated (both sources), rolled up to success; only the
 	// check GraphQL reports as required is marked Required.

go/internal/forge/provider.go

@@ -98,6 +98,10 @@ type PullRequest struct {
 	HeadRef string
 	// BaseRef is the target branch name.
 	BaseRef string
+	// HeadSHA is the commit currently at the source branch head.
+	HeadSHA string
+	// MergeableState is the forge's mergeability assessment for the PR.
+	MergeableState string
 	// ForgeAccount is the native forge login that authored the PR.
 	ForgeAccount string
 	// Draft reports whether the PR is a draft on the forge.
@@ -160,6 +164,8 @@ type Review struct {
 	Verdict string
 	// Body is the review's summary body.
 	Body string
+	// CommitID is the commit the reviewer submitted this review against.
+	CommitID string
 }
 
 // ReviewThread is one inline (or PR-level) review comment thread.

go/internal/forge/testdata/github/create_pull_request.json

@@ -41,6 +41,8 @@
 			"URL": "https://github.com/org/repo/pull/13",
 			"HeadRef": "feature",
 			"BaseRef": "main",
+			"HeadSHA": "",
+			"MergeableState": "",
 			"ForgeAccount": "octocat",
 			"Draft": true,
 			"Changed": { "Files": 0, "Additions": 0, "Deletions": 0 },

go/internal/forge/testdata/github/get_pull_request.json

@@ -20,6 +20,7 @@
 			"deletions": 30,
 			"changed_files": 4,
 			"merged": true,
+			"mergeable_state": "blocked",
 			"head": { "ref": "feature", "sha": "abc123" },
 			"base": { "ref": "main" },
 			"user": { "login": "octocat" }
@@ -31,11 +32,13 @@
 					{
 						"body": "lgtm",
 						"state": "APPROVED",
+						"commit_id": "review-sha",
 						"user": { "login": "botly", "type": "Bot" }
 					},
 					{
 						"body": "needs work",
 						"state": "CHANGES_REQUESTED",
+						"commit_id": "old-sha",
 						"user": { "login": "carol", "type": "User" }
 					}
 				]
@@ -165,6 +168,8 @@
 			"State": "merged",
 			"URL": "https://github.com/org/repo/pull/42",
 			"HeadRef": "feature",
+			"HeadSHA": "abc123",
+			"MergeableState": "blocked",
 			"BaseRef": "main",
 			"ForgeAccount": "octocat",
 			"Draft": false,
@@ -192,13 +197,15 @@
 					"Author": "botly",
 					"IsBot": true,
 					"Verdict": "approved",
-					"Body": "lgtm"
+					"Body": "lgtm",
+					"CommitID": "review-sha"
 				},
 				{
 					"Author": "carol",
 					"IsBot": false,
 					"Verdict": "changes_requested",
-					"Body": "needs work"
+					"Body": "needs work",
+					"CommitID": "old-sha"
 				}
 			],
 			"Threads": [

go/internal/forge/testdata/github/transition_pull_request_close.json

@@ -30,6 +30,7 @@
 			"deletions": 2,
 			"changed_files": 3,
 			"merged": false,
+			"mergeable_state": "blocked",
 			"head": { "ref": "feature", "sha": "abc123" },
 			"base": { "ref": "main" },
 			"user": { "login": "octocat" }
@@ -41,6 +42,8 @@
 			"State": "closed",
 			"URL": "https://github.com/org/repo/pull/7",
 			"HeadRef": "feature",
+			"HeadSHA": "abc123",
+			"MergeableState": "blocked",
 			"BaseRef": "main",
 			"ForgeAccount": "octocat",
 			"Draft": false,

go/internal/forge/testdata/github/transition_pull_request_reopen.json

@@ -30,6 +30,7 @@
 			"deletions": 2,
 			"changed_files": 3,
 			"merged": false,
+			"mergeable_state": "blocked",
 			"head": { "ref": "feature", "sha": "abc123" },
 			"base": { "ref": "main" },
 			"user": { "login": "octocat" }
@@ -41,6 +42,8 @@
 			"State": "open",
 			"URL": "https://github.com/org/repo/pull/7",
 			"HeadRef": "feature",
+			"HeadSHA": "abc123",
+			"MergeableState": "blocked",
 			"BaseRef": "main",
 			"ForgeAccount": "octocat",
 			"Draft": false,

go/internal/forge/translate.go

@@ -69,15 +69,17 @@ func TranslateIssue(in Issue, attr *compassv1.AgentAttribution) *compassv1.Issue
 // caller/projection supplies them).
 func TranslatePullRequest(in PullRequest, attr *compassv1.AgentAttribution) *compassv1.PullRequest {
 	return &compassv1.PullRequest{
-		Number:       narrowNumber(in.Number),
-		Title:        in.Title,
-		ForgeState:   in.State,
-		Url:          in.URL,
-		HeadRef:      in.HeadRef,
-		BaseRef:      in.BaseRef,
-		ForgeAccount: in.ForgeAccount,
-		Draft:        in.Draft,
-		Agent:        attr,
+		Number:         narrowNumber(in.Number),
+		Title:          in.Title,
+		ForgeState:     in.State,
+		Url:            in.URL,
+		HeadRef:        in.HeadRef,
+		BaseRef:        in.BaseRef,
+		HeadSha:        in.HeadSHA,
+		MergeableState: in.MergeableState,
+		ForgeAccount:   in.ForgeAccount,
+		Draft:          in.Draft,
+		Agent:          attr,
 		Changed: &compassv1.ChangedStats{
 			Files:     in.Changed.Files,
 			Additions: in.Changed.Additions,
@@ -131,10 +133,11 @@ func translateReviews(in []Review) []*compassv1.Review {
 	out := make([]*compassv1.Review, len(in))
 	for i, r := range in {
 		out[i] = &compassv1.Review{
-			Author:  r.Author,
-			IsBot:   r.IsBot,
-			Verdict: r.Verdict,
-			Body:    r.Body,
+			Author:   r.Author,
+			IsBot:    r.IsBot,
+			Verdict:  r.Verdict,
+			Body:     r.Body,
+			CommitId: r.CommitID,
 		}
 	}
 	return out

go/internal/forge/translate_test.go

@@ -172,8 +172,8 @@ func assertPRReviews(t *testing.T, got *compassv1.PullRequest) {
 		t.Fatalf("Reviews len = %d, want 1", len(got.GetReviews()))
 	}
 	r := got.GetReviews()[0]
-	if r.GetAuthor() != "rev" || !r.GetIsBot() || r.GetVerdict() != "approved" || r.GetBody() != "lgtm" {
-		t.Errorf("Review = %+v, want {rev bot approved lgtm}", r)
+	if r.GetAuthor() != "rev" || !r.GetIsBot() || r.GetVerdict() != "approved" || r.GetBody() != "lgtm" || r.GetCommitId() != "review-sha" {
+		t.Errorf("Review = %+v, want {rev bot approved lgtm review-sha}", r)
 	}
 }
 
@@ -198,22 +198,23 @@ func assertPRThreads(t *testing.T, got *compassv1.PullRequest) {
 func TestTranslatePullRequestRoundTrip(t *testing.T) {
 	attr := &compassv1.AgentAttribution{AgentHandle: "agent-pr"}
 	in := PullRequest{
-		Number:       7,
-		Title:        "a pr",
-		Body:         "<!-- compass-owner: agent-pr -->\npr body",
-		State:        "merged",
-		URL:          "https://forge/pull/7",
-		HeadRef:      "feature",
-		BaseRef:      "main",
-		ForgeAccount: "octocat",
-		Draft:        true,
-		Changed:      ChangedStats{Files: 3, Additions: 10, Deletions: 2},
+		Number:         7,
+		Title:          "a pr",
+		State:          "merged",
+		URL:            "https://forge/pull/7",
+		HeadRef:        "feature",
+		BaseRef:        "main",
+		HeadSHA:        "head-sha",
+		MergeableState: "blocked",
+		ForgeAccount:   "octocat",
+		Draft:          true,
+		Changed:        ChangedStats{Files: 3, Additions: 10, Deletions: 2},
 		Checks: Checks{
 			HeadSHA: "abc123",
 			State:   "success",
 			Checks:  []Check{{Name: "ci", State: "success", URL: "https://ci/1", Required: true}},
 		},
-		Reviews: []Review{{Author: "rev", IsBot: true, Verdict: "approved", Body: "lgtm"}},
+		Reviews: []Review{{Author: "rev", IsBot: true, Verdict: "approved", Body: "lgtm", CommitID: "review-sha"}},
 		Threads: []ReviewThread{{
 			Path:     "file.go",
 			Resolved: true,
@@ -238,6 +239,9 @@ func TestTranslatePullRequestRoundTrip(t *testing.T) {
 	if got.GetHeadRef() != "feature" {
 		t.Errorf("HeadRef = %q, want %q", got.GetHeadRef(), "feature")
 	}
+	if got.GetHeadSha() != "head-sha" || got.GetMergeableState() != "blocked" {
+		t.Errorf("HeadSha/MergeableState = %q/%q, want head-sha/blocked", got.GetHeadSha(), got.GetMergeableState())
+	}
 	if got.GetBaseRef() != "main" {
 		t.Errorf("BaseRef = %q, want %q", got.GetBaseRef(), "main")
 	}