fix(ci): make PR-template inline-description contract explicit (#10558)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors use the PR template to describe changes before review > - The linked-issue gate reads literal template labels, not freeform prose > - The template did not state that rule, so a good prose summary could fail on first submission > - The gate also skipped two issue template paths that the PR template already points to > - This pull request makes the template and the gate match the same contract > - The benefit is that a good-faith author can pass the check on the first try ## Linked Issues or Issue Description No public issue exists. Related public PR: #7762. The problem is a mismatch between the PR template and the linked-issue gate. The template gave a bare placeholder and did not explain the literal-label rule. The gate also missed the enhancement and docs issue templates. ## What Changed - Replaced the bare PR-template placeholder with labeled inline skeletons for bug, feature, and adapter paths. - Added one sentence that says the gate reads literal labels on separate lines. - Added the enhancement and docs issue template field sets to the linked-issue gate. - Added and updated tests for prose-only bodies, template skeleton bodies, and extra issue template coverage. ## Verification - `node --test .github/scripts/tests/check-pr-linked-issue.test.mjs` - The branch contains one commit: `561f1ed3a1434ed4562306f74d40968163ef1444` ## Risks Low risk. The three-field minimum stays in place. The main change is clearer author guidance in the PR template. ## Model Used OpenAI Codex, GPT-5, tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach
GitHub
parent
8c910b9a40
commit
131d476a7e
|
|
@ -19,10 +19,11 @@
|
|||
## Linked Issues or Issue Description
|
||||
|
||||
<!--
|
||||
Required. Pick ONE of the following two paths:
|
||||
Required. Pick ONE of the two paths below.
|
||||
|
||||
(A) Issue exists — tag each linked issue with `Fixes: #123`, `Closes #123`,
|
||||
or `Refs #123`. Include duplicates and closely related issues too.
|
||||
(A) Issue exists — replace the placeholder below with your issue links.
|
||||
Tag each linked issue with `Fixes: #123`, `Closes #123`, or `Refs #123`.
|
||||
Include duplicates and closely related issues too.
|
||||
|
||||
Only reference PUBLIC GitHub issues/PRs here. Do NOT paste internal,
|
||||
instance-local Paperclip references — ticket ids like PAPA-123 / PAP-224,
|
||||
|
|
@ -30,11 +31,18 @@
|
|||
contributors cannot open them. See CONTRIBUTING.md → "No Internal Issue
|
||||
References".
|
||||
|
||||
(B) No issue exists — describe the underlying problem here, following the
|
||||
relevant issue template so reviewers get the same fields:
|
||||
• Bug: .github/ISSUE_TEMPLATE/bug_report.yml
|
||||
• Feature: .github/ISSUE_TEMPLATE/feature_request.yml
|
||||
• Adapter: .github/ISSUE_TEMPLATE/adapter_request.yml
|
||||
(B) No issue exists — describe the underlying problem here. Follow the issue
|
||||
template that fits your change. Open the matching file and copy its field
|
||||
labels into your description:
|
||||
• Bug: .github/ISSUE_TEMPLATE/bug_report.yml
|
||||
• Feature: .github/ISSUE_TEMPLATE/feature_request.yml
|
||||
• Adapter: .github/ISSUE_TEMPLATE/adapter_request.yml
|
||||
• Enhancement: .github/ISSUE_TEMPLATE/enhancement.yml
|
||||
• Docs: .github/ISSUE_TEMPLATE/docs_issue.yml
|
||||
An automated check reads the literal bold labels AND the content under
|
||||
each label. Keep at least three of these labels, each alone on its own
|
||||
line, and write real content under each. A label with only the bare "-"
|
||||
placeholder does not count, and the check fails.
|
||||
|
||||
See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
|
||||
-->
|
||||
|
|
|
|||
|
|
@ -47,34 +47,111 @@ const TEMPLATE_FIELDS = {
|
|||
["Why this adapter is useful", "Why it's useful", 'Why useful', 'Use case'],
|
||||
['How the agent is invoked', 'How it is invoked', "How it's invoked", 'Invocation'],
|
||||
],
|
||||
// Labels below match .github/ISSUE_TEMPLATE/enhancement.yml exactly.
|
||||
enhancement: [
|
||||
['What existing behavior does this improve?', 'What existing behavior does this improve'],
|
||||
['Subsystem affected'],
|
||||
['Current behavior'],
|
||||
['Proposed behavior'],
|
||||
['Reason and benefit'],
|
||||
['Breaking changes'],
|
||||
],
|
||||
// Labels below match .github/ISSUE_TEMPLATE/docs_issue.yml exactly. The
|
||||
// template has 4 distinct fields, so it meets the 3-field minimum. A
|
||||
// "docs"-prefixed PR skips this check; this set helps a non-"docs"-prefixed
|
||||
// PR that describes a documentation issue inline.
|
||||
docs: [
|
||||
['Issue type'],
|
||||
['Where is the issue?', 'Where is the issue'],
|
||||
["What's wrong?", "What's wrong"],
|
||||
['Suggested fix'],
|
||||
],
|
||||
};
|
||||
|
||||
function escapeRegExp(s) {
|
||||
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
}
|
||||
|
||||
function countMatchedFields(body, fieldSet) {
|
||||
// A generic "label line" is a markdown heading (`## Label`) or a bolded label on
|
||||
// its own line (`**Label**`). The content scan stops at a label line, because
|
||||
// that line starts a new field.
|
||||
const LABEL_LINE = /^\s*(?:#{1,6}\s+\S|(?:\*\*|__)[^*_].*(?:\*\*|__)\s*[:?]?\s*$)/;
|
||||
|
||||
// Build the regex that matches one field label on its own line.
|
||||
function labelLinePattern(label) {
|
||||
const esc = escapeRegExp(label);
|
||||
// Accept markdown headings or bolded/plain labels on their own line.
|
||||
// Examples: "## What happened?", "**Expected behavior**", "Problem:".
|
||||
return new RegExp(
|
||||
`^\\s*(?:#{1,6}\\s+|\\*\\*\\s*|__\\s*)?${esc}(?:\\s*[:?])?(?:\\s*\\*\\*|\\s*__)?\\s*$`,
|
||||
'i'
|
||||
);
|
||||
}
|
||||
|
||||
// Every known field label from every template, precompiled. The generic
|
||||
// LABEL_LINE regex sees a heading or a bold label as a field boundary, but not a
|
||||
// plain "Label:" line. A skeleton of stacked plain labels needs each label to
|
||||
// act as a boundary. Without this list the scan reads the next label as
|
||||
// content, so it counts an empty field as filled.
|
||||
const KNOWN_LABEL_PATTERNS = Object.values(TEMPLATE_FIELDS)
|
||||
.flat(2)
|
||||
.map(labelLinePattern);
|
||||
|
||||
// Return true if the line starts a new field. The line is a heading, a bold
|
||||
// label, or a plain line that equals a known field label.
|
||||
function isFieldBoundary(line) {
|
||||
return LABEL_LINE.test(line) || KNOWN_LABEL_PATTERNS.some(p => p.test(line));
|
||||
}
|
||||
|
||||
// Return true if the line holds real content, not a bare placeholder. The
|
||||
// default template skeleton puts a lone "-" under each label, so a label with
|
||||
// only "-", blank lines, or a "[...]" placeholder does not count as filled.
|
||||
function lineHasContent(line) {
|
||||
let text = line.trim();
|
||||
if (!text) return false;
|
||||
// Drop a leading list marker ("- ", "* ", "1. ") before the check.
|
||||
text = text.replace(/^[-*+]\s*/, '').replace(/^\d+[.)]\s*/, '').trim();
|
||||
if (!text) return false;
|
||||
// Treat a whole-line bracket placeholder ("[describe here]") as empty.
|
||||
if (/^\[.*\]$/.test(text)) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
// Return true if a label variant appears on its own line AND at least one
|
||||
// content line follows it before the next label line.
|
||||
function isFieldFilled(lines, variants) {
|
||||
const patterns = variants.map(labelLinePattern);
|
||||
for (let i = 0; i < lines.length; i += 1) {
|
||||
if (!patterns.some(p => p.test(lines[i]))) continue;
|
||||
for (let j = i + 1; j < lines.length; j += 1) {
|
||||
if (isFieldBoundary(lines[j])) break; // next field starts here
|
||||
if (lineHasContent(lines[j])) return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function countMatchedFields(lines, fieldSet) {
|
||||
let matched = 0;
|
||||
for (const variants of fieldSet) {
|
||||
const hasMatch = variants.some(label => {
|
||||
const esc = escapeRegExp(label);
|
||||
// Accept markdown headings or bolded/plain labels on their own line.
|
||||
// Examples: "## What happened?", "**Expected behavior**", "Problem:".
|
||||
const pattern = new RegExp(
|
||||
`^\\s*(?:#{1,6}\\s+|\\*\\*\\s*|__\\s*)?${esc}(?:\\s*[:?])?(?:\\s*\\*\\*|\\s*__)?\\s*$`,
|
||||
'im'
|
||||
);
|
||||
return pattern.test(body);
|
||||
});
|
||||
if (hasMatch) matched += 1;
|
||||
if (isFieldFilled(lines, variants)) matched += 1;
|
||||
}
|
||||
return matched;
|
||||
}
|
||||
|
||||
// Remove HTML comments. The PR template puts its guidance and its example
|
||||
// issue links ("Fixes: #123") inside comments, so the gate must not read them
|
||||
// as author content.
|
||||
function stripHtmlComments(body) {
|
||||
return body.replace(/<!--[\s\S]*?-->/g, '');
|
||||
}
|
||||
|
||||
export function hasInlineIssueDescription(body) {
|
||||
if (!body || !body.trim()) return false;
|
||||
// Strip the guidance comments, then scan the body line by line.
|
||||
const lines = stripHtmlComments(body).split(/\r?\n/);
|
||||
for (const fieldSet of Object.values(TEMPLATE_FIELDS)) {
|
||||
if (countMatchedFields(body, fieldSet) >= INLINE_DESCRIPTION_MIN_FIELDS) {
|
||||
if (countMatchedFields(lines, fieldSet) >= INLINE_DESCRIPTION_MIN_FIELDS) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -98,7 +175,7 @@ export function checkLinkedIssue(body, prTitle = '') {
|
|||
return { passed: false, failures: ['PR body is empty — please fill out the PR template'] };
|
||||
}
|
||||
|
||||
const linked = ISSUE_PATTERNS.some(p => p.test(body));
|
||||
const linked = ISSUE_PATTERNS.some(p => p.test(stripHtmlComments(body)));
|
||||
const inlined = hasInlineIssueDescription(body);
|
||||
const passed = linked || inlined;
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { checkLinkedIssue, hasInlineIssueDescription } from '../check-pr-linked-issue.mjs';
|
||||
|
||||
// Existing tests with title parameter added (defaults to no prefix, so still required)
|
||||
|
|
@ -221,3 +223,184 @@ None.
|
|||
`;
|
||||
assert.equal(hasInlineIssueDescription(body), true);
|
||||
});
|
||||
|
||||
// Prose-only description (no template labels) must fail. A good paragraph of
|
||||
// prose matches zero labels, so the gate rejects it.
|
||||
test('fails with a prose-only description that has no template labels', () => {
|
||||
const body = `
|
||||
This pull request rewrites the retry loop so the worker gives up after five
|
||||
attempts instead of looping forever. The previous loop could hang a job when
|
||||
the upstream service was down. I also added a log line for each retry so an
|
||||
operator can see the backoff in the run output.
|
||||
`;
|
||||
const result = checkLinkedIssue(body, 'feat: bounded retry');
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures.length > 0);
|
||||
});
|
||||
|
||||
// An author who copies the feature template labels into the PR body must pass.
|
||||
// The labels use the bold-label-on-its-own-line form the gate accepts.
|
||||
const FEATURE_BOLD_LABEL_BODY = `
|
||||
**Problem or motivation:**
|
||||
- The gate rejects a good prose description.
|
||||
|
||||
**Proposed solution:**
|
||||
- Copy the feature template labels into the PR body.
|
||||
|
||||
**Alternatives considered:**
|
||||
- Lower the field threshold — rejected, it weakens the gate.
|
||||
`;
|
||||
|
||||
test('passes with the feature template labels (bold labels)', () => {
|
||||
assert.equal(checkLinkedIssue(FEATURE_BOLD_LABEL_BODY, 'feat: inline feature description').passed, true);
|
||||
});
|
||||
|
||||
// Enhancement template set (matches .github/ISSUE_TEMPLATE/enhancement.yml).
|
||||
const ENHANCEMENT_INLINE_BODY = `
|
||||
## What existing behavior does this improve?
|
||||
|
||||
The board task list sort order.
|
||||
|
||||
## Current behavior
|
||||
|
||||
The list sorts by creation time only.
|
||||
|
||||
## Proposed behavior
|
||||
|
||||
The list sorts by priority, then creation time.
|
||||
|
||||
## Reason and benefit
|
||||
|
||||
Users miss high-priority tasks that were created early.
|
||||
`;
|
||||
|
||||
test('passes with inline enhancement description (4 template fields)', () => {
|
||||
assert.equal(checkLinkedIssue(ENHANCEMENT_INLINE_BODY, 'feat: sort by priority').passed, true);
|
||||
});
|
||||
|
||||
test('hasInlineIssueDescription returns true for ≥3 enhancement fields', () => {
|
||||
assert.equal(hasInlineIssueDescription(ENHANCEMENT_INLINE_BODY), true);
|
||||
});
|
||||
|
||||
// Empty default skeleton must fail. A label with only the bare "-" placeholder
|
||||
// under it is not filled, so it must not count toward the field minimum.
|
||||
const EMPTY_SKELETON_BODY = `
|
||||
**What happened?**
|
||||
-
|
||||
|
||||
**Expected behavior:**
|
||||
-
|
||||
|
||||
**Steps to reproduce:**
|
||||
-
|
||||
`;
|
||||
|
||||
test('fails with an empty template skeleton (labels but no content)', () => {
|
||||
const result = checkLinkedIssue(EMPTY_SKELETON_BODY, 'feat: something');
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures.length > 0);
|
||||
});
|
||||
|
||||
test('hasInlineIssueDescription returns false for an empty skeleton', () => {
|
||||
assert.equal(hasInlineIssueDescription(EMPTY_SKELETON_BODY), false);
|
||||
});
|
||||
|
||||
// A filled bug skeleton in the bold-label form must pass, even with list-marker
|
||||
// content. This proves the fix does not reject real author content.
|
||||
const FILLED_BUG_SKELETON_BODY = `
|
||||
**What happened?**
|
||||
- The login button does nothing.
|
||||
|
||||
**Expected behavior:**
|
||||
- The login button authenticates the user.
|
||||
|
||||
**Steps to reproduce:**
|
||||
- Open the app, then click login.
|
||||
`;
|
||||
|
||||
test('passes with a filled bug skeleton (three filled fields)', () => {
|
||||
assert.equal(checkLinkedIssue(FILLED_BUG_SKELETON_BODY, 'feat: fix login').passed, true);
|
||||
});
|
||||
|
||||
// Stacked plain labels with no content must fail. Each label sits on its own
|
||||
// line with the next label directly under it. The scan must treat the next
|
||||
// label as a field boundary, not as content, so every field stays empty.
|
||||
const STACKED_FEATURE_LABELS = `
|
||||
Problem or motivation:
|
||||
Proposed solution:
|
||||
Alternatives considered:
|
||||
Roadmap alignment:
|
||||
`;
|
||||
|
||||
const STACKED_BUG_LABELS = `
|
||||
What happened?:
|
||||
Expected behavior:
|
||||
Steps to reproduce:
|
||||
Paperclip version:
|
||||
`;
|
||||
|
||||
const STACKED_ENHANCEMENT_LABELS = `
|
||||
What existing behavior does this improve?
|
||||
Subsystem affected
|
||||
Current behavior
|
||||
Proposed behavior
|
||||
Reason and benefit
|
||||
`;
|
||||
|
||||
const STACKED_DOCS_LABELS = `
|
||||
Issue type
|
||||
Where is the issue?
|
||||
What's wrong?
|
||||
Suggested fix
|
||||
`;
|
||||
|
||||
test('fails with stacked plain feature labels and no content', () => {
|
||||
assert.equal(checkLinkedIssue(STACKED_FEATURE_LABELS, 'feat: x').passed, false);
|
||||
});
|
||||
|
||||
test('fails with stacked plain bug labels and no content', () => {
|
||||
assert.equal(checkLinkedIssue(STACKED_BUG_LABELS, 'feat: x').passed, false);
|
||||
});
|
||||
|
||||
test('fails with stacked plain enhancement labels and no content', () => {
|
||||
assert.equal(checkLinkedIssue(STACKED_ENHANCEMENT_LABELS, 'feat: x').passed, false);
|
||||
});
|
||||
|
||||
test('fails with stacked plain docs labels and no content', () => {
|
||||
assert.equal(checkLinkedIssue(STACKED_DOCS_LABELS, 'feat: x').passed, false);
|
||||
});
|
||||
|
||||
// A plain-label skeleton with real content under each label must still pass.
|
||||
// The boundary fix must not reject a field that has genuine content.
|
||||
const FILLED_PLAIN_FEATURE_LABELS = `
|
||||
Problem or motivation:
|
||||
- The gate rejects a good prose description.
|
||||
Proposed solution:
|
||||
- Copy the feature template labels into the PR body.
|
||||
Alternatives considered:
|
||||
- Lower the field threshold — rejected, it weakens the gate.
|
||||
`;
|
||||
|
||||
test('passes with plain feature labels and real content under each', () => {
|
||||
assert.equal(checkLinkedIssue(FILLED_PLAIN_FEATURE_LABELS, 'feat: inline feature').passed, true);
|
||||
});
|
||||
|
||||
// The real .github/PULL_REQUEST_TEMPLATE.md, submitted unchanged, must fail the
|
||||
// gate. Its skeleton labels have no content and its example issue links live in
|
||||
// HTML comments, so neither the inline path nor the linked path may pass it.
|
||||
const PR_TEMPLATE_PATH = fileURLToPath(
|
||||
new URL('../../PULL_REQUEST_TEMPLATE.md', import.meta.url)
|
||||
);
|
||||
|
||||
test('fails with the unfilled default PR template body', () => {
|
||||
const body = readFileSync(PR_TEMPLATE_PATH, 'utf8');
|
||||
const result = checkLinkedIssue(body, 'feat: unfilled template');
|
||||
assert.equal(result.passed, false);
|
||||
});
|
||||
|
||||
// An issue link that appears only inside an HTML comment must not satisfy the
|
||||
// linked-issue check. The template ships such an example ("Fixes: #123").
|
||||
test('fails when the only issue link is inside an HTML comment', () => {
|
||||
const body = '<!-- Example: Fixes: #123 -->\n\nSome prose with no real link.';
|
||||
assert.equal(checkLinkedIssue(body, 'feat: commented link').passed, false);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue