fix(github): validate conflicting commit IDs and extract test helper
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 28s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m28s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m24s
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 28s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m28s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m24s
Address review findings from sonnet-review-bot (review 3086): - PostReview now returns ErrConflictingCommitIDs when comments specify different non-empty CommitIDs, since the GitHub API accepts only a single commit_id per review. Previously the discrepancy was silently ignored, using only the first commit's ID. - Extract newTestClient into helpers_test.go to make cross-file sharing between review_test.go and identity_test.go explicit. Refs: #81
This commit is contained in:
+19
-4
@@ -17,6 +17,11 @@ import (
|
||||
// a submitted review should use DismissReview instead.
|
||||
var ErrCannotDeleteSubmittedReview = errors.New("cannot delete submitted review: use DismissReview instead")
|
||||
|
||||
// ErrConflictingCommitIDs is returned when PostReview receives comments with
|
||||
// differing non-empty CommitIDs. The GitHub API accepts only a single commit_id
|
||||
// per review submission; callers must ensure all comments target the same commit.
|
||||
var ErrConflictingCommitIDs = errors.New("comments contain conflicting commit IDs: all must target the same commit")
|
||||
|
||||
// postReviewRequest is the GitHub API request body for creating a review.
|
||||
type postReviewRequest struct {
|
||||
CommitID string `json:"commit_id,omitempty"`
|
||||
@@ -75,6 +80,11 @@ func translateGitHubReviewState(state string) string {
|
||||
// ReviewComment.Position maps directly to the GitHub API position field.
|
||||
// When req.Comments is empty, the payload omits the comments field entirely
|
||||
// (via the omitempty tag on postReviewRequest.Comments).
|
||||
//
|
||||
// The GitHub API accepts a single commit_id per review submission. PostReview
|
||||
// extracts it from the first comment with a non-empty CommitID. If any subsequent
|
||||
// comment specifies a different CommitID, PostReview returns ErrConflictingCommitIDs.
|
||||
// Comments with an empty CommitID are allowed and inherit the review-level value.
|
||||
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) {
|
||||
reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
|
||||
c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
|
||||
@@ -84,11 +94,16 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
|
||||
Event: string(req.Event),
|
||||
}
|
||||
|
||||
// Populate CommitID from the first comment and build the payload in one pass.
|
||||
// All comments in a single review share the same commit_id.
|
||||
// Build the payload in one pass. The GitHub API accepts a single commit_id
|
||||
// per review; we extract it from the first comment that supplies one and
|
||||
// reject the request if any other comment disagrees.
|
||||
for _, comment := range req.Comments {
|
||||
if payload.CommitID == "" && comment.CommitID != "" {
|
||||
payload.CommitID = comment.CommitID
|
||||
if comment.CommitID != "" {
|
||||
if payload.CommitID == "" {
|
||||
payload.CommitID = comment.CommitID
|
||||
} else if payload.CommitID != comment.CommitID {
|
||||
return nil, ErrConflictingCommitIDs
|
||||
}
|
||||
}
|
||||
payload.Comments = append(payload.Comments, reviewCommentEntry{
|
||||
Path: comment.Path,
|
||||
|
||||
Reference in New Issue
Block a user