ci: run PR workflows when a PR's base branch changes - #4022
Conversation
When a stacked PR is reordered, its head is pushed before its base is retargeted. The push can conflict with the old base, so GitHub skips the `synchronize` run, and the retarget is an `edited` event, which the workflow did not listen to. The commit then never gets a CodSpeed run, and PRs stacked on it are compared against an older base. Also listen to `edited`, but only run on base changes, and keep other edits out of the concurrency group so they can't cancel a run. Check that the test merge commit contains the PR head before uploading results filed under it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4022 +/- ##
==========================================
- Coverage 96.98% 96.84% -0.14%
==========================================
Files 113 118 +5
Lines 39657 40002 +345
Branches 39657 40002 +345
==========================================
+ Hits 38461 38740 +279
- Misses 1182 1236 +54
- Partials 14 26 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Merging this PR will regress 4 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | coalesce_acked_from_zero 1 ranges |
2.8 µs | 3 µs | -5.51% |
| ❌ | Simulation | simulated/pacing-false/varying-seeds |
72.7 ms | 76.6 ms | -5.06% |
| ❌ | Simulation | coalesce_acked_from_zero 3 ranges |
3.8 µs | 4 µs | -4.11% |
| ❌ | Simulation | simulated/pacing-false/same-seed |
75.6 ms | 77.6 ms | -2.57% |
| ⚡ | WallTime | walltime/pacing-true/same-seed |
2.1 ms | 2 ms | +5.31% |
| ⚡ | WallTime | quiche-neqo |
51.2 ms | 48.7 ms | +5.21% |
| ⚡ | WallTime | walltime/1000-streams/each-1000-bytes |
11.6 ms | 11.1 ms | +4.58% |
| ⚡ | WallTime | walltime/1000-streams/each-1-bytes |
3.8 ms | 3.6 ms | +4.5% |
| ⚡ | WallTime | 1-conn/1-1b-resp (aka. HPS) |
3 ms | 2.9 ms | +4.31% |
| ⚡ | WallTime | walltime/1-streams/each-1000-bytes |
131.5 µs | 126.7 µs | +3.83% |
| ⚡ | WallTime | walltime/10-streams/each-1048576-bytes |
28.3 ms | 27.2 ms | +3.79% |
| ⚡ | WallTime | walltime/1-streams/each-4194304-bytes |
10.6 ms | 10.2 ms | +3.6% |
| ⚡ | WallTime | walltime/pacing-false/varying-seeds |
2.1 ms | 2 ms | +3.1% |
| ⚡ | WallTime | walltime/pacing-true/varying-seeds |
2.1 ms | 2 ms | +2.71% |
| ⚡ | WallTime | walltime/pacing-false/same-seed |
2 ms | 2 ms | +2.28% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing codspeed/setup-base-change-trigger (cacde7d) with main (e9c87f3)
|
@codspeedbot fix all workflows affected by this in this PR |
I applied the same base-change fix to every other Workflows changed (17): Each one got the same three changes as
Not changed:
Check before merging: a title or description edit now adds a new run to the head commit in which every job is skipped. Skipped jobs count as passing for required status checks. If branch protection uses the newest check of each name (I couldn't confirm this), editing a PR whose required check had failed could hide that failure. This affects single-job workflows such as I also updated the PR title and description to match the wider scope. Summary
Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot. |
Apply the fix from codspeed.yml to every other `pull_request` workflow. A stack reorder can push a head that conflicts with the old base, so GitHub skips the `synchronize` run, and the retarget that follows is an `edited` event. These commits then got no CI checks, benchmarks or QNS runs at all. Listen to `edited`, run jobs only when the base changed, and give other edits their own concurrency group so that a title or description edit can't cancel a run in progress. Jobs that run after skipped dependencies (`!cancelled()`, `always()`) get the same condition.
codspeed
Bot
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
CodSpeed’s validation can accept a stale merge commit containing the current head but the previous base.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Updates PR workflows to rerun CI after base-branch changes while ignoring unrelated edits.
Changes:
- Adds
editedtriggers and base-change job guards. - Isolates non-base edits from active concurrency groups.
- Adds CodSpeed merge-commit validation and dependency gating.
| File | Description |
|---|---|
.github/workflows/actionlint.yml |
Handles base edits for action linting. |
.github/workflows/bench.yml |
Handles base edits for benchmarks. |
.github/workflows/check.yml |
Handles base edits across CI jobs. |
.github/workflows/check-mtu.yml |
Handles base edits for MTU checks. |
.github/workflows/clippy.yml |
Handles base edits for Clippy. |
.github/workflows/codspeed.yml |
Adds edit handling and merge validation. |
.github/workflows/deny.yml |
Handles base edits for cargo-deny. |
.github/workflows/dependency-review.yml |
Handles base edits for dependency review. |
.github/workflows/firefox.yml |
Handles base edits for Gecko vendoring. |
.github/workflows/fuzz-bench.yml |
Handles base edits for fuzz/bench checks. |
.github/workflows/machete.yml |
Handles base edits for Machete. |
.github/workflows/mutants-pr.yml |
Handles base edits for mutation testing. |
.github/workflows/perfcompare.yml |
Handles base edits for performance comparisons. |
.github/workflows/qns-pr.yml |
Handles base edits for QNS runs. |
.github/workflows/readme.yml |
Handles base edits for README checks. |
.github/workflows/rustfmt.yml |
Handles base edits for formatting checks. |
.github/workflows/sanitize.yml |
Handles base edits for sanitizer runs. |
.github/workflows/semver.yml |
Handles base edits for semver checks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@codspeedbot check the review comments |
Copilot left one review comment, on the merge-commit check in The problem: a retarget doesn't change the PR head SHA. So the old check, "does the merge commit have the head as a parent?", still passed when the merge had been made against the old base. CodSpeed could then benchmark against the wrong base. The The new check in
The error message changed too. It used to say "Re-run this workflow", but a re-run reuses the same Tested: I ran the step's script against a shallow clone of this PR's real
One risk: if Summary
Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot. |
A retarget leaves the head SHA unchanged, so a stale merge commit made against the old base still contained the PR head. Require the merge commit to have the PR head as second parent and the PR base (or a descendant of it, if the base branch moved on) as first parent.
Client/server transfer resultsPerformance differences relative to e9c87f3. Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Table above only shows statistically significant changes. See all results below. All resultsTransfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Download data for |
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
|
Follow-up to the discussion in #3989.
Problem
When jj-stack reorders a stack, it force-pushes each PR's head and retargets the base about 8 s later. If the new head conflicts with the old base, GitHub skips the
synchronizerun becausepull_requestworkflows don't run on conflicting PRs. The retarget that follows is aneditedevent, and none of ourpull_requestworkflows listen for it. That commit then gets no CI checks, benchmarks, QNS or CodSpeed runs, and the PR stacked on top of it is compared against an older base. This happened to #3988 @49a9494and #3989 @00c69c0: GitHub shows 0 Actions runs for either SHA.Changes
All
pull_requestworkflows (actionlint,bench,check,check-mtu,clippy,codspeed,deny,dependency-review,firefox,fuzz-bench,machete,mutants-pr,perfcompare,qns-pr,readme,rustfmt,sanitize,semver):editedto thepull_requesttypes, next to the existing types (the defaultopened,synchronize,reopened, plusready_for_reviewinqns-pr.yml).github.event.action != 'edited' || github.event.changes.base. Editing a title or description skips every job. Jobs that would otherwise run after skipped dependencies (!cancelled()/always():bench-variantsandresultsinbench.yml,resultsinperfcompare.yml,reportinqns-pr.yml,crapincheck.yml,check-bindingsincheck-mtu.yml) also check it.cancel-in-progress: true, which applies even when all of the new run's jobs get skipped. Edits that don't change the base now get a unique group (suffixed withrun_id), so they cancel nothing. Base changes and pushes still share the per-PR group.skipped, sopr-comment.yml(which only acts onsuccess/failure) leaves existing bot comments alone.codspeed.ymlonly:benchmarksalso requiresneeds.bench-matrix.result == 'success'. Its!cancelled()condition would otherwise start it after a skipped or failedbench-matrix(with an empty matrix output).pull_request.head.sha, but the job benchmarksrefs/pull/N/merge.bench-matrixfails unlessGITHUB_SHAis a two-parent merge whose second parent is the head and whose first parent ispull_request.base.sha, or a descendant of it in case the base branch moved on (checked via the compare API only on a mismatch). A retarget leaves the head SHA unchanged, so checking only the head would accept a stale merge against the old base. Parents are read withgit cat-file, becauseHEAD^2isn't resolvable in the shallow checkout.perfcomparenow alsoneeds: bench-matrix.Another benefit: when a PR in the stack merges and its branch is deleted, GitHub retargets the next PR to
main. That retarget is also aneditedevent withchanges.base, so the next PR now gets a fresh CI run againstmain.Not changed
claude.ymlusespull_request_target, which runs on the base branch's context even when the PR conflicts.pr-comment.ymlruns onworkflow_run. Neither is affected.Validation
actionlintpasses.zizmor --offlinereports no findings, the same as before.main, and fails for a base that isn't an ancestor of the first parent or a wrong head.