Skip to content

Commit d618e7b

Browse files
committed
Move the gate logic to a script and soften the contributor docs
Review feedback on the intake gate: - The rules now live in .github/scripts/pr_intake_gate.js, structured as entry points -> numbered rules -> outcomes -> helpers, and the workflow file is reduced to triggers, routing and a checkout + require. Dropped the refused-reopen diagnosis, comment minimizing and the check re-run in favour of one honest message, delete-on-pass, and not failing the check on a verdict, which also removes the actions: write permission. - CONTRIBUTING.md is reworded to be clear without being combative, is explicit that mentoring depends on maintainer capacity, and its new headings follow the file's existing title case. - AGENTS.md just points agents at CONTRIBUTING.md. - The PR template is the org template plus a short note about the gate, a Fixes line, and two checklist items. No-Verification-Needed: workflow and docs only; exercised with a mock harness, actionlint and zizmor Signed-off-by: Max Isbey <224885523+maxisbey@users.noreply.github.com>
1 parent ed4c143 commit d618e7b

5 files changed

Lines changed: 376 additions & 503 deletions

File tree

.github/pull_request_template.md

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,38 @@
11
<!--
2-
PRs from outside the maintainer team are auto-closed unless they link an open
3-
issue you are assigned to (or one labeled `help wanted`). If that's not you
4-
yet, file or find the issue first — see CONTRIBUTING.md#how-pull-requests-get-in.
5-
Keep the description short: what changed and why, in your own words.
2+
Pull requests from outside the maintainer team need to link an open issue that
3+
a maintainer has assigned to you (or one labeled `help wanted`); others are
4+
closed automatically until that's in place. See CONTRIBUTING.md for details.
65
-->
76

87
Fixes #
98

10-
## What and why
9+
<!-- Provide a brief summary of your changes -->
1110

12-
## How it was tested
11+
## Motivation and Context
12+
<!-- Why is this change needed? What problem does it solve? -->
13+
14+
## How Has This Been Tested?
15+
<!-- Have you tested this in a real application? Which scenarios were tested? -->
16+
17+
## Breaking Changes
18+
<!-- Will users need to update their code or configurations? -->
19+
20+
## Types of changes
21+
<!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
22+
- [ ] Bug fix (non-breaking change which fixes an issue)
23+
- [ ] New feature (non-breaking change which adds functionality)
24+
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
25+
- [ ] Documentation update
1326

1427
## Checklist
28+
<!-- Go over all the following points, and put an `x` in all the boxes that apply. -->
29+
- [ ] I am assigned to the linked issue (or it is labeled `help wanted`, or I'm a maintainer)
30+
- [ ] I have disclosed any AI assistance and can explain the change in my own words
31+
- [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io)
32+
- [ ] My code follows the repository's style guidelines
33+
- [ ] New and existing tests pass locally
34+
- [ ] I have added appropriate error handling
35+
- [ ] I have added or updated documentation as needed
1536

16-
- [ ] I'm assigned to the linked issue, it's labeled `help wanted`, or I'm exempt (maintainer / trusted contributor)
17-
- [ ] AI assistance, if any, is disclosed above and I can explain every line of this diff
18-
- [ ] Tests added/updated; `uv run --frozen pytest` and `uv run --frozen pyright` pass locally
19-
- [ ] Docs and `docs/migration.md` updated if behaviour or public API changed
37+
## Additional context
38+
<!-- Add any other context, implementation notes, or design decisions -->

.github/scripts/pr_intake_gate.js

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
// PR intake gate. The policy lives in CONTRIBUTING.md ("How pull requests get
2+
// in"); .github/workflows/require-linked-issue.yml wires this up to events.
3+
//
4+
// A pull request from someone without triage rights stays open only if its
5+
// description links (Fixes/Closes/Resolves #N) an open issue in this repo that
6+
// is either assigned to the PR author or labeled `help wanted`. Otherwise the
7+
// gate labels it `missing-issue-link`, leaves one comment, and closes it. It
8+
// re-evaluates — and reopens — the PR when the description is edited or the
9+
// author is assigned to the issue. A triage+ user reopening the PR or removing
10+
// the label is a sticky override (`bypass-issue-check`).
11+
//
12+
// Everything that writes goes through mutate(), so with ENFORCE unset the run
13+
// only logs what it would have done.
14+
'use strict';
15+
16+
const LABEL = 'missing-issue-link'; // marks PRs the gate has closed
17+
const BYPASS_LABEL = 'bypass-issue-check'; // sticky maintainer override
18+
const OPEN_LABEL = 'help wanted'; // issue label that waives assignment
19+
const MARKER = '<!-- require-linked-issue -->';
20+
const BOT_LOGIN = 'github-actions[bot]';
21+
const MAX_ISSUES = 5;
22+
23+
module.exports = async function run({ github, context, core }) {
24+
const { owner, repo } = context.repo;
25+
const enforce = process.env.ENFORCE === 'true';
26+
const contributingUrl = `https://github.com/${owner}/${repo}/blob/main/CONTRIBUTING.md#how-pull-requests-get-in`;
27+
28+
// ── Entry points ─────────────────────────────────────────────────────────
29+
30+
if (context.eventName === 'issues') {
31+
// Someone was assigned an issue: re-evaluate their gate-closed PRs that
32+
// reference it (they may pass now).
33+
const issueNumber = context.payload.issue.number;
34+
const assignee = context.payload.assignee.login;
35+
const closed = await github.paginate(github.rest.issues.listForRepo, {
36+
owner, repo, state: 'closed', creator: assignee, labels: LABEL, per_page: 100,
37+
});
38+
const prs = closed.filter((i) => i.pull_request && closingRefs(i.body).includes(issueNumber));
39+
console.log(`#${issueNumber} assigned to ${assignee}: ${prs.length} gate-closed PR(s) reference it`);
40+
for (const pr of prs) await evaluate(pr.number, 'assigned', context.payload.sender?.login);
41+
return;
42+
}
43+
44+
if (context.eventName === 'workflow_dispatch') {
45+
const n = parseInt(process.env.PR_NUMBER_INPUT, 10);
46+
if (!Number.isInteger(n) || n <= 0) throw new Error(`Bad pr_number input: ${process.env.PR_NUMBER_INPUT}`);
47+
await evaluate(n, 'dispatch', context.payload.sender?.login);
48+
return;
49+
}
50+
51+
await evaluate(context.payload.pull_request.number, context.payload.action, context.payload.sender?.login);
52+
53+
// ── The rules ────────────────────────────────────────────────────────────
54+
55+
async function evaluate(prNumber, action, sender) {
56+
// Always read the PR live; the event payload can be stale by the time a
57+
// queued run starts.
58+
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber });
59+
const labels = pr.labels.map((l) => l.name);
60+
// An `unlabeled` run only fires for LABEL (see the workflow `if:`), so the
61+
// event itself proves the label was there a moment ago.
62+
const gated = action === 'unlabeled' || labels.includes(LABEL);
63+
console.log(`PR #${prNumber} by ${pr.user.login} (${pr.state}${pr.draft ? ', draft' : ''}) — ${action} by ${sender ?? '-'}, enforce=${enforce}`);
64+
65+
// 0. Scope: open PRs, plus closed PRs the gate closed itself. A PR someone
66+
// closed for other reasons is left alone.
67+
if (pr.state === 'closed' && !gated) return log('closed by someone else — not ours');
68+
69+
// 1. Exempt authors: bots, anyone with triage or better, and drafts (which
70+
// are checked again on ready_for_review).
71+
if (pr.user.type === 'Bot') return log('author is a bot — exempt');
72+
if (await isTrusted(pr.user.login)) return pass('author has triage+ on this repo');
73+
if (pr.draft) return log('draft — skipped until ready for review');
74+
75+
// 2. Overrides: a triage+ user reopening the PR or removing the label wants
76+
// it open. Anyone else doing so just triggers a re-check.
77+
if ((action === 'reopened' || action === 'unlabeled') && sender && (await isTrusted(sender))) {
78+
return pass(`${sender} ${action === 'reopened' ? 'reopened it' : 'removed the label'} — override`, { sticky: true });
79+
}
80+
if (labels.includes(BYPASS_LABEL)) return pass(`carries ${BYPASS_LABEL}`);
81+
82+
// 3. The rule: the description links an open issue in this repo that is
83+
// labeled `help wanted` or assigned to the author.
84+
const author = pr.user.login.toLowerCase();
85+
const linked = [];
86+
for (const num of closingRefs(pr.body).slice(0, MAX_ISSUES)) {
87+
const issue = await getIssue(num);
88+
if (!issue) continue; // missing, a PR, closed, or transferred away
89+
linked.push(num);
90+
if (issue.labels.some((l) => (l.name ?? l).toLowerCase() === OPEN_LABEL)) return pass(`#${num} is labeled "${OPEN_LABEL}"`);
91+
if (issue.assignees.some((a) => a.login.toLowerCase() === author)) return pass(`author is assigned to #${num}`);
92+
}
93+
return fail(linked);
94+
95+
// ── Outcomes ─────────────────────────────────────────────────────────
96+
97+
async function pass(reason, { sticky = false } = {}) {
98+
console.log(`PASS: ${reason}`);
99+
if (sticky) await addLabel(prNumber, BYPASS_LABEL);
100+
if (pr.state === 'closed' && !(await reopen(pr, reason))) return;
101+
if (gated) {
102+
await removeLabel(prNumber, LABEL);
103+
await deleteGateComment(prNumber);
104+
}
105+
}
106+
107+
async function fail(linkedIssues) {
108+
console.log(`FAIL: ${linkedIssues.length ? `not assigned to ${linkedIssues.map((n) => `#${n}`).join(', ')}` : 'no usable issue link'}`);
109+
await addLabel(prNumber, LABEL);
110+
await upsertGateComment(prNumber, closedComment(linkedIssues));
111+
if (pr.state === 'open') {
112+
await mutate(`close PR #${prNumber}`, () => github.rest.pulls.update({ owner, repo, pull_number: prNumber, state: 'closed' }));
113+
}
114+
}
115+
116+
function log(msg) {
117+
console.log(msg);
118+
}
119+
}
120+
121+
// ── Comment text ─────────────────────────────────────────────────────────
122+
123+
function closedComment(linkedIssues) {
124+
const issues = linkedIssues.map((n) => `#${n}`).join(', ');
125+
const why = linkedIssues.length
126+
? `you aren't currently assigned to ${issues}`
127+
: "its description doesn't yet link an open issue in this repository (with `Fixes #123` or similar)";
128+
const next = linkedIssues.length
129+
? `If a maintainer would like this change as a PR from you, they'll assign you to ${issues} and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.)`
130+
: `If there isn't an issue for this yet, please [open one](https://github.com/${owner}/${repo}/issues/new/choose) — a clear description of the problem is genuinely the most useful thing for us. Then add \`Fixes #<number>\` to this PR's description. If a maintainer would like the change as a PR from you, they'll assign you to the issue and this PR will reopen automatically.`;
131+
return [
132+
MARKER,
133+
`Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — [CONTRIBUTING.md](${contributingUrl}) explains why and how we work. This PR has been closed for now because ${why}.`,
134+
'',
135+
next,
136+
'',
137+
"There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten.",
138+
'',
139+
`*Maintainers: reopening this PR or removing the \`${LABEL}\` label bypasses the check.*`,
140+
].join('\n');
141+
}
142+
143+
function cannotReopenComment(pr, reason) {
144+
return [
145+
MARKER,
146+
`This PR now passes the intake check (${reason}), but GitHub won't let it be reopened — usually because the branch was force-pushed or deleted while the PR was closed, or because another open PR uses the same branch.`,
147+
'',
148+
`If you have another open PR from this branch, please continue there. Otherwise, either push the branch back to \`${pr.head.sha.slice(0, 7)}\` and edit this PR's description to retry, or open a new PR with the same \`Fixes #<issue>\` line.`,
149+
].join('\n');
150+
}
151+
152+
// ── Helpers ──────────────────────────────────────────────────────────────
153+
154+
async function mutate(description, fn) {
155+
if (!enforce) {
156+
console.log(`[dry-run] would ${description}`);
157+
return undefined;
158+
}
159+
return fn();
160+
}
161+
162+
// Triage-or-better on this repo, from the permission endpoint's capability
163+
// flags (role names can be custom; author_association hides private org
164+
// members). Only a nonexistent user 404s; any other error must throw rather
165+
// than be read as "untrusted", or a maintainer's PR could be closed.
166+
async function isTrusted(username) {
167+
try {
168+
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo, username });
169+
const p = data.user?.permissions;
170+
if (!p) throw new Error(`permission response for ${username} has no capability flags`);
171+
const trusted = Boolean(p.triage || p.push || p.maintain || p.admin);
172+
console.log(` ${username}: ${trusted ? 'trusted' : 'not trusted'} (role ${data.role_name || '-'})`);
173+
return trusted;
174+
} catch (e) {
175+
if (e.status === 404) return false;
176+
throw new Error(`Permission check failed for ${username} (HTTP ${e.status ?? '?'}): ${e.message}`);
177+
}
178+
}
179+
180+
// Issue numbers referenced with a closing keyword, in the forms GitHub itself
181+
// honors: `Fixes #1`, `closes owner/repo#1`, `Resolved https://github.com/owner/repo/issues/1`.
182+
function closingRefs(body) {
183+
const repoRef = `${owner}/${repo}`.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
184+
const re = new RegExp(
185+
`\\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\\s*:?\\s*(?:${repoRef}#|#|https?://github\\.com/${repoRef}/issues/)(\\d+)`,
186+
'gi',
187+
);
188+
return [...new Set([...(body || '').matchAll(re)].map((m) => parseInt(m[1], 10)))];
189+
}
190+
191+
// The linked issue, or null if it doesn't exist, is actually a PR, isn't
192+
// open, or has been transferred to another repository.
193+
async function getIssue(num) {
194+
let issue;
195+
try {
196+
({ data: issue } = await github.rest.issues.get({ owner, repo, issue_number: num }));
197+
} catch (e) {
198+
if (e.status === 404 || e.status === 410) return null;
199+
throw new Error(`Cannot fetch issue #${num} (HTTP ${e.status ?? '?'}): ${e.message}`);
200+
}
201+
if (issue.pull_request || issue.state !== 'open') return null;
202+
if (!issue.repository_url?.endsWith(`/${owner}/${repo}`)) return null;
203+
return issue;
204+
}
205+
206+
// Reopen a gate-closed PR. GitHub refuses (422) if the branch was rewritten
207+
// or deleted while closed, or another open PR uses it; in that case keep
208+
// the label so the PR stays findable and explain in the comment.
209+
async function reopen(pr, reason) {
210+
try {
211+
await mutate(`reopen PR #${pr.number}`, () => github.rest.pulls.update({ owner, repo, pull_number: pr.number, state: 'open' }));
212+
return true;
213+
} catch (e) {
214+
if (e.status !== 422) throw e;
215+
core.warning(`GitHub refused to reopen PR #${pr.number}: ${e.message}`);
216+
await upsertGateComment(pr.number, cannotReopenComment(pr, reason));
217+
return false;
218+
}
219+
}
220+
221+
async function addLabel(prNumber, name) {
222+
await mutate(`add "${name}" to PR #${prNumber}`, async () => {
223+
await ensureLabelExists(name);
224+
await github.rest.issues.addLabels({ owner, repo, issue_number: prNumber, labels: [name] });
225+
});
226+
}
227+
228+
async function removeLabel(prNumber, name) {
229+
await mutate(`remove "${name}" from PR #${prNumber}`, async () => {
230+
try {
231+
await github.rest.issues.removeLabel({ owner, repo, issue_number: prNumber, name });
232+
} catch (e) {
233+
if (e.status !== 404) throw e;
234+
}
235+
});
236+
}
237+
238+
async function ensureLabelExists(name) {
239+
const meta = {
240+
[LABEL]: ['b76e79', 'Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)'],
241+
[BYPASS_LABEL]: ['0e8a16', 'Maintainer override for the linked-issue intake gate'],
242+
}[name];
243+
try {
244+
await github.rest.issues.getLabel({ owner, repo, name });
245+
} catch (e) {
246+
if (e.status !== 404) throw e;
247+
try {
248+
await github.rest.issues.createLabel({ owner, repo, name, color: meta[0], description: meta[1] });
249+
} catch (createErr) {
250+
if (createErr.status !== 422) throw createErr; // created concurrently
251+
}
252+
}
253+
}
254+
255+
// The gate keeps at most one comment per PR: authored by the Actions bot and
256+
// carrying MARKER. It's created or updated on failure and deleted on pass.
257+
async function findGateComment(prNumber) {
258+
const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: prNumber, per_page: 100 });
259+
return comments.find((c) => c.user?.login === BOT_LOGIN && c.body?.includes(MARKER));
260+
}
261+
262+
async function upsertGateComment(prNumber, body) {
263+
const existing = await findGateComment(prNumber);
264+
if (!existing) {
265+
await mutate(`comment on PR #${prNumber}`, () => github.rest.issues.createComment({ owner, repo, issue_number: prNumber, body }));
266+
} else if (existing.body !== body) {
267+
await mutate(`update the gate comment on PR #${prNumber}`, () => github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }));
268+
}
269+
}
270+
271+
async function deleteGateComment(prNumber) {
272+
const existing = await findGateComment(prNumber);
273+
if (existing) await mutate(`delete the gate comment on PR #${prNumber}`, () => github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id }));
274+
}
275+
};

0 commit comments

Comments
 (0)