From 60fb7fb4ae7e774782ae790951af83659df49648 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Tue, 4 Aug 2026 21:57:28 +0100 Subject: [PATCH] fix(nightly): separate "blocked at a gate" from "driver broke", fix the page detail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to the nightly release driver's reporting, both found triaging eight consecutive red nights that were all the gate working correctly. Outcome contract. nightly.sh already distinguished blocked (exit 2/3) from a driver error (exit 1), but the workflow flattened every non-zero into a red run — so a night the gate correctly stopped looked identical to a broken driver, and the channel became one nobody watched. The workflow now classifies the exit code: 0/2/3 leave the job green, with a named "Blocked at a gate" step plus a ::warning:: and a job summary when it stopped; only a driver error (exit 1, the night was never judged) turns the run red. Red now means "this workflow needs a human". So a blocked night cannot become silently green, overnight_status.sh reads that step and gives it its own ⏸ line and tally, distinct from both green and failed. Page detail. The Stage-3 summary read 1 failed: database/start_here.py, None verify_install because summary.failed counts SCRIPTS while failures[] also carries non-script legs (project: null, reason: "verify_install FAILED") — so the count disagreed with its own list and the null stringified as "None". Moved the formatter out of its heredoc into stage_failure_summary.py, which reports the two kinds as separate segments, and covered it with tests built from the real stage report: 1 failed: database/start_here.py; verify_install FAILED Verified against that real 2026-08-04 report, and the blocked/not-blocked branches of overnight_status.sh control-tested both ways with a gh shim. Refs #194 Co-Authored-By: Claude Opus 5 --- .github/workflows/nightly-release.yml | 64 ++++++++- agents/conductors/release/nightly.sh | 34 ++--- .../release/stage_failure_summary.py | 94 +++++++++++++ bin/overnight_status.sh | 41 +++++- tests/test_stage_failure_summary.py | 128 ++++++++++++++++++ 5 files changed, 330 insertions(+), 31 deletions(-) create mode 100644 agents/conductors/release/stage_failure_summary.py create mode 100644 tests/test_stage_failure_summary.py diff --git a/.github/workflows/nightly-release.yml b/.github/workflows/nightly-release.yml index 8826204..36a09bd 100644 --- a/.github/workflows/nightly-release.yml +++ b/.github/workflows/nightly-release.yml @@ -21,6 +21,25 @@ name: Nightly Release # checklist steps 2-3 were consciously waived; open release-blocker issues # (e.g. PyAutoBuild#126) still stop every night at step 3 until closed or # de-labelled. Pausing is one act: unset NIGHTLY_RELEASES. +# +# OUTCOME CONTRACT (2026-08-04). The driver's exit codes already distinguished +# "a gate stopped the night" from "the driver broke", but this workflow flattened +# every non-zero into a red run — so eight consecutive nights of the gate working +# correctly looked identical to a broken driver, and the channel became one +# nobody watched. The mapping is now explicit: +# +# exit 0 shipped / skipped / dry-run -> job SUCCESS +# exit 2|3 blocked at a gate, no release -> job SUCCESS + ::warning:: + the +# "Blocked at a gate" step below +# exit 1|* driver error, night NOT judged -> job FAILURE +# +# Red is therefore reserved for "the driver itself is broken" — the one state +# that needs a human to look at THIS workflow. A blocked night is a normal, +# expected outcome: Slack carries which gate stopped it, and the run keeps a +# named step + job summary so it is never silently green. +# +# `bin/overnight_status.sh` reads that step name to report a blocked night +# distinctly in the morning glance — keep the step name in sync if it changes. on: schedule: @@ -68,6 +87,7 @@ jobs: run: pip install --quiet pyyaml - name: Run the nightly driver + id: driver env: GH_TOKEN: ${{ secrets.PAT_PYAUTOLABS }} PYAUTO_RELEASE_WEBHOOK_URL: ${{ secrets.PYAUTO_RELEASE_WEBHOOK_URL }} @@ -76,4 +96,46 @@ jobs: # (design §9; the kill switch is the NIGHTLY_RELEASES repo var). DRY_RUN: ${{ inputs.dry_run || 'false' }} PYAUTO_ROOT: ${{ github.workspace }} - run: bash agents/conductors/release/nightly.sh + # `set +e` first: the default shell is `bash -e`, which would abort on + # the driver's exit code before it can be classified (see OUTCOME + # CONTRACT above). This step never fails — the two steps below decide. + run: | + set +e + bash agents/conductors/release/nightly.sh + rc=$? + set -e + echo "rc=$rc" >> "$GITHUB_OUTPUT" + case "$rc" in + 0) echo "outcome=reported" >> "$GITHUB_OUTPUT" ;; + 2|3) echo "outcome=blocked" >> "$GITHUB_OUTPUT" ;; + *) echo "outcome=driver-error" >> "$GITHUB_OUTPUT" ;; + esac + + # Named, not just annotated: overnight_status.sh keys the morning glance + # off this step, so a blocked night is never reported as a plain green. + - name: Blocked at a gate — no release was made + if: steps.driver.outputs.outcome == 'blocked' + run: | + echo "::warning title=Nightly release blocked::The driver stopped at a gate (exit ${{ steps.driver.outputs.rc }}); no release was made. This is the gate working — Slack carries which one." + { + echo "## ⏸ Blocked at a gate — no release was made" + echo + echo "The driver stopped deliberately (exit \`${{ steps.driver.outputs.rc }}\`):" + echo "\`2\` = a gate blocked the night, \`3\` = readiness was not GREEN." + echo + echo "This is the gate doing its job, not a driver fault, so the run is" + echo "green. The Slack page names which gate stopped it." + } >> "$GITHUB_STEP_SUMMARY" + + - name: Driver error — the night was NOT judged + if: steps.driver.outputs.outcome == 'driver-error' + run: | + echo "::error title=Nightly driver error::The driver failed with exit ${{ steps.driver.outputs.rc }} — the night was NOT judged and no gate verdict exists." + { + echo "## 🚨 Driver error — the night was NOT judged" + echo + echo "\`nightly.sh\` exited \`${{ steps.driver.outputs.rc }}\`, which is not a gate" + echo "outcome. No release was made AND no gate verdict was reached —" + echo "this workflow needs a human." + } >> "$GITHUB_STEP_SUMMARY" + exit 1 diff --git a/agents/conductors/release/nightly.sh b/agents/conductors/release/nightly.sh index b905a89..fd162f4 100644 --- a/agents/conductors/release/nightly.sh +++ b/agents/conductors/release/nightly.sh @@ -49,6 +49,12 @@ # # Exit: 0 on a reported outcome (shipped / skipped / dry-run) — 2 blocked at a # gate (paged) — 3 not GREEN / preflight red (paged) — 1 driver error (paged). +# +# These codes are a CONTRACT, not just a status: .github/workflows/ +# nightly-release.yml maps 0/2/3 to a green run (a blocked night is the gate +# working) and 1 to a red one (the driver itself broke, and the night was never +# judged). Changing what a code means changes when a human is alarmed — keep the +# workflow's OUTCOME CONTRACT block in step. set -uo pipefail @@ -373,31 +379,9 @@ s3_artifact="$(plan_field "$phase_b" "[s for s in plan['steps'] if s['step']=='d if ! dispatch_and_await "$s3_repo" "$s3_wf" "$s3_inputs" "$s3_artifact" "$ART_DIR"; then # Name the failing scripts straight from the downloaded stage report so the # page is actionable without opening the run (no report → plain page). - detail="$(python3 - "$ART_DIR/stage_report.json" <<'PY' 2>/dev/null -import json, sys - -try: - r = json.load(open(sys.argv[1])) -except Exception: - sys.exit(0) -s = r.get("summary") or {} -parts = [ - f"{int(s.get(k, 0) or 0)} {k}" for k in ("failed", "timeout") if int(s.get(k, 0) or 0) -] -names = [] -for f in r.get("failures") or []: - tail = "/".join(str(f.get("script") or "").rstrip("/").split("/")[-2:]) - names.append(f"{f.get('project')} {tail}") -if not parts and not names: - sys.exit(0) -line = ", ".join(parts) if parts else "failures" -if names: - line += ": " + ", ".join(names[:3]) - if len(names) > 3: - line += f", +{len(names) - 3} more" -print(line) -PY -)" + # Lives in its own file, not a heredoc, so it carries a regression test: + # script failures and non-script legs (verify_install) count differently. + detail="$(python3 "$HERE/stage_failure_summary.py" "$ART_DIR/stage_report.json" 2>/dev/null)" page "Stage 3 (release-fidelity integration) failed — <${LAST_RUN_URL:-$RUN_URL}|run>${detail:+ $detail}" exit 2 diff --git a/agents/conductors/release/stage_failure_summary.py b/agents/conductors/release/stage_failure_summary.py new file mode 100644 index 0000000..78adeda --- /dev/null +++ b/agents/conductors/release/stage_failure_summary.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python3 +"""One line naming what failed in a validation stage report, for the page text. + +`nightly.sh` pages Slack when a stage fails. The page is far more useful if it +names the failures, so this reads the stage report the run produced and prints a +single line; printing nothing (exit 0) means "no report, or nothing nameable" and +the caller pages without detail. + +The report separates two kinds of failure, and conflating them is what made the +old inline version misread: + +* ``summary`` counts SCRIPTS — ``{"failed": 1, "passed": 654, "timeout": 0, …}``. +* ``failures`` lists script failures (each with a ``project``) AND non-script + legs, which carry ``project: null`` and a ``reason`` — e.g. + ``{"project": null, "script": "verify_install", "reason": "verify_install FAILED"}``. + +A non-script leg is NOT in ``summary.failed``, so listing it beside the scripts +produced "1 failed: " — a count that reads as a bug in the reporter +— and ``f"{f['project']} …"`` stringified the null as a literal "None" +("None verify_install", 2026-08-04). Both kinds are worth paging, so neither is +dropped; they are reported as separate segments instead. + +Usage: stage_failure_summary.py +""" + +from __future__ import annotations + +import json +import sys + +# Long pages get truncated by chat clients; name enough to act on, then count. +MAX_NAMED = 3 + + +def summarise(report: dict) -> str: + """The page's detail line — '' when the report names nothing useful.""" + summary = report.get("summary") or {} + counts = [] + for key in ("failed", "timeout"): + try: + value = int(summary.get(key, 0) or 0) + except (TypeError, ValueError): + continue # a malformed count must not cost us the whole page + if value: + counts.append(f"{value} {key}") + + scripts: list[str] = [] + checks: list[str] = [] + for failure in report.get("failures") or []: + if not isinstance(failure, dict): + continue + project = failure.get("project") + script = str(failure.get("script") or "").rstrip("/") + if project: + # The tail is enough to identify a script; the full runner path is + # noise in a chat message. + tail = "/".join(script.split("/")[-2:]) + scripts.append(f"{project} {tail}".strip()) + else: + checks.append(str(failure.get("reason") or script or "unnamed check")) + + segments = [] + if counts or scripts: + head = ", ".join(counts) if counts else "failures" + if scripts: + head += ": " + ", ".join(scripts[:MAX_NAMED]) + if len(scripts) > MAX_NAMED: + head += f", +{len(scripts) - MAX_NAMED} more" + segments.append(head) + segments.extend(checks[:MAX_NAMED]) + if len(checks) > MAX_NAMED: + segments.append(f"+{len(checks) - MAX_NAMED} more checks") + + return "; ".join(segments) + + +def main(argv: list[str]) -> int: + if len(argv) != 2: + return 0 # no path given: the caller pages without detail + try: + with open(argv[1]) as handle: + report = json.load(handle) + except (OSError, ValueError): + return 0 # absent or malformed report is not itself a paging failure + if not isinstance(report, dict): + return 0 + line = summarise(report) + if line: + print(line) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/bin/overnight_status.sh b/bin/overnight_status.sh index e0af048..e008cc0 100755 --- a/bin/overnight_status.sh +++ b/bin/overnight_status.sh @@ -5,6 +5,14 @@ # CLI, on mobile Claude Code chat, and in Codex alike. # # Output: one line per job — icon owner/repo/workflow conclusion (age). +# +# A green run does not always mean "nothing to see". The nightly release driver +# deliberately renders a night it BLOCKED at a gate as a successful run (red is +# reserved for a broken driver — see PyAutoBrain/.github/workflows/ +# nightly-release.yml, OUTCOME CONTRACT), so a blocked night would otherwise be +# indistinguishable here from a night that shipped. Any workflow that names a +# step with $BLOCKED_STEP_PREFIX gets its own ⏸ line: not green, not a failure, +# but something a human should read. set -u command -v gh >/dev/null 2>&1 || { echo "gh not found — cannot fetch run status" >&2; exit 1; } @@ -30,25 +38,48 @@ age() { # ISO8601 -> "Nh" (<48h) or "Nd" ago if [ "$diff" -lt 48 ]; then echo "${diff}h"; else echo "$(( diff / 24 ))d"; fi } +# A successful run carrying a step with this name prefix stopped on purpose and +# made no change. Keep in sync with the step name in nightly-release.yml. +BLOCKED_STEP_PREFIX="Blocked at a gate" + fails=0 +blocked=0 for job in "${JOBS[@]}"; do repo="${job%%:*}"; wf="${job##*:}" [[ "$repo" == */* ]] || repo="PyAutoLabs/$repo" - read -r concl created < <(gh api "repos/$repo/actions/workflows/$wf/runs?per_page=1" \ - -q '.workflow_runs[0] | "\(.conclusion // .status) \(.created_at)"' 2>/dev/null) + read -r concl created run_id < <(gh api "repos/$repo/actions/workflows/$wf/runs?per_page=1" \ + -q '.workflow_runs[0] | "\(.conclusion // .status) \(.created_at) \(.id)"' 2>/dev/null) # No runs yet: workflow_runs[0] is null, so jq emits "null null" and # read leaves concl="null" (created gets the second "null"). if [ -z "${concl:-}" ] || [ "$concl" = "null" ]; then printf ' – %-42s no runs\n' "$repo/$wf" continue fi + # A green run may still have stopped on purpose; ask the run's steps before + # calling it clean. Only on success — a red run is already reported as red. + if [ "$concl" = "success" ] && [ -n "${run_id:-}" ] && [ "$run_id" != "null" ]; then + hits=$(gh api "repos/$repo/actions/runs/$run_id/jobs" \ + -q "[.jobs[].steps[]? | select(.conclusion == \"success\") + | select(.name | startswith(\"$BLOCKED_STEP_PREFIX\"))] | length" 2>/dev/null) + if [ "${hits:-0}" != "0" ] && [ -n "${hits:-}" ]; then + blocked=$((blocked+1)) + printf ' ⏸ %-42s blocked — no release made (%s)\n' "$repo/$wf" "$(age "$created")" + continue + fi + fi if [ "$concl" = "success" ]; then icon="✓"; else icon="✗"; fails=$((fails+1)); fi printf ' %s %-42s %s (%s)\n' "$icon" "$repo/$wf" "$concl" "$(age "$created")" done echo -if [ "$fails" -eq 0 ]; then +if [ "$fails" -eq 0 ] && [ "$blocked" -eq 0 ]; then echo "Overnight: all scheduled jobs green." -else - echo "Overnight: $fails job(s) not green — see above (e.g. a blocked nightly-release)." +fi +# Separate `if`s, not `[ … ] && echo`: a false test as the script's last command +# would make it exit non-zero and read as a tool failure to /wake_up. +if [ "$fails" -gt 0 ]; then + echo "Overnight: $fails job(s) not green — see above." +fi +if [ "$blocked" -gt 0 ]; then + echo "Overnight: $blocked job(s) blocked at a gate — ran correctly, made no change; the reason is in the run summary / Slack." fi diff --git a/tests/test_stage_failure_summary.py b/tests/test_stage_failure_summary.py new file mode 100644 index 0000000..5b20354 --- /dev/null +++ b/tests/test_stage_failure_summary.py @@ -0,0 +1,128 @@ +"""The nightly page's stage-failure detail line. + +Shapes here mirror a REAL `stage_report.json` pulled from the 2026-08-04 +release-fidelity run: `summary` counts scripts only, while `failures` also +carries non-script legs with `project: null` + a `reason`. The old inline +formatter mixed the two and printed + + 1 failed: database/start_here.py, None verify_install + +— a count that disagrees with its own list, and a stringified null. +""" + +import sys +from pathlib import Path + +BRAIN_HOME = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(BRAIN_HOME / "agents" / "conductors" / "release")) + +import stage_failure_summary as sfs # noqa: E402 + + +def _report(**over): + report = { + "summary": {"failed": 1, "passed": 654, "skipped": 102, "timeout": 0}, + "failures": [ + { + "project": "libx", + "script": "/runner/work/checkout/workspace/scripts/guides/results/database/start_here.py", + }, + { + "project": None, + "script": "verify_install", + "reason": "verify_install FAILED", + }, + ], + } + report.update(over) + return report + + +def test_the_two_regressions_are_gone(): + line = sfs.summarise(_report()) + + # No stringified null, and the leg is named by its reason. + assert "None" not in line + assert "verify_install FAILED" in line + # The script count still describes only the scripts it introduces. + assert line.startswith("1 failed: libx database/start_here.py") + # The leg is a separate segment, so "1 failed" is not read as covering it. + assert line == "1 failed: libx database/start_here.py; verify_install FAILED" + + +def test_script_paths_are_shortened_to_their_tail(): + line = sfs.summarise(_report(failures=[ + {"project": "libx", "script": "/a/very/long/runner/path/imaging/modeling.py"}, + ])) + assert line == "1 failed: libx imaging/modeling.py" + + +def test_a_check_only_failure_still_reports(): + # No scripts failed (summary all zero) but a leg did — the page must not be + # blank, or the night looks unexplained. + line = sfs.summarise({ + "summary": {"failed": 0, "timeout": 0}, + "failures": [{"project": None, "script": "verify_install", + "reason": "verify_install FAILED"}], + }) + assert line == "verify_install FAILED" + + +def test_a_leg_without_a_reason_falls_back_to_its_name(): + line = sfs.summarise({ + "summary": {}, + "failures": [{"project": None, "script": "some_check"}], + }) + assert line == "some_check" + + +def test_long_lists_are_truncated_with_a_count(): + failures = [ + {"project": "libx", "script": f"dir/script_{i}.py"} for i in range(5) + ] + line = sfs.summarise({"summary": {"failed": 5}, "failures": failures}) + assert line.startswith("5 failed: libx dir/script_0.py") + assert line.endswith("+2 more") + assert "script_3.py" not in line + + +def test_timeouts_are_named_alongside_failures(): + line = sfs.summarise({"summary": {"failed": 1, "timeout": 2}, "failures": []}) + assert line == "1 failed, 2 timeout" + + +def test_nothing_nameable_is_an_empty_line_not_an_error(): + assert sfs.summarise({}) == "" + assert sfs.summarise({"summary": {"failed": 0}, "failures": []}) == "" + + +def test_malformed_report_does_not_raise(): + # A garbled count or a non-dict failure entry must not cost the whole page. + line = sfs.summarise({ + "summary": {"failed": "not-a-number", "timeout": None}, + "failures": ["junk", {"project": "libx", "script": "d/s.py"}], + }) + assert line == "failures: libx d/s.py" + + +def test_missing_file_prints_nothing_and_exits_zero(tmp_path, capsys): + assert sfs.main(["prog", str(tmp_path / "absent.json")]) == 0 + assert capsys.readouterr().out == "" + + +def test_malformed_file_prints_nothing_and_exits_zero(tmp_path, capsys): + bad = tmp_path / "stage_report.json" + bad.write_text("{not json") + assert sfs.main(["prog", str(bad)]) == 0 + assert capsys.readouterr().out == "" + + +def test_main_prints_the_line_for_a_real_shaped_report(tmp_path, capsys): + import json + + path = tmp_path / "stage_report.json" + path.write_text(json.dumps(_report())) + assert sfs.main(["prog", str(path)]) == 0 + assert capsys.readouterr().out.strip() == ( + "1 failed: libx database/start_here.py; verify_install FAILED" + )