From 1d9a98ee928e4fcdf183505787b7f4b8e28b1dc2 Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Thu, 17 Sep 2026 18:07:46 +0200 Subject: [PATCH 1/5] ci: skip the whole test job on Release PRs, not just its steps main required the six matrix legs of test-build by name, and a job-level `if:` is evaluated before the matrix expands, so skipping the job would have produced one check run instead of six and left those required contexts permanently absent. The guard therefore sat on the steps, and the job still spun up six runners to check out the repo and do nothing. Those six contexts have been removed from branch protection, so the job can now skip outright and this repo uses the same rule as the other five SDKs. lint.yml and reviewdog.yml stay unguarded: golangci-lint compiles the package, so they are the only check on release-please's rewrite of version.go. --- .github/workflows/run_tests.yml | 42 ++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index d3a31c4..22de774 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -17,26 +17,26 @@ concurrency: permissions: contents: read -env: - # True only for a Release PR opened by release-please and last pushed by it. Keying on the - # branch name alone would let any PR, a fork's included, name its branch release-please--x - # and skip every required check, which branch protection then counts as satisfied. The - # github.actor clause is the pusher rather than the PR author, so a human commit pushed - # onto the Release PR is tested like any other instead of riding the skip into main. - 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--') }} - -# The guard sits on the steps here rather than on the job, unlike the other five SDKs. main -# requires the six matrix legs of test-build by name, and a job-level `if:` is evaluated -# before the matrix expands, which is why the matrix context is not available there. A -# skipped job would produce one check run instead of six, so those required contexts would -# never appear and the Release PR would sit blocked forever. Guarding the steps keeps every -# leg reporting while the 20-minute integration leg is the part that goes away. +# This job skips on a Release PR. release-please only bumps the version and rewrites the +# changelog, and every source commit in one already passed this suite on the PR it came +# from, so the only thing running it again can do is fail on a flaky integration test and +# hold up a release. The guard sits on the job rather than on the calling job in ci.yml, +# because 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. It tests the +# author as well as the branch name: on its own, the name would let any PR, a fork's +# included, call its branch release-please--x and skip every required check, which branch +# protection counts as met. github.actor is the pusher rather than the PR author, so a +# human commit pushed onto the Release PR is tested like any other commit. +# +# lint.yml and reviewdog.yml stay unguarded on purpose: golangci-lint compiles the package, +# so they are the only check on release-please's rewrite of version.go. jobs: test-build: + if: >- + ${{ !(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--')) }} name: 👷 Test & Build ${{ matrix.goVer }}${{ matrix.label }} environment: feeds-enabled-shard runs-on: ubuntu-latest @@ -57,7 +57,7 @@ jobs: go-version: ${{ matrix.goVer }} - name: Unit tests via ${{ matrix.goVer }} - if: matrix.goVer != '1.19' && env.RELEASE_PR != 'true' + if: matrix.goVer != '1.19' env: STREAM_BASE_URL: ${{ vars.STREAM_BASE_URL }} STREAM_API_KEY: ${{ vars.STREAM_API_KEY }} @@ -65,7 +65,7 @@ jobs: run: go test -short -v -race ./... - name: Integration tests via ${{ matrix.goVer }} - if: matrix.goVer == '1.19' && env.RELEASE_PR != 'true' + if: matrix.goVer == '1.19' env: STREAM_BASE_URL: ${{ vars.STREAM_BASE_URL }} STREAM_API_KEY: ${{ vars.STREAM_API_KEY }} @@ -75,7 +75,7 @@ jobs: go tool cover -func=cover.out - name: Upload coverage to Codecov - if: matrix.goVer == '1.19' && env.RELEASE_PR != 'true' + if: matrix.goVer == '1.19' uses: codecov/codecov-action@v5 with: token: ${{ secrets.CODECOV_TOKEN }} From dea50ce051ee342b12cd8ddc106ced169aaafeca Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Thu, 17 Sep 2026 18:42:45 +0200 Subject: [PATCH 2/5] ci: fire the Release PR skip on the documented release path The github.actor clause made the skip miss the path CONTRIBUTING tells maintainers to take. With strict: true the Release PR has to be brought up to date whenever main moves, and Update branch attributes that merge commit to the person who clicked, so github.actor is a human and the guard fell through to running the full suite. Verified on #163: 8c31d76 is exactly that merge, and its run had actor=mogita and spent 3m51s on the 1.19 integration leg. Dropping the clause fires the skip on both Update branch and Approve and run. The remaining three clauses still cannot be forged: a PR author cannot be set to github-actions[bot], and the head repository must be this one. The cost is that a human commit pushed onto a Release PR is no longer tested here. release.yml calls this same workflow against the merge commit before it tags, which is what actually gates the release, and a `run-tests` label now forces the suite on any Release PR, since a re-run keeps the original actor and neither workflow has a dispatch trigger. Also corrects two comments. The claim that lint.yml and reviewdog.yml compile version.go was wrong on both counts: lint.yml only runs `go mod tidy`, and reviewdog.yml pipes golangci-lint into reviewdog under `bash -e` without pipefail, so the job takes reviewdog's exit status and its default -fail-level=none returns 0. And the note that a job-level `if:` is evaluated before the matrix expands is restored, because it is the only record of why re-adding the six required contexts would block every Release PR forever. --- .github/workflows/run_tests.yml | 36 +++++++++++++++++++-------------- CONTRIBUTING.md | 3 ++- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 22de774..abc3741 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -17,24 +17,30 @@ concurrency: permissions: contents: read -# This job skips on a Release PR. release-please only bumps the version and rewrites the -# changelog, and every source commit in one already passed this suite on the PR it came -# from, so the only thing running it again can do is fail on a flaky integration test and -# hold up a release. The guard sits on the job rather than on the calling job in ci.yml, -# because 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. It tests the -# author as well as the branch name: on its own, the name would let any PR, a fork's -# included, call its branch release-please--x and skip every required check, which branch -# protection counts as met. github.actor is the pusher rather than the PR author, so a -# human commit pushed onto the Release PR is tested like any other commit. -# -# lint.yml and reviewdog.yml stay unguarded on purpose: golangci-lint compiles the package, -# so they are the only check on release-please's rewrite of version.go. +# If main ever requires the matrix legs of test-build by name again, this guard must move +# back to the steps. A job-level `if:` is evaluated before the matrix expands, which is why +# the matrix context is not available there, so a skipped job produces ONE check run and not +# six, and those six required contexts would never appear: every Release PR would block +# forever. main currently requires only the PR-title check, which is what makes the +# job-level form safe. Keep the guard on the job rather than on the calling job in ci.yml +# either way, because 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. jobs: test-build: + # Skip a Release PR that release-please opened on a branch in this repository. The three + # clauses are all needed: the PR author cannot be forged, but on its own the branch name + # would let any PR, a fork's included, call itself release-please--x and skip everything. + # + # github.actor is deliberately not a clause. CONTRIBUTING tells maintainers to click + # Update branch, which attributes that merge commit to them, so keying on the pusher + # would mean the skip never fires on the documented release path. The cost is that a + # human commit pushed onto a Release PR is not tested here. release.yml runs this same + # workflow against the merge commit before it tags, and that is what gates the release. + # + # Label a Release PR `run-tests` to force the suite to run anyway. if: >- - ${{ !(github.actor == 'github-actions[bot]' - && github.event.pull_request.user.login == 'github-actions[bot]' + ${{ contains(github.event.pull_request.labels.*.name, 'run-tests') + || !(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--')) }} name: 👷 Test & Build ${{ matrix.goVer }}${{ matrix.label }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 413e04b..6047108 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,7 +52,8 @@ Releases are driven by [release-please](https://github.com/googleapis/release-pl - Merge PRs to `main` with conventional-commit titles. The PR title becomes the commit subject and is what determines the next version, so a non-conventional title ships nothing. - release-please keeps a Release PR open with the version bump and the generated changelog. Review it. - The Release PR is opened by `github-actions[bot]`, so its CI runs are held at "action required". If the PR is behind `main`, clicking **Update branch** also releases them, because that commit is attributed to you; otherwise click **Approve and run**. It needs a code-owner approval like any other PR. -- Merge the Release PR. That creates the tag and the GitHub Release, and `proxy.golang.org` picks the tag up. There is no separate publish step. +- The test suite does not run on a Release PR, whichever of those two paths you take. A Release PR only bumps the version and rewrites the changelog, and every commit in it already passed on the PR it came from, so all it can do is fail on the shared integration app and stall the release. `Lint`, `reviewdog`, CodeQL and the PR-title check still run. Label the PR `run-tests` if you want the suite anyway. +- Merge the Release PR. That creates the tag and the GitHub Release, and `proxy.golang.org` picks the tag up. There is no separate publish step. The suite does run before the tag: `release.yml` calls it on the merge commit, and a failure there leaves `autorelease: pending` set and needs the manual recovery below. Only `feat`, `fix`, `perf` and breaking changes produce a release (`revert` may also). A window of only `chore`, `ci`, `docs`, `test`, `refactor`, `style` or `build` commits produces no Release PR, which is intended. From 91cb292cf04434449929397ffa473ad1cdb5a36a Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Thu, 17 Sep 2026 20:05:36 +0200 Subject: [PATCH 3/5] ci: make the run-tests label actually start a run The label was wired into run_tests.yml but nothing triggered on it. ci.yml is `on: pull_request` with no `types:`, and the default activity types are opened, synchronize and reopened, so applying the label started no run and the escape hatch did nothing. That matters more than it sounds: with the github.actor clause gone, neither a push nor Update branch forces the suite either, so the label is the only way to run it on a Release PR. Adds `labeled` to the trigger, and an `if:` on the caller job so only `run-tests` counts. Without that guard every label would fire the six-leg suite against the shared Stream app, including release-please stamping `autorelease: pending`. Also drops a comment in ci.yml claiming main requires the matrix checks by name, which stopped being true when those six contexts were removed. --- .github/workflows/ci.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9faabe7..77c317e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,10 @@ name: build on: pull_request: + # `labeled` is here only so the `run-tests` escape hatch in run_tests.yml can fire at + # all. The default activity types are opened, synchronize and reopened, so without it + # applying the label starts no run and the hatch silently does nothing. + types: [opened, synchronize, reopened, labeled] branches: - main @@ -13,8 +17,11 @@ permissions: contents: read jobs: - # Keep this job id: main requires the matrix checks by name, and calling through a - # reusable workflow prefixes each with it. + # Keep this job id: it prefixes every check name that calling through a reusable workflow + # produces, so renaming it renames the checks. ci: + # Every other label has to be ignored, or release-please stamping `autorelease: pending` + # on each merged PR would fire the whole six-leg suite against the shared Stream app. + if: github.event.action != 'labeled' || github.event.label.name == 'run-tests' uses: ./.github/workflows/run_tests.yml secrets: inherit From 6203432beab2010d4321528bc18662d43b8d30ac Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Thu, 17 Sep 2026 21:07:13 +0200 Subject: [PATCH 4/5] ci: stop a label cancelling an in-flight suite, and restore the test gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two consequences of earlier changes in this PR. Adding `labeled` to the trigger gave every label the power to cancel a running suite. A labeled event lands in the same concurrency group as the synchronize it may be racing, and concurrency is resolved when the run is queued, before the ci job's `if:` is evaluated, so a skipped job never releases the slot. Adding `bug` to a PR mid-run cancelled it and replaced it with a run whose only job skips. cancel-in-progress now excludes labeled events; a push still cancels an older run. Removing the six matrix contexts from branch protection to unblock Release PRs also removed the test gate from every ordinary PR: main requires only the PR-title check, so a feature PR can merge with a red suite. tests-passed restores it with one non-matrix context that reports whether the job ran or skipped, so requiring it gates feature PRs without blocking Release PRs. Requiring `👷 Tests` on main is a settings change, not part of this commit. --- .github/workflows/ci.yml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77c317e..0ccfe8f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,11 @@ on: concurrency: group: ${{ github.workflow }}-${{ github.head_ref }} - cancel-in-progress: true + # A labeled event lands in the same group as the synchronize it may be racing, and + # concurrency is resolved when the run is queued, before the ci job's `if:` runs, so a + # skipped job does not release the slot. Without this, adding any label to a PR cancels + # an in-flight suite and replaces it with a run that tests nothing. A push still cancels. + cancel-in-progress: ${{ github.event.action != 'labeled' }} permissions: contents: read @@ -25,3 +29,22 @@ jobs: if: github.event.action != 'labeled' || github.event.label.name == 'run-tests' uses: ./.github/workflows/run_tests.yml secrets: inherit + + # The one context main requires for the suite. Requiring the matrix legs by name instead + # would block every Release PR, because a job-level `if:` is evaluated before the matrix + # expands and a skipped job yields one check run rather than six. This job is not a matrix, + # so it reports either way, and `skipped` counts as satisfied. + tests-passed: + name: 👷 Tests + needs: ci + if: always() + runs-on: ubuntu-latest + steps: + - name: Check the test job + env: + RESULT: ${{ needs.ci.result }} + run: | + case "$RESULT" in + success|skipped) echo "test job: $RESULT" ;; + *) echo "::error::test job reported $RESULT"; exit 1 ;; + esac From ddf9a25a46816533d5a88f7fa81b19da79884e38 Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Thu, 17 Sep 2026 21:31:54 +0200 Subject: [PATCH 5/5] ci: move the run-tests hatch out of build so a label cannot fake the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Triggering build on `labeled` let any label turn a red gate green. types: cannot filter by label name, so every label started a build run; ci skipped itself, tests-passed ran under always(), read needs.ci.result as skipped and exited 0, republishing 👷 Tests as green on a SHA whose tests had failed. Branch protection reads the latest check run for a context, so the red one no longer counted. Giving tests-passed the same label guard would not help: a job skipped by `if:` still posts a check run with conclusion skipped, which branch protection also treats as satisfied. The hatch moves to its own workflow. No label event reaches build any more, so nothing but a real build run can publish 👷 Tests, and the cancellation fix from the previous commit is no longer needed: types:, the ci guard and the cancel-in-progress expression all revert. Job id `forced` keeps the forced run's check names off `ci / …`, so they cannot stand in for build's own. --- .github/workflows/ci.yml | 20 ++++++-------------- .github/workflows/label_tests.yml | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/label_tests.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ccfe8f..7f08f82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,20 +2,12 @@ name: build on: pull_request: - # `labeled` is here only so the `run-tests` escape hatch in run_tests.yml can fire at - # all. The default activity types are opened, synchronize and reopened, so without it - # applying the label starts no run and the hatch silently does nothing. - types: [opened, synchronize, reopened, labeled] branches: - main concurrency: group: ${{ github.workflow }}-${{ github.head_ref }} - # A labeled event lands in the same group as the synchronize it may be racing, and - # concurrency is resolved when the run is queued, before the ci job's `if:` runs, so a - # skipped job does not release the slot. Without this, adding any label to a PR cancels - # an in-flight suite and replaces it with a run that tests nothing. A push still cancels. - cancel-in-progress: ${{ github.event.action != 'labeled' }} + cancel-in-progress: true permissions: contents: read @@ -24,16 +16,16 @@ jobs: # Keep this job id: it prefixes every check name that calling through a reusable workflow # produces, so renaming it renames the checks. ci: - # Every other label has to be ignored, or release-please stamping `autorelease: pending` - # on each merged PR would fire the whole six-leg suite against the shared Stream app. - if: github.event.action != 'labeled' || github.event.label.name == 'run-tests' uses: ./.github/workflows/run_tests.yml secrets: inherit - # The one context main requires for the suite. Requiring the matrix legs by name instead + # The context to require on main for the suite. Requiring the matrix legs by name instead # would block every Release PR, because a job-level `if:` is evaluated before the matrix # expands and a skipped job yields one check run rather than six. This job is not a matrix, - # so it reports either way, and `skipped` counts as satisfied. + # so it reports either way, and `skipped` counts as satisfied, which is what lets a Release + # PR through. Only a real run of this workflow can republish it, which is why the run-tests + # escape hatch lives in label_tests.yml: a label event reaching here would republish the + # context having tested nothing, turning a red gate green. tests-passed: name: 👷 Tests needs: ci diff --git a/.github/workflows/label_tests.yml b/.github/workflows/label_tests.yml new file mode 100644 index 0000000..560c2aa --- /dev/null +++ b/.github/workflows/label_tests.yml @@ -0,0 +1,26 @@ +name: label-tests + +# The escape hatch for a Release PR, which build skips. It lives in its own workflow rather +# than as a `labeled` trigger on build, because types: cannot filter by label name, so every +# label would start a build run, and that run republishes the 👷 Tests context having run +# nothing. A red gate would go green the moment anyone added a label. +on: + pull_request: + types: [labeled] + branches: + - main + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + # Job id `forced` keeps these check names off `ci / …`, so they never stand in for the + # build run's own checks. + forced: + if: github.event.label.name == 'run-tests' + uses: ./.github/workflows/run_tests.yml + secrets: inherit