Skip to content

Commit 6d1b67e

Browse files
Copilotpelikhan
andauthored
Fix review feedback: inline reset into activateEventSection, expand coverage, revert smoke if
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent 53ce335 commit 6d1b67e

4 files changed

Lines changed: 78 additions & 24 deletions

File tree

.github/workflows/smoke-claude-on-copilot.lock.yml

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/workflow/compiler_draft_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,50 @@ func TestCommentOutProcessedFieldsInOnSection(t *testing.T) {
388388
workflow_dispatch:`,
389389
description: "Should reset bots array tracker before entering workflow_run section",
390390
},
391+
{
392+
name: "bots before workflow_run with multi-line arrays do not comment workflow_run list items",
393+
input: `on:
394+
bots:
395+
- dependabot
396+
workflow_run:
397+
workflows:
398+
- CI
399+
types:
400+
- completed
401+
workflow_dispatch:`,
402+
expected: `on:
403+
# bots: # Bots processed as bot check in pre-activation job
404+
# - dependabot # Bots processed as bot check in pre-activation job
405+
workflow_run:
406+
workflows:
407+
- CI
408+
types:
409+
- completed
410+
workflow_dispatch:`,
411+
description: "Should not comment out multi-line workflow_run.workflows/types items when bots precedes workflow_run",
412+
},
413+
{
414+
name: "skip-if-check-failing before workflow_run does not corrupt workflow_run list items",
415+
input: `on:
416+
skip-if-check-failing:
417+
- build
418+
workflow_run:
419+
workflows:
420+
- CI
421+
types:
422+
- completed
423+
workflow_dispatch:`,
424+
expected: `on:
425+
# skip-if-check-failing: # Skip-if-check-failing processed as check status gate in pre-activation job
426+
# - build
427+
workflow_run:
428+
workflows:
429+
- CI
430+
types:
431+
- completed
432+
workflow_dispatch:`,
433+
description: "Should reset inSkipIfCheckFailing before entering workflow_run to prevent list-item corruption",
434+
},
391435
{
392436
name: "roles before workflow_run do not comment workflow_run list items",
393437
input: `on:

pkg/workflow/frontmatter_extraction_yaml.go

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,28 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
164164
deploymentStatusIndent := -1
165165
workflowRunIndent := -1
166166
// activateEventSection resets all event-section flags and then activates the selected section.
167+
// It also clears every top-level on: extension-array tracker (inBotsArray, inRolesArray,
168+
// inSkipIfCheckFailing, etc.) before entering the new section. This reset is required
169+
// because each activateEventSection call ends with "continue", which bypasses the
170+
// indent-based deactivation logic further down the loop. Without the explicit reset here,
171+
// a stale flag from a preceding bots:/roles:/skip-if-check-failing: block would cause that
172+
// section's list items (e.g. "workflow_run.workflows: - CI") to be incorrectly commented out.
167173
activateEventSection := func(section string, indent int) {
174+
// Clear all top-level on: extension-array state so no sibling section leaks in.
175+
inSkipRolesArray = false
176+
inSkipBotsArray = false
177+
inRolesArray = false
178+
inBotsArray = false
179+
inLabelsArray = false
180+
inNeedsArray = false
181+
// These trackers share the same exit-check-ordering issue: their deactivation
182+
// logic runs after the "continue" that terminates each activateEventSection call,
183+
// so they must also be reset here explicitly.
184+
inSkipIfMatch = false
185+
inSkipIfNoMatch = false
186+
inSkipIfCheckFailing = false
187+
inSkipAuthorAssociations = false
188+
168189
inPullRequest = section == "pull_request"
169190
inIssues = section == "issues"
170191
inDiscussion = section == "discussion"
@@ -194,16 +215,6 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
194215
workflowRunIndent = -1
195216
}
196217
}
197-
// resetOnArrayTrackers clears top-level on: extension array state when a new
198-
// event section starts, preventing sibling sections from inheriting stale flags.
199-
resetOnArrayTrackers := func() {
200-
inSkipRolesArray = false
201-
inSkipBotsArray = false
202-
inRolesArray = false
203-
inBotsArray = false
204-
inLabelsArray = false
205-
inNeedsArray = false
206-
}
207218

208219
for _, line := range lines {
209220
trimmedLine := strings.TrimSpace(line)
@@ -216,37 +227,31 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
216227
// the permission comment-out logic.
217228
if !inOnPermissions && !inOnSteps && !inSkipAuthorAssociations {
218229
if (lineIndent == 2 || lineIndent == 4) && trimmedLine == "pull_request:" {
219-
resetOnArrayTrackers()
220230
activateEventSection("pull_request", lineIndent)
221231
result = append(result, line)
222232
continue
223233
}
224234
if (lineIndent == 2 || lineIndent == 4) && trimmedLine == "issues:" {
225-
resetOnArrayTrackers()
226235
activateEventSection("issues", lineIndent)
227236
result = append(result, line)
228237
continue
229238
}
230239
if (lineIndent == 2 || lineIndent == 4) && trimmedLine == "discussion:" {
231-
resetOnArrayTrackers()
232240
activateEventSection("discussion", lineIndent)
233241
result = append(result, line)
234242
continue
235243
}
236244
if (lineIndent == 2 || lineIndent == 4) && trimmedLine == "issue_comment:" {
237-
resetOnArrayTrackers()
238245
activateEventSection("issue_comment", lineIndent)
239246
result = append(result, line)
240247
continue
241248
}
242249
if (lineIndent == 2 || lineIndent == 4) && trimmedLine == "deployment_status:" {
243-
resetOnArrayTrackers()
244250
activateEventSection("deployment_status", lineIndent)
245251
result = append(result, line)
246252
continue
247253
}
248254
if (lineIndent == 2 || lineIndent == 4) && trimmedLine == "workflow_run:" {
249-
resetOnArrayTrackers()
250255
activateEventSection("workflow_run", lineIndent)
251256
result = append(result, line)
252257
continue

pkg/workflow/workflow_run_validation_test.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,15 @@ Test workflow content.`,
123123
name: "workflow_run with sibling bots - strict mode - should pass",
124124
frontmatter: `---
125125
on:
126+
bots:
127+
- dependabot
126128
workflow_run:
127-
workflows: ["build"]
128-
types: [completed]
129+
workflows:
130+
- build
131+
types:
132+
- completed
129133
branches:
130134
- main
131-
bots:
132-
- dependabot
133135
tools:
134136
github:
135137
toolsets: [repos]
@@ -147,12 +149,14 @@ Test workflow content.`,
147149
name: "workflow_run with sibling roles all - strict mode - should pass",
148150
frontmatter: `---
149151
on:
152+
roles: all
150153
workflow_run:
151-
workflows: ["build"]
152-
types: [completed]
154+
workflows:
155+
- build
156+
types:
157+
- completed
153158
branches:
154159
- main
155-
roles: all
156160
tools:
157161
github:
158162
toolsets: [repos]

0 commit comments

Comments
 (0)