test(forge): cover Check, auth errors and the PR, issue and commit-review fetchers

internal/analyze/forge/auth_check_test.go

@@ -0,0 +1,185 @@
+package forge
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/google/go-github/v66/github"
+	"github.com/stretchr/testify/require"
+)
+
+// fakeGitHub serves the three endpoints Check calls, as a GitHub Enterprise
+// host would (under /api/v3/), each answering with the status it is given.
+type fakeGitHub struct {
+	userStatus, repoStatus, rateStatus int
+	calls                              []string
+}
+
+func (f *fakeGitHub) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	f.calls = append(f.calls, r.URL.Path)
+	w.Header().Set("Content-Type", "application/json")
+	reply := func(status int, body string) {
+		w.WriteHeader(status)
+		if status == http.StatusOK {
+			_, _ = fmt.Fprint(w, body)
+			return
+		}
+		_, _ = fmt.Fprintf(w, `{"message":"%s"}`, http.StatusText(status))
+	}
+	switch r.URL.Path {
+	case "/api/v3/user":
+		reply(f.userStatus, `{"login":"octocat"}`)
+	case "/api/v3/repos/acme/widgets":
+		reply(f.repoStatus, `{"name":"widgets","owner":{"login":"acme"}}`)
+	case "/api/v3/rate_limit":
+		reply(f.rateStatus, `{"resources":{"core":{"limit":5000,"remaining":4321,"reset":1767225600}}}`)
+	default:
+		http.NotFound(w, r)
+	}
+}
+
+// startFakeGitHub serves f over TLS and returns the host to pass to Check or NewClient. The
+// client Check builds uses http.DefaultTransport, which is pointed at the test
+// server's trusted transport for the duration of the test.
+func startFakeGitHub(t *testing.T, f http.Handler) string {
+	t.Helper()
+	srv := httptest.NewTLSServer(f)
+	t.Cleanup(srv.Close)
+	prev := http.DefaultTransport
+	http.DefaultTransport = srv.Client().Transport
+	t.Cleanup(func() { http.DefaultTransport = prev })
+	u, err := url.Parse(srv.URL)
+	require.NoError(t, err)
+	return u.Host
+}
+
+var testCreds = &Credentials{Token: "t0ken", Source: "GH_TOKEN"}
+var testRepo = Repo{Owner: "acme", Name: "widgets"}
+
+func TestCheck_UserTokenWithRepoAccess(t *testing.T) {
+	host := startFakeGitHub(t, &fakeGitHub{userStatus: 200, repoStatus: 200, rateStatus: 200})
+
+	res, err := Check(context.Background(), testCreds, host, testRepo)
+	require.NoError(t, err)
+	require.Equal(t, "octocat", res.Login)
+	require.Equal(t, "GH_TOKEN", res.Source)
+	require.Equal(t, 4321, res.RateLimitRemaining)
+	require.Equal(t, 5000, res.RateLimitLimit)
+	require.Equal(t, time.Unix(1767225600, 0).UTC(), res.RateLimitResetsAt.UTC())
+}
+
+func TestCheck_UserRejectedWithoutRepoIsATokenError(t *testing.T) {
+	f := &fakeGitHub{userStatus: 401, repoStatus: 200, rateStatus: 200}
+	host := startFakeGitHub(t, f)
+
+	_, err := Check(context.Background(), testCreds, host, Repo{})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "GitHub rejected the token from GH_TOKEN")
+	require.NotContains(t, f.calls, "/api/v3/repos/acme/widgets", "with no repository there is nothing else to try")
+}
+
+// GitHub Actions' GITHUB_TOKEN cannot read /user; reading the repository is
+// the validity check, and the result has no login.
+func TestCheck_RepositoryScopedToken(t *testing.T) {
+	host := startFakeGitHub(t, &fakeGitHub{userStatus: 403, repoStatus: 200, rateStatus: 200})
+
+	res, err := Check(context.Background(), testCreds, host, testRepo)
+	require.NoError(t, err)
+	require.Empty(t, res.Login)
+	require.Equal(t, 4321, res.RateLimitRemaining)
+	require.Contains(t, res.String(), "repository-scoped token via GH_TOKEN")
+}
+
+func TestCheck_RepositoryScopedTokenWithoutRateLimit(t *testing.T) {
+	host := startFakeGitHub(t, &fakeGitHub{userStatus: 403, repoStatus: 200, rateStatus: 500})
+
+	res, err := Check(context.Background(), testCreds, host, testRepo)
+	require.NoError(t, err)
+	require.Equal(t, CheckResult{Source: "GH_TOKEN"}, *res)
+}
+
+func TestCheck_RepositoryScopedTokenThatCannotSeeTheRepo(t *testing.T) {
+	host := startFakeGitHub(t, &fakeGitHub{userStatus: 403, repoStatus: 404, rateStatus: 200})
+
+	_, err := Check(context.Background(), testCreds, host, testRepo)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "authenticated as repository-scoped token")
+}
+
+// A valid user token that cannot see a private repository gets 404, not 403;
+// the error must say it is an access problem and name the token and login.
+func TestCheck_UserTokenCannotSeeTheRepo(t *testing.T) {
+	host := startFakeGitHub(t, &fakeGitHub{userStatus: 200, repoStatus: 404, rateStatus: 200})
+
+	_, err := Check(context.Background(), testCreds, host, testRepo)
+	require.Error(t, err)
+	msg := err.Error()
+	require.Contains(t, msg, "the GitHub token from GH_TOKEN is valid (authenticated as octocat) but cannot read acme/widgets")
+	require.Contains(t, msg, "almost certainly\n  an access problem")
+}
+
+// The quota is informational: failing to read it does not fail the check.
+func TestCheck_RateLimitFailureIsNotFatal(t *testing.T) {
+	host := startFakeGitHub(t, &fakeGitHub{userStatus: 200, repoStatus: 200, rateStatus: 500})
+
+	res, err := Check(context.Background(), testCreds, host, testRepo)
+	require.NoError(t, err)
+	require.Equal(t, CheckResult{Login: "octocat", Source: "GH_TOKEN"}, *res)
+}
+
+func TestRepoAccessError(t *testing.T) {
+	raw := errors.New("boom")
+	resp := func(code int) *github.Response {
+		return &github.Response{Response: &http.Response{StatusCode: code}}
+	}
+
+	for _, code := range []int{http.StatusNotFound, http.StatusForbidden} {
+		err := repoAccessError(testCreds, "octocat", testRepo, resp(code), raw)
+		require.Contains(t, err.Error(), "the GitHub token from GH_TOKEN is valid (authenticated as octocat) but cannot read acme/widgets", "status %d", code)
+		require.NotErrorIs(t, err, raw, "the access message replaces the bare status")
+	}
+
+	for _, r := range []*github.Response{resp(http.StatusInternalServerError), nil} {
+		err := repoAccessError(testCreds, "octocat", testRepo, r, raw)
+		require.Equal(t, "GitHub could not read acme/widgets: boom", err.Error())
+		require.ErrorIs(t, err, raw, "any other failure keeps the raw error")
+	}
+}
+
+func TestComma(t *testing.T) {
+	for n, want := range map[int]string{
+		0: "0", 7: "7", 999: "999", 1000: "1,000", 12345: "12,345",
+		123456: "123,456", 1234567: "1,234,567",
+	} {
+		require.Equal(t, want, comma(n), "comma(%d)", n)
+	}
+}
+
+func TestCheckResultString(t *testing.T) {
+	user := CheckResult{Login: "octocat", Source: "gh CLI", RateLimitRemaining: 4321, RateLimitLimit: 5000}
+	require.Equal(t, "GitHub authentication: ok (octocat via gh CLI, 4,321/5,000 requests remaining)", user.String())
+
+	scoped := CheckResult{Source: "GITHUB_TOKEN", RateLimitRemaining: 999, RateLimitLimit: 1000}
+	require.Equal(t, "GitHub authentication: ok (repository-scoped token via GITHUB_TOKEN, 999/1,000 requests remaining)", scoped.String())
+}
+
+func TestNewClientEnterpriseHost(t *testing.T) {
+	c, err := NewClient(testCreds, "ghe.example.com")
+	require.NoError(t, err)
+	require.Equal(t, "https://ghe.example.com/api/v3/", c.GitHub().BaseURL.String())
+
+	c, err = NewClient(testCreds, "github.com")
+	require.NoError(t, err)
+	require.Equal(t, "https://api.github.com/", c.GitHub().BaseURL.String(), "the public host keeps the default API URL")
+
+	_, err = NewClient(testCreds, "bad host%zz")
+	require.Error(t, err)
+	require.True(t, strings.HasPrefix(err.Error(), "configure GitHub Enterprise host bad host%zz:"), err.Error())
+}

internal/analyze/forge/fetch_test.go

@@ -0,0 +1,190 @@
+package forge
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+// ghMux is a GitHub Enterprise API (under /api/v3/) answering fixed JSON per
+// path. A path with no route is a 404.
+func ghMux(routes map[string]string) http.Handler {
+	mux := http.NewServeMux()
+	for path, body := range routes {
+		body := body
+		mux.HandleFunc("/api/v3"+path, func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = fmt.Fprint(w, body)
+		})
+	}
+	return mux
+}
+
+func fakeClient(t *testing.T, routes map[string]string) *Client {
+	t.Helper()
+	host := startFakeGitHub(t, ghMux(routes))
+	c, err := NewClient(testCreds, host)
+	require.NoError(t, err)
+	return c
+}
+
+var (
+	t0      = time.Date(2026, 10, 1, 9, 0, 0, 0, time.UTC)
+	at      = func(d time.Duration) string { return t0.Add(d).Format(time.RFC3339) }
+	fetchRp = Repo{Owner: "acme", Name: "widgets", DefaultBranch: "main"}
+)
+
+func TestFetchPullRequestsHydratesStateReviewsAndResponses(t *testing.T) {
+	c := fakeClient(t, map[string]string{
+		"/repos/acme/widgets/pulls": `[
+			{"number":3,"title":"merged","html_url":"u3","user":{"login":"ann"},"created_at":"` + at(0) + `","updated_at":"` + at(72*time.Hour) + `","merged_at":"` + at(5*time.Hour) + `","closed_at":"` + at(5*time.Hour) + `","merged_by":{"login":"bob"},"additions":10,"deletions":2,"changed_files":1,"comments":2},
+			{"number":2,"title":"rejected","user":{"login":"ann"},"created_at":"` + at(0) + `","updated_at":"` + at(48*time.Hour) + `","closed_at":"` + at(6*time.Hour) + `"},
+			{"number":1,"title":"too old","user":{"login":"ann"},"created_at":"` + at(-200*time.Hour) + `","updated_at":"` + at(-100*time.Hour) + `"}
+		]`,
+		"/repos/acme/widgets/issues/3/timeline": `[]`,
+		"/repos/acme/widgets/issues/2/timeline": `[]`,
+		"/repos/acme/widgets/pulls/3/reviews": `[
+			{"user":{"login":"ann"},"state":"COMMENTED","submitted_at":"` + at(30*time.Minute) + `"},
+			{"user":{"login":"ci","type":"Bot"},"state":"COMMENTED","submitted_at":"` + at(40*time.Minute) + `"},
+			{"user":{"login":"bob"},"state":"APPROVED","submitted_at":"` + at(3*time.Hour) + `"}
+		]`,
+		"/repos/acme/widgets/pulls/2/reviews": `[]`,
+		"/repos/acme/widgets/issues/3/comments": `[
+			{"user":{"login":"ann"},"created_at":"` + at(10*time.Minute) + `"},
+			{"user":{"login":"dependabot[bot]","type":"Bot"},"created_at":"` + at(20*time.Minute) + `"},
+			{"user":{"login":"cid"},"created_at":"` + at(2*time.Hour) + `"}
+		]`,
+		"/repos/acme/widgets/issues/2/comments": `[]`,
+	})
+
+	prs, err := c.FetchPullRequests(context.Background(), fetchRp, t0)
+	require.NoError(t, err)
+	require.Len(t, prs, 2, "the PR updated before the window ends the walk")
+
+	merged := prs[0]
+	require.Equal(t, 3, merged.Number)
+	require.Equal(t, "merged", merged.State)
+	require.Equal(t, "bob", merged.MergedBy.Login)
+	require.Equal(t, 10, merged.Additions)
+	require.Len(t, merged.Reviews, 3)
+	require.Equal(t, "approved", merged.Reviews[2].State, "review states are lower-cased")
+	require.Equal(t, t0.Add(3*time.Hour), merged.FirstReviewAt.UTC(), "the author's own and a bot's review do not count")
+	require.Equal(t, t0.Add(2*time.Hour), merged.FirstResponseAt.UTC(), "the first non-author, non-bot comment came before the review")
+	require.Equal(t, t0, merged.ReadyForReviewAt.UTC(), "never a draft: ready when opened")
+
+	rejected := prs[1]
+	require.Equal(t, "closed", rejected.State, "closed without merging is a rejection, not a merge")
+	require.True(t, rejected.MergedAt.IsZero())
+	require.True(t, rejected.FirstResponseAt.IsZero())
+}
+
+func TestDraftHistorySumsEveryDraftInterval(t *testing.T) {
+	c := fakeClient(t, map[string]string{
+		"/repos/acme/widgets/pulls": `[{"number":7,"draft":false,"user":{"login":"ann"},"created_at":"` + at(0) + `","updated_at":"` + at(10*time.Hour) + `"}]`,
+		"/repos/acme/widgets/issues/7/timeline": `[
+			{"event":"convert_to_draft","created_at":"` + at(1*time.Hour) + `"},
+			{"event":"ready_for_review","created_at":"` + at(2*time.Hour) + `"},
+			{"event":"convert_to_draft","created_at":"` + at(3*time.Hour) + `"},
+			{"event":"ready_for_review","created_at":"` + at(5*time.Hour) + `"}
+		]`,
+		"/repos/acme/widgets/pulls/7/reviews": `[]`,
+		"/repos/acme/widgets/issues/7/comments": `[
+			{"user":{"login":"bob"},"created_at":"` + at(90*time.Minute) + `"}
+		]`,
+	})
+
+	prs, err := c.FetchPullRequests(context.Background(), fetchRp, t0)
+	require.NoError(t, err)
+	require.Len(t, prs, 1)
+	p := prs[0]
+	require.Equal(t, t0.Add(2*time.Hour), p.ReadyForReviewAt.UTC(), "ready at the first ready_for_review")
+	require.Equal(t, int((3 * time.Hour).Seconds()), p.TimeInDraftSeconds, "1h + 2h across both draft cycles")
+	require.True(t, p.FirstResponseAt.IsZero(), "a comment before the PR was ready is not a response to it")
+}
+
+func TestFetchIssuesSkipsPullRequests(t *testing.T) {
+	c := fakeClient(t, map[string]string{
+		"/repos/acme/widgets/issues": `[
+			{"number":10,"title":"bug","state":"closed","state_reason":"completed","user":{"login":"ann"},"labels":[{"name":"bug"},{"name":"p1"}],"created_at":"` + at(0) + `","closed_at":"` + at(time.Hour) + `"},
+			{"number":11,"title":"a PR","user":{"login":"ann"},"pull_request":{"url":"x"},"created_at":"` + at(0) + `"}
+		]`,
+		"/repos/acme/widgets/issues/10/comments": `[{"user":{"login":"bob"},"created_at":"` + at(30*time.Minute) + `"}]`,
+	})
+
+	issues, err := c.FetchIssues(context.Background(), fetchRp, t0)
+	require.NoError(t, err)
+	require.Len(t, issues, 1, "the PR the issues endpoint returns is not an issue")
+	is := issues[0]
+	require.Equal(t, 10, is.Number)
+	require.Equal(t, "completed", is.StateReason)
+	require.Equal(t, []string{"bug", "p1"}, is.Labels)
+	require.Equal(t, t0.Add(30*time.Minute), is.FirstResponseAt.UTC())
+}
+
+func TestFetchCommitReviews(t *testing.T) {
+	c := fakeClient(t, map[string]string{
+		// approved on a PR into main
+		"/repos/acme/widgets/commits/aaa/pulls": `[
+			{"number":20,"html_url":"u20","user":{"login":"ann"},"base":{"ref":"release"}},
+			{"number":21,"html_url":"u21","user":{"login":"ann"},"base":{"ref":"main"}}
+		]`,
+		"/repos/acme/widgets/pulls/21/reviews": `[{"user":{"login":"bob"},"state":"APPROVED","submitted_at":"` + at(0) + `"}]`,
+		// a PR into main nobody but the author approved
+		"/repos/acme/widgets/commits/bbb/pulls": `[{"number":22,"user":{"login":"ann"},"base":{"ref":"main"}}]`,
+		"/repos/acme/widgets/pulls/22/reviews":  `[{"user":{"login":"ann"},"state":"APPROVED","submitted_at":"` + at(0) + `"}]`,
+		// no PR into main at all
+		"/repos/acme/widgets/commits/ccc/pulls": `[{"number":23,"user":{"login":"ann"},"base":{"ref":"release"}}]`,
+		// "ddd" has no route: the lookup fails and the commit is unknown
+	})
+
+	got, err := c.FetchCommitReviews(context.Background(), fetchRp, []string{"aaa", "bbb", "ccc", "ddd"})
+	require.NoError(t, err)
+	require.Equal(t, []CommitReview{
+		{SHA: "aaa", PRNumber: 21, PRURL: "u21", Status: "approved"},
+		{SHA: "bbb", PRNumber: 22, Status: "review_required"},
+		{SHA: "ccc", Status: "unknown"},
+		{SHA: "ddd", Status: "unknown"},
+	}, got)
+}
+
+// fakeGh puts a gh on PATH that runs script.
+func fakeGh(t *testing.T, script string) {
+	t.Helper()
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "gh"), []byte("#!/bin/sh\n"+script+"\n"), 0o755))
+	t.Setenv("PATH", dir)
+}
+
+func TestResolveCredentials_GhCLI(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("uses a shell script as gh")
+	}
+	t.Setenv("GITHUB_TOKEN", "")
+	t.Setenv("GH_TOKEN", "")
+
+	fakeGh(t, `[ "$1 $2" = "auth token" ] && echo "  from-gh-cli  "`)
+	creds, err := ResolveCredentials(context.Background())
+	require.NoError(t, err)
+	require.Equal(t, "from-gh-cli", creds.Token, "gh's output is trimmed")
+	require.Equal(t, "gh CLI", creds.Source)
+
+	for name, script := range map[string]string{
+		"gh fails":        "exit 1",
+		"gh prints blank": "echo",
+	} {
+		fakeGh(t, script)
+		_, err := ResolveCredentials(context.Background())
+		require.ErrorIs(t, err, ErrNoCredentials, name)
+	}
+
+	t.Setenv("PATH", t.TempDir()) // no gh at all
+	_, err = ResolveCredentials(context.Background())
+	require.ErrorIs(t, err, ErrNoCredentials, "gh absent")
+}