From bc5a4a1dcd0aa606a0ef88af5ead00cca472d0c9 Mon Sep 17 00:00:00 2001 From: Rodin Date: Sat, 2 May 2026 12:15:52 -0700 Subject: [PATCH] feat: resolve old inline comments when superseding review Closes #27 After superseding an old review, resolves all its inline comments via POST /pulls/comments/{id}/resolve. This clears unresolved conversation markers from the PR timeline and diff view. New API methods: - ListReviewComments: paginated GET /repos/.../pulls/{n}/reviews/{id}/comments - ResolveComment: POST /repos/.../pulls/comments/{id}/resolve Behavior: - Only resolves after successful supersede (gated on supersedeOK) - Aggregates failures and logs at warn level - Truncates error bodies to 256 bytes (security) - Non-fatal: review still posts even if resolution fails --- cmd/review-bot/main.go | 29 +++++++++++++++++++++ gitea/client.go | 58 +++++++++++++++++++++++++++++++++++++++++ gitea/client_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+) diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 9f85d73..8ed8ee5 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -369,10 +369,39 @@ func main() { if existingReview != nil && existingCommentID > 0 { newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID) supersededBody := buildSupersededBody(existingReview.Body, existingReview.CommitID, newReviewURL, sentinel) + supersedeOK := false if err := giteaClient.EditComment(ctx, owner, repoName, existingCommentID, supersededBody); err != nil { slog.Warn("could not mark old review as superseded", "comment_id", existingCommentID, "error", err) } else { slog.Info("marked old review as superseded", "old_state", existingReview.State, "new_review_id", posted.ID, "pr", prNumber) + supersedeOK = true + } + + // Resolve old review's inline comments only after successful supersede + if supersedeOK { + oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, existingReview.ID) + if err != nil { + slog.Warn("could not list old review comments for resolution", "review_id", existingReview.ID, "error", err) + } else { + resolved, failed := 0, 0 + for _, c := range oldComments { + if c.ID == 0 { + continue + } + if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil { + slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err) + failed++ + } else { + resolved++ + } + } + if resolved > 0 { + slog.Info("resolved old inline comments", "count", resolved, "pr", prNumber) + } + if failed > 0 { + slog.Warn("some inline comments could not be resolved", "failed", failed, "pr", prNumber) + } + } } } diff --git a/gitea/client.go b/gitea/client.go index cf200cd..aa42f3b 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -82,6 +82,7 @@ type ChangedFile struct { // ReviewComment represents an inline comment to attach to a review. type ReviewComment struct { + ID int64 `json:"id,omitempty"` Path string `json:"path"` NewPosition int64 `json:"new_position"` Body string `json:"body"` @@ -513,3 +514,60 @@ func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number } return nil } + +// ListReviewComments returns the inline comments attached to a specific review. +// Paginates through all pages. +func (c *Client) ListReviewComments(ctx context.Context, owner, repo string, prNumber int, reviewID int64) ([]ReviewComment, error) { + const pageSize = 50 + var all []ReviewComment + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments?limit=%d&page=%d", + c.baseURL, + url.PathEscape(owner), + url.PathEscape(repo), + prNumber, + reviewID, + pageSize, + page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list review comments (page %d): %w", page, err) + } + var batch []ReviewComment + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse review comments (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < pageSize { + break + } + } + return all, nil +} + +// ResolveComment marks an inline review comment as resolved. +func (c *Client) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error { + reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/comments/%d/resolve", + c.baseURL, + url.PathEscape(owner), + url.PathEscape(repo), + commentID) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, nil) + if err != nil { + return fmt.Errorf("create resolve request: %w", err) + } + req.Header.Set("Authorization", "token "+c.token) + + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("resolve comment: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 256)) + return fmt.Errorf("resolve comment failed (status %d): %s", resp.StatusCode, body) + } + return nil +} diff --git a/gitea/client_test.go b/gitea/client_test.go index eda3ab4..d09e38b 100644 --- a/gitea/client_test.go +++ b/gitea/client_test.go @@ -684,3 +684,62 @@ func TestRequestReviewer_Error(t *testing.T) { t.Errorf("error should mention status code: %v", err) } } + +func TestListReviewComments(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.Path, "/pulls/1/reviews/42/comments") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `[{"id":100,"path":"main.go","new_position":5,"body":"finding"},{"id":101,"path":"lib.go","new_position":10,"body":"another"}]`) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + comments, err := client.ListReviewComments(context.Background(), "owner", "repo", 1, 42) + if err != nil { + t.Fatalf("ListReviewComments() error = %v", err) + } + if len(comments) != 2 { + t.Fatalf("got %d comments, want 2", len(comments)) + } + if comments[0].ID != 100 { + t.Errorf("comments[0].ID = %d, want 100", comments[0].ID) + } + if comments[1].Path != "lib.go" { + t.Errorf("comments[1].Path = %q, want %q", comments[1].Path, "lib.go") + } +} + +func TestResolveComment(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + if !strings.Contains(r.URL.Path, "/pulls/comments/99/resolve") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + err := client.ResolveComment(context.Background(), "owner", "repo", 99) + if err != nil { + t.Fatalf("ResolveComment() error = %v", err) + } +} + +func TestResolveComment_Error(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, "not found") + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + err := client.ResolveComment(context.Background(), "owner", "repo", 99) + if err == nil { + t.Fatal("expected error for 404 response") + } +} -- 2.47.3