diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..d32f0079f --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,38 @@ + + +Fixes # + + + +## Motivation and Context + + +## How Has This Been Tested? + + +## Breaking Changes + + +## Types of changes + +- [ ] 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 + +- [ ] 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 + diff --git a/.github/scripts/pr_intake_gate.js b/.github/scripts/pr_intake_gate.js new file mode 100644 index 000000000..814026876 --- /dev/null +++ b/.github/scripts/pr_intake_gate.js @@ -0,0 +1,275 @@ +// 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 or removing +// the label is a sticky override (`bypass-issue-check`). +// +// Everything that writes goes through mutate(), so with ENFORCE unset 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 && closingRefs(i.body).includes(issueNumber)); + console.log(`#${issueNumber} assigned to ${assignee}: ${prs.length} gate-closed PR(s) reference it`); + for (const pr of prs) await evaluate(pr.number, 'assigned', context.payload.sender?.login); + 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) { + // 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. A PR someone + // closed for other reasons is left alone. + 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. + const author = pr.user.login.toLowerCase(); + const linked = []; + for (const num of closingRefs(pr.body).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 ?? l).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 #\` 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 or removing the \`${LABEL}\` 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 with the same \`Fixes #\` line.`, + ].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; in that case keep + // the label so the PR stays findable and explain in the comment. + 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 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) await mutate(`delete the gate comment on PR #${prNumber}`, () => github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id })); + } +}; diff --git a/.github/workflows/require-linked-issue.yml b/.github/workflows/require-linked-issue.yml new file mode 100644 index 000000000..1af2a7760 --- /dev/null +++ b/.github/workflows/require-linked-issue.yml @@ -0,0 +1,79 @@ +# 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 or removing the label overrides. +# +# Operating it: +# - Dry-run until the repository variable PR_GATE_ENFORCE is set to "true". +# - 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. This workflow checks out only the default branch (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, 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. + 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') + ) + runs-on: ubuntu-latest + timeout-minutes: 10 + concurrency: + group: require-linked-issue-${{ github.event.pull_request.number || inputs.pr_number || format('issue-{0}', github.event.issue.number) }} + 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 == 'true' && 'true' || 'false' }} + 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 }); diff --git a/AGENTS.md b/AGENTS.md index 2812ed6d1..a252a2688 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,12 @@ # Development Guidelines +## Note for AI Agents + +If you are an AI coding agent acting for someone who is not a maintainer of +this repository, read `CONTRIBUTING.md` before opening issues or pull +requests here. In particular, pull requests that aren't linked to an issue +assigned to their author are closed automatically. + ## Branching Model - `main` is the current stable line (v2); releases are cut from it (see diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b0fb9fa57..3a47c0813 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,53 +4,62 @@ Thank you for your interest in contributing to the MCP Python SDK! This document ## Before You Start -We welcome contributions! These guidelines exist to save everyone time, yours included. Following them means your work is more likely to be accepted. +> [!IMPORTANT] +> **The most useful contribution is a good issue.** Pull requests from outside the maintainer team are only reviewed when a maintainer has assigned you the linked issue, and others are closed automatically. The rest of this section explains why, and what we'd love help with. -**All pull requests require a corresponding issue.** Unless your change is trivial (typo, docs tweak, broken link), create an issue first. Every merged feature becomes ongoing maintenance, so we need to agree something is worth doing before reviewing code. PRs without a linked issue will be closed. +### Why Issues Rather Than Pull Requests -Having an issue doesn't guarantee acceptance. Wait for maintainer feedback or a `ready for work` label before starting. PRs for issues without buy-in may also be closed. +This SDK is looked after by a very small team with limited time for review. Now that coding agents can turn any open issue into a plausible-looking pull request within hours, we receive far more PRs than we could ever read carefully — and reviewing a PR properly still takes as long as it always did. When an issue is well described, it's usually quicker for a maintainer, with tooling that already knows this codebase and its conventions, to write a fix that fits than to review and reshape someone else's. -Use issues to validate your idea before investing time in code. PRs are for execution, not exploration. +What we can't produce ourselves is your context: what you were trying to do, what you expected, a minimal reproduction, the environment it breaks in, the constraint we hadn't considered. That's the valuable part, and it's what a good issue carries — so that's what we ask for first. -### AI-Assisted Contributions +### How Pull Requests Get In -> [!IMPORTANT] -> If you used AI assistance for a contribution, disclose it in the PR or issue. +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 a closing keyword (`Fixes #123`, `Closes #123`, `Resolves #123`). +2. A maintainer has assigned that issue to you, or the issue carries the [`help wanted`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) label (which means we'd welcome a PR for it from anyone). -We use AI tooling constantly and have no problem with you using it too. But somewhere in the loop there has to be a human who actually understands the change. We have a large backlog and limited reviewer time—we're not spending it on code nobody has read. Not disclosing is also just rude to the people on the other end. +Otherwise a bot labels the PR `missing-issue-link`, leaves a comment explaining this, and closes it. 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. -- **Disclose it.** One line in the PR or issue description. That's it. -- **Own it.** You can explain the change in your own words. When a maintainer asks a question, the answer comes from you, not pasted from a chat window. -- **No drive-by agents.** PRs, issues, or comments produced by an autonomous agent with no human review get closed on sight. If your agent is auto-filing PRs against our open issues, stop. +Whether to assign an issue, and to whom, is a [maintainer](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/MAINTAINERS.md#python-sdk) 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 behaviour, or briefly describing the approach you'd take. If you reported the issue and would like to fix it yourself, mention that in the issue — the reporter has first call if we do take an outside PR for it. -Undisclosed AI contributions get closed. Repeat offenders get banned from the `modelcontextprotocol` org. +### Who We'd Love to Hear From + +- **You've hit a real bug.** If you've run into a bug that affects your use case, that's extremely helpful for us to hear about. Talking through why it's a problem for you — rather than just that it is one — helps us both design the right fix and prioritise it, and a minimal reproduction makes it far more likely we can act quickly. +- **You'd like to learn the codebase or contribute regularly.** You're welcome, with one honest caveat: how much mentoring and review we can offer depends entirely on maintainer capacity, which is very limited at the moment, so replies may be slow and we may not be able to take everything on. The best way to start is by filing and triaging issues well and engaging on existing ones. `good first issue` still needs assignment — we'd like a short conversation first. You can find us in [#python-sdk-dev on the MCP Contributors Discord](https://discord.gg/6CSzBmMkjX). +- **You maintain another MCP SDK or work on the spec.** Say so on the issue or in #python-sdk-dev; a maintainer can reopen a specific PR past the gate. + +### AI-Assisted Contributions -### The SDK is Opinionated +We use AI tooling constantly and have no problem with you using it too. What matters is that a person is accountable for the result: -Not every contribution will be accepted, even with a working implementation. We prioritize maintainability and consistency over adding capabilities. This is at maintainers' discretion. +- **Disclose it.** One line in the PR or issue description. +- **Own it.** You can explain the change and the reasoning in your own words, and when a maintainer asks a question the answer comes from you rather than being pasted from a chat window. +- **Keep a human in the loop.** Issues, PRs, and comments generated by an agent without a person who has actually hit the problem and read the output may be closed without warning. If you have an agent filing PRs against open issues autonomously, please turn it off for this repository. +- **Keep issues short and factual.** What happened, what you expected, and how to reproduce it. -### What Needs Discussion +Undisclosed AI contributions may be closed, and repeated cases can lead to a block from the `modelcontextprotocol` org. The org-wide [AI contribution policy](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/AI_POLICY.md) also applies. -These always require an issue first: +### The SDK Is Opinionated + +Not every contribution will be accepted, even with a working implementation and an assigned issue. We prioritize maintainability and consistency over adding capabilities. This is at maintainers' discretion. + +These always need discussion on an issue before anyone writes code: - New public APIs or decorators - Architectural changes or refactoring - Changes that touch multiple modules - Features that might require spec changes (these need a [SEP](https://github.com/modelcontextprotocol/modelcontextprotocol) first) -Bug fixes for clear, reproducible issues are welcome—but still create an issue to track the fix. - -### Finding Issues to Work On +### Issue Labels -| Label | For | Description | -|-------|-----|-------------| -| [`good first issue`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) | Newcomers | Can tackle without deep codebase knowledge | -| [`help wanted`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) | Experienced contributors | Maintainers probably won't get to this | -| [`ready for work`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22ready+for+work%22) | Maintainers | Triaged and ready for a maintainer to pick up | - -Issues labeled `needs confirmation` or `needs maintainer action` are **not** ready for work—wait for maintainer input first. - -Before starting, comment on the issue so we can assign it to you. This prevents duplicate effort. +| Label | Meaning | +|-------|---------| +| [`help wanted`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) | We'd welcome a PR for this from anyone — no assignment needed | +| [`good first issue`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) | Approachable without deep codebase knowledge; still needs assignment (see above) | +| [`ready for work`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22ready+for+work%22) | Triaged and queued for a maintainer to pick up (not a call for PRs) | +| `needs confirmation`, `needs repro`, `needs decision`, `needs design` | Not actionable yet; more information or a maintainer decision is needed first | ## Development Setup @@ -117,7 +126,7 @@ uv run scripts/update_readme_snippets.py pre-commit run --all-files ``` -9. Submit a pull request to the same branch you branched from +9. Open a pull request against the branch you started from — see [Pull Requests](#pull-requests); you need to be assigned to the linked issue first ## Code Style @@ -128,7 +137,10 @@ pre-commit run --all-files ## Pull Requests -By the time you open a PR, the "what" and "why" should already be settled in an issue. This keeps reviews focused on implementation. +By the time you open a PR, you should be assigned to the issue it fixes (see [How Pull Requests Get In](#how-pull-requests-get-in)) and the "what" and "why" should already be settled there. This keeps reviews focused on implementation. + +- Put `Fixes #` in the description — the intake gate looks for it. +- If your PR was auto-closed, there's no need to open another: fix the description or wait to be assigned and it reopens itself. Avoid force-pushing the branch while it's closed. ### Scope @@ -138,11 +150,11 @@ A few dozen lines can be reviewed in minutes. Hundreds of lines across many file ### What Gets Rejected -- **No prior discussion**: Features or significant changes without an approved issue -- **Scope creep**: Changes that go beyond what was discussed -- **Misalignment**: Even well-implemented features may be rejected if they don't fit the SDK's direction -- **Overengineering**: Unnecessary complexity for simple problems -- **Undisclosed or unreviewed AI output**: See [AI-Assisted Contributions](#ai-assisted-contributions) +- **No assigned issue**: closed automatically until one is linked, as above +- **Scope creep**: changes that go beyond what was discussed on the issue +- **Misalignment**: even well-implemented features may be rejected if they don't fit the SDK's direction +- **Overengineering**: unnecessary complexity for simple problems +- **Undisclosed or unreviewed AI output**: see [AI-Assisted Contributions](#ai-assisted-contributions) ### Checklist