fix(review-bot): approve with Gitea's APPROVED event
The bot posted approvals with event "APPROVE". Gitea knows APPROVED,
REQUEST_CHANGES and COMMENT and stores any other value as a pending review
that only its author sees (routers/api/v1/repo/pull_review.go,
preparePullReviewType, default branch). Every approval since cdba8b62 is such
a pending draft: homelab #1270, #1278, #1287, #1318, homeassistant #385.
The bot also counted its own pending review as "reviewed at this head", so
those PRs were never reviewed again. A pending review no longer counts; the
sweep reviews them again, and Gitea submits the existing pending review with
the new event.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+3
-2
@@ -135,10 +135,11 @@ func eligible(pr PR, reviews []Review, bot string) string {
|
||||
return "ok"
|
||||
}
|
||||
|
||||
// headReview is the bot's last review of the PR's head commit.
|
||||
// headReview is the bot's last submitted review of the PR's head commit. A pending review is
|
||||
// visible only to the bot, so it does not count.
|
||||
func headReview(pr PR, reviews []Review, bot string) (Review, bool) {
|
||||
for i := len(reviews) - 1; i >= 0; i-- {
|
||||
if r := reviews[i]; r.User.Login == bot && strings.Contains(r.Body, marker(pr.Head.SHA)) {
|
||||
if r := reviews[i]; r.User.Login == bot && r.State != "PENDING" && strings.Contains(r.Body, marker(pr.Head.SHA)) {
|
||||
return r, true
|
||||
}
|
||||
}
|
||||
|
||||
+10
-3
@@ -74,17 +74,24 @@ func splitVerdict(answer string) (rest, verdict string) {
|
||||
return strings.Join(lines[:len(lines)-1], "\n"), strings.ToLower(strings.NewReplacer("_", "-", " ", "-").Replace(m[1]))
|
||||
}
|
||||
|
||||
// Gitea's review events. Gitea stores any other value as a pending review only its author sees.
|
||||
const (
|
||||
eventApproved = "APPROVED"
|
||||
eventRequestChanges = "REQUEST_CHANGES"
|
||||
eventComment = "COMMENT"
|
||||
)
|
||||
|
||||
// reviewEvent maps the agent's verdict to the Gitea review event. Changes are requested and
|
||||
// comments posted as the agent said; an approval is only honoured for a non-empty change whose
|
||||
// report holds no red or yellow section, and anything else is a comment.
|
||||
func reviewEvent(verdict, report string, nonEmptyChange bool) string {
|
||||
switch verdict {
|
||||
case "request-changes":
|
||||
return "REQUEST_CHANGES"
|
||||
return eventRequestChanges
|
||||
case "approve":
|
||||
if nonEmptyChange && !hasHeading(report, "## 🔴") && !hasHeading(report, "## 🟡") {
|
||||
return "APPROVE"
|
||||
return eventApproved
|
||||
}
|
||||
}
|
||||
return "COMMENT"
|
||||
return eventComment
|
||||
}
|
||||
|
||||
@@ -146,10 +146,10 @@ func TestReviewEvent(t *testing.T) {
|
||||
}{
|
||||
{"the agent requests changes", "request-changes", clean, true, "REQUEST_CHANGES"},
|
||||
{"a red heading alone does not request changes", "comment", red, true, "COMMENT"},
|
||||
{"approve", "approve", clean, true, "APPROVE"},
|
||||
{"approve", "approve", clean, true, "APPROVED"},
|
||||
{"approve with a yellow section is only a comment", "approve", yellow, true, "COMMENT"},
|
||||
{"approve with a red section is only a comment", "approve", red, true, "COMMENT"},
|
||||
{"approve with an unverified section approves", "approve", unverified, true, "APPROVE"},
|
||||
{"approve with an unverified section approves", "approve", unverified, true, "APPROVED"},
|
||||
{"an empty change never approves", "approve", clean, false, "COMMENT"},
|
||||
{"comment", "comment", yellow, true, "COMMENT"},
|
||||
{"no verdict is a comment", "", clean, true, "COMMENT"},
|
||||
|
||||
@@ -280,6 +280,21 @@ func TestReviewSkipsWhatIsAlreadyReviewed(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A pending review (an event Gitea did not know) is visible only to the bot: the PR is reviewed
|
||||
// again and the approval goes out as APPROVED.
|
||||
func TestPendingReviewIsReviewedAgain(t *testing.T) {
|
||||
f := newFixture(t)
|
||||
f.reviews = `[{"body":"<!-- pr-review:sha=abc123 -->","state":"PENDING","user":{"login":"review-bot"}}]`
|
||||
f.answer = `{"outputs":[{"type":"message.output","content":"## Summary\n\nfine\n\n## Recommendation\n\nLooks reasonable.\n\nVerdict: approve"}]}`
|
||||
if err := f.rev.Review(context.Background(), "k.renner/homelab", 7); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
posted := f.postedReviews()
|
||||
if len(posted) != 1 || posted[0]["event"] != "APPROVED" {
|
||||
t.Fatalf("want one APPROVED review, got %v", posted)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReviewRefusesWriteToolsAndPostsNothing(t *testing.T) {
|
||||
f := newFixture(t)
|
||||
f.answer = `{"outputs":[{"type":"tool.execution","name":"conn_create_or_update_file"},
|
||||
|
||||
Reference in New Issue
Block a user