fix: address review findings
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 24s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 37s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m4s

- 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
This commit is contained in:
Rodin
2026-05-02 13:31:59 -07:00
parent d4bf13eeab
commit a0fd882b0d
2 changed files with 12 additions and 7 deletions
+8 -6
View File
@@ -320,17 +320,15 @@ func main() {
// 2. Then supersede old review with link to the new one // 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. // Order matters: post first so we have the new review's URL for the supersede message.
var oldReviews []gitea.Review var oldReviews []gitea.Review
var sharedTokenMode bool
if *reviewerName != "" { if *reviewerName != "" {
existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
if err != nil { if err != nil {
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err) slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
} else { } else {
sharedTokenMode = hasSharedToken(existingReviews, sentinel) if hasSharedToken(existingReviews, sentinel) {
if !sharedTokenMode {
oldReviews = findAllOwnReviews(existingReviews, sentinel)
} else {
slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review") 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) slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
continue continue
} }
resolved := 0 resolved, failed := 0, 0
for _, c := range oldComments { for _, c := range oldComments {
if c.ID == 0 { if c.ID == 0 {
continue continue
} }
if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil { if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err) slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
failed++
} else { } else {
resolved++ resolved++
} }
@@ -392,6 +391,9 @@ func main() {
if resolved > 0 { if resolved > 0 {
slog.Info("resolved old inline comments", "review_id", oldReview.ID, "count", resolved, "pr", prNumber) 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)
}
} }
} }
+4 -1
View File
@@ -442,6 +442,9 @@ func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner,
} }
var review struct { var review struct {
Body string `json:"body"` Body string `json:"body"`
User struct {
Login string `json:"login"`
} `json:"user"`
} }
if err := json.Unmarshal(body, &review); err != nil { if err := json.Unmarshal(body, &review); err != nil {
return 0, fmt.Errorf("parse review %d: %w", reviewID, err) 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) return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
} }
for _, ev := range events { 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 return ev.ID, nil
} }
} }