diff --git a/internal/prinfo/prinfo.go b/internal/prinfo/prinfo.go index 0c69efb30..d8686f7e5 100644 --- a/internal/prinfo/prinfo.go +++ b/internal/prinfo/prinfo.go @@ -15,13 +15,31 @@ package prinfo +import ( + "encoding/json" + "fmt" +) + 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.2/pr-info.schema.json" + EvidenceSchemaURL = "https://schemas.chainloop.dev/prinfo/1.3/pr-info.schema.json" + + // AuthorTypeUser represents a human user account + AuthorTypeUser = "User" + // AuthorTypeBot represents a bot/service account + AuthorTypeBot = "Bot" + // AuthorTypeUnknown represents an account with unknown type + AuthorTypeUnknown = "unknown" ) +// Author represents the author of the PR/MR with account type information +type Author struct { + Login string `json:"login" jsonschema:"required,description=Username of the PR/MR author"` + Type string `json:"type" jsonschema:"required,enum=User,enum=Bot,enum=unknown,description=Account type of the PR/MR author"` +} + // Reviewer represents a reviewer of the PR/MR type Reviewer struct { Login string `json:"login" jsonschema:"required,description=Username of the reviewer"` @@ -40,10 +58,48 @@ type Data struct { SourceBranch string `json:"source_branch" jsonschema:"description=The source branch name"` TargetBranch string `json:"target_branch" jsonschema:"description=The target branch name"` URL string `json:"url" jsonschema:"required,format=uri,description=Direct URL to the PR/MR"` - Author string `json:"author" jsonschema:"description=Username of the PR/MR author"` + Author *Author `json:"author,omitempty" jsonschema:"description=The PR/MR author"` Reviewers []Reviewer `json:"reviewers,omitempty" jsonschema:"description=List of reviewers who reviewed or were requested to review"` } +// UnmarshalJSON implements custom JSON unmarshaling for Data to handle +// backwards compatibility where author can be either a string (v1.0-v1.2) +// or an object (v1.3+). +func (d *Data) UnmarshalJSON(b []byte) error { + // Use an alias to avoid infinite recursion + type Alias Data + aux := &struct { + Author json.RawMessage `json:"author,omitempty"` + *Alias + }{ + Alias: (*Alias)(d), + } + + if err := json.Unmarshal(b, aux); err != nil { + return err + } + + if len(aux.Author) == 0 || string(aux.Author) == "null" { + return nil + } + + // Try object format first + var author Author + if err := json.Unmarshal(aux.Author, &author); err == nil { + d.Author = &author + return nil + } + + // Fall back to string format + var login string + if err := json.Unmarshal(aux.Author, &login); err == nil { + d.Author = &Author{Login: login, Type: AuthorTypeUnknown} + return nil + } + + return fmt.Errorf("author field must be a string or an object with login and type fields") +} + // Evidence represents the complete evidence structure for PR/MR metadata type Evidence struct { ID string `json:"chainloop.material.evidence.id"` diff --git a/internal/prinfo/prinfo_test.go b/internal/prinfo/prinfo_test.go index d3f206730..098d121a8 100644 --- a/internal/prinfo/prinfo_test.go +++ b/internal/prinfo/prinfo_test.go @@ -308,3 +308,133 @@ func TestValidatePRInfoV1_0BackwardCompat(t *testing.T) { }) } } + +func TestDataUnmarshalJSON(t *testing.T) { + testCases := []struct { + name string + input string + wantAuthor *Author + wantErr bool + }{ + { + name: "string author (v1.0-v1.2 backwards compat)", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":"dependabot[bot]"}`, + wantAuthor: &Author{Login: "dependabot[bot]", Type: "unknown"}, + }, + { + name: "object author (v1.3)", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":{"login":"dependabot[bot]","type":"Bot"}}`, + wantAuthor: &Author{Login: "dependabot[bot]", Type: "Bot"}, + }, + { + name: "no author field", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1"}`, + wantAuthor: nil, + }, + { + name: "null author field", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":null}`, + wantAuthor: nil, + }, + { + name: "invalid author type", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":123}`, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var data Data + err := json.Unmarshal([]byte(tc.input), &data) + if tc.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.wantAuthor, data.Author) + }) + } +} + +func TestValidatePRInfoV1_3(t *testing.T) { + testCases := []struct { + name string + data string + wantErr bool + }{ + { + name: "v1.3 author as object", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": {"login": "dependabot[bot]", "type": "Bot"} + }`, + wantErr: false, + }, + { + name: "v1.3 author as string (backwards compat)", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": "username" + }`, + wantErr: false, + }, + { + name: "v1.3 author object missing type", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": {"login": "username"} + }`, + wantErr: true, + }, + { + name: "v1.3 author object invalid type", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": {"login": "username", "type": "InvalidType"} + }`, + wantErr: true, + }, + { + name: "v1.3 with reviewers and object author", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "author": {"login": "renovate[bot]", "type": "Bot"}, + "reviewers": [ + {"login": "reviewer1", "type": "User", "requested": true, "review_status": "APPROVED"} + ] + }`, + wantErr: false, + }, + } + + 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_3) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.3.schema.json b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.3.schema.json new file mode 100644 index 000000000..3e185d6ee --- /dev/null +++ b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.3.schema.json @@ -0,0 +1,133 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://schemas.chainloop.dev/prinfo/1.3/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": { + "oneOf": [ + { + "type": "string", + "description": "Username of the PR/MR author (deprecated, use object form)" + }, + { + "type": "object", + "properties": { + "login": { + "type": "string", + "description": "Username of the PR/MR author" + }, + "type": { + "type": "string", + "enum": [ + "User", + "Bot", + "unknown" + ], + "description": "Account type of the PR/MR author" + } + }, + "required": [ + "login", + "type" + ], + "additionalProperties": false, + "description": "The PR/MR author with account type" + } + ], + "description": "The PR/MR author (string for backwards compatibility, or object with login and type)" + }, + "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 e95473c6d..594f9d4b5 100644 --- a/internal/schemavalidators/schemavalidators.go +++ b/internal/schemavalidators/schemavalidators.go @@ -52,6 +52,8 @@ const ( 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" + // PRInfoVersion1_3 represents PR/MR Info version 1.3 schema (author as object with type). + PRInfoVersion1_3 PRInfoVersion = "1.3" // CycloneDXVersion1_5 represents CycloneDX version 1.5 schema. CycloneDXVersion1_5 CycloneDXVersion = "1.5" // CycloneDXVersion1_6 represents CycloneDX version 1.6 schema. @@ -100,6 +102,8 @@ var ( prInfoSpecVersion1_1 string //go:embed internal_schemas/prinfo/pr-info-1.2.schema.json prInfoSpecVersion1_2 string + //go:embed internal_schemas/prinfo/pr-info-1.3.schema.json + prInfoSpecVersion1_3 string // AI Agent Config schemas //go:embed internal_schemas/aiagentconfig/ai-agent-config-0.1.schema.json @@ -125,6 +129,7 @@ var schemaURLMapping = map[string]string{ "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/prinfo/1.3/pr-info.schema.json": prInfoSpecVersion1_3, "https://schemas.chainloop.dev/aiagentconfig/0.1/ai-agent-config.schema.json": aiAgentConfigSpecVersion0_1, } @@ -155,6 +160,7 @@ func init() { 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") + compiledPRInfoSchemas[PRInfoVersion1_3] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.3/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") @@ -252,7 +258,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_2 + version = PRInfoVersion1_3 } 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 f13759fd0..e5a2c421d 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info.go @@ -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_2); err != nil { + if err := schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3); 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 6daebb3cd..8e433819e 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go @@ -47,7 +47,7 @@ func TestChainloopPRInfoCrafter_Validation(t *testing.T) { SourceBranch: "feature", TargetBranch: "main", URL: "https://github.com/org/repo/pull/123", - Author: "testuser", + Author: &prinfo.Author{Login: "testuser", Type: "User"}, }), wantErr: false, }, @@ -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_2) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3) if tc.wantErr { require.Error(t, err) @@ -129,24 +129,36 @@ func TestChainloopPRInfoCrafter_Validation(t *testing.T) { } } -func TestChainloopPRInfoCrafter_V1_0BackwardCompat(t *testing.T) { - // Data without reviewers should still validate against v1.0 - data := prinfo.Data{ - Platform: "github", - Type: "pull_request", - Number: "123", - URL: "https://github.com/org/repo/pull/123", - Author: "testuser", - } +func TestChainloopPRInfoCrafter_BackwardCompat(t *testing.T) { + // Old string author format should still validate against v1.3 + oldFormatJSON := `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/org/repo/pull/123", + "author": "testuser" + }` - dataBytes, err := json.Marshal(data) + var rawData interface{} + err := json.Unmarshal([]byte(oldFormatJSON), &rawData) require.NoError(t, err) - var rawData interface{} - err = json.Unmarshal(dataBytes, &rawData) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3) + require.NoError(t, err) + + // New object author format should also validate against v1.3 + newFormatJSON := `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/org/repo/pull/123", + "author": {"login": "dependabot[bot]", "type": "Bot"} + }` + + err = json.Unmarshal([]byte(newFormatJSON), &rawData) require.NoError(t, err) - err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_0) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3) require.NoError(t, err) } diff --git a/pkg/attestation/crafter/prmetadata.go b/pkg/attestation/crafter/prmetadata.go index 72d6daef7..5b62702d3 100644 --- a/pkg/attestation/crafter/prmetadata.go +++ b/pkg/attestation/crafter/prmetadata.go @@ -39,7 +39,7 @@ type PRMetadata struct { SourceBranch string TargetBranch string URL string - Author string + Author *prinfo.Author Reviewers []prinfo.Reviewer } @@ -107,6 +107,7 @@ func extractGitHubPRMetadata(ctx context.Context, envVars map[string]string) (bo HTMLURL string `json:"html_url"` User struct { Login string `json:"login"` + Type string `json:"type"` } `json:"user"` RequestedReviewers []struct { Login string `json:"login"` @@ -181,8 +182,11 @@ func extractGitHubPRMetadata(ctx context.Context, envVars map[string]string) (bo SourceBranch: envVars["GITHUB_HEAD_REF"], TargetBranch: envVars["GITHUB_BASE_REF"], URL: event.PullRequest.HTMLURL, - Author: event.PullRequest.User.Login, - Reviewers: reviewers, + Author: &prinfo.Author{ + Login: event.PullRequest.User.Login, + Type: normalizeAuthorType(event.PullRequest.User.Type), + }, + Reviewers: reviewers, } return true, metadata, nil @@ -345,14 +349,27 @@ func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bo projectURL := envVars["CI_MERGE_REQUEST_PROJECT_URL"] mrURL := fmt.Sprintf("%s/-/merge_requests/%s", projectURL, mrIID) - // Fetch reviewers from GitLab API (best-effort). + // Fetch MR details (author + reviewers) from GitLab API (best-effort). // Prefer CI_MERGE_REQUEST_PROJECT_PATH for fork-based MRs where CI_PROJECT_PATH points to the fork. projectPath := envVars["CI_MERGE_REQUEST_PROJECT_PATH"] if projectPath == "" { projectPath = envVars["CI_PROJECT_PATH"] } // CI_JOB_TOKEN is read via os.Getenv to avoid persisting it in the attestation envVars map. - reviewers := fetchGitLabReviewers(ctx, envVars["CI_SERVER_URL"], projectPath, mrIID, os.Getenv("CI_JOB_TOKEN")) + mrDetails := fetchGitLabMRDetails(ctx, envVars["CI_SERVER_URL"], projectPath, mrIID, os.Getenv("CI_JOB_TOKEN")) + + // Use API author if available (includes bot detection), fall back to env var + author := &prinfo.Author{ + Login: envVars["GITLAB_USER_LOGIN"], + Type: "unknown", + } + var reviewers []prinfo.Reviewer + if mrDetails != nil { + if mrDetails.Author != nil { + author = mrDetails.Author + } + reviewers = mrDetails.Reviewers + } metadata := &PRMetadata{ Platform: "gitlab", @@ -363,16 +380,22 @@ func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bo SourceBranch: envVars["CI_MERGE_REQUEST_SOURCE_BRANCH_NAME"], TargetBranch: envVars["CI_MERGE_REQUEST_TARGET_BRANCH_NAME"], URL: mrURL, - Author: envVars["GITLAB_USER_LOGIN"], + Author: author, Reviewers: reviewers, } return true, metadata, nil } -// fetchGitLabReviewers fetches MR reviewers from the GitLab API. +// gitlabMRDetails holds the author and reviewer data extracted from the GitLab MR API. +type gitlabMRDetails struct { + Author *prinfo.Author + Reviewers []prinfo.Reviewer +} + +// fetchGitLabMRDetails fetches MR details (author + reviewers) from the GitLab API. // Returns nil on any failure (best-effort). -func fetchGitLabReviewers(ctx context.Context, baseURL, projectPath, mrIID, token string) []prinfo.Reviewer { +func fetchGitLabMRDetails(ctx context.Context, baseURL, projectPath, mrIID, token string) *gitlabMRDetails { if baseURL == "" || projectPath == "" || token == "" { return nil } @@ -399,22 +422,56 @@ func fetchGitLabReviewers(ctx context.Context, baseURL, projectPath, mrIID, toke } var mrResponse struct { + Author struct { + Username string `json:"username"` + Bot bool `json:"bot"` + } `json:"author"` Reviewers []struct { Username string `json:"username"` + Bot bool `json:"bot"` } `json:"reviewers"` } if err := json.NewDecoder(resp.Body).Decode(&mrResponse); err != nil { return nil } - var reviewers []prinfo.Reviewer + details := &gitlabMRDetails{} + + // Extract author with bot detection + if mrResponse.Author.Username != "" { + authorType := prinfo.AuthorTypeUser + if mrResponse.Author.Bot { + authorType = prinfo.AuthorTypeBot + } + details.Author = &prinfo.Author{ + Login: mrResponse.Author.Username, + Type: authorType, + } + } + + // Extract reviewers with bot detection for _, r := range mrResponse.Reviewers { - reviewers = append(reviewers, prinfo.Reviewer{ + reviewerType := prinfo.AuthorTypeUser + if r.Bot { + reviewerType = prinfo.AuthorTypeBot + } + details.Reviewers = append(details.Reviewers, prinfo.Reviewer{ Login: r.Username, - Type: "unknown", + Type: reviewerType, Requested: true, }) } - return reviewers + return details +} + +// normalizeAuthorType normalizes the author type string from the CI platform +// to one of the allowed values: "User", "Bot", or "unknown". +func normalizeAuthorType(authorType string) string { + switch authorType { + case prinfo.AuthorTypeUser, prinfo.AuthorTypeBot: + return authorType + default: + return prinfo.AuthorTypeUnknown + } } diff --git a/pkg/attestation/crafter/prmetadata_test.go b/pkg/attestation/crafter/prmetadata_test.go index f305267b3..017996a91 100644 --- a/pkg/attestation/crafter/prmetadata_test.go +++ b/pkg/attestation/crafter/prmetadata_test.go @@ -56,7 +56,7 @@ func TestExtractGitHubPRMetadata(t *testing.T) { assert.Equal(t, "feature-branch", metadata.SourceBranch) assert.Equal(t, "main", metadata.TargetBranch) assert.Equal(t, "https://github.com/owner/repo/pull/123", metadata.URL) - assert.Equal(t, "johndoe", metadata.Author) + assert.Equal(t, &prinfo.Author{Login: "johndoe", Type: "unknown"}, 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) @@ -80,7 +80,7 @@ func TestExtractGitHubPRMetadata(t *testing.T) { validate: func(t *testing.T, metadata *PRMetadata) { assert.Equal(t, "github", metadata.Platform) assert.Equal(t, "456", metadata.Number) - assert.Equal(t, "janedoe", metadata.Author) + assert.Equal(t, &prinfo.Author{Login: "janedoe", Type: "unknown"}, metadata.Author) assert.Empty(t, metadata.Reviewers) }, }, @@ -164,7 +164,7 @@ func TestExtractGitLabMRMetadata(t *testing.T) { assert.Equal(t, "feature-branch", metadata.SourceBranch) assert.Equal(t, "main", metadata.TargetBranch) assert.Equal(t, "https://gitlab.com/owner/repo/-/merge_requests/42", metadata.URL) - assert.Equal(t, "testuser", metadata.Author) + assert.Equal(t, &prinfo.Author{Login: "testuser", Type: "unknown"}, metadata.Author) // No reviewers without API access in tests assert.Empty(t, metadata.Reviewers) }, @@ -207,7 +207,7 @@ func TestExtractGitLabMRMetadata(t *testing.T) { } } -func TestFetchGitLabReviewers(t *testing.T) { +func TestFetchGitLabMRDetails(t *testing.T) { testCases := []struct { name string handler http.HandlerFunc @@ -215,33 +215,38 @@ func TestFetchGitLabReviewers(t *testing.T) { projectPath string mrIID string token string - expected []prinfo.Reviewer + expected *gitlabMRDetails }{ { name: "successful response with reviewers", handler: func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "test-token", r.Header.Get("JOB-TOKEN")) w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"reviewers": [{"username": "alice"}, {"username": "bot-reviewer"}]}`) + fmt.Fprint(w, `{"author": {"username": "mr-author", "bot": false}, "reviewers": [{"username": "alice", "bot": false}, {"username": "bot-reviewer", "bot": true}]}`) }, projectPath: "group/project", mrIID: "10", token: "test-token", - expected: []prinfo.Reviewer{ - {Login: "alice", Type: "unknown", Requested: true}, - {Login: "bot-reviewer", Type: "unknown", Requested: true}, + expected: &gitlabMRDetails{ + Author: &prinfo.Author{Login: "mr-author", Type: "User"}, + Reviewers: []prinfo.Reviewer{ + {Login: "alice", Type: "User", Requested: true}, + {Login: "bot-reviewer", Type: "Bot", Requested: true}, + }, }, }, { name: "empty reviewers", handler: func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"reviewers": []}`) + fmt.Fprint(w, `{"author": {"username": "author", "bot": false}, "reviewers": []}`) }, projectPath: "group/project", mrIID: "10", token: "test-token", - expected: nil, + expected: &gitlabMRDetails{ + Author: &prinfo.Author{Login: "author", Type: "User"}, + }, }, { name: "API returns error", @@ -286,18 +291,18 @@ func TestFetchGitLabReviewers(t *testing.T) { baseURL = serverURL } - reviewers := fetchGitLabReviewers(context.Background(), baseURL, tc.projectPath, tc.mrIID, tc.token) + details := fetchGitLabMRDetails(context.Background(), baseURL, tc.projectPath, tc.mrIID, tc.token) if tc.expected == nil { - assert.Nil(t, reviewers) + assert.Nil(t, details) } else { - assert.Equal(t, tc.expected, reviewers) + assert.Equal(t, tc.expected, details) } }) } } -func TestFetchGitLabReviewersRequestPath(t *testing.T) { +func TestFetchGitLabMRDetailsRequestPath(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/api/v4/projects/group%2Fproject/merge_requests/42", r.URL.RawPath) w.Header().Set("Content-Type", "application/json") @@ -305,14 +310,14 @@ func TestFetchGitLabReviewersRequestPath(t *testing.T) { })) defer server.Close() - fetchGitLabReviewers(context.Background(), server.URL, "group/project", "42", "token") + fetchGitLabMRDetails(context.Background(), server.URL, "group/project", "42", "token") } func TestExtractGitLabMRMetadataWithReviewers(t *testing.T) { // Set up a mock GitLab API that returns reviewers server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"reviewers": [{"username": "alice"}, {"username": "coderabbitai"}]}`) + fmt.Fprint(w, `{"author": {"username": "author", "bot": false}, "reviewers": [{"username": "alice", "bot": false}, {"username": "coderabbitai", "bot": true}]}`) })) defer server.Close() @@ -336,10 +341,15 @@ func TestExtractGitLabMRMetadataWithReviewers(t *testing.T) { require.True(t, isMR) require.Len(t, metadata.Reviewers, 2) assert.Equal(t, "alice", metadata.Reviewers[0].Login) - assert.Equal(t, "unknown", metadata.Reviewers[0].Type) + 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) + // Author should come from the API, not env var + require.NotNil(t, metadata.Author) + assert.Equal(t, "author", metadata.Author.Login) + assert.Equal(t, "User", metadata.Author.Type) } func TestFetchGitHubReviews(t *testing.T) {