fix(#130): cleanup and test improvements

- Fix extra blank lines between GetPullRequest and GetPullRequestFiles
- Add TestPostReview_CommitIDFromRequest to verify CommitID passes through

cmd/review-bot/main.go

@@ -17,6 +17,7 @@ import (
 	"gitea.weiker.me/rodin/review-bot/github"
 	"gitea.weiker.me/rodin/review-bot/llm"
 	"gitea.weiker.me/rodin/review-bot/review"
+	"gitea.weiker.me/rodin/review-bot/vcs"
 )
 
 var version = "dev"
@@ -85,6 +86,7 @@ func main() {
 	systemPromptFile := flag.String("system-prompt-file", envOrDefault("SYSTEM_PROMPT_FILE", ""), "Local file with additional system prompt instructions")
 	patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
 	patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", ""), "Comma-separated file paths to fetch from patterns repo (empty = all files)")
+	vcsType := flag.String("vcs-type", envOrDefault("VCS_TYPE", "gitea"), "VCS type: gitea or github")
 	dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
 	llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
 	llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
@@ -169,40 +171,19 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Initialize clients
-	// 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 {
+	// Initialize VCS client
+	var vcsClientImpl vcsClient
+	switch strings.ToLower(*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)
+		vcsClientImpl = newGitHubVCSAdapter(github.NewClient(*reviewerToken, *vcsURL))
+		slog.Info("using GitHub VCS client", "url", *vcsURL)
+	case "gitea", "":
+		vcsClientImpl = newGiteaVCSAdapter(gitea.NewClient(*vcsURL, *reviewerToken))
 		slog.Info("using Gitea VCS client", "url", *vcsURL)
 	default:
-		slog.Error("unsupported VCS type", "vcs_type", vcsType, "valid", "gitea, github")
+		slog.Error("invalid vcs-type", "type", *vcsType, "valid", "gitea, github")
 		os.Exit(1)
 	}
-
 	llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel)
 	if *llmTemp < 0 || *llmTemp > 2 {
 		slog.Error("invalid LLM temperature", "temperature", *llmTemp, "range", "0-2")
@@ -240,7 +221,7 @@ func main() {
 	var persona *review.Persona
 	if *personaName != "" {
 		// Try loading from repo first, then fall back to built-in
-		repoPersonas, err := review.LoadRepoPersonas(ctx, vcs, owner, repoName)
+		repoPersonas, err := review.LoadRepoPersonas(ctx, newReviewClientAdapter(vcsClientImpl), owner, repoName)
 		if err != nil {
 			slog.Warn("could not load repo personas", "repo", owner+"/"+repoName, "error", err)
 			// Continue with built-in personas only.
@@ -276,7 +257,7 @@ func main() {
 	slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName))
 
 	// Step 1: Fetch PR metadata
-	pr, err := vcs.GetPullRequest(ctx, owner, repoName, prNumber)
+	pr, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Error("failed to fetch PR", "pr", prNumber, "error", err)
 		os.Exit(1)
@@ -284,7 +265,7 @@ func main() {
 	slog.Info("fetched PR metadata", "pr", prNumber, "title", pr.Title)
 
 	// Step 2: Fetch diff
-	diff, err := vcs.GetPullRequestDiff(ctx, owner, repoName, prNumber)
+	diff, err := vcsClientImpl.GetPullRequestDiff(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Error("failed to fetch diff", "pr", prNumber, "error", err)
 		os.Exit(1)
@@ -293,21 +274,21 @@ func main() {
 
 	// Step 3: Fetch full file content for modified files
 	fileContext := ""
-	files, err := vcs.GetPullRequestFiles(ctx, owner, repoName, prNumber)
+	files, err := vcsClientImpl.GetPullRequestFiles(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Warn("could not fetch PR files list", "pr", prNumber, "error", err)
 	} else {
-		fileContext = fetchFileContext(ctx, vcs, owner, repoName, pr.Head.Ref, files)
+		fileContext = fetchFileContext(ctx, vcsClientImpl, owner, repoName, pr.HeadRef, files)
 		slog.Debug("fetched file context", "files", len(files))
 	}
 
 	// Step 4: Check CI status
 	ciPassed := true
 	ciDetails := ""
-	if pr.Head.Sha != "" {
-		statuses, err := vcs.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha)
+	if pr.HeadSha != "" {
+		statuses, err := vcsClientImpl.GetCommitStatuses(ctx, owner, repoName, pr.HeadSha)
 		if err != nil {
-			slog.Warn("could not fetch CI status", "sha", pr.Head.Sha, "error", err)
+			slog.Warn("could not fetch CI status", "sha", pr.HeadSha, "error", err)
 		} else {
 			ciPassed, ciDetails = evaluateCIStatus(statuses)
 			slog.Info("CI status checked", "passed", ciPassed)
@@ -317,7 +298,7 @@ func main() {
 	// Step 5: Load conventions file if specified
 	conventions := ""
 	if *conventionsFile != "" {
-		content, err := vcs.GetFileContent(ctx, owner, repoName, *conventionsFile)
+		content, err := vcsClientImpl.GetFileContent(ctx, owner, repoName, *conventionsFile)
 		if err != nil {
 			slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err)
 		} else {
@@ -329,7 +310,7 @@ func main() {
 	// Step 6: Load patterns from external repo if specified
 	patterns := ""
 	if *patternsRepo != "" {
-		patterns = fetchPatterns(ctx, vcs, *patternsRepo, *patternsFiles)
+		patterns = fetchPatterns(ctx, vcsClientImpl, *patternsRepo, *patternsFiles)
 		slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns))
 	}
 
@@ -422,15 +403,15 @@ func main() {
 	}
 
 	// Add commit footer so readers know which commit was evaluated
-	if pr.Head.Sha != "" {
-		shortSHA := pr.Head.Sha
+	if pr.HeadSha != "" {
+		shortSHA := pr.HeadSha
 		if len(shortSHA) > 8 {
 			shortSHA = shortSHA[:8]
 		}
 		reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
 	}
 
-	event := review.GiteaEvent(result.Verdict)
+	event := verdictToVCSEvent(result.Verdict)
 
 	if *dryRun {
 		fmt.Println("--- DRY RUN ---")
@@ -442,14 +423,14 @@ func main() {
 	sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
 
 	// Stale check: verify HEAD hasn't moved since we started
-	evaluatedSHA := pr.Head.Sha
+	evaluatedSHA := pr.HeadSha
 	var currentSHA string
-	currentPR, err := vcs.GetPullRequest(ctx, owner, repoName, prNumber)
+	currentPR, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err)
 		// currentSHA stays empty — shouldSkipStaleReview will return false
 	} else {
-		currentSHA = currentPR.Head.Sha
+		currentSHA = currentPR.HeadSha
 	}
 	if shouldSkipStaleReview(evaluatedSHA, currentSHA) {
 		slog.Warn("HEAD moved during review — skipping stale review",
@@ -460,28 +441,30 @@ func main() {
 	}
 
 	// Map findings to inline comments for lines present in the diff
-	diffRanges := gitea.ParseDiffNewLines(diff)
-	var inlineComments []vcsReviewComment
-	for _, f := range result.Findings {
-		if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
-			inlineComments = append(inlineComments, vcsReviewComment{
-				Path:        f.File,
-				NewPosition: int64(f.Line),
-				Body:        fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
-			})
+	var inlineComments []vcs.ReviewComment
+	if ext, ok := vcsClientImpl.(giteaExtendedClient); ok {
+		diffRanges := ext.ParseDiffNewLines(diff)
+		for _, f := range result.Findings {
+			if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
+				inlineComments = append(inlineComments, vcs.ReviewComment{
+					Path:     f.File,
+					Position: f.Line,
+					Body:     fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
+				})
+			}
+		}
+		if len(inlineComments) > 0 {
+			slog.Debug("attaching inline comments", "count", len(inlineComments))
 		}
-	}
-	if len(inlineComments) > 0 {
-		slog.Debug("attaching inline comments", "count", len(inlineComments))
 	}
 
 	// --- Review update strategy ---
 	// 1. POST new review first (gets non-stale approval badge on HEAD)
 	// 2. Then supersede old review with link to the new one
 	// Order matters: post first so we have the new review's URL for the supersede message.
-	var oldReviews []vcsReview
+	var oldReviews []reviewInfo
 	if *reviewerName != "" {
-		existingReviews, err := vcs.ListReviews(ctx, owner, repoName, prNumber)
+		existingReviews, err := vcsClientImpl.ListReviews(ctx, owner, repoName, prNumber)
 		if err != nil {
 			slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
 		} else {
@@ -494,47 +477,56 @@ func main() {
 	}
 
 	// Self-request as reviewer (ensures we appear in required-reviewer checks)
-	authUser, err := vcs.GetAuthenticatedUser(ctx)
+	authUser, err := vcsClientImpl.GetAuthenticatedUser(ctx)
 	if err != nil {
 		slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
 	} else if authUser != "" {
-		if err := 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)
+		if ext, ok := vcsClientImpl.(giteaExtendedClient); ok {
+			if err := ext.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
+				slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
+			} else {
+				slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
+			}
 		}
 	}
 
 	// POST new review
 	slog.Info("posting review", "event", event, "pr", prNumber)
-	posted, err := vcs.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, evaluatedSHA, inlineComments)
+	posted, err := vcsClientImpl.PostReview(ctx, owner, repoName, prNumber, vcs.ReviewRequest{
+		Body:     reviewBody,
+		Event:    event,
+		CommitID: evaluatedSHA,
+		Comments: inlineComments,
+	})
 	if err != nil {
 		slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
 		os.Exit(1)
 	}
 	slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
 
-	// 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 {
+	// Supersede all old reviews with link to the new one
+	if len(oldReviews) > 0 {
 		newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*vcsURL, "/"), owner, repoName, prNumber, posted.ID)
+		ext, hasExt := vcsClientImpl.(giteaExtendedClient)
 		for _, oldReview := range oldReviews {
-			cid, err := extVCS.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, int64(prNumber), oldReview.ID)
+			if !hasExt {
+				slog.Debug("VCS client does not support review supersede; skipping", "review_id", oldReview.ID)
+				continue
+			}
+			cid, err := ext.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
 			if err != nil {
 				slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
 				continue
 			}
 			supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
-			if err := extVCS.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
+			if err := ext.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
 				slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err)
 				continue
 			}
 			slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)
 
 			// Resolve old review's inline comments
-			oldComments, err := extVCS.ListReviewComments(ctx, owner, repoName, int64(prNumber), oldReview.ID)
+			oldComments, err := ext.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
 			if err != nil {
 				slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
 				continue
@@ -544,7 +536,7 @@ func main() {
 				if c.ID == 0 {
 					continue
 				}
-				if err := extVCS.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
+				if err := ext.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
 					slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
 					failed++
 				} else {
@@ -558,14 +550,12 @@ 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 vcsClient, owner, repo, ref string, files []vcsChangedFile) string {
+func fetchFileContext(ctx context.Context, client vcsClient, owner, repo, ref string, files []changedFileInfo) string {
 	var sb strings.Builder
 	for _, f := range files {
 		if ctx.Err() != nil {
@@ -629,8 +619,13 @@ func fetchPatterns(ctx context.Context, client vcsClient, patternsRepo, patterns
 		var repoLoadedFiles []string
 		var repoSkippedFiles []string
 
+		giteaRaw, isGitea := client.(*giteaClientVCSAdapter)
+		if !isGitea {
+			slog.Warn("patterns fetching is only supported with the Gitea VCS client; skipping", "repo", repoRef)
+			continue
+		}
 		for _, path := range paths {
-			files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
+			files, err := giteaRaw.client.GetAllFilesInPath(ctx, owner, repo, path)
 			if err != nil {
 				slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err)
 				continue
@@ -669,7 +664,7 @@ func isPatternFile(path string) bool {
 }
 
 // evaluateCIStatus checks if all CI statuses indicate success.
-func evaluateCIStatus(statuses []vcsCommitStatus) (passed bool, details string) {
+func evaluateCIStatus(statuses []commitStatusInfo) (passed bool, details string) {
 	if len(statuses) == 0 {
 		return true, "no CI statuses found"
 	}
@@ -692,19 +687,6 @@ func evaluateCIStatus(statuses []vcsCommitStatus) (passed bool, details string)
 	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
@@ -820,7 +802,7 @@ func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string)
 // Gitea user. This indicates misconfiguration where two roles share a token
 // instead of having separate Gitea accounts. Returns true if shared token
 // detected (caller should skip update-in-place logic to avoid clobbering).
-func hasSharedToken(reviews []vcsReview, ownSentinel string) bool {
+func hasSharedToken(reviews []reviewInfo, ownSentinel string) bool {
 	ownLogin := ""
 	for _, r := range reviews {
 		if strings.Contains(r.Body, ownSentinel) {
@@ -858,8 +840,8 @@ func extractSentinelName(body string) string {
 }
 
 // findOwnReview locates the most recent non-superseded review matching the sentinel.
-func findOwnReview(reviews []vcsReview, sentinel string) *vcsReview {
-	var best *vcsReview
+func findOwnReview(reviews []reviewInfo, sentinel string) *reviewInfo {
+	var best *reviewInfo
 	for i := range reviews {
 		if !strings.Contains(reviews[i].Body, sentinel) {
 			continue
@@ -875,8 +857,8 @@ func findOwnReview(reviews []vcsReview, sentinel string) *vcsReview {
 }
 
 // findAllOwnReviews returns all non-superseded reviews matching the sentinel.
-func findAllOwnReviews(reviews []vcsReview, sentinel string) []vcsReview {
-	var result []vcsReview
+func findAllOwnReviews(reviews []reviewInfo, sentinel string) []reviewInfo {
+	var result []reviewInfo
 	for i := range reviews {
 		if !strings.Contains(reviews[i].Body, sentinel) {
 			continue
@@ -889,6 +871,22 @@ func findAllOwnReviews(reviews []vcsReview, sentinel string) []vcsReview {
 	return result
 }
 
+// verdictToVCSEvent converts a review verdict string to a vcs.ReviewEvent.
+// The verdict comes from the LLM result and uses values: "APPROVE", "REQUEST_CHANGES",
+// or any other string (treated as COMMENT).
+// vcs.ReviewEvent constants follow GitHub API format ("APPROVE", "REQUEST_CHANGES", "COMMENT").
+// The Gitea adapter translates these back to Gitea format ("APPROVED", etc.) before posting.
+func verdictToVCSEvent(verdict string) vcs.ReviewEvent {
+	switch verdict {
+	case "APPROVE":
+		return vcs.ReviewEventApprove
+	case "REQUEST_CHANGES":
+		return vcs.ReviewEventRequestChanges
+	default:
+		return vcs.ReviewEventComment
+	}
+}
+
 // shouldSkipStaleReview reports whether to skip posting because HEAD moved.
 // Returns true (skip) if evaluatedSHA differs from currentSHA.
 // Returns false (don't skip) if:
@@ -902,4 +900,32 @@ func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool {
 	return evaluatedSHA != currentSHA
 }
 
+// reviewClientAdapter adapts a vcsClient to review.GiteaClient for persona loading.
+// The review package only needs ListContents and GetFileContent, which all vcsClients provide.
+type reviewClientAdapter struct {
+	client vcsClient
+}
 
+func newReviewClientAdapter(c vcsClient) *reviewClientAdapter {
+	return &reviewClientAdapter{client: c}
+}
+
+func (a *reviewClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) {
+	entries, err := a.client.ListContents(ctx, owner, repo, path)
+	if err != nil {
+		return nil, err
+	}
+	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 *reviewClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+	return a.client.GetFileContent(ctx, owner, repo, filepath)
+}

cmd/review-bot/main_test.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"testing"
 
+	"gitea.weiker.me/rodin/review-bot/vcs"
 )
 
 func TestValidateReviewerName(t *testing.T) {
@@ -153,13 +154,14 @@ func TestValidateWorkspacePath(t *testing.T) {
 	}
 }
 
-func makeReview(id int64, login, state string, _ bool, body string) vcsReview {
-	r := vcsReview{
+func makeReview(id int64, login, state string, stale bool, body string) reviewInfo {
+	r := reviewInfo{
 		ID:    id,
 		Body:  body,
 		State: state,
+		Stale: stale,
 	}
-	r.User.Login = login
+	r.User = vcs.UserInfo{Login: login}
 	return r
 }
 
@@ -214,7 +216,7 @@ func TestBuildSupersededBodyShortSHA(t *testing.T) {
 func TestFindOwnReview(t *testing.T) {
 	tests := []struct {
 		name     string
-		reviews  []vcsReview
+		reviews  []reviewInfo
 		sentinel string
 		wantID   int64
 		wantNil  bool
@@ -227,7 +229,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "found by sentinel",
-			reviews: []vcsReview{
+			reviews: []reviewInfo{
 				makeReview(42, "bot", "APPROVED", false, "review body\n<!-- review-bot:sonnet -->"),
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
@@ -235,7 +237,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "wrong sentinel",
-			reviews: []vcsReview{
+			reviews: []reviewInfo{
 				makeReview(42, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
@@ -243,7 +245,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "multiple reviews, returns first match",
-			reviews: []vcsReview{
+			reviews: []reviewInfo{
 				makeReview(10, "bot", "APPROVED", false, "old\n<!-- review-bot:gpt -->"),
 				makeReview(20, "bot", "APPROVED", false, "new\n<!-- review-bot:sonnet -->"),
 			},
@@ -252,7 +254,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "skips superseded review",
-			reviews: []vcsReview{
+			reviews: []reviewInfo{
 				makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
 				makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
 			},
@@ -261,7 +263,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "only superseded reviews exist",
-			reviews: []vcsReview{
+			reviews: []reviewInfo{
 				makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
@@ -269,7 +271,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "picks highest ID among matches",
-			reviews: []vcsReview{
+			reviews: []reviewInfo{
 				makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
 				makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
 			},
@@ -300,7 +302,7 @@ func TestFindOwnReview(t *testing.T) {
 func TestHasSharedToken(t *testing.T) {
 	tests := []struct {
 		name     string
-		reviews  []vcsReview
+		reviews  []reviewInfo
 		sentinel string
 		want     bool
 	}{
@@ -312,36 +314,36 @@ func TestHasSharedToken(t *testing.T) {
 		},
 		{
 			name: "no own review yet - cannot detect",
-			reviews: []vcsReview{
-				{ID: 1, User: struct{ Login string }{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     false,
 		},
 		{
 			name: "separate users - no shared token",
-			reviews: []vcsReview{
-				{ID: 1, User: struct{ Login string }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
-				{ID: 2, User: struct{ Login string }{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: vcs.UserInfo{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     false,
 		},
 		{
 			name: "shared token detected - same user different sentinels",
-			reviews: []vcsReview{
-				{ID: 1, User: struct{ Login string }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
-				{ID: 2, User: struct{ Login string }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     true,
 		},
 		{
 			name: "three roles same user",
-			reviews: []vcsReview{
-				{ID: 1, User: struct{ Login string }{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
-				{ID: 2, User: struct{ Login string }{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
-				{ID: 3, User: struct{ Login string }{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
+				{ID: 3, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     true,
@@ -551,7 +553,7 @@ func TestBuildPatternPaths(t *testing.T) {
 func TestEvaluateCIStatus(t *testing.T) {
 	tests := []struct {
 		name       string
-		statuses   []vcsCommitStatus
+		statuses   []commitStatusInfo
 		wantPassed bool
 		wantSubstr string
 	}{
@@ -563,7 +565,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "all success",
-			statuses: []vcsCommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "success", Context: "ci/build", Description: "Build passed"},
 				{Status: "success", Context: "ci/test", Description: "Tests passed"},
 			},
@@ -572,7 +574,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "one failure",
-			statuses: []vcsCommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "success", Context: "ci/build", Description: "Build passed"},
 				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
 			},
@@ -581,7 +583,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "error status",
-			statuses: []vcsCommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "error", Context: "ci/lint", Description: "Lint error"},
 			},
 			wantPassed: false,
@@ -589,7 +591,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "pending treated as not-failed",
-			statuses: []vcsCommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "pending", Context: "ci/build", Description: "In progress"},
 				{Status: "success", Context: "ci/test", Description: "Tests passed"},
 			},
@@ -598,7 +600,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "multiple failures",
-			statuses: []vcsCommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "failure", Context: "ci/build", Description: "Build failed"},
 				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
 			},
@@ -607,7 +609,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "mixed with pending and failure",
-			statuses: []vcsCommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "success", Context: "ci/build", Description: "Build passed"},
 				{Status: "pending", Context: "ci/deploy", Description: "Deploying"},
 				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
@@ -995,7 +997,7 @@ func cleanEnv() []string {
 }
 
 func TestFindAllOwnReviews(t *testing.T) {
-	reviews := []vcsReview{
+	reviews := []reviewInfo{
 		{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
 		{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
 		{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},

cmd/review-bot/vcs.go

@@ -1,361 +0,0 @@
-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)
-}

cmd/review-bot/vcs_client.go

@@ -0,0 +1,405 @@
+package main
+
+import (
+	"context"
+
+	"gitea.weiker.me/rodin/review-bot/gitea"
+	"gitea.weiker.me/rodin/review-bot/github"
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+// vcsClient is the unified interface for VCS operations used by the review flow.
+// Both gitea.Client and github.Client satisfy this interface via their respective adapters.
+type vcsClient interface {
+	// GetPullRequest fetches PR metadata.
+	GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error)
+
+	// GetPullRequestDiff fetches the unified diff.
+	GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error)
+
+	// GetPullRequestFiles fetches the list of changed files.
+	GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error)
+
+	// GetCommitStatuses fetches CI statuses for a commit.
+	GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error)
+
+	// GetFileContent fetches the content of a file at HEAD.
+	GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error)
+
+	// GetFileContentRef fetches the content of a file at a given ref.
+	GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error)
+
+	// ListContents lists the files and directories at a path.
+	ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error)
+
+	// ListReviews returns all reviews for a PR.
+	ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error)
+
+	// PostReview submits a review.
+	PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error)
+
+	// GetAuthenticatedUser returns the login of the authenticated user.
+	GetAuthenticatedUser(ctx context.Context) (string, error)
+}
+
+// giteaExtendedClient is implemented by the Gitea adapter and exposes
+// Gitea-specific operations that have no GitHub equivalent in the current scope.
+// Callers should type-assert to this interface and skip gracefully when it is absent.
+type giteaExtendedClient interface {
+	// RequestReviewer adds the authenticated user as a reviewer on a PR.
+	RequestReviewer(ctx context.Context, owner, repo string, number int, user string) error
+
+	// GetTimelineReviewCommentIDForReview maps a review ID to its timeline comment ID.
+	GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error)
+
+	// EditComment updates the body of an existing PR comment.
+	EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error
+
+	// ListReviewComments lists the inline comments attached to a review.
+	ListReviewComments(ctx context.Context, owner, repo string, number int, reviewID int64) ([]inlineCommentInfo, error)
+
+	// ResolveComment marks an inline comment as resolved.
+	ResolveComment(ctx context.Context, owner, repo string, commentID int64) error
+
+	// ParseDiffNewLines returns the diff line ranges for inline comment positioning.
+	ParseDiffNewLines(diff string) diffLineRanges
+}
+
+// Shared adapter types to avoid duplicating gitea/github-specific types throughout main.go.
+
+type pullRequestInfo struct {
+	Title   string
+	Body    string
+	HeadSha string
+	HeadRef string
+}
+
+type changedFileInfo struct {
+	Filename string
+	Status   string
+}
+
+type commitStatusInfo struct {
+	Status      string
+	Context     string
+	Description string
+	TargetURL   string
+}
+
+type contentEntryInfo struct {
+	Name string
+	Path string
+	Type string
+}
+
+type reviewInfo struct {
+	ID       int64
+	Body     string
+	User     vcs.UserInfo
+	State    string
+	CommitID string
+	Stale    bool
+}
+
+type inlineCommentInfo struct {
+	ID          int64
+	Path        string
+	NewPosition int64
+	Body        string
+}
+
+// diffLineRanges is a type alias for gitea.DiffLineRanges to allow the
+// extended client interface to be defined without importing gitea directly.
+// In practice, only the giteaClientVCSAdapter returns this type, and callers
+// that use it will already have performed the type assertion.
+type diffLineRanges = *gitea.DiffLineRanges
+
+// --- Gitea adapter ---
+
+// giteaClientVCSAdapter wraps gitea.Client to satisfy the vcsClient interface.
+// It also implements giteaExtendedClient for Gitea-specific operations.
+type giteaClientVCSAdapter struct {
+	client *gitea.Client
+}
+
+func newGiteaVCSAdapter(c *gitea.Client) *giteaClientVCSAdapter {
+	return &giteaClientVCSAdapter{client: c}
+}
+
+func (a *giteaClientVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error) {
+	pr, err := a.client.GetPullRequest(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	return &pullRequestInfo{
+		Title:   pr.Title,
+		Body:    pr.Body,
+		HeadSha: pr.Head.Sha,
+		HeadRef: pr.Head.Ref,
+	}, nil
+}
+
+func (a *giteaClientVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
+	return a.client.GetPullRequestDiff(ctx, owner, repo, number)
+}
+
+func (a *giteaClientVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error) {
+	files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]changedFileInfo, len(files))
+	for i, f := range files {
+		result[i] = changedFileInfo{Filename: f.Filename, Status: f.Status}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error) {
+	statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]commitStatusInfo, len(statuses))
+	for i, s := range statuses {
+		result[i] = commitStatusInfo{
+			Status:      s.Status,
+			Context:     s.Context,
+			Description: s.Description,
+			TargetURL:   s.TargetURL,
+		}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+	return a.client.GetFileContent(ctx, owner, repo, filepath)
+}
+
+func (a *giteaClientVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
+	return a.client.GetFileContentRef(ctx, owner, repo, filepath, ref)
+}
+
+func (a *giteaClientVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error) {
+	entries, err := a.client.ListContents(ctx, owner, repo, path)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]contentEntryInfo, len(entries))
+	for i, e := range entries {
+		result[i] = contentEntryInfo{Name: e.Name, Path: e.Path, Type: e.Type}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error) {
+	reviews, err := a.client.ListReviews(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]reviewInfo, len(reviews))
+	for i, r := range reviews {
+		result[i] = reviewInfo{
+			ID:       r.ID,
+			Body:     r.Body,
+			User:     vcs.UserInfo{Login: r.User.Login},
+			State:    r.State,
+			CommitID: r.CommitID,
+			Stale:    r.Stale,
+		}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error) {
+	// Translate vcs.ReviewComment to gitea.ReviewComment.
+	// The Gitea API uses NewPosition instead of Position.
+	var comments []gitea.ReviewComment
+	for _, c := range req.Comments {
+		comments = append(comments, gitea.ReviewComment{
+			Path:        c.Path,
+			NewPosition: int64(c.Position),
+			Body:        c.Body,
+		})
+	}
+
+	// Translate vcs.ReviewEvent (GitHub format) to Gitea API event string.
+	// vcs uses "APPROVE"; Gitea API expects "APPROVED".
+	var event string
+	switch req.Event {
+	case vcs.ReviewEventApprove:
+		event = "APPROVED"
+	case vcs.ReviewEventRequestChanges:
+		event = "REQUEST_CHANGES"
+	default:
+		event = "COMMENT"
+	}
+
+	posted, err := a.client.PostReview(ctx, owner, repo, number, event, req.Body, req.CommitID, comments)
+	if err != nil {
+		return nil, err
+	}
+	return &reviewInfo{
+		ID:       posted.ID,
+		Body:     posted.Body,
+		User:     vcs.UserInfo{Login: posted.User.Login},
+		State:    posted.State,
+		CommitID: posted.CommitID,
+	}, nil
+}
+
+func (a *giteaClientVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	return a.client.GetAuthenticatedUser(ctx)
+}
+
+// giteaExtendedClient implementation:
+
+func (a *giteaClientVCSAdapter) RequestReviewer(ctx context.Context, owner, repo string, number int, user string) error {
+	return a.client.RequestReviewer(ctx, owner, repo, number, user)
+}
+
+func (a *giteaClientVCSAdapter) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) {
+	return a.client.GetTimelineReviewCommentIDForReview(ctx, owner, repo, number, reviewID)
+}
+
+func (a *giteaClientVCSAdapter) EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error {
+	return a.client.EditComment(ctx, owner, repo, commentID, body)
+}
+
+func (a *giteaClientVCSAdapter) ListReviewComments(ctx context.Context, owner, repo string, number int, reviewID int64) ([]inlineCommentInfo, error) {
+	comments, err := a.client.ListReviewComments(ctx, owner, repo, number, reviewID)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]inlineCommentInfo, len(comments))
+	for i, c := range comments {
+		result[i] = inlineCommentInfo{
+			ID:          c.ID,
+			Path:        c.Path,
+			NewPosition: c.NewPosition,
+			Body:        c.Body,
+		}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error {
+	return a.client.ResolveComment(ctx, owner, repo, commentID)
+}
+
+func (a *giteaClientVCSAdapter) ParseDiffNewLines(diff string) diffLineRanges {
+	return gitea.ParseDiffNewLines(diff)
+}
+
+// --- GitHub adapter ---
+
+// githubClientVCSAdapter wraps github.Client to satisfy the vcsClient interface.
+type githubClientVCSAdapter struct {
+	client *github.Client
+}
+
+func newGitHubVCSAdapter(c *github.Client) *githubClientVCSAdapter {
+	return &githubClientVCSAdapter{client: c}
+}
+
+func (a *githubClientVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error) {
+	pr, err := a.client.GetPullRequest(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	return &pullRequestInfo{
+		Title:   pr.Title,
+		Body:    pr.Body,
+		HeadSha: pr.Head.Sha,
+		HeadRef: pr.Head.Ref,
+	}, nil
+}
+
+func (a *githubClientVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
+	return a.client.GetPullRequestDiff(ctx, owner, repo, number)
+}
+
+func (a *githubClientVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error) {
+	files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]changedFileInfo, len(files))
+	for i, f := range files {
+		result[i] = changedFileInfo{Filename: f.Filename, Status: f.Status}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error) {
+	statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]commitStatusInfo, len(statuses))
+	for i, s := range statuses {
+		result[i] = commitStatusInfo{
+			Status:      s.Status,
+			Context:     s.Context,
+			Description: s.Description,
+			TargetURL:   s.TargetURL,
+		}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+	return a.client.GetFileContent(ctx, owner, repo, filepath)
+}
+
+func (a *githubClientVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
+	return a.client.GetFileContentRef(ctx, owner, repo, filepath, ref)
+}
+
+func (a *githubClientVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error) {
+	entries, err := a.client.ListContents(ctx, owner, repo, path)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]contentEntryInfo, len(entries))
+	for i, e := range entries {
+		result[i] = contentEntryInfo{Name: e.Name, Path: e.Path, Type: e.Type}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error) {
+	reviews, err := a.client.ListReviews(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]reviewInfo, len(reviews))
+	for i, r := range reviews {
+		result[i] = reviewInfo{
+			ID:       r.ID,
+			Body:     r.Body,
+			User:     r.User,
+			State:    r.State,
+			CommitID: r.CommitID,
+		}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error) {
+	posted, err := a.client.PostReview(ctx, owner, repo, number, req)
+	if err != nil {
+		return nil, err
+	}
+	return &reviewInfo{
+		ID:       posted.ID,
+		Body:     posted.Body,
+		User:     posted.User,
+		State:    posted.State,
+		CommitID: posted.CommitID,
+	}, nil
+}
+
+func (a *githubClientVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	return a.client.GetAuthenticatedUser(ctx)
+}

github/api_test.go

@@ -0,0 +1,582 @@
+package github
+
+import (
+	"context"
+	"encoding/base64"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+func TestGetPullRequest(t *testing.T) {
+	want := PullRequest{
+		Title: "My PR",
+		Body:  "Description",
+	}
+	want.Head.Sha = "abc123"
+	want.Head.Ref = "feature/foo"
+
+	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.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(want)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	pr, err := c.GetPullRequest(context.Background(), "owner", "repo", 42)
+	if err != nil {
+		t.Fatalf("GetPullRequest: %v", err)
+	}
+	if pr.Title != want.Title {
+		t.Errorf("Title = %q, want %q", pr.Title, want.Title)
+	}
+	if pr.Head.Sha != want.Head.Sha {
+		t.Errorf("Head.Sha = %q, want %q", pr.Head.Sha, want.Head.Sha)
+	}
+	if pr.Head.Ref != want.Head.Ref {
+		t.Errorf("Head.Ref = %q, want %q", pr.Head.Ref, want.Head.Ref)
+	}
+}
+
+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("not found"))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	_, err := c.GetPullRequest(context.Background(), "owner", "repo", 1)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if !IsNotFound(err) {
+		t.Errorf("expected IsNotFound, got %v", err)
+	}
+}
+
+func TestGetPullRequestDiff(t *testing.T) {
+	const wantDiff = "diff --git a/foo.go b/foo.go\n+added line\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.Header().Set("Content-Type", "text/plain")
+		fmt.Fprint(w, wantDiff)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	diff, err := c.GetPullRequestDiff(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("GetPullRequestDiff: %v", err)
+	}
+	if diff != wantDiff {
+		t.Errorf("diff = %q, want %q", diff, wantDiff)
+	}
+}
+
+func TestGetPullRequestDiff_TooLarge(t *testing.T) {
+	// Return a diff larger than DefaultMaxDiffSize.
+	hugeDiff := make([]byte, DefaultMaxDiffSize+1)
+	for i := range hugeDiff {
+		hugeDiff[i] = 'x'
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		w.Write(hugeDiff)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	_, err := c.GetPullRequestDiff(context.Background(), "owner", "repo", 1)
+	if err == nil {
+		t.Fatal("expected ErrDiffTooLarge, got nil")
+	}
+	if err != ErrDiffTooLarge {
+		t.Errorf("err = %v, want ErrDiffTooLarge", err)
+	}
+}
+
+func TestGetPullRequestFiles(t *testing.T) {
+	files := []ChangedFile{
+		{Filename: "foo.go", Status: "modified"},
+		{Filename: "bar.go", Status: "added"},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/pulls/7/files" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(files)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 7)
+	if err != nil {
+		t.Fatalf("GetPullRequestFiles: %v", err)
+	}
+	if len(got) != len(files) {
+		t.Fatalf("len = %d, want %d", len(got), len(files))
+	}
+	for i, f := range files {
+		if got[i].Filename != f.Filename || got[i].Status != f.Status {
+			t.Errorf("file[%d] = %+v, want %+v", i, got[i], f)
+		}
+	}
+}
+
+func TestGetCommitStatuses(t *testing.T) {
+	statuses := []CommitStatus{
+		{Status: "success", Context: "ci/tests", Description: "All tests passed", TargetURL: "https://ci.example.com"},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/commits/deadbeef/statuses" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(statuses)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetCommitStatuses(context.Background(), "owner", "repo", "deadbeef")
+	if err != nil {
+		t.Fatalf("GetCommitStatuses: %v", err)
+	}
+	if len(got) != 1 {
+		t.Fatalf("len = %d, want 1", len(got))
+	}
+	if got[0].Status != "success" || got[0].Context != "ci/tests" {
+		t.Errorf("status = %+v, want %+v", got[0], statuses[0])
+	}
+}
+
+func TestGetFileContent(t *testing.T) {
+	const wantContent = "package main\n\nfunc main() {}\n"
+	encoded := base64.StdEncoding.EncodeToString([]byte(wantContent))
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/contents/main.go" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Name:     "main.go",
+			Path:     "main.go",
+			Encoding: "base64",
+			Content:  encoded,
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetFileContent(context.Background(), "owner", "repo", "main.go")
+	if err != nil {
+		t.Fatalf("GetFileContent: %v", err)
+	}
+	if got != wantContent {
+		t.Errorf("content = %q, want %q", got, wantContent)
+	}
+}
+
+func TestGetFileContentRef(t *testing.T) {
+	const wantContent = "version: 2\n"
+	encoded := base64.StdEncoding.EncodeToString([]byte(wantContent))
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/contents/config.yml" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		if r.URL.Query().Get("ref") != "feature/x" {
+			t.Errorf("ref = %q, want feature/x", r.URL.Query().Get("ref"))
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Name:     "config.yml",
+			Path:     "config.yml",
+			Encoding: "base64",
+			Content:  encoded,
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetFileContentRef(context.Background(), "owner", "repo", "config.yml", "feature/x")
+	if err != nil {
+		t.Fatalf("GetFileContentRef: %v", err)
+	}
+	if got != wantContent {
+		t.Errorf("content = %q, want %q", got, wantContent)
+	}
+}
+
+func TestGetFileContent_NewlinesInBase64(t *testing.T) {
+	// GitHub inserts newlines every 60 chars in the base64-encoded content.
+	// Verify we strip them before decoding.
+	const wantContent = "hello world this is a long string that gets split by github with newlines in the base64 encoding"
+	rawEncoded := base64.StdEncoding.EncodeToString([]byte(wantContent))
+	// Insert newlines to simulate GitHub's format.
+	var chunked string
+	for i := 0; i < len(rawEncoded); i += 60 {
+		end := i + 60
+		if end > len(rawEncoded) {
+			end = len(rawEncoded)
+		}
+		chunked += rawEncoded[i:end] + "\n"
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Encoding: "base64",
+			Content:  chunked,
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetFileContent(context.Background(), "owner", "repo", "file.txt")
+	if err != nil {
+		t.Fatalf("GetFileContent: %v", err)
+	}
+	if got != wantContent {
+		t.Errorf("content = %q, want %q", got, wantContent)
+	}
+}
+
+func TestListContents_Directory(t *testing.T) {
+	entries := []ContentEntry{
+		{Name: "main.go", Path: "main.go", Type: "file"},
+		{Name: "lib", Path: "lib", Type: "dir"},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/contents/" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(entries)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.ListContents(context.Background(), "owner", "repo", "")
+	if err != nil {
+		t.Fatalf("ListContents: %v", err)
+	}
+	if len(got) != len(entries) {
+		t.Fatalf("len = %d, want %d", len(got), len(entries))
+	}
+	for i, e := range entries {
+		if got[i].Name != e.Name || got[i].Type != e.Type {
+			t.Errorf("entry[%d] = %+v, want %+v", i, got[i], e)
+		}
+	}
+}
+
+func TestListContents_File(t *testing.T) {
+	// When path points to a single file, GitHub returns an object, not array.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		// Return a single file object (not an array).
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Name:     "README.md",
+			Path:     "README.md",
+			Encoding: "base64",
+			Content:  base64.StdEncoding.EncodeToString([]byte("# Hello")),
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.ListContents(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("ListContents (file): %v", err)
+	}
+	if len(got) != 1 {
+		t.Fatalf("len = %d, want 1", len(got))
+	}
+	if got[0].Name != "README.md" || got[0].Type != "file" {
+		t.Errorf("entry = %+v", got[0])
+	}
+}
+
+func TestGetAuthenticatedUser(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/user" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(userResponse{Login: "rodin"})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	login, err := c.GetAuthenticatedUser(context.Background())
+	if err != nil {
+		t.Fatalf("GetAuthenticatedUser: %v", err)
+	}
+	if login != "rodin" {
+		t.Errorf("login = %q, want %q", login, "rodin")
+	}
+}
+
+func TestPostReview(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/5/reviews" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		var payload postReviewRequest
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		if payload.Event != "REQUEST_CHANGES" {
+			t.Errorf("event = %q, want REQUEST_CHANGES", payload.Event)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviewResponse{
+			ID:    99,
+			Body:  payload.Body,
+			State: "CHANGES_REQUESTED",
+			User:  struct{ Login string `json:"login"` }{Login: "rodin"},
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	review, err := c.PostReview(context.Background(), "owner", "repo", 5, vcs.ReviewRequest{
+		Body:  "needs work",
+		Event: vcs.ReviewEventRequestChanges,
+	})
+	if err != nil {
+		t.Fatalf("PostReview: %v", err)
+	}
+	if review.ID != 99 {
+		t.Errorf("review.ID = %d, want 99", review.ID)
+	}
+	// Verify state translation: CHANGES_REQUESTED -> REQUEST_CHANGES
+	if review.State != "REQUEST_CHANGES" {
+		t.Errorf("review.State = %q, want REQUEST_CHANGES", review.State)
+	}
+	if review.User.Login != "rodin" {
+		t.Errorf("review.User.Login = %q, want rodin", review.User.Login)
+	}
+}
+
+func TestPostReview_ConflictingCommitIDs(t *testing.T) {
+	c := NewClient("tok", "https://api.github.com")
+	_, err := c.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
+		Body:  "test",
+		Event: vcs.ReviewEventComment,
+		Comments: []vcs.ReviewComment{
+			{Path: "a.go", Position: 1, Body: "comment 1", CommitID: "sha1"},
+			{Path: "b.go", Position: 2, Body: "comment 2", CommitID: "sha2"},
+		},
+	})
+	if err == nil {
+		t.Fatal("expected ErrConflictingCommitIDs, got nil")
+	}
+	if err != ErrConflictingCommitIDs {
+		t.Errorf("err = %v, want ErrConflictingCommitIDs", err)
+	}
+}
+
+func TestListReviews(t *testing.T) {
+	reviews := []reviewResponse{
+		{ID: 1, Body: "lgtm", State: "APPROVED", User: struct{ Login string `json:"login"` }{Login: "alice"}},
+		{ID: 2, Body: "needs work", State: "CHANGES_REQUESTED", User: struct{ Login string `json:"login"` }{Login: "bob"}},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/pulls/3/reviews" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviews)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.ListReviews(context.Background(), "owner", "repo", 3)
+	if err != nil {
+		t.Fatalf("ListReviews: %v", err)
+	}
+	if len(got) != 2 {
+		t.Fatalf("len = %d, want 2", len(got))
+	}
+	if got[0].State != "APPROVED" {
+		t.Errorf("got[0].State = %q, want APPROVED", got[0].State)
+	}
+	if got[1].State != "REQUEST_CHANGES" {
+		t.Errorf("got[1].State = %q, want REQUEST_CHANGES (translated from CHANGES_REQUESTED)", got[1].State)
+	}
+}
+
+func TestDeleteReview_Pending(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("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		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("DeleteReview: %v", err)
+	}
+}
+
+func TestDeleteReview_Submitted_Returns422(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// GitHub returns 422 when trying to delete a submitted review.
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		w.Write([]byte(`{"message":"Cannot delete a submitted review"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	err := c.DeleteReview(context.Background(), "owner", "repo", 1, 42)
+	if err == nil {
+		t.Fatal("expected error for submitted review deletion")
+	}
+	// Should be wrapped as ErrCannotDeleteSubmittedReview.
+	if !isErrCannotDeleteSubmittedReview(err) {
+		t.Errorf("err = %v, want ErrCannotDeleteSubmittedReview", err)
+	}
+}
+
+// isErrCannotDeleteSubmittedReview checks if err wraps ErrCannotDeleteSubmittedReview.
+func isErrCannotDeleteSubmittedReview(err error) bool {
+	return err != nil && errors.Is(err, ErrCannotDeleteSubmittedReview)
+}
+
+func TestDismissReview(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPut {
+			t.Errorf("method = %s, want PUT", r.Method)
+		}
+		if r.URL.Path != "/repos/owner/repo/pulls/2/reviews/10/dismissals" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		var payload dismissReviewRequest
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		if payload.Event != "DISMISS" {
+			t.Errorf("event = %q, want DISMISS", payload.Event)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviewResponse{ID: 10, State: "DISMISSED"})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	err := c.DismissReview(context.Background(), "owner", "repo", 2, 10, "outdated review")
+	if err != nil {
+		t.Fatalf("DismissReview: %v", err)
+	}
+}
+
+func TestDoRequestWithBody_RejectsHTTP(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Fatal("request should not have been sent")
+	}))
+	defer srv.Close()
+
+	// Without AllowInsecureHTTPForTest, HTTP should be rejected.
+	c := NewClient("tok", srv.URL)
+	_, err := c.doRequestWithBody(context.Background(), http.MethodPost, srv.URL+"/test", []byte(`{}`))
+	if err == nil {
+		t.Fatal("expected error for HTTP request")
+	}
+}
+
+func TestPostReview_CommitIDFromRequest(t *testing.T) {
+	const wantCommitID = "abc123def456"
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		var payload postReviewRequest
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		if payload.CommitID != wantCommitID {
+			t.Errorf("commit_id = %q, want %q", payload.CommitID, wantCommitID)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviewResponse{
+			ID:       1,
+			Body:     payload.Body,
+			State:    "COMMENTED",
+			CommitID: payload.CommitID,
+			User:     struct{ Login string `json:"login"` }{Login: "rodin"},
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	review, err := c.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
+		Body:     "looks good",
+		Event:    vcs.ReviewEventApprove,
+		CommitID: wantCommitID,
+	})
+	if err != nil {
+		t.Fatalf("PostReview: %v", err)
+	}
+	if review.CommitID != wantCommitID {
+		t.Errorf("review.CommitID = %q, want %q", review.CommitID, wantCommitID)
+	}
+}

github/client.go

@@ -376,58 +376,11 @@ 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))
-		}
-	}
+// DefaultMaxDiffSize is the default maximum diff size in bytes (10 MB).
+const DefaultMaxDiffSize = 10 * 1024 * 1024
 
-	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 ---
+// ErrDiffTooLarge is returned when a PR diff exceeds the configured MaxDiffSize.
+var ErrDiffTooLarge = errors.New("diff size exceeds maximum allowed size")
 
 // PullRequest holds relevant PR metadata.
 type PullRequest struct {
@@ -437,14 +390,11 @@ type PullRequest 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"
+	Status      string `json:"state"`
 	Context     string `json:"context"`
 	Description string `json:"description"`
 	TargetURL   string `json:"target_url"`
@@ -456,372 +406,198 @@ type ChangedFile struct {
 	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.
+// ContentEntry represents a file or directory in a repository.
 type ContentEntry struct {
 	Name string `json:"name"`
 	Path string `json:"path"`
-	Type string `json:"type"` // "file" or "dir"
+	Type string `json:"type"`
 }
 
-// --- PR methods ---
+// contentResponse is the GitHub API response for a single file's content.
+type contentResponse struct {
+	Content  string `json:"content"`
+	Encoding string `json:"encoding"`
+	Type     string `json:"type"`
+	Name     string `json:"name"`
+	Path     string `json:"path"`
+}
 
 // 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 nil, fmt.Errorf("parse PR response: %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
-		}
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/files",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("fetch PR files: %w", err)
 	}
-	return all, nil
+	var files []ChangedFile
+	if err := json.Unmarshal(body, &files); err != nil {
+		return nil, fmt.Errorf("parse PR files response: %w", err)
+	}
+	return files, 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
-		}
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/commits/%s/statuses",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha))
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("fetch commit statuses: %w", err)
 	}
-	return all, nil
+	var statuses []CommitStatus
+	if err := json.Unmarshal(body, &statuses); err != nil {
+		return nil, fmt.Errorf("parse commit statuses response: %w", err)
+	}
+	return statuses, nil
 }
 
-// --- File content methods ---
+// decodeFileContent decodes the content from a GitHub contents API response.
+// GitHub returns content as base64-encoded string with newlines inserted;
+// this function strips the newlines before decoding.
+func decodeFileContent(resp contentResponse) (string, error) {
+	if resp.Encoding != "base64" {
+		return "", fmt.Errorf("unsupported content encoding: %q", resp.Encoding)
+	}
+	// GitHub inserts newlines every 60 characters; strip them before decoding.
+	stripped := strings.ReplaceAll(resp.Content, "\n", "")
+	decoded, err := base64.StdEncoding.DecodeString(stripped)
+	if err != nil {
+		return "", fmt.Errorf("decode base64 content: %w", err)
+	}
+	return string(decoded), nil
+}
 
-// GetFileContent fetches a file from the default branch of a repo.
-// GitHub returns base64-encoded content; this method decodes it.
+// GetFileContent fetches the content of a file at the default branch HEAD.
 func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
-	return c.getFileContentAtRef(ctx, owner, repo, filepath, "")
+	return c.GetFileContentRef(ctx, owner, repo, filepath, "")
 }
 
-// GetFileContentRef fetches a file from a specific ref (branch/tag/sha).
+// GetFileContentRef fetches the content of a file at the given ref (branch, tag, or SHA).
+// If ref is empty, the default branch is used.
 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)
+		return "", fmt.Errorf("fetch file content %q: %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)
+		return "", fmt.Errorf("parse file content response: %w", err)
 	}
-	if resp.Type != "file" {
-		return "", fmt.Errorf("path %s is a %s, not a file", filepath, resp.Type)
+	content, err := decodeFileContent(resp)
+	if err != nil {
+		return "", fmt.Errorf("decode file content %q: %w", filepath, err)
 	}
-	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
+	return 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.
+// ListContents lists the files and directories at the given path.
+// If path points to a file instead of a directory, it returns a single-entry slice.
 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))
-	}
+	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)
+		return nil, fmt.Errorf("list contents %q: %w", path, err)
 	}
 
+	// The GitHub API returns an array for directories and an object for files.
+	// Try array first; if it fails, try object and wrap in a slice.
 	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,
-		}}
+	if err := json.Unmarshal(body, &entries); err == nil {
+		return entries, nil
 	}
-	return entries, nil
+
+	// Try as a single ContentEntry (file path).
+	var single contentResponse
+	if err := json.Unmarshal(body, &single); err != nil {
+		return nil, fmt.Errorf("parse contents response for %q: %w", path, err)
+	}
+	return []ContentEntry{{
+		Name: single.Name,
+		Path: single.Path,
+		Type: single.Type,
+	}}, 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)
+// doRequestWithBody performs an HTTP request with a JSON request body.
+// Unlike doRequest, it does NOT retry — write operations should not be
+// retried automatically.
+func (c *Client) doRequestWithBody(ctx context.Context, method, reqURL string, data []byte) ([]byte, error) {
+	if !c.allowInsecureHTTP {
+		parsed, err := url.Parse(reqURL)
 		if err != nil {
-			return nil, fmt.Errorf("list reviews (page %d): %w", page, err)
+			return nil, fmt.Errorf("parse request URL: %w", 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
+		if strings.EqualFold(parsed.Scheme, "http") {
+			return nil, fmt.Errorf("refusing HTTP request to %s: use HTTPS or set AllowInsecureHTTP option", redactURL(reqURL))
 		}
 	}
-	return all, nil
+
+	var bodyReader *bytes.Reader
+	if data != nil {
+		bodyReader = bytes.NewReader(data)
+	} else {
+		bodyReader = bytes.NewReader(nil)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, method, reqURL, bodyReader)
+	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 data != 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)
+	}
+
+	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
+		body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodyBytes))
+		resp.Body.Close()
+		if err != nil {
+			return nil, fmt.Errorf("read response body: %w", err)
+		}
+		return body, nil
+	}
+
+	errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
+	resp.Body.Close()
+
+	return nil, &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
 }
 
-// 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.
+// escapePath escapes each segment of a file path for use in URL path components.
+// Slashes are preserved as separators; individual segments are PathEscaped.
 func escapePath(p string) string {
 	parts := strings.Split(p, "/")
 	for i, part := range parts {
@@ -829,3 +605,53 @@ func escapePath(p string) string {
 	}
 	return strings.Join(parts, "/")
 }
+
+// GetPullRequestDiff fetches the unified diff for a PR.
+// Returns ErrDiffTooLarge if the response exceeds DefaultMaxDiffSize bytes.
+//
+// It reads up to DefaultMaxDiffSize+1 bytes and checks for truncation:
+// if exactly DefaultMaxDiffSize+1 bytes are available, the diff is too large.
+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)
+
+	if !c.allowInsecureHTTP {
+		parsed, err := url.Parse(reqURL)
+		if err != nil {
+			return "", fmt.Errorf("parse request URL: %w", err)
+		}
+		if strings.EqualFold(parsed.Scheme, "http") {
+			return "", fmt.Errorf("refusing HTTP request to %s: use HTTPS or set AllowInsecureHTTP option", redactURL(reqURL))
+		}
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
+	if err != nil {
+		return "", fmt.Errorf("create PR diff request: %w", err)
+	}
+	req.Header.Set("Authorization", "Bearer "+c.token)
+	req.Header.Set("Accept", "application/vnd.github.diff")
+
+	resp, err := c.httpClient.Do(req)
+	if err != nil {
+		return "", fmt.Errorf("fetch PR diff: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
+		return "", &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
+	}
+
+	// Read up to DefaultMaxDiffSize+1 bytes. If we get exactly that many,
+	// the diff exceeds the limit.
+	limit := int64(DefaultMaxDiffSize) + 1
+	raw, err := io.ReadAll(io.LimitReader(resp.Body, limit))
+	if err != nil {
+		return "", fmt.Errorf("read PR diff: %w", err)
+	}
+	if int64(len(raw)) == limit {
+		return "", ErrDiffTooLarge
+	}
+	return string(raw), nil
+}

github/client_test.go

@@ -2,9 +2,7 @@ package github
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -658,475 +656,3 @@ 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)
-		}
-	}
-}

github/identity.go

@@ -0,0 +1,29 @@
+package github
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+)
+
+// userResponse is the GitHub API response for the authenticated user.
+type userResponse struct {
+	Login string `json:"login"`
+}
+
+// GetAuthenticatedUser returns the login of the currently authenticated user.
+func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	reqURL := fmt.Sprintf("%s/user", c.baseURL)
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return "", fmt.Errorf("get authenticated user: %w", err)
+	}
+
+	var resp userResponse
+	if err := json.Unmarshal(body, &resp); err != nil {
+		return "", fmt.Errorf("parse user response: %w", err)
+	}
+
+	return resp.Login, nil
+}

github/review.go

@@ -0,0 +1,212 @@
+package github
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/url"
+
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+// ErrCannotDeleteSubmittedReview is returned when DeleteReview is called on
+// a review that has already been submitted (APPROVED, REQUEST_CHANGES, COMMENT).
+// GitHub only allows deletion of PENDING reviews. Callers that need to replace
+// a submitted review should use DismissReview instead.
+var ErrCannotDeleteSubmittedReview = errors.New("cannot delete submitted review: use DismissReview instead")
+
+// ErrConflictingCommitIDs is returned when PostReview receives comments with
+// differing non-empty CommitIDs. The GitHub API accepts only a single commit_id
+// per review submission; callers must ensure all comments target the same commit.
+var ErrConflictingCommitIDs = errors.New("comments contain conflicting commit IDs: all must target the same commit")
+
+// postReviewRequest is the GitHub API request body for creating a review.
+type postReviewRequest struct {
+	CommitID string               `json:"commit_id,omitempty"`
+	Body     string               `json:"body"`
+	Event    string               `json:"event"`
+	Comments []reviewCommentEntry `json:"comments,omitempty"`
+}
+
+// reviewCommentEntry is a single inline comment in a review creation request.
+type reviewCommentEntry struct {
+	Path     string `json:"path"`
+	Position int    `json:"position"`
+	Body     string `json:"body"`
+}
+
+// reviewResponse is the GitHub API response for a review.
+type reviewResponse struct {
+	ID       int64  `json:"id"`
+	Body     string `json:"body"`
+	State    string `json:"state"`
+	CommitID string `json:"commit_id"`
+	User     struct {
+		Login string `json:"login"`
+	} `json:"user"`
+}
+
+// dismissReviewRequest is the GitHub API request body for dismissing a review.
+type dismissReviewRequest struct {
+	Message string `json:"message"`
+	Event   string `json:"event"`
+}
+
+// translateGitHubReviewState translates a GitHub API review state to the
+// canonical vcs.Review.State value.
+func translateGitHubReviewState(state string) string {
+	switch state {
+	case "CHANGES_REQUESTED":
+		return "REQUEST_CHANGES"
+	case "COMMENTED":
+		return "COMMENT"
+	default:
+		// States like APPROVED, DISMISSED, and PENDING pass through unchanged
+		// as they already match the canonical vcs representation. PENDING appears
+		// on draft reviews that have not yet been submitted via the GitHub UI or API.
+		return state
+	}
+}
+
+// PostReview submits a review on a pull request.
+//
+// The vcs.ReviewEvent constants (ReviewEventApprove, ReviewEventRequestChanges,
+// ReviewEventComment) have string values that match GitHub's wire-format event
+// strings (APPROVE, REQUEST_CHANGES, COMMENT), so Event is cast directly to
+// string without translation.
+//
+// ReviewComment.Position maps directly to the GitHub API position field.
+// When req.Comments is empty, the payload omits the comments field entirely
+// (via the omitempty tag on postReviewRequest.Comments).
+//
+// The GitHub API accepts a single commit_id per review submission. PostReview
+// extracts it from the first comment with a non-empty CommitID. If any subsequent
+// comment specifies a different CommitID, PostReview returns ErrConflictingCommitIDs.
+// Comments with an empty CommitID are allowed and inherit the review-level value.
+func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	payload := postReviewRequest{
+		Body:     req.Body,
+		Event:    string(req.Event),
+		CommitID: req.CommitID,
+	}
+
+	// Build the comments in one pass. Inline comment CommitIDs must match the
+	// review-level CommitID; reject if any disagree.
+	for _, comment := range req.Comments {
+		if comment.CommitID != "" {
+			if payload.CommitID == "" {
+				payload.CommitID = comment.CommitID
+			} else if payload.CommitID != comment.CommitID {
+				return nil, ErrConflictingCommitIDs
+			}
+		}
+		payload.Comments = append(payload.Comments, reviewCommentEntry{
+			Path:     comment.Path,
+			Position: comment.Position,
+			Body:     comment.Body,
+		})
+	}
+
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return nil, fmt.Errorf("marshal review request: %w", err)
+	}
+
+	body, err := c.doRequestWithBody(ctx, http.MethodPost, reqURL, data)
+	if err != nil {
+		return nil, fmt.Errorf("post review: %w", err)
+	}
+
+	var resp reviewResponse
+	if err := json.Unmarshal(body, &resp); err != nil {
+		return nil, fmt.Errorf("parse review response: %w", err)
+	}
+
+	return &vcs.Review{
+		ID:       resp.ID,
+		Body:     resp.Body,
+		User:     vcs.UserInfo{Login: resp.User.Login},
+		State:    translateGitHubReviewState(resp.State),
+		CommitID: resp.CommitID,
+	}, nil
+}
+
+// ListReviews retrieves all reviews for a pull request.
+// GitHub review states are translated to canonical vcs values.
+func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcs.Review, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("list reviews: %w", err)
+	}
+
+	var responses []reviewResponse
+	if err := json.Unmarshal(body, &responses); err != nil {
+		return nil, fmt.Errorf("parse reviews response: %w", err)
+	}
+
+	reviews := make([]vcs.Review, len(responses))
+	for i, r := range responses {
+		reviews[i] = vcs.Review{
+			ID:       r.ID,
+			Body:     r.Body,
+			User:     vcs.UserInfo{Login: r.User.Login},
+			State:    translateGitHubReviewState(r.State),
+			CommitID: r.CommitID,
+		}
+	}
+	return reviews, nil
+}
+
+// DeleteReview deletes a pull request review.
+// Only PENDING reviews can be deleted; attempting to delete a submitted review
+// (APPROVED, CHANGES_REQUESTED, or COMMENTED per GitHub API naming) returns
+// ErrCannotDeleteSubmittedReview.
+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 {
+		var apiErr *APIError
+		if errors.As(err, &apiErr) && apiErr.StatusCode == 422 {
+			return fmt.Errorf("delete review: %w", ErrCannotDeleteSubmittedReview)
+		}
+		return fmt.Errorf("delete review: %w", err)
+	}
+	return nil
+}
+
+// DismissReview dismisses a submitted review on a pull request.
+// This is the correct way to "remove" a submitted review (APPROVED, REQUEST_CHANGES).
+// GitHub does not allow deleting submitted reviews — they must be dismissed.
+func (c *Client) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d/dismissals",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID)
+
+	payload := dismissReviewRequest{
+		Message: message,
+		// Event is required by the GitHub API for dismissal requests, even though
+		// "DISMISS" is the only valid value for this endpoint.
+		Event: "DISMISS",
+	}
+
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return fmt.Errorf("marshal dismiss request: %w", err)
+	}
+
+	_, err = c.doRequestWithBody(ctx, http.MethodPut, reqURL, data)
+	if err != nil {
+		return fmt.Errorf("dismiss review: %w", err)
+	}
+	return nil
+}

vcs/types.go

@@ -0,0 +1,78 @@
+// Package vcs defines shared types used across VCS client implementations
+// (gitea, github). Keeping them here prevents the client packages from
+// importing each other or duplicating definitions.
+package vcs
+
+// ReviewEvent is the verdict submitted with a pull request review.
+type ReviewEvent string
+
+const (
+	// ReviewEventApprove approves the pull request.
+	ReviewEventApprove ReviewEvent = "APPROVE"
+
+	// ReviewEventRequestChanges requests changes before the PR can be merged.
+	ReviewEventRequestChanges ReviewEvent = "REQUEST_CHANGES"
+
+	// ReviewEventComment leaves a comment review without explicit approval or rejection.
+	ReviewEventComment ReviewEvent = "COMMENT"
+)
+
+// UserInfo holds the identity of a user.
+type UserInfo struct {
+	Login string
+}
+
+// ReviewComment is a single inline comment attached to a pull request review.
+type ReviewComment struct {
+	// Path is the file path the comment applies to.
+	Path string
+
+	// Position is the line position within the diff (1-indexed).
+	// GitHub and Gitea differ in how they compute position; callers must
+	// supply the correct value for the target VCS.
+	Position int
+
+	// Body is the text content of the comment.
+	Body string
+
+	// CommitID is the SHA of the commit the comment applies to.
+	// For GitHub, all comments within a single review must target the same commit.
+	CommitID string
+}
+
+// ReviewRequest is the payload for submitting a pull request review.
+type ReviewRequest struct {
+	// Body is the top-level review comment body.
+	Body string
+
+	// Event is the review verdict.
+	Event ReviewEvent
+
+	// CommitID is the commit SHA that this review targets.
+	// When non-empty, the review is anchored to this commit.
+	// For GitHub, this is passed as commit_id in the review request.
+	// For Gitea, this is passed as the commitID parameter to PostReview.
+	CommitID string
+
+	// Comments are optional inline file-level comments.
+	Comments []ReviewComment
+}
+
+// Review represents a submitted pull request review.
+type Review struct {
+	// ID is the provider-assigned review identifier.
+	ID int64
+
+	// Body is the top-level review comment body.
+	Body string
+
+	// User is the author of the review.
+	User UserInfo
+
+	// State is the canonical review state string.
+	// Values: APPROVE, REQUEST_CHANGES, COMMENT, DISMISSED, PENDING.
+	State string
+
+	// CommitID is the commit SHA the review was evaluated against.
+	CommitID string
+}