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 ' + '';