diff --git a/cmd/kosli/assertPRGithub.go b/cmd/kosli/assertPRGithub.go index 05241c89f..8979281ef 100644 --- a/cmd/kosli/assertPRGithub.go +++ b/cmd/kosli/assertPRGithub.go @@ -59,7 +59,7 @@ func newAssertPullRequestGithubCmd(out io.Writer) *cobra.Command { } func (o *assertPullRequestGithubOptions) run(args []string) error { - pullRequestsEvidence, err := o.retriever.PREvidenceForCommitV2(o.commit) + pullRequestsEvidence, err := o.retriever.PREvidenceForCommitHybrid(o.commit) if err != nil { return err } diff --git a/cmd/kosli/pullrequest.go b/cmd/kosli/pullrequest.go index 8766a760f..5f1c329d5 100644 --- a/cmd/kosli/pullrequest.go +++ b/cmd/kosli/pullrequest.go @@ -42,7 +42,7 @@ func (o *attestPROptions) run(args []string) error { o.payload.GitProvider, label = o.getRetriever().ProviderAndLabel() var pullRequestsEvidence []*types.PREvidence - pullRequestsEvidence, err = o.getRetriever().PREvidenceForCommitV2(o.payload.Commit.Sha1) + pullRequestsEvidence, err = o.getRetriever().PREvidenceForCommitHybrid(o.payload.Commit.Sha1) if err != nil { return err } diff --git a/internal/azure/azure.go b/internal/azure/azure.go index 67250c02d..6f6628484 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -76,6 +76,10 @@ func (c *AzureConfig) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, return pullRequestsEvidence, nil } +func (c *AzureConfig) PREvidenceForCommitHybrid(commit string) ([]*types.PREvidence, error) { + return c.PREvidenceForCommitV2(commit) +} + // This is the new implementation, it will be used for Azure func (c *AzureConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence, error) { pullRequestsEvidence := []*types.PREvidence{} diff --git a/internal/bitbucket/bitbucket.go b/internal/bitbucket/bitbucket.go index 73e2cb7ba..40cb9cc7a 100644 --- a/internal/bitbucket/bitbucket.go +++ b/internal/bitbucket/bitbucket.go @@ -43,6 +43,10 @@ func (c *Config) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, erro return c.getPullRequestsFromBitbucketApi(commit, 1) } +func (c *Config) PREvidenceForCommitHybrid(commit string) ([]*types.PREvidence, error) { + return c.PREvidenceForCommitV2(commit) +} + // This is the new implementation, it will be used for Bitbucket func (c *Config) PREvidenceForCommitV2(commit string) ([]*types.PREvidence, error) { return c.getPullRequestsFromBitbucketApi(commit, 2) diff --git a/internal/github/fake_github.go b/internal/github/fake_github.go index bac828b0c..d4833dd7b 100644 --- a/internal/github/fake_github.go +++ b/internal/github/fake_github.go @@ -12,10 +12,17 @@ var errInjected = errors.New("injected error") // FakeGitHubClient is an in-memory implementation of types.PRRetriever for // testing. Seed PRsByCommit with the commits and PR evidence you want returned. +// Seed PRsByNumber with PR numbers and their evidence for PREvidenceByPRNumber. +// Seed PRNumbersByCommit + PRsByNumber to exercise the hybrid fallback path +// (V2 empty → V1 discovery → per-PR GraphQL). // Set Err to simulate a network or API failure. type FakeGitHubClient struct { // PRsByCommit maps a commit SHA to the PR evidence returned for that commit. PRsByCommit map[string][]*types.PREvidence + // PRsByNumber maps a PR number to the PR evidence returned for that number. + PRsByNumber map[int]*types.PREvidence + // PRNumbersByCommit maps a commit SHA to PR numbers for the hybrid fallback path. + PRNumbersByCommit map[string][]int // Err, if set, is returned by all calls regardless of commit. Err error } @@ -38,6 +45,49 @@ func (f *FakeGitHubClient) PREvidenceForCommitV1(commit string) ([]*types.PREvid return prs, nil } +// PREvidenceForCommitHybrid mirrors the hybrid strategy: tries V2 (PRsByCommit) +// first; if empty, falls back through PRNumbersByCommit + PREvidenceByPRNumber. +// Returns empty (no error) when the commit is not found in either map. +func (f *FakeGitHubClient) PREvidenceForCommitHybrid(commit string) ([]*types.PREvidence, error) { + if f.Err != nil { + return nil, f.Err + } + prs, err := f.PREvidenceForCommitV2(commit) + if err != nil { + return nil, err + } + if len(prs) > 0 { + return prs, nil + } + // Fallback: use PRNumbersByCommit for V1-style discovery. + prNumbers := f.PRNumbersByCommit[commit] + result := []*types.PREvidence{} + for _, n := range prNumbers { + evidence, err := f.PREvidenceByPRNumber(n) + if err != nil { + return nil, err + } + if evidence != nil { + result = append(result, evidence) + } + } + return result, nil +} + +// PREvidenceByPRNumber mirrors the GraphQL API: returns an error for PR numbers +// not present in PRsByNumber (matching the real GitHub behaviour of returning +// an error for non-existent pull requests). +func (f *FakeGitHubClient) PREvidenceByPRNumber(prNumber int) (*types.PREvidence, error) { + if f.Err != nil { + return nil, f.Err + } + pr, ok := f.PRsByNumber[prNumber] + if !ok { + return nil, fmt.Errorf("could not resolve to a pull request with number %d", prNumber) + } + return pr, nil +} + // PREvidenceForCommitV2 mirrors the GraphQL API: returns empty with no error // for commits not present in PRsByCommit (matching the real GitHub V2 behaviour // of returning null for unknown commits). diff --git a/internal/github/github.go b/internal/github/github.go index b450cf6c5..ea2e5176e 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -21,6 +21,9 @@ type GithubConfig struct { BaseURL string Org string Repository string + // Sleep is called between retries in PREvidenceByPRNumber. Defaults to + // time.Sleep when nil. Override in tests to avoid real delays. + Sleep func(time.Duration) } type GithubFlagsTempValueHolder struct { @@ -90,6 +93,222 @@ func ResetGithubRetrieverFunc() { NewGithubRetrieverFunc = defaultNewGithubRetriever } +// PREvidenceForCommitHybrid tries PREvidenceForCommitV2 first. If it returns +// no results it falls back to V1 REST discovery (immediately consistent) + +// PREvidenceByPRNumber for each PR found, preserving all rich V2 fields. +func (c *GithubConfig) PREvidenceForCommitHybrid(commit string) ([]*types.PREvidence, error) { + prs, err := c.PREvidenceForCommitV2(commit) + if err != nil { + return nil, err + } + if len(prs) > 0 { + return prs, nil + } + + // V2 returned nothing — fall back to REST discovery. + restPRs, err := c.PullRequestsForCommit(commit) + if err != nil { + return nil, err + } + + result := []*types.PREvidence{} + for _, pr := range restPRs { + evidence, err := c.PREvidenceByPRNumber(pr.GetNumber()) + if err != nil { + return nil, err + } + if evidence != nil { + result = append(result, evidence) + } + } + return result, nil +} + +// graphqlCommitNode is the shared GraphQL node type for commits on a PR, +// used in both PREvidenceByPRNumber and PREvidenceForCommitV2. +type graphqlCommitNode struct { + Commit struct { + Oid graphql.String + MessageHeadline graphql.String + CommittedDate graphql.String + URL graphql.String + Committer struct { + Name graphql.String + Email graphql.String + Date graphql.String + User *struct { + Login graphql.String + } + } + } +} + +// graphqlReviewNode is the shared GraphQL node type for approved reviews on a PR. +type graphqlReviewNode struct { + Author struct { + Login graphql.String + } + State graphql.String + SubmittedAt graphql.String +} + +// buildPREvidence constructs a PREvidence from pre-resolved fields and the +// raw GraphQL commit/review nodes. mergeCommit must be resolved by the caller +// (it differs between commit-SHA queries and PR-number queries). +func buildPREvidence( + url, mergeCommit, state, author, createdAtStr, mergedAtStr, title, headRef string, + commitNodes []graphqlCommitNode, + reviewNodes []graphqlReviewNode, +) (*types.PREvidence, error) { + createdAt, err := time.Parse(time.RFC3339, createdAtStr) + if err != nil { + return nil, err + } + mergedAt := int64(0) + if mergedAtStr != "" { + mergedAtTime, err := time.Parse(time.RFC3339, mergedAtStr) + if err != nil { + return nil, err + } + mergedAt = mergedAtTime.Unix() + } + + evidence := &types.PREvidence{ + URL: url, + MergeCommit: mergeCommit, + State: state, + Author: author, + CreatedAt: createdAt.Unix(), + MergedAt: mergedAt, + Title: title, + HeadRef: headRef, + Approvers: []any{}, + Commits: []types.Commit{}, + } + + for _, n := range commitNodes { + timestamp, err := time.Parse(time.RFC3339, string(n.Commit.CommittedDate)) + if err != nil { + return nil, err + } + committerUsername := "" + if n.Commit.Committer.User != nil { + committerUsername = string(n.Commit.Committer.User.Login) + } + evidence.Commits = append(evidence.Commits, types.Commit{ + SHA: string(n.Commit.Oid), + Message: string(n.Commit.MessageHeadline), + Committer: fmt.Sprintf("%s <%s>", string(n.Commit.Committer.Name), string(n.Commit.Committer.Email)), + CommitterUsername: committerUsername, + Timestamp: timestamp.Unix(), + Branch: headRef, + URL: string(n.Commit.URL), + }) + } + + for _, r := range reviewNodes { + submittedAt, err := time.Parse(time.RFC3339, string(r.SubmittedAt)) + if err != nil { + return nil, err + } + evidence.Approvers = append(evidence.Approvers, types.PRApprovals{ + Username: string(r.Author.Login), + State: string(r.State), + Timestamp: submittedAt.Unix(), + }) + } + + return evidence, nil +} + +// PREvidenceByPRNumber fetches full PR evidence for a single PR number via +// GraphQL. Returns an error when the PR does not exist. +func (c *GithubConfig) PREvidenceByPRNumber(prNumber int) (*types.PREvidence, error) { + ctx := context.Background() + + ghClient, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL) + if err != nil { + return nil, err + } + httpClient := ghClient.Client() + client := graphql.NewClient(graphqlEndpoint(c.BaseURL), httpClient) + + var query struct { + Repository struct { + PullRequest *struct { + Title graphql.String + State graphql.String + HeadRefName graphql.String + URL graphql.String + CreatedAt graphql.String + MergedAt graphql.String + MergeCommit *struct { + Oid graphql.String + } + Author struct { + Login graphql.String + } + Commits struct { + Nodes []graphqlCommitNode + PageInfo struct { + HasNextPage graphql.Boolean + EndCursor graphql.String + } + } `graphql:"commits(first: 100, after: $commitCursor)"` + Reviews struct { + Nodes []graphqlReviewNode + PageInfo struct { + HasNextPage graphql.Boolean + EndCursor graphql.String + } + } `graphql:"reviews(first: 100, states: APPROVED, after: $reviewCursor)"` + } `graphql:"pullRequest(number: $prNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + variables := map[string]interface{}{ + "owner": graphql.String(c.Org), + "repo": graphql.String(c.Repository), + "prNumber": graphql.Int(prNumber), + "commitCursor": (*graphql.String)(nil), + "reviewCursor": (*graphql.String)(nil), + } + + sleep := c.Sleep + if sleep == nil { + sleep = time.Sleep + } + delays := []time.Duration{0, 10 * time.Second, 20 * time.Second, 30 * time.Second} + for _, delay := range delays { + if delay > 0 { + sleep(delay) + } + err = client.Query(ctx, &query, variables) + if err == nil { + break + } + } + if err != nil { + return nil, err + } + + pr := query.Repository.PullRequest + if pr == nil { + return nil, nil + } + + mergeCommit := "" + if pr.MergeCommit != nil { + mergeCommit = string(pr.MergeCommit.Oid) + } + + return buildPREvidence( + string(pr.URL), mergeCommit, string(pr.State), string(pr.Author.Login), + string(pr.CreatedAt), string(pr.MergedAt), string(pr.Title), string(pr.HeadRefName), + pr.Commits.Nodes, pr.Reviews.Nodes, + ) +} + func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence, error) { ctx := context.Background() pullRequestsEvidence := []*types.PREvidence{} @@ -121,22 +340,7 @@ func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence } Commits struct { - Nodes []struct { - Commit struct { - Oid graphql.String - MessageHeadline graphql.String - CommittedDate graphql.String - URL graphql.String - Committer struct { - Name graphql.String - Email graphql.String - Date graphql.String - User *struct { - Login graphql.String - } - } - } - } + Nodes []graphqlCommitNode PageInfo struct { HasNextPage graphql.Boolean EndCursor graphql.String @@ -144,13 +348,7 @@ func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence } `graphql:"commits(first: 100, after: $commitCursor)"` Reviews struct { - Nodes []struct { - Author struct { - Login graphql.String - } - State graphql.String - SubmittedAt graphql.String - } + Nodes []graphqlReviewNode PageInfo struct { HasNextPage graphql.Boolean EndCursor graphql.String @@ -181,69 +379,17 @@ func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence return pullRequestsEvidence, err } - // Print results for demonstration for _, pr := range query.Repository.Object.Commit.AssociatedPullRequests.Nodes { - createdAt, err := time.Parse(time.RFC3339, string(pr.CreatedAt)) + // MergeCommit is set to the queried commit SHA — V2 queries by commit SHA + // so the commit is by definition the merge commit. + evidence, err := buildPREvidence( + string(pr.URL), commit, string(pr.State), string(pr.Author.Login), + string(pr.CreatedAt), string(pr.MergedAt), string(pr.Title), string(pr.HeadRefName), + pr.Commits.Nodes, pr.Reviews.Nodes, + ) if err != nil { return pullRequestsEvidence, err } - mergedAt := int64(0) - if pr.MergedAt != "" { - mergedAtTime, err := time.Parse(time.RFC3339, string(pr.MergedAt)) - if err != nil { - return pullRequestsEvidence, err - } - mergedAt = mergedAtTime.Unix() - } - - evidence := &types.PREvidence{ - URL: string(pr.URL), - MergeCommit: commit, - State: string(pr.State), - Author: string(pr.Author.Login), - CreatedAt: createdAt.Unix(), - MergedAt: mergedAt, - Title: string(pr.Title), - HeadRef: string(pr.HeadRefName), - Approvers: []any{}, - Commits: []types.Commit{}, - } - - for _, c := range pr.Commits.Nodes { - timestamp, err := time.Parse(time.RFC3339, string(c.Commit.CommittedDate)) - if err != nil { - return pullRequestsEvidence, err - } - - committerUsername := "" - if c.Commit.Committer.User != nil { - committerUsername = string(c.Commit.Committer.User.Login) - } - - evidence.Commits = append(evidence.Commits, types.Commit{ - SHA: string(c.Commit.Oid), - Message: string(c.Commit.MessageHeadline), - Committer: fmt.Sprintf("%s <%s>", string(c.Commit.Committer.Name), string(c.Commit.Committer.Email)), - CommitterUsername: committerUsername, - Timestamp: timestamp.Unix(), - Branch: string(pr.HeadRefName), - URL: string(c.Commit.URL), - }) - } - - for _, r := range pr.Reviews.Nodes { - submittedAt, err := time.Parse(time.RFC3339, string(r.SubmittedAt)) - if err != nil { - return pullRequestsEvidence, err - } - - evidence.Approvers = append(evidence.Approvers, types.PRApprovals{ - Username: string(r.Author.Login), - State: string(r.State), - Timestamp: submittedAt.Unix(), - }) - } - pullRequestsEvidence = append(pullRequestsEvidence, evidence) } return pullRequestsEvidence, nil diff --git a/internal/github/github_contract_test.go b/internal/github/github_contract_test.go index e55921d0c..bf387050b 100644 --- a/internal/github/github_contract_test.go +++ b/internal/github/github_contract_test.go @@ -53,6 +53,15 @@ func runGitHubContractTests(t *testing.T, provider types.PRRetriever, commitWith require.Error(t, err) }) + t.Run("Hybrid returns PRs for commit with PRs", func(t *testing.T) { + prs, err := provider.PREvidenceForCommitHybrid(commitWithPR) + require.NoError(t, err) + require.NotEmpty(t, prs) + require.NotEmpty(t, prs[0].URL, "URL should be present") + require.NotEmpty(t, prs[0].State, "State should be present") + require.NotEmpty(t, prs[0].MergeCommit, "MergeCommit should be present") + }) + t.Run("ProviderAndLabel returns github and pull request", func(t *testing.T) { providerName, label := provider.ProviderAndLabel() require.Equal(t, "github", providerName) @@ -60,6 +69,31 @@ func runGitHubContractTests(t *testing.T, provider types.PRRetriever, commitWith }) } +// prByNumberRetriever is a local interface used to test PREvidenceByPRNumber +// independently of the PRRetriever contract. +type prByNumberRetriever interface { + PREvidenceByPRNumber(int) (*types.PREvidence, error) +} + +func runPRByNumberContractTests(t *testing.T, provider prByNumberRetriever, knownPRNumber int) { + t.Helper() + + t.Run("returns evidence for known PR number", func(t *testing.T) { + pr, err := provider.PREvidenceByPRNumber(knownPRNumber) + require.NoError(t, err) + require.NotNil(t, pr) + require.NotEmpty(t, pr.URL, "URL should be present") + require.NotEmpty(t, pr.State, "State should be present") + require.NotEmpty(t, pr.MergeCommit, "MergeCommit should be present") + }) + + t.Run("returns error for unknown PR number", func(t *testing.T) { + pr, err := provider.PREvidenceByPRNumber(999999999) + require.Error(t, err) + require.Nil(t, pr) + }) +} + func TestGitHubContract_Fake(t *testing.T) { commitWithPR := "abc123" commitUnknown := "0000000000000000000000000000000000000000" @@ -93,6 +127,44 @@ func TestGitHubContract_Fake(t *testing.T) { _, err := client.PREvidenceForCommitV1(commitWithPR) require.Error(t, err) }) + + // Hybrid fallback path: V2 returns empty (PRsByCommit not seeded for this + // commit), so the fake falls back through PRNumbersByCommit + PRsByNumber. + commitFallback := "fallback-commit" + prNumber := testHelpers.GithubPRNumber() + fallbackPR := &types.PREvidence{ + URL: "https://github.com/kosli-dev/cli/pull/6", + State: "MERGED", + MergeCommit: commitFallback, + } + fallbackClient := &FakeGitHubClient{ + PRNumbersByCommit: map[string][]int{ + commitFallback: {prNumber}, + }, + PRsByNumber: map[int]*types.PREvidence{ + prNumber: fallbackPR, + }, + } + + t.Run("Hybrid returns PRs via fallback path when V2 returns empty", func(t *testing.T) { + prs, err := fallbackClient.PREvidenceForCommitHybrid(commitFallback) + require.NoError(t, err) + require.NotEmpty(t, prs) + require.Equal(t, fallbackPR.URL, prs[0].URL) + }) + + t.Run("Hybrid returns empty when commit has no PRs in either path", func(t *testing.T) { + prs, err := fallbackClient.PREvidenceForCommitHybrid("commit-with-no-prs") + require.NoError(t, err) + require.Empty(t, prs) + }) + + t.Run("Hybrid returns error when Err is injected", func(t *testing.T) { + client.Err = errInjected + defer func() { client.Err = nil }() + _, err := client.PREvidenceForCommitHybrid(commitWithPR) + require.Error(t, err) + }) } func TestGitHubContract_RealGitHub(t *testing.T) { @@ -109,3 +181,38 @@ func TestGitHubContract_RealGitHub(t *testing.T) { commitUnknown := "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" runGitHubContractTests(t, config, testHelpers.GithubCommitWithPR(), commitUnknown) } + +func TestGitHubContract_Fake_PRByNumber(t *testing.T) { + knownPRNumber := testHelpers.GithubPRNumber() + pr := &types.PREvidence{ + URL: "https://github.com/kosli-dev/cli/pull/6", + State: "MERGED", + MergeCommit: "e21a8afff429e0c87ee523d683f2438113f0a105", + } + client := &FakeGitHubClient{ + PRsByNumber: map[int]*types.PREvidence{ + knownPRNumber: pr, + }, + } + runPRByNumberContractTests(t, client, knownPRNumber) + + t.Run("returns error when Err is injected", func(t *testing.T) { + client.Err = errInjected + defer func() { client.Err = nil }() + _, err := client.PREvidenceByPRNumber(knownPRNumber) + require.Error(t, err) + }) +} + +func TestGitHubContract_RealGitHub_PRByNumber(t *testing.T) { + testHelpers.SkipIfEnvVarUnset(t, []string{"KOSLI_GITHUB_TOKEN"}) + + config := NewGithubConfig( + os.Getenv("KOSLI_GITHUB_TOKEN"), + "", + "kosli-dev", + "cli", + ) + + runPRByNumberContractTests(t, config, testHelpers.GithubPRNumber()) +} diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 38bf8e0f1..8e9bb9d0c 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -2,7 +2,11 @@ package github import ( "context" + "fmt" + "net/http" + "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -67,3 +71,61 @@ func (suite *GithubTestSuite) TestExtractRepoName() { func TestGithubTestSuite(t *testing.T) { suite.Run(t, new(GithubTestSuite)) } + +// graphqlNullPRResponse is a minimal valid GraphQL response where the PR is null. +// PREvidenceByPRNumber returns nil, nil in this case. +const graphqlNullPRResponse = `{"data":{"repository":{"pullRequest":null}}}` + +func newRetryTestServer(t *testing.T, failCount int) (*httptest.Server, *int) { + t.Helper() + attempts := 0 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/graphql" { + w.WriteHeader(http.StatusNotFound) + return + } + attempts++ + if attempts <= failCount { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprint(w, graphqlNullPRResponse) + })) + return ts, &attempts +} + +func newRetryConfig(serverURL string, sleepFn func(time.Duration)) *GithubConfig { + return &GithubConfig{ + Token: "fake-token", + BaseURL: serverURL, + Org: "test-org", + Repository: "test-repo", + Sleep: sleepFn, + } +} + +func TestPREvidenceByPRNumber_RetriesOnGraphQLError(t *testing.T) { + ts, attempts := newRetryTestServer(t, 2) + defer ts.Close() + + var sleptDurations []time.Duration + config := newRetryConfig(ts.URL, func(d time.Duration) { sleptDurations = append(sleptDurations, d) }) + + pr, err := config.PREvidenceByPRNumber(1) + require.NoError(t, err) + require.Nil(t, pr) + require.Equal(t, 3, *attempts, "should have retried twice before succeeding") + require.Len(t, sleptDurations, 2, "should have slept between retries") +} + +func TestPREvidenceByPRNumber_ReturnsErrorAfterAllRetriesExhausted(t *testing.T) { + ts, attempts := newRetryTestServer(t, 999) + defer ts.Close() + + config := newRetryConfig(ts.URL, func(time.Duration) {}) + + _, err := config.PREvidenceByPRNumber(1) + require.Error(t, err) + require.Equal(t, 4, *attempts, "should have made 1 initial attempt + 3 retries") +} diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index bdb7bae97..a58226763 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -59,6 +59,10 @@ func (c *GitlabConfig) PREvidenceForCommitV1(commit string) ([]*types.PREvidence return pullRequestsEvidence, nil } +func (c *GitlabConfig) PREvidenceForCommitHybrid(commit string) ([]*types.PREvidence, error) { + return c.PREvidenceForCommitV2(commit) +} + // This is the new implementation, it will be used for all VCS providers func (c *GitlabConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence, error) { pullRequestsEvidence := []*types.PREvidence{} diff --git a/internal/testHelpers/testHelpers.go b/internal/testHelpers/testHelpers.go index ef29a3edd..09df22df4 100644 --- a/internal/testHelpers/testHelpers.go +++ b/internal/testHelpers/testHelpers.go @@ -45,6 +45,12 @@ func GithubCommitWithPR() string { return "e21a8afff429e0c87ee523d683f2438113f0a105" } +// GithubPRNumber returns the PR number associated with GithubCommitWithPR in +// kosli-dev/cli. It is stable because merged PRs are never deleted. +func GithubPRNumber() int { + return 6 +} + func CloneGitRepo(url, cloneTo string) (*git.Repository, error) { // the repo worktree filesystem. It has to be osfs so that we can give it a path fs := osfs.New(cloneTo) diff --git a/internal/types/types.go b/internal/types/types.go index e4d3dd889..cc84f2173 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -32,6 +32,11 @@ type Commit struct { type PRRetriever interface { PREvidenceForCommitV2(string) ([]*PREvidence, error) PREvidenceForCommitV1(string) ([]*PREvidence, error) + // PREvidenceForCommitHybrid tries V2 (GraphQL by commit SHA) first. If it + // returns no results it falls back to V1 REST discovery + per-PR GraphQL so + // that GitHub's eventual consistency on associatedPullRequests never causes + // a false "no PR found". + PREvidenceForCommitHybrid(string) ([]*PREvidence, error) // ProviderAndLabel returns the provider name (e.g. "github") and the label // for a pull request (e.g. "pull request", or "merge request" for GitLab). ProviderAndLabel() (string, string)