Gate external PRs on an assigned, linked issue (#3291)
Signed-off-by: Max Isbey <224885523+maxisbey@users.noreply.github.com>
This commit is contained in:
parent
e473cca6a1
commit
2a1cc94493
5 changed files with 874 additions and 35 deletions
38
.github/pull_request_template.md
vendored
Normal file
38
.github/pull_request_template.md
vendored
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
<!--
|
||||
Pull requests from outside the maintainer team need to link an open issue that
|
||||
a maintainer has assigned to you (or one labeled `help wanted`); others are
|
||||
closed automatically until that's in place. See CONTRIBUTING.md for details.
|
||||
-->
|
||||
|
||||
Fixes #
|
||||
|
||||
<!-- Provide a brief summary of your changes -->
|
||||
|
||||
## Motivation and Context
|
||||
<!-- Why is this change needed? What problem does it solve? -->
|
||||
|
||||
## How Has This Been Tested?
|
||||
<!-- Have you tested this in a real application? Which scenarios were tested? -->
|
||||
|
||||
## Breaking Changes
|
||||
<!-- Will users need to update their code or configurations? -->
|
||||
|
||||
## Types of changes
|
||||
<!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
|
||||
- [ ] Bug fix (non-breaking change which fixes an issue)
|
||||
- [ ] New feature (non-breaking change which adds functionality)
|
||||
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
|
||||
- [ ] Documentation update
|
||||
|
||||
## Checklist
|
||||
<!-- Go over all the following points, and put an `x` in all the boxes that apply. -->
|
||||
- [ ] I am assigned to the linked issue (or it is labeled `help wanted`, or I'm a maintainer)
|
||||
- [ ] I have disclosed any AI assistance and can explain the change in my own words
|
||||
- [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io)
|
||||
- [ ] My code follows the repository's style guidelines
|
||||
- [ ] New and existing tests pass locally
|
||||
- [ ] I have added appropriate error handling
|
||||
- [ ] I have added or updated documentation as needed
|
||||
|
||||
## Additional context
|
||||
<!-- Add any other context, implementation notes, or design decisions -->
|
||||
299
.github/scripts/pr_intake_gate.js
vendored
Normal file
299
.github/scripts/pr_intake_gate.js
vendored
Normal file
|
|
@ -0,0 +1,299 @@
|
|||
// PR intake gate. The policy lives in CONTRIBUTING.md ("How pull requests get
|
||||
// in"); .github/workflows/require-linked-issue.yml wires this up to events.
|
||||
//
|
||||
// A pull request from someone without triage rights stays open only if its
|
||||
// description links (Fixes/Closes/Resolves #N) an open issue in this repo that
|
||||
// is either assigned to the PR author or labeled `help wanted`. Otherwise the
|
||||
// gate labels it `missing-issue-link`, leaves one comment, and closes it. It
|
||||
// re-evaluates — and reopens — the PR when the description is edited or the
|
||||
// author is assigned to the issue. A triage+ user reopening the PR, removing
|
||||
// the label, or adding `bypass-issue-check` overrides it, and the override
|
||||
// sticks.
|
||||
//
|
||||
// Everything that writes goes through mutate(); when the workflow passes
|
||||
// ENFORCE=false (its kill switch) the run only logs what it would have done.
|
||||
'use strict';
|
||||
|
||||
const LABEL = 'missing-issue-link'; // marks PRs the gate has closed
|
||||
const BYPASS_LABEL = 'bypass-issue-check'; // sticky maintainer override
|
||||
const OPEN_LABEL = 'help wanted'; // issue label that waives assignment
|
||||
const MARKER = '<!-- require-linked-issue -->';
|
||||
const BOT_LOGIN = 'github-actions[bot]';
|
||||
const MAX_ISSUES = 5;
|
||||
|
||||
module.exports = async function run({ github, context, core }) {
|
||||
const { owner, repo } = context.repo;
|
||||
const enforce = process.env.ENFORCE === 'true';
|
||||
const contributingUrl = `https://github.com/${owner}/${repo}/blob/main/CONTRIBUTING.md#how-pull-requests-get-in`;
|
||||
|
||||
// ── Entry points ─────────────────────────────────────────────────────────
|
||||
|
||||
if (context.eventName === 'issues') {
|
||||
// Someone was assigned an issue: re-evaluate their gate-closed PRs that
|
||||
// reference it (they may pass now).
|
||||
const issueNumber = context.payload.issue.number;
|
||||
const assignee = context.payload.assignee.login;
|
||||
const closed = await github.paginate(github.rest.issues.listForRepo, {
|
||||
owner, repo, state: 'closed', creator: assignee, labels: LABEL, per_page: 100,
|
||||
});
|
||||
const prs = closed.filter((i) => i.pull_request && closingRefs(i.body).includes(issueNumber));
|
||||
console.log(`#${issueNumber} assigned to ${assignee}: ${prs.length} gate-closed PR(s) reference it`);
|
||||
// Evaluate each independently so one transient failure doesn't strand
|
||||
// the rest (this event won't fire again for the same assignment).
|
||||
const failures = [];
|
||||
for (const pr of prs) {
|
||||
try {
|
||||
await evaluate(pr.number, 'assigned', context.payload.sender?.login, issueNumber);
|
||||
} catch (e) {
|
||||
failures.push(`#${pr.number}: ${e.message}`);
|
||||
}
|
||||
}
|
||||
if (failures.length) throw new Error(`Could not re-evaluate ${failures.join('; ')}`);
|
||||
return;
|
||||
}
|
||||
|
||||
if (context.eventName === 'workflow_dispatch') {
|
||||
const n = parseInt(process.env.PR_NUMBER_INPUT, 10);
|
||||
if (!Number.isInteger(n) || n <= 0) throw new Error(`Bad pr_number input: ${process.env.PR_NUMBER_INPUT}`);
|
||||
await evaluate(n, 'dispatch', context.payload.sender?.login);
|
||||
return;
|
||||
}
|
||||
|
||||
await evaluate(context.payload.pull_request.number, context.payload.action, context.payload.sender?.login);
|
||||
|
||||
// ── The rules ────────────────────────────────────────────────────────────
|
||||
|
||||
async function evaluate(prNumber, action, sender, hintIssue = null) {
|
||||
// Always read the PR live; the event payload can be stale by the time a
|
||||
// queued run starts.
|
||||
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber });
|
||||
const labels = pr.labels.map((l) => l.name);
|
||||
// An `unlabeled` run only fires for LABEL (see the workflow `if:`), so the
|
||||
// event itself proves the label was there a moment ago.
|
||||
const gated = action === 'unlabeled' || labels.includes(LABEL);
|
||||
console.log(`PR #${prNumber} by ${pr.user.login} (${pr.state}${pr.draft ? ', draft' : ''}) — ${action} by ${sender ?? '-'}, enforce=${enforce}`);
|
||||
|
||||
// 0. Scope: open PRs, plus closed PRs the gate closed itself. Merged PRs
|
||||
// and PRs someone closed for other reasons are left alone.
|
||||
if (pr.merged_at) return log('merged — nothing to do');
|
||||
if (pr.state === 'closed' && !gated) return log('closed by someone else — not ours');
|
||||
|
||||
// 1. Exempt authors: bots, anyone with triage or better, and drafts (which
|
||||
// are checked again on ready_for_review).
|
||||
if (pr.user.type === 'Bot') return log('author is a bot — exempt');
|
||||
if (await isTrusted(pr.user.login)) return pass('author has triage+ on this repo');
|
||||
if (pr.draft) return log('draft — skipped until ready for review');
|
||||
|
||||
// 2. Overrides: a triage+ user reopening the PR or removing the label wants
|
||||
// it open. Anyone else doing so just triggers a re-check.
|
||||
if ((action === 'reopened' || action === 'unlabeled') && sender && (await isTrusted(sender))) {
|
||||
return pass(`${sender} ${action === 'reopened' ? 'reopened it' : 'removed the label'} — override`, { sticky: true });
|
||||
}
|
||||
if (labels.includes(BYPASS_LABEL)) return pass(`carries ${BYPASS_LABEL}`);
|
||||
|
||||
// 3. The rule: the description links an open issue in this repo that is
|
||||
// labeled `help wanted` or assigned to the author. Only the first few
|
||||
// references are fetched; a just-assigned issue is checked first.
|
||||
const author = pr.user.login.toLowerCase();
|
||||
const refs = closingRefs(pr.body);
|
||||
if (hintIssue && refs.includes(hintIssue)) refs.unshift(...refs.splice(refs.indexOf(hintIssue), 1));
|
||||
const linked = [];
|
||||
for (const num of refs.slice(0, MAX_ISSUES)) {
|
||||
const issue = await getIssue(num);
|
||||
if (!issue) continue; // missing, a PR, closed, or transferred away
|
||||
linked.push(num);
|
||||
if (issue.labels.some((l) => l.name.toLowerCase() === OPEN_LABEL)) return pass(`#${num} is labeled "${OPEN_LABEL}"`);
|
||||
if (issue.assignees.some((a) => a.login.toLowerCase() === author)) return pass(`author is assigned to #${num}`);
|
||||
}
|
||||
return fail(linked);
|
||||
|
||||
// ── Outcomes ─────────────────────────────────────────────────────────
|
||||
|
||||
async function pass(reason, { sticky = false } = {}) {
|
||||
console.log(`PASS: ${reason}`);
|
||||
if (sticky) await addLabel(prNumber, BYPASS_LABEL);
|
||||
if (pr.state === 'closed' && !(await reopen(pr, reason))) return;
|
||||
if (gated) {
|
||||
await removeLabel(prNumber, LABEL);
|
||||
await deleteGateComment(prNumber);
|
||||
}
|
||||
}
|
||||
|
||||
async function fail(linkedIssues) {
|
||||
console.log(`FAIL: ${linkedIssues.length ? `not assigned to ${linkedIssues.map((n) => `#${n}`).join(', ')}` : 'no usable issue link'}`);
|
||||
await addLabel(prNumber, LABEL);
|
||||
await upsertGateComment(prNumber, closedComment(linkedIssues));
|
||||
if (pr.state === 'open') {
|
||||
await mutate(`close PR #${prNumber}`, () => github.rest.pulls.update({ owner, repo, pull_number: prNumber, state: 'closed' }));
|
||||
}
|
||||
}
|
||||
|
||||
function log(msg) {
|
||||
console.log(msg);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Comment text ─────────────────────────────────────────────────────────
|
||||
|
||||
function closedComment(linkedIssues) {
|
||||
const issues = linkedIssues.map((n) => `#${n}`).join(', ');
|
||||
const why = linkedIssues.length
|
||||
? `you aren't currently assigned to ${issues}`
|
||||
: "its description doesn't yet link an open issue in this repository (with `Fixes #123` or similar)";
|
||||
const next = linkedIssues.length
|
||||
? `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.)`
|
||||
: `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.`;
|
||||
return [
|
||||
MARKER,
|
||||
`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}.`,
|
||||
'',
|
||||
next,
|
||||
'',
|
||||
"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.",
|
||||
'',
|
||||
`*Maintainers: reopening this PR, removing the \`${LABEL}\` label, or adding \`${BYPASS_LABEL}\` bypasses the check.*`,
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
function cannotReopenComment(pr, reason) {
|
||||
return [
|
||||
MARKER,
|
||||
`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.`,
|
||||
'',
|
||||
`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 that links the same issue (if a maintainer had waved this one through, mention that so they can do the same there).`,
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
// ── Helpers ──────────────────────────────────────────────────────────────
|
||||
|
||||
async function mutate(description, fn) {
|
||||
if (!enforce) {
|
||||
console.log(`[dry-run] would ${description}`);
|
||||
return undefined;
|
||||
}
|
||||
return fn();
|
||||
}
|
||||
|
||||
// Triage-or-better on this repo, from the permission endpoint's capability
|
||||
// flags (role names can be custom; author_association hides private org
|
||||
// members). Only a nonexistent user 404s; any other error must throw rather
|
||||
// than be read as "untrusted", or a maintainer's PR could be closed.
|
||||
async function isTrusted(username) {
|
||||
try {
|
||||
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo, username });
|
||||
const p = data.user?.permissions;
|
||||
if (!p) throw new Error(`permission response for ${username} has no capability flags`);
|
||||
const trusted = Boolean(p.triage || p.push || p.maintain || p.admin);
|
||||
console.log(` ${username}: ${trusted ? 'trusted' : 'not trusted'} (role ${data.role_name || '-'})`);
|
||||
return trusted;
|
||||
} catch (e) {
|
||||
if (e.status === 404) return false;
|
||||
throw new Error(`Permission check failed for ${username} (HTTP ${e.status ?? '?'}): ${e.message}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Issue numbers referenced with a closing keyword, in the forms GitHub itself
|
||||
// honors: `Fixes #1`, `closes owner/repo#1`, `Resolved https://github.com/owner/repo/issues/1`.
|
||||
function closingRefs(body) {
|
||||
const repoRef = `${owner}/${repo}`.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
const re = new RegExp(
|
||||
`\\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\\s*:?\\s*(?:${repoRef}#|#|https?://github\\.com/${repoRef}/issues/)(\\d+)`,
|
||||
'gi',
|
||||
);
|
||||
return [...new Set([...(body || '').matchAll(re)].map((m) => parseInt(m[1], 10)))];
|
||||
}
|
||||
|
||||
// The linked issue, or null if it doesn't exist, is actually a PR, isn't
|
||||
// open, or has been transferred to another repository.
|
||||
async function getIssue(num) {
|
||||
let issue;
|
||||
try {
|
||||
({ data: issue } = await github.rest.issues.get({ owner, repo, issue_number: num }));
|
||||
} catch (e) {
|
||||
if (e.status === 404 || e.status === 410) return null;
|
||||
throw new Error(`Cannot fetch issue #${num} (HTTP ${e.status ?? '?'}): ${e.message}`);
|
||||
}
|
||||
if (issue.pull_request || issue.state !== 'open') return null;
|
||||
if (!issue.repository_url?.endsWith(`/${owner}/${repo}`)) return null;
|
||||
return issue;
|
||||
}
|
||||
|
||||
// Reopen a gate-closed PR. GitHub refuses (422) if the branch was rewritten
|
||||
// or deleted while closed, or another open PR uses it. Explain that in the
|
||||
// comment and make sure the control label is (still) on, so the PR stays
|
||||
// gate-managed and a later edit or override retries the reopen.
|
||||
async function reopen(pr, reason) {
|
||||
try {
|
||||
await mutate(`reopen PR #${pr.number}`, () => github.rest.pulls.update({ owner, repo, pull_number: pr.number, state: 'open' }));
|
||||
return true;
|
||||
} catch (e) {
|
||||
if (e.status !== 422) throw e;
|
||||
core.warning(`GitHub refused to reopen PR #${pr.number}: ${e.message}`);
|
||||
await addLabel(pr.number, LABEL);
|
||||
await upsertGateComment(pr.number, cannotReopenComment(pr, reason));
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function addLabel(prNumber, name) {
|
||||
await mutate(`add "${name}" to PR #${prNumber}`, async () => {
|
||||
await ensureLabelExists(name);
|
||||
await github.rest.issues.addLabels({ owner, repo, issue_number: prNumber, labels: [name] });
|
||||
});
|
||||
}
|
||||
|
||||
async function removeLabel(prNumber, name) {
|
||||
await mutate(`remove "${name}" from PR #${prNumber}`, async () => {
|
||||
try {
|
||||
await github.rest.issues.removeLabel({ owner, repo, issue_number: prNumber, name });
|
||||
} catch (e) {
|
||||
if (e.status !== 404) throw e;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
async function ensureLabelExists(name) {
|
||||
const meta = {
|
||||
[LABEL]: ['b76e79', 'Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)'],
|
||||
[BYPASS_LABEL]: ['0e8a16', 'Maintainer override for the linked-issue intake gate'],
|
||||
}[name];
|
||||
try {
|
||||
await github.rest.issues.getLabel({ owner, repo, name });
|
||||
} catch (e) {
|
||||
if (e.status !== 404) throw e;
|
||||
try {
|
||||
await github.rest.issues.createLabel({ owner, repo, name, color: meta[0], description: meta[1] });
|
||||
} catch (createErr) {
|
||||
if (createErr.status !== 422) throw createErr; // created concurrently
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The gate keeps at most one comment per PR: authored by the Actions bot and
|
||||
// carrying MARKER. It's created or updated on failure and deleted on pass.
|
||||
async function findGateComment(prNumber) {
|
||||
const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: prNumber, per_page: 100 });
|
||||
return comments.find((c) => c.user?.login === BOT_LOGIN && c.body?.includes(MARKER));
|
||||
}
|
||||
|
||||
async function upsertGateComment(prNumber, body) {
|
||||
const existing = await findGateComment(prNumber);
|
||||
if (!existing) {
|
||||
await mutate(`comment on PR #${prNumber}`, () => github.rest.issues.createComment({ owner, repo, issue_number: prNumber, body }));
|
||||
} else if (existing.body !== body) {
|
||||
await mutate(`update the gate comment on PR #${prNumber}`, () => github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }));
|
||||
}
|
||||
}
|
||||
|
||||
async function deleteGateComment(prNumber) {
|
||||
const existing = await findGateComment(prNumber);
|
||||
if (!existing) return;
|
||||
await mutate(`delete the gate comment on PR #${prNumber}`, async () => {
|
||||
try {
|
||||
await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
|
||||
} catch (e) {
|
||||
if (e.status !== 404) throw e; // already deleted by a concurrent run
|
||||
}
|
||||
});
|
||||
}
|
||||
};
|
||||
392
.github/scripts/pr_intake_gate.test.js
vendored
Normal file
392
.github/scripts/pr_intake_gate.test.js
vendored
Normal file
|
|
@ -0,0 +1,392 @@
|
|||
// Scenario tests for pr_intake_gate.js. Each case sets up a tiny in-memory
|
||||
// repository (pull requests, issues, people), fires one event at the gate, and
|
||||
// checks what the gate left behind: PR state, labels, and its comment.
|
||||
//
|
||||
// node --test .github/scripts/pr_intake_gate.test.js
|
||||
//
|
||||
// No dependencies; the GitHub client is a small fake defined at the bottom.
|
||||
// CI runs it in the checks job (.github/workflows/shared.yml).
|
||||
'use strict';
|
||||
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const gate = require('./pr_intake_gate.js');
|
||||
|
||||
const LABEL = 'missing-issue-link';
|
||||
const BYPASS = 'bypass-issue-check';
|
||||
const REPO = { owner: 'modelcontextprotocol', repo: 'python-sdk' };
|
||||
|
||||
// People. Only the capability flags matter to the gate.
|
||||
const PEOPLE = {
|
||||
maintainer: { type: 'User', perms: { admin: true, maintain: true, push: true, triage: true, pull: true } },
|
||||
triager: { type: 'User', perms: { triage: true, pull: true } }, // e.g. a trusted-contributors team
|
||||
outsider: { type: 'User', perms: { pull: true } },
|
||||
'dependabot[bot]': { type: 'Bot', perms: {} },
|
||||
'somebot[bot]': { type: 'Bot', perms: {} },
|
||||
};
|
||||
|
||||
// ── Scenarios ──────────────────────────────────────────────────────────────
|
||||
// `prs` / `issues` describe the world before the event; `expect` describes each
|
||||
// PR afterwards: state, labels, and comment ('closed' = the "this PR has been
|
||||
// closed" comment, 'cannot-reopen' = the refused-reopen comment, null = none).
|
||||
// `writes: 0` additionally asserts the gate touched nothing at all.
|
||||
|
||||
const scenarios = [
|
||||
{
|
||||
name: 'outsider opens a PR with no issue link → labeled, commented, closed',
|
||||
prs: [pr(3300, 'outsider')],
|
||||
event: opened(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed' } },
|
||||
},
|
||||
{
|
||||
name: 'outsider links an open issue reported by someone else, not assigned → closed',
|
||||
prs: [pr(3300, 'outsider', { body: 'Fixes #10' })],
|
||||
issues: [issue(10)],
|
||||
event: opened(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed' } },
|
||||
},
|
||||
{
|
||||
name: 'outsider links an issue they are assigned to → stays open, untouched',
|
||||
prs: [pr(3300, 'outsider', { body: 'closes #10' })],
|
||||
issues: [issue(10, { assignees: ['outsider'] })],
|
||||
event: opened(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'outsider links a `help wanted` issue → stays open without assignment',
|
||||
prs: [pr(3300, 'outsider', { body: 'Resolves modelcontextprotocol/python-sdk#10' })],
|
||||
issues: [issue(10, { labels: ['help wanted'] })],
|
||||
event: opened(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'links only count when they use a closing keyword and point at an open issue here',
|
||||
prs: [pr(3300, 'outsider', { body: 'Related to #10, fixes owner/other#10, prefixes #10, Fixes #11, Fixes #12' })],
|
||||
issues: [
|
||||
issue(10, { assignees: ['outsider'] }), // referenced, but never with a closing keyword for this repo
|
||||
issue(11, { assignees: ['outsider'], state: 'closed' }), // closed issues don't count
|
||||
issue(12, { assignees: ['outsider'], repository_url: 'https://api.github.com/repos/modelcontextprotocol/typescript-sdk' }), // transferred away
|
||||
],
|
||||
event: opened(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed' } },
|
||||
},
|
||||
{
|
||||
name: 'gate-closed PR: author edits the description to link a qualifying issue → reopened, label and comment removed',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [LABEL], body: 'Fixes #10', gateComment: true })],
|
||||
issues: [issue(10, { labels: ['help wanted'] })],
|
||||
event: edited(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
},
|
||||
{
|
||||
name: 'gate-closed PR: maintainer assigns the author on the linked issue → reopened',
|
||||
prs: [
|
||||
pr(3300, 'outsider', { state: 'closed', labels: [LABEL], body: 'Fixes #10', gateComment: true }),
|
||||
pr(3301, 'outsider', { state: 'closed', labels: [LABEL], body: 'Fixes #99', gateComment: true }), // different issue: untouched
|
||||
],
|
||||
issues: [issue(10, { assignees: ['outsider'] }), issue(99)],
|
||||
event: assigned(10, 'outsider', 'maintainer'),
|
||||
expect: {
|
||||
3300: { state: 'open', labels: [], comment: null },
|
||||
3301: { state: 'closed', labels: [LABEL], comment: 'closed' },
|
||||
},
|
||||
},
|
||||
{
|
||||
name: 'gate-closed PR: maintainer reopens it → stays open with the sticky bypass label',
|
||||
prs: [pr(3300, 'outsider', { state: 'open', labels: [LABEL], gateComment: true })], // payload arrives post-reopen
|
||||
event: reopened(3300, 'maintainer'),
|
||||
expect: { 3300: { state: 'open', labels: [BYPASS], comment: null } },
|
||||
},
|
||||
{
|
||||
name: 'gate-closed PR: triage-role user removes the label → reopened with the sticky bypass label',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [], gateComment: true })], // payload arrives post-unlabel
|
||||
event: unlabeled(3300, 'triager'),
|
||||
expect: { 3300: { state: 'open', labels: [BYPASS], comment: null } },
|
||||
},
|
||||
{
|
||||
name: 'gate-closed PR: some other bot strips the label → re-checked, label restored, still closed',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [], gateComment: true })],
|
||||
event: unlabeled(3300, 'somebot[bot]'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed' } },
|
||||
},
|
||||
{
|
||||
name: 'PR carrying the bypass label is never re-closed, even after edits that would fail',
|
||||
prs: [pr(3300, 'outsider', { labels: [BYPASS], body: 'no link at all' })],
|
||||
event: edited(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'open', labels: [BYPASS], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'previously passing PR edited to drop its link → closed again',
|
||||
prs: [pr(3300, 'outsider', { body: 'see #10' })],
|
||||
issues: [issue(10, { assignees: ['outsider'] })],
|
||||
event: edited(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed' } },
|
||||
},
|
||||
{
|
||||
name: "maintainer's own PR is exempt (and reopening it adds no labels)",
|
||||
prs: [pr(3300, 'maintainer')],
|
||||
event: reopened(3300, 'maintainer'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'triage-role author is exempt',
|
||||
prs: [pr(3300, 'triager')],
|
||||
event: opened(3300, 'triager'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'bot-authored PR (Dependabot) is exempt',
|
||||
prs: [pr(3300, 'dependabot[bot]')],
|
||||
event: opened(3300, 'dependabot[bot]'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'draft PR is skipped until it is marked ready for review',
|
||||
prs: [pr(3300, 'outsider', { draft: true })],
|
||||
event: opened(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'draft marked ready for review with no link → closed',
|
||||
prs: [pr(3300, 'outsider')],
|
||||
event: readyForReview(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed' } },
|
||||
},
|
||||
{
|
||||
name: 'PR a maintainer closed by hand (no gate label) is left alone when edited',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed' })],
|
||||
event: edited(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'merged PR still carrying the label is left alone',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', merged_at: '2026-08-15T00:00:00Z', labels: [LABEL] })],
|
||||
event: edited(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: null } },
|
||||
writes: 0,
|
||||
},
|
||||
{
|
||||
name: 'GitHub refuses to reopen (branch rewritten while closed) → stays closed, comment explains',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [LABEL], body: 'Fixes #10', gateComment: true, refuseReopen: true })],
|
||||
issues: [issue(10, { assignees: ['outsider'] })],
|
||||
event: edited(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'cannot-reopen' } },
|
||||
},
|
||||
{
|
||||
name: 'gate-closed PR: maintainer adds the bypass label → reopened, and the label sticks',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [LABEL, BYPASS], gateComment: true })], // payload arrives post-label
|
||||
event: labeled(3300, 'maintainer', BYPASS),
|
||||
expect: { 3300: { state: 'open', labels: [BYPASS], comment: null } },
|
||||
},
|
||||
{
|
||||
name: 'refused reopen after a label-removal override → both labels on, so the PR stays gate-managed',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [], gateComment: true, refuseReopen: true })],
|
||||
event: unlabeled(3300, 'triager'),
|
||||
expect: { 3300: { state: 'closed', labels: [BYPASS, LABEL], comment: 'cannot-reopen' } },
|
||||
},
|
||||
{
|
||||
name: 'after a refused reopen, once the branch is restored an edit retries and reopens',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [BYPASS, LABEL], comments: [{ user: 'github-actions[bot]', body: "<!-- require-linked-issue -->\n…GitHub won't let it be reopened…" }] })],
|
||||
event: edited(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'open', labels: [BYPASS], comment: null } },
|
||||
},
|
||||
{
|
||||
name: 'assignment reopens the PR even when the assigned issue is referenced after several others',
|
||||
prs: [pr(3300, 'outsider', { state: 'closed', labels: [LABEL], body: 'Fixes #1 fixes #2 fixes #3 fixes #4 fixes #5 fixes #6', gateComment: true })],
|
||||
issues: [issue(1), issue(2), issue(3), issue(4), issue(5), issue(6, { assignees: ['outsider'] })],
|
||||
event: assigned(6, 'outsider', 'maintainer'),
|
||||
expect: { 3300: { state: 'open', labels: [], comment: null } },
|
||||
},
|
||||
{
|
||||
name: 'a comment planted by the author with the marker is ignored; the gate posts its own',
|
||||
prs: [pr(3300, 'outsider', { comments: [{ user: 'outsider', body: '<!-- require-linked-issue -->\nnice try' }] })],
|
||||
event: opened(3300, 'outsider'),
|
||||
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed', foreignComments: 1 } },
|
||||
},
|
||||
{
|
||||
name: 'manual dispatch evaluates an old, never-gated PR like a new one',
|
||||
prs: [pr(3150, 'outsider')],
|
||||
event: dispatch(3150, 'maintainer'),
|
||||
expect: { 3150: { state: 'closed', labels: [LABEL], comment: 'closed' } },
|
||||
},
|
||||
];
|
||||
|
||||
for (const s of scenarios) {
|
||||
test(s.name, async () => {
|
||||
const world = makeWorld(s);
|
||||
await run(world, s.event, { enforce: true });
|
||||
assert.deepEqual(observe(world, s.expect), s.expect);
|
||||
if (s.writes !== undefined) assert.equal(world.writes.length, s.writes, `writes: ${world.writes.join(', ')}`);
|
||||
});
|
||||
}
|
||||
|
||||
test('kill switch (ENFORCE=false) reaches the same verdicts but writes nothing', async () => {
|
||||
for (const s of scenarios) {
|
||||
const world = makeWorld(s);
|
||||
await run(world, s.event, { enforce: false });
|
||||
assert.equal(world.writes.length, 0, `${s.name}: ${world.writes.join(', ')}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('on assignment, one PR failing to re-evaluate does not stop the others (run still fails)', async () => {
|
||||
const world = makeWorld({
|
||||
prs: [
|
||||
pr(3300, 'outsider', { state: 'closed', labels: [LABEL], body: 'Fixes #10', gateComment: true }),
|
||||
pr(3301, 'outsider', { state: 'closed', labels: [LABEL], body: 'Fixes #10', gateComment: true }),
|
||||
],
|
||||
issues: [issue(10, { assignees: ['outsider'] })],
|
||||
});
|
||||
world.failPullsGet = 3300;
|
||||
await assert.rejects(run(world, assigned(10, 'outsider', 'maintainer'), { enforce: true }), /Could not re-evaluate #3300/);
|
||||
assert.equal(world.prs.get(3301).state, 'open');
|
||||
});
|
||||
|
||||
test('an API error while checking permissions fails the run rather than closing the PR', async () => {
|
||||
const world = makeWorld({ prs: [pr(3300, 'outsider')] });
|
||||
world.failPermissionLookup = true;
|
||||
await assert.rejects(run(world, opened(3300, 'outsider'), { enforce: true }), /Permission check failed/);
|
||||
assert.equal(world.writes.length, 0);
|
||||
});
|
||||
|
||||
// ── Scenario helpers ───────────────────────────────────────────────────────
|
||||
|
||||
function pr(number, author, o = {}) {
|
||||
return { number, author, state: 'open', draft: false, body: '', labels: [], merged_at: null, comments: [], gateComment: false, refuseReopen: false, ...o };
|
||||
}
|
||||
function issue(number, o = {}) {
|
||||
return { number, state: 'open', labels: [], assignees: [], repository_url: `https://api.github.com/repos/${REPO.owner}/${REPO.repo}`, ...o };
|
||||
}
|
||||
function prEvent(action, number, sender, extra = {}) {
|
||||
return { eventName: 'pull_request_target', payload: { action, pull_request: { number }, sender: { login: sender }, ...extra } };
|
||||
}
|
||||
function opened(number, sender) { return prEvent('opened', number, sender); }
|
||||
function edited(number, sender) { return prEvent('edited', number, sender); }
|
||||
function reopened(number, sender) { return prEvent('reopened', number, sender); }
|
||||
function readyForReview(number, sender) { return prEvent('ready_for_review', number, sender); }
|
||||
function unlabeled(number, sender) { return prEvent('unlabeled', number, sender, { label: { name: LABEL } }); }
|
||||
function labeled(number, sender, name) { return prEvent('labeled', number, sender, { label: { name } }); }
|
||||
function assigned(issueNumber, assignee, sender) {
|
||||
return { eventName: 'issues', payload: { action: 'assigned', issue: { number: issueNumber }, assignee: { login: assignee }, sender: { login: sender } } };
|
||||
}
|
||||
function dispatch(number, sender) {
|
||||
return { eventName: 'workflow_dispatch', prNumber: number, payload: { inputs: { pr_number: String(number) }, sender: { login: sender } } };
|
||||
}
|
||||
|
||||
async function run(world, event, { enforce }) {
|
||||
process.env.ENFORCE = enforce ? 'true' : 'false';
|
||||
process.env.PR_NUMBER_INPUT = event.prNumber ? String(event.prNumber) : '';
|
||||
const core = { warning: () => {}, setFailed: (m) => { throw new Error(`setFailed: ${m}`); } };
|
||||
const quiet = console.log;
|
||||
console.log = () => {};
|
||||
try {
|
||||
await gate({ github: world.github, context: { repo: REPO, ...event }, core });
|
||||
} finally {
|
||||
console.log = quiet;
|
||||
}
|
||||
}
|
||||
|
||||
// What the gate left behind, in the same shape as a scenario's `expect`.
|
||||
function observe(world, expect) {
|
||||
const out = {};
|
||||
for (const num of Object.keys(expect)) {
|
||||
const p = world.prs.get(Number(num));
|
||||
const gateComments = p.comments.filter((c) => c.user === 'github-actions[bot]' && c.body.includes('<!-- require-linked-issue -->'));
|
||||
assert.ok(gateComments.length <= 1, `PR #${num} has ${gateComments.length} gate comments`);
|
||||
const kind = !gateComments.length ? null : gateComments[0].body.includes("won't let it be reopened") ? 'cannot-reopen' : 'closed';
|
||||
out[num] = { state: p.state, labels: [...p.labels].sort(), comment: kind };
|
||||
if ('foreignComments' in expect[num]) out[num].foreignComments = p.comments.length - gateComments.length;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
// ── A tiny in-memory GitHub ────────────────────────────────────────────────
|
||||
|
||||
function makeWorld({ prs = [], issues = [] }) {
|
||||
const world = { prs: new Map(), issues: new Map(), writes: [], failPermissionLookup: false, failPullsGet: null, nextCommentId: 100 };
|
||||
for (const p of prs) {
|
||||
const copy = structuredClone(p);
|
||||
copy.comments = copy.comments.map((c) => ({ id: world.nextCommentId++, ...c }));
|
||||
if (copy.gateComment) copy.comments.push({ id: world.nextCommentId++, user: 'github-actions[bot]', body: '<!-- require-linked-issue -->\nThanks for the contribution. …closed for now…' });
|
||||
world.prs.set(p.number, copy);
|
||||
}
|
||||
for (const i of issues) world.issues.set(i.number, structuredClone(i));
|
||||
|
||||
const err = (status, message = 'fake error') => Object.assign(new Error(message), { status });
|
||||
const write = (what) => world.writes.push(what);
|
||||
const getPr = (n) => { const p = world.prs.get(n); if (!p) throw err(404); return p; };
|
||||
const apiPr = (p) => ({
|
||||
number: p.number, state: p.state, draft: p.draft, body: p.body, merged_at: p.merged_at,
|
||||
labels: p.labels.map((name) => ({ name })),
|
||||
user: { login: p.author, type: PEOPLE[p.author].type },
|
||||
head: { sha: 'abc1234def', ref: 'patch-1', repo: { name: REPO.repo, owner: { login: p.author } } },
|
||||
});
|
||||
|
||||
const rest = {
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async ({ username }) => {
|
||||
if (world.failPermissionLookup) throw err(500, 'boom');
|
||||
const person = PEOPLE[username];
|
||||
if (!person) throw err(404, 'not a user');
|
||||
return { data: { role_name: 'custom', user: { permissions: { admin: false, maintain: false, push: false, triage: false, pull: false, ...person.perms } } } };
|
||||
},
|
||||
},
|
||||
pulls: {
|
||||
get: async ({ pull_number }) => { if (world.failPullsGet === pull_number) throw err(502, 'bad gateway'); return { data: apiPr(getPr(pull_number)) }; },
|
||||
update: async ({ pull_number, state }) => {
|
||||
const p = getPr(pull_number);
|
||||
write(`${state === 'open' ? 'reopen' : 'close'} #${pull_number}`);
|
||||
if (state === 'open' && p.refuseReopen) throw err(422, 'Validation Failed: state cannot be changed');
|
||||
p.state = state;
|
||||
return { data: apiPr(p) };
|
||||
},
|
||||
},
|
||||
issues: {
|
||||
get: async ({ issue_number }) => {
|
||||
if (world.prs.has(issue_number)) return { data: { ...apiPr(getPr(issue_number)), pull_request: {} } };
|
||||
const i = world.issues.get(issue_number);
|
||||
if (!i) throw err(404);
|
||||
return { data: { ...i, labels: i.labels.map((name) => ({ name })), assignees: i.assignees.map((login) => ({ login })) } };
|
||||
},
|
||||
listForRepo: async ({ state, creator, labels }) => ({
|
||||
data: [...world.prs.values()]
|
||||
.filter((p) => p.state === state && p.author === creator && p.labels.includes(labels))
|
||||
.map((p) => ({ number: p.number, body: p.body, pull_request: {} })),
|
||||
}),
|
||||
getLabel: async () => ({ data: {} }),
|
||||
createLabel: async ({ name }) => { write(`create label ${name}`); },
|
||||
addLabels: async ({ issue_number, labels }) => {
|
||||
const p = getPr(issue_number);
|
||||
write(`label #${issue_number} ${labels}`);
|
||||
for (const l of labels) if (!p.labels.includes(l)) p.labels.push(l);
|
||||
},
|
||||
removeLabel: async ({ issue_number, name }) => {
|
||||
const p = getPr(issue_number);
|
||||
write(`unlabel #${issue_number} ${name}`);
|
||||
if (!p.labels.includes(name)) throw err(404);
|
||||
p.labels = p.labels.filter((l) => l !== name);
|
||||
},
|
||||
listComments: async ({ issue_number }) => ({ data: getPr(issue_number).comments.map((c) => ({ id: c.id, body: c.body, user: { login: c.user } })) }),
|
||||
createComment: async ({ issue_number, body }) => {
|
||||
write(`comment on #${issue_number}`);
|
||||
getPr(issue_number).comments.push({ id: world.nextCommentId++, user: 'github-actions[bot]', body });
|
||||
},
|
||||
updateComment: async ({ comment_id, body }) => {
|
||||
write(`edit comment ${comment_id}`);
|
||||
for (const p of world.prs.values()) for (const c of p.comments) if (c.id === comment_id) c.body = body;
|
||||
},
|
||||
deleteComment: async ({ comment_id }) => {
|
||||
write(`delete comment ${comment_id}`);
|
||||
for (const p of world.prs.values()) p.comments = p.comments.filter((c) => c.id !== comment_id);
|
||||
},
|
||||
},
|
||||
};
|
||||
world.github = { rest, paginate: async (fn, args) => (await fn(args)).data };
|
||||
return world;
|
||||
}
|
||||
85
.github/workflows/require-linked-issue.yml
vendored
Normal file
85
.github/workflows/require-linked-issue.yml
vendored
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
# PR intake gate — see CONTRIBUTING.md ("How pull requests get in") for the
|
||||
# policy and .github/scripts/pr_intake_gate.js for the rules as applied.
|
||||
#
|
||||
# In short: a PR from someone without triage rights stays open only if it links
|
||||
# an open issue here that is assigned to them (or labeled `help wanted`);
|
||||
# otherwise it is labeled `missing-issue-link`, gets one comment, and is closed,
|
||||
# and it reopens automatically once the author is assigned. Bots and drafts are
|
||||
# skipped. A triage+ user reopening the PR, removing the label, or adding
|
||||
# `bypass-issue-check` overrides.
|
||||
#
|
||||
# Operating it:
|
||||
# - Live by default. To pause it without a revert, set the repository
|
||||
# variable PR_GATE_ENFORCE to "false": runs then only log their verdicts.
|
||||
# - PRs below the number in the job `if:` predate the gate and are ignored
|
||||
# unless evaluated by hand: `gh workflow run require-linked-issue.yml -f pr_number=N`.
|
||||
#
|
||||
# Security: pull_request_target runs with a write token in the base repo's
|
||||
# context, and GitHub takes both this file and the checkout from the default
|
||||
# branch whatever the PR targets (so PRs against v1.x are covered too). The
|
||||
# job checks out only that, for the script, and never fetches, builds, or runs
|
||||
# anything from the pull request.
|
||||
#
|
||||
# Adapted from PrefectHQ/fastmcp's require-issue-link.yml (Apache-2.0), itself
|
||||
# from langchain-ai/langchain (MIT).
|
||||
|
||||
name: Require Linked Issue
|
||||
|
||||
on:
|
||||
pull_request_target: # zizmor: ignore[dangerous-triggers] checks out the default branch only and never runs PR code — see header
|
||||
types: [opened, edited, reopened, ready_for_review, labeled, unlabeled]
|
||||
issues:
|
||||
types: [assigned]
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
pr_number:
|
||||
description: PR number to evaluate
|
||||
required: true
|
||||
type: number
|
||||
|
||||
permissions: {}
|
||||
|
||||
jobs:
|
||||
gate:
|
||||
name: Evaluate
|
||||
# Routing only; the rules are in the script. PR events run at or above the
|
||||
# grandfathering floor, or for PRs the gate has already labeled; label
|
||||
# events only for the two labels the gate cares about.
|
||||
if: >-
|
||||
github.event_name == 'workflow_dispatch' ||
|
||||
(github.event_name == 'issues' && !github.event.issue.pull_request && github.event.issue.state == 'open') ||
|
||||
(
|
||||
github.event_name == 'pull_request_target' &&
|
||||
(
|
||||
github.event.pull_request.number >= 3200 ||
|
||||
contains(github.event.pull_request.labels.*.name, 'missing-issue-link') ||
|
||||
github.event.action == 'unlabeled'
|
||||
) &&
|
||||
(github.event.action != 'unlabeled' || github.event.label.name == 'missing-issue-link') &&
|
||||
(github.event.action != 'labeled' || github.event.label.name == 'bypass-issue-check')
|
||||
)
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
concurrency:
|
||||
group: require-linked-issue-${{ github.event.pull_request.number || inputs.pr_number || format('issue-{0}-{1}', github.event.issue.number, github.event.assignee.login) }}
|
||||
cancel-in-progress: false
|
||||
permissions:
|
||||
contents: read # check out the gate script from the default branch
|
||||
issues: write # read linked issues; label and comment on the PR
|
||||
pull-requests: write # close and reopen the PR
|
||||
env:
|
||||
ENFORCE: ${{ vars.PR_GATE_ENFORCE != 'false' && 'true' || 'false' }} # kill switch: set the variable to "false" to go log-only
|
||||
PR_NUMBER_INPUT: ${{ inputs.pr_number }}
|
||||
steps:
|
||||
- name: Check out the gate script (default branch)
|
||||
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
|
||||
with:
|
||||
persist-credentials: false
|
||||
sparse-checkout: .github/scripts
|
||||
|
||||
- name: Evaluate
|
||||
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
|
||||
with:
|
||||
script: |
|
||||
const run = require('./.github/scripts/pr_intake_gate.js');
|
||||
await run({ github, context, core });
|
||||
6
.github/workflows/shared.yml
vendored
6
.github/workflows/shared.yml
vendored
|
|
@ -41,6 +41,12 @@ jobs:
|
|||
with:
|
||||
extra_args: --all-files --verbose
|
||||
|
||||
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
|
||||
with:
|
||||
node-version: 24 # match the runtime actions/github-script@v8 uses
|
||||
- name: PR intake gate scenarios
|
||||
run: node --test .github/scripts/pr_intake_gate.test.js
|
||||
|
||||
- name: Surface types match vendored schema
|
||||
run: |
|
||||
uv sync --group codegen --frozen
|
||||
|
|
|
|||
Loading…
Reference in a new issue