Comment on pull requests with an incomplete checklist

.github/PULL_REQUEST_TEMPLATE.md

@@ -23,7 +23,7 @@ Fill in every section that applies, and delete the entry-type sections that don'
 <!-- These are gates. A PR is not ready for review until each applicable box is checked. -->
 
 - [ ] **Repository layout is correct** — skills in `/skills`, MCP servers in `/mcp`, custom agents in `/custom-agents`; skills and MCP servers are never mixed in one directory.
-- [ ] **A domain SME has reviewed and signed off.** A named subject-matter expert for this domain — someone other than the author — has reviewed the tool for technical correctness and approved it in this PR. Required, not optional.
+- [ ] **A domain SME has reviewed and signed off.** A named subject-matter expert for this domain — someone other than the author — has reviewed the tool for technical correctness and approved it in this PR. Required for any change to what the tool tells the agent; an edit that changes no technical claim can be marked n/a with the reason, as CONTRIBUTING.md describes.
 - [ ] **Evaluation evidence is included** (see Testing).
 - [ ] **No internal-only or proprietary content** — no internal hostnames, account IDs, customer names, ticket numbers, internal URLs, or secrets in the tool, its examples, or its tests.
 - [ ] **README is complete** — prerequisites, setup/deployment, and at least one sample prompt or usage example.

.github/scripts/check_pr_checklist.py

@@ -0,0 +1,210 @@
+#!/usr/bin/env python3
+"""Check that a pull request's description has its required checklist items done.
+
+Run by ``.github/workflows/validate-pr-checklist.yml``. Reads the ``pull_request``
+event payload and the files the pull request changes, lists the unmet items in the
+job's step summary, and writes the verdict for ``label-pr-checklist-result.yml``::
+
+    <out>/outcome   pass | fail | error
+    <out>/reason    complete | incomplete | draft | bot | predating
+
+The unmet items are never written to ``<out>``. The labeling workflow runs with
+write access and posts a comment, so it is handed only values it checks against a
+fixed list; text from a PR description never reaches it.
+
+Which items apply comes from the files the PR changes, not from the "Type of
+change" boxes, one of which names three tool types at once. A PR that touches no
+tool directory is held only to the items that apply to every contribution. Inside
+a tool PR an item that genuinely does not apply can be marked ``n/a:`` with a
+reason on its own line, and the reviewer judges the reason; the universal items
+cannot be waived that way.
+
+Exit status: 0 pass, 1 fail, 2 the check could not run. An error says nothing
+about the PR, and the labeler leaves the PR's label and comment alone on it.
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import re
+import subprocess
+import sys
+from pathlib import Path
+
+# When the gated PR template reached main (commit 49f31c1, merged with PR #135),
+# not when this check merged: a PR opened after it had the checklist in front of
+# its author. A PR opened earlier passes with warnings unless ENFORCE_LABEL is on it.
+CHECK_INTRODUCED = "2026-10-13T07:33:08Z"
+ENFORCE_LABEL = "enforce-checklist"
+
+# Matched against the description's "## " headings by prefix, case-insensitively,
+# so rewording the rest of a heading keeps its section. The tests pin each one
+# against .github/PULL_REQUEST_TEMPLATE.md.
+ALWAYS_SECTIONS = ("Required before review", "License confirmation")
+TOOL_SECTION = "Security and safety"
+TOOL_DIRS = {"skills/": "Skills", "mcp/": "MCP servers", "custom-agents/": "Custom agents"}
+# connectors/ is not a tool directory here, deliberately: connector submissions are
+# not open yet, and the template has no connector section to enforce.
+
+# Items every PR is held to, tool or not, and that can never be marked n/a: the
+# repository is public, and the licence is a statement only the author can make.
+UNIVERSAL_ITEMS = (
+    re.compile(r"internal[-\s]only or proprietary", re.I),
+    re.compile(r"Apache License 2\.0", re.I),
+)
+
+HEADING = re.compile(r"^##\s+(.*?)\s*$")
+UNCHECKED = re.compile(r"^\s*-\s*\[\s*\]\s*")
+# `n/a:` and a reason. A bare "n/a" waives nothing: the reason is what gets judged.
+NA_WITH_REASON = re.compile(r"\bn/a\s*:\s*\S", re.I)
+
+
+def sections(body: str) -> dict[str, list[str]]:
+    """The description's lines, grouped under their "## " heading."""
+    found: dict[str, list[str]] = {}
+    current = None
+    for line in body.splitlines():
+        heading = HEADING.match(line)
+        if heading:
+            current = heading.group(1)
+            found[current] = []
+        elif current is not None:
+            found[current].append(line)
+    return found
+
+
+def find_section(found: dict[str, list[str]], prefix: str) -> list[str] | None:
+    for heading, lines in found.items():
+        if heading.lower().startswith(prefix.lower()):
+            return lines
+    return None
+
+
+def required_sections(paths: list[str]) -> tuple[list[str], bool]:
+    """The sections the changed files call for, and whether this is a tool PR."""
+    tool_dirs = [d for d in TOOL_DIRS if any(p.startswith(d) for p in paths)]
+    names = list(ALWAYS_SECTIONS)
+    if tool_dirs:
+        names.append(TOOL_SECTION)
+        names += [TOOL_DIRS[d] for d in tool_dirs]
+    return names, bool(tool_dirs)
+
+
+def evaluate(body: str, paths: list[str]) -> tuple[list[str], list[str], bool]:
+    """(unmet, waived, is_tool_pr). Each entry reads "<section> → <item>"."""
+    names, is_tool = required_sections(paths)
+    found = sections(body)
+    unmet: list[str] = []
+    waived: list[str] = []
+    for name in names:
+        lines = find_section(found, name)
+        if lines is None:
+            # Deleting a required section must not be a way past it.
+            unmet.append(f"{name} → section missing from the description; restore it from the template")
+            continue
+        for line in lines:
+            if not UNCHECKED.match(line):
+                continue
+            item = UNCHECKED.sub("", line).replace("**", "").strip()
+            if not item:
+                continue
+            universal = any(p.search(item) for p in UNIVERSAL_ITEMS)
+            if not is_tool and not universal:
+                continue
+            if NA_WITH_REASON.search(item):
+                if not universal:
+                    waived.append(f"{name} → {item}")
+                    continue
+                item += "  (this item cannot be marked n/a)"
+            unmet.append(f"{name} → {item}")
+    return unmet, waived, is_tool
+
+
+def decide(pr: dict, unmet: list[str]) -> tuple[str, str]:
+    """(outcome, reason). The reason lets the labeler say why a PR passed, so it
+    never tells the author of a draft or a grandfathered PR they are done."""
+    if not unmet:
+        return "pass", "complete"
+    # A bot cannot fill in a template, and a draft is not submitted for review. The
+    # label does not override either: labeling a draft would nag an author who has
+    # said they are not finished.
+    if (pr.get("user") or {}).get("type") == "Bot":
+        return "pass", "bot"
+    if pr.get("draft"):
+        return "pass", "draft"
+    created = pr.get("created_at") or ""
+    labels = {label.get("name") for label in pr.get("labels") or []}
+    if created and created < CHECK_INTRODUCED and ENFORCE_LABEL not in labels:
+        return "pass", "predating"
+    return "fail", "incomplete"
+
+
+def summary(unmet: list[str], waived: list[str], is_tool: bool, reason: str) -> str:
+    lines = ["## PR checklist", ""]
+    lines.append(
+        "This PR touches a tool directory, so every required item applies."
+        if is_tool
+        else "This PR touches no tool directory, so only the items that apply to every contribution are enforced."
+    )
+    if waived:
+        lines += ["", "Marked n/a, with a reason:", ""] + [f"- {w}" for w in waived]
+    heading = {
+        "complete": "All applicable required items are met.",
+        "incomplete": "Unmet required items:",
+        "bot": "Opened by a bot, so the checklist is not enforced. Reported only:",
+        "draft": "A draft, so the checklist is not enforced until it is marked ready for review. Reported only:",
+        "predating": f"Opened before this check existed, so these are warnings. Apply `{ENFORCE_LABEL}` to require them:",
+    }[reason]
+    lines += ["", heading]
+    if unmet:
+        lines += [""] + [f"- {u}" for u in unmet]
+    return "\n".join(lines) + "\n"
+
+
+def changed_paths(base_ref: str, head_ref: str) -> list[str]:
+    out = subprocess.run(
+        ["git", "diff", "--name-only", f"{base_ref}...{head_ref}"],
+        check=True,
+        capture_output=True,
+        text=True,
+    ).stdout
+    return [p for p in out.splitlines() if p]
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+    parser.add_argument("--event", required=True, help="the pull_request event payload ($GITHUB_EVENT_PATH)")
+    parser.add_argument("--base-ref", required=True, help="the base branch, e.g. origin/main")
+    parser.add_argument("--head-ref", default="HEAD")
+    parser.add_argument("--out", type=Path, required=True, help="directory for outcome and reason")
+    args = parser.parse_args(argv)
+    args.out.mkdir(parents=True, exist_ok=True)
+
+    try:
+        pr = json.loads(Path(args.event).read_text(encoding="utf-8"))["pull_request"]
+        unmet, waived, is_tool = evaluate(pr.get("body") or "", changed_paths(args.base_ref, args.head_ref))
+        outcome, reason = decide(pr, unmet)
+        report = summary(unmet, waived, is_tool, reason)
+        step_summary = os.environ.get("GITHUB_STEP_SUMMARY")
+        if step_summary:
+            with open(step_summary, "a", encoding="utf-8") as f:
+                f.write(report)
+    except Exception as e:  # noqa: BLE001 - failing to reach a verdict is an error, never a finding
+        (args.out / "outcome").write_text("error\n", encoding="utf-8")
+        print(f"::error::The PR checklist check could not run ({type(e).__name__}: {e}). "
+              "Nothing was concluded about this pull request.")
+        return 2
+
+    (args.out / "outcome").write_text(outcome + "\n", encoding="utf-8")
+    (args.out / "reason").write_text(reason + "\n", encoding="utf-8")
+    print(report)
+    if reason == "predating":
+        for item in unmet:
+            print(f"::warning::{item}")
+    return 0 if outcome == "pass" else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())

.github/workflows/label-pr-checklist-result.yml

@@ -0,0 +1,151 @@
+name: Label PR checklist result
+
+# Applies the `needs-checklist` label and an explanatory comment to a PR whose
+# "Validate PR checklist" check failed, and removes the label and updates the
+# comment once the check passes.
+#
+# `needs-checklist` is an *output* of the check and is owned by this workflow
+# alone. Reviewers filter it out: a PR carrying it is on hold and not reviewed.
+#
+# Why a separate workflow: for pull_request events from a fork, GITHUB_TOKEN is
+# read-only regardless of the `permissions:` block, so the validating job cannot
+# label or comment on its own PR. A workflow_run job runs in the base repository's
+# context with write access, and uses the workflow definition from the default
+# branch rather than from the PR.
+#
+# Nothing from the artifact is trusted beyond a fixed list. The validating job runs
+# a workflow definition the PR itself controls, so its artifact can say anything.
+# Hence:
+#   - The PR is resolved from this event's payload, which GitHub writes: an open PR
+#     whose head is the run's head repository, branch and commit. The artifact's
+#     PR number only has to name one of those matches, so it cannot point the
+#     label or comment at another PR. A run whose commit is no longer the PR's head
+#     matches nothing and does nothing, which also keeps a stale run from
+#     overwriting a newer one.
+#   - The comment is fixed text chosen by `outcome` and `reason`, both checked
+#     against the lists below. It links to the run for the unmet items rather than
+#     repeating them, so no text from a PR description is ever posted by the bot.
+#   - `error` means the check did not reach a verdict. The label and comment are
+#     left as they are: neither "incomplete" nor "complete" is a claim anyone is in
+#     a position to make. The validating job has already failed, saying why.
+
+on:
+  workflow_run:
+    workflows: ["Validate PR checklist"]
+    types: [completed]
+
+permissions:
+  contents: read
+  actions: read # download the artifact from the triggering run
+  pull-requests: write # add / remove the label and comment
+
+# Serialized per pull request, not per run. Keying on `workflow_run.id` would make
+# every group unique and the setting a no-op, so two validate runs finishing close
+# together could both reach the commenting step and race. The head repository and
+# branch identify the PR. `cancel-in-progress: false` so a labeler mid-write is
+# never killed; runs queue.
+concurrency:
+  group: label-pr-checklist-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }}
+  cancel-in-progress: false
+
+jobs:
+  label:
+    runs-on: ubuntu-latest
+    if: >-
+      github.event.workflow_run.conclusion == 'success' ||
+      github.event.workflow_run.conclusion == 'failure'
+    steps:
+      - name: Download the check outcome
+        uses: actions/download-artifact@v4
+        continue-on-error: true
+        with:
+          name: checklist-check-result
+          path: result
+          run-id: ${{ github.event.workflow_run.id }}
+          github-token: ${{ github.token }}
+
+      - name: Apply or remove the label and comment
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const fs = require("fs");
+            const LABEL = "needs-checklist";
+            const MARKER = "<!-- pr-checklist-enforcement -->";
+            const REASONS = { pass: ["complete", "draft", "bot", "predating"], fail: ["incomplete"] };
+            const run = context.payload.workflow_run;
+            const {owner, repo} = context.repo;
+            const read = (name) => {
+              try { return fs.readFileSync(`result/${name}`, "utf8").trim(); } catch { return ""; }
+            };
+
+            const outcome = read("outcome");
+            const reason = read("reason");
+            if (!Object.hasOwn(REASONS, outcome)) {
+              core.notice(`Outcome '${outcome || "none"}': the check reached no verdict, so the label and comment are left as they are.`);
+              return;
+            }
+            if (!REASONS[outcome].includes(reason)) {
+              core.warning(`Unexpected reason '${reason}' for outcome '${outcome}'; the PR is left as it is.`);
+              return;
+            }
+
+            const head = run.head_repository;
+            const claimed = Number(read("pr-number"));
+            if (!head) {
+              core.notice("The run's head repository no longer exists; nothing to label.");
+              return;
+            }
+            const candidates = await github.paginate(github.rest.pulls.list, {
+              owner, repo, state: "open", head: `${head.owner.login}:${run.head_branch}`, per_page: 100,
+            });
+            const pr = candidates.find((p) => p.number === claimed && p.head.sha === run.head_sha);
+            if (!pr) {
+              core.warning(`No open PR #${claimed} has ${head.full_name}:${run.head_branch} at ${run.head_sha} as its head; nothing labeled.`);
+              return;
+            }
+
+            // Ensure the label exists.
+            try { await github.rest.issues.getLabel({owner, repo, name: LABEL}); }
+            catch { await github.rest.issues.createLabel({owner, repo, name: LABEL, color: "D93F0B", description: "PR checklist incomplete — not reviewed until resolved"}); }
+
+            const hasLabel = (pr.labels || []).some((l) => l.name === LABEL);
+            const comments = await github.paginate(github.rest.issues.listComments, {owner, repo, issue_number: pr.number});
+            const prev = comments.find((c) => (c.body || "").includes(MARKER));
+
+            if (outcome === "fail") {
+              if (!hasLabel) await github.rest.issues.addLabels({owner, repo, issue_number: pr.number, labels: [LABEL]});
+              const comment = [
+                MARKER,
+                "### This PR is on hold — checklist incomplete",
+                "",
+                "Required items in the PR description are unchecked. A maintainer will not review this PR until each one is checked **and actually done**.",
+                "",
+                `The [check's summary](${run.html_url}) lists them. Complete the work, tick the boxes, and this clears automatically. CONTRIBUTING.md explains each item.`,
+              ].join("\n");
+              if (prev) await github.rest.issues.updateComment({owner, repo, comment_id: prev.id, body: comment});
+              else await github.rest.issues.createComment({owner, repo, issue_number: pr.number, body: comment});
+              core.notice(`Labeled PR #${pr.number} with ${LABEL}.`);
+              return;
+            }
+
+            // Passing is not the same as complete. Say "complete" only when nothing
+            // was unmet, and otherwise say why it is not enforced, so an author is
+            // never told they are done when they are not. A PR that never had the
+            // comment gets none: there is nothing to correct.
+            if (hasLabel) {
+              try {
+                await github.rest.issues.removeLabel({owner, repo, issue_number: pr.number, name: LABEL});
+              } catch (e) {
+                // 404 is normal: the label was already removed by hand or by a
+                // concurrent run. Anything else is a real failure worth seeing.
+                if (e.status !== 404) core.warning(`Could not remove ${LABEL} from #${pr.number}: ${e.message}`);
+              }
+            }
+            const bodies = {
+              complete: "### Checklist complete ✅\nAll required items are checked. This PR is ready for maintainer review.",
+              draft: "### Checklist not enforced while this PR is a draft\nSome required items are still unmet. Mark the PR ready for review and this check runs again.",
+              bot: "### Checklist not enforced for bot-authored PRs\nThis PR was opened by a bot, so the contribution checklist does not apply.",
+              predating: "### Checklist not enforced on this PR\nIt was opened before this check existed, so the unmet items are reported as warnings in the check's summary rather than blocking. A maintainer can apply `enforce-checklist` to require them.",
+            };
+            if (prev) await github.rest.issues.updateComment({owner, repo, comment_id: prev.id, body: `${MARKER}\n${bodies[reason]}`});
+            core.notice(`PR #${pr.number} passed the checklist check (${reason}).`);