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

title

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,457 @@ 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, "")
 }
+
+// doRequestWithBody performs an HTTP request with an optional body, applying the
+// same HTTPS enforcement as doRequest. It is used by write methods (POST, PUT,
+// DELETE) that bypass the retry loop in doRequest because write operations are
+// not idempotent.
+//
+// body may be nil for requests that carry no payload (e.g. DELETE).
+// When body is non-nil, Content-Type is set to application/json.
+func (c *Client) doRequestWithBody(ctx context.Context, method, reqURL string, body []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 reqBody io.Reader
+	if body != nil {
+		reqBody = bytes.NewReader(body)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, method, reqURL, reqBody)
+	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 body != 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)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
+		respBody, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodyBytes))
+		if err != nil {
+			return nil, fmt.Errorf("read response body: %w", err)
+		}
+		return respBody, nil
+	}
+
+	errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
+	return nil, &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
+}
+
+// --- API types ---
+
+// 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"`
+	Draft bool `json:"draft"`
+}
+
+// CommitStatus represents a single CI status entry.
+// GitHub returns "state" not "status"; this type uses Status for consistency
+// with the gitea package (both are normalized before use).
+type CommitStatus struct {
+	Status      string `json:"state"` // GitHub field is "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"`
+}
+
+// ReviewComment represents an inline comment to attach to a review.
+// GitHub uses "position" (diff hunk position), whereas Gitea uses "new_position" (line number).
+// When posting inline comments on GitHub, position is required; line numbers
+// from the diff cannot be used directly.
+type ReviewComment struct {
+	ID       int64  `json:"id,omitempty"`
+	Path     string `json:"path"`
+	Position int64  `json:"position,omitempty"`   // GitHub diff hunk position
+	Line     int64  `json:"line,omitempty"`        // GitHub absolute line number (alternative to position)
+	Side     string `json:"side,omitempty"`        // "RIGHT" or "LEFT"
+	Body     string `json:"body"`
+}
+
+// Review represents a pull request review from the GitHub API.
+type Review struct {
+	ID   int64  `json:"id"`
+	Body string `json:"body"`
+	User struct {
+		Login string `json:"login"`
+	} `json:"user"`
+	State string `json:"state"`
+}
+
+// contentResponse is the GitHub contents API response for a single file.
+type contentResponse struct {
+	Name     string `json:"name"`
+	Path     string `json:"path"`
+	Type     string `json:"type"`     // "file" or "dir" or "symlink" or "submodule"
+	Content  string `json:"content"`  // Base64-encoded file content (with embedded newlines)
+	Encoding string `json:"encoding"` // "base64" or ""
+}
+
+// ContentEntry represents a file or directory entry from the contents API.
+type ContentEntry struct {
+	Name string `json:"name"`
+	Path string `json:"path"`
+	Type string `json:"type"` // "file" or "dir"
+}
+
+// --- PR methods ---
+
+// 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 JSON: %w", err)
+	}
+	return &pr, nil
+}
+
+// GetPullRequestDiff fetches the unified diff for a PR.
+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)
+	body, err := c.doRequest(ctx, http.MethodGet, reqURL, "application/vnd.github.diff")
+	if err != nil {
+		return "", fmt.Errorf("fetch diff: %w", err)
+	}
+	return string(body), nil
+}
+
+// GetPullRequestFiles fetches the list of files changed in a PR.
+// GitHub paginates this endpoint (100 per page max).
+func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
+	const perPage = 100
+	var all []ChangedFile
+	for page := 1; ; page++ {
+		reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/files?per_page=%d&page=%d",
+			c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, perPage, page)
+		body, err := c.doGet(ctx, reqURL)
+		if err != nil {
+			return nil, fmt.Errorf("fetch PR files (page %d): %w", page, err)
+		}
+		var batch []ChangedFile
+		if err := json.Unmarshal(body, &batch); err != nil {
+			return nil, fmt.Errorf("parse PR files JSON (page %d): %w", page, err)
+		}
+		all = append(all, batch...)
+		if len(batch) < perPage {
+			break
+		}
+	}
+	return all, nil
+}
+
+// GetCommitStatuses fetches CI statuses for a commit SHA.
+// GitHub has two status systems: legacy "commit statuses" and newer "check runs".
+// This method returns commit statuses only; check runs are a separate API.
+// Note: GitHub returns "state" in the JSON; CommitStatus.Status is tagged accordingly.
+func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) {
+	const perPage = 100
+	var all []CommitStatus
+	for page := 1; ; page++ {
+		reqURL := fmt.Sprintf("%s/repos/%s/%s/commits/%s/statuses?per_page=%d&page=%d",
+			c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha), perPage, page)
+		body, err := c.doGet(ctx, reqURL)
+		if err != nil {
+			return nil, fmt.Errorf("fetch commit statuses (page %d): %w", page, err)
+		}
+		var batch []CommitStatus
+		if err := json.Unmarshal(body, &batch); err != nil {
+			return nil, fmt.Errorf("parse statuses JSON (page %d): %w", page, err)
+		}
+		all = append(all, batch...)
+		if len(batch) < perPage {
+			break
+		}
+	}
+	return all, nil
+}
+
+// --- File content methods ---
+
+// GetFileContent fetches a file from the default branch of a repo.
+// GitHub returns base64-encoded content; this method decodes it.
+func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+	return c.getFileContentAtRef(ctx, owner, repo, filepath, "")
+}
+
+// GetFileContentRef fetches a file from a specific ref (branch/tag/sha).
+func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
+	return c.getFileContentAtRef(ctx, owner, repo, filepath, ref)
+}
+
+// getFileContentAtRef fetches a file at the given ref (empty = default branch).
+// GitHub's contents API returns base64-encoded file content.
+func (c *Client) getFileContentAtRef(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 %s: %w", filepath, err)
+	}
+	var resp contentResponse
+	if err := json.Unmarshal(body, &resp); err != nil {
+		return "", fmt.Errorf("parse file content JSON for %s: %w", filepath, err)
+	}
+	if resp.Type != "file" {
+		return "", fmt.Errorf("path %s is a %s, not a file", filepath, resp.Type)
+	}
+	if resp.Encoding == "base64" {
+		// GitHub embeds newlines in the base64 content for readability.
+		// Strip them before decoding.
+		cleaned := strings.ReplaceAll(resp.Content, "\n", "")
+		decoded, err := base64.StdEncoding.DecodeString(cleaned)
+		if err != nil {
+			return "", fmt.Errorf("decode base64 content for %s: %w", filepath, err)
+		}
+		return string(decoded), nil
+	}
+	// Non-base64 encoding (shouldn't happen normally, but handle gracefully).
+	return resp.Content, nil
+}
+
+// ListContents lists files and directories at a given path.
+// Pass an empty path to list the repository root.
+// GitHub returns a single object (not array) when path is a file — this
+// method normalizes both cases to a slice, matching Gitea's behavior.
+func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
+	var reqURL string
+	if path == "" || path == "." {
+		reqURL = fmt.Sprintf("%s/repos/%s/%s/contents",
+			c.baseURL, url.PathEscape(owner), url.PathEscape(repo))
+	} else {
+		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 %s: %w", path, err)
+	}
+
+	var entries []ContentEntry
+	if err := json.Unmarshal(body, &entries); err != nil {
+		// GitHub returns a single object when path is a file (not an array).
+		var single contentResponse
+		if err2 := json.Unmarshal(body, &single); err2 != nil {
+			return nil, fmt.Errorf("parse contents JSON: %w", err)
+		}
+		if single.Name == "" && single.Path == "" {
+			return nil, fmt.Errorf("parse contents JSON: empty response for path %q", path)
+		}
+		entries = []ContentEntry{{
+			Name: single.Name,
+			Path: single.Path,
+			Type: single.Type,
+		}}
+	}
+	return entries, nil
+}
+
+// GetAllFilesInPath recursively fetches all file contents under a path.
+// If the path is a file, returns just that file's content.
+// If the path is a directory, recursively fetches all files within it.
+func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) {
+	results := make(map[string]string)
+
+	entries, err := c.ListContents(ctx, owner, repo, path)
+	if err != nil {
+		if !IsNotFound(err) {
+			return nil, fmt.Errorf("list contents %q: %w", path, err)
+		}
+		// 404 means path may be a file — try fetching directly.
+		content, fileErr := c.GetFileContent(ctx, owner, repo, path)
+		if fileErr != nil {
+			return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, fileErr)
+		}
+		results[path] = content
+		return results, nil
+	}
+
+	for _, entry := range entries {
+		switch entry.Type {
+		case "file":
+			content, err := c.GetFileContent(ctx, owner, repo, entry.Path)
+			if err != nil {
+				slog.Warn("could not fetch file from patterns repo", "file", entry.Path, "error", err)
+				continue
+			}
+			results[entry.Path] = content
+		case "dir":
+			subResults, err := c.GetAllFilesInPath(ctx, owner, repo, entry.Path)
+			if err != nil {
+				slog.Warn("could not recurse into directory", "dir", entry.Path, "error", err)
+				continue
+			}
+			for k, v := range subResults {
+				results[k] = v
+			}
+		}
+	}
+	return results, nil
+}
+
+// --- Review methods ---
+
+// PostReview submits a review to a PR.
+// event should be one of "APPROVE", "REQUEST_CHANGES", or "COMMENT".
+// commitID anchors the review to a specific commit SHA. If empty, defaults to current HEAD.
+// comments are optional inline comments; GitHub uses diff hunk position (not line numbers).
+// Note: unlike Gitea, GitHub does not support deleting submitted reviews.
+// Use COMMENT event to supersede old reviews.
+func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []ReviewComment) (*Review, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	payload := struct {
+		Body     string          `json:"body"`
+		Event    string          `json:"event"`
+		CommitID string          `json:"commit_id,omitempty"`
+		Comments []ReviewComment `json:"comments,omitempty"`
+	}{
+		Body:     body,
+		Event:    event,
+		CommitID: commitID,
+		Comments: comments,
+	}
+
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return nil, fmt.Errorf("marshal review payload: %w", err)
+	}
+
+	respBody, err := c.doRequestWithBody(ctx, http.MethodPost, reqURL, data)
+	if err != nil {
+		return nil, fmt.Errorf("post review: %w", err)
+	}
+
+	var review Review
+	if err := json.Unmarshal(respBody, &review); err != nil {
+		return nil, fmt.Errorf("parse review response: %w", err)
+	}
+	return &review, nil
+}
+
+// ListReviews returns all reviews on a pull request.
+// GitHub paginates via Link header; this method uses per_page=100.
+func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error) {
+	const perPage = 100
+	var all []Review
+	for page := 1; ; page++ {
+		reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews?per_page=%d&page=%d",
+			c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, perPage, page)
+		body, err := c.doGet(ctx, reqURL)
+		if err != nil {
+			return nil, fmt.Errorf("list reviews (page %d): %w", page, err)
+		}
+		var batch []Review
+		if err := json.Unmarshal(body, &batch); err != nil {
+			return nil, fmt.Errorf("parse reviews (page %d): %w", page, err)
+		}
+		all = append(all, batch...)
+		if len(batch) < perPage {
+			break
+		}
+	}
+	return all, nil
+}
+
+// DeleteReview attempts to delete a pull request review.
+// GitHub only allows deleting PENDING (draft) reviews. Submitted reviews cannot
+// be deleted via the API; this method returns a descriptive error in that case.
+// review-bot callers should handle this error gracefully (e.g., by not attempting
+// supersede and instead posting a new review alongside the old one).
+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 {
+		return fmt.Errorf("delete review: %w", err)
+	}
+	return nil
+}
+
+// GetAuthenticatedUser returns the login of the authenticated user.
+func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	reqURL := c.baseURL + "/user"
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return "", fmt.Errorf("get authenticated user: %w", err)
+	}
+	var result struct {
+		Login string `json:"login"`
+	}
+	if err := json.Unmarshal(body, &result); err != nil {
+		return "", fmt.Errorf("parse user response: %w", err)
+	}
+	return result.Login, nil
+}
+
+// RequestReviewer adds a user as a requested reviewer on a pull request.
+// This is idempotent — requesting an already-requested reviewer is a no-op.
+func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/requested_reviewers",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	payload := struct {
+		Reviewers []string `json:"reviewers"`
+	}{Reviewers: []string{reviewer}}
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return fmt.Errorf("marshal reviewer request: %w", err)
+	}
+
+	_, err = c.doRequestWithBody(ctx, http.MethodPost, reqURL, data)
+	if err != nil {
+		return fmt.Errorf("request reviewer: %w", err)
+	}
+	return nil
+}
+
+// --- helpers ---
+
+// escapePath escapes each segment of a relative file path for use in URLs.
+// Slashes are preserved as path separators; other special characters are escaped.
+func escapePath(p string) string {
+	parts := strings.Split(p, "/")
+	for i, part := range parts {
+		parts[i] = url.PathEscape(part)
+	}
+	return strings.Join(parts, "/")
+}

github/client_test.go

@@ -2,7 +2,9 @@ package github
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -656,3 +658,475 @@ func TestRedactURL_UserinfoWithQuery(t *testing.T) {
 		t.Errorf("redactURL = %q, want %q", got, want)
 	}
 }
+
+// --- Tests for API methods ---
+
+func TestGetPullRequest_Success(t *testing.T) {
+	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.StatusOK)
+		w.Write([]byte(`{"title":"Test PR","body":"description","head":{"sha":"abc123","ref":"feature"},"draft":false}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	pr, err := c.GetPullRequest(context.Background(), "owner", "repo", 42)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if pr.Title != "Test PR" {
+		t.Errorf("Title = %q, want %q", pr.Title, "Test PR")
+	}
+	if pr.Head.Sha != "abc123" {
+		t.Errorf("Head.Sha = %q, want %q", pr.Head.Sha, "abc123")
+	}
+	if pr.Head.Ref != "feature" {
+		t.Errorf("Head.Ref = %q, want %q", pr.Head.Ref, "feature")
+	}
+}
+
+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(`{"message":"Not Found"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	_, err := c.GetPullRequest(context.Background(), "owner", "repo", 99)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if !IsNotFound(err) {
+		t.Errorf("expected IsNotFound=true, got false for error: %v", err)
+	}
+}
+
+func TestGetPullRequestDiff_Success(t *testing.T) {
+	const wantDiff = "diff --git a/foo.go b/foo.go\n--- a/foo.go\n+++ b/foo.go\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.WriteHeader(http.StatusOK)
+		w.Write([]byte(wantDiff))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	diff, err := c.GetPullRequestDiff(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if diff != wantDiff {
+		t.Errorf("diff = %q, want %q", diff, wantDiff)
+	}
+}
+
+func TestGetPullRequestFiles_Success(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`[{"filename":"foo.go","status":"modified"},{"filename":"bar.go","status":"added"}]`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	files, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(files) != 2 {
+		t.Fatalf("len(files) = %d, want 2", len(files))
+	}
+	if files[0].Filename != "foo.go" || files[0].Status != "modified" {
+		t.Errorf("files[0] = %+v, want {foo.go modified}", files[0])
+	}
+}
+
+func TestGetPullRequestFiles_Paginated(t *testing.T) {
+	page := 0
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		page++
+		if page == 1 {
+			// Return 100 items (page full → expect another request)
+			items := make([]map[string]string, 100)
+			for i := range items {
+				items[i] = map[string]string{"filename": fmt.Sprintf("file%d.go", i), "status": "modified"}
+			}
+			data, _ := json.Marshal(items)
+			w.WriteHeader(http.StatusOK)
+			w.Write(data)
+			return
+		}
+		// Page 2: return fewer than perPage → stop
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`[{"filename":"last.go","status":"added"}]`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	files, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(files) != 101 {
+		t.Errorf("len(files) = %d, want 101", len(files))
+	}
+	if page != 2 {
+		t.Errorf("page = %d, want 2", page)
+	}
+}
+
+func TestGetCommitStatuses_Success(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		// GitHub uses "state" field
+		w.Write([]byte(`[{"state":"success","context":"ci/test","description":"Tests pass","target_url":"https://ci.example.com"}]`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	statuses, err := c.GetCommitStatuses(context.Background(), "owner", "repo", "deadbeef")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(statuses) != 1 {
+		t.Fatalf("len(statuses) = %d, want 1", len(statuses))
+	}
+	if statuses[0].Status != "success" {
+		t.Errorf("Status = %q, want %q", statuses[0].Status, "success")
+	}
+	if statuses[0].Context != "ci/test" {
+		t.Errorf("Context = %q, want %q", statuses[0].Context, "ci/test")
+	}
+}
+
+func TestGetFileContent_Base64(t *testing.T) {
+	// "hello world\n" base64-encoded with embedded newlines (as GitHub does it)
+	encoded := "aGVsbG8gd29ybGQK"
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !strings.HasSuffix(r.URL.Path, "/contents/README.md") {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+		}
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"name":"README.md","path":"README.md","type":"file","content":"` + encoded + `","encoding":"base64"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	content, err := c.GetFileContent(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if content != "hello world\n" {
+		t.Errorf("content = %q, want %q", content, "hello world\n")
+	}
+}
+
+func TestGetFileContent_Base64WithNewlines(t *testing.T) {
+	// GitHub embeds newlines in base64 content for readability (every 60 chars)
+	// Test that we strip them correctly before decoding
+	// "hello world\n" = aGVsbG8gd29ybGQK — split it with embedded \n
+	encoded := "aGVs\nbG8g\nd29y\nbGQK"
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		// JSON-encode the embedded newlines as \n
+		body := `{"name":"README.md","path":"README.md","type":"file","content":"aGVs\nbG8g\nd29y\nbGQK","encoding":"base64"}`
+		_ = encoded // suppress unused warning
+		w.Write([]byte(body))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	content, err := c.GetFileContent(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if content != "hello world\n" {
+		t.Errorf("content = %q, want %q", content, "hello world\n")
+	}
+}
+
+func TestGetFileContent_IsDirectory(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"name":"docs","path":"docs","type":"dir","content":"","encoding":""}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	_, err := c.GetFileContent(context.Background(), "owner", "repo", "docs")
+	if err == nil {
+		t.Fatal("expected error for directory, got nil")
+	}
+}
+
+func TestGetFileContentRef_Success(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Query().Get("ref") != "main" {
+			t.Errorf("ref = %q, want %q", r.URL.Query().Get("ref"), "main")
+		}
+		encoded := "dGVzdA==" // "test"
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"name":"foo.go","path":"foo.go","type":"file","content":"` + encoded + `","encoding":"base64"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	content, err := c.GetFileContentRef(context.Background(), "owner", "repo", "foo.go", "main")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if content != "test" {
+		t.Errorf("content = %q, want %q", content, "test")
+	}
+}
+
+func TestListContents_Directory(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`[{"name":"foo.go","path":"foo.go","type":"file"},{"name":"bar","path":"bar","type":"dir"}]`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	entries, err := c.ListContents(context.Background(), "owner", "repo", "")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(entries) != 2 {
+		t.Fatalf("len(entries) = %d, want 2", len(entries))
+	}
+	if entries[0].Name != "foo.go" || entries[0].Type != "file" {
+		t.Errorf("entries[0] = %+v, unexpected", entries[0])
+	}
+}
+
+func TestListContents_SingleFile(t *testing.T) {
+	// GitHub returns a single object when the path is a file
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"name":"README.md","path":"README.md","type":"file","content":"","encoding":""}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	entries, err := c.ListContents(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(entries) != 1 {
+		t.Fatalf("len(entries) = %d, want 1", len(entries))
+	}
+	if entries[0].Name != "README.md" {
+		t.Errorf("entries[0].Name = %q, want README.md", entries[0].Name)
+	}
+}
+
+func TestPostReview_Success(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/1/reviews" {
+			t.Errorf("path = %s, unexpected", r.URL.Path)
+		}
+		var payload map[string]interface{}
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+		}
+		if payload["event"] != "APPROVE" {
+			t.Errorf("event = %v, want APPROVE", payload["event"])
+		}
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"id":99,"body":"looks good","user":{"login":"bot"},"state":"APPROVED"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	review, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "looks good", "abc", nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if review.ID != 99 {
+		t.Errorf("review.ID = %d, want 99", review.ID)
+	}
+	if review.User.Login != "bot" {
+		t.Errorf("review.User.Login = %q, want bot", review.User.Login)
+	}
+}
+
+func TestPostReview_Unauthorized(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+		w.Write([]byte(`{"message":"Bad credentials"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("bad-tok", srv.URL, AllowInsecureHTTPForTest())
+	_, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "body", "", nil)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if !IsUnauthorized(err) {
+		t.Errorf("expected IsUnauthorized=true, got false for error: %v", err)
+	}
+}
+
+func TestListReviews_Success(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`[{"id":1,"body":"review 1","user":{"login":"alice"},"state":"APPROVED"},{"id":2,"body":"review 2","user":{"login":"bob"},"state":"CHANGES_REQUESTED"}]`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	reviews, err := c.ListReviews(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(reviews) != 2 {
+		t.Fatalf("len(reviews) = %d, want 2", len(reviews))
+	}
+	if reviews[0].ID != 1 || reviews[0].User.Login != "alice" {
+		t.Errorf("reviews[0] = %+v, unexpected", reviews[0])
+	}
+}
+
+func TestDeleteReview_Success(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("path = %s, unexpected", r.URL.Path)
+		}
+		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("unexpected error: %v", err)
+	}
+}
+
+func TestDeleteReview_SubmittedReview(t *testing.T) {
+	// GitHub returns 422 for trying to delete a non-pending review
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		w.Write([]byte(`{"message":"Can only delete a pending review"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	err := c.DeleteReview(context.Background(), "owner", "repo", 1, 99)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+}
+
+func TestGetAuthenticatedUser_Success(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/user" {
+			t.Errorf("path = %s, want /user", r.URL.Path)
+		}
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"login":"review-bot","id":12345}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	login, err := c.GetAuthenticatedUser(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if login != "review-bot" {
+		t.Errorf("login = %q, want review-bot", login)
+	}
+}
+
+func TestRequestReviewer_Success(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/1/requested_reviewers" {
+			t.Errorf("path = %s, unexpected", r.URL.Path)
+		}
+		var payload map[string]interface{}
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+		}
+		reviewers, ok := payload["reviewers"].([]interface{})
+		if !ok || len(reviewers) != 1 || reviewers[0] != "reviewer1" {
+			t.Errorf("reviewers = %v, unexpected", payload["reviewers"])
+		}
+		w.WriteHeader(http.StatusCreated)
+		w.Write([]byte(`{}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	err := c.RequestReviewer(context.Background(), "owner", "repo", 1, "reviewer1")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestPostReview_RejectsHTTP(t *testing.T) {
+	// PostReview must reject http:// base URLs — tokens must not be sent in plaintext.
+	c := NewClient("tok", "http://127.0.0.1:1")
+	_, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "body", "", nil)
+	if err == nil {
+		t.Fatal("expected error for HTTP base URL in PostReview")
+	}
+	if !strings.Contains(err.Error(), "refusing HTTP request") {
+		t.Errorf("unexpected error message: %v", err)
+	}
+}
+
+func TestDeleteReview_RejectsHTTP(t *testing.T) {
+	// DeleteReview must reject http:// base URLs — tokens must not be sent in plaintext.
+	c := NewClient("tok", "http://127.0.0.1:1")
+	err := c.DeleteReview(context.Background(), "owner", "repo", 1, 42)
+	if err == nil {
+		t.Fatal("expected error for HTTP base URL in DeleteReview")
+	}
+	if !strings.Contains(err.Error(), "refusing HTTP request") {
+		t.Errorf("unexpected error message: %v", err)
+	}
+}
+
+func TestRequestReviewer_RejectsHTTP(t *testing.T) {
+	// RequestReviewer must reject http:// base URLs — tokens must not be sent in plaintext.
+	c := NewClient("tok", "http://127.0.0.1:1")
+	err := c.RequestReviewer(context.Background(), "owner", "repo", 1, "reviewer1")
+	if err == nil {
+		t.Fatal("expected error for HTTP base URL in RequestReviewer")
+	}
+	if !strings.Contains(err.Error(), "refusing HTTP request") {
+		t.Errorf("unexpected error message: %v", err)
+	}
+}
+
+func TestEscapePath_SpecialChars(t *testing.T) {
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"README.md", "README.md"},
+		{"docs/guide.md", "docs/guide.md"},
+		{"path with spaces/file.md", "path%20with%20spaces/file.md"},
+		{"path/with [brackets]/file.md", "path/with%20%5Bbrackets%5D/file.md"},
+	}
+	for _, tt := range tests {
+		got := escapePath(tt.input)
+		if got != tt.want {
+			t.Errorf("escapePath(%q) = %q, want %q", tt.input, got, tt.want)
+		}
+	}
+}