diff --git a/heart/checks/ci_status.py b/heart/checks/ci_status.py index 10f3fc9..8c5e963 100644 --- a/heart/checks/ci_status.py +++ b/heart/checks/ci_status.py @@ -8,6 +8,20 @@ line. Keeping it in Python (not inlined ``python3 -c`` in bash) makes the gating logic unit-testable in isolation. +Two input shapes are accepted (see ``normalize_runs``): the REST +``/actions/runs`` payload that ``ci_status.sh`` now sends, and the older bare +list from ``gh run list --json``. The shell fetches over ``gh api`` rather than +``gh run list`` because ``gh run list``'s flags and ``--json`` field names moved +between ``gh`` releases (``--branch`` does not exist before gh 2.9), whereas the +REST endpoint is stable across every ``gh`` version that has ``gh api``. + +A *failed fetch* is not a *pending CI*. When the shell cannot retrieve runs it +passes ``--fetch-error``, and the sidecar records ``status="unavailable"`` plus +an ``error`` string instead of silently looking like an in-progress run. This +distinction matters because the release gate leans on this evidence: before it +existed, a broken ``gh`` call rendered identically to "CI still running" on +every repo at once, so a genuine red and a dead query were indistinguishable. + Why per-workflow, not "the newest run": each workspace gates on several workflows (e.g. ``Smoke Tests`` + ``Navigator Check``), each a matrix over two Pythons. ``gh run list --limit 1`` returns one run of *some* workflow, so it can @@ -24,6 +38,7 @@ "required": ["Smoke Tests", "Navigator Check"], "conclusion": "failure", # rolled-up over required workflows (back-compat) "status": "completed", # rolled-up status (back-compat) + "error": "", # non-empty => the runs fetch itself failed "sha": "abc1234", # short HEAD sha (back-compat with old sidecar) "workflow": "Smoke Tests", # the failing/representative workflow, for the summary "url": "https://github.com/.../actions/runs/123", @@ -89,6 +104,58 @@ def required_for(group: str, config_path: Path | str = CONFIG_PATH) -> list[str] return load_required_workflows(config_path).get(group, []) +def normalize_runs(payload: Any, branch: str = "main") -> list[dict[str, Any]]: + """Coerce a runs payload into the internal run shape, filtered to ``branch``. + + Accepts either shape: + + - the REST ``GET /repos/{owner}/{repo}/actions/runs`` object, i.e. + ``{"workflow_runs": [...]}`` with snake_case keys, which is what + ``ci_status.sh`` sends; or + - a bare list of camelCase runs, the historical ``gh run list --json`` + output (still accepted so older cached payloads and the existing tests + keep working). + + REST runs carry the workflow's display name in ``name`` (the commit subject + lives in ``display_title``), which is exactly the key ``required_workflows`` + is written against. Runs whose ``head_branch`` is known and is not + ``branch`` are dropped — the REST query already filters by branch, so this + is belt-and-braces for the legacy shape, which was never branch-filtered on + a ``gh`` too old to support ``--branch``. + """ + if isinstance(payload, dict): + raw = payload.get("workflow_runs") or [] + elif isinstance(payload, list): + raw = payload + else: + raw = [] + + runs: list[dict[str, Any]] = [] + for run in raw: + if not isinstance(run, dict): + continue + if "workflow_runs" in run: # guard against a doubly-wrapped payload + continue + # REST shape is detected by its snake_case keys; camelCase wins when + # present so an already-normalized run passes through untouched. + head_branch = run.get("headBranch", run.get("head_branch")) + if head_branch and branch and head_branch != branch: + continue + runs.append( + { + "workflowName": run.get("workflowName") or run.get("name") or "", + "conclusion": run.get("conclusion") or "", + "status": run.get("status") or "", + "headSha": run.get("headSha") or run.get("head_sha") or "", + "createdAt": run.get("createdAt") or run.get("created_at") or "", + "url": run.get("url") or run.get("html_url") or "", + "event": run.get("event") or "", + "headBranch": head_branch or "", + } + ) + return runs + + def latest_per_workflow(runs: list[dict[str, Any]]) -> dict[str, dict[str, Any]]: """Collapse ``gh run list`` output to the newest run of each workflow on main. @@ -174,17 +241,27 @@ def rollup(workflows: dict[str, dict[str, Any]], required: list[str]) -> dict[st def build_sidecar( name: str, group: str, - runs: list[dict[str, Any]], + runs: Any, head_sha: str, ts: str, config_path: Path | str = CONFIG_PATH, + error: str = "", ) -> dict[str, Any]: - """Construct the full ci_status sidecar dict for one repo.""" + """Construct the full ci_status sidecar dict for one repo. + + ``error`` is the reason the runs fetch failed. When set, the sidecar + reports ``status="unavailable"`` rather than the ``in_progress`` that an + empty run list would otherwise roll up to, so "we could not ask" is + readable as itself instead of impersonating "CI is still running". + """ required = required_for(group, config_path) - latest = latest_per_workflow(runs) + latest = latest_per_workflow(normalize_runs(runs)) workflows = {wf: _wf_entry(run, head_sha) for wf, run in latest.items()} - roll = rollup(workflows, required) + if error: + roll = {"conclusion": "", "status": "unavailable", "workflow": ""} + else: + roll = rollup(workflows, required) # Pick a representative url: the failing workflow's, else HEAD's newest. rep_wf = roll["workflow"] rep = workflows.get(rep_wf) if rep_wf else None @@ -200,6 +277,7 @@ def build_sidecar( "conclusion": roll["conclusion"], "status": roll["status"], "workflow": roll["workflow"], + "error": error, "url": (rep or {}).get("url", ""), "workflows": workflows, "ts": ts, @@ -218,7 +296,10 @@ def summary_line(sidecar: dict[str, Any]) -> str: status = sidecar.get("status", "") sha = sidecar.get("sha", "") workflows = sidecar.get("workflows") or {} + error = sidecar.get("error", "") + if error: + return f"{glyph_warn()} {c_info(name)} {c_warn('CI UNAVAILABLE')} {c_meta(error)}" if not workflows and not sha: return f"{c_meta('·')} {c_info(name)} {c_meta('(no runs)')}" if conclusion == "success": @@ -234,13 +315,14 @@ def summary_line(sidecar: dict[str, Any]) -> str: def write_and_summarise( name: str, group: str, - runs: list[dict[str, Any]], + runs: Any, head_sha: str, ts: str, out_path: Path, config_path: Path | str = CONFIG_PATH, + error: str = "", ) -> dict[str, Any]: - sidecar = build_sidecar(name, group, runs, head_sha, ts, config_path) + sidecar = build_sidecar(name, group, runs, head_sha, ts, config_path, error) sys.path.insert(0, str(HEART_HOME)) from heart import state @@ -255,17 +337,24 @@ def main(argv: list[str] | None = None) -> int: ap.add_argument("--head-sha", default="") ap.add_argument("--ts", required=True) ap.add_argument("--out", required=True) + ap.add_argument( + "--fetch-error", + default="", + help="reason the runs fetch failed; recorded instead of a bogus pending state", + ) ns = ap.parse_args(argv) + error = ns.fetch_error try: - runs = json.load(sys.stdin) + runs: Any = json.load(sys.stdin) except (json.JSONDecodeError, ValueError): + # Unparseable stdin is itself a fetch failure: report it as one rather + # than falling through to an empty run list that reads as "pending". runs = [] - if not isinstance(runs, list): - runs = [] + error = error or "runs payload was not valid JSON" sidecar = write_and_summarise( - ns.name, ns.group, runs, ns.head_sha, ns.ts, Path(ns.out) + ns.name, ns.group, runs, ns.head_sha, ns.ts, Path(ns.out), error=error ) print(summary_line(sidecar)) return 0 diff --git a/heart/checks/ci_status.sh b/heart/checks/ci_status.sh index 922a401..a945cbf 100755 --- a/heart/checks/ci_status.sh +++ b/heart/checks/ci_status.sh @@ -1,9 +1,9 @@ #!/usr/bin/env bash # heart/checks/ci_status.sh — per-required-workflow CI conclusions on main HEAD. # -# For each polled repo this fetches, via `gh` (cheap metadata reads only): +# For each polled repo this fetches, via `gh api` (cheap metadata reads only): # 1. the `main` HEAD commit sha (`gh api .../commits/main`) -# 2. the recent workflow runs on `main` (`gh run list --branch main`) +# 2. the recent workflow runs on `main` (`gh api .../actions/runs?branch=main`) # and pipes the runs JSON to `heart.checks.ci_status`, which picks the latest # run of each workflow, rolls the *required* workflows for the repo's group # (config/repos.yaml `required_workflows`) into one conclusion, writes the @@ -15,29 +15,50 @@ # heavier per-workflow detail is still just metadata — two cheap `gh` calls per # repo, run in parallel — so the <30s tick budget holds. # -# If `gh` is unavailable or a repo has no runs, the sidecar is written with an -# empty conclusion (dashboard shows "(no runs)"); the continuous tick degrades +# A repo with no runs is written with an empty conclusion (dashboard shows +# "(no runs)"). A *failed fetch* is different and is reported as such: the +# sidecar records `status="unavailable"` plus the `gh` stderr, so a broken query +# never masquerades as "CI still in progress". Either way the tick degrades # gracefully rather than failing. +# +# The runs are read with `gh api` rather than `gh run list` on purpose. +# `gh run list`'s surface has moved between releases — `--branch` only exists +# from gh 2.9, and the `--json` field names changed (`workflowName` is newer +# than `name`) — so on an older `gh` the whole call exited non-zero, the old +# `|| echo '[]'` swallowed it, and every repo silently rendered as "CI +# in_progress". `gh api` + the REST endpoint is stable across every `gh` that +# has `gh api` at all, which is the same call already used for the HEAD sha. set -u source "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")/../_common.sh" -# Fields the Python roll-up needs from each run. -_CI_RUN_FIELDS="workflowName,name,conclusion,status,headSha,createdAt,url,event" +# Recent runs on main, newest first. `per_page=30` mirrors the old --limit 30. +_CI_RUNS_PATH="actions/runs?branch=main&per_page=30&exclude_pull_requests=true" check_one_repo_ci() { local owner_name="$1" local group="$2" local name="${owner_name##*/}" - local runs head_sha ts - runs="$(gh run list --repo "$owner_name" --branch main --limit 30 \ - --json "$_CI_RUN_FIELDS" 2>/dev/null || echo '[]')" + local runs head_sha ts err rc + err="$(mktemp)" + runs="$(gh api "repos/$owner_name/$_CI_RUNS_PATH" 2>"$err")" + rc=$? head_sha="$(gh api "repos/$owner_name/commits/main" --jq '.sha' 2>/dev/null || echo '')" ts="$(date -Iseconds)" + local fetch_error="" + if [[ $rc -ne 0 || -z "$runs" ]]; then + # Collapse gh's stderr to one line so it fits the sidecar and the log. + fetch_error="$(tr '\n' ' ' <"$err" | cut -c1-200)" + [[ -z "${fetch_error// }" ]] && fetch_error="gh api exited $rc" + runs='{}' + fi + rm -f "$err" + printf '%s' "$runs" | PYTHONPATH="$HEART_HOME" python3 -m heart.checks.ci_status \ --name "$name" --group "$group" --head-sha "$head_sha" --ts "$ts" \ + --fetch-error "$fetch_error" \ --out "$HEART_PER_REPO_DIR/$name.ci_status.json" } diff --git a/heart/dashboard.py b/heart/dashboard.py index bebafef..3179426 100644 --- a/heart/dashboard.py +++ b/heart/dashboard.py @@ -222,6 +222,11 @@ def _ci_fragment(ci: dict) -> tuple[str, str] | None: """(state, text) for a repo's CI, or None when there is no CI signal.""" if not ci: return None + # A failed CI query is not a CI state: say so, rather than letting it read + # as a pending run. Without this the panel showed "CI in_progress" on every + # repo at once when the underlying `gh` call was simply broken. + if ci.get("error"): + return WARN, "CI unavailable (query failed)" concl = ci.get("conclusion") if concl == "success": return OK, "CI ✓" diff --git a/heart/readiness.py b/heart/readiness.py index 751b2f6..2dea3e9 100644 --- a/heart/readiness.py +++ b/heart/readiness.py @@ -274,7 +274,13 @@ def scope_local(msg: str, key: str) -> None: continue ci = body.get("ci_status", {}) or {} conclusion = ci.get("conclusion") - if conclusion not in (None, "", "success"): + if ci.get("error"): + # The CI query failed, so we have no evidence either way. That is a + # STALE "unknown", never a pass — and it must be *said*, because a + # silent unknown here is indistinguishable from a healthy repo. + stale.append(f"{lib}: CI status unavailable ({ci['error']})") + hit("lib_ci_unavailable") + elif conclusion not in (None, "", "success"): red.append(f"{lib}: CI {conclusion}") hit("lib_ci") rs = body.get("repo_state", {}) or {} diff --git a/tests/test_ci_status.py b/tests/test_ci_status.py index a25223c..602ee8a 100644 --- a/tests/test_ci_status.py +++ b/tests/test_ci_status.py @@ -166,3 +166,153 @@ def test_main_writes_sidecar(tmp_path, monkeypatch, capsys): side = json.loads(out.read_text()) assert side["conclusion"] == "failure" assert "FAILURE" in capsys.readouterr().out + + +# --- normalize_runs: REST payload + legacy gh shape ------------------------ + +def _rest_run(workflow, conclusion, status="completed", sha=HEAD, + created="2026-06-29T00:00:00Z", event="push", branch="main"): + """A run in the REST /actions/runs shape (snake_case). + + Mirrors the real payload: `name` is the workflow's display name and the + commit subject lives in `display_title` — the opposite of newer `gh run + list --json`, where `name` is the commit subject. + """ + return { + "name": workflow, "display_title": "some commit subject", + "conclusion": conclusion, "status": status, "head_sha": sha, + "head_branch": branch, "created_at": created, "event": event, + "html_url": "u", "path": ".github/workflows/x.yml", + } + + +def test_normalize_runs_reads_rest_payload(): + payload = {"workflow_runs": [_rest_run("Smoke Tests", "failure")]} + runs = ci.normalize_runs(payload) + assert len(runs) == 1 + # The workflow display name, not the commit subject. + assert runs[0]["workflowName"] == "Smoke Tests" + assert runs[0]["conclusion"] == "failure" + assert runs[0]["headSha"] == HEAD + assert runs[0]["url"] == "u" + + +def test_normalize_runs_still_accepts_legacy_gh_list(): + """The old `gh run list --json` shape must keep working unchanged.""" + runs = ci.normalize_runs([_run("Tests", "success")]) + assert runs[0]["workflowName"] == "Tests" + assert runs[0]["conclusion"] == "success" + + +def test_normalize_runs_filters_other_branches(): + payload = {"workflow_runs": [ + _rest_run("Tests", "failure", branch="feature/x"), + _rest_run("Tests", "success", branch="main"), + ]} + runs = ci.normalize_runs(payload) + assert len(runs) == 1 + assert runs[0]["conclusion"] == "success" + + +def test_normalize_runs_handles_garbage(): + assert ci.normalize_runs(None) == [] + assert ci.normalize_runs({}) == [] + assert ci.normalize_runs({"workflow_runs": [None, "x"]}) == [] + + +def test_build_sidecar_accepts_rest_payload(tmp_path): + cfg = _cfg(tmp_path) + payload = {"workflow_runs": [_rest_run("Tests", "success")]} + side = ci.build_sidecar("PyAutoFit", "libraries", payload, HEAD, "T", config_path=cfg) + assert side["conclusion"] == "success" + + +# --- fetch failure is not a pending run ------------------------------------ + +def test_fetch_error_is_unavailable_not_in_progress(tmp_path): + """The regression this guards: a broken `gh` call used to be indistinguishable + from CI genuinely still running, on every repo at once.""" + cfg = _cfg(tmp_path) + side = ci.build_sidecar( + "PyAutoLens", "libraries", {}, HEAD, "T", config_path=cfg, + error="unknown flag: --branch", + ) + assert side["status"] == "unavailable" + assert side["status"] != "in_progress" + assert side["conclusion"] == "" # never green + assert side["error"] == "unknown flag: --branch" + + +def test_fetch_error_never_reports_success(tmp_path): + """Even if stale runs were somehow present, an errored fetch cannot be green.""" + cfg = _cfg(tmp_path) + side = ci.build_sidecar( + "PyAutoFit", "libraries", {"workflow_runs": [_rest_run("Tests", "success")]}, + HEAD, "T", config_path=cfg, error="boom", + ) + assert side["conclusion"] != "success" + assert side["status"] == "unavailable" + + +def test_no_error_key_is_empty_when_fetch_succeeds(tmp_path): + cfg = _cfg(tmp_path) + side = ci.build_sidecar("PyAutoFit", "libraries", [], HEAD, "T", config_path=cfg) + assert side["error"] == "" + + +def test_summary_line_flags_unavailable(monkeypatch): + monkeypatch.setenv("NO_COLOR", "1") + line = ci.summary_line({ + "name": "PyAutoLens", "conclusion": "", "status": "unavailable", + "sha": "abc1234", "workflows": {}, "error": "unknown flag: --branch", + }) + assert "UNAVAILABLE" in line + assert "in_progress" not in line + + +def test_main_records_fetch_error(tmp_path, monkeypatch, capsys): + monkeypatch.setenv("HEART_STATE_DIR", str(tmp_path)) + monkeypatch.setenv("NO_COLOR", "1") + out = tmp_path / "PyAutoLens.ci_status.json" + monkeypatch.setattr("sys.stdin", __import__("io").StringIO("{}")) + rc = ci.main(["--name", "PyAutoLens", "--group", "libraries", + "--head-sha", HEAD, "--ts", "T", "--out", str(out), + "--fetch-error", "unknown flag: --branch"]) + assert rc == 0 + side = json.loads(out.read_text()) + assert side["status"] == "unavailable" + assert side["error"] == "unknown flag: --branch" + assert "UNAVAILABLE" in capsys.readouterr().out + + +def test_main_treats_unparseable_stdin_as_fetch_error(tmp_path, monkeypatch, capsys): + monkeypatch.setenv("HEART_STATE_DIR", str(tmp_path)) + monkeypatch.setenv("NO_COLOR", "1") + out = tmp_path / "PyAutoFit.ci_status.json" + monkeypatch.setattr("sys.stdin", __import__("io").StringIO("not json at all")) + rc = ci.main(["--name", "PyAutoFit", "--group", "libraries", + "--head-sha", HEAD, "--ts", "T", "--out", str(out)]) + assert rc == 0 + side = json.loads(out.read_text()) + assert side["status"] == "unavailable" + assert side["error"] + capsys.readouterr() + + +def test_main_reads_real_rest_payload(tmp_path, monkeypatch, capsys): + """End-to-end over the exact shape `gh api .../actions/runs` returns.""" + monkeypatch.setenv("HEART_STATE_DIR", str(tmp_path)) + monkeypatch.setenv("NO_COLOR", "1") + out = tmp_path / "autolens_workspace.ci_status.json" + payload = json.dumps({"total_count": 2, "workflow_runs": [ + _rest_run("Smoke Tests", "failure"), + _rest_run("Navigator Check", "success"), + ]}) + monkeypatch.setattr("sys.stdin", __import__("io").StringIO(payload)) + rc = ci.main(["--name", "autolens_workspace", "--group", "workspaces", + "--head-sha", HEAD, "--ts", "T", "--out", str(out)]) + assert rc == 0 + side = json.loads(out.read_text()) + assert side["conclusion"] == "failure" + assert side["workflow"] == "Smoke Tests" + assert "FAILURE" in capsys.readouterr().out diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 51e2cb6..0b9e7eb 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -397,3 +397,27 @@ def test_malformed_stage_entry_does_not_break_the_board(): board = dashboard.build_board(make_snapshot(validation_report=vr), make_verdict("red", 45), now=FRESH_NOW) assert _section(board, "release_validation").state == dashboard.FAIL + + +# --- CI query failure is reported as itself, not as a pending run ---------- + +def test_ci_fragment_flags_failed_query_distinctly(): + """Regression: a broken `gh` call rendered as "CI in_progress" on every + repo at once, so a dead query and a genuine red looked identical.""" + state, text = dashboard._ci_fragment( + {"conclusion": "", "status": "unavailable", "error": "unknown flag: --branch"} + ) + assert "unavailable" in text.lower() + assert "in_progress" not in text + + +def test_ci_fragment_still_reports_real_pending(): + state, text = dashboard._ci_fragment({"conclusion": "", "status": "in_progress"}) + assert text == "CI in_progress" + + +def test_ci_fragment_success_and_failure_unchanged(): + assert dashboard._ci_fragment({"conclusion": "success"})[1] == "CI ✓" + assert "Smoke Tests" in dashboard._ci_fragment( + {"conclusion": "failure", "workflow": "Smoke Tests"} + )[1] diff --git a/tests/test_readiness.py b/tests/test_readiness.py index 98a843b..79f0988 100644 --- a/tests/test_readiness.py +++ b/tests/test_readiness.py @@ -985,3 +985,27 @@ def test_malformed_stages_never_raises(stages): report["stages"] = stages v = compute(make_snapshot(validation_report=report)) assert v["verdict"] == "red" + + +# --- CI query failure is an explicit unknown, never a silent pass ---------- + +def test_library_ci_fetch_error_is_stale_not_green(): + """A failed CI query means no evidence. It must be said out loud rather + than leaving the repo looking healthy — the release gate leans on this.""" + snap = make_snapshot() + snap["repos"]["PyAutoLens"]["ci_status"] = { + "conclusion": "", "status": "unavailable", "error": "unknown flag: --branch", + } + v = compute(snap) + assert v["verdict"] != "green" + assert any("PyAutoLens" in r and "unavailable" in r for r in v["stale_reasons"]) + + +def test_library_ci_fetch_error_is_not_counted_as_red(): + """Unknown is not the same as failing: an unreachable API must not fake a red.""" + snap = make_snapshot() + snap["repos"]["PyAutoLens"]["ci_status"] = { + "conclusion": "", "status": "unavailable", "error": "boom", + } + v = compute(snap) + assert not any("PyAutoLens" in r for r in v["red_reasons"])