🏗️✨：check pull request titles and sign-offs - #1813
Merged
Merged
Conversation
The title becomes the subject of the commit that lands, because the squash takes it verbatim, and until now nothing checked it until the commits already existed. It answers to the same rules as a subject now, through the same module, so the two cannot drift. The description has to say something. 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 words say. Both arrive through the environment rather than the command line, which is how nodejs.org does it and for the reason they do it: a title is written by whoever opened the pull request, and one pasted into a shell can run as code. Bots are left to their own formats, as with commits. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
✅ 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}
| // 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() === '') { |
This repository has been signing off for a long time -- 1055 commits carry the trailer, and 19 of the 40 before this session. The handbook described it as one trailer among several without ever saying it was expected. It says so now, and says why the name has to be the author's: a tool cannot certify the DCO for you, and neither can a bot. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
DerekNonGeneric
force-pushed
the
feat/lint-pull-requests
branch
from
August 13, 2026 11:46
fff1b74 to
a92a126
Compare
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 2, plus the DCO check you asked me to consider. This pull request is checked by the workflow it adds — the run below is the demonstration.
The title is the landed subject, and nothing checked it
squash_merge_commit_titleisPR_TITLE, so the title becomes the commit subject verbatim.verify.commitsnever saw it, because a title is not a commit.verify.pullRequestnow holds it to the same rules through the same module, so the two cannot drift.The description only has to exist. The template is one long HTML comment, so a pull request opened without a word written renders as nothing at all, and that is what this catches — not what the words say.
Probed with eight titles and bodies:
fix the thing：🏗️🔧：fix the thing #1810PR-URL:carries that🔧：fix the thing🏗️🔧：fix it"; touch /tmp/pwned; #What I took from nodejs.org
Their
pull-request-policy.ymlchecks the title only — no body check at all — so this is already stricter than their bar. Four things copied:env:, never interpolated intorun:. A title is written by whoever opened the pull request; pasting one into a shell is how a title comes to run as code. That last probe above is the test for it.types: [opened, edited, synchronize, reopened, ready_for_review]— withoutedited, a title fixed after opening is never re-checked, which is the statelint-and-test.ymlis in today.permissions: contents: read, andconcurrencywithcancel-in-progress, since a title gets fixed two or three times in a row.No install step. The task imports one module from this package and nothing from
node_modules, and node resolves a self-reference on its own — verified in a worktree with nonode_modulespresent. So the job is a checkout and a node invocation.The DCO: you already had one, and I had stopped
Going looking, as you suggested: 1055 commits in this repository carry
Signed-off-by, including 19 of the 40 immediately before this session. All nine I landed today carry none. I dropped a long-standing convention without noticing, and that is the real answer to whether a DCO check is worth having.Two other things the history shows, both of which the check now stops:
Signed-off-by: @OpenINFbot— that is not an identity at all.So
verify.commitsnow requires each non-bot commit to carry aSigned-off-by:naming its own author, which catches both. Five cases probed: missing, matching, signed by a bot, the@OpenINFbotform, and the author plus a co-signer — only the last two of those pass, correctly.nodejs/nodeitself enforces nothing here; the DCO is asserted by opening a pull request against a template that contains the certificate. This is stricter, which I think is right for a repository where most commits are written with a tool:Assisted-by:names the tool,Signed-off-by:names the person answerable for it.One artifact worth knowing about
The sign-off names the branch commit's author,
Derek Lewis <DerekNonGeneric@inf.is>, which is your local git identity. GitHub rewrites the author when it squashes, to the address on your account —dereknongeneric@open.inf.is— so a landed commit's author and its sign-off will not be identical strings.verify.commitsonly ever reads a branch, so it never sees this and never fails on it. Aligning the two would mean changinguser.email, and I have not touched it:gpg.formatissshand~/.ssh/allowed_signersbinds your key toDerekNonGeneric@inf.is, so changing the address would break local signature verification. Adding the second address to that file, or to your GitHub account, is the tidier fix if you want them to match.Verification
nps test— 16/16 tasks, 45/45 testsverify.pullRequestsits outsidebuild/tasks/verify/on purpose:verify.allglobs that directory, and a task that needs a pull request in its environment would either fail locally or have to pass silently. Confirmed it is not picked up.