Update staging-lint-check workflow to block PR merge upon failure#823
Update staging-lint-check workflow to block PR merge upon failure#823AkhilaIlla wants to merge 3 commits intomainfrom
Conversation
AkhilaIlla
requested review from
mikeharder,
raosuhas,
tejaswiMinnu and
weshaggard
as code owners
AkhilaIlla
changed the title
| if (!pr) return false; | ||
|
|
||
| // Paginate to handle PRs with many changed files | ||
| const files = await github.paginate(github.rest.pulls.listFiles, { |
There was a problem hiding this comment.
you could also get this info from git.exe locally if you cloned with depth 2, but aov repo doesn't use much GH API quota, so this also works
|
@copilot: fix the linter error |
|
@mikeharder I've opened a new pull request, #828, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Updates the staging lint checks workflow to become merge-blocking, relying on PR labels for rule selection and using the GitHub API to detect when rule source files change.
Changes:
- Make staging lint checks fail the workflow (and block merge) on command failures and on validation errors unless
errors-acknowledgedis present. - Detect linter rule source changes via
pulls.listFilesand requiretest-<RuleName>labels when rule files change. - Remove PR-body-based rule selection and document the new gating/label behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/test/runner.test.js | Updates tests to reflect removal of FAIL_ON_ERRORS env usage. |
| .github/workflows/test/extract-rule-names.test.js | Updates tests for label-only selection and adds coverage for new gating/detection helpers. |
| .github/workflows/staging-lint-checks.yaml | Updates workflow triggers/sparse checkout and switches to default export + passes github into the script. |
| .github/workflows/src/extract-rule-names-and-run-validation.js | Implements merge-blocking gates, rule-change detection via GitHub API, and label-only rule selection. |
| .github/workflows/README-staging-lint-checks.md | Documents the new merge-blocking gates and label-only usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uses: actions/github-script@v8 | ||
| with: | ||
| script: | | ||
| const { runInGitHubActions } = await import('${{ github.workspace }}/.github/workflows/src/extract-rule-names-and-run-validation.js'); | ||
| await runInGitHubActions({ context, core }); | ||
| const { default: runInGitHubActions } = await import('${{ github.workspace }}/.github/workflows/src/extract-rule-names-and-run-validation.js'); | ||
| await runInGitHubActions({ github, context, core }); |
There was a problem hiding this comment.
This step now passes github into the script, which (per the updated JS) calls github.rest.pulls.listFiles to detect rule changes. However, the workflow’s top-level permissions only grants contents: read; with explicit permissions, all other scopes become none, so GITHUB_TOKEN may lack pull-requests: read and the pulls.listFiles call can fail (403), breaking the check.
Add pull-requests: read to the workflow permissions so rule-change detection can reliably query PR files.
| * Covers both spectral and native rule paths. | ||
| */ | ||
| export const RULE_FILE_PATTERN = | ||
| /^packages\/rulesets\/src\/spectral\/(az-arm|az-common|az-dataplane|functions\/.+)\.ts$|^packages\/rulesets\/src\/native\/(legacyRules|functions|rulesets)\/.+\.ts$/; |
There was a problem hiding this comment.
RULE_FILE_PATTERN is used to decide whether “rule changes” occurred, but it currently misses rule-impacting source files outside the matched folders (e.g. packages/rulesets/src/spectral/utils.ts is exported from packages/rulesets/src/index.ts, and native rules import helpers from packages/rulesets/src/native/utilities/*).
As written, changes to those files won’t trigger ruleChangesDetected, so a PR could modify rule logic without being forced to add test-<RuleName> labels. Consider broadening the pattern (or switching to directory-prefix checks) to include those utility paths too.
| /^packages\/rulesets\/src\/spectral\/(az-arm|az-common|az-dataplane|functions\/.+)\.ts$|^packages\/rulesets\/src\/native\/(legacyRules|functions|rulesets)\/.+\.ts$/; | |
| /^packages\/rulesets\/src\/spectral\/(az-arm|az-common|az-dataplane|functions\/.+|utils)\.ts$|^packages\/rulesets\/src\/native\/(legacyRules|functions|rulesets|utilities)\/.+\.ts$/; |
| * Blocking scenarios: | ||
| * 1. Command failures — AutoRest or script crashes | ||
| * 2. Validation errors without "errors-acknowledged" label | ||
| * |
There was a problem hiding this comment.
The numbering in the “Blocking scenarios” comment doesn’t match the numbering used in the code below (comment mentions scenarios 1–2, while the implementation refers to “Scenario 2” and “Scenario 3”). This makes the gating logic harder to follow.
Align the comment and the inline scenario labels so they refer to the same set of scenarios.
This PR improves the staging lint check workflow by adding merge-blocking capabilities, detecting linter rule source changes via the GitHub API, and simplifying rule selection to labels only.