diff --git a/internal/prinfo/prinfo.go b/internal/prinfo/prinfo.go index a1f79d7e5..0c69efb30 100644 --- a/internal/prinfo/prinfo.go +++ b/internal/prinfo/prinfo.go @@ -19,13 +19,15 @@ const ( // EvidenceID is the identifier for the PR/MR info material type EvidenceID = "CHAINLOOP_PR_INFO" // EvidenceSchemaURL is the URL to the JSON schema for PR/MR info - EvidenceSchemaURL = "https://schemas.chainloop.dev/prinfo/1.1/pr-info.schema.json" + EvidenceSchemaURL = "https://schemas.chainloop.dev/prinfo/1.2/pr-info.schema.json" ) // Reviewer represents a reviewer of the PR/MR type Reviewer struct { - Login string `json:"login" jsonschema:"required,description=Username of the reviewer"` - Type string `json:"type" jsonschema:"required,enum=User,enum=Bot,enum=unknown,description=Account type of the reviewer"` + Login string `json:"login" jsonschema:"required,description=Username of the reviewer"` + Type string `json:"type" jsonschema:"required,enum=User,enum=Bot,enum=unknown,description=Account type of the reviewer"` + Requested bool `json:"requested" jsonschema:"required,description=Whether the reviewer was explicitly requested to review"` + ReviewStatus string `json:"review_status,omitempty" jsonschema:"enum=APPROVED,enum=CHANGES_REQUESTED,enum=COMMENTED,enum=DISMISSED,enum=PENDING,description=The reviewer's current review state if they have submitted a review"` } // Data represents the data payload of the PR/MR info evidence diff --git a/internal/prinfo/prinfo_test.go b/internal/prinfo/prinfo_test.go index cfdf1e78b..d3f206730 100644 --- a/internal/prinfo/prinfo_test.go +++ b/internal/prinfo/prinfo_test.go @@ -185,6 +185,84 @@ func TestValidatePRInfo(t *testing.T) { } } +func TestValidatePRInfoV1_2(t *testing.T) { + testCases := []struct { + name string + data string + wantErr bool + }{ + { + name: "valid reviewer with requested and review_status", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [ + {"login": "reviewer1", "type": "User", "requested": true, "review_status": "COMMENTED"}, + {"login": "reviewer2", "type": "User", "requested": false, "review_status": "APPROVED"}, + {"login": "coderabbitai", "type": "Bot", "requested": true} + ] + }`, + wantErr: false, + }, + { + name: "reviewer missing required requested field fails validation", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [ + {"login": "reviewer1", "type": "User"} + ] + }`, + wantErr: true, + }, + { + name: "invalid review_status value", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [ + {"login": "reviewer1", "type": "User", "review_status": "UNKNOWN_STATE"} + ] + }`, + wantErr: true, + }, + { + name: "additional property in reviewer not allowed", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [ + {"login": "reviewer1", "type": "User", "extra": "not allowed"} + ] + }`, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var data interface{} + err := json.Unmarshal([]byte(tc.data), &data) + require.NoError(t, err) + + err = schemavalidators.ValidatePRInfo(data, schemavalidators.PRInfoVersion1_2) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + func TestValidatePRInfoV1_0BackwardCompat(t *testing.T) { testCases := []struct { name string diff --git a/internal/prinfo/schemas/generate.go b/internal/prinfo/schemas/generate.go index dfb21d03c..c4267bd47 100644 --- a/internal/prinfo/schemas/generate.go +++ b/internal/prinfo/schemas/generate.go @@ -1,5 +1,5 @@ // -// Copyright 2025 The Chainloop Authors. +// Copyright 2025-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -23,13 +23,13 @@ import ( "github.com/chainloop-dev/chainloop/internal/prinfo" ) -//go:generate go run ./generate.go --output-dir ../../../internal/schemavalidators/internal_schemas/prinfo --version 1.1 +//go:generate go run ./generate.go --output-dir ../../../internal/schemavalidators/internal_schemas/prinfo --version 1.2 func main() { var outputDir string var version string flag.StringVar(&outputDir, "output-dir", "../../../internal/schemavalidators/internal_schemas/prinfo", "Directory to output the schema files") - flag.StringVar(&version, "version", "1.1", "Schema version") + flag.StringVar(&version, "version", "1.2", "Schema version") flag.Parse() generator := prinfo.NewGenerator() diff --git a/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.2.schema.json b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.2.schema.json new file mode 100644 index 000000000..96e31b1ee --- /dev/null +++ b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.2.schema.json @@ -0,0 +1,104 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://schemas.chainloop.dev/prinfo/1.2/pr-info.schema.json", + "properties": { + "platform": { + "type": "string", + "enum": [ + "github", + "gitlab" + ], + "description": "The CI/CD platform" + }, + "type": { + "type": "string", + "enum": [ + "pull_request", + "merge_request" + ], + "description": "The type of change request" + }, + "number": { + "type": "string", + "description": "The PR/MR number or identifier" + }, + "title": { + "type": "string", + "description": "The PR/MR title" + }, + "description": { + "type": "string", + "description": "The PR/MR description or body" + }, + "source_branch": { + "type": "string", + "description": "The source branch name" + }, + "target_branch": { + "type": "string", + "description": "The target branch name" + }, + "url": { + "type": "string", + "format": "uri", + "description": "Direct URL to the PR/MR" + }, + "author": { + "type": "string", + "description": "Username of the PR/MR author" + }, + "reviewers": { + "items": { + "properties": { + "login": { + "type": "string", + "description": "Username of the reviewer" + }, + "type": { + "type": "string", + "enum": [ + "User", + "Bot", + "unknown" + ], + "description": "Account type of the reviewer" + }, + "requested": { + "type": "boolean", + "description": "Whether the reviewer was explicitly requested to review" + }, + "review_status": { + "type": "string", + "enum": [ + "APPROVED", + "CHANGES_REQUESTED", + "COMMENTED", + "DISMISSED", + "PENDING" + ], + "description": "The reviewer's current review state if they have submitted a review" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "login", + "type", + "requested" + ] + }, + "type": "array", + "description": "List of reviewers who reviewed or were requested to review" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "platform", + "type", + "number", + "url" + ], + "title": "Pull Request / Merge Request Information", + "description": "Schema for Pull Request or Merge Request metadata collected during attestation" +} diff --git a/internal/schemavalidators/schemavalidators.go b/internal/schemavalidators/schemavalidators.go index 1356f6da7..e95473c6d 100644 --- a/internal/schemavalidators/schemavalidators.go +++ b/internal/schemavalidators/schemavalidators.go @@ -50,6 +50,8 @@ const ( PRInfoVersion1_0 PRInfoVersion = "1.0" // PRInfoVersion1_1 represents PR/MR Info version 1.1 schema (adds reviewers). PRInfoVersion1_1 PRInfoVersion = "1.1" + // PRInfoVersion1_2 represents PR/MR Info version 1.2 schema (adds requested and review_status to reviewers). + PRInfoVersion1_2 PRInfoVersion = "1.2" // CycloneDXVersion1_5 represents CycloneDX version 1.5 schema. CycloneDXVersion1_5 CycloneDXVersion = "1.5" // CycloneDXVersion1_6 represents CycloneDX version 1.6 schema. @@ -96,6 +98,8 @@ var ( prInfoSpecVersion1_0 string //go:embed internal_schemas/prinfo/pr-info-1.1.schema.json prInfoSpecVersion1_1 string + //go:embed internal_schemas/prinfo/pr-info-1.2.schema.json + prInfoSpecVersion1_2 string // AI Agent Config schemas //go:embed internal_schemas/aiagentconfig/ai-agent-config-0.1.schema.json @@ -120,6 +124,7 @@ var schemaURLMapping = map[string]string{ "https://chainloop.dev/schemas/runner-context-response-0.1.schema.json": runnerContextSpecVersion0_1, "https://schemas.chainloop.dev/prinfo/1.0/pr-info.schema.json": prInfoSpecVersion1_0, "https://schemas.chainloop.dev/prinfo/1.1/pr-info.schema.json": prInfoSpecVersion1_1, + "https://schemas.chainloop.dev/prinfo/1.2/pr-info.schema.json": prInfoSpecVersion1_2, "https://schemas.chainloop.dev/aiagentconfig/0.1/ai-agent-config.schema.json": aiAgentConfigSpecVersion0_1, } @@ -149,6 +154,7 @@ func init() { compiledPRInfoSchemas = make(map[PRInfoVersion]*jsonschema.Schema) compiledPRInfoSchemas[PRInfoVersion1_0] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.0/pr-info.schema.json") compiledPRInfoSchemas[PRInfoVersion1_1] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.1/pr-info.schema.json") + compiledPRInfoSchemas[PRInfoVersion1_2] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.2/pr-info.schema.json") compiledAIAgentConfigSchemas = make(map[AIAgentConfigVersion]*jsonschema.Schema) compiledAIAgentConfigSchemas[AIAgentConfigVersion0_1] = compiler.MustCompile("https://schemas.chainloop.dev/aiagentconfig/0.1/ai-agent-config.schema.json") @@ -246,7 +252,7 @@ func ValidateChainloopRunnerContext(data interface{}, version RunnerContextVersi // ValidatePRInfo validates the PR/MR info schema. func ValidatePRInfo(data interface{}, version PRInfoVersion) error { if version == "" { - version = PRInfoVersion1_1 + version = PRInfoVersion1_2 } schema, ok := compiledPRInfoSchemas[version] diff --git a/pkg/attestation/crafter/materials/chainloop_pr_info.go b/pkg/attestation/crafter/materials/chainloop_pr_info.go index 552a2b124..f13759fd0 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info.go @@ -1,5 +1,5 @@ // -// Copyright 2025 The Chainloop Authors. +// Copyright 2025-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -75,7 +75,7 @@ func (i *ChainloopPRInfoCrafter) Craft(ctx context.Context, artifactPath string) } // Validate the data against JSON schema - if err := schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_1); err != nil { + if err := schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_2); err != nil { i.logger.Debug().Err(err).Msg("schema validation failed") return nil, fmt.Errorf("PR info validation failed: %w", err) } diff --git a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go index 339b4498d..6daebb3cd 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go @@ -118,7 +118,7 @@ func TestChainloopPRInfoCrafter_Validation(t *testing.T) { require.NoError(t, err) // Validate the data against JSON schema - err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_1) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_2) if tc.wantErr { require.Error(t, err) diff --git a/pkg/attestation/crafter/prmetadata.go b/pkg/attestation/crafter/prmetadata.go index dec0e1f8f..72d6daef7 100644 --- a/pkg/attestation/crafter/prmetadata.go +++ b/pkg/attestation/crafter/prmetadata.go @@ -22,6 +22,7 @@ import ( "net/http" "net/url" "os" + "strings" "time" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" @@ -59,14 +60,14 @@ func DetectPRContext(ctx context.Context, runner SupportedRunner) (bool, *PRMeta switch runner.ID() { case schemaapi.CraftingSchema_Runner_GITHUB_ACTION: - return extractGitHubPRMetadata(envVars) + return extractGitHubPRMetadata(ctx, envVars) case schemaapi.CraftingSchema_Runner_GITLAB_PIPELINE: return extractGitLabMRMetadata(ctx, envVars) case schemaapi.CraftingSchema_Runner_DAGGER_PIPELINE: // When running in Dagger, check for parent CI context passed through as env vars // Try Github first if envVars["GITHUB_EVENT_NAME"] != "" { - return extractGitHubPRMetadata(envVars) + return extractGitHubPRMetadata(ctx, envVars) } // Then try Gitlab if envVars["CI_PIPELINE_SOURCE"] != "" { @@ -79,7 +80,7 @@ func DetectPRContext(ctx context.Context, runner SupportedRunner) (bool, *PRMeta } // extractGitHubPRMetadata reads GITHUB_EVENT_PATH JSON and extracts PR metadata -func extractGitHubPRMetadata(envVars map[string]string) (bool, *PRMetadata, error) { +func extractGitHubPRMetadata(ctx context.Context, envVars map[string]string) (bool, *PRMetadata, error) { eventName := envVars["GITHUB_EVENT_NAME"] // Check if this is a pull request event if eventName != "pull_request" && eventName != "pull_request_target" { @@ -118,18 +119,59 @@ func extractGitHubPRMetadata(envVars map[string]string) (bool, *PRMetadata, erro return false, nil, fmt.Errorf("failed to parse event JSON: %w", err) } + // GITHUB_TOKEN is read via os.Getenv to avoid persisting it in the attestation envVars map. + // GITHUB_API_BASE_URL can be overridden (e.g. in tests); defaults to api.github.com. + owner := envVars["GITHUB_REPOSITORY_OWNER"] + _, repo, _ := strings.Cut(envVars["GITHUB_REPOSITORY"], "/") + prNumber := fmt.Sprintf("%d", event.PullRequest.Number) + githubAPIBase := os.Getenv("GITHUB_API_BASE_URL") + if githubAPIBase == "" { + githubAPIBase = "https://api.github.com" + } + token := os.Getenv("GITHUB_TOKEN") + + // Seed the reviewer map from both the event payload and the API requested_reviewers endpoint. + // Both sources mark Requested: true. The event file is always available (no token needed), + // while the API may surface reviewers not yet reflected in the event (e.g. added after dispatch). + reviewerMap := make(map[string]int) // login → index in reviewers slice var reviewers []prinfo.Reviewer + for _, r := range event.PullRequest.RequestedReviewers { + if _, exists := reviewerMap[r.Login]; exists { + continue + } reviewerType := r.Type if reviewerType == "" { reviewerType = "unknown" } + reviewerMap[r.Login] = len(reviewers) reviewers = append(reviewers, prinfo.Reviewer{ - Login: r.Login, - Type: reviewerType, + Login: r.Login, + Type: reviewerType, + Requested: true, }) } + // Also fetch from the API: the event payload is a snapshot at dispatch time, so reviewers + // added after the event fires won't appear in event.PullRequest.RequestedReviewers. + // The API reflects current state and may include those late additions. + for _, r := range fetchGitHubRequestedReviewers(ctx, githubAPIBase, owner, repo, prNumber, token) { + if _, exists := reviewerMap[r.Login]; !exists { + reviewerMap[r.Login] = len(reviewers) + reviewers = append(reviewers, r) + } + } + + // Merge review activity: update status for existing entries, add new ones with Requested: false. + for _, r := range fetchGitHubReviews(ctx, githubAPIBase, owner, repo, prNumber, token) { + if idx, exists := reviewerMap[r.Login]; exists { + reviewers[idx].ReviewStatus = r.ReviewStatus + } else { + reviewerMap[r.Login] = len(reviewers) + reviewers = append(reviewers, r) + } + } + metadata := &PRMetadata{ Platform: "github", Type: "pull_request", @@ -146,6 +188,146 @@ func extractGitHubPRMetadata(envVars map[string]string) (bool, *PRMetadata, erro return true, metadata, nil } +// fetchGitHubRequestedReviewers fetches the list of users explicitly requested to review a PR. +// Returns nil on any failure (best-effort). +// baseURL is the GitHub API base (e.g. "https://api.github.com"); can be overridden in tests. +func fetchGitHubRequestedReviewers(ctx context.Context, baseURL, owner, repo, prNumber, token string) []prinfo.Reviewer { + if baseURL == "" || owner == "" || repo == "" || prNumber == "" || token == "" { + return nil + } + + apiURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%s/requested_reviewers", baseURL, owner, repo, prNumber) + + client := &http.Client{Timeout: 10 * time.Second} + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return nil + } + req.Header.Set("Authorization", "Bearer "+token) + + resp, err := client.Do(req) + if err != nil { + return nil + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil + } + + var result struct { + Users []prinfo.Reviewer `json:"users"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil + } + + if len(result.Users) == 0 { + return nil + } + + for i := range result.Users { + result.Users[i].Requested = true + if result.Users[i].Type == "" { + result.Users[i].Type = "unknown" + } + } + return result.Users +} + +// fetchGitHubReviews fetches all PR reviews from the GitHub API, following pagination. +// Returns nil on any failure (best-effort). +// Reviews are deduplicated by login, keeping the most recent state (last entry wins). +// Returned reviewers have Requested: false; callers should set Requested: true for those +// already present in the requested_reviewers list. +// baseURL is the GitHub API base (e.g. "https://api.github.com"); can be overridden in tests. +func fetchGitHubReviews(ctx context.Context, baseURL, owner, repo, prNumber, token string) []prinfo.Reviewer { + if baseURL == "" || owner == "" || repo == "" || prNumber == "" || token == "" { + return nil + } + + client := &http.Client{Timeout: 10 * time.Second} + + // Deduplicate by login, keeping insertion order but updating to the most recent state. + seen := make(map[string]int) // login → index + var reviewers []prinfo.Reviewer + + nextURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%s/reviews?per_page=100", baseURL, owner, repo, prNumber) + for nextURL != "" { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, nextURL, nil) + if err != nil { + return nil + } + req.Header.Set("Authorization", "Bearer "+token) + + resp, err := client.Do(req) + if err != nil { + return nil + } + + if resp.StatusCode != http.StatusOK { + resp.Body.Close() + return nil + } + + var page []struct { + User struct { + Login string `json:"login"` + Type string `json:"type"` + } `json:"user"` + State string `json:"state"` + } + err = json.NewDecoder(resp.Body).Decode(&page) + nextURL = nextPageURL(resp.Header.Get("Link")) + resp.Body.Close() + if err != nil { + return nil + } + + for _, r := range page { + reviewerType := r.User.Type + if reviewerType == "" { + reviewerType = "unknown" + } + if idx, exists := seen[r.User.Login]; exists { + reviewers[idx].ReviewStatus = r.State + continue + } + seen[r.User.Login] = len(reviewers) + reviewers = append(reviewers, prinfo.Reviewer{ + Login: r.User.Login, + Type: reviewerType, + ReviewStatus: r.State, + }) + } + } + + if len(reviewers) == 0 { + return nil + } + return reviewers +} + +// nextPageURL parses the GitHub Link header and returns the URL for the next page, +// or an empty string if there is no next page. +// Link header format: ; rel="next", ; rel="last" +func nextPageURL(linkHeader string) string { + for _, part := range strings.Split(linkHeader, ",") { + part = strings.TrimSpace(part) + sections := strings.SplitN(part, ";", 2) + if len(sections) < 2 { + continue + } + urlPart := strings.TrimSpace(sections[0]) + relPart := strings.TrimSpace(sections[1]) + if relPart == `rel="next"` && len(urlPart) >= 2 { + return urlPart[1 : len(urlPart)-1] // strip angle brackets + } + } + return "" +} + // extractGitLabMRMetadata extracts from GitLab environment variables func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bool, *PRMetadata, error) { pipelineSource := envVars["CI_PIPELINE_SOURCE"] @@ -228,8 +410,9 @@ func fetchGitLabReviewers(ctx context.Context, baseURL, projectPath, mrIID, toke var reviewers []prinfo.Reviewer for _, r := range mrResponse.Reviewers { reviewers = append(reviewers, prinfo.Reviewer{ - Login: r.Username, - Type: "unknown", + Login: r.Username, + Type: "unknown", + Requested: true, }) } diff --git a/pkg/attestation/crafter/prmetadata_test.go b/pkg/attestation/crafter/prmetadata_test.go index 9a32d9df7..f305267b3 100644 --- a/pkg/attestation/crafter/prmetadata_test.go +++ b/pkg/attestation/crafter/prmetadata_test.go @@ -38,7 +38,7 @@ func TestExtractGitHubPRMetadata(t *testing.T) { validate func(t *testing.T, metadata *PRMetadata) }{ { - name: "valid pull_request event with reviewers", + name: "valid pull_request event with reviewers from event file", envVars: map[string]string{ "GITHUB_EVENT_NAME": "pull_request", "GITHUB_EVENT_PATH": filepath.Join("testdata", "github_pr_event.json"), @@ -57,11 +57,14 @@ func TestExtractGitHubPRMetadata(t *testing.T) { assert.Equal(t, "main", metadata.TargetBranch) assert.Equal(t, "https://github.com/owner/repo/pull/123", metadata.URL) assert.Equal(t, "johndoe", metadata.Author) + // Event file provides requested_reviewers without needing a token. require.Len(t, metadata.Reviewers, 2) assert.Equal(t, "reviewer1", metadata.Reviewers[0].Login) assert.Equal(t, "User", metadata.Reviewers[0].Type) + assert.True(t, metadata.Reviewers[0].Requested) assert.Equal(t, "coderabbitai", metadata.Reviewers[1].Login) assert.Equal(t, "Bot", metadata.Reviewers[1].Type) + assert.True(t, metadata.Reviewers[1].Requested) }, }, { @@ -112,7 +115,7 @@ func TestExtractGitHubPRMetadata(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - isPR, metadata, err := extractGitHubPRMetadata(tc.envVars) + isPR, metadata, err := extractGitHubPRMetadata(context.Background(), tc.envVars) if tc.expectError { require.Error(t, err) @@ -225,8 +228,8 @@ func TestFetchGitLabReviewers(t *testing.T) { mrIID: "10", token: "test-token", expected: []prinfo.Reviewer{ - {Login: "alice", Type: "unknown"}, - {Login: "bot-reviewer", Type: "unknown"}, + {Login: "alice", Type: "unknown", Requested: true}, + {Login: "bot-reviewer", Type: "unknown", Requested: true}, }, }, { @@ -334,5 +337,345 @@ func TestExtractGitLabMRMetadataWithReviewers(t *testing.T) { require.Len(t, metadata.Reviewers, 2) assert.Equal(t, "alice", metadata.Reviewers[0].Login) assert.Equal(t, "unknown", metadata.Reviewers[0].Type) + assert.True(t, metadata.Reviewers[0].Requested) assert.Equal(t, "coderabbitai", metadata.Reviewers[1].Login) + assert.True(t, metadata.Reviewers[1].Requested) +} + +func TestFetchGitHubReviews(t *testing.T) { + testCases := []struct { + name string + handler http.HandlerFunc + owner string + repo string + prNumber string + token string + expected []prinfo.Reviewer + }{ + { + name: "successful response, last state wins for duplicate user", + handler: func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + w.Header().Set("Content-Type", "application/json") + // reviewer1 submits two reviews with different states; last state wins + fmt.Fprint(w, `[{"user":{"login":"reviewer1","type":"User"},"state":"COMMENTED"},{"user":{"login":"reviewer1","type":"User"},"state":"APPROVED"},{"user":{"login":"bot-reviewer","type":"Bot"},"state":"COMMENTED"}]`) + }, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: []prinfo.Reviewer{ + {Login: "reviewer1", Type: "User", ReviewStatus: "APPROVED"}, + {Login: "bot-reviewer", Type: "Bot", ReviewStatus: "COMMENTED"}, + }, + }, + { + name: "empty reviews array", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `[]`) + }, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: nil, + }, + { + name: "API returns error", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + }, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: nil, + }, + { + name: "missing token", + handler: nil, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "", + expected: nil, + }, + { + name: "missing owner", + handler: nil, + owner: "", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var baseURL string + if tc.handler != nil { + server := httptest.NewServer(tc.handler) + defer server.Close() + baseURL = server.URL + } + + reviewers := fetchGitHubReviews(context.Background(), baseURL, tc.owner, tc.repo, tc.prNumber, tc.token) + + if tc.expected == nil { + assert.Nil(t, reviewers) + } else { + assert.Equal(t, tc.expected, reviewers) + } + }) + } +} + +func TestFetchGitHubReviewsRequestPath(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/repos/owner/repo/pulls/42/reviews", r.URL.Path) + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode([]any{})) + })) + defer server.Close() + + fetchGitHubReviews(context.Background(), server.URL, "owner", "repo", "42", "test-token") +} + +func TestFetchGitHubReviewsPagination(t *testing.T) { + page := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + page++ + switch page { + case 1: + // Set Link header pointing to page 2 + w.Header().Set("Link", fmt.Sprintf(`<%s/repos/owner/repo/pulls/42/reviews?per_page=100&page=2>; rel="next"`, r.Host)) + fmt.Fprint(w, `[{"user":{"login":"reviewer1","type":"User"},"state":"COMMENTED"}]`) + case 2: + // Last page — no Link header + fmt.Fprint(w, `[{"user":{"login":"reviewer2","type":"User"},"state":"APPROVED"},{"user":{"login":"reviewer1","type":"User"},"state":"APPROVED"}]`) + } + })) + defer server.Close() + + // Fix up the Link header host to use the test server URL + page = 0 + server2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + page++ + switch page { + case 1: + w.Header().Set("Link", fmt.Sprintf(`<%s/repos/owner/repo/pulls/42/reviews?per_page=100&page=2>; rel="next"`, server.URL)) + fmt.Fprint(w, `[{"user":{"login":"reviewer1","type":"User"},"state":"COMMENTED"}]`) + case 2: + fmt.Fprint(w, `[{"user":{"login":"reviewer2","type":"User"},"state":"APPROVED"},{"user":{"login":"reviewer1","type":"User"},"state":"APPROVED"}]`) + } + })) + defer server2.Close() + + reviewers := fetchGitHubReviews(context.Background(), server2.URL, "owner", "repo", "42", "test-token") + require.Len(t, reviewers, 2) + // reviewer1 appears on both pages; last state (APPROVED from page 2) wins + assert.Equal(t, "reviewer1", reviewers[0].Login) + assert.Equal(t, "APPROVED", reviewers[0].ReviewStatus) + assert.Equal(t, "reviewer2", reviewers[1].Login) + assert.Equal(t, "APPROVED", reviewers[1].ReviewStatus) +} + +func TestNextPageURL(t *testing.T) { + testCases := []struct { + name string + header string + expected string + }{ + { + name: "next page present", + header: `; rel="next", ; rel="last"`, + expected: "https://api.github.com/repos/o/r/pulls/1/reviews?per_page=100&page=2", + }, + { + name: "no next page", + header: `; rel="first"`, + expected: "", + }, + { + name: "empty header", + header: "", + expected: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, nextPageURL(tc.header)) + }) + } +} + +func TestFetchGitHubRequestedReviewers(t *testing.T) { + testCases := []struct { + name string + handler http.HandlerFunc + owner string + repo string + prNumber string + token string + expected []prinfo.Reviewer + }{ + { + name: "successful response with users", + handler: func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"users":[{"login":"reviewer1","type":"User"},{"login":"copilot-pull-request-reviewer[bot]","type":"Bot"}],"teams":[]}`) + }, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: []prinfo.Reviewer{ + {Login: "reviewer1", Type: "User", Requested: true}, + {Login: "copilot-pull-request-reviewer[bot]", Type: "Bot", Requested: true}, + }, + }, + { + name: "empty users array", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"users":[],"teams":[]}`) + }, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: nil, + }, + { + name: "API returns error", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + }, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: nil, + }, + { + name: "missing token", + handler: nil, + owner: "owner", + repo: "repo", + prNumber: "42", + token: "", + expected: nil, + }, + { + name: "missing owner", + handler: nil, + owner: "", + repo: "repo", + prNumber: "42", + token: "test-token", + expected: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var baseURL string + if tc.handler != nil { + server := httptest.NewServer(tc.handler) + defer server.Close() + baseURL = server.URL + } + + reviewers := fetchGitHubRequestedReviewers(context.Background(), baseURL, tc.owner, tc.repo, tc.prNumber, tc.token) + + if tc.expected == nil { + assert.Nil(t, reviewers) + } else { + assert.Equal(t, tc.expected, reviewers) + } + }) + } +} + +func TestFetchGitHubRequestedReviewersRequestPath(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/repos/owner/repo/pulls/42/requested_reviewers", r.URL.Path) + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"users": []any{}, "teams": []any{}})) + })) + defer server.Close() + + fetchGitHubRequestedReviewers(context.Background(), server.URL, "owner", "repo", "42", "test-token") +} + +func TestExtractGitHubPRMetadataWithAPIReviews(t *testing.T) { + // Mock server handles both /requested_reviewers and /reviews endpoints. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/repos/owner/repo/pulls/123/requested_reviewers": + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + // reviewer1 and coderabbitai are already in the event file; newreviewer is only in the API. + // The event file entry for reviewer1/coderabbitai wins on dedup (inserted first). + fmt.Fprint(w, `{"users":[{"login":"reviewer1","type":"User"},{"login":"coderabbitai","type":"Bot"},{"login":"newreviewer","type":"User"}],"teams":[]}`) + case "/repos/owner/repo/pulls/123/reviews": + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + // reviewer2 has reviewed (not in requested); reviewer1 has also reviewed. + fmt.Fprint(w, `[{"user":{"login":"reviewer2","type":"User"},"state":"APPROVED"},{"user":{"login":"reviewer1","type":"User"},"state":"COMMENTED"}]`) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + t.Setenv("GITHUB_TOKEN", "test-token") + t.Setenv("GITHUB_API_BASE_URL", server.URL) + + envVars := map[string]string{ + "GITHUB_EVENT_NAME": "pull_request", + // github_pr_event.json has requested_reviewers: reviewer1 (User), coderabbitai (Bot) + "GITHUB_EVENT_PATH": filepath.Join("testdata", "github_pr_event.json"), + "GITHUB_HEAD_REF": "feature-branch", + "GITHUB_BASE_REF": "main", + "GITHUB_REPOSITORY": "owner/repo", + "GITHUB_REPOSITORY_OWNER": "owner", + } + + isPR, metadata, err := extractGitHubPRMetadata(context.Background(), envVars) + require.NoError(t, err) + require.True(t, isPR) + + // reviewer1: event file + API requested + has reviewed → Requested: true, ReviewStatus: COMMENTED + // coderabbitai: event file + API requested, no review → Requested: true, ReviewStatus: "" + // newreviewer: API requested only, no review → Requested: true, ReviewStatus: "" + // reviewer2: not requested, has reviewed → Requested: false, ReviewStatus: APPROVED + require.Len(t, metadata.Reviewers, 4) + + assert.Equal(t, "reviewer1", metadata.Reviewers[0].Login) + assert.Equal(t, "User", metadata.Reviewers[0].Type) + assert.True(t, metadata.Reviewers[0].Requested) + assert.Equal(t, "COMMENTED", metadata.Reviewers[0].ReviewStatus) + + assert.Equal(t, "coderabbitai", metadata.Reviewers[1].Login) + assert.Equal(t, "Bot", metadata.Reviewers[1].Type) + assert.True(t, metadata.Reviewers[1].Requested) + assert.Equal(t, "", metadata.Reviewers[1].ReviewStatus) + + assert.Equal(t, "newreviewer", metadata.Reviewers[2].Login) + assert.Equal(t, "User", metadata.Reviewers[2].Type) + assert.True(t, metadata.Reviewers[2].Requested) + assert.Equal(t, "", metadata.Reviewers[2].ReviewStatus) + + assert.Equal(t, "reviewer2", metadata.Reviewers[3].Login) + assert.Equal(t, "User", metadata.Reviewers[3].Type) + assert.False(t, metadata.Reviewers[3].Requested) + assert.Equal(t, "APPROVED", metadata.Reviewers[3].ReviewStatus) }