diff --git a/.github/scripts/check-pr-linked-issue.mjs b/.github/scripts/check-pr-linked-issue.mjs new file mode 100644 index 00000000..865b8865 --- /dev/null +++ b/.github/scripts/check-pr-linked-issue.mjs @@ -0,0 +1,53 @@ +#!/usr/bin/env node +/** + * check-pr-linked-issue.mjs + * Checks that a PR body references a GitHub issue. Respects conventional commit + * prefixes — skips check for docs/chore/build/ci/style/test prefixed PRs. + * Export: checkLinkedIssue(prBody: string, prTitle: string) → { passed, failures } + */ +import { fileURLToPath } from 'node:url'; + +const ISSUE_PATTERNS = [ + /(?:fixes|closes|resolves)\s+#\d+/i, + /(?:^|[\s(])https:\/\/github\.com\/paperclipai\/paperclip\/issues\/\d+(?=$|[\s),:;!?]|[.](?![\w-]))/i, + /(? p.test(body)); + return { + passed: found, + failures: found ? [] : [ + 'No linked issue found — please add `Fixes #NNN` to your PR description. ' + + 'If no issue exists yet, please file one first: ' + + 'https://github.com/paperclipai/paperclip/issues/new', + ], + }; +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const body = process.env.PR_BODY ?? ''; + const title = process.env.PR_TITLE ?? ''; + const result = checkLinkedIssue(body, title); + console.log(JSON.stringify(result)); + process.exit(result.passed ? 0 : 1); +} diff --git a/.github/scripts/run-quality-gates.mjs b/.github/scripts/run-quality-gates.mjs index 8cf1ad09..a5b6abae 100644 --- a/.github/scripts/run-quality-gates.mjs +++ b/.github/scripts/run-quality-gates.mjs @@ -11,6 +11,7 @@ import { fileURLToPath } from 'node:url'; import { ghFetch } from './get-bot-token.mjs'; import { fetchAllPullRequestFiles } from './fetch-pr-files.mjs'; import { checkTemplate } from './check-pr-template.mjs'; +import { checkLinkedIssue } from './check-pr-linked-issue.mjs'; import { checkTestCoverage } from './check-pr-test-coverage.mjs'; import { checkLockfile } from './check-pr-lockfile.mjs'; import { checkDependencies } from './check-pr-dependencies.mjs'; @@ -109,9 +110,10 @@ async function main() { // Run all quality gates (pure functions run sync, deps check is async) const prTitle = pr.title ?? ''; - const [templateResult, testResult, lockfileResult, depsResult] = + const [templateResult, issueResult, testResult, lockfileResult, depsResult] = await Promise.all([ Promise.resolve(checkTemplate(prBody)), + Promise.resolve(checkLinkedIssue(prBody, prTitle)), Promise.resolve(checkTestCoverage(files, prTitle)), Promise.resolve(checkLockfile(files, author, branch)), checkDependencies(files, GH_TOKEN, GH_REPO, prNumber, pr.base?.ref), @@ -119,6 +121,7 @@ async function main() { const allFailures = [ ...templateResult.failures, + ...issueResult.failures, ...testResult.failures, ...lockfileResult.failures, ]; diff --git a/.github/scripts/tests/check-pr-linked-issue.test.mjs b/.github/scripts/tests/check-pr-linked-issue.test.mjs new file mode 100644 index 00000000..6b2745db --- /dev/null +++ b/.github/scripts/tests/check-pr-linked-issue.test.mjs @@ -0,0 +1,111 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { checkLinkedIssue } from '../check-pr-linked-issue.mjs'; + +// Existing tests with title parameter added (defaults to no prefix, so still required) + +test('passes with bare #NNN reference', () => { + assert.equal(checkLinkedIssue('This fixes the bug in #123', 'fix: something').passed, true); +}); + +test('passes with "Fixes #NNN"', () => { + assert.equal(checkLinkedIssue('Fixes #456\n\nSome description', 'fix: something').passed, true); +}); + +test('passes with "Closes #NNN" (case-insensitive)', () => { + assert.equal(checkLinkedIssue('closes #789', 'fix: something').passed, true); +}); + +test('passes with "Resolves #NNN"', () => { + assert.equal(checkLinkedIssue('Resolves #101', 'fix: something').passed, true); +}); + +test('passes with full github.com URL', () => { + assert.equal( + checkLinkedIssue('See https://github.com/paperclipai/paperclip/issues/202', 'fix: bug').passed, + true + ); +}); + +test('passes with a full github.com URL followed by punctuation', () => { + assert.equal( + checkLinkedIssue('See (https://github.com/paperclipai/paperclip/issues/202).', 'fix: bug').passed, + true + ); +}); + +test('fails with empty body when no skip prefix', () => { + const result = checkLinkedIssue('', 'fix: bug'); + assert.equal(result.passed, false); + assert.ok(result.failures.length > 0); +}); + +test('fails with no issue reference when no skip prefix', () => { + const result = checkLinkedIssue('Added a cool feature, no issue linked', 'feat: something'); + assert.equal(result.passed, false); + assert.ok(result.failures[0].includes('Fixes #NNN')); +}); + +test('fails with cross-repo issue reference', () => { + const result = checkLinkedIssue('See https://github.com/other/repo/issues/123', 'fix: bug'); + assert.equal(result.passed, false); +}); + +test('fails when the Paperclip issue URL is embedded inside another host', () => { + const result = checkLinkedIssue( + 'See https://evil.example/https://github.com/paperclipai/paperclip/issues/123', + 'fix: bug' + ); + assert.equal(result.passed, false); +}); + +test('fails when the Paperclip issue URL continues into another host', () => { + const result = checkLinkedIssue( + 'See https://github.com/paperclipai/paperclip/issues/123.evil.example', + 'fix: bug' + ); + assert.equal(result.passed, false); +}); + +test('fails when #NNN is part of a word (no space before)', () => { + const result = checkLinkedIssue('This is version#123 not an issue link', 'fix: bug'); + assert.equal(result.passed, false); +}); + +// New tests for prefix-aware skip behavior + +test('skips check for docs: prefix', () => { + assert.equal(checkLinkedIssue('', 'docs: update README').passed, true); +}); + +test('skips check for chore: prefix', () => { + assert.equal(checkLinkedIssue('', 'chore: bump deps').passed, true); +}); + +test('skips check for build: prefix', () => { + assert.equal(checkLinkedIssue('', 'build: update Dockerfile').passed, true); +}); + +test('skips check for ci: prefix', () => { + assert.equal(checkLinkedIssue('', 'ci: add workflow').passed, true); +}); + +test('skips check for test: prefix', () => { + assert.equal(checkLinkedIssue('', 'test: add coverage').passed, true); +}); + +test('skips check with scoped prefix like docs(api):', () => { + assert.equal(checkLinkedIssue('', 'docs(api): document endpoint').passed, true); +}); + +test('requires issue for feat: prefix', () => { + assert.equal(checkLinkedIssue('Some description without issue', 'feat: new thing').passed, false); +}); + +test('requires issue for refactor: prefix', () => { + assert.equal(checkLinkedIssue('Some refactor', 'refactor: rewrite thing').passed, false); +}); + +test('requires issue when no prefix (encourages prefix usage)', () => { + assert.equal(checkLinkedIssue('No prefix here', 'Add some feature').passed, false); +});