feat(github): Reviewer + Identity client (Phase 4) #81

New Issue

2026-05-12T16:59:57Z

rodin commented

2026-05-12 16:59:57 +00:00

Architecture Reference

This issue is self-contained. For any design questions, the canonical reference is:
docs/DESIGN-vcs-abstraction.md
(Parts 2, 3, and 4 are most relevant to this issue)

Base branch: feature/github-support — open all PRs against this branch.

Goal

Complete the GitHub client by implementing Reviewer and Identity, making github.Client a full vcs.Client.

Background

The GitHub client struct and HTTP helpers were created in issue #80. This issue adds the remaining interface methods.

The vcs.Client interface and all types live in vcs/ (created in issue #78). The canonical ReviewRequest.Event values are GitHub's strings: "APPROVE", "REQUEST_CHANGES", "COMMENT" — no translation needed for the GitHub client (it sends them directly to the API).

GitHub uses diff-position natively — ReviewComment.Position maps directly to the position field in the POST body. No translation needed.

ReviewComment.CommitID must be the PR head SHA. Callers are responsible for populating it (see issue #82).

Design note: DismissReview vs DeleteReview

GitHub does not allow deleting a submitted review (APPROVED, REQUEST_CHANGES, COMMENT) — only PENDING reviews can be deleted. Attempting to delete a submitted review returns 422.

review-bot replaces old reviews when reposting. The correct cross-platform primitive is DismissReview:

GitHub: PUT /repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}/dismissals
Gitea adapter (issue #79): calls the underlying DeleteReview (Gitea allows full deletion)

DeleteReview stays on the interface for PENDING-only cleanup. Callers that need to replace a submitted review must use DismissReview.

Work

`github/review.go`

PostReview — POST /repos/{owner}/{repo}/pulls/{number}/reviews
- Body: { "commit_id": "...", "body": "...", "event": "APPROVE|REQUEST_CHANGES|COMMENT", "comments": [{"path", "position", "body"}] }
- event values are passed directly — they match GitHub's canonical values

ListReviews — GET /repos/{owner}/{repo}/pulls/{number}/reviews. Map each review to vcs.Review. Canonical State values — GitHub returns native strings that differ from the canonical vcs values; translate in the adapter:

GitHub API `state`	`vcs.Review.State` (canonical)
`"APPROVED"`	`"APPROVED"`
`"CHANGES_REQUESTED"`	`"REQUEST_CHANGES"`
`"COMMENTED"`	`"COMMENT"`
`"DISMISSED"`	`"DISMISSED"`
anything else	pass through as-is

The Gitea adapter must also translate to these same canonical values. Both adapters must agree so that main.go can compare review.State without knowing the provider.

DeleteReview — DELETE /repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}
- Only works for PENDING reviews. On 422, return ErrCannotDeleteSubmittedReview (exported sentinel error defined in github/review.go). Callers in main.go do not need to check for this specifically — the supersede flow uses DismissReview, not DeleteReview, for submitted reviews.
DismissReview — PUT /repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}/dismissals
- Body: {"message": "<message>", "event": "DISMISS"}

`github/identity.go`

GetAuthenticatedUser — GET /user, returns login field

Compile-time check

var _ vcs.Client = (*Client)(nil)

Unit tests

PostReview: happy path, 401, 422, malformed response
ListReviews: happy path, 404, 401
DeleteReview: happy path (PENDING), 422 → ErrCannotDeleteSubmittedReview
DismissReview: happy path, 404, 401
GetAuthenticatedUser: happy path, 401

Exit criteria

var _ vcs.Client = (*github.Client)(nil) compiles
go test ./github/... passes for all methods

Out of scope

CLI flag wiring (issue #82)
Gitea adapter (issue #79)

## Architecture Reference > This issue is self-contained. For any design questions, the canonical reference is: > [`docs/DESIGN-vcs-abstraction.md`](https://gitea.weiker.me/rodin/review-bot/src/branch/feature/github-support/docs/DESIGN-vcs-abstraction.md) > (Parts 2, 3, and 4 are most relevant to this issue) **Base branch:** `feature/github-support` — open all PRs against this branch. --- ## Goal Complete the GitHub client by implementing `Reviewer` and `Identity`, making `github.Client` a full `vcs.Client`. ## Background The GitHub client struct and HTTP helpers were created in issue #80. This issue adds the remaining interface methods. The `vcs.Client` interface and all types live in `vcs/` (created in issue #78). The canonical `ReviewRequest.Event` values are GitHub's strings: `"APPROVE"`, `"REQUEST_CHANGES"`, `"COMMENT"` — no translation needed for the GitHub client (it sends them directly to the API). GitHub uses diff-position natively — `ReviewComment.Position` maps directly to the `position` field in the POST body. No translation needed. `ReviewComment.CommitID` must be the PR head SHA. Callers are responsible for populating it (see issue #82). ## Design note: DismissReview vs DeleteReview GitHub does **not** allow deleting a submitted review (APPROVED, REQUEST_CHANGES, COMMENT) — only PENDING reviews can be deleted. Attempting to delete a submitted review returns 422. review-bot replaces old reviews when reposting. The correct cross-platform primitive is `DismissReview`: - **GitHub**: `PUT /repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}/dismissals` - **Gitea adapter** (issue #79): calls the underlying `DeleteReview` (Gitea allows full deletion) `DeleteReview` stays on the interface for PENDING-only cleanup. Callers that need to replace a submitted review must use `DismissReview`. ## Work ### `github/review.go` - `PostReview` — `POST /repos/{owner}/{repo}/pulls/{number}/reviews` - Body: `{ "commit_id": "...", "body": "...", "event": "APPROVE|REQUEST_CHANGES|COMMENT", "comments": [{"path", "position", "body"}] }` - `event` values are passed directly — they match GitHub's canonical values - `ListReviews` — `GET /repos/{owner}/{repo}/pulls/{number}/reviews`. Map each review to `vcs.Review`. **Canonical `State` values** — GitHub returns native strings that differ from the canonical vcs values; translate in the adapter: | GitHub API `state` | `vcs.Review.State` (canonical) | |--------------------|-------------------------------| | `"APPROVED"` | `"APPROVED"` | | `"CHANGES_REQUESTED"` | `"REQUEST_CHANGES"` | | `"COMMENTED"` | `"COMMENT"` | | `"DISMISSED"` | `"DISMISSED"` | | anything else | pass through as-is | The Gitea adapter must also translate to these same canonical values. Both adapters must agree so that `main.go` can compare `review.State` without knowing the provider. - `DeleteReview` — `DELETE /repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}` - Only works for PENDING reviews. On 422, return `ErrCannotDeleteSubmittedReview` (exported sentinel error defined in `github/review.go`). Callers in `main.go` do not need to check for this specifically — the supersede flow uses `DismissReview`, not `DeleteReview`, for submitted reviews. - `DismissReview` — `PUT /repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}/dismissals` - Body: `{"message": "<message>", "event": "DISMISS"}` ### `github/identity.go` - `GetAuthenticatedUser` — `GET /user`, returns `login` field ### Compile-time check ```go var _ vcs.Client = (*Client)(nil) ``` ### Unit tests - `PostReview`: happy path, 401, 422, malformed response - `ListReviews`: happy path, 404, 401 - `DeleteReview`: happy path (PENDING), 422 → `ErrCannotDeleteSubmittedReview` - `DismissReview`: happy path, 404, 401 - `GetAuthenticatedUser`: happy path, 401 ## Exit criteria - `var _ vcs.Client = (*github.Client)(nil)` compiles - `go test ./github/...` passes for all methods ## Out of scope - CLI flag wiring (issue #82) - Gitea adapter (issue #79)

rodin referenced this issue

2026-05-12 17:01:13 +00:00

feat(github): PRReader + FileReader client (Phase 3) #80

rodin referenced this issue

2026-05-12 17:01:41 +00:00

feat(cmd): wire --provider and --base-url flags into CLI (Phase 5) #82

rodin referenced this issue

2026-05-12 17:16:29 +00:00

Add VCS abstraction layer for GitHub support #76

rodin referenced this issue

2026-05-12 17:24:49 +00:00

feat(vcs): extract interfaces and types from gitea/ (Phase 1) #78