feat(ci): a gate keeps commit messages in the house style (#929)

.github/workflows/publish.yml

@@ -197,6 +197,24 @@ jobs:
             | CHANGELOG_STDIN=1 \
               CHANGELOG_BASE_FILE="$RUNNER_TEMP/changelog-base.md" \
               npm run changelog:check --silent
+      # Commit message gate: every commit this PR would land meets the house
+      # style. The commit-msg hook catches most at authorship; this catches
+      # `--no-verify`, the GitHub web editor, and bots. It runs here because
+      # rebase-merge keeps each message verbatim, and a merged message cannot be
+      # fixed without force-pushing main. Messages come from the API for the
+      # same shallow-checkout reason as the changelog step.
+      - name: Commit messages
+        if: ${{ !cancelled() && github.event_name == 'pull_request' }}
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          set -o pipefail
+          # `tojson` makes each commit one line; gh prints a string result raw.
+          gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/commits" \
+            --paginate --jq '.[] | {sha, message: .commit.message} | tojson' \
+            > "$RUNNER_TEMP/commits.ndjson"
+          COMMIT_MESSAGES_FILE="$RUNNER_TEMP/commits.ndjson" \
+            npm run commits:check --silent
 
   # Split from `test` (#306): the integration suite is DB-bound (~4.5 min) and
   # was the long pole of the required check. Running it in parallel cuts the

.husky/commit-msg

@@ -0,0 +1,5 @@
+# Commit message gate: the house style in AGENTS.md under 'Commit workflow'.
+# A merged message is permanent, so it is checked here, where it is still cheap
+# to fix. The file is passed by environment variable, not argv: argv does not
+# survive npm reliably (see src/scripts/check-changelog.ts).
+COMMIT_MSG_FILE="$1" npm run commits:check --silent

AGENTS.md

@@ -85,6 +85,7 @@
 npm run openapi:failure-coverage # does every operation declare its handler's 4xx, or sit in noFailureModes? (gated)
 npm run prisma:guard-coverage # do Prisma writes that can violate a constraint translate the code? (#564; CI gates it)
 npm run changelog:check  # does this branch owe a CHANGELOG entry? (#386; CI runs it per-PR)
+npm run commits:check    # do this branch's commit messages meet the house style? (#929; commit-msg hook + CI per-PR)
 npm run env:coverage     # do the code, .env.default, docs/README.md and this file agree on env vars? (#682; gated)
 npm run db:migrate       # prisma migrate dev (requires interactive TTY)
 npm run db:seed          # recreate default user ranks after a DB reset; then go to /install
@@ -96,7 +97,7 @@
 
 ## Commit workflow
 
-The `.husky/pre-commit` hook runs on every commit and gates: lint-staged (`eslint --fix` + `prettier --write` on staged files), `npx tsc --noEmit`, `npm run typecheck:test`, and `npm run version:check`. So format/lint/type-check are enforced automatically — don't re-run them by hand as a separate pre-commit ritual.
+The `.husky/pre-commit` hook runs on every commit and gates: lint-staged (`eslint --fix` + `prettier --write` on staged files), `npx tsc --noEmit`, `npm run typecheck:test`, and `npm run version:check`. `.husky/commit-msg` runs `npm run commits:check` on the message. So format/lint/type-check and the message style are enforced automatically — don't re-run them by hand as a separate pre-commit ritual.
 
 What the hook does **not** cover, run yourself before committing:
 
@@ -118,7 +119,18 @@
 
    A cut therefore renames `[Unreleased]` to `## [<version>] — <date>`, leaving an empty one above it.
 
-4. Commit with a descriptive message following existing log style
+4. Commit with a message in the house style, which `npm run commits:check` gates at commit time and per-PR:
+
+   ```
+   fix(seed): the boot seed creates ranks and rules on a fresh install only (#882)
+   ```
+
+   - `type(scope): subject`, with a lowercase type from Conventional Commits; scopes may be comma-separated.
+   - The subject states the outcome, or uses the imperative. It stays on one line, at most 100 characters before the `(#NNN)` suffix.
+   - A blank line, then a body that explains why.
+   - A contract change carries `!` before the colon and a `BREAKING CHANGE:` footer. The gate warns on these but cannot fail them, since it reads text, not the diff.
+
+   Rebase-merge lands each message verbatim, and a merged message cannot be fixed without force-pushing `main`. Reword before the merge instead.
 
 > If you do run the checks manually (e.g. before staging, or committing with `--no-verify`): order matters — format before lint (Prettier violations surface as ESLint errors), and lint before type-check.
 
@@ -353,6 +365,7 @@
     userRankAccess.ts       # computeUserRankAccess/getUserRankAccess + canAccessForumLevel — resolved rank capability slice
     version.ts              # appVersion — running API version read from package.json at module load
     versionConsistency.ts   # Pure checkVersionConsistency (#79) — manifest is the source of truth; CLI wrapper in scripts/ feeds it the real surfaces
+    commitMessage.ts        # Pure checkCommitMessage — the commit message house style; the commit-msg hook and the per-PR CI step feed it via scripts/
     sentry.ts               # userContextFromRequest + sentryBeforeSend — Sentry scrubbing/user context
   types/
     auth.ts                 # AuthUser type (id, userRankId, userRankLevel; optional contributed/consumed)

CHANGELOG.md

@@ -39,6 +39,10 @@ All notable changes to stellar-api are documented here.
   - `POST /users/{id}/invite-subtree/action` `{ action, reason, expectedCount }` applies one action in a single transaction. It needs `invites_manage` plus the action's own permission (`users_edit`, `users_disable` or `invites_edit`); without that it answers `403`. It answers `409` if the subtree no longer has `expectedCount` members.
   - Every member gets a note with the reason. Members the action changes are audited as the single-member routes audit them, and the root gets one run row listing the ids changed. No member is messaged, and there is no bulk undo.
 - `POST /users/{id}/disable` now writes through the same shared disable as the subtree tool. Its responses are unchanged.
+- **A gate keeps commit messages in the house style: `npm run commits:check`** (#929), run by a new `commit-msg` hook and per-PR in CI. Rebase-merge lands each message verbatim, so a defect that merges is permanent.
+  - **It fails** a subject with no lowercase Conventional Commits type, one wrapped onto line 2, one longer than 100 characters before its `(#NNN)` suffix, a `WIP` subject, and on a PR a leftover `fixup!` commit.
+  - **It warns** on past-tense or `-ing` subjects, and on a breaking change marked only one way: `!` with no `BREAKING CHANGE:` footer, or the reverse.
+  - Existing history is not rewritten. Of the last 400 commits on `main`, 8 would fail.
 
 ### Changed
 

package.json

@@ -29,6 +29,7 @@
     "version:check": "ts-node -T src/scripts/check-version-consistency.ts",
     "env:coverage": "ts-node -T src/scripts/check-env-coverage.ts",
     "changelog:check": "ts-node -T src/scripts/check-changelog.ts",
+    "commits:check": "ts-node -T src/scripts/check-commit-messages.ts",
     "db:generate": "prisma generate",
     "db:erd": "prisma generate --generator erd",
     "db:migrate": "prisma migrate dev",

src/lib/commitMessage.spec.ts

@@ -0,0 +1,124 @@
+import { checkCommitMessage, cleanMessage, SUBJECT_MAX } from './commitMessage';
+
+const rules = (raw: string, allowAutosquash = false): string[] =>
+  checkCommitMessage(raw, { allowAutosquash }).map((f) => f.rule);
+
+const errors = (raw: string): string[] =>
+  checkCommitMessage(raw)
+    .filter((f) => f.severity === 'error')
+    .map((f) => f.rule);
+
+describe('checkCommitMessage', () => {
+  it.each([
+    "feat(invites): staff act on a member's whole invite subtree (#639)",
+    'fix(seed): the boot seed creates ranks and rules on a fresh install only (#882)',
+    'fix(assets,stats): guard every assetStore.ts and statsHistory.ts write (#843, #844)',
+    'fix(releases): delete a release with no contributions (#793)',
+    'docs(context): the ratio policy disabled state is DOWNLOAD_DISABLED',
+    'chore(release): the 0.10.2 cut',
+    'chore(deps): update npm non-major'
+  ])('passes house-style subject %p', (subject) => {
+    expect(rules(`${subject}\n\nWhy it changed.\n`)).toEqual([]);
+  });
+
+  it('passes a body, comment lines and everything below the scissors', () => {
+    const raw = [
+      'fix(auth): the session list marks the current session (#277)',
+      '',
+      'Body.',
+      '# Please enter the commit message',
+      '# ------------------------ >8 ------------------------',
+      'diff --git a/x b/x'
+    ].join('\n');
+    expect(rules(raw)).toEqual([]);
+  });
+
+  describe('errors', () => {
+    it('flags a subject wrapped onto line 2 (8b27cf5)', () => {
+      const raw =
+        'fix(auth): GET /auth/sessions never sent isCurrent, so the UI could not\n' +
+        ' mark the session you were using\n\nBody.';
+      expect(errors(raw)).toEqual(['subject-wrapped']);
+    });
+
+    it('measures length without the issue suffix', () => {
+      const atLimit = `fix(x): ${'a'.repeat(SUBJECT_MAX - 'fix(x): '.length)}`;
+      expect(errors(`${atLimit} (#843, #844)`)).toEqual([]);
+      expect(errors(`${atLimit}a (#843)`)).toEqual(['subject-too-long']);
+    });
+
+    it.each([
+      ['initial import', 'type-missing'],
+      ['Feat: add retry', 'type-case'],
+      ['release: promote develop → main (v0.5.4)', 'type-unknown'],
+      ['fix: ', 'empty-subject'],
+      ['feat(x): WIP retry', 'wip']
+    ])('flags %p as %s', (subject, rule) => {
+      expect(errors(subject)).toContain(rule);
+    });
+
+    it('flags an empty message', () => {
+      expect(errors('# only a comment\n\n')).toEqual(['empty']);
+    });
+
+    it('refuses fixup! commits on a pull request but accepts them at commit time', () => {
+      expect(errors('fixup! feat(x): y')).toEqual(['autosquash-pending']);
+      expect(rules('fixup! feat(x): y', true)).toEqual([]);
+    });
+
+    it('leaves subjects git writes itself alone', () => {
+      expect(rules("Merge branch 'main' into feature")).toEqual([]);
+      expect(rules('Revert "feat(x): y"\n\nThis reverts commit abc.')).toEqual(
+        []
+      );
+    });
+  });
+
+  describe('warnings', () => {
+    it.each([
+      ['fix: added retry', 'past-tense'],
+      ['fix: adds retry', 'third-person'],
+      ['fix: adding retry', 'gerund'],
+      ['fix: add retry.', 'subject-period'],
+      ['fix():  add retry', 'empty-scope']
+    ])('warns on %p as %s', (subject, rule) => {
+      const findings = checkCommitMessage(subject);
+      expect(findings.map((f) => f.rule)).toContain(rule);
+      expect(findings.every((f) => f.severity === 'warning')).toBe(true);
+    });
+
+    it('wants a footer on a subject marked breaking (1315669)', () => {
+      expect(
+        rules(
+          'feat(settings)!: privacy is five flags, not a level (#586)\n\n' +
+            'Breaking change: `UserSettings.paranoia` is removed.'
+        )
+      ).toEqual(['breaking-footer']);
+      expect(
+        rules(
+          'feat(settings)!: privacy is five flags, not a level (#586)\n\n' +
+            'BREAKING CHANGE: read the five show* flags instead.'
+        )
+      ).toEqual([]);
+    });
+
+    it('wants `!` on a body that describes a breaking change (38018f5)', () => {
+      expect(
+        rules(
+          "fix(comments): comment threads follow their page's access (#697)\n\n" +
+            '- GET /comments requires context and pageId (breaking)'
+        )
+      ).toEqual(['breaking-unmarked']);
+    });
+  });
+});
+
+describe('cleanMessage', () => {
+  it('drops CRLFs, a BOM and surrounding blank lines', () => {
+    expect(cleanMessage('\ufeff\r\nfix: x\r\n\r\nbody\r\n\r\n')).toEqual([
+      'fix: x',
+      '',
+      'body'
+    ]);
+  });
+});