diff --git a/.github/workflows/require-issue-link.yml b/.github/workflows/require-issue-link.yml index cb4bc4243..68faa1ab2 100644 --- a/.github/workflows/require-issue-link.yml +++ b/.github/workflows/require-issue-link.yml @@ -1,7 +1,8 @@ # Require external PRs to reference an issue with an auto-close keyword -# (e.g. "Fixes #123"). When missing, the PR is labeled "missing-issue-link", -# commented on, and closed. CONTRIBUTING.md requires every PR to address a -# tracked issue; this enforces that for outside contributors. +# (e.g. "Fixes #123") AND have the PR author assigned to that issue. +# Otherwise the PR is labeled "missing-issue-link", commented on, and +# closed. CONTRIBUTING.md requires external contributors to be assigned to +# an issue before opening a PR; this enforces that. # # Adapted from langchain-ai/langchain's require_issue_link.yml. Differences: # - Self-contained: it does NOT depend on a separate labeler workflow @@ -13,9 +14,6 @@ # private appears as CONTRIBUTOR/NONE, so gating on it would wrongly # enforce against private-member maintainers. getCollaboratorPermission # reflects effective write access regardless of membership visibility. -# - No assignee requirement. fastmcp's CONTRIBUTING.md states that -# referencing an issue "isn't a permission step" — only the link is -# enforced, not issue assignment. # - Single github-script step (the upstream version is split across four, # forcing the label/comment/reopen helpers to be duplicated per scope). # @@ -227,12 +225,10 @@ jobs: return; } // Only triage/admin can manage labels, so a non-write actor - // reaching here is rare (triage role). Re-add and fall through - // to enforcement. addLabels fires a "labeled" event, which the - // job gate ignores — no re-trigger loop. - console.log(`Non-maintainer ${sender} removed ${LABEL} — re-enforcing`); - await closeForMissingLink(); - return; + // reaching here is rare (triage role). Fall through to the + // normal check, which recomputes link + assignment and + // re-enforces with the correct message if still failing. + console.log(`Non-maintainer ${sender} removed ${LABEL} — re-checking`); } // ── Maintainer override: reopened a PR we had closed ─────────── @@ -261,29 +257,92 @@ jobs: const pattern = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s*#(\d+)/gi; const matches = [...body.matchAll(pattern)]; - if (matches.length > 0) { - const issues = [...new Set(matches.map(m => `#${m[1]}`))].join(', '); - console.log(`Found issue link(s): ${issues} — clearing any prior enforcement`); - await clearEnforcement(); + if (matches.length === 0) { + console.log('No issue link found in PR body'); + await enforceFailure('no-link'); return; } - console.log('No issue link found in PR body'); - await closeForMissingLink(); + // The author must be assigned to at least one linked issue. + // CONTRIBUTING.md requires external contributors to be assigned + // before opening a PR (so maintainers can deconflict / steer + // approach first). + const MAX_ISSUES = 5; + const allNumbers = [...new Set(matches.map(m => parseInt(m[1], 10)))]; + const numbers = allNumbers.slice(0, MAX_ISSUES); + if (allNumbers.length > MAX_ISSUES) { + core.warning(`PR references ${allNumbers.length} issues — checking only the first ${MAX_ISSUES}`); + } + + const prAuthor = pr.user.login.toLowerCase(); + let sawRealIssue = false; + let assignedToAny = false; + for (const num of numbers) { + let issue; + try { + ({ data: issue } = await github.rest.issues.get({ + owner, repo, issue_number: num, + })); + } catch (e) { + if (e.status === 404) { + console.log(`#${num} does not exist — ignoring`); + continue; + } + // Same safe-direction rule as hasWriteAccess: a transient + // error must not be read as "not assigned" and close the PR. + throw new Error(`Cannot fetch issue #${num} (HTTP ${e.status ?? 'unknown'}): ${e.message}`); + } + sawRealIssue = true; + const assignees = (issue.assignees || []).map(a => a.login.toLowerCase()); + if (assignees.includes(prAuthor)) { + console.log(`PR author ${pr.user.login} is assigned to #${num}`); + assignedToAny = true; + break; + } + console.log(`PR author ${pr.user.login} is NOT assigned to #${num} (assignees: ${assignees.join(', ') || 'none'})`); + } + + if (!sawRealIssue) { + console.log('Referenced issue(s) do not exist'); + await enforceFailure('no-link'); + return; + } + if (!assignedToAny) { + await enforceFailure('not-assigned'); + return; + } + + console.log('Linked and assigned — clearing any prior enforcement'); + await clearEnforcement(); // ── Label, comment, close, and fail ──────────────────────────── - async function closeForMissingLink() { + // `kind`: 'no-link' (no valid issue reference) or 'not-assigned' + // (referenced an issue, but the author isn't assigned to it). + async function enforceFailure(kind) { await addLabel(); + const intro = kind === 'no-link' + ? '**This PR has been automatically closed** because its description does not reference a tracked issue.' + : '**This PR has been automatically closed** because you are not assigned to the issue it references.'; + const steps = kind === 'no-link' + ? [ + `1. Find or [open an issue](https://github.com/${owner}/${repo}/issues/new/choose) describing the change.`, + '2. Comment on the issue to ask a maintainer to assign it to you.', + '3. Add `Fixes #`, `Closes #`, or `Resolves #` to the PR description.', + '4. Once you are assigned and the link is present, the PR reopens automatically.', + ] + : [ + '1. Comment on the linked issue to ask a maintainer to assign it to you.', + '2. Once a maintainer assigns you, the PR reopens automatically.', + ]; + const commentBody = [ MARKER, - '**This PR has been automatically closed** because its description does not reference a tracked issue.', + intro, '', - `Per [CONTRIBUTING.md](https://github.com/${owner}/${repo}/blob/main/CONTRIBUTING.md), every PR should address a tracked issue. To proceed:`, + `Per [CONTRIBUTING.md](https://github.com/${owner}/${repo}/blob/main/CONTRIBUTING.md), an external PR must reference an issue that is assigned to its author. To proceed:`, '', - `1. Find or [open an issue](https://github.com/${owner}/${repo}/issues/new/choose) describing the change.`, - '2. Add `Fixes #`, `Closes #`, or `Resolves #` to the PR description.', - '3. The PR reopens automatically once the description links an issue.', + ...steps, '', `*Maintainers: reopen this PR or remove the \`${LABEL}\` label to bypass this check.*`, ].join('\n'); @@ -313,7 +372,9 @@ jobs: if (enforce) { core.setFailed( - 'PR must reference a tracked issue using an auto-close keyword (e.g. "Fixes #123").', + kind === 'no-link' + ? 'PR must reference a tracked issue using an auto-close keyword (e.g. "Fixes #123").' + : 'PR author must be assigned to the referenced issue.', ); } } diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6f861c50a..44b035746 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,7 +20,7 @@ We encourage you to use LLMs to help identify bugs, write MREs, and prepare cont ## When to open a pull request -An open issue is not an invitation to submit a PR. Issues track problems; whether and how to solve them is a separate decision. If you want to work on something, propose your approach in the issue first — especially for anything beyond a trivial fix. +An open issue is not an invitation to submit a PR. Issues track problems; whether and how to solve them is a separate decision. If you want to work on something, propose your approach in the issue first and ask a maintainer to assign it to you — especially for anything beyond a trivial fix. External PRs that reference an issue not assigned to their author are closed automatically (see [PR guidelines](#pr-guidelines)). **Bug fixes** — PRs are welcome for simple, well-scoped bug fixes where the problem and solution are both straightforward. "The function raises `TypeError` when passed `None` because of a missing guard" is a good candidate. If the fix requires design decisions or touches multiple subsystems, open an issue with a design proposal instead. @@ -34,7 +34,7 @@ An open issue is not an invitation to submit a PR. Issues track problems; whethe If you do open a PR: -- **Reference an issue.** Every PR should address a tracked issue. If there isn't one, open an issue first. This isn't a permission step — you don't need to wait for a response. But the issue gives us context on the problem, and if a maintainer is already working on it, we can let you know before you invest time in code. +- **Reference an issue you're assigned to.** Every PR must reference a tracked issue using an auto-close keyword (`Fixes #123`, `Closes #123`, or `Resolves #123`), and the referenced issue must be assigned to you. If there isn't an issue, open one; then comment to ask a maintainer to assign it to you. This lets us deconflict effort and steer the approach before you invest time in code. External PRs that don't meet both conditions are automatically labeled `missing-issue-link` and closed; they reopen automatically once the link is present and you're assigned. (Maintainers can bypass the check by reopening the PR or removing the label.) - **Keep it focused.** One logical change per PR. Don't bundle unrelated fixes or refactors. - **Match existing patterns.** Follow the code style, type annotation conventions, and test patterns you see in the codebase. Run `uv run prek run --all-files` before submitting. - **Write tests.** Bug fixes should include a test that fails without the fix. Enhancements should include tests for the new behavior.