fix(778): pin the arm order, and correct a wrong explanation I had just written

Two self-found defects while pre-empting the confirmation round's own questions. THE REORDER WAS UNPINNED. Swapping the classifier arms back to exact-first left all 29 tests green, so the previous commit's central change was invisible to the suite — an unproven change shipping under a green run. The missing fixture is the one that distinguishes the orders: a list holding BOTH an exactly-named rule that requires `review-verdict/h10` AND a glob rule that could also govern the base and does not. Exact-first inspects the rule that requires h10 and auto-grants; undecidable-first asks. Now mutation-proven in both directions. I HAD MISDIAGNOSED THE TRAP, and asserted the wrong cause in a comment. A three-line repro disproves "an EXIT trap suppresses output" — it does not. The real mechanism is that this file already owns its EXIT trap: `scripts/hook-fire-log.sh` installs `trap 'etv_hook_fire_end "$?"' EXIT` (#776), and in capture mode that handler is what REPLAYS the decision JSON to stdout. A second `trap ... EXIT` silently replaces it, so the decision is captured and never emitted. The trap slot is a single shared resource and the sourced library claimed it first. That second one is the shape this whole PR is about, committed against my own work: an explanation that fits the symptom, is written down as fact, and is wrong. It would have told the next person the construct is unusable rather than that the slot is taken. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-19 19:54:47 +02:00
parent d8ff6fa229
commit 356cddbb59
2 changed files with 45 additions and 8 deletions
@@ -408,13 +408,19 @@ else
  # would read as "unprotected" and turn into a hard DENY whose reason names a specific, false cause.
  bp_ref=$(printf '%s' "$base_ref" | jq -sRr @uri)
  bp_file=$(mktemp) || decide ask "H6/H10 merge gate: could not allocate a temp file to read branch protection for '$base_ref'. Confirm the 'review-verdict/h10' required check manually before scheduling an auto-merge."
-  # Cleanup is hand-placed on each exit path rather than a `trap ... EXIT`. That was tried and
-  # REVERTED: this hook is driven through a `decide` helper that prints its JSON and exits, and
-  # installing an EXIT trap here suppressed the decision output entirely — 20 tests went from green
-  # to "the hook passed through instead of auto-granting". A gate that prints nothing is the one
-  # outcome it must never produce, so the tidier construct loses to the one that demonstrably works.
-  # Every path between here and the reads below removes the file exactly once; cold review traced
-  # them independently.
+  # Cleanup is hand-placed on each exit path, and a `trap 'rm -f' EXIT` MUST NOT be added here.
+  #
+  # It was tried, and it took 20 tests from green to "the hook passed through instead of
+  # auto-granting". The first explanation written here — that an EXIT trap suppresses output — was
+  # WRONG, and a three-line repro disproves it. The real cause is that THIS FILE ALREADY OWNS ITS
+  # EXIT TRAP: `scripts/hook-fire-log.sh` installs `trap 'etv_hook_fire_end "$?"' EXIT` (ersatztv#776),
+  # and in `capture` mode that handler is what REPLAYS the hook's decision JSON to stdout. A second
+  # `trap ... EXIT` silently replaces it, so the decision is captured and then never emitted — a
+  # consent gate printing nothing at all, which is the one outcome it must never produce.
+  #
+  # The trap slot is a single shared resource, and the sourced library claimed it first. Every path
+  # between here and the reads below removes the file exactly once; cold review traced them
+  # independently.
  bp_get() { # $1 = path under the repo
    if [ -n "${ETV_GITEA_TOKEN:-}" ]; then
      curl -s -o "$bp_file" -w '%{http_code}' -H "Authorization: token $ETV_GITEA_TOKEN" "$base_url/repos/$owner/$repo/$1" 2>/dev/null || true
@@ -70,6 +70,15 @@ if url.rstrip("/").endswith("/branch_protections"):
    # The LIST endpoint, consulted only after a 404 on the by-name lookup. Gitea keys the by-name
    # endpoint on the RULE name, so a base covered by a glob rule 404s there while being protected.
    mode = (state / "bp").read_text().strip()
+    if mode == "EXACT-PLUS-GLOB-RULE":
+        # BOTH an exactly-named rule that requires h10 AND a glob rule that could also govern this
+        # base and does NOT. Gitea sorts by Priority before plain-name-ness, so the glob rule can be
+        # the one actually applied — which means inspecting the exact rule would auto-grant on a base
+        # where h10 is not enforced. Undecidable must win.
+        respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
+                             "status_check_contexts": ["review-verdict/h10"]},
+                            {"branch_name": "m*", "enable_status_check": True,
+                             "status_check_contexts": ["Build ErsatzTV Image / Build & test (.NET)"]}]))
    if mode == "ESCAPED-META-RULE":
        # gobwas/glob reads `\\{` as a LITERAL brace, so this rule governs the base `a{b`. If the
        # superset treats `\\` as literal it builds `a\\.*b`, misses, and answers `none` — denying a
@@ -119,7 +128,7 @@ if "/branch_protections/" in url:
        respond("", "000")
    if mode in ("NOT-FOUND", "GLOB-RULE", "LIST-UNREADABLE", "REGEX-META-RULE",
                "GLOB-WITH-DOT-RULE", "CHARCLASS-RULE", "CASEFOLD-RULE",
-                "ESCAPED-META-RULE"):
+                "ESCAPED-META-RULE", "EXACT-PLUS-GLOB-RULE"):
        respond('{"message":"not found"}', "404")
    if mode == "FORBIDDEN":
        respond('{"message":"token does not have at least one of required scope(s)"}', "403")
@@ -700,3 +709,25 @@ def test_a_BACKSLASH_ESCAPED_metacharacter_in_a_rule_name_is_undecidable_not_abs
        "a rule whose escaped brace governs this base was reported as unable to govern it")
    assert "ask" in reason, (
        "an escaped-metacharacter rule is undecidable here and must ask")
+
+
+def test_a_GLOB_rule_that_could_outrank_an_exact_one_wins_and_ASKS(hook):
+    """The arm ORDER, pinned. Without this the reorder is invisible to the suite — swapping the arms
+    back left all 29 tests green, which is how an unproven change ships.
+
+    Gitea picks the governing rule with `GetFirstMatched` over a list sorted by Priority and THEN by
+    plain-name-ness, so a glob rule can outrank an exactly-named one. Here `main` requires
+    `review-verdict/h10` and `m*` does not. Evaluating `exact` first inspects the rule that requires
+    h10, concludes the base is protected, and AUTO-GRANTS a scheduled merge onto a base where the
+    check may not be enforced at all — #622's hole, reached through the block written to close it.
+
+    Evaluating `undecidable` first is sound without knowing Gitea's precedence rules, which is the
+    only claim this code is entitled to make about somebody else's resolver.
+    """
+    hook.set_branch_protection("EXACT-PLUS-GLOB-RULE")
+    reason = hook.reason()
+    assert "ask" in reason, (
+        "an exact rule was trusted while a glob rule could outrank it — the gate granted on a base "
+        "whose enforced rule it never identified")
+    assert "could govern it" in reason, (
+        "the ask must come from the undecidable-glob arm, not from the classifier failing")