ci: prioritize cheap, high-signal jobs ahead of expensive matrices - #63
Merged
jnasbyupgrade merged 4 commits intoAug 1, 2026
Conversation
Gate the five expensive jobs (pg-upgrade-test, pg-upgrade-stepwise, extension-update-test, pg-tle-test, pg-tle-upgrade-test) behind test+lint via `needs:`, so a broken fresh-install test or lint violation is caught before ~24 costly matrix legs (several rebuilding pg_tle from source, several doing binary pg_upgrades) get queued on the same broken baseline. GitHub Actions has no native job-priority mechanism -- the dependency graph is the only real lever for controlling which jobs claim runner-request slots first when concurrent-job caps are hit. Also add a workflow-level concurrency group with cancel-in-progress for non-master refs, so a superseded PR push no longer lets its full matrix run to completion for nothing.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Neither ci.yml's push/pull_request triggers nor claude-code-review.yml's pull_request_target trigger include the "closed" activity type, so a PR that merges or closes today lets any run already in flight for it -- including the paid Claude review -- finish on its own instead of being cancelled. Add a dedicated workflow triggered only on PR close whose two jobs do nothing except join the exact concurrency groups ci.yml and claude-code-review.yml already use; joining a group with cancel-in-progress: true cancels whatever's still running there. claude.yml (the @claude mention workflow) is deliberately left out -- its own comment explains it has no concurrency group because those requests are independent and shouldn't be dropped by a PR closing.
cancel-on-close.yml cancels in-flight runs by joining ci.yml's and claude-code-review.yml's concurrency groups using hand-copied strings, not references (there's no way to reference another workflow's group formula). If either file were renamed, its `name:` changed, or its concurrency.group formula edited, that copy would silently stop matching -- cancellation would just quietly become a no-op, with no error anywhere. Add a job that parses both files on every push and fails loudly if any of cancel-on-close.yml's assumptions (file exists, ci.yml's name is "CI", both concurrency.group formulas match, claude-review job still exists) no longer hold, telling whoever broke it to update cancel-on-close.yml's copies.
The "expected" strings were written as literal ${{ ... }} text meant to be
compared, unevaluated, against what PyYAML reads from disk. But GitHub
Actions pre-substitutes ANY ${{ ... }} it finds anywhere in a run: block's
raw text -- including inside this heredoc -- before the script ever runs.
So `expected = "${{ github.workflow }}-${{ github.ref }}"` never reached
Python as written: Actions evaluated it first into this run's own actual
values (e.g. "CI-refs/pull/63/merge"), while the value read from disk via
PyYAML is the genuine unevaluated literal text -- the two could never
match, on any run, regardless of whether the files actually drifted.
Confirmed failing exactly this way on cat_tools#63.
Fix: build each expected string by concatenating a bare "$" with the "{{
... }}" text, so the four-character expression-opener sequence never
appears contiguously anywhere in this file for Actions to pre-substitute.
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
1 participant
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.
Gate the five expensive jobs (
pg-upgrade-test,pg-upgrade-stepwise,extension-update-test,pg-tle-test,pg-tle-upgrade-test) behindtest+lintvianeeds:, so a broken fresh-install test or lint violation is caught before ~24 costly matrix legs (several rebuilding pg_tle from source, several doing binary pg_upgrades) get queued on the same broken baseline. GitHub Actions has no native job-priority mechanism -- the dependency graph (needs:) is the only real lever for controlling which jobs claim runner-request slots first once concurrent-job caps are hit, which is increasingly the case now that this workflow has enough matrix legs to queue.Also add a workflow-level
concurrency:group withcancel-in-progressfor non-master refs, so a superseded PR push no longer lets its full matrix run to completion for nothing -- there was noconcurrency:block at all before this.Each gated job's
if:now needs an explicitsuccess()alongside the existingdocs_onlycheck, since GitHub only assumessuccess()by default when a job has noif:at all -- once you write one, you own the whole condition.Add a small
cancel-on-close.ymlworkflow that triggers only on PR close (merged or not) and cancels any run still in flight forci.yml's andclaude-code-review.yml's concurrency groups -- previously a PR merging/closing let anything already running, including the paid Claude review, finish on its own.claude.yml(the@claudemention workflow) is intentionally excluded since it has no concurrency group by design.Since
cancel-on-close.ymlcancels by joining hand-copied concurrency-group strings (not references -- there's no way to reference another workflow's group formula), a rename or formula change in either target file would silently turn that cancellation into a no-op. Addverify-cancel-on-close-coupling, a job that parses both files on every push and fails loudly if any of those assumptions no longer hold. (Its first version had a real bug: it compared against a literal${{ ... }}string written inside arun:block, not realizing GitHub Actions pre-substitutes any expression marker it finds anywhere in that block's raw text before the script runs -- so the comparison was against this run's own live values, not the genuine unevaluated text on disk. Fixed by concatenating a bare$with{{ ... }}fragments so the marker never appears contiguously in the file.)No change to
claude-code-review.ymlitself: its cost gate already discovers all sibling check-runs on the PR head SHA dynamically and polls until they're all green (or times out to skip), so it's unaffected by how the check graph is internally sequenced.