ci: cancel superseded pull request runs - #4854
Conversation
The per-job concurrency groups already cancelled a superseded job when its replacement was queued, but they cancelled pushes to the release branches the same way. Those runs publish coverage, and cancelling one also hides a breakage that is already on the branch. Replace them with a single workflow-level group. Only pull request runs share it; every other run gets `github.run_id`, a group of one, because a group holds at most one pending run and GitHub evicts that pending run whenever a newer one enters the group — `cancel-in-progress` protects the running run, not the queued one. Besides fixing that, one workflow-level group cancels the whole superseded run at once rather than job by job as each replacement is queued, it covers jobs that have no group of their own, and a matrix dimension added later cannot be forgotten in a group key. `release.yml` is untouched: its string-form group leaves `cancel-in-progress` false, so releases queue rather than cancel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016aGHrb1YaEvaGwNELGEHjF
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughThe workflow adds one top-level Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to Pull-request runs are consolidated and superseded runs are cancelled, while other runs remain independent; no merge-blocking risk was found. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Issue
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This PR is packaged and the instant preview is available (1dd90a4). Install it locally:
npm i -D webpack-cli@https://pkg.pr.new/webpack-cli@1dd90a4
yarn add -D webpack-cli@https://pkg.pr.new/webpack-cli@1dd90a4
pnpm add -D webpack-cli@https://pkg.pr.new/webpack-cli@1dd90a4 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4854 +/- ##
==========================================
+ Coverage 94.81% 94.83% +0.01%
==========================================
Files 14 14
Lines 5598 5598
Branches 837 837
==========================================
+ Hits 5308 5309 +1
+ Misses 289 288 -1
Partials 1 1 see 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
nodejs.ymlalready cancelled superseded jobs, but through three per-job concurrency groups that also cancel pushes tomainandnext. This replaces them with one workflow-level group that only cancels pull request runs.Why the branch pushes should not be superseded
mainandnextruns publish the coverage later comparisons are measured against, and a cancelled run also hides a breakage that is already on the branch — which is exactly when you want to know.Note that
cancel-in-progress: falsealone would not be enough: a concurrency group holds at most one pending run, and GitHub evicts that pending run whenever a newer one enters the group regardless of the flag — "any existing pending job or workflow in the same concurrency group will be canceled and the new queued job or workflow will take its place." Hencegithub.run_idfor non-pull-request events: each gets a group of one, so it can be neither cancelled nor evicted.Why one workflow-level group rather than fixing the three in place
Adding the condition to the existing blocks would have been a smaller diff, but the per-job arrangement has two other costs:
lintandsmoketests.testkey currently spells out four of them (os,node-version,webpack-version,dev-server-version); a dimension added later and forgotten would make cells cancel each other, which is a quiet and confusing failure. This is not hypothetical — ci: stop sharding the test matrix and drop Node 25 webpack-dev-server#5741 had to edit its group key purely because the matrix changed. Keying on the workflow and ref cannot drift.Happy to switch to the minimal version (keep the per-job groups, just add the condition) if you would rather keep the existing shape.
Verification
actionlint1.7.7 is clean onnodejs.yml, and I checked that this means something by typo'ing the expression togithub.event_nam, which it catches at exactly that line. The file is Prettier- and cspell-clean using this repo's.cspell.json.Unrelated, but worth flagging since you will see it if you run the linter:
actionlintreports two pre-existing errors inpublish-to-pkg-pr-new.yml(steps.publish.outputs.sha/.urlsreferencing a step id that is not defined in that job). They reproduce onmainuntouched, so they are not from this PR and I have left them alone.Untouched
release.yml— already uses the string form, so releases queue instead of cancelling.dependabot.yml,dependency-review.yml,publish-to-pkg-pr-new.yml,update-docs.yml— left as they are to keep this diff to the expensive workflow.publish-to-pkg-pr-new.ymlin particular publishes preview packages, so I would not want to add cancellation to it without a maintainer's view.The same change is already open on
tapable(webpack/tapable#273),watchpack(webpack/watchpack#341),webpack-sources(webpack/webpack-sources#282),schema-utils(webpack/schema-utils#228),enhanced-resolve(webpack/enhanced-resolve#673),webpack-dev-server(webpack/webpack-dev-server#5742) andwebpack-dev-middleware(webpack/webpack-dev-middleware#2412).🤖 Generated with Claude Code
https://claude.ai/code/session_016aGHrb1YaEvaGwNELGEHjF
Generated by Claude Code
Summary by CodeRabbit