Add on.skip-bots field to cancel workflows for specific GitHub actors#15993
Add on.skip-bots field to cancel workflows for specific GitHub actors#15993
Conversation
- Add extractSkipUsers() function in role_checks.go - Add SkipUsers field to WorkflowData type - Add CheckSkipUsersStepID and SkipUsersOkOutput constants - Create check_skip_users.cjs JavaScript check script - Generate skip-users check step in pre-activation job - Add skip-users condition to activated output - Update frontmatter YAML extraction for skip-users comments - Add skip-users to JSON schema - Add comprehensive tests in skip_users_test.go (all passing) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Add MergedSkipRoles and MergedSkipUsers to ImportsResult - Create extractSkipRolesFromContent and extractSkipUsersFromContent functions - Add extractOnSectionField helper for extracting from on: section - Merge skip-roles and skip-users from imports in import_processor.go - Add merge functions mergeSkipRoles and mergeSkipUsers in role_checks.go - Update frontmatter hash to include merged skip-roles and skip-users - All skip-users tests passing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot add skip-users github-actions and copilot to ai-moderator. Match user name or bot name (+ [bot]) |
|
@copilot rename skip-users to skip-bots |
- Update check_skip_users.cjs to match both username and username[bot] - Support bidirectional matching: "github-actions" matches "github-actions[bot]" and vice versa - Add skip-users: [github-actions, copilot] to ai-moderator workflow - Add comprehensive tests for bot name matching (9 tests, all passing) - Compiled ai-moderator.lock.yml with skip-users check integrated Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit 3406284. Added The implementation now supports flexible bot matching:
All tests passing (9 JavaScript tests + 5 Go integration tests). Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
- Rename check_skip_users.cjs → check_skip_bots.cjs - Rename check_skip_users.test.cjs → check_skip_bots.test.cjs - Rename skip_users_test.go → skip_bots_test.go - Update all function names: extractSkipUsers → extractSkipBots - Update all variable names: SkipUsers → SkipBots, skipUsers → skipBots - Update all constants: CheckSkipUsersStepID → CheckSkipBotsStepID, SkipUsersOkOutput → SkipBotsOkOutput - Update environment variable: GH_AW_SKIP_USERS → GH_AW_SKIP_BOTS - Update frontmatter field: skip-users → skip-bots in schema and extraction - Update ai-moderator.md: skip-users → skip-bots - Recompiled ai-moderator.lock.yml with skip-bots - All tests passing (Go: 5/5, JavaScript: 9/9) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit a39d53f. Renamed
All tests passing (Go: 5/5, JavaScript: 9/9). Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR adds the on.skip-bots frontmatter field to enable workflows to skip execution based on the github.actor identity. The feature mirrors the existing skip-roles pattern, providing a mechanism to prevent workflows from running for specific GitHub users or bots.
Changes:
- Core implementation for extracting, merging, and checking skip-bots configuration with flexible bot name matching
- Import merging support for skip-bots across workflow imports (union merge)
- Schema updates, YAML commenting in lock files, and integration with pre-activation job
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/skip_bots_test.go | Comprehensive test suite for skip-bots functionality including pre-activation job creation, single/multiple bot configurations, and interaction with roles field |
| pkg/workflow/role_checks.go | Added extractSkipBots() and mergeSkipBots() functions following the skip-roles pattern |
| pkg/workflow/frontmatter_extraction_yaml.go | Added logic to comment out skip-bots field in compiled lock files |
| pkg/workflow/compiler_types.go | Added SkipBots field to WorkflowData struct |
| pkg/workflow/compiler_orchestrator_workflow.go | Integrated skip-bots extraction and merging in workflow configuration |
| pkg/workflow/compiler_jobs.go | Added hasSkipBots flag to pre-activation job trigger logic |
| pkg/workflow/compiler_activation_jobs.go | Added skip-bots check step and condition to pre-activation job |
| pkg/parser/schemas/main_workflow_schema.json | Added skip-bots schema definition supporting string or array formats |
| pkg/parser/import_processor.go | Added MergedSkipBots field and union merge logic for imported workflows |
| pkg/parser/frontmatter_hash.go | Added merged-skip-bots to canonical frontmatter hash |
| pkg/parser/content_extractor.go | Added extractSkipBotsFromContent() and extractOnSectionField() helper for on: section extraction |
| pkg/constants/constants.go | Added CheckSkipBotsStepID and SkipBotsOkOutput constants |
| actions/setup/js/check_skip_bots.test.cjs | Test suite for bot matching logic including exact matches, [bot] suffix handling, and whitespace handling |
| actions/setup/js/check_skip_bots.cjs | JavaScript check implementation with flexible bot name matching (base name matches both plain and [bot] suffix variants) |
| .github/workflows/ai-moderator.md | Applied skip-bots configuration to skip github-actions and copilot bots |
| .github/workflows/ai-moderator.lock.yml | Compiled lock file showing skip-bots commented out and integrated into pre-activation job |
Comments suppressed due to low confidence (5)
pkg/workflow/skip_bots_test.go:53
- Inconsistent terminology in comment: Says "skip users environment variable" but should say "skip-bots environment variable" to match the feature name and variable name (GH_AW_SKIP_BOTS).
// Verify the skip users environment variable is set correctly
pkg/workflow/skip_bots_test.go:107
- Inconsistent terminology: Test title says "No Skip Users Workflow" but should say "No Skip Bots Workflow" to match the feature name.
# No Skip Users Workflow
pkg/workflow/skip_bots_test.go:140
- Inconsistent terminology: Test title says "Skip Users with Roles Field" but should say "Skip Bots with Roles Field" to match the feature name.
# Skip Users with Roles Field
pkg/workflow/skip_bots_test.go:182
- Inconsistent terminology: Test title says "Skip Users and Skip Roles Combined" but should say "Skip Bots and Skip Roles Combined" to match the feature name.
# Skip Users and Skip Roles Combined
pkg/workflow/skip_bots_test.go:75
- Inconsistent terminology: Test title says "Skip Users Single User" but should say "Skip Bots Single User" or "Skip Bots Single Bot" to match the feature name.
# Skip Users Single User
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| engine: copilot | ||
| --- | ||
|
|
||
| # Skip Users Workflow |
There was a problem hiding this comment.
Inconsistent terminology: The test workflow title says "Skip Users Workflow" but the feature is called "skip-bots". This should be updated to "Skip Bots Workflow" to match the feature name.
This issue also appears in the following locations of the same file:
- line 53
- line 107
- line 140
- line 182
- line 75
| 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 |
There was a problem hiding this comment.
Inconsistent variable naming: The function is called mergeSkipBots and uses parameter names like topSkipBots and importedSkipBots, but the internal variables are named usersSet and user. For consistency with the function name and to match the pattern used in mergeSkipRoles (which uses rolesSet and role), these should be renamed to botsSet and bot.
| 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]) |
There was a problem hiding this comment.
Inconsistent terminology in comment: Says "users to skip workflow for" but should say "bots to skip workflow for" or "GitHub actors to skip workflow for" to be consistent with the field name SkipBots. The PR description mentions this is for bots/users, so "GitHub actors" might be more accurate.
| SkipBots []string // users to skip workflow for (e.g., [user1, user2]) | |
| SkipBots []string // GitHub actors to skip workflow for (e.g., [user1, user2]) |
| "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)." | ||
| }, |
There was a problem hiding this comment.
Schema description inconsistency: The description says "Single GitHub username" and "List of GitHub usernames" but the overall description mentions "specific accounts (e.g., bots, specific team members)". Consider using more consistent terminology throughout, such as "GitHub actor" or explicitly mentioning both users and bots in all descriptions. For example: "Single GitHub username (user or bot)" and "List of GitHub usernames (users or bots)".
| 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) |
There was a problem hiding this comment.
Inconsistent terminology in comment: Says "union of usernames" but should say "union of bot/user names" or "union of GitHub actors" to be consistent with the field name MergedSkipBots and the feature purpose.
| MergedSkipBots []string // Merged skip-bots list from all imports (union of usernames) | |
| MergedSkipBots []string // Merged skip-bots list from all imports (union of bot/user names) |
| for _, user := range importedSkipBots { | ||
| if !skipBotsSet[user] { | ||
| skipBotsSet[user] = true | ||
| skipBots = append(skipBots, user) |
There was a problem hiding this comment.
Inconsistent variable naming: When iterating over importedSkipBots, the loop variable is named user. For consistency with the feature name and to match the pattern used for importedSkipRoles (which uses role), this should be renamed to bot or actor.
| for _, user := range importedSkipBots { | |
| if !skipBotsSet[user] { | |
| skipBotsSet[user] = true | |
| skipBots = append(skipBots, user) | |
| for _, bot := range importedSkipBots { | |
| if !skipBotsSet[bot] { | |
| skipBotsSet[bot] = true | |
| skipBots = append(skipBots, bot) |
Adds
on.skip-botsfrontmatter field to skip workflow execution based ongithub.actoridentity, complementing the existingon.skip-rolesfunctionality.Changes
extractSkipBots(),check_skip_bots.cjsstep, pre-activation job integration withCheckSkipBotsStepIDconstant[bot]suffix (e.g.,github-actionsmatches bothgithub-actionsandgithub-actions[bot]actors)MergedSkipBotsinImportsResult,extractOnSectionField()helper foron:section extractionskip-botsto main workflow schema supporting string or array formatsskip-botsin compiled lock files (treated as execution-time check, not static config)skip-bots: [github-actions, copilot]to ai-moderator workflowUsage
Workflow skips when
github.actormatches any listed bot/user. Bot names are matched flexibly - specifyinggithub-actionswill match bothgithub-actionsandgithub-actions[bot]actors automatically. Check happens in pre-activation job before agent execution. Merged as union when importing workflows.Follows identical pattern to
skip-rolesfor consistency.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.