mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 21:05:21 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work, and it takes contributions from outside the core team > - Those contributions arrive as PRs, and this repository squash-merges every one of them > - A squash collapses the whole branch into a single commit authored by whoever pressed the button > - So when a maintainer rebases and lands a contributor's stale PR, the contributor's name survives only if the squash message carries a `Co-Authored-By` trailer > - Nothing prompts for that trailer, and the PR page keeps showing the original author either way, so losing it is invisible at the moment it happens > - This pull request has commitperclip detect the situation and print the exact trailers to paste > - The benefit is that keeping an outside contributor's name is a default rather than something a maintainer has to remember ## Linked Issues or Issue Description No public issue exists. The problem follows, and it is not hypothetical. **What happened?** #11370, #11371 and #11379 landed @stubbi's work yesterday. Each of those PRs carries a comment from me telling them their authorship would be preserved. All three squash commits went in without a `Co-Authored-By` trailer, so `git log` credits none of them: | commit | landed from | credited | | --- | --- | --- | | `66515582e` | #9900 | Claude only | | `bc0b5a164` | #9501 | Claude only | | `35a9b9873` | #8982 | Claude only | | `6542ad1f4` | #11259 | ✅ Jannes Stubbemann + Claude | The last one has the trailer because that message was written by hand with the contributor in mind. The only difference between the two outcomes was memory. Master history cannot be rewritten, so those three are now credited by comment on the original PRs — which is a worse record than a commit trailer, and the reason to make this automatic. **Expected behavior** When a branch carries commits by someone other than the PR author, the merger is told what trailers the squash needs. **Paperclip version or commit** `master` at `92047cac4`. ## What Changed - `.github/scripts/check-pr-coauthors.mjs` — new gate. - `.github/scripts/run-quality-gates.mjs` — fetches the PR's commits and runs it. - `.github/scripts/tests/check-pr-coauthors.test.mjs` — 12 cases. - `.github/workflows/pr.yml` — runs `.github/scripts/tests/`. ### Informational, not a failure The squash message does not exist while the PR is open. This can neither be verified there nor fixed there, so failing a PR on it would block work on something its author cannot satisfy. The gate notices that the situation applies and prints the lines to paste. Run against #11370's actual commits it produces exactly what was missing: ``` This branch carries commits by stubbi. Squash-merging drops that authorship unless the squash message carries their trailers, and nothing else will notice if it does not. Add to the squash body when merging: Co-Authored-By: Jannes Stubbemann <stubbi@users.noreply.github.com> ``` ### Edge cases it handles Bots skipped; the PR author's own commits skipped; logins compared case-insensitively (`PR_AUTHOR` does not always arrive in the same case as the commit author login); each contributor listed once however many commits they wrote; and a commit GitHub could not match to an account falls back to its raw git author — that identity being the one most likely to be lost, not least likely. Paging stops at the API's own 250-commit ceiling rather than spinning on full pages of nothing new. ### The test directory was not running `.github/scripts/tests/` held ten test files covering the existing gates, and no workflow ran any of them. Adding an eleventh would have meant adding a test that never executes, so `pr.yml` now runs the directory. All **149** pass, including the 137 that were already there and previously unverified in CI. ## Verification - 149 tests pass via `node --test '.github/scripts/tests/*.test.mjs'` — the exact command CI now runs. - The gate was run against the real commit shape from #11370 and produces the missing trailer verbatim. This PR is its own negative control: the branch carries only my commits, so the new gate should stay silent on it. If commitperclip prints a co-author note below, the gate is wrong. ## Risks Low. Informational output only — it cannot fail a PR, and `allPassed` is unchanged. It adds one API call per gate run (`/pulls/{n}/commits`), fetched in the same `Promise.all` as the existing PR and files calls. Enabling the previously-unrun test directory could in principle surface a pre-existing failure; all 149 pass locally, so it does not. Revert the commit to restore. ## Model Used Claude Opus 5 (`claude-opus-5`), through Claude Code. Extended thinking enabled. Tool use enabled: file read and edit, shell for test runs. ## 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 - [x] My branch name describes the change and contains no internal ticket id - [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: Claude Opus 5 <noreply@anthropic.com>
168 lines
6.1 KiB
JavaScript
168 lines
6.1 KiB
JavaScript
#!/usr/bin/env node
|
|
/**
|
|
* run-quality-gates.mjs
|
|
* Orchestrates all quality gates. Fetches PR data once, runs all gates,
|
|
* posts or updates a single consolidated comment via commitperclip.
|
|
*
|
|
* Env: GH_TOKEN, GH_REPO, PR_NUMBER, PR_AUTHOR, PR_BRANCH
|
|
* Exit: 0 if all quality gates pass, 1 if any fail.
|
|
*/
|
|
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 { checkDedupSearch } from './check-pr-dedup-search.mjs';
|
|
import { checkTestCoverage } from './check-pr-test-coverage.mjs';
|
|
import { checkLockfile } from './check-pr-lockfile.mjs';
|
|
import { checkDependencies } from './check-pr-dependencies.mjs';
|
|
import { checkReleaseBootstrap } from './check-pr-release-bootstrap.mjs';
|
|
import { checkCoauthors, fetchAllPullRequestCommits } from './check-pr-coauthors.mjs';
|
|
|
|
const COMMENT_SIGNATURE = '— commitperclip';
|
|
|
|
function buildComment(author, failures, informational) {
|
|
if (failures.length === 0 && informational.length === 0) {
|
|
return `✅ All checks passing — ready for Greptile review and maintainer approval.\n\n${COMMENT_SIGNATURE}`;
|
|
}
|
|
|
|
const lines = [
|
|
`Hey @${author}! Before this PR can be reviewed, a few things need attention:\n`,
|
|
];
|
|
|
|
if (failures.length > 0) {
|
|
lines.push('**Missing or incomplete:**');
|
|
for (const f of failures) lines.push(`- [ ] ${f}`);
|
|
}
|
|
|
|
if (informational.length > 0) {
|
|
if (failures.length > 0) lines.push('');
|
|
lines.push('**Informational:**');
|
|
for (const i of informational) lines.push(`- ${i}`);
|
|
}
|
|
|
|
lines.push(
|
|
'\nOnce updated, push a new commit and these checks will re-run automatically.\n',
|
|
COMMENT_SIGNATURE
|
|
);
|
|
|
|
return lines.join('\n');
|
|
}
|
|
|
|
export async function findExistingComment(fetchFromGitHub, token, repo, prNumber) {
|
|
for (let page = 1; ; page += 1) {
|
|
const comments = await fetchFromGitHub(
|
|
`/repos/${repo}/issues/${prNumber}/comments?per_page=100&page=${page}`,
|
|
token
|
|
);
|
|
|
|
const existing = comments.find(
|
|
c => (c.user.login === 'commitperclip[bot]' || c.user.login === 'commitperclip') &&
|
|
c.body.includes(COMMENT_SIGNATURE)
|
|
);
|
|
if (existing) return existing;
|
|
|
|
if (comments.length < 100) return null;
|
|
}
|
|
}
|
|
|
|
async function upsertComment(token, repo, prNumber, body, existing) {
|
|
if (existing) {
|
|
await ghFetch(`/repos/${repo}/issues/comments/${existing.id}`, token, {
|
|
method: 'PATCH',
|
|
headers: { 'Content-Type': 'application/json' },
|
|
body: JSON.stringify({ body }),
|
|
});
|
|
} else {
|
|
await ghFetch(`/repos/${repo}/issues/${prNumber}/comments`, token, {
|
|
method: 'POST',
|
|
headers: { 'Content-Type': 'application/json' },
|
|
body: JSON.stringify({ body }),
|
|
});
|
|
}
|
|
}
|
|
|
|
async function main() {
|
|
const { GH_TOKEN, GH_REPO, PR_NUMBER, PR_AUTHOR, PR_BRANCH } = process.env;
|
|
|
|
if (!GH_TOKEN || !GH_REPO || !PR_NUMBER) {
|
|
console.error('ERROR: GH_TOKEN, GH_REPO, PR_NUMBER env vars required');
|
|
process.exit(1);
|
|
}
|
|
|
|
// Sanitize inputs before use in URL construction (prevents SSRF)
|
|
const prNumber = parseInt(PR_NUMBER, 10);
|
|
if (!Number.isInteger(prNumber) || prNumber <= 0) {
|
|
console.error('ERROR: PR_NUMBER must be a positive integer');
|
|
process.exit(1);
|
|
}
|
|
if (!/^[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/.test(GH_REPO)) {
|
|
console.error('ERROR: GH_REPO must be in owner/repo format');
|
|
process.exit(1);
|
|
}
|
|
|
|
// Fetch PR data once — gates use this, no redundant API calls
|
|
const [pr, files] = await Promise.all([
|
|
ghFetch(`/repos/${GH_REPO}/pulls/${prNumber}`, GH_TOKEN),
|
|
fetchAllPullRequestFiles(ghFetch, GH_REPO, prNumber, GH_TOKEN),
|
|
]);
|
|
|
|
// Separate, and allowed to fail. The co-author note is informational: it
|
|
// cannot fail a PR by design, so it must not be able to fail the workflow by
|
|
// accident either. Sharing the Promise.all above would let one transient
|
|
// 5xx on this request take down every gate, including the ones that block.
|
|
let commits = [];
|
|
try {
|
|
commits = await fetchAllPullRequestCommits(ghFetch, GH_REPO, prNumber, GH_TOKEN);
|
|
} catch (error) {
|
|
console.error(`co-author lookup skipped: ${error.message}`);
|
|
}
|
|
|
|
const prBody = pr.body ?? '';
|
|
const author = PR_AUTHOR ?? pr.user.login;
|
|
const branch = PR_BRANCH ?? pr.head.ref;
|
|
|
|
// Run all quality gates (pure functions run sync, deps check is async)
|
|
const prTitle = pr.title ?? '';
|
|
const [templateResult, issueResult, dedupResult, testResult, lockfileResult, depsResult, bootstrapResult] =
|
|
await Promise.all([
|
|
Promise.resolve(checkTemplate(prBody)),
|
|
Promise.resolve(checkLinkedIssue(prBody, prTitle)),
|
|
Promise.resolve(checkDedupSearch(prBody, prTitle)),
|
|
Promise.resolve(checkTestCoverage(files, prTitle)),
|
|
Promise.resolve(checkLockfile(files, author, branch)),
|
|
checkDependencies(files, GH_TOKEN, GH_REPO, prNumber, pr.base?.ref),
|
|
checkReleaseBootstrap(files, GH_TOKEN, GH_REPO, prNumber, pr.base?.ref),
|
|
]);
|
|
const coauthorResult = checkCoauthors(commits, author);
|
|
|
|
const allFailures = [
|
|
...templateResult.failures,
|
|
...issueResult.failures,
|
|
...dedupResult.failures,
|
|
...testResult.failures,
|
|
...lockfileResult.failures,
|
|
];
|
|
const informational = [
|
|
...(depsResult.informational ?? []),
|
|
...(bootstrapResult.informational ?? []),
|
|
...coauthorResult.informational,
|
|
];
|
|
const allPassed = allFailures.length === 0;
|
|
|
|
const commentBody = buildComment(author, allFailures, informational);
|
|
|
|
// Post comment if there are failures/informational, or update existing comment
|
|
const existing = await findExistingComment(ghFetch, GH_TOKEN, GH_REPO, prNumber);
|
|
if (allFailures.length > 0 || informational.length > 0 || existing) {
|
|
await upsertComment(GH_TOKEN, GH_REPO, prNumber, commentBody, existing);
|
|
}
|
|
|
|
console.log(JSON.stringify({ passed: allPassed, failures: allFailures, informational }));
|
|
process.exit(allPassed ? 0 : 1);
|
|
}
|
|
|
|
if (process.argv[1] === fileURLToPath(import.meta.url)) {
|
|
main().catch(e => { console.error(e.message); process.exit(1); });
|
|
}
|