fix: supersede ALL old reviews, not just the most recent
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 46s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m7s
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 46s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m7s
Previously findOwnReview returned only the single most-recent matching review, so on PRs with multiple force-pushes only the latest old review got superseded. The rest accumulated as unsuperseded stale reviews. Changes: - Add findAllOwnReviews() to collect all non-superseded matching reviews - Loop over all old reviews in the supersede phase - Add GetTimelineReviewCommentIDForReview() to find comment IDs by review ID (fetches review body, matches in timeline by prefix) - Each old review gets independently superseded and its inline comments resolved The old findOwnReview is kept for backward compat (tested, may be useful as a utility).
This commit is contained in:
+52
-49
@@ -319,25 +319,16 @@ func main() {
|
|||||||
// 1. POST new review first (gets non-stale approval badge on HEAD)
|
// 1. POST new review first (gets non-stale approval badge on HEAD)
|
||||||
// 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 existingReview *gitea.Review
|
var oldReviews []gitea.Review
|
||||||
var existingCommentID int64
|
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 {
|
||||||
sharedToken := hasSharedToken(existingReviews, sentinel)
|
sharedTokenMode = hasSharedToken(existingReviews, sentinel)
|
||||||
if !sharedToken {
|
if !sharedTokenMode {
|
||||||
existingReview = findOwnReview(existingReviews, sentinel)
|
oldReviews = findAllOwnReviews(existingReviews, sentinel)
|
||||||
if existingReview != nil {
|
|
||||||
cid, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
|
||||||
if err != nil {
|
|
||||||
slog.Warn("could not find old review comment ID for supersede", "error", err)
|
|
||||||
existingReview = nil // can't supersede without comment ID
|
|
||||||
} else {
|
|
||||||
existingCommentID = cid
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} else {
|
} 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")
|
||||||
}
|
}
|
||||||
@@ -365,43 +356,42 @@ func main() {
|
|||||||
}
|
}
|
||||||
slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
|
slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
|
||||||
|
|
||||||
// Supersede old review with link to the new one
|
// Supersede all old reviews with link to the new one
|
||||||
if existingReview != nil && existingCommentID > 0 {
|
if len(oldReviews) > 0 {
|
||||||
newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID)
|
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)
|
for _, oldReview := range oldReviews {
|
||||||
supersedeOK := false
|
cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
|
||||||
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 {
|
if err != nil {
|
||||||
slog.Warn("could not list old review comments for resolution", "review_id", existingReview.ID, "error", err)
|
slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
|
||||||
} else {
|
continue
|
||||||
resolved, failed := 0, 0
|
}
|
||||||
for _, c := range oldComments {
|
supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
|
||||||
if c.ID == 0 {
|
if err := giteaClient.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
|
||||||
continue
|
slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err)
|
||||||
}
|
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)
|
slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)
|
||||||
failed++
|
|
||||||
} else {
|
// Resolve old review's inline comments
|
||||||
resolved++
|
oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
|
||||||
}
|
if err != nil {
|
||||||
|
slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
resolved := 0
|
||||||
|
for _, c := range oldComments {
|
||||||
|
if c.ID == 0 {
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
if resolved > 0 {
|
if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
|
||||||
slog.Info("resolved old inline comments", "count", resolved, "pr", prNumber)
|
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
|
||||||
}
|
} else {
|
||||||
if failed > 0 {
|
resolved++
|
||||||
slog.Warn("some inline comments could not be resolved", "failed", failed, "pr", prNumber)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if resolved > 0 {
|
||||||
|
slog.Info("resolved old inline comments", "review_id", oldReview.ID, "count", resolved, "pr", prNumber)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -627,21 +617,34 @@ func extractSentinelName(body string) string {
|
|||||||
return rest[:end]
|
return rest[:end]
|
||||||
}
|
}
|
||||||
|
|
||||||
// findOwnReview locates a review matching the given sentinel in its body.
|
// findOwnReview locates the most recent non-superseded review matching the sentinel.
|
||||||
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
|
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
|
||||||
var best *gitea.Review
|
var best *gitea.Review
|
||||||
for i := range reviews {
|
for i := range reviews {
|
||||||
if !strings.Contains(reviews[i].Body, sentinel) {
|
if !strings.Contains(reviews[i].Body, sentinel) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
// Skip superseded reviews (they contain our sentinel in the collapsed body)
|
|
||||||
if strings.Contains(reviews[i].Body, "~~Original review~~") {
|
if strings.Contains(reviews[i].Body, "~~Original review~~") {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
// Take the highest ID (most recent)
|
|
||||||
if best == nil || reviews[i].ID > best.ID {
|
if best == nil || reviews[i].ID > best.ID {
|
||||||
best = &reviews[i]
|
best = &reviews[i]
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return best
|
return best
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// findAllOwnReviews returns all non-superseded reviews matching the sentinel.
|
||||||
|
func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
|
||||||
|
var result []gitea.Review
|
||||||
|
for i := range reviews {
|
||||||
|
if !strings.Contains(reviews[i].Body, sentinel) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if strings.Contains(reviews[i].Body, "~~Original review~~") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
result = append(result, reviews[i])
|
||||||
|
}
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
|||||||
@@ -841,3 +841,24 @@ func cleanEnv() []string {
|
|||||||
}
|
}
|
||||||
return env
|
return env
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestFindAllOwnReviews(t *testing.T) {
|
||||||
|
reviews := []gitea.Review{
|
||||||
|
{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
|
||||||
|
{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
|
||||||
|
{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},
|
||||||
|
{ID: 4, Body: "~~Original review~~\n<!-- review-bot:sonnet -->\nsuperseded"},
|
||||||
|
{ID: 5, Body: "<!-- review-bot:sonnet -->\nthird review"},
|
||||||
|
}
|
||||||
|
|
||||||
|
got := findAllOwnReviews(reviews, "<!-- review-bot:sonnet -->")
|
||||||
|
if len(got) != 3 {
|
||||||
|
t.Fatalf("findAllOwnReviews() returned %d, want 3", len(got))
|
||||||
|
}
|
||||||
|
wantIDs := []int64{1, 3, 5}
|
||||||
|
for i, r := range got {
|
||||||
|
if r.ID != wantIDs[i] {
|
||||||
|
t.Errorf("got[%d].ID = %d, want %d", i, r.ID, wantIDs[i])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -426,6 +426,65 @@ func (c *Client) GetTimelineReviewCommentID(ctx context.Context, owner, repo str
|
|||||||
return 0, fmt.Errorf("no timeline event found with sentinel")
|
return 0, fmt.Errorf("no timeline event found with sentinel")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetTimelineReviewCommentIDForReview finds the timeline comment ID for a
|
||||||
|
// specific review by matching its body content in the timeline.
|
||||||
|
func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) {
|
||||||
|
// Use the reviews API to get the review body, then find in timeline
|
||||||
|
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d",
|
||||||
|
c.baseURL,
|
||||||
|
url.PathEscape(owner),
|
||||||
|
url.PathEscape(repo),
|
||||||
|
number,
|
||||||
|
reviewID)
|
||||||
|
body, err := c.doGet(ctx, reqURL)
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("get review %d: %w", reviewID, err)
|
||||||
|
}
|
||||||
|
var review struct {
|
||||||
|
Body string `json:"body"`
|
||||||
|
}
|
||||||
|
if err := json.Unmarshal(body, &review); err != nil {
|
||||||
|
return 0, fmt.Errorf("parse review %d: %w", reviewID, err)
|
||||||
|
}
|
||||||
|
if review.Body == "" {
|
||||||
|
return 0, fmt.Errorf("review %d has empty body", reviewID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Use a prefix for matching (handles minor trailing whitespace differences)
|
||||||
|
matchPrefix := review.Body
|
||||||
|
if len(matchPrefix) > 200 {
|
||||||
|
matchPrefix = matchPrefix[:200]
|
||||||
|
}
|
||||||
|
|
||||||
|
const pageSize = 50
|
||||||
|
for page := 1; ; page++ {
|
||||||
|
timelineURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/%d/timeline?limit=%d&page=%d",
|
||||||
|
c.baseURL,
|
||||||
|
url.PathEscape(owner),
|
||||||
|
url.PathEscape(repo),
|
||||||
|
number,
|
||||||
|
pageSize,
|
||||||
|
page)
|
||||||
|
tlBody, err := c.doGet(ctx, timelineURL)
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("get timeline (page %d): %w", page, err)
|
||||||
|
}
|
||||||
|
var events []TimelineEvent
|
||||||
|
if err := json.Unmarshal(tlBody, &events); err != nil {
|
||||||
|
return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
|
||||||
|
}
|
||||||
|
for _, ev := range events {
|
||||||
|
if ev.Type == "review" && strings.HasPrefix(ev.Body, matchPrefix) {
|
||||||
|
return ev.ID, nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(events) < pageSize {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return 0, fmt.Errorf("no timeline event found for review %d", reviewID)
|
||||||
|
}
|
||||||
|
|
||||||
// EditComment updates the body of an issue/review comment.
|
// EditComment updates the body of an issue/review comment.
|
||||||
func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID int64, newBody string) error {
|
func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID int64, newBody string) error {
|
||||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/comments/%d",
|
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/comments/%d",
|
||||||
|
|||||||
Reference in New Issue
Block a user