Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| c35b041d5e |
@@ -1,79 +1,42 @@
|
|||||||
## Dev Loop: review-bot — 2026-05-14 20:10 UTC
|
## Dev Loop: review-bot — 2026-05-14 19:15 UTC
|
||||||
|
|
||||||
### Latest: ✅ STABLE STATE — REPO HEALTH COMPLETE
|
### Latest: ✅ issue-123 MERGED
|
||||||
- **Last action:** health check; verified tests pass, repo clean, no action needed
|
- **PR #129:** Merged to main at commit 4440823
|
||||||
- **Repository:** Clean, all merges complete, no open issues/PRs
|
- **Feature:** IP-level SSRF defense with RFC6598 CGN check
|
||||||
- **Main branch:** Up to date with origin/main
|
- **Status:** Complete, all review feedback addressed
|
||||||
- **Test suite:** All passing (cached)
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Repository Status
|
## Review-Bot Current State
|
||||||
|
|
||||||
### ✅ Merged to main (recent):
|
### Merged to main:
|
||||||
- issue-123 (IP-level SSRF defense) — 6 commits, main at 4440823
|
- ✅ issue-123 (SSRF defense) — 5 commits
|
||||||
- issue-125 (VCS_URL rename + deprecation) — merged
|
- ✅ issue-125 (VCS_URL rename + deprecation)
|
||||||
- issue-124 (multi-arch binary support) — merged
|
- ✅ issue-124 (multi-arch binary support)
|
||||||
- issue-120 (GitHub Actions + VCS abstraction) — merged
|
- ✅ issue-120 (GitHub Actions + VCS abstraction) — partial merge
|
||||||
- issue-121 (VCS host type detection for binary download) — merged
|
- And 100+ prior completed issues
|
||||||
|
|
||||||
### 🧹 Cleanup COMPLETE:
|
### Open Branches (>0 commits ahead of main):
|
||||||
- ✅ Removed old worktrees (issue-123, review-bot-issue-125)
|
- **issue-120:** 30 commits ahead (GHE release dry-run + extensive VCS abstraction work)
|
||||||
- ✅ Test suite passes (all packages)
|
- Status: Awaiting human review/decision on integration
|
||||||
- ✅ No TODO/FIXME in code except expected GitHub client notes
|
|
||||||
- ✅ No open issues or pull requests
|
### Completed Recently:
|
||||||
- ✅ Dependencies up to date
|
- Multiple issue-* and review-bot-issue-* branches
|
||||||
|
- All recent work landed in main or merged into feature branches
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Current Feature Completeness
|
## Next Actions for Dev-Loop
|
||||||
|
|
||||||
✅ **Core Capabilities:**
|
1. **Check if issue-120 needs human intervention or PR created**
|
||||||
- Multi-provider LLM support (OpenAI, Anthropic, SAP AI Core)
|
2. **Identify next highest-priority open issue to start**
|
||||||
- Gitea PR integration with structured reviews
|
3. **Review any blocked or stalled branches**
|
||||||
- SSRF defense with IP-level validation
|
4. **Perform health check:**
|
||||||
- VCS abstraction (Gitea/GitHub support)
|
- No orphaned worktrees
|
||||||
- Multi-architecture binary support
|
- No merge conflicts
|
||||||
- GitHub Actions composite action
|
- All tests passing on main
|
||||||
|
|
||||||
✅ **Recent Security Work:**
|
|
||||||
- RFC6598 CGN range detection
|
|
||||||
- IP fallback dialing for local endpoint rejection
|
|
||||||
- URL validation for SSRF prevention
|
|
||||||
|
|
||||||
✅ **Code Quality:**
|
|
||||||
- Comprehensive test coverage (all packages tested)
|
|
||||||
- Consistent error handling with context propagation
|
|
||||||
- Secure credential handling (unexported fields)
|
|
||||||
- Concurrency-safe designs
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Next Priority Actions
|
## Worktrees Active
|
||||||
|
worktrees active
|
||||||
### Phase 2: Feature Exploration (NEXT SESSION)
|
|
||||||
- Scan code for potential improvements per REVIEW.md findings
|
|
||||||
- Assess performance under load
|
|
||||||
- Review REVIEW.md findings for targeted fixes
|
|
||||||
- Consider backlog items from design docs
|
|
||||||
|
|
||||||
### Phase 3: Optional Enhancements (BACKLOG)
|
|
||||||
- Address REVIEW.md context propagation findings (if prioritized)
|
|
||||||
- Additional LLM provider support
|
|
||||||
- Enhanced context detection
|
|
||||||
- Custom report formats
|
|
||||||
- Webhook management improvements
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Worktrees Status
|
|
||||||
All old worktrees cleaned up. Ready for new issue work.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Dev-Loop Metadata
|
|
||||||
- **Repo:** /home/ubuntu/review-bot
|
|
||||||
- **Main branch SHA:** ed3a5dd (last commit)
|
|
||||||
- **Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c
|
|
||||||
- **Scheduled:** Every 4 hours
|
|
||||||
- **Last health check:** 2026-05-14 20:10 UTC (✅ all healthy)
|
|
||||||
|
|||||||
+58
-108
@@ -14,10 +14,8 @@ import (
|
|||||||
|
|
||||||
"gitea.weiker.me/rodin/review-bot/budget"
|
"gitea.weiker.me/rodin/review-bot/budget"
|
||||||
"gitea.weiker.me/rodin/review-bot/gitea"
|
"gitea.weiker.me/rodin/review-bot/gitea"
|
||||||
"gitea.weiker.me/rodin/review-bot/github"
|
|
||||||
"gitea.weiker.me/rodin/review-bot/llm"
|
"gitea.weiker.me/rodin/review-bot/llm"
|
||||||
"gitea.weiker.me/rodin/review-bot/review"
|
"gitea.weiker.me/rodin/review-bot/review"
|
||||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
var version = "dev"
|
var version = "dev"
|
||||||
@@ -86,7 +84,6 @@ func main() {
|
|||||||
systemPromptFile := flag.String("system-prompt-file", envOrDefault("SYSTEM_PROMPT_FILE", ""), "Local file with additional system prompt instructions")
|
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)")
|
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)")
|
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")
|
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)")
|
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
|
||||||
llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
|
llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
|
||||||
@@ -171,19 +168,8 @@ func main() {
|
|||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Initialize VCS client
|
// Initialize clients
|
||||||
var vcsClientImpl vcsClient
|
giteaClient := gitea.NewClient(*vcsURL, *reviewerToken)
|
||||||
switch strings.ToLower(*vcsType) {
|
|
||||||
case "github":
|
|
||||||
vcsClientImpl = newGitHubVCSAdapter(github.NewClient(*reviewerToken, *vcsURL))
|
|
||||||
slog.Info("using GitHub VCS client", "url", *vcsURL)
|
|
||||||
case "gitea", "":
|
|
||||||
vcsClientImpl = newGiteaVCSAdapter(gitea.NewClient(*vcsURL, *reviewerToken))
|
|
||||||
slog.Info("using Gitea VCS client", "url", *vcsURL)
|
|
||||||
default:
|
|
||||||
slog.Error("invalid vcs-type", "type", *vcsType, "valid", "gitea, github")
|
|
||||||
os.Exit(1)
|
|
||||||
}
|
|
||||||
llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel)
|
llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel)
|
||||||
if *llmTemp < 0 || *llmTemp > 2 {
|
if *llmTemp < 0 || *llmTemp > 2 {
|
||||||
slog.Error("invalid LLM temperature", "temperature", *llmTemp, "range", "0-2")
|
slog.Error("invalid LLM temperature", "temperature", *llmTemp, "range", "0-2")
|
||||||
@@ -221,7 +207,7 @@ func main() {
|
|||||||
var persona *review.Persona
|
var persona *review.Persona
|
||||||
if *personaName != "" {
|
if *personaName != "" {
|
||||||
// Try loading from repo first, then fall back to built-in
|
// Try loading from repo first, then fall back to built-in
|
||||||
repoPersonas, err := review.LoadRepoPersonas(ctx, newReviewClientAdapter(vcsClientImpl), owner, repoName)
|
repoPersonas, err := review.LoadRepoPersonas(ctx, newGiteaClientAdapter(giteaClient), owner, repoName)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not load repo personas", "repo", owner+"/"+repoName, "error", err)
|
slog.Warn("could not load repo personas", "repo", owner+"/"+repoName, "error", err)
|
||||||
// Continue with built-in personas only.
|
// Continue with built-in personas only.
|
||||||
@@ -257,7 +243,7 @@ func main() {
|
|||||||
slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName))
|
slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName))
|
||||||
|
|
||||||
// Step 1: Fetch PR metadata
|
// Step 1: Fetch PR metadata
|
||||||
pr, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber)
|
pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Error("failed to fetch PR", "pr", prNumber, "error", err)
|
slog.Error("failed to fetch PR", "pr", prNumber, "error", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
@@ -265,7 +251,7 @@ func main() {
|
|||||||
slog.Info("fetched PR metadata", "pr", prNumber, "title", pr.Title)
|
slog.Info("fetched PR metadata", "pr", prNumber, "title", pr.Title)
|
||||||
|
|
||||||
// Step 2: Fetch diff
|
// Step 2: Fetch diff
|
||||||
diff, err := vcsClientImpl.GetPullRequestDiff(ctx, owner, repoName, prNumber)
|
diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Error("failed to fetch diff", "pr", prNumber, "error", err)
|
slog.Error("failed to fetch diff", "pr", prNumber, "error", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
@@ -274,21 +260,21 @@ func main() {
|
|||||||
|
|
||||||
// Step 3: Fetch full file content for modified files
|
// Step 3: Fetch full file content for modified files
|
||||||
fileContext := ""
|
fileContext := ""
|
||||||
files, err := vcsClientImpl.GetPullRequestFiles(ctx, owner, repoName, prNumber)
|
files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not fetch PR files list", "pr", prNumber, "error", err)
|
slog.Warn("could not fetch PR files list", "pr", prNumber, "error", err)
|
||||||
} else {
|
} else {
|
||||||
fileContext = fetchFileContext(ctx, vcsClientImpl, owner, repoName, pr.HeadRef, files)
|
fileContext = fetchFileContext(ctx, giteaClient, owner, repoName, pr.Head.Ref, files)
|
||||||
slog.Debug("fetched file context", "files", len(files))
|
slog.Debug("fetched file context", "files", len(files))
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 4: Check CI status
|
// Step 4: Check CI status
|
||||||
ciPassed := true
|
ciPassed := true
|
||||||
ciDetails := ""
|
ciDetails := ""
|
||||||
if pr.HeadSha != "" {
|
if pr.Head.Sha != "" {
|
||||||
statuses, err := vcsClientImpl.GetCommitStatuses(ctx, owner, repoName, pr.HeadSha)
|
statuses, err := giteaClient.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not fetch CI status", "sha", pr.HeadSha, "error", err)
|
slog.Warn("could not fetch CI status", "sha", pr.Head.Sha, "error", err)
|
||||||
} else {
|
} else {
|
||||||
ciPassed, ciDetails = evaluateCIStatus(statuses)
|
ciPassed, ciDetails = evaluateCIStatus(statuses)
|
||||||
slog.Info("CI status checked", "passed", ciPassed)
|
slog.Info("CI status checked", "passed", ciPassed)
|
||||||
@@ -298,7 +284,7 @@ func main() {
|
|||||||
// Step 5: Load conventions file if specified
|
// Step 5: Load conventions file if specified
|
||||||
conventions := ""
|
conventions := ""
|
||||||
if *conventionsFile != "" {
|
if *conventionsFile != "" {
|
||||||
content, err := vcsClientImpl.GetFileContent(ctx, owner, repoName, *conventionsFile)
|
content, err := giteaClient.GetFileContent(ctx, owner, repoName, *conventionsFile)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err)
|
slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err)
|
||||||
} else {
|
} else {
|
||||||
@@ -310,7 +296,7 @@ func main() {
|
|||||||
// Step 6: Load patterns from external repo if specified
|
// Step 6: Load patterns from external repo if specified
|
||||||
patterns := ""
|
patterns := ""
|
||||||
if *patternsRepo != "" {
|
if *patternsRepo != "" {
|
||||||
patterns = fetchPatterns(ctx, vcsClientImpl, *patternsRepo, *patternsFiles)
|
patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles)
|
||||||
slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns))
|
slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -403,15 +389,15 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Add commit footer so readers know which commit was evaluated
|
// Add commit footer so readers know which commit was evaluated
|
||||||
if pr.HeadSha != "" {
|
if pr.Head.Sha != "" {
|
||||||
shortSHA := pr.HeadSha
|
shortSHA := pr.Head.Sha
|
||||||
if len(shortSHA) > 8 {
|
if len(shortSHA) > 8 {
|
||||||
shortSHA = shortSHA[:8]
|
shortSHA = shortSHA[:8]
|
||||||
}
|
}
|
||||||
reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
|
reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
|
||||||
}
|
}
|
||||||
|
|
||||||
event := verdictToVCSEvent(result.Verdict)
|
event := review.GiteaEvent(result.Verdict)
|
||||||
|
|
||||||
if *dryRun {
|
if *dryRun {
|
||||||
fmt.Println("--- DRY RUN ---")
|
fmt.Println("--- DRY RUN ---")
|
||||||
@@ -423,14 +409,14 @@ func main() {
|
|||||||
sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
|
sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
|
||||||
|
|
||||||
// Stale check: verify HEAD hasn't moved since we started
|
// Stale check: verify HEAD hasn't moved since we started
|
||||||
evaluatedSHA := pr.HeadSha
|
evaluatedSHA := pr.Head.Sha
|
||||||
var currentSHA string
|
var currentSHA string
|
||||||
currentPR, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber)
|
currentPR, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err)
|
slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err)
|
||||||
// currentSHA stays empty — shouldSkipStaleReview will return false
|
// currentSHA stays empty — shouldSkipStaleReview will return false
|
||||||
} else {
|
} else {
|
||||||
currentSHA = currentPR.HeadSha
|
currentSHA = currentPR.Head.Sha
|
||||||
}
|
}
|
||||||
if shouldSkipStaleReview(evaluatedSHA, currentSHA) {
|
if shouldSkipStaleReview(evaluatedSHA, currentSHA) {
|
||||||
slog.Warn("HEAD moved during review — skipping stale review",
|
slog.Warn("HEAD moved during review — skipping stale review",
|
||||||
@@ -441,30 +427,28 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Map findings to inline comments for lines present in the diff
|
// Map findings to inline comments for lines present in the diff
|
||||||
var inlineComments []vcs.ReviewComment
|
diffRanges := gitea.ParseDiffNewLines(diff)
|
||||||
if ext, ok := vcsClientImpl.(giteaExtendedClient); ok {
|
var inlineComments []gitea.ReviewComment
|
||||||
diffRanges := ext.ParseDiffNewLines(diff)
|
for _, f := range result.Findings {
|
||||||
for _, f := range result.Findings {
|
if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
|
||||||
if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
|
inlineComments = append(inlineComments, gitea.ReviewComment{
|
||||||
inlineComments = append(inlineComments, vcs.ReviewComment{
|
Path: f.File,
|
||||||
Path: f.File,
|
NewPosition: int64(f.Line),
|
||||||
Position: f.Line,
|
Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
|
||||||
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 ---
|
// --- Review update strategy ---
|
||||||
// 1. POST new review first (gets non-stale approval badge on HEAD)
|
// 1. POST new review first (gets non-stale approval badge on HEAD)
|
||||||
// 2. Then supersede old review with link to the new one
|
// 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.
|
// Order matters: post first so we have the new review's URL for the supersede message.
|
||||||
var oldReviews []reviewInfo
|
var oldReviews []gitea.Review
|
||||||
if *reviewerName != "" {
|
if *reviewerName != "" {
|
||||||
existingReviews, err := vcsClientImpl.ListReviews(ctx, owner, repoName, prNumber)
|
existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
|
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
|
||||||
} else {
|
} else {
|
||||||
@@ -477,27 +461,20 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Self-request as reviewer (ensures we appear in required-reviewer checks)
|
// Self-request as reviewer (ensures we appear in required-reviewer checks)
|
||||||
authUser, err := vcsClientImpl.GetAuthenticatedUser(ctx)
|
authUser, err := giteaClient.GetAuthenticatedUser(ctx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
|
slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
|
||||||
} else if authUser != "" {
|
} else if authUser != "" {
|
||||||
if ext, ok := vcsClientImpl.(giteaExtendedClient); ok {
|
if err := giteaClient.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
|
||||||
if err := ext.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
|
slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
|
||||||
slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
|
} else {
|
||||||
} else {
|
slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
|
||||||
slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// POST new review
|
// POST new review
|
||||||
slog.Info("posting review", "event", event, "pr", prNumber)
|
slog.Info("posting review", "event", event, "pr", prNumber)
|
||||||
posted, err := vcsClientImpl.PostReview(ctx, owner, repoName, prNumber, vcs.ReviewRequest{
|
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, evaluatedSHA, inlineComments)
|
||||||
Body: reviewBody,
|
|
||||||
Event: event,
|
|
||||||
CommitID: evaluatedSHA,
|
|
||||||
Comments: inlineComments,
|
|
||||||
})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
|
slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
@@ -507,26 +484,21 @@ func main() {
|
|||||||
// Supersede all old reviews with link to the new one
|
// Supersede all old reviews with link to the new one
|
||||||
if len(oldReviews) > 0 {
|
if len(oldReviews) > 0 {
|
||||||
newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*vcsURL, "/"), owner, repoName, prNumber, posted.ID)
|
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 {
|
for _, oldReview := range oldReviews {
|
||||||
if !hasExt {
|
cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
|
||||||
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 {
|
if err != nil {
|
||||||
slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
|
slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
|
supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
|
||||||
if err := ext.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
|
if err := giteaClient.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)
|
slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)
|
slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)
|
||||||
|
|
||||||
// Resolve old review's inline comments
|
// Resolve old review's inline comments
|
||||||
oldComments, err := ext.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
|
oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
|
slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
|
||||||
continue
|
continue
|
||||||
@@ -536,7 +508,7 @@ func main() {
|
|||||||
if c.ID == 0 {
|
if c.ID == 0 {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if err := ext.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
|
if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
|
||||||
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
|
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
|
||||||
failed++
|
failed++
|
||||||
} else {
|
} else {
|
||||||
@@ -555,7 +527,7 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// fetchFileContext fetches the full content of modified files from the PR branch.
|
// fetchFileContext fetches the full content of modified files from the PR branch.
|
||||||
func fetchFileContext(ctx context.Context, client vcsClient, owner, repo, ref string, files []changedFileInfo) string {
|
func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, ref string, files []gitea.ChangedFile) string {
|
||||||
var sb strings.Builder
|
var sb strings.Builder
|
||||||
for _, f := range files {
|
for _, f := range files {
|
||||||
if ctx.Err() != nil {
|
if ctx.Err() != nil {
|
||||||
@@ -582,7 +554,7 @@ func fetchFileContext(ctx context.Context, client vcsClient, owner, repo, ref st
|
|||||||
// patternsFiles is comma-separated list of file paths or directories.
|
// patternsFiles is comma-separated list of file paths or directories.
|
||||||
// If a path ends with / or is a directory, all files within it are fetched recursively.
|
// If a path ends with / or is a directory, all files within it are fetched recursively.
|
||||||
// If patternsFiles is empty, all files from the repo root are fetched.
|
// If patternsFiles is empty, all files from the repo root are fetched.
|
||||||
func fetchPatterns(ctx context.Context, client vcsClient, patternsRepo, patternsFiles string) string {
|
func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patternsFiles string) string {
|
||||||
var sb strings.Builder
|
var sb strings.Builder
|
||||||
|
|
||||||
repos := strings.Split(patternsRepo, ",")
|
repos := strings.Split(patternsRepo, ",")
|
||||||
@@ -619,13 +591,8 @@ func fetchPatterns(ctx context.Context, client vcsClient, patternsRepo, patterns
|
|||||||
var repoLoadedFiles []string
|
var repoLoadedFiles []string
|
||||||
var repoSkippedFiles []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 {
|
for _, path := range paths {
|
||||||
files, err := giteaRaw.client.GetAllFilesInPath(ctx, owner, repo, path)
|
files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err)
|
slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err)
|
||||||
continue
|
continue
|
||||||
@@ -664,7 +631,7 @@ func isPatternFile(path string) bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// evaluateCIStatus checks if all CI statuses indicate success.
|
// evaluateCIStatus checks if all CI statuses indicate success.
|
||||||
func evaluateCIStatus(statuses []commitStatusInfo) (passed bool, details string) {
|
func evaluateCIStatus(statuses []gitea.CommitStatus) (passed bool, details string) {
|
||||||
if len(statuses) == 0 {
|
if len(statuses) == 0 {
|
||||||
return true, "no CI statuses found"
|
return true, "no CI statuses found"
|
||||||
}
|
}
|
||||||
@@ -802,7 +769,7 @@ func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string)
|
|||||||
// Gitea user. This indicates misconfiguration where two roles share a token
|
// Gitea user. This indicates misconfiguration where two roles share a token
|
||||||
// instead of having separate Gitea accounts. Returns true if shared token
|
// instead of having separate Gitea accounts. Returns true if shared token
|
||||||
// detected (caller should skip update-in-place logic to avoid clobbering).
|
// detected (caller should skip update-in-place logic to avoid clobbering).
|
||||||
func hasSharedToken(reviews []reviewInfo, ownSentinel string) bool {
|
func hasSharedToken(reviews []gitea.Review, ownSentinel string) bool {
|
||||||
ownLogin := ""
|
ownLogin := ""
|
||||||
for _, r := range reviews {
|
for _, r := range reviews {
|
||||||
if strings.Contains(r.Body, ownSentinel) {
|
if strings.Contains(r.Body, ownSentinel) {
|
||||||
@@ -840,8 +807,8 @@ func extractSentinelName(body string) string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// findOwnReview locates the most recent non-superseded review matching the sentinel.
|
// findOwnReview locates the most recent non-superseded review matching the sentinel.
|
||||||
func findOwnReview(reviews []reviewInfo, sentinel string) *reviewInfo {
|
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
|
||||||
var best *reviewInfo
|
var best *gitea.Review
|
||||||
for i := range reviews {
|
for i := range reviews {
|
||||||
if !strings.Contains(reviews[i].Body, sentinel) {
|
if !strings.Contains(reviews[i].Body, sentinel) {
|
||||||
continue
|
continue
|
||||||
@@ -857,8 +824,8 @@ func findOwnReview(reviews []reviewInfo, sentinel string) *reviewInfo {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// findAllOwnReviews returns all non-superseded reviews matching the sentinel.
|
// findAllOwnReviews returns all non-superseded reviews matching the sentinel.
|
||||||
func findAllOwnReviews(reviews []reviewInfo, sentinel string) []reviewInfo {
|
func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
|
||||||
var result []reviewInfo
|
var result []gitea.Review
|
||||||
for i := range reviews {
|
for i := range reviews {
|
||||||
if !strings.Contains(reviews[i].Body, sentinel) {
|
if !strings.Contains(reviews[i].Body, sentinel) {
|
||||||
continue
|
continue
|
||||||
@@ -871,22 +838,6 @@ func findAllOwnReviews(reviews []reviewInfo, sentinel string) []reviewInfo {
|
|||||||
return result
|
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.
|
// shouldSkipStaleReview reports whether to skip posting because HEAD moved.
|
||||||
// Returns true (skip) if evaluatedSHA differs from currentSHA.
|
// Returns true (skip) if evaluatedSHA differs from currentSHA.
|
||||||
// Returns false (don't skip) if:
|
// Returns false (don't skip) if:
|
||||||
@@ -900,17 +851,16 @@ func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool {
|
|||||||
return evaluatedSHA != currentSHA
|
return evaluatedSHA != currentSHA
|
||||||
}
|
}
|
||||||
|
|
||||||
// reviewClientAdapter adapts a vcsClient to review.GiteaClient for persona loading.
|
// giteaClientAdapter adapts gitea.Client to review.GiteaClient interface.
|
||||||
// The review package only needs ListContents and GetFileContent, which all vcsClients provide.
|
type giteaClientAdapter struct {
|
||||||
type reviewClientAdapter struct {
|
client *gitea.Client
|
||||||
client vcsClient
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func newReviewClientAdapter(c vcsClient) *reviewClientAdapter {
|
func newGiteaClientAdapter(c *gitea.Client) *giteaClientAdapter {
|
||||||
return &reviewClientAdapter{client: c}
|
return &giteaClientAdapter{client: c}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (a *reviewClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) {
|
func (a *giteaClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) {
|
||||||
entries, err := a.client.ListContents(ctx, owner, repo, path)
|
entries, err := a.client.ListContents(ctx, owner, repo, path)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
@@ -926,6 +876,6 @@ func (a *reviewClientAdapter) ListContents(ctx context.Context, owner, repo, pat
|
|||||||
return result, nil
|
return result, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (a *reviewClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
|
func (a *giteaClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
|
||||||
return a.client.GetFileContent(ctx, owner, repo, filepath)
|
return a.client.GetFileContent(ctx, owner, repo, filepath)
|
||||||
}
|
}
|
||||||
|
|||||||
+32
-32
@@ -10,7 +10,7 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
"gitea.weiker.me/rodin/review-bot/gitea"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestValidateReviewerName(t *testing.T) {
|
func TestValidateReviewerName(t *testing.T) {
|
||||||
@@ -154,14 +154,14 @@ func TestValidateWorkspacePath(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func makeReview(id int64, login, state string, stale bool, body string) reviewInfo {
|
func makeReview(id int64, login, state string, stale bool, body string) gitea.Review {
|
||||||
r := reviewInfo{
|
r := gitea.Review{
|
||||||
ID: id,
|
ID: id,
|
||||||
Body: body,
|
Body: body,
|
||||||
State: state,
|
State: state,
|
||||||
Stale: stale,
|
Stale: stale,
|
||||||
}
|
}
|
||||||
r.User = vcs.UserInfo{Login: login}
|
r.User.Login = login
|
||||||
return r
|
return r
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -216,7 +216,7 @@ func TestBuildSupersededBodyShortSHA(t *testing.T) {
|
|||||||
func TestFindOwnReview(t *testing.T) {
|
func TestFindOwnReview(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
reviews []reviewInfo
|
reviews []gitea.Review
|
||||||
sentinel string
|
sentinel string
|
||||||
wantID int64
|
wantID int64
|
||||||
wantNil bool
|
wantNil bool
|
||||||
@@ -229,7 +229,7 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "found by sentinel",
|
name: "found by sentinel",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
makeReview(42, "bot", "APPROVED", false, "review body\n<!-- review-bot:sonnet -->"),
|
makeReview(42, "bot", "APPROVED", false, "review body\n<!-- review-bot:sonnet -->"),
|
||||||
},
|
},
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
@@ -237,7 +237,7 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "wrong sentinel",
|
name: "wrong sentinel",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
makeReview(42, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
|
makeReview(42, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
|
||||||
},
|
},
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
@@ -245,7 +245,7 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "multiple reviews, returns first match",
|
name: "multiple reviews, returns first match",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
makeReview(10, "bot", "APPROVED", false, "old\n<!-- review-bot:gpt -->"),
|
makeReview(10, "bot", "APPROVED", false, "old\n<!-- review-bot:gpt -->"),
|
||||||
makeReview(20, "bot", "APPROVED", false, "new\n<!-- review-bot:sonnet -->"),
|
makeReview(20, "bot", "APPROVED", false, "new\n<!-- review-bot:sonnet -->"),
|
||||||
},
|
},
|
||||||
@@ -254,7 +254,7 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "skips superseded review",
|
name: "skips superseded review",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
|
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 -->"),
|
makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
|
||||||
},
|
},
|
||||||
@@ -263,7 +263,7 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "only superseded reviews exist",
|
name: "only superseded reviews exist",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
|
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
|
||||||
},
|
},
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
@@ -271,7 +271,7 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "picks highest ID among matches",
|
name: "picks highest ID among matches",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
|
makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
|
||||||
makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
|
makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
|
||||||
},
|
},
|
||||||
@@ -302,7 +302,7 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
func TestHasSharedToken(t *testing.T) {
|
func TestHasSharedToken(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
reviews []reviewInfo
|
reviews []gitea.Review
|
||||||
sentinel string
|
sentinel string
|
||||||
want bool
|
want bool
|
||||||
}{
|
}{
|
||||||
@@ -314,36 +314,36 @@ func TestHasSharedToken(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "no own review yet - cannot detect",
|
name: "no own review yet - cannot detect",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
{ID: 1, User: vcs.UserInfo{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
|
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
|
||||||
},
|
},
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
want: false,
|
want: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "separate users - no shared token",
|
name: "separate users - no shared token",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
{ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
|
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
|
||||||
{ID: 2, User: vcs.UserInfo{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
|
{ID: 2, User: struct{ Login string `json:"login"` }{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
|
||||||
},
|
},
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
want: false,
|
want: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "shared token detected - same user different sentinels",
|
name: "shared token detected - same user different sentinels",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
{ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
|
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
|
||||||
{ID: 2, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
|
{ID: 2, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
|
||||||
},
|
},
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
want: true,
|
want: true,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "three roles same user",
|
name: "three roles same user",
|
||||||
reviews: []reviewInfo{
|
reviews: []gitea.Review{
|
||||||
{ID: 1, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
|
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
|
||||||
{ID: 2, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
|
{ID: 2, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
|
||||||
{ID: 3, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
|
{ID: 3, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
|
||||||
},
|
},
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
want: true,
|
want: true,
|
||||||
@@ -553,7 +553,7 @@ func TestBuildPatternPaths(t *testing.T) {
|
|||||||
func TestEvaluateCIStatus(t *testing.T) {
|
func TestEvaluateCIStatus(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
statuses []commitStatusInfo
|
statuses []gitea.CommitStatus
|
||||||
wantPassed bool
|
wantPassed bool
|
||||||
wantSubstr string
|
wantSubstr string
|
||||||
}{
|
}{
|
||||||
@@ -565,7 +565,7 @@ func TestEvaluateCIStatus(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "all success",
|
name: "all success",
|
||||||
statuses: []commitStatusInfo{
|
statuses: []gitea.CommitStatus{
|
||||||
{Status: "success", Context: "ci/build", Description: "Build passed"},
|
{Status: "success", Context: "ci/build", Description: "Build passed"},
|
||||||
{Status: "success", Context: "ci/test", Description: "Tests passed"},
|
{Status: "success", Context: "ci/test", Description: "Tests passed"},
|
||||||
},
|
},
|
||||||
@@ -574,7 +574,7 @@ func TestEvaluateCIStatus(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "one failure",
|
name: "one failure",
|
||||||
statuses: []commitStatusInfo{
|
statuses: []gitea.CommitStatus{
|
||||||
{Status: "success", Context: "ci/build", Description: "Build passed"},
|
{Status: "success", Context: "ci/build", Description: "Build passed"},
|
||||||
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
|
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
|
||||||
},
|
},
|
||||||
@@ -583,7 +583,7 @@ func TestEvaluateCIStatus(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "error status",
|
name: "error status",
|
||||||
statuses: []commitStatusInfo{
|
statuses: []gitea.CommitStatus{
|
||||||
{Status: "error", Context: "ci/lint", Description: "Lint error"},
|
{Status: "error", Context: "ci/lint", Description: "Lint error"},
|
||||||
},
|
},
|
||||||
wantPassed: false,
|
wantPassed: false,
|
||||||
@@ -591,7 +591,7 @@ func TestEvaluateCIStatus(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "pending treated as not-failed",
|
name: "pending treated as not-failed",
|
||||||
statuses: []commitStatusInfo{
|
statuses: []gitea.CommitStatus{
|
||||||
{Status: "pending", Context: "ci/build", Description: "In progress"},
|
{Status: "pending", Context: "ci/build", Description: "In progress"},
|
||||||
{Status: "success", Context: "ci/test", Description: "Tests passed"},
|
{Status: "success", Context: "ci/test", Description: "Tests passed"},
|
||||||
},
|
},
|
||||||
@@ -600,7 +600,7 @@ func TestEvaluateCIStatus(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "multiple failures",
|
name: "multiple failures",
|
||||||
statuses: []commitStatusInfo{
|
statuses: []gitea.CommitStatus{
|
||||||
{Status: "failure", Context: "ci/build", Description: "Build failed"},
|
{Status: "failure", Context: "ci/build", Description: "Build failed"},
|
||||||
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
|
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
|
||||||
},
|
},
|
||||||
@@ -609,7 +609,7 @@ func TestEvaluateCIStatus(t *testing.T) {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "mixed with pending and failure",
|
name: "mixed with pending and failure",
|
||||||
statuses: []commitStatusInfo{
|
statuses: []gitea.CommitStatus{
|
||||||
{Status: "success", Context: "ci/build", Description: "Build passed"},
|
{Status: "success", Context: "ci/build", Description: "Build passed"},
|
||||||
{Status: "pending", Context: "ci/deploy", Description: "Deploying"},
|
{Status: "pending", Context: "ci/deploy", Description: "Deploying"},
|
||||||
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
|
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
|
||||||
@@ -997,7 +997,7 @@ func cleanEnv() []string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestFindAllOwnReviews(t *testing.T) {
|
func TestFindAllOwnReviews(t *testing.T) {
|
||||||
reviews := []reviewInfo{
|
reviews := []gitea.Review{
|
||||||
{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
|
{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
|
||||||
{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
|
{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
|
||||||
{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},
|
{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},
|
||||||
|
|||||||
@@ -1,405 +0,0 @@
|
|||||||
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)
|
|
||||||
}
|
|
||||||
@@ -1,582 +0,0 @@
|
|||||||
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)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
+4
-283
@@ -4,10 +4,7 @@
|
|||||||
package github
|
package github
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
|
||||||
"context"
|
"context"
|
||||||
"encoding/base64"
|
|
||||||
"encoding/json"
|
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
@@ -95,6 +92,10 @@ func asAPIError(err error) (*APIError, bool) {
|
|||||||
// SetHTTPClient and SetRetryBackoff are intended for test setup only and must
|
// SetHTTPClient and SetRetryBackoff are intended for test setup only and must
|
||||||
// be called before any goroutines issue requests; they have no synchronization.
|
// be called before any goroutines issue requests; they have no synchronization.
|
||||||
type Client struct {
|
type Client struct {
|
||||||
|
// TODO: baseURL is populated by NewClient but not yet consumed by doRequest/doGet.
|
||||||
|
// Higher-level exported methods (GetPullRequest, etc.) will use it to
|
||||||
|
// construct request URLs; remove this field if those methods end up
|
||||||
|
// accepting full URLs instead.
|
||||||
baseURL string
|
baseURL string
|
||||||
token string
|
token string
|
||||||
httpClient *http.Client
|
httpClient *http.Client
|
||||||
@@ -375,283 +376,3 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st
|
|||||||
func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
|
func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
|
||||||
return c.doRequest(ctx, http.MethodGet, url, "")
|
return c.doRequest(ctx, http.MethodGet, url, "")
|
||||||
}
|
}
|
||||||
|
|
||||||
// DefaultMaxDiffSize is the default maximum diff size in bytes (10 MB).
|
|
||||||
const DefaultMaxDiffSize = 10 * 1024 * 1024
|
|
||||||
|
|
||||||
// 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 {
|
|
||||||
Title string `json:"title"`
|
|
||||||
Body string `json:"body"`
|
|
||||||
Head struct {
|
|
||||||
Sha string `json:"sha"`
|
|
||||||
Ref string `json:"ref"`
|
|
||||||
} `json:"head"`
|
|
||||||
}
|
|
||||||
|
|
||||||
// CommitStatus represents a single CI status entry.
|
|
||||||
type CommitStatus struct {
|
|
||||||
Status string `json:"state"`
|
|
||||||
Context string `json:"context"`
|
|
||||||
Description string `json:"description"`
|
|
||||||
TargetURL string `json:"target_url"`
|
|
||||||
}
|
|
||||||
|
|
||||||
// ChangedFile represents a file modified in a PR.
|
|
||||||
type ChangedFile struct {
|
|
||||||
Filename string `json:"filename"`
|
|
||||||
Status string `json:"status"`
|
|
||||||
}
|
|
||||||
|
|
||||||
// ContentEntry represents a file or directory in a repository.
|
|
||||||
type ContentEntry struct {
|
|
||||||
Name string `json:"name"`
|
|
||||||
Path string `json:"path"`
|
|
||||||
Type string `json:"type"`
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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 response: %w", err)
|
|
||||||
}
|
|
||||||
return &pr, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// GetPullRequestFiles fetches the list of files changed in a PR.
|
|
||||||
func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
|
|
||||||
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)
|
|
||||||
}
|
|
||||||
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.
|
|
||||||
func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) {
|
|
||||||
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)
|
|
||||||
}
|
|
||||||
var statuses []CommitStatus
|
|
||||||
if err := json.Unmarshal(body, &statuses); err != nil {
|
|
||||||
return nil, fmt.Errorf("parse commit statuses response: %w", err)
|
|
||||||
}
|
|
||||||
return statuses, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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 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.GetFileContentRef(ctx, owner, repo, filepath, "")
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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) {
|
|
||||||
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 content %q: %w", filepath, err)
|
|
||||||
}
|
|
||||||
var resp contentResponse
|
|
||||||
if err := json.Unmarshal(body, &resp); err != nil {
|
|
||||||
return "", fmt.Errorf("parse file content response: %w", err)
|
|
||||||
}
|
|
||||||
content, err := decodeFileContent(resp)
|
|
||||||
if err != nil {
|
|
||||||
return "", fmt.Errorf("decode file content %q: %w", filepath, err)
|
|
||||||
}
|
|
||||||
return content, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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) {
|
|
||||||
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 %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 {
|
|
||||||
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
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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("parse request URL: %w", err)
|
|
||||||
}
|
|
||||||
if strings.EqualFold(parsed.Scheme, "http") {
|
|
||||||
return nil, fmt.Errorf("refusing HTTP request to %s: use HTTPS or set AllowInsecureHTTP option", redactURL(reqURL))
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
var 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)}
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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 {
|
|
||||||
parts[i] = url.PathEscape(part)
|
|
||||||
}
|
|
||||||
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
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -1,29 +0,0 @@
|
|||||||
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
|
|
||||||
}
|
|
||||||
@@ -1,212 +0,0 @@
|
|||||||
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
|
|
||||||
}
|
|
||||||
@@ -1,78 +0,0 @@
|
|||||||
// 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
|
|
||||||
}
|
|
||||||
Reference in New Issue
Block a user