diff --git a/.github/workflows/test-claude-create-pull-request-review-comment.lock.yml b/.github/workflows/test-claude-create-pull-request-review-comment.lock.yml index 21fb4c7700..6b11e45cd7 100644 --- a/.github/workflows/test-claude-create-pull-request-review-comment.lock.yml +++ b/.github/workflows/test-claude-create-pull-request-review-comment.lock.yml @@ -569,7 +569,7 @@ jobs: uses: actions/github-script@v7 env: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} - GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{}" + GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"create-pull-request-review-comment\":{\"enabled\":true,\"max\":3}}" with: script: | async function main() { diff --git a/.github/workflows/test-claude-create-security-report.lock.yml b/.github/workflows/test-claude-create-security-report.lock.yml index 6ffd567758..cdd08b9b26 100644 --- a/.github/workflows/test-claude-create-security-report.lock.yml +++ b/.github/workflows/test-claude-create-security-report.lock.yml @@ -561,7 +561,7 @@ jobs: uses: actions/github-script@v7 env: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} - GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{}" + GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"create-security-report\":{\"enabled\":true,\"max\":10}}" with: script: | async function main() { diff --git a/.github/workflows/test-codex-create-pull-request-review-comment.lock.yml b/.github/workflows/test-codex-create-pull-request-review-comment.lock.yml index 080c1e2245..62449a7614 100644 --- a/.github/workflows/test-codex-create-pull-request-review-comment.lock.yml +++ b/.github/workflows/test-codex-create-pull-request-review-comment.lock.yml @@ -401,7 +401,7 @@ jobs: uses: actions/github-script@v7 env: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} - GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{}" + GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"create-pull-request-review-comment\":{\"enabled\":true,\"max\":3}}" with: script: | async function main() { diff --git a/.github/workflows/test-codex-create-security-report.lock.yml b/.github/workflows/test-codex-create-security-report.lock.yml index 796544f1c9..139f42fbe1 100644 --- a/.github/workflows/test-codex-create-security-report.lock.yml +++ b/.github/workflows/test-codex-create-security-report.lock.yml @@ -393,7 +393,7 @@ jobs: uses: actions/github-script@v7 env: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} - GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{}" + GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"create-security-report\":{\"enabled\":true,\"max\":10}}" with: script: | async function main() { diff --git a/.github/workflows/test-safe-outputs-custom-engine.lock.yml b/.github/workflows/test-safe-outputs-custom-engine.lock.yml index b9f5cc1664..9a024684bb 100644 --- a/.github/workflows/test-safe-outputs-custom-engine.lock.yml +++ b/.github/workflows/test-safe-outputs-custom-engine.lock.yml @@ -382,7 +382,7 @@ jobs: uses: actions/github-script@v7 env: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} - GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"add-issue-comment\":{\"enabled\":true,\"target\":\"*\"},\"add-issue-label\":true,\"create-issue\":true,\"create-pull-request\":true,\"missing-tool\":{\"enabled\":true,\"max\":5},\"push-to-branch\":{\"branch\":\"triggering\",\"enabled\":true,\"target\":\"*\"},\"update-issue\":true}" + GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"add-issue-comment\":{\"enabled\":true,\"target\":\"*\"},\"add-issue-label\":true,\"create-discussion\":{\"enabled\":true,\"max\":1},\"create-issue\":true,\"create-pull-request\":true,\"create-pull-request-review-comment\":{\"enabled\":true,\"max\":1},\"missing-tool\":{\"enabled\":true,\"max\":5},\"push-to-branch\":{\"branch\":\"triggering\",\"enabled\":true,\"target\":\"*\"},\"update-issue\":true}" with: script: | async function main() { diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 9785700a29..b5b1c39edc 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -3712,9 +3712,37 @@ func (c *Compiler) generateOutputCollectionStep(yaml *strings.Builder, data *Wor } safeOutputsConfig["add-issue-comment"] = commentConfig } + if data.SafeOutputs.CreateDiscussions != nil { + discussionConfig := map[string]interface{}{ + "enabled": true, + } + if data.SafeOutputs.CreateDiscussions.Max > 0 { + discussionConfig["max"] = data.SafeOutputs.CreateDiscussions.Max + } + safeOutputsConfig["create-discussion"] = discussionConfig + } if data.SafeOutputs.CreatePullRequests != nil { safeOutputsConfig["create-pull-request"] = true } + if data.SafeOutputs.CreatePullRequestReviewComments != nil { + prReviewCommentConfig := map[string]interface{}{ + "enabled": true, + } + if data.SafeOutputs.CreatePullRequestReviewComments.Max > 0 { + prReviewCommentConfig["max"] = data.SafeOutputs.CreatePullRequestReviewComments.Max + } + safeOutputsConfig["create-pull-request-review-comment"] = prReviewCommentConfig + } + if data.SafeOutputs.CreateSecurityReports != nil { + securityReportConfig := map[string]interface{}{ + "enabled": true, + } + // Security reports typically have unlimited max, but check if configured + if data.SafeOutputs.CreateSecurityReports.Max > 0 { + securityReportConfig["max"] = data.SafeOutputs.CreateSecurityReports.Max + } + safeOutputsConfig["create-security-report"] = securityReportConfig + } if data.SafeOutputs.AddIssueLabels != nil { safeOutputsConfig["add-issue-label"] = true } diff --git a/pkg/workflow/output_config_test.go b/pkg/workflow/output_config_test.go index be77c5daf1..4c63908acd 100644 --- a/pkg/workflow/output_config_test.go +++ b/pkg/workflow/output_config_test.go @@ -1,6 +1,7 @@ package workflow import ( + "strings" "testing" ) @@ -115,6 +116,130 @@ func TestAllowedDomainsInWorkflow(t *testing.T) { } } +func TestSafeOutputsConfigGeneration(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + expectedInConfig []string + unexpectedInConfig []string + }{ + { + name: "create-discussion config", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "create-discussion": map[string]any{ + "title-prefix": "[discussion] ", + "max": 2, + }, + }, + }, + expectedInConfig: []string{"create-discussion"}, + }, + { + name: "create-pull-request-review-comment config", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "create-pull-request-review-comment": map[string]any{ + "max": 5, + }, + }, + }, + expectedInConfig: []string{"create-pull-request-review-comment"}, + }, + { + name: "create-security-report config", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "create-security-report": map[string]any{}, + }, + }, + expectedInConfig: []string{"create-security-report"}, + }, + { + name: "multiple safe outputs including previously missing ones", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "create-issue": map[string]any{"max": 1}, + "create-discussion": map[string]any{"max": 3}, + "create-pull-request-review-comment": map[string]any{"max": 10}, + "create-security-report": map[string]any{}, + "add-issue-comment": map[string]any{}, + }, + }, + expectedInConfig: []string{ + "create-issue", + "create-discussion", + "create-pull-request-review-comment", + "create-security-report", + "add-issue-comment", + }, + }, + { + name: "no safe outputs config", + frontmatter: map[string]any{ + "engine": "claude", + }, + expectedInConfig: []string{}, + unexpectedInConfig: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler(false, "", "test") + config := compiler.extractSafeOutputsConfig(tt.frontmatter) + + // Test the config generation in generateOutputCollectionStep by creating a mock workflow data + workflowData := &WorkflowData{ + SafeOutputs: config, + } + + // Use the compiler's generateOutputCollectionStep to test the GITHUB_AW_SAFE_OUTPUTS_CONFIG generation + var yamlBuilder strings.Builder + compiler.generateOutputCollectionStep(&yamlBuilder, workflowData) + generatedYAML := yamlBuilder.String() + + // Look specifically for the GITHUB_AW_SAFE_OUTPUTS_CONFIG environment variable line + configLinePresent := strings.Contains(generatedYAML, "GITHUB_AW_SAFE_OUTPUTS_CONFIG:") + + if len(tt.expectedInConfig) > 0 { + // If we expect items in config, the config line should be present + if !configLinePresent { + t.Errorf("Expected GITHUB_AW_SAFE_OUTPUTS_CONFIG environment variable to be present, but it was not found") + return + } + + // Extract the config line to check its contents + configLine := "" + lines := strings.Split(generatedYAML, "\n") + for _, line := range lines { + if strings.Contains(line, "GITHUB_AW_SAFE_OUTPUTS_CONFIG:") { + configLine = line + break + } + } + + // Check expected items are present in the config line + for _, expected := range tt.expectedInConfig { + if !strings.Contains(configLine, expected) { + t.Errorf("Expected %q to be in GITHUB_AW_SAFE_OUTPUTS_CONFIG, but it was not found in config line: %s", expected, configLine) + } + } + + // Check unexpected items are not present in the config line + for _, unexpected := range tt.unexpectedInConfig { + if strings.Contains(configLine, unexpected) { + t.Errorf("Did not expect %q to be in GITHUB_AW_SAFE_OUTPUTS_CONFIG, but it was found in config line: %s", unexpected, configLine) + } + } + } + // If we don't expect any items and no unexpected items specified, + // the config line may or may not be present (depending on whether SafeOutputs is nil) + // This is acceptable behavior + }) + } +} + func TestCreateDiscussionConfigParsing(t *testing.T) { tests := []struct { name string