diff --git a/.github/aw/schemas/agentic-workflow.json b/.github/aw/schemas/agentic-workflow.json index f8f6e5c73a..510479189b 100644 --- a/.github/aw/schemas/agentic-workflow.json +++ b/.github/aw/schemas/agentic-workflow.json @@ -273,7 +273,47 @@ "type": "string" } } - } + }, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ], + "allOf": [ + { + "oneOf": [ + { + "required": ["paths"], + "not": { "required": ["paths-ignore"] } + }, + { + "required": ["paths-ignore"], + "not": { "required": ["paths"] } + }, + { + "not": { + "anyOf": [ + { "required": ["paths"] }, + { "required": ["paths-ignore"] } + ] + } + } + ] + } + ] }, "pull_request": { "description": "Pull request event trigger that runs the workflow when pull requests are created, updated, or closed", @@ -379,10 +419,50 @@ ] } }, - "additionalProperties": false + "additionalProperties": false, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ], + "allOf": [ + { + "oneOf": [ + { + "required": ["paths"], + "not": { "required": ["paths-ignore"] } + }, + { + "required": ["paths-ignore"], + "not": { "required": ["paths"] } + }, + { + "not": { + "anyOf": [ + { "required": ["paths"] }, + { "required": ["paths-ignore"] } + ] + } + } + ] + } + ] }, "issues": { - "description": "Issues event trigger that runs the workflow when repository issues are created, updated, or managed", + "description": "Issues event trigger that runs when repository issues are created, updated, or managed", "type": "object", "additionalProperties": false, "properties": { @@ -577,7 +657,25 @@ "type": "string" } } - } + }, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ] }, "release": { "description": "Release event trigger", @@ -886,7 +984,47 @@ ] } }, - "additionalProperties": false + "additionalProperties": false, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ], + "allOf": [ + { + "oneOf": [ + { + "required": ["paths"], + "not": { "required": ["paths-ignore"] } + }, + { + "required": ["paths-ignore"], + "not": { "required": ["paths"] } + }, + { + "not": { + "anyOf": [ + { "required": ["paths"] }, + { "required": ["paths-ignore"] } + ] + } + } + ] + } + ] }, "pull_request_review": { "description": "Pull request review event trigger that runs when a pull request review is submitted, edited, or dismissed", @@ -4982,6 +5120,16 @@ } }, "required": ["pull_request_review_comment"] + }, + { + "properties": { + "label": { + "not": { + "type": "null" + } + } + }, + "required": ["label"] } ] } diff --git a/pkg/parser/schema_oneof_test.go b/pkg/parser/schema_oneof_test.go new file mode 100644 index 0000000000..8fcf36c45b --- /dev/null +++ b/pkg/parser/schema_oneof_test.go @@ -0,0 +1,332 @@ +package parser + +import ( + "strings" + "testing" +) + +// TestValidateOneOfConstraints tests the oneOf constraints added to the schema +// to prevent mutually exclusive fields from being specified together. +func TestValidateOneOfConstraints(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + wantErr bool + errContains string + }{ + // branches vs branches-ignore in push event + { + name: "invalid: both branches and branches-ignore in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "branches": []string{"main"}, + "branches-ignore": []string{"dev"}, + }, + }, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "valid: only branches in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "branches": []string{"main"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid: only branches-ignore in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "branches-ignore": []string{"dev"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid: neither branches nor branches-ignore in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "tags": []string{"v*"}, + }, + }, + }, + wantErr: false, + }, + + // paths vs paths-ignore in push event + { + name: "invalid: both paths and paths-ignore in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "paths": []string{"src/**"}, + "paths-ignore": []string{"docs/**"}, + }, + }, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "valid: only paths in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "paths": []string{"src/**"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid: only paths-ignore in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "paths-ignore": []string{"docs/**"}, + }, + }, + }, + wantErr: false, + }, + + // branches vs branches-ignore in pull_request event + { + name: "invalid: both branches and branches-ignore in pull_request", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{ + "branches": []string{"main"}, + "branches-ignore": []string{"dev"}, + }, + }, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "valid: only branches in pull_request", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{ + "branches": []string{"main"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid: only branches-ignore in pull_request", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{ + "branches-ignore": []string{"dev"}, + }, + }, + }, + wantErr: false, + }, + + // paths vs paths-ignore in pull_request event + { + name: "invalid: both paths and paths-ignore in pull_request", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{ + "paths": []string{"src/**"}, + "paths-ignore": []string{"docs/**"}, + }, + }, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "valid: only paths in pull_request", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{ + "paths": []string{"src/**"}, + }, + }, + }, + wantErr: false, + }, + + // branches vs branches-ignore in pull_request_target event + { + name: "invalid: both branches and branches-ignore in pull_request_target", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request_target": map[string]any{ + "branches": []string{"main"}, + "branches-ignore": []string{"dev"}, + }, + }, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "valid: only branches in pull_request_target", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request_target": map[string]any{ + "branches": []string{"main"}, + }, + }, + }, + wantErr: false, + }, + + // paths vs paths-ignore in pull_request_target event + { + name: "invalid: both paths and paths-ignore in pull_request_target", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request_target": map[string]any{ + "paths": []string{"src/**"}, + "paths-ignore": []string{"docs/**"}, + }, + }, + }, + wantErr: true, + errContains: "oneOf", + }, + + // branches vs branches-ignore in workflow_run event + { + name: "invalid: both branches and branches-ignore in workflow_run", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []string{"CI"}, + "branches": []string{"main"}, + "branches-ignore": []string{"dev"}, + }, + }, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "valid: only branches in workflow_run", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []string{"CI"}, + "branches": []string{"main"}, + }, + }, + }, + wantErr: false, + }, + + // slash_command vs label events + { + name: "invalid: slash_command with label event", + frontmatter: map[string]any{ + "on": map[string]any{ + "slash_command": "mybot", + "label": map[string]any{ + "types": []string{"created"}, + }, + }, + }, + wantErr: true, + errContains: "not", + }, + { + name: "valid: slash_command without label event", + frontmatter: map[string]any{ + "on": map[string]any{ + "slash_command": "mybot", + }, + }, + wantErr: false, + }, + { + name: "valid: label event without slash_command", + frontmatter: map[string]any{ + "on": map[string]any{ + "label": map[string]any{ + "types": []string{"created"}, + }, + }, + }, + wantErr: false, + }, + + // command vs label events (deprecated command field) + { + name: "invalid: command with label event", + frontmatter: map[string]any{ + "on": map[string]any{ + "command": "mybot", + "label": map[string]any{ + "types": []string{"created"}, + }, + }, + }, + wantErr: true, + errContains: "not", + }, + + // Valid combinations of branches and paths + { + name: "valid: branches and paths in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "branches": []string{"main"}, + "paths": []string{"src/**"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid: branches-ignore and paths-ignore in push", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{ + "branches-ignore": []string{"dev"}, + "paths-ignore": []string{"docs/**"}, + }, + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateMainWorkflowFrontmatterWithSchema(tt.frontmatter) + + if tt.wantErr && err == nil { + t.Errorf("ValidateMainWorkflowFrontmatterWithSchema() expected error, got nil") + return + } + + if !tt.wantErr && err != nil { + t.Errorf("ValidateMainWorkflowFrontmatterWithSchema() error = %v", err) + return + } + + if err != nil && tt.errContains != "" { + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("ValidateMainWorkflowFrontmatterWithSchema() error = %v, expected to contain %q", err.Error(), tt.errContains) + } + } + }) + } +} diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index f8f6e5c73a..510479189b 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -273,7 +273,47 @@ "type": "string" } } - } + }, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ], + "allOf": [ + { + "oneOf": [ + { + "required": ["paths"], + "not": { "required": ["paths-ignore"] } + }, + { + "required": ["paths-ignore"], + "not": { "required": ["paths"] } + }, + { + "not": { + "anyOf": [ + { "required": ["paths"] }, + { "required": ["paths-ignore"] } + ] + } + } + ] + } + ] }, "pull_request": { "description": "Pull request event trigger that runs the workflow when pull requests are created, updated, or closed", @@ -379,10 +419,50 @@ ] } }, - "additionalProperties": false + "additionalProperties": false, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ], + "allOf": [ + { + "oneOf": [ + { + "required": ["paths"], + "not": { "required": ["paths-ignore"] } + }, + { + "required": ["paths-ignore"], + "not": { "required": ["paths"] } + }, + { + "not": { + "anyOf": [ + { "required": ["paths"] }, + { "required": ["paths-ignore"] } + ] + } + } + ] + } + ] }, "issues": { - "description": "Issues event trigger that runs the workflow when repository issues are created, updated, or managed", + "description": "Issues event trigger that runs when repository issues are created, updated, or managed", "type": "object", "additionalProperties": false, "properties": { @@ -577,7 +657,25 @@ "type": "string" } } - } + }, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ] }, "release": { "description": "Release event trigger", @@ -886,7 +984,47 @@ ] } }, - "additionalProperties": false + "additionalProperties": false, + "oneOf": [ + { + "required": ["branches"], + "not": { "required": ["branches-ignore"] } + }, + { + "required": ["branches-ignore"], + "not": { "required": ["branches"] } + }, + { + "not": { + "anyOf": [ + { "required": ["branches"] }, + { "required": ["branches-ignore"] } + ] + } + } + ], + "allOf": [ + { + "oneOf": [ + { + "required": ["paths"], + "not": { "required": ["paths-ignore"] } + }, + { + "required": ["paths-ignore"], + "not": { "required": ["paths"] } + }, + { + "not": { + "anyOf": [ + { "required": ["paths"] }, + { "required": ["paths-ignore"] } + ] + } + } + ] + } + ] }, "pull_request_review": { "description": "Pull request review event trigger that runs when a pull request review is submitted, edited, or dismissed", @@ -4982,6 +5120,16 @@ } }, "required": ["pull_request_review_comment"] + }, + { + "properties": { + "label": { + "not": { + "type": "null" + } + } + }, + "required": ["label"] } ] }