mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 01:41:42 +02:00
feat(commitperclip): widen linked-issue gate, add dedup-search check
- Accept `Refs #N` alongside `Fixes`/`Closes`/`Resolves` in the linked-issue patterns. - Add `hasInlineIssueDescription` so the linked-issue gate passes when a PR body inlines an issue-template-shaped description (≥3 fields matched across the bug, feature, or adapter templates). - New `check-pr-dedup-search.mjs` gate looks for a checked dedup-search affirmation in the PR body, wired into `run-quality-gates.mjs`. Regex anchors the trailing PR/PRs token with a word boundary so it doesn't match `pr` inside ordinary words like "approach". - CONTRIBUTING.md "Before You Start: Search First" now points at the PR template checkbox commitperclip enforces. - Unit tests for `Refs #N`, inline-description threshold, bolded labels, the dedup-search gate across checked/unchecked/missing/skip-prefix, and the word-boundary false-positive guard. Refs #4260 Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
2c7ac09e65
commit
b809e021ba
6 files changed
+365
-14
No files matched your search
@@ -0,0 +1,93 @@
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { checkDedupSearch } from '../check-pr-dedup-search.mjs';
|
||||
|
||||
test('passes when dedup checkbox is checked (lowercase x)', () => {
|
||||
const body = `
|
||||
## Checklist
|
||||
|
||||
- [x] I searched the GitHub PR list (open + recently closed) for similar PRs and confirmed this is not a duplicate
|
||||
- [ ] Other thing
|
||||
`;
|
||||
assert.equal(checkDedupSearch(body, 'feat: thing').passed, true);
|
||||
});
|
||||
|
||||
test('passes when dedup checkbox is checked (uppercase X)', () => {
|
||||
const body = `- [X] I have searched GitHub for duplicate or related PRs and linked them above`;
|
||||
assert.equal(checkDedupSearch(body, 'feat: thing').passed, true);
|
||||
});
|
||||
|
||||
test('passes with current PR template wording (master)', () => {
|
||||
const body = `
|
||||
- [x] I have searched GitHub for duplicate or related PRs and linked them above
|
||||
`;
|
||||
assert.equal(checkDedupSearch(body, 'feat: thing').passed, true);
|
||||
});
|
||||
|
||||
test('fails when dedup checkbox is present but unchecked', () => {
|
||||
const body = `
|
||||
- [ ] I searched for similar open/closed PRs and confirmed this is not a duplicate
|
||||
- [ ] Other thing
|
||||
`;
|
||||
const result = checkDedupSearch(body, 'feat: thing');
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures.length > 0);
|
||||
});
|
||||
|
||||
test('fails when dedup checkbox is missing entirely', () => {
|
||||
const body = `
|
||||
## Checklist
|
||||
|
||||
- [x] I have run tests locally
|
||||
- [x] I have updated documentation
|
||||
`;
|
||||
const result = checkDedupSearch(body, 'feat: thing');
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures.length > 0);
|
||||
});
|
||||
|
||||
test('fails when PR body is empty (and no skip prefix)', () => {
|
||||
const result = checkDedupSearch('', 'feat: thing');
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures.length > 0);
|
||||
});
|
||||
|
||||
test('skips check for docs: prefix', () => {
|
||||
assert.equal(checkDedupSearch('', 'docs: update README').passed, true);
|
||||
});
|
||||
|
||||
test('skips check for chore: prefix', () => {
|
||||
assert.equal(checkDedupSearch('', 'chore: bump deps').passed, true);
|
||||
});
|
||||
|
||||
test('skips check for ci: prefix', () => {
|
||||
assert.equal(checkDedupSearch('', 'ci: tweak workflow').passed, true);
|
||||
});
|
||||
|
||||
test('skips check for test: prefix', () => {
|
||||
assert.equal(checkDedupSearch('', 'test: add coverage').passed, true);
|
||||
});
|
||||
|
||||
test('requires checkbox for feat: prefix', () => {
|
||||
assert.equal(checkDedupSearch('Some description without checkbox', 'feat: new thing').passed, false);
|
||||
});
|
||||
|
||||
test('does not match unrelated checked items mentioning "PR"', () => {
|
||||
const body = `
|
||||
- [x] I have run tests locally and they pass
|
||||
- [x] All Paperclip CI gates are green
|
||||
`;
|
||||
assert.equal(checkDedupSearch(body, 'feat: thing').passed, false);
|
||||
});
|
||||
|
||||
test('matches asterisk bullet style', () => {
|
||||
const body = `* [x] I searched GitHub for similar PRs and this is not a duplicate`;
|
||||
assert.equal(checkDedupSearch(body, 'feat: thing').passed, true);
|
||||
});
|
||||
|
||||
test('does not match when "pr" only appears inside an unrelated word', () => {
|
||||
// "approaches" contains the letters "pr" but is not the token "PR/PRs".
|
||||
// Without a word-boundary anchor the regex would incorrectly pass.
|
||||
const body = `- [x] I searched for similar approaches to this problem`;
|
||||
assert.equal(checkDedupSearch(body, 'feat: thing').passed, false);
|
||||
});
|
||||
@@ -1,6 +1,6 @@
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { checkLinkedIssue } from '../check-pr-linked-issue.mjs';
|
||||
import { checkLinkedIssue, hasInlineIssueDescription } from '../check-pr-linked-issue.mjs';
|
||||
|
||||
// Existing tests with title parameter added (defaults to no prefix, so still required)
|
||||
|
||||
@@ -20,6 +20,14 @@ test('passes with "Resolves #NNN"', () => {
|
||||
assert.equal(checkLinkedIssue('Resolves #101', 'fix: something').passed, true);
|
||||
});
|
||||
|
||||
test('passes with "Refs #NNN"', () => {
|
||||
assert.equal(checkLinkedIssue('Refs #202', 'fix: something').passed, true);
|
||||
});
|
||||
|
||||
test('passes with "refs #NNN" (case-insensitive)', () => {
|
||||
assert.equal(checkLinkedIssue('refs #303', '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,
|
||||
@@ -72,7 +80,7 @@ test('fails when #NNN is part of a word (no space before)', () => {
|
||||
assert.equal(result.passed, false);
|
||||
});
|
||||
|
||||
// New tests for prefix-aware skip behavior
|
||||
// Prefix-aware skip behavior
|
||||
|
||||
test('skips check for docs: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('', 'docs: update README').passed, true);
|
||||
@@ -109,3 +117,107 @@ test('requires issue for refactor: prefix', () => {
|
||||
test('requires issue when no prefix (encourages prefix usage)', () => {
|
||||
assert.equal(checkLinkedIssue('No prefix here', 'Add some feature').passed, false);
|
||||
});
|
||||
|
||||
// Inline issue description (path 2)
|
||||
|
||||
const BUG_INLINE_BODY = `
|
||||
## What happened?
|
||||
|
||||
Login button does nothing when clicked.
|
||||
|
||||
## Expected behavior
|
||||
|
||||
Clicking the login button should authenticate the user.
|
||||
|
||||
## Steps to reproduce
|
||||
|
||||
1. Open the app
|
||||
2. Click login
|
||||
3. Nothing happens
|
||||
`;
|
||||
|
||||
const FEATURE_INLINE_BODY = `
|
||||
## Problem or motivation
|
||||
|
||||
We don't have a way to bulk-tag issues.
|
||||
|
||||
## Proposed solution
|
||||
|
||||
Add a bulk-tag action to the issues list.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
Tagging individually — too slow.
|
||||
`;
|
||||
|
||||
const ADAPTER_INLINE_BODY = `
|
||||
## Agent or provider
|
||||
|
||||
Gemini CLI
|
||||
|
||||
## Why this adapter is useful
|
||||
|
||||
Lots of users want Gemini as an alternative model option.
|
||||
|
||||
## How the agent is invoked
|
||||
|
||||
Via the \`gemini\` CLI binary with stdin/stdout JSON.
|
||||
`;
|
||||
|
||||
test('passes with inline bug description (3 template fields, feat: prefix)', () => {
|
||||
assert.equal(checkLinkedIssue(BUG_INLINE_BODY, 'feat: fix login button').passed, true);
|
||||
});
|
||||
|
||||
test('passes with inline feature description (3 template fields)', () => {
|
||||
assert.equal(checkLinkedIssue(FEATURE_INLINE_BODY, 'feat: bulk tag').passed, true);
|
||||
});
|
||||
|
||||
test('passes with inline adapter description (3 template fields)', () => {
|
||||
assert.equal(checkLinkedIssue(ADAPTER_INLINE_BODY, 'feat: gemini adapter').passed, true);
|
||||
});
|
||||
|
||||
test('fails with only two bug template fields (below threshold)', () => {
|
||||
const body = `
|
||||
## What happened?
|
||||
|
||||
Something broke.
|
||||
|
||||
## Expected behavior
|
||||
|
||||
It should work.
|
||||
`;
|
||||
assert.equal(checkLinkedIssue(body, 'feat: fix').passed, false);
|
||||
});
|
||||
|
||||
test('fails with a single stray template-like heading', () => {
|
||||
const body = `
|
||||
This is mostly a free-form description but one heading happens to match.
|
||||
|
||||
## Expected behavior
|
||||
|
||||
Everything works.
|
||||
`;
|
||||
assert.equal(checkLinkedIssue(body, 'feat: fix').passed, false);
|
||||
});
|
||||
|
||||
test('hasInlineIssueDescription returns true for ≥3 bug fields', () => {
|
||||
assert.equal(hasInlineIssueDescription(BUG_INLINE_BODY), true);
|
||||
});
|
||||
|
||||
test('hasInlineIssueDescription returns false for empty body', () => {
|
||||
assert.equal(hasInlineIssueDescription(''), false);
|
||||
});
|
||||
|
||||
test('hasInlineIssueDescription accepts bolded labels with colons', () => {
|
||||
const body = `
|
||||
**Problem:**
|
||||
We need this.
|
||||
|
||||
**Proposed solution:**
|
||||
Build it.
|
||||
|
||||
**Alternatives considered:**
|
||||
None.
|
||||
`;
|
||||
assert.equal(hasInlineIssueDescription(body), true);
|
||||
});
|
||||
Reference in new issue
Block a user