fix: post new review first, then supersede old with link
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 44s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m8s
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 44s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m8s
Changes the order of operations: 1. POST new review (gets non-stale badge immediately) 2. PATCH old review with superseded message linking to the new one This gives the superseded comment a clickable link to the current review, making navigation between review iterations easy. buildSupersededBody now accepts a newReviewURL parameter.
This commit is contained in:
+26
-16
@@ -316,29 +316,26 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// --- Review update strategy ---
|
// --- Review update strategy ---
|
||||||
// Always post a fresh review on the current commit. If a previous review
|
// 1. POST new review first (gets non-stale approval badge on HEAD)
|
||||||
// exists, mark it as superseded (preserves old threads as navigable history).
|
// 2. Then supersede old review with link to the new one
|
||||||
// Never skip posting — the fresh review must land on HEAD to clear stale badges.
|
// Order matters: post first so we have the new review's URL for the supersede message.
|
||||||
|
var existingReview *gitea.Review
|
||||||
|
var existingCommentID int64
|
||||||
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 {
|
||||||
// In shared-token mode, skip superseding to avoid clobbering sibling reviews.
|
|
||||||
sharedToken := hasSharedToken(existingReviews, sentinel)
|
sharedToken := hasSharedToken(existingReviews, sentinel)
|
||||||
if !sharedToken {
|
if !sharedToken {
|
||||||
existing := findOwnReview(existingReviews, sentinel)
|
existingReview = findOwnReview(existingReviews, sentinel)
|
||||||
if existing != nil {
|
if existingReview != nil {
|
||||||
commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
cid, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not find old review comment ID for supersede", "error", err)
|
slog.Warn("could not find old review comment ID for supersede", "error", err)
|
||||||
|
existingReview = nil // can't supersede without comment ID
|
||||||
} else {
|
} else {
|
||||||
supersededBody := buildSupersededBody(existing.Body, existing.CommitID, sentinel)
|
existingCommentID = cid
|
||||||
if err := giteaClient.EditComment(ctx, owner, repoName, commentID, supersededBody); err != nil {
|
|
||||||
slog.Warn("could not mark old review as superseded", "comment_id", commentID, "error", err)
|
|
||||||
} else {
|
|
||||||
slog.Info("marked old review as superseded", "old_state", existing.State, "pr", prNumber)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
@@ -347,7 +344,7 @@ func main() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// POST new review (first run, or state transition fallthrough)
|
// POST new review
|
||||||
slog.Info("posting review", "event", event, "pr", prNumber)
|
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, inlineComments)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -356,6 +353,17 @@ 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
|
||||||
|
if existingReview != nil && existingCommentID > 0 {
|
||||||
|
newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", *giteaURL, owner, repoName, prNumber, posted.ID)
|
||||||
|
supersededBody := buildSupersededBody(existingReview.Body, existingReview.CommitID, newReviewURL, sentinel)
|
||||||
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// fetchFileContext fetches the full content of modified files from the PR branch.
|
// fetchFileContext fetches the full content of modified files from the PR branch.
|
||||||
@@ -514,14 +522,16 @@ func validateReviewerName(name string) error {
|
|||||||
|
|
||||||
// buildSupersededBody creates the body for a superseded review: struck-through banner
|
// buildSupersededBody creates the body for a superseded review: struck-through banner
|
||||||
// with collapsed original content and the commit it was evaluated against.
|
// with collapsed original content and the commit it was evaluated against.
|
||||||
func buildSupersededBody(originalBody, commitSHA, sentinel string) string {
|
func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string) string {
|
||||||
shortSHA := commitSHA
|
shortSHA := commitSHA
|
||||||
if len(shortSHA) > 8 {
|
if len(shortSHA) > 8 {
|
||||||
shortSHA = shortSHA[:8]
|
shortSHA = shortSHA[:8]
|
||||||
}
|
}
|
||||||
var sb strings.Builder
|
var sb strings.Builder
|
||||||
sb.WriteString("~~Original review~~\n\n")
|
sb.WriteString("~~Original review~~\n\n")
|
||||||
sb.WriteString("**Superseded** \u2014 see current review for up-to-date findings.\n\n")
|
sb.WriteString("**Superseded** \u2014 [see current review](")
|
||||||
|
sb.WriteString(newReviewURL)
|
||||||
|
sb.WriteString(") for up-to-date findings.\n\n")
|
||||||
if shortSHA != "" {
|
if shortSHA != "" {
|
||||||
sb.WriteString("<details><summary>Previous findings (commit ")
|
sb.WriteString("<details><summary>Previous findings (commit ")
|
||||||
sb.WriteString(shortSHA)
|
sb.WriteString(shortSHA)
|
||||||
|
|||||||
@@ -60,17 +60,21 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
|
|||||||
func TestBuildSupersededBody(t *testing.T) {
|
func TestBuildSupersededBody(t *testing.T) {
|
||||||
original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
|
original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
|
||||||
sentinel := "<!-- review-bot:sonnet -->"
|
sentinel := "<!-- review-bot:sonnet -->"
|
||||||
|
newURL := "https://gitea.example.com/owner/repo/pulls/1#pullrequestreview-99"
|
||||||
|
|
||||||
result := buildSupersededBody(original, "abcdef1234567890", sentinel)
|
result := buildSupersededBody(original, "abcdef1234567890", newURL, sentinel)
|
||||||
|
|
||||||
// Should contain the struck-through banner
|
// Should contain the struck-through banner
|
||||||
if !strings.Contains(result, "~~Original review~~") {
|
if !strings.Contains(result, "~~Original review~~") {
|
||||||
t.Error("missing struck-through banner")
|
t.Error("missing struck-through banner")
|
||||||
}
|
}
|
||||||
// Should contain superseded notice
|
// Should contain superseded notice with link
|
||||||
if !strings.Contains(result, "**Superseded**") {
|
if !strings.Contains(result, "**Superseded**") {
|
||||||
t.Error("missing superseded notice")
|
t.Error("missing superseded notice")
|
||||||
}
|
}
|
||||||
|
if !strings.Contains(result, "[see current review]("+newURL+")") {
|
||||||
|
t.Error("missing link to new review")
|
||||||
|
}
|
||||||
// Should contain collapsed original
|
// Should contain collapsed original
|
||||||
if !strings.Contains(result, "<details>") {
|
if !strings.Contains(result, "<details>") {
|
||||||
t.Error("missing details/collapse")
|
t.Error("missing details/collapse")
|
||||||
@@ -95,7 +99,7 @@ func TestBuildSupersededBody(t *testing.T) {
|
|||||||
|
|
||||||
func TestBuildSupersededBodyShortSHA(t *testing.T) {
|
func TestBuildSupersededBodyShortSHA(t *testing.T) {
|
||||||
// Short SHA should pass through without panic
|
// Short SHA should pass through without panic
|
||||||
result := buildSupersededBody("body", "abc", "<!-- review-bot:x -->")
|
result := buildSupersededBody("body", "abc", "https://example.com/review", "<!-- review-bot:x -->")
|
||||||
if !strings.Contains(result, "abc") {
|
if !strings.Contains(result, "abc") {
|
||||||
t.Error("short SHA not preserved")
|
t.Error("short SHA not preserved")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user