review-bot/github/client_test.go

package github

import (
	"context"
	"encoding/json"
	"errors"
	"fmt"
	"net/http"
	"net/http/httptest"
	"net/url"
	"strings"
	"testing"
	"time"
)

func TestNewClient_DefaultBaseURL(t *testing.T) {
	c := NewClient("tok", "")
	if c.baseURL != defaultBaseURL {
		t.Errorf("baseURL = %q, want %q", c.baseURL, defaultBaseURL)
	}
}

func TestNewClient_CustomBaseURL(t *testing.T) {
	c := NewClient("tok", "https://github.concur.com/api/v3/")
	if c.baseURL != "https://github.concur.com/api/v3" {
		t.Errorf("baseURL = %q, want trailing slash stripped", c.baseURL)
	}
}

func TestDoRequest_Success(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if got := r.Header.Get("Authorization"); got != "Bearer test-token" {
			t.Errorf("Authorization = %q, want Bearer test-token", got)
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte(`{"ok":true}`))
	}))
	defer srv.Close()

	c := NewClient("test-token", srv.URL, AllowInsecureHTTPForTest())
	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != `{"ok":true}` {
		t.Errorf("body = %q, want %q", body, `{"ok":true}`)
	}
}

func TestDoRequest_429_RetryAfter_IntegerSeconds(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", "0")
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
	if attempts != 2 {
		t.Errorf("attempts = %d, want 2", attempts)
	}
}

func TestDoRequest_429_RetryAfter_HTTPDate(t *testing.T) {
	// Fix "now" to a known time for deterministic testing.
	fixedNow := time.Date(2025, 12, 1, 15, 59, 59, 0, time.UTC)
	retryAt := "Mon, 01 Dec 2025 16:00:00 GMT" // 1 second in the future

	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", retryAt)
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	c.now = func() time.Time { return fixedNow }
	// Initial backoff is 0; the HTTP-date parser will compute 1s and override.
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
	if attempts != 2 {
		t.Errorf("attempts = %d, want 2", attempts)
	}
}

func TestDoRequest_429_RetryAfter_HTTPDate_InPast(t *testing.T) {
	// If the HTTP-date is in the past, delay should be 0 (retry immediately).
	fixedNow := time.Date(2025, 12, 1, 17, 0, 0, 0, time.UTC)
	retryAt := "Mon, 01 Dec 2025 16:00:00 GMT" // 1 hour in the past

	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", retryAt)
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	c.now = func() time.Time { return fixedNow }
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
}

func TestDoRequest_429_NoRetryAfter_UsesDefaultBackoff(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
	if attempts != 2 {
		t.Errorf("attempts = %d, want 2", attempts)
	}
}

func TestDoRequest_429_InvalidRetryAfter_UsesDefaultBackoff(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", "not-a-number-or-date")
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
}

func TestDoRequest_404_NoRetry(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		w.WriteHeader(http.StatusNotFound)
		w.Write([]byte("not found"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	_, err := c.doGet(context.Background(), srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error, got nil")
	}
	if !IsNotFound(err) {
		t.Errorf("expected IsNotFound, got %v", err)
	}
	if attempts != 1 {
		t.Errorf("attempts = %d, want 1 (no retry on 404)", attempts)
	}
}

func TestDoRequest_401_NoRetry(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		w.WriteHeader(http.StatusUnauthorized)
		w.Write([]byte("unauthorized"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	_, err := c.doGet(context.Background(), srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error, got nil")
	}
	if !IsUnauthorized(err) {
		t.Errorf("expected IsUnauthorized, got %v", err)
	}
	if attempts != 1 {
		t.Errorf("attempts = %d, want 1 (no retry on 401)", attempts)
	}
}

func TestDoRequest_ContextCanceled(t *testing.T) {
	// This test exercises the timer-cancel path in the retry select:
	//   select { case <-timer.C; case <-ctx.Done() }
	// The server returns 429 with a long Retry-After, and we cancel the
	// context shortly after the first response so that cancellation races
	// against the timer rather than preventing the initial HTTP round-trip.
	requestReceived := make(chan struct{}, 1)
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		select {
		case requestReceived <- struct{}{}:
		default:
		}
		w.Header().Set("Retry-After", "10")
		w.WriteHeader(http.StatusTooManyRequests)
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	c.SetRetryBackoff([]time.Duration{10 * time.Second, 10 * time.Second})

	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	// Cancel the context after the first request completes, while the
	// client is blocked in the retry timer select.
	go func() {
		<-requestReceived
		// Small delay to ensure we're inside the timer select.
		time.Sleep(50 * time.Millisecond)
		cancel()
	}()

	_, err := c.doGet(ctx, srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error, got nil")
	}
	if !errors.Is(err, context.Canceled) {
		t.Errorf("err = %v, want context.Canceled", err)
	}
}

func TestParseRetryAfter_IntegerSeconds(t *testing.T) {
	c := NewClient("tok", "")
	delay, ok := c.parseRetryAfter("42")
	if !ok {
		t.Fatal("expected ok=true")
	}
	if delay != 42*time.Second {
		t.Errorf("delay = %v, want 42s", delay)
	}
}

func TestParseRetryAfter_ZeroSeconds(t *testing.T) {
	c := NewClient("tok", "")
	delay, ok := c.parseRetryAfter("0")
	if !ok {
		t.Fatal("expected ok=true for zero seconds (RFC 7231 allows immediate retry)")
	}
	if delay != 0 {
		t.Errorf("delay = %v, want 0", delay)
	}
}

func TestParseRetryAfter_NegativeSeconds(t *testing.T) {
	c := NewClient("tok", "")
	_, ok := c.parseRetryAfter("-5")
	if ok {
		t.Error("expected ok=false for negative seconds")
	}
}

func TestParseRetryAfter_HTTPDate_Future(t *testing.T) {
	fixedNow := time.Date(2025, 12, 1, 15, 59, 50, 0, time.UTC)
	c := NewClient("tok", "")
	c.now = func() time.Time { return fixedNow }

	delay, ok := c.parseRetryAfter("Mon, 01 Dec 2025 16:00:00 GMT")
	if !ok {
		t.Fatal("expected ok=true")
	}
	// Should be 10 seconds in the future.
	if delay != 10*time.Second {
		t.Errorf("delay = %v, want 10s", delay)
	}
}

func TestParseRetryAfter_HTTPDate_Past(t *testing.T) {
	fixedNow := time.Date(2025, 12, 1, 17, 0, 0, 0, time.UTC)
	c := NewClient("tok", "")
	c.now = func() time.Time { return fixedNow }

	delay, ok := c.parseRetryAfter("Mon, 01 Dec 2025 16:00:00 GMT")
	if !ok {
		t.Fatal("expected ok=true")
	}
	if delay != 0 {
		t.Errorf("delay = %v, want 0 (past date)", delay)
	}
}

func TestParseRetryAfter_RFC850_Format(t *testing.T) {
	fixedNow := time.Date(2025, 12, 1, 15, 59, 50, 0, time.UTC)
	c := NewClient("tok", "")
	c.now = func() time.Time { return fixedNow }

	// RFC 850 format
	delay, ok := c.parseRetryAfter("Monday, 01-Dec-25 16:00:00 GMT")
	if !ok {
		t.Fatal("expected ok=true for RFC 850 format")
	}
	if delay != 10*time.Second {
		t.Errorf("delay = %v, want 10s", delay)
	}
}

func TestParseRetryAfter_Invalid(t *testing.T) {
	c := NewClient("tok", "")
	_, ok := c.parseRetryAfter("not-valid")
	if ok {
		t.Error("expected ok=false for invalid value")
	}
}

func TestParseRetryAfter_EmptyString(t *testing.T) {
	c := NewClient("tok", "")
	_, ok := c.parseRetryAfter("")
	if ok {
		t.Error("expected ok=false for empty string")
	}
}

func TestParseRetryAfter_MaxCap(t *testing.T) {
	// Verify that parseRetryAfter returns the raw value (capping is done by caller).
	c := NewClient("tok", "")
	delay, ok := c.parseRetryAfter("3600")
	if !ok {
		t.Fatal("expected ok=true")
	}
	if delay != 3600*time.Second {
		t.Errorf("delay = %v, want 3600s (caller is responsible for capping)", delay)
	}
}

func TestAPIError_Error_Truncation(t *testing.T) {
	longBody := make([]byte, 300)
	for i := range longBody {
		longBody[i] = 'x'
	}
	apiErr := &APIError{StatusCode: 500, Body: string(longBody)}
	msg := apiErr.Error()
	if len(msg) > 250 {
		// "HTTP 500: " (10) + 200 + "...(truncated)" (14) = 224
		t.Errorf("error message too long: %d chars", len(msg))
	}
}

func TestAPIError_Error_NewlineSanitized(t *testing.T) {
	apiErr := &APIError{StatusCode: 400, Body: "line1\nline2\rline3"}
	msg := apiErr.Error()
	for _, c := range msg {
		if c == '\n' || c == '\r' {
			t.Errorf("error message contains unsanitized newline: %q", msg)
			break
		}
	}
}

func TestNewClient_HasCheckRedirect(t *testing.T) {
	c := NewClient("secret-token", "https://api.github.com")
	if c.httpClient.CheckRedirect == nil {
		t.Fatal("expected CheckRedirect to be set")
	}
}

func TestDefaultCheckRedirect_RejectsHTTPSToHTTP(t *testing.T) {
	prev := &http.Request{URL: &url.URL{Scheme: "https", Host: "api.github.com", Path: "/foo"}}
	req := &http.Request{
		URL:    &url.URL{Scheme: "http", Host: "api.github.com", Path: "/foo"},
		Header: http.Header{"Authorization": []string{"Bearer token"}},
	}
	err := defaultCheckRedirect(req, []*http.Request{prev})
	if err == nil {
		t.Fatal("expected error on HTTPS->HTTP redirect")
	}
	if !strings.Contains(err.Error(), "HTTPS to HTTP downgrade") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestDefaultCheckRedirect_RejectsCrossHost(t *testing.T) {
	prev := &http.Request{URL: &url.URL{Scheme: "https", Host: "api.github.com", Path: "/foo"}}
	req := &http.Request{
		URL:    &url.URL{Scheme: "https", Host: "objects.githubusercontent.com", Path: "/bar"},
		Header: http.Header{"Authorization": []string{"Bearer token"}},
	}
	err := defaultCheckRedirect(req, []*http.Request{prev})
	if err == nil {
		t.Fatal("expected error on cross-host redirect")
	}
	if !strings.Contains(err.Error(), "cross-host") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestDefaultCheckRedirect_AllowsSameHost(t *testing.T) {
	prev := &http.Request{URL: &url.URL{Scheme: "https", Host: "api.github.com", Path: "/foo"}}
	req := &http.Request{
		URL:    &url.URL{Scheme: "https", Host: "api.github.com", Path: "/bar"},
		Header: http.Header{"Authorization": []string{"Bearer token"}},
	}
	err := defaultCheckRedirect(req, []*http.Request{prev})
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	// Auth should be preserved on same-host redirect
	if auth := req.Header.Get("Authorization"); auth != "Bearer token" {
		t.Errorf("expected Authorization to be preserved, got %q", auth)
	}
}

func TestDefaultCheckRedirect_AllowsSameHostHTTPToHTTP(t *testing.T) {
	prev := &http.Request{URL: &url.URL{Scheme: "http", Host: "localhost:8080", Path: "/foo"}}
	req := &http.Request{
		URL:    &url.URL{Scheme: "http", Host: "localhost:8080", Path: "/bar"},
		Header: http.Header{},
	}
	err := defaultCheckRedirect(req, []*http.Request{prev})
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
}

func TestDefaultCheckRedirect_RejectsTooManyRedirects(t *testing.T) {
	via := make([]*http.Request, 10)
	for i := range via {
		via[i] = &http.Request{URL: &url.URL{Scheme: "https", Host: "api.github.com", Path: "/"}}
	}
	req := &http.Request{URL: &url.URL{Scheme: "https", Host: "api.github.com", Path: "/final"}}
	err := defaultCheckRedirect(req, via)
	if err == nil {
		t.Fatal("expected error after 10 redirects")
	}
	if !strings.Contains(err.Error(), "10 redirects") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestDefaultCheckRedirect_EmptyViaAllowed(t *testing.T) {
	req := &http.Request{URL: &url.URL{Scheme: "https", Host: "api.github.com", Path: "/foo"}}
	err := defaultCheckRedirect(req, nil)
	if err != nil {
		t.Fatalf("unexpected error with empty via: %v", err)
	}
}

func TestSetHTTPClient_NilRestoresDefault(t *testing.T) {
	c := NewClient("token", "https://api.github.com")
	c.SetHTTPClient(nil)
	if c.httpClient == nil {
		t.Fatal("expected non-nil httpClient after SetHTTPClient(nil)")
	}
	if c.httpClient.Timeout != 30*time.Second {
		t.Errorf("expected 30s timeout, got %v", c.httpClient.Timeout)
	}
	if c.httpClient.CheckRedirect == nil {
		t.Fatal("expected CheckRedirect policy after SetHTTPClient(nil)")
	}
}

func TestAllowInsecureHTTPForTest_PermitsHTTP(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("ok"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "ok" {
		t.Errorf("body = %q, want %q", body, "ok")
	}
}

func TestNoInsecureOption_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()

	c := NewClient("tok", srv.URL)
	_, err := c.doGet(context.Background(), srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error for HTTP request without AllowInsecureHTTP")
	}
	if !strings.Contains(err.Error(), "refusing HTTP request") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestNoInsecureOption_RejectsUppercaseHTTP(t *testing.T) {
	// Verify case-insensitive scheme check (RFC 3986).
	c := NewClient("tok", "HTTP://127.0.0.1:1")
	_, err := c.doGet(context.Background(), "HTTP://127.0.0.1:1/test")
	if err == nil {
		t.Fatal("expected error for uppercase HTTP scheme")
	}
	if !strings.Contains(err.Error(), "refusing HTTP request") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestNoInsecureOption_RejectsMixedCaseHTTP(t *testing.T) {
	// Verify mixed case like "Http://" is also rejected.
	c := NewClient("tok", "Http://127.0.0.1:1")
	_, err := c.doGet(context.Background(), "Http://127.0.0.1:1/test")
	if err == nil {
		t.Fatal("expected error for mixed-case HTTP scheme")
	}
	if !strings.Contains(err.Error(), "refusing HTTP request") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestAllowInsecureHTTP_WithoutEnvVar_Rejected(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()

	t.Setenv("REVIEW_BOT_ALLOW_INSECURE", "")

	c := NewClient("tok", srv.URL, AllowInsecureHTTP())
	_, err := c.doGet(context.Background(), srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error: AllowInsecureHTTP without env var should be rejected")
	}
	if !strings.Contains(err.Error(), "refusing HTTP request") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestAllowInsecureHTTP_WithEnvVar_Permitted(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("insecure-ok"))
	}))
	defer srv.Close()

	t.Setenv("REVIEW_BOT_ALLOW_INSECURE", "1")

	c := NewClient("tok", srv.URL, AllowInsecureHTTP())
	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "insecure-ok" {
		t.Errorf("body = %q, want %q", body, "insecure-ok")
	}
}

func TestAllowInsecureHTTP_EnvVarNotOne_Rejected(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()

	// "true" is not "1" — strict check
	t.Setenv("REVIEW_BOT_ALLOW_INSECURE", "true")

	c := NewClient("tok", srv.URL, AllowInsecureHTTP())
	_, err := c.doGet(context.Background(), srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error: env var 'true' is not '1'")
	}
	if !strings.Contains(err.Error(), "refusing HTTP request") {
		t.Errorf("unexpected error message: %v", err)
	}
}

func TestRedactURL_WithQuery(t *testing.T) {
	got := redactURL("http://localhost:1234/path?secret=token&foo=bar")
	want := "http://localhost:1234/path?<redacted>"
	if got != want {
		t.Errorf("redactURL = %q, want %q", got, want)
	}
}

func TestRedactURL_NoQuery(t *testing.T) {
	got := redactURL("http://localhost:1234/path")
	want := "http://localhost:1234/path"
	if got != want {
		t.Errorf("redactURL = %q, want %q", got, want)
	}
}

func TestRedactURL_Userinfo(t *testing.T) {
	got := redactURL("http://user:pass@localhost:1234/path")
	want := "http://localhost:1234/path"
	if got != want {
		t.Errorf("redactURL = %q, want %q", got, want)
	}
}

func TestRedactURL_UserinfoWithQuery(t *testing.T) {
	got := redactURL("http://user:pass@localhost:1234/path?secret=token")
	want := "http://localhost:1234/path?<redacted>"
	if got != want {
		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)
		}
	}
}

func TestGetAllFilesInPath_DirectoryWithFiles(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		switch r.URL.Path {
		case "/repos/owner/repo/contents/patterns":
			// Directory listing
			w.WriteHeader(http.StatusOK)
			w.Write([]byte(`[{"name":"go.md","path":"patterns/go.md","type":"file"}]`))
		case "/repos/owner/repo/contents/patterns/go.md":
			// GitHub file response with base64 content
			w.WriteHeader(http.StatusOK)
			w.Write([]byte(`{"name":"go.md","path":"patterns/go.md","type":"file","encoding":"base64","content":"IyBHbyBwYXR0ZXJucwo="}`))
		default:
			t.Errorf("unexpected path: %s", r.URL.Path)
			w.WriteHeader(http.StatusNotFound)
		}
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	files, err := c.GetAllFilesInPath(context.Background(), "owner", "repo", "patterns")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if len(files) != 1 {
		t.Fatalf("len(files) = %d, want 1", len(files))
	}
	if files["patterns/go.md"] != "# Go patterns\n" {
		t.Errorf("unexpected content: %q", files["patterns/go.md"])
	}
}

func TestGetAllFilesInPath_404FallsBackToFile(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		switch r.URL.Path {
		case "/repos/owner/repo/contents/README.md":
			// ListContents returns 404 for file paths
			w.WriteHeader(http.StatusNotFound)
			w.Write([]byte(`{"message":"Not Found"}`))
		default:
			t.Errorf("unexpected path: %s", r.URL.Path)
			w.WriteHeader(http.StatusNotFound)
		}
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	// GetFileContent also goes to /contents/ — this will 404 too.
	// The function should return the path-not-found error.
	_, err := c.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
	if err == nil {
		t.Fatal("expected error when both dir and file 404, got nil")
	}
}

func TestGetAllFilesInPath_DirectoryWithSubdir(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		switch r.URL.Path {
		case "/repos/owner/repo/contents/src":
			w.WriteHeader(http.StatusOK)
			w.Write([]byte(`[
				{"name":"main.go","path":"src/main.go","type":"file"},
				{"name":"sub","path":"src/sub","type":"dir"}
			]`))
		case "/repos/owner/repo/contents/src/main.go":
			w.WriteHeader(http.StatusOK)
			w.Write([]byte(`{"name":"main.go","path":"src/main.go","type":"file","encoding":"base64","content":"cGFja2FnZSBtYWluCg=="}`))
		case "/repos/owner/repo/contents/src/sub":
			w.WriteHeader(http.StatusOK)
			w.Write([]byte(`[{"name":"util.go","path":"src/sub/util.go","type":"file"}]`))
		case "/repos/owner/repo/contents/src/sub/util.go":
			w.WriteHeader(http.StatusOK)
			w.Write([]byte(`{"name":"util.go","path":"src/sub/util.go","type":"file","encoding":"base64","content":"cGFja2FnZSBzdWIK"}`))
		default:
			t.Errorf("unexpected path: %s", r.URL.Path)
			w.WriteHeader(http.StatusNotFound)
		}
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
	files, err := c.GetAllFilesInPath(context.Background(), "owner", "repo", "src")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if len(files) != 2 {
		t.Fatalf("len(files) = %d, want 2: %v", len(files), files)
	}
	if files["src/main.go"] != "package main\n" {
		t.Errorf("src/main.go content unexpected: %q", files["src/main.go"])
	}
	if files["src/sub/util.go"] != "package sub\n" {
		t.Errorf("src/sub/util.go content unexpected: %q", files["src/sub/util.go"])
	}
}