From 7247cac3ef294797417ad19a6f213b7b03598313 Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Tue, 5 May 2026 19:26:08 -0700 Subject: [PATCH] Make CI/Docs always report a single required check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Background ---------- Both `CI` and `Documentation` had trigger-level `paths:` filters. When a PR touched no in-path files, the entire workflow was skipped — which, for GitHub branch protection, is *not* the same as passing. Required checks that never report stay pending forever, the PR shows "all checks passed" in the conversation view, but the merge button is disabled for non-admins. PR #209 hit this. Fix --- - Drop trigger-level `paths:` from both workflows so the workflow always starts and the aggregator always reports a status. - Add a `changes` job using `dorny/paths-filter@v3` that exposes a boolean output for the same path set the trigger filter used to gate. - Gate each previously-conditional job with `if: github.event_name \!= 'pull_request' || needs.changes.outputs. == 'true'`, so out-of-path PRs skip the heavy jobs (preserving prior behavior) but the workflow itself still runs. - Add a `test-summary` aggregator (and `docs-summary` in docs.yml) that runs with `if: always()` and passes iff every needed job is `success` or `skipped`. Real failures still block; legitimate skips don't. Branch protection follow-up (UI, not code) ------------------------------------------ In repo Settings → Rules → Rulesets (or legacy Branch protection rules for `main`), the required-status-checks list needs to be updated to match this restructure: - Remove the per-job names (e.g. `(CPU) - Julia 1.12.5`, `(GPU)`, `Data Downloading - …`, `Reactant extension - …`, `Build documentation`, `deploy-docs`, plus the old `CI` aggregator). - Add only: - `test-summary` - `docs-summary` Leaving any of the old job names in the required list would reproduce the bug regardless of these workflow changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 62 ++++++++++++++++++++++++++++++-------- .github/workflows/docs.yml | 61 ++++++++++++++++++++++++++++++++----- 2 files changed, 104 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1171dc75a..adca510d3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,15 +1,15 @@ name: CI +# Path-based gating is implemented *inside* this workflow (via the `changes` +# job and per-job `if:` conditions) rather than via a trigger-level `paths:` +# filter. With a trigger-level filter, the entire workflow — including the +# `test-summary` aggregator — would not run on out-of-path PRs, leaving any +# required status checks pending forever and blocking the merge button for +# non-admins. By dropping `paths:` here, the workflow always starts and +# `test-summary` always reports a status to branch protection. on: pull_request: - paths: &paths - - ".github/workflows/ci.yml" - - "ext/**" - - "src/**" - - "test/**" - - "Project.toml" push: - paths: *paths branches: - main tags: "*" @@ -38,8 +38,31 @@ env: ##### jobs: + # Detect whether any CI-relevant paths changed on this PR / push. Each + # downstream job gates itself on the `ci` output (with an exception for + # `push` events, which always run regardless). + changes: + name: Detect path changes + runs-on: ubuntu-latest + outputs: + ci: ${{ steps.filter.outputs.ci }} + steps: + - uses: actions/checkout@v6 + - uses: dorny/paths-filter@v3 + id: filter + with: + filters: | + ci: + - '.github/workflows/ci.yml' + - 'ext/**' + - 'src/**' + - 'test/**' + - 'Project.toml' + cpu_tests: name: (CPU) - Julia ${{ matrix.version }} + needs: changes + if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.ci == 'true' }} runs-on: ${{ matrix.os }} timeout-minutes: 120 strategy: @@ -95,6 +118,8 @@ jobs: gpu_tests: name: (GPU) + needs: changes + if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.ci == 'true' }} runs-on: aws-linux-nvidia-gpu-l4 container: image: ghcr.io/numericalearth/numerical-earth-docker-images:test-julia_1.12.5 @@ -166,6 +191,8 @@ jobs: cds_downloading: name: Data Downloading - Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }} + needs: changes + if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.ci == 'true' }} runs-on: ${{ matrix.os }} timeout-minutes: 60 strategy: @@ -228,6 +255,8 @@ jobs: reactant: name: Reactant extension - Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }} + needs: changes + if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.ci == 'true' }} runs-on: ${{ matrix.os }} timeout-minutes: 80 strategy: @@ -312,17 +341,26 @@ jobs: # run: ./mpiexecjl -np 4 julia --project --color=yes -e 'using Pkg; Pkg.test()' ##### - ##### The final test (a final check) + ##### Aggregator: single required check for branch protection ##### + ##### `test-summary` is intended to be the *only* CI status check listed as + ##### required in branch protection / rulesets. It runs unconditionally + ##### (`if: always()`) and passes iff every needed job is `success` or + ##### `skipped` — so PRs that legitimately skip jobs (e.g. out-of-path + ##### changes) can still merge, while real failures still block. - ci: - name: CI + test-summary: + name: test-summary + if: always() needs: + - changes - cpu_tests - gpu_tests - cds_downloading - reactant runs-on: ubuntu-latest - if: ${{ success() }} steps: - - run: echo "All CI jobs passed successfully." + - name: Check that all needed jobs passed or were skipped + run: | + echo '${{ toJSON(needs) }}' + jq -e 'to_entries | all(.value.result == "success" or .value.result == "skipped")' <<< '${{ toJSON(needs) }}' diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 8d583b64b..a87a9cc96 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -1,15 +1,15 @@ name: Documentation +# Path-based gating is implemented *inside* this workflow (via the `changes` +# job and per-job `if:` conditions) rather than via a trigger-level `paths:` +# filter. With a trigger-level filter, the entire workflow — including the +# `docs-summary` aggregator — would not run on out-of-path PRs, leaving any +# required status checks pending forever and blocking the merge button for +# non-admins. By dropping `paths:` here, the workflow always starts and +# `docs-summary` always reports a status to branch protection. on: pull_request: - paths: &paths - - ".github/workflows/docs.yml" - - "docs/**" - - "examples/**" - - "src/**" - - "Project.toml" push: - paths: *paths branches: - main tags: '*' @@ -31,8 +31,31 @@ env: ECCO_WEBDAV_PASSWORD: ${{ secrets.ECCO_WEBDAV_PASSWORD }} jobs: + # Detect whether any docs-relevant paths changed on this PR / push. The + # `build-docs` job gates itself on the `docs` output (`push` events always + # build). + changes: + name: Detect path changes + runs-on: ubuntu-latest + outputs: + docs: ${{ steps.filter.outputs.docs }} + steps: + - uses: actions/checkout@v6 + - uses: dorny/paths-filter@v3 + id: filter + with: + filters: | + docs: + - '.github/workflows/docs.yml' + - 'docs/**' + - 'examples/**' + - 'src/**' + - 'Project.toml' + build-docs: name: Build documentation + needs: changes + if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.docs == 'true' }} runs-on: [self-hosted, tartarus, gpu-3] timeout-minutes: 1440 permissions: @@ -112,3 +135,27 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} DOCUMENTER_KEY: ${{ secrets.DOCUMENTER_KEY }} run: julia --color=yes docs/deploy.jl + + ##### + ##### Aggregator: single required check for branch protection + ##### + ##### `docs-summary` is intended to be the *only* Documentation status check + ##### listed as required in branch protection / rulesets. It runs + ##### unconditionally (`if: always()`) and passes iff every needed job is + ##### `success` or `skipped` — so PRs that legitimately skip jobs (e.g. + ##### out-of-path changes, or `deploy-docs` on a fork PR) can still merge, + ##### while real failures still block. + + docs-summary: + name: docs-summary + if: always() + needs: + - changes + - build-docs + - deploy-docs + runs-on: ubuntu-latest + steps: + - name: Check that all needed jobs passed or were skipped + run: | + echo '${{ toJSON(needs) }}' + jq -e 'to_entries | all(.value.result == "success" or .value.result == "skipped")' <<< '${{ toJSON(needs) }}'