diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 1519cc6..d5e4734 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -67,7 +67,6 @@ func main() { patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)") patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo") dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting") - updateExisting := flag.Bool("update-existing", envOrDefaultBool("UPDATE_EXISTING", true), "Delete previous review from same bot before posting (default true)") llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)") llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)") llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic") @@ -279,6 +278,16 @@ func main() { // Step 10: Format and post review reviewBody := review.FormatMarkdown(result, *reviewerName) + + // Add commit footer so readers know which commit was evaluated + if pr.Head.Sha != "" { + shortSHA := pr.Head.Sha + if len(shortSHA) > 8 { + shortSHA = shortSHA[:8] + } + reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA) + } + event := review.GiteaEvent(result.Verdict) if *dryRun { @@ -307,56 +316,33 @@ func main() { } // --- Review update strategy --- - // 1. No existing review → POST new - // 2. Existing review, same state → PATCH body in place (preserves threads) - // 3. Existing review, state change → PATCH old to "Superseded", POST new - if *updateExisting && *reviewerName != "" { + // Always post a fresh review on the current commit. If a previous review + // exists, mark it as superseded (preserves old threads as navigable history). + // Never skip posting — the fresh review must land on HEAD to clear stale badges. + if *reviewerName != "" { existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) if err != nil { slog.Warn("could not list existing reviews", "pr", prNumber, "error", err) } else { - // Detect shared-token misconfiguration: if detected, skip all - // update logic (PATCH/supersede) to avoid clobbering a sibling's review. + // In shared-token mode, skip superseding to avoid clobbering sibling reviews. sharedToken := hasSharedToken(existingReviews, sentinel) - if sharedToken { - slog.Warn("shared token mode: skipping update-in-place logic to avoid clobbering sibling review") - } else { + if !sharedToken { existing := findOwnReview(existingReviews, sentinel) - if existing != nil { - if reviewUnchanged(existingReviews, reviewBody, event, sentinel) { - slog.Info("review unchanged from previous run; skipping to preserve threads", "pr", prNumber) - return - } - - // Same state → PATCH in place - if existing.State == event { - commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel) - if err != nil { - slog.Warn("could not find review comment ID, falling back to new post", "error", err) - } else { - if err := giteaClient.EditComment(ctx, owner, repoName, commentID, reviewBody); err != nil { - slog.Warn("could not edit review, falling back to new post", "comment_id", commentID, "error", err) - } else { - slog.Info("review updated in place", "comment_id", commentID, "pr", prNumber) - return - } - } + commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel) + if err != nil { + slog.Warn("could not find old review comment ID for supersede", "error", err) } else { - // State change → mark old as superseded, post new below - commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel) - if err != nil { - slog.Warn("could not find old review comment ID", "error", err) + supersededBody := buildSupersededBody(existing.Body, existing.CommitID, sentinel) + 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 { - supersededBody := fmt.Sprintf("~~*This review has been superseded by a newer review below.*~~\n\n%s", sentinel) - 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, "new_state", event, "pr", prNumber) - } + slog.Info("marked old review as superseded", "old_state", existing.State, "pr", prNumber) } } } + } else { + slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review") } } } @@ -526,22 +512,23 @@ func validateReviewerName(name string) error { return nil } -// reviewUnchanged checks if an existing review with the same sentinel -// already has identical body and state. Returns true if a re-post would -// produce the same result (skip to preserve conversation threads). -func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string) bool { - for _, r := range reviews { - if r.Stale { - continue - } - if !strings.Contains(r.Body, sentinel) { - continue - } - if r.State == newEvent && r.Body == newBody { - return true - } +// buildSupersededBody creates the body for a superseded review: struck-through banner +// with collapsed original content and the commit it was evaluated against. +func buildSupersededBody(originalBody, commitSHA, sentinel string) string { + shortSHA := commitSHA + if len(shortSHA) > 8 { + shortSHA = shortSHA[:8] } - return false + var sb strings.Builder + sb.WriteString("~~Original review~~\n\n") + sb.WriteString("**Superseded** \u2014 see current review for up-to-date findings.\n\n") + sb.WriteString("
Previous findings (commit ") + sb.WriteString(shortSHA) + sb.WriteString(")\n\n") + sb.WriteString(originalBody) + sb.WriteString("\n\n
\n\n") + sb.WriteString(sentinel) + return sb.String() } // hasSharedToken detects if another review-bot role posted under the same diff --git a/cmd/review-bot/main_test.go b/cmd/review-bot/main_test.go index e2f6dcb..9944185 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -56,82 +56,48 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re return r } -func TestReviewUnchanged(t *testing.T) { - tests := []struct { - name string - existing []gitea.Review - newBody string - newEvent string - sentinel string - want bool - }{ - { - name: "no existing review", - existing: nil, - newBody: "new review", - newEvent: "APPROVED", - sentinel: "", - want: false, - }, - { - name: "identical body and state", - existing: []gitea.Review{ - makeReview(100, "bot", "APPROVED", false, "same body\n"), - }, - newBody: "same body\n", - newEvent: "APPROVED", - sentinel: "", - want: true, - }, - { - name: "same body but different state", - existing: []gitea.Review{ - makeReview(100, "bot", "APPROVED", false, "body\n"), - }, - newBody: "body\n", - newEvent: "REQUEST_CHANGES", - sentinel: "", - want: false, - }, - { - name: "different body same state", - existing: []gitea.Review{ - makeReview(100, "bot", "APPROVED", false, "old body\n"), - }, - newBody: "new body\n", - newEvent: "APPROVED", - sentinel: "", - want: false, - }, - { - name: "stale review with same body (should still post)", - existing: []gitea.Review{ - makeReview(100, "bot", "APPROVED", true, "same\n"), - }, - newBody: "same\n", - newEvent: "APPROVED", - sentinel: "", - want: false, - }, - { - name: "different sentinel (not our review)", - existing: []gitea.Review{ - makeReview(100, "bot", "APPROVED", false, "body\n"), - }, - newBody: "body\n", - newEvent: "APPROVED", - sentinel: "", - want: false, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got := reviewUnchanged(tc.existing, tc.newBody, tc.newEvent, tc.sentinel) - if got != tc.want { - t.Errorf("reviewUnchanged() = %v, want %v", got, tc.want) - } - }) +func TestBuildSupersededBody(t *testing.T) { + original := "# Review\n\nLooks good.\n\n" + sentinel := "" + + result := buildSupersededBody(original, "abcdef1234567890", sentinel) + + // Should contain the struck-through banner + if !strings.Contains(result, "~~Original review~~") { + t.Error("missing struck-through banner") + } + // Should contain superseded notice + if !strings.Contains(result, "**Superseded**") { + t.Error("missing superseded notice") + } + // Should contain collapsed original + if !strings.Contains(result, "
") { + t.Error("missing details/collapse") + } + // Should contain short commit SHA + if !strings.Contains(result, "abcdef12") { + t.Error("missing short SHA") + } + // Should NOT contain full SHA + if strings.Contains(result, "abcdef1234567890") { + t.Error("should truncate SHA to 8 chars") + } + // Should contain the original body inside details + if !strings.Contains(result, original) { + t.Error("original body not preserved in collapsed section") + } + // Should end with sentinel + if !strings.Contains(result, sentinel) { + t.Error("missing sentinel") + } +} + +func TestBuildSupersededBodyShortSHA(t *testing.T) { + // Short SHA should pass through without panic + result := buildSupersededBody("body", "abc", "") + if !strings.Contains(result, "abc") { + t.Error("short SHA not preserved") } } diff --git a/gitea/client.go b/gitea/client.go index c7648f1..1f5f44e 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -316,13 +316,14 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string // Review represents a pull request review from the Gitea API. type Review struct { - ID int64 `json:"id"` - Body string `json:"body"` - User struct { + ID int64 `json:"id"` + Body string `json:"body"` + User struct { Login string `json:"login"` } `json:"user"` - State string `json:"state"` - Stale bool `json:"stale"` + State string `json:"state"` + Stale bool `json:"stale"` + CommitID string `json:"commit_id"` } // ListReviews returns all reviews on a pull request.