From 10ef451c20d6e4740b8d26b83c3fc8a0e0d03d97 Mon Sep 17 00:00:00 2001 From: Rodin Date: Thu, 14 May 2026 20:43:21 +0000 Subject: [PATCH] feat(cmd): add VCS routing for GitHub PR reviews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire up the new GitHub API methods to the review-bot CLI via VCS type detection. review-bot can now review PRs on both Gitea and GitHub (including GitHub Enterprise Server). Changes: - vcs.go: define vcsClient interface with all PR operations - giteaVCSAdapter: wraps gitea.Client, satisfies vcsClient + giteaExtClient - githubVCSAdapter: wraps github.Client, satisfies vcsClient - giteaExtClient: Gitea-specific extension (supersede, comment resolution) - main.go: detect VCS type via VCS_TYPE env var (auto-detects github.com URLs) - Creates appropriate client (gitea or github) based on vcs_type - GitHub API URL derived from server URL (github.com → api.github.com, GHES → /api/v3) - All main flow uses vcsClient interface - Gitea-specific supersede operations gated via giteaExtClient type assertion - GitHub: logs info when skipping supersede (not supported) - Removes old giteaClientAdapter (replaced by giteaVCSAdapter in vcs.go) - giteaVCSAdapter satisfies review.GiteaClient for persona loading GitHub limitations handled gracefully: - Review supersede skipped (GitHub doesn't allow editing submitted reviews) - DeleteReview returns error for non-pending reviews (documented in adapter) - Inline comments use absolute line + side='RIGHT' instead of diff position Closes #130. Co-authored-by: Rodin --- cmd/review-bot/main.go | 140 ++++++++------ cmd/review-bot/main_test.go | 62 +++---- cmd/review-bot/vcs.go | 361 ++++++++++++++++++++++++++++++++++++ 3 files changed, 473 insertions(+), 90 deletions(-) create mode 100644 cmd/review-bot/vcs.go diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 4db81b4..1771f94 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -14,6 +14,7 @@ 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" ) @@ -169,7 +170,39 @@ func main() { } // Initialize clients - giteaClient := gitea.NewClient(*vcsURL, *reviewerToken) + // Detect VCS type: explicit flag > env var > URL heuristic (default: gitea). + vcsType := envOrDefault("VCS_TYPE", "") + if vcsType == "" { + // Heuristic: if the URL looks like github.com or a GitHub Enterprise host, + // default to GitHub. The composite action sets VCS_TYPE explicitly, so this + // is a fallback for manual invocations. + if strings.Contains(*vcsURL, "github.com") || strings.Contains(*vcsURL, "github.concur.com") { + vcsType = "github" + } else { + vcsType = "gitea" + } + } + slog.Info("VCS type detected", "vcs_type", vcsType, "vcs_url", *vcsURL) + + var vcs vcsClient + switch vcsType { + case "github": + // GitHub: baseURL is the API URL, derived from server URL. + // github.com → https://api.github.com + // GHES (e.g. https://ghe.example.com) → https://ghe.example.com/api/v3 + apiURL := githubAPIURL(*vcsURL) + ghClient := github.NewClient(*reviewerToken, apiURL) + vcs = newGithubVCSAdapter(ghClient) + slog.Info("using GitHub VCS client", "api_url", apiURL) + case "gitea": + giteaClient := gitea.NewClient(*vcsURL, *reviewerToken) + vcs = newGiteaVCSAdapter(giteaClient) + slog.Info("using Gitea VCS client", "url", *vcsURL) + default: + slog.Error("unsupported VCS type", "vcs_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 +240,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, vcs, 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 +276,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 := vcs.GetPullRequest(ctx, owner, repoName, prNumber) if err != nil { slog.Error("failed to fetch PR", "pr", prNumber, "error", err) os.Exit(1) @@ -251,7 +284,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 := vcs.GetPullRequestDiff(ctx, owner, repoName, prNumber) if err != nil { slog.Error("failed to fetch diff", "pr", prNumber, "error", err) os.Exit(1) @@ -260,11 +293,11 @@ func main() { // Step 3: Fetch full file content for modified files fileContext := "" - files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber) + files, err := vcs.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, vcs, owner, repoName, pr.Head.Ref, files) slog.Debug("fetched file context", "files", len(files)) } @@ -272,7 +305,7 @@ func main() { ciPassed := true ciDetails := "" if pr.Head.Sha != "" { - statuses, err := giteaClient.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha) + statuses, err := vcs.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha) if err != nil { slog.Warn("could not fetch CI status", "sha", pr.Head.Sha, "error", err) } else { @@ -284,7 +317,7 @@ func main() { // Step 5: Load conventions file if specified conventions := "" if *conventionsFile != "" { - content, err := giteaClient.GetFileContent(ctx, owner, repoName, *conventionsFile) + content, err := vcs.GetFileContent(ctx, owner, repoName, *conventionsFile) if err != nil { slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err) } else { @@ -296,7 +329,7 @@ func main() { // Step 6: Load patterns from external repo if specified patterns := "" if *patternsRepo != "" { - patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles) + patterns = fetchPatterns(ctx, vcs, *patternsRepo, *patternsFiles) slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns)) } @@ -411,7 +444,7 @@ func main() { // Stale check: verify HEAD hasn't moved since we started evaluatedSHA := pr.Head.Sha var currentSHA string - currentPR, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) + currentPR, err := vcs.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 @@ -428,10 +461,10 @@ func main() { // Map findings to inline comments for lines present in the diff diffRanges := gitea.ParseDiffNewLines(diff) - var inlineComments []gitea.ReviewComment + var inlineComments []vcsReviewComment for _, f := range result.Findings { if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) { - inlineComments = append(inlineComments, gitea.ReviewComment{ + inlineComments = append(inlineComments, vcsReviewComment{ Path: f.File, NewPosition: int64(f.Line), Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding), @@ -446,9 +479,9 @@ func main() { // 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 []vcsReview if *reviewerName != "" { - existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) + existingReviews, err := vcs.ListReviews(ctx, owner, repoName, prNumber) if err != nil { slog.Warn("could not list existing reviews", "pr", prNumber, "error", err) } else { @@ -461,11 +494,11 @@ func main() { } // Self-request as reviewer (ensures we appear in required-reviewer checks) - authUser, err := giteaClient.GetAuthenticatedUser(ctx) + authUser, err := vcs.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 { + if err := vcs.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) @@ -474,31 +507,34 @@ func main() { // 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 := vcs.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, evaluatedSHA, inlineComments) if err != nil { slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err) os.Exit(1) } slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber) - // Supersede all old reviews with link to the new one - if len(oldReviews) > 0 { + // Supersede all old reviews with link to the new one. + // This is only supported on Gitea (requires timeline API); GitHub reviews cannot + // be edited after submission, so we skip the supersede step there. + extVCS, isGiteaExt := vcs.(giteaExtClient) + if len(oldReviews) > 0 && isGiteaExt { newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*vcsURL, "/"), owner, repoName, prNumber, posted.ID) for _, oldReview := range oldReviews { - cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID) + cid, err := extVCS.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, int64(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 := extVCS.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 := extVCS.ListReviewComments(ctx, owner, repoName, int64(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 +544,7 @@ func main() { if c.ID == 0 { continue } - if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil { + if err := extVCS.ResolveComment(ctx, owner, repoName, c.ID); err != nil { slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err) failed++ } else { @@ -522,12 +558,14 @@ func main() { slog.Warn("some inline comments could not be resolved", "review_id", oldReview.ID, "failed", failed, "pr", prNumber) } } + } else if len(oldReviews) > 0 { + slog.Info("skipping supersede of old reviews (not supported on this VCS)", "old_count", len(oldReviews), "pr", prNumber) } } // 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 []vcsChangedFile) string { var sb strings.Builder for _, f := range files { if ctx.Err() != nil { @@ -554,7 +592,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, ",") @@ -631,7 +669,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 []vcsCommitStatus) (passed bool, details string) { if len(statuses) == 0 { return true, "no CI statuses found" } @@ -654,6 +692,19 @@ func evaluateCIStatus(statuses []gitea.CommitStatus) (passed bool, details strin return true, "all checks passed" } +// githubAPIURL converts a GitHub server URL to its API base URL. +// github.com → https://api.github.com +// GHES (e.g. https://ghe.example.com) → https://ghe.example.com/api/v3 +func githubAPIURL(serverURL string) string { + const canonicalGitHub = "https://github.com" + const githubAPIBase = "https://api.github.com" + if serverURL == "" || strings.TrimRight(serverURL, "/") == canonicalGitHub { + return githubAPIBase + } + // GitHub Enterprise Server: /api/v3 suffix + return strings.TrimRight(serverURL, "/") + "/api/v3" +} + func envOrDefault(key, defaultVal string) string { if v := os.Getenv(key); v != "" { return v @@ -769,7 +820,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 []vcsReview, ownSentinel string) bool { ownLogin := "" for _, r := range reviews { if strings.Contains(r.Body, ownSentinel) { @@ -807,8 +858,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 []vcsReview, sentinel string) *vcsReview { + var best *vcsReview for i := range reviews { if !strings.Contains(reviews[i].Body, sentinel) { continue @@ -824,8 +875,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 []vcsReview, sentinel string) []vcsReview { + var result []vcsReview for i := range reviews { if !strings.Contains(reviews[i].Body, sentinel) { continue @@ -851,31 +902,4 @@ func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool { return evaluatedSHA != currentSHA } -// giteaClientAdapter adapts gitea.Client to review.GiteaClient interface. -type giteaClientAdapter struct { - client *gitea.Client -} -func newGiteaClientAdapter(c *gitea.Client) *giteaClientAdapter { - return &giteaClientAdapter{client: c} -} - -func (a *giteaClientAdapter) 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 - } - result := make([]review.ContentEntry, len(entries)) - for i, e := range entries { - result[i] = review.ContentEntry{ - Name: e.Name, - Path: e.Path, - Type: e.Type, - } - } - return result, nil -} - -func (a *giteaClientAdapter) 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..ed458b6 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -10,7 +10,6 @@ import ( "strings" "testing" - "gitea.weiker.me/rodin/review-bot/gitea" ) func TestValidateReviewerName(t *testing.T) { @@ -154,12 +153,11 @@ 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, _ bool, body string) vcsReview { + r := vcsReview{ ID: id, Body: body, State: state, - Stale: stale, } r.User.Login = login return r @@ -216,7 +214,7 @@ func TestBuildSupersededBodyShortSHA(t *testing.T) { func TestFindOwnReview(t *testing.T) { tests := []struct { name string - reviews []gitea.Review + reviews []vcsReview sentinel string wantID int64 wantNil bool @@ -229,7 +227,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "found by sentinel", - reviews: []gitea.Review{ + reviews: []vcsReview{ makeReview(42, "bot", "APPROVED", false, "review body\n"), }, sentinel: "", @@ -237,7 +235,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "wrong sentinel", - reviews: []gitea.Review{ + reviews: []vcsReview{ makeReview(42, "bot", "APPROVED", false, "body\n"), }, sentinel: "", @@ -245,7 +243,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "multiple reviews, returns first match", - reviews: []gitea.Review{ + reviews: []vcsReview{ makeReview(10, "bot", "APPROVED", false, "old\n"), makeReview(20, "bot", "APPROVED", false, "new\n"), }, @@ -254,7 +252,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "skips superseded review", - reviews: []gitea.Review{ + reviews: []vcsReview{ makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n"), makeReview(20, "bot", "APPROVED", false, "fresh review\n"), }, @@ -263,7 +261,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "only superseded reviews exist", - reviews: []gitea.Review{ + reviews: []vcsReview{ makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n"), }, sentinel: "", @@ -271,7 +269,7 @@ func TestFindOwnReview(t *testing.T) { }, { name: "picks highest ID among matches", - reviews: []gitea.Review{ + reviews: []vcsReview{ makeReview(50, "bot", "APPROVED", false, "v1\n"), makeReview(30, "bot", "APPROVED", false, "v0\n"), }, @@ -302,7 +300,7 @@ func TestFindOwnReview(t *testing.T) { func TestHasSharedToken(t *testing.T) { tests := []struct { name string - reviews []gitea.Review + reviews []vcsReview sentinel string want bool }{ @@ -314,36 +312,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: []vcsReview{ + {ID: 1, User: struct{ Login string }{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: []vcsReview{ + {ID: 1, User: struct{ Login string }{Login: "sonnet-review-bot"}, Body: " body"}, + {ID: 2, User: struct{ Login string }{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: []vcsReview{ + {ID: 1, User: struct{ Login string }{Login: "sonnet-review-bot"}, Body: " body"}, + {ID: 2, User: struct{ Login string }{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: []vcsReview{ + {ID: 1, User: struct{ Login string }{Login: "bot"}, Body: " body"}, + {ID: 2, User: struct{ Login string }{Login: "bot"}, Body: " body"}, + {ID: 3, User: struct{ Login string }{Login: "bot"}, Body: " body"}, }, sentinel: "", want: true, @@ -553,7 +551,7 @@ func TestBuildPatternPaths(t *testing.T) { func TestEvaluateCIStatus(t *testing.T) { tests := []struct { name string - statuses []gitea.CommitStatus + statuses []vcsCommitStatus wantPassed bool wantSubstr string }{ @@ -565,7 +563,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "all success", - statuses: []gitea.CommitStatus{ + statuses: []vcsCommitStatus{ {Status: "success", Context: "ci/build", Description: "Build passed"}, {Status: "success", Context: "ci/test", Description: "Tests passed"}, }, @@ -574,7 +572,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "one failure", - statuses: []gitea.CommitStatus{ + statuses: []vcsCommitStatus{ {Status: "success", Context: "ci/build", Description: "Build passed"}, {Status: "failure", Context: "ci/test", Description: "Tests failed"}, }, @@ -583,7 +581,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "error status", - statuses: []gitea.CommitStatus{ + statuses: []vcsCommitStatus{ {Status: "error", Context: "ci/lint", Description: "Lint error"}, }, wantPassed: false, @@ -591,7 +589,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "pending treated as not-failed", - statuses: []gitea.CommitStatus{ + statuses: []vcsCommitStatus{ {Status: "pending", Context: "ci/build", Description: "In progress"}, {Status: "success", Context: "ci/test", Description: "Tests passed"}, }, @@ -600,7 +598,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "multiple failures", - statuses: []gitea.CommitStatus{ + statuses: []vcsCommitStatus{ {Status: "failure", Context: "ci/build", Description: "Build failed"}, {Status: "failure", Context: "ci/test", Description: "Tests failed"}, }, @@ -609,7 +607,7 @@ func TestEvaluateCIStatus(t *testing.T) { }, { name: "mixed with pending and failure", - statuses: []gitea.CommitStatus{ + statuses: []vcsCommitStatus{ {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 +995,7 @@ func cleanEnv() []string { } func TestFindAllOwnReviews(t *testing.T) { - reviews := []gitea.Review{ + reviews := []vcsReview{ {ID: 1, Body: "\nfirst review"}, {ID: 2, Body: "\nother bot"}, {ID: 3, Body: "\nsecond review"}, diff --git a/cmd/review-bot/vcs.go b/cmd/review-bot/vcs.go new file mode 100644 index 0000000..194dc50 --- /dev/null +++ b/cmd/review-bot/vcs.go @@ -0,0 +1,361 @@ +package main + +// vcs.go defines the vcsClient interface that both gitea.Client (via giteaVCSAdapter) +// and github.Client (via githubVCSAdapter) satisfy, enabling VCS-type routing in main.go. +// +// Interface design: +// - Methods cover all PR review operations used by main.go. +// - Gitea-specific operations (supersede, comment resolution) are in the separate +// giteaExtClient interface. GitHub implementations return ErrNotSupported for those. +// - Types are defined here as package-local VCS types; each adapter converts from +// its respective client package's types. + +import ( + "context" + "errors" + + "gitea.weiker.me/rodin/review-bot/gitea" + "gitea.weiker.me/rodin/review-bot/github" + "gitea.weiker.me/rodin/review-bot/review" +) + +// ErrNotSupported is returned by VCS methods that have no implementation for +// a particular VCS backend (e.g., Gitea-specific timeline APIs on GitHub). +var ErrNotSupported = errors.New("operation not supported on this VCS backend") + +// vcsClient is the interface for all PR operations used by main.go. +// It is implemented by both giteaVCSAdapter and githubVCSAdapter. +// Interface defined here (in the consumer package) per Go idiom. +type vcsClient interface { + // PR metadata and content + GetPullRequest(ctx context.Context, owner, repo string, number int) (*vcsPullRequest, error) + GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) + GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]vcsChangedFile, error) + GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]vcsCommitStatus, error) + GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) + GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) + ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) + GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) + + // Review operations + PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []vcsReviewComment) (*vcsReview, error) + ListReviews(ctx context.Context, owner, repo string, number int) ([]vcsReview, error) + DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error + GetAuthenticatedUser(ctx context.Context) (string, error) + RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error +} + +// giteaExtClient extends vcsClient with Gitea-specific operations that have no +// GitHub equivalent. Code that uses these methods should first do a type assertion. +type giteaExtClient interface { + vcsClient + GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, prNum, reviewID int64) (int64, error) + EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error + ListReviewComments(ctx context.Context, owner, repo string, prNum, reviewID int64) ([]gitea.ReviewComment, error) + ResolveComment(ctx context.Context, owner, repo string, commentID int64) error +} + +// --- shared VCS types --- + +// vcsPullRequest is VCS-agnostic PR metadata. +type vcsPullRequest struct { + Title string + Body string + Head struct { + Sha string + Ref string + } +} + +// vcsChangedFile is a file changed in a PR. +type vcsChangedFile struct { + Filename string + Status string +} + +// vcsCommitStatus is a CI status entry. +type vcsCommitStatus struct { + Status string + Context string + Description string + TargetURL string +} + +// vcsReviewComment is an inline review comment. +type vcsReviewComment struct { + Path string + NewPosition int64 // Gitea: absolute line; GitHub: diff hunk position + Body string +} + +// vcsReview is a submitted PR review. +type vcsReview struct { + ID int64 + Body string + CommitID string + User struct { + Login string + } + State string +} + +// ============================================================ +// giteaVCSAdapter +// ============================================================ + +// giteaVCSAdapter wraps gitea.Client to implement vcsClient + giteaExtClient. +type giteaVCSAdapter struct { + c *gitea.Client +} + +func newGiteaVCSAdapter(c *gitea.Client) *giteaVCSAdapter { return &giteaVCSAdapter{c: c} } + +func (a *giteaVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*vcsPullRequest, error) { + pr, err := a.c.GetPullRequest(ctx, owner, repo, number) + if err != nil { + return nil, err + } + r := &vcsPullRequest{Title: pr.Title, Body: pr.Body} + r.Head.Sha = pr.Head.Sha + r.Head.Ref = pr.Head.Ref + return r, nil +} + +func (a *giteaVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { + return a.c.GetPullRequestDiff(ctx, owner, repo, number) +} + +func (a *giteaVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]vcsChangedFile, error) { + files, err := a.c.GetPullRequestFiles(ctx, owner, repo, number) + if err != nil { + return nil, err + } + out := make([]vcsChangedFile, len(files)) + for i, f := range files { + out[i] = vcsChangedFile{Filename: f.Filename, Status: f.Status} + } + return out, nil +} + +func (a *giteaVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]vcsCommitStatus, error) { + statuses, err := a.c.GetCommitStatuses(ctx, owner, repo, sha) + if err != nil { + return nil, err + } + out := make([]vcsCommitStatus, len(statuses)) + for i, s := range statuses { + out[i] = vcsCommitStatus{Status: s.Status, Context: s.Context, Description: s.Description, TargetURL: s.TargetURL} + } + return out, nil +} + +func (a *giteaVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { + return a.c.GetFileContent(ctx, owner, repo, filepath) +} + +func (a *giteaVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + return a.c.GetFileContentRef(ctx, owner, repo, filepath, ref) +} + +func (a *giteaVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) { + entries, err := a.c.ListContents(ctx, owner, repo, path) + if err != nil { + return nil, err + } + out := make([]review.ContentEntry, len(entries)) + for i, e := range entries { + out[i] = review.ContentEntry{Name: e.Name, Path: e.Path, Type: e.Type} + } + return out, nil +} + +func (a *giteaVCSAdapter) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) { + return a.c.GetAllFilesInPath(ctx, owner, repo, path) +} + +func (a *giteaVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []vcsReviewComment) (*vcsReview, error) { + gc := make([]gitea.ReviewComment, len(comments)) + for i, c := range comments { + gc[i] = gitea.ReviewComment{Path: c.Path, NewPosition: c.NewPosition, Body: c.Body} + } + r, err := a.c.PostReview(ctx, owner, repo, number, event, body, commitID, gc) + if err != nil { + return nil, err + } + out := &vcsReview{ID: r.ID, Body: r.Body, CommitID: r.CommitID, State: r.State} + out.User.Login = r.User.Login + return out, nil +} + +func (a *giteaVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcsReview, error) { + reviews, err := a.c.ListReviews(ctx, owner, repo, number) + if err != nil { + return nil, err + } + out := make([]vcsReview, len(reviews)) + for i, r := range reviews { + out[i] = vcsReview{ID: r.ID, Body: r.Body, CommitID: r.CommitID, State: r.State} + out[i].User.Login = r.User.Login + } + return out, nil +} + +func (a *giteaVCSAdapter) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error { + return a.c.DeleteReview(ctx, owner, repo, number, reviewID) +} + +func (a *giteaVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) { + return a.c.GetAuthenticatedUser(ctx) +} + +func (a *giteaVCSAdapter) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error { + return a.c.RequestReviewer(ctx, owner, repo, number, reviewer) +} + +// Gitea-specific extension methods. + +func (a *giteaVCSAdapter) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, prNum, reviewID int64) (int64, error) { + return a.c.GetTimelineReviewCommentIDForReview(ctx, owner, repo, int(prNum), reviewID) +} + +func (a *giteaVCSAdapter) EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error { + return a.c.EditComment(ctx, owner, repo, commentID, body) +} + +func (a *giteaVCSAdapter) ListReviewComments(ctx context.Context, owner, repo string, prNum, reviewID int64) ([]gitea.ReviewComment, error) { + return a.c.ListReviewComments(ctx, owner, repo, int(prNum), reviewID) +} + +func (a *giteaVCSAdapter) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error { + return a.c.ResolveComment(ctx, owner, repo, commentID) +} + +// ============================================================ +// githubVCSAdapter +// ============================================================ + +// githubVCSAdapter wraps github.Client to implement vcsClient. +// Gitea-specific extension methods (GetTimelineReviewCommentIDForReview, EditComment, +// ListReviewComments, ResolveComment) are not available on GitHub and will not be called +// because main.go gates them with a type assertion to giteaExtClient. +type githubVCSAdapter struct { + c *github.Client +} + +func newGithubVCSAdapter(c *github.Client) *githubVCSAdapter { return &githubVCSAdapter{c: c} } + +func (a *githubVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*vcsPullRequest, error) { + pr, err := a.c.GetPullRequest(ctx, owner, repo, number) + if err != nil { + return nil, err + } + r := &vcsPullRequest{Title: pr.Title, Body: pr.Body} + r.Head.Sha = pr.Head.Sha + r.Head.Ref = pr.Head.Ref + return r, nil +} + +func (a *githubVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { + return a.c.GetPullRequestDiff(ctx, owner, repo, number) +} + +func (a *githubVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]vcsChangedFile, error) { + files, err := a.c.GetPullRequestFiles(ctx, owner, repo, number) + if err != nil { + return nil, err + } + out := make([]vcsChangedFile, len(files)) + for i, f := range files { + out[i] = vcsChangedFile{Filename: f.Filename, Status: f.Status} + } + return out, nil +} + +func (a *githubVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]vcsCommitStatus, error) { + statuses, err := a.c.GetCommitStatuses(ctx, owner, repo, sha) + if err != nil { + return nil, err + } + out := make([]vcsCommitStatus, len(statuses)) + for i, s := range statuses { + // CommitStatus.Status is tagged as json:"state" — already the normalized "state" value + out[i] = vcsCommitStatus{Status: s.Status, Context: s.Context, Description: s.Description, TargetURL: s.TargetURL} + } + return out, nil +} + +func (a *githubVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { + return a.c.GetFileContent(ctx, owner, repo, filepath) +} + +func (a *githubVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + return a.c.GetFileContentRef(ctx, owner, repo, filepath, ref) +} + +func (a *githubVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) { + entries, err := a.c.ListContents(ctx, owner, repo, path) + if err != nil { + return nil, err + } + out := make([]review.ContentEntry, len(entries)) + for i, e := range entries { + out[i] = review.ContentEntry{Name: e.Name, Path: e.Path, Type: e.Type} + } + return out, nil +} + +func (a *githubVCSAdapter) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) { + return a.c.GetAllFilesInPath(ctx, owner, repo, path) +} + +func (a *githubVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []vcsReviewComment) (*vcsReview, error) { + gc := make([]github.ReviewComment, len(comments)) + for i, c := range comments { + // GitHub inline comments use diff hunk "position", not absolute line numbers. + // NewPosition from gitea diff parsing gives absolute line numbers, which + // will not match GitHub's position values. For initial GitHub support, we + // attach comments with Line+Side (absolute line on the RIGHT side) instead. + // Comments that cannot be mapped will be omitted (GitHub rejects invalid positions). + gc[i] = github.ReviewComment{ + Path: c.Path, + Line: c.NewPosition, + Side: "RIGHT", + Body: c.Body, + } + } + r, err := a.c.PostReview(ctx, owner, repo, number, event, body, commitID, gc) + if err != nil { + return nil, err + } + out := &vcsReview{ID: r.ID, Body: r.Body, State: r.State} + out.User.Login = r.User.Login + return out, nil +} + +func (a *githubVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcsReview, error) { + reviews, err := a.c.ListReviews(ctx, owner, repo, number) + if err != nil { + return nil, err + } + out := make([]vcsReview, len(reviews)) + for i, r := range reviews { + out[i] = vcsReview{ID: r.ID, Body: r.Body, State: r.State} + out[i].User.Login = r.User.Login + } + return out, nil +} + +func (a *githubVCSAdapter) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error { + // GitHub only allows deleting PENDING (draft) reviews. review-bot posts submitted + // reviews, so this will return an error for any review we actually posted. + // Callers should treat 422 errors here gracefully. + return a.c.DeleteReview(ctx, owner, repo, number, reviewID) +} + +func (a *githubVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) { + return a.c.GetAuthenticatedUser(ctx) +} + +func (a *githubVCSAdapter) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error { + return a.c.RequestReviewer(ctx, owner, repo, number, reviewer) +}