feat: implement GitHub API methods for PR review #130

New Issue

Closed

opened 2026-05-14 20:32:23 +00:00 by rodin · 1 comment

rodin commented 2026-05-14 20:32:23 +00:00

Owner

Copy Link

Copy Source

Problem

The github package has the HTTP transport layer (retry logic, rate limiting, redirect safety, SSRF protection) but no higher-level API methods. The TODO comment in client.go notes that baseURL is populated but not yet used.

Currently, review-bot supports Gitea PRs but cannot review GitHub PRs despite having GitHub Actions runner support via the composite action.

What to implement

Mirror the methods from gitea/client.go for the GitHub API:

• GetPullRequest(ctx, owner, repo, number) → /repos/{owner}/{repo}/pulls/{number}
• GetPullRequestDiff(ctx, owner, repo, number) → /repos/{owner}/{repo}/pulls/{number} with Accept: application/vnd.github.diff
• GetPullRequestFiles(ctx, owner, repo, number) → /repos/{owner}/{repo}/pulls/{number}/files
• GetCommitStatuses(ctx, owner, repo, sha) → /repos/{owner}/{repo}/commits/{sha}/statuses (or /check-runs)
• GetFileContent(ctx, owner, repo, filepath) → /repos/{owner}/{repo}/contents/{path}
• GetFileContentRef(ctx, owner, repo, filepath, ref) → same with ?ref=
• ListContents(ctx, owner, repo, path) → /repos/{owner}/{repo}/contents/{path}
• PostReview(ctx, owner, repo, number, event, body, commitID, comments) → /repos/{owner}/{repo}/pulls/{number}/reviews
• ListReviews(ctx, owner, repo, number) → /repos/{owner}/{repo}/pulls/{number}/reviews
• DeleteReview(ctx, owner, repo, number, reviewID) → DELETE /repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}
• GetAuthenticatedUser(ctx) → /user

Notes

• The action.yml already supports GitHub runners and detects VCS_TYPE=github
• The binary needs a way to route to either gitea.Client or github.Client based on VCS_TYPE
• Both clients should satisfy a common VCSClient interface (defined in cmd/review-bot per Go conventions)
• Review verdict mapping: GitHub uses APPROVE, REQUEST_CHANGES, COMMENT (same as Gitea)

Acceptance Criteria

• All listed API methods implemented in github/client.go
• Each method has a corresponding test using httptest.NewServer
• TODO comment removed from client.go
• cmd/review-bot/main.go routes to either client based on VCS_TYPE env var (or --vcs-type flag)
• Integration test updated/extended for GitHub
• go test ./... passes

## Problem The `github` package has the HTTP transport layer (retry logic, rate limiting, redirect safety, SSRF protection) but no higher-level API methods. The `TODO` comment in `client.go` notes that `baseURL` is populated but not yet used. Currently, review-bot supports Gitea PRs but cannot review GitHub PRs despite having GitHub Actions runner support via the composite action. ## What to implement Mirror the methods from `gitea/client.go` for the GitHub API: - `GetPullRequest(ctx, owner, repo, number)` → `/repos/{owner}/{repo}/pulls/{number}` - `GetPullRequestDiff(ctx, owner, repo, number)` → `/repos/{owner}/{repo}/pulls/{number}` with `Accept: application/vnd.github.diff` - `GetPullRequestFiles(ctx, owner, repo, number)` → `/repos/{owner}/{repo}/pulls/{number}/files` - `GetCommitStatuses(ctx, owner, repo, sha)` → `/repos/{owner}/{repo}/commits/{sha}/statuses` (or `/check-runs`) - `GetFileContent(ctx, owner, repo, filepath)` → `/repos/{owner}/{repo}/contents/{path}` - `GetFileContentRef(ctx, owner, repo, filepath, ref)` → same with `?ref=` - `ListContents(ctx, owner, repo, path)` → `/repos/{owner}/{repo}/contents/{path}` - `PostReview(ctx, owner, repo, number, event, body, commitID, comments)` → `/repos/{owner}/{repo}/pulls/{number}/reviews` - `ListReviews(ctx, owner, repo, number)` → `/repos/{owner}/{repo}/pulls/{number}/reviews` - `DeleteReview(ctx, owner, repo, number, reviewID)` → DELETE `/repos/{owner}/{repo}/pulls/{number}/reviews/{review_id}` - `GetAuthenticatedUser(ctx)` → `/user` ## Notes - The action.yml already supports GitHub runners and detects `VCS_TYPE=github` - The binary needs a way to route to either `gitea.Client` or `github.Client` based on `VCS_TYPE` - Both clients should satisfy a common `VCSClient` interface (defined in `cmd/review-bot` per Go conventions) - Review verdict mapping: GitHub uses `APPROVE`, `REQUEST_CHANGES`, `COMMENT` (same as Gitea) ## Acceptance Criteria - [ ] All listed API methods implemented in `github/client.go` - [ ] Each method has a corresponding test using `httptest.NewServer` - [ ] `TODO` comment removed from `client.go` - [ ] `cmd/review-bot/main.go` routes to either client based on `VCS_TYPE` env var (or `--vcs-type` flag) - [ ] Integration test updated/extended for GitHub - [ ] `go test ./...` passes

rodin self-assigned this 2026-05-14 20:35:50 +00:00

rodin commented 2026-05-14 20:40:56 +00:00

Author

Owner

Copy Link

Copy Source

Plan: feat: implement GitHub API methods for PR review

Problem

The github package has the HTTP transport layer but no higher-level API methods. The review-bot supports Gitea PRs but cannot route to GitHub despite the action.yml supporting GitHub runners. This issue adds the missing API methods to github/client.go and wires them into cmd/review-bot/main.go via a VCS_TYPE flag.

Constraints

• No new third-party dependencies (CONVENTIONS.md strict allowlist)
• All new code must pass go test ./... and go vet ./...
• Each method needs a test using httptest.NewServer
• VCSClient interface defined in cmd/review-bot per Go conventions
• GitHub uses APPROVE, REQUEST_CHANGES, COMMENT — same event names as Gitea

Proposed Approach

Phase 1: Create vcs package with shared types

New package at vcs/types.go with:

• type ReviewEvent string with constants ReviewEventApprove, ReviewEventRequestChanges, ReviewEventComment
• type ReviewComment struct { Path, Body string; Position int; CommitID string }
• type Review struct { ID int64; Body, State, CommitID string; User UserInfo }
• type UserInfo struct { Login string }
• type ReviewRequest struct { Body string; Event ReviewEvent; Comments []ReviewComment }

This package is imported by the github package review methods.

Phase 2: Add doRequestWithBody to github/client.go

New helper method that handles POST/PUT/DELETE with a JSON body. No retry (non-idempotent). Sets Content-Type: application/json. Respects HTTP/SSRF security (blocks HTTP unless AllowInsecureHTTP set).

Phase 3: Add local types to github/client.go

• type PullRequest struct (Title, Body, Head.Sha, Head.Ref)
• type CommitStatus struct (Status, Context, Description, TargetURL)
• type ChangedFile struct (Filename, Status)
• type ContentEntry struct (Name, Path, Type)
• Plus max diff size constant + ErrDiffTooLarge error

Phase 4: Add API methods to github/client.go

• GetPullRequest → GET /repos/{owner}/{repo}/pulls/{number}
• GetPullRequestDiff → same URL with Accept: application/vnd.github.diff
• GetPullRequestFiles → GET /repos/{owner}/{repo}/pulls/{number}/files
• GetCommitStatuses → GET /repos/{owner}/{repo}/commits/{sha}/statuses
• GetFileContent → GET /repos/{owner}/{repo}/contents/{path} (base64 decode response)
• GetFileContentRef → same with ?ref= param
• ListContents → GET /repos/{owner}/{repo}/contents/{path} (returns array)
• Remove TODO comment

Phase 5: Add review.go and identity.go to github package

• review.go: PostReview, ListReviews, DeleteReview, DismissReview using vcs types
• identity.go: GetAuthenticatedUser

Phase 6: Update cmd/review-bot/main.go

Add --vcs-type flag (values: gitea, github; default: gitea for backward compat).
Define VCSClient interface in cmd/review-bot/ using local adapter types.
Extend giteaClientAdapter and add githubClientAdapter to satisfy the interface.
Route based on VCS_TYPE env var or --vcs-type flag.

Open Questions

1. GetCommitStatuses uses GitHub's legacy /statuses endpoint. If github.concur.com uses only check-runs, a separate GetCheckRuns method will be needed. Starting with statuses for now (matches issue spec).
2. ListContents: GitHub returns an object (not array) when path is a file. Will handle both shapes.
3. Integration test: adding a note that the integration test can be run against GitHub by setting VCS_TYPE=github.

Acceptance Criteria

• All listed API methods implemented in github/client.go
• Each method has a corresponding test using httptest.NewServer
• TODO comment removed from client.go
• cmd/review-bot/main.go routes to either client based on VCS_TYPE env var / --vcs-type flag
• Integration test updated/extended for GitHub
• go test ./... passes

## Plan: feat: implement GitHub API methods for PR review ## Problem The `github` package has the HTTP transport layer but no higher-level API methods. The review-bot supports Gitea PRs but cannot route to GitHub despite the action.yml supporting GitHub runners. This issue adds the missing API methods to `github/client.go` and wires them into `cmd/review-bot/main.go` via a `VCS_TYPE` flag. ## Constraints - No new third-party dependencies (CONVENTIONS.md strict allowlist) - All new code must pass `go test ./...` and `go vet ./...` - Each method needs a test using `httptest.NewServer` - VCSClient interface defined in `cmd/review-bot` per Go conventions - GitHub uses `APPROVE`, `REQUEST_CHANGES`, `COMMENT` — same event names as Gitea ## Proposed Approach ### Phase 1: Create `vcs` package with shared types New package at `vcs/types.go` with: - `type ReviewEvent string` with constants `ReviewEventApprove`, `ReviewEventRequestChanges`, `ReviewEventComment` - `type ReviewComment struct { Path, Body string; Position int; CommitID string }` - `type Review struct { ID int64; Body, State, CommitID string; User UserInfo }` - `type UserInfo struct { Login string }` - `type ReviewRequest struct { Body string; Event ReviewEvent; Comments []ReviewComment }` This package is imported by the `github` package review methods. ### Phase 2: Add `doRequestWithBody` to `github/client.go` New helper method that handles POST/PUT/DELETE with a JSON body. No retry (non-idempotent). Sets `Content-Type: application/json`. Respects HTTP/SSRF security (blocks HTTP unless AllowInsecureHTTP set). ### Phase 3: Add local types to `github/client.go` - `type PullRequest struct` (Title, Body, Head.Sha, Head.Ref) - `type CommitStatus struct` (Status, Context, Description, TargetURL) - `type ChangedFile struct` (Filename, Status) - `type ContentEntry struct` (Name, Path, Type) - Plus max diff size constant + `ErrDiffTooLarge` error ### Phase 4: Add API methods to `github/client.go` - `GetPullRequest` → `GET /repos/{owner}/{repo}/pulls/{number}` - `GetPullRequestDiff` → same URL with `Accept: application/vnd.github.diff` - `GetPullRequestFiles` → `GET /repos/{owner}/{repo}/pulls/{number}/files` - `GetCommitStatuses` → `GET /repos/{owner}/{repo}/commits/{sha}/statuses` - `GetFileContent` → `GET /repos/{owner}/{repo}/contents/{path}` (base64 decode response) - `GetFileContentRef` → same with `?ref=` param - `ListContents` → `GET /repos/{owner}/{repo}/contents/{path}` (returns array) - Remove `TODO` comment ### Phase 5: Add `review.go` and `identity.go` to `github` package - `review.go`: `PostReview`, `ListReviews`, `DeleteReview`, `DismissReview` using `vcs` types - `identity.go`: `GetAuthenticatedUser` ### Phase 6: Update `cmd/review-bot/main.go` Add `--vcs-type` flag (values: `gitea`, `github`; default: `gitea` for backward compat). Define `VCSClient` interface in `cmd/review-bot/` using local adapter types. Extend `giteaClientAdapter` and add `githubClientAdapter` to satisfy the interface. Route based on `VCS_TYPE` env var or `--vcs-type` flag. ## Open Questions 1. `GetCommitStatuses` uses GitHub's legacy `/statuses` endpoint. If `github.concur.com` uses only check-runs, a separate `GetCheckRuns` method will be needed. Starting with statuses for now (matches issue spec). 2. `ListContents`: GitHub returns an object (not array) when path is a file. Will handle both shapes. 3. Integration test: adding a note that the integration test can be run against GitHub by setting `VCS_TYPE=github`. ## Acceptance Criteria - [ ] All listed API methods implemented in `github/client.go` - [ ] Each method has a corresponding test using `httptest.NewServer` - [ ] `TODO` comment removed from `client.go` - [ ] `cmd/review-bot/main.go` routes to either client based on `VCS_TYPE` env var / `--vcs-type` flag - [ ] Integration test updated/extended for GitHub - [ ] `go test ./...` passes

rodin referenced this issue from a commit 2026-05-14 20:43:28 +00:00

feat(github): add PR review API methods

rodin referenced this issue from a commit 2026-05-14 20:43:28 +00:00

feat(cmd): add VCS routing for GitHub PR reviews

rodin referenced this issue 2026-05-14 20:44:26 +00:00

feat: implement GitHub API methods and VCS routing (issue #130) #131

rodin referenced a pull request that will close this issue 2026-05-14 20:44:26 +00:00

feat: implement GitHub API methods and VCS routing (issue #130) #131

rodin referenced this issue from a commit 2026-05-14 20:55:19 +00:00

feat(#130): add vcs package with shared review types

rodin referenced this issue from a commit 2026-05-14 20:55:19 +00:00

feat(#130): implement GitHub API methods and vcs types

rodin referenced this issue from a commit 2026-05-14 20:55:19 +00:00

feat(#130): add VCS routing in cmd/review-bot via --vcs-type flag

rodin referenced a pull request that will close this issue 2026-05-14 20:55:50 +00:00

feat: implement GitHub API methods for PR review #132

rodin referenced this issue from a commit 2026-05-14 20:58:04 +00:00

fix(#130): cleanup and test improvements

rodin referenced this issue from a commit 2026-05-14 21:11:20 +00:00

fix(#130): enforce HTTPS scheme consistently in GitHub client write methods

rodin closed this issue 2026-05-14 22:06:21 +00:00

rodin referenced this issue from a commit 2026-05-14 22:06:23 +00:00

feat: implement GitHub API methods and VCS routing (issue #130) (#131)

rodin referenced this issue 2026-05-14 22:16:03 +00:00

PR #131: integration test not extended for GitHub (issue #130 acceptance criterion) #133

rodin referenced this issue 2026-05-14 22:16:13 +00:00

PR #131: githubAPIURL() function has no unit tests #134

rodin referenced this issue 2026-05-14 22:16:23 +00:00

PR #131: cleanEnv() does not strip VCS_TYPE or VCS_URL env vars #135

rodin referenced this issue 2026-05-14 22:22:52 +00:00

feat: implement GitHub API methods for PR review #132

rodin referenced this issue 2026-05-14 22:32:59 +00:00

test: fix cleanEnv VCS_ leak, add githubAPIURL tests, add GitHub integration test #136

rodin referenced this issue from a commit 2026-05-15 00:56:03 +00:00

chore: dev-loop cleanup — remove orphaned untracked files, update TODO

rodin referenced this issue from a commit 2026-05-15 08:59:29 +00:00

fix(#130): pass VCS_TYPE env var from action.yml Run review step

rodin referenced this issue from a commit 2026-05-15 08:59:29 +00:00

docs(#130): fix README CLI example and env var table for VCS-agnostic usage

rodin referenced this issue from a commit 2026-05-15 08:59:29 +00:00

refactor(#130): rename vcsReviewComment.NewPosition to NewLine with clearer semantics

rodin referenced this issue from a commit 2026-05-15 08:59:29 +00:00

refactor(#130): move IsBlockedIP to internal/netutil to remove gitea import in validateurl.go

rodin referenced this issue from a commit 2026-05-15 08:59:29 +00:00

chore(#130): mark self-review findings as addressed in TODO.md

rodin referenced this issue from a commit 2026-05-15 09:10:16 +00:00

fix(#130): pass VCS_TYPE env var from action.yml Run review step

rodin referenced this issue from a commit 2026-05-15 09:10:16 +00:00

docs(#130): fix README CLI example and env var table for VCS-agnostic usage

rodin referenced this issue from a commit 2026-05-15 09:10:16 +00:00

refactor(#130): rename vcsReviewComment.NewPosition to NewLine with clearer semantics

rodin referenced this issue from a commit 2026-05-15 09:10:16 +00:00

refactor(#130): move IsBlockedIP to internal/netutil to remove gitea import in validateurl.go

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

ci/cleanup

ci-selfreview-gate

issue-150

issue-157

issue-141

issue-154

review-bot-dev-loop

issue-143

issue-146

pr-153

review-bot-issue-130-work

issue-148

issue-139

issue-137

review-bot-fixes

review-bot-issue-133

review-bot-issue-130

issue-130

github-support

issue-123-work

issue-123

review-bot-issue-120

fix/125-readme-cli-example

issue-125

issue-124

issue-120

feature/github-support

review-bot-issue-116

review-bot-issue-115

review-bot-issue-114

review-bot-issue-96

review-bot-issue-107

review-bot-issue-82

review-bot-issue-95

review-bot-issue-92

review-bot-issue-94

review-bot-issue-81

review-bot-issue-91

review-bot-issue-97

issue-80-c-file-reader

issue-80-b-pr-reader

issue-80-a-client

review-bot-issue-80

review-bot-issue-87

review-bot-issue-79

review-bot-issue-84

review-bot-issue-78

issue-73

issue-70

issue-68

issue-66

issue-64

issue-60-remote-personas

issue-60

issue-57

allow-deps

feat/aicore-provider-v2

issue-51

ci/pr-ready-gate

fix/stale-commit-check

feat/aicore-provider

fix/response-body-truncation

fix/json-repair

fix/sonnet-reviewer

fix/consistent-path-escape

feat/inline-review-comments

feat/6-update-existing-review

fix/19-context-overflow

feat/18-anthropic-api

fix/url-escaping-and-shadow

fix/quick-wins

fix/context-and-encapsulation

docs/code-review-report

ci/release-workflow

v0.4.0

v0.3.2

v0.3.1

v0.3.0

v0.2.0

v0.1.0

Labels

Clear labels

ai-review

bug

needs-review

ready

Ready for human review

self-reviewed

Self-review passed since last push

wip

Work in progress - do not start new work on this PR

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

security-review-bot rodin (Rodin)

No Assignees rodin

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: rodin/review-bot#130