From 634cda69e0044dc9ef373a43bde35c5ee17d5c89 Mon Sep 17 00:00:00 2001 From: Andrew Bates Date: Mon, 26 Oct 2026 07:29:18 -0500 Subject: [PATCH 1/2] ci: AGENTS.md and the pull-request gates AGENTS.md sets the repo's rules (logging, conventional commits, the credit rule, review, workflows, entries). scripts/gates.py enforces what a machine can: a conventional PR title and commit headers, well-formed Co-Authored-By trailers, a current approval from someone other than the author on anything but docs, and every action pinned by commit. Each gate prints a JSON record and fails loudly; the existing submission workflow's actions are pinned. The header check is adapted from nevr-runtime's tools/check_pr_title.py. Closes #11 Co-Authored-By: Andrew Bates --- .github/workflows/check-submission.yml | 10 +- .github/workflows/gates.yml | 115 ++++++++++++++ AGENTS.md | 111 ++++++++++++++ scripts/gates.py | 201 +++++++++++++++++++++++++ tests/test_gates.py | 170 +++++++++++++++++++++ 5 files changed, 602 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/gates.yml create mode 100644 AGENTS.md create mode 100644 scripts/gates.py create mode 100644 tests/test_gates.py diff --git a/.github/workflows/check-submission.yml b/.github/workflows/check-submission.yml index 9837c0d..ee8e8a9 100644 --- a/.github/workflows/check-submission.yml +++ b/.github/workflows/check-submission.yml @@ -26,11 +26,11 @@ jobs: permissions: contents: read steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: persist-credentials: false - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: repository: EchoTools/echo-mod-template-repo path: template @@ -50,7 +50,7 @@ jobs: ISSUE_NUMBER: ${{ github.event.issue.number }} run: python3 scripts/inspect_submission.py --template template --smoke-host "$PWD/smoke_host.exe" --out report.json - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: report path: report.json @@ -63,11 +63,11 @@ jobs: contents: read issues: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: persist-credentials: false - - uses: actions/download-artifact@v4 + - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 with: name: report diff --git a/.github/workflows/gates.yml b/.github/workflows/gates.yml new file mode 100644 index 0000000..4a63357 --- /dev/null +++ b/.github/workflows/gates.yml @@ -0,0 +1,115 @@ +# The pull-request gates (scripts/gates.py): a conventional PR title and commits, well-formed credit +# trailers, a current approval from someone other than the author on changes to anything but docs, and +# actions pinned by commit. Each prints one JSON record; the records are kept as an artifact. +# +# Trust boundary: `pull_request` and `pull_request_review`, never `pull_request_target`. The token is +# read-only, no secret is named, nothing from the PR is executed except this repo's own scripts, and the +# title reaches the script through env only. +name: gates + +on: + pull_request: + types: [opened, edited, synchronize, reopened] + pull_request_review: + types: [submitted, dismissed] + +permissions: {} + +concurrency: + group: gates-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + title: + if: github.event_name == 'pull_request' + runs-on: ubuntu-24.04 + permissions: + contents: read + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + persist-credentials: false + - name: The PR title is a conventional commit header + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: python3 -I scripts/gates.py title | tee gate-title.jsonl + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + if: always() + with: + name: gate-title + path: gate-title.jsonl + retention-days: 90 + + commits: + if: github.event_name == 'pull_request' + runs-on: ubuntu-24.04 + permissions: + contents: read + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + fetch-depth: 0 + persist-credentials: false + - name: Every commit is conventional and its credit trailers are well formed + env: + BASE: ${{ github.event.pull_request.base.sha }} + HEAD: ${{ github.event.pull_request.head.sha }} + run: python3 -I scripts/gates.py commits --base "$BASE" --head "$HEAD" | tee gate-commits.jsonl + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + if: always() + with: + name: gate-commits + path: gate-commits.jsonl + retention-days: 90 + + review: + runs-on: ubuntu-24.04 + permissions: + contents: read + pull-requests: read + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + persist-credentials: false + - name: A current approval from someone else, when more than docs changed + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number }} + run: python3 -I scripts/gates.py review --repo "$REPO" --pr "$PR" | tee gate-review.jsonl + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + if: always() + with: + name: gate-review + path: gate-review.jsonl + retention-days: 90 + + pins: + if: github.event_name == 'pull_request' + runs-on: ubuntu-24.04 + permissions: + contents: read + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + persist-credentials: false + - name: Every action is pinned by commit + run: python3 -I scripts/gates.py pins | tee gate-pins.jsonl + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + if: always() + with: + name: gate-pins + path: gate-pins.jsonl + retention-days: 90 + + tests: + if: github.event_name == 'pull_request' + runs-on: ubuntu-24.04 + permissions: + contents: read + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + persist-credentials: false + - name: The gates' own tests + run: python3 -I -m unittest discover -s tests -k test_gates -v diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..3fb042b --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,111 @@ +# AGENTS.md + +Rules for any agent or contributor working in this repo. The home-level `AGENTS.md` files (logging, read +before acting, verify then say done) apply as well; this file adds to them and wins where it is more specific. + +## What this repo is + +The catalog and submission place for nEVR plugins. An author submits once (an issue from the form); a +check comments what it found; a maintainer approves; each listed plugin points at its EchoTools fork and +release and carries what a plugin browser needs: description, media, versions, author, credits. + +| Path | What | +|---|---| +| `.github/ISSUE_TEMPLATE/` | The submit and pull-a-version forms | +| `.github/workflows/check-submission.yml`, `scripts/inspect_submission.py`, `scripts/report_submission.py` | The automatic check of a submission, in a read-only job, and its comment | +| `mods/.json`, `schema/entry.schema.json`, `docs/entry-format.md`, `scripts/validate_entries.py` | The listed entries and their format | +| `.github/workflows/gates.yml`, `scripts/gates.py`, `tests/test_gates.py` | The pull-request gates below | + +Read `README.md` and `docs/entry-format.md` before changing an entry or the check. The signing design this +repo is part of is release-signer's `docs/plugin-release.md`. + +## Logging + +Everything here that runs logs what it did, as JSON, durably. Nothing in it is allowed to pass quietly. + +- The gates and the entry validator print one JSON record per check (`event`, `at`, `ok`, `errors`, and + what was checked). CI keeps those records as artifacts for 90 days. A gate that cannot run prints a + `cannot_run` record and exits 2; it never passes by default. +- The submission check writes `report.json` (an artifact) and an issue comment for every run. +- A new script or workflow step is not done until you have shown a real record from a real run and a test + that asserts it. Never print or log a token or a secrets file. + +## Commits and PR titles + +- Commits and PR titles follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/): + `type(scope)!: description`, with `type` one of `feat fix docs style refactor perf test build ci chore + revert`. One logical change per commit. The history before this file is free text; new commits are not. +- The `gates` workflow checks the title and every commit of a PR (`scripts/gates.py title`, `commits`). +- Version tags are SemVer, `v`. The changelog is the git log. + +## Credit + +Credit the people whose work we integrate, every time. + +- A commit that draws on a person's code, patch, research or findings carries `Co-Authored-By: Name + ` for that person (`gh api users/` gives the id). The gate + rejects a trailer that is not `Name ` with a GitHub noreply address or the house addresses + (`a@sprock.io`, `agents@sprock.io`). Someone with no GitHub account is credited by name in the commit + body. +- A listed mod credits everyone whose work it builds on in the entry's `credits` (name, role, link). + The mod's own repo keeps their license files and says who they are in `NOTICE` and its README. +- Never list a mod whose credits you know to be missing: send it back to the author. + +## Review + +- Every PR that changes more than `README.md` and `docs/` needs a current approval from an owner, member + or collaborator who did not write it. "Current" means given on the PR's latest commit. An open request + for changes blocks. `gates.yml`'s `review` job (`scripts/gates.py review`) checks it. +- The reviewer reads the diff and the code around it, runs what the PR claims, and reports findings with + `file:line`. An entry's reviewer also reads the mod: what it hooks, what it sends where, and whether its + capabilities say what it does. +- A change to a gate (`scripts/gates.py`, `.github/workflows/`) is reviewed with extra care: the PR's own + copy of the gate is the one that runs on it. +- Owner setting, not in this repo: make `gates / review`, `title`, `commits`, `pins` and `tests` required + checks on `main`, and restrict who can merge. Until then the gates report but do not block. + +## Issues, branches and PRs + +- Every unit of work has an issue before it starts. The PR body names it (`Closes #N`, or `Refs #N` while + part stays open) and carries the commands you ran and their output. +- One worktree per PR: `git worktree add --no-track -b origin/main`. Push with + `git push origin :refs/heads/`. Never push `main`, never force-push, never rewrite a + pushed branch: fix with a new commit. +- A PR is one small change. When it is merged, delete its branch and remove its worktree + (`git worktree remove `); the repo owner merges, not the author. + +## Workflows + +- Pin every action by its 40-hex commit, with the tag in a comment. The `pins` gate checks it. +- Trust boundary: use `pull_request` (and `issues` for the submission check), never `pull_request_target`. + A job that opens, downloads or loads anything a submitter controls holds a read-only token and no secret; + the job that writes (the comment) runs nothing from the submission. Untrusted text reaches a script + through `env`, never inside a script body. +- `permissions: {}` at the top of a workflow, and the least each job needs. + +## Entries + +- `mods/.json` is created by a maintainer when a submission is approved, and changed when a release + is listed or pulled. It is checked against `schema/entry.schema.json`; `docs/entry-format.md` says what + each field means. +- The `signer` block records what the release-signer did. Copy its digests from the signer's public + record; never invent or edit them by hand to make the check pass. +- Pulling a version sets `pulled` on it; it never edits the release's digests. + +## Commands + +``` +python3 -I -m unittest discover -s tests -v # the tests (validate_entries needs: pip install jsonschema==4.26.0) +python3 -I scripts/validate_entries.py # every mods/*.json +python3 -I scripts/gates.py pins # actions pinned +python3 -I scripts/gates.py title --title "fix: x" +python3 -I scripts/gates.py commits --base origin/main --head HEAD +python3 -I scripts/gates.py review --repo EchoTools/echo-mods --pr +``` + +## Never + +- Sign, hold keys or secrets, or add a secret to a workflow here: signing happens only in release-signer. +- Delete, rename or reorganize files, or delete logs, without the owner's word. +- List a mod that failed the check without a maintainer saying why in the issue. +- Turn a check into a warning, or add `|| true`, to make a PR green. Fix the thing it found. diff --git a/scripts/gates.py b/scripts/gates.py new file mode 100644 index 0000000..22196f2 --- /dev/null +++ b/scripts/gates.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python3 +"""The pull-request gates for this repo. Each prints one JSON record and exits 0 (ok) or 1 (fail). + + gates.py title [--title T] # $PR_TITLE: a Conventional Commits header + gates.py commits --base SHA --head SHA # every commit: a conventional header, credit trailers well formed + gates.py review --repo OWNER/NAME --pr N # a changed gated path has a current approval from someone else + gates.py pins [WORKFLOW ...] # every `uses:` is pinned by a 40-hex commit + +A record is {"event": "gate.", "at", "ok", "errors": [...], ...}: what was checked and why it +failed. A gate that can't run (no PR, no git, no gh) exits 2 with a `gate.cannot_run` record: it never +passes by default. Python 3.9+, standard library only. Needs `git` (commits) and `gh` (review). + +The conventional-commit header check is adapted from EchoTools/nevr-runtime's tools/check_pr_title.py +(Andrew Bates). +""" +import argparse +import json +import os +import re +import subprocess +import sys +import time +from pathlib import Path + +TYPES = ("feat", "fix", "docs", "style", "refactor", "perf", "test", "build", "ci", "chore", "revert") +HEADER = re.compile(r"^(?P[A-Za-z]+)(?:\((?P[^()\s][^()]*)\))?(?P!)?: (?P\S.*)$") + +# Co-Authored-By trailers: a GitHub noreply address (credits a person: +@users.noreply.github.com), +# or the project's house addresses. +TRAILER = re.compile(r"^co-authored-by:[ \t]*(?P.*)$", re.I | re.M) +TRAILER_VALUE = re.compile(r"^(?P[^<>\n]*[^<>\s]) <(?P[^<>\s]+)>$") +NOREPLY = re.compile(r"^\d+\+[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})@users\.noreply\.github\.com$") +HOUSE = {"a@sprock.io", "agents@sprock.io"} + +# A change to any of these needs a review. README and docs alone don't. +GATED = ("mods/", "schema/", "scripts/", "tests/", ".github/", "AGENTS.md") +REVIEWERS = {"OWNER", "MEMBER", "COLLABORATOR"} + +USES = re.compile(r"^\s*(?:-\s+)?uses:\s*(?P[^\s#]+)") +PINNED = re.compile(r"^[^@\s]+@[0-9a-f]{40}$") + + +def now(): + return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) + + +def header_error(title): + """None when `title` is a Conventional Commits 1.0.0 header, else why not.""" + if "\n" in title or "\r" in title: + return "must be one line" + m = HEADER.match(title) + if not m: + return (f"{title!r} is not `type(scope)!: description` (https://www.conventionalcommits.org/en/v1.0.0/); " + "the scope and the ! are optional, the colon and the space after it are not") + if m.group("type").lower() not in TYPES: + return f"unknown type {m.group('type')!r}; use one of: {', '.join(TYPES)}" + return None + + +def trailer_errors(message): + errors = [] + for m in TRAILER.finditer(message): + value = m.group("value").strip() + t = TRAILER_VALUE.match(value) + if not t: + errors.append(f"Co-Authored-By {value!r}: write `Name `") + elif not (NOREPLY.match(t.group("email")) or t.group("email") in HOUSE): + errors.append(f"Co-Authored-By {value!r}: credit a person by their GitHub noreply address " + "`+@users.noreply.github.com` (`gh api users/` gives the id)") + return errors + + +def commit_errors(message): + subject = message.split("\n", 1)[0] + errors = [] + e = header_error(subject) + if e: + errors.append(f"subject: {e}") + return errors + trailer_errors(message) + + +def gated(path): + return path.startswith(GATED) + + +def review_errors(author, head_sha, files, reviews): + """Why a PR that changes gated paths has no current approval; [] when it has one or needs none.""" + touched = sorted(f for f in files if gated(f)) + if not touched: + return [], "no gated path changed" + latest = {} + for r in reviews: # in the order GitHub lists them: later wins; comments don't count + if r["state"] in ("APPROVED", "CHANGES_REQUESTED", "DISMISSED"): + latest[r["user"]] = r + errors = [f"{u} requested changes" for u, r in latest.items() if r["state"] == "CHANGES_REQUESTED"] + ok = [u for u, r in latest.items() + if r["state"] == "APPROVED" and u.lower() != author.lower() and r["association"] in REVIEWERS + and r["commit_id"] == head_sha] + if not ok and not errors: + errors.append(f"no approval of {head_sha[:12]} from an owner, member or collaborator other than {author} " + f"(changed: {', '.join(touched[:5])}{' ...' if len(touched) > 5 else ''})") + return errors, f"approved by {', '.join(sorted(ok))}" if ok else "" + + +def pin_errors(path, text): + errors = [] + for n, line in enumerate(text.splitlines(), 1): + m = USES.match(line) + if m and not m.group("ref").startswith("./") and not PINNED.match(m.group("ref")): + errors.append(f"{path}:{n}: `{m.group('ref')}` is not pinned by a 40-hex commit") + return errors + + +def gh_json(path): + r = subprocess.run(["gh", "api", "--paginate", "--slurp", path], capture_output=True, text=True) + if r.returncode != 0: + raise RuntimeError(f"gh api {path}: {r.stderr.strip()[:300]}") + data = json.loads(r.stdout) + return [x for page in data for x in (page if isinstance(page, list) else [page])] + + +def emit(gate, errors, **fields): + record = {"event": f"gate.{gate}", "at": now(), "ok": not errors, "errors": errors, **fields} + print(json.dumps(record, sort_keys=True), flush=True) + return 1 if errors else 0 + + +def cannot_run(gate, why): + print(json.dumps({"event": "gate.cannot_run", "at": now(), "gate": gate, "error": why}, sort_keys=True), flush=True) + return 2 + + +def run_title(args): + title = args.title if args.title is not None else os.environ.get("PR_TITLE") + if title is None: + return cannot_run("title", "no title: --title or $PR_TITLE") + e = header_error(title) + return emit("title", [e] if e else [], title=title) + + +def run_commits(args): + r = subprocess.run(["git", "log", "--no-merges", "--format=%H%x1f%B%x1e", f"{args.base}..{args.head}"], + capture_output=True, text=True) + if r.returncode != 0: + return cannot_run("commits", r.stderr.strip()[:300]) + errors, shas = [], [] + for chunk in filter(None, (c.strip("\n") for c in r.stdout.split("\x1e"))): + sha, _, message = chunk.partition("\x1f") + shas.append(sha) + errors.extend(f"{sha[:12]}: {e}" for e in commit_errors(message)) + if not shas: + return cannot_run("commits", f"no commits in {args.base}..{args.head}") + return emit("commits", errors, base=args.base, head=args.head, commits=len(shas)) + + +def run_review(args): + try: + pr = gh_json(f"repos/{args.repo}/pulls/{args.pr}")[0] + files = [f["filename"] for f in gh_json(f"repos/{args.repo}/pulls/{args.pr}/files")] + reviews = [{"user": r["user"]["login"], "state": r["state"], "commit_id": r["commit_id"], + "association": r["author_association"]} for r in gh_json(f"repos/{args.repo}/pulls/{args.pr}/reviews")] + except (RuntimeError, ValueError, KeyError, IndexError) as e: + return cannot_run("review", str(e)) + errors, note = review_errors(pr["user"]["login"], pr["head"]["sha"], files, reviews) + return emit("review", errors, repo=args.repo, pr=args.pr, author=pr["user"]["login"], head=pr["head"]["sha"], + files=len(files), note=note) + + +def run_pins(args): + paths = args.paths or sorted(Path(".github/workflows").glob("*.y*ml")) + if not paths: + return cannot_run("pins", "no workflow files found") + errors = [] + for p in paths: + errors.extend(pin_errors(str(p), Path(p).read_text(encoding="utf-8"))) + return emit("pins", errors, files=[str(p) for p in paths]) + + +def main(argv=None): + ap = argparse.ArgumentParser() + sub = ap.add_subparsers(dest="gate", required=True) + t = sub.add_parser("title") + t.add_argument("--title") + t.set_defaults(fn=run_title) + c = sub.add_parser("commits") + c.add_argument("--base", required=True) + c.add_argument("--head", required=True) + c.set_defaults(fn=run_commits) + r = sub.add_parser("review") + r.add_argument("--repo", required=True) + r.add_argument("--pr", required=True, type=int) + r.set_defaults(fn=run_review) + p = sub.add_parser("pins") + p.add_argument("paths", nargs="*", type=Path) + p.set_defaults(fn=run_pins) + args = ap.parse_args(argv) + return args.fn(args) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_gates.py b/tests/test_gates.py new file mode 100644 index 0000000..5d6fc30 --- /dev/null +++ b/tests/test_gates.py @@ -0,0 +1,170 @@ +"""Runs scripts/gates.py as the CI does. Every gate has a passing case and a failing one, so a gate that +always passes (or always fails) is caught. + + python3 -I -m unittest discover -s tests -v +""" +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +GATES = ROOT / "scripts" / "gates.py" +sys.path.insert(0, str(ROOT / "scripts")) +import gates # noqa: E402 + +NOREPLY = "Dualgame <26533696+Dualgame@users.noreply.github.com>" + + +def run(*args, cwd=None, env=None): + r = subprocess.run([sys.executable, "-I", str(GATES), *map(str, args)], capture_output=True, text=True, cwd=cwd, + env={**os.environ, **(env or {})}) + return r.returncode, [json.loads(line) for line in r.stdout.splitlines()] + + +class Title(unittest.TestCase): + def test_conventional_titles_pass(self): + for title in ("feat: entry schema", "fix(check)!: refuse a bad tag", "Docs: README", "ci: pin the actions"): + code, records = run("title", "--title", title) + self.assertEqual(code, 0, (title, records)) + self.assertEqual(records[0]["event"], "gate.title") + self.assertTrue(records[0]["ok"]) + + def test_other_titles_fail(self): + for title in ("Form: the EchoVRCE username is contact only", "feat:no space", "wip: x", "feat: ", "update readme"): + code, records = run("title", "--title", title) + self.assertEqual(code, 1, title) + self.assertFalse(records[0]["ok"]) + self.assertTrue(records[0]["errors"]) + + def test_title_from_the_environment(self): + self.assertEqual(run("title", env={"PR_TITLE": "fix: x"})[0], 0) + self.assertEqual(run("title", env={"PR_TITLE": "x"})[0], 1) + + def test_no_title_cannot_run(self): + env = {k: v for k, v in os.environ.items() if k != "PR_TITLE"} + r = subprocess.run([sys.executable, "-I", str(GATES), "title"], capture_output=True, text=True, env=env) + self.assertEqual(r.returncode, 2) + self.assertEqual(json.loads(r.stdout)["event"], "gate.cannot_run") + + +class Commits(unittest.TestCase): + def repo(self, d, *messages): + git = lambda *a: subprocess.run(["git", "-c", "user.name=t", "-c", "user.email=t@example.com", + "-c", "commit.gpgsign=false", *a], cwd=d, check=True, capture_output=True, + text=True).stdout.strip() + git("init", "-q", "-b", "main") + git("commit", "-q", "--allow-empty", "-m", "chore: root") + base = git("rev-parse", "HEAD") + for m in messages: + git("commit", "-q", "--allow-empty", "-m", m) + return base, git("rev-parse", "HEAD") + + def check(self, *messages): + with tempfile.TemporaryDirectory() as d: + base, head = self.repo(d, *messages) + return run("commits", "--base", base, "--head", head, cwd=d) + + def test_good_commits(self): + code, records = self.check("feat: a", f"fix(x): b\n\nbody\n\nCo-Authored-By: {NOREPLY}\n" + "Co-Authored-By: Andrew Bates ") + self.assertEqual(code, 0, records) + self.assertEqual(records[0]["commits"], 2) + + def test_free_text_subject_fails(self): + code, records = self.check("feat: a", "Pull a version form") + self.assertEqual(code, 1) + self.assertEqual(len(records[0]["errors"]), 1) + self.assertIn("subject", records[0]["errors"][0]) + + def test_credit_must_be_a_noreply_address(self): + code, records = self.check("feat: a\n\nCo-Authored-By: Dualgame ") + self.assertEqual(code, 1) + self.assertIn("noreply", records[0]["errors"][0]) + + def test_trailer_needs_an_address(self): + code, records = self.check("feat: a\n\nCo-Authored-By: Dualgame") + self.assertEqual(code, 1) + self.assertIn("Name ", records[0]["errors"][0]) + + def test_no_commits_cannot_run(self): + with tempfile.TemporaryDirectory() as d: + base, _ = self.repo(d) + code, records = run("commits", "--base", base, "--head", base, cwd=d) + self.assertEqual(code, 2) + self.assertEqual(records[0]["event"], "gate.cannot_run") + + +class Review(unittest.TestCase): + HEAD = "a" * 40 + + def review(self, user="mia", state="APPROVED", commit=None, assoc="OWNER"): + return {"user": user, "state": state, "commit_id": commit or self.HEAD, "association": assoc} + + def errors(self, files, reviews, author="author"): + return gates.review_errors(author, self.HEAD, files, reviews)[0] + + def test_docs_only_needs_no_review(self): + self.assertEqual(self.errors(["README.md", "docs/x.md"], []), []) + + def test_gated_path_without_review_fails(self): + for path in ("mods/x.json", "schema/entry.schema.json", "scripts/gates.py", "tests/t.py", + ".github/workflows/x.yml", "AGENTS.md"): + self.assertTrue(self.errors(["README.md", path], []), path) + + def test_approval_by_someone_else_passes(self): + self.assertEqual(self.errors(["mods/x.json"], [self.review()]), []) + + def test_own_approval_does_not_count(self): + self.assertTrue(self.errors(["mods/x.json"], [self.review(user="Author")])) + + def test_stale_approval_does_not_count(self): + self.assertTrue(self.errors(["mods/x.json"], [self.review(commit="b" * 40)])) + + def test_outsider_approval_does_not_count(self): + self.assertTrue(self.errors(["mods/x.json"], [self.review(assoc="NONE")])) + + def test_comment_does_not_count(self): + self.assertTrue(self.errors(["mods/x.json"], [self.review(state="COMMENTED")])) + + def test_dismissed_approval_does_not_count(self): + self.assertTrue(self.errors(["mods/x.json"], [self.review(), self.review(state="DISMISSED")])) + + def test_changes_requested_blocks_even_with_another_approval(self): + errors = self.errors(["mods/x.json"], [self.review(), self.review(user="sprockee", state="CHANGES_REQUESTED")]) + self.assertEqual(errors, ["sprockee requested changes"]) + + def test_later_approval_clears_the_users_own_request(self): + self.assertEqual(self.errors(["mods/x.json"], [self.review(state="CHANGES_REQUESTED"), self.review()]), []) + + +class Pins(unittest.TestCase): + def check(self, text): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "w.yml" + path.write_text(text, encoding="utf-8") + return run("pins", path) + + def test_pinned_passes(self): + code, records = self.check("steps:\n - uses: actions/checkout@" + "a" * 40 + " # v4\n - uses: ./local\n - run: x\n") + self.assertEqual(code, 0, records) + + def test_tag_fails(self): + code, records = self.check("steps:\n - uses: actions/checkout@v4\n") + self.assertEqual(code, 1) + self.assertIn("w.yml:2", records[0]["errors"][0]) + + def test_short_sha_and_branch_fail(self): + self.assertEqual(self.check(" uses: a/b@abc1234\n")[0], 1) + self.assertEqual(self.check(" uses: a/b@main\n")[0], 1) + + def test_the_repos_workflows_are_pinned(self): + code, records = run("pins", cwd=ROOT) + self.assertEqual(code, 0, records) + + +if __name__ == "__main__": + unittest.main() From 0770bf9253f3a98f8dad90242565a9c1a33303f0 Mon Sep 17 00:00:00 2001 From: Andrew Bates Date: Mon, 26 Oct 2026 07:31:09 -0500 Subject: [PATCH 2/2] fix: a failing gate failed its job in name only The gates ran as `gates.py ... | tee`, and GitHub's default bash has no pipefail, so the job took tee's exit code: the review gate printed ok:false and the check was green. Run every step with `shell: bash` (-eo pipefail), and test that the workflow says so. Refs #11 Co-Authored-By: Andrew Bates --- .github/workflows/gates.yml | 5 +++++ tests/test_gates.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/.github/workflows/gates.yml b/.github/workflows/gates.yml index 4a63357..76a6bb5 100644 --- a/.github/workflows/gates.yml +++ b/.github/workflows/gates.yml @@ -15,6 +15,11 @@ on: permissions: {} +# `shell: bash` runs with -eo pipefail; the default does not, and `| tee` would turn a failed gate green. +defaults: + run: + shell: bash + concurrency: group: gates-${{ github.event.pull_request.number }} cancel-in-progress: true diff --git a/tests/test_gates.py b/tests/test_gates.py index 5d6fc30..b3b99f0 100644 --- a/tests/test_gates.py +++ b/tests/test_gates.py @@ -141,6 +141,14 @@ def test_later_approval_clears_the_users_own_request(self): self.assertEqual(self.errors(["mods/x.json"], [self.review(state="CHANGES_REQUESTED"), self.review()]), []) +class Workflow(unittest.TestCase): + def test_a_piped_gate_still_fails_the_job(self): + """`gates.py ... | tee` is green whatever gates.py exits unless the shell has pipefail.""" + text = (ROOT / ".github" / "workflows" / "gates.yml").read_text(encoding="utf-8") + self.assertIn("| tee", text) + self.assertRegex(text, r"(?m)^defaults:\n run:\n shell: bash$") + + class Pins(unittest.TestCase): def check(self, text): with tempfile.TemporaryDirectory() as d: