Gate draft PRs too and rewrite the auto-close comment - #3378
Conversation
Drafts were skipped until marked ready for review, which let unlinked PRs sit open indefinitely as drafts. They now go through the same rule as any other PR. The comment the gate leaves is rewritten to be clearer about what happened, what (if anything) the author can do, and what to expect: it no longer implies the PR will be reopened, and says plainly that review capacity for community PRs is limited.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
.github/scripts/pr_intake_gate.js— nit, pre-existing:refs.slice(0, MAX_ISSUES)only fetches the first five closing refs, and the front-loading hint exists solely on theissues assignedpath — aneditedre-run has no hint. So an author who follows the new closed-comment bullet (line 152: "addFixes #<n>to the description and the PR will reopen on its own") stays closed if the description already carries five closing refs before the qualifying one; the base failed the same way but never promised reopen-on-edit for already-assigned authors. A fix should evaluate all refs for assignment (only fail-listing is what needs a cap), or prioritize refs the edit added.Extended reasoning...
Path: an outsider's PR body already contains five distinct closing refs to non-qualifying issues (e.g. an AI-written description with "Fixes #1, Fixes #2, ... Fixes #5"). The gate closes it. A maintainer assigns the author to issue #N — but #N wasn't referenced, so the
issues: assignedjob filter (line 39,closingRefs(i.body).includes(issueNumber)) skips the PR. The author then follows the posted comment's bullet 1 (line 152) and appendsFixes #N. Theeditedevent re-runs evaluate(): hintIssue is null (only the assigned path at line 46 passes one), refs preserves appearance order, line 100'srefs.slice(0, MAX_ISSUES)(MAX_ISSUES=5, line 22) drops #N, so lines 104-105 never see the issue the author is assigned to and fail(linked) keeps the PR closed with a comment claiming they "aren't currently assigned to #1, #2...". No later event retries: the assignment already fired before the edit, so the reopen promise the merged comment makes is a dead end until a maintainer overrides. The base branch truncated identically on the edited path (pre-existing mechanism), but its commVerification: nit. .github/scripts/pr_intake_gate.js:100
refs.slice(0, MAX_ISSUES)(MAX_ISSUES=5, line 22) fetches only the first five closing refs, and the front-loading at line 98 only runs whenhintIssueis passed — which happens solely on theissues: assignedpath (line 46); theeditedpath (line 62) passes no hint. Theissues: assignedpath itself skips PRs that don't already reference the assign
| ? [ | ||
| `${rule}, and you aren't currently assigned to ${issues}.`, | ||
| '', | ||
| `If a maintainer assigns you to ${issues}, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take.`, |
There was a problem hiding this comment.
🟡 nit: now that drafts get gate-closed, a draft with a linked-but-unassigned issue receives the linked-issues comment saying "this PR reopens on its own and there's nothing more you need to do here" — false for drafts, which after reopening are still drafts and need the author to mark them ready before anyone reviews them. Only the opener (line 140) is draft-aware; the situation text isn't. Add a draft-conditional clause to the assigned-path text (e.g. "once reopened, mark it ready for review") so draft authors aren't told they're done.
Extended reasoning...
Before this PR, if (pr.draft) return log(...) meant a draft could never reach fail() and closedComment() was only ever rendered for non-draft PRs, so "nothing more you need to do here" was accurate. After this change, fail() at line 124 passes pr.draft into closedComment(), but only the opener at lines 139-141 varies by draft; the linkedIssues.length branch at line 147 keeps the unconditional "this PR reopens on its own and there's nothing more you need to do here". Trigger: an outsider opens a draft PR with Fixes #N where #N is open but unassigned — the gate closes it with this comment. When a maintainer later assigns them, reopen() at line 240 reopens the PR still in draft state and deleteGateComment removes the comment; the PR then sits as an unreviewed draft because the author was explicitly told there was nothing more to do, when in fact they must click "ready for review" (which fires the ready_for_review event and re-passes). Consequence relative to base: base never showed this comment to draft authors, so the misleading promise is newly reachable. Fix: make the assig
Verification: nit — the described text mismatch is real and newly reachable, but it is only slightly misleading wording; nothing breaks. In .github/scripts/pr_intake_gate.js, fail() at line 124 now passes pr.draft into closedComment(pr.draft, linkedIssues), and drafts reach fail() because the diff removed the base branch's if (pr.draft) return log('draft — skipped until ready for review') guard (old lines
| # 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, |
There was a problem hiding this comment.
🟡 nit: the PR description says "CONTRIBUTING.md gets one clause noting drafts are covered" and ticks the Documentation update box, but the diff never touches CONTRIBUTING.md (the last commit, ccd46b7, deliberately reverts that change), so after merge the drafts-are-gated policy is documented only in this workflow header while the file the auto-close comment links contributors to stays silent. Either restore the one-clause CONTRIBUTING.md edit or update the PR description so it matches what merges.
Extended reasoning...
The PR body claims: "CONTRIBUTING.md gets one clause noting drafts are covered", and checks "I have added or updated documentation as needed". git log for the range shows commits b3a5054..ccd46b7, the last titled "Leave CONTRIBUTING.md unchanged" — grep confirms CONTRIBUTING.md contains no mention of drafts at all. The gate's closed comment (pr_intake_gate.js:162) points authors at CONTRIBUTING.md#how-pull-requests-get-in for "the full reasoning", but that section never says drafts are gated; the only place the new draft policy is written down is the header comment at .github/workflows/require-linked-issue.yml:7-8, which contributors do not read. No runtime failure — the blanket rule in CONTRIBUTING.md covers all PRs by silence — but the merged state does not match what the PR description promises, so a reviewer approving based on the description would believe the contributor-facing doc was updated when it was not.
Verification: nit — every factual link in the candidate checks out. (1) The PR description claims "CONTRIBUTING.md gets one clause noting drafts are covered" and ticks "Documentation update", but the net diff git diff 57394b0..HEAD --stat touches only .github/scripts/pr_intake_gate.js, .github/scripts/pr_intake_gate.test.js, and .github/workflows/require-linked-issue.yml — CONTRIBUTING.md is net-unc
1 participant
Follow-up to #3291. Two changes to the PR intake gate:
Drafts are gated too. The gate skipped drafts until they were marked ready for review, which meant an unlinked PR could sit open indefinitely as a draft. Drafts now go through the same rule as everything else. The
ready_for_reviewtrigger stays so drafts opened before this change are still evaluated when they flip.The auto-close comment is rewritten. The old text implied the PR would be reopened ("this one will be reopened", "nothing more you need to do") and packed everything into two dense paragraphs. The new one says what happened and why in one sentence, lists what the author can actually do for each situation (already assigned / no issue yet / issue exists but not assigned), and is upfront that review capacity for community PRs is very limited and most auto-closed PRs stay closed. When the PR is a draft, the opener adds one sentence explaining why it's closed before being marked ready (so the author doesn't put more time in only to have it closed then).
Motivation and Context
Drafts were being used to keep unlinked PRs visible past the gate, and the closing comment was setting expectations we can't meet.
How Has This Been Tested?
node --test .github/scripts/pr_intake_gate.test.js— the two draft scenarios now expect a close; 29/29 pass. Both comment variants rendered and read through by hand.Breaking Changes
None for SDK users. External draft PRs without an assigned, linked issue are now closed on open/edit rather than on ready-for-review.
Types of changes
Checklist
Additional context
Already-closed PRs keep the old comment until their next gate event (edit, assignment, label change). Currently-open external drafts are evaluated on their next event, or by hand with
gh workflow run require-linked-issue.yml -f pr_number=N.AI Disclaimer