From bf37a4abaa7bfe1d30d072df3ba67d6dfd845caa Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Tue, 1 Sep 2026 14:35:24 +0000 Subject: [PATCH] ci: check for `Closes #` (#45462) Due to a long-standing issue with `semantic-release` not checking if `#...` reference is for a Discussion or Issue, if a contributor says `Closes #`, then our release process fails. We can extend our existing checks to try and catch this in the PR itself, as users may not read the big warning that says "don't add a Discussion here". Co-authored-by: Claude Sonnet 5 --- .github/workflows/check-commit-messages.yml | 46 +++------- tools/check-closes-discussion.cjs | 97 +++++++++++++++++++++ 2 files changed, 108 insertions(+), 35 deletions(-) create mode 100644 tools/check-closes-discussion.cjs diff --git a/.github/workflows/check-commit-messages.yml b/.github/workflows/check-commit-messages.yml index 5996a30986..e451bc8c5c 100644 --- a/.github/workflows/check-commit-messages.yml +++ b/.github/workflows/check-commit-messages.yml @@ -9,6 +9,8 @@ on: permissions: contents: read + discussions: read + issues: read statuses: write concurrency: @@ -19,44 +21,18 @@ jobs: closes-issue-or-discussion: runs-on: ubuntu-slim steps: - - name: Check for 'Closes \#' in commit messages + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + show-progress: false + + - name: Check for closing keywords that would close a Discussion uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: | - // Co-authored-by: Claude Sonnet 4.5 (GitHub Copilot) - - // Get all commits in this PR using the GitHub API - const { data: commits } = await github.rest.pulls.listCommits({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - }); - - // Check each commit message for closing keywords - const violatingCommits = []; - // Matches: closes #123, fixes #123, closes: #123, etc. - const closingPattern = /(closes|fixes|resolves|fixed|closed|resolved):?\s+#\d+/i; - - for (const commit of commits) { - const message = commit.commit.message; - if (closingPattern.test(message)) { - const shortSha = commit.sha.substring(0, 7); - const firstLine = message.split('\n')[0]; - violatingCommits.push({ sha: shortSha, message: firstLine }); - } - } - - // Report results - if (violatingCommits.length > 0) { - let errorMessage = `Found ${violatingCommits.length} commit(s) with issue closing keywords:\n\n`; - for (const commit of violatingCommits) { - errorMessage += `- ${commit.sha}: ${commit.message}\n`; - } - errorMessage += `\nCommit messages should not contain closing keywords (closes, fixes, resolves, etc.). Use the PR description instead.`; - core.setFailed(errorMessage); - } else { - core.info('✓ No commits with issue closing keywords found'); - } + const script = require('./tools/check-closes-discussion.cjs') + await script({ github, context, core }) # Runs a full `semantic-release` dry-run for the state that would exist right # after this PR merges, and fails if it would publish a new *major* version. diff --git a/tools/check-closes-discussion.cjs b/tools/check-closes-discussion.cjs new file mode 100644 index 0000000000..9ffeba771f --- /dev/null +++ b/tools/check-closes-discussion.cjs @@ -0,0 +1,97 @@ +// Co-authored-by: Claude Sonnet 4.5 (GitHub Copilot) + +/** + * @param {{ github: any, context: any, core: any }} params + */ +module.exports = async ({ github, context, core }) => { + // Get all commits in this PR using the GitHub API + const { data: commits } = await github.rest.pulls.listCommits({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + }); + + // GitHub honours closing keywords in both commit messages and the + // PR description. `#123` may refer to a Discussion rather than an + // Issue - since Issues and Discussions are numbered independently + // - and closing keywords must not auto-close a Discussion (see + // #45439, which unintentionally closed discussion #40885). + // Matches: closes #123, fixes #123, closes: #123, etc. + const closingPattern = + /(closes|fixes|resolves|fixed|closed|resolved):?\s+#(\d+)/gi; + + // Collect every closing-keyword reference, keeping one example + // line per source for the error report. + const references = new Map(); // number -> { source, line } + /** + * @param {string} text + * @param {string} source + */ + function collect(text, source) { + for (const match of text.matchAll(closingPattern)) { + const number = match[2]; + if (!references.has(number)) { + const line = text.slice(0, match.index).split('\n').length; + const lineText = text.split('\n')[line - 1].trim(); + references.set(number, { source, line: lineText }); + } + } + } + + for (const commit of commits) { + const shortSha = commit.sha.substring(0, 7); + collect(commit.commit.message, `commit ${shortSha}`); + } + collect(context.payload.pull_request.body ?? '', 'PR description'); + + // Only fail for references that resolve to a Discussion - an + // Issue reference is the intended, supported use of these + // keywords (see the PR template). + const violations = []; + for (const [number, { source, line }] of references) { + let isIssue = true; + try { + await github.rest.issues.get({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: parseInt(number, 10), + }); + } catch (err) { + if (err.status !== 404) { + throw err; + } + isIssue = false; + } + if (isIssue) { + continue; + } + + const { repository } = await github.graphql( + `query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + discussion(number: $number) { id } + } + }`, + { + owner: context.repo.owner, + repo: context.repo.repo, + number: parseInt(number, 10), + }, + ); + if (repository.discussion) { + violations.push({ source, line }); + } + } + + // Report results + if (violations.length > 0) { + let errorMessage = `Found ${violations.length} closing keyword(s) referencing a Discussion:\n\n`; + for (const violation of violations) { + errorMessage += `- ${violation.source}: ${violation.line}\n`; + } + errorMessage += `\nClosing keywords (closes, fixes, resolves, etc.) must not reference a Discussion, as merging would auto-close it. Reference the discussion without a closing keyword instead, e.g. 'Related discussion: #123'.`; + core.setFailed(errorMessage); + } else { + core.info('✓ No closing keywords referencing a Discussion found'); + } +};