🏗️✨：validate commit messages - #1810
Merged
Merged
Conversation
DerekNonGeneric
force-pushed
the
test/unit-tests-for-glob
branch
from
August 13, 2026 01:20
3c2b074 to
e7f33b6
Compare
DerekNonGeneric
force-pushed
the
feat/validate-commit-messages
branch
2 times, most recently
from
August 13, 2026 01:58
b1642c5 to
a50fe17
Compare
DerekNonGeneric
force-pushed
the
test/unit-tests-for-glob
branch
from
August 13, 2026 02:35
e7f33b6 to
9487f49
Compare
DerekNonGeneric
force-pushed
the
feat/validate-commit-messages
branch
from
August 13, 2026 02:35
f5c5de6 to
9aab21a
Compare
DerekNonGeneric
force-pushed
the
test/unit-tests-for-glob
branch
from
August 13, 2026 03:10
9487f49 to
bf48347
Compare
DerekNonGeneric
force-pushed
the
feat/validate-commit-messages
branch
2 times, most recently
from
August 13, 2026 03:12
4eda330 to
5ae08a1
Compare
DerekNonGeneric
force-pushed
the
test/unit-tests-for-glob
branch
from
August 13, 2026 03:26
bf48347 to
1837a6e
Compare
Issue #1539 asked for commit message validation and named the lengths. This is the rest of the format written down as rules: the category and action emoji from the pull request template, the ideographic colon, and a trailer block git will actually read. The vocabulary is duplicated from the template rather than parsed out of it, with a test that fails if the two drift apart. Assisted-by: Claude-Code:claude-opus-5 Refs: #1539
Compares the branch against its base, and cross-checks each verdict against `git interpret-trailers --parse` -- git has the final say on what a trailer is, so the rules are held against it rather than trusted alone. Runs unconditionally in CI, since every pull request has commit messages whatever files it touches, and the checkout needs full history to see the base branch at all. Assisted-by: Claude-Code:claude-opus-5 Fixes: #1539
Trailer tokens are now matched case-sensitively. git and GitHub would take them either way, so this buys a history that reads the same throughout rather than anything functional. The emoji complaint was the wrong one. 🏗🔧 says infrastructure fix as plainly as 🏗️🔧 does, and rejecting it as an unknown category was misleading: it is the same character without the U+FE0F that asks for the emoji rendering, so the message now says that. A selector on an emoji that does not need one is reported the same way, since either mistake leaves two spellings of one symbol. Assisted-by: Claude-Code:claude-opus-5
The handbook page for this was a placeholder. It now documents what `nps verify.commits` enforces and why, including the three ways a trailer block goes unread, and keeps the Classification anchor that colons.md links to. The pull request template points at it, and says outright that the emoji want copying rather than typing. Assisted-by: Claude-Code:claude-opus-5
The rule stands: an emoji carries the invisible selector when it would otherwise be drawn as flat text, and does not when it would not. A test holds the whole vocabulary to that, both ways. What is gone is the explaining. Contributing should not require reading about code points, so the handbook says to copy the emoji from the table and the check names the one to copy and links to it. The reasoning lives in this repository's history, not in a contributor's way. Assisted-by: Claude-Code:claude-opus-5
git generates Signed-off-by, GitHub documents Co-authored-by, and the kernel established the form for all of them: the `by` is lowercase. nodejs is the one place that writes Reviewed-By, and following it here put this repository at odds with its own landed history, which had the lowercase form all along. Assisted-by: Claude-Code:claude-opus-5
`Co-authored-by` says a person wrote part of the change. An assistant is not a person, and the trailer for it is `Assisted-by`, which the Linux kernel defines as agent-name:model-version and nodejs/node lands in that form -- no address, since there is nobody to address. The check enforces that shape, so the form this repository had been using is now rejected by name. Also documented: an assistant never gets a `Signed-off-by`, because only a human can certify the DCO. Assisted-by: Claude-Code:claude-opus-5
Three false positives and one false negative, all found in review before this landed, all now covered by tests that check the rules against `git interpret-trailers --parse` on the same input. A last paragraph that is not all trailers is prose, and git reads no trailers in it -- so a commit closing on “Note: only staging.” is no longer refused. A trailer folded onto an indented line is git's own syntax and is now read as one trailer. A carriage return no longer makes the blank line look non-blank, which had been hiding every trailer problem behind it. The cross-check in the task compared git's count against a different notion of a trailer than the rules used, so it could fail a commit that had nothing wrong with it. Both sides now read the same block. Assisted-by: Claude-Code:claude-opus-5
DerekNonGeneric
force-pushed
the
feat/validate-commit-messages
branch
from
August 13, 2026 03:29
5ae08a1 to
ab70f37
Compare
✅ Deploy Preview for gh-pages-openinf ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * @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.length > 0 && | ||
| TRAILER_LINE.test(last[0] ?? '') && | ||
| last.every( | ||
| (line) => TRAILER_LINE.test(line) || CONTINUATION_LINE.test(line) |
| // meant as one of ours, since it is going unread. | ||
| if (block.length === 0) { | ||
| for (const line of last) { | ||
| const token = line.match(TRAILER_LINE)?.groups?.token ?? ''; |
| } | ||
| } else { | ||
| for (const line of block) { | ||
| const found = line.match(TRAILER_LINE)?.groups; |
|
|
||
| if ( | ||
| token.toLowerCase() === 'assisted-by' && | ||
| !ASSISTED_BY_VALUE.test(found?.value ?? '') |
| // about the message itself. Carriage returns say nothing either: git reads | ||
| // trailers through them, so a message written on Windows must not be judged | ||
| // differently from the same message written anywhere else. | ||
| const lines = message.replace(/[\r\n]+$/, '').split(/\r?\n/); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of the landing plan, and closes #1539. Subject limit 50, and the
#NNNNsuffix is now rejected rather than expected.Stacked on #1809 → #1804, since the rules get unit tests and the runner for those arrives in #1809.
What it checks
build/shared/commit-message.mtsholds the format as rules;verify.commitsapplies them to every commit between the branch and its base.：(U+FF1A), non-empty description#NNNN, no trailing full stop, no space after the colonCo-authored-by,Signed-off-by,Assisted-by,PR-URL,Fixes,Refs,Reviewed-by---line; nothing stranded above the final paragraphThe emoji vocabulary is the one in
.github/PULL_REQUEST_TEMPLATE.md— 12 categories, 8 actions — duplicated into the module rather than parsed out of a markdown comment, with a test that fails if the two drift.Trailer order and spelling
The order is what nodejs/node actually lands, not invented.
MetadataGenerator.getMetadata()in node-core-utils appends its metadata, so whatever the branch commit already carried —Co-authored-by,Signed-off-by— ends up abovePR-URL. Confirmed against real commits in that repo.The spelling is not Node's. Checked each source directly:
--signoffSigned-off-byinterpret-trailersdocumentationReviewed-bysubmitting-patches.rstReviewed-by,Acked-by,Co-developed-byCo-authored-bymetadata_gen.jsReviewed-ByReviewed-bygit, GitHub and the kernel agree on a lowercase
by; nodejs is the only one that capitalises it. An earlier revision of this branch followed nodejs and so put the project at odds with its own history — corrected, and the handbook now records which authority each spelling comes from.Case is checked. git and GitHub would match a token either way, so this buys nothing functional; it buys a history that reads the same throughout. Every open branch was rewritten to suit — see below.
The emoji spelling
Some of these have two spellings that look identical, because of an invisible character that may or may not be present. The rule is the obvious one: an emoji carries it when the character would otherwise be drawn as flat monochrome text, and does not carry it when the character is already drawn in colour. Nothing else is a valid spelling, in either direction.
A test holds the whole vocabulary to that, both ways, so a future addition cannot get it wrong. I also swept every tracked file for the same two mistakes — the pull request template included, where all 20 check out by code point. The only hits anywhere are two deliberately wrong fixtures in the test file, commented as such.
What is not here is any of that explanation. Contributing should not require reading about code points, so the handbook says to copy the emoji from the table, and the check names the one to copy and links to it:
Disclosing an AI assistant
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>was wrong twice over: the wrong model, and the wrong trailer. Traced it to the source.The Linux kernel's
coding-assistants.rstdefines the format, and it takes no address, because it names a tool rather than a person:The optional tools are for specialized analysis tools; everyday ones — git, a compiler, an editor — are explicitly left out. Real nodejs/node commits land exactly that:
Assisted-by: codex:gpt-5.6-sol, sitting afterSigned-off-byand beforePR-URL, which is where it now goes in our order.core-validate-commithas a rule for it, though that rule only checks position, not content — so the content check here is ours.The policy behind it matters more than the format. Per the kernel and the OpenJS Foundation, an assistant must not carry a
Signed-off-by: only a human can certify the DCO, and disclosure is not a transfer of responsibility. The handbook says so, andCo-authored-byis documented as being for people.So the value is now
Assisted-by: Claude-Code:claude-opus-5, and the old form is rejected by name:All 30 commits across the eight open branches were rewritten to it.
Note
One thing to decide, from
doc/contributing/ai-guidelines.md: nodejs recommends against naming for-profit brands in commit messages, since a commit message is part of the codebase and can be mined for marketing. Their suggestion is to anonymize —a closed-source coding agent— or to name the brand only in the pull request description. That conflicts with the kernel's agent:model precision, and it is your call which way this repository goes. I have used the precise form, since your landed history already names the model.git has the final say on trailers
The rules describe git's behaviour, so they are cross-checked against it: for each commit the task also runs
git interpret-trailers --parseand reports a disagreement over how many trailer lines are readable. If my reimplementation of the last-paragraph rule ever drifts from the tool it describes, that shows up as a failure rather than as a wrong verdict.One consequence worth pinning down, and now a test: a bare URL as the final paragraph is a trailer as far as git is concerned —
https://example.com/pparses as a trailer calledhttps. So the task warns about it.It caught its own author
The first version of the second commit here used
Closes:for the issue. Not in the vocabulary —Fixes:is the one — and the task said so before it was pushed.Documented, not just enforced
collections/_docs/handbook/style/commit-messages.mdwas a placeholder. It now carries the format and the reasoning: the subject rules, both emoji tables, the trailer order and where each spelling comes from, and the three ways a trailer block goes unread. It keeps theClassificationanchor thatcolons.mdlinks to.The pull request template points at the page, notes that the title becomes the commit subject and so takes the same rules, and says to copy the emoji rather than type them.
Every open pull request was brought into line
Ran the rules over all eight open branches, then rewrote the messages that failed. Message-only rewrites — no tree changed, and both stack relationships were re-checked afterwards.
Co-authored-by: Claude … <…>Assisted-by: Claude-Code:claude-opus-5format the files that were escaping the formatters(53)…that escaped the formatters(47)wait a week before dependabot proposes a new version(55)hold dependabot to a week before proposing(45)raise renovate's wait to a week & apply it beyond npm(56)hold renovate to a week, and beyond npm(42)hold pnpm to the same week before installing a version(57)hold pnpm to the same week at install time(45)stop dependabot proposing what renovate already proposes(59)stop dependabot duplicating renovate(39)verify only the files meant to be run are executable(55)verify only programs are executable(38)♻️：kebab-case the two asset names the new task flags(51)🏗️♻️：kebab-case two asset names(29)That last one also had
♻️where a category belongs — an action alone says what is happening without saying where — so it gained the 🏗️ category.The merge-order constraint I flagged earlier is gone: every open pull request passes on its own, in any order.
Verification
nps test— 15/15 tasks, 40/40 tests