🏗️✨：check pull request titles and sign-offs

.github/workflows/pull-request-policy.yml

@@ -0,0 +1,42 @@
+# The title and description of a pull request are written by whoever opened
+# it, so both are read from the environment rather than interpolated into a
+# command: a title pasted into a shell is a title that can run as code.
+#
+# Actions are pinned by commit, never by tag.
+name: Pull Request Policy
+
+on:
+  pull_request:
+    types: [opened, edited, synchronize, reopened, ready_for_review]
+
+permissions:
+  contents: read
+
+# A title is often fixed two or three times in a row. Only the last attempt
+# is worth reporting on.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+jobs:
+  title_and_description:
+    name: Title and description
+    # Renovate writes `chore(deps): …` and dependabot writes `Bump x from y to
+    # z`. Neither is this project's format, and neither is ours to change --
+    # the same allowance verify.commits makes for their commit messages.
+    if: ${{ !endsWith(github.event.pull_request.user.login, '[bot]') }}
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out project repository
+        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+      - name: Set up Node.js runtime
+        uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
+        with:
+          node-version-file: 'package.json'
+      # No install step: the task imports one module from this package and
+      # nothing from node_modules, and node resolves that by itself.
+      - name: Verify title and description
+        run: node build/tasks/verify-pull-request.mts
+        env:
+          PR_TITLE: ${{ github.event.pull_request.title }}
+          PR_BODY: ${{ github.event.pull_request.body }}

build/shared/commit-message.mts

@@ -139,13 +139,13 @@
  * @returns {string[]} The trailer lines, one per trailer, empty if there is no block.
  */
 export function readTrailers(message: string) {
   const [, ...rest] = message.replace(/[\r\n]+$/, '').split(/\r?\n/);
   const last = paragraphsOf(rest).at(-1) ?? [];
   const isBlock =
     last.length > 0 &&
     TRAILER_LINE.test(last[0] ?? '') &&
     last.every(
       (line) => TRAILER_LINE.test(line) || CONTINUATION_LINE.test(line)
     );
 
   return isBlock ? last.filter((line) => !CONTINUATION_LINE.test(line)) : [];
@@ -386,3 +386,30 @@
 
   return problems;
 }
+
+/**
+ * Checks that a human certified the change. Only the person named as author
+ * can do that: an assistant discloses itself with `Assisted-by` and does not
+ * sign anything, and a bot certifying on someone's behalf is the thing this
+ * exists to stop.
+ * @param {string} message The whole commit message.
+ * @param {string} author The commit's author, as `Name <email>`.
+ * @returns {string[]} What is wrong with it, empty if nothing.
+ */
+export function checkSignOff(message: string, author: string) {
+  const signed = readTrailers(message)
+    .filter((line) => /^Signed-off-by:/.test(line))
+    .map((line) => line.slice(line.indexOf(':') + 1).trim());
+
+  if (signed.length === 0) {
+    return [
+      `no “Signed-off-by: ${author}”; the Developer Certificate of Origin is certified by the author, and “Assisted-by:” is what discloses a tool`,
+    ];
+  }
+
+  return signed.includes(author)
+    ? []
+    : [
+        `\`Signed-off-by:\` names ${signed.join(', ')}, but the author is ${author}`,
+      ];
+}

build/tasks/verify-pull-request.mts

@@ -0,0 +1,50 @@
+/**
+ * @file Verify a pull request's title and description before it can land.
+ * @author The OpenINF Authors & Friends
+ * @license MIT OR Apache-2.0 OR BlueOak-1.0.0
+ * @module {type ES6Module} build/tasks/verify/verify-pull-request
+ */
+
+import { validateCommitMessage } from '@openinf/portal/build/commit-message';
+
+// Read from the environment rather than interpolated into a command by the
+// workflow: a title is written by whoever opened the pull request, and pasting
+// one into a shell is how a title comes to run as code.
+const title = process.env.PR_TITLE ?? '';
+const body = process.env.PR_BODY ?? '';
+
+if (process.env.PR_TITLE === undefined) {
+  console.error(
+    'PR_TITLE is not set. This task reads the pull request from the ' +
+      'environment; see .github/workflows/pull-request-policy.yml.'
+  );
+  process.exitCode = 1;
+} else {
+  const problems: string[] = [];
+
+  // The title becomes the subject of the commit that lands, because the
+  // squash takes it verbatim. So it answers to the same rules, and checking
+  // it here is the only chance to say so before the subject is history.
+  for (const problem of validateCommitMessage(title)) {
+    problems.push(`title: ${problem}`);
+  }
+
+  // The template is one long HTML comment, so a pull request opened without a
+  // word written renders as nothing at all. That is what this looks for --
+  // not what the description says, only that there is one.
+  if (body.replaceAll(/<!--[\s\S]*?-->/g, '').trim() === '') {
+    problems.push(
+      'description: say why the change is needed and what it does. The ' +
+        'template is a comment, so a description that is only the template ' +
+        'shows a reader nothing.'
+    );
+  }
+
+  if (problems.length > 0) {
+    console.error(`This pull request is not ready to land:\n`);
+    for (const problem of problems) console.error(`  ${problem}`);
+    process.exitCode = 1;
+  } else {
+    console.log('Title and description are in order.');
+  }
+}

build/tasks/verify/verify-commits.mts

@@ -7,6 +7,7 @@
 
 import { execFileSync } from 'node:child_process';
 import {
+  checkSignOff,
   readTrailers,
   validateCommitMessage,
 } from '@openinf/portal/build/commit-message';
@@ -71,7 +72,10 @@ if (base === '') {
     }
 
     const message = git('log', '-1', '--format=%B', sha);
-    const problems = validateCommitMessage(message);
+    const problems = [
+      ...validateCommitMessage(message),
+      ...checkSignOff(message, git('log', '-1', '--format=%an <%ae>', sha)),
+    ];
 
     // git has the final say on what counts as a trailer, so the rules above
     // are cross-checked against it rather than trusted on their own. A

collections/_docs/handbook/style/commit-messages.md

@@ -109,6 +109,13 @@ throughout rather than about being understood.
 `PR-URL:` and `Reviewed-by:` are added when the commit lands. The others belong
 in the commit as you write it.
 
+`Signed-off-by:` is **required**, and has to name the commit's own author —
+`git commit -s` writes it for you. It is how you certify the [Developer
+Certificate of Origin][]: that you have the right to contribute this change. A
+tool cannot do that on your behalf, and neither can a bot, which is why the
+check compares the name against the author rather than merely looking for the
+line.
+
 ### Disclosing an AI assistant
 
 An AI tool that helped write a change is disclosed with `Assisted-by:`, and

package-scripts.yml

@@ -18,6 +18,9 @@ scripts:
     json: node build/tasks/verify/verify-json.mts
     liquid: node build/tasks/verify/verify-liquid.mts
     md: node build/tasks/verify/verify-md.mts
+    # Outside verify/ on purpose: it needs a pull request in the environment,
+    # and verify.all runs everything in that directory.
+    pullRequest: node build/tasks/verify-pull-request.mts
     svg: node build/tasks/verify/verify-svg.mts
     toml: node build/tasks/verify/verify-toml.mts
     ts: node build/tasks/verify/verify-ts.mts