From bc0521d6d2be94bb39c178142af7bb1d77211fbd Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:23:37 +0000 Subject: [PATCH] Ask cubic for a review when the intake gate lets a PR back in cubic starts on `opened`, abandons the run when the gate closes the PR a few seconds later, and does not act on `reopened`. A PR that comes back after the author is assigned, the description is fixed, or a maintainer overrides therefore sat unreviewed until its next push. The gate now leaves one `@cubic-dev-ai review this PR` comment when a PR it had closed returns to open. Drafts are skipped; cubic picks those up itself on ready_for_review. --- .github/scripts/pr_intake_gate.js | 15 ++++++++++- .github/scripts/pr_intake_gate.test.js | 30 ++++++++++++++++------ .github/workflows/require-linked-issue.yml | 3 ++- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/.github/scripts/pr_intake_gate.js b/.github/scripts/pr_intake_gate.js index 0fb2864da4..436585ef03 100644 --- a/.github/scripts/pr_intake_gate.js +++ b/.github/scripts/pr_intake_gate.js @@ -8,7 +8,8 @@ // 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. +// sticks. A PR that comes back this way also gets a comment asking the review +// bot for a review, since it doesn't act on `reopened` by itself. // // Everything that writes goes through mutate(); when the workflow passes // ENFORCE=false (its kill switch) the run only logs what it would have done. @@ -20,6 +21,10 @@ const OPEN_LABEL = 'help wanted'; // issue label that waives assignment const MARKER = ''; const BOT_LOGIN = 'github-actions[bot]'; const MAX_ISSUES = 5; +// cubic starts on `opened` and abandons the run when the gate closes the PR +// seconds later; it ignores `reopened`, so a PR the gate lets back in would +// otherwise wait for its next push to be reviewed. +const REVIEW_REQUEST = '@cubic-dev-ai review this PR'; module.exports = async function run({ github, context, core }) { const { owner, repo } = context.repo; @@ -115,6 +120,8 @@ module.exports = async function run({ github, context, core }) { if (gated) { await removeLabel(prNumber, LABEL); await deleteGateComment(prNumber); + // Not for drafts: cubic picks those up itself on ready_for_review. + if (!pr.draft) await requestReview(prNumber); } } @@ -298,6 +305,12 @@ module.exports = async function run({ github, context, core }) { } } + // Runs once per return: the caller only gets here while the PR still counts + // as gate-closed, and it has just removed the label that says so. + async function requestReview(prNumber) { + await mutate(`request a review on PR #${prNumber}`, () => github.rest.issues.createComment({ owner, repo, issue_number: prNumber, body: REVIEW_REQUEST })); + } + async function deleteGateComment(prNumber) { const existing = await findGateComment(prNumber); if (!existing) return; diff --git a/.github/scripts/pr_intake_gate.test.js b/.github/scripts/pr_intake_gate.test.js index a7cde4bf32..469430d0f8 100644 --- a/.github/scripts/pr_intake_gate.test.js +++ b/.github/scripts/pr_intake_gate.test.js @@ -14,6 +14,7 @@ const gate = require('./pr_intake_gate.js'); const LABEL = 'missing-issue-link'; const BYPASS = 'bypass-issue-check'; +const REVIEW_REQUEST = '@cubic-dev-ai review this PR'; const REPO = { owner: 'modelcontextprotocol', repo: 'python-sdk' }; // People. Only the capability flags matter to the gate. @@ -29,7 +30,8 @@ const PEOPLE = { // `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, 'closed-draft' = its draft wording, 'cannot-reopen' = the -// refused-reopen comment, null = none). +// refused-reopen comment, null = none). `reviewRequested`, where given, is +// whether the gate left its comment asking the review bot for a review. // `writes: 0` additionally asserts the gate touched nothing at all. const scenarios = [ @@ -78,7 +80,7 @@ const scenarios = [ 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 } }, + expect: { 3300: { state: 'open', labels: [], comment: null, reviewRequested: true } }, }, { name: 'gate-closed PR: maintainer assigns the author on the linked issue → reopened', @@ -89,21 +91,21 @@ const scenarios = [ 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' }, + 3300: { state: 'open', labels: [], comment: null, reviewRequested: true }, + 3301: { state: 'closed', labels: [LABEL], comment: 'closed', reviewRequested: false }, }, }, { 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 } }, + expect: { 3300: { state: 'open', labels: [BYPASS], comment: null, reviewRequested: true } }, }, { 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 } }, + expect: { 3300: { state: 'open', labels: [BYPASS], comment: null, reviewRequested: true } }, }, { name: 'gate-closed PR: some other bot strips the label → re-checked, label restored, still closed', @@ -177,13 +179,20 @@ const scenarios = [ 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' } }, + expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'cannot-reopen', reviewRequested: false } }, }, { 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 } }, + expect: { 3300: { state: 'open', labels: [BYPASS], comment: null, reviewRequested: true } }, + }, + { + name: 'gate-closed draft comes back → reopened, but no review is requested while it is a draft', + prs: [pr(3300, 'outsider', { state: 'closed', draft: true, labels: [LABEL], body: 'Fixes #10', gateComment: true })], + issues: [issue(10, { assignees: ['outsider'] })], + event: assigned(10, 'outsider', 'maintainer'), + expect: { 3300: { state: 'open', labels: [], comment: null, reviewRequested: false } }, }, { name: 'refused reopen after a label-removal override → both labels on, so the PR stays gate-managed', @@ -303,6 +312,11 @@ function observe(world, expect) { const kind = !body ? null : body.includes("won't let it be reopened") ? 'cannot-reopen' : body.includes('still a draft') ? 'closed-draft' : '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; + if ('reviewRequested' in expect[num]) { + const requests = p.comments.filter((c) => c.user === 'github-actions[bot]' && c.body === REVIEW_REQUEST); + assert.ok(requests.length <= 1, `PR #${num} has ${requests.length} review requests`); + out[num].reviewRequested = requests.length === 1; + } } return out; } diff --git a/.github/workflows/require-linked-issue.yml b/.github/workflows/require-linked-issue.yml index 309978e813..3ebeccf1e3 100644 --- a/.github/workflows/require-linked-issue.yml +++ b/.github/workflows/require-linked-issue.yml @@ -6,7 +6,8 @@ # 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. +# or adding `bypass-issue-check` overrides. A PR that comes back open gets a +# comment asking cubic to review it, because cubic doesn't act on `reopened`. # # Operating it: # - Live by default. To pause it without a revert, set the repository