diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 4db81b4..3845bb5 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -14,8 +14,10 @@ import ( "gitea.weiker.me/rodin/review-bot/budget" "gitea.weiker.me/rodin/review-bot/gitea" + "gitea.weiker.me/rodin/review-bot/github" "gitea.weiker.me/rodin/review-bot/llm" "gitea.weiker.me/rodin/review-bot/review" + "gitea.weiker.me/rodin/review-bot/vcs" ) var version = "dev" @@ -84,6 +86,7 @@ func main() { systemPromptFile := flag.String("system-prompt-file", envOrDefault("SYSTEM_PROMPT_FILE", ""), "Local file with additional system prompt instructions") 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", ""), "Comma-separated file paths to fetch from patterns repo (empty = all files)") + vcsType := flag.String("vcs-type", envOrDefault("VCS_TYPE", "gitea"), "VCS type: gitea or github") dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting") 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)") @@ -168,8 +171,19 @@ func main() { os.Exit(1) } - // Initialize clients - giteaClient := gitea.NewClient(*vcsURL, *reviewerToken) + // Initialize VCS client + var vcsClientImpl vcsClient + switch strings.ToLower(*vcsType) { + case "github": + vcsClientImpl = newGitHubVCSAdapter(github.NewClient(*reviewerToken, *vcsURL)) + slog.Info("using GitHub VCS client", "url", *vcsURL) + case "gitea", "": + vcsClientImpl = newGiteaVCSAdapter(gitea.NewClient(*vcsURL, *reviewerToken)) + slog.Info("using Gitea VCS client", "url", *vcsURL) + default: + slog.Error("invalid vcs-type", "type", *vcsType, "valid", "gitea, github") + os.Exit(1) + } llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel) if *llmTemp < 0 || *llmTemp > 2 { slog.Error("invalid LLM temperature", "temperature", *llmTemp, "range", "0-2") @@ -207,7 +221,7 @@ func main() { var persona *review.Persona if *personaName != "" { // Try loading from repo first, then fall back to built-in - repoPersonas, err := review.LoadRepoPersonas(ctx, newGiteaClientAdapter(giteaClient), owner, repoName) + repoPersonas, err := review.LoadRepoPersonas(ctx, newReviewClientAdapter(vcsClientImpl), owner, repoName) if err != nil { slog.Warn("could not load repo personas", "repo", owner+"/"+repoName, "error", err) // Continue with built-in personas only. @@ -243,7 +257,7 @@ func main() { slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName)) // Step 1: Fetch PR metadata - pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) + pr, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber) if err != nil { slog.Error("failed to fetch PR", "pr", prNumber, "error", err) os.Exit(1) @@ -251,7 +265,7 @@ func main() { slog.Info("fetched PR metadata", "pr", prNumber, "title", pr.Title) // Step 2: Fetch diff - diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber) + diff, err := vcsClientImpl.GetPullRequestDiff(ctx, owner, repoName, prNumber) if err != nil { slog.Error("failed to fetch diff", "pr", prNumber, "error", err) os.Exit(1) @@ -260,21 +274,21 @@ func main() { // Step 3: Fetch full file content for modified files fileContext := "" - files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber) + files, err := vcsClientImpl.GetPullRequestFiles(ctx, owner, repoName, prNumber) if err != nil { slog.Warn("could not fetch PR files list", "pr", prNumber, "error", err) } else { - fileContext = fetchFileContext(ctx, giteaClient, owner, repoName, pr.Head.Ref, files) + fileContext = fetchFileContext(ctx, vcsClientImpl, owner, repoName, pr.HeadRef, files) slog.Debug("fetched file context", "files", len(files)) } // Step 4: Check CI status ciPassed := true ciDetails := "" - if pr.Head.Sha != "" { - statuses, err := giteaClient.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha) + if pr.HeadSha != "" { + statuses, err := vcsClientImpl.GetCommitStatuses(ctx, owner, repoName, pr.HeadSha) if err != nil { - slog.Warn("could not fetch CI status", "sha", pr.Head.Sha, "error", err) + slog.Warn("could not fetch CI status", "sha", pr.HeadSha, "error", err) } else { ciPassed, ciDetails = evaluateCIStatus(statuses) slog.Info("CI status checked", "passed", ciPassed) @@ -284,7 +298,7 @@ func main() { // Step 5: Load conventions file if specified conventions := "" if *conventionsFile != "" { - content, err := giteaClient.GetFileContent(ctx, owner, repoName, *conventionsFile) + content, err := vcsClientImpl.GetFileContent(ctx, owner, repoName, *conventionsFile) if err != nil { slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err) } else { @@ -296,7 +310,7 @@ func main() { // Step 6: Load patterns from external repo if specified patterns := "" if *patternsRepo != "" { - patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles) + patterns = fetchPatterns(ctx, vcsClientImpl, *patternsRepo, *patternsFiles) slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns)) } @@ -389,15 +403,15 @@ func main() { } // Add commit footer so readers know which commit was evaluated - if pr.Head.Sha != "" { - shortSHA := pr.Head.Sha + if pr.HeadSha != "" { + shortSHA := pr.HeadSha if len(shortSHA) > 8 { shortSHA = shortSHA[:8] } reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA) } - event := review.GiteaEvent(result.Verdict) + event := verdictToVCSEvent(result.Verdict) if *dryRun { fmt.Println("--- DRY RUN ---") @@ -409,14 +423,14 @@ func main() { sentinel := fmt.Sprintf("", *reviewerName) // Stale check: verify HEAD hasn't moved since we started - evaluatedSHA := pr.Head.Sha + evaluatedSHA := pr.HeadSha var currentSHA string - currentPR, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) + currentPR, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber) if err != nil { slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err) // currentSHA stays empty — shouldSkipStaleReview will return false } else { - currentSHA = currentPR.Head.Sha + currentSHA = currentPR.HeadSha } if shouldSkipStaleReview(evaluatedSHA, currentSHA) { slog.Warn("HEAD moved during review — skipping stale review", @@ -427,28 +441,30 @@ func main() { } // Map findings to inline comments for lines present in the diff - diffRanges := gitea.ParseDiffNewLines(diff) - var inlineComments []gitea.ReviewComment - for _, f := range result.Findings { - if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) { - inlineComments = append(inlineComments, gitea.ReviewComment{ - Path: f.File, - NewPosition: int64(f.Line), - Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding), - }) + var inlineComments []vcs.ReviewComment + if ext, ok := vcsClientImpl.(giteaExtendedClient); ok { + diffRanges := ext.ParseDiffNewLines(diff) + for _, f := range result.Findings { + if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) { + inlineComments = append(inlineComments, vcs.ReviewComment{ + Path: f.File, + Position: f.Line, + Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding), + }) + } + } + if len(inlineComments) > 0 { + slog.Debug("attaching inline comments", "count", len(inlineComments)) } - } - if len(inlineComments) > 0 { - slog.Debug("attaching inline comments", "count", len(inlineComments)) } // --- Review update strategy --- // 1. POST new review first (gets non-stale approval badge on HEAD) // 2. Then supersede old review with link to the new one // Order matters: post first so we have the new review's URL for the supersede message. - var oldReviews []gitea.Review + var oldReviews []reviewInfo if *reviewerName != "" { - existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) + existingReviews, err := vcsClientImpl.ListReviews(ctx, owner, repoName, prNumber) if err != nil { slog.Warn("could not list existing reviews", "pr", prNumber, "error", err) } else { @@ -461,20 +477,27 @@ func main() { } // Self-request as reviewer (ensures we appear in required-reviewer checks) - authUser, err := giteaClient.GetAuthenticatedUser(ctx) + authUser, err := vcsClientImpl.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) + if ext, ok := vcsClientImpl.(giteaExtendedClient); ok { + if err := ext.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) - posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, evaluatedSHA, inlineComments) + posted, err := vcsClientImpl.PostReview(ctx, owner, repoName, prNumber, vcs.ReviewRequest{ + Body: reviewBody, + Event: event, + CommitID: evaluatedSHA, + Comments: inlineComments, + }) if err != nil { slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err) os.Exit(1) @@ -484,21 +507,26 @@ func main() { // Supersede all old reviews with link to the new one if len(oldReviews) > 0 { newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*vcsURL, "/"), owner, repoName, prNumber, posted.ID) + ext, hasExt := vcsClientImpl.(giteaExtendedClient) for _, oldReview := range oldReviews { - cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID) + if !hasExt { + slog.Debug("VCS client does not support review supersede; skipping", "review_id", oldReview.ID) + continue + } + cid, err := ext.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID) if err != nil { slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err) continue } supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel) - if err := giteaClient.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil { + if err := ext.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil { slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err) continue } slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber) // Resolve old review's inline comments - oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID) + oldComments, err := ext.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID) if err != nil { slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err) continue @@ -508,7 +536,7 @@ func main() { if c.ID == 0 { continue } - if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil { + if err := ext.ResolveComment(ctx, owner, repoName, c.ID); err != nil { slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err) failed++ } else { @@ -527,7 +555,7 @@ func main() { } // fetchFileContext fetches the full content of modified files from the PR branch. -func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, ref string, files []gitea.ChangedFile) string { +func fetchFileContext(ctx context.Context, client vcsClient, owner, repo, ref string, files []changedFileInfo) string { var sb strings.Builder for _, f := range files { if ctx.Err() != nil { @@ -554,7 +582,7 @@ func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, re // patternsFiles is comma-separated list of file paths or directories. // If a path ends with / or is a directory, all files within it are fetched recursively. // If patternsFiles is empty, all files from the repo root are fetched. -func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patternsFiles string) string { +func fetchPatterns(ctx context.Context, client vcsClient, patternsRepo, patternsFiles string) string { var sb strings.Builder repos := strings.Split(patternsRepo, ",") @@ -591,8 +619,13 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt var repoLoadedFiles []string var repoSkippedFiles []string + giteaRaw, isGitea := client.(*giteaClientVCSAdapter) + if !isGitea { + slog.Warn("patterns fetching is only supported with the Gitea VCS client; skipping", "repo", repoRef) + continue + } for _, path := range paths { - files, err := client.GetAllFilesInPath(ctx, owner, repo, path) + files, err := giteaRaw.client.GetAllFilesInPath(ctx, owner, repo, path) if err != nil { slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err) continue @@ -631,7 +664,7 @@ func isPatternFile(path string) bool { } // evaluateCIStatus checks if all CI statuses indicate success. -func evaluateCIStatus(statuses []gitea.CommitStatus) (passed bool, details string) { +func evaluateCIStatus(statuses []commitStatusInfo) (passed bool, details string) { if len(statuses) == 0 { return true, "no CI statuses found" } @@ -769,7 +802,7 @@ func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string) // Gitea user. This indicates misconfiguration where two roles share a token // instead of having separate Gitea accounts. Returns true if shared token // detected (caller should skip update-in-place logic to avoid clobbering). -func hasSharedToken(reviews []gitea.Review, ownSentinel string) bool { +func hasSharedToken(reviews []reviewInfo, ownSentinel string) bool { ownLogin := "" for _, r := range reviews { if strings.Contains(r.Body, ownSentinel) { @@ -807,8 +840,8 @@ func extractSentinelName(body string) string { } // findOwnReview locates the most recent non-superseded review matching the sentinel. -func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review { - var best *gitea.Review +func findOwnReview(reviews []reviewInfo, sentinel string) *reviewInfo { + var best *reviewInfo for i := range reviews { if !strings.Contains(reviews[i].Body, sentinel) { continue @@ -824,8 +857,8 @@ func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review { } // findAllOwnReviews returns all non-superseded reviews matching the sentinel. -func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review { - var result []gitea.Review +func findAllOwnReviews(reviews []reviewInfo, sentinel string) []reviewInfo { + var result []reviewInfo for i := range reviews { if !strings.Contains(reviews[i].Body, sentinel) { continue @@ -838,6 +871,22 @@ func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review { return result } +// verdictToVCSEvent converts a review verdict string to a vcs.ReviewEvent. +// The verdict comes from the LLM result and uses values: "APPROVE", "REQUEST_CHANGES", +// or any other string (treated as COMMENT). +// vcs.ReviewEvent constants follow GitHub API format ("APPROVE", "REQUEST_CHANGES", "COMMENT"). +// The Gitea adapter translates these back to Gitea format ("APPROVED", etc.) before posting. +func verdictToVCSEvent(verdict string) vcs.ReviewEvent { + switch verdict { + case "APPROVE": + return vcs.ReviewEventApprove + case "REQUEST_CHANGES": + return vcs.ReviewEventRequestChanges + default: + return vcs.ReviewEventComment + } +} + // shouldSkipStaleReview reports whether to skip posting because HEAD moved. // Returns true (skip) if evaluatedSHA differs from currentSHA. // Returns false (don't skip) if: @@ -851,16 +900,17 @@ func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool { return evaluatedSHA != currentSHA } -// giteaClientAdapter adapts gitea.Client to review.GiteaClient interface. -type giteaClientAdapter struct { - client *gitea.Client +// reviewClientAdapter adapts a vcsClient to review.GiteaClient for persona loading. +// The review package only needs ListContents and GetFileContent, which all vcsClients provide. +type reviewClientAdapter struct { + client vcsClient } -func newGiteaClientAdapter(c *gitea.Client) *giteaClientAdapter { - return &giteaClientAdapter{client: c} +func newReviewClientAdapter(c vcsClient) *reviewClientAdapter { + return &reviewClientAdapter{client: c} } -func (a *giteaClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) { +func (a *reviewClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) { entries, err := a.client.ListContents(ctx, owner, repo, path) if err != nil { return nil, err @@ -876,6 +926,6 @@ func (a *giteaClientAdapter) ListContents(ctx context.Context, owner, repo, path return result, nil } -func (a *giteaClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { +func (a *reviewClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { return a.client.GetFileContent(ctx, owner, repo, filepath) } diff --git a/cmd/review-bot/main_test.go b/cmd/review-bot/main_test.go index 539d1b7..5c4162d 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -10,7 +10,7 @@ import ( "strings" "testing" - "gitea.weiker.me/rodin/review-bot/gitea" + "gitea.weiker.me/rodin/review-bot/vcs" ) func TestValidateReviewerName(t *testing.T) { @@ -154,14 +154,14 @@ func TestValidateWorkspacePath(t *testing.T) { } } -func makeReview(id int64, login, state string, stale bool, body string) gitea.Review { - r := gitea.Review{ +func makeReview(id int64, login, state string, stale bool, body string) reviewInfo { + r := reviewInfo{ ID: id, Body: body, State: state, Stale: stale, } - r.User.Login = login + r.User = vcs.UserInfo{Login: login} return r } @@ -216,7 +216,7 @@ func TestBuildSupersededBodyShortSHA(t *testing.T) { func TestFindOwnReview(t *testing.T) { tests := []struct { name string - reviews []gitea.Review + reviews []reviewInfo sentinel string wantID int64 wantNil bool @@ -229,7 +229,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "found by sentinel", - reviews: []gitea.Review{ + reviews: []reviewInfo{ makeReview(42, "bot", "APPROVED", false, "review body\n"), }, sentinel: "", @@ -237,7 +237,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "wrong sentinel", - reviews: []gitea.Review{ + reviews: []reviewInfo{ makeReview(42, "bot", "APPROVED", false, "body\n"), }, sentinel: "", @@ -245,7 +245,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "multiple reviews, returns first match", - reviews: []gitea.Review{ + reviews: []reviewInfo{ makeReview(10, "bot", "APPROVED", false, "old\n"), makeReview(20, "bot", "APPROVED", false, "new\n"), }, @@ -254,7 +254,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "skips superseded review", - reviews: []gitea.Review{ + reviews: []reviewInfo{ makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n"), makeReview(20, "bot", "APPROVED", false, "fresh review\n"), }, @@ -263,7 +263,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "only superseded reviews exist", - reviews: []gitea.Review{ + reviews: []reviewInfo{ makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n"), }, sentinel: "", @@ -271,7 +271,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "picks highest ID among matches", - reviews: []gitea.Review{ + reviews: []reviewInfo{ makeReview(50, "bot", "APPROVED", false, "v1\n"), makeReview(30, "bot", "APPROVED", false, "v0\n"), }, @@ -302,7 +302,7 @@ func TestFindOwnReview(t *testing.T) { func TestHasSharedToken(t *testing.T) { tests := []struct { name string - reviews []gitea.Review + reviews []reviewInfo sentinel string want bool }{ @@ -314,36 +314,36 @@ func TestHasSharedToken(t *testing.T) { }, { name: "no own review yet - cannot detect", - reviews: []gitea.Review{ - {ID: 1, User: struct{ Login string `json:"login"` }{Login: "other"}, Body: " body"}, + reviews: []reviewInfo{ + {ID: 1, User: vcs.UserInfo{Login: "other"}, Body: " body"}, }, sentinel: "", want: false, }, { name: "separate users - no shared token", - reviews: []gitea.Review{ - {ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: " body"}, - {ID: 2, User: struct{ Login string `json:"login"` }{Login: "security-review-bot"}, Body: " body"}, + reviews: []reviewInfo{ + {ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: " body"}, + {ID: 2, User: vcs.UserInfo{Login: "security-review-bot"}, Body: " body"}, }, sentinel: "", want: false, }, { name: "shared token detected - same user different sentinels", - reviews: []gitea.Review{ - {ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: " body"}, - {ID: 2, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: " body"}, + reviews: []reviewInfo{ + {ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: " body"}, + {ID: 2, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: " body"}, }, sentinel: "", want: true, }, { name: "three roles same user", - reviews: []gitea.Review{ - {ID: 1, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: " body"}, - {ID: 2, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: " body"}, - {ID: 3, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: " body"}, + reviews: []reviewInfo{ + {ID: 1, User: vcs.UserInfo{Login: "bot"}, Body: " body"}, + {ID: 2, User: vcs.UserInfo{Login: "bot"}, Body: " body"}, + {ID: 3, User: vcs.UserInfo{Login: "bot"}, Body: " body"}, }, sentinel: "", want: true, @@ -553,7 +553,7 @@ func TestBuildPatternPaths(t *testing.T) { func TestEvaluateCIStatus(t *testing.T) { tests := []struct { name string - statuses []gitea.CommitStatus + statuses []commitStatusInfo wantPassed bool wantSubstr string }{ @@ -565,7 +565,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "all success", - statuses: []gitea.CommitStatus{ + statuses: []commitStatusInfo{ {Status: "success", Context: "ci/build", Description: "Build passed"}, {Status: "success", Context: "ci/test", Description: "Tests passed"}, }, @@ -574,7 +574,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "one failure", - statuses: []gitea.CommitStatus{ + statuses: []commitStatusInfo{ {Status: "success", Context: "ci/build", Description: "Build passed"}, {Status: "failure", Context: "ci/test", Description: "Tests failed"}, }, @@ -583,7 +583,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "error status", - statuses: []gitea.CommitStatus{ + statuses: []commitStatusInfo{ {Status: "error", Context: "ci/lint", Description: "Lint error"}, }, wantPassed: false, @@ -591,7 +591,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "pending treated as not-failed", - statuses: []gitea.CommitStatus{ + statuses: []commitStatusInfo{ {Status: "pending", Context: "ci/build", Description: "In progress"}, {Status: "success", Context: "ci/test", Description: "Tests passed"}, }, @@ -600,7 +600,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "multiple failures", - statuses: []gitea.CommitStatus{ + statuses: []commitStatusInfo{ {Status: "failure", Context: "ci/build", Description: "Build failed"}, {Status: "failure", Context: "ci/test", Description: "Tests failed"}, }, @@ -609,7 +609,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "mixed with pending and failure", - statuses: []gitea.CommitStatus{ + statuses: []commitStatusInfo{ {Status: "success", Context: "ci/build", Description: "Build passed"}, {Status: "pending", Context: "ci/deploy", Description: "Deploying"}, {Status: "failure", Context: "ci/test", Description: "Tests failed"}, @@ -997,7 +997,7 @@ func cleanEnv() []string { } func TestFindAllOwnReviews(t *testing.T) { - reviews := []gitea.Review{ + reviews := []reviewInfo{ {ID: 1, Body: "\nfirst review"}, {ID: 2, Body: "\nother bot"}, {ID: 3, Body: "\nsecond review"}, diff --git a/cmd/review-bot/vcs_client.go b/cmd/review-bot/vcs_client.go new file mode 100644 index 0000000..799e2c3 --- /dev/null +++ b/cmd/review-bot/vcs_client.go @@ -0,0 +1,405 @@ +package main + +import ( + "context" + + "gitea.weiker.me/rodin/review-bot/gitea" + "gitea.weiker.me/rodin/review-bot/github" + "gitea.weiker.me/rodin/review-bot/vcs" +) + +// vcsClient is the unified interface for VCS operations used by the review flow. +// Both gitea.Client and github.Client satisfy this interface via their respective adapters. +type vcsClient interface { + // GetPullRequest fetches PR metadata. + GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error) + + // GetPullRequestDiff fetches the unified diff. + GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) + + // GetPullRequestFiles fetches the list of changed files. + GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error) + + // GetCommitStatuses fetches CI statuses for a commit. + GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error) + + // GetFileContent fetches the content of a file at HEAD. + GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) + + // GetFileContentRef fetches the content of a file at a given ref. + GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) + + // ListContents lists the files and directories at a path. + ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error) + + // ListReviews returns all reviews for a PR. + ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error) + + // PostReview submits a review. + PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error) + + // GetAuthenticatedUser returns the login of the authenticated user. + GetAuthenticatedUser(ctx context.Context) (string, error) +} + +// giteaExtendedClient is implemented by the Gitea adapter and exposes +// Gitea-specific operations that have no GitHub equivalent in the current scope. +// Callers should type-assert to this interface and skip gracefully when it is absent. +type giteaExtendedClient interface { + // RequestReviewer adds the authenticated user as a reviewer on a PR. + RequestReviewer(ctx context.Context, owner, repo string, number int, user string) error + + // GetTimelineReviewCommentIDForReview maps a review ID to its timeline comment ID. + GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) + + // EditComment updates the body of an existing PR comment. + EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error + + // ListReviewComments lists the inline comments attached to a review. + ListReviewComments(ctx context.Context, owner, repo string, number int, reviewID int64) ([]inlineCommentInfo, error) + + // ResolveComment marks an inline comment as resolved. + ResolveComment(ctx context.Context, owner, repo string, commentID int64) error + + // ParseDiffNewLines returns the diff line ranges for inline comment positioning. + ParseDiffNewLines(diff string) diffLineRanges +} + +// Shared adapter types to avoid duplicating gitea/github-specific types throughout main.go. + +type pullRequestInfo struct { + Title string + Body string + HeadSha string + HeadRef string +} + +type changedFileInfo struct { + Filename string + Status string +} + +type commitStatusInfo struct { + Status string + Context string + Description string + TargetURL string +} + +type contentEntryInfo struct { + Name string + Path string + Type string +} + +type reviewInfo struct { + ID int64 + Body string + User vcs.UserInfo + State string + CommitID string + Stale bool +} + +type inlineCommentInfo struct { + ID int64 + Path string + NewPosition int64 + Body string +} + +// diffLineRanges is a type alias for gitea.DiffLineRanges to allow the +// extended client interface to be defined without importing gitea directly. +// In practice, only the giteaClientVCSAdapter returns this type, and callers +// that use it will already have performed the type assertion. +type diffLineRanges = *gitea.DiffLineRanges + +// --- Gitea adapter --- + +// giteaClientVCSAdapter wraps gitea.Client to satisfy the vcsClient interface. +// It also implements giteaExtendedClient for Gitea-specific operations. +type giteaClientVCSAdapter struct { + client *gitea.Client +} + +func newGiteaVCSAdapter(c *gitea.Client) *giteaClientVCSAdapter { + return &giteaClientVCSAdapter{client: c} +} + +func (a *giteaClientVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error) { + pr, err := a.client.GetPullRequest(ctx, owner, repo, number) + if err != nil { + return nil, err + } + return &pullRequestInfo{ + Title: pr.Title, + Body: pr.Body, + HeadSha: pr.Head.Sha, + HeadRef: pr.Head.Ref, + }, nil +} + +func (a *giteaClientVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { + return a.client.GetPullRequestDiff(ctx, owner, repo, number) +} + +func (a *giteaClientVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error) { + files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number) + if err != nil { + return nil, err + } + result := make([]changedFileInfo, len(files)) + for i, f := range files { + result[i] = changedFileInfo{Filename: f.Filename, Status: f.Status} + } + return result, nil +} + +func (a *giteaClientVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error) { + statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha) + if err != nil { + return nil, err + } + result := make([]commitStatusInfo, len(statuses)) + for i, s := range statuses { + result[i] = commitStatusInfo{ + Status: s.Status, + Context: s.Context, + Description: s.Description, + TargetURL: s.TargetURL, + } + } + return result, nil +} + +func (a *giteaClientVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { + return a.client.GetFileContent(ctx, owner, repo, filepath) +} + +func (a *giteaClientVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + return a.client.GetFileContentRef(ctx, owner, repo, filepath, ref) +} + +func (a *giteaClientVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error) { + entries, err := a.client.ListContents(ctx, owner, repo, path) + if err != nil { + return nil, err + } + result := make([]contentEntryInfo, len(entries)) + for i, e := range entries { + result[i] = contentEntryInfo{Name: e.Name, Path: e.Path, Type: e.Type} + } + return result, nil +} + +func (a *giteaClientVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error) { + reviews, err := a.client.ListReviews(ctx, owner, repo, number) + if err != nil { + return nil, err + } + result := make([]reviewInfo, len(reviews)) + for i, r := range reviews { + result[i] = reviewInfo{ + ID: r.ID, + Body: r.Body, + User: vcs.UserInfo{Login: r.User.Login}, + State: r.State, + CommitID: r.CommitID, + Stale: r.Stale, + } + } + return result, nil +} + +func (a *giteaClientVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error) { + // Translate vcs.ReviewComment to gitea.ReviewComment. + // The Gitea API uses NewPosition instead of Position. + var comments []gitea.ReviewComment + for _, c := range req.Comments { + comments = append(comments, gitea.ReviewComment{ + Path: c.Path, + NewPosition: int64(c.Position), + Body: c.Body, + }) + } + + // Translate vcs.ReviewEvent (GitHub format) to Gitea API event string. + // vcs uses "APPROVE"; Gitea API expects "APPROVED". + var event string + switch req.Event { + case vcs.ReviewEventApprove: + event = "APPROVED" + case vcs.ReviewEventRequestChanges: + event = "REQUEST_CHANGES" + default: + event = "COMMENT" + } + + posted, err := a.client.PostReview(ctx, owner, repo, number, event, req.Body, req.CommitID, comments) + if err != nil { + return nil, err + } + return &reviewInfo{ + ID: posted.ID, + Body: posted.Body, + User: vcs.UserInfo{Login: posted.User.Login}, + State: posted.State, + CommitID: posted.CommitID, + }, nil +} + +func (a *giteaClientVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) { + return a.client.GetAuthenticatedUser(ctx) +} + +// giteaExtendedClient implementation: + +func (a *giteaClientVCSAdapter) RequestReviewer(ctx context.Context, owner, repo string, number int, user string) error { + return a.client.RequestReviewer(ctx, owner, repo, number, user) +} + +func (a *giteaClientVCSAdapter) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) { + return a.client.GetTimelineReviewCommentIDForReview(ctx, owner, repo, number, reviewID) +} + +func (a *giteaClientVCSAdapter) EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error { + return a.client.EditComment(ctx, owner, repo, commentID, body) +} + +func (a *giteaClientVCSAdapter) ListReviewComments(ctx context.Context, owner, repo string, number int, reviewID int64) ([]inlineCommentInfo, error) { + comments, err := a.client.ListReviewComments(ctx, owner, repo, number, reviewID) + if err != nil { + return nil, err + } + result := make([]inlineCommentInfo, len(comments)) + for i, c := range comments { + result[i] = inlineCommentInfo{ + ID: c.ID, + Path: c.Path, + NewPosition: c.NewPosition, + Body: c.Body, + } + } + return result, nil +} + +func (a *giteaClientVCSAdapter) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error { + return a.client.ResolveComment(ctx, owner, repo, commentID) +} + +func (a *giteaClientVCSAdapter) ParseDiffNewLines(diff string) diffLineRanges { + return gitea.ParseDiffNewLines(diff) +} + +// --- GitHub adapter --- + +// githubClientVCSAdapter wraps github.Client to satisfy the vcsClient interface. +type githubClientVCSAdapter struct { + client *github.Client +} + +func newGitHubVCSAdapter(c *github.Client) *githubClientVCSAdapter { + return &githubClientVCSAdapter{client: c} +} + +func (a *githubClientVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error) { + pr, err := a.client.GetPullRequest(ctx, owner, repo, number) + if err != nil { + return nil, err + } + return &pullRequestInfo{ + Title: pr.Title, + Body: pr.Body, + HeadSha: pr.Head.Sha, + HeadRef: pr.Head.Ref, + }, nil +} + +func (a *githubClientVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { + return a.client.GetPullRequestDiff(ctx, owner, repo, number) +} + +func (a *githubClientVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error) { + files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number) + if err != nil { + return nil, err + } + result := make([]changedFileInfo, len(files)) + for i, f := range files { + result[i] = changedFileInfo{Filename: f.Filename, Status: f.Status} + } + return result, nil +} + +func (a *githubClientVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error) { + statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha) + if err != nil { + return nil, err + } + result := make([]commitStatusInfo, len(statuses)) + for i, s := range statuses { + result[i] = commitStatusInfo{ + Status: s.Status, + Context: s.Context, + Description: s.Description, + TargetURL: s.TargetURL, + } + } + return result, nil +} + +func (a *githubClientVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { + return a.client.GetFileContent(ctx, owner, repo, filepath) +} + +func (a *githubClientVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + return a.client.GetFileContentRef(ctx, owner, repo, filepath, ref) +} + +func (a *githubClientVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error) { + entries, err := a.client.ListContents(ctx, owner, repo, path) + if err != nil { + return nil, err + } + result := make([]contentEntryInfo, len(entries)) + for i, e := range entries { + result[i] = contentEntryInfo{Name: e.Name, Path: e.Path, Type: e.Type} + } + return result, nil +} + +func (a *githubClientVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error) { + reviews, err := a.client.ListReviews(ctx, owner, repo, number) + if err != nil { + return nil, err + } + result := make([]reviewInfo, len(reviews)) + for i, r := range reviews { + result[i] = reviewInfo{ + ID: r.ID, + Body: r.Body, + User: r.User, + State: r.State, + CommitID: r.CommitID, + } + } + return result, nil +} + +func (a *githubClientVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error) { + posted, err := a.client.PostReview(ctx, owner, repo, number, req) + if err != nil { + return nil, err + } + return &reviewInfo{ + ID: posted.ID, + Body: posted.Body, + User: posted.User, + State: posted.State, + CommitID: posted.CommitID, + }, nil +} + +func (a *githubClientVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) { + return a.client.GetAuthenticatedUser(ctx) +} diff --git a/github/review.go b/github/review.go index 1a54fcf..3fddd4e 100644 --- a/github/review.go +++ b/github/review.go @@ -90,13 +90,13 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) payload := postReviewRequest{ - Body: req.Body, - Event: string(req.Event), + Body: req.Body, + Event: string(req.Event), + CommitID: req.CommitID, } - // Build the payload in one pass. The GitHub API accepts a single commit_id - // per review; we extract it from the first comment that supplies one and - // reject the request if any other comment disagrees. + // Build the comments in one pass. Inline comment CommitIDs must match the + // review-level CommitID; reject if any disagree. for _, comment := range req.Comments { if comment.CommitID != "" { if payload.CommitID == "" { diff --git a/vcs/types.go b/vcs/types.go index c4f4512..e3cb44e 100644 --- a/vcs/types.go +++ b/vcs/types.go @@ -48,6 +48,12 @@ type ReviewRequest struct { // Event is the review verdict. Event ReviewEvent + // CommitID is the commit SHA that this review targets. + // When non-empty, the review is anchored to this commit. + // For GitHub, this is passed as commit_id in the review request. + // For Gitea, this is passed as the commitID parameter to PostReview. + CommitID string + // Comments are optional inline file-level comments. Comments []ReviewComment }