fix: address review feedback on PR #106

- Replace interface{} with any in github/reviews.go (Go 1.18+ idiom)
- Add default panic case to VCS client init switch
- Refactor supersedeOldReviews to return error instead of os.Exit(1)
- Remove spurious blank lines in formatter.go and formatter_test.go
- Add doc comment to DeleteReview explaining when to use vs DismissReview
- Sanitize extractSentinelName output to prevent log injection
This commit is contained in:
claw
2026-05-13 01:51:21 -07:00
parent bee4d98b8c
commit 0ea9c93af6
4 changed files with 29 additions and 10 deletions
+25 -6
View File
@@ -169,6 +169,8 @@ func main() {
ghBaseURL = "https://api.github.com"
}
client = github.NewClient(*reviewerToken, ghBaseURL)
default:
panic("unreachable: unhandled provider " + *provider)
}
slog.Info("VCS client initialized", "provider", *provider)
@@ -498,7 +500,10 @@ func main() {
// Supersede all old reviews
if len(oldReviews) > 0 {
supersedeOldReviews(ctx, client, *provider, *vcsURL, owner, repoName, prNumber, oldReviews, posted.ID, sentinel)
if err := supersedeOldReviews(ctx, client, *provider, *vcsURL, owner, repoName, prNumber, oldReviews, posted.ID, sentinel); err != nil {
slog.Error("failed to supersede old reviews", "error", err)
os.Exit(1)
}
}
}
@@ -517,7 +522,7 @@ func verdictToEvent(verdict string) vcs.ReviewEvent {
// supersedeOldReviews marks old reviews as superseded.
// For GitHub: dismisses old reviews.
// For Gitea: edits the review body and resolves inline comments.
func supersedeOldReviews(ctx context.Context, client vcs.Client, provider, vcsURL, owner, repoName string, prNumber int, oldReviews []vcs.Review, newReviewID int64, sentinel string) {
func supersedeOldReviews(ctx context.Context, client vcs.Client, provider, vcsURL, owner, repoName string, prNumber int, oldReviews []vcs.Review, newReviewID int64, sentinel string) error {
if provider == "github" {
for _, old := range oldReviews {
if err := client.DismissReview(ctx, owner, repoName, prNumber, old.ID, "Superseded by new review"); err != nil {
@@ -526,14 +531,13 @@ func supersedeOldReviews(ctx context.Context, client vcs.Client, provider, vcsUR
slog.Info("dismissed old review", "review_id", old.ID, "new_review_id", newReviewID, "pr", prNumber)
}
}
return
return nil
}
// Gitea: existing EditComment + ResolveComment flow
giteaAdapter, ok := client.(*gitea.Adapter)
if !ok {
slog.Error("expected gitea.Adapter for gitea provider")
os.Exit(1)
return fmt.Errorf("expected gitea.Adapter for gitea provider, got %T", client)
}
underlying := giteaAdapter.Underlying()
@@ -576,6 +580,7 @@ func supersedeOldReviews(ctx context.Context, client vcs.Client, provider, vcsUR
slog.Warn("some inline comments could not be resolved", "review_id", oldReview.ID, "failed", failed, "pr", prNumber)
}
}
return nil
}
// fetchFileContext fetches the full content of modified files from the PR branch.
@@ -855,7 +860,21 @@ func extractSentinelName(body string) string {
if end < 0 {
return "unknown"
}
return rest[:end]
name := rest[:end]
// Sanitize: strip control characters to prevent log injection.
name = strings.Map(func(r rune) rune {
if r < 0x20 || r == 0x7f {
return -1
}
return r
}, name)
if len(name) > 64 {
name = name[:64]
}
if name == "" {
return "unknown"
}
return name
}
// findOwnReview locates the most recent non-superseded review matching the sentinel.