diff --git a/.github/workflows/ai-moderator.lock.yml b/.github/workflows/ai-moderator.lock.yml index c0f3c58654..e6e562b3e2 100644 --- a/.github/workflows/ai-moderator.lock.yml +++ b/.github/workflows/ai-moderator.lock.yml @@ -26,7 +26,7 @@ # Imports: # - shared/mood.md # -# frontmatter-hash: 8c8c0cf5f9b5c9de3acbec28f94efe65b00e4c888c6be93df443665a560ce698 +# frontmatter-hash: 2ebd01943a22487e072101433dd9319edd66b82cddac1934c765b2ad80f94251 name: "AI Moderator" "on": @@ -38,6 +38,9 @@ name: "AI Moderator" # lock-for-agent: true # Lock-for-agent processed as issue locking in activation job types: - opened + # skip-bots: # Skip-bots processed as bot check in pre-activation job + # - github-actions # Skip-bots processed as bot check in pre-activation job + # - copilot # Skip-bots processed as bot check in pre-activation job # skip-roles: # Skip-roles processed as role check in pre-activation job # - admin # Skip-roles processed as role check in pre-activation job # - maintainer # Skip-roles processed as role check in pre-activation job @@ -915,7 +918,7 @@ jobs: actions: read contents: read outputs: - activated: ${{ (steps.check_skip_roles.outputs.skip_roles_ok == 'true') && (steps.check_rate_limit.outputs.rate_limit_ok == 'true') }} + activated: ${{ ((steps.check_skip_roles.outputs.skip_roles_ok == 'true') && (steps.check_skip_bots.outputs.skip_bots_ok == 'true')) && (steps.check_rate_limit.outputs.rate_limit_ok == 'true') }} steps: - name: Checkout actions folder uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -955,6 +958,18 @@ jobs: setupGlobals(core, github, context, exec, io); const { main } = require('/opt/gh-aw/actions/check_skip_roles.cjs'); await main(); + - name: Check skip-bots + id: check_skip_bots + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_SKIP_BOTS: github-actions,copilot + GH_AW_WORKFLOW_NAME: "AI Moderator" + with: + script: | + const { setupGlobals } = require('/opt/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('/opt/gh-aw/actions/check_skip_bots.cjs'); + await main(); safe_outputs: needs: diff --git a/.github/workflows/ai-moderator.md b/.github/workflows/ai-moderator.md index fed3ecf875..a498167c18 100644 --- a/.github/workflows/ai-moderator.md +++ b/.github/workflows/ai-moderator.md @@ -10,6 +10,7 @@ on: types: [created] lock-for-agent: true skip-roles: [admin, maintainer, write, triage] + skip-bots: [github-actions, copilot] rate-limit: max: 5 window: 60 diff --git a/actions/setup/js/check_skip_bots.cjs b/actions/setup/js/check_skip_bots.cjs new file mode 100644 index 0000000000..3dc528beb5 --- /dev/null +++ b/actions/setup/js/check_skip_bots.cjs @@ -0,0 +1,63 @@ +// @ts-check +/// + +/** + * Check if the workflow should be skipped based on bot/user identity + * Reads skip-bots from GH_AW_SKIP_BOTS environment variable + * If the github.actor is in the skip-bots list, set skip_bots_ok to false (skip the workflow) + * Otherwise, set skip_bots_ok to true (allow the workflow to proceed) + */ +async function main() { + const { eventName } = context; + const actor = context.actor; + + // Parse skip-bots from environment variable + const skipBotsEnv = process.env.GH_AW_SKIP_BOTS; + if (!skipBotsEnv || skipBotsEnv.trim() === "") { + // No skip-bots configured, workflow should proceed + core.info("✅ No skip-bots configured, workflow will proceed"); + core.setOutput("skip_bots_ok", "true"); + core.setOutput("result", "no_skip_bots"); + return; + } + + const skipBots = skipBotsEnv + .split(",") + .map(u => u.trim()) + .filter(u => u); + core.info(`Checking if user '${actor}' is in skip-bots: ${skipBots.join(", ")}`); + + // Check if the actor is in the skip-bots list + // Match both exact username and username with [bot] suffix + // e.g., "github-actions" matches both "github-actions" and "github-actions[bot]" + const isSkipped = skipBots.some(skipBot => { + // Exact match + if (actor === skipBot) { + return true; + } + // Match with [bot] suffix + if (actor === `${skipBot}[bot]`) { + return true; + } + // Match if skip-bot has [bot] suffix and actor matches base name + if (skipBot.endsWith("[bot]") && actor === skipBot.slice(0, -5)) { + return true; + } + return false; + }); + + if (isSkipped) { + // User is in skip-bots, skip the workflow + core.info(`❌ User '${actor}' is in skip-bots [${skipBots.join(", ")}]. Workflow will be skipped.`); + core.setOutput("skip_bots_ok", "false"); + core.setOutput("result", "skipped"); + core.setOutput("error_message", `Workflow skipped: User '${actor}' is in skip-bots: [${skipBots.join(", ")}]`); + } else { + // User is NOT in skip-bots, allow workflow to proceed + core.info(`✅ User '${actor}' is NOT in skip-bots [${skipBots.join(", ")}]. Workflow will proceed.`); + core.setOutput("skip_bots_ok", "true"); + core.setOutput("result", "not_skipped"); + } +} + +module.exports = { main }; diff --git a/actions/setup/js/check_skip_bots.test.cjs b/actions/setup/js/check_skip_bots.test.cjs new file mode 100644 index 0000000000..765e07279f --- /dev/null +++ b/actions/setup/js/check_skip_bots.test.cjs @@ -0,0 +1,140 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +describe("check_skip_bots.cjs", () => { + let mockCore; + let mockContext; + + beforeEach(() => { + // Mock core actions methods + mockCore = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), + }; + + mockContext = { + actor: "test-user", + eventName: "issues", + repo: { + owner: "test-owner", + repo: "test-repo", + }, + }; + + // Set up global mocks + global.core = mockCore; + global.context = mockContext; + + // Clear environment variables + delete process.env.GH_AW_SKIP_BOTS; + + // Clear module cache to ensure fresh import + vi.resetModules(); + }); + + afterEach(() => { + vi.clearAllMocks(); + delete global.core; + delete global.context; + }); + + it("should allow workflow when no skip-bots configured", async () => { + delete process.env.GH_AW_SKIP_BOTS; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "true"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "no_skip_bots"); + }); + + it("should skip workflow for exact username match", async () => { + process.env.GH_AW_SKIP_BOTS = "test-user,other-user"; + mockContext.actor = "test-user"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "false"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "skipped"); + }); + + it("should allow workflow when user not in skip-bots", async () => { + process.env.GH_AW_SKIP_BOTS = "other-user,another-user"; + mockContext.actor = "test-user"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "true"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "not_skipped"); + }); + + it("should skip workflow for bot with [bot] suffix when base name in skip-bots", async () => { + process.env.GH_AW_SKIP_BOTS = "github-actions,copilot"; + mockContext.actor = "github-actions[bot]"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "false"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "skipped"); + }); + + it("should skip workflow for base name when skip-bots has [bot] suffix", async () => { + process.env.GH_AW_SKIP_BOTS = "github-actions[bot],copilot[bot]"; + mockContext.actor = "github-actions"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "false"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "skipped"); + }); + + it("should skip workflow for exact match with [bot] suffix", async () => { + process.env.GH_AW_SKIP_BOTS = "github-actions[bot]"; + mockContext.actor = "github-actions[bot]"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "false"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "skipped"); + }); + + it("should handle multiple users with mixed bot syntax", async () => { + process.env.GH_AW_SKIP_BOTS = "user1,github-actions,copilot[bot]"; + mockContext.actor = "copilot"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "false"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "skipped"); + }); + + it("should not skip for partial matches", async () => { + process.env.GH_AW_SKIP_BOTS = "github-actions"; + mockContext.actor = "github-actions-bot"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "true"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "not_skipped"); + }); + + it("should handle whitespace in skip-bots list", async () => { + process.env.GH_AW_SKIP_BOTS = " github-actions , copilot , renovate "; + mockContext.actor = "copilot[bot]"; + + const { main } = await import("./check_skip_bots.cjs"); + await main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("skip_bots_ok", "false"); + expect(mockCore.setOutput).toHaveBeenCalledWith("result", "skipped"); + }); +}); diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index f80782a1f9..38a0f587ad 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -649,6 +649,7 @@ const CheckSkipIfNoMatchStepID StepID = "check_skip_if_no_match" const CheckCommandPositionStepID StepID = "check_command_position" const CheckRateLimitStepID StepID = "check_rate_limit" const CheckSkipRolesStepID StepID = "check_skip_roles" +const CheckSkipBotsStepID StepID = "check_skip_bots" // Output names for pre-activation job steps const IsTeamMemberOutput = "is_team_member" @@ -659,6 +660,7 @@ const CommandPositionOkOutput = "command_position_ok" const MatchedCommandOutput = "matched_command" const RateLimitOkOutput = "rate_limit_ok" const SkipRolesOkOutput = "skip_roles_ok" +const SkipBotsOkOutput = "skip_bots_ok" const ActivatedOutput = "activated" // Rate limit defaults diff --git a/pkg/parser/content_extractor.go b/pkg/parser/content_extractor.go index 3dd6d707ec..c99321dd1c 100644 --- a/pkg/parser/content_extractor.go +++ b/pkg/parser/content_extractor.go @@ -145,6 +145,16 @@ func extractBotsFromContent(content string) (string, error) { return extractFrontmatterField(content, "bots", "[]") } +// extractSkipRolesFromContent extracts skip-roles from on: section as JSON string +func extractSkipRolesFromContent(content string) (string, error) { + return extractOnSectionField(content, "skip-roles") +} + +// extractSkipBotsFromContent extracts skip-bots from on: section as JSON string +func extractSkipBotsFromContent(content string) (string, error) { + return extractOnSectionField(content, "skip-bots") +} + // extractPluginsFromContent extracts plugins section from frontmatter as JSON string func extractPluginsFromContent(content string) (string, error) { return extractFrontmatterField(content, "plugins", "[]") @@ -213,3 +223,65 @@ func extractFrontmatterField(content, fieldName, emptyValue string) (string, err contentExtractorLog.Printf("Successfully extracted field %s: size=%d bytes", fieldName, len(fieldJSON)) return strings.TrimSpace(string(fieldJSON)), nil } + +// extractOnSectionField extracts a specific field from the on: section in frontmatter as JSON string +func extractOnSectionField(content, fieldName string) (string, error) { + contentExtractorLog.Printf("Extracting on: section field: %s", fieldName) + result, err := ExtractFrontmatterFromContent(content) + if err != nil { + contentExtractorLog.Printf("Failed to extract frontmatter for field %s: %v", fieldName, err) + return "[]", nil // Return empty array on error + } + + // Extract the "on" section + onValue, exists := result.Frontmatter["on"] + if !exists { + contentExtractorLog.Printf("Field 'on' not found in frontmatter") + return "[]", nil + } + + // The on: section should be a map + onMap, ok := onValue.(map[string]any) + if !ok { + contentExtractorLog.Printf("Field 'on' is not a map: %T", onValue) + return "[]", nil + } + + // Extract the requested field from the on: section + fieldValue, exists := onMap[fieldName] + if !exists { + contentExtractorLog.Printf("Field %s not found in 'on' section", fieldName) + return "[]", nil + } + + // Normalize field value to an array + var normalizedValue []any + switch v := fieldValue.(type) { + case string: + // Single string value + if v != "" { + normalizedValue = []any{v} + } + case []any: + // Already an array + normalizedValue = v + case []string: + // String array - convert to []any + for _, s := range v { + normalizedValue = append(normalizedValue, s) + } + default: + contentExtractorLog.Printf("Unexpected type for field %s: %T", fieldName, fieldValue) + return "[]", nil + } + + // Return JSON string + jsonData, err := json.Marshal(normalizedValue) + if err != nil { + contentExtractorLog.Printf("Failed to marshal field %s to JSON: %v", fieldName, err) + return "[]", nil + } + + contentExtractorLog.Printf("Successfully extracted field %s from on: section: %d bytes", fieldName, len(jsonData)) + return string(jsonData), nil +} diff --git a/pkg/parser/frontmatter_hash.go b/pkg/parser/frontmatter_hash.go index 5357abdbb0..111d5c16fc 100644 --- a/pkg/parser/frontmatter_hash.go +++ b/pkg/parser/frontmatter_hash.go @@ -135,6 +135,8 @@ func buildCanonicalFrontmatter(frontmatter map[string]any, result *ImportsResult addString("merged-secret-masking", result.MergedSecretMasking) addSlice("merged-bots", result.MergedBots) addString("merged-post-steps", result.MergedPostSteps) + addSlice("merged-skip-roles", result.MergedSkipRoles) + addSlice("merged-skip-bots", result.MergedSkipBots) addSlice("merged-labels", result.MergedLabels) addSlice("merged-caches", result.MergedCaches) diff --git a/pkg/parser/import_processor.go b/pkg/parser/import_processor.go index efe9a1abf5..2a387d9f65 100644 --- a/pkg/parser/import_processor.go +++ b/pkg/parser/import_processor.go @@ -31,6 +31,8 @@ type ImportsResult struct { MergedSecretMasking string // Merged secret-masking steps from all imports MergedBots []string // Merged bots list from all imports (union of bot names) MergedPlugins []string // Merged plugins list from all imports (union of plugin repos) + MergedSkipRoles []string // Merged skip-roles list from all imports (union of role names) + MergedSkipBots []string // Merged skip-bots list from all imports (union of usernames) MergedPostSteps string // Merged post-steps configuration from all imports (appended in order) MergedLabels []string // Merged labels from all imports (union of label names) MergedCaches []string // Merged cache configurations from all imports (appended in order) @@ -182,19 +184,23 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a var engines []string var safeOutputs []string var safeInputs []string - var bots []string // Track unique bot names - botsSet := make(map[string]bool) // Set for deduplicating bots - var plugins []string // Track unique plugin repos - pluginsSet := make(map[string]bool) // Set for deduplicating plugins - var labels []string // Track unique labels - labelsSet := make(map[string]bool) // Set for deduplicating labels - var caches []string // Track cache configurations (appended in order) - var jobsBuilder strings.Builder // Track jobs from imported YAML workflows - var features []map[string]any // Track features configurations from imports (parsed structures) - var agentFile string // Track custom agent file - var agentImportSpec string // Track agent import specification for remote imports - var repositoryImports []string // Track repository-only imports for .github folder merging - importInputs := make(map[string]any) // Aggregated input values from all imports + var bots []string // Track unique bot names + botsSet := make(map[string]bool) // Set for deduplicating bots + var plugins []string // Track unique plugin repos + pluginsSet := make(map[string]bool) // Set for deduplicating plugins + var labels []string // Track unique labels + labelsSet := make(map[string]bool) // Set for deduplicating labels + var skipRoles []string // Track unique skip-roles + skipRolesSet := make(map[string]bool) // Set for deduplicating skip-roles + var skipBots []string // Track unique skip-bots + skipBotsSet := make(map[string]bool) // Set for deduplicating skip-bots + var caches []string // Track cache configurations (appended in order) + var jobsBuilder strings.Builder // Track jobs from imported YAML workflows + var features []map[string]any // Track features configurations from imports (parsed structures) + var agentFile string // Track custom agent file + var agentImportSpec string // Track agent import specification for remote imports + var repositoryImports []string // Track repository-only imports for .github folder merging + importInputs := make(map[string]any) // Aggregated input values from all imports // Seed the queue with initial imports for _, importSpec := range importSpecs { @@ -570,7 +576,7 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a secretMaskingBuilder.WriteString(secretMaskingContent + "\n") } - // Extract bots from imported file (merge into set to avoid duplicates) + // Extract and merge bots from imported file (merge into set to avoid duplicates) botsContent, err := extractBotsFromContent(string(content)) if err == nil && botsContent != "" && botsContent != "[]" { // Parse bots JSON array @@ -585,7 +591,37 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a } } - // Extract plugins from imported file (merge into set to avoid duplicates) + // Extract and merge skip-roles from imported file (merge into set to avoid duplicates) + skipRolesContent, err := extractSkipRolesFromContent(string(content)) + if err == nil && skipRolesContent != "" && skipRolesContent != "[]" { + // Parse skip-roles JSON array + var importedSkipRoles []string + if jsonErr := json.Unmarshal([]byte(skipRolesContent), &importedSkipRoles); jsonErr == nil { + for _, role := range importedSkipRoles { + if !skipRolesSet[role] { + skipRolesSet[role] = true + skipRoles = append(skipRoles, role) + } + } + } + } + + // Extract and merge skip-bots from imported file (merge into set to avoid duplicates) + skipBotsContent, err := extractSkipBotsFromContent(string(content)) + if err == nil && skipBotsContent != "" && skipBotsContent != "[]" { + // Parse skip-bots JSON array + var importedSkipBots []string + if jsonErr := json.Unmarshal([]byte(skipBotsContent), &importedSkipBots); jsonErr == nil { + for _, user := range importedSkipBots { + if !skipBotsSet[user] { + skipBotsSet[user] = true + skipBots = append(skipBots, user) + } + } + } + } + + // Extract and merge plugins from imported file (merge into set to avoid duplicates) // This now handles both simple string format and object format with MCP configs pluginsContent, err := extractPluginsFromContent(string(content)) if err == nil && pluginsContent != "" && pluginsContent != "[]" { @@ -676,6 +712,8 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a MergedSecretMasking: secretMaskingBuilder.String(), MergedBots: bots, MergedPlugins: plugins, + MergedSkipRoles: skipRoles, + MergedSkipBots: skipBots, MergedPostSteps: postStepsBuilder.String(), MergedLabels: labels, MergedCaches: caches, diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index e68ba93073..8fb28ed72e 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1362,6 +1362,24 @@ ], "description": "Skip workflow execution for users with specific repository roles. Useful for workflows that should only run for external contributors or specific permission levels." }, + "skip-bots": { + "oneOf": [ + { + "type": "string", + "minLength": 1, + "description": "Single GitHub username to skip workflow for (e.g., 'user1'). If the triggering user matches, the workflow will be skipped." + }, + { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + }, + "description": "List of GitHub usernames to skip workflow for (e.g., ['user1', 'user2']). If the triggering user is in this list, the workflow will be skipped." + } + ], + "description": "Skip workflow execution for specific GitHub users. Useful for preventing workflows from running for specific accounts (e.g., bots, specific team members)." + }, "manual-approval": { "type": "string", "description": "Environment name that requires manual approval before the workflow can run. Must match a valid environment configured in the repository settings." diff --git a/pkg/workflow/compiler_activation_jobs.go b/pkg/workflow/compiler_activation_jobs.go index 4dddcc47b2..bcd6a08ff2 100644 --- a/pkg/workflow/compiler_activation_jobs.go +++ b/pkg/workflow/compiler_activation_jobs.go @@ -171,6 +171,22 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec steps = append(steps, generateGitHubScriptWithRequire("check_skip_roles.cjs")) } + // Add skip-bots check if configured + if len(data.SkipBots) > 0 { + // Extract workflow name for the skip-bots check + workflowName := data.Name + + steps = append(steps, " - name: Check skip-bots\n") + steps = append(steps, fmt.Sprintf(" id: %s\n", constants.CheckSkipBotsStepID)) + steps = append(steps, fmt.Sprintf(" uses: %s\n", GetActionPin("actions/github-script"))) + steps = append(steps, " env:\n") + steps = append(steps, fmt.Sprintf(" GH_AW_SKIP_BOTS: %s\n", strings.Join(data.SkipBots, ","))) + steps = append(steps, fmt.Sprintf(" GH_AW_WORKFLOW_NAME: %q\n", workflowName)) + steps = append(steps, " with:\n") + steps = append(steps, " script: |\n") + steps = append(steps, generateGitHubScriptWithRequire("check_skip_bots.cjs")) + } + // Add command position check if this is a command workflow if len(data.Command) > 0 { steps = append(steps, " - name: Check command position\n") @@ -247,6 +263,16 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec conditions = append(conditions, skipRolesCheckOk) } + if len(data.SkipBots) > 0 { + // Add skip-bots check condition + skipBotsCheckOk := BuildComparison( + BuildPropertyAccess(fmt.Sprintf("steps.%s.outputs.%s", constants.CheckSkipBotsStepID, constants.SkipBotsOkOutput)), + "==", + BuildStringLiteral("true"), + ) + conditions = append(conditions, skipBotsCheckOk) + } + if data.RateLimit != nil { // Add rate limit check condition rateLimitCheck := BuildComparison( diff --git a/pkg/workflow/compiler_jobs.go b/pkg/workflow/compiler_jobs.go index f47226ab15..8b539bba39 100644 --- a/pkg/workflow/compiler_jobs.go +++ b/pkg/workflow/compiler_jobs.go @@ -178,12 +178,13 @@ func (c *Compiler) buildPreActivationAndActivationJobs(data *WorkflowData, front hasSkipIfMatch := data.SkipIfMatch != nil hasSkipIfNoMatch := data.SkipIfNoMatch != nil hasSkipRoles := len(data.SkipRoles) > 0 + hasSkipBots := len(data.SkipBots) > 0 hasCommandTrigger := len(data.Command) > 0 hasRateLimit := data.RateLimit != nil - compilerJobsLog.Printf("Job configuration: needsPermissionCheck=%v, hasStopTime=%v, hasSkipIfMatch=%v, hasSkipIfNoMatch=%v, hasSkipRoles=%v, hasCommand=%v, hasRateLimit=%v", needsPermissionCheck, hasStopTime, hasSkipIfMatch, hasSkipIfNoMatch, hasSkipRoles, hasCommandTrigger, hasRateLimit) + compilerJobsLog.Printf("Job configuration: needsPermissionCheck=%v, hasStopTime=%v, hasSkipIfMatch=%v, hasSkipIfNoMatch=%v, hasSkipRoles=%v, hasSkipBots=%v, hasCommand=%v, hasRateLimit=%v", needsPermissionCheck, hasStopTime, hasSkipIfMatch, hasSkipIfNoMatch, hasSkipRoles, hasSkipBots, hasCommandTrigger, hasRateLimit) - // Build pre-activation job if needed (combines membership checks, stop-time validation, skip-if-match check, skip-if-no-match check, skip-roles check, rate limit check, and command position check) - if needsPermissionCheck || hasStopTime || hasSkipIfMatch || hasSkipIfNoMatch || hasSkipRoles || hasCommandTrigger || hasRateLimit { + // Build pre-activation job if needed (combines membership checks, stop-time validation, skip-if-match check, skip-if-no-match check, skip-roles check, skip-bots check, rate limit check, and command position check) + if needsPermissionCheck || hasStopTime || hasSkipIfMatch || hasSkipIfNoMatch || hasSkipRoles || hasSkipBots || hasCommandTrigger || hasRateLimit { compilerJobsLog.Print("Building pre-activation job") preActivationJob, err := c.buildPreActivationJob(data, needsPermissionCheck) if err != nil { diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 1e233ff00d..8ccab3c550 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -437,7 +437,8 @@ func (c *Compiler) extractAdditionalConfigurations( workflowData.Roles = c.extractRoles(frontmatter) workflowData.Bots = c.extractBots(frontmatter) workflowData.RateLimit = c.extractRateLimitConfig(frontmatter) - workflowData.SkipRoles = c.extractSkipRoles(frontmatter) + workflowData.SkipRoles = c.mergeSkipRoles(c.extractSkipRoles(frontmatter), importsResult.MergedSkipRoles) + workflowData.SkipBots = c.mergeSkipBots(c.extractSkipBots(frontmatter), importsResult.MergedSkipBots) // Use the already extracted output configuration workflowData.SafeOutputs = safeOutputs diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 9fb4382184..013fbb89f0 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -417,6 +417,7 @@ type WorkflowData struct { SkipIfMatch *SkipIfMatchConfig // skip-if-match configuration with query and max threshold SkipIfNoMatch *SkipIfNoMatchConfig // skip-if-no-match configuration with query and min threshold SkipRoles []string // roles to skip workflow for (e.g., [admin, maintainer, write]) + SkipBots []string // users to skip workflow for (e.g., [user1, user2]) ManualApproval string // environment name for manual approval from on: section Command []string // for /command trigger support - multiple command names CommandEvents []string // events where command should be active (nil = all events) diff --git a/pkg/workflow/frontmatter_extraction_yaml.go b/pkg/workflow/frontmatter_extraction_yaml.go index eb227509e3..50dd76c69a 100644 --- a/pkg/workflow/frontmatter_extraction_yaml.go +++ b/pkg/workflow/frontmatter_extraction_yaml.go @@ -156,6 +156,7 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inSkipIfMatch := false inSkipIfNoMatch := false inSkipRolesArray := false + inSkipBotsArray := false currentSection := "" // Track which section we're in ("issues", "pull_request", "discussion", or "issue_comment") for _, line := range lines { @@ -230,6 +231,13 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inSkipRolesArray = true } + // Check if we're entering skip-bots array + if !inPullRequest && !inIssues && !inDiscussion && !inIssueComment && strings.HasPrefix(trimmedLine, "skip-bots:") { + // Check if this is an array (next line will be "- ") + // We'll set the flag and handle it on the next iteration + inSkipBotsArray = true + } + // Check if we're entering skip-if-match object if !inPullRequest && !inIssues && !inDiscussion && !inIssueComment && !inSkipIfMatch { // Check both uncommented and commented forms @@ -296,6 +304,17 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat } } + // Check if we're leaving the skip-bots array by encountering another top-level field + if inSkipBotsArray && 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 skip-bots (2 spaces), we're out of the array + if lineIndent == 2 && !strings.HasPrefix(trimmedLine, "-") && !strings.HasPrefix(trimmedLine, "skip-bots:") && !strings.HasPrefix(trimmedLine, "#") { + inSkipBotsArray = false + } + } + // Determine if we should comment out this line shouldComment := false var commentReason string @@ -329,6 +348,13 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat // Comment out array items in skip-roles shouldComment = true commentReason = " # Skip-roles processed as role check in pre-activation job" + } else if strings.HasPrefix(trimmedLine, "skip-bots:") { + shouldComment = true + commentReason = " # Skip-bots processed as bot check in pre-activation job" + } else if inSkipBotsArray && strings.HasPrefix(trimmedLine, "-") { + // Comment out array items in skip-bots + shouldComment = true + commentReason = " # Skip-bots processed as bot check in pre-activation job" } else if strings.HasPrefix(trimmedLine, "reaction:") { shouldComment = true commentReason = " # Reaction processed as activation job step" diff --git a/pkg/workflow/role_checks.go b/pkg/workflow/role_checks.go index f31b40645c..5ae43b5eab 100644 --- a/pkg/workflow/role_checks.go +++ b/pkg/workflow/role_checks.go @@ -489,6 +489,27 @@ func (c *Compiler) extractSkipRoles(frontmatter map[string]any) []string { return nil } +// extractSkipBots extracts the 'skip-bots' field from the 'on:' section of frontmatter +// Returns nil if skip-bots is not configured +func (c *Compiler) extractSkipBots(frontmatter map[string]any) []string { + // Check the "on" section in frontmatter + onValue, exists := frontmatter["on"] + if !exists || onValue == nil { + return nil + } + + // Handle different formats of the on: section + switch on := onValue.(type) { + case map[string]any: + // Complex object format - look for skip-bots + if skipBotsValue, exists := on["skip-bots"]; exists && skipBotsValue != nil { + return extractStringSliceField(skipBotsValue, "skip-bots") + } + } + + return nil +} + // extractStringSliceField extracts a string slice from various input formats // Handles: string, []string, []any (with string elements) // Returns nil if the input is empty or invalid @@ -525,3 +546,61 @@ func extractStringSliceField(value any, fieldName string) []string { roleLog.Printf("No valid %s found or unsupported type: %T", fieldName, value) return nil } + +// mergeSkipRoles merges top-level skip-roles with imported skip-roles (union) +func (c *Compiler) mergeSkipRoles(topSkipRoles []string, importedSkipRoles []string) []string { + // Create a set for deduplication + rolesSet := make(map[string]bool) + var result []string + + // Add top-level skip-roles first + for _, role := range topSkipRoles { + if !rolesSet[role] { + rolesSet[role] = true + result = append(result, role) + } + } + + // Merge imported skip-roles + for _, role := range importedSkipRoles { + if !rolesSet[role] { + rolesSet[role] = true + result = append(result, role) + } + } + + if len(result) > 0 { + roleLog.Printf("Merged skip-roles: %v (top=%d, imported=%d, total=%d)", result, len(topSkipRoles), len(importedSkipRoles), len(result)) + } + + return result +} + +// mergeSkipBots merges top-level skip-bots with imported skip-bots (union) +func (c *Compiler) mergeSkipBots(topSkipBots []string, importedSkipBots []string) []string { + // Create a set for deduplication + usersSet := make(map[string]bool) + var result []string + + // Add top-level skip-bots first + for _, user := range topSkipBots { + if !usersSet[user] { + usersSet[user] = true + result = append(result, user) + } + } + + // Merge imported skip-bots + for _, user := range importedSkipBots { + if !usersSet[user] { + usersSet[user] = true + result = append(result, user) + } + } + + if len(result) > 0 { + roleLog.Printf("Merged skip-bots: %v (top=%d, imported=%d, total=%d)", result, len(topSkipBots), len(importedSkipBots), len(result)) + } + + return result +} diff --git a/pkg/workflow/skip_bots_test.go b/pkg/workflow/skip_bots_test.go new file mode 100644 index 0000000000..607bc2d7dd --- /dev/null +++ b/pkg/workflow/skip_bots_test.go @@ -0,0 +1,313 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + + "github.com/github/gh-aw/pkg/stringutil" + "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestSkipBotsPreActivationJob tests that skip-bots check is created correctly in pre-activation job +func TestSkipBotsPreActivationJob(t *testing.T) { + tmpDir := testutil.TempDir(t, "skip-bots-test") + compiler := NewCompiler() + + t.Run("pre_activation_job_created_with_skip_bots", func(t *testing.T) { + workflowContent := `--- +on: + issues: + types: [opened] + skip-bots: [user1, user2, user3] +engine: copilot +--- + +# Skip Users Workflow + +This workflow has a skip-bots configuration. +` + workflowFile := filepath.Join(tmpDir, "skip-bots-workflow.md") + err := os.WriteFile(workflowFile, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to write workflow file") + + err = compiler.CompileWorkflow(workflowFile) + require.NoError(t, err, "Compilation failed") + + lockFile := stringutil.MarkdownToLockFile(workflowFile) + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + + lockContentStr := string(lockContent) + + // Verify pre_activation job exists + assert.Contains(t, lockContentStr, "pre_activation:", "Expected pre_activation job to be created") + + // Verify skip-bots check is present + assert.Contains(t, lockContentStr, "Check skip-bots", "Expected skip-bots check to be present") + + // Verify the skip users environment variable is set correctly + assert.Contains(t, lockContentStr, "GH_AW_SKIP_BOTS: user1,user2,user3", "Expected GH_AW_SKIP_BOTS environment variable with correct value") + + // Verify the check_skip_bots step ID is present + assert.Contains(t, lockContentStr, "id: check_skip_bots", "Expected check_skip_bots step ID") + + // Verify the activated output includes skip_bots_ok condition + assert.Contains(t, lockContentStr, "steps.check_skip_bots.outputs.skip_bots_ok", "Expected activated output to include skip_bots_ok condition") + + // Verify skip-bots is commented out in the frontmatter + assert.Contains(t, lockContentStr, "# skip-bots:", "Expected skip-bots to be commented out in lock file") + }) + + t.Run("skip_bots_with_single_user", func(t *testing.T) { + workflowContent := `--- +on: + issues: + types: [opened] + skip-bots: user1 +engine: copilot +--- + +# Skip Users Single User + +This workflow skips only for user1. +` + workflowFile := filepath.Join(tmpDir, "skip-bots-single.md") + err := os.WriteFile(workflowFile, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to write workflow file") + + err = compiler.CompileWorkflow(workflowFile) + require.NoError(t, err, "Compilation failed") + + lockFile := stringutil.MarkdownToLockFile(workflowFile) + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + + lockContentStr := string(lockContent) + + // Verify skip-bots check is present + assert.Contains(t, lockContentStr, "Check skip-bots", "Expected skip-bots check to be present") + + // Verify single user + assert.Contains(t, lockContentStr, "GH_AW_SKIP_BOTS: user1", "Expected GH_AW_SKIP_BOTS with single user") + }) + + t.Run("no_skip_bots_no_check_created", func(t *testing.T) { + workflowContent := `--- +on: + issues: + types: [opened] +engine: copilot +--- + +# No Skip Users Workflow + +This workflow has no skip-bots configuration. +` + workflowFile := filepath.Join(tmpDir, "no-skip-bots.md") + err := os.WriteFile(workflowFile, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to write workflow file") + + err = compiler.CompileWorkflow(workflowFile) + require.NoError(t, err, "Compilation failed") + + lockFile := stringutil.MarkdownToLockFile(workflowFile) + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + + lockContentStr := string(lockContent) + + // Verify skip-bots check is NOT present + assert.NotContains(t, lockContentStr, "Check skip-bots", "Expected skip-bots check to NOT be present") + assert.NotContains(t, lockContentStr, "GH_AW_SKIP_BOTS", "Expected GH_AW_SKIP_BOTS to NOT be present") + assert.NotContains(t, lockContentStr, "check_skip_bots", "Expected check_skip_bots step to NOT be present") + }) + + t.Run("skip_bots_with_roles_field", func(t *testing.T) { + workflowContent := `--- +on: + issues: + types: [opened] + skip-bots: [user1, user2] +roles: [maintainer] +engine: copilot +--- + +# Skip Users with Roles Field + +This workflow has both roles and skip-bots. +` + workflowFile := filepath.Join(tmpDir, "skip-bots-with-roles.md") + err := os.WriteFile(workflowFile, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to write workflow file") + + err = compiler.CompileWorkflow(workflowFile) + require.NoError(t, err, "Compilation failed") + + lockFile := stringutil.MarkdownToLockFile(workflowFile) + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + + lockContentStr := string(lockContent) + + // Verify both membership check and skip-bots check are present + assert.Contains(t, lockContentStr, "Check team membership", "Expected team membership check to be present") + assert.Contains(t, lockContentStr, "Check skip-bots", "Expected skip-bots check to be present") + + // Verify GH_AW_REQUIRED_ROLES is set + assert.Contains(t, lockContentStr, "GH_AW_REQUIRED_ROLES: maintainer", "Expected GH_AW_REQUIRED_ROLES for roles field") + + // Verify GH_AW_SKIP_BOTS is set + assert.Contains(t, lockContentStr, "GH_AW_SKIP_BOTS: user1,user2", "Expected GH_AW_SKIP_BOTS for skip-bots field") + + // Verify both conditions in activated output + assert.Contains(t, lockContentStr, "steps.check_membership.outputs.is_team_member", "Expected membership check in activated output") + assert.Contains(t, lockContentStr, "steps.check_skip_bots.outputs.skip_bots_ok", "Expected skip-bots check in activated output") + }) + + t.Run("skip_bots_and_skip_roles_combined", func(t *testing.T) { + workflowContent := `--- +on: + issues: + types: [opened] + skip-roles: [admin, write] + skip-bots: [user1, user2] +engine: copilot +--- + +# Skip Users and Skip Roles Combined + +This workflow has both skip-roles and skip-bots. +` + workflowFile := filepath.Join(tmpDir, "skip-bots-and-roles.md") + err := os.WriteFile(workflowFile, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to write workflow file") + + err = compiler.CompileWorkflow(workflowFile) + require.NoError(t, err, "Compilation failed") + + lockFile := stringutil.MarkdownToLockFile(workflowFile) + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + + lockContentStr := string(lockContent) + + // Verify both skip-roles and skip-bots checks are present + assert.Contains(t, lockContentStr, "Check skip-roles", "Expected skip-roles check to be present") + assert.Contains(t, lockContentStr, "Check skip-bots", "Expected skip-bots check to be present") + + // Verify both environment variables are set + assert.Contains(t, lockContentStr, "GH_AW_SKIP_ROLES: admin,write", "Expected GH_AW_SKIP_ROLES for skip-roles field") + assert.Contains(t, lockContentStr, "GH_AW_SKIP_BOTS: user1,user2", "Expected GH_AW_SKIP_BOTS for skip-bots field") + + // Verify both conditions in activated output + assert.Contains(t, lockContentStr, "steps.check_skip_roles.outputs.skip_roles_ok", "Expected skip-roles check in activated output") + assert.Contains(t, lockContentStr, "steps.check_skip_bots.outputs.skip_bots_ok", "Expected skip-bots check in activated output") + }) +} + +// TestExtractSkipBots tests the extractSkipBots function +func TestExtractSkipBots(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + frontmatter map[string]any + expected []string + }{ + { + name: "skip-bots as array of strings", + frontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []string{"opened"}, + }, + "skip-bots": []string{"user1", "user2"}, + }, + }, + expected: []string{"user1", "user2"}, + }, + { + name: "skip-bots as single string", + frontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []string{"opened"}, + }, + "skip-bots": "user1", + }, + }, + expected: []string{"user1"}, + }, + { + name: "skip-bots as array of any", + frontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []string{"opened"}, + }, + "skip-bots": []any{"user1", "user2", "user3"}, + }, + }, + expected: []string{"user1", "user2", "user3"}, + }, + { + name: "no skip-bots configured", + frontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []string{"opened"}, + }, + }, + }, + expected: nil, + }, + { + name: "empty skip-bots array", + frontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []string{"opened"}, + }, + "skip-bots": []string{}, + }, + }, + expected: nil, + }, + { + name: "skip-bots as empty string", + frontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []string{"opened"}, + }, + "skip-bots": "", + }, + }, + expected: nil, + }, + { + name: "on as string (no skip-bots possible)", + frontmatter: map[string]any{ + "on": "push", + }, + expected: nil, + }, + { + name: "no on section", + frontmatter: map[string]any{}, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := compiler.extractSkipBots(tt.frontmatter) + assert.Equal(t, tt.expected, result, "extractSkipBots result mismatch") + }) + } +}