Compare commits

...

13 Commits

Author SHA1 Message Date
rodin 23443ef378 Merge pull request 'feat: resolve old inline comments when superseding review' (#42) from feat/27-resolve-inline-comments into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
Release / release (push) Successful in 32s
2026-05-02 19:18:41 +00:00
Rodin bc5a4a1dcd feat: resolve old inline comments when superseding review
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, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 44s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 46s
Closes #27

After superseding an old review, resolves all its inline comments via
POST /pulls/comments/{id}/resolve. This clears unresolved conversation
markers from the PR timeline and diff view.

New API methods:
- ListReviewComments: paginated GET /repos/.../pulls/{n}/reviews/{id}/comments
- ResolveComment: POST /repos/.../pulls/comments/{id}/resolve

Behavior:
- Only resolves after successful supersede (gated on supersedeOK)
- Aggregates failures and logs at warn level
- Truncates error bodies to 256 bytes (security)
- Non-fatal: review still posts even if resolution fails
2026-05-02 12:15:52 -07:00
rodin d30f3d4278 Merge pull request 'feat: self-request as reviewer before posting' (#41) from feat/35-self-request-reviewer into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 19:11:15 +00:00
Rodin 2507ee22e7 fix: address review findings on RequestReviewer
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 35s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m3s
- Accept 204 No Content as success (idempotent operations)
- Truncate error response body to 256 bytes (prevent log leakage)
- Add unit tests for GetAuthenticatedUser and RequestReviewer
2026-05-02 12:09:25 -07:00
Rodin c39845ca03 feat: self-request as reviewer before posting
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 48s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 58s
Closes #35

Before posting a review, the bot:
1. Discovers its own Gitea login via GET /user
2. Calls POST /requested_reviewers to add itself

This ensures the bot appears in the required-reviewers list without
manual configuration on the repo. The call is idempotent (no-op if
already requested).

Both failures are non-fatal (warn + continue) — the review still posts
even if the self-request fails.
2026-05-02 12:04:55 -07:00
rodin cd601bdcf4 Merge pull request 'fix: trim trailing slash from giteaURL when building review link' (#40) from fix/url-normalization into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 18:52:13 +00:00
Rodin 50091941e1 fix: trim trailing slash from giteaURL when building review link
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 20s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 28s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 41s
Prevents double-slash in supersede URL if GITEA_URL ends with '/'.
Aligns with how gitea.NewClient already normalizes the base URL.
2026-05-02 11:49:24 -07:00
rodin ed06cdd942 Merge pull request 'fix: post new review first, then supersede old with link' (#39) from fix/34-supersede-order into main
CI / test (push) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 18:46:50 +00:00
Rodin ed69d26e87 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
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.
2026-05-02 11:43:53 -07:00
rodin da586a512a Merge pull request 'feat: always post fresh review, supersede old with collapsed body' (#38) from feat/34-always-post-fresh into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 18:36:42 +00:00
Rodin f6baa41b2c fix: remove findOwnReviewStrict, use findOwnReview directly
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 42s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m31s
The strict authorship check compared reviewer-name to User.Login which
could mismatch. The sentinel is already role-specific (e.g.
<!-- review-bot:sonnet -->) and Gitea's API blocks editing others'
comments (403). Defense-in-depth via login comparison is unnecessary
complexity that introduced a bug. Removed.
2026-05-02 11:33:57 -07:00
Rodin ecbae332f4 fix: address review findings
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m17s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m22s
- findOwnReview: skip superseded reviews, pick highest ID (most recent)
- findOwnReviewStrict: verify authorship before superseding (defense-in-depth)
- buildSupersededBody: handle empty commitSHA gracefully
- Tests: add cases for superseded skip, highest-ID selection
2026-05-02 11:30:34 -07:00
Rodin fdd75699d9 feat: always post fresh review, supersede old with collapsed body
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m2s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m25s
Closes #34

- Remove reviewUnchanged() skip logic — every push gets a fresh review
- Remove edit-in-place (PATCH same body) — always POST new
- Supersede old review: PATCH with struck-through banner + collapsed
  original body in <details> for historical reference
- Add commit footer to every review: 'Evaluated against <sha>'
- Remove --update-existing flag (no longer needed)
- Add CommitID field to Review struct
- Add TestBuildSupersededBody tests
2026-05-02 11:26:06 -07:00
4 changed files with 442 additions and 142 deletions
+113 -62
View File
@@ -67,7 +67,6 @@ func main() {
patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)") 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") 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") 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)") 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)") 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") 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 // Step 10: Format and post review
reviewBody := review.FormatMarkdown(result, *reviewerName) 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) event := review.GiteaEvent(result.Verdict)
if *dryRun { if *dryRun {
@@ -307,61 +316,47 @@ func main() {
} }
// --- Review update strategy --- // --- Review update strategy ---
// 1. No existing review → POST new // 1. POST new review first (gets non-stale approval badge on HEAD)
// 2. Existing review, same state → PATCH body in place (preserves threads) // 2. Then supersede old review with link to the new one
// 3. Existing review, state change → PATCH old to "Superseded", POST new // Order matters: post first so we have the new review's URL for the supersede message.
if *updateExisting && *reviewerName != "" { var existingReview *gitea.Review
var existingCommentID int64
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 {
// Detect shared-token misconfiguration: if detected, skip all
// update logic (PATCH/supersede) to avoid clobbering a sibling's review.
sharedToken := hasSharedToken(existingReviews, sentinel) sharedToken := hasSharedToken(existingReviews, sentinel)
if sharedToken { if !sharedToken {
slog.Warn("shared token mode: skipping update-in-place logic to avoid clobbering sibling review") existingReview = findOwnReview(existingReviews, sentinel)
} else { if existingReview != nil {
existing := findOwnReview(existingReviews, sentinel) cid, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
if err != nil {
if existing != nil { slog.Warn("could not find old review comment ID for supersede", "error", err)
if reviewUnchanged(existingReviews, reviewBody, event, sentinel) { existingReview = nil // can't supersede without comment ID
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
}
}
} else { } else {
// State change → mark old as superseded, post new below existingCommentID = cid
commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
if err != nil {
slog.Warn("could not find old review comment ID", "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)
}
}
} }
} }
} else {
slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review")
} }
} }
} }
// POST new review (first run, or state transition fallthrough) // Self-request as reviewer (ensures we appear in required-reviewer checks)
authUser, err := giteaClient.GetAuthenticatedUser(ctx)
if err != nil {
slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
} else if authUser != "" {
if err := giteaClient.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
} else {
slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
}
}
// 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 {
@@ -370,6 +365,46 @@ 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", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID)
supersededBody := buildSupersededBody(existingReview.Body, existingReview.CommitID, newReviewURL, sentinel)
supersedeOK := false
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)
supersedeOK = true
}
// Resolve old review's inline comments only after successful supersede
if supersedeOK {
oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, existingReview.ID)
if err != nil {
slog.Warn("could not list old review comments for resolution", "review_id", existingReview.ID, "error", err)
} else {
resolved, failed := 0, 0
for _, c := range oldComments {
if c.ID == 0 {
continue
}
if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
failed++
} else {
resolved++
}
}
if resolved > 0 {
slog.Info("resolved old inline comments", "count", resolved, "pr", prNumber)
}
if failed > 0 {
slog.Warn("some inline comments could not be resolved", "failed", failed, "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.
@@ -526,22 +561,29 @@ func validateReviewerName(name string) error {
return nil return nil
} }
// reviewUnchanged checks if an existing review with the same sentinel // buildSupersededBody creates the body for a superseded review: struck-through banner
// already has identical body and state. Returns true if a re-post would // with collapsed original content and the commit it was evaluated against.
// produce the same result (skip to preserve conversation threads). func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string) string {
func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string) bool { shortSHA := commitSHA
for _, r := range reviews { if len(shortSHA) > 8 {
if r.Stale { shortSHA = shortSHA[:8]
continue
}
if !strings.Contains(r.Body, sentinel) {
continue
}
if r.State == newEvent && r.Body == newBody {
return true
}
} }
return false var sb strings.Builder
sb.WriteString("~~Original review~~\n\n")
sb.WriteString("**Superseded** \u2014 [see current review](")
sb.WriteString(newReviewURL)
sb.WriteString(") for up-to-date findings.\n\n")
if shortSHA != "" {
sb.WriteString("<details><summary>Previous findings (commit ")
sb.WriteString(shortSHA)
sb.WriteString(")</summary>\n\n")
} else {
sb.WriteString("<details><summary>Previous findings</summary>\n\n")
}
sb.WriteString(originalBody)
sb.WriteString("\n\n</details>\n\n")
sb.WriteString(sentinel)
return sb.String()
} }
// hasSharedToken detects if another review-bot role posted under the same // hasSharedToken detects if another review-bot role posted under the same
@@ -587,10 +629,19 @@ func extractSentinelName(body string) string {
// findOwnReview locates a review matching the given sentinel in its body. // findOwnReview locates a review matching the given sentinel in its body.
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review { func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
var best *gitea.Review
for i := range reviews { for i := range reviews {
if strings.Contains(reviews[i].Body, sentinel) { if !strings.Contains(reviews[i].Body, sentinel) {
return &reviews[i] continue
}
// Skip superseded reviews (they contain our sentinel in the collapsed body)
if strings.Contains(reviews[i].Body, "~~Original review~~") {
continue
}
// Take the highest ID (most recent)
if best == nil || reviews[i].ID > best.ID {
best = &reviews[i]
} }
} }
return nil return best
} }
+71 -75
View File
@@ -56,82 +56,52 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
return r 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: "<!-- review-bot:sonnet -->",
want: false,
},
{
name: "identical body and state",
existing: []gitea.Review{
makeReview(100, "bot", "APPROVED", false, "same body\n<!-- review-bot:sonnet -->"),
},
newBody: "same body\n<!-- review-bot:sonnet -->",
newEvent: "APPROVED",
sentinel: "<!-- review-bot:sonnet -->",
want: true,
},
{
name: "same body but different state",
existing: []gitea.Review{
makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:sonnet -->"),
},
newBody: "body\n<!-- review-bot:sonnet -->",
newEvent: "REQUEST_CHANGES",
sentinel: "<!-- review-bot:sonnet -->",
want: false,
},
{
name: "different body same state",
existing: []gitea.Review{
makeReview(100, "bot", "APPROVED", false, "old body\n<!-- review-bot:sonnet -->"),
},
newBody: "new body\n<!-- review-bot:sonnet -->",
newEvent: "APPROVED",
sentinel: "<!-- review-bot:sonnet -->",
want: false,
},
{
name: "stale review with same body (should still post)",
existing: []gitea.Review{
makeReview(100, "bot", "APPROVED", true, "same\n<!-- review-bot:sonnet -->"),
},
newBody: "same\n<!-- review-bot:sonnet -->",
newEvent: "APPROVED",
sentinel: "<!-- review-bot:sonnet -->",
want: false,
},
{
name: "different sentinel (not our review)",
existing: []gitea.Review{
makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
},
newBody: "body\n<!-- review-bot:sonnet -->",
newEvent: "APPROVED",
sentinel: "<!-- review-bot:sonnet -->",
want: false,
},
}
for _, tc := range tests { func TestBuildSupersededBody(t *testing.T) {
t.Run(tc.name, func(t *testing.T) { original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
got := reviewUnchanged(tc.existing, tc.newBody, tc.newEvent, tc.sentinel) sentinel := "<!-- review-bot:sonnet -->"
if got != tc.want { newURL := "https://gitea.example.com/owner/repo/pulls/1#pullrequestreview-99"
t.Errorf("reviewUnchanged() = %v, want %v", got, tc.want)
} result := buildSupersededBody(original, "abcdef1234567890", newURL, sentinel)
})
// Should contain the struck-through banner
if !strings.Contains(result, "~~Original review~~") {
t.Error("missing struck-through banner")
}
// Should contain superseded notice with link
if !strings.Contains(result, "**Superseded**") {
t.Error("missing superseded notice")
}
if !strings.Contains(result, "[see current review]("+newURL+")") {
t.Error("missing link to new review")
}
// Should contain collapsed original
if !strings.Contains(result, "<details>") {
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", "https://example.com/review", "<!-- review-bot:x -->")
if !strings.Contains(result, "abc") {
t.Error("short SHA not preserved")
} }
} }
@@ -174,6 +144,32 @@ func TestFindOwnReview(t *testing.T) {
sentinel: "<!-- review-bot:sonnet -->", sentinel: "<!-- review-bot:sonnet -->",
wantID: 20, wantID: 20,
}, },
{
name: "skips superseded review",
reviews: []gitea.Review{
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantID: 20,
},
{
name: "only superseded reviews exist",
reviews: []gitea.Review{
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantNil: true,
},
{
name: "picks highest ID among matches",
reviews: []gitea.Review{
makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantID: 50,
},
} }
for _, tc := range tests { for _, tc := range tests {
+117 -5
View File
@@ -82,6 +82,7 @@ type ChangedFile struct {
// ReviewComment represents an inline comment to attach to a review. // ReviewComment represents an inline comment to attach to a review.
type ReviewComment struct { type ReviewComment struct {
ID int64 `json:"id,omitempty"`
Path string `json:"path"` Path string `json:"path"`
NewPosition int64 `json:"new_position"` NewPosition int64 `json:"new_position"`
Body string `json:"body"` Body string `json:"body"`
@@ -316,13 +317,14 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
// Review represents a pull request review from the Gitea API. // Review represents a pull request review from the Gitea API.
type Review struct { type Review struct {
ID int64 `json:"id"` ID int64 `json:"id"`
Body string `json:"body"` Body string `json:"body"`
User struct { User struct {
Login string `json:"login"` Login string `json:"login"`
} `json:"user"` } `json:"user"`
State string `json:"state"` State string `json:"state"`
Stale bool `json:"stale"` Stale bool `json:"stale"`
CommitID string `json:"commit_id"`
} }
// ListReviews returns all reviews on a pull request. // ListReviews returns all reviews on a pull request.
@@ -459,3 +461,113 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID
} }
return nil return nil
} }
// GetAuthenticatedUser returns the login of the user authenticated by the token.
func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/user", c.baseURL)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return "", fmt.Errorf("get authenticated user: %w", err)
}
var result struct {
Login string `json:"login"`
}
if err := json.Unmarshal(body, &result); err != nil {
return "", fmt.Errorf("parse user response: %w", err)
}
return result.Login, nil
}
// RequestReviewer adds the given user as a requested reviewer on a pull request.
// This is idempotent — requesting an already-requested reviewer is a no-op.
func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/requested_reviewers",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
number)
payload := struct {
Reviewers []string `json:"reviewers"`
}{Reviewers: []string{reviewer}}
data, err := json.Marshal(payload)
if err != nil {
return fmt.Errorf("marshal reviewer request: %w", err)
}
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))
if err != nil {
return fmt.Errorf("create reviewer request: %w", err)
}
req.Header.Set("Authorization", "token "+c.token)
req.Header.Set("Content-Type", "application/json")
resp, err := c.http.Do(req)
if err != nil {
return fmt.Errorf("request reviewer: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
return fmt.Errorf("request reviewer failed (status %d): %s", resp.StatusCode, body)
}
return nil
}
// ListReviewComments returns the inline comments attached to a specific review.
// Paginates through all pages.
func (c *Client) ListReviewComments(ctx context.Context, owner, repo string, prNumber int, reviewID int64) ([]ReviewComment, error) {
const pageSize = 50
var all []ReviewComment
for page := 1; ; page++ {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments?limit=%d&page=%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
prNumber,
reviewID,
pageSize,
page)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return nil, fmt.Errorf("list review comments (page %d): %w", page, err)
}
var batch []ReviewComment
if err := json.Unmarshal(body, &batch); err != nil {
return nil, fmt.Errorf("parse review comments (page %d): %w", page, err)
}
all = append(all, batch...)
if len(batch) < pageSize {
break
}
}
return all, nil
}
// ResolveComment marks an inline review comment as resolved.
func (c *Client) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/comments/%d/resolve",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
commentID)
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, nil)
if err != nil {
return fmt.Errorf("create resolve request: %w", err)
}
req.Header.Set("Authorization", "token "+c.token)
resp, err := c.http.Do(req)
if err != nil {
return fmt.Errorf("resolve comment: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
return fmt.Errorf("resolve comment failed (status %d): %s", resp.StatusCode, body)
}
return nil
}
+141
View File
@@ -5,8 +5,10 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"io"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"strings"
"testing" "testing"
) )
@@ -602,3 +604,142 @@ func TestIsNotFound(t *testing.T) {
}) })
} }
} }
func TestGetAuthenticatedUser(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/user" {
t.Errorf("unexpected path: %s", r.URL.Path)
http.NotFound(w, r)
return
}
if r.Header.Get("Authorization") != "token test-token" {
t.Error("missing or wrong auth header")
}
w.Header().Set("Content-Type", "application/json")
fmt.Fprint(w, `{"login":"my-bot","id":42}`)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
login, err := client.GetAuthenticatedUser(context.Background())
if err != nil {
t.Fatalf("GetAuthenticatedUser() error = %v", err)
}
if login != "my-bot" {
t.Errorf("login = %q, want %q", login, "my-bot")
}
}
func TestRequestReviewer(t *testing.T) {
var gotBody []byte
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
expected := "/api/v1/repos/owner/repo/pulls/7/requested_reviewers"
if r.URL.Path != expected {
t.Errorf("path = %q, want %q", r.URL.Path, expected)
}
gotBody, _ = io.ReadAll(r.Body)
w.WriteHeader(http.StatusCreated)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.RequestReviewer(context.Background(), "owner", "repo", 7, "bot-user")
if err != nil {
t.Fatalf("RequestReviewer() error = %v", err)
}
if !strings.Contains(string(gotBody), `"bot-user"`) {
t.Errorf("body = %s, want to contain bot-user", gotBody)
}
}
func TestRequestReviewer_204(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNoContent)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
if err != nil {
t.Fatalf("RequestReviewer() should accept 204, got error = %v", err)
}
}
func TestRequestReviewer_Error(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
fmt.Fprint(w, "no permission")
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
if err == nil {
t.Fatal("expected error for 403 response")
}
if !strings.Contains(err.Error(), "403") {
t.Errorf("error should mention status code: %v", err)
}
}
func TestListReviewComments(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if !strings.Contains(r.URL.Path, "/pulls/1/reviews/42/comments") {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
fmt.Fprint(w, `[{"id":100,"path":"main.go","new_position":5,"body":"finding"},{"id":101,"path":"lib.go","new_position":10,"body":"another"}]`)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
comments, err := client.ListReviewComments(context.Background(), "owner", "repo", 1, 42)
if err != nil {
t.Fatalf("ListReviewComments() error = %v", err)
}
if len(comments) != 2 {
t.Fatalf("got %d comments, want 2", len(comments))
}
if comments[0].ID != 100 {
t.Errorf("comments[0].ID = %d, want 100", comments[0].ID)
}
if comments[1].Path != "lib.go" {
t.Errorf("comments[1].Path = %q, want %q", comments[1].Path, "lib.go")
}
}
func TestResolveComment(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
if !strings.Contains(r.URL.Path, "/pulls/comments/99/resolve") {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.WriteHeader(http.StatusOK)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
if err != nil {
t.Fatalf("ResolveComment() error = %v", err)
}
}
func TestResolveComment_Error(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
fmt.Fprint(w, "not found")
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
if err == nil {
t.Fatal("expected error for 404 response")
}
}