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
2 changes: 1 addition & 1 deletion .github/workflows/issue-monster.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 28 additions & 1 deletion pkg/workflow/frontmatter_extraction_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (c *Compiler) extractTopLevelYAMLSection(frontmatter map[string]any, key st
return yamlStr
}

// commentOutProcessedFieldsInOnSection comments out draft, fork, forks, names, manual-approval, stop-after, skip-if-match, skip-if-no-match, skip-roles, reaction, lock-for-agent, steps, permissions, and stale-check fields in the on section
// commentOutProcessedFieldsInOnSection comments out draft, fork, forks, names, labels, manual-approval, stop-after, skip-if-match, skip-if-no-match, skip-roles, reaction, lock-for-agent, steps, permissions, and stale-check fields in the on section
// These fields are processed separately and should be commented for documentation
// Exception: names fields in sections with __gh_aw_native_label_filter__ marker in frontmatter are NOT commented out
func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmatter map[string]any) string {
Expand Down Expand Up @@ -143,6 +143,7 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
inSkipBotsArray := false
inRolesArray := false
inBotsArray := false
inLabelsArray := false
inGitHubApp := false
inOnSteps := false
inOnPermissions := false
Expand Down Expand Up @@ -252,6 +253,7 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
}

trimmedLine := strings.TrimSpace(line)
lineIndent := len(line) - len(strings.TrimLeft(line, " \t"))

// Skip marker lines in the YAML output
if (inPullRequest || inIssues || inDiscussion || inIssueComment) && strings.Contains(trimmedLine, "__gh_aw_native_label_filter__:") {
Expand Down Expand Up @@ -290,6 +292,13 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
inBotsArray = true
}

// Check if we're entering labels array
if !inPullRequest && !inIssues && !inDiscussion && !inIssueComment &&
!inOnSteps && !inOnPermissions &&
lineIndent == 2 && trimmedLine == "labels:" {
inLabelsArray = true
}

// Check if we're entering on.steps array
if !inPullRequest && !inIssues && !inDiscussion && !inIssueComment && strings.HasPrefix(trimmedLine, "steps:") {
inOnSteps = true
Expand Down Expand Up @@ -444,6 +453,17 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
}
}

// Check if we're leaving the labels array by encountering another top-level field
if inLabelsArray && strings.TrimSpace(line) != "" {
// Get the indentation of the current line
lineIndent := len(line) - len(strings.TrimLeft(line, " \t"))

// If this is a non-dash line at the same level as labels (2 spaces), we're out of the array
if lineIndent == 2 && !strings.HasPrefix(trimmedLine, "-") && !strings.HasPrefix(trimmedLine, "labels:") && !strings.HasPrefix(trimmedLine, "#") {
inLabelsArray = false
}
}

// Check if we're leaving the on.steps array by encountering another top-level field
if inOnSteps && strings.TrimSpace(line) != "" {
lineIndent := len(line) - len(strings.TrimLeft(line, " \t"))
Expand Down Expand Up @@ -524,6 +544,13 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat
// Comment out array items in bots
shouldComment = true
commentReason = " # Bots processed as bot check in pre-activation job"
} else if !inOnSteps && !inOnPermissions && lineIndent == 2 && 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"
} else if strings.HasPrefix(trimmedLine, "steps:") {
shouldComment = true
commentReason = " # Steps injected into pre-activation job"
Expand Down
104 changes: 104 additions & 0 deletions pkg/workflow/frontmatter_extraction_yaml_fuzz_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
//go:build !integration

package workflow

import (
"fmt"
"strconv"
"strings"
"testing"
)

func FuzzCommentOutProcessedFieldsInOnSectionTopLevelLabels(f *testing.F) {
f.Add("bug", "enhancement", "triage", "needs-info")
f.Add("panel-review", "can't-repro", "nested-1", "nested-2")
f.Add("", "", "", "")

compiler := NewCompiler()
f.Fuzz(func(t *testing.T, topLevelLabelA, topLevelLabelB, nestedLabelA, nestedLabelB string) {
topAQuoted := strconv.Quote(topLevelLabelA)
topBQuoted := strconv.Quote(topLevelLabelB)
nestedAQuoted := strconv.Quote(nestedLabelA)
nestedBQuoted := strconv.Quote(nestedLabelB)

yamlStr := fmt.Sprintf(`on:
issues:
types: [labeled]
labels:
- %s
- %s
steps:
- name: Nested labels in step input
uses: actions/github-script@v8
with:
labels:
- %s
- %s
script: |
core.info('label')
`, topAQuoted, topBQuoted, nestedAQuoted, nestedBQuoted)

result := compiler.commentOutProcessedFieldsInOnSection(yamlStr, map[string]any{})

mustContain := func(expected string) {
if !strings.Contains(result, expected) {
t.Fatalf("expected %q in result:\n%s", expected, result)
}
}

mustContain(" # labels: # Label filtering applied via job conditions")
mustContain("# - " + topAQuoted + " # Label filtering applied via job conditions")
mustContain("# - " + topBQuoted + " # Label filtering applied via job conditions")
mustContain(" # - " + nestedAQuoted)
mustContain(" # - " + nestedBQuoted)
if strings.Contains(result, " # - "+nestedAQuoted+" # Label filtering applied via job conditions") {
t.Fatalf("nested labels item should not be marked as top-level label filtering:\n%s", result)
}
if strings.Contains(result, " # - "+nestedBQuoted+" # Label filtering applied via job conditions") {
t.Fatalf("nested labels item should not be marked as top-level label filtering:\n%s", result)
}

expectedTopLevelLabelItems := 2
expectedLabelFilterAnnotations := expectedTopLevelLabelItems + 1 // labels key + top-level items
if got := strings.Count(result, "Label filtering applied via job conditions"); got != expectedLabelFilterAnnotations {
t.Fatalf("expected %d label-filter annotations (labels key + top-level items), got %d:\n%s", expectedLabelFilterAnnotations, got, result)
}
})
}

func FuzzCommentOutProcessedFieldsInOnSectionNoTopLevelLabels(f *testing.F) {
f.Add("triage", "needs-info")
f.Add("", "")

compiler := NewCompiler()
f.Fuzz(func(t *testing.T, nestedA, nestedB string) {
nestedAQuoted := strconv.Quote(nestedA)
nestedBQuoted := strconv.Quote(nestedB)

yamlStr := fmt.Sprintf(`on:
issues:
types: [labeled]
steps:
- name: Nested labels in step input
uses: actions/github-script@v8
with:
labels:
- %s
- %s
script: |
core.info('label')
`, nestedAQuoted, nestedBQuoted)

result := compiler.commentOutProcessedFieldsInOnSection(yamlStr, map[string]any{})

if !strings.Contains(result, " # - "+nestedAQuoted) {
t.Fatalf("expected nested labels item to remain in on.steps output:\n%s", result)
}
if !strings.Contains(result, " # - "+nestedBQuoted) {
t.Fatalf("expected nested labels item to remain in on.steps output:\n%s", result)
}
if strings.Contains(result, "Label filtering applied via job conditions") {
t.Fatalf("unexpected top-level label filter annotation without on.labels:\n%s", result)
}
})
}
78 changes: 74 additions & 4 deletions pkg/workflow/label_names_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,12 @@ func TestLabelNamesPreActivationFilter(t *testing.T) {
compiler := NewCompiler()

tests := []struct {
name string
frontmatter string
expectedIf string
shouldHaveIf bool
name string
frontmatter string
expectedIf string
shouldHaveIf bool
shouldCheckLabelArrayItems bool
labelItems []string
}{
{
name: "pull_request_target with single labels",
Expand All @@ -135,6 +137,8 @@ tools:
---`,
expectedIf: "github.event.label == null || github.event.label.name == 'panel-review'",
shouldHaveIf: true,
labelItems: []string{"panel-review"},
shouldCheckLabelArrayItems: false,
},
{
name: "pull_request_target with multiple labels",
Expand All @@ -156,6 +160,8 @@ tools:
---`,
expectedIf: "github.event.label == null || github.event.label.name == 'panel-review' || github.event.label.name == 'needs-triage'",
shouldHaveIf: true,
labelItems: []string{"panel-review", "needs-triage"},
shouldCheckLabelArrayItems: true,
},
{
// Negative test: no on.labels specified → the label-filter condition should not appear.
Expand Down Expand Up @@ -199,6 +205,8 @@ tools:
---`,
expectedIf: "github.event.label == null || github.event.label.name == 'bug' || github.event.label.name == 'enhancement'",
shouldHaveIf: true,
labelItems: []string{"bug", "enhancement"},
shouldCheckLabelArrayItems: true,
},
}

Expand All @@ -223,10 +231,72 @@ tools:
if tt.shouldHaveIf {
assert.Contains(t, lockContent, tt.expectedIf,
"pre_activation job should have if condition matching label filter")
assert.Contains(t, lockContent, "# labels:",
"on.labels should be commented out in generated workflow")
assert.Contains(t, lockContent, "Label filtering applied via job conditions",
"on.labels comment should explain filter handling")
if tt.shouldCheckLabelArrayItems {
for _, item := range tt.labelItems {
assert.Contains(t, lockContent, "# - "+item,
"on.labels array items should be commented out in generated workflow")
}
}
} else {
assert.NotContains(t, lockContent, tt.expectedIf,
"pre_activation job should not have label-name if condition when labels not specified")
}
})
}
}

func TestLabelNamesDoesNotAffectNestedOnStepsLabels(t *testing.T) {
tmpDir := testutil.TempDir(t, "labels-nested-steps-test")
compiler := NewCompiler()

frontmatter := `---
on:
issues:
types: [labeled]
labels: bug
steps:
- name: Nested labels in step input
uses: actions/github-script@v8
with:
labels:
- triage
- needs-info
script: |
core.info('label')

permissions:
contents: read
issues: read
pull-requests: read

strict: false
tools:
github:
allowed: [issue_read]
---`

testFile := tmpDir + "/test-nested-labels.md"
content := frontmatter + "\n\n# Test Workflow\n\nNested labels in on.steps should not be treated as on.labels."
require.NoError(t, os.WriteFile(testFile, []byte(content), 0644), "should write test file")
defer os.Remove(testFile)

err := compiler.CompileWorkflow(testFile)
require.NoError(t, err, "should compile workflow successfully")

lockFile := stringutil.MarkdownToLockFile(testFile)
lockBytes, err := os.ReadFile(lockFile)
require.NoError(t, err, "should read lock file")
defer os.Remove(lockFile)
lockContent := string(lockBytes)

assert.Equal(t, 1, strings.Count(lockContent, "Label filtering applied via job conditions"),
"label-filter annotation should appear exactly once for top-level on.labels")
assert.NotContains(t, lockContent, "- name: Nested labels in step input # Label filtering applied via job conditions",
"on.steps list items should not be annotated as label filtering")
assert.NotContains(t, lockContent, "- triage # Label filtering applied via job conditions",
"nested labels in on.steps should not be annotated as top-level label filtering")
}