Repository navigation
NO-ISSUE: run PR CI suites on merge_group - #418
Merged
Merged
Conversation
Merge queue waits for checks on the speculative SHA. Without a merge_group trigger those jobs never report, so GitHub kicks the PR (checks_timed_out).
|
@minmzzhang: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe workflows now trigger on ChangesMerge queue CI support
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR updates CI workflows to run required suites for merge-queue commits; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
🤖 Finished Review · ✅ Success · Started 1:47 PM UTC · Completed 1:59 PM UTC Commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ReviewFindingsHigh
Labels: PR modifies GitHub Actions workflow files under .github/workflows/ Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
github_actions
omer-vishlitzky
dismissed
fullsend-ai-review[bot]’s stale review
August 20, 2026 14:00
Auto-dismissed: only Prow labels gate merging
Contributor
Author
|
/retest |
|
Re-triggered failed runs:
|
eliorerz
approved these changes
Aug 26, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eliorerz, minmzzhang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5 tasks
Merged
via the queue into
osac-project:main
with commit Aug 26, 2026
d913486
124 of 134 checks passed
4 of 5 tasks
3 of 4 tasks
eliorerz
added a commit
to eliorerz/osac
that referenced
this pull request
Sep 2, 2026
…ests/pre-commit PR osac-project#418 correctly runs the full Unit Tests and Integration Tests suites on merge_group -- that behavior is unchanged here and stays exactly as osac-project#418 intended. The actual problem is narrower: none of unit-tests.yml, integration-tests.yml, or pre-commit.yaml cancel a stale run when GitHub's merge queue rebases a PR onto a new ephemeral gh-readonly-queue ref, which happens routinely as normal merge-queue behavior. Every rebase spawns a brand-new full run of these 3 workflows for that PR; the previous run for the now-superseded ref keeps consuming a runner until it finishes naturally. Confirmed live: 13 concurrent Unit Tests runs and 10 concurrent Integration Tests runs against only 4 active merge-queue slots (max_entries_to_build: 4, via the rulesets API) -- consistent with a handful of PRs each stacking up multiple stale, uncancelled reruns from repeated rebases. Also confirmed this repo's actual hosted-runner concurrency (~30-40 in use) is nowhere near the plan's 180-job ceiling, so a hard concurrency limit is not the bottleneck -- the queue stalls because cheap, load-bearing jobs (label-gate, auto-queue, Slash Command) get starved behind piles of stale heavy runs, not because of a runner cap. integration-tests.yml already had a concurrency block, but it keyed non-PR events on github.sha (the merge-preview commit), which changes on every rebase -- so the group itself changed every rebase and could never collapse a prior run even with cancel-in-progress true. unit-tests.yml and pre-commit.yaml had no concurrency block at all. Fix: key the concurrency group on something that stays stable across rebases of the same PR. github.ref_name for a merge_group event is GitHub's ephemeral gh-readonly-queue/<base>/pr-<number>-<sha> ref -- the pr-<number> segment is constant across rebases; only the trailing sha changes. Verified this directly against this repo's own run history (gh api repos/osac-project/osac/actions/runs?event=merge_group): PR osac-project#503 was requeued at gh-readonly-queue/main/pr-503-b2986acb...61 and .../pr-503-f05965f9...53 sixteen minutes apart; PR osac-project#307 similarly at .../pr-307-18b1c7ed...58 and .../pr-307-0c86346b...c1 -- both pairs extract to the identical "pr-503"/"pr-307" via `grep -oE 'pr-[0-9]+'` despite the trailing sha differing every time. Workflow-level `concurrency:` blocks are evaluated before any job runs and can't reference a computed value, so this can't be done as a single top-of-file block. Instead, each file computes the key once (in the existing `changes` job for unit-tests.yml/integration-tests.yml; in a new tiny `concurrency-key` job for pre-commit.yaml, which has no `changes` job) and each actual test-execution job gets its own job-level `concurrency:` block referencing that computed output -- job-level blocks can reference `needs.*.outputs.*`. Nothing from the previously-closed osac-project#544 approach is reintroduced: no compile-check job, no gating the heavy test jobs off of merge_group. Test execution behavior (what runs, on what trigger, with what coverage) is completely unchanged on all 3 files -- this only cancels superseded/stale runs of the same PR's own prior queue entry.
eliorerz
added a commit
to eliorerz/osac
that referenced
this pull request
Sep 12, 2026
…ests/pre-commit PR osac-project#418 correctly runs the full Unit Tests and Integration Tests suites on merge_group -- that behavior is unchanged here and stays exactly as osac-project#418 intended. The actual problem is narrower: none of unit-tests.yml, integration-tests.yml, or pre-commit.yaml cancel a stale run when GitHub's merge queue rebases a PR onto a new ephemeral gh-readonly-queue ref, which happens routinely as normal merge-queue behavior. Every rebase spawns a brand-new full run of these 3 workflows for that PR; the previous run for the now-superseded ref keeps consuming a runner until it finishes naturally. Confirmed live: 13 concurrent Unit Tests runs and 10 concurrent Integration Tests runs against only 4 active merge-queue slots (max_entries_to_build: 4, via the rulesets API) -- consistent with a handful of PRs each stacking up multiple stale, uncancelled reruns from repeated rebases. Also confirmed this repo's actual hosted-runner concurrency (~30-40 in use) is nowhere near the plan's 180-job ceiling, so a hard concurrency limit is not the bottleneck -- the queue stalls because cheap, load-bearing jobs (label-gate, auto-queue, Slash Command) get starved behind piles of stale heavy runs, not because of a runner cap. integration-tests.yml already had a concurrency block, but it keyed non-PR events on github.sha (the merge-preview commit), which changes on every rebase -- so the group itself changed every rebase and could never collapse a prior run even with cancel-in-progress true. unit-tests.yml and pre-commit.yaml had no concurrency block at all. Fix: key the concurrency group on something that stays stable across rebases of the same PR. github.ref_name for a merge_group event is GitHub's ephemeral gh-readonly-queue/<base>/pr-<number>-<sha> ref -- the pr-<number> segment is constant across rebases; only the trailing sha changes. Verified this directly against this repo's own run history (gh api repos/osac-project/osac/actions/runs?event=merge_group): PR osac-project#503 was requeued at gh-readonly-queue/main/pr-503-b2986acb...61 and .../pr-503-f05965f9...53 sixteen minutes apart; PR osac-project#307 similarly at .../pr-307-18b1c7ed...58 and .../pr-307-0c86346b...c1 -- both pairs extract to the identical "pr-503"/"pr-307" via `grep -oE 'pr-[0-9]+'` despite the trailing sha differing every time. Workflow-level `concurrency:` blocks are evaluated before any job runs and can't reference a computed value, so this can't be done as a single top-of-file block. Instead, each file computes the key once (in the existing `changes` job for unit-tests.yml/integration-tests.yml; in a new tiny `concurrency-key` job for pre-commit.yaml, which has no `changes` job) and each actual test-execution job gets its own job-level `concurrency:` block referencing that computed output -- job-level blocks can reference `needs.*.outputs.*`. Nothing from the previously-closed osac-project#544 approach is reintroduced: no compile-check job, no gating the heavy test jobs off of merge_group. Test execution behavior (what runs, on what trigger, with what coverage) is completely unchanged on all 3 files -- this only cancels superseded/stale runs of the same PR's own prior queue entry.
omer-vishlitzky
pushed a commit
to omer-vishlitzky/osac
that referenced
this pull request
Sep 13, 2026
…ests/pre-commit (osac-project#550) osac-project#544 scoped full test execution on `merge_group` down to a lightweight compile-check and was correctly rejected -- the owner wants the FULL Unit Tests and Integration Tests suites to keep running on `merge_group` exactly as osac-project#418 intended, and live data showed the 180-concurrent-job Enterprise runner ceiling wasn't even close to being hit (~30-40 in use at the time). **Nothing from osac-project#544 is reintroduced here: no compile-check job, no gating the heavy test jobs off of `merge_group`, no change to what runs or when.** This PR only cancels superseded/stale runs. ## The actual problem None of `unit-tests.yml`, `integration-tests.yml`, or `pre-commit.yaml` cancel a stale run when GitHub's merge queue rebases a PR onto a new ephemeral `gh-readonly-queue` ref -- normal, routine merge-queue behavior that will keep happening regardless of the ref-staleness git-clone incident fixed separately. Every rebase spawns a brand-new full run of these 3 workflows for that PR; the previous run for the now-superseded ref is not cancelled and keeps consuming a runner until it finishes naturally. Confirmed live: **13 concurrent Unit Tests runs and 10 concurrent Integration Tests runs** against only **4 active merge-queue slots** (`max_entries_to_build: 4`, confirmed via `gh api repos/osac-project/osac/rulesets`) -- consistent with a handful of PRs each stacking up multiple stale, uncancelled reruns from repeated rebases. Also confirmed this repo's real hosted-runner concurrency (~30-40 in use) is nowhere near the plan's 180-job ceiling, so a hard concurrency limit is not the bottleneck here -- the queue stalls because cheap, load-bearing jobs (`label-gate`, `auto-queue`, Slash Command) get starved behind piles of stale heavy runs, not because of a runner cap. `integration-tests.yml` already had a `concurrency:` block, but its non-PR fallback key was `github.sha` -- the merge-preview commit, which changes on every rebase. So the group key itself changed every rebase and could never collapse a prior run even with `cancel-in-progress` true for `merge_group`. `unit-tests.yml` and `pre-commit.yaml` had no `concurrency:` block at all. ## The fix, and why the key is actually stable `github.ref_name` for a `merge_group` event is GitHub's ephemeral `gh-readonly-queue/<base>/pr-<number>-<sha>` ref. The `pr-<number>` segment is constant across rebases of the same PR; only the trailing sha changes. **Verified directly against this repo's own run history** (`gh api repos/osac-project/osac/actions/runs?event=merge_group`), not assumed: - PR osac-project#503 requeued at `gh-readonly-queue/main/pr-503-b2986acb...61` and `.../pr-503-f05965f9...53`, 16 minutes apart. - PR osac-project#307 requeued at `gh-readonly-queue/main/pr-307-18b1c7ed...58` and `.../pr-307-0c86346b...c1`. - PR osac-project#502 requeued at `gh-readonly-queue/main/pr-502-25cf0673...26` and `.../pr-502-ade9ac85...04`. All three pairs extract to the identical `pr-503`/`pr-307`/`pr-502` via `grep -oE 'pr-[0-9]+'`, despite the trailing sha differing every time -- this is the key that actually collapses repeated rebases. Workflow-level `concurrency:` blocks are evaluated before any job runs and can't reference a computed value, so this can't be a single top-of-file block (GitHub Actions expressions have no substring-extraction function to pull `pr-<number>` out of the ref inline). Instead: - **`unit-tests.yml`, `integration-tests.yml`**: the existing `changes` job gets one new step that computes the stable key (`pr-<number>` for `pull_request`, the extracted `pr-<number>` for `merge_group`, `run-<id>` as a no-op fallback for `schedule`/`workflow_dispatch`) and exposes it as a new `concurrency-key` output. Every test-execution job already `needs: changes`, so each gets its own job-level `concurrency:` block referencing `needs.changes.outputs.concurrency-key` -- job-level blocks, unlike workflow-level ones, can reference `needs.*.outputs.*`. - **`pre-commit.yaml`**: has no `changes` job, so it gets a new, tiny `concurrency-key` job computing the same thing, and `pre-commit` now `needs:` it and carries the same job-level `concurrency:` block. - `integration-tests.yml`'s old workflow-level block is removed entirely, superseded by the per-job blocks above. `cancel-in-progress` is `true` for both `pull_request` and `merge_group` on every one of these blocks -- `pull_request` behavior is unchanged in substance (still cancels on new pushes), `merge_group` now actually works. ## Explicitly not changed - No job is skipped or gated off `merge_group`. Every test-execution job that ran before still runs, on the same triggers, with the same coverage. - No new compile-check job. - `pre-commit.yaml`'s actual gitleaks/lint logic is untouched -- only the new upstream `concurrency-key` job and the `needs:`/`concurrency:` addition on `pre-commit` itself. ## Verification - [x] `actionlint` on all 3 changed files -- clean (also ran actionlint against the whole `.github/workflows/` tree; the only findings are pre-existing, in unrelated files, and not introduced by this PR) - [x] All 3 files validated as parseable YAML - [x] Concurrency-key extraction regex verified against 6 real `head_branch` values pulled from this repo's own run history (3 rebase pairs, listed above) - [ ] A real merge-queue rebase exercises the cancellation end-to-end <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary - **CI:** Added stable per-PR concurrency keys to the unit-test, integration-test, and pre-commit workflows. - **CI:** Cancel superseded pull-request and merge-group runs with `cancel-in-progress: true`. - **CI:** Added `pr-<number>` extraction, `run-<run-id>` fallbacks, and restricted permissions for key computation. - **CI:** Preserved existing triggers, test coverage, and behavior for scheduled, manual, and other events. - **API, controllers, database, auth, deployment, and documentation:** No changes. ### Backward compatibility No application or public API behavior changes. CI behavior changes only for overlapping pull-request and merge-group runs. End-to-end cancellation during an actual merge-queue rebase remains unverified. ## Risk classification **risk:show** — The change affects CI execution and can cancel in-progress workflows, but it does not change application behavior, deployment behavior, public APIs, or test coverage. It was close to **risk:ship** because the scope is limited to workflow configuration, but unverified merge-queue cancellation behavior requires visible review. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pre-commit, unit tests, and integration tests onmerge_groupso merge queue gets real pass/fail on the speculative SHA (fixes thechecks_timed_outkick from github-config#191 / osac#375).dorny/paths-filterhas no PR Files API there).merge_group.base_sha/head_sha; keep the rotation comment PR-only (no issue number on merge_group).Test plan
mainSummary by CodeRabbit