rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

fix: post new review first, then supersede old with link #39

Merged
rodin merged 1 commits from fix/34-supersede-order into main 2026-05-02 18:46:51 +00:00
Conversation 3 Commits 1 Files Changed 2
+33 -19
rodin commented 2026-05-02 18:44:03 +00:00
Owner
Copy Link
Copy Source

Summary

Changes the order of operations when posting reviews:

  1. POST new review first — gets non-stale badge on HEAD immediately
  2. PATCH old review — marks as superseded with a clickable link to the new review

Why

Previously the bot superseded the old review before posting the new one. This meant:

  • The superseded message had no link ("see current review" pointed nowhere)
  • If the POST failed, the old review was already destroyed

New order ensures the link is always valid and the old review is only touched after the new one succeeds.

Changes

  • buildSupersededBody now accepts newReviewURL parameter
  • Supersede message: **Superseded** — [see current review](url) for up-to-date findings.
  • URL format: {giteaURL}/{owner}/{repo}/pulls/{number}#pullrequestreview-{id}
  • Tests updated for new signature
## Summary Changes the order of operations when posting reviews: 1. **POST new review first** — gets non-stale badge on HEAD immediately 2. **PATCH old review** — marks as superseded with a clickable link to the new review ## Why Previously the bot superseded the old review *before* posting the new one. This meant: - The superseded message had no link ("see current review" pointed nowhere) - If the POST failed, the old review was already destroyed New order ensures the link is always valid and the old review is only touched after the new one succeeds. ## Changes - `buildSupersededBody` now accepts `newReviewURL` parameter - Supersede message: `**Superseded** — [see current review](url) for up-to-date findings.` - URL format: `{giteaURL}/{owner}/{repo}/pulls/{number}#pullrequestreview-{id}` - Tests updated for new signature
rodin added 1 commit 2026-05-02 18:44:04 +00:00
fix: post new review first, then supersede old with link
CI / test (pull_request) Successful in 14s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 44s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m8s
Details
ed69d26e87 
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.
gpt-review-bot approved these changes 2026-05-02 18:44:40 +00:00
gpt-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Gpt Review

Summary

This PR makes the order of review posting more robust, ensuring links to superseded reviews are always valid, and updates both implementation and tests accordingly. All changes are correct and idiomatic, with no issues found.

Recommendation

APPROVE — The changes improve the atomicity and reliability of the review-posting flow by posting the new review first and only marking the old one as superseded using the new review's URL. The updates to both production and test code are correct, maintain compatibility, and follow repository conventions and Go idioms. No issues were found; merge as is.

Review by gpt

Evaluated against ed69d26e

# Gpt Review ## Summary This PR makes the order of review posting more robust, ensuring links to superseded reviews are always valid, and updates both implementation and tests accordingly. All changes are correct and idiomatic, with no issues found. ## Recommendation **APPROVE** — The changes improve the atomicity and reliability of the review-posting flow by posting the new review first and only marking the old one as superseded using the new review's URL. The updates to both production and test code are correct, maintain compatibility, and follow repository conventions and Go idioms. No issues were found; merge as is. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against ed69d26e*
security-review-bot approved these changes 2026-05-02 18:45:02 +00:00
security-review-bot left a comment
Collaborator
Copy Link
Copy Source

Security Review

Summary

The changes reorder review posting and superseding to ensure links are valid and reduce failure risk. No new security-relevant issues are introduced by this diff, and CI has passed.

Recommendation

APPROVE — The update to first post the new review and then supersede the old one with a concrete link is sound. Given that the URL is constructed from operator-controlled inputs and the original review body is authored by the bot, this change does not introduce meaningful security risk. Proceed with merge.

Review by security

Evaluated against ed69d26e

# Security Review ## Summary The changes reorder review posting and superseding to ensure links are valid and reduce failure risk. No new security-relevant issues are introduced by this diff, and CI has passed. ## Recommendation **APPROVE** — The update to first post the new review and then supersede the old one with a concrete link is sound. Given that the URL is constructed from operator-controlled inputs and the original review body is authored by the bot, this change does not introduce meaningful security risk. Proceed with merge. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against ed69d26e*
sonnet-review-bot approved these changes 2026-05-02 18:45:28 +00:00
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Sonnet Review

Summary

Solid change that reverses the order: posting the new review before superseding the old ensures a valid link and avoids destroying the old review if POST fails. Tests are updated accordingly and CI passed.

Findings

# Severity File Line Finding
1 [NIT] cmd/review-bot/main.go 356 When constructing newReviewURL via string formatting, consider using net/url.JoinPath (and adding the fragment separately) or trimming a trailing slash to avoid possible double slashes if giteaURL ends with '/'. This is minor, as browsers typically handle double slashes, but it improves robustness.
2 [NIT] cmd/review-bot/main.go 522 The doc comment for buildSupersededBody mentions only collapsing and commit info; consider updating it to also note that it includes a link to the new review via newReviewURL for clarity.

Recommendation

APPROVE — The change correctly posts the new review first and only then supersedes the old one with a link to the new review, preventing dangling references and preserving history. Tests have been updated to validate the new signature and behavior, and CI has passed. Consider the minor improvements noted (more robust URL construction and a clarified comment), but overall this is ready to merge.

Review by sonnet

Evaluated against ed69d26e

# Sonnet Review ## Summary Solid change that reverses the order: posting the new review before superseding the old ensures a valid link and avoids destroying the old review if POST fails. Tests are updated accordingly and CI passed. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [NIT] | `cmd/review-bot/main.go` | 356 | When constructing newReviewURL via string formatting, consider using net/url.JoinPath (and adding the fragment separately) or trimming a trailing slash to avoid possible double slashes if giteaURL ends with '/'. This is minor, as browsers typically handle double slashes, but it improves robustness. | | 2 | [NIT] | `cmd/review-bot/main.go` | 522 | The doc comment for buildSupersededBody mentions only collapsing and commit info; consider updating it to also note that it includes a link to the new review via newReviewURL for clarity. | ## Recommendation **APPROVE** — The change correctly posts the new review first and only then supersedes the old one with a link to the new review, preventing dangling references and preserving history. Tests have been updated to validate the new signature and behavior, and CI has passed. Consider the minor improvements noted (more robust URL construction and a clarified comment), but overall this is ready to merge. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against ed69d26e*
cmd/review-bot/main.go
@@ -356,6 +353,17 @@ func main() {
}
slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
// Supersede old review with link to the new one
sonnet-review-bot commented 2026-05-02 18:45:28 +00:00
First-time contributor
Copy Link
Copy Source

[NIT] When constructing newReviewURL via string formatting, consider using net/url.JoinPath (and adding the fragment separately) or trimming a trailing slash to avoid possible double slashes if giteaURL ends with '/'. This is minor, as browsers typically handle double slashes, but it improves robustness.

**[NIT]** When constructing newReviewURL via string formatting, consider using net/url.JoinPath (and adding the fragment separately) or trimming a trailing slash to avoid possible double slashes if giteaURL ends with '/'. This is minor, as browsers typically handle double slashes, but it improves robustness.
cmd/review-bot/main.go
@@ -514,14 +522,16 @@ func validateReviewerName(name string) error {
sonnet-review-bot commented 2026-05-02 18:45:28 +00:00
First-time contributor
Copy Link
Copy Source

[NIT] The doc comment for buildSupersededBody mentions only collapsing and commit info; consider updating it to also note that it includes a link to the new review via newReviewURL for clarity.

**[NIT]** The doc comment for buildSupersededBody mentions only collapsing and commit info; consider updating it to also note that it includes a link to the new review via newReviewURL for clarity.
rodin merged commit ed06cdd942 into main 2026-05-02 18:46:51 +00:00
rodin deleted branch fix/34-supersede-order 2026-05-02 18:46:51 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
gpt-review-bot
security-review-bot
sonnet-review-bot
Labels
Clear labels
ai-review
bug
needs-review
ready
Ready for human review
 self-reviewed
Self-review passed since last push
 wip
Work in progress - do not start new work on this PR
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
security-review-bot rodin (Rodin)
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#39
Delete Branch "fix/34-supersede-order"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?