From b9b9c44c0c6bb0f7895d3ec6b9b83b5b47fb6a25 Mon Sep 17 00:00:00 2001 From: Derek Lewis Date: Sun, 13 Sep 2026 00:30:48 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=97=EF=B8=8F=F0=9F=94=A7=EF=BC=9Akeep?= =?UTF-8?q?=20a=20tool=20out=20of=20the=20author=20credit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handbook has always said `Co-authored-by:` is for people and that an assistant belongs in `Assisted-by:` instead. Nothing checked it, and a commit went out this week crediting a model as a co-author. That is not a formatting slip. Authorship is a claim only a person can make: the Developer Certificate of Origin is certified by whoever wrote the code, and a tool certifies nothing. The trailer that discloses one says what was used. The trailer that credits one says who is answerable, and a model cannot be. An agent writing its own commit message reaches for that trailer out of habit, because that is what most of the code it learnt from does, and a reviewer reading a long message is exactly who does not notice one line at the bottom. Once it lands it is in the history for good. So the check refuses it, on three things: the `[bot]` suffix, which GitHub reserves so that no person can hold it; the addresses the agents commit under, which are theirs alone and not the ones their staff use; and a handful of product names. Only the last can reach a person, and realistically only `claude`, which is also a name people have. That is the trade, and if it ever refuses a real co-author the pattern gets narrowed rather than the credit dropped. Renovate and Dependabot are unaffected, since their commits are skipped whole and never reach this. Nor does an agent that commits as a `[bot]` account need naming, which is why Devin and Gemini are absent: both are ordinary names before they are products. A trailer whose value runs onto an indented line is read whole, which it was not. The continuation was dropped before any check saw it, so these two were one trailer to git and `git interpret-trailers --parse` printed both as the same line, but only the first was refused: Co-authored-by: Some Person Co-authored-by: Some Person The address is the half that says who a trailer names, it sits at the end, and so it is the half a fold hides. Sign-off had the same hole from the other side: a wrapped `Signed-off-by:` lost the address the author is compared against. The continuation is now joined onto the value the way git joins it. Signed-off-by: Derek Lewis Assisted-by: Claude-Code:claude-opus-5 --- CONTRIBUTING.md | 8 +++ build/shared/commit-message.mts | 63 ++++++++++++++++--- build/shared/commit-message.test.mts | 92 +++++++++++++++++++++++++++- 3 files changed, 154 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d472a76..d8b4076 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -169,6 +169,14 @@ Developer Certificate of Origin, reproduced in full in the pull request template, and only the author can certify it -- an assistant discloses itself with `Assisted-by:` and signs nothing. `git commit -s` writes the line for you. +`Co-authored-by:` is for people, and the check refuses one naming an assistant +or a bot account. Authorship is a claim only a person can make, and an agent +writing its own commit message reaches for that trailer by habit. It refuses on +the `[bot]` suffix, which GitHub reserves so that no person can hold it, on the +addresses the agents commit under, and on a handful of product names. Only the +last can reach a person. If it ever refuses a real co-author, narrow the pattern +in the same pull request rather than dropping the credit. + `pnpm run lint:commits` holds every commit on your branch to all of this, and cross-checks its own reading of the trailers against `git interpret-trailers`, so the rules cannot quietly drift from the tool they describe. Commits written diff --git a/build/shared/commit-message.mts b/build/shared/commit-message.mts index cdbe172..fa38462 100644 --- a/build/shared/commit-message.mts +++ b/build/shared/commit-message.mts @@ -119,6 +119,28 @@ const TRAILER_LINE = /^(?[A-Za-z][\w-]*):[ \t]*(?.*)$/; */ const ASSISTED_BY_VALUE = /^[^\s:]+:\S+( \S+)*$/; +/** + * A `Co-authored-by` value naming something that is not a person. Authorship + * is a claim only a person can make: the Developer Certificate of Origin is + * certified by whoever wrote the code, and a tool certifies nothing. An + * assistant is disclosed with `Assisted-by` instead, which says what was used + * rather than who wrote it. + * + * `[bot]` cannot catch a person: GitHub reserves the suffix and no account may + * be named with it, which is the same fact the commit checker relies on to + * recognize a bot author. The agents' noreply addresses are theirs alone. Only + * the product names can reach a person, and only one of them realistically: + * Claude is a name people have. That is the trade accepted here, since the + * alternative is a tool standing in the history as an author. If it ever + * refuses a real co-author, narrow the pattern rather than drop the credit. + * + * An agent whose integration commits as a `[bot]` account needs no name here. + * Renovate and Dependabot are out of reach either way: their commits are + * skipped whole, so their own `[bot]` co-authors are not this check's business. + */ +const TOOL_COAUTHOR = + /\[bot]|\bnoreply@(?:anthropic|openai)\.com\b|\b(?:aider|chatgpt|claude|codex|copilot|cursor)\b/i; + /** git folds a trailer whose value runs onto an indented line beneath it. */ export const CONTINUATION_LINE = /^\s/; @@ -199,7 +221,25 @@ const trailerBlockOf = (paragraphs: string[][]) => { (line) => TRAILER_LINE.test(line) || CONTINUATION_LINE.test(line) ); - return isBlock ? last.filter((line) => !CONTINUATION_LINE.test(line)) : []; + // A folded trailer is one trailer, not a trailer and a stray line. git + // joins the indented line onto the value, and `git interpret-trailers + // --parse` prints the two back as one, so this does the same. Dropping the + // continuation instead would put whatever got wrapped out of reach of every + // check below -- an address most of all, which is the half that says who a + // trailer names. + return isBlock + ? last.reduce((folded, line) => { + const previous = folded.at(-1); + + if (previous !== undefined && CONTINUATION_LINE.test(line)) { + folded[folded.length - 1] = `${previous} ${line.trim()}`; + } else { + folded.push(line); + } + + return folded; + }, []) + : []; }; /** @@ -371,6 +411,15 @@ const checkTrailers = (lines: string[]) => { ); } + if ( + token.toLowerCase() === 'co-authored-by' && + TOOL_COAUTHOR.test(found?.value ?? '') + ) { + problems.push( + `“Co-authored-by: ${found?.value}” credits a tool with authorship: an assistant is disclosed with “Assisted-by:” and co-authors nothing` + ); + } + // Case is part of the spelling. git and GitHub would match these either // way, so this is about a history that reads the same throughout rather // than about being understood. @@ -423,12 +472,12 @@ export function validateCommitMessage(message: string) { problems.push('the line after the subject has to be blank'); } - // Where the trailer block starts, or past the end when there is none. A - // trailer is one line by construction here: git would read a folded value, - // but `readTrailers` keeps only the token line, so wrapping a long - // `Signed-off-by:` to fit would put the author out of reach of the sign-off - // check. The block is exempt from the width limit rather than made to fit - // inside it, which is also what the limit is for -- prose that is read. + // Where the trailer block starts, or past the end when there is none. The + // block is exempt from the width limit rather than made to fit inside it, + // which is what the limit is for -- prose that is read. A trailer long + // enough to need wrapping is wrapped by nobody here, and one that arrives + // wrapped anyway is read whole, since `trailerBlockOf` folds it back the + // way git does. const paragraphs = paragraphsOf(rest); const blockStart = trailerBlockOf(paragraphs).length > 0 diff --git a/build/shared/commit-message.test.mts b/build/shared/commit-message.test.mts index 53ca224..f3ce70e 100644 --- a/build/shared/commit-message.test.mts +++ b/build/shared/commit-message.test.mts @@ -14,6 +14,7 @@ import { ACTIONS, BODY_MAX, CATEGORIES, + checkSignOff, readTrailers, validateCommitMessage, } from './commit-message.mts'; @@ -250,6 +251,92 @@ describe('validateCommitMessage: the trailers', () => { ); }); + test('accepts a person as a co-author', () => { + deepStrictEqual( + validateCommitMessage( + '🏗️🔧：fix it\n\nCo-authored-by: Ada Lovelace ' + ), + [] + ); + }); + + test('rejects an assistant credited as a co-author', () => { + // The mistake this exists to stop. An assistant is disclosed with + // `Assisted-by`, and putting it here instead claims it wrote the code and + // certified the Developer Certificate of Origin, neither of which a tool + // can do. + match( + soleProblem( + '🏗️🔧：fix it\n\nCo-authored-by: Claude Opus 5 \nSigned-off-by: Ada Lovelace ' + ), + /credits a tool with authorship/ + ); + }); + + test('rejects an assistant co-author with no address to give it away', () => { + match( + soleProblem('🏗️🔧：fix it\n\nCo-authored-by: GitHub Copilot'), + /credits a tool with authorship/ + ); + }); + + test('rejects a bot account as a co-author', () => { + match( + soleProblem( + '🏗️🔧：fix it\n\nCo-authored-by: some-app[bot] <1234+some-app[bot]@users.noreply.github.com>' + ), + /credits a tool with authorship/ + ); + }); + + test('leaves a person at one of those companies alone', () => { + // Only the agents' `noreply` address is theirs. Somebody who works there + // and writes part of a change is a co-author like anybody else. + deepStrictEqual( + validateCommitMessage( + '🏗️🔧：fix it\n\nCo-authored-by: Ada Lovelace ' + ), + [] + ); + }); + + test('leaves a person whose name an agent also goes by alone', () => { + // `devin` and `gemini` are names before they are products, and both of + // those integrations commit as `[bot]` accounts anyway, so neither has to + // be named here. `claude` is the one kept, and the one that can misfire. + deepStrictEqual( + validateCommitMessage( + '🏗️🔧：fix it\n\nCo-authored-by: Devin Gemini ' + ), + [] + ); + }); + + test('rejects an assistant whose address is on a folded line', () => { + // git joins the indented line onto the value and reads one trailer, so a + // wrapped address is still the address. Keeping only the token line would + // have made folding a way around the check. + match( + soleProblem( + '🏗️🔧：fix it\n\nCo-authored-by: Some Person\n ' + ), + /credits a tool with authorship/ + ); + }); + + test('reads a folded sign-off as the whole address', () => { + // The same fold, on the trailer whose value the sign-off check compares + // against the author. Dropping the continuation put the author out of + // reach and the sign-off passed for nobody. + deepStrictEqual( + checkSignOff( + '🏗️🔧：fix it\n\nSigned-off-by: Ada Lovelace\n ', + 'Ada Lovelace ' + ), + [] + ); + }); + test('rejects a token this project does not use', () => { match( soleProblem('🏗️🔧：fix it\n\nCloses: https://x/1'), @@ -372,8 +459,9 @@ describe('the vocabulary', () => { describe('the width limit', () => { test('leaves a trailer that cannot be wrapped alone', () => { - // Folding this to fit would put the author on a continuation line, where - // `readTrailers` does not look and so `checkSignOff` could not match it. + // Trailers are metadata rather than prose, and the limit is for prose. + // Folding this one to fit would be read correctly either way, but there + // is no reason to make somebody wrap an address to please a linter. const long = 'Signed-off-by: Christopher Alexander Montgomery ' + '';