ci: skip the whole test job on Release PRs, not just its steps

[mogita]

Ticket

CHA-5511

Problem

Two things, the second found in review.

1. #165 had to guard this repo at step level, unlike the other five SDKs. main required the six matrix legs of test-build by name, and a job-level if: is evaluated before the matrix expands:

context "matrix" is not allowed here. available contexts are "github", "inputs", "needs", "vars"

so skipping the job would have produced one check run instead of six, leaving those required contexts permanently absent and the Release PR blocked. The job therefore still spun up six runners on every Release PR to check out the repo and do nothing. Those six contexts have since been removed from branch protection; main now requires only 👮 Conventional PR title.

2. The guard included github.actor == 'github-actions[bot]', which made it miss the documented release path. CONTRIBUTING.md:54:

If the PR is behind main, clicking Update branch also releases them, because that commit is attributed to you

With strict: true, a Release PR behind main must be updated before it can merge, and that merge commit is attributed to whoever clicked, so github.actor is a human and the suite ran in full. Confirmed on #163: 8c31d76 is exactly that merge commit, its run had actor=mogita, and the 1.19 integration leg took 3m51s.

Solution

Move the guard from the three steps to the job, and drop the github.actor clause:

if: >-
  ${{ 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--')) }}

The three remaining clauses cannot be forged: a PR author cannot be set to github-actions[bot], and the head repository must be this one, so the branch name is never trusted alone. The skip now fires on both Update branch and Approve and run.

A run-tests label forces the suite. This is not a convenience: with github.actor gone, neither a push nor Update branch changes the PR author or the branch name, and a re-run keeps the original actor, so the label is the only way to run the suite on a Release PR at all.

The hatch lives in its own workflow, label_tests.yml, rather than as a labeled trigger on build. types: cannot filter by label name, so every label would start a build run in which ci skips itself, tests-passed reads skipped and exits 0, and 👷 Tests is republished green on a SHA whose tests had failed. Keeping label events out of build is what prevents that.

Restoring the test gate

Stripping the six matrix contexts to unblock Release PRs also left main requiring only 👮 Conventional PR title, so any feature PR could merge with a red suite. tests-passed is one non-matrix context that fails unless ci reported success or skipped, so requiring it gates feature PRs while still letting a Release PR through.

Settings change needed after this merges: add 👷 Tests to required_status_checks on main. The context does not exist until the first run lands.

Corrections carried in this PR

• The claim that lint.yml and reviewdog.yml compile version.go was wrong, and is removed. lint.yml runs only go mod tidy. reviewdog.yml pipes golangci-lint into reviewdog under bash -e with no pipefail, so the job takes reviewdog's status and its default -fail-level=none exits 0. The real compile checks are CodeQL's Analyze (go) and release.yml's pre-tag tests job. Both workflows still run on Release PRs, because neither touches the shared app and both finish inside CodeQL's window.
• The note explaining that a job-level if: precedes matrix expansion is restored. It is the only record of why re-adding the six required contexts would block every Release PR forever.

Known trade

The flaky leg is relocated, not removed. release.yml's tests job calls this same workflow on the merge commit, so a flake now fires after the Release PR has merged, leaving autorelease: pending set and needing a manual re-run. Documented in CONTRIBUTING.md. The better fix is porting scheduled_test.yml's for _ in 1 2 3; do go test ... && break retry into run_tests.yml, which helps every PR rather than exempting one shape; that is a separate change.

How to verify

1. This PR is not a Release PR, so all six legs should run as usual.
2. On the next Release PR, build should report Skipped on either release path, while Lint, reviewdog and CodeQL run normally.
3. Adding the run-tests label to a Release PR should bring the six legs back.

[mogita]

Two findings that sit outside the diff.

Branch protection now gates nothing. required_status_checks.contexts on main is exactly ["👮 Conventional PR title"]. 👮 Lint, 🐶 Reviewdog, CodeQL and all six ci / 👷 Test & Build legs are advisory, so any ordinary feature PR can merge with a red suite once it has one code-owner approval. Unblocking Release PRs by stripping the matrix contexts removed the test gate for every PR, not just Release PRs.

An aggregating job keeps both properties: add one job to ci.yml with needs: [ci], if: always(), failing unless needs.ci.result is success or skipped; require only that context and leave the matrix legs non-required. A skipped job satisfies a required check, so the Release-PR skip still works.

CONTRIBUTING.md is not updated. Line 54 presents Update branch and Approve and run as interchangeable ways to release the held checks. After this PR they differ by roughly 20 minutes and by whether the suite runs at all: Approve and run gives one skipped build check, Update branch gives a full six-leg run including the shared-app integration leg. Line 87's major-release recipe ("Click Update branch on the major Release PR, then merge it") always takes the expensive path.

[mogita]

Handled the first, and the second is now stale. Sorry for the slow reply, I worked the inline threads and did not see this one until now.

Branch protection. Valid, and it is the cost I named when recommending the change without proposing a replacement. Added your aggregating job:

  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

Not a matrix, so it reports on a Release PR too, where needs.ci.result is skipped and it passes. Requiring 👷 Tests on main gates feature PRs again without blocking Release PRs. That is a settings change, not part of the diff: it has to be added after this merges, or the context will not exist yet.

CONTRIBUTING. This was written against the diff before dea50ce. Dropping the github.actor clause removed the asymmetry you describe: the guard now reads the PR author, the head repository and the branch name, and Update branch changes none of them, so both paths give one skipped build check. Line 87's major-release recipe no longer takes the expensive path either.

CONTRIBUTING.md is updated in this PR regardless, with a bullet stating that the suite does not run on a Release PR whichever path you take, that the run-tests label forces it, and that release.yml runs the suite on the merge commit before tagging with manual recovery if it fails.