From 8b73f0af2c90180036fa819c7ccd4aa594acdd86 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Fri, 17 Jul 2026 10:33:26 +1000 Subject: [PATCH 1/2] =?UTF-8?q?Add=20the=20consumed-file=20check=20?= =?UTF-8?q?=E2=80=94=20the=20go-live=20CI=20guardrail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The narrowest possible test that a PR cannot break a live lecture (PLAN Phase 5, go-live guardrails; prerequisite for the first repoint). For every manifest sidecar in lectures/: - the manifest's `filename` must match the sidecar's own name (the pairing Phase 2 promised CI would assert) - if `consumers` is non-empty — a lecture reads the file in production — the data file must exist, integrity.sha256 must be recorded, and the committed bytes must hash to it. The mismatch message points at the corrections-vs-vintages rule so a deliberate in-place fix knows to update the hash in the same PR. Files with no manifest yet (Phase 6 backfill) or empty consumers are out of scope; the full Phase 5 validation suite subsumes this job when it lands. Runs on every PR and on pushes to main. Checkout uses lfs: true proactively: nothing is LFS-tracked today (Phase 3), but if a consumed file ever is, the job must hash bytes, not pointer text. Verified locally: passes on the clean tree (1 consumed file checked), and fails correctly on each of the four failure modes — tampered bytes, deleted data file, sidecar/filename mismatch, and a consumed file with no recorded hash. Ticks the PLAN go-live checklist item in the same PR, per convention. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/check_consumed_files.py | 92 +++++++++++++++++++++++ .github/workflows/consumed-file-check.yml | 27 +++++++ PLAN.md | 2 +- 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 .github/scripts/check_consumed_files.py create mode 100644 .github/workflows/consumed-file-check.yml diff --git a/.github/scripts/check_consumed_files.py b/.github/scripts/check_consumed_files.py new file mode 100644 index 0000000..5ab2879 --- /dev/null +++ b/.github/scripts/check_consumed_files.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +"""Go-live guardrail: a PR must not break a file a live lecture consumes. + +For every manifest sidecar lectures/.yml: + + - the manifest's `filename` must match the sidecar's own name + - if `consumers` is non-empty (a lecture reads this file in production): + * the data file must exist + * `integrity.sha256` must be recorded + * the committed bytes must hash to it + +Files with no manifest yet (Phase 6 backfill pending) or an empty `consumers` +list are out of scope here — the full validation suite (schema, dtypes, +invariants) is PLAN Phase 5 and will subsume this check. +""" +from __future__ import annotations + +import hashlib +import pathlib +import sys + +import yaml + +LECTURES = pathlib.Path(__file__).resolve().parents[2] / "lectures" + + +def sha256(path: pathlib.Path) -> str: + h = hashlib.sha256() + with open(path, "rb") as f: + for chunk in iter(lambda: f.read(1 << 20), b""): + h.update(chunk) + return h.hexdigest() + + +def main() -> int: + errors = [] + manifests = sorted(LECTURES.glob("*.yml")) + checked = 0 + + for manifest_path in manifests: + manifest = yaml.safe_load(manifest_path.read_text()) + declared = manifest.get("filename") + expected = manifest_path.name[: -len(".yml")] + if declared != expected: + errors.append( + f"{manifest_path.name}: `filename: {declared}` does not match " + f"the sidecar's own name (expected {expected!r})" + ) + continue + + consumers = manifest.get("consumers") or [] + if not consumers: + continue # nothing live reads it — out of scope for this guardrail + + checked += 1 + data_path = LECTURES / declared + if not data_path.exists(): + errors.append( + f"{declared}: consumed by {len(consumers)} lecture(s) but the " + f"data file is missing — this would break a live lecture build" + ) + continue + + recorded = (manifest.get("integrity") or {}).get("sha256") + if not recorded: + errors.append( + f"{declared}: consumed but integrity.sha256 is not recorded — " + f"a consumed file must carry its hash so changes are deliberate" + ) + continue + + actual = sha256(data_path) + if actual != recorded: + errors.append( + f"{declared}: bytes do not match the manifest (sha256 {actual} " + f"!= recorded {recorded}). If this is a deliberate in-place " + f"correction, update integrity.sha256 in the same PR and plan " + f"rebuilds for its consumers (AGENTS.md, 'Corrections vs " + f"vintages')" + ) + + for e in errors: + print(f"::error::{e}") + print( + f"{len(manifests)} manifest(s) found, {checked} consumed file(s) " + f"checked, {len(errors)} error(s)" + ) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/consumed-file-check.yml b/.github/workflows/consumed-file-check.yml new file mode 100644 index 0000000..3c55a38 --- /dev/null +++ b/.github/workflows/consumed-file-check.yml @@ -0,0 +1,27 @@ +# Go-live guardrail (PLAN Phase 5): a PR must not break a file a live lecture +# consumes. The narrowest possible check — full PR validation (manifest schema, +# dtypes, invariant tests) comes later in Phase 5 and will subsume this job. + +name: consumed-file-check + +on: + pull_request: + push: + branches: [main] + +jobs: + consumed-files: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # No LFS-tracked files today (PLAN Phase 3). Set proactively: if a + # consumed file is ever LFS-tracked, this job must hash the bytes, + # not the pointer — a pointer hash would fail loudly here, which is + # the correct behaviour, but only with lfs available to fetch. + lfs: true + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - run: pip install pyyaml + - run: python .github/scripts/check_consumed_files.py diff --git a/PLAN.md b/PLAN.md index 40ed156..97d1313 100644 --- a/PLAN.md +++ b/PLAN.md @@ -80,7 +80,7 @@ The sidecar naming uses the **full filename** (`mpd2020.xlsx.yml`, not `mpd2020. **Go-live guardrails** — the minimal subset that must precede the first repoint (Phase 8); the rest of this phase follows at its own pace: - [x] Branch protection on `main`: PRs required (no direct pushes; zero approvals so a solo maintainer can still merge), force-pushes and deletion blocked. Once a lecture repoints, `raw/main` is a production URL and an accidental force-push is a lecture outage (ruleset added 2026-07-17) -- [ ] Minimal consumed-file check: CI that asserts every file in `lectures/` whose manifest has a non-empty `consumers` list still exists and matches its manifest `sha256` — the narrowest possible test that a PR cannot break a live lecture. Subsumed later by the full PR validation below +- [x] Minimal consumed-file check: CI that asserts every file in `lectures/` whose manifest has a non-empty `consumers` list still exists and matches its manifest `sha256` — the narrowest possible test that a PR cannot break a live lecture. Subsumed later by the full PR validation below (added 2026-07-17: `.github/workflows/consumed-file-check.yml`) Full automation: From 07aa367f4b45ab1504a6959b677ce6b8561ca6f7 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Fri, 17 Jul 2026 10:44:53 +1000 Subject: [PATCH 2/2] Harden the checker against malformed manifests (Copilot review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both crashes would still have failed CI (non-zero exit via the traceback), so the guardrail was never bypassable — but a guardrail should diagnose, not stack-trace, and should keep scanning past one bad file: - Empty or invalid YAML: caught per file (yaml.YAMLError, or a non-mapping parse result) and reported as ::error:: with the filename; the remaining manifests still get checked. - Non-dict `integrity` (e.g. a stray string): treated as missing, so a consumed file lands in the existing "sha256 not recorded" error instead of raising AttributeError. Re-verified all five original scenarios plus three new ones: empty sidecar, unparseable YAML, and integrity-as-string — each fails cleanly with the right message and exit 1, no tracebacks. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/check_consumed_files.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/.github/scripts/check_consumed_files.py b/.github/scripts/check_consumed_files.py index 5ab2879..2091292 100644 --- a/.github/scripts/check_consumed_files.py +++ b/.github/scripts/check_consumed_files.py @@ -38,7 +38,18 @@ def main() -> int: checked = 0 for manifest_path in manifests: - manifest = yaml.safe_load(manifest_path.read_text()) + try: + manifest = yaml.safe_load(manifest_path.read_text()) + except yaml.YAMLError as exc: + errors.append(f"{manifest_path.name}: invalid YAML — {exc}") + continue + if not isinstance(manifest, dict): + errors.append( + f"{manifest_path.name}: manifest must be a YAML mapping, got " + f"{type(manifest).__name__} — is the file empty or malformed?" + ) + continue + declared = manifest.get("filename") expected = manifest_path.name[: -len(".yml")] if declared != expected: @@ -61,7 +72,10 @@ def main() -> int: ) continue - recorded = (manifest.get("integrity") or {}).get("sha256") + integrity = manifest.get("integrity") + # A non-dict integrity (e.g. a stray string) is treated as missing, so + # it lands in the "not recorded" error below instead of crashing here. + recorded = integrity.get("sha256") if isinstance(integrity, dict) else None if not recorded: errors.append( f"{declared}: consumed but integrity.sha256 is not recorded — "