Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions build/shared/commit-message.mts
Original file line number Diff line number Diff line change
Expand Up @@ -120,26 +120,33 @@ const TRAILER_LINE = /^(?<token>[A-Za-z][\w-]*):[ \t]*(?<value>.*)$/;
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.
* A trailer value naming something that is not a person. Both trailers that
* name one are held to it: `Co-authored-by`, which says who wrote the change,
* and `Signed-off-by`, which certifies the Developer Certificate of Origin.
* Neither is a claim a tool can make. An assistant is disclosed with
* `Assisted-by` instead, which says what was used rather than who is
* answerable.
*
* `[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
* recognize a bot author. It is matched where an account name ends -- before
* the `@` of an address, or at the end of the value -- because a trailer is
* free text and not an account name, so an unanchored `[bot]` would also find
* one sitting inside somebody's name. That mattered little while only a
* co-author was read this way and matters now: a sign-off cannot be dropped to
* get around a false positive, the way a co-author credited in error could be.
* 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.
* refuses a real contributor, narrow the pattern rather than drop the trailer.
*
* 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;
const TOOL_IDENTITY =
/\[bot](?=@|\s*(?:<[^>]*>)?\s*$)|\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/;
Expand Down Expand Up @@ -413,13 +420,28 @@ const checkTrailers = (lines: string[]) => {

if (
token.toLowerCase() === 'co-authored-by' &&
TOOL_COAUTHOR.test(found?.value ?? '')
TOOL_IDENTITY.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`
);
}

// `checkSignOff` compares this trailer against the author, which catches
// a tool signing on somebody's behalf and nothing else: an agent that
// commits under its own name satisfies it, because the two agree. The
// trailer has to name a person whoever the author is, and saying so here
// leaves an agent-authored commit no spelling that passes -- the sign-off
// is refused as a tool, or it is refused for not naming the author.
if (
token.toLowerCase() === 'signed-off-by' &&
TOOL_IDENTITY.test(found?.value ?? '')
) {
problems.push(
`“Signed-off-by: ${found?.value}” certifies the Developer Certificate of Origin as a tool: only the person answerable for the change can sign it off, and an assistant is disclosed with “Assisted-by:”`
);
}

// 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.
Expand Down Expand Up @@ -521,10 +543,12 @@ export function validateCommitMessage(message: string) {
}

/**
* 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.
* Checks that the sign-off names the commit's own author. Whether it names a
* person at all is `validateCommitMessage`'s to say, since that question needs
* no author to answer and the trailer is read the same way in a pull request
* body the commit queue is about to land. The two together leave an
* agent-authored commit nowhere to stand: sign off as itself and the trailer
* names a tool, sign off as anybody else and it is not the author.
* @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.
Expand Down
88 changes: 88 additions & 0 deletions build/shared/commit-message.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,94 @@ describe('validateCommitMessage: the trailers', () => {
);
});

test('rejects an assistant signing off as itself', () => {
// The hole the author comparison left open. An agent that commits under
// its own name and signs off to match is consistent with itself, so
// `checkSignOff` finds nothing to say, and the message went through.
const message =
'🏗️🔧:fix it\n\nSigned-off-by: Claude <noreply@anthropic.com>';

deepStrictEqual(
checkSignOff(message, 'Claude <noreply@anthropic.com>'),
[]
);
match(
soleProblem(message),
/certifies the Developer Certificate of Origin as a tool/
);
});

test('rejects an assistant signing off for somebody else, twice over', () => {
// Refused by both halves now: it does not name the author, and it does
// not name a person. Only the second survives a change of author.
const message =
'🏗️🔧:fix it\n\nSigned-off-by: Claude Code <noreply@anthropic.com>';

match(
checkSignOff(message, 'Ada Lovelace <ada@example.com>')[0] ?? '',
/but the author is/
);
match(
soleProblem(message),
/certifies the Developer Certificate of Origin as a tool/
);
});

test('rejects a bot account signing off', () => {
match(
soleProblem(
'🏗️🔧:fix it\n\nSigned-off-by: some-app[bot] <1234+some-app[bot]@users.noreply.github.com>'
),
/certifies the Developer Certificate of Origin as a tool/
);
});

test('leaves a person with [bot] inside their name or address alone', () => {
// `[bot]` is read where an account name ends, not wherever it appears. A
// trailer is free text, so an unanchored match also finds one sitting in
// the middle of somebody's name -- and a sign-off, unlike a co-author
// credit, cannot be dropped to get past a false positive.
deepStrictEqual(
validateCommitMessage(
'🏗️🔧:fix it\n\nSigned-off-by: Ada [bot] Smith <ada@example.com>'
),
[]
);
deepStrictEqual(
validateCommitMessage(
'🏗️🔧:fix it\n\nCo-authored-by: Ada Smith <ada[bot]smith@example.com>'
),
[]
);
});

test('still reads a bot account with no address to end it', () => {
match(
soleProblem('🏗️🔧:fix it\n\nCo-authored-by: some-app[bot]'),
/credits a tool with authorship/
);
});

test('leaves a person at one of those companies signing off alone', () => {
deepStrictEqual(
validateCommitMessage(
'🏗️🔧:fix it\n\nSigned-off-by: Ada Lovelace <ada@anthropic.com>'
),
[]
);
});

test('rejects an assistant whose sign-off address is on a folded line', () => {
// The fold hides the address on this trailer the same way it hides one on
// a co-author, and the value is read whole here too.
match(
soleProblem(
'🏗️🔧:fix it\n\nSigned-off-by: Some Person\n <noreply@anthropic.com>'
),
/certifies the Developer Certificate of Origin as a tool/
);
});

test('rejects a token this project does not use', () => {
match(
soleProblem('🏗️🔧:fix it\n\nCloses: https://x/1'),
Expand Down
40 changes: 29 additions & 11 deletions collections/_docs/handbook/style/commit-messages.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,11 @@ 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.
Certificate of Origin][]: that you have the right to contribute this change.
Neither a tool nor a bot can certify it, on your behalf or on its own, so the
check asks two things of the line: that it names the author, and that it names a
person. The second is the one an agent writing its own commit message gets
wrong, and it is covered below.

### Disclosing an AI assistant

Expand All @@ -146,18 +147,35 @@ Certificate of Origin][], and saying a tool helped is not a transfer of
responsibility. You are answerable for every line in your pull request, whatever
wrote it.

The check refuses a `Co-authored-by:` that names an assistant or a bot account,
rather than leaving it to a reviewer to notice. An agent writing its own commit
message reaches for that trailer by habit, and once a wrong one lands it is in
the history for good.
**The author of a commit has to be a person as well.** That is the half of the
rule that goes without saying to everybody except the reader it is aimed at. An
agent running under its own git identity is the author of what it commits, and
`-s` copies that identity into the sign-off, so the trailer names a tool and
agrees with the author while doing it:

```text
Author: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
```

Nothing in that is inconsistent, which is the whole difficulty: for a while the
check had nothing to object to, and a program stood in the history certifying
the origin of the work. An agent committing on somebody's behalf commits _as
them_ — `git config` set to their identity, or `--author` with the sign-off to
match — and names itself in `Assisted-by:`.

The check refuses an assistant or a bot account in either trailer, rather than
leaving it to a reviewer to notice. An agent reaches for `Co-authored-by:` by
habit and is handed `Signed-off-by:` by `-s`, and once a wrong one lands it is
in the history for good.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

It refuses 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. If it ever refuses a real co-author, narrow the pattern in the same
pull request. Do not drop the credit -- a co-author belongs in the trailer,
where GitHub reads it, and not in a sentence in the body.
people have. If it ever refuses a real contributor, narrow the pattern in the
same pull request. Do not drop the trailer — a co-author belongs where GitHub
reads it and not in a sentence in the body, and a sign-off is not optional.

> [!WARNING]
>
Expand Down
Loading