diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4e39345..05f94ad 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -30,9 +30,9 @@ env: GIT_CONFIG_VALUE_0: main # Concurrency is intentionally job-scoped. Superseded read-only jobs cancel -# independently, including on main. Write-capable release jobs share one FIFO -# queue so a publication that has started is never cancelled and subsequent -# publications run one at a time. +# independently away from main. Jobs on main are never superseded, which lets +# the terminal status gate reliably treat a cancellation there as a timeout or +# manual cancellation. Write-capable release jobs share one FIFO queue. jobs: # === DETECT CHANGES - determines which jobs should run === detect-changes: @@ -41,7 +41,7 @@ jobs: timeout-minutes: 5 concurrency: group: ${{ github.workflow }}-${{ github.ref }}-detect-changes - cancel-in-progress: true + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} if: github.event_name != 'workflow_dispatch' outputs: any-code-changed: ${{ steps.changes.outputs.any-code-changed }} @@ -100,7 +100,7 @@ jobs: timeout-minutes: 20 concurrency: group: ${{ github.workflow }}-${{ github.ref }}-lint - cancel-in-progress: true + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} needs: [detect-changes] # !cancelled() lets this job evaluate even though detect-changes is skipped # for workflow_dispatch while still propagating workflow cancellation. @@ -187,7 +187,7 @@ jobs: timeout-minutes: 30 concurrency: group: ${{ github.workflow }}-${{ github.ref }}-test - cancel-in-progress: true + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} needs: [detect-changes] env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} @@ -263,7 +263,7 @@ jobs: timeout-minutes: 20 concurrency: group: ${{ github.workflow }}-${{ github.ref }}-build - cancel-in-progress: true + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} needs: [detect-changes, lint, test] # Build change-bearing automatic events and all manually dispatched releases. if: | @@ -336,7 +336,7 @@ jobs: timeout-minutes: 10 concurrency: group: ${{ github.workflow }}-${{ github.ref }}-changelog - cancel-in-progress: true + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} needs: [detect-changes] if: github.event_name == 'pull_request' && needs.detect-changes.outputs.any-code-changed == 'true' steps: @@ -424,7 +424,7 @@ jobs: timeout-minutes: 60 concurrency: group: ${{ github.workflow }}-${{ github.ref }}-docker-build - cancel-in-progress: true + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} needs: [detect-changes] if: github.event_name == 'pull_request' && needs.detect-changes.outputs.any-code-changed == 'true' steps: @@ -691,3 +691,21 @@ jobs: --version "${{ steps.version.outputs.new_version }}" \ --repository "${{ github.repository }}" \ --repository-root "." + + # A job killed by timeout-minutes concludes "cancelled", not "failure". + # Observe every other job so that state cannot silently hide a broken run. + pipeline-status: + name: Pipeline Status + runs-on: ubuntu-latest + timeout-minutes: 5 + if: always() + needs: [detect-changes, lint, test, build, changelog, docker-build, + auto-release, manual-release] + steps: + - uses: actions/checkout@v6 + + - name: Fail the run when a required job was cancelled or failed + env: + NEEDS_JSON: ${{ toJSON(needs) }} + IS_MAIN: ${{ github.ref == 'refs/heads/main' && github.event_name == 'push' }} + run: bash scripts/check-pipeline-status.sh diff --git a/changelog.d/20260809_issue_50_pipeline_timeout_status.md b/changelog.d/20260809_issue_50_pipeline_timeout_status.md new file mode 100644 index 0000000..cf15541 --- /dev/null +++ b/changelog.d/20260809_issue_50_pipeline_timeout_status.md @@ -0,0 +1,5 @@ +### Fixed + +- Added a terminal CI/CD status gate so failed jobs always fail the aggregate + check, while cancelled jobs fail pushes to `main` instead of allowing job + timeouts to look like benign workflow cancellations. diff --git a/scripts/check-pipeline-status.sh b/scripts/check-pipeline-status.sh new file mode 100755 index 0000000..5e8bf84 --- /dev/null +++ b/scripts/check-pipeline-status.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# Make failures and timeout-shaped cancellations visible in the aggregate run. +set -euo pipefail + +: "${NEEDS_JSON:?NEEDS_JSON is required (pass toJSON(needs))}" +IS_MAIN="${IS_MAIN:-false}" + +select_by_result() { + printf '%s' "$NEEDS_JSON" \ + | jq -r --arg want "$1" \ + 'to_entries | map(select(.value.result == $want) | .key) | join(", ")' +} + +failed="$(select_by_result failure)" +cancelled="$(select_by_result cancelled)" + +echo "Failed jobs: ${failed:-}" +echo "Cancelled jobs: ${cancelled:-}" + +status=0 +if [ -n "$failed" ]; then + echo "::error::Pipeline failed. Failing jobs: ${failed}" + status=1 +fi + +if [ -n "$cancelled" ]; then + if [ "$IS_MAIN" = "true" ]; then + echo "::error::Pipeline has cancelled jobs on main: ${cancelled}. A job killed by 'timeout-minutes' is reported as cancelled; check job annotations for an exceeded maximum execution time." + status=1 + else + echo "::warning::Cancelled jobs: ${cancelled}. On a non-default ref this is usually a superseded run." + fi +fi + +if [ "$status" -eq 0 ]; then + echo "All required jobs succeeded or were legitimately skipped." +fi + +exit "$status" diff --git a/tests/test_workflows.py b/tests/test_workflows.py index b315f08..3365f2f 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3,6 +3,8 @@ from __future__ import annotations import re +import shutil +import subprocess from pathlib import Path ROOT = Path(__file__).resolve().parents[1] @@ -180,7 +182,7 @@ def test_changelog_check_safely_requires_a_fragment() -> None: def test_release_workflow_separates_check_and_write_concurrency() -> None: - """Checks cancel independently while release writes share a FIFO queue.""" + """Checks supersede off main while release writes share a FIFO queue.""" workflow = read_workflow("release.yml") workflow_header = workflow.split("\njobs:\n", maxsplit=1)[0] @@ -199,7 +201,7 @@ def test_release_workflow_separates_check_and_write_concurrency() -> None: "group: ${{ github.workflow }}-${{ github.ref }}-" f"{job_name}" ) assert expected_group in block - assert "cancel-in-progress: true" in block + assert "cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}" in block write_concurrency = "\n".join( ( @@ -237,6 +239,7 @@ def test_release_workflow_jobs_have_explicit_timeouts() -> None: "docker-build": 60, "auto-release": 30, "manual-release": 30, + "pipeline-status": 5, } for job_name, timeout in expected_timeouts.items(): @@ -244,11 +247,64 @@ def test_release_workflow_jobs_have_explicit_timeouts() -> None: assert f"timeout-minutes: {timeout}" in block +def test_pipeline_status_gate_covers_every_other_release_job() -> None: + """Every release job must feed the terminal timeout/failure gate.""" + workflow = read_workflow("release.yml") + jobs_section = workflow.split("\njobs:\n", maxsplit=1)[1] + job_names = re.findall(r"^ ([A-Za-z0-9_-]+):$", jobs_section, re.MULTILINE) + gate = workflow_job_block(workflow, "pipeline-status") + + assert "if: always()" in gate + assert "run: bash scripts/check-pipeline-status.sh" in gate + assert "NEEDS_JSON: ${{ toJSON(needs) }}" in gate + assert ( + "IS_MAIN: ${{ github.ref == 'refs/heads/main' && " + "github.event_name == 'push' }}" in gate + ) + for job_name in job_names: + if job_name != "pipeline-status": + assert re.search(rf"(?:^|[\s,[])({re.escape(job_name)})(?=[\s,\]])", gate) + + +def test_pipeline_status_script_handles_all_job_conclusions() -> None: + """The gate fails failures and main cancellations without breaking supersedes.""" + script = ROOT / "scripts" / "check-pipeline-status.sh" + assert script.exists() + assert "set -euo pipefail" in script.read_text(encoding="utf-8") + + if shutil.which("jq") is None: + return + + cases = ( + ({"lint": "success", "test": "skipped"}, True, True), + ({"lint": "failure"}, False, False), + ({"auto-release": "cancelled"}, True, False), + ({"test": "cancelled"}, False, True), + ) + for results, is_main, should_pass in cases: + needs_json = ( + "{" + + ",".join( + f'"{job}":{{"result":"{result}"}}' for job, result in results.items() + ) + + "}" + ) + completed = subprocess.run( + ["bash", str(script)], + cwd=ROOT, + env={"NEEDS_JSON": needs_json, "IS_MAIN": str(is_main).lower()}, + capture_output=True, + text=True, + check=False, + ) + assert completed.returncode == (0 if should_pass else 1), completed.stdout + + def test_release_workflow_action_versions_are_current() -> None: """Release workflow actions should use the current major versions.""" release_workflow = read_workflow("release.yml") - assert_action_pin_count(release_workflow, "actions/checkout", "v6", 8) + assert_action_pin_count(release_workflow, "actions/checkout", "v6", 9) assert_action_pin_count(release_workflow, "actions/setup-python", "v6", 7) assert_action_pin_count(release_workflow, "actions/upload-artifact", "v7", 1) assert_action_pin_count(release_workflow, "actions/download-artifact", "v7", 1)