-
Notifications
You must be signed in to change notification settings - Fork 49
Description
Objective
Add compiler-level validation to detect and reject workflows that use both branches/branches-ignore or paths/paths-ignore together, matching GitHub Actions requirements.
Context
GitHub Actions rejects workflows that specify both branches and branches-ignore (or both paths and paths-ignore) in the same event filter. Currently gh-aw allows these invalid configurations to compile, leading to runtime failures in GitHub Actions.
This issue implements the runtime validation that complements the schema documentation added in #5804.
Approach
Add validation logic in the compiler to check for mutual exclusivity violations in push and pull_request event filters.
Files to Modify
- Update:
pkg/workflow/compiler.go(add validation function) - Update:
pkg/parser/schema.go(or create new validator if preferred) - Create:
pkg/workflow/compiler_filters_test.go(test coverage)
Implementation
1. Add Validation Function
// ValidateEventFilters checks for GitHub Actions filter mutual exclusivity rules
func ValidateEventFilters(frontmatter map[string]any) error {
on, exists := frontmatter["on"]
if !exists {
return nil
}
onMap, ok := on.(map[string]any)
if !ok {
return nil
}
// Check push event
if pushVal, exists := onMap["push"]; exists {
if err := validateFilterExclusivity(pushVal, "push"); err != nil {
return err
}
}
// Check pull_request event
if prVal, exists := onMap["pull_request"]; exists {
if err := validateFilterExclusivity(prVal, "pull_request"); err != nil {
return err
}
}
return nil
}
func validateFilterExclusivity(eventVal any, eventName string) error {
eventMap, ok := eventVal.(map[string]any)
if !ok {
return nil
}
// Check branches/branches-ignore
if _, hasBranches := eventMap["branches"]; hasBranches {
if _, hasIgnore := eventMap["branches-ignore"]; hasIgnore {
return fmt.Errorf("%s event cannot specify both 'branches' and 'branches-ignore' - they are mutually exclusive per GitHub Actions requirements", eventName)
}
}
// Check paths/paths-ignore
if _, hasPaths := eventMap["paths"]; hasPaths {
if _, hasIgnore := eventMap["paths-ignore"]; hasIgnore {
return fmt.Errorf("%s event cannot specify both 'paths' and 'paths-ignore' - they are mutually exclusive per GitHub Actions requirements", eventName)
}
}
return nil
}2. Call Validation
Add call in compiler's validation chain (around existing event validation):
// In CompileWorkflow or validateWorkflow function
if err := ValidateEventFilters(frontmatter); err != nil {
return err
}3. Add Tests
Create comprehensive test coverage for all cases:
func TestValidateEventFilters(t *testing.T) {
tests := []struct {
name string
frontmatter map[string]any
wantErr bool
}{
{
name: "valid branches only",
frontmatter: map[string]any{
"on": map[string]any{
"push": map[string]any{
"branches": []string{"main"},
},
},
},
wantErr: false,
},
{
name: "invalid both branches and branches-ignore",
frontmatter: map[string]any{
"on": map[string]any{
"push": map[string]any{
"branches": []string{"main"},
"branches-ignore": []string{"dev"},
},
},
},
wantErr: true,
},
// Add similar tests for paths/paths-ignore, pull_request event, etc.
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateEventFilters(tt.frontmatter)
if (err != nil) != tt.wantErr {
t.Errorf("ValidateEventFilters() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}Acceptance Criteria
- Validation function added to compiler
- Both branches/branches-ignore and paths/paths-ignore conflicts detected
- Both push and pull_request events validated
- Clear error messages using console.FormatErrorMessage
- Comprehensive test coverage in new test file
- Tests pass:
make test-unit - No regressions:
make test
Testing
# Rebuild and test
make build
make test-unit
# Manual validation
echo '---
on:
push:
branches: [main]
branches-ignore: [dev]
---
Test' > /tmp/test-invalid.md
./gh-aw compile /tmp/test-invalid.md # Should fail with clear error
echo '---
on:
pull_request:
paths: ["src/**"]
paths-ignore: ["docs/**"]
---
Test' > /tmp/test-invalid2.md
./gh-aw compile /tmp/test-invalid2.md # Should fail with clear error
echo '---
on:
push:
branches: [main]
---
Test' > /tmp/test-valid.md
./gh-aw compile /tmp/test-valid.md # Should succeedNotes
- Follow the example of existing validation in
pkg/parser/schema.go(e.g.,IsLabelOnlyEvent) - Use console.FormatErrorMessage for error output
- Consider adding this to the strict mode validation path as well
Related to [plan] Fix schema validation gaps from conditional logic analysis #5804
AI generated by Plan Command for discussion #5799