From 5704d167a5d95c4c53f48418fbd4d6d97c84143a Mon Sep 17 00:00:00 2001 From: claw Date: Thu, 14 May 2026 13:45:36 -0700 Subject: [PATCH] feat(#130): implement GitHub API methods and vcs types - Add vcs package with shared review types (ReviewEvent, Review, ReviewRequest, ReviewComment, UserInfo) - Add GetPullRequest, GetPullRequestDiff, GetPullRequestFiles, GetCommitStatuses to github/client.go - Add GetFileContent, GetFileContentRef, ListContents to github/client.go - Add doRequestWithBody helper for POST/PUT/DELETE operations - Add review.go with PostReview, ListReviews, DeleteReview, DismissReview - Add identity.go with GetAuthenticatedUser - Remove TODO comment from github/client.go - Add escapePath helper for URL path construction - Add comprehensive tests for all new methods using httptest.NewServer --- github/api_test.go | 544 +++++++++++++++++++++++++++++++++++++++++++++ github/client.go | 289 +++++++++++++++++++++++- github/identity.go | 29 +++ github/review.go | 212 ++++++++++++++++++ 4 files changed, 1070 insertions(+), 4 deletions(-) create mode 100644 github/api_test.go create mode 100644 github/identity.go create mode 100644 github/review.go diff --git a/github/api_test.go b/github/api_test.go new file mode 100644 index 0000000..2265897 --- /dev/null +++ b/github/api_test.go @@ -0,0 +1,544 @@ +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") + } +} diff --git a/github/client.go b/github/client.go index 227df06..4f6df67 100644 --- a/github/client.go +++ b/github/client.go @@ -4,7 +4,10 @@ package github import ( + "bytes" "context" + "encoding/base64" + "encoding/json" "errors" "fmt" "io" @@ -92,10 +95,6 @@ func asAPIError(err error) (*APIError, bool) { // SetHTTPClient and SetRetryBackoff are intended for test setup only and must // be called before any goroutines issue requests; they have no synchronization. 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 token string httpClient *http.Client @@ -376,3 +375,285 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) { 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 +} diff --git a/github/identity.go b/github/identity.go new file mode 100644 index 0000000..46c26a8 --- /dev/null +++ b/github/identity.go @@ -0,0 +1,29 @@ +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 +} diff --git a/github/review.go b/github/review.go new file mode 100644 index 0000000..1a54fcf --- /dev/null +++ b/github/review.go @@ -0,0 +1,212 @@ +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), + } + + // Build the payload in one pass. The GitHub API accepts a single commit_id + // per review; we extract it from the first comment that supplies one and + // reject the request if any other comment disagrees. + 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 +}