Conversation
- Updated test to expect 20 codemods instead of 19 - Added bots-to-on-bots to expected codemod order - Fixed workflow tests to use on.roles and on.bots structure - Fixed TestExtractAdditionalConfigurations to use workflow_dispatch instead of invalid push: null - Fixed TestParseWorkflowFile_CompleteWorkflowWithAllSections to include all required frontmatter fields - Fixed role_checks.go to skip roles and bots when checking for unsafe events Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan
deleted the
copilot/fix-tests-c349c77c-e393-415d-b63b-928ad3c03cde
branch
There was a problem hiding this comment.
Pull request overview
Updates failing tests and aligns workflow parsing behavior with the newer on.roles / on.bots frontmatter structure.
Changes:
- Treat
on.rolesandon.botsas non-event configuration when evaluating whether a workflow uses only “safe” events. - Update workflow parsing/extraction tests to configure roles/bots under
on:and expand the “complete workflow” fixture. - Update codemod registry tests for the new codemod count and expected ordering.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/workflow/role_checks.go | Updates safe-event detection to ignore roles/bots entries under on:. |
| pkg/workflow/compiler_orchestrator_workflow_test.go | Adjusts tests to set roles/bots under on: and expands the full-workflow fixture. |
| pkg/workflow/compiler_orchestrator_test.go | Updates YAML fixture trigger to keep extraction tests passing with current parsing expectations. |
| pkg/cli/fix_codemods_test.go | Updates expected codemod count and ordering to include the new codemod ID. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Skip roles and bots as they are configuration, not event types | ||
| if eventName == "command" || eventName == "stop-after" || eventName == "reaction" || eventName == "roles" || eventName == "bots" { | ||
| continue |
There was a problem hiding this comment.
hasSafeEventsOnly now correctly ignores on.roles/on.bots, but it still treats other non-event configuration keys under on: (notably status-comment) as an event. That can cause workflows that only use safe triggers (e.g., workflow_dispatch/schedule) to be classified as having “unsafe events” and unnecessarily require the membership/role precheck. Consider also skipping and subtracting status-comment here (consistent with compiler_safe_outputs.go filtering it out of the event set).
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.