From 0dfe3c634fa20f2cc49919bdee2dca2aac1aea69 Mon Sep 17 00:00:00 2001 From: Derek Lewis Date: Sun, 13 Sep 2026 01:40:38 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=97=EF=B8=8F=F0=9F=94=A7=EF=BC=9Aread?= =?UTF-8?q?=20a=20wrapped=20trailer=20the=20way=20git=20does?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A trailer whose value ran onto an indented line was read as far as the fold and no further. The continuation was dropped before any check saw it, so whatever had been wrapped was invisible. That is a way around the check that just landed. These two are one trailer to git, and `git interpret-trailers --parse` prints 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, and it is the half at the end, 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, and the check passed for nobody. So the continuation is joined onto the value rather than discarded, which is what git does with it. Reading a message now agrees with `git interpret-trailers` on a folded trailer as well as a plain one, which is the agreement the verify task already cross-checks. Nothing has to be wrapped to begin with: the trailer block stays exempt from the width limit, because a trailer is metadata and the limit is for prose. The comment saying a trailer is one line by construction went with the construction. Signed-off-by: Derek Lewis Assisted-by: Claude-Code:claude-opus-5 --- build/shared/commit-message.mts | 32 ++++++++++++++++++++++------ build/shared/commit-message.test.mts | 31 +++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/build/shared/commit-message.mts b/build/shared/commit-message.mts index 18994cf79..fa384626c 100644 --- a/build/shared/commit-message.mts +++ b/build/shared/commit-message.mts @@ -221,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; + }, []) + : []; }; /** @@ -454,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 719ecd487..4941cc37c 100644 --- a/build/shared/commit-message.test.mts +++ b/build/shared/commit-message.test.mts @@ -13,6 +13,7 @@ import { ACTIONS, BODY_MAX, CATEGORIES, + checkSignOff, readTrailers, validateCommitMessage, } from '@openinf/portal/build/commit-message'; @@ -308,6 +309,31 @@ describe('validateCommitMessage: the trailers', () => { ); }); + 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'), @@ -430,8 +456,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 ' + '';