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
8 changes: 8 additions & 0 deletions pkg/workflow/concurrency.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ func isDiscussionWorkflow(on string) bool {
return strings.Contains(on, "discussion")
}

// isPushWorkflow checks if a workflow's "on" section contains push triggers
func isPushWorkflow(on string) bool {
return strings.Contains(on, "push")
}

// buildConcurrencyGroupKeys builds an array of keys for the concurrency group
func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool) []string {
keys := []string{"gh-aw", "${{ github.workflow }}"}
Expand All @@ -68,6 +73,9 @@ func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool
} else if isDiscussionWorkflow(workflowData.On) {
// Discussion workflows: use discussion number
keys = append(keys, "${{ github.event.discussion.number }}")
} else if isPushWorkflow(workflowData.On) {
// Push workflows: use ref to differentiate between branches
keys = append(keys, "${{ github.ref }}")
}

return keys
Expand Down
101 changes: 96 additions & 5 deletions pkg/workflow/concurrency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ tools:
description: "Regular workflows should use static concurrency without cancellation",
},
{
name: "push workflow should use static concurrency without cancel",
name: "push workflow should use dynamic concurrency with ref",
frontmatter: `---
on:
push:
Expand All @@ -86,9 +86,9 @@ tools:
---`,
filename: "push-workflow.md",
expectedConcurrency: `concurrency:
group: "gh-aw-${{ github.workflow }}"`,
group: "gh-aw-${{ github.workflow }}-${{ github.ref }}"`,
shouldHaveCancel: false,
description: "Push workflows should use static concurrency without cancellation",
description: "Push workflows should use dynamic concurrency with github.ref",
},
{
name: "issue workflow should have dynamic concurrency with issue number",
Expand Down Expand Up @@ -146,10 +146,11 @@ This is a test workflow for concurrency behavior.
t.Errorf("Did not expect cancel-in-progress: true for %s workflow, but found in: %s", tt.name, workflowData.Concurrency)
}

// For PR workflows, check for PR number inclusion; for alias workflows, check for issue/PR numbers; for issue workflows, check for issue number
// For PR workflows, check for PR number inclusion; for alias workflows, check for issue/PR numbers; for issue workflows, check for issue number; for push workflows, check for github.ref
isPRWorkflow := strings.Contains(tt.name, "PR workflow")
isAliasWorkflow := strings.Contains(tt.name, "alias workflow")
isIssueWorkflow := strings.Contains(tt.name, "issue workflow")
isPushWorkflow := strings.Contains(tt.name, "push workflow")

if isPRWorkflow {
if !strings.Contains(workflowData.Concurrency, "github.event.pull_request.number") {
Expand All @@ -163,8 +164,13 @@ This is a test workflow for concurrency behavior.
if !strings.Contains(workflowData.Concurrency, "github.event.issue.number") {
t.Errorf("Expected concurrency to include github.event.issue.number for %s workflow, got: %s", tt.name, workflowData.Concurrency)
}
} else if isPushWorkflow {
if !strings.Contains(workflowData.Concurrency, "github.ref") {
t.Errorf("Expected concurrency to include github.ref for %s workflow, got: %s", tt.name, workflowData.Concurrency)
}
} else {
if strings.Contains(workflowData.Concurrency, "github.ref") {
// For regular workflows (like schedule), don't expect github.ref unless it's also a push workflow
if strings.Contains(workflowData.Concurrency, "github.ref") && !isPushWorkflow {
t.Errorf("Did not expect concurrency to include github.ref for %s workflow, got: %s", tt.name, workflowData.Concurrency)
}
}
Expand Down Expand Up @@ -207,6 +213,19 @@ func TestGenerateConcurrencyConfig(t *testing.T) {
group: "gh-aw-${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number }}"`,
description: "Alias workflows should use dynamic concurrency with ref but without cancellation",
},
{
name: "Push workflow should have dynamic concurrency with ref",
workflowData: &WorkflowData{
On: `on:
push:
branches: [main]`,
Concurrency: "", // Empty, should be generated
},
isAliasTrigger: false,
expected: `concurrency:
group: "gh-aw-${{ github.workflow }}-${{ github.ref }}"`,
description: "Push workflows should use github.ref without cancellation",
},
{
name: "Regular workflow should use static concurrency without cancel",
workflowData: &WorkflowData{
Expand Down Expand Up @@ -436,6 +455,54 @@ func TestIsIssueWorkflow(t *testing.T) {
}
}

func TestIsPushWorkflow(t *testing.T) {
tests := []struct {
name string
on string
expected bool
}{
{
name: "Push workflow should be identified",
on: `on:
push:
branches: [main]`,
expected: true,
},
{
name: "Pull request workflow should not be identified as push workflow",
on: `on:
pull_request:
types: [opened, synchronize]`,
expected: false,
},
{
name: "Schedule workflow should not be identified as push workflow",
on: `on:
schedule:
- cron: "0 9 * * 1"`,
expected: false,
},
{
name: "Mixed workflow with push should be identified",
on: `on:
push:
branches: [main]
pull_request:
types: [opened, synchronize]`,
expected: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isPushWorkflow(tt.on)
if result != tt.expected {
t.Errorf("isPushWorkflow() for %s = %v, expected %v", tt.name, result, tt.expected)
}
})
}
}

func TestIsDiscussionWorkflow(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -562,6 +629,30 @@ func TestBuildConcurrencyGroupKeys(t *testing.T) {
expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.event.issue.number || github.event.discussion.number }}"},
description: "Mixed issue and discussion workflows should use issue/discussion number",
},
{
name: "Push workflow should include github.ref",
workflowData: &WorkflowData{
On: `on:
push:
branches: [main]`,
},
isAliasTrigger: false,
expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.ref }}"},
description: "Push workflows should use github.ref",
},
{
name: "Mixed push and PR workflow should use PR logic (PR takes priority)",
workflowData: &WorkflowData{
On: `on:
push:
branches: [main]
pull_request:
types: [opened, synchronize]`,
},
isAliasTrigger: false,
expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.event.pull_request.number || github.ref }}"},
description: "Mixed push+PR workflows should use PR logic since PR is checked first",
},
{
name: "Other workflow should not include additional keys",
workflowData: &WorkflowData{
Expand Down
Loading