Skip to content

feat(attestation): expand PR reviewers with requested status and review state #2894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
javirln wants to merge 1 commit into from
Closed

feat(attestation): expand PR reviewers with requested status and review state #2894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions internal/prinfo/prinfo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:"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
Expand Down
78 changes: 78 additions & 0 deletions internal/prinfo/prinfo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: "backward compat: reviewer without new fields is still valid",
data: `{
"platform": "github",
"type": "pull_request",
"number": "789",
"url": "https://github.com/owner/repo/pull/789",
"reviewers": [
{"login": "reviewer1", "type": "User"}
]
}`,
wantErr: false,
},
{
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
Expand Down
6 changes: 3 additions & 3 deletions internal/prinfo/schemas/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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()
Expand Down
103 changes: 103 additions & 0 deletions internal/schemavalidators/internal_schemas/prinfo/pr-info-1.2.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
{
"$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": [

@cubic-dev-ai cubic-dev-ai Bot Mar 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Require reviewers[].requested in the schema so reviewer request state is explicit (true/false) rather than omitted.

(Based on your team's feedback about preferring explicit states over omission/nil.)

View Feedback

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At internal/schemavalidators/internal_schemas/prinfo/pr-info-1.2.schema.json, line 84:

<comment>Require `reviewers[].requested` in the schema so reviewer request state is explicit (true/false) rather than omitted.

(Based on your team's feedback about preferring explicit states over omission/nil.) </comment>

<file context>
@@ -0,0 +1,103 @@
+        },
+        "additionalProperties": false,
+        "type": "object",
+        "required": [
+          "login",
+          "type"
</file context>
Fix with Cubic

"login",
"type"
]
},
"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"
}
8 changes: 7 additions & 1 deletion internal/schemavalidators/schemavalidators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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,
}

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions pkg/attestation/crafter/materials/chainloop_pr_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/attestation/crafter/materials/chainloop_pr_info_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading