From b67a9c1cea922869689df807c8dd97f0fba55994 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 02:26:26 +0000 Subject: [PATCH 1/2] Hand the live loop off explicitly instead of hoping the cron does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop holds a runner ~5.5h and exits under GitHub's 6h per-job ceiling. Continuation depended on the `*/15` cron having left a queued successor in the shared concurrency group during that window — which the workflow header described as something the coarse cron did "reliably". On 2026-08-27, round 2 of Worlds, it did not. Run #1019 yielded at 20:01Z announcing "the queued successor takes over" and nothing took over. GitHub delivered zero scheduled fires for this workflow between 14:30Z and 23:17Z, when it was restarted by hand — including the 3h11m after the yield when the concurrency slot was completely free, so a congested queue does not explain it. `refresh.yml`'s `0 11 * * *` cron fired that day at 20:57Z, ~10 hours late, which is the corroborating evidence that the scheduler rather than the queue was at fault; that late run is also the only reason the site was 2h15m stale instead of 3h16m. An earlier gap the same day went unnoticed: #1018 ended 08:52Z, #1019 started 14:30Z, 5h38m uncovered. Nothing in the pipeline was broken — the loop, the gate, the publish path and the invariants were all healthy, which is exactly why no run went red and nobody was notified. A liveness failure that produces no failing run produces no alert either. So the yielding run now dispatches its own successor and confirms it exists. `workflow_dispatch` (with `repository_dispatch`) is the documented exception to the rule that GITHUB_TOKEN-triggered events start no workflow run, so this needs no PAT, only `actions: write`. - .github/dispatch-successor.sh dispatches and then POLLS for the run, re-dispatching once and failing non-zero if it cannot confirm one. The verification is the substance: a 204 is not a run, as the 2026-08-26 Actions incident showed, and a hand-off that reports success with nothing enqueued reproduces the outage exactly. - The dispatch happens before the current run exits, so the successor lands in the concurrency group as the pending run and starts as the slot frees. - A failed hand-off fails the run, rather than exiting quietly with nothing lined up — the silence is what made this expensive. - A crash-out gets one automatic retry, marked after_failure=true; a run carrying that flag does not chain again, so transient breaks self-heal while persistent ones cannot spin runners. Still on the cron: starting the loop when an event goes live. That window is more forgiving (the loop exits in seconds when nothing is live, so a late start costs the opening minutes of a round, not hours mid-round), and HARDENING item 10 records the fix if it ever bites. Tests stub `gh` on PATH and drive the SUCCESSOR_* knobs to zero delay. Verified by mutation: removing the poll-for-the-run step fails 4 of the 7. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018qBS4DHn53TNPoQZXRsiPq --- .github/dispatch-successor.sh | 101 +++++++++++++++++++ .github/workflows/live-refresh.yml | 62 ++++++++++-- HARDENING.md | 57 ++++++++++- tests/test_dispatch_successor.py | 150 +++++++++++++++++++++++++++++ 4 files changed, 359 insertions(+), 11 deletions(-) create mode 100755 .github/dispatch-successor.sh create mode 100644 tests/test_dispatch_successor.py diff --git a/.github/dispatch-successor.sh b/.github/dispatch-successor.sh new file mode 100755 index 00000000..1047c3c0 --- /dev/null +++ b/.github/dispatch-successor.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# +# Hand the live-refresh loop off to a fresh run of itself, and prove it landed. +# +# The loop yields at ~5.5h to stay under GitHub's 6h per-job ceiling, and used +# to depend on the `*/15` cron having left a queued successor in the shared +# concurrency group during that window. On 2026-08-27 it had not: run #1019 +# yielded at 20:01Z announcing "the queued successor takes over" and nothing +# took over. GitHub delivered ZERO scheduled fires for the workflow between +# 14:30Z and 23:17Z, when the loop was restarted by hand — 3h16m with no live +# updates during round 2 of Worlds, and 5h38m earlier the same day (08:52Z -> +# 14:30Z). Corroborating the scheduler as the culprit rather than the queue: +# refresh.yml's `0 11 * * *` cron fired that day at 20:57Z, ~10 hours late. +# +# So the hand-off no longer waits for a cron fire that may never come — the +# yielding run dispatches its own successor. `workflow_dispatch` (with +# `repository_dispatch`) is the documented exception to the rule that events +# triggered by GITHUB_TOKEN do not create a new workflow run, so this needs no +# PAT; it needs `actions: write`, which live-refresh.yml now requests. +# +# The successor is dispatched BEFORE the current run exits, so it lands in the +# concurrency group as the pending run and starts the instant the slot frees — +# the same shape the cron was supposed to produce, minus the hoping. +# +# Verification is the point, not a nicety. On 2026-08-26 an Actions incident +# left dispatches returning 204 and then silently dropping: accepted, never +# enqueued. A hand-off that reports success without a run to show for it is +# exactly the failure this script exists to prevent, so it polls for the new +# run, re-dispatches once, and only then gives up — loudly, non-zero. +# +# Usage: dispatch-successor.sh [key=value ...] +# key=value pairs are passed through as workflow inputs (`gh -f key=value`). +# +# Env: +# GH_TOKEN required; the workflow's github.token is enough +# GITHUB_REPOSITORY owner/repo (set by Actions; falls back to gh's remote) +# SUCCESSOR_WORKFLOW workflow to dispatch (default live-refresh.yml) +# SUCCESSOR_REF ref to dispatch it on (default main) +# SUCCESSOR_ATTEMPTS dispatch attempts (default 2) +# SUCCESSOR_POLL_TRIES polls per attempt (default 10) +# SUCCESSOR_POLL_SECS seconds between polls (default 3) +# The four tunables exist so the test suite can drive this without sleeping. +# +# Deliberately not `set -e`: every failure here is handled and reported, and +# the caller decides what a failed hand-off means for its own exit status. +set -uo pipefail + +workflow="${SUCCESSOR_WORKFLOW:-live-refresh.yml}" +ref="${SUCCESSOR_REF:-main}" +attempts="${SUCCESSOR_ATTEMPTS:-2}" +poll_tries="${SUCCESSOR_POLL_TRIES:-10}" +poll_secs="${SUCCESSOR_POLL_SECS:-3}" + +repo="${GH_REPO:-${GITHUB_REPOSITORY:-}}" +repo_args=() +[ -n "$repo" ] && repo_args=(--repo "$repo") + +input_args=() +for pair in "$@"; do + input_args+=(-f "$pair") +done + +# Newest run id for this workflow. Run ids increase, and the current run is +# already in the list, so "the newest id changed" is a sufficient and cheap +# test for "a new run exists" without needing to identify it precisely. +newest_run_id() { + gh run list "${repo_args[@]}" --workflow "$workflow" --limit 1 \ + --json databaseId --jq '.[0].databaseId' 2>/dev/null +} + +before="$(newest_run_id)" +if [ -z "$before" ]; then + # Not fatal on its own: we can still dispatch, we just cannot confirm by + # comparison. Treat an unreadable baseline as "nothing seen yet" so any id + # appearing below counts as the successor. + echo "warning: could not read the current newest run id for $workflow" >&2 + before="" +fi + +for attempt in $(seq 1 "$attempts"); do + if gh workflow run "$workflow" "${repo_args[@]}" --ref "$ref" "${input_args[@]}"; then + echo "dispatch attempt $attempt: accepted" + else + echo "dispatch attempt $attempt: gh workflow run failed" >&2 + continue + fi + + # A 204 is not a run. Poll until one actually shows up. + for _ in $(seq 1 "$poll_tries"); do + sleep "$poll_secs" + now="$(newest_run_id)" + if [ -n "$now" ] && [ "$now" != "$before" ]; then + echo "successor queued: run $now (dispatched $workflow on $ref)" + exit 0 + fi + done + echo "dispatch attempt $attempt: accepted but no new run appeared" >&2 +done + +echo "::error::Could not hand off to a successor run of $workflow after $attempts attempts — live updates stop here until the schedule restarts the loop." +exit 1 diff --git a/.github/workflows/live-refresh.yml b/.github/workflows/live-refresh.yml index d47b655f..e131e0b7 100644 --- a/.github/workflows/live-refresh.yml +++ b/.github/workflows/live-refresh.yml @@ -10,20 +10,40 @@ name: Live refresh # scores actually moved. It keeps looping until no points event is live or it # nears the 6h per-job ceiling, then yields. # -# Continuation across that ceiling needs no PAT: the shared concurrency group -# keeps exactly one run active and one queued, and the coarse cron reliably -# leaves a queued successor during any 5.5h window, so it starts the instant -# the active run yields. Between rounds / overnight the loop idles on the cheap -# check (no pip, no sim) and, when nothing is live, exits in seconds. Public -# repo => Actions minutes are free, so holding a runner during an event is fine. +# Continuation across that ceiling is explicit, and needs no PAT: before it +# yields, the run dispatches its own successor (.github/dispatch-successor.sh) +# and confirms a run actually appeared. `workflow_dispatch` is the documented +# exception to the rule that GITHUB_TOKEN-triggered events create no workflow +# run, so this costs only the `actions: write` permission below. +# +# It used to rely instead on the coarse cron having left a queued successor in +# the concurrency group during the 5.5h window. On 2026-08-27 none had been: +# GitHub delivered zero scheduled fires for this workflow between 14:30Z and +# 23:17Z, so the loop yielded into nothing and the site sat on a stale bundle +# for 3h16m during round 2 of Worlds (and 5h38m earlier the same day). The cron +# now only has to START the loop when an event begins; it is no longer what +# keeps it alive. Between rounds / overnight the loop idles on the cheap check +# (no pip, no sim) and, when nothing is live, exits in seconds. Public repo => +# Actions minutes are free, so holding a runner during an event is fine. on: schedule: - - cron: "*/15 * * * *" # only needs to (re)start / keep a successor queued - workflow_dispatch: {} + - cron: "*/15 * * * *" # only needs to (re)start the loop when one goes live + workflow_dispatch: + inputs: + after_failure: + # Deliberately a string, not a boolean: this is dispatched by + # `gh workflow run -f after_failure=true`, which sends inputs as + # strings, and the loop compares it as one. A boolean-typed input + # would put a coercion between the two for no benefit — nothing + # reads this except the [ "$AFTER_FAILURE" = "true" ] below. + description: "Internal: marks the one automatic retry that follows a failed run, so the chain stops there." + type: string + default: "false" permissions: contents: write pages: write + actions: write # dispatch our own successor; see .github/dispatch-successor.sh concurrency: group: refresh-forecast-v2 # share with the weekly full refresh; never overlap @@ -75,6 +95,7 @@ jobs: PDGA_USERNAME: ${{ secrets.PDGA_USERNAME }} PDGA_PASSWORD: ${{ secrets.PDGA_PASSWORD }} GH_TOKEN: ${{ github.token }} + AFTER_FAILURE: ${{ github.event.inputs.after_failure || 'false' }} run: | set -uo pipefail git config user.name "github-actions[bot]" @@ -91,11 +112,24 @@ jobs: python -c 'import sys; from dgpt import schedule; sys.exit(0 if schedule.live_events() else 3)' } + # A crash-out gets ONE automatic retry: this run hands off with + # after_failure=true, and a run carrying that flag does not chain + # again. So a transient break (a PDGA blip, a bad runner) self-heals + # in a minute instead of waiting on a cron fire that may be hours + # away, while a persistent one cannot spin runners in a tight loop — + # past that single hop the cron is the only restart, which is the + # right amount of patience for a break that is not going to fix + # itself. The run still goes red either way; the owner is notified. fail_step() { failures=$((failures + 1)) echo "::warning::$1 (consecutive failures: $failures)" if [ "$failures" -ge 3 ]; then - echo "::error::Three consecutive live-refresh failures — failing the run so it gets seen. The cron restarts the loop." + echo "::error::Three consecutive live-refresh failures — failing the run so it gets seen." + if [ "$AFTER_FAILURE" = "true" ]; then + echo "::warning::Already the post-failure retry — not chaining again. The cron restarts the loop." + elif ! .github/dispatch-successor.sh after_failure=true; then + echo "::warning::Could not hand off after failure; the cron restarts the loop." + fi exit 1 fi } @@ -187,7 +221,15 @@ jobs: fi if [ "$(date +%s)" -ge "$BUDGET" ]; then - echo "Near job time limit — yielding; the queued successor takes over." + # Dispatch BEFORE breaking, so the successor sits in the + # concurrency group as the pending run and starts the moment this + # one releases the slot. A hand-off that cannot be confirmed is + # precisely the 2026-08-27 outage, so it fails the run instead of + # exiting quietly with nothing lined up to take over. + echo "Near job time limit — handing off to a fresh run." + if ! .github/dispatch-successor.sh; then + exit 1 + fi exit_reason=yield break fi diff --git a/HARDENING.md b/HARDENING.md index ec90679b..06fc06f4 100644 --- a/HARDENING.md +++ b/HARDENING.md @@ -103,6 +103,31 @@ pattern across them, not any single bug, is what this backlog addresses. UTC-rollover bug waiting for a US Sunday finish, and the second one (permanent caching keyed on date) was sitting behind the first. +11. **The loop yielded at the job ceiling and nothing took over.** The + live loop holds a runner ~5.5h and exits under GitHub's 6h per-job + ceiling; continuation depended on the `*/15` cron having left a + queued successor in the shared concurrency group — described in the + workflow as something the coarse cron did "reliably". On 2026-08-27, + round 2 of Worlds, it had not. Run #1019 yielded at 20:01Z + announcing "the queued successor takes over" and nothing did. GitHub + delivered **zero** scheduled fires for the workflow between 14:30Z + and 23:17Z, when it was restarted by hand — including the 3h11m + after the yield, when the concurrency slot was completely free, so a + congested queue does not explain it. Corroboration that the + scheduler rather than the queue was at fault: `refresh.yml`'s + `0 11 * * *` cron fired that day at 20:57Z, ~10 hours late. That + late run is also the only reason the site was 2h15m stale instead of + 3h16m — the once-a-day job happened to land in the hole. An earlier + gap the same day went unnoticed entirely: #1018 ended 08:52Z, #1019 + started 14:30Z, 5h38m uncovered. Nothing in the pipeline was broken; + the loop, the gate, the publish path and the invariants were all + healthy, which is why no run went red and nobody was notified. + Lesson: "best effort" in GitHub's cron documentation means the + delivery rate can go to zero for hours, so anything load-bearing + built on "a fire will arrive within this window" is a scheduled + outage rather than a design — and a liveness failure that produces + no failing run produces no alert either. + ## The plan Ordered by expected payoff. "In-season safe" = additive, can't change a @@ -114,7 +139,8 @@ published number. failure posture in `.github/workflows/live-refresh.yml`. Item 6 shipped 2026-08 (`.github/publish-site.sh`). Items 7 and 9 are offseason work; item 8's first two pieces (a resnapshot command, the invariant gate on -recording) are in-season safe and still open.* +recording) are in-season safe and still open. Item 10 shipped 2026-08 +(`.github/dispatch-successor.sh`).* ### 1. Commit the regression corpus; make it a test suite *(shipped 2026-07)* @@ -264,3 +290,32 @@ broken. - `validate.py`'s HTML parsing is regex over ``; if item 7 lands, a parse failure downgrades from "blind spot" to "lost redundancy," which is the right severity for it. + +### 10. Hand the live loop off explicitly *(shipped 2026-08)* + +`.github/dispatch-successor.sh`, after incident 11. Before yielding at +the ~5.5h budget the run dispatches its own successor and polls until +that run actually exists, re-dispatching once and failing the run if it +cannot confirm one. The verification is the substance, not politeness: a +204 from the dispatch endpoint is not a run, as the 2026-08-26 Actions +incident showed, and a hand-off that reports success with nothing +enqueued reproduces the outage exactly. `workflow_dispatch` (with +`repository_dispatch`) is the documented exception to the rule that +GITHUB_TOKEN-triggered events start no workflow run, so this needs no +PAT — only `actions: write`. + +The dispatch happens *before* the current run exits, so the successor +lands in the concurrency group as the pending run and starts the moment +the slot frees: the shape the cron was supposed to produce, minus the +dependency on delivery. A crash-out gets one automatic retry, marked +`after_failure=true`, and a run carrying that flag does not chain again +— transient breaks self-heal in a minute, persistent ones cannot spin +runners. + +What still rides on the cron is *starting* the loop when an event goes +live. That is the more forgiving window — the loop exits in seconds when +nothing is live, so a late start costs the opening minutes of a round +rather than hours mid-round — but it is the same dependency, and a +multi-hour delivery gap across a Sunday-morning tee time would show. The +fix if it bites: keep looping while a points event starts within the +next several hours, instead of exiting on "nothing live right now". diff --git a/tests/test_dispatch_successor.py b/tests/test_dispatch_successor.py new file mode 100644 index 00000000..369ea54e --- /dev/null +++ b/tests/test_dispatch_successor.py @@ -0,0 +1,150 @@ +"""The live loop's hand-off: .github/dispatch-successor.sh. + +This is the script that keeps the site updating across GitHub's 6h per-job +ceiling. It replaced an assumption that failed in production on 2026-08-27 — +that the `*/15` cron would have left a queued successor in the concurrency +group — so the thing worth testing is that it never reports success without a +run to show for it. A dispatch that returns 204 and enqueues nothing is a real +observed failure (2026-08-26), and it is indistinguishable from a good one at +the call site. + +`gh` is stubbed by a script on PATH: the run-id file it reads is the whole +world model. Polling is driven to zero delay through the SUCCESSOR_* knobs the +script exposes for exactly this purpose. +""" +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).parent.parent / ".github" / "dispatch-successor.sh" + +STUB_GH = """#!/usr/bin/env bash +echo "$*" >> "$GH_LOG" +case "$1 $2" in + "run list") + cat "$STUB_STATE/newest" 2>/dev/null + ;; + "workflow run") + if [ -f "$STUB_STATE/fail_once" ]; then + rm -f "$STUB_STATE/fail_once" + echo "stub: dispatch refused" >&2 + exit 1 + fi + # Whether the accepted dispatch actually produces a run is the variable + # under test: STUB_APPEAR=0 reproduces the accepted-then-dropped case. + if [ "${STUB_APPEAR:-1}" = "1" ]; then + echo $(( $(cat "$STUB_STATE/newest") + 1 )) > "$STUB_STATE/newest" + fi + ;; +esac +exit 0 +""" + + +@pytest.fixture +def harness(tmp_path): + """A stubbed `gh` on PATH plus a runner for the script under test.""" + bindir = tmp_path / "bin" + bindir.mkdir() + gh = bindir / "gh" + gh.write_text(STUB_GH, encoding="utf-8") + gh.chmod(0o755) + + state = tmp_path / "state" + state.mkdir() + (state / "newest").write_text("1000\n", encoding="utf-8") + log = tmp_path / "gh.log" + log.write_text("", encoding="utf-8") + + def run(*inputs: str, appear: bool = True, fail_once: bool = False, **env_over): + if fail_once: + (state / "fail_once").write_text("", encoding="utf-8") + env = { + **os.environ, + "PATH": f"{bindir}:{os.environ['PATH']}", + "GH_LOG": str(log), + "STUB_STATE": str(state), + "STUB_APPEAR": "1" if appear else "0", + "GH_TOKEN": "stub-token", + "GITHUB_REPOSITORY": "dgoodenough/discgolf", + # No sleeping in tests; two attempts, two polls each. + "SUCCESSOR_ATTEMPTS": "2", + "SUCCESSOR_POLL_TRIES": "2", + "SUCCESSOR_POLL_SECS": "0", + **env_over, + } + proc = subprocess.run( + [str(SCRIPT), *inputs], capture_output=True, text=True, env=env, timeout=60, + ) + return proc, log.read_text(encoding="utf-8") + + return run + + +def _dispatches(gh_log: str) -> list[str]: + return [ln for ln in gh_log.splitlines() if ln.startswith("workflow run")] + + +def test_hands_off_and_confirms_the_new_run(harness): + proc, gh_log = harness() + assert proc.returncode == 0, proc.stderr + assert "successor queued: run 1001" in proc.stdout + assert len(_dispatches(gh_log)) == 1 + assert "workflow run live-refresh.yml --repo dgoodenough/discgolf --ref main" in gh_log + + +def test_passes_workflow_inputs_through(harness): + """The post-failure retry is marked so the chain stops at one hop; if the + flag does not reach the successor, a persistent crash loops forever.""" + proc, gh_log = harness("after_failure=true") + assert proc.returncode == 0, proc.stderr + assert "-f after_failure=true" in gh_log + + +def test_fails_loudly_when_the_dispatch_is_accepted_but_drops(harness): + """The 2026-08-26 shape: 204 accepted, nothing enqueued. Silence here is + the outage, so the script must exhaust its retries and exit non-zero.""" + proc, gh_log = harness(appear=False) + assert proc.returncode == 1 + assert "::error::" in proc.stdout + assert "Could not hand off to a successor" in proc.stdout + assert len(_dispatches(gh_log)) == 2 # SUCCESSOR_ATTEMPTS + + +def test_retries_when_the_dispatch_call_itself_fails(harness): + """A refused `gh workflow run` is worth one more try before giving up.""" + proc, gh_log = harness(fail_once=True) + assert proc.returncode == 0, proc.stderr + assert len(_dispatches(gh_log)) == 2 + assert "successor queued" in proc.stdout + + +def test_reports_the_run_it_actually_saw(harness): + """The confirmation names the new run id, so a hand-off can be audited + from the log of the run that made it rather than inferred.""" + proc, _ = harness() + assert "run 1001" in proc.stdout + + +def test_survives_an_unreadable_baseline(harness, tmp_path): + """If the pre-dispatch run list cannot be read, the dispatch still has to + happen — an unknown baseline is not a reason to skip the hand-off.""" + (tmp_path / "state" / "newest").unlink() + proc, gh_log = harness() + assert proc.returncode == 0, proc.stderr + assert len(_dispatches(gh_log)) == 1 + + +def test_runs_without_an_explicit_repo(harness): + """Outside Actions there is no GITHUB_REPOSITORY and `gh` infers the repo + from the remote. The point of the case is the shell: `--repo` is built as + an array that is empty here, and an empty array expanded under `set -u` + aborts on bash before 4.4. Runners are 5.x, but the script is also run by + hand.""" + proc, gh_log = harness(GITHUB_REPOSITORY="", GH_REPO="") + assert proc.returncode == 0, proc.stderr + assert "--repo" not in gh_log From 6759622a11618119c90006da382283b280a14873 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 20:28:37 +0000 Subject: [PATCH 2/2] Stop the invariant alert from taking the live loop down with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two PDGA rows at Worlds carried impossible cumulative scores on 2026-08-29: MPO Sander Bahnerth cur=+981 at 16:10Z, and two hours later FPO Samantha Zaborowski cur=+849, both far outside the per-round bounds. The publish-gate checks did exactly their job — published the data, then failed the run so the owner was notified. What that costs had not been noticed. `exit 1` ends the run, and the run IS the live loop, so each bad row also stopped the site updating. Worse, the de-dupe meant to keep a persistent violation from re-alerting could never fire: its marker lived in data/cache/, which is gitignored and carried between runs only by actions/cache, whose save step is skipped when the job ends non-zero. The alerting run is always the failing run, so the marker written by the run that alerts was precisely the one guaranteed not to survive. Both runs show "Post Restore results cache: skipped". Every restart re-alerted and died again; round 4 ran on hourly updates (16:10, 17:26, 18:38Z) instead of six-minute ones, and the 17:26 one was the daily cron rather than the loop. The hand-off added in the previous commit did not cover this. It handles the budget yield and the three-crash path via fail_step; the invariant block has its own bare `exit 1`, a third exit with no successor. Found in production, not in review. - .github/invariant-alert.sh makes the alert decision and owns the marker. Exit 3 = new violations, 0 = clean or already alerted, 1 = broken, so the caller can order publish/commit/alert around it. - The marker moves to data/invariant_alerted.txt, tracked alongside the other cross-run state the pipeline reads back, and the decision now runs BEFORE the state commit so it is committed rather than lost with the run. - The invariant exit hands off to a successor, which reads that marker, sees the same violation set and stays quiet — the loop keeps publishing while the run still goes red. - Alerting is per episode: a clean run clears the marker, so a row that breaks, is fixed, and breaks again is reported both times. - The hand-off is withheld if the state push failed. That is the one way this could chain: a successor starting from the stale marker would re-alert. Not addressed: refresh.yml's own invariant step has no de-dupe and will still go red once a day while a violation persists. It is a daily job rather than the liveness path, so that is the intended signal. 8 tests drive the script through the episode semantics. Verified by mutation: disabling the de-dupe fails test_same_violation_stays_quiet, and not clearing on a clean run fails test_recurrence_after_a_clean_run_alerts_again. The unusable-path case uses a file-as-parent rather than chmod, which root ignores. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018qBS4DHn53TNPoQZXRsiPq --- .github/invariant-alert.sh | 71 ++++++++++++++++ .github/workflows/live-refresh.yml | 53 ++++++++++-- HARDENING.md | 49 ++++++++++- data/invariant_alerted.txt | 0 tests/test_invariant_alert.py | 129 +++++++++++++++++++++++++++++ 5 files changed, 293 insertions(+), 9 deletions(-) create mode 100755 .github/invariant-alert.sh create mode 100644 data/invariant_alerted.txt create mode 100644 tests/test_invariant_alert.py diff --git a/.github/invariant-alert.sh b/.github/invariant-alert.sh new file mode 100755 index 00000000..9453cf7c --- /dev/null +++ b/.github/invariant-alert.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# +# Decide whether the current invariant violations are worth alerting on. +# +# dgpt/invariants.py rewrites data/cache/invariant_violations.txt on every +# refresh (empty = clean). Its lines are stable keys with no scores in them, +# precisely so this comparison is possible: the same bad player-row seen six +# minutes later produces a byte-identical line. +# +# Exit codes, so the caller can order its publish/commit/alert steps around it: +# 0 nothing to alert — clean, or this exact violation set was already alerted +# 3 NEW violations; the caller should go red (and hand off before it dies) +# 1 the script itself could not do its job +# +# Why the alerted marker is NOT kept in data/cache/ (where it used to live): +# that directory is gitignored and carried between runs only by actions/cache, +# whose save runs in a post step — and a job that ends non-zero skips it. The +# alerting run ALWAYS ends non-zero, so the marker written by the run that +# alerts is the one guaranteed never to be saved. Observed 2026-08-29 on runs +# #1027 (MPO Sander Bahnerth cur=+981) and #1029 (FPO Samantha Zaborowski +# cur=+849): both wrote the marker, both showed "Post Restore results cache: +# skipped", and neither left anything behind for the next run to compare +# against. The de-dupe the workflow comment promised could not have worked. +# +# So the marker lives at data/invariant_alerted.txt, tracked in git alongside +# the other cross-run state the pipeline reads back (data/live_signature.txt, +# data/current_ratings.json). The caller must run this BEFORE its state commit +# so the updated marker rides along with it. +# +# A clean run clears the marker. Alerting is once per contiguous episode, not +# once per season: if a bad row disappears and later comes back, that is news +# again. +set -uo pipefail + +viol="${1:-data/cache/invariant_violations.txt}" +alerted="${2:-data/invariant_alerted.txt}" + +if ! mkdir -p "$(dirname "$alerted")" 2>/dev/null; then + echo "invariant-alert: cannot create $(dirname "$alerted")" >&2 + exit 1 +fi + +# No marker file at all means the refresh never got as far as writing one. +# That is not "clean" — it is "unknown" — but it is also not a violation to +# alert on, and the refresh failing is reported by its own path. +if [ ! -f "$viol" ]; then + echo "invariant-alert: no violations file at $viol — nothing to compare" + exit 0 +fi + +if [ ! -s "$viol" ]; then + if [ -s "$alerted" ]; then + : > "$alerted" || exit 1 + echo "invariant-alert: violations cleared; marker reset" + else + echo "invariant-alert: clean" + fi + exit 0 +fi + +if cmp -s "$viol" "$alerted"; then + echo "invariant-alert: same violations already alerted; staying quiet" + exit 0 +fi + +if ! cp "$viol" "$alerted"; then + echo "invariant-alert: could not update $alerted" >&2 + exit 1 +fi +echo "invariant-alert: NEW violations ($(wc -l < "$viol" | tr -d ' ')); marker updated" +exit 3 diff --git a/.github/workflows/live-refresh.yml b/.github/workflows/live-refresh.yml index e131e0b7..6a1ce0b6 100644 --- a/.github/workflows/live-refresh.yml +++ b/.github/workflows/live-refresh.yml @@ -200,22 +200,61 @@ jobs: fi gh api -X POST "repos/${GITHUB_REPOSITORY}/pages/builds" >/dev/null 2>&1 || true + # Publish-gate invariants: DECIDE here, ALERT after the push. + # The decision has to happen before the state commit so the + # de-dupe marker is committed with it. It used to live in + # data/cache/, which is gitignored and carried only by + # actions/cache — whose save step is skipped when the job ends + # non-zero, which the alerting run always does. So the marker + # written by the run that alerts was the one guaranteed not to + # survive, and every restart re-alerted and died again (observed + # 2026-08-29, runs #1027 and #1029). See .github/invariant-alert.sh. + alert=0 + .github/invariant-alert.sh || alert=$? + if [ "$alert" = 1 ]; then + fail_step "invariant-alert failed" + sleep 300; continue + fi + # Cross-run state only: the bundle no longer lands on main, so - # most live iterations now make no commit here at all. + # most live iterations now make no commit here at all. This is + # also what persists data/invariant_alerted.txt. git add data predictions + state_pushed=1 if git diff --cached --quiet; then echo "no state change after refresh" else git commit -m "Refresh pipeline state ($(date -u +'%F %H:%MZ'))" # a concurrent merge/weekly refresh could have moved main - git push || { git pull --rebase --autostash && git push; } + if git push || { git pull --rebase --autostash && git push; }; then + : + else + state_pushed=0 + echo "::warning::could not push pipeline state to main" + fi fi - # publish-gate invariants: alert on NEW violations, after the push - viol=data/cache/invariant_violations.txt - if [ -s "$viol" ] && ! cmp -s "$viol" data/cache/invariant_alerted.txt; then - cp "$viol" data/cache/invariant_alerted.txt + + if [ "$alert" = 3 ]; then echo "::error::Live data failed invariant checks (data still published — see dgpt/invariants.py):" - cat "$viol" + cat data/cache/invariant_violations.txt + # Hand off before dying. The run still goes red so the owner is + # notified, but the loop no longer stays down until someone + # restarts it by hand: the successor reads the marker committed + # just above, sees this violation set as already alerted, and + # keeps publishing. On 2026-08-29 two of these deaths left the + # site on hourly updates for 2h26m and 70m during round 4. + # Unbounded on purpose — a repeat cannot re-alert, so this + # cannot chain; only a genuinely new violation set alerts again. + # That guarantee rests entirely on the marker having reached + # main, so a failed state push withholds the hand-off: a + # successor starting from the old marker WOULD re-alert, and + # that is the one way this could spin. + if [ "$state_pushed" = 1 ]; then + .github/dispatch-successor.sh \ + || echo "::warning::could not hand off after an invariant alert; the cron restarts the loop" + else + echo "::warning::state push failed, so the marker is not on main — not handing off, because the successor would re-alert. The cron restarts the loop." + fi exit 1 fi fi diff --git a/HARDENING.md b/HARDENING.md index 06fc06f4..b54985ff 100644 --- a/HARDENING.md +++ b/HARDENING.md @@ -128,6 +128,29 @@ pattern across them, not any single bug, is what this backlog addresses. outage rather than a design — and a liveness failure that produces no failing run produces no alert either. +12. **The publish-gate alert took the live loop down, and could not + de-duplicate.** Two PDGA rows at Worlds carried impossible cumulative + scores on 2026-08-29 — MPO Sander Bahnerth `cur=+981` and, two hours + later, FPO Samantha Zaborowski `cur=+849`, both far outside the + per-round bounds. The checks did exactly their job: published the + data, then failed the run so the owner was notified. What nobody had + noticed is what that costs. `exit 1` ends the run, and the run is the + live loop, so each bad row also stopped the site updating — and the + de-dupe meant to keep a persistent violation from re-alerting could + never fire, because its marker lived in `data/cache/`, which is + gitignored and carried between runs only by `actions/cache`, whose + save step is skipped when the job ends non-zero. The alerting run is + always the failing run, so the marker written by the run that alerts + was precisely the one guaranteed not to survive. Every restart + re-alerted and died again. Round 4 ran on hourly updates (16:10, + 17:26, 18:38Z) instead of six-minute ones, the 17:26 refresh being the + daily cron rather than the loop. Fix: item 10's hand-off extended to + this exit, and the marker moved into tracked state so it is committed + with the rest of the cross-run state before the run dies. Lesson: an + alerting path that shares a process with the thing it monitors will + take that thing down with it, and state that only exists on the + success path cannot protect the failure path. + ## The plan Ordered by expected payoff. "In-season safe" = additive, can't change a @@ -139,8 +162,8 @@ published number. failure posture in `.github/workflows/live-refresh.yml`. Item 6 shipped 2026-08 (`.github/publish-site.sh`). Items 7 and 9 are offseason work; item 8's first two pieces (a resnapshot command, the invariant gate on -recording) are in-season safe and still open. Item 10 shipped 2026-08 -(`.github/dispatch-successor.sh`).* +recording) are in-season safe and still open. Items 10 and 11 shipped +2026-08 (`.github/dispatch-successor.sh`, `.github/invariant-alert.sh`).* ### 1. Commit the regression corpus; make it a test suite *(shipped 2026-07)* @@ -319,3 +342,25 @@ rather than hours mid-round — but it is the same dependency, and a multi-hour delivery gap across a Sunday-morning tee time would show. The fix if it bites: keep looping while a points event starts within the next several hours, instead of exiting on "nothing live right now". + +### 11. Stop the invariant alert from killing the loop *(shipped 2026-08)* + +`.github/invariant-alert.sh`, after incident 12. The alert decision moved +out of the workflow body and ahead of the state commit, so the de-dupe +marker (`data/invariant_alerted.txt`, tracked — not `data/cache/`, which +no failing run can save) is committed before the run exits. The publish +still happens first and the run still goes red; what changed is that the +loop hands off to a successor on the way out, and that successor reads +the committed marker, sees the same violation set, and stays quiet +instead of dying on it. + +The semantics are per episode, not per season: a clean run clears the +marker, so a row that breaks, gets fixed, and breaks again is reported +both times. The hand-off here is deliberately unbounded — unlike the +crash-out retry, a repeat cannot re-alert, so it cannot chain. + +Not addressed: `refresh.yml`'s own invariant step has no de-dupe and will +still go red once a day while a violation persists. That is a daily job +rather than the liveness path, so a red run there is the intended signal +and costs nothing but noise — but if a violation ever persists for a week +it will be a week of red dailies. diff --git a/data/invariant_alerted.txt b/data/invariant_alerted.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_invariant_alert.py b/tests/test_invariant_alert.py new file mode 100644 index 00000000..016ff734 --- /dev/null +++ b/tests/test_invariant_alert.py @@ -0,0 +1,129 @@ +"""The publish-gate alert decision: .github/invariant-alert.sh. + +This script is what stops a bad live row from taking the loop down every six +minutes. On 2026-08-29 two PDGA rows at Worlds carried impossible cumulative +scores — MPO Sander Bahnerth cur=+981, FPO Samantha Zaborowski cur=+849 — and +each one published, alerted, and killed its run. The de-dupe that was supposed +to make the second sighting quiet could not work: its marker lived in +data/cache/, which only actions/cache carries between runs, and that save is +skipped when the job fails. The alerting run is always the failing run. + +So the marker moved into tracked state, and the contract this file pins down +is the episode semantics: alert once when a violation set appears, stay quiet +while it persists, and alert again if it clears and comes back. +""" +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).parent.parent / ".github" / "invariant-alert.sh" + +CLEAN = 0 +NEW = 3 +BROKEN = 1 + +# Real marker lines, in the shape dgpt/invariants.py writes: stable keys with +# no scores in them, which is what makes byte-comparison a valid de-dupe. +BAHNERTH = "97344 MPO bounds cur Sander Bahnerth\n" +ZABOROWSKI = "97344 FPO bounds cur Samantha Zaborowski\n" + + +@pytest.fixture +def run(tmp_path): + viol = tmp_path / "cache" / "invariant_violations.txt" + alerted = tmp_path / "invariant_alerted.txt" + viol.parent.mkdir(parents=True) + + def go(violations: str | None): + """Write the violations file (None = the refresh wrote none) and decide.""" + if violations is None: + viol.unlink(missing_ok=True) + else: + viol.write_text(violations, encoding="utf-8") + proc = subprocess.run( + [str(SCRIPT), str(viol), str(alerted)], + capture_output=True, text=True, timeout=30, + ) + marker = alerted.read_text(encoding="utf-8") if alerted.exists() else None + return proc, marker + + return go + + +def test_clean_run_is_quiet(run): + proc, marker = run("") + assert proc.returncode == CLEAN, proc.stderr + assert "clean" in proc.stdout + + +def test_first_violation_alerts_and_records_it(run): + proc, marker = run(BAHNERTH) + assert proc.returncode == NEW, proc.stderr + assert "NEW violations" in proc.stdout + assert marker == BAHNERTH + + +def test_same_violation_stays_quiet(run): + """The whole point: a persistent bad row must not re-kill the loop.""" + assert run(BAHNERTH)[0].returncode == NEW + proc, marker = run(BAHNERTH) + assert proc.returncode == CLEAN, proc.stderr + assert "already alerted" in proc.stdout + assert marker == BAHNERTH + + +def test_a_different_violation_alerts_again(run): + """08-29 saw two distinct rows hours apart; the second is real news.""" + assert run(BAHNERTH)[0].returncode == NEW + proc, marker = run(ZABOROWSKI) + assert proc.returncode == NEW, proc.stderr + assert marker == ZABOROWSKI + + +def test_a_violation_added_alongside_an_existing_one_alerts(run): + assert run(BAHNERTH)[0].returncode == NEW + both = BAHNERTH + ZABOROWSKI + proc, marker = run(both) + assert proc.returncode == NEW, proc.stderr + assert marker == both + + +def test_recurrence_after_a_clean_run_alerts_again(run): + """Alert once per episode, not once per season. If PDGA fixes a row and + then breaks it again, the second break has to be seen.""" + assert run(BAHNERTH)[0].returncode == NEW + proc, marker = run("") + assert proc.returncode == CLEAN + assert marker == "" # cleared, so a recurrence is news + assert run(BAHNERTH)[0].returncode == NEW + + +def test_missing_violations_file_is_not_an_alert(run): + """A refresh that died before writing the marker is reported by its own + path; absence of evidence must not be published as evidence of a fault.""" + proc, _ = run(None) + assert proc.returncode == CLEAN, proc.stderr + assert "nothing to compare" in proc.stdout + + +def test_unusable_marker_path_reports_broken_rather_than_quiet(tmp_path): + """Exit 1 is distinct from 'clean' so the caller can tell a failed check + from a passing one — swallowing this would silently disable the gate. + + The unusable path is a marker whose parent is a regular file. Chmod-based + denial would not do: CI runs as a normal user but this suite is also run + as root, where the mode bits are simply ignored and the test would pass + locally for the wrong reason.""" + viol = tmp_path / "violations.txt" + viol.write_text(BAHNERTH, encoding="utf-8") + not_a_dir = tmp_path / "not_a_dir" + not_a_dir.write_text("", encoding="utf-8") + proc = subprocess.run( + [str(SCRIPT), str(viol), str(not_a_dir / "invariant_alerted.txt")], + capture_output=True, text=True, timeout=30, + ) + assert proc.returncode == BROKEN, proc.stdout + assert "cannot create" in proc.stderr