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

feat: delete previous review before posting new one (#6) #22

Merged
aweiker merged 9 commits from feat/6-update-existing-review into main 2026-05-02 04:50:16 +00:00
Conversation 3 Commits 9 Files Changed 10
+824 -61
rodin commented 2026-05-02 03:17:10 +00:00
Owner
Copy Link
Copy Source

Before posting a review, the bot now:

  1. Calls GET /api/v1/user to identify its own login
  2. Lists all reviews on the PR
  3. Deletes any existing reviews from itself
  4. Posts the fresh review

This keeps PR threads clean — one review per bot at any time.

New Gitea client methods

  • GetAuthenticatedUser() — token self-identification
  • ListReviews() — fetch reviews on a PR
  • DeleteReview() — delete a review by ID

Flag

--update-existing / UPDATE_EXISTING (default true)
Set to false to preserve old behavior (stack reviews).

All delete failures are non-fatal (logged as warnings).

Closes #6

Before posting a review, the bot now: 1. Calls `GET /api/v1/user` to identify its own login 2. Lists all reviews on the PR 3. Deletes any existing reviews from itself 4. Posts the fresh review This keeps PR threads clean — one review per bot at any time. ## New Gitea client methods - `GetAuthenticatedUser()` — token self-identification - `ListReviews()` — fetch reviews on a PR - `DeleteReview()` — delete a review by ID ## Flag `--update-existing` / `UPDATE_EXISTING` (default `true`) Set to `false` to preserve old behavior (stack reviews). All delete failures are non-fatal (logged as warnings). Closes #6
rodin added 1 commit 2026-05-02 04:36:35 +00:00
feat: add role title as H1 header for visual differentiation
CI / test (pull_request) Successful in 14s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 19s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m27s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m42s
Details
2287a8238c 
When reviewer-name is set, prepend "# Security Review" / "# Sonnet Review"
etc. as a top-level header. Makes it immediately obvious which role each
review represents in the Gitea UI, especially when multiple reviews come
from the same bot account.
rodin force-pushed feat/6-update-existing-review from dd6c6c610f to 2287a8238c 2026-05-02 04:36:35 +00:00 Compare
rodin added 1 commit 2026-05-02 04:42:52 +00:00
refactor: validate reviewer-name early (fail fast before LLM call)
CI / test (pull_request) Successful in 13s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 24s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m0s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m40s
Details
55391c66d8 
Moved validateReviewerName check to right after flag parsing. Previously
it ran after the LLM request completed — wasting an expensive API call
if the name was invalid.

Sonnet review finding #1.
gpt-review-bot approved these changes 2026-05-02 04:43:31 +00:00
gpt-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Gpt Review

Summary

This change implements robust duplicate review cleanup, adds targeted review sentinel logic, and enhances configuration and documentation for specialized review workflows. All code is thoroughly tested and idiomatic.

Recommendation

APPROVE — The implementation is correct, thoroughly tested, and aligns well with Go and repository conventions. New features (review cleanup, escalations, system prompt support) are composable and safe by default. All APIs are well-encapsulated, and the action/docs reflect the feature set. No changes required.

Review by gpt

# Gpt Review ## Summary This change implements robust duplicate review cleanup, adds targeted review sentinel logic, and enhances configuration and documentation for specialized review workflows. All code is thoroughly tested and idiomatic. ## Recommendation **APPROVE** — The implementation is correct, thoroughly tested, and aligns well with Go and repository conventions. New features (review cleanup, escalations, system prompt support) are composable and safe by default. All APIs are well-encapsulated, and the action/docs reflect the feature set. No changes required. --- *Review by gpt* <!-- review-bot:gpt -->
sonnet-review-bot approved these changes 2026-05-02 04:44:06 +00:00
Dismissed
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Security Review

Summary

The changes add safe review cleanup and optional system prompt loading with appropriate validation and authorization checks. No exploitable vulnerabilities were identified, and CI passed.

Recommendation

APPROVE — The new features (sentinel-based cleanup, worst-wins escalation, and system-prompt-file support) are implemented with solid security considerations: reviewer-name is validated, deletion is restricted to the same authenticated user and matching sentinel, and system prompt file reading is constrained to the workspace with symlink checks. HTTP calls use auth headers without leaking secrets in logs. Approve as submitted.

Review by security

# Security Review ## Summary The changes add safe review cleanup and optional system prompt loading with appropriate validation and authorization checks. No exploitable vulnerabilities were identified, and CI passed. ## Recommendation **APPROVE** — The new features (sentinel-based cleanup, worst-wins escalation, and system-prompt-file support) are implemented with solid security considerations: reviewer-name is validated, deletion is restricted to the same authenticated user and matching sentinel, and system prompt file reading is constrained to the workspace with symlink checks. HTTP calls use auth headers without leaking secrets in logs. Approve as submitted. --- *Review by security* <!-- review-bot:security -->
sonnet-review-bot requested changes 2026-05-02 04:44:47 +00:00
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Sonnet Review

Summary

Solid feature addition with good tests and CI passing. However, there is a logic flaw when escalating an APPROVED review to REQUEST_CHANGES: the event is changed but the review body still reflects the original APPROVE verdict, which is misleading.

Findings

# Severity File Line Finding
1 [MAJOR] cmd/review-bot/main.go 263 When worst-wins escalation triggers (event == "APPROVED" and shouldEscalate == true), the code deletes the APPROVED review and re-posts with event REQUEST_CHANGES but reuses the original reviewBody, which contains the original verdict text (e.g., APPROVE). This creates a mismatch between the review event and the displayed content, potentially confusing users.
2 [MINOR] gitea/client.go 152 Owner/repo path handling is inconsistent across methods. ListReviews/DeleteReview use url.PathEscape for owner and repo, but other methods (e.g., GetPullRequest, GetPullRequestDiff) interpolate owner/repo directly. While typical owner/repo names are safe, it's better to consistently path-escape to avoid edge cases and for uniformity.

Recommendation

REQUEST_CHANGES — Overall the changes are well-structured, follow repository conventions, and include thorough tests. CI has passed. However, before merging, address the escalation mismatch: when escalating an APPROVED review to REQUEST_CHANGES, rebuild the review body to reflect the REQUEST_CHANGES verdict. For example, clone the parsed result, set Verdict to "REQUEST_CHANGES", regenerate the body via review.FormatMarkdown, and post that. Alternatively, adjust the existing body’s recommendation section to reflect the escalated verdict. Also consider harmonizing owner/repo URL construction in the Gitea client by path-escaping owner and repo across all endpoints for consistency. With these fixes, the PR can be safely merged.

Review by sonnet

# Sonnet Review ## Summary Solid feature addition with good tests and CI passing. However, there is a logic flaw when escalating an APPROVED review to REQUEST_CHANGES: the event is changed but the review body still reflects the original APPROVE verdict, which is misleading. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MAJOR] | `cmd/review-bot/main.go` | 263 | When worst-wins escalation triggers (event == "APPROVED" and shouldEscalate == true), the code deletes the APPROVED review and re-posts with event REQUEST_CHANGES but reuses the original reviewBody, which contains the original verdict text (e.g., **APPROVE**). This creates a mismatch between the review event and the displayed content, potentially confusing users. | | 2 | [MINOR] | `gitea/client.go` | 152 | Owner/repo path handling is inconsistent across methods. ListReviews/DeleteReview use url.PathEscape for owner and repo, but other methods (e.g., GetPullRequest, GetPullRequestDiff) interpolate owner/repo directly. While typical owner/repo names are safe, it's better to consistently path-escape to avoid edge cases and for uniformity. | ## Recommendation **REQUEST_CHANGES** — Overall the changes are well-structured, follow repository conventions, and include thorough tests. CI has passed. However, before merging, address the escalation mismatch: when escalating an APPROVED review to REQUEST_CHANGES, rebuild the review body to reflect the REQUEST_CHANGES verdict. For example, clone the parsed result, set Verdict to "REQUEST_CHANGES", regenerate the body via review.FormatMarkdown, and post that. Alternatively, adjust the existing body’s recommendation section to reflect the escalated verdict. Also consider harmonizing owner/repo URL construction in the Gitea client by path-escaping owner and repo across all endpoints for consistency. With these fixes, the PR can be safely merged. --- *Review by sonnet* <!-- review-bot:sonnet -->
aweiker merged commit 177d56f218 into main 2026-05-02 04:50:16 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
gpt-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
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#22
Delete Branch "feat/6-update-existing-review"

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?