Comment out top-level on.labels in compiled workflows to prevent push-time workflow parse failures#31411
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
on.labels in compiled workflows to prevent push-time workflow parse failures
There was a problem hiding this comment.
Pull request overview
This PR fixes GitHub Actions push-time workflow parse failures caused by emitting an invalid top-level on.labels key in compiled .lock.yml workflows by commenting out on.labels (and its list items) while preserving label filtering behavior via job-level if conditions.
Changes:
- Updated
commentOutProcessedFieldsInOnSectionto comment out top-levelon.labelsand any multiline array items. - Extended label-name filter tests to assert that
on.labelsis emitted as commented documentation and list items are commented when applicable.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/frontmatter_extraction_yaml.go | Adds logic to comment out top-level on.labels (including multiline list items) in generated YAML. |
| pkg/workflow/label_names_test.go | Adds assertions ensuring compiled workflows contain commented on.labels documentation and commented list items for label arrays. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/workflow/frontmatter_extraction_yaml.go:550
- The
labels:comment-out branch matches anylabels:key whenever we’re not in an event subsection. This can also hit nestedlabels:keys insideon.steps(or other nested objects) and annotate them as “Label filtering…”, even though they’re unrelated toon.labels. To avoid mis-commenting user-authored step configuration, restrict this to the top-levelon.labelsfield (e.g., by checkinglineIndent == 2/strings.HasPrefix(line, " labels:"), and/or ensuring we’re not currently ininOnSteps/inOnPermissions).
} else if strings.HasPrefix(trimmedLine, "labels:") {
shouldComment = true
commentReason = " # Label filtering applied via job conditions"
} else if inLabelsArray && strings.HasPrefix(trimmedLine, "-") {
// Comment out array items in labels
shouldComment = true
commentReason = " # Label filtering applied via job conditions"
- Files reviewed: 2/2 changed files
- Comments generated: 1
| } | ||
|
|
||
| // Check if we're entering labels array | ||
| if !inPullRequest && !inIssues && !inDiscussion && !inIssueComment && trimmedLine == "labels:" { |
|
@copilots review all comments |
|
@copilot review all |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Hey One small thing before this is ready for maintainer review:
If you'd like a hand, you can assign this prompt to your coding agent:
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed. I fixed the review-thread issue by restricting
Done — all actionable review comments have been addressed in |
|
@copilot add fuzz tests |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added fuzz tests in They cover top-level |
on.labelsextraction/commenting behavior