diff --git a/.github/workflows/issue-monster.lock.yml b/.github/workflows/issue-monster.lock.yml index 2d3dc4ad71..182901c0d8 100644 --- a/.github/workflows/issue-monster.lock.yml +++ b/.github/workflows/issue-monster.lock.yml @@ -389,7 +389,7 @@ name: "Issue Monster" # return { # number: issue.number, # title: issue.title, - # labels: issue.labels.map(l => l.name), + # labels: issue.labels.map(l => l.name), # Label filtering applied via job conditions # body: issue.body, # created_at: issue.created_at, # score diff --git a/pkg/workflow/frontmatter_extraction_yaml.go b/pkg/workflow/frontmatter_extraction_yaml.go index 6db40dfa03..13f82bc39f 100644 --- a/pkg/workflow/frontmatter_extraction_yaml.go +++ b/pkg/workflow/frontmatter_extraction_yaml.go @@ -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 { @@ -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 @@ -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__:") { @@ -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 @@ -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")) @@ -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" diff --git a/pkg/workflow/frontmatter_extraction_yaml_fuzz_test.go b/pkg/workflow/frontmatter_extraction_yaml_fuzz_test.go new file mode 100644 index 0000000000..b51c1c4c60 --- /dev/null +++ b/pkg/workflow/frontmatter_extraction_yaml_fuzz_test.go @@ -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) + } + }) +} diff --git a/pkg/workflow/label_names_test.go b/pkg/workflow/label_names_test.go index b1ebadf977..f7d9500a66 100644 --- a/pkg/workflow/label_names_test.go +++ b/pkg/workflow/label_names_test.go @@ -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", @@ -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", @@ -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. @@ -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, }, } @@ -223,6 +231,16 @@ 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") @@ -230,3 +248,55 @@ tools: }) } } + +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") +}