feat(gitea): pass commit_id explicitly in PostReview (#107)
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 25s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 42s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 57s

Add commitID parameter to gitea.Client.PostReview so the review is
anchored to the specific commit that was evaluated. The caller
(cmd/review-bot) already computes evaluatedSHA from pr.Head.Sha;
this wires it through to the Gitea API payload.

When commitID is empty, omitempty drops it from the JSON and Gitea
defaults to the current PR head (backward-compatible).

Closes #107
This commit is contained in:
claw
2026-05-13 10:29:05 -07:00
parent dc2e1ca5de
commit e59c2bc831
5 changed files with 40 additions and 12 deletions
+1 -1
View File
@@ -130,7 +130,7 @@ func TestIntegration_PostAndCleanup(t *testing.T) {
// Post a test review
sentinel := "<!-- review-bot:integration-test -->"
testBody := "# Integration Test Review\n\nThis is a test review.\n\n" + sentinel
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, "COMMENT", testBody, nil)
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, "COMMENT", testBody, "", nil)
if err != nil {
t.Fatalf("PostReview: %v", err)
}
+1 -1
View File
@@ -444,7 +444,7 @@ func main() {
// POST new review
slog.Info("posting review", "event", event, "pr", prNumber)
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, inlineComments)
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, evaluatedSHA, inlineComments)
if err != nil {
slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
os.Exit(1)
+5 -1
View File
@@ -263,17 +263,21 @@ func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, r
// PostReview submits a review to a PR and returns the created review.
// event should be "APPROVED" or "REQUEST_CHANGES".
// commitID anchors the review to a specific commit SHA. If empty, Gitea
// defaults to the current PR head.
// comments are optional inline comments attached to specific lines.
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string, comments []ReviewComment) (*Review, error) {
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []ReviewComment) (*Review, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
payload := struct {
Body string `json:"body"`
Event string `json:"event"`
CommitID string `json:"commit_id,omitempty"`
Comments []ReviewComment `json:"comments,omitempty"`
}{
Body: body,
Event: event,
CommitID: commitID,
Comments: comments,
}
+31 -7
View File
@@ -117,8 +117,9 @@ func TestPostReview(t *testing.T) {
}
var payload struct {
Body string `json:"body"`
Event string `json:"event"`
Body string `json:"body"`
Event string `json:"event"`
CommitID string `json:"commit_id"`
}
if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
t.Fatalf("failed to decode payload: %v", err)
@@ -129,14 +130,16 @@ func TestPostReview(t *testing.T) {
if payload.Event != "APPROVED" {
t.Errorf("expected event %q, got %q", "APPROVED", payload.Event)
}
if payload.CommitID != "abc123def" {
t.Errorf("expected commit_id %q, got %q", "abc123def", payload.CommitID)
}
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"id":100,"user":{"login":"review-bot"},"state":"APPROVED","stale":false}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM", nil)
review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM", "abc123def", nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -183,12 +186,35 @@ func TestPostReview_Non200(t *testing.T) {
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test", nil)
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test", "", nil)
if err == nil {
t.Fatal("expected error for 403, got nil")
}
}
func TestPostReview_EmptyCommitID_OmittedFromPayload(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
body, _ := io.ReadAll(r.Body)
var raw map[string]interface{}
if err := json.Unmarshal(body, &raw); err != nil {
t.Fatalf("failed to decode payload: %v", err)
}
if _, exists := raw["commit_id"]; exists {
t.Errorf("expected commit_id to be omitted from payload when empty, but it was present")
}
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"id":200,"user":{"login":"bot"},"state":"APPROVED","stale":false}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "ok", "", nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestGetFileContent(t *testing.T) {
expected := "# Conventions\n- Be nice\n"
@@ -945,8 +971,6 @@ func TestDoGet_RespectsContextCancellation(t *testing.T) {
t.Errorf("attempts = %d, expected 1 before context cancel during backoff", attempts)
}
}
// mockTransport is a test helper that returns errors for the first N calls,
// then delegates to a real server.
type mockTransport struct {
+2 -2
View File
@@ -37,7 +37,7 @@ func TestPostReview_WithComments(t *testing.T) {
{Path: "util.go", NewPosition: 10, Body: "[MINOR] Style issue"},
}
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "REQUEST_CHANGES", "summary", comments)
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "REQUEST_CHANGES", "summary", "", comments)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -72,7 +72,7 @@ func TestPostReview_NilComments(t *testing.T) {
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "all good", nil)
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "all good", "", nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}