CI: an advisory PR-shape job — #510 leg 1's other half

.github/workflows/tools.yml

@@ -204,3 +204,45 @@ jobs:
             echo "   or the marker moved. Both print this line; neither is a pass."
           fi
           echo "⛔ This job is continue-on-error. Its green is NOT evidence."
+
+  # ⛔ ADVISORY, AND DELIBERATELY NOT `continue-on-error`. The sibling job above uses it,
+  # and for that job it is right — its failures mean "CI has no fleet". Here a finding
+  # means a PR would REMOVE content it did not add, and `continue-on-error` renders that
+  # as a warning annotation: a detector firing into a stream nobody classifies (#397).
+  #
+  # ⇒ So this job goes RED on a finding and is NOT in the required-contexts list, which
+  # is the whole of "advisory": visible, and blocking nothing. ⛔ Adding it to that list
+  # is branch protection and operator-only (docs/MERGE-AUTHORITY.md rule 2). Nothing here
+  # touches it.
+  #
+  # ⚠ What this CANNOT check: who may merge. A runner has no holder session and cannot
+  # have one — the holder is a running pane. `--shape-only` omits leg 0 and SAYS SO on
+  # every run; it establishes nothing about authority. That half stays merge-time.
+  pr-shape:
+    name: PR shape (advisory, NOT gating)
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: read
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0        # ⛔ three-dot needs history; a shallow clone reads as empty
+      - uses: actions/setup-python@v5
+        with:
+          python-version: '3.12'
+
+      # ⇒ #510 leg 1: "a check exists — CI STEP or merge-time — that refuses a PR whose
+      # diff against main removes content the PR did not add." merge-time landed first;
+      # this is the other half, so the #572 class is caught when the PR opens rather than
+      # when someone happens to run the guard by hand.
+      - name: revert-shape and PR hygiene
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          git fetch origin main --quiet || true
+          # ⚠ POSITIONAL. merge-guard takes `prs ...`; late-push takes `--pr N`. The
+          # first version of this step wrote `--pr` here and CI refused it with exit 2
+          # — the flag-strictness working, on the author of the flag-strictness.
+          python3 tools/merge-guard.py --shape-only ${{ github.event.pull_request.number }}

tools/merge-guard.py

@@ -120,28 +120,55 @@ def pr_json(n, fields):
         raise Unestablished(f"gh --json {fields} was not parseable: {exc}")
 
 
-def evaluate(n, session, authority_text):
+def evaluate(n, session, authority_text, shape_only=False):
     legs = []
 
     def leg(name, ok, detail):
         legs.append((name, ok, detail))
 
-    ok, detail = holder_check(authority_text, session)
-    leg("0 holder == session", ok, detail)
+    # ⛔ --shape-only OMITS leg 0 AND SAYS SO. It exists for CI, which has no holder
+    # session and cannot have one: the holder is a running pane, not a runner. A flag
+    # that made leg 0 PASS without a session would be a hole shaped exactly like the
+    # thing this tool guards, so it is omitted and named rather than defaulted true.
+    # ⇒ Shape-only ESTABLISHES NOTHING ABOUT AUTHORITY. It answers #510 leg 1's other
+    # half — "a check exists, CI step OR merge-time" — and only that half.
+    if shape_only:
+        leg("0 holder == session", True, "⚠ SKIPPED — --shape-only. This run establishes "
+                                         "NOTHING about who may merge.")
+    else:
+        ok, detail = holder_check(authority_text, session)
+        leg("0 holder == session", ok, detail)
 
     d = pr_json(n, "baseRefName,mergeStateStatus,reviews,headRefOid,createdAt,state,statusCheckRollup")
     if d.get("state") != "OPEN":
         raise Unestablished(f"PR #{n} is {d.get('state')}, not OPEN")
 
     leg("1 base == main", d["baseRefName"] == "main", d["baseRefName"])
 
-    rollup = d.get("statusCheckRollup") or []
-    req = [c for c in rollup if (c.get("name") or c.get("context") or "") == "hermetic suites (gating)"]
-    if not req:
-        leg("2 required gate", False, "UNESTABLISHED — 'hermetic suites (gating)' absent from rollup")
+    # ⛔ --shape-only OMITS LEG 2 TOO, and for a reason measured on this tool's own PR.
+    # A CI job asking "is the required gate green?" from INSIDE the run that CONTAINS
+    # that gate is asking a self-referential question. Measured on PR #597, both jobs
+    # in the same run:
+    #     hermetic suites (gating)     started 23:22:19  completed 23:23:29  success
+    #     PR shape (advisory)          started 23:22:19  completed 23:22:25  FAILURE
+    # ⇒ pr-shape read the gate SIX SECONDS in, 64s before the gate finished. The gate
+    # was necessarily unfinished, so leg 2 was necessarily unestablished, and the job
+    # went red for a fact about ITS OWN CONCURRENCY rather than about the PR.
+    # ⚠ `needs:` would serialise it, but that is the wrong fix: it makes an ADVISORY job
+    # a prerequisite of nothing while doubling the run's latency, and it still leaves the
+    # runner asserting a green gate that the merger must re-read at merge time anyway.
+    # ⇒ The honest move is the same one leg 0 already makes: SKIP AND SAY SO.
+    if shape_only:
+        leg("2 required gate", True, "⚠ SKIPPED — --shape-only. A run cannot establish "
+                                     "the outcome of a gate it CONTAINS. Re-read at merge.")
     else:
-        concl = req[0].get("conclusion") or req[0].get("state") or ""
-        leg("2 required gate", concl == "SUCCESS", concl or "UNESTABLISHED")
+        rollup = d.get("statusCheckRollup") or []
+        req = [c for c in rollup if (c.get("name") or c.get("context") or "") == "hermetic suites (gating)"]
+        if not req:
+            leg("2 required gate", False, "UNESTABLISHED — 'hermetic suites (gating)' absent from rollup")
+        else:
+            concl = req[0].get("conclusion") or req[0].get("state") or ""
+            leg("2 required gate", concl == "SUCCESS", concl or "UNESTABLISHED")
 
     revs = d.get("reviews") or []
     changes = [r for r in revs if r.get("state") == "CHANGES_REQUESTED"]
@@ -236,6 +263,9 @@ def main():
     ap.add_argument("--session", default=os.environ.get("CLAUDE_CODE_SESSION_ID", ""),
                     help="session id to test as (default: $CLAUDE_CODE_SESSION_ID)")
     ap.add_argument("--authority", default=AUTHORITY, help=f"path to {AUTHORITY}")
+    ap.add_argument("--shape-only", action="store_true",
+                    help="omit the holder leg — for CI, which has no holder session. "
+                         "⛔ Establishes nothing about authority.")
     ap.add_argument("--self-test", action="store_true", help="run the controls; no network")
     args = ap.parse_args()
 
@@ -248,6 +278,10 @@ def main():
     # condition's own command could not be satisfied by the instrument written for it.
     # ⇒ A holder check needs no PR: "may this session merge at all?" is answerable, and
     # is exactly the question those four issues pose.
+    if not args.prs and args.shape_only:
+        print("⛔ VOID — --shape-only needs a PR: without one there is no shape to check, "
+              "and it is not a holder check.", file=sys.stderr)
+        return 2
     if not args.prs:
         try:
             text = Path(args.authority).read_text(encoding="utf-8")
@@ -260,19 +294,32 @@ def main():
                         if ok else "REFUSED — this session is not the holder"))
         return 0 if ok else 1
 
-    try:
-        text = Path(args.authority).read_text(encoding="utf-8")
-    except OSError as exc:
-        print(f"⛔ VOID — cannot read {args.authority}: {exc}\n"
-              f"   ADDABLE — run from a checkout that has it, or pass --authority.",
-              file=sys.stderr)
-        return 2
+    # ⛔ --shape-only MUST NOT READ THE AUTHORITY FILE. It skips leg 0, so `text` is
+    # never used — but reading it made a MISSING file fatal, and CI checks out the PR's
+    # own tree. ⇒ A PR that MOVED OR DELETED docs/MERGE-AUTHORITY.md turned pr-shape red
+    # with exit 2 for a fact about the authority record, not about the PR's shape. Same
+    # defect as leg 2 read from inside its own run: the advisory job going red for the
+    # wrong reason, which is how an advisory job stops being read at all.
+    # ⇒ Found by CodeRabbit in review of this PR, and confirmed here by measurement
+    #   before it was believed:
+    #     --shape-only 597 --authority /nonexistent/AUTH.md   exit 2
+    #     --shape-only 597                                    exit 0
+    if args.shape_only:
+        text = ""
+    else:
+        try:
+            text = Path(args.authority).read_text(encoding="utf-8")
+        except OSError as exc:
+            print(f"⛔ VOID — cannot read {args.authority}: {exc}\n"
+                  f"   ADDABLE — run from a checkout that has it, or pass --authority.",
+                  file=sys.stderr)
+            return 2
 
     worst = 0
     for arg in args.prs:
         print(f"══ PR #{arg} ══")
         try:
-            legs = evaluate(int(arg), args.session, text)
+            legs = evaluate(int(arg), args.session, text, args.shape_only)
         except (Unestablished, ValueError) as exc:
             print(f"  ⛔ UNESTABLISHED — {exc}")
             print("  ⇒ BLOCK. A leg that cannot be measured is not a leg that passed.\n")

tools/test_merge_guard.py

@@ -212,6 +212,99 @@ def test_no_pr_named_is_the_HOLDER_CHECK_not_a_refusal(self):
             sys.argv = old
         self.assertEqual(rc_void, 2, "an unreadable authority file is still VOID")
 
+    # ── --shape-only, for CI ──
+
+    def test_shape_only_SKIPS_leg0_and_says_so(self):
+        """⛔ It must not read as authorization. A runner has no holder session and
+        cannot have one, so leg 0 is OMITTED and NAMED — never quietly passed."""
+        rc, out, _ = drive(self.mod, prd(), ["1", "--shape-only"], session=OTHER)
+        self.assertEqual(rc, 0, "the shape legs pass; the holder leg is not evaluated")
+        self.assertIn("SKIPPED", out)
+        self.assertIn("establishes\nNOTHING about who may merge".replace("\n", " "), out)
+
+    def test_shape_only_does_NOT_need_the_authority_file(self):
+        """⛔ Found by CodeRabbit reviewing this PR, confirmed by measurement first.
+
+        --shape-only skips leg 0, so the authority text is never used — but main() read
+        the file anyway and returned 2 when it was missing. CI checks out the PR's OWN
+        tree, so a PR that MOVED docs/MERGE-AUTHORITY.md turned pr-shape red for a fact
+        about the authority record rather than about the PR's shape.
+
+        ⇒ The shape legs must still be evaluated with no authority file at all."""
+        self.mod.pr_json = lambda n, f: prd()
+        self.mod.sh = lambda a, allow_fail=False: ("1\t900\ttools/README.md"
+                                                  if a[:2] == ["git", "diff"] else "")
+        old = sys.argv
+        sys.argv = ["merge-guard.py", "--shape-only", "1",
+                    "--session", OTHER, "--authority", "/nonexistent/AUTH.md"]
+        out = io.StringIO()
+        try:
+            with redirect_stdout(out), redirect_stderr(io.StringIO()):
+                rc = self.mod.main()
+        finally:
+            sys.argv = old
+        self.assertNotEqual(rc, 2, "a missing authority file must not VOID a shape check")
+        # ★ and it must have REACHED the shape legs, not merely exited non-2
+        self.assertIn("net-negative in 1 file(s)", out.getvalue())
+        self.assertEqual(rc, 1, "it blocks on the REVERT, which is what it is for")
+
+    def test_a_missing_authority_STILL_voids_a_real_merge(self):
+        """★ THE KNOWN-NEGATIVE. The exemption above is scoped to --shape-only ONLY.
+        Without it, an unreadable authority record still establishes nothing about who
+        may merge, and must still be VOID — otherwise the fix is a bypass."""
+        old = sys.argv
+        sys.argv = ["merge-guard.py", "1", "--session", HOLDER,
+                    "--authority", "/nonexistent/AUTH.md"]
+        try:
+            with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()):
+                rc = self.mod.main()
+        finally:
+            sys.argv = old
+        self.assertEqual(rc, 2, "no authority record ⇒ VOID, for a real merge")
+
+    def test_shape_only_SKIPS_leg2_because_the_gate_is_IN_ITS_OWN_RUN(self):
+        """⛔ THE KNOWN-POSITIVE, and it is this tool's own PR #597.
+
+        A pr-shape job and the gating job start in the SAME run at the same instant.
+        pr-shape finished at 23:22:25; the gate finished at 23:23:29. So pr-shape read
+        a gate with `conclusion: null` and went red for a fact about concurrency, not
+        about the PR. Modelled here EXACTLY as GitHub reports it: an in-progress check
+        run carries conclusion null, not a falsy string."""
+        d = prd()
+        d["statusCheckRollup"] = [{"name": "hermetic suites (gating)",
+                                   "conclusion": None, "status": "IN_PROGRESS"}]
+        rc, out, _ = drive(self.mod, d, ["1", "--shape-only"], session=OTHER)
+        self.assertEqual(rc, 0, "an unfinished gate must not fail an ADVISORY shape check")
+        self.assertIn("a gate it CONTAINS", out)
+
+    def test_the_SAME_pending_gate_still_BLOCKS_a_real_merge(self):
+        """★ THE KNOWN-NEGATIVE, without which the fix above is just a hole.
+
+        Identical input, `--shape-only` removed. The skip must be scoped to the advisory
+        path ONLY: at merge time a pending gate is not a green one, and leg 2 must still
+        refuse. Same data, opposite verdict — that is what makes the pair evidence."""
+        d = prd()
+        d["statusCheckRollup"] = [{"name": "hermetic suites (gating)",
+                                   "conclusion": None, "status": "IN_PROGRESS"}]
+        rc, out, _ = drive(self.mod, d, ["1"])          # HOLDER session, no --shape-only
+        self.assertEqual(rc, 1, "a pending gate BLOCKS a merge")
+        self.assertIn("2 required gate", out)
+        self.assertNotIn("a gate it CONTAINS", out)
+
+    def test_shape_only_still_catches_a_revert(self):
+        """The whole point: #572's class caught at PR time, not merge time."""
+        rc, out, _ = drive(self.mod, prd(), ["1", "--shape-only"], session=OTHER,
+                           numstat="1\t900\ttools/README.md")
+        self.assertEqual(rc, 1)
+        self.assertIn("net-negative in 1 file(s)", out)
+
+    def test_shape_only_without_a_pr_is_VOID_not_a_holder_check(self):
+        """⛔ The bypass that must not exist: --shape-only with no PR could otherwise
+        read as 'this session may merge'. It refuses."""
+        rc, _, err = drive(self.mod, prd(), ["--shape-only"], session=OTHER)
+        self.assertEqual(rc, 2)
+        self.assertIn("not a holder check", err)
+
     # ── ⛔ criterion 4: shown to FAIL ──
 
     def test_control_can_fail(self):