diff --git a/.changes/source-control/fix-babysit-hold-line-1c7c3278.md b/.changes/source-control/fix-babysit-hold-line-1c7c3278.md new file mode 100644 index 0000000000..d50c8c50c4 --- /dev/null +++ b/.changes/source-control/fix-babysit-hold-line-1c7c3278.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- babysit-prs: the merge gate and the merge-entry hold also treat a PR body line starting `Hold:` (any case) as a human hold on the merge (the `pull-request ready` flip does not stop for it); before, only "do not merge" or "don't merge" in the body held the merge. diff --git a/plugins/source-control/skills/babysit-loop/SKILL.md b/plugins/source-control/skills/babysit-loop/SKILL.md index cb4448e654..773b236de3 100644 --- a/plugins/source-control/skills/babysit-loop/SKILL.md +++ b/plugins/source-control/skills/babysit-loop/SKILL.md @@ -219,10 +219,8 @@ and user settings itself. ## do-not-merge -A do-not-merge label, or a PR body that says "do not merge", is respected by default in every tier -and at every rung, the PR is reported, never merged, and the label is never removed. Stripping it happens only behind the explicit -`--strip-do-not-merge` invocation flag: a per-invocation direct order, never a config key, never -persisted. +A do-not-merge label, or a PR body that says "do not merge" or has a line starting `Hold:`, is respected by default in every tier and at every rung: the PR is reported, never merged, and the label is never removed. +Stripping it happens only behind the explicit `--strip-do-not-merge` invocation flag: a per-invocation direct order, never a config key, never persisted. ## Escalation diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index 12aeed4e4b..ba35116b96 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -146,8 +146,8 @@ never routed around. Advisory-only fix attempts are bounded per PR (the fix-roun never merged autonomously in ANY tier, the merge gate refuses it absent `--allow-dependency`, which is passed only on an explicit user instruction to merge that specific PR. -**Do-not-merge hold:** a PR whose body says "do not merge" (or "don't merge") is held in ANY tier: -the merge gate reads the live body and reports a blocker, and a PR carrying a `do-not-merge` +**Do-not-merge hold:** a PR whose body says "do not merge" (or "don't merge"), or has a line +starting `Hold:`, is held in ANY tier: the merge gate reads the live body and reports a blocker, and a PR carrying a `do-not-merge` label is held the same way: the gate blocks on that label unconditionally, and `--block-labels` adds further labels in the autopilot merge tier. The PR is reported and left unmerged. The hyphenated label name in a body is not a hold. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index be571d56d1..f5b19f33bf 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -187,11 +187,15 @@ # merge) so ordinary prose does not false-block. HUMAN_MERGE_VETO_RE = re.compile(r"\bdo(?:n['’]?t| not|-not)[\s-]*merge\b", re.I) -# The PR author's own hold, written in the body ("Do not merge this draft"): -# the label is not the only way a PR says it must not merge. The hyphenated -# form is the label's name and is not matched, so a body that merely names the -# label does not block. -BODY_MERGE_HOLD_RE = re.compile(r"\bdo(?:n['’]?t| not)\s+merge\b", re.I) +# The PR author's own hold, written in the body ("Do not merge this draft", or +# a line starting "Hold:"): the label is not the only way a PR says it must not +# merge. The hyphenated form is the label's name and is not matched, so a body +# that merely names the label does not block. A "Hold:" line may follow any +# whitespace, NBSP included, and a bare CR counts as a line break. A "Hold:" +# line holds the merge only: the pull-request ready flip ignores it. +BODY_MERGE_HOLD_RE = re.compile( + r"\bdo(?:n['’]?t| not)\s+merge\b|(?:^|\r)[^\S\r\n]*hold:", re.I | re.M +) # 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_LABEL = "do-not-merge" @@ -1451,7 +1455,8 @@ def evaluate( blockers.append("PR is a draft -- mark ready first") if BODY_MERGE_HOLD_RE.search(str(pr.get("body") or "")): blockers.append( - "PR body says do not merge -- a human hold, report it and leave it" + "PR body holds the merge (do not merge, or a Hold: line) -- a human " + "hold, report it and leave it" ) if any(str(name).casefold() == MERGE_HOLD_LABEL for name in labels): blockers.append( diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py index 973b15252b..d4667ca874 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py @@ -670,10 +670,35 @@ def test_body_saying_do_not_merge_blocks_with_and_without_the_tier(self) -> None result = self._evaluate(_pr(body=body), tier=tier) self.assertFalse(result["ready"]) self.assertTrue( - any("body says do not merge" in b for b in result["blockers"]), + any("body holds the merge" in b for b in result["blockers"]), result["blockers"], ) + def test_body_line_starting_hold_blocks_with_and_without_the_tier(self) -> None: + for body in ( + "Refs: #1\r\n\r\nHold: a human merges this\r\n", + "## Summary\n hold: until the release ships", + "Refs: #1\n Hold: pasted from a rich-text editor", + "Refs: #1\rHold: a bare CR line break", + ): + for tier in (TIER, None): + with self.subTest(body=body, tier=tier is not None): + result = self._evaluate(_pr(body=body), tier=tier) + self.assertFalse(result["ready"]) + self.assertTrue( + any("body holds the merge" in b for b in result["blockers"]), + result["blockers"], + ) + + def test_hold_word_inside_a_line_does_not_block(self) -> None: + for body in ( + "The merge lane reads a hold: label or body line.", + "Threshold: 3 retries", + ): + with self.subTest(body=body): + result = self._evaluate(_pr(body=body), tier=None) + self.assertTrue(result["ready"], result["blockers"]) + def test_do_not_merge_label_blocks_with_and_without_the_tier(self) -> None: for name in ("do-not-merge", "Do-Not-Merge"): for tier in (TIER, None): 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 8c930175b0..4df41301b7 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 @@ -1303,6 +1303,10 @@ def test_a_body_hold(self) -> None: self.fake(body="Please do not merge until the release") self.assertEqual(self.hold(), "body-hold") + def test_a_body_hold_line(self) -> None: + self.fake(body="Refs: #1\r\n\r\nHold: a human merges this") + self.assertEqual(self.hold(), "body-hold") + def test_a_human_blocking_comment(self) -> None: gh = self.fake() gh.rest[f"repos/{REPO}/issues/5/comments?per_page=100"] = [