The generated All checks passed gate counts a skipped job as passed #16

Versions: python-library-template 8ebf45d, Copier 9.18.2

The all-checks-passed job is the one check setup_repo.py makes required, and in the generated CI it fails only on failure or cancelled (template/.github/workflows/ci.yml.jinja:66-71). A job that ends skipped counts as passed. In the generated workflow, the needed jobs are skipped only when lint fails, and the gate already fails on lint. But if someone later adds an if: or a paths filter to test-unit or test-integration, then "the tests did not run" shows up as a green required check. The template's own CI already uses the stricter rule: .github/workflows/ci.yml:77 passes only when every needed job is success.

pg-partsmith already depends on the lenient rule. Its dependency-review job runs only on pull requests (if: github.event_name == 'pull_request'), so on pushes to master it is skipped and the gate is green. That skip is by design, and it looks exactly like an accidental one.

Reproduction

# render of 8ebf45d
sed -n '59,72p' demo-lib/.github/workflows/ci.yml
# the template repository's own gate
sed -n '76,77p' .github/workflows/ci.yml
# pg-partsmith, latest push to master
id=$(gh api "repos/bedrock-python/pg-partsmith/actions/workflows/ci.yml/runs?event=push&branch=master&per_page=1" \
     --jq '.workflow_runs[0].id')
gh api "repos/bedrock-python/pg-partsmith/actions/runs/$id/jobs?per_page=100" \
  --jq '.jobs[] | select(.conclusion != "success" or .name == "All checks passed") | "\(.name)\t\(.conclusion)"'

Actual

  # Sentinel job — use this as the single required status check in branch protection.
  all-checks-passed:
    name: All checks passed
    if: always()
    needs: [lint, test-unit, test-integration]
    runs-on: ubuntu-latest
    steps:
      - name: Check all jobs succeeded
        run: |
          if [[ "${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}" == "true" ]]; then
            echo "One or more required checks failed."
            exit 1
          fi
          echo "All checks passed."

      - name: Every job above succeeded
        run: echo '${{ toJSON(needs) }}' | jq -e 'all(.[]; .result == "success")'

pg-partsmith run 38169905669 (push, master, 2890538):

New dependencies carry no known vulnerability	skipped
All checks passed	success

Why

A required check is only worth something if it fails when the work did not happen. The template already settled this question for its own CI. The generated projects, where tests matter more, got the weaker version.

Proposal

Don't simply copy the template's all(... == "success"), because that would break pg-partsmith and anything else with a job skipped on purpose. Instead, require success from every job except an explicit list of jobs that may be skipped:

      - name: Check all jobs succeeded
        env:
          NEEDS: ${{ toJSON(needs) }}
          MAY_SKIP: '[]'   # jobs skipped by design, e.g. '["dependency-review"]' in pg-partsmith
        run: |
          echo "$NEEDS" | jq -e --argjson skip "$MAY_SKIP" \
            'to_entries | all(.value.result == "success" or (.value.result == "skipped" and (.key | IN($skip[]))))'

In the .jinja file, the ${{ }} needs {% raw %} as usual. I checked the jq expression with jq 1.7.1. success plus an allowed skip gives true, exit 0. A skipped test-unit that is not on the list gives false, exit 1. All success gives true, exit 0.