From 6b41ea27526808db0cd0212da31e5df594dfc890 Mon Sep 17 00:00:00 2001 From: fujibee Date: Tue, 18 Aug 2026 09:29:31 -0700 Subject: [PATCH 1/3] ci: name the shard that stopped, and say what cancelled means The matrix roll-up is one word for twelve jobs, and the word it uses for a shard that ran out of timeout-minutes is 'cancelled'. Two pull requests from an outside contributor sat red for a day on exactly that. Every ubuntu shard, every Windows leg and three of four macOS shards were green; the fourth had been stopped at exactly 25 minutes. The only thing this job said was 'the bats shards did not all pass (result: cancelled)', which names no shard and does not connect 'cancelled' to a runner limit. From the outside it reads as the repository rejecting the change. The roll-up still decides the exit code. What is added is the listing that makes the exit code actionable, and a sentence for the cancelled case saying it is a limit in this repository's CI rather than a defect in the change. The listing is best-effort by construction: if the API call fails, the roll-up's own message stands. It needs actions:read beyond the workflow's contents:read, granted at the job, or that call is a 403 the step swallows and the listing becomes a feature that reads as present and never runs. --- .github/workflows/tests.yml | 45 ++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 54649be2..8cfa0382 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -372,19 +372,54 @@ jobs: if: ${{ !cancelled() }} runs-on: ubuntu-latest timeout-minutes: 10 + # Beyond the workflow's `contents: read`, so the failure path can ask which + # shard stopped. Without it that call is a 403 the step swallows, and the + # listing below would be a feature that reads as present and never runs. + permissions: + contents: read + actions: read steps: - uses: actions/checkout@v4 + # `result` is the matrix's roll-up: one word for twelve jobs, and the word + # it uses for a shard that ran out of `timeout-minutes` is `cancelled`. + # That cost an outside contributor a day. Two of their pull requests went + # red with every ubuntu shard, every Windows leg and three of four macOS + # shards green; the fourth had been stopped at exactly 25 minutes, and + # the only thing this job said was `result: cancelled`. Nothing on the + # page connected that to a runner limit, so it read as the repository + # rejecting their change. + # + # So name the jobs. The roll-up decides the exit code, as before; the + # listing is what makes the exit code actionable. - name: Check shard results env: RESULT: ${{ needs.bats-shard.result }} + GH_TOKEN: ${{ github.token }} run: | set -euo pipefail - case "$RESULT" in - success) ;; - skipped) echo "::error::the bats shards did not run"; exit 1 ;; - *) echo "::error::the bats shards did not all pass (result: $RESULT)"; exit 1 ;; - esac + if [ "$RESULT" = "success" ]; then exit 0; fi + if [ "$RESULT" = "skipped" ]; then echo "::error::the bats shards did not run"; exit 1; fi + + # Best-effort: the diagnosis must never decide the outcome. A failure + # to list the jobs leaves the roll-up's own message standing rather + # than turning a red into a green or into a different red. + if gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}/jobs" \ + --jq '.jobs[] | select(.name | startswith("bats (")) | select(.conclusion != "success") | "\(.conclusion)\t\(.name)"' \ + > /tmp/bad-shards.txt 2>/dev/null && [ -s /tmp/bad-shards.txt ]; then + echo "Shards that did not pass:" + sed 's/^/ /' /tmp/bad-shards.txt + if grep -q '^cancelled' /tmp/bad-shards.txt; then + echo + echo "A shard reported as 'cancelled' usually means it hit this workflow's" + echo "timeout-minutes rather than failing a test. Its log ends mid-suite and" + echo "names no failing case. That is a limit in this repository's CI, not a" + echo "defect in the change under test." + fi + fi + echo "::error::the bats shards did not all pass (result: $RESULT)" + exit 1 - name: Docs-only change — no shard manifests to verify if: needs.changes.outputs.docs_only == 'true' From c98c055e48c5741440181d41a63b007e76ad0063 Mon Sep 17 00:00:00 2001 From: fujibee Date: Tue, 18 Aug 2026 12:14:51 -0700 Subject: [PATCH 2/3] ci: say only what the query saw, and let a green run prove it can see MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections from review. The emitted text asserted three things this code does not establish: that the cap had been reached, that the log ended mid-suite, and that the change under test was innocent. The query observes a conclusion and a job name, and nothing else. The last claim is worse than vague — a change that hangs drives the shard into the same 25-minute cancel, so the message would have told an outside contributor the opposite of what is known, in exactly the case where it matters. It now names the job and hands over the discriminator: open it, and if it ran about 25 minutes and ends mid-suite naming no failing case, the cap was reached — while saying plainly that the cap being reached does not by itself say whose fault it is. Second, the listing was said to be untestable without a failing shard. It is not. It now runs on every outcome and prints how many matrix jobs it saw, so a permissions or API problem surfaces on a green run as a warning rather than waiting for a red to discover the diagnosis was never able to speak. Also moved the scratch files to $RUNNER_TEMP. --- .github/workflows/tests.yml | 57 +++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8cfa0382..19ef5f09 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -386,36 +386,57 @@ jobs: # That cost an outside contributor a day. Two of their pull requests went # red with every ubuntu shard, every Windows leg and three of four macOS # shards green; the fourth had been stopped at exactly 25 minutes, and - # the only thing this job said was `result: cancelled`. Nothing on the - # page connected that to a runner limit, so it read as the repository - # rejecting their change. + # the only thing this job said was `result: cancelled`, which names no + # shard. From the outside it read as the repository rejecting the change. # - # So name the jobs. The roll-up decides the exit code, as before; the - # listing is what makes the exit code actionable. + # So name the job. Naming is all this does: `conclusion` plus a job name + # is the whole of what the query observes, and the text below says only + # that. An earlier draft went on to assert the cap had been hit, that the + # log ended mid-suite, and that the change under test was innocent -- none + # of which this code establishes, and the last of which is false whenever + # the change is itself what hangs. Give the reader the discriminator and + # let them apply it. + # + # The listing also runs on success, where it must find matrix jobs and + # finds no failures. That is its positive control: a permissions or API + # problem shows up on a green run as "could not list", instead of waiting + # for a red to discover the diagnosis was never able to speak. - name: Check shard results env: RESULT: ${{ needs.bats-shard.result }} GH_TOKEN: ${{ github.token }} run: | set -euo pipefail - if [ "$RESULT" = "success" ]; then exit 0; fi - if [ "$RESULT" = "skipped" ]; then echo "::error::the bats shards did not run"; exit 1; fi + listing="$RUNNER_TEMP/shard-jobs.txt" - # Best-effort: the diagnosis must never decide the outcome. A failure - # to list the jobs leaves the roll-up's own message standing rather - # than turning a red into a green or into a different red. + # Inside `if`, so a failed call cannot end the step: the roll-up below + # stays the only thing that decides this job's status. if gh api --paginate \ "repos/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}/jobs" \ - --jq '.jobs[] | select(.name | startswith("bats (")) | select(.conclusion != "success") | "\(.conclusion)\t\(.name)"' \ - > /tmp/bad-shards.txt 2>/dev/null && [ -s /tmp/bad-shards.txt ]; then + --jq '.jobs[] | select(.name | startswith("bats (")) | "\(.conclusion)\t\(.name)"' \ + > "$listing" 2>/dev/null && [ -s "$listing" ]; then + echo "Matrix jobs seen: $(wc -l < "$listing" | tr -d ' ')" + else + echo "::warning::could not list the shard jobs (the diagnosis below will be missing)" + : > "$listing" + fi + + if [ "$RESULT" = "success" ]; then exit 0; fi + if [ "$RESULT" = "skipped" ]; then echo "::error::the bats shards did not run"; exit 1; fi + + if grep -v '^success' "$listing" > "$RUNNER_TEMP/bad.txt" 2>/dev/null \ + && [ -s "$RUNNER_TEMP/bad.txt" ]; then echo "Shards that did not pass:" - sed 's/^/ /' /tmp/bad-shards.txt - if grep -q '^cancelled' /tmp/bad-shards.txt; then + sed 's/^/ /' "$RUNNER_TEMP/bad.txt" + if grep -q '^cancelled' "$RUNNER_TEMP/bad.txt"; then echo - echo "A shard reported as 'cancelled' usually means it hit this workflow's" - echo "timeout-minutes rather than failing a test. Its log ends mid-suite and" - echo "names no failing case. That is a limit in this repository's CI, not a" - echo "defect in the change under test." + echo "A shard whose conclusion is 'cancelled' was stopped rather than" + echo "failing a case, and this job cannot tell you why: reaching the" + echo "25-minute job cap and being cancelled from outside the run look" + echo "identical from here. Open the job named above. If it ran for about" + echo "25 minutes and its log ends mid-suite naming no failing case, it" + echo "reached the cap -- which a change that hangs can also cause, so the" + echo "cap being reached does not by itself say whose fault it is." fi fi echo "::error::the bats shards did not all pass (result: $RESULT)" From 4871ec14cf061469ac616ab691ece9e65a722506 Mon Sep 17 00:00:00 2001 From: fujibee Date: Tue, 18 Aug 2026 16:29:07 -0700 Subject: [PATCH 3/3] ci: list the jobs the roll-up actually summarises, and check the count Review found the query selecting a different set from the one it reports on. `needs.bats-shard.result` rolls up 8 jobs: ubuntu and macos, four shards each. The predicate was `startswith("bats (")`, which also matches three separately gated Windows legs -- 11 jobs against the roll-up's 8. On a run where both the matrix and a Windows leg were red, the diagnosis could have named the Windows failure as though it explained the roll-up. Match the matrix shape instead, and compare the count against SHARD_TOTAL times the matrix's OS list. A mismatch suppresses the listing with a warning rather than describing some other set of jobs: the control is the comparison, not the fact that rows came back. The control added in the previous commit is what exposed this. It printed 11 on a green run, where the roll-up covers 8. --- .github/workflows/tests.yml | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 19ef5f09..ea1c8025 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -411,11 +411,30 @@ jobs: # Inside `if`, so a failed call cannot end the step: the roll-up below # stays the only thing that decides this job's status. + # The exact matrix shape, not the `bats (` prefix. Three Windows legs + # share that prefix and are gated separately -- they are NOT part of + # `needs.bats-shard.result`, so listing them here would let the + # diagnosis name an unrelated Windows failure as if it explained the + # roll-up. The prefix matched 11 jobs where the roll-up covers 8. + # 2 = the matrix's `os:` list (ubuntu-latest, macos-latest). If a third + # is added, `seen` stops matching and the listing goes quiet with a + # warning rather than describing the wrong set -- the failure mode + # this count exists to prevent, announcing itself. + expected=$(( SHARD_TOTAL * 2 )) if gh api --paginate \ "repos/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}/jobs" \ - --jq '.jobs[] | select(.name | startswith("bats (")) | "\(.conclusion)\t\(.name)"' \ + --jq '.jobs[] | select(.name | test("^bats \\((ubuntu|macos)-latest [0-9]+/[0-9]+\\)$")) | "\(.conclusion)\t\(.name)"' \ > "$listing" 2>/dev/null && [ -s "$listing" ]; then - echo "Matrix jobs seen: $(wc -l < "$listing" | tr -d ' ')" + seen=$(wc -l < "$listing" | tr -d ' ') + echo "Matrix jobs seen: $seen (expected $expected)" + # The control is the comparison, not the listing. A count that does + # not match means this query is no longer selecting the set the + # roll-up summarises, and the lines below would describe some other + # set of jobs. + [ "$seen" = "$expected" ] || { + echo "::warning::listed $seen matrix jobs, expected $expected — not reporting shard names from a set that does not match the roll-up" + : > "$listing" + } else echo "::warning::could not list the shard jobs (the diagnosis below will be missing)" : > "$listing"