diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index da44ef515..2a0b0a27b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -35,6 +35,7 @@ See [CONTRIBUTING.md](../CONTRIBUTING.md) for the full process. --- - + + diff --git a/.github/workflows/pr-template-format.yml b/.github/workflows/pr-template-format.yml index 33fd2d325..462a4a6f5 100644 --- a/.github/workflows/pr-template-format.yml +++ b/.github/workflows/pr-template-format.yml @@ -18,11 +18,23 @@ jobs: - name: Check out policy from base branch uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Get changed files + id: changed_files + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + files=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json files --jq '.files[].path' | tr '\n' '\n') + echo "files<> "$GITHUB_OUTPUT" + echo "$files" >> "$GITHUB_OUTPUT" + echo "EOF" >> "$GITHUB_OUTPUT" + - name: Evaluate PR template format id: policy env: PR_BODY: ${{ github.event.pull_request.body }} AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }} + CHANGED_FILES: ${{ steps.changed_files.outputs.files }} run: node scripts/pr-template-policy.cjs - name: Warn trusted contributor about missing template diff --git a/scripts/pr-template-policy.cjs b/scripts/pr-template-policy.cjs index 2100ba622..59ce578e7 100644 --- a/scripts/pr-template-policy.cjs +++ b/scripts/pr-template-policy.cjs @@ -1,5 +1,7 @@ #!/usr/bin/env node +const { minimatch } = require('minimatch'); + const TRUSTED_AUTHOR_ASSOCIATIONS = new Set([ 'CONTRIBUTOR', 'COLLABORATOR', @@ -13,6 +15,42 @@ const DEFAULT_TEMPLATE_MARKERS = [ 'Select the template that matches your PR', ]; +/** + * Glob patterns for files that are considered CI/tooling/docs scope. + * If ALL changed files in a PR match at least one of these patterns, + * template enforcement is skipped automatically. + */ +const TOOLING_PATH_ALLOWLIST = [ + '.github/**', + 'scripts/**', + 'docs/**', + '*.md', + '.changeset/**', + 'package.json', + 'package-lock.json', + 'pnpm-lock.yaml', + 'yarn.lock', + 'Pipfile', + 'Pipfile.lock', + 'requirements.txt', + 'requirements*.txt', + 'poetry.lock', + 'Gemfile', + 'Gemfile.lock', + 'go.sum', + 'go.mod', +]; + +/** + * Matches an explicit PR-template exemption marker of the form: + * + * + * Capture group 1 is the reason (trimmed). Empty/whitespace-only reasons + * do NOT match because the pattern requires at least one non-whitespace + * character in the captured body. + */ +const EXEMPT_MARKER_REGEX = //; + const TEMPLATES = [ { name: 'fix', @@ -99,11 +137,61 @@ function matchingTemplate(body) { }; } -function evaluatePrTemplate(body, authorAssociation) { +/** + * Returns true iff every path in changedFiles matches at least one glob + * pattern in the allowlist. Returns false for an empty file list (no + * files means we cannot confirm it is a tooling-only PR). + */ +function allPathsAreTooling(changedFiles, allowlist) { + if (!Array.isArray(changedFiles) || changedFiles.length === 0) return false; + return changedFiles.every((file) => + allowlist.some((pattern) => minimatch(file, pattern, { matchBase: false, dot: true })), + ); +} + +/** + * Returns true iff the body contains an explicit exemption marker with a + * non-empty (non-whitespace) reason. + */ +function hasExemptMarker(body, regex) { + const match = regex.exec(String(body || '')); + if (!match) return false; + return match[1].trim().length > 0; +} + +function evaluatePrTemplate(body, authorAssociation, changedFiles) { const association = String(authorAssociation || '').toUpperCase(); const trusted = TRUSTED_AUTHOR_ASSOCIATIONS.has(association); const normalizedBody = String(body || '').trim(); + // --- Carve-out 1: all changed files are in the tooling allowlist --- + if (allPathsAreTooling(changedFiles, TOOLING_PATH_ALLOWLIST)) { + return { + valid: true, + action: 'pass', + trusted, + authorAssociation: association || 'UNKNOWN', + template: null, + reason: null, + missingHeadings: [], + skipped: 'tooling-paths', + }; + } + + // --- Carve-out 2: explicit exemption marker in the PR body --- + if (hasExemptMarker(normalizedBody, EXEMPT_MARKER_REGEX)) { + return { + valid: true, + action: 'pass', + trusted, + authorAssociation: association || 'UNKNOWN', + template: null, + reason: null, + missingHeadings: [], + skipped: 'exempt-marker', + }; + } + let valid = true; let reason = 'PR body uses a typed pull request template.'; let template = null; @@ -147,7 +235,14 @@ function evaluatePrTemplate(body, authorAssociation) { } function main() { - const result = evaluatePrTemplate(process.env.PR_BODY || '', process.env.AUTHOR_ASSOCIATION || ''); + const changedFiles = process.env.CHANGED_FILES + ? process.env.CHANGED_FILES.split('\n').map((f) => f.trim()).filter(Boolean) + : undefined; + const result = evaluatePrTemplate( + process.env.PR_BODY || '', + process.env.AUTHOR_ASSOCIATION || '', + changedFiles, + ); process.stdout.write(`${JSON.stringify(result)}\n`); if (process.env.GITHUB_OUTPUT) { const fs = require('fs'); @@ -166,4 +261,8 @@ module.exports = { extractHeadings, includesDefaultTemplate, matchingTemplate, + allPathsAreTooling, + hasExemptMarker, + TOOLING_PATH_ALLOWLIST, + EXEMPT_MARKER_REGEX, }; diff --git a/tests/pr-template-policy.test.cjs b/tests/pr-template-policy.test.cjs index f274ba545..5e4ca450d 100644 --- a/tests/pr-template-policy.test.cjs +++ b/tests/pr-template-policy.test.cjs @@ -1,7 +1,7 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { evaluatePrTemplate } = require('../scripts/pr-template-policy.cjs'); +const { evaluatePrTemplate, allPathsAreTooling, hasExemptMarker, TOOLING_PATH_ALLOWLIST, EXEMPT_MARKER_REGEX } = require('../scripts/pr-template-policy.cjs'); const fixBody = [ '## Fix PR', @@ -82,6 +82,108 @@ const featureBody = [ '- [x] Tests pass', ].join('\n'); +describe('pr-template-policy carve-out', () => { + // A. Path-scope auto-skip — CI-only PR is accepted + test('auto-skips enforcement for CI-only changed files', () => { + const result = evaluatePrTemplate('This is a CI change with no template.', 'NONE', ['.github/workflows/test.yml']); + assert.equal(result.valid, true); + assert.equal(result.action, 'pass'); + assert.equal(result.skipped, 'tooling-paths'); + }); + + // B. Path-scope auto-skip — docs-only PR is accepted + test('auto-skips enforcement for docs-only changed files', () => { + const result = evaluatePrTemplate('Updated documentation.', 'NONE', ['docs/CONFIGURATION.md', 'README.md']); + assert.equal(result.valid, true); + assert.equal(result.action, 'pass'); + assert.equal(result.skipped, 'tooling-paths'); + }); + + // C. Path-scope auto-skip — dependency-bump PR is accepted + test('auto-skips enforcement for dependency-bump changed files', () => { + const result = evaluatePrTemplate('Bump deps.', 'NONE', ['package.json', 'package-lock.json']); + assert.equal(result.valid, true); + assert.equal(result.action, 'pass'); + assert.equal(result.skipped, 'tooling-paths'); + }); + + // D. Path-scope auto-skip is path-strict — mixed PR still enforced + test('does NOT skip enforcement when any changed file is outside the tooling allowlist', () => { + const result = evaluatePrTemplate('Mixed change.', 'NONE', ['.github/workflows/test.yml', 'src/feature.ts']); + assert.equal(result.valid, false); + assert.equal(result.action, 'close'); + }); + + // E. Explicit marker accepted (non-empty reason) + test('auto-skips enforcement when PR body contains a valid exempt marker with a reason', () => { + const body = '\n\nThis removes the Node 26 CI lane.'; + const result = evaluatePrTemplate(body, 'NONE', ['src/anything.ts']); + assert.equal(result.valid, true); + assert.equal(result.action, 'pass'); + assert.equal(result.skipped, 'exempt-marker'); + }); + + // F. Explicit marker requires non-empty reason + test('does NOT skip enforcement when exempt marker has an empty reason', () => { + const bodyEmpty = '\n\nSome description.'; + const bodyNoReason = '\n\nSome description.'; + const r1 = evaluatePrTemplate(bodyEmpty, 'NONE', ['src/anything.ts']); + const r2 = evaluatePrTemplate(bodyNoReason, 'NONE', ['src/anything.ts']); + assert.equal(r1.valid, false); + assert.equal(r2.valid, false); + }); + + // G. Regression — DEFAULT_TEMPLATE_MARKERS still rejected + test('regression: DEFAULT_TEMPLATE_MARKERS body is still rejected when no carve-out applies', () => { + const body = 'Wrong template — please use a typed template.\n\nEvery PR must use a typed template.'; + const result = evaluatePrTemplate(body, 'NONE', ['src/feature.ts']); + assert.equal(result.valid, false); + assert.match(result.reason, /default wrong-template guidance/); + }); + + // H. Regression — fix template still accepted + test('regression: fix template PR body is still accepted', () => { + const result = evaluatePrTemplate(fixBody, 'NONE', ['src/feature.ts']); + assert.equal(result.valid, true); + assert.equal(result.action, 'pass'); + assert.equal(result.template, 'fix'); + }); +}); + +describe('pr-template-policy helper: allPathsAreTooling', () => { + test('returns true when all paths match the allowlist', () => { + assert.equal(allPathsAreTooling(['.github/workflows/ci.yml'], TOOLING_PATH_ALLOWLIST), true); + assert.equal(allPathsAreTooling(['package.json', 'package-lock.json'], TOOLING_PATH_ALLOWLIST), true); + assert.equal(allPathsAreTooling(['README.md'], TOOLING_PATH_ALLOWLIST), true); + }); + + test('returns false when any path does not match the allowlist', () => { + assert.equal(allPathsAreTooling(['.github/workflows/ci.yml', 'src/index.ts'], TOOLING_PATH_ALLOWLIST), false); + }); + + test('returns false for an empty file list', () => { + assert.equal(allPathsAreTooling([], TOOLING_PATH_ALLOWLIST), false); + }); +}); + +describe('pr-template-policy helper: hasExemptMarker', () => { + test('matches a marker with a non-empty reason', () => { + assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), true); + assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), true); + assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), true); + }); + + test('does not match a marker with empty or whitespace-only reason', () => { + assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), false); + assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), false); + }); + + test('does not match a marker with no pr-template-exempt keyword', () => { + assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), false); + assert.equal(hasExemptMarker('some random text', EXEMPT_MARKER_REGEX), false); + }); +}); + describe('pr-template-policy', () => { test('passes PR bodies that use the fix template', () => { const result = evaluatePrTemplate(fixBody, 'NONE');