diff --git a/.changes/source-control/fix-merge-hold-pr-verify-failed-5d2c8e61.md b/.changes/source-control/fix-merge-hold-pr-verify-failed-5d2c8e61.md new file mode 100644 index 0000000000..61e875af72 --- /dev/null +++ b/.changes/source-control/fix-merge-hold-pr-verify-failed-5d2c8e61.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- The merge gate holds a pull request labeled `pr-verify-failed` as it holds `do-not-merge` (`MERGE_HOLD_LABELS`): pr-merge's prefilter sets it aside, the gate does not merge or enqueue it, adoption refuses it, and a queued entry that gains the label leaves the queue. pr-merge's enable-auto-merge job neither arms it nor leaves it armed. diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index c36326494d..a629534e44 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -183,7 +183,7 @@ const REPO = /^([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)$/; const SHA = /^[0-9a-f]{40}$/; const BOT_LOGIN = /^([a-z0-9][a-z0-9-]*)\[bot\]$/; // babysit_merge_policy.MERGE_HOLD_LABELS, plus the human-gated label. -const HOLD_LABELS = Object.freeze(["do-not-merge"]); +export const HOLD_LABELS = Object.freeze(["do-not-merge", "pr-verify-failed"]); // 100 pull requests a page; more than this many open is a listing the run // stops on rather than one it reads only part of. const MAX_PAGES = 10; diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index f7d1748fa6..552a952ccc 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -906,6 +906,18 @@ test("an armed pull request that now holds is disarmed", async () => { ]); }); +test("a pr-verify-failed pull request is not armed, and an armed one is disarmed", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + for (const EVENT_NAME of ["schedule", "pull_request_target"]) { + const { code, writes } = await run( + [pull(1, { labels: labels("pr-verify-failed") }), pull(2, { autoMergeRequest: armed, labels: labels("PR-Verify-Failed") })], + { EVENT_NAME }, + ); + assert.equal(code, 0); + assert.deepEqual(writes, [{ mutation: "disarm", id: "PR_2" }], EVENT_NAME); + } +}); + test("only the lanes App's ready pull requests from this repository are armed", async () => { const { writes } = await run([ pull(1, { author: { __typename: "User", login: "kyle-sexton" } }), diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py index 997d53e170..2e89dbd0bd 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py @@ -110,7 +110,9 @@ ) # The label form of the same hold: blocks in every tier, whether or not the # autopilot merge tier is engaged. `--block-labels` only adds to it. -MERGE_HOLD_LABELS = ("do-not-merge",) +# `pr-verify-failed` is pr-verify's FAIL on a person's pull request, which +# ci-status enforces only in the merge queue: held here, it is never queued. +MERGE_HOLD_LABELS = ("do-not-merge", "pr-verify-failed") # An `adoption.idle-hours` value outside 1 to `MAX_ADOPTION_IDLE_HOURS`, or # none, adopts nothing. MAX_ADOPTION_IDLE_HOURS = 720 diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py index 3eb55f1d41..f824e7052a 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py @@ -2298,6 +2298,11 @@ def test_the_do_not_merge_label(self) -> None: self.fake(labels={"totalCount": 1, "nodes": [{"name": "Do-Not-Merge"}]}) self.assertEqual(self.hold(), "do-not-merge") + def test_the_pr_verify_failed_label(self) -> None: + # A queued entry whose pr-verify FAIL labels it leaves the queue. + self.fake(labels={"totalCount": 1, "nodes": [{"name": "PR-Verify-Failed"}]}) + self.assertEqual(self.hold(), "pr-verify-failed") + def test_the_needs_human_label(self) -> None: # A queued entry that gains the human-gated label leaves the queue. self.fake(labels={"totalCount": 1, "nodes": [{"name": "Needs-Human"}]}) @@ -4609,6 +4614,10 @@ def test_the_do_not_merge_label_holds(self) -> None: pr = owner_pr(labels=[{"name": "Do-Not-Merge"}]) self.assertEqual(self.refusal(pr), "hold-label:do-not-merge") + def test_the_pr_verify_failed_label_holds(self) -> None: + pr = owner_pr(labels=[{"name": "pr-verify-failed"}]) + self.assertEqual(self.refusal(pr), "hold-label:pr-verify-failed") + def test_the_resolved_human_gated_label_holds(self) -> None: pr = owner_pr(labels=[{"name": "awaiting-owner"}]) self.assertEqual( diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_prefilter.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_prefilter.py index 1b459d7f5c..cf4e171f02 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_prefilter.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_prefilter.py @@ -98,6 +98,15 @@ def test_one_held_candidate_stays_when_every_candidate_is_held(self) -> None: [(h["number"], h["reason"]) for h in held], [(6, "do-not-merge")] ) + def test_a_pr_verify_failed_candidate_is_dropped(self) -> None: + kept, held = self._run( + [_candidate(1), _candidate(8, labels=["pr-verify-failed"])] + ) + self.assertEqual([c["number"] for c in kept], [1]) + self.assertEqual( + [(h["number"], h["reason"]) for h in held], [(8, "pr-verify-failed")] + ) + def test_no_candidates_stay_none(self) -> None: self.assertEqual(self._run([]), ([], [])) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py index ee0e184b28..5e8c5adfc7 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py @@ -414,6 +414,23 @@ def _list(*actors: Any, version: Any = 1) -> str: } # fmt: skip +@unittest.skipUnless(shutil.which("node"), "node not installed") +class TheHoldLabelsAreTheGates(unittest.TestCase): + def test_both_hold_the_same_labels(self) -> None: + module = ROOT / ".github/scripts/pr-merge/enable-auto-merge.mjs" + script = ( + f"const {{ HOLD_LABELS }} = await import({json.dumps(module.as_uri())});" + "console.log(JSON.stringify(HOLD_LABELS));" + ) + proc = subprocess.run( + ["node", "--input-type=module", "-e", script], + capture_output=True, + text=True, + check=True, + ) + self.assertEqual(json.loads(proc.stdout), list(policy.MERGE_HOLD_LABELS)) + + @unittest.skipUnless(shutil.which("node"), "node not installed") class TheWideningHoldIsTheGates(unittest.TestCase): def test_both_hold_the_same_trusted_actor_list_changes(self) -> None: