Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 9 additions & 9 deletions pkg/workflow/compiler_filters_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ func ValidateEventFilters(frontmatter map[string]any) error {
}

// ValidatePushBranchScope ensures that any push event in the on: section specifies a
// branches or branches-ignore filter. An unscoped push trigger fires on every push to
// every branch, which causes unintended workflow fan-out on feature branches (the
// workflows activate immediately after new lock files are first pushed to the branch,
// producing zero-turn failures for every agentic workflow in the repository).
// branch or tag ref filter. An unscoped push trigger fires on every push to every
// branch and tag, which causes unintended workflow fan-out on feature branches (the workflows
// activate immediately after new lock files are first pushed to the branch, producing
// zero-turn failures for every agentic workflow in the repository).
Comment on lines 91 to +95

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in c975eda: the ValidatePushBranchScope doc comment now states that unscoped push fires for both branches and tags.

func ValidatePushBranchScope(frontmatter map[string]any) error {
filterValidationLog.Print("Validating push event branch scope")
filterValidationLog.Print("Validating push event branch/tag scope")

on, exists := frontmatter["on"]
if !exists {
Expand All @@ -113,7 +113,7 @@ func ValidatePushBranchScope(frontmatter map[string]any) error {

// A nil push value (bare `push:` key with no sub-keys) is unscoped.
if pushVal == nil {
filterValidationLog.Print("ERROR: push event has no branch scope (nil push value)")
filterValidationLog.Print("ERROR: push event has no branch/tag scope (nil push value)")
return newUnScopedPushError()
}

Expand All @@ -140,9 +140,9 @@ func ValidatePushBranchScope(frontmatter map[string]any) error {
func newUnScopedPushError() *WorkflowValidationError {
return NewValidationError(
"on.push",
"push (no branch filter)",
"push event must specify a 'branches', 'branches-ignore', 'tags', or 'tags-ignore' filter; an unscoped push trigger fires on every push to every branch and causes unintended workflow fan-out on feature branches",
"Add a branch or tag filter to the push trigger:\n\non:\n push:\n branches:\n - main",
"push (no branch or tag filter)",
"push event must specify a 'branches', 'branches-ignore', 'tags', or 'tags-ignore' filter; an unscoped push trigger fires on every push to every branch and tag and causes unintended workflow fan-out on feature branches",
"Add a branch or tag filter to the push trigger:\n\non:\n push:\n branches:\n - main\n\n# or for tag-based releases:\n\non:\n push:\n tags:\n - 'v*.*.*'",
)
}

Expand Down
22 changes: 22 additions & 0 deletions pkg/workflow/compiler_filters_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,28 @@ func TestValidatePushBranchScope(t *testing.T) {
},
wantErr: false,
},
{
name: "push with tags filter",
frontmatter: map[string]any{
"on": map[string]any{
"push": map[string]any{
"tags": []string{"v*.*.*"},
},
},
},
wantErr: false,
},
{
name: "push with tags-ignore filter",
frontmatter: map[string]any{
"on": map[string]any{
"push": map[string]any{
"tags-ignore": []string{"nightly-*"},
},
},
},
wantErr: false,
},
{
name: "push with branches and paths filter",
frontmatter: map[string]any{
Expand Down
6 changes: 3 additions & 3 deletions pkg/workflow/compiler_orchestrator_frontmatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,14 @@ func (c *Compiler) parseFrontmatterSection(markdownPath string) (*frontmatterPar
return nil, err
}

// Validate that push triggers are scoped to specific branches to prevent fan-out.
// Validate that push triggers are scoped to specific branches or tags to prevent fan-out.
// In strict mode this is an error; in non-strict mode it is downgraded to a warning.
if err := ValidatePushBranchScope(frontmatterForValidation); err != nil {
if c.effectiveStrictMode(frontmatterForValidation) {
orchestratorFrontmatterLog.Printf("Push branch scope validation failed: %v", err)
orchestratorFrontmatterLog.Printf("Push branch/tag scope validation failed: %v", err)
return nil, err
}
orchestratorFrontmatterLog.Printf("Push branch scope warning (non-strict mode): %v", err)
orchestratorFrontmatterLog.Printf("Push branch/tag scope warning (non-strict mode): %v", err)
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(err.Error()))
c.IncrementWarningCount()
}
Expand Down
17 changes: 16 additions & 1 deletion pkg/workflow/push_branch_scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// TestPushBranchScopeStrictVsNonStrict verifies that an unscoped push trigger
// (missing branches/branches-ignore) is an error in strict mode and a warning in non-strict mode.
// (missing branch/tag ref filters) is an error in strict mode and a warning in non-strict mode.
func TestPushBranchScopeStrictVsNonStrict(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -65,6 +65,21 @@ on:
- main
---

# Test Workflow
`,
expectError: false,
expectWarning: false,
},
{
name: "scoped push with tags in strict mode is allowed",
content: `---
name: Tag Scoped Push Strict
on:
push:
tags:
- 'v*.*.*'
---

# Test Workflow
`,
expectError: false,
Expand Down
Loading