fix(source-control): fold every composing ruleset rule instead of the last

plugins/source-control/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
   "name": "source-control",
-  "version": "0.51.8",
+  "version": "0.51.9",
   "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and standing merge-rung raises binding from the team-tracked layer only — with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply — interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.",
   "author": {
     "name": "Melodic Software",

plugins/source-control/CHANGELOG.md

@@ -3,6 +3,39 @@
 All notable changes to the `source-control` plugin are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.
 
+## [0.51.9]
+
+### Fixed
+
+- **`babysit_merge` no longer drops required status contexts when more than one ruleset governs the
+  base branch.** `branch_rules` assigned `requiredContexts` inside its loop over
+  `repos/{repo}/rules/branches/{branch}`, but that endpoint returns one rule of a given type PER
+  RULESET, so each `required_status_checks` rule overwrote the previous one and only the last
+  survived. On a branch governed by two rulesets this reported one of four required contexts,
+  under-reporting `effectiveRules` and the "required checks not satisfied" blocker. The single-rule
+  assumption was correct under classic branch protection, which has exactly one such rule, and does
+  not hold under rulesets. Contexts are now unioned across all rules, deduped (two rulesets may
+  legitimately require the same context) and sorted (stable regardless of the order rulesets are
+  returned in). An entry carrying no `context` is dropped rather than surfacing as a literal `None`
+  required context. Not a merge-safety hole: the gate refuses independently on `mergeStateStatus`,
+  which GitHub computes from all required checks. Its one safety-adjacent effect ran in the
+  over-holding direction — `baseUnprotected` is true when the context list is empty, which under
+  the bug meant "the LAST status-checks rule is empty" and now means "ALL of them are", a subset —
+  so the bug produced a false hold on a superset of cases and never retired one. Latent on this
+  repository, where neither ruleset carries an empty context list.
+- **`pull_request` rules are folded across rulesets too.** Same assign-in-loop shape, same
+  function. `requiredApprovingReviews` now takes the max and `requireThreadResolution` the OR — the
+  fail-closed direction whatever GitHub's own composition rule is, since max/OR can only
+  over-report and hold a PR for a human, where last-wins can under-report and release one. This
+  one could lose a blocker outright: a trailing rule with `required_approving_review_count: 0`
+  erased an earlier ruleset's requirement and dropped the "needs N approving review(s)" hold. Not
+  observed — one such rule governs the branch today. The count fold is a behaviour change; the
+  boolean is report-only, never consumed as a blocker. The count also distinguishes an ABSENT
+  `required_approving_review_count` (the rule requires no reviews — zero) from one present but
+  unreadable (`null`, `""`, `0.0`, `[]`, `{}` — a requirement is stated and its size is unknown, so
+  it counts as one). Collapsing a falsy non-int to zero would be the single fail-open step in a
+  fold whose guarantee is that it may only ever over-report.
+
 ## [0.51.8]
 
 ### Fixed

plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py

@@ -232,7 +232,23 @@ def repository_default_branch(repo: str) -> str | None:
 
 
 def branch_rules(repo: str, branch: str) -> dict[str, object]:
-    """Summarize the effective merge-governing rules for the base branch."""
+    """Summarize the effective merge-governing rules for the base branch.
+
+    Rulesets COMPOSE: `/rules/branches/{branch}` returns one rule of a given
+    type PER RULESET governing the branch, so the single-rule assumption that
+    held under classic branch protection does not hold here. Every repeatable
+    rule is therefore folded across all rules rather than assigned from one:
+
+    * `requiredContexts` is the union, deduped and sorted -- keeping a single
+      rule's list drops every other ruleset's contexts from both
+      `effectiveRules` and the unmet-required blocker. Two rulesets may
+      legitimately require the same context, hence the dedupe; the sort makes
+      the reported set stable regardless of the order rulesets are returned in.
+    * `requiredApprovingReviews` takes the max and `requireThreadResolution`
+      the OR. That is the fail-closed direction whatever GitHub's own
+      composition rule turns out to be: max/OR can only ever over-report, which
+      holds a PR for a human, where last-wins can under-report and release one.
+    """
     summary: dict[str, object] = {
         "requiredContexts": [],
         "requiredApprovingReviews": 0,
@@ -247,6 +263,9 @@ def branch_rules(repo: str, branch: str) -> dict[str, object]:
         # Rules are advisory context; a read failure must never fail the run.
         summary["error"] = f"could not read branch rules: {exc}"
         return summary
+    required_contexts: set[str] = set()
+    required_reviews = 0
+    require_thread_resolution = False
     for rule in cast(list[Any], rules) if isinstance(rules, list) else []:
         if not isinstance(rule, dict):
             continue
@@ -257,24 +276,39 @@ def branch_rules(repo: str, branch: str) -> dict[str, object]:
             cast(dict[str, Any], raw_params) if isinstance(raw_params, dict) else {}
         )
         if rtype == "required_status_checks":
-            summary["requiredContexts"] = [
-                cast(dict[str, Any], c).get("context")
+            # A context-less entry is dropped rather than carried: it names no
+            # check to reconcile, and a None would sort-crash the union and
+            # surface downstream as a literal "None" required context.
+            required_contexts.update(
+                str(cast(dict[str, Any], c)["context"])
                 for c in params.get("required_status_checks", [])
-                if isinstance(c, dict)
-            ]
-        elif rtype == "pull_request":
-            summary["requiredApprovingReviews"] = params.get(
-                "required_approving_review_count", 0
+                if isinstance(c, dict) and cast(dict[str, Any], c).get("context")
             )
-            summary["requireThreadResolution"] = params.get(
-                "required_review_thread_resolution", False
+        elif rtype == "pull_request":
+            # Absence and unreadability are different facts. No key means the
+            # rule requires no reviews, which is 0. A key holding anything this
+            # code cannot read as a count -- null, "", 0.0, [], {} -- means a
+            # requirement IS stated and its size is unknown, so it counts as
+            # one: a falsy non-int must not collapse to 0, which would be the
+            # single fail-OPEN step in a fold that may only ever over-report.
+            if "required_approving_review_count" not in params:
+                count = 0
+            else:
+                raw = params["required_approving_review_count"]
+                count = int(raw) if isinstance(raw, int) else 1
+            required_reviews = max(required_reviews, count)
+            require_thread_resolution = require_thread_resolution or bool(
+                params.get("required_review_thread_resolution", False)
             )
         elif rtype == "required_signatures":
             summary["requireSignatures"] = True
         elif rtype == "required_linear_history":
             summary["requireLinearHistory"] = True
         elif rtype == "merge_queue":
             summary["mergeQueueRequired"] = True
+    summary["requiredContexts"] = sorted(required_contexts)
+    summary["requiredApprovingReviews"] = required_reviews
+    summary["requireThreadResolution"] = require_thread_resolution
     return summary
 
 

plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_branch_rules.py

@@ -0,0 +1,237 @@
+"""Ruleset composition in `babysit_merge.branch_rules`.
+
+`repos/{repo}/rules/branches/{branch}` returns one rule of a given type PER
+RULESET governing the branch, not one rule overall -- the shape classic branch
+protection never produced. These tests pin the fold: every ruleset's contexts
+survive, a context required by two rulesets is reported once, and a rule
+carrying an empty context list cannot erase the contexts an earlier rule
+established (which would flip `baseUnprotected` and silently drop the
+unprotected-base hold on a non-self-authored PR). `pull_request` rules compose
+the same way, folded max/OR so the summary can only ever over-report.
+
+Network is stubbed by monkeypatching `babysit_merge`'s gh seams; no real gh
+process is spawned.
+"""
+
+from __future__ import annotations
+
+import pathlib
+import sys
+import unittest
+from typing import Any
+from unittest import mock
+
+sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent))
+
+import babysit_merge as merge
+
+HEAD = "a" * 40
+PR_NUMBER = 2157
+CI_GATE = ["pr-title / pr-title", "do-not-merge / do-not-merge", "ci-status"]
+SECURITY_GATE = ["security-review / security-review"]
+
+
+def _status_checks_rule(contexts: list[str]) -> dict[str, Any]:
+    return {
+        "type": "required_status_checks",
+        "parameters": {
+            "required_status_checks": [{"context": c} for c in contexts]
+        },
+    }
+
+
+# A base whose `pull_request` rule requires zero approving reviews, as this
+# repository's does: `baseUnprotected` then hangs entirely on the context union.
+NO_REQUIRED_REVIEWS = {
+    "type": "pull_request",
+    "parameters": {"required_approving_review_count": 0},
+}
+
+
+def _rules(*rules: dict[str, Any]) -> list[dict[str, Any]]:
+    return list(rules)
+
+
+def _pr(**overrides: Any) -> dict[str, Any]:
+    pr: dict[str, Any] = {
+        "state": "OPEN",
+        "isDraft": False,
+        "mergeable": "MERGEABLE",
+        "mergeStateStatus": "CLEAN",
+        "reviewDecision": "APPROVED",
+        "headRefOid": HEAD,
+        "baseRefName": "main",
+        "author": {"login": "someone-else"},
+        "url": "https://example/pr",
+        "title": "t",
+        "labels": [],
+        "statusCheckRollup": [],
+        "closingIssuesReferences": [],
+    }
+    pr.update(overrides)
+    return pr
+
+
+class BranchRulesUnionsEveryRulesetsContexts(unittest.TestCase):
+    def _branch_rules(self, rules: list[dict[str, Any]]) -> dict[str, object]:
+        with mock.patch.object(merge, "gh_json", return_value=rules):
+            return merge.branch_rules("owner/repo", "main")
+
+    def test_contexts_from_every_ruleset_survive(self) -> None:
+        summary = self._branch_rules(
+            _rules(
+                NO_REQUIRED_REVIEWS,
+                _status_checks_rule(CI_GATE),
+                _status_checks_rule(SECURITY_GATE),
+            )
+        )
+        self.assertEqual(
+            summary["requiredContexts"], sorted(CI_GATE + SECURITY_GATE)
+        )
+
+    def test_a_context_required_by_two_rulesets_is_reported_once(self) -> None:
+        summary = self._branch_rules(
+            _rules(
+                _status_checks_rule(["ci-status", "pr-title / pr-title"]),
+                _status_checks_rule(["ci-status"]),
+            )
+        )
+        self.assertEqual(
+            summary["requiredContexts"], ["ci-status", "pr-title / pr-title"]
+        )
+
+    def test_a_context_less_entry_is_dropped(self) -> None:
+        summary = self._branch_rules(
+            _rules(
+                {
+                    "type": "required_status_checks",
+                    "parameters": {
+                        "required_status_checks": [{"integration_id": 1}]
+                    },
+                }
+            )
+        )
+        self.assertEqual(summary["requiredContexts"], [])
+
+
+class PullRequestRulesFoldFailClosed(unittest.TestCase):
+    """`pull_request` composes too; max/OR can only ever hold more, never less."""
+
+    def _branch_rules(self, rules: list[dict[str, Any]]) -> dict[str, object]:
+        with mock.patch.object(merge, "gh_json", return_value=rules):
+            return merge.branch_rules("owner/repo", "main")
+
+    def test_the_strictest_approval_count_wins(self) -> None:
+        summary = self._branch_rules(
+            _rules(
+                {
+                    "type": "pull_request",
+                    "parameters": {"required_approving_review_count": 2},
+                },
+                NO_REQUIRED_REVIEWS,
+            )
+        )
+        self.assertEqual(summary["requiredApprovingReviews"], 2)
+
+    def test_thread_resolution_required_by_any_ruleset_survives(self) -> None:
+        summary = self._branch_rules(
+            _rules(
+                {
+                    "type": "pull_request",
+                    "parameters": {"required_review_thread_resolution": True},
+                },
+                NO_REQUIRED_REVIEWS,
+            )
+        )
+        self.assertTrue(summary["requireThreadResolution"])
+
+    def test_an_unreadable_count_counts_as_one_not_zero(self) -> None:
+        """Every FALSY non-int too -- the shape that keeps re-opening this hole.
+
+        `None` is the realistic one: a ruleset payload carrying the key with a
+        null value. A truthiness test reads all of these as zero, which is the
+        fail-open this fold exists to prevent.
+        """
+        for raw in (None, "", 0.0, [], {}, "two", object()):
+            with self.subTest(raw=raw):
+                summary = self._branch_rules(
+                    _rules(
+                        {
+                            "type": "pull_request",
+                            "parameters": {
+                                "required_approving_review_count": raw
+                            },
+                        }
+                    )
+                )
+                self.assertEqual(summary["requiredApprovingReviews"], 1)
+
+    def test_an_absent_count_is_zero_not_one(self) -> None:
+        """Absence is readable: the rule states no review requirement."""
+        summary = self._branch_rules(
+            _rules({"type": "pull_request", "parameters": {}})
+        )
+        self.assertEqual(summary["requiredApprovingReviews"], 0)
+
+    def test_a_readable_count_is_reported_as_an_int(self) -> None:
+        """`bool` is an `int` subclass; it must not leak into the summary."""
+        summary = self._branch_rules(
+            _rules(
+                {
+                    "type": "pull_request",
+                    "parameters": {"required_approving_review_count": True},
+                }
+            )
+        )
+        self.assertIs(type(summary["requiredApprovingReviews"]), int)
+        self.assertEqual(summary["requiredApprovingReviews"], 1)
+
+
+class AnEmptyLaterRuleCannotUnprotectTheBase(unittest.TestCase):
+    """The union keeps `baseUnprotected` honest, and with it the merge hold.
+
+    Only the first case regresses. `test_no_context_anywhere_...` passes against
+    the unfixed code too, by design: it is the over-correction guard, pinning
+    that a genuinely context-less base still reports unprotected. Do not count
+    it among the regression tests.
+    """
+
+    def _evaluate(self, rules: list[dict[str, Any]]) -> dict[str, Any]:
+        def gh_json(args: list[str]) -> Any:
+            if args[:2] == ["pr", "view"]:
+                return _pr()
+            if args[0] == "api" and "/rules/branches/" in args[1]:
+                return rules
+            if args[0] == "api":  # repository metadata (default-branch read)
+                return {"name": "main"}
+            raise AssertionError(f"unexpected gh_json call: {args}")
+
+        with (
+            mock.patch.object(merge, "gh_json", side_effect=gh_json),
+            mock.patch.object(merge, "fetch_review_threads", return_value=[]),
+        ):
+            return merge.evaluate(
+                "owner/repo", PR_NUMBER, None, {"owner"}, frozenset(), False, False,
+            )
+
+    def test_empty_trailing_rule_leaves_the_base_protected(self) -> None:
+        result = self._evaluate(
+            _rules(
+                NO_REQUIRED_REVIEWS,
+                _status_checks_rule(["ci-status"]),
+                _status_checks_rule([]),
+            )
+        )
+        self.assertFalse(result["baseUnprotected"])
+        self.assertEqual(
+            [b for b in result["blockers"] if "unprotected" in b], []
+        )
+
+    def test_no_context_anywhere_still_reports_an_unprotected_base(self) -> None:
+        result = self._evaluate(_rules(NO_REQUIRED_REVIEWS, _status_checks_rule([])))
+        self.assertTrue(result["baseUnprotected"])
+        self.assertTrue([b for b in result["blockers"] if "unprotected" in b])
+
+
+if __name__ == "__main__":
+    unittest.main()