From 4d776b46703252665641da054efd97ac127ecfb7 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sun, 3 May 2026 13:30:07 -0400 Subject: [PATCH] ci/github-script/lint-commits: confirm Git names/emails are present Prevents issues reported on Matrix by Jujutsu users, caused by people omitting these fields. --- ci/github-script/get-pr-commit-details.js | 34 +++++++--- ci/github-script/lint-commits.js | 76 +++++++++++++++++++++-- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/ci/github-script/get-pr-commit-details.js b/ci/github-script/get-pr-commit-details.js index fcccfeacd75e..b268e7cf6202 100644 --- a/ci/github-script/get-pr-commit-details.js +++ b/ci/github-script/get-pr-commit-details.js @@ -2,6 +2,17 @@ const { promisify } = require('node:util') const execFile = promisify(require('node:child_process').execFile) +/** + * @typedef {{ + * subject: string, + * sha: string, + * author: { name: string, email: string }, + * committer: { name: string, email: string} + * changedPaths: string[], + * changedPathSegments: Set, + * }} Commit + */ + /** * @param {{ * args: string[] @@ -34,12 +45,7 @@ async function runGit({ args, repoPath, core, quiet }) { * repoPath?: string, * }} GetCommitMessagesForPRProps * - * @returns {Promise<{ - * subject: string, - * sha: string, - * changedPaths: string[], - * changedPathSegments: Set, - * }[]>} + * @returns {Promise} */ async function getCommitDetailsForPR({ core, pr, repoPath }) { await runGit({ @@ -70,17 +76,25 @@ async function getCommitDetailsForPR({ core, pr, repoPath }) { return Promise.all( shas.map(async (sha) => { - // Subject first, then a blank line, then filenames. + // Subject, author name, author email, committer name, committer email (all tab-seperated) + // then a blank line, then filenames. const result = ( await runGit({ - args: ['log', '--format=%s', '--name-only', '-1', sha], + args: [ + 'log', + '--format=%s\t%aN\t%aE\t%cN\t%cE', + '--name-only', + '-1', + sha, + ], repoPath, core, quiet: true, }) ).stdout.split('\n') - const subject = result[0] + const [subject, authorName, authorEmail, committerName, committerEmail] = + result[0].split('\t') const changedPaths = result.slice(2, -1) @@ -91,6 +105,8 @@ async function getCommitDetailsForPR({ core, pr, repoPath }) { return { sha, subject, + author: { name: authorName, email: authorEmail }, + committer: { name: committerName, email: committerEmail }, changedPaths, changedPathSegments, } diff --git a/ci/github-script/lint-commits.js b/ci/github-script/lint-commits.js index 1aa18e9477b0..0828db23a2bc 100644 --- a/ci/github-script/lint-commits.js +++ b/ci/github-script/lint-commits.js @@ -2,15 +2,17 @@ const { classify } = require('../supportedBranches.js') const { getCommitDetailsForPR } = require('./get-pr-commit-details.js') +/** @typedef {import('./get-pr-commit-details.js').Commit} Commit */ + /** * @param {{ * github: InstanceType, - * context: import('@actions/github/lib/context').Context, + * context: typeof import('@actions/github').context, * core: import('@actions/core'), * repoPath?: string, - * }} CheckCommitMessagesProps + * }} LintCommitsProps */ -async function checkCommitMessages({ github, context, core, repoPath }) { +async function lintCommits({ github, context, core, repoPath }) { // This check should only be run when we have the pull_request context. const pull_number = context.payload.pull_request?.number if (!pull_number) { @@ -48,6 +50,17 @@ async function checkCommitMessages({ github, context, core, repoPath }) { const commits = await getCommitDetailsForPR({ core, pr, repoPath }) + await checkCommitMessages({ commits, core }) + await checkCommitMetadata({ commits, core }) +} + +/** + * @param {{ + * commits: Commit[], + * core: import('@actions/core'), + * }} CheckCommitMessagesProps + */ +async function checkCommitMessages({ commits, core }) { const failures = new Set() const conventionalCommitTypes = [ @@ -152,4 +165,59 @@ async function checkCommitMessages({ github, context, core, repoPath }) { } } -module.exports = checkCommitMessages +/** + * @param {{ + * commits: Commit[], + * core: import('@actions/core'), + * }} CheckGitFieldsProps + */ +async function checkCommitMetadata({ commits, core }) { + const failures = new Set() + + /** @type {(s: string) => boolean} */ + const isEmail = (s) => /^.+@.*$/.test(s) + + for (const commit of commits) { + if (!commit.author.name) { + core.error(`Commit ${commit.sha} author's name field is missing`) + failures.add(commit.sha) + } + + if (!commit.author.email || !isEmail(commit.author.email)) { + core.error( + `Commit ${commit.sha} author's email field is missing or invalid`, + ) + failures.add(commit.sha) + } + + if (!commit.committer.name) { + core.error(`Commit ${commit.sha} committer's name field is missing`) + failures.add(commit.sha) + } + + if (!commit.committer.email || !isEmail(commit.committer.email)) { + core.error( + `Commit ${commit.sha} committer's email field is missing or invalid`, + ) + failures.add(commit.sha) + } + + if (!failures.has(commit.sha)) { + core.info( + `Commit ${commit.sha}'s git fields passed our automated checks!`, + ) + } + } + + if (failures.size !== 0) { + core.error( + 'Please add the missing commit fields. ' + + 'You can use the noreply email address generated for you by GitHub ' + + '(https://docs.github.com/en/account-and-profile/reference/email-addresses-reference#your-noreply-email-address) ' + + "if you'd like.", + ) + core.setFailed('Committers: merging is discouraged.') + } +} + +module.exports = lintCommits