diff --git a/.github/workflows/monitoring.yml b/.github/workflows/monitoring.yml index f3b485356..a411cf22f 100644 --- a/.github/workflows/monitoring.yml +++ b/.github/workflows/monitoring.yml @@ -48,7 +48,11 @@ jobs: - name: Measure publication heartbeat id: heartbeat run: | + # GitHub's default shell already sets -e, so a non-zero exit would + # abort before the code is recorded and the delivery step would be + # skipped — the check would detect the outage and tell no one. set -uo pipefail + set +e python3 scripts/publish_heartbeat.py \ --repository-root . \ --summary-output "$RUNNER_TEMP/heartbeat.md" @@ -85,10 +89,15 @@ jobs: - name: Enforce divergence and stale thresholds id: release_state + if: always() env: MAIN_SHA: ${{ github.sha }} run: | + # GitHub's default shell already sets -e, so a non-zero exit would + # abort before the code is recorded and the delivery step would be + # skipped — the check would detect the outage and tell no one. set -uo pipefail + set +e main_sha="$(git rev-parse HEAD)" main_committed_at="$(git show -s --format=%cI HEAD)" python3 scripts/production_monitor.py \ @@ -139,10 +148,15 @@ jobs: # so it has to be caught here rather than hours later via staleness. - name: Check recent release runs id: release_runs + if: always() env: GH_TOKEN: ${{ github.token }} run: | + # GitHub's default shell already sets -e, so a non-zero exit would + # abort before the code is recorded and the delivery step would be + # skipped — the check would detect the outage and tell no one. set -uo pipefail + set +e python3 scripts/check_release_runs.py \ --repository "$GITHUB_REPOSITORY" \ --summary-output "$RUNNER_TEMP/release_runs.md" @@ -180,10 +194,15 @@ jobs: # scheduled work rather than an outage. - name: Measure capacity headroom id: capacity + if: always() env: PYTHONPATH: ${{ github.workspace }} run: | + # GitHub's default shell already sets -e, so a non-zero exit would + # abort before the code is recorded and the delivery step would be + # skipped — the check would detect the outage and tell no one. set -uo pipefail + set +e python3 scripts/capacity_report.py \ --summary-output "$RUNNER_TEMP/capacity.md" \ --fail-on never > "$RUNNER_TEMP/capacity.json" diff --git a/tests/test_monitoring_alert_wiring.py b/tests/test_monitoring_alert_wiring.py new file mode 100644 index 000000000..a5e854de8 --- /dev/null +++ b/tests/test_monitoring_alert_wiring.py @@ -0,0 +1,70 @@ +"""Wiring invariants for the monitoring job's alert delivery. + +Run 30798868809 detected a real outage — the publication heartbeat correctly +exited 1 after 101 hours without a new post — and told nobody. GitHub's default +shell already sets -e, so the non-zero exit aborted the step before +`exit_code` reached $GITHUB_OUTPUT, and the delivery step's +`steps..outputs.exit_code != ''` guard was therefore false. The alert was +skipped. + +That is exactly the failure this monitoring exists to prevent, so it is pinned +here rather than left to review. +""" + +from __future__ import annotations + +from pathlib import Path + +import yaml + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = ROOT / ".github" / "workflows" / "monitoring.yml" + + +def _steps() -> list[dict]: + document = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8")) + return document["jobs"]["verify-production-state"]["steps"] + + +def test_every_measurement_disables_errexit_before_capturing_its_code() -> None: + for step in _steps(): + run = str(step.get("run") or "") + if "GITHUB_OUTPUT" not in run or "exit_code" not in run: + continue + assert "set +e" in run, ( + f"{step.get('name')} records an exit code, but GitHub's default shell " + "sets -e; without 'set +e' the step aborts before writing the output " + "and its alert is skipped" + ) + + +def test_every_delivery_step_runs_even_after_its_measurement_fails() -> None: + for step in _steps(): + name = str(step.get("name") or "") + if not name.startswith("Deliver"): + continue + condition = str(step.get("if") or "") + assert condition.startswith("always()"), ( + f"{name} must run after a failed measurement, otherwise a detected " + "outage produces no notification" + ) + + +def test_checks_do_not_block_one_another() -> None: + # A failing heartbeat previously skipped the release-state and capacity + # checks entirely, hiding whatever else was wrong. + ids = {"release_state", "release_runs", "capacity"} + for step in _steps(): + if step.get("id") in ids: + assert str(step.get("if") or "").startswith("always()"), ( + f"{step.get('name')} must not be skipped because an earlier check failed" + ) + + +def test_the_job_still_reports_unhealthy_in_its_own_colour() -> None: + gate = [s for s in _steps() if "Fail the job" in str(s.get("name") or "")] + assert gate, "the job must fail on its own when production is unhealthy" + run = str(gate[0]["run"]) + # A broken alert channel must not be able to make an outage look healthy. + for signal in ("heartbeat", "release_state", "release_runs", "capacity"): + assert signal in run