ci: skip the test suite on release-please Release PRs - #165
Conversation
A Release PR bumps the version and rewrites the changelog. Every source commit in it already passed the suite on the PR it came from, so running it again only delays the release. The guard sits on each job rather than on the calling job in ci.yml: a job skipped by `if:` reports success and satisfies a required status check, while a reusable workflow that is never called produces no check at all and would leave a required one pending forever. The pre-tag gate is unaffected. release.yml triggers on push, where github.head_ref is empty, so the suite still runs before a tag lands.
| permissions: | ||
| contents: read | ||
|
|
||
| # Release PRs only bump the version and rewrite the changelog, and every commit in one |
There was a problem hiding this comment.
The premise does not hold for one file per repo. release-please rewrites version.go (versionName = "v7.1.0" // x-release-please-version), and those bytes have never been compiled or linted on any other PR, so "already passed on the PR it came from" is not true of them. The Release PR was the only PR-level check of that edit. Same shape in the other five: src/Constant.php, src/Client.cs + src/stream-feed-net.csproj, gradle.properties, lib/getstream_ruby/version.rb, pyproject.toml + uv.lock.
There was a problem hiding this comment.
Correct, and only partly fixed here.
For this repo it is now covered: the lint.yml and reviewdog.yml guards are reverted (see below), and golangci-lint compiles the package, so release-please's rewrite of version.go is still built and linted on the Release PR.
For the other five it is not, and I am taking that as a deliberate trade rather than claiming it away. A broken version-file substitution now first surfaces in release.yml's pre-tag gate, after the merge, as the stuck release you describe on getstream-ruby#89. Recovery is the documented manual path. The alternative is splitting a cheap lint leg out of the combined test job in five repos, which is more config and more drift than the failure is worth at its observed rate of zero.
| contents: read | ||
|
|
||
| # Release PRs only bump the version and rewrite the changelog, and every commit in one | ||
| # already passed this suite on its own PR. The guard sits on each job, not on the caller, |
There was a problem hiding this comment.
This does not unblock the Release PRs that are stuck right now. On #163 (chore(main): release 7.1.1, cfe9163bb352) mergeable_state is blocked and all four workflow runs are status: completed, conclusion: action_required with zero jobs (Lint 35220712754, reviewdog 35220712828, build 35220712957, Lint PR title 35220712977). The combined status on that SHA is state: pending, total_count: 0, with only CodeQL check runs present.
A job-level if: is evaluated only after someone clicks Approve and run, so the manual approval and the missing contexts survive this change untouched. getstream-php #68 is in the same state.
There was a problem hiding this comment.
Agreed, this PR does not touch that. The action_required hold comes from the workflow-approval policy and is evaluated before any job if:, so a stuck Release PR stays stuck until someone clicks Approve and run.
Separate problem, separate fix. Not folding it in here.
| # already passed this suite on its own PR. The guard sits on each job, not on the caller, | ||
| # so a skipped job still reports success to a required status check. | ||
| jobs: | ||
| test-build: |
There was a problem hiding this comment.
main requires six contexts by name, and they are the matrix legs of this job: ci / 👷 Test & Build 1.19 & Integration, ... 1.20 through ... 1.24 (plus 👮 Conventional PR title), with strict: true.
The change rests on a job-level if: still producing one skipped check run per matrix leg. If it does not, those six contexts are never created and the Release PR sits on six permanently-pending required checks, which is worse than the ~20 minutes saved. Worth confirming on one real Release PR before merging this one. The other five repos do not have this exposure (java requires one non-matrix context, php/net/ruby/py require none).
There was a problem hiding this comment.
Confirmed, and it does not work. matrix is not available in jobs.<job_id>.if:
context "matrix" is not allowed here. available contexts are "github", "inputs", "needs", "vars"
The job if: is therefore evaluated before the matrix expands, so a skipped job produces one check run, not six. Those six required contexts would never be created and the Release PR would sit permanently blocked, which is worse than the time saved.
Moved the guard to the steps instead. All six legs still run checkout and setup-go and report, and the integration step plus the Codecov upload are what skip. This repo is now the only one of the six guarded at step level, and the comment above jobs: says why.
| # so a skipped job still reports success to a required status check. | ||
| jobs: | ||
| test-build: | ||
| if: ${{ !startsWith(github.head_ref, 'release-please--') }} |
There was a problem hiding this comment.
github.head_ref is chosen by whoever opens the PR, including from a fork. A PR from a branch named release-please--branches--main skips test-build, lint and reviewdog; the six required contexts then report skipped, which branch protection counts as satisfied, and the PR is mergeable with zero test signal. startsWith in GitHub expressions is case-insensitive, so Release-Please--x works too.
Suggest a second clause tying it to the bot: github.event.pull_request.user.login == 'github-actions[bot]', or at minimum github.event.pull_request.head.repo.full_name == github.repository.
There was a problem hiding this comment.
Fixed. The guard now requires release-please provenance, not just the name:
RELEASE_PR: >-
${{ github.actor == 'github-actions[bot]'
&& github.event.pull_request.user.login == 'github-actions[bot]'
&& github.event.pull_request.head.repo.full_name == github.repository
&& startsWith(github.head_ref, 'release-please--') }}Verified against the live Release PRs: getstream-go#163, getstream-php#68, getstream-net#81 and stream-py#289 are all user.login == github-actions[bot] with head.repo.full_name equal to the repository.
The github.actor clause also closes getstream-net#82: it is the pusher rather than the PR author, so a human commit pushed onto the Release PR is tested like any other. Every clause fails open, so a missing one runs the suite.
| permissions: | ||
| contents: read | ||
|
|
||
| # Release PRs only bump the version and rewrite the changelog, so there is nothing here |
There was a problem hiding this comment.
release.yml's pre-tag gate calls only run_tests.yml, and this workflow is pull_request-only, so the Release PR is the sole place golangci-lint and go mod tidy ever see the release tree, which is also the only tree containing release-please's rewrite of version.go. After this change nothing lints it, on any event.
The comment is also inaccurate here: the required contexts on main are the six ci / 👷 Test & Build ... legs plus 👮 Conventional PR title. 👮 Lint and 🐶 Reviewdog are not required, so there is no check being kept satisfied. Same applies to reviewdog.yml.
There was a problem hiding this comment.
Both reverted. The comment was wrong, 👮 Lint and 🐶 Reviewdog are not required contexts, so there was nothing being kept satisfied, and they are the only workflows that would still have compiled and linted release-please's rewrite of version.go.
They cost about a minute against a 20-minute integration leg, so removing them was most of the risk for almost none of the saving.
The branch-name guard was spoofable: any PR, a fork's included, could name its head branch release-please--x and skip every required check, which branch protection then counts as satisfied. The guard now also requires the PR to be opened by github-actions[bot] from a branch in this repository. A job-level `if:` could not be used here at all. main requires the six matrix legs of test-build by name, and a job `if:` is evaluated before the matrix expands, so a skipped job produces one check run and not six: those required contexts would never appear and the Release PR would sit blocked. The guard moved to the steps, so every leg still reports while the integration leg is the part that goes away. The lint and reviewdog guards are reverted. Neither is a required context, so there was nothing to keep satisfied, and they are the only workflows that would still have linted release-please's rewrite of version.go.
GitHub Actions
Error
GitHub Actions
Error
The guard keyed on who opened the PR, so a commit pushed by hand onto the release-please branch, to fix a conflict or a changelog entry, inherited the skip and reached the default branch having run nothing. github.actor is the pusher rather than the PR author, so that commit is now tested like any other and only release-please's own pushes skip.
This branch was successfully deployed
1 participant
Ticket
CHA-5511
Problem
release-please opens a Release PR that bumps the version and rewrites the changelog. Every source commit in it already passed this suite on the PR it came from, so the 20-minute integration leg on the Release PR only delays the release.
Solution
This repo is guarded at step level, unlike the five sibling SDK PRs, which guard at job level.
mainrequires the six matrix legs oftest-buildby name, andmatrixis not available injobs.<job_id>.if:so the job
if:is evaluated before the matrix expands. A skipped job produces one check run, not six, and those six required contexts would never be created, leaving the Release PR permanently blocked. Guarding the steps keeps every leg reporting.All four clauses must hold, and each fails open, so anything short of a Release PR that release-please both opened and last pushed runs the suite. The branch name alone would not do: any PR, a fork's included, could call its branch
release-please--xand skip all six required checks, which branch protection counts as satisfied.github.actoris the pusher rather than the PR author, so a human commit pushed onto the Release PR is tested like any other.What skips on a Release PR: the unit step, the integration step and the Codecov upload. What still runs: checkout and setup-go on all six legs, plus
lint.ymlandreviewdog.yml.lint.ymlandreviewdog.ymlare deliberately not guarded. Neither is a required context, so there is nothing to keep satisfied, and golangci-lint compiles the package, making them the only check on release-please's rewrite ofversion.go. They cost about a minute against the 20-minute leg being removed.The pre-tag gate is unaffected:
release.ymltriggers onpush, where there is no pull request, so the suite still runs before a tag lands.How to verify
ci / 👷 Test & Build ...contexts should still appear and pass in about a minute each, with the test steps reported as skipped, and👮 Lintand🐶 Reviewdogshould run normally.Out of scope
Release PRs here are currently held at
action_requiredby the workflow-approval policy, which is evaluated before any job or stepif:. This PR does not change that, and a stuck Release PR stays stuck until someone clicks Approve and run.