From a0fd882b0d2a8a7afee7664424350f9217d1dc00 Mon Sep 17 00:00:00 2001 From: Rodin Date: Sat, 2 May 2026 13:31:59 -0700 Subject: [PATCH] fix: address review findings - Tighten timeline matching: also check ev.User.Login matches the review author (prevents collision on identical body prefix) - Remove unused sharedTokenMode variable (inline condition) - Aggregate resolution failures with warn-level summary --- cmd/review-bot/main.go | 14 ++++++++------ gitea/client.go | 5 ++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index d125af8..777b6b4 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -320,17 +320,15 @@ func main() { // 2. Then supersede old review with link to the new one // Order matters: post first so we have the new review's URL for the supersede message. var oldReviews []gitea.Review - var sharedTokenMode bool if *reviewerName != "" { existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) if err != nil { slog.Warn("could not list existing reviews", "pr", prNumber, "error", err) } else { - sharedTokenMode = hasSharedToken(existingReviews, sentinel) - if !sharedTokenMode { - oldReviews = findAllOwnReviews(existingReviews, sentinel) - } else { + if hasSharedToken(existingReviews, sentinel) { slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review") + } else { + oldReviews = findAllOwnReviews(existingReviews, sentinel) } } } @@ -378,13 +376,14 @@ func main() { slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err) continue } - resolved := 0 + 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++ } @@ -392,6 +391,9 @@ func main() { if resolved > 0 { slog.Info("resolved old inline comments", "review_id", oldReview.ID, "count", resolved, "pr", prNumber) } + if failed > 0 { + slog.Warn("some inline comments could not be resolved", "review_id", oldReview.ID, "failed", failed, "pr", prNumber) + } } } diff --git a/gitea/client.go b/gitea/client.go index 333cf07..150b3c0 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -442,6 +442,9 @@ func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, } var review struct { Body string `json:"body"` + User struct { + Login string `json:"login"` + } `json:"user"` } if err := json.Unmarshal(body, &review); err != nil { return 0, fmt.Errorf("parse review %d: %w", reviewID, err) @@ -474,7 +477,7 @@ func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, return 0, fmt.Errorf("parse timeline (page %d): %w", page, err) } for _, ev := range events { - if ev.Type == "review" && strings.HasPrefix(ev.Body, matchPrefix) { + if ev.Type == "review" && ev.User.Login == review.User.Login && strings.HasPrefix(ev.Body, matchPrefix) { return ev.ID, nil } }