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) +} diff --git a/github/client.go b/github/client.go index 227df06..d77aa8c 100644 --- a/github/client.go +++ b/github/client.go @@ -4,7 +4,10 @@ package github import ( + "bytes" "context" + "encoding/base64" + "encoding/json" "errors" "fmt" "io" @@ -92,10 +95,6 @@ func asAPIError(err error) (*APIError, bool) { // SetHTTPClient and SetRetryBackoff are intended for test setup only and must // be called before any goroutines issue requests; they have no synchronization. type Client struct { - // TODO: baseURL is populated by NewClient but not yet consumed by doRequest/doGet. - // Higher-level exported methods (GetPullRequest, etc.) will use it to - // construct request URLs; remove this field if those methods end up - // accepting full URLs instead. baseURL string token string httpClient *http.Client @@ -376,3 +375,457 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) { return c.doRequest(ctx, http.MethodGet, url, "") } + +// doRequestWithBody performs an HTTP request with an optional body, applying the +// same HTTPS enforcement as doRequest. It is used by write methods (POST, PUT, +// DELETE) that bypass the retry loop in doRequest because write operations are +// not idempotent. +// +// body may be nil for requests that carry no payload (e.g. DELETE). +// When body is non-nil, Content-Type is set to application/json. +func (c *Client) doRequestWithBody(ctx context.Context, method, reqURL string, body []byte) ([]byte, error) { + if !c.allowInsecureHTTP { + parsed, err := url.Parse(reqURL) + if err != nil { + return nil, fmt.Errorf("parse request URL: %w", err) + } + if strings.EqualFold(parsed.Scheme, "http") { + return nil, fmt.Errorf("refusing HTTP request to %s: use HTTPS or set AllowInsecureHTTP option", redactURL(reqURL)) + } + } + + var reqBody io.Reader + if body != nil { + reqBody = bytes.NewReader(body) + } + + req, err := http.NewRequestWithContext(ctx, method, reqURL, reqBody) + if err != nil { + return nil, fmt.Errorf("create request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github+json") + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("do request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + respBody, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodyBytes)) + if err != nil { + return nil, fmt.Errorf("read response body: %w", err) + } + return respBody, nil + } + + errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes)) + return nil, &APIError{StatusCode: resp.StatusCode, Body: string(errBody)} +} + +// --- API types --- + +// PullRequest holds relevant PR metadata. +type PullRequest struct { + Title string `json:"title"` + Body string `json:"body"` + Head struct { + Sha string `json:"sha"` + Ref string `json:"ref"` + } `json:"head"` + Draft bool `json:"draft"` +} + +// CommitStatus represents a single CI status entry. +// GitHub returns "state" not "status"; this type uses Status for consistency +// with the gitea package (both are normalized before use). +type CommitStatus struct { + Status string `json:"state"` // GitHub field is "state" + Context string `json:"context"` + Description string `json:"description"` + TargetURL string `json:"target_url"` +} + +// ChangedFile represents a file modified in a PR. +type ChangedFile struct { + Filename string `json:"filename"` + Status string `json:"status"` +} + +// ReviewComment represents an inline comment to attach to a review. +// GitHub uses "position" (diff hunk position), whereas Gitea uses "new_position" (line number). +// When posting inline comments on GitHub, position is required; line numbers +// from the diff cannot be used directly. +type ReviewComment struct { + ID int64 `json:"id,omitempty"` + Path string `json:"path"` + Position int64 `json:"position,omitempty"` // GitHub diff hunk position + Line int64 `json:"line,omitempty"` // GitHub absolute line number (alternative to position) + Side string `json:"side,omitempty"` // "RIGHT" or "LEFT" + Body string `json:"body"` +} + +// Review represents a pull request review from the GitHub API. +type Review struct { + ID int64 `json:"id"` + Body string `json:"body"` + User struct { + Login string `json:"login"` + } `json:"user"` + State string `json:"state"` +} + +// contentResponse is the GitHub contents API response for a single file. +type contentResponse struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` // "file" or "dir" or "symlink" or "submodule" + Content string `json:"content"` // Base64-encoded file content (with embedded newlines) + Encoding string `json:"encoding"` // "base64" or "" +} + +// ContentEntry represents a file or directory entry from the contents API. +type ContentEntry struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` // "file" or "dir" +} + +// --- PR methods --- + +// GetPullRequest fetches PR metadata. +func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("fetch PR: %w", err) + } + var pr PullRequest + if err := json.Unmarshal(body, &pr); err != nil { + return nil, fmt.Errorf("parse PR JSON: %w", err) + } + return &pr, nil +} + +// GetPullRequestDiff fetches the unified diff for a PR. +func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + body, err := c.doRequest(ctx, http.MethodGet, reqURL, "application/vnd.github.diff") + if err != nil { + return "", fmt.Errorf("fetch diff: %w", err) + } + return string(body), nil +} + +// GetPullRequestFiles fetches the list of files changed in a PR. +// GitHub paginates this endpoint (100 per page max). +func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) { + const perPage = 100 + var all []ChangedFile + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/files?per_page=%d&page=%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, perPage, page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("fetch PR files (page %d): %w", page, err) + } + var batch []ChangedFile + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse PR files JSON (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < perPage { + break + } + } + return all, nil +} + +// GetCommitStatuses fetches CI statuses for a commit SHA. +// GitHub has two status systems: legacy "commit statuses" and newer "check runs". +// This method returns commit statuses only; check runs are a separate API. +// Note: GitHub returns "state" in the JSON; CommitStatus.Status is tagged accordingly. +func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) { + const perPage = 100 + var all []CommitStatus + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/repos/%s/%s/commits/%s/statuses?per_page=%d&page=%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha), perPage, page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("fetch commit statuses (page %d): %w", page, err) + } + var batch []CommitStatus + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse statuses JSON (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < perPage { + break + } + } + return all, nil +} + +// --- File content methods --- + +// GetFileContent fetches a file from the default branch of a repo. +// GitHub returns base64-encoded content; this method decodes it. +func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { + return c.getFileContentAtRef(ctx, owner, repo, filepath, "") +} + +// GetFileContentRef fetches a file from a specific ref (branch/tag/sha). +func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + return c.getFileContentAtRef(ctx, owner, repo, filepath, ref) +} + +// getFileContentAtRef fetches a file at the given ref (empty = default branch). +// GitHub's contents API returns base64-encoded file content. +func (c *Client) getFileContentAtRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/contents/%s", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(filepath)) + if ref != "" { + reqURL += "?ref=" + url.QueryEscape(ref) + } + body, err := c.doGet(ctx, reqURL) + if err != nil { + return "", fmt.Errorf("fetch file %s: %w", filepath, err) + } + var resp contentResponse + if err := json.Unmarshal(body, &resp); err != nil { + return "", fmt.Errorf("parse file content JSON for %s: %w", filepath, err) + } + if resp.Type != "file" { + return "", fmt.Errorf("path %s is a %s, not a file", filepath, resp.Type) + } + if resp.Encoding == "base64" { + // GitHub embeds newlines in the base64 content for readability. + // Strip them before decoding. + cleaned := strings.ReplaceAll(resp.Content, "\n", "") + decoded, err := base64.StdEncoding.DecodeString(cleaned) + if err != nil { + return "", fmt.Errorf("decode base64 content for %s: %w", filepath, err) + } + return string(decoded), nil + } + // Non-base64 encoding (shouldn't happen normally, but handle gracefully). + return resp.Content, nil +} + +// ListContents lists files and directories at a given path. +// Pass an empty path to list the repository root. +// GitHub returns a single object (not array) when path is a file — this +// method normalizes both cases to a slice, matching Gitea's behavior. +func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) { + var reqURL string + if path == "" || path == "." { + reqURL = fmt.Sprintf("%s/repos/%s/%s/contents", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo)) + } else { + reqURL = fmt.Sprintf("%s/repos/%s/%s/contents/%s", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(path)) + } + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list contents %s: %w", path, err) + } + + var entries []ContentEntry + if err := json.Unmarshal(body, &entries); err != nil { + // GitHub returns a single object when path is a file (not an array). + var single contentResponse + if err2 := json.Unmarshal(body, &single); err2 != nil { + return nil, fmt.Errorf("parse contents JSON: %w", err) + } + if single.Name == "" && single.Path == "" { + return nil, fmt.Errorf("parse contents JSON: empty response for path %q", path) + } + entries = []ContentEntry{{ + Name: single.Name, + Path: single.Path, + Type: single.Type, + }} + } + return entries, nil +} + +// GetAllFilesInPath recursively fetches all file contents under a path. +// If the path is a file, returns just that file's content. +// If the path is a directory, recursively fetches all files within it. +func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) { + results := make(map[string]string) + + entries, err := c.ListContents(ctx, owner, repo, path) + if err != nil { + if !IsNotFound(err) { + return nil, fmt.Errorf("list contents %q: %w", path, err) + } + // 404 means path may be a file — try fetching directly. + content, fileErr := c.GetFileContent(ctx, owner, repo, path) + if fileErr != nil { + return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, fileErr) + } + results[path] = content + return results, nil + } + + for _, entry := range entries { + switch entry.Type { + case "file": + content, err := c.GetFileContent(ctx, owner, repo, entry.Path) + if err != nil { + slog.Warn("could not fetch file from patterns repo", "file", entry.Path, "error", err) + continue + } + results[entry.Path] = content + case "dir": + subResults, err := c.GetAllFilesInPath(ctx, owner, repo, entry.Path) + if err != nil { + slog.Warn("could not recurse into directory", "dir", entry.Path, "error", err) + continue + } + for k, v := range subResults { + results[k] = v + } + } + } + return results, nil +} + +// --- Review methods --- + +// PostReview submits a review to a PR. +// event should be one of "APPROVE", "REQUEST_CHANGES", or "COMMENT". +// commitID anchors the review to a specific commit SHA. If empty, defaults to current HEAD. +// comments are optional inline comments; GitHub uses diff hunk position (not line numbers). +// Note: unlike Gitea, GitHub does not support deleting submitted reviews. +// Use COMMENT event to supersede old reviews. +func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []ReviewComment) (*Review, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + + payload := struct { + Body string `json:"body"` + Event string `json:"event"` + CommitID string `json:"commit_id,omitempty"` + Comments []ReviewComment `json:"comments,omitempty"` + }{ + Body: body, + Event: event, + CommitID: commitID, + Comments: comments, + } + + data, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("marshal review payload: %w", err) + } + + respBody, err := c.doRequestWithBody(ctx, http.MethodPost, reqURL, data) + if err != nil { + return nil, fmt.Errorf("post review: %w", err) + } + + var review Review + if err := json.Unmarshal(respBody, &review); err != nil { + return nil, fmt.Errorf("parse review response: %w", err) + } + return &review, nil +} + +// ListReviews returns all reviews on a pull request. +// GitHub paginates via Link header; this method uses per_page=100. +func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error) { + const perPage = 100 + var all []Review + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews?per_page=%d&page=%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, perPage, page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list reviews (page %d): %w", page, err) + } + var batch []Review + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse reviews (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < perPage { + break + } + } + return all, nil +} + +// DeleteReview attempts to delete a pull request review. +// GitHub only allows deleting PENDING (draft) reviews. Submitted reviews cannot +// be deleted via the API; this method returns a descriptive error in that case. +// review-bot callers should handle this error gracefully (e.g., by not attempting +// supersede and instead posting a new review alongside the old one). +func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID) + + // nil body: the GitHub DELETE endpoint for reviews requires no request body. + _, err := c.doRequestWithBody(ctx, http.MethodDelete, reqURL, nil) + if err != nil { + return fmt.Errorf("delete review: %w", err) + } + return nil +} + +// GetAuthenticatedUser returns the login of the authenticated user. +func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) { + reqURL := c.baseURL + "/user" + 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 a 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/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) + } + + _, err = c.doRequestWithBody(ctx, http.MethodPost, reqURL, data) + if err != nil { + return fmt.Errorf("request reviewer: %w", err) + } + return nil +} + +// --- helpers --- + +// escapePath escapes each segment of a relative file path for use in URLs. +// Slashes are preserved as path separators; other special characters are escaped. +func escapePath(p string) string { + parts := strings.Split(p, "/") + for i, part := range parts { + parts[i] = url.PathEscape(part) + } + return strings.Join(parts, "/") +} diff --git a/github/client_test.go b/github/client_test.go index bb2f9f9..8aa22fb 100644 --- a/github/client_test.go +++ b/github/client_test.go @@ -2,7 +2,9 @@ package github import ( "context" + "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "net/url" @@ -656,3 +658,475 @@ func TestRedactURL_UserinfoWithQuery(t *testing.T) { t.Errorf("redactURL = %q, want %q", got, want) } } + +// --- Tests for API methods --- + +func TestGetPullRequest_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/repos/owner/repo/pulls/42" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"title":"Test PR","body":"description","head":{"sha":"abc123","ref":"feature"},"draft":false}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + pr, err := c.GetPullRequest(context.Background(), "owner", "repo", 42) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pr.Title != "Test PR" { + t.Errorf("Title = %q, want %q", pr.Title, "Test PR") + } + if pr.Head.Sha != "abc123" { + t.Errorf("Head.Sha = %q, want %q", pr.Head.Sha, "abc123") + } + if pr.Head.Ref != "feature" { + t.Errorf("Head.Ref = %q, want %q", pr.Head.Ref, "feature") + } +} + +func TestGetPullRequest_NotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"message":"Not Found"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + _, err := c.GetPullRequest(context.Background(), "owner", "repo", 99) + if err == nil { + t.Fatal("expected error, got nil") + } + if !IsNotFound(err) { + t.Errorf("expected IsNotFound=true, got false for error: %v", err) + } +} + +func TestGetPullRequestDiff_Success(t *testing.T) { + const wantDiff = "diff --git a/foo.go b/foo.go\n--- a/foo.go\n+++ b/foo.go\n" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Accept") != "application/vnd.github.diff" { + t.Errorf("Accept = %q, want application/vnd.github.diff", r.Header.Get("Accept")) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(wantDiff)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + diff, err := c.GetPullRequestDiff(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if diff != wantDiff { + t.Errorf("diff = %q, want %q", diff, wantDiff) + } +} + +func TestGetPullRequestFiles_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"filename":"foo.go","status":"modified"},{"filename":"bar.go","status":"added"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + files, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 2 { + t.Fatalf("len(files) = %d, want 2", len(files)) + } + if files[0].Filename != "foo.go" || files[0].Status != "modified" { + t.Errorf("files[0] = %+v, want {foo.go modified}", files[0]) + } +} + +func TestGetPullRequestFiles_Paginated(t *testing.T) { + page := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page++ + if page == 1 { + // Return 100 items (page full → expect another request) + items := make([]map[string]string, 100) + for i := range items { + items[i] = map[string]string{"filename": fmt.Sprintf("file%d.go", i), "status": "modified"} + } + data, _ := json.Marshal(items) + w.WriteHeader(http.StatusOK) + w.Write(data) + return + } + // Page 2: return fewer than perPage → stop + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"filename":"last.go","status":"added"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + files, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 101 { + t.Errorf("len(files) = %d, want 101", len(files)) + } + if page != 2 { + t.Errorf("page = %d, want 2", page) + } +} + +func TestGetCommitStatuses_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + // GitHub uses "state" field + w.Write([]byte(`[{"state":"success","context":"ci/test","description":"Tests pass","target_url":"https://ci.example.com"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + statuses, err := c.GetCommitStatuses(context.Background(), "owner", "repo", "deadbeef") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(statuses) != 1 { + t.Fatalf("len(statuses) = %d, want 1", len(statuses)) + } + if statuses[0].Status != "success" { + t.Errorf("Status = %q, want %q", statuses[0].Status, "success") + } + if statuses[0].Context != "ci/test" { + t.Errorf("Context = %q, want %q", statuses[0].Context, "ci/test") + } +} + +func TestGetFileContent_Base64(t *testing.T) { + // "hello world\n" base64-encoded with embedded newlines (as GitHub does it) + encoded := "aGVsbG8gd29ybGQK" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasSuffix(r.URL.Path, "/contents/README.md") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"README.md","path":"README.md","type":"file","content":"` + encoded + `","encoding":"base64"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + content, err := c.GetFileContent(context.Background(), "owner", "repo", "README.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if content != "hello world\n" { + t.Errorf("content = %q, want %q", content, "hello world\n") + } +} + +func TestGetFileContent_Base64WithNewlines(t *testing.T) { + // GitHub embeds newlines in base64 content for readability (every 60 chars) + // Test that we strip them correctly before decoding + // "hello world\n" = aGVsbG8gd29ybGQK — split it with embedded \n + encoded := "aGVs\nbG8g\nd29y\nbGQK" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + // JSON-encode the embedded newlines as \n + body := `{"name":"README.md","path":"README.md","type":"file","content":"aGVs\nbG8g\nd29y\nbGQK","encoding":"base64"}` + _ = encoded // suppress unused warning + w.Write([]byte(body)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + content, err := c.GetFileContent(context.Background(), "owner", "repo", "README.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if content != "hello world\n" { + t.Errorf("content = %q, want %q", content, "hello world\n") + } +} + +func TestGetFileContent_IsDirectory(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"docs","path":"docs","type":"dir","content":"","encoding":""}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + _, err := c.GetFileContent(context.Background(), "owner", "repo", "docs") + if err == nil { + t.Fatal("expected error for directory, got nil") + } +} + +func TestGetFileContentRef_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("ref") != "main" { + t.Errorf("ref = %q, want %q", r.URL.Query().Get("ref"), "main") + } + encoded := "dGVzdA==" // "test" + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"foo.go","path":"foo.go","type":"file","content":"` + encoded + `","encoding":"base64"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + content, err := c.GetFileContentRef(context.Background(), "owner", "repo", "foo.go", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if content != "test" { + t.Errorf("content = %q, want %q", content, "test") + } +} + +func TestListContents_Directory(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"name":"foo.go","path":"foo.go","type":"file"},{"name":"bar","path":"bar","type":"dir"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + entries, err := c.ListContents(context.Background(), "owner", "repo", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 2 { + t.Fatalf("len(entries) = %d, want 2", len(entries)) + } + if entries[0].Name != "foo.go" || entries[0].Type != "file" { + t.Errorf("entries[0] = %+v, unexpected", entries[0]) + } +} + +func TestListContents_SingleFile(t *testing.T) { + // GitHub returns a single object when the path is a file + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"README.md","path":"README.md","type":"file","content":"","encoding":""}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + entries, err := c.ListContents(context.Background(), "owner", "repo", "README.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 1 { + t.Fatalf("len(entries) = %d, want 1", len(entries)) + } + if entries[0].Name != "README.md" { + t.Errorf("entries[0].Name = %q, want README.md", entries[0].Name) + } +} + +func TestPostReview_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("method = %s, want POST", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/1/reviews" { + t.Errorf("path = %s, unexpected", r.URL.Path) + } + var payload map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Errorf("decode body: %v", err) + } + if payload["event"] != "APPROVE" { + t.Errorf("event = %v, want APPROVE", payload["event"]) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"id":99,"body":"looks good","user":{"login":"bot"},"state":"APPROVED"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + review, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "looks good", "abc", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if review.ID != 99 { + t.Errorf("review.ID = %d, want 99", review.ID) + } + if review.User.Login != "bot" { + t.Errorf("review.User.Login = %q, want bot", review.User.Login) + } +} + +func TestPostReview_Unauthorized(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte(`{"message":"Bad credentials"}`)) + })) + defer srv.Close() + + c := NewClient("bad-tok", srv.URL, AllowInsecureHTTPForTest()) + _, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "body", "", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !IsUnauthorized(err) { + t.Errorf("expected IsUnauthorized=true, got false for error: %v", err) + } +} + +func TestListReviews_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"id":1,"body":"review 1","user":{"login":"alice"},"state":"APPROVED"},{"id":2,"body":"review 2","user":{"login":"bob"},"state":"CHANGES_REQUESTED"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + reviews, err := c.ListReviews(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(reviews) != 2 { + t.Fatalf("len(reviews) = %d, want 2", len(reviews)) + } + if reviews[0].ID != 1 || reviews[0].User.Login != "alice" { + t.Errorf("reviews[0] = %+v, unexpected", reviews[0]) + } +} + +func TestDeleteReview_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + t.Errorf("method = %s, want DELETE", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/1/reviews/42" { + t.Errorf("path = %s, unexpected", r.URL.Path) + } + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + err := c.DeleteReview(context.Background(), "owner", "repo", 1, 42) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestDeleteReview_SubmittedReview(t *testing.T) { + // GitHub returns 422 for trying to delete a non-pending review + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + w.Write([]byte(`{"message":"Can only delete a pending review"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + err := c.DeleteReview(context.Background(), "owner", "repo", 1, 99) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestGetAuthenticatedUser_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/user" { + t.Errorf("path = %s, want /user", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"login":"review-bot","id":12345}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + login, err := c.GetAuthenticatedUser(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if login != "review-bot" { + t.Errorf("login = %q, want review-bot", login) + } +} + +func TestRequestReviewer_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("method = %s, want POST", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/1/requested_reviewers" { + t.Errorf("path = %s, unexpected", r.URL.Path) + } + var payload map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Errorf("decode body: %v", err) + } + reviewers, ok := payload["reviewers"].([]interface{}) + if !ok || len(reviewers) != 1 || reviewers[0] != "reviewer1" { + t.Errorf("reviewers = %v, unexpected", payload["reviewers"]) + } + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + err := c.RequestReviewer(context.Background(), "owner", "repo", 1, "reviewer1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestPostReview_RejectsHTTP(t *testing.T) { + // PostReview must reject http:// base URLs — tokens must not be sent in plaintext. + c := NewClient("tok", "http://127.0.0.1:1") + _, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "body", "", nil) + if err == nil { + t.Fatal("expected error for HTTP base URL in PostReview") + } + if !strings.Contains(err.Error(), "refusing HTTP request") { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestDeleteReview_RejectsHTTP(t *testing.T) { + // DeleteReview must reject http:// base URLs — tokens must not be sent in plaintext. + c := NewClient("tok", "http://127.0.0.1:1") + err := c.DeleteReview(context.Background(), "owner", "repo", 1, 42) + if err == nil { + t.Fatal("expected error for HTTP base URL in DeleteReview") + } + if !strings.Contains(err.Error(), "refusing HTTP request") { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestRequestReviewer_RejectsHTTP(t *testing.T) { + // RequestReviewer must reject http:// base URLs — tokens must not be sent in plaintext. + c := NewClient("tok", "http://127.0.0.1:1") + err := c.RequestReviewer(context.Background(), "owner", "repo", 1, "reviewer1") + if err == nil { + t.Fatal("expected error for HTTP base URL in RequestReviewer") + } + if !strings.Contains(err.Error(), "refusing HTTP request") { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestEscapePath_SpecialChars(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"README.md", "README.md"}, + {"docs/guide.md", "docs/guide.md"}, + {"path with spaces/file.md", "path%20with%20spaces/file.md"}, + {"path/with [brackets]/file.md", "path/with%20%5Bbrackets%5D/file.md"}, + } + for _, tt := range tests { + got := escapePath(tt.input) + if got != tt.want { + t.Errorf("escapePath(%q) = %q, want %q", tt.input, got, tt.want) + } + } +}