From e4fba65e72adb840ed1de093f925b855393c4fc6 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Sat, 31 Oct 2026 17:44:33 +0100 Subject: [PATCH 1/2] Gate PRs and only accept when issue assigned Signed-off-by: Daniel Garnier-Moiroux --- .github/pull_request_template.md | 10 + .github/scripts/pr_intake_gate.js | 310 +++++++++++++++++++++ .github/workflows/require-linked-issue.yml | 84 ++++++ CONTRIBUTING.md | 74 ++++- 4 files changed, 466 insertions(+), 12 deletions(-) create mode 100644 .github/pull_request_template.md create mode 100644 .github/scripts/pr_intake_gate.js create mode 100644 .github/workflows/require-linked-issue.yml diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..f820eaa6a --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,10 @@ + + +- Link the issue with `Fixes #N`, or `Updates #N` for a partial fix. You must be + assigned to that issue (or it must be labeled `help wanted`). +- Describe the change at a high level: why it is needed, and anything that is + not obvious from the diff. diff --git a/.github/scripts/pr_intake_gate.js b/.github/scripts/pr_intake_gate.js new file mode 100644 index 000000000..8a4b04a7f --- /dev/null +++ b/.github/scripts/pr_intake_gate.js @@ -0,0 +1,310 @@ +// 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/Updates #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 = ''; +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 && linkedRefs(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 and anyone with triage or better. Drafts are + // gated like any other PR. + 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'); + + // 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 = linkedRefs(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(pr.draft, 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(draft, linkedIssues) { + const issues = linkedIssues.map((n) => `#${n}`).join(', '); + const opener = draft + ? "This PR has been closed automatically. It's still a draft, but we close those early so you don't put in more time only to have it closed the moment you mark it ready.\n\n" + : 'This PR has been closed automatically. '; + const rule = `${opener}This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue`; + const situation = linkedIssues.length + ? [ + `${rule}, and you aren't currently assigned to ${issues}.`, + ] + : [ + `${rule}, and this PR doesn't link an open issue yet.`, + '', + "- **If you're already assigned to an issue for this**, add `Fixes #` (or `Updates #` for a partial fix) to the description and the PR will reopen on its own.", + `- **If there's no issue yet**, please [open one](https://github.com/${owner}/${repo}/issues/new/choose) instead: what you ran into, where you hit it, and a minimal reproduction. That context is what we use to decide what to prioritize.`, + "- **If there's an issue but you're not assigned**, add `Fixes #` anyway so they're linked, then engage on the issue itself by confirming the repro or describing the approach you'd take. Assignment is a maintainer call based on capacity. If you are assigned, this PR reopens automatically.", + ]; + return [ + MARKER, + ...situation, + '', + "You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned. There's no need to open a new PR either way.", + '', + `See [CONTRIBUTING.md](${contributingUrl}) for the full policy.`, + '', + `*Maintainers: reopen, remove \`${LABEL}\`, or add \`${BYPASS_LABEL}\` to override.*`, + ].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 + // honors (`Fixes #1`, `closes owner/repo#1`, `Resolved + // https://github.com/owner/repo/issues/1`), plus `Updates #1` for partial + // fixes. + function linkedRefs(body) { + const repoRef = `${owner}/${repo}`.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const re = new RegExp( + `\\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|updates?)\\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 + } + }); + } +}; diff --git a/.github/workflows/require-linked-issue.yml b/.github/workflows/require-linked-issue.yml new file mode 100644 index 000000000..93622cb41 --- /dev/null +++ b/.github/workflows/require-linked-issue.yml @@ -0,0 +1,84 @@ +# 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. Drafts are gated +# too; bots 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. The job checks out only that, for the script, +# and never fetches, builds, or runs anything from the pull request. +# +# Adapted from modelcontextprotocol/go-sdk's require-linked-issue.yml, itself +# adapted from modelcontextprotocol/python-sdk's. + +name: Require Linked Issue + +on: + pull_request_target: # 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 >= 1289 || + 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@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + sparse-checkout: .github/scripts + + - name: Evaluate + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const run = require('./.github/scripts/pr_intake_gate.js'); + await run({ github, context, core }); diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ed2e42626..736e5d92e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,6 +3,57 @@ Thank you for your interest in contributing to the Model Context Protocol Java SDK! This document outlines how to contribute to this project. +## Before you start + +> [!IMPORTANT] +> **The most useful contribution is a good issue.** Pull requests from outside +> the maintainer team are closed automatically unless they link an issue +> assigned to their author. + +### Why issues rather than pull requests + +Coding agents can now turn any open issue into a plausible-looking pull request within +hours, and generate speculative bug reports from a scan of the code, so we +receive far more issues and PRs than we can read carefully. Reviewing a PR +properly still takes as long as it always did. When an issue is well described, +it is usually quicker for a maintainer to write a fix that fits the codebase +than to review and reshape someone else's. + +What we can't produce ourselves is your context: what you were doing, what you +expected, a minimal reproduction, and the system where it broke. +That is what a good issue carries, so that is what we ask for first. + +### How pull requests get in + +A PR from someone outside the maintainer team stays open when both of these +hold: + +1. Its description links an open issue in this repository with `Fixes #123` + (or `Closes`, `Resolves`), or with `Updates #123` for a partial fix. +2. A maintainer has assigned that issue to you, or the issue carries the [`help + wanted`](https://github.com/modelcontextprotocol/java-sdk/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22help%20wanted%22) + label, which means we'd welcome a PR for it from anyone. + +Otherwise a bot labels the PR `missing-issue-link`, leaves a comment explaining +this, and closes it. Drafts are treated the same way. If that happens to yours, +there's no need to open a new one: it reopens automatically as soon as a +maintainer assigns you the issue, or when you edit the description to link one +that qualifies. While it's closed, push updates as new commits rather than +force-pushing, since GitHub can't reopen a PR whose branch has been rewritten. +This applies to small fixes like typos too; for those, an issue pointing at the +problem is all we need. + +Whether to assign an issue, and to whom, is a maintainer call, and it depends +on our capacity at the time as much as on the change itself. Comments that only +ask to be assigned don't factor into it, so please skip those (and don't have +an agent post them). What does help is engaging with the issue itself: +confirming the reproduction, asking about the intended behavior, or briefly +describing the approach you'd take. If you reported the issue and would like to +fix it yourself, say so in the issue. + +Maintainers can override the check by reopening the PR, by removing its +`missing-issue-link` label, or by adding `bypass-issue-check`. + ## Prerequisites The following software is required to work on the codebase: @@ -76,18 +127,17 @@ Please don't submit pull requests: When submitting changes: -1. For non-trivial changes, please clarify with the maintainers in an issue whether - you can contribute the change and the desired scope of the change. -2. For trivial changes (for example a couple of lines or documentation changes) there - is no need to open an issue first. -3. Maintainers will triage issues and act on them. There is no such guarantee on pull requests. - PRs opened without being discussed in an issue MAY be ignored, and maintainers reserve the right - to close these PRs without justification. -4. Push your changes to your fork. -5. Submit a pull request to the main repository. -6. Follow the pull request template. -7. Wait for review. -8. For any follow-up work, please add new commits instead of force-pushing. This will +1. Every PR from outside the maintainer team must link an issue that a maintainer + has assigned to you, or one labeled `help wanted`, as described in [How pull + requests get in](#how-pull-requests-get-in). For changes that don't relate to an + existing issue, file an issue first, clarify the desired scope of the change with + the maintainers, and wait for it to be assigned before opening a PR. +2. Maintainers will triage issues and act on them. There is no such guarantee on pull requests. +3. Push your changes to your fork. +4. Submit a pull request to the main repository. +5. Follow the pull request template. +6. Wait for review. +7. For any follow-up work, please add new commits instead of force-pushing. This will allow the reviewer to focus on incremental changes instead of having to restart the review process. From 2e090e902006680a902a4893c4260107d1f881cc Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Mon, 2 Nov 2026 14:55:21 +0100 Subject: [PATCH 2/2] Polish wording in CONTRIBUTING.md --- CONTRIBUTING.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 736e5d92e..ff9b60261 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -120,10 +120,9 @@ git checkout -b feature/your-feature-name ## Submitting Changes -Please don't submit pull requests: +**Before submitting a PR, please first open an issue and wait until it is assigned to you.** -- With GitHub accounts managed by autonomous AI bots -- For already assigned issues (as the assignee is or plans to work on it) +Please don't submit issues or pull requests with GitHub accounts managed by autonomous AI bots. When submitting changes: