scripts/tests/test_pr_changed_files.py

@@ -567,7 +567,27 @@ if "-X" in args and args[args.index("-X") + 1] == "POST":
     sys.exit(0)
 
 if "/status" in url:
-    # No verdict yet for this head — the case where the job goes on to classify.
+    # Configurable. Hardcoding "no verdict yet" left the ersatztv#647 emptiness guard and the
+    # never-overwrite short-circuit unreachable: neither could be made to fire, so mutations
+    # deleting them survived the whole suite.
+    mode = os.environ.get("STUB_STATUS_MODE", "none")
+    if mode == "transport-error":
+        # Real `gh()` is `curl -sf`: an HTTP error exits 22 with EMPTY stdout.
+        sys.exit(22)
+    if mode == "garbage":
+        print("<html>502 Bad Gateway</html>")
+        sys.exit(0)
+    if mode.startswith("existing:"):
+        # DECOY contexts either side, because real heads carry several (a live head showed 7 rows,
+        # the first an unrelated CI context). Their status is deliberately `pending`, NOT `success`:
+        # a `success` decoy triggers the very same short-circuit as a real verdict, so dropping
+        # `select(.context == $c)` produced an identical outcome and the mutation survived. With
+        # `pending` decoys, mis-selecting means no short-circuit — the job posts, and the test sees it.
+        print(json.dumps({"statuses": [
+            {"context": "Build & test (.NET)", "status": "pending"},
+            {"context": "review-verdict/h10", "status": mode.split(":", 1)[1]},
+            {"context": "Functional E2E", "status": "pending"}]}))
+        sys.exit(0)
     print(json.dumps({"statuses": []}))
     sys.exit(0)
 
@@ -575,13 +595,19 @@ print("{}")
 '''
 
 
-def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy"):
+def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy",
+                  status_mode: str = "none", jq16: bool = False):
     """Execute the workflow's classify `run:` block with a stubbed enumeration script.
 
     Returns the status payload the job POSTed, or None if it posted nothing.
     """
     bindir = tmp_path / "bin"; bindir.mkdir()
     curl = bindir / "curl"; curl.write_text(WORKFLOW_STUB_CURL); curl.chmod(0o755)
+    if jq16:
+        # Reproduce the runner's jq 1.6 (`-e` over EMPTY input exits 0, not 4). The shim is the one
+        # already used for pr-changed-files.sh in this file, so its fidelity is covered by that
+        # suite's own verify-the-verifier test.
+        jq = bindir / "jq"; jq.write_text(_JQ16_SHIM); jq.chmod(0o755)
     scripts = tmp_path / "scripts"; scripts.mkdir()
     if enum_stub is not None:
         enum = scripts / "pr-changed-files.sh"
@@ -591,6 +617,7 @@ def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy"):
     env = dict(os.environ)
     env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}"
     env["STUB_DIR"] = str(tmp_path)
+    env["STUB_STATUS_MODE"] = status_mode
     env.update({
         "GITEA_TOKEN": "stub",
         "BASE_URL": "http://gitea.example/api/v1",
@@ -708,3 +735,124 @@ def test_bot_positive_control_a_plain_bot_pr_IS_exempt(tmp_path):
     assert posted is not None
     assert posted["state"] == "success", (
         "the bot exemption never fires at all, so the protected-path test above proves nothing")
+
+
+# --- The STATUS READ: the ersatztv#647 guard and the never-overwrite short-circuit -------------
+#
+# These were unreachable until the stub's status response became configurable. Four mutations
+# survived the full suite without them, including re-introducing the literal ersatztv#647 fail-open.
+
+def test_a_transport_failure_on_the_STATUS_READ_posts_NOTHING(tmp_path):
+    """`gh()` is `curl -sf`, so an HTTP error yields exit 22 and EMPTY stdout. Reading that as "no
+    verdict exists" would let the job post over a real human verdict. It must fail WITHOUT posting.
+    """
+    posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="transport-error")
+    assert r.returncode != 0, "an unreadable status read must fail the job"
+    assert posted is None, "nothing may be posted when the existing verdict state is unknown"
+
+
+def test_a_GARBAGE_status_response_posts_NOTHING(tmp_path):
+    """A proxy error page is a 200 with a non-JSON body — not an absent verdict."""
+    posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="garbage")
+    assert r.returncode != 0
+    assert posted is None
+
+
+# Two mutations to this read are NOT covered, and both are behaviourally equivalent rather than gaps:
+#   * `first` -> `last`: `select(.context == $c)` yields exactly ONE row, because the job reads the
+#     COMBINED status endpoint, which returns latest-per-context by contract. A fixture with two
+#     `review-verdict/h10` rows would be testing something the API does not produce.
+#   * `test_a_GARBAGE_status_response_posts_NOTHING` does not, on its own, defend the type guard:
+#     with the guard gone, the following `jq -r` fails on non-JSON and `set -e` kills the job anyway.
+#     That is fail-closed by REDUNDANCY. The guard's own behaviour is pinned by the jq-1.6 test below,
+#     which is where it actually matters.
+
+@pytest.mark.parametrize("existing", ["success", "failure"])
+@pytest.mark.parametrize("paths,author", [
+    (("ErsatzTV/Program.cs",), "timothy"),        # non-exempt: only the short-circuit can stop it
+    (("docs/a.md",), "timothy"),                  # docs-only EXEMPT
+    (("Directory.Packages.props",), "renovate"),  # bot EXEMPT
+])
+def test_an_existing_verdict_on_this_head_is_NEVER_overwritten(tmp_path, existing, paths, author):
+    """A human verdict for this exact head may already exist — the reviewer ran
+    post-review-verdict.sh before this job finished, or the job re-ran. Re-posting would un-approve
+    a reviewed head, or (worse) approve one a human marked BLOCKED.
+
+    `failure` is the sharp case: that is a human saying NO, and an exemption posted over it would
+    turn a rejection into a merge.
+    """
+    posted, r = _run_classify(tmp_path, _emitting(*paths), author=author,
+                              status_mode=f"existing:{existing}")
+    assert r.returncode == 0, r.stderr
+    assert posted is None, (
+        f"overwrote an existing '{existing}' verdict on this head with {paths} as {author}")
+
+
+# --- The `count -eq 0` guard, on the path where it is the ONLY guard ---------------------------
+
+def test_an_EMPTY_enumeration_is_not_exempt_even_for_a_BOT(tmp_path):
+    """The author must be a BOT for this to test anything.
+
+    With a non-bot author the blank line an empty list produces already fails `DOCS_ONLY`, so the
+    `[ "${count:-0}" -eq 0 ]` guard never decides the outcome — the same short-circuit that made an
+    earlier `PROTECTED` test vacuous. On the bot path that guard is the ONLY thing between an
+    unreadable-but-successful enumeration and an unattended `success`.
+
+    Verified by mutation: changing `grep -c .` to `grep -c ''` (counting the blank line, so
+    `count=1`) grants a bot PR `success` here while every other test stays green.
+    """
+    posted, r = _run_classify(tmp_path, "#!/usr/bin/env bash\nexit 0\n", author="renovate")
+    assert posted is not None, f"the job posted no status at all: {r.stderr}"
+    assert posted["state"] == "pending", (
+        "an empty file list was treated as a bot exemption — the enumeration returning nothing is "
+        "not evidence that nothing was changed")
+
+
+# --- Anchors in the classifier predicates ------------------------------------------------------
+
+def test_the_BOT_match_is_whole_line_not_substring(tmp_path):
+    """`grep -qxF` is anchored; plain `grep -qF` would exempt any author whose name CONTAINS a bot
+    name. `ova` is a substring of `renovate`."""
+    posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), author="ova")
+    assert posted is not None
+    assert posted["state"] == "pending", "a substring of a bot name was granted the bot exemption"
+
+
+def test_DOCS_ONLY_anchors_the_markdown_extension(tmp_path):
+    r"""`[^/]*\.md$` must match only a top-level file ENDING in .md. Losing the `$` exempts
+    `evil.mdx`, and the docs-only exemption posts a green status with nobody in the loop."""
+    posted, _ = _run_classify(tmp_path, _emitting("evil.mdx"))
+    assert posted is not None
+    assert posted["state"] == "pending", "a .mdx file was accepted as docs-only"
+
+
+def test_a_transport_failure_under_jq_1_6_STILL_posts_nothing(tmp_path):
+    """The ersatztv#647 fail-open, tested where it actually lives: jq 1.6.
+
+    The shell emptiness check exists because `jq -e` over EMPTY input exits 4 on jq >= 1.7 but **0 on
+    jq 1.6, which is what the runner ships**. Remove that check and, on a dev Mac's 1.8, the guard
+    still fires and every test stays green — the bug is invisible locally.
+
+    An earlier version of this pinned the construct STRUCTURALLY instead, on the stated grounds that
+    "no behavioural test can catch this on a dev machine". That was wrong: this file already imports
+    `_JQ16_SHIM` for `pr-changed-files.sh`, so the runner's quirk is reproducible here. The structural
+    version was also weaker than it looked — it stripped only FULL-LINE comments, so leaving the
+    literal as a trailing comment on the surviving `if` satisfied it while the real guard was gone.
+
+    This test is strictly stronger: it catches that mutant, needs no comment-stripping, and fails for
+    the right reason. Verified by mutation under both jq versions.
+    """
+    posted, r = _run_classify(tmp_path, _emitting("docs/a.md"),
+                              status_mode="transport-error", jq16=True)
+    assert r.returncode != 0, (
+        "under jq 1.6 an unreadable status read must still fail the job — this is the exact "
+        "ersatztv#647 fail-open")
+    assert posted is None, "nothing may be posted when the existing verdict state is unknown"
+
+
+def test_DOCS_ONLY_anchors_the_START_of_the_path_too(tmp_path):
+    r"""`^(docs/|...)` must match only at the start. Losing the `^` exempts `ErsatzTV/docs/Evil.cs`,
+    which is a C# file — fail-OPEN, and the sibling of the `$` case above."""
+    posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/docs/Evil.cs"))
+    assert posted is not None
+    assert posted["state"] == "pending", "a path merely CONTAINING docs/ was accepted as docs-only"