rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

feat(github): implement Reviewer and Identity interfaces (#81) #105

Merged
aweiker merged 7 commits from review-bot-issue-81 into feature/github-support 2026-05-13 13:39:14 +00:00
Conversation 40 Commits 7 Files Changed 7
+757 -16
7 Commits
Author SHA1 Message Date
clawandaweiker 027bad2f7c fix(github): add DismissReview Event comment; use t.Fatalf for routing assertions
PR Ready Gate / clear-labels (pull_request) Successful in 2s
Details
CI / test (pull_request) Successful in 19s
Details
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 33s
Details
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 50s
Details
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m48s
Details 
- Add comment in DismissReview explaining why the Event field is required
  by the GitHub API even though DISMISS is the only valid value (#18652).
- Change t.Errorf to t.Fatalf for method/path routing assertions in test
  handlers so failures are immediately fatal instead of silently
  continuing handler execution (#18653).
 2026-05-13 13:18:46 +00:00
clawandaweiker cd8a1becb3 test(github): use t.Run subtests in TestTranslateGitHubReviewState; doc: note nil body in DeleteReview 2026-05-13 13:18:46 +00:00
clawandaweiker 9dd5e8dbac fix(github): validate conflicting commit IDs and extract test helper 
Address review findings from sonnet-review-bot (review 3086):

- PostReview now returns ErrConflictingCommitIDs when comments specify
  different non-empty CommitIDs, since the GitHub API accepts only a
  single commit_id per review. Previously the discrepancy was silently
  ignored, using only the first commit's ID.

- Extract newTestClient into helpers_test.go to make cross-file sharing
  between review_test.go and identity_test.go explicit.

Refs: #81
 2026-05-13 13:18:46 +00:00
clawandaweiker 8b256360bf fix(github): clarify PostReview doc comment, rename test field to 'want' 
Address review feedback from round-3 sonnet review:
- PostReview doc comment now accurately describes vcs.ReviewEvent → GitHub
  wire-format string cast and notes nil-Comments omitempty behavior.
- Rename 'expected' field to 'want' in TestTranslateGitHubReviewState to
  match the project's established naming convention.
 2026-05-13 13:18:46 +00:00
clawandaweiker 293296b50c address review feedback: wrap ErrCannotDeleteSubmittedReview, fix nits 
- Wrap ErrCannotDeleteSubmittedReview with operation context via fmt.Errorf
  so callers get both sentinel identity and context (MINOR fix)
- Combine double iteration in PostReview into single loop (NIT)
- Remove extra trailing blank line in review_test.go (NIT)
- Clarify translateGitHubReviewState comment re: PENDING state (NIT)
- Update requestOptions.bodyFn comment to mention DELETE-with-body (NIT)
 2026-05-13 13:18:46 +00:00
clawandaweiker eba97321ad refactor(github): extract doRequestCore, address review feedback 
- MAJOR: Extract doRequestCore to eliminate doRequest/doRequestWithBody
  duplication. Both now delegate to a shared implementation with the
  retry/backoff logic in a single place.

- MINOR: Replace custom containsStr/containsSubstring helpers with
  strings.Contains in review_test.go.

- MINOR: Use http.Method* constants (MethodPost, MethodDelete, MethodPut)
  in review.go for consistency with doGet.

- MINOR: Remove redundant APPROVED/DISMISSED cases from
  translateGitHubReviewState that were identical to the default passthrough.

- NIT: Clarify DeleteReview comment about COMMENTED being a GitHub API
  state name.

- DismissReview Event field verified as required by GitHub API docs;
  kept as-is.
 2026-05-13 13:18:46 +00:00
clawandaweiker be3f696a70 feat(github): implement Reviewer and Identity interfaces (#81) 
Implement the remaining vcs.Client interface methods for github.Client:

Reviewer:
- PostReview: POST /repos/{owner}/{repo}/pulls/{number}/reviews
- ListReviews: GET /repos/{owner}/{repo}/pulls/{number}/reviews
  with state translation (CHANGES_REQUESTED → REQUEST_CHANGES, etc.)
- DeleteReview: DELETE /repos/{owner}/{repo}/pulls/{number}/reviews/{id}
  Returns ErrCannotDeleteSubmittedReview on 422
- DismissReview: PUT /repos/{owner}/{repo}/pulls/{number}/reviews/{id}/dismissals

Identity:
- GetAuthenticatedUser: GET /user

Infrastructure:
- Add doRequestWithBody helper for POST/PUT/DELETE with JSON bodies
- Update conformance_test.go: var _ vcs.Client = (*github.Client)(nil)

All unit tests pass including error cases (401, 404, 422, malformed).
 2026-05-13 13:18:46 +00:00