From 7adfbcfb2217105cb3b76219ff2bc0ad27fcfe48 Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Tue, 10 Nov 2026 17:47:08 +0100 Subject: [PATCH 1/2] fix(gate): cancel PR runs only for new commits A newer run on the same commit (for editing the description, marking the pull request ready, reopening it) cancelled the run in progress, whose gate (`if: always()`) then failed on the jobs the cancellation stopped. As the newest check run of a name on a commit counts, that failure blocked the merge (cloud-registry-ui#242). Let only a new head commit (action `synchronize`) cancel the runs of a pull request in progress; runs for other actions wait instead, and a waiting run that is replaced starts no job. A cancelled run then only ever tests an old commit, whose failed gate blocks nothing. The gate keeps `always()` and fails closed: `!cancelled()` would skip it in cancelled runs, and GitHub counts a skipped required check as passed. Apply this to this repository's CI and the example in the gate's README, document why, and test that both use the settings. Closes #70 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011FSoqMpmDCfbACLj3uy6xV --- .github/workflows/ci.yml | 7 +++-- actions/gate/README.md | 66 +++++++++++++++++++++++++++++++++++++--- tests/gate/test.sh | 63 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0091a9c..0a628f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,11 +19,13 @@ on: permissions: contents: read -# Cancel superseded runs of a pull request; runs for pushes get their own +# Cancel superseded runs of a pull request, but only for a new head commit, +# so that a newer run on the same commit doesn't leave a failed gate (see +# "Cancelled runs" in actions/gate/README.md); runs for pushes get their own # group, so that queued runs for successive pushes are not dropped concurrency: group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.sha || github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + cancel-in-progress: ${{ github.event_name == 'pull_request' && github.event.action == 'synchronize' }} jobs: lint: @@ -224,6 +226,7 @@ jobs: name: CI passed runs-on: ubuntu-latest timeout-minutes: 5 + # Not `!cancelled()`: see "Cancelled runs" in actions/gate/README.md if: always() needs: [lint, test] permissions: {} diff --git a/actions/gate/README.md b/actions/gate/README.md index a2b9f3d..a8ee81c 100644 --- a/actions/gate/README.md +++ b/actions/gate/README.md @@ -37,6 +37,21 @@ passed: allow-skipped: ${{ github.event_name == 'pull_request' && 'push' || '' }} ``` +Let only a new head commit of a pull request cancel its runs in progress +(see [Cancelled runs](#cancelled-runs)), as in this repository's +`ci.yml`: + +```yaml +concurrency: + # Runs for pushes get their own group, so that none is dropped + group: >- + ${{ github.workflow }}-${{ github.event_name == 'push' + && github.sha || github.ref }} + cancel-in-progress: >- + ${{ github.event_name == 'pull_request' + && github.event.action == 'synchronize' }} +``` + ## Inputs - `needs` (required): the results of the needed jobs, as @@ -45,9 +60,52 @@ passed: separated by whitespace; by default none. Compute it in the expression, e.g. from the event, so that a skip is allowed only where it is intended. +## Cancelled runs + +The gate job runs `always()`, so that it fails whenever a needed job failed +or was cancelled, whether by its `timeout-minutes`, by `fail-fast` in a +matrix, by hand or with the whole run. A cancelled run thus leaves a failed +`CI passed` on its commit, and as the newest result of a check on a commit +counts, that failure blocks the merge until a newer run on the same commit +passes. + +That is harmless when the cancelled run was testing an old commit, but it +blocks a pull request when a newer run on the **same** commit cancels it, +e.g. a run for editing the description, marking the pull request ready or +reopening it (event `pull_request`, actions `edited`, `ready_for_review`, +`reopened`): the cancelled run's failed gate is then the newest result on +the commit until the newer run's gate reports, and stays so if it never +does. Therefore only a new head commit (action +`synchronize`) may cancel the runs of a pull request in progress +(`cancel-in-progress` above); a run for another action waits until the run +in progress has finished, and replaces a waiting one. A waiting run that is +replaced never starts any job, so it leaves no `CI passed` at all. + +The gate job doesn't use `if: ${{ !cancelled() }}`, which would skip it in +cancelled runs: GitHub counts a skipped required check as passed +([Troubleshooting required status +checks](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks)), +so a run cancelled by hand, or replaced by a newer run that hasn't reported +yet, would allow the merge without tests. + +A run cancelled by hand, or by rerunning a run of an older commit (a rerun +keeps its event, so it cancels the run in progress on the head commit), +fails the gate: rerun the head commit's run, or push, before you merge. + +The price: after an event other than a push, e.g. marking a pull request +ready, its result comes only after the run in progress and then a whole new +run. Trigger CI only on the `pull_request` actions that it needs (`types`). + ## Tests -`tests/gate/test.sh` runs the script on sets of job results (all success, -a failure, a cancel, allowed and unexpected skips, empty `needs`); the -`test` job of this repository's CI also runs it, and the action itself on -passing and failing results. +`tests/gate/test.sh`: + +- runs the script on sets of job results (all success, a failure, a + cancel, allowed and unexpected skips, empty `needs`); +- checks that the gate jobs of this repository's CI and of the example + above run `always()`, and that only a new head commit of a pull request + cancels runs in progress. + +The `test` job of this repository's CI runs it, and the action itself on +passing and failing results. How GitHub applies the conditions and the +concurrency group shows only in real runs. diff --git a/tests/gate/test.sh b/tests/gate/test.sh index 4fcc270..7e35612 100755 --- a/tests/gate/test.sh +++ b/tests/gate/test.sh @@ -31,6 +31,7 @@ expect() { ok='{"result":"success","outputs":{}}' expect pass "{\"lint\":${ok},\"test\":${ok}}" expect fail "{\"lint\":${ok},\"test\":{\"result\":\"failure\"}}" "" "test: failure" +# Cancelled jobs fail the gate, whatever cancelled them (see the README) expect fail "{\"lint\":{\"result\":\"cancelled\"},\"test\":${ok}}" "" "lint: cancelled" expect pass "{\"lint\":${ok},\"push\":{\"result\":\"skipped\"}}" "push publish" # IDs on separate lines (e.g. a YAML block scalar) @@ -64,6 +65,68 @@ else echo "ok: step summary" fi +# The gate jobs of this repository's CI and of the README's example run +# `always()` (not `!cancelled()`), and only a new head commit of a pull +# request cancels runs in progress (see "Cancelled runs" in the README) +# gate_if : the job-level `if:` of the job `passed` +gate_if() { + awk ' + # The indent of the first non-blank character + { indent = match($0, /[^ ]/) } + /^ *passed:$/ { job = indent; next } + !job || !/^ *[^ #]/ { next } + indent <= job { exit } + # The indent of the job keys, from the first one + !key { key = indent } + indent == key && /^ *if:/ { sub(/^ *if: */, ""); print; exit } + ' "$1" +} +# The parser finds the job-level condition of `passed` only, not those of +# other jobs, comments or steps +fixture="$(mktemp)" +trap 'rm -f "${summary}" "${fixture}"' EXIT +printf 'jobs:\n lint:\n if: a\n passed:\n # if: b\n needs: [lint]\n if: c\n next:\n if: d\n' > "${fixture}" +if [[ "$(gate_if "${fixture}")" == c ]]; then + echo "ok: gate condition parser" +else + echo "NOT OK: gate condition parser: $(gate_if "${fixture}")" + failures=$((failures + 1)) +fi +printf 'jobs:\n passed:\n steps:\n - if: s\n run: x\n next:\n if: d\n' > "${fixture}" +if [[ -z "$(gate_if "${fixture}")" ]]; then + echo "ok: gate condition parser, no job-level condition" +else + echo "NOT OK: gate condition parser took a step's or job's condition: $(gate_if "${fixture}")" + failures=$((failures + 1)) +fi +# shellcheck disable=SC2016 # a GitHub expression, not a shell one +cancel_expected='${{ github.event_name == '\''pull_request'\'' && github.event.action == '\''synchronize'\'' }}' +for file in .github/workflows/ci.yml actions/gate/README.md; do + condition="$(gate_if "${file}")" + if [[ "${condition}" == "always()" ]]; then + echo "ok: gate condition in ${file}" + else + echo "NOT OK: gate condition in ${file}: ${condition@Q}" + failures=$((failures + 1)) + fi +done +cancel="$(sed -n 's/^ *cancel-in-progress: *//p' .github/workflows/ci.yml)" +# The README's example is the same, folded (`>-`) to fit the line length +cancel_readme="$(awk ' + /^ *cancel-in-progress: >-$/ { inside = 1; next } + inside && /^ +[^ ]/ { sub(/^ +/, ""); out = out (out ? " " : "") $0; next } + inside { exit } + END { print out } +' actions/gate/README.md)" +for pair in "ci.yml:${cancel}" "README.md:${cancel_readme}"; do + if [[ "${pair#*:}" == "${cancel_expected}" ]]; then + echo "ok: cancel-in-progress in ${pair%%:*}" + else + echo "NOT OK: cancel-in-progress in ${pair%%:*}: ${pair#*:}" + failures=$((failures + 1)) + fi +done + if ((failures > 0)); then echo "${failures} test(s) failed" exit 1 From fdbdb10c308b6a3f0e7d93ce7dace70b4faeca0b Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Tue, 10 Nov 2026 18:11:47 +0100 Subject: [PATCH 2/2] fix(gate): don't let reruns cancel PR runs A rerun keeps the event of its run, so rerunning an older commit's `synchronize` run cancelled the run on the head commit, whose gate then failed. Cancel only in a run's first attempt; reruns wait instead. Scope the test of `cancel-in-progress` to the workflow-level `concurrency` block, and read the README's folded example the same way. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011FSoqMpmDCfbACLj3uy6xV --- .github/workflows/ci.yml | 7 ++++--- actions/gate/README.md | 20 ++++++++++--------- tests/gate/test.sh | 43 ++++++++++++++++++++++++++++------------ 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a628f3..7f79508 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,13 +19,14 @@ on: permissions: contents: read -# Cancel superseded runs of a pull request, but only for a new head commit, -# so that a newer run on the same commit doesn't leave a failed gate (see +# Cancel superseded runs of a pull request, but only for a new head commit +# (not for a rerun, which may be of an older commit), so that a newer run on +# the same commit doesn't leave a failed gate (see # "Cancelled runs" in actions/gate/README.md); runs for pushes get their own # group, so that queued runs for successive pushes are not dropped concurrency: group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.sha || github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' && github.event.action == 'synchronize' }} + cancel-in-progress: ${{ github.event_name == 'pull_request' && github.event.action == 'synchronize' && github.run_attempt == 1 }} jobs: lint: diff --git a/actions/gate/README.md b/actions/gate/README.md index a8ee81c..584d58a 100644 --- a/actions/gate/README.md +++ b/actions/gate/README.md @@ -49,7 +49,8 @@ concurrency: && github.sha || github.ref }} cancel-in-progress: >- ${{ github.event_name == 'pull_request' - && github.event.action == 'synchronize' }} + && github.event.action == 'synchronize' + && github.run_attempt == 1 }} ``` ## Inputs @@ -75,11 +76,14 @@ e.g. a run for editing the description, marking the pull request ready or reopening it (event `pull_request`, actions `edited`, `ready_for_review`, `reopened`): the cancelled run's failed gate is then the newest result on the commit until the newer run's gate reports, and stays so if it never -does. Therefore only a new head commit (action -`synchronize`) may cancel the runs of a pull request in progress -(`cancel-in-progress` above); a run for another action waits until the run -in progress has finished, and replaces a waiting one. A waiting run that is -replaced never starts any job, so it leaves no `CI passed` at all. +does. Therefore only a new head commit (action `synchronize`, first +attempt) may cancel the runs of a pull request in progress +(`cancel-in-progress` above). A rerun keeps the event of its run, and a +rerun of an older commit's run must not cancel the run on the head commit, +so reruns never cancel. A run for another action, or a rerun, waits until +the run in progress has finished, and replaces a waiting one. A waiting +run that is replaced never starts any job, so it leaves no `CI passed` at +all. The gate job doesn't use `if: ${{ !cancelled() }}`, which would skip it in cancelled runs: GitHub counts a skipped required check as passed @@ -88,9 +92,7 @@ checks](https://docs.github.com/en/pull-requests/collaborating-with-pull-request so a run cancelled by hand, or replaced by a newer run that hasn't reported yet, would allow the merge without tests. -A run cancelled by hand, or by rerunning a run of an older commit (a rerun -keeps its event, so it cancels the run in progress on the head commit), -fails the gate: rerun the head commit's run, or push, before you merge. +A run cancelled by hand fails the gate: rerun it, or push, before you merge. The price: after an event other than a push, e.g. marking a pull request ready, its result comes only after the run in progress and then a whole new diff --git a/tests/gate/test.sh b/tests/gate/test.sh index 7e35612..d300abe 100755 --- a/tests/gate/test.sh +++ b/tests/gate/test.sh @@ -100,7 +100,7 @@ else failures=$((failures + 1)) fi # shellcheck disable=SC2016 # a GitHub expression, not a shell one -cancel_expected='${{ github.event_name == '\''pull_request'\'' && github.event.action == '\''synchronize'\'' }}' +cancel_expected='${{ github.event_name == '\''pull_request'\'' && github.event.action == '\''synchronize'\'' && github.run_attempt == 1 }}' for file in .github/workflows/ci.yml actions/gate/README.md; do condition="$(gate_if "${file}")" if [[ "${condition}" == "always()" ]]; then @@ -110,19 +110,36 @@ for file in .github/workflows/ci.yml actions/gate/README.md; do failures=$((failures + 1)) fi done -cancel="$(sed -n 's/^ *cancel-in-progress: *//p' .github/workflows/ci.yml)" -# The README's example is the same, folded (`>-`) to fit the line length -cancel_readme="$(awk ' - /^ *cancel-in-progress: >-$/ { inside = 1; next } - inside && /^ +[^ ]/ { sub(/^ +/, ""); out = out (out ? " " : "") $0; next } - inside { exit } - END { print out } -' actions/gate/README.md)" -for pair in "ci.yml:${cancel}" "README.md:${cancel_readme}"; do - if [[ "${pair#*:}" == "${cancel_expected}" ]]; then - echo "ok: cancel-in-progress in ${pair%%:*}" +# cancel_in_progress : `cancel-in-progress` of the workflow-level +# `concurrency` (not of a job's), also folded (`>-`) over several lines as +# in the README +cancel_in_progress() { + awk ' + /^concurrency:/ { top = 1; next } + top && /^[^ ]/ { exit } + top && /^ cancel-in-progress:/ { + sub(/^ cancel-in-progress: */, "") + if ($0 != ">-") { print; exit } + folded = 1; next + } + folded && /^ [^ ]/ { sub(/^ +/, ""); out = out (out ? " " : "") $0; next } + folded { exit } + END { if (out) print out } + ' "$1" +} +printf 'concurrency:\n group: g\njobs:\n a:\n concurrency:\n cancel-in-progress: x\n' > "${fixture}" +if [[ -z "$(cancel_in_progress "${fixture}")" ]]; then + echo "ok: cancel-in-progress parser, workflow level only" +else + echo "NOT OK: cancel-in-progress parser took a job's setting" + failures=$((failures + 1)) +fi +for file in .github/workflows/ci.yml actions/gate/README.md; do + cancel="$(cancel_in_progress "${file}")" + if [[ "${cancel}" == "${cancel_expected}" ]]; then + echo "ok: cancel-in-progress in ${file}" else - echo "NOT OK: cancel-in-progress in ${pair%%:*}: ${pair#*:}" + echo "NOT OK: cancel-in-progress in ${file}: ${cancel@Q}" failures=$((failures + 1)) fi done