Revert "Remove linked-issue gate from commitperclip" (#7426)

Reverts paperclipai/paperclip#7423

Decided to keep this in place so we can automate issue reproduction in
the future. We all make mistakes. Even me, if you can believe it.
This commit is contained in:
Devin Foley 2026-06-03 08:54:52 -07:00 committed by GitHub
parent 70b1a9109d
commit 03e1e3abd2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 168 additions and 1 deletions

View file

@ -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,
/(?<!\w)#\d+/,
];
// Prefixes where a linked issue is NOT required
const SKIP_ISSUE_PREFIXES = ['docs', 'chore', 'build', 'ci', 'style', 'test', 'revert'];
function parsePrefix(title) {
if (!title) return null;
const match = title.match(/^([a-z]+)(?:\([^)]*\))?:/);
return match ? match[1].toLowerCase() : null;
}
export function checkLinkedIssue(body, prTitle = '') {
const prefix = parsePrefix(prTitle);
if (prefix && SKIP_ISSUE_PREFIXES.includes(prefix)) {
return { passed: true, failures: [] };
}
if (!body || !body.trim()) {
return { passed: false, failures: ['PR body is empty — please fill out the PR template'] };
}
const found = ISSUE_PATTERNS.some(p => 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);
}

View file

@ -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,
];

View file

@ -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);
});