Skip to content

Commit fad43e3

Browse files
Copilotpelikhan
Copilot
and
pelikhan
authored
Fix concurrency helpers to correctly handle synthetic events (slash_command, schedule) (#18184)
* Initial plan * Fix concurrency helpers to handle synthetic events (slash_command, schedule) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent db8724d commit fad43e3

3 files changed

Lines changed: 314 additions & 14 deletions

File tree

‎pkg/workflow/concurrency.go‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func GenerateJobConcurrencyConfig(workflowData *WorkflowData) string {
7676

7777
// hasSpecialTriggers checks if the workflow has special trigger types that require
7878
// workflow-level concurrency handling (issues, PRs, discussions, push, command,
79-
// or workflow_dispatch-only)
79+
// slash_command, or workflow_dispatch-only)
8080
func hasSpecialTriggers(workflowData *WorkflowData) bool {
8181
// Check for specific trigger types that have special concurrency handling
8282
on := workflowData.On
@@ -101,6 +101,11 @@ func hasSpecialTriggers(workflowData *WorkflowData) bool {
101101
return true
102102
}
103103

104+
// Check for slash_command triggers (synthetic event that expands to issue_comment + workflow_dispatch)
105+
if isSlashCommandWorkflow(on) {
106+
return true
107+
}
108+
104109
// workflow_dispatch-only workflows represent explicit user intent, so the
105110
// top-level workflow concurrency group is sufficient – no engine-level group needed
106111
if isWorkflowDispatchOnly(on) {
@@ -129,6 +134,8 @@ func isDiscussionWorkflow(on string) bool {
129134

130135
// isWorkflowDispatchOnly returns true when workflow_dispatch is the only trigger in the
131136
// "on" section, indicating the workflow is always started by explicit user intent.
137+
// It handles both rendered YAML (standard GitHub Actions events) and input YAML
138+
// (which may contain synthetic events like slash_command before they are expanded).
132139
func isWorkflowDispatchOnly(on string) bool {
133140
if !strings.Contains(on, "workflow_dispatch") {
134141
return false
@@ -137,13 +144,17 @@ func isWorkflowDispatchOnly(on string) bool {
137144
// workflow_dispatch-only workflow. We check for the trigger name followed by
138145
// ':' (YAML key in object form) or as the sole inline value to avoid false
139146
// matches from input parameter names (e.g., "push_branch" ≠ "push" trigger).
147+
// slash_command is included here because it is a synthetic event that expands
148+
// to issue_comment + workflow_dispatch at compile time; its presence means the
149+
// workflow is not triggered solely by explicit user dispatch.
140150
otherTriggers := []string{
141151
"push", "pull_request", "pull_request_review", "pull_request_review_comment",
142152
"pull_request_target", "issues", "issue_comment", "discussion",
143153
"discussion_comment", "schedule", "repository_dispatch", "workflow_run",
144154
"create", "delete", "release", "deployment", "fork", "gollum",
145155
"label", "milestone", "page_build", "public", "registry_package",
146156
"status", "watch", "merge_group", "check_run", "check_suite",
157+
"slash_command",
147158
}
148159
for _, trigger := range otherTriggers {
149160
// Trigger in object format: "push:" / " push:"
@@ -163,12 +174,21 @@ func isPushWorkflow(on string) bool {
163174
return strings.Contains(on, "push")
164175
}
165176

177+
// isSlashCommandWorkflow checks if a workflow's "on" section contains the slash_command
178+
// synthetic trigger. slash_command is an input-level event that expands to
179+
// issue_comment + workflow_dispatch at compile time. Detecting it here allows
180+
// the concurrency helpers to produce correct results even when they are called
181+
// with the pre-rendered "on" YAML (before the event expansion has taken place).
182+
func isSlashCommandWorkflow(on string) bool {
183+
return strings.Contains(on, "slash_command")
184+
}
185+
166186
// buildConcurrencyGroupKeys builds an array of keys for the concurrency group
167187
func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool) []string {
168188
keys := []string{"gh-aw", "${{ github.workflow }}"}
169189

170-
if isCommandTrigger {
171-
// For command workflows: use issue/PR number
190+
if isCommandTrigger || isSlashCommandWorkflow(workflowData.On) {
191+
// For command/slash_command workflows: use issue/PR number
172192
keys = append(keys, "${{ github.event.issue.number || github.event.pull_request.number }}")
173193
} else if isPullRequestWorkflow(workflowData.On) && isIssueWorkflow(workflowData.On) {
174194
// Mixed workflows with both issue and PR triggers: use issue/PR number

‎pkg/workflow/concurrency_test.go‎

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,36 @@ tools:
106106
shouldHaveCancel: false,
107107
description: "Issue workflows use global concurrency with engine ID and slot",
108108
},
109+
{
110+
name: "slash_command workflow should have dynamic concurrency with issue/PR number",
111+
frontmatter: `---
112+
on:
113+
slash_command:
114+
name: test-bot
115+
tools:
116+
github:
117+
allowed: [list_issues]
118+
---`,
119+
filename: "slash-command-workflow.md",
120+
expectedConcurrency: `concurrency:
121+
group: "gh-aw-${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number }}"`,
122+
shouldHaveCancel: false,
123+
description: "slash_command workflows should use dynamic concurrency with issue/PR number without cancellation",
124+
},
125+
{
126+
name: "slash_command shorthand workflow should have dynamic concurrency with issue/PR number",
127+
frontmatter: `---
128+
on: /test-bot
129+
tools:
130+
github:
131+
allowed: [list_issues]
132+
---`,
133+
filename: "slash-command-shorthand-workflow.md",
134+
expectedConcurrency: `concurrency:
135+
group: "gh-aw-${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number }}"`,
136+
shouldHaveCancel: false,
137+
description: "slash_command shorthand workflows should use dynamic concurrency with issue/PR number without cancellation",
138+
},
109139
}
110140

111141
for _, tt := range tests {
@@ -294,6 +324,33 @@ func TestGenerateConcurrencyConfig(t *testing.T) {
294324
group: "custom-group"`,
295325
description: "Existing concurrency configuration should be preserved",
296326
},
327+
{
328+
name: "slash_command input YAML should have dynamic concurrency with issue/PR number",
329+
workflowData: &WorkflowData{
330+
On: `on:
331+
slash_command: test-bot
332+
workflow_dispatch:`,
333+
Concurrency: "",
334+
},
335+
isAliasTrigger: false,
336+
expected: `concurrency:
337+
group: "gh-aw-${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number }}"`,
338+
description: "slash_command (input-level YAML) should use issue/PR number, same as command trigger",
339+
},
340+
{
341+
name: "slash_command rendered YAML (issue_comment + workflow_dispatch) should have dynamic concurrency",
342+
workflowData: &WorkflowData{
343+
On: `on:
344+
issue_comment:
345+
types: [created]
346+
workflow_dispatch:`,
347+
Concurrency: "",
348+
},
349+
isAliasTrigger: false,
350+
expected: `concurrency:
351+
group: "gh-aw-${{ github.workflow }}-${{ github.event.issue.number }}"`,
352+
description: "Rendered slash_command YAML (issue_comment + workflow_dispatch) uses issue number via isIssueWorkflow",
353+
},
297354
}
298355

299356
for _, tt := range tests {
@@ -399,6 +456,24 @@ func TestGenerateJobConcurrencyConfig(t *testing.T) {
399456
group: "gh-aw-copilot-${{ github.workflow }}"`,
400457
description: "workflow_dispatch combined with schedule should still get default concurrency (not workflow_dispatch-only)",
401458
},
459+
{
460+
name: "No default concurrency for slash_command input YAML (pre-rendered)",
461+
workflowData: &WorkflowData{
462+
On: "on:\n slash_command: test-bot\n workflow_dispatch:",
463+
EngineConfig: &EngineConfig{ID: "copilot"},
464+
},
465+
expected: "",
466+
description: "slash_command in input YAML should NOT get default concurrency (isSlashCommandWorkflow detects the synthetic event)",
467+
},
468+
{
469+
name: "No default concurrency for slash_command rendered YAML (issue_comment + workflow_dispatch)",
470+
workflowData: &WorkflowData{
471+
On: "on:\n issue_comment:\n types: [created]\n workflow_dispatch:",
472+
EngineConfig: &EngineConfig{ID: "copilot"},
473+
},
474+
expected: "",
475+
description: "Rendered slash_command YAML (issue_comment + workflow_dispatch) should NOT get default concurrency (isIssueWorkflow detects it)",
476+
},
402477
}
403478

404479
for _, tt := range tests {
@@ -740,6 +815,17 @@ func TestBuildConcurrencyGroupKeys(t *testing.T) {
740815
expected: []string{"gh-aw", "${{ github.workflow }}"},
741816
description: "Other workflows should use just workflow name",
742817
},
818+
{
819+
name: "slash_command input YAML should include issue/PR number",
820+
workflowData: &WorkflowData{
821+
On: `on:
822+
slash_command: test-bot
823+
workflow_dispatch:`,
824+
},
825+
isAliasTrigger: false,
826+
expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.event.issue.number || github.event.pull_request.number }}"},
827+
description: "slash_command (input-level YAML) should include issue/PR number in concurrency group",
828+
},
743829
}
744830

745831
for _, tt := range tests {
@@ -836,3 +922,191 @@ func TestShouldEnableCancelInProgress(t *testing.T) {
836922
})
837923
}
838924
}
925+
926+
func TestIsWorkflowDispatchOnly(t *testing.T) {
927+
tests := []struct {
928+
name string
929+
on string
930+
expected bool
931+
desc string
932+
}{
933+
{
934+
name: "Pure workflow_dispatch should be identified as dispatch-only",
935+
on: "on:\n workflow_dispatch:",
936+
expected: true,
937+
desc: "A workflow with only workflow_dispatch is dispatch-only",
938+
},
939+
{
940+
name: "workflow_dispatch with inputs should be identified as dispatch-only",
941+
on: `on:
942+
workflow_dispatch:
943+
inputs:
944+
environment:
945+
description: "Environment"`,
946+
expected: true,
947+
desc: "workflow_dispatch with inputs is still dispatch-only",
948+
},
949+
{
950+
name: "No workflow_dispatch should not be identified as dispatch-only",
951+
on: `on:
952+
schedule:
953+
- cron: "0 9 * * 1"`,
954+
expected: false,
955+
desc: "A workflow without workflow_dispatch is not dispatch-only",
956+
},
957+
{
958+
name: "workflow_dispatch combined with schedule should not be dispatch-only",
959+
on: `on:
960+
workflow_dispatch:
961+
schedule:
962+
- cron: "0 9 * * 1"`,
963+
expected: false,
964+
desc: "schedule is a real trigger so the workflow is not dispatch-only",
965+
},
966+
{
967+
name: "workflow_dispatch combined with push should not be dispatch-only",
968+
on: `on:
969+
workflow_dispatch:
970+
push:
971+
branches: [main]`,
972+
expected: false,
973+
desc: "push makes the workflow not dispatch-only",
974+
},
975+
{
976+
name: "workflow_dispatch combined with pull_request should not be dispatch-only",
977+
on: `on:
978+
workflow_dispatch:
979+
pull_request:
980+
types: [opened]`,
981+
expected: false,
982+
desc: "pull_request makes the workflow not dispatch-only",
983+
},
984+
{
985+
name: "workflow_dispatch combined with issues should not be dispatch-only",
986+
on: `on:
987+
workflow_dispatch:
988+
issues:
989+
types: [opened]`,
990+
expected: false,
991+
desc: "issues makes the workflow not dispatch-only",
992+
},
993+
{
994+
name: "slash_command with workflow_dispatch should not be dispatch-only",
995+
on: `on:
996+
slash_command: test-bot
997+
workflow_dispatch:`,
998+
expected: false,
999+
desc: "slash_command is a synthetic event that expands to issue_comment; its presence means the workflow is not dispatch-only",
1000+
},
1001+
{
1002+
name: "slash_command map format with workflow_dispatch should not be dispatch-only",
1003+
on: `on:
1004+
slash_command:
1005+
name: test-bot
1006+
workflow_dispatch:`,
1007+
expected: false,
1008+
desc: "slash_command in map format is still a synthetic event that makes the workflow not dispatch-only",
1009+
},
1010+
}
1011+
1012+
for _, tt := range tests {
1013+
t.Run(tt.name, func(t *testing.T) {
1014+
result := isWorkflowDispatchOnly(tt.on)
1015+
if result != tt.expected {
1016+
t.Errorf("isWorkflowDispatchOnly() for %q = %v, want %v: %s", tt.name, result, tt.expected, tt.desc)
1017+
}
1018+
})
1019+
}
1020+
}
1021+
1022+
func TestHasSpecialTriggers(t *testing.T) {
1023+
tests := []struct {
1024+
name string
1025+
on string
1026+
expected bool
1027+
desc string
1028+
}{
1029+
{
1030+
name: "Issue workflow is a special trigger",
1031+
on: `on:
1032+
issues:
1033+
types: [opened]`,
1034+
expected: true,
1035+
desc: "issues trigger should be detected as special",
1036+
},
1037+
{
1038+
name: "PR workflow is a special trigger",
1039+
on: `on:
1040+
pull_request:
1041+
types: [opened]`,
1042+
expected: true,
1043+
desc: "pull_request trigger should be detected as special",
1044+
},
1045+
{
1046+
name: "Push workflow is a special trigger",
1047+
on: `on:
1048+
push:
1049+
branches: [main]`,
1050+
expected: true,
1051+
desc: "push trigger should be detected as special",
1052+
},
1053+
{
1054+
name: "Discussion workflow is a special trigger",
1055+
on: `on:
1056+
discussion:
1057+
types: [created]`,
1058+
expected: true,
1059+
desc: "discussion trigger should be detected as special",
1060+
},
1061+
{
1062+
name: "workflow_dispatch-only is a special trigger",
1063+
on: "on:\n workflow_dispatch:",
1064+
expected: true,
1065+
desc: "workflow_dispatch-only is treated as special (explicit user intent)",
1066+
},
1067+
{
1068+
name: "slash_command input YAML is a special trigger",
1069+
on: `on:
1070+
slash_command: test-bot
1071+
workflow_dispatch:`,
1072+
expected: true,
1073+
desc: "slash_command is a synthetic event that should be detected as special",
1074+
},
1075+
{
1076+
name: "slash_command map format is a special trigger",
1077+
on: `on:
1078+
slash_command:
1079+
name: test-bot
1080+
workflow_dispatch:`,
1081+
expected: true,
1082+
desc: "slash_command in map format should also be detected as special",
1083+
},
1084+
{
1085+
name: "schedule-only is NOT a special trigger",
1086+
on: `on:
1087+
schedule:
1088+
- cron: "0 9 * * 1"`,
1089+
expected: false,
1090+
desc: "schedule alone is not a special trigger and should receive default job concurrency",
1091+
},
1092+
{
1093+
name: "schedule + workflow_dispatch is NOT a special trigger",
1094+
on: `on:
1095+
schedule:
1096+
- cron: "0 9 * * 1"
1097+
workflow_dispatch:`,
1098+
expected: false,
1099+
desc: "schedule + workflow_dispatch is not a special trigger and should receive default job concurrency",
1100+
},
1101+
}
1102+
1103+
for _, tt := range tests {
1104+
t.Run(tt.name, func(t *testing.T) {
1105+
wd := &WorkflowData{On: tt.on}
1106+
result := hasSpecialTriggers(wd)
1107+
if result != tt.expected {
1108+
t.Errorf("hasSpecialTriggers() for %q = %v, want %v: %s", tt.name, result, tt.expected, tt.desc)
1109+
}
1110+
})
1111+
}
1112+
}

0 commit comments

Comments
 (0)