Files
ersatztv/scripts/tests/test_pr_changed_files.py
T
timothyandClaude Opus 5 11287a54ba
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 10s
review-verdict/h10 Review-verdict: MERGEABLE @ 11287a5 (base: main)
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 19s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 19s
PR Gates / Docs update reminder (pull_request) Successful in 16s
Review verdict / Set review-verdict status (pull_request_target) Successful in 20s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 17s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 6m51s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m24s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m5s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m6s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 7s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 5s
fix(763): satisfy ruff — E741 and formatting on the touched test file
CI's `Script lint and tests` job went red. Cause: I never ran ruff locally,
which this repo's Python convention requires after any .py change.

  - E741 twice: `l` as a comprehension variable in the sort-order guard.
  - `ruff format --check`: the file was correctly formatted on `main`; my edits
    broke it. One of them left a docstring line at column 0, which `ruff format`
    then "corrected" by over-indenting the rest of the paragraph — repaired at
    the source rather than accepting that rewrite.

Verified the way CI does: local ruff is the pinned 0.12.11, and both
`ruff check` and `ruff format --check` run under bash over the full tracked
population (`git ls-files -z '*.py' '*.pyi' '*.ipynb'`, 46 files) are clean.
The population is counted, not assumed — an empty glob would pass vacuously,
which is the failure `scripts/tests` guards against elsewhere.

`scripts/tests` 1097 passed, 2 skipped after the reformat.

refs #763

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-28 18:51:32 +02:00

4347 lines
238 KiB
Python

"""Tests for `scripts/pr-changed-files.sh` — the SHARED PR file enumeration (ersatztv#649).
Why this file exists. The enumeration used to be written twice: once in
`.claude/hooks/pretooluse-merge-consent.sh` (ADVISORY — a failure produces a human prompt) and once
inline in `.gitea/workflows/review-verdict.yml` (ENFORCED — it writes the branch-protection-required
`review-verdict/h10` status). They drifted, and in the dangerous direction: four rounds of
ersatztv#643 hardening landed on the advisory copy and never reached the enforced one, so the copy
with real authority ended up strictly weaker than the copy without.
The specific thing this suite pins is the point of ersatztv#649's second Done-when box. A round-4
review traced that the enforced copy's fail-closed behaviour on a garbage response was INCIDENTAL,
not designed: `n` came back empty, `[ "$n" -lt 50 ]` errored to false, the loop ran to MAX_PAGES and
left complete=no. The right answer, reached through a bash arithmetic error that any refactor of the
loop could have silently flipped. Every failure-path test below therefore asserts a NON-ZERO exit
explicitly, so the behaviour is a contract rather than a coincidence.
Observable contract of the script:
exit 0 -> enumeration complete and bound to the expected head; stdout is the authoritative path set
exit 1 -> could not enumerate/verify; stdout meaningless, caller MUST withhold any exemption
exit 2 -> usage error
"""
from __future__ import annotations
import json
import os
import re
import subprocess
from pathlib import Path
import pytest
from scripts.tests.tracked_files import tracked_paths
REPO_ROOT = Path(__file__).resolve().parents[2]
SCRIPT = REPO_ROOT / "scripts" / "pr-changed-files.sh"
HOOK = REPO_ROOT / ".claude" / "hooks" / "pretooluse-merge-consent.sh"
WORKFLOW = REPO_ROOT / ".gitea" / "workflows" / "review-verdict.yml"
SHA = "a9e3e23abf337980ca4c05854f5b1e210099d08b"
OTHER_SHA = "b71c0d4e2f8a91b3c5d7e9f1a3b5c7d9e1f3a5b7"
# Serves paged `pulls/N/files`, plus the PR object the script re-reads to bind the enumeration.
CURL_SHIM = r"""#!/usr/bin/env python3
import json, os, sys, pathlib, urllib.parse
state = pathlib.Path(os.environ["STUB_DIR"])
args = sys.argv[1:]
url = [a for a in args if a.startswith("http")][-1]
if "/pulls/" in url and "/files" in url:
q = urllib.parse.parse_qs(urllib.parse.urlparse(url).query)
page = int(q.get("page", ["1"])[0])
pages = json.loads((state / "pages.json").read_text())
if page > len(pages):
print("[]"); sys.exit(0)
entry = pages[page - 1]
if entry == "ERROR": # transport failure on this page
sys.exit(22)
if entry == "GARBAGE": # 200 with a non-array body (proxy/error page)
print('{"message":"internal error"}'); sys.exit(0)
print(json.dumps(entry)); sys.exit(0)
if "/pulls/" in url:
# The PR object is read TWICE now: once before paging to bind the base, once after to bind both
# the head and the base (ersatztv#698). A counter distinguishes them, so a test can move either
# field at either end and the two guards can be isolated from each other.
counter = state / "pr_reads.txt"
n = int(counter.read_text()) if counter.exists() else 0
counter.write_text(str(n + 1))
sha = os.environ["STUB_SHA"]
alt = state / "pr_sha_after.txt"
if alt.exists() and n >= 1: # force-push landing between pagination round-trips
sha = alt.read_text().strip()
base = os.environ.get("STUB_BASE", "main")
base_before = state / "pr_base_before.txt"
base_after = state / "pr_base_after.txt"
if base_before.exists() and n == 0: # already retargeted when the job started
base = base_before.read_text().strip()
if base_after.exists() and n >= 1: # retargeted mid-enumeration
base = base_after.read_text().strip()
# The base's TIP sha (ersatztv#707) — independent of the branch NAME above. Defaults to a fixed
# value so an unmoved base is the default for every existing test that never touches this.
base_sha = os.environ.get("STUB_BASE_SHA", "aaaa000000000000000000000000000000000a")
base_sha_before = state / "pr_base_sha_before.txt"
base_sha_after = state / "pr_base_sha_after.txt"
if base_sha_before.exists() and n == 0:
base_sha = base_sha_before.read_text().strip()
if base_sha_after.exists() and n >= 1:
base_sha = base_sha_after.read_text().strip()
base_obj = {"ref": base}
if base_sha != "MISSING":
base_obj["sha"] = base_sha
print(json.dumps({"head": {"sha": sha}, "base": base_obj}))
sys.exit(0)
print("{}")
"""
def _rows(paths, status="modified"):
return [{"filename": p, "status": status} for p in paths]
@pytest.fixture
def enumerate_files(tmp_path):
bindir = tmp_path / "bin"
bindir.mkdir()
shim = bindir / "curl"
shim.write_text(CURL_SHIM)
shim.chmod(0o755)
state = tmp_path / "state"
state.mkdir()
env = dict(os.environ)
env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}"
env["STUB_DIR"] = str(state)
env["STUB_SHA"] = SHA
env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment
env["ETV_GITEA_URL"] = "http://gitea.example"
env.pop("ETV_GITEA_BASICAUTH", None)
env.pop("GITEA_TOKEN", None)
class Handle:
def __init__(self, env_, state_):
self.env = env_
self.state = state_
def set_pages(self, *pages):
(state / "pages.json").write_text(json.dumps(list(pages)))
def head_moves_to(self, sha):
(state / "pr_sha_after.txt").write_text(sha)
def base_is_already(self, ref):
"""The PR targets `ref` before the first page is requested (retargeted pre-run)."""
(state / "pr_base_before.txt").write_text(ref)
def base_moves_to(self, ref):
"""The PR is retargeted to `ref` between the files pages and the binding re-read."""
(state / "pr_base_after.txt").write_text(ref)
def base_sha_is_already(self, sha):
"""The base tip sits at `sha` for the WHOLE run — i.e. it advanced before this
enumeration started, which is outside the window the #707 guard polices."""
(state / "pr_base_sha_before.txt").write_text(sha)
(state / "pr_base_sha_after.txt").write_text(sha)
def base_sha_moves_to(self, sha):
"""The base branch's TIP advances to `sha` between the files pages and the binding
re-read, with the branch NAME unchanged (ersatztv#707)."""
(state / "pr_base_sha_after.txt").write_text(sha)
def base_sha_missing_after(self):
"""The base tip sha is absent/unparseable on the post-paging re-read."""
(state / "pr_base_sha_after.txt").write_text("MISSING")
def run(self, expected_sha=SHA, args=("timothy", "ersatztv", "42"), expected_base="main"):
argv = ["bash", str(SCRIPT), *args, expected_sha]
if expected_base is not None:
argv.append(expected_base)
return subprocess.run(argv, env=self.env, capture_output=True, text=True)
def paths(self):
"""Assert success and return the enumerated path set."""
r = self.run()
assert r.returncode == 0, f"expected success, got {r.returncode}: {r.stderr}"
return [ln for ln in r.stdout.splitlines() if ln.strip()]
def fails_closed(self):
"""The whole point: a NON-ZERO exit, asserted, not inferred."""
r = self.run()
return r.returncode != 0
return Handle(env, state)
# --- The happy path, so the failure-path assertions below cannot pass vacuously ----------------
def test_complete_enumeration_returns_every_path(enumerate_files):
enumerate_files.set_pages(_rows(["docs/a.md", "ErsatzTV/Program.cs", "README.md"]))
assert enumerate_files.paths() == ["docs/a.md", "ErsatzTV/Program.cs", "README.md"]
def test_a_rename_contributes_BOTH_sides(enumerate_files):
"""One row, two paths — the `git mv` hole. Reading `.filename` alone hides the source."""
enumerate_files.set_pages(
[
{
"filename": "docs/innocuous-note.md",
"status": "renamed",
"previous_filename": ".gitea/workflows/renovate.yml",
}
]
)
assert sorted(enumerate_files.paths()) == [".gitea/workflows/renovate.yml", "docs/innocuous-note.md"]
def test_paths_on_a_later_page_are_included(enumerate_files):
enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), _rows(["scripts/decisions_lib.py"]))
assert "scripts/decisions_lib.py" in enumerate_files.paths()
# --- Fail-closed contract: each of these MUST be non-zero, by design ---------------------------
def test_transport_failure_mid_pagination_fails_closed(enumerate_files):
"""The defect that started all of this: an errored page counted as zero rows and read as
'end of list', completing the enumeration over a PARTIAL list."""
enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), "ERROR", _rows(["docs/tail.md"]))
assert enumerate_files.fails_closed()
def test_non_array_body_fails_closed(enumerate_files):
enumerate_files.set_pages("GARBAGE")
assert enumerate_files.fails_closed()
def test_an_OBJECT_OF_VALID_ROWS_isolates_the_top_level_array_check(enumerate_files):
"""Cold review found `test_non_array_body_fails_closed` passing for the wrong reason, and the
first attempt to fix it failed for a THIRD reason — worth recording, because both near-misses
look like coverage.
`jq`'s `all(.[]; …)` iterates an object's VALUES, so the top-level `type == "array"` check is
only load-bearing when those values would themselves validate:
* `{"message":"internal error"}` — values are strings, `.filename` on a string errors. Rejected
with the clause deleted, so it never isolated it.
* `{"filename":"docs/a.md","status":"modified"}` — a single row, but its values are still
strings. Same non-isolation, one level less obvious.
* `{"0": {"filename":"docs/a.md","status":"modified"}}` — values ARE valid rows. With the clause
deleted this validates, `length` is 1, the path is collected, and the next page ends the
enumeration cleanly: a non-array body enumerated as a complete docs-only list. That is the
fail-open, and only this shape exposes it.
"""
enumerate_files.set_pages({"0": {"filename": "docs/a.md", "status": "modified"}})
assert enumerate_files.fails_closed()
def test_rows_without_filename_fail_closed(enumerate_files):
"""`[{}]` is a well-formed array that yields no paths — a partial list wearing a valid shape."""
enumerate_files.set_pages([{}, {}])
assert enumerate_files.fails_closed()
def test_a_row_with_a_VALID_status_but_no_filename_isolates_the_filename_check(enumerate_files):
"""Same wrong-reason problem: `[{}]` is also rejected by the closed `.status` allow-list, since
an absent status is not in it. A row carrying a legitimate `status` and no `filename` removes
that second reason, leaving only the guard under test."""
enumerate_files.set_pages([{"status": "modified"}])
assert enumerate_files.fails_closed()
def test_array_of_scalars_fails_closed(enumerate_files):
enumerate_files.set_pages(["docs/a.md", "docs/b.md"])
assert enumerate_files.fails_closed()
@pytest.mark.parametrize("evil", ["safe.md\ndocs/Program.cs", "safe.md\rdocs/Program.cs"])
def test_CRLF_in_filename_fails_closed(enumerate_files, evil):
"""A newline splits one path into two lines that are each allow-list-matched separately, so
`safe.md\\ndocs/Program.cs` reads as two exempt paths while the real path ends in .cs."""
enumerate_files.set_pages(_rows([evil]))
assert enumerate_files.fails_closed()
def test_CRLF_in_previous_filename_on_a_NON_renamed_row_fails_closed(enumerate_files):
"""The hole one predicate wide: `previous_filename` is CONSUMED on every row, so it must be
VALIDATED on every row — not only where `.status == "renamed"` makes it semantically expected."""
enumerate_files.set_pages(
[{"filename": "docs/a.md", "status": "modified", "previous_filename": "safe.md\ndocs/Program.cs"}]
)
assert enumerate_files.fails_closed()
def test_dotdot_path_component_fails_closed(enumerate_files):
"""The callers' allow-lists anchor `^docs/`, which `docs/../ErsatzTV/Program.cs` matches."""
enumerate_files.set_pages(_rows(["docs/../ErsatzTV/Program.cs"]))
assert enumerate_files.fails_closed()
@pytest.mark.parametrize("status", ["Renamed", "moved", ""])
def test_status_outside_the_closed_allow_list_fails_closed(enumerate_files, status):
"""Without a closed set, the `renamed => previous_filename REQUIRED` clause is dodgeable by any
other value, letting a `git mv` drop its source path and read as docs-only."""
enumerate_files.set_pages([{"filename": "docs/a.md", "status": status, "previous_filename": "ErsatzTV/Program.cs"}])
assert enumerate_files.fails_closed()
def test_renamed_row_without_previous_filename_fails_closed(enumerate_files):
enumerate_files.set_pages([{"filename": "docs/a.md", "status": "renamed"}])
assert enumerate_files.fails_closed()
def test_head_differing_from_the_expected_sha_fails_closed(enumerate_files):
"""Narrowed to what this actually proves, per cold review.
The stub serves the alternate sha from the BINDING read (the one after paging; since
ersatztv#698 the script also reads the PR object BEFORE paging, to bind the base). So this
exercises "the final head does not equal the expected sha", not movement *during* enumeration.
The distinction matters: an A->B->A force-push round trip would restore the expected sha and
pass this check while the pages came from two different states. That race is inherent to
enumerating a mutable list over several round-trips against an API with no commit-pinned files
endpoint, and is tracked separately rather than papered over with a test name that implies it is
covered.
"""
enumerate_files.set_pages(_rows(["docs/a.md"]))
enumerate_files.head_moves_to(OTHER_SHA)
assert enumerate_files.fails_closed()
def test_missing_credentials_fails_closed(enumerate_files):
enumerate_files.set_pages(_rows(["docs/a.md"]))
enumerate_files.env.pop("ETV_GITEA_TOKEN", None)
assert enumerate_files.fails_closed()
@pytest.mark.parametrize("args", [("timothy", "ersatztv"), ("timothy", "ersatztv", "")])
def test_usage_errors_exit_2(enumerate_files, args):
enumerate_files.set_pages(_rows(["docs/a.md"]))
r = (
enumerate_files.run(args=args)
if len(args) == 3
else subprocess.run(["bash", str(SCRIPT), *args], env=enumerate_files.env, capture_output=True, text=True)
)
assert r.returncode == 2, r.stderr
# --- The base-ref binding (ersatztv#698 route 1) ------------------------------------------------
#
# `/pulls/{n}/files` diffs against the PR's LIVE base, so retargeting changes the answer without
# moving the head. Reproduced on the real instance as probe PR #703: opened into `main`, retargeted
# mid-run, enumerated docs-only, granted `review-verdict/h10=success` while its diff against `main`
# carried a C# file.
def test_a_FOUR_argument_call_is_a_usage_error_not_an_unbound_enumeration(enumerate_files):
"""The binding is REQUIRED, not optional.
This is the test that matters most for the shape of the fix. Had the base ref been added as an
OPTIONAL 5th argument, every existing caller would have kept compiling and kept running
unbound — an opt-out that is invisible at the call site, and the caller most likely to omit it
is the one that most needed it. Dropping the argument must be LOUD.
"""
enumerate_files.set_pages(_rows(["docs/a.md"]))
r = enumerate_files.run(expected_base=None)
assert r.returncode == 2, (
"a 4-argument call was accepted, so the base binding is effectively optional and any caller "
f"that forgets it silently enumerates against a mutable base (stdout={r.stdout!r})"
)
def test_an_EMPTY_base_ref_argument_fails_closed(enumerate_files):
"""The hook passes `.base.ref` straight from PR JSON; unparseable JSON yields an empty string."""
enumerate_files.set_pages(_rows(["docs/a.md"]))
r = enumerate_files.run(expected_base="")
assert r.returncode == 2, r.stderr
def test_a_PR_already_retargeted_before_the_enumeration_fails_closed(enumerate_files):
"""The pre-paging read. Without it, a PR retargeted before the job started would enumerate
against the scratch base with every page agreeing with every other page — internally consistent
and entirely wrong."""
enumerate_files.set_pages(_rows(["docs/a.md"]))
enumerate_files.base_is_already("probe/scratch-base")
assert enumerate_files.fails_closed()
def test_a_RETARGET_DURING_the_enumeration_fails_closed(enumerate_files):
"""The post-paging read. The head never moves here, which is the entire point: the existing
head-sha binding cannot see a retarget, so removing the base half of the binding leaves this
case exempted."""
enumerate_files.set_pages(_rows(["docs/a.md"]))
enumerate_files.base_moves_to("probe/scratch-base")
assert enumerate_files.fails_closed()
def test_positive_control_an_unmoved_base_still_enumerates(enumerate_files):
"""Without this, the three tests above could pass because the 5-argument call is broken outright
rather than because the binding works."""
enumerate_files.set_pages(_rows(["docs/a.md", "ErsatzTV/Program.cs"]))
assert enumerate_files.paths() == ["docs/a.md", "ErsatzTV/Program.cs"]
def test_a_BASE_ADVANCE_DURING_enumeration_fails_closed(enumerate_files):
"""ersatztv#707. `/pulls/{n}/files` diffs EACH PAGE against the base's LIVE tip; if `main`
advances between the pre-paging read and the post-paging re-read, later pages can be diffed
against a base earlier pages never saw. Rows can drop out of the result entirely (a file `main`
no longer differs on) while later rows shift into offset ranges already consumed against the old
tip. `.base.ref` never changes here — that is the entire point of the defect and of this being an
ADDITIONAL guard layered on top of the existing retarget check, not a reclassification of it."""
enumerate_files.set_pages(_rows(["docs/a.md", "ErsatzTV/Program.cs"]))
enumerate_files.base_sha_moves_to("cccc000000000000000000000000000000000c")
r = enumerate_files.run()
assert r.returncode != 0, f"expected fail-closed, got {r.returncode}: stdout={r.stdout!r}"
assert not r.stdout.strip(), f"stdout must be meaningless on a failed enumeration: {r.stdout!r}"
assert "base" in r.stderr and "advanced" in r.stderr, (
f"stderr should name the base-advance failure distinctly, got: {r.stderr!r}"
)
assert re.search(r"[0-9a-f]{7}", r.stderr), f"stderr should name both short shas involved, got: {r.stderr!r}"
def test_positive_control_an_UNMOVED_base_sha_still_enumerates_across_pages(enumerate_files):
"""Without this, the fail-closed tests around it could be passing only because the new guard
broke the ordinary path outright rather than because it correctly distinguishes movement from
no movement. Deliberately multi-page, to prove the guard survives several round trips."""
enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), _rows(["ErsatzTV/Program.cs"]))
paths = enumerate_files.paths()
assert len(paths) == 51
assert "ErsatzTV/Program.cs" in paths
def test_the_707_WINDOW_base_ref_unchanged_but_TIP_advanced_still_fails(enumerate_files):
"""The exact #707 window, and the centerpiece of this suite. The base BRANCH NAME is unchanged
throughout (`main` -> `main`), so the pre-existing `.base.ref` retarget guard sees nothing to
object to — by construction, since a mere advance is not a retarget. Only the branch's TIP moved.
A test that passes here proves the NEW sha-based guard is what fired, not the old ref guard,
which cannot see this case at all."""
enumerate_files.set_pages(_rows(["docs/a.md"]), _rows(["ErsatzTV/Program.cs"]))
enumerate_files.base_sha_moves_to("dddd000000000000000000000000000000000d")
r = enumerate_files.run() # expected_base="main" throughout; ref never moves
assert r.returncode != 0, (
"base ref stayed 'main' but its tip advanced mid-enumeration; must still fail closed "
f"(got {r.returncode}, stdout={r.stdout!r}, stderr={r.stderr!r})"
)
def test_a_base_that_ADVANCED_BEFORE_the_enumeration_STILL_SUCCEEDS(enumerate_files):
"""ersatztv#707, the deadlock this guard must NOT cause — and the reason the check is scoped to
the window rather than to the base being "current".
`main` advances constantly. If the guard compared the base tip against anything other than what
it observed at the START of THIS enumeration, every open PR would fail closed on every unrelated
merge to `main` — losing its exemption for reasons that have nothing to do with it. Here the base
already sits at a tip different from the suite's default before the first page is requested, and
then holds still: that is an ordinary, healthy PR and it must enumerate normally.
Note this is deliberately NOT the same test as the unmoved-base control: that one pins "nothing
happened", this one pins "something happened, but OUTSIDE the window, so it is none of our
business."
"""
enumerate_files.base_sha_is_already("eeee000000000000000000000000000000000e")
enumerate_files.set_pages(_rows(["docs/a.md", "docs/b.md"]), [])
assert enumerate_files.paths() == ["docs/a.md", "docs/b.md"], (
"a PR whose base advanced BEFORE this enumeration began was failed closed; the guard is "
"comparing against a stale expectation instead of the tip it actually started from"
)
def test_an_UNREADABLE_base_sha_on_the_AFTER_read_fails_closed(enumerate_files):
"""Consistent with every other guard in this script: an absent/unparseable field is never read
as 'no movement' — it is fail-closed, same as the head-sha and base-ref binding above."""
enumerate_files.set_pages(_rows(["docs/a.md"]))
enumerate_files.base_sha_missing_after()
assert enumerate_files.fails_closed()
def test_the_base_comparator_is_the_BRANCH_NAME_never_the_TIP_SHA():
"""`.base.ref` is compared, never `.base.sha` — matching `post-review-verdict.sh` (ersatztv#632).
Comparing tips would fail every enumeration on every unrelated merge to `main`: a self-inflicted
deadlock dressed as a security control.
This is a STRUCTURAL assertion on purpose, and the previous version of this test is why. It was
written behaviourally as `head_moves_to(SHA)` — the sha that was ALREADY current — so it modelled
no movement at all and was simply a duplicate positive control. It would have passed just as
happily against a script comparing tip shas. The stub serves only branch names, so no behavioural
test in this harness can distinguish the two comparators; say so and assert the source instead.
"""
src = SCRIPT.read_text()
assert ".base.ref" in src, "the enumeration no longer reads .base.ref"
binding = [ln for ln in src.splitlines() if "base_before=" in ln or "base_after=" in ln]
assert binding, "no base binding assignments found"
for ln in binding:
assert ".base.ref" in ln and ".base.sha" not in ln, (
f"the base binding compares a tip sha, which deadlocks on ordinary churn: {ln.strip()!r}"
)
def test_a_SHORT_page_does_not_end_the_enumeration(enumerate_files):
"""Gitea caps `limit` at the server-wide MAX_RESPONSE_ITEMS and may return fewer than asked.
'Fewer than 50 rows means last page' would complete over a partial list without any transport
error — so termination requires a validated EMPTY page. A 30-row page followed by code must be
seen."""
enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(30)]), _rows(["ErsatzTV/Program.cs"]))
assert "ErsatzTV/Program.cs" in enumerate_files.paths()
# --- The same guard, under the runner's jq 1.6 ------------------------------------------------
#
# `test_transport_failure_mid_pagination_fails_closed` above does NOT isolate the explicit
# `if [ -z "${raw//[[:space:]]/}" ]` clause — cold review claimed this and mutation confirmed it:
# deleting that clause leaves the whole suite green on a developer Mac, because jq 1.8 rejects empty
# input on its own. jq 1.6 does not, and the runner ships 1.6 — so the one environment where the
# clause is load-bearing was the one environment with no coverage. That is the #643/#647 failure
# class exactly, reproduced in the test suite meant to prevent it.
#
# The shim is IMPORTED, not copied. A second copy of a version-quirk emulator is the same
# two-implementations-drift problem this whole issue is about, one level down.
from scripts.tests.test_merge_consent_exemption import _JQ16_SHIM # noqa: E402
@pytest.fixture
def enumerate_files_jq16(enumerate_files, tmp_path):
"""The same harness, plus a jq shim reproducing jq 1.6's empty-input `-e` exit status."""
jq = tmp_path / "bin" / "jq"
jq.write_text(_JQ16_SHIM)
jq.chmod(0o755)
return enumerate_files
def test_jq16_shim_actually_reproduces_the_quirk(enumerate_files_jq16, tmp_path):
"""Verify the verifier. A shim that failed to install would make the test below pass vacuously,
reporting the guard safe on jq 1.6 without ever exercising the quirk."""
assert enumerate_files_jq16 is not None # the fixture is what installs the shim
jq = str(tmp_path / "bin" / "jq")
empty = subprocess.run([jq, "-e", "."], input="", capture_output=True, text=True)
assert empty.returncode == 0, "the shim does not reproduce jq 1.6's empty-input exit 0"
real = subprocess.run([jq, "-e", ".a"], input='{"a":1}', capture_output=True, text=True)
assert real.returncode == 0 and real.stdout.strip() == "1", "the shim broke ordinary jq"
false = subprocess.run([jq, "-e", ".a"], input='{"a":false}', capture_output=True, text=True)
assert false.returncode == 1, "the shim broke jq's real -e semantics for a false result"
def test_transport_failure_mid_pagination_fails_closed_on_jq16(enumerate_files_jq16):
"""The property, asserted on the interpreter that actually runs it in CI."""
enumerate_files_jq16.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), "ERROR", _rows(["docs/tail.md"]))
assert enumerate_files_jq16.fails_closed()
def test_a_docs_only_pr_still_enumerates_cleanly_on_jq16(enumerate_files_jq16):
"""Positive control: the shim must not make everything fail, or the test above proves nothing."""
enumerate_files_jq16.set_pages(_rows(["docs/a.md", "docs/b.md"]))
assert enumerate_files_jq16.paths() == ["docs/a.md", "docs/b.md"]
# --- The CALLER contract: a non-zero exit must withhold the exemption, on its own -------------
#
# This is the single line the whole extraction rests on, and it was the one guard nothing pinned. A
# review mutated the hook's `if files=$(...)` into `files=$(...) || true; files_complete=yes` — i.e.
# ignore the exit status entirely — and the ENTIRE suite still passed.
#
# It survives today only by REDUNDANCY: the script writes stdout once, immediately before `exit 0`,
# so every failure path also happens to yield empty stdout, and the hook's independent
# `[ -n "$files" ]` check catches it. That is exactly the shape this PR criticises elsewhere — safe
# by accident rather than by assertion. Any future change that streams pages, or prints a partial
# list before failing, turns it into a live false exemption.
#
# So the stub below FAILS *while emitting a perfectly docs-only list*, which is the one combination
# the redundancy cannot absorb. The positive control immediately after it proves the harness can
# actually observe the difference, rather than reporting "not exempt" for some unrelated reason.
_HOOK_STUB_CURL_TEMPLATE = r"""#!/usr/bin/env python3
import json, sys
url = [a for a in sys.argv[1:] if a.startswith("http")][-1]
if "/pulls/" in url and "/files" not in url:
print(json.dumps({"head": {"sha": "%s"}, "body": "no linked issue"}))
else:
print("{}")
"""
# Percent formatting is required: the template is Python source containing literal `{}`,
# which .format() would eat.
HOOK_STUB_CURL = _HOOK_STUB_CURL_TEMPLATE % SHA
def _mirror_tree(tmp_path, stub_body):
"""Lay out a minimal repo mirror so the hook resolves OUR stub as the shared script.
The hook finds the script via `${BASH_SOURCE[0]}/../..`, so the mirror must reproduce the real
`.claude/hooks/` + `scripts/` shape rather than just dropping the stub anywhere.
"""
hooks = tmp_path / ".claude" / "hooks"
hooks.mkdir(parents=True)
(hooks / "pretooluse-merge-consent.sh").write_text(HOOK.read_text())
scripts = tmp_path / "scripts"
scripts.mkdir()
stub = scripts / "pr-changed-files.sh"
stub.write_text(stub_body)
stub.chmod(0o755)
bindir = tmp_path / "bin"
bindir.mkdir()
curl = bindir / "curl"
curl.write_text(HOOK_STUB_CURL)
curl.chmod(0o755)
env = dict(os.environ)
env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}"
env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment
env["ETV_GITEA_URL"] = "http://gitea.example"
env.pop("ETV_GITEA_BASICAUTH", None)
payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}}
r = subprocess.run(
["bash", str(hooks / "pretooluse-merge-consent.sh")],
input=json.dumps(payload),
env=env,
capture_output=True,
text=True,
)
assert r.returncode == 0, r.stderr
# Exempt == passthrough == exit 0 with no decision JSON on stdout.
return r.stdout.strip() == ""
DOCS_ONLY_OUTPUT = 'printf "docs/a.md\\ndocs/b.md\\n"\n'
def test_a_FAILING_script_withholds_the_exemption_even_when_stdout_looks_docs_only(tmp_path):
exempt = _mirror_tree(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY_OUTPUT + "exit 1\n")
assert exempt is False, (
"the hook granted a docs-only exemption from the stdout of a script that FAILED — the exit "
"status is not being checked"
)
def test_positive_control_the_same_output_with_exit_0_DOES_exempt(tmp_path):
"""Without this, the test above could pass for any unrelated reason and prove nothing."""
exempt = _mirror_tree(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY_OUTPUT + "exit 0\n")
assert exempt is True, (
"the positive control failed, so the negative test above cannot be trusted to be measuring "
"the exit status at all"
)
def test_a_script_that_crashes_also_withholds_the_exemption(tmp_path):
"""Not every failure is a clean `exit 1` — a crash must not read as success either."""
exempt = _mirror_tree(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY_OUTPUT + "kill -TERM $$\n")
assert exempt is False
def test_a_MISSING_script_withholds_the_exemption(tmp_path):
"""The transition window: the shared checkout has no such script until this lands."""
hooks = tmp_path / ".claude" / "hooks"
hooks.mkdir(parents=True)
(hooks / "pretooluse-merge-consent.sh").write_text(HOOK.read_text())
(tmp_path / "scripts").mkdir()
bindir = tmp_path / "bin"
bindir.mkdir()
curl = bindir / "curl"
curl.write_text(HOOK_STUB_CURL)
curl.chmod(0o755)
env = dict(os.environ)
env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}"
env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment
env["ETV_GITEA_URL"] = "http://gitea.example"
payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}}
r = subprocess.run(
["bash", str(hooks / "pretooluse-merge-consent.sh")],
input=json.dumps(payload),
env=env,
capture_output=True,
text=True,
)
assert r.returncode == 0, r.stderr
assert r.stdout.strip() != "", "a missing shared script must not grant an exemption"
# --- Drift guard: the reason this file is worth having at all ----------------------------------
#
# Scope is BOTH callers. Until the enforced workflow was rewired (the follow-up half of ersatztv#649)
# this guard could only assert the hook, which left the copy with real authority — the one that
# writes the branch-protection-required `review-verdict/h10` status — unpinned. That asymmetry was
# the entire subject of the issue, so a guard that covered only the advisory side would have been
# the same mistake one level up.
CALLERS = pytest.mark.parametrize("caller", [HOOK, WORKFLOW], ids=["advisory-hook", "enforced-workflow"])
def _code_lines(path: Path) -> str:
"""Strip comment lines.
Both callers' comments legitimately discuss the `pulls/N/files` endpoint, and a future comment
writing `pulls/$pr/files?limit=100` as an example of what NOT to do would redden these tests —
which, since `script-tests` red blocks merges via the combined status, would block the repo over
a piece of prose. `#`-prefixed works for both files: YAML comments and the shell comments inside
the workflow's `run:` block share the marker.
"""
return "\n".join(ln for ln in path.read_text().splitlines() if not ln.lstrip().startswith("#"))
@CALLERS
def test_the_caller_uses_the_shared_script(caller):
assert "scripts/pr-changed-files.sh" in _code_lines(caller), (
f"{caller.relative_to(REPO_ROOT)} no longer calls the shared enumeration"
)
@CALLERS
def test_the_caller_passes_the_BASE_REF_argument(caller):
"""Both callers must invoke the 5-argument form (ersatztv#698 route 1).
The runtime tests prove the SCRIPT rejects a 4-argument call. They cannot prove a caller still
makes a 5-argument one, and the failure is quiet in opposite directions at the two sites: the
workflow would die on `set -u` (fail-closed, but it takes every PR with it), while the hook would
pass an empty base and lose every docs-only exemption. Pin the call site itself.
Matching is deliberately narrow. A first draft keyed on "any line mentioning the script name" and
matched the enum_error MESSAGE string, failing for a reason that had nothing to do with the call.
The workflow also invokes through `"$ENUM"` rather than the literal path, so the alias is resolved
here and asserted to point at the shared script — otherwise this test could be satisfied while
`ENUM` pointed somewhere else entirely.
"""
code = _code_lines(caller)
if '"$ENUM"' in code:
assert re.search(r"^\s*ENUM=\S*scripts/pr-changed-files\.sh\s*$", code, re.M), (
f'{caller.relative_to(REPO_ROOT)} invokes "$ENUM" but ENUM is not assigned the shared enumeration script'
)
invocations = [ln for ln in code.splitlines() if re.search(r'\$\(\s*"(\$ENUM|[^"]*pr-changed-files\.sh)"', ln)]
assert invocations, f"{caller.relative_to(REPO_ROOT)} has no executable call to the shared enumeration"
for ln in invocations:
after = ln.split('"', 2)[2] if '"$ENUM"' in ln else ln.split("pr-changed-files.sh", 1)[1]
args = re.findall(r'"[^"]*\$[^"]*"', after)
assert len(args) >= 5, (
f"{caller.relative_to(REPO_ROOT)} calls the enumeration with {len(args)} quoted "
f"arguments, expected 5 including the expected base ref: {ln.strip()!r}"
)
# Counting five arguments is not enough — cold review caught that passing `"$SHA"` twice
# satisfied the count while stalling every real exemption. Name the fifth.
assert re.search(r"(?i)base", args[4]), (
f"{caller.relative_to(REPO_ROOT)} passes {args[4]} as the 5th argument; it must be the "
f"expected BASE ref: {ln.strip()!r}"
)
@CALLERS
def test_the_caller_does_not_reimplement_the_enumeration(caller):
"""Structural rather than behavioural on purpose. Behavioural equivalence tests would still pass
if someone pasted the loop back inline and kept it correct *that day* — which is exactly how the
drift happened the first time. What must be prevented is a SECOND implementation existing.
"""
# An inline `pulls/<n>/files?` fetch is the signature of a re-inlined copy. Match on the endpoint
# alone, NOT on `?limit=` — an earlier version anchored the query string, so a copy written as
# `files?page=1&limit=50` would have walked straight past a guard that exists to stop exactly
# that. Still evadable by a copy that builds the URL without a literal `?`, so this narrows the
# gap rather than closing it.
assert not re.search(r"pulls/\$?\{?\w+\}?/files\?", _code_lines(caller)), (
f"{caller.relative_to(REPO_ROOT)} appears to enumerate PR files inline again — "
"that is the duplication ersatztv#649 removed"
)
# --- The ENFORCED caller's own preconditions ---------------------------------------------------
def _workflow_steps():
import yaml
wf = yaml.safe_load(WORKFLOW.read_text())
return wf["jobs"]["set-verdict-status"]["steps"]
def _classify_step():
for step in _workflow_steps():
if "review-verdict/h10" in (step.get("run") or ""):
return step
raise AssertionError("no step in review-verdict.yml posts review-verdict/h10")
def _workflow_triggers():
"""The `on:` block, tolerating YAML 1.1's `on` -> True coercion.
`yaml.safe_load` parses the bare key `on` as the BOOLEAN True, not the string "on" — so the
obvious `wf["on"]` raises KeyError against a perfectly valid workflow. Looking up both is not
defensive padding: a test that died on a KeyError here would read as "the trigger assertion is
broken" rather than "the trigger changed", which is the wrong failure to hand a maintainer.
"""
import yaml
wf = yaml.safe_load(WORKFLOW.read_text())
on = wf.get("on", wf.get(True))
assert isinstance(on, dict), f"review-verdict.yml has no parseable `on:` mapping (got {on!r})"
return on
def test_the_workflow_trigger_is_pull_request_TARGET_scoped_to_main():
"""The definition-rewrite hole (ersatztv#672) — sibling of the base-ref checkout below.
That checkout binds the SCRIPTS this job runs to the base. It cannot bind the job DEFINITION:
Gitea resolves a `pull_request` workflow definition from the PR's own head, so a PR editing
review-verdict.yml ran its own rewritten copy and could post `review-verdict/h10=success` for
itself. Branch protection does not care who posted the context, and carries
`required_approvals: 0`.
Both halves are asserted because either alone closes nothing:
- `pull_request_target` resolves the definition from the base.
- `branches: [main]` keeps "the base" from being an attacker-pushed branch. Base resolution
without it merely moves the rewrite from the head to a scratch base — and since a commit
status is repo-global per sha (ersatztv#663), a success forged there is inherited by a later
real PR into `main` with the same head.
Parsed, not substring-matched, for the reason the checkout test gives — and here the text-level
version is actively broken rather than merely weak: `pull_request` is a PREFIX of
`pull_request_target`, so `"pull_request" in text` cannot tell the safe trigger from the
vulnerable one, and `"pull_request_target" in text` stays green when a plain `pull_request`
trigger is ADDED back alongside it.
"""
on = _workflow_triggers()
# EXACT SET, not "target present and plain absent". Cold review found the weaker pair of
# assertions green after ADDING `workflow_dispatch:` or `push:` alongside the safe trigger —
# both are ref-resolved and both get secrets, so either one restores an equivalent
# self-supplied-definition path while the test reports clean. Enumerating the two known-bad
# extra triggers would have the same hole one trigger later; pinning the whole set does not.
assert set(on) == {"pull_request_target"}, (
f"review-verdict.yml must trigger on `pull_request_target` and NOTHING else; got "
f"{sorted(map(str, on))}. Plain `pull_request` takes the workflow DEFINITION from the PR "
"head (ersatztv#672), and `push`/`workflow_dispatch` resolve it from an arbitrary ref — "
"any of them, ADDED ALONGSIDE rather than replacing, reopens the hole"
)
branches = (on["pull_request_target"] or {}).get("branches")
assert branches == ["main"], (
f"`pull_request_target.branches` is {branches!r}; it must be exactly ['main']. Base "
"resolution means the BASE branch supplies the gate, so an unfiltered trigger lets a PR "
"into an attacker-pushed base run that branch's rewritten copy (ersatztv#672)"
)
def test_no_OTHER_workflow_writes_the_review_verdict_status():
"""`review-verdict.yml` is the only workflow allowed to write `review-verdict/h10`.
The trigger test above hardens ONE file, and by construction cannot see the more general
problem: the gate is forgeable by *any* head-resolved workflow holding credentials that can POST
a commit status, not only by the gate's own definition. `docker-build.yml` is exactly that — it
triggers on `pull_request` (correctly; it builds the PR's code) and carries `ETV_STATUS_AUTH`.
A probe once confirmed those credentials could WRITE statuses, not merely read them
(ersatztv#697); a follow-up probe confirmed `REGISTRY_PASSWORD` is now scoped off
`write:repository` and the same POST returns 403 (see `ci.actions-credential-scoping`). That
closes the registry-credential route, not the general problem — `GITEA_TOKEN`, `RENOVATE_TOKEN`,
and a collaborator's own token can all still POST a status, and none of that is closed by this
test.
What this test does close is the cheap regression: a second workflow quietly starting to write
the context. It is a guard against drift, not a security boundary — a workflow can still write
the status through an indirection this scan cannot see (a variable, a script it calls). Said
plainly so nobody reads a green here as "only one writer is possible".
"""
offenders = []
for wf in _workflow_files():
if wf.name == WORKFLOW.name:
continue
# CODE lines only — `docs`-style commentary in a workflow header legitimately discusses the
# context by name, and a raw-text scan would red on the prose explaining this very rule.
if "review-verdict/h10" in _code_lines(wf):
offenders.append(wf.name)
assert not offenders, (
f"{offenders} reference `review-verdict/h10` in executable lines. Only review-verdict.yml "
"may write the gate's own status; another workflow doing so is a forgery path (see "
"ersatztv#697) or, at best, a second implementation of the gate that will drift"
)
def test_the_workflow_checks_out_the_BASE_ref_never_the_head():
"""The self-rewrite hole. This workflow judges the PR, so the PR must not supply the code that
judges it: checking out `head.sha` would let a PR rewrite `scripts/pr-changed-files.sh` to
return an empty list and exempt itself.
Asserted through a YAML parse rather than a substring search, because the dangerous edit is
subtle — `head.sha` for `base.sha` is a nine-character diff in a `with:` block — and a
text-level `"base.sha" in text` check would still pass if a second checkout step took the head
afterwards and won.
"""
checkouts = [s for s in _workflow_steps() if "actions/checkout" in (s.get("uses") or "")]
assert len(checkouts) == 1, (
f"expected exactly one checkout step, found {len(checkouts)} — a second checkout can "
"silently overwrite the base ref with the PR head"
)
with_ = checkouts[0].get("with") or {}
ref = str(with_.get("ref", ""))
assert "pull_request.base.sha" in ref, (
f"the checkout ref is {ref!r}; it must be the PR's BASE sha, so a PR cannot rewrite the gate that judges it"
)
assert "head" not in ref, f"the checkout ref {ref!r} references the PR head"
assert with_.get("persist-credentials") is False, (
"persist-credentials must be false — nothing here pushes, and a token left in .git/config "
"is handed to every script the job runs"
)
def test_the_workflow_runs_the_jq_preflight_in_FLOOR_mode_only():
"""`--expect` pins an exact jq version and fails when it drifts. That is right for the advisory
`script-tests` job and catastrophic here: this workflow writes `review-verdict/h10`, a REQUIRED
check on `main`, so a pin would turn any jq upgrade on the runner into a repo-wide merge
deadlock — a required gate failing because an upstream package manager did its job.
"""
# CODE only, for the reason `_code_lines` documents: the first draft of this assertion read the
# raw text and went red on the workflow's own comment explaining why `--expect` is banned here.
code = _code_lines(WORKFLOW)
# A bare `"jq-preflight.sh" in code` is NOT enough, and cold review was right to say so: the
# path also appears in the `if [ -x ./scripts/jq-preflight.sh ]` presence guard, so deleting the
# actual invocation would leave that substring behind and the assertion green. Require a line
# that INVOKES it.
steps = [s for s in _workflow_steps() if "jq-preflight.sh" in (s.get("run") or "")]
assert steps, (
"review-verdict.yml no longer runs the jq preflight, so the version its shell gates run "
"under is unobservable again (ersatztv#648)"
)
invocations = [
ln.strip() for ln in steps[0]["run"].splitlines() if re.match(r"^\s*(\./)?scripts/jq-preflight\.sh(\s|$)", ln)
]
assert invocations, "the jq preflight is referenced but never actually invoked"
assert "--expect" not in code, (
"review-verdict.yml must run jq-preflight.sh in floor-only mode; --expect here deadlocks "
"every merge on `main` the day the runner's jq changes"
)
assert all("--expect" not in ln for ln in invocations)
# --- The ENFORCED caller's contract, EXECUTED ---------------------------------------------------
#
# The structural guards above prove the workflow *calls* the shared script. They cannot prove it
# reacts correctly when the script FAILS — and that is precisely the mutation that survived the last
# round on the hook side: making the caller ignore the exit status left the entire suite green,
# because every failure path also happened to produce empty stdout. So the stub below FAILS while
# emitting a perfectly docs-only list, the one combination that redundancy cannot absorb.
#
# The step's `run:` block is extracted from the YAML and executed directly. That is a real
# behavioural test of the shipped text — not a paraphrase of it — at the cost of not exercising the
# runner's step wiring, which no local test can reach anyway.
WORKFLOW_STUB_CURL = r"""#!/usr/bin/env python3
import json, os, pathlib, sys
args = sys.argv[1:]
url = [a for a in args if a.startswith("http")][-1]
out = pathlib.Path(os.environ["STUB_DIR"])
if "-X" in args and args[args.index("-X") + 1] == "POST":
payload = args[args.index("-d") + 1]
(out / "posted.json").write_text(payload)
(out / "posted_url.txt").write_text(url)
# EVERY post is also appended (ersatztv#706 race 2): the repair path POSTs a second time, and a
# test that only ever saw the last write could not tell "posted success, then repaired it to
# pending" from "posted pending once".
#
# Re-serialized COMPACT rather than appended verbatim: `jq -n` pretty-prints by default, so the
# payloads arrive spanning several lines and a raw append would produce a file that is not JSONL
# at all. (Found the honest way — the first draft of this recorder broke both race-2 tests.)
with (out / "posted_all.jsonl").open("a") as fh:
fh.write(json.dumps(json.loads(payload)) + "\n")
print("{}")
sys.exit(0)
# The PR timeline, which the retarget fence counts (ersatztv#706 race 1). Checked BEFORE the status
# branch: the fence's URL does not contain "/status", but keeping the order explicit means a later
# edit to either pattern cannot silently reroute one endpoint into the other's handler.
#
# Real shapes, measured on this instance and deliberately mirrored, because the job's guards are
# type-sensitive: a NON-EMPTY page is a bare array of events, each with a `type`, and a retarget is
# `change_target_branch` (confirmed on PR #703, the route-1 reproduction, which carries exactly two;
# and on PR #717, never retargeted, which carries none). A page PAST THE END is the JSON value `null`
# — NOT an empty array (measured again at 1.27.1 on PR #752, ersatztv#751). The earlier version of
# this comment claimed measured fidelity while the terminator below printed `[]`; that discrepancy is
# why the fence's type gate was never exercised and shipped rejecting every real timeline.
if "/timeline" in url:
mode = os.environ.get("STUB_TIMELINE_MODE", "none")
page = 1
for part in url.split("?", 1)[-1].split("&"):
if part.startswith("page="):
page = int(part.split("=", 1)[1])
if mode == "empty-first-page":
# A terminator on PAGE 1 — no real page ever read (ersatztv#751). The job must NOT certify a
# zero retarget count from this, so the exemption is withheld.
print(os.environ.get("STUB_TIMELINE_TERMINATOR", "null"))
sys.exit(0)
if mode == "unreadable":
print("<html>502 Bad Gateway</html>")
sys.exit(0)
if mode == "transport-error":
sys.exit(22)
# Page 2+ terminates the walk. THE DEFAULT IS `null`, NOT `[]`, because that is what this
# endpoint really returns past the end — measured at Gitea 1.27.1 (ersatztv#751). This stub
# printed `[]` for two rounds of #706 work while the comment above claimed it mirrored measured
# reality, so every fence test was green against a shape the server never produces, and the
# fence's `array`-only type gate — which reads `null` as unreadable — was never exercised. On the
# real instance that made `rt_ok` false for EVERY pr, so the fence withheld every exemption.
# Parameterised rather than simply corrected: `/issues/{n}/comments` really does return `[]` when
# empty, so both shapes are live on this server and the job must accept either.
if page > 1:
print(os.environ.get("STUB_TIMELINE_TERMINATOR", "null"))
sys.exit(0)
n_before, n_after = 0, 0
if mode.startswith("stable:"):
n_before = n_after = int(mode.split(":", 1)[1])
elif mode.startswith("moves:"):
# "moves:A,B" — A retarget events on the fence's FIRST count, B on the re-count taken just
# before the POST. This is the race-1 window: the PR was retargeted while the job classified.
a, b = mode.split(":", 1)[1].split(",")
n_before, n_after = int(a), int(b)
ctr = out / "timeline_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
n = n_before if seen == 0 else n_after
print(json.dumps([{"id": 1000 + i, "type": "change_target_branch",
"old_ref": "main", "new_ref": "scratch"} for i in range(n)]
+ [{"id": 900, "type": "comment"}]))
sys.exit(0)
# The per-POST status HISTORY (`/statuses/{sha}`), read by the race-2 post-write verification. It is
# a BARE ARRAY, one row per POST — a different shape AND a different meaning from the combined
# `/commits/{sha}/status` below, which returns an object carrying the LATEST row per context. Both
# shapes were measured on the same live head (24 rows vs 12). Modelling them identically would have
# made the post-write check untestable, since the thing it looks for is precisely a row the combined
# endpoint no longer shows.
if "/statuses/" in url:
mode = os.environ.get("STUB_HISTORY_MODE", "none")
# PAGES FAITHFULLY (ersatztv#763). The job walks this endpoint to a validated empty page instead
# of reading one clamped page, so a stub describing page 1 only no longer describes anything the
# job does. The snapshot is a LIST OF PAGES, not a flat list, because two modes below need a page
# boundary the uniform 50-row slicing cannot express.
#
# 1. PAGE SIZE AND TERMINATOR match the real endpoint. Measured at 1.27.1 on 2026-08-28 against
# PR #761's 114-row head: pages 1 and 2 return 50, page 3 returns 14, page 4 is `[]`. So the
# slice width is 50 and past the end is `[]` — NOT `null`, which is what
# `/issues/{n}/timeline` returns and what `/commits/{sha}/status` spells `{"statuses": null}`.
# Three distinct empty shapes on one server; each stub owes its own measurement.
#
# 2. ONE LOGICAL READ IS ONE SNAPSHOT. The counter modes resolve their rows from how many times
# the job has LOOKED, and the job now looks two or three times per logical read. Recomputing
# on page 2 from an advanced counter would describe a different history than page 1 and
# surface `human-after-post`'s verdict halfway through the PRE-write read, inverting the race
# being modelled. So the pages are resolved once, on page 1, and cached.
#
# This is a MODELLING CHOICE, not a fidelity claim, and the difference matters. These are
# independent offset-paginated GETs with no snapshot token, so the REAL history can change
# between page requests — overlapping rows and shifting offsets are all reachable live. The
# cache deliberately suppresses that, because the tests here are about the job's paging
# logic, not about mid-walk mutation. Mid-walk mutation is untested, and saying so is the
# honest form of the claim.
#
# DELIBERATELY ORDERING-BLIND, and this bounds what the paging tests prove. The real endpoint
# serves `created_unix DESC`, so a row created DURING the write window is among the NEWEST and
# lands on page 1 — a genuinely raced verdict would not sit on page 2 at all. These pages are
# served in insertion order regardless, so a fixture can place a row beyond page 1 and observe
# whether the walk reaches it.
#
# So `verdict-on-page-2` and `verdict-after-short-page` prove WALK COMPLETENESS — that the job
# reads past page 1, and past a short page, to a validated empty one. They do NOT prove that a
# real raced verdict would otherwise be missed; under the server's ordering it would not be. That
# is the honest scope, and the workflow comment says the same thing: the walk exists so the one
# fail-toward-SUCCESS path stops depending on an undocumented ordering, not because the ordering
# is currently wrong.
#
# Counter branches are confined to the page-1 arm below, so a later page can never advance them.
HIST_PAGE_SIZE = 50
# ORDER-FAITHFUL MODES HONOUR `sort`. Most modes here are ordering-blind on purpose (see above):
# they test walk COMPLETENESS, which is an order-independent property, and serving insertion order
# lets a fixture place a row beyond page 1. But the partial-mark fallback's safety is a claim ABOUT
# the ordering, so the mode that pins it serves the real arrangement — DESC by default, ASC when
# the request asks for `sort=highestindex`. That is what lets re-adding the withdrawn parameter be
# caught by BEHAVIOUR rather than only by a structural assertion on the request line.
want_asc = "sort=highestindex" in url
order_faithful = False
hist_page = 1
for part in url.split("?", 1)[-1].split("&"):
if part.startswith("page="):
try:
hist_page = int(part.split("=", 1)[1])
except ValueError:
hist_page = 1
ordinary = [{"id": 7000 + i, "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"} for i in range(60)]
raced_row = {"id": 9000, "context": "review-verdict/h10", "status": "failure",
"creator": {"login": "timothy"},
"description": "Review-verdict: BLOCKED @ a9e3e23 (base: main)"}
snapshot = out / "history_snapshot.json"
logical = out / "history_logical_reads.txt"
if hist_page == 1:
n_logical = (int(logical.read_text()) if logical.exists() else 0) + 1
logical.write_text(str(n_logical))
rows = []
pages = None
if mode.startswith("human-after-post"):
# The raced verdict: absent when the high-water mark is taken, present afterwards. Its id
# is ABOVE the mark, which is what makes it detectable.
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen > 0:
# Configurable creator (ersatztv#742). The RACED test must stay broader than the
# INHERITANCE test: a verdict from an account off `$H10_REVIEWERS` still counts as
# "something human landed while we were writing", because narrowing it here would
# leave the exemption green over that row instead of repairing to pending.
rows = [dict(raced_row, id=5000,
creator={"login": os.environ.get("STUB_HISTORY_CREATOR", "timothy")})]
elif mode == "sentinel-after-post":
# Another overlapping run repaired mid-flight: its SENTINEL lands above this run's mark,
# while the human row it records sits BELOW the mark and is therefore invisible here.
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen > 0:
rows = [{"id": 6000, "context": "review-verdict/h10", "status": "pending",
"creator": None,
"description": "Human verdict raced this exemption write — re-post the verdict"}]
elif mode == "stale-human-already-present":
# A human verdict that was ALREADY in the history before this run — e.g. one whose
# recorded base did not match, which `read_existing_verdict` deliberately declines to
# honour. It must NOT be mistaken for a raced write, or every later run of that PR would
# repair its own exemption to pending forever.
rows = [{"id": 10, "context": "review-verdict/h10", "status": "success",
"creator": {"login": "timothy"},
"description": "Review-verdict: MERGEABLE @ a9e3e23 (base: other)"}]
elif mode in ("second-page", "verdict-on-page-2", "premark-page2-error",
"flaky-page2", "premark-page1-error"):
# A history that genuinely runs past one page: 60 ORDINARY rows, no verdict and no
# sentinel. Under the pre-#763 page-2 probe the mere existence of these rows forced a
# repair; now they are simply read, and `second-page` asserts the exemption STANDS.
rows = list(ordinary)
if mode in ("verdict-on-page-2", "premark-page2-error"):
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen > 0:
# THE RACED VERDICT, PLACED BEYOND THE FIRST PAGE. Its id is above every ordinary
# row, so it is above the high-water mark too. A job that reads only page 1 cannot
# see it — the fail-toward-SUCCESS hole #763 closes.
rows.insert(55, dict(raced_row))
elif mode == "premark-page2-error-desc":
# THE PARTIAL-MARK FALLBACK, MEASURED UNDER THE REAL ORDERING. 60 rows, one of them a
# PRE-EXISTING base-mismatched human verdict at id 7055 — genuinely older than the newest
# row and therefore not something that raced this write.
#
# Under the server default (DESC) page 1 carries 7059..7010, so a walk that fails on page 2
# still saw the true maximum: the mark is 7059, the verdict at 7055 is below it, and the
# exemption correctly STANDS. Under ASC page 1 carries 7000..7049, the salvaged mark is
# 7049, and that same pre-existing verdict tests as NEWER than the mark — a spurious sticky
# repair on a head nothing raced, which is the #761 failure. This fixture is what makes
# that difference observable.
order_faithful = True
rows = [{"id": 7000 + i, "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"}
for i in range(60) if 7000 + i != 7055]
rows.append({"id": 7055, "context": "review-verdict/h10", "status": "success",
"creator": {"login": "timothy"},
"description": "Review-verdict: MERGEABLE @ a9e3e23 (base: other)"})
rows.sort(key=lambda r: r["id"])
elif mode == "null-page1-after-post":
# The post-write read answers "no statuses exist" for a sha this job has just POSTed to.
# That is impossible — the endpoint returns one row per POST — but it is well-formed, so
# nothing retries it and the walk reports SUCCESS over it.
#
# NO RACED VERDICT IS PLANTED, deliberately. An earlier shape appended one to dramatise
# the stakes, and it was inert twice over: unconditionally it also joined the PRE-write
# read, which set the mark to its OWN id — the strict `> $since` then excluded it — and
# gated to the post-write read it is never served at all, because page 1 answers `null`
# before any row reaches the wire. A row the test
# cannot observe is decoration that reads as coverage. What is actually under test is
# narrower and sufficient: a response asserting an empty history for a sha this job wrote
# to must not be accepted as proof that nothing raced.
rows = [{"id": 7000 + i, "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"} for i in range(3)]
elif mode == "null-terminator":
# This endpoint returns `[]` past the end today. `null` is what `/issues/{n}/timeline`
# returns, and an array-only gate on THAT endpoint was ersatztv#751 — the walk never
# reached a validated empty page and every exemption was withheld. Same shape, second
# endpoint, worse blast radius (the sticky sentinel), so `null` is tolerated and this
# fixture proves it rather than asserting it.
rows = [{"id": 7000 + i, "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"} for i in range(60)]
elif mode == "string-id-preexisting-row":
# THE TWIN of `string-id-inflates-mark`, on the COMPARISON rather than the maximum. A
# PRE-EXISTING row (a base-mismatched verdict `read_existing_verdict` declines to honour)
# carries a string id. jq orders strings above every number, so `.id > $since` reads it as
# newer than any mark and counts it as having raced this write — a sticky sentinel and a
# false "was overwritten" on every later run of that PR, forever.
rows = [{"id": 9999, "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"},
{"id": "3", "context": "review-verdict/h10", "status": "success",
"creator": {"login": "timothy"},
"description": "Review-verdict: MERGEABLE @ a9e3e23 (base: other)"}]
elif mode == "no-numeric-ids":
# A NON-EMPTY history in which no row carries a numeric id. Distinct from an EMPTY history,
# whose mark of 0 is correct; collapsing this case to 0 too would be a schema we cannot
# read reported as one we can.
rows = [{"id": "abc", "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"}]
elif mode == "over-cap":
# Longer than the 20-page cap (1050 rows), so the walk can never reach a validated empty
# page. That is "could not establish", not exhaustion.
rows = [{"id": 20000 + i, "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"} for i in range(1050)]
elif mode == "string-id-inflates-mark":
# A STRING ID. jq orders strings above every number, so `max` over raw ids returns
# `"99999"` — which then passes the numeric gate as a plain `99999` and sets a high-water
# mark far above anything real. Every later row looks OLDER than the mark, so the raced
# verdict below is invisible and the exemption stands over it. One corrupt row is enough.
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
rows = [{"id": "99999", "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"},
{"id": 10, "context": "ci/other", "status": "success",
"creator": None, "description": "unrelated"}]
if seen > 0:
rows.append(dict(raced_row))
elif mode == "malformed-creator-beside-verdict":
# A SCHEMA-CORRUPT ROW NEXT TO A REAL ONE. `creator` is a number, so `.creator.login`
# hard-errors in jq ("Cannot index number with string"), jq exits 5, and an unguarded
# `raced=$(...)` takes the whole step down under `set -e` — after the exemption `success`
# is posted and with the repair never attempted. The genuine verdict beside it is what
# makes the consequence visible: the correct behaviour is to drop the malformed row and
# still repair.
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen > 0:
rows = [{"id": 8500, "context": "review-verdict/h10", "status": "failure",
"creator": 7, "description": "Review-verdict: BLOCKED @ a9e3e23 (base: main)"},
dict(raced_row)]
elif mode == "verdict-after-short-page":
# DELIBERATELY UNFAITHFUL, and that is the point. Page 2 is SHORT (10 rows) and yet page 3
# still carries rows — a shape the measured server does not produce, but exactly what a
# truncated or partially-served response looks like. It is the only way to observe the
# rule "terminate ONLY on a validated EMPTY page, never on a short one": against a
# faithful double a short page is always the last one, so an implementation that stops
# there is indistinguishable from a correct one.
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
pages = [ordinary[:50], ordinary[50:], [dict(raced_row)] if seen > 0 else []]
if order_faithful and not want_asc:
rows = list(reversed(rows))
if pages is None:
pages = [rows[i:i + HIST_PAGE_SIZE] for i in range(0, len(rows), HIST_PAGE_SIZE)] or [[]]
# THE JOB'S OWN WRITES APPEAR IN THE HISTORY. `/statuses/{sha}` returns one row per POST, so
# once this job has posted its exemption the next read MUST show it. The stub did not model
# that at all — the history was whatever the mode described, before and after the write alike,
# so every ordinary run looked like a head nothing had ever been posted to. `creator: null` is
# measured (an Actions-token POST records no creator), which is also what keeps our own row
# out of the raced count; the id is one above everything present, mirroring the real
# endpoint's per-head monotonic ids (verified live: 114 rows, ids strictly increasing with
# `created_at`, no duplicates).
posted_log = out / "posted_all.jsonl"
if posted_log.exists():
for line in posted_log.read_text().splitlines():
if not line.strip():
continue
body = json.loads(line)
# NUMERIC IDS ONLY when computing the next one. A fixture may deliberately carry a
# schema-corrupt id (see `string-id-inflates-mark`), and `max()` over a str beside an
# int raises TypeError — which fails the whole stub request, makes the walk look
# unreadable, and produces a repair for a reason the test was not asking about. That
# is a test passing for the wrong reason: the string-id mutation stayed green because
# the double crashed rather than because the mark was computed correctly.
nxt = max([r.get("id") for pg in pages for r in pg
if isinstance(r.get("id"), int)] or [0]) + 1
own = {"id": nxt, "context": body.get("context"),
"status": body.get("state"), "creator": None,
"description": body.get("description", "")}
# WHERE the newest row goes depends on the served ordering. Under DESC it is FIRST;
# appending it last would put the job's own POST on the OLDEST page, the arrangement
# the order-faithful fixture exists to rule out. Ordering-blind modes keep insertion
# order, which is what lets them place a row beyond page 1.
if order_faithful and not want_asc:
pages[0].insert(0, own)
else:
pages[-1].append(own)
snapshot.write_text(json.dumps(pages))
else:
n_logical = int(logical.read_text()) if logical.exists() else 1
pages = json.loads(snapshot.read_text()) if snapshot.exists() else [[]]
if hist_page == 1 and mode == "premark-page1-error":
# PAGE 1 ITSELF FAILS, for the whole first logical read (both attempts), so the walk returns
# nothing at all and the mark must be abandoned. Counted in its own file because the logical
# read counter below counts page-1 REQUESTS, retries included — not logical reads.
ctr = out / "p1_attempts.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen < 2:
sys.exit(22)
if hist_page > 1 and mode == "flaky-page2":
# TRANSIENT: fails the first attempt of each logical read and succeeds on the retry. This is
# the only fixture that exercises the retry at all — the other error modes fail every attempt,
# so against them a one-shot walk and a retrying walk are indistinguishable.
ctr = out / "page2_attempts.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen % 2 == 0:
sys.exit(22)
if hist_page > 1 and mode == "second-page-garbage":
print("<html>502 Bad Gateway</html>")
sys.exit(0)
if hist_page > 1 and mode == "second-page-error":
sys.exit(22)
if hist_page == 1 and mode == "null-page1-after-post" and n_logical >= 2:
print("null")
sys.exit(0)
if hist_page > 1 and mode == "null-terminator" and hist_page > len(pages):
print("null")
sys.exit(0)
if hist_page > 1 and mode in ("premark-page2-error", "premark-page2-error-desc") and n_logical == 1:
# Fails ONLY on the PRE-write read, so the high-water mark must be salvaged from the partial
# list; the post-write read then pages cleanly and must still catch the raced verdict.
# `n_logical` is incremented on every page-1 REQUEST, including retried ones — which is fine
# here because page 1 never fails in this mode, but it counts requests, not logical reads.
sys.exit(22)
print(json.dumps(pages[hist_page - 1] if hist_page - 1 < len(pages) else []))
sys.exit(0)
if "/status" in url:
# 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")
# PAGE, honoured — the job now reads page 2 to justify "no verdict exists" (ersatztv#751). The
# real endpoint pages properly: measured at 1.27.1 on a 6-context head, `?limit=3&page=2` returns
# 3 more rows and `page=9` returns the `statuses: null` terminator. Every mode below describes
# page 1 only, so page 2+ must terminate, or the job would read its own page-1 rows again and
# conclude the list is longer than it is. `twopage` is the one mode with a real second page.
status_page = 1
for part in url.split("?", 1)[-1].split("&"):
if part.startswith("page="):
try:
status_page = int(part.split("=", 1)[1])
except ValueError:
status_page = 1
# THE EMPTY SHAPE IS `statuses: null`, NOT `[]` (ersatztv#751). Measured on this instance: a head
# with no statuses returns `{"state":"pending","total_count":0,"statuses":null}` — PR #739's head
# 5fa672e2. This stub printed `{"statuses": []}` at all three no-verdict sites, which is a shape
# the server does not produce for that case, so the job's `.statuses | type == "array"` gate was
# never exercised against reality and its `exit 1` branch — posting nothing at all — was
# unreachable in the suite. Same class as the timeline terminator, one function over.
# Parameterised, not merely corrected: a head that HAS statuses really does return an array, so
# both shapes are live and the job must read either.
def empty_statuses():
if os.environ.get("STUB_STATUS_EMPTY_SHAPE", "null") == "array":
return json.dumps({"state": "pending", "total_count": 0, "statuses": []})
return json.dumps({"state": "pending", "total_count": 0, "statuses": None})
# BEFORE ANY READ-COUNTING MODE. The `appears-on-read:N` modes count how many times the job has
# LOOKED at the combined status, and the page-2 completeness probe is part of the same look, not a
# further one — letting it increment those counters shifted "the verdict appears on read N" by one
# and broke three mid-run-race tests. Page 2 also has to terminate here for every mode that
# describes page 1 only, or the job would re-read page 1's rows as a second page and conclude the
# list is longer than it is.
if status_page > 1 and mode == "page2-garbage":
print("<html>502 Bad Gateway</html>")
sys.exit(0)
if status_page > 1 and mode == "page2-error":
sys.exit(22)
if status_page > 1 and mode != "twopage":
print(empty_statuses())
sys.exit(0)
if mode.startswith("appears-on-read:"):
# A human verdict that does NOT exist at the first read and DOES exist at the re-read made
# immediately before the POST (ersatztv#706). Models a reviewer posting BLOCKED while the job
# is still enumerating — the window the first read structurally cannot see.
nth = int(mode.split(":", 1)[1])
ctr = out / "status_reads.txt"
n = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(n + 1))
if n + 1 < nth:
# STUB_PRE_ROW makes the earlier reads return a PRE-EXISTING row instead of an empty
# head (ersatztv#742). Without it every mid-run test starts from "no status at all", so
# only the ARRIVAL case is exercised and a presence-only abstain (`[ -z "$pre_state" ]`)
# is indistinguishable from a changed-row abstain. The REPLACEMENT case is the one that
# co-occurs with #742's own scenario: the head already carries an off-list verdict — that
# is WHY the job is re-deriving — and a real reviewer posts over it mid-run.
pre = os.environ.get("STUB_PRE_ROW", "")
if pre:
pre_creator, pre_state, pre_desc = pre.split("|", 2)
# DECOY CONTEXT + `total_count`, matching the shape the `existing:` modes model and the
# server actually returns. Without a decoy a dropped `select(.context == $c)` is
# invisible; without `total_count` the emptiness cross-check is never exercised.
rows = [
{"context": "ci/decoy", "status": "pending", "creator": None,
"description": "unrelated"},
{"context": "review-verdict/h10", "status": pre_state,
"creator": ({"login": pre_creator} if pre_creator else None),
"description": pre_desc},
]
print(json.dumps({"state": "pending", "total_count": len(rows), "statuses": rows}))
sys.exit(0)
print(empty_statuses())
sys.exit(0)
# Configurable creator (ersatztv#742). This site asks "did anything human land while we were
# classifying", NOT "may this be inherited", so it must stay broad: an off-allow-list account
# posting BLOCKED here still has to make the job abstain.
# STUB_MIDRUN_ROW overrides the whole row, so a test can make the mid-run row differ from
# STUB_PRE_ROW in exactly ONE field of the state/creator/description triple. Without that,
# every mid-run test moves all three at once and no single clause of the comparison is pinned.
mid = os.environ.get("STUB_MIDRUN_ROW", "")
if mid:
mid_creator, mid_state, mid_desc = mid.split("|", 2)
else:
mid_creator = os.environ.get("STUB_MIDRUN_CREATOR", "timothy")
mid_state, mid_desc = "failure", "Review-verdict: BLOCKED @ a9e3e23 (base: main)"
mid_rows = [
{"context": "ci/decoy", "status": "pending", "creator": None, "description": "unrelated"},
{"context": "review-verdict/h10", "status": mid_state,
"creator": ({"login": mid_creator} if mid_creator else None),
"description": mid_desc},
]
print(json.dumps({"state": "pending", "total_count": len(mid_rows), "statuses": mid_rows}))
sys.exit(0)
if mode.startswith("sentinel-appears-on-read:"):
# A repair sentinel written by ANOTHER, overlapping run between this job's first read and its
# last-moment re-read (ersatztv#706 round 3). Creator is null: the sentinel is machine-written.
nth = int(mode.split(":", 1)[1])
ctr = out / "status_reads.txt"
n = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(n + 1))
if n + 1 < nth:
print(empty_statuses()); sys.exit(0)
print(json.dumps({"statuses": [
{"context": "review-verdict/h10", "status": "pending", "creator": None,
"description": "Human verdict raced this exemption write — re-post the verdict"}]}))
sys.exit(0)
if mode == "twopage":
# Page 1 AND page 2 both carry decoy contexts and NO `review-verdict/h10`: the list is longer
# than one page, so "no verdict exists" is not established and the job must refuse. Models a
# head with more contexts than the server-wide page cap (measured 50 here) — which is exactly
# why the guard cannot be written as a comparison against a hardcoded limit.
print(json.dumps({"state": "pending", "total_count": 3,
"statuses": [{"context": f"ci/p{status_page}x{i}", "status": "pending"}
for i in range(3)]}))
sys.exit(0)
if mode.startswith("rows:"):
# N decoy contexts and NO `review-verdict/h10`, to exercise the truncation guard. `total_count`
# deliberately MIRRORS the page length, because that is what the real endpoint does — it is the
# count for the page returned, not for the commit (measured at 1.27.1: `?limit=1` on a 6-context
# head returns len=1, total_count=1). A stub that reported the true total would make a
# length-vs-total guard look like it worked, which is the trap this models.
n = int(mode.split(":", 1)[1])
print(json.dumps({"state": "pending", "total_count": n,
"statuses": [{"context": f"ci/decoy{i}", "status": "pending"}
for i in range(n)]}))
sys.exit(0)
if mode == "total-count-string":
# A schema-corrupted `total_count` as a STRING alongside a null `.statuses`. `jq -r` renders 0
# and "0" identically, so a text compare would accept this.
print(json.dumps({"state": "pending", "total_count": "0", "statuses": None}))
sys.exit(0)
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.
#
# The row's PROVENANCE is configurable (ersatztv#698 route 3). A status POSTed by a user
# credential carries `.creator.login` and, for a real verdict, a `Review-verdict:`
# description; one POSTed by an Actions job carries `"creator": null`. Both were measured on
# the live combined endpoint. Defaults model a HUMAN verdict, so a test that does not opt out
# exercises the never-overwrite property.
creator = os.environ.get("STUB_STATUS_CREATOR", "timothy")
desc = os.environ.get("STUB_STATUS_DESC", "Review-verdict: MERGEABLE @ a9e3e23 (base: main)")
print(json.dumps({"statuses": [
{"context": "Build & test (.NET)", "status": "pending"},
{"context": "review-verdict/h10", "status": mode.split(":", 1)[1],
"creator": ({"login": creator} if creator else None), "description": desc},
{"context": "Functional E2E", "status": "pending"}]}))
sys.exit(0)
print(empty_statuses())
sys.exit(0)
print("{}")
"""
def _run_classify(
tmp_path,
enum_stub: str | None,
author: str = "timothy",
status_mode: str = "none",
jq16: bool = False,
status_creator: str | None = "timothy",
status_desc: str = "Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
timeline_mode: str = "none",
history_mode: str = "none",
history_creator: str = "timothy",
midrun_creator: str = "timothy",
pre_row: str = "",
midrun_row: str = "",
reviewers: str | None = None,
timeline_terminator: str = "null",
status_empty_shape: str = "null",
):
"""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.
"""
tmp_path.mkdir(parents=True, exist_ok=True) # the chained sentinel test passes sub-paths
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"
enum.write_text(enum_stub)
enum.chmod(0o755)
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["STUB_STATUS_CREATOR"] = status_creator or ""
env["STUB_STATUS_DESC"] = status_desc
env["STUB_TIMELINE_MODE"] = timeline_mode
env["STUB_TIMELINE_TERMINATOR"] = timeline_terminator
env["STUB_STATUS_EMPTY_SHAPE"] = status_empty_shape
env["STUB_HISTORY_MODE"] = history_mode
env["STUB_HISTORY_CREATOR"] = history_creator
env["STUB_MIDRUN_CREATOR"] = midrun_creator
env["STUB_PRE_ROW"] = pre_row
env["STUB_MIDRUN_ROW"] = midrun_row
env.update(
{
"GITEA_TOKEN": "stub",
"BASE_URL": "http://gitea.example/api/v1",
"GITEA_BASE_URL": "http://gitea.example/api/v1",
"REPO": "timothy/ersatztv",
"PR": "42",
"SHA": SHA,
"BASE_SHA": OTHER_SHA,
# The base branch the event was raised for, threaded to the enumeration (ersatztv#698).
# Omitting it is not a soft failure: `set -u` kills the step, no status is posted, and the
# absent required check blocks the merge — fail-closed, but it would take every PR with it.
"BASE_REF": "main",
"AUTHOR": author,
"PR_URL": "http://gitea.example/timothy/ersatztv/pulls/42",
}
)
script = tmp_path / "step.sh"
body = _classify_step()["run"]
if reviewers is not None:
# Substitute the allow-list into the REAL step body so the shipped membership loop runs
# against a multi-entry value. Every other test runs the singleton `timothy`, under which a
# loop that can only ever match its LAST entry is indistinguishable from a correct one — and
# the workflow's own diagnostic tells operators to add a second entry.
body, n = re.subn(
r'^(\s*)H10_REVIEWERS="[^"]*"$',
lambda m: f'{m.group(1)}H10_REVIEWERS="{reviewers}"',
body,
flags=re.MULTILINE,
)
assert n == 1, f"expected exactly one H10_REVIEWERS assignment to override, substituted {n}"
script.write_text(body)
r = subprocess.run(["bash", str(script)], cwd=tmp_path, env=env, capture_output=True, text=True)
# Assert the WIRING, not only the classification. The stub accepts every POST, so a status aimed
# at the wrong endpoint, sha, host or repo would otherwise leave these tests green while the real
# required check was never written.
#
# Two ways this check could disable itself, both found by cold review of an earlier draft:
# * it was guarded by `if url_file.exists()`, so deleting the recorder in the stub turned it
# into a no-op and every test stayed green — a verifier that silently opts out;
# * it compared only the URL SUFFIX, so a POST to the right path on the WRONG HOST OR REPO
# passed. Compare the whole URL against the env this job was given.
posted = tmp_path / "posted.json"
url_file = tmp_path / "posted_url.txt"
if posted.exists():
assert url_file.exists(), (
"a status was POSTed but its URL was not recorded — the wiring assertion below would have silently skipped"
)
expected = f"{env['BASE_URL']}/repos/{env['REPO']}/statuses/{SHA}"
assert url_file.read_text().strip() == expected, (
f"the status was POSTed to {url_file.read_text().strip()!r}, expected {expected!r}"
)
return (json.loads(posted.read_text()) if posted.exists() else None), r
DOCS_ONLY = 'printf "docs/a.md\\ndocs/b.md\\n"\n'
def test_a_FAILING_enumeration_withholds_the_exemption_even_when_stdout_looks_docs_only(tmp_path):
"""The mutation that previously survived: ignore the exit status, trust stdout."""
posted, r = _run_classify(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY + "exit 1\n")
assert posted is not None, f"the job posted no status at all: {r.stderr}"
assert posted["state"] == "pending", (
f"a docs-only exemption was granted from the stdout of a script that FAILED "
f"(state={posted['state']}, desc={posted['description']!r}) — the exit status is not being "
"checked"
)
def test_workflow_positive_control_the_same_output_with_exit_0_DOES_exempt(tmp_path):
"""Without this, the test above could pass for any unrelated reason and prove nothing."""
posted, r = _run_classify(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY + "exit 0\n")
assert posted is not None, f"the job posted no status at all: {r.stderr}"
assert posted["state"] == "success", (
"the positive control failed, so the negative test above cannot be trusted to be measuring "
f"the exit status at all (state={posted['state']}, desc={posted['description']!r})"
)
def test_a_crashing_enumeration_also_withholds_the_exemption(tmp_path):
posted, _ = _run_classify(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY + "kill -TERM $$\n")
assert posted is not None
assert posted["state"] == "pending"
def test_a_MISSING_enumeration_script_withholds_the_exemption(tmp_path):
"""A PR whose BASE predates the script's introduction. It must post an actionable `pending`
rather than dying with no status — an absent required check blocks the merge either way, but a
stalled PR with no explanation is how a gate acquires a reputation for being flaky.
"""
posted, r = _run_classify(tmp_path, None)
assert posted is not None, f"the job posted no status at all: {r.stderr}"
assert posted["state"] == "pending"
def _emitting(*paths):
body = "".join(f'printf "%s\\n" "{p}"\n' for p in paths)
return "#!/usr/bin/env bash\n" + body + "exit 0\n"
def test_a_code_file_defeats_the_docs_only_exemption(tmp_path):
"""The workflow's own classification — deliberately NOT shared with the hook, whose allow-list
is wider because a match there falls through to a human prompt rather than a green status.
"""
posted, _ = _run_classify(tmp_path, _emitting("docs/a.md", "ErsatzTV/Program.cs"))
assert posted is not None
assert posted["state"] == "pending"
def test_a_BOT_pr_touching_a_protected_path_is_NOT_exempt(tmp_path):
"""`PROTECTED` guards the BOT exemption specifically, and mutation testing is how that got
stated correctly. The first version of this test used a docs-only+protected file list and
passed even with the `PROTECTED` clause deleted — `PROTECTED` (`.claude/ .gitea/ .husky/
scripts/ docker/ci/`) and `DOCS_ONLY` (`docs/`, root `*.md`) are DISJOINT, so on the docs-only
path that clause can never fire and the `DOCS_ONLY` check was doing all the work. The test
looked like it covered the self-exemption hole and covered nothing.
Renovate lands patch bumps unattended via Gitea's own auto-merge, so a bot PR that edits the
gate, CI, the hooks, or the scripts they call is the one path where an unreviewed change to the
merge gate could actually merge itself.
"""
posted, _ = _run_classify(
tmp_path, _emitting("ErsatzTV/Program.cs", "scripts/pr-changed-files.sh"), author="renovate"
)
assert posted is not None
assert posted["state"] == "pending", (
"a bot PR editing the shared enumeration was auto-exempted — a PR that weakens the merge "
"gate must never be able to exempt itself from the merge gate"
)
def test_bot_positive_control_a_plain_bot_pr_IS_exempt(tmp_path):
"""Proves the test above measures `PROTECTED` and not merely 'bot PRs are never exempt'."""
posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props"), author="renovate")
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"
# --- Route 2: a bot ACCOUNT does not attribute the CODE (ersatztv#698) --------------------------
#
# `AUTHOR` is `pull_request.user.login` — the PR's CREATOR, which is immutable. The head a PR points
# at is not. Force-push application code onto an open Renovate branch and the PR is still authored by
# `renovate`, still touches no protected path, and was exempted. Checking the PUSHER instead does not
# help: a git author/committer is self-asserted text. So the exemption is constrained by what a
# dependency bump can legitimately BE.
#
# The allow-list is measured, not guessed: across all 11 Renovate PRs this repo has ever had, the
# paths touched were `Directory.Packages.props` (10) and `.config/dotnet-tools.json` (1). The one
# historical outlier, PR #20, touched a `.csproj` AND two C# files — and received an unattended bot
# exemption for a source change.
def test_a_BOT_pr_carrying_a_CODE_file_is_NOT_exempt(tmp_path):
"""Route 2, stated as a behaviour: the hijacked-branch case."""
posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), author="renovate")
assert posted is not None
assert posted["state"] == "pending", (
"a PR authored by the renovate account was exempted while changing a C# file — the bot "
"identity is being read as attribution for code it did not write"
)
def test_a_BOT_pr_mixing_a_manifest_WITH_code_is_NOT_exempt(tmp_path):
"""The realistic shape of the attack: keep the manifest edit so the PR still looks like a bump,
and smuggle the code alongside it. A rule that asked 'does it touch a manifest' rather than 'is
EVERY path a manifest' would exempt this."""
posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props", "ErsatzTV/Program.cs"), author="renovate")
assert posted is not None
assert posted["state"] == "pending", (
"a manifest edit was enough to carry a C# file through the bot exemption — the allow-list "
"is being applied as 'any' rather than 'all'"
)
def test_bot_positive_control_a_REAL_dependency_bump_IS_still_exempt(tmp_path):
"""Renovate uses platformAutomerge, so breaking this deadlocks every dependency PR. The negative
tests above prove nothing if the exemption no longer works at all."""
posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props"), author="renovate")
assert posted is not None
assert posted["state"] == "success", (
f"a plain dependency bump lost its exemption ({posted['description']!r}) — every Renovate PR "
"would now stall waiting on a human verdict"
)
@pytest.mark.parametrize("path", ["Directory.Packages.props", ".config/dotnet-tools.json"])
def test_every_manifest_in_the_allow_list_is_actually_exempt(tmp_path, path):
"""Pin the SET, not one example of it. A test that only ever exercises Directory.Packages.props
cannot see a typo in any of the other three alternations."""
posted, _ = _run_classify(tmp_path, _emitting(path), author="renovate")
assert posted is not None, path
assert posted["state"] == "success", f"{path} is in the allow-list but was not exempted"
def test_the_manifest_allow_list_is_ANCHORED(tmp_path):
"""Unanchored, `Directory.Packages.props` would match `evil/Directory.Packages.props.cs`. Same
class as the two DOCS_ONLY anchoring tests above, which is why it is tested the same way."""
posted, _ = _run_classify(tmp_path, _emitting("evil/Directory.Packages.props.cs"), author="renovate")
assert posted is not None
assert posted["state"] == "pending", "the manifest allow-list matched mid-path"
def test_a_BOT_docs_only_pr_is_STILL_exempt_via_the_docs_rule(tmp_path):
"""The regression the restructure exists to prevent.
Written as an `elif` chain, a Renovate PR touching only `docs/` enters the bot branch, fails the
manifest test, and never reaches the docs-only branch — silently withdrawing an exemption the
docs-only rule grants on its own merits for ANY author. The predicates are therefore evaluated
independently and the decision made afterwards.
"""
posted, _ = _run_classify(tmp_path, _emitting("docs/note.md"), author="renovate")
assert posted is not None
assert posted["state"] == "success", (
"a docs-only PR lost its docs-only exemption merely because its author is a bot — the "
"exemptions are chained rather than composed"
)
def test_a_BOT_pr_touching_a_protected_path_is_still_NOT_exempt(tmp_path):
"""PROTECTED must keep outranking both exemptions, including the manifest one."""
posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props", "scripts/evil.sh"), author="renovate")
assert posted is not None
assert posted["state"] == "pending"
# --- Route 3: an inherited `success` is re-derived unless it is an ALLOW-LISTED reviewer's verdict
# (ersatztv#742; `attributable` alone is the weaker test that governs an inherited `failure`) -------
#
# The short-circuit used to exit on ANY existing `success`, so an exemption this job wrote was
# indistinguishable from a verdict a human wrote. Obtained once, a forged success was thereafter
# accepted unchanged on every later run, because the guard exited before looking at the PR, the base,
# the author or the files.
#
# MEASURED on Gitea 1.25.4, on the COMBINED endpoint this job reads: a status POSTed with a user
# credential carries `.creator.login` (`timothy`), one POSTed by an Actions job carries
# `"creator": null`. Both halves are required, and the test is written in the POSITIVE direction —
# short-circuit only on something identified as a human verdict — so an unrecognised shape is
# re-derived rather than trusted.
def test_a_MACHINE_written_exemption_success_is_RE_DERIVED_not_inherited(tmp_path):
"""Route 3. The status looks exactly like one this job writes: creator null, `Exempt:` wording.
The PR now carries a C# file, so re-deriving must downgrade it to `pending`. Inheriting it would
leave a forged exemption standing forever.
"""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator=None,
status_desc="Exempt: docs-only change (no code, no protected path)",
)
assert posted is not None, (
"an existing machine-written success was inherited unchanged — this is route 3, and it is "
"how a forgery obtained once survives every subsequent run"
)
assert posted["state"] == "pending"
def test_a_success_with_NO_creator_but_a_VERDICT_LOOKING_description_is_re_derived(tmp_path):
"""Isolates the creator half. Both conditions are required; either alone is forgeable by the
other party. If a future Gitea populates `creator` for Actions, the description half still
fails — the guard degrades toward re-deriving, never toward trusting."""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator=None,
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
)
assert posted is not None, "a creatorless status was trusted on the strength of its description"
assert posted["state"] == "pending"
def test_a_success_with_a_creator_but_an_EXEMPT_description_is_re_derived(tmp_path):
"""Isolates the description half, the mirror of the test above."""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Exempt: docs-only change (no code, no protected path)",
)
assert posted is not None, "a status was trusted on the strength of its creator alone"
assert posted["state"] == "pending"
# --- ersatztv#742: WHOSE creator counts -------------------------------------------------------
#
# "Non-null creator" proved only that a status was POSTed with SOME user credential. `RENOVATE_TOKEN`
# is a `write:repository` PAT of the `renovate` bot; it cannot be scoped down the way #697 scoped the
# registry credential, and secrets are a per-repo STORE, so any workflow can reference it. A status
# POSTed with it reads back `creator: renovate` — non-null, and therefore inherited as a verdict.
@pytest.mark.parametrize("creator", ["renovate", "someone-else"])
def test_a_verdict_shaped_SUCCESS_from_a_NON_allowlisted_account_is_RE_DERIVED(tmp_path, creator):
"""The route this issue closes. Everything else about the row is a perfect verdict: a real
creator, a `Review-verdict:` description recording THIS base. Only the account is wrong.
Paired with a C# file so re-deriving is visible as a downgrade to `pending`; inheriting would
leave the forgery standing on every later run, which is the property that made route 3 durable.
`success` ONLY — see the `failure` test below, which pins the opposite behaviour deliberately.
"""
posted, r = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator=creator,
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
)
assert posted is not None, (
f"a verdict-shaped success written by '{creator}' was inherited — non-null creator is being "
f"read as 'a reviewer wrote this'. Log:\n{r.stdout[-900:]}"
)
assert posted["state"] == "pending"
# THE DIAGNOSTIC IS PART OF THE CONTRACT, not decoration. It is the only thing that says so when
# a reviewer has been added to the repo and forgotten from `H10_REVIEWERS`, and a tidy-up that
# drops any of its three values must redden something. Asserted on the LOG LINE the job always
# reaches, not on a separate annotation: the annotation that used to carry this was withdrawn
# after cold review measured that deleting it left the whole suite green.
assert creator in r.stdout, (
f"the diagnostic does not name the account whose verdict was discarded. Log:\n{r.stdout[-1200:]}"
)
# The VALUE, not the word. An earlier draft asserted `"H10_REVIEWERS" in stdout`, which the
# remediation sentence satisfies on its own — so deleting the allow-list from the diagnostic left
# it green. Derived from the shipped literal rather than restated here.
assert f"allow-list='{_h10_reviewers_literal()}'" in r.stdout, (
"the diagnostic does not print the accepted set, so a reviewer left off it cannot tell why "
f"their verdict was re-derived. Log:\n{r.stdout[-1200:]}"
)
assert "add them to H10_REVIEWERS" in r.stdout, (
f"the diagnostic does not say how to fix it. Log:\n{r.stdout[-1200:]}"
)
@pytest.mark.parametrize("creator", ["renovate", "someone-else"])
def test_an_off_list_FAILURE_is_LEFT_ALONE_even_on_an_EXEMPT_PR(tmp_path, creator):
"""THE FAIL-OPEN A SYMMETRIC ALLOW-LIST SHIPS, and the reason `failure` is treated differently.
Scenario: a real reviewer (`alice`) was added to the repo and forgotten from `H10_REVIEWERS`.
They post `Review-verdict: BLOCKED`. The PR is docs-only, so it is EXEMPT. If the allow-list
governed `failure` too, every guard downstream declines in turn — the short-circuit
(`ex_human=no`), the mid-run abstain (the triple never changed), and the post-write repair (the
row predates the high-water mark) — and the job posts an exemption `success` over an explicit
human rejection.
Deliberately paired with a DOCS-ONLY file list: with a C# file the job would post `pending`
anyway and the test would pass for the wrong reason, measuring nothing.
The asymmetry costs nothing #742 bought. An inherited `failure` cannot green anything; the worst
a forged one achieves is a stall, and `post-review-verdict.sh` POSTs unconditionally, so any
human clears it in one command.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:failure",
status_creator=creator,
status_desc="Review-verdict: BLOCKED @ a9e3e23 (base: main)",
)
assert r.returncode == 0, r.stderr
assert posted is None, (
f"an exemption was posted over a BLOCKED verdict from '{creator}' — the allow-list is "
f"governing `failure`, which turns a rejection green. Posted: {posted}. "
f"Log:\n{r.stdout[-900:]}"
)
@pytest.mark.parametrize("creator", ["timothy2", "xtimothy", "TIMOTHY", "timoth", " timothy"])
def test_an_account_that_merely_RESEMBLES_an_allowlisted_login_is_RE_DERIVED(tmp_path, creator):
"""Membership must be whole-value equality, not a substring, prefix or case-folded test.
Gitea logins are first-come, so `timothy2` is registerable by anyone; a containment test would
hand them the gate. `TIMOTHY` is the case arm — Gitea logins are case-insensitive for LOGIN but
the API returns the stored form, and accepting a fold would widen the set to whatever Gitea
happens to consider equal rather than to what this file lists.
"""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator=creator,
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
)
assert posted is not None, f"'{creator}' was accepted as the allow-listed reviewer"
assert posted["state"] == "pending"
def test_the_RACED_verdict_check_still_counts_a_NON_allowlisted_creator(tmp_path):
"""THE POLARITY TEST, and the reason the allow-list was not applied everywhere `creator` is read.
NOT for the obvious reason, which cold review showed to be unchecked: a genuine reviewer is on
`$H10_REVIEWERS` by construction, so membership here would still count every real verdict, and an
off-list row is one the job deliberately overwrote a few lines earlier.
The reason is the MISCONFIGURATION case — a second human account added to the repo and forgotten
from the literal — which is the only way an off-list row is ever a real verdict. This is the last
net before a green stands, and the errors are not symmetric: a repair to `pending` is recoverable
by re-posting, a wrongly-standing `success` on an unreviewed head is not. The cost of being broad
is real and is documented at the site: any verdict-shaped row landing in the write window trips
the sticky sentinel and costs that head its exemption until a human clears it.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
history_mode="human-after-post",
history_creator="renovate",
)
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"the exemption was not repaired after a non-allow-listed account raced the write — the "
f"allow-list has been applied to the raced check too. Posts: {seq}. Log:\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success"
assert seq[1]["state"] == "pending"
assert posted is not None and posted["state"] == "pending"
def test_the_MID_RUN_abstain_still_fires_for_a_NON_allowlisted_creator(tmp_path):
"""The SECOND mirror-image site, and the one the first draft of #742 got wrong.
`read_existing_verdict` is called twice. The first call decides "may this be INHERITED" and is
correctly allow-list-gated. The second — the last-moment re-read immediately before the POST —
asks the OTHER question: "did a reviewer post a verdict while we were classifying?" Narrowing
that one does not make it stricter, it makes the job stop abstaining: it posts its exemption
`success` straight over the row instead.
The post-write repair usually catches that, but it is skipped entirely whenever the high-water
mark could not be established (`max_id_before=-1`, a `::warning::` and nothing more), so the
green would stand over a `BLOCKED`. Fail-toward-SUCCESS, at the exact site the design says must
stay broad.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="appears-on-read:2",
midrun_creator="renovate",
)
assert r.returncode == 0, r.stderr
assert posted is None, (
"the job posted over a verdict that landed mid-run from a non-allow-listed account — the "
f"allow-list has been applied to the mid-run abstain as well. Posted: {posted}. "
f"Log:\n{r.stdout[-900:]}"
)
def test_a_verdict_REPLACING_a_pre_existing_row_mid_run_still_makes_the_job_ABSTAIN(tmp_path):
"""The REPLACEMENT half of the mid-run abstain, and the clause a presence test cannot reach.
The abstain used to be free: both reads computed an identical predicate, so "fired at the second
and not the first" meant "arrived in between". Splitting `ex_human` from `ex_attributable` broke
that, and the repair is an explicit comparison against the state/creator/description triple
snapshotted at the first read. `[ -z "$pre_state" ]` — abstain only when the first read saw
NOTHING — is the plausible wrong version, and every other mid-run test starts from an empty head,
so nothing distinguishes the two.
This is the ordering that actually co-occurs with #742: the head ALREADY carries an off-list
`Review-verdict:` success (which is why the job is re-deriving at all), and a real reviewer posts
`BLOCKED` over it while the job classifies. Under the presence-only version `pre_state` is
non-empty, the abstain is skipped, and the docs-only exemption is posted straight over the
rejection — with the post-write repair blind to it whenever the high-water mark is unestablished.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="appears-on-read:2",
pre_row="renovate|success|Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
midrun_creator="timothy",
)
assert r.returncode == 0, r.stderr
assert posted is None, (
"the job posted an exemption over a verdict that REPLACED a pre-existing row mid-run — the "
"abstain is testing presence ('was the head empty before?') rather than change. "
f"Posted: {posted}. Log:\n{r.stdout[-1200:]}"
)
def test_every_test_name_cited_by_the_exemption_provenance_record_EXISTS():
"""A record that names a test as its mutation proof must name one that can be run.
This is not hypothetical: renaming a test while splitting the `success`/`failure` cases left
`ci.exemption-provenance` citing `..._status_from_...` when the tree had `..._SUCCESS_from_...`,
found by cold review. A citation that cannot be resolved is worse than none — it reads as a
checked proof and sends the next reader looking for a test that does not exist.
Deliberately scoped to the ONE record that carries this change's proofs, and derived from the
record's own text rather than from a list restated here, so a newly cited name is covered the
moment it is written.
"""
record = REPO_ROOT / "docs" / "decisions" / "records" / "ci" / "exemption-provenance.md"
assert record.exists(), f"{record} is missing — the record this suite is the proof for"
cited = set(re.findall(r"`(test_[A-Za-z0-9_]+)`", record.read_text()))
assert cited, "the record cites no test by name; if that is deliberate, delete this guard"
defined = set(re.findall(r"^def (test_[A-Za-z0-9_]+)", Path(__file__).read_text(), re.MULTILINE))
for other in sorted(Path(__file__).parent.glob("test_*.py")):
defined |= set(re.findall(r"^def (test_[A-Za-z0-9_]+)", other.read_text(), re.MULTILINE))
missing = sorted(cited - defined)
assert not missing, (
f"ci.exemption-provenance cites {missing}, which no test in scripts/tests defines — the "
"record names a mutation proof nobody can run"
)
_PRE = "renovate|success|Review-verdict: MERGEABLE @ a9e3e23 (base: main)"
@pytest.mark.parametrize(
"clause,midrun_row",
[
("description", "renovate|success|Review-verdict: BLOCKED @ a9e3e23 (base: main)"),
("creator", "timothy|success|Review-verdict: MERGEABLE @ a9e3e23 (base: main)"),
("state", "renovate|failure|Review-verdict: MERGEABLE @ a9e3e23 (base: main)"),
],
)
def test_EACH_clause_of_the_changed_row_comparison_is_load_bearing(tmp_path, clause, midrun_row):
"""One case per field, because a triple whose cases all move all three fields pins none of them.
The mid-run row here differs from the pre-existing one in EXACTLY the named field, so dropping
that field's comparison makes the job miss the change, skip the abstain, and post its docs-only
exemption over the row. Reviewed cold: with only the two "everything moves at once" cases, three
single-clause deletions and a reduction of the whole triple to one comparison all shipped green.
The pre-existing row is an OFF-LIST `success` so the first read does not short-circuit — which is
also the realistic setup, since an off-list row is exactly why the job would be re-deriving.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="appears-on-read:2",
pre_row=_PRE,
midrun_row=midrun_row,
)
assert r.returncode == 0, r.stderr
assert posted is None, (
f"the mid-run row differed from the pre-existing one only in its {clause}, and the job did "
f"not notice — that clause of the changed-row comparison is not being evaluated. "
f"Posted: {posted}. Log:\n{r.stdout[-1200:]}"
)
def test_an_allow_list_with_SEVERAL_entries_honours_the_FIRST_one(tmp_path):
"""Multi-entry semantics, which every other test leaves unexercised.
All the behavioural cases run the singleton `H10_REVIEWERS="timothy"`, under which a loop that
can only ever match its LAST entry — drop the `break`, add an `else ex_human=no` — behaves
identically to a correct one. That matters more than it looks: the `::warning::` this change adds
tells an operator whose reviews are being discarded to ADD THEMSELVES to the list, so a
two-entry list is the first thing the fix produces, and the first entry is the one that breaks.
Docs-only on purpose: if the verdict is inherited the job posts nothing, and if it is not, the
exemption `success` is posted. So `posted is None` IS the inheritance.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:success",
status_creator="alice",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
reviewers="alice timothy",
)
assert r.returncode == 0, r.stderr
assert posted is None, (
"a verdict from the FIRST entry of a two-entry H10_REVIEWERS was not inherited — membership "
f"is not scanning the whole list. Posted: {posted}. Log:\n{r.stdout[-900:]}"
)
def test_the_multi_entry_control_the_LAST_entry_is_honoured_too(tmp_path):
"""Without this, the test above could pass for any reason unrelated to list position."""
posted, _ = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
reviewers="alice timothy",
)
assert posted is None, "the positive control failed, so the first-entry test proves nothing"
def test_an_account_on_NEITHER_entry_is_still_re_derived(tmp_path):
"""The negative control for the two above: widening the list must not widen it to everyone."""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="mallory",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
reviewers="alice timothy",
)
assert posted is not None and posted["state"] == "pending", (
"an account absent from a two-entry allow-list was inherited"
)
@pytest.mark.parametrize(
"shape,creator,desc",
[
("a null creator (an Actions-token write)", None, "Review-verdict: BLOCKED @ a9e3e23 (base: main)"),
("a description that is not a verdict", "timothy", "blocked by hand"),
("a verdict recorded against ANOTHER base", "timothy", "Review-verdict: BLOCKED @ a9e3e23 (base: other)"),
],
)
def test_an_UNATTRIBUTABLE_failure_is_still_RE_DERIVED(tmp_path, shape, creator, desc):
"""`failure` needs only attributability — but "only attributability" is still a REQUIREMENT.
The clause is `[ "$ex_attributable" = yes ] && [ "$ex_state" = "failure" ]`, and the comment at
the site says plainly that widening it to a bare `[ "$ex_state" = failure ]` is deliberately NOT
done: that would quietly opt `failure` out of the base binding, which exists because a verdict
earned against another base is not a verdict for this diff. Cold cross-family review pointed out
that every existing `failure` case supplied a well-formed, base-matching, non-null-creator row,
so the bare-widening mutation passed the whole suite.
Docs-only on purpose: re-deriving is then visible as an exemption `success` being posted, where
short-circuiting would post nothing at all.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:failure",
status_creator=creator,
status_desc=desc,
)
assert r.returncode == 0, r.stderr
assert posted is not None, (
f"a `failure` with {shape} was left alone — the failure short-circuit has been widened past "
f"attributability, taking the base binding with it. Log:\n{r.stdout[-1200:]}"
)
assert posted["state"] == "success"
def _h10_reviewers_literal():
"""The allow-list as the workflow actually declares it — read from the shipped file, never
restated here, so a test cannot pass against a value the gate does not use."""
body = _classify_step()["run"]
# findall + a SINGLE-ASSIGNMENT assertion, not `search`. `search` returns the FIRST assignment
# while the shell runs the LAST one executed, so a later or conditional reassignment would leave
# this guard validating a value the gate does not use — the same extractor-vs-classifier gap
# ersatztv#774 hit on `POS_RE`.
found = re.findall(r'^\s*H10_REVIEWERS="([^"]*)"\s*$', body, re.MULTILINE)
assert found, "review-verdict.yml no longer declares H10_REVIEWERS as a double-quoted literal"
assert len(found) == 1, (
f"H10_REVIEWERS is assigned {len(found)} times ({found}); the shell uses the last one "
"executed, so this guard can no longer tell which value the gate runs with"
)
return found[0]
def test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty():
"""The membership loop word-splits `$H10_REVIEWERS`, so pathname expansion applies to each entry.
An entry containing `*`, `?` or `[` would be expanded against the runner's CWD before the
comparison — normally to itself (no match leaves the word alone), but to a FILENAME whenever one
happens to match, which silently changes who the gate accepts. Cheaper to forbid the characters
than to reason about the working directory.
Empty is checked too: `for rv in ""` iterates zero times, so an emptied list would make every
verdict non-inheritable and re-post `pending` over real human verdicts forever.
A FILE-TEXT LINT, deliberately, and it claims nothing more. It reads the literal out of the
shipped workflow so it cannot drift from the gate, but it does not execute the membership loop;
the behavioural consequences above are argued from the shell's semantics, not measured here.
"""
reviewers = _h10_reviewers_literal()
assert reviewers.strip(), "H10_REVIEWERS is empty — no verdict could ever be inherited"
for entry in reviewers.split():
assert re.fullmatch(r"[A-Za-z0-9._-]+", entry), (
f"H10_REVIEWERS entry {entry!r} is not a plain login; `*`, `?` and `[` are glob "
"metacharacters in the membership loop's word-splitting"
)
def test_the_allowlisted_reviewer_in_the_TESTS_is_the_one_the_WORKFLOW_ships():
"""De-shadowing, not a safety net — and the distinction was a review finding.
The BEHAVIOURAL tests do not need this: change the literal to `alice` and
`test_a_REAL_human_verdict_is_still_NEVER_overwritten` reddens on its own, because a `timothy`
verdict would stop being inherited. What this pins is the set of tests that use
`status_creator="timothy"` to reach a DIFFERENT clause — the base-marker cases below. If the
literal drifted they would start failing via the membership clause instead, still green, while
the clause they were written for went unexercised.
"""
assert "timothy" in _h10_reviewers_literal().split(), (
"the tests' allow-listed account is no longer in the workflow's H10_REVIEWERS, so every "
"inheritance test above is asserting against a value the gate does not use"
)
def test_an_UNRECOGNISED_success_shape_is_re_derived_rather_than_trusted(tmp_path):
"""The direction-of-test check. Written as 'skip if it looks machine-written', anything novel
would fall through to TRUSTED. Written as 'skip only if positively identified as human', novel
shapes are re-derived. This test fails under the first spelling and passes under the second."""
posted, _ = _run_classify(
tmp_path, _emitting("ErsatzTV/Program.cs"), status_mode="existing:success", status_creator="", status_desc=""
)
assert posted is not None
assert posted["state"] == "pending"
@pytest.mark.parametrize("existing", ["success", "failure"])
def test_a_REAL_human_verdict_is_still_NEVER_overwritten(tmp_path, existing):
"""The property the short-circuit exists for, which the fix must not break.
SCOPE, because the test name says NEVER: this is about a verdict already VISIBLE AT THE FIRST
READ. A verdict landing later, inside this run's own write window, is a different route with its
own (incomplete) guard — see ersatztv#849.
`failure` is the sharp case: that is a human saying NO, and an exemption posted over it would
turn a rejection into a merge. Deliberately paired with a docs-only file list, so the job WOULD
have posted an exemption `success` had it not short-circuited — without that, a passing test
would prove only that nothing was posted for some unrelated reason.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode=f"existing:{existing}",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
)
assert r.returncode == 0, r.stderr
assert posted is None, (
f"overwrote a human '{existing}' verdict written by timothy — the never-overwrite property "
"has been lost while fixing route 3"
)
# NO "the diagnostic did not fire" ASSERTION HERE. An earlier version of this test carried one,
# captioned as pinning a guard on the annotation — but both parametrizations exit at a
# short-circuit two guards earlier and never reach the diagnostic at all, so the assertion was
# true by construction and could not fail. The annotation it guarded has since been withdrawn;
# the lesson is kept because a caption claiming a proof is worse than no assertion.
def test_a_PENDING_status_from_a_previous_run_is_replaced_normally(tmp_path):
"""`pending` was never short-circuited and must stay that way, or a PR that becomes exempt after
an earlier pending run could never reach `success`."""
posted, _ = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:pending",
status_creator=None,
status_desc="Awaiting review verdict for a9e3e23",
)
assert posted is not None
assert posted["state"] == "success"
@pytest.mark.parametrize("path", ["web/package.json", "web/package-lock.json"])
def test_the_npm_manifests_are_NOT_exempt(tmp_path, path):
"""Cross-family review called an earlier draft's inclusion of these a Blocker, correctly.
`renovate.json` sets `enabledManagers: ["nuget", "github-actions", "dockerfile"]`, so Renovate does
not manage npm here at all — the entry bought nothing. Meanwhile `package.json` carries `scripts`
that CI EXECUTES (`npm ci`, `npm run build`), so exempting it lets a hijacked bot branch run
arbitrary shell in CI while every path still matches a "manifest" allow-list. Widening an exemption
to a code-execution vector for no operational benefit is strictly worse than the hole being closed.
"""
posted, _ = _run_classify(tmp_path, _emitting(path), author="renovate")
assert posted is not None
assert posted["state"] == "pending", (
f"{path} was exempted; package.json scripts execute in CI, and npm is not a managed ecosystem in this repo"
)
# --- The `grep -q` + `pipefail` inversion (found by cross-family review of this PR) ---------------
#
# `grep -q` exits at its FIRST match, so the upstream writer takes SIGPIPE (141) once the path list
# exceeds the pipe buffer. Under `set -o pipefail` the pipeline is then a FAILURE even though grep
# MATCHED, inverting every guard built on `printf … | grep -q`. Reproduced at 171KB / 1901 paths,
# comfortably inside the enumerator's 2000-file cap.
#
# These are the regression guards, and they are the tests the original round did not have: every
# earlier case used a handful of short paths, far below the buffer, so the whole class was invisible.
# The construct PREDATES #698, so `main` carried this hole with no retarget or bot account required.
def _many_docs(n=1900):
return [f"docs/{'d' * 40}-{i:040d}.md" for i in range(n)]
def test_a_LARGE_pr_with_a_code_file_is_not_classified_docs_only(tmp_path):
"""The code file goes FIRST so `grep -qv` matches immediately and the writer is left with ~171KB
still to push — the exact shape that produced exit 141 and `docs_only=yes`."""
posted, r = _run_classify(tmp_path, _emitting("A.cs", *_many_docs()))
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending", (
"a PR containing A.cs was exempted as docs-only because the classification pipeline was "
f"inverted by SIGPIPE on a large file list (desc={posted['description']!r})"
)
def test_a_LARGE_pr_touching_a_PROTECTED_path_still_voids_the_exemptions(tmp_path):
"""The worse direction: here the inversion makes the PROTECTED guard MISS, so a PR editing the
gate's own workflow falls through to the docs-only exemption and self-exempts."""
posted, r = _run_classify(tmp_path, _emitting(".gitea/workflows/review-verdict.yml", *_many_docs()))
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending", (
"a PR editing .gitea/workflows/ was exempted — the protected-path guard was skipped on a "
f"large file list (desc={posted['description']!r})"
)
def test_a_LARGE_bot_pr_with_a_code_file_is_not_manifest_exempt(tmp_path):
"""The same inversion reached through the third guard added by this change."""
posted, r = _run_classify(tmp_path, _emitting("A.cs", *_many_docs()), author="renovate")
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending"
def test_positive_control_a_LARGE_genuinely_docs_only_pr_IS_still_exempt(tmp_path):
"""Without this the three tests above could pass because large lists now fail outright, which
would be a merge deadlock rather than a fix."""
posted, r = _run_classify(tmp_path, _emitting(*_many_docs()))
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "success", (
f"a large but genuinely docs-only PR lost its exemption (desc={posted['description']!r})"
)
def test_a_human_verdict_landing_MID_RUN_is_not_overwritten(tmp_path):
"""ersatztv#706, the half that IS mitigated here.
The first status read finds nothing, so the job proceeds to classify. A reviewer then posts a human
`failure` (BLOCKED). Without the re-read immediately before the POST, this docs-only PR would post
an exemption `success` OVER an explicit human rejection — turning a "no" into a merge, which is the
worst outcome in this class.
Note precisely what this does and does not prove: it pins the re-read, not atomicity. A verdict
landing between the re-read and the POST is still lost — there is no compare-and-set on Gitea's
status API. That remainder is #706, not a claim made here.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="appears-on-read:2")
assert r.returncode == 0, r.stderr
assert posted is None, (
"an exemption success was posted over a human BLOCKED verdict that landed while the job was "
"classifying — the pre-POST re-read is missing or ineffective"
)
def test_positive_control_no_late_verdict_still_posts_normally(tmp_path):
"""Without this, the test above would pass against a job that had simply stopped posting."""
posted, _ = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="none")
assert posted is not None and posted["state"] == "success"
# --- The PROTECTED branch must actually EXECUTE, not merely coincide with the right answer --------
#
# Round-3 review found `count_matching` being called before its definition, so it was
# `command not found` on every run and the PROTECTED branch never fired. Three "protected path" tests
# passed anyway, because a protected path is also not a manifest and not docs-only, so the job reached
# `pending` down a different route. Asserting the STATE could not see it; the guard was dead and the
# suite was green.
#
# The lesson generalises: when several branches produce the same outcome, asserting the outcome cannot
# tell you which branch ran. Assert the DISCRIMINATOR — here the reason string the branch writes.
def test_a_protected_path_is_rejected_BY_THE_PROTECTED_BRANCH(tmp_path):
posted, r = _run_classify(tmp_path, _emitting("scripts/evil.sh", "docs/a.md"))
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending"
# The DISCRIMINATOR is the job's `Decision:` line, not the status description: for `pending` the
# description is always "Awaiting review verdict for <sha>", identical no matter which branch
# produced it. A first draft of this test asserted on the description and failed against a WORKING
# guard — the assertion has to be aimed at something that actually differs per branch.
assert "protected" in r.stdout.lower(), (
"the PR was not exempted, but NOT via the protected-path branch — it reached the same verdict "
f"by another route, so that guard may be dead. Decision log:\n{r.stdout[-800:]}"
)
def test_a_protected_path_defeats_the_BOT_exemption_by_the_protected_branch(tmp_path):
"""A bot PR whose every path IS a manifest, plus one protected path. Without a live PROTECTED
branch this still lands on `pending` (the protected file is not a manifest), so again only the
reason string distinguishes a working guard from a dead one."""
posted, r = _run_classify(
tmp_path, _emitting("Directory.Packages.props", ".gitea/workflows/renovate.yml"), author="renovate"
)
assert posted is not None
assert posted["state"] == "pending"
assert "protected" in r.stdout.lower(), (
f"the bot exemption was refused, but not by the PROTECTED branch:\n{r.stdout[-800:]}"
)
@pytest.mark.parametrize(
"paths,author",
[
(("docs/a.md",), "timothy"),
(("Directory.Packages.props",), "renovate"),
(("ErsatzTV/Program.cs",), "timothy"),
(("scripts/evil.sh",), "timothy"),
],
)
def test_the_classify_step_runs_without_SHELL_ERRORS(tmp_path, paths, author):
"""A cheap, general trap-catcher for the whole step.
`command not found`, `integer expression expected`, `unbound variable` — each of these silently
skips a branch inside an `if`/`elif` (the condition just evaluates false) while the job exits 0 and
posts a plausible status. That is precisely how the dead PROTECTED guard survived a green suite.
Scope, stated so this is not mistaken for more than it is: it catches guards that die NOISILY on
stderr. It is NOT a general liveness check — a clean mutation such as hardcoding `n_protected=0`
emits none of these and passes here. That case is covered by the branch-discriminator test above,
which is the actual liveness guard; this one is the cheap net for the whole error-emitting family.
"""
_, r = _run_classify(tmp_path, _emitting(*paths), author=author)
bad = [
ln
for ln in r.stderr.splitlines()
if "command not found" in ln
or "integer expression expected" in ln
or "unbound variable" in ln
or "syntax error" in ln
]
assert not bad, f"the classification step emitted shell errors, so a guard is not running: {bad}"
def test_a_human_verdict_formed_against_ANOTHER_BASE_is_not_inherited(tmp_path):
"""Round-4 review: the sha-binding is escapable through the HUMAN verdict path.
Get a genuine `success` on head H while it targets scratch base S (benign diff there), then
retarget H onto `main`, where its diff contains unreviewed code. Creator is real, prefix is real,
so the short-circuit preserved it — a green required check over code nobody reviewed. The
merge-consent hook compares the base, but that is advisory; a merge through the Gitea UI or API
only sees the status.
`post-review-verdict.sh` already records the base it reviewed (`(base: …)`, ersatztv#632); this
asserts the gate actually READS it.
"""
posted, r = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: probe/scratch-base)",
)
assert posted is not None, (
"a human verdict formed against a DIFFERENT base was inherited unchanged — the reviewed diff "
f"is not this PR's diff. Log:\n{r.stdout[-600:]}"
)
assert posted["state"] == "pending"
def test_a_human_verdict_for_THIS_base_is_still_honoured(tmp_path):
"""The positive control the test above needs: matching bases must still short-circuit, or the
check has simply broken every verdict."""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
)
assert posted is None, "a verdict formed against THIS base was re-derived; the base check is too strict"
def test_a_LEGACY_verdict_with_no_recorded_base_is_still_honoured(tmp_path):
"""Verdicts predating ersatztv#632 carry no `(base: …)`. Absent is deliberately not treated as a
mismatch: re-deriving over one would un-approve a genuinely reviewed head. Only a base that is
PRESENT and DIFFERENT is rejected."""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23",
)
assert posted is None
def test_an_APPENDED_base_cannot_override_the_real_one(tmp_path):
"""The description is attacker-influencable by anyone who can POST a status — the registry
credential's own route closed with #697, but `GITEA_TOKEN`, `RENOVATE_TOKEN`, and a
collaborator's own token still can — so the parse must not be trickable into reading a second,
appended base.
This defeated the greedy `##` parse. The implementation no longer parses at all — it requires the
description to END with the exact literal marker AND to contain exactly one marker — so a second
appended marker makes the count 2 and is rejected outright.
"""
posted, r = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: probe/scratch) (base: main)",
)
assert posted is not None, (
"an appended '(base: main)' overrode the real recorded base, so a verdict formed elsewhere was "
f"inherited. Log:\n{r.stdout[-600:]}"
)
assert posted["state"] == "pending"
def test_an_EMPTY_recorded_base_is_treated_as_a_mismatch(tmp_path):
"""`(base: )` is not 'absent' — it is present and not equal to the PR's base, so it fails closed
rather than being waved through by the legacy-verdict allowance."""
posted, _ = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: )",
)
assert posted is not None and posted["state"] == "pending"
def test_a_branch_name_containing_a_PAREN_cannot_truncate_into_the_current_base(tmp_path):
"""Round-5 review, and the sharpest finding of the five: it needs no forgery and no #697.
`main)evil` is a VALID git branch name (`git check-ref-format --branch 'main)evil'` succeeds). A
genuine verdict earned while head H targeted it is written `(base: main)evil)`. Any implementation
that extracts the value by truncating at the first `)` gets exactly `main`, matches a PR that now
targets `main`, and inherits a verdict covering a completely different diff.
An earlier comment in the workflow asserted that a `)` in a branch name "mismatches — safe
direction". That was generalised from `feat/foo)bar` (which does mismatch) and is false for every
branch whose name STARTS with the target base. Hence the rule the code now follows: compare against
the exact expected literal, never parse a value out of attacker- or user-influenced text.
"""
posted, r = _run_classify(
tmp_path,
_emitting("ErsatzTV/Program.cs"),
status_mode="existing:success",
status_creator="timothy",
status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)evil)",
)
assert posted is not None, (
"a verdict recorded against branch 'main)evil' was inherited by a PR targeting 'main' — the "
f"base value is being truncated at the first ')'. Log:\n{r.stdout[-600:]}"
)
assert posted["state"] == "pending"
# --- ersatztv#706: the retarget fence (race 1) and the post-write repair (race 2) -----------------
#
# Race 1 was reproduced LIVE before any of this was written, because the fix turns on which Gitea
# behaviours are real rather than on what the docs imply. Measured on Gitea 1.25.4:
# * `pull_request_target` runs for one PR are NOT auto-cancelled — probe PR #722, run 7520
# (`opened`) ran to completion 20s AFTER run 7521 (`synchronize`) started. The older run finished
# LAST, which is exactly the mechanism by which a stale classification overwrites a fresh one.
# * A NON-CANCELLING concurrency group does not serialize them either: with the group active, runs
# 7528 and 7529 still overlapped and 7528 ended 36s after 7529 began. #706's headline proposal is
# therefore refuted, not merely unattractive.
# * Gitea DOES auto-cancel superseded `push` runs on a branch — a control workflow carrying no
# `concurrency:` key at all showed that — which is why the concurrency-group probe was confounded
# until the control separated the two.
# The fence keys on the timeline's `change_target_branch` COUNT because the branch NAME is
# ABA-vulnerable (`main → S → main` reads `main` at both ends), while the count is monotonic.
def _posted_sequence(tmp_path):
"""Every status POST the job made, in order — not just the last one."""
f = tmp_path / "posted_all.jsonl"
return [json.loads(line) for line in f.read_text().splitlines() if line.strip()] if f.exists() else []
def test_a_RETARGET_DURING_the_run_posts_NOTHING(tmp_path):
"""THE race-1 test. A docs-only PR that would otherwise be exempted is retargeted while the job
classifies, so this run's answer describes a base the PR may no longer target. It must write
nothing at all and leave the field to the successor run the retarget's `edited` event queues."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="moves:0,1")
assert r.returncode == 0, r.stderr
assert posted is None, (
"a stale run posted its classification after the PR was retargeted underneath it — the "
f"retarget fence did not fire. Log:\n{r.stdout[-900:]}"
)
# Assert the DISCRIMINATOR, not just the outcome: several unrelated failures also end in "posted
# nothing", so the state alone cannot tell a working fence from a broken job.
assert "retargeted while this job was classifying" in r.stdout, (
f"nothing was posted, but NOT via the retarget fence. Log:\n{r.stdout[-900:]}"
)
def test_positive_control_a_QUIET_run_still_posts_its_exemption(tmp_path):
"""Without this, the test above passes against a job that simply never posts."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="none")
assert posted is not None and posted["state"] == "success", (
f"an undisturbed docs-only PR lost its exemption. Log:\n{r.stdout[-900:]}"
)
def test_a_PR_retargeted_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt(tmp_path):
"""The fence keys on MOTION during this run, never on "has ever been retargeted".
A PR legitimately retargeted once, long before this run, carries a permanently non-zero event
count. Keying on the count being non-zero — rather than on it CHANGING — would deadlock that PR's
exemption forever, which is a worse failure than the race being fixed.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="stable:2")
assert posted is not None and posted["state"] == "success", (
"a PR with a settled, non-zero retarget history was refused its exemption — the fence is "
f"testing the count's VALUE instead of its MOVEMENT. Log:\n{r.stdout[-900:]}"
)
@pytest.mark.parametrize("mode", ["unreadable", "transport-error"])
def test_an_UNTRUSTED_retarget_count_withholds_the_EXEMPTION(tmp_path, mode):
"""A `success` that cannot be shown to describe the PR's current base must not be written. An
absent required check blocks the merge, which is the safe direction."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode=mode)
assert posted is None, (
"an exemption success was posted even though the retarget count could not be established "
f"({mode}). Log:\n{r.stdout[-900:]}"
)
@pytest.mark.parametrize("mode", ["unreadable", "transport-error"])
def test_an_UNTRUSTED_retarget_count_STILL_LETS_PENDING_THROUGH(tmp_path, mode):
"""The other half, and the one that keeps a timeline outage from taking every PR down with it.
`pending` blocks the merge, so letting it through is the right anti-stranding trade — withholding
it would leave ordinary PRs with no status at all. NOT because it is inert: a generic `pending`
masks a rejection landing in its own write window, gets no post-write verification, and a later
run re-derives it green (ersatztv#849). Immediate block versus later re-derivation risk, not
"buys no safety". Only the exemption is gated.
"""
posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), timeline_mode=mode)
assert posted is not None and posted["state"] == "pending", (
"an ordinary PR was left with NO status because the timeline was unreadable; only the "
f"exemption success should be gated on the count. Log:\n{r.stdout[-900:]}"
)
def test_a_human_verdict_landing_AFTER_the_POST_is_repaired_to_pending(tmp_path):
"""Race 2's remainder. The pre-POST re-read cannot see a verdict that lands between it and the
write, and there is no compare-and-set to make the two one operation. So the write is verified
AFTERWARDS and repaired in the safe direction: silently greening an explicit human BLOCKED is the
worst outcome this gate can produce, and strictly worse than a stall.
The repair is `pending`, never a copy of the human's `failure` — re-posting their state under the
machine credential would attribute a human verdict to this job.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="human-after-post")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"expected the exemption write followed by a repair; the post-write verification did not "
f"fire. Posts: {seq}. Log:\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success"
assert seq[1]["state"] == "pending", f"the raced exemption was not repaired: {seq}"
assert posted is not None and posted["state"] == "pending"
def test_a_PRE_EXISTING_human_row_does_NOT_trigger_a_repair(tmp_path):
"""The false-positive guard, and the reason the check compares ids instead of asking "does a human
verdict exist".
`read_existing_verdict` deliberately declines to honour a human verdict whose recorded base does
not match this PR's — so such a row sits in the history forever. A presence test would fire on it
on EVERY later run, repair each exemption to `pending`, and permanently deadlock a PR that had one
mismatched verdict once. Only rows newer than the high-water mark count.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="stale-human-already-present")
seq = _posted_sequence(tmp_path)
assert len(seq) == 1, (
"an old, already-present human verdict row was mistaken for one that raced this run's write, "
f"so the exemption was repaired away. Posts: {seq}. Log:\n{r.stdout[-900:]}"
)
assert posted is not None and posted["state"] == "success"
# --- ersatztv#711: the PROTECTED list must cover every mirror of the enforcement hooks -----------
def test_a_CODEX_hook_copy_is_a_PROTECTED_path(tmp_path):
"""`.codex/hooks/` is a byte-identical mirror of `.claude/hooks/`, including
`pretooluse-merge-consent.sh`. Editing the `.claude/` copy correctly voids both exemptions;
before this, editing its `.codex/` twin did not — the rule "a PR that can weaken the gate must
not exempt itself from the gate" was written as a path list and the list had gone incomplete.
Latent rather than live today (`.codex/` is untracked and gitignored, and a PR cannot touch a
path that is not in the repo), which is exactly why it needs a test: it becomes live silently,
the moment anyone tracks the directory.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md", ".codex/hooks/pretooluse-merge-consent.sh"))
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending", (
"a PR editing a .codex/ copy of the merge-consent hook was granted the docs-only exemption "
f"(desc={posted['description']!r})"
)
# Assert the DISCRIMINATOR: a non-exempt outcome is reachable by several routes, so the state
# alone cannot show the PROTECTED branch is what rejected it.
assert "protected" in r.stdout.lower(), (
f"rejected, but NOT by the protected-path branch. Decision log:\n{r.stdout[-800:]}"
)
def test_BOTH_hook_directories_are_in_the_PROTECTED_pattern():
"""A structural pin over the list itself. The behavioural test above proves `.codex/` is covered
today; this one states the INVARIANT — the two hook directories are mirrors, so any future edit
that drops one while keeping the other is the bug, not a simplification."""
src = _code_lines(WORKFLOW)
protected = [line for line in src.splitlines() if line.strip().startswith("PROTECTED=")]
assert len(protected) == 1, f"expected exactly one PROTECTED definition, found {protected}"
for d in (r"\.claude/", r"\.codex/"):
assert d in protected[0], (
f"{d} is missing from PROTECTED ({protected[0].strip()!r}) — a directory carrying a copy "
"of the enforcement hooks can exempt itself from the gate it enforces"
)
# --- ersatztv#706, round 2: findings from the cold adversarial review ----------------------------
def test_the_high_water_MARK_is_captured_BEFORE_the_last_moment_re_read():
"""The High finding of round 2, pinned as the ORDERING property it actually is.
The mark was originally taken "as late as possible", just before the POST. That leaves everything
between the last-moment re-read and the mark as a blind gap: a human verdict landing there is
invisible to the re-read (already done) and excluded from the post-write check (its id is BELOW a
mark taken afterwards), so it is overwritten with no repair. The gap spanned the whole retarget
re-count — up to 20 timeline round-trips — not the single round-trip that was being claimed.
Taking the mark FIRST closes the read side: any row newer than the mark is caught either by the
re-read (abstain) or by the post-write check (repair). This is asserted structurally because the
defect is an order, not an output — with the mark late the job still posts and still repairs in
every scenario a stub can pose; only the id arithmetic silently changes.
"""
src = _classify_step()["run"]
# Assert on BOTH the init and the FETCH: keying only on the init line would let a refactor that
# splits initialisation from the read slide the actual round-trip back past the re-read while this
# test stayed green (round-3 review nit).
# The FETCH is now the first bare `page_statuses` CALL (ersatztv#763 replaced the single
# `hist_before=$(gh …)` read with the paged walk). Anchoring on the call and not on the function
# definition matters: the definition sits with the other helpers near the top of the step, so
# keying on it would place the "fetch" far earlier than the round-trip actually happens and this
# assertion would hold vacuously.
assert "\npage_statuses\n" in src, (
"no bare `page_statuses` call site found; the mark is no longer fetched where this test "
"believes it is, and the ordering assertion below would be vacuous"
)
mark = max(src.index("max_id_before=-1"), src.index("\npage_statuses\n"))
# The LAST-MOMENT re-read is the second bare `read_existing_verdict` call.
calls = [i for i in range(len(src)) if src.startswith("read_existing_verdict\n", i)]
assert len(calls) >= 2, f"expected two read_existing_verdict call sites, found {len(calls)}"
assert mark < calls[-1], (
"the high-water mark is captured AFTER the last-moment re-read, reopening the blind window "
"in which a human verdict is neither seen by the re-read nor repaired by the post-write check"
)
def test_a_previously_REPAIRED_head_is_never_re_exempted(tmp_path):
"""The Medium finding of round 2: the repair used to last exactly one event.
After a repair, the status is a machine-written `pending` — indistinguishable, to the next run,
from an ordinary one. That run re-derived it, posted `success`, and took a fresh high-water mark
ABOVE the human row, so the post-write check stayed silent and the human's rejection went green
again one event later. The repair description is now a sentinel the classification recognises.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:pending",
status_creator=None,
status_desc="Human verdict raced this exemption write — re-post the verdict",
)
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending", (
"a head whose human verdict was previously overwritten was granted a FRESH exemption, burying "
f"the rejection again (desc={posted['description']!r})"
)
assert "raced a previous exemption" in r.stdout, (
f"rejected, but not via the repair-sentinel branch. Log:\n{r.stdout[-800:]}"
)
def test_positive_control_an_ORDINARY_machine_pending_is_still_re_derived(tmp_path):
"""Without this, the test above would pass against a job that had stopped exempting anything with
a pre-existing `pending`. An ordinary machine `pending` — no sentinel — must still re-derive to
`success` for a docs-only PR."""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:pending",
status_creator=None,
status_desc="Awaiting review verdict for a9e3e23",
)
assert posted is not None and posted["state"] == "success", (
f"an ordinary pending was not re-derived to an exemption. Log:\n{r.stdout[-800:]}"
)
def test_the_fence_gates_PENDING_TOO_not_only_the_exemption(tmp_path):
"""The fence deliberately refuses to write `pending` as well as `success` when the PR was
retargeted mid-run. Untested, a success-only-fence mutant would have passed the whole suite.
A stale `pending` is only a stall rather than a forged green, so gating it is not strictly
required — but the successor run is guaranteed either way, so there is nothing to buy by writing a
value computed against a base the PR may no longer target.
"""
posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), timeline_mode="moves:0,1")
assert posted is None, (
"the fence let a stale `pending` through; it is gating only the exemption path "
f"(desc={posted['description']!r})"
if posted
else ""
)
assert "retargeted while this job was classifying" in r.stdout
def test_an_UNTRUSTED_count_withholds_the_exemption_BY_THAT_BRANCH(tmp_path):
"""Round 2 test-gap: the existing untrusted-count test asserted only "posted nothing", which a
crash also produces. Assert the discriminator and a clean exit."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="unreadable")
assert posted is None
assert r.returncode == 0, f"the job died rather than declining cleanly: {r.stderr[-800:]}"
assert "Could not establish a trusted retarget count" in r.stdout, (
f"nothing posted, but not via the untrusted-count branch. Log:\n{r.stdout[-800:]}"
)
def test_the_repair_sentinel_is_a_FIXED_POINT_across_consecutive_runs(tmp_path):
"""Round-3 finding: durability is a fixed point, and only a CHAIN can assert a fixed point.
The first version of the sentinel refused the exemption but posted the GENERIC pending
description, erasing the marker it depends on. The next run then saw an ordinary machine
`pending`, re-derived it, and posted `success` — burying the human rejection two events after the
repair instead of one. The single-hop test passed throughout, and the positive control asserting
that an ordinary machine `pending` DOES re-derive was itself the proof of the second hop.
So chain two runs: feed run N's posted description in as run N+1's existing status. The property
is that the sentinel branch's own output re-triggers the sentinel branch, forever.
"""
first, r1 = _run_classify(
tmp_path / "run1",
_emitting("docs/a.md"),
status_mode="existing:pending",
status_creator=None,
status_desc="Human verdict raced this exemption write — re-post the verdict",
)
assert first is not None and first["state"] == "pending", f"run 1: {r1.stdout[-600:]}"
second, r2 = _run_classify(
tmp_path / "run2",
_emitting("docs/a.md"),
status_mode="existing:pending",
status_creator=None,
status_desc=first["description"],
) # <-- the chain
assert second is not None, f"run 2 posted nothing: {r2.stderr[-800:]}"
assert second["state"] == "pending", (
"the repair decayed: run 1's own posted description did not re-trigger the sentinel, so run 2 "
f"re-derived the exemption and buried the human verdict again (run1 desc={first['description']!r}, "
f"run2 state={second['state']}, run2 desc={second['description']!r})"
)
assert second["description"] == first["description"], (
"the sentinel is not a fixed point — run 2 wrote a different description than run 1, so run 3 "
f"would not recognise it ({first['description']!r} -> {second['description']!r})"
)
def test_a_sentinel_APPEARING_MID_RUN_stops_a_stale_run_overwriting_it(tmp_path):
"""Round-3 finding, and the only one in this series that failed toward SUCCESS.
Two runs overlap for the same sha — the regime this branch measured live (probe PR #722: the older
run finished 20s after the newer one started). Run B catches a raced human BLOCKED and repairs to
the sentinel. Run A is still in flight: its FIRST read predates all of it, so it classified
`success`; its high-water mark was taken after the human row, so the post-write check stays silent;
and the retarget fence sees nothing. Until this guard, A posted its stale `success` straight over
the sentinel — burying the human rejection with no repair and no log.
The re-read recomputes `ex_repair`; the bug was that nothing downstream consulted it.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="sentinel-appears-on-read:2")
assert r.returncode == 0, r.stderr
assert posted is None, (
"a stale run posted its exemption over a repair sentinel written mid-run, burying the human "
f"verdict it records (posted={posted})"
)
assert "repair sentinel was written" in r.stdout, (
f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}"
)
def test_positive_control_a_sentinel_present_from_the_START_still_posts_pending(tmp_path):
"""Without this, the guard above could be an unconditional abstain whenever a sentinel exists.
A sentinel present at the FIRST read is the ordinary repaired-head case: it must still POST
`pending` carrying the sentinel forward (the fixed point), not abstain. That difference is exactly
what makes the mid-run guard's condition exact rather than conservative.
"""
posted, r = _run_classify(
tmp_path,
_emitting("docs/a.md"),
status_mode="existing:pending",
status_creator=None,
status_desc="Human verdict raced this exemption write — re-post the verdict",
)
assert posted is not None, f"the repaired head stopped posting entirely: {r.stdout[-800:]}"
assert posted["state"] == "pending"
assert posted["description"] == "Human verdict raced this exemption write — re-post the verdict"
def test_a_PENDING_path_run_also_refuses_to_clobber_a_mid_run_sentinel(tmp_path):
"""Round-4 finding: the guard's first form tested `state = success`, one branch too narrow.
A run can reach the POST on `pending` carrying the GENERIC description — most realistically after a
transient enumeration failure — and a success-only guard waves it through. It then overwrites the
sentinel with ordinary text, the next run sees no sentinel, re-derives, and posts `success`: the
same buried human rejection, two steps instead of one.
Hence the rule is "never replace a sentinel with a non-sentinel", compared on the DESCRIPTION. That
is strictly more general and exactly as precise, because the carry-forward branch guarantees a
first-read sentinel already sets `desc` to the sentinel.
"""
posted, r = _run_classify(
tmp_path,
"#!/usr/bin/env bash\n" + DOCS_ONLY + "exit 1\n", # enumeration fails -> pending, generic desc
status_mode="sentinel-appears-on-read:2",
)
assert r.returncode == 0, r.stderr
assert posted is None, (
"a pending-path run overwrote a repair sentinel written mid-run with the generic description; "
f"the next run would re-derive the exemption and bury the human verdict (posted={posted})"
)
assert "repair sentinel was written" in r.stdout, (
f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}"
)
def test_a_SENTINEL_landing_above_the_mark_also_triggers_the_repair(tmp_path):
"""Round-5 finding: the post-write filter counted only HUMAN rows, and that is not sufficient.
With two overlapping runs A and B, the human BLOCKED can land BELOW A's high-water mark — so A
cannot see it — while B masks it with an exemption `success` and only afterwards writes the
sentinel. A then finds nothing human above its mark, does not repair, and posts its own `success`
on top of the sentinel: a PERMANENT forged green over a human rejection, and the repair race
failing toward success rather than pending, which the decision record explicitly promises it does
not do.
A sentinel above the mark can only have been written by another run mid-flight — a pre-existing one
would have been seen at the first read and forced the pending path — so counting it cannot
false-fire, and it converges both runs on the fixed point.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="sentinel-after-post")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"the run posted its exemption on top of a sentinel written by an overlapping run and did not "
f"repair, leaving a human rejection permanently green. Posts: {seq}\n{r.stdout[-800:]}"
)
assert seq[0]["state"] == "success"
assert seq[1]["state"] == "pending"
assert seq[1]["description"] == "Human verdict raced this exemption write — re-post the verdict", (
f"repaired, but not to the sentinel, so the fixed point is broken: {seq[1]}"
)
# --- The expression-delimiter class (ersatztv#751) ----------------------------------------------
#
# WHAT HAPPENED, because the shape of it is what these tests are written against. A `run:` body is
# not shell yet when the runner reads it. The runner scans the whole scalar for the expression
# opener, and a single occurrence makes it rewrite the ENTIRE body into one `format(...)` call so
# the evaluated result can be spliced back in. That rewrite is all-or-nothing, so a payload that
# does not parse takes the whole step with it — and the runner then DROPS THE STEP AND CONCLUDES THE
# JOB `success`.
#
# The occurrence that did it was in a SHELL COMMENT: the #706 note explaining why a concurrency
# group does not work quoted a `concurrency:` snippet containing a PR-number expression as an
# illustration. `pr number` is not an expression. The step stopped running on 2026-08-03 and nothing
# went red until 2026-08-06.
#
# WHY EVERY EXISTING GUARD IN THIS FILE WAS BLIND TO IT, which is the part worth keeping: they all
# read `_code_lines()`, which strips comment lines. That choice is correct for what it was for — its
# own docstring explains that prose legitimately discusses `pulls/N/files`, and a raw scan would
# redden the repo over a piece of writing. But it encodes an assumption this bug falsifies: that a
# comment in a workflow cannot change behaviour. Inside a `run:` scalar it can. So the test below
# reads the RAW text, deliberately, and is the one test here that must never adopt `_code_lines`.
_EXPR = re.compile(r"\$\{\{(.*?)\}\}", re.S)
# Roots of a dotted context path, and the callable functions. Both lists are what the runner
# actually accepts; anything outside them cannot evaluate, and an expression that cannot evaluate
# does not fail loudly — it silently removes the step it appears in.
_EXPR_CONTEXTS = frozenset(
{
"github",
"env",
"vars",
"secrets",
"inputs",
"runner",
"steps",
"needs",
"matrix",
"job",
"jobs",
"strategy",
}
)
_EXPR_FUNCTIONS = frozenset(
{
"always",
"success",
"failure",
"cancelled",
"hashFiles",
"format",
"toJSON",
"toJson",
"fromJSON",
"fromJson",
"contains",
"startsWith",
"endsWith",
"join",
}
)
_EXPR_LITERALS = frozenset({"true", "false", "null"})
def _yaml_string_scalars(path: Path):
"""Every string scalar in the parsed document — keys and values, recursively.
Deliberately PARSED rather than raw (ersatztv#751, cold re-review). A `${{ … }}` in an ordinary
top-level YAML comment is inert: the runner never evaluates it, so redding on it would be a false
positive of exactly the kind this file has now produced twice. PyYAML drops those comments, which
is the behaviour wanted here.
A `run:` body IS one of these scalars, and it keeps its SHELL comments — which is the whole point,
because inside a `run:` scalar a comment is not inert. So this covers the real defect class in
every workflow while ignoring the one place a delimiter is genuinely harmless.
"""
import yaml
out: list[str] = []
def walk(node):
if isinstance(node, str):
out.append(node)
elif isinstance(node, dict):
for k, v in node.items():
if isinstance(k, str):
out.append(k)
walk(v)
elif isinstance(node, list):
for v in node:
walk(v)
walk(yaml.safe_load(path.read_text()))
return out
def _workflow_files():
# `*.yml` AND `*.yaml`: a workflow added as `.yaml` is just as executable and would otherwise go
# unscanned. From the GIT INDEX rather than a directory listing (ersatztv#806) — a stray
# untracked `.yaml` in `.gitea/workflows/` is not part of the repo, and enumerating it made
# these two completeness claims red on that checkout and green in CI, which is #778's shape.
return tracked_paths(".gitea/workflows", ("*.yml", "*.yaml"))
def test_the_verdict_workflow_has_NO_expression_delimiter_in_any_run_body():
"""The absolute rule, and it is scoped to this one file on purpose.
Other workflows legitimately interpolate into a `run:` body (5 occurrences today across
ci-image, docker-build's `api-docs`/`format`, and pr-checks' two git-diff gates), so a
repo-wide ban would be false and would be deleted the first time it got in someone's way.
`test`, `migrations` and `build` carry their own absolute ban — see
scripts/tests/test_ci_dropped_step_guard.py (ersatztv#756). This file is different in two ways that justify the
strict rule: it writes the branch-protection-required status, so a dropped step here is a dead
merge gate rather than a failed build; and its `run:` bodies are ~700 lines of dense prose,
which is the only place the delimiter has ever appeared by accident.
The rule is also what keeps the dropped-step guard trustworthy. A guard that the guarded
mechanism can silently delete is worse than no guard, because its absence is silent too.
The `run:` SCALAR AS PARSED, comments and all — never `_code_lines`, which strips them. The
distinction matters and is easy to garble: what must not be filtered is the SHELL comments inside
the body, because those are what the runner scans. Ordinary YAML comments outside a `run:` body are
genuinely inert and are handled by the repo-wide test below, which is why neither test reads the
file as flat text any more.
"""
offenders = []
for job_name, step in _iter_workflow_steps(WORKFLOW):
for m in _EXPR.finditer(step.get("run") or ""):
offenders.append(f"{job_name}/{step.get('name', '?')}: {m.group(1).strip()!r}")
assert not offenders, (
f"review-verdict.yml has an expression delimiter inside a run: body — {offenders}. Even in "
"a comment this is unsafe: the runner rewrites the WHOLE body into a format(...) call, and "
"if the payload does not parse it drops the step and reports the job GREEN, leaving the "
"required review-verdict/h10 unposted (ersatztv#751). To describe an expression in prose, "
"name it (`a github.event.pull_request.number expression`) instead of quoting the "
"delimiters. Pass values in through the step's `env:` block, which is interpolated per "
"value, so a bad payload there cannot take the body with it."
)
# ANTI-VACUITY, and it has to be the right property. A `run:` body the YAML walk never reached
# would make the assertion above vacuously green — the failure mode to guard against. An earlier
# version of this guard compared the file's TOTAL delimiter count against the count inside
# `with:`/`env:` values, which is a different and wrong claim: it banned expressions everywhere
# else in the file too. Both reviewers reproduced the false red — writing `if: ${{ always() }}`,
# the standard and equivalent spelling of the `if:` two steps below, turned this test red, as did
# a delimiter in an inert top-level YAML comment. Neither is unsafe, and a red here blocks every
# merge through the combined status, so the guard was strictly more dangerous than the thing it
# was protecting against.
#
# ANTI-VACUITY WITHOUT A SECOND PARSER. A first attempt counted `run:` keys in the raw text and
# compared that to the walk. Cold re-review showed the regex only recognised an indented `run:`
# whose value starts with `|` or `>`, so legal spellings (`- run: |`, a single-line
# `run: echo ok`) counted as zero declarations and false-redded the file, while a `run: |` line
# sitting INSIDE a shell heredoc counted as a declaration. Hand-parsing YAML to check a YAML parse
# is the wrong shape: it adds a second, worse parser whose disagreements are all false alarms, and
# a red here blocks every merge through the combined status.
#
# The property that actually matters is that the walk reached the body that carries the risk. The
# classifier is ~700 lines; a walk that returned nothing, or only the short steps, is the failure
# to catch. Both are asserted on content, which no spelling change can spoof.
bodies = [s["run"] for _, s in _iter_workflow_steps(WORKFLOW) if s.get("run")]
# NOT `>= 3`, and not merely non-empty either. `>= 3` had zero slack — cold review deleted the
# optional jq-preflight step, a legitimate simplification, and this redded claiming the classifier
# had not been examined, which was untrue. But relaxing it to `assert bodies` threw away the only
# check that the walk reached ALL run-bearing steps: `max(len) > 5000` proves it reached the
# classifier and nothing about the short ones, so a helper that silently stopped yielding them
# would let an unscanned delimiter through.
#
# So count against the job's own step list, read here rather than through the helper under test.
# That catches the failure the count existed for (a helper looking at the wrong key, or dropping
# steps) without breaking when a step is legitimately added or removed.
import yaml as _yaml
_steps = _yaml.safe_load(WORKFLOW.read_text())["jobs"]["set-verdict-status"]["steps"] or []
declared = sum(1 for s in _steps if isinstance(s, dict) and s.get("run"))
assert len(bodies) == declared, (
f"the YAML walk reached {len(bodies)} run: bodies but the job declares {declared} — the "
"assertion above did not examine every body, so a green here proves nothing"
)
assert max(len(b) for b in bodies) > 5000, (
"the YAML walk did not reach a substantial run: body — the ~700-line classifier is the one "
"that must be scanned, so this test would be vacuous"
)
def _iter_workflow_steps(path: Path):
import yaml
doc = yaml.safe_load(path.read_text()) or {}
for job_name, job in (doc.get("jobs") or {}).items():
for step in job.get("steps") or []:
yield job_name, step
def test_every_workflow_expression_names_a_REAL_context_or_function():
"""The general form of #751, across every workflow and every field.
The strict test above hardens the gate file. It cannot see the class, which is not "prose in a
run body" but "a payload that does not evaluate" — a typo in an `if:`, a renamed output, a
context that does not exist. All of them fail the same silent way, and in an `if:` the
consequence is the same shape as #751: the step does not run and nothing is red.
BE PRECISE ABOUT WHAT THIS ENFORCES, because the first version of this docstring was not and
both reviewers caught it: it checks that THE HEAD TOKEN of each dotted path is a known context or
function. Nothing more. That catches the historical defect — `pr number` fails on `pr` — and a
payload naming a context that does not exist. It does NOT catch:
* syntactically invalid expressions whose tokens are all known: `${{ github.ref == }}` and
`${{ github.event.pull_request.head.sha + }}` both pass, verified;
* a renamed or misspelled output or property, because every token after the first is preceded
by `.` and is deliberately skipped: `steps.metadata.outputs.shortsha` passes;
* an unclosed opener, since `_EXPR` requires the closing braces to match at all.
A real fix for those is an expression parser, which is a different and much larger change. This is
a cheap net under the specific class that has bitten us, deliberately kept permissive so it cannot
false-red the repo (a red here blocks every merge through the combined status). Verified against
all 31 distinct payloads in this repo today, which pass. Do not restate this test as "catches any
payload that cannot evaluate" — that claim was in the docs and the decision record and was false.
"""
offenders = []
for wf in _workflow_files():
for scalar in _yaml_string_scalars(wf):
for m in _EXPR.finditer(scalar):
payload = m.group(1).strip()
# Strip string literals first: a path inside `hashFiles('web/package-lock.json')` is
# data, not an identifier, and would otherwise read as an unknown context.
bare = re.sub(r"'[^']*'", "''", payload)
for ident in re.finditer(r"(?<![.\w'])([A-Za-z_][A-Za-z0-9_-]*)", bare):
name = ident.group(1)
if name in _EXPR_CONTEXTS or name in _EXPR_FUNCTIONS or name in _EXPR_LITERALS:
continue
offenders.append(f"{wf.name}: {payload!r} -> unknown '{name}'")
assert not offenders, (
"these workflow expressions name something the runner cannot resolve, so they will fail to "
f"interpolate — which DROPS THE STEP and still reports the job green (ersatztv#751): "
f"{offenders}. If this is prose describing an expression, do not write the delimiters; if "
"it is a real new context or function, add it to _EXPR_CONTEXTS / _EXPR_FUNCTIONS here."
)
def test_a_dropped_classify_step_FAILS_the_job_instead_of_going_green():
"""The silent-green half of #751 — the actual defect, and the only part that generalises.
The stray delimiter was one bug in one comment. What made it cost three days of a dead merge
gate was the REPORTING: a dropped step concludes `success`, the workflow's own status context
goes green, and the required `review-verdict/h10` is merely ABSENT — which on a normal PR is
indistinguishable from the correct "not reviewed yet" state. Nothing surfaced it. So the
classifier writes a start marker and a later `if: always()` step fails the job when it is
missing.
Asserted structurally, and the marker PATH is compared across the two steps rather than
hardcoded here twice: the failure mode of a divergent path is a job that reddens on every run
(fail-closed, but it would look like this guard is broken rather than like a real drop), and a
test carrying its own third copy of the literal could not see the divergence at all.
The premise — that the runner still executes a LATER step after dropping an earlier one — is
MEASURED, not assumed, and could not be settled from the #751 report because the classifier was
the job's last step. Gitea 1.27.1, 2026-08-06: probe run 1863 dropped the classifier on a
reintroduced bad payload, ran the guard anyway (`always()` evaluated true), and the job concluded
`failure`; run 1866 is the positive control. So the guard is load-bearing in production and the
body does not need to move into `scripts/`. This test pins the SHAPE; the probe pinned the
BEHAVIOUR, and neither substitutes for the other — which is why the behavioural test below
EXECUTES the guard body rather than only reading it.
"""
# ONE parse, and both steps located inside it. Using `_classify_step()` here instead was wrong
# in a way worth recording, because it went green-adjacent rather than loud: that helper does its
# own `yaml.safe_load`, so the dict it returns is never the same OBJECT as the equivalent step in
# this list. An `is not` filter against it therefore excluded nothing, the classify step matched
# as its own guard, and the assertions below ran against the wrong step.
steps = [s for _, s in _iter_workflow_steps(WORKFLOW)]
classifiers = [i for i, s in enumerate(steps) if "review-verdict/h10" in (s.get("run") or "")]
assert classifiers, "no step in review-verdict.yml posts review-verdict/h10"
classify = steps[classifiers[0]]
# The FULL path expansion is compared between the two steps, not a basename. The path is keyed on
# the run id, so a basename match would accept two steps that agree on the prefix and disagree on
# the key — which is precisely the divergence that would make the guard fail on every run.
marker_write = re.search(r'(\w+)="(\$\{RUNNER_TEMP[^"]*)"', classify["run"])
assert marker_write, (
"the classify step no longer assigns a start-marker path under RUNNER_TEMP, so a step the "
"runner drops goes green again (ersatztv#751)"
)
var, marker_name = marker_write.group(1), marker_write.group(2)
assert re.search(rf':\s*>\s*"\${var}"', classify["run"]), (
f"the classify step defines {var} but never creates the marker, so the guard below will "
"fail on every run and read as broken rather than as a real dropped step"
)
guard_idx = [i for i, s in enumerate(steps) if i != classifiers[0] and marker_name in (s.get("run") or "")]
assert guard_idx, (
f"no step checks for the {marker_name!r} start marker. Without it a dropped classify step "
"concludes success and the merge gate is silently dead (ersatztv#751)"
)
# AFTER the classifier, not merely present. A guard placed before it would read a marker that
# has not been written yet and fail on every run — fail-closed, but it would deadlock `main` and
# read as this guard being broken, which is how a correct-looking guard gets deleted.
assert guard_idx[0] > classifiers[0], (
f"the dropped-step guard is step {guard_idx[0]} but the classifier is step "
f"{classifiers[0]} — a guard that runs first always fails"
)
guard = steps[guard_idx[0]]
# `always()` and `${{ always() }}` are the same condition; the runner accepts both and the second
# is the more common spelling. Pinning the bare form EXACTLY would red the repo over a
# semantically identical edit, so normalise instead. What must not change is that the guard runs
# when the classifier failed.
guard_if = re.sub(r"\s+", "", str(guard.get("if", "")))
assert guard_if in ("always()", "${{always()}}"), (
f"the dropped-step guard's `if:` is {guard.get('if')!r}; it must be `always()` (bare or "
"wrapped), or it will be skipped on exactly the runs where the classifier failed"
)
# INSIDE the missing-marker branch, not merely somewhere in the body. Cold review pointed out
# that a bare `exit 1` substring is satisfied by an unreachable `if false; then exit 1; fi` while
# the real branch says `exit 0` — the test passes and a dropped classifier goes green again. The
# behavioural test below is the real proof; this keeps the structural one from being satisfiable
# by dead code.
missing_branch = re.search(r'if \[ ! -f "\$marker" \]; then(.*?)\bfi\b', guard["run"], re.S)
assert missing_branch, (
'the dropped-step guard no longer tests for a MISSING marker with `if [ ! -f "$marker" ]`, '
"so the assertion below cannot locate the branch that must fail the job"
)
assert re.search(r"exit\s+1", missing_branch.group(1)), (
"the dropped-step guard detects the missing marker but does not `exit 1` inside that branch, "
"so it observes the failure and still lets the job go green — which is the whole defect"
)
assert not _EXPR.search(guard["run"]), (
"the dropped-step guard's own run body contains an expression delimiter, so the mechanism "
"it guards against can drop the guard too — and that absence would be silent as well"
)
@pytest.mark.parametrize("terminator", ["null", "[]"], ids=["null-page", "empty-array-page"])
def test_the_fence_TRUSTS_the_count_and_POSTS_when_the_timeline_terminates(tmp_path, terminator):
"""The second defect found by the ersatztv#751 probe, and the one that actually kept the gate
from posting anything.
A page past the end of `/issues/{n}/timeline` is the JSON value `null` on this instance, not `[]`.
`count_retargets` gated on `type == "array"` and so treated the real terminator as unreadable: the
walk never reached a validated empty page, `rt_ok` was never `yes` for ANY pull request, and the
fence therefore withheld every exemption `success`. Renovate and docs-only PRs got NO status —
the same user-visible outcome as #751, by an unrelated route.
It hid for two reasons worth keeping written down. It shipped in the same commit (8f6d4f443) that
stopped the step from executing, so the fence had never once run in production; and the test
double printed `[]` while its comment claimed to mirror measured reality, so the type gate was
never exercised by the suite either. A green suite over an unfaithful double is what let a
fail-closed-by-accident branch look deliberate.
Both shapes are asserted because both are live on this server: comments really do return `[]`.
Asserting the POSTED STATUS, not the log, because the log said `Decision: state=success` on the
real probe run and the job still posted nothing — the decision and the write are different events,
and only the write is what a merge reads.
"""
posted, r = _run_classify(
tmp_path, _emitting("docs/a.md"), timeline_mode="stable:0", timeline_terminator=terminator
)
assert r.returncode == 0, r.stderr
assert posted is not None, (
f"a docs-only PR whose timeline terminates with `{terminator}` got NO status at all. The "
"fence could not establish a trusted retarget count, so it withheld the exemption — which "
"leaves the required review-verdict/h10 absent and the PR unmergeable with no bypass "
f"(ersatztv#751).\n{r.stdout[-1200:]}"
)
assert posted["state"] == "success", f"expected the docs-only exemption, got {posted}"
assert "trusted=yes" in r.stdout, (
"the exemption was posted but the fence did not report a trusted count — the two must agree, "
f"or this test is passing for a different reason than it claims.\n{r.stdout[-800:]}"
)
def _marker_steps():
"""(classify body PREFIX through the marker write, guard body that CHECKS it) — from one parse.
THE PREFIX, NOT THE MATCHING LINES. An earlier version picked out the lines matching `RAN_MARKER=`
and `: > "$RAN_MARKER"` and ran those two alone. Cold re-review showed that passes even if the
write is moved into a function nobody calls, or under `if false`: the extractor finds the text,
runs it at top level, the marker appears, and the test reports the guard proven while production
writes no marker at all. Executing the real prefix — everything from the top of the body down to
and including the write — reproduces the production control flow instead of a reconstruction of
it, so a write that is defined-but-not-reached simply does not happen and case B fails.
#
It also pins the property the marker needs anyway: that the write happens EARLY, before anything
in the classifier can fail. If it drifts down past code that needs credentials or network, this
prefix stops executing cleanly and the test says so.
"""
steps = [s for _, s in _iter_workflow_steps(WORKFLOW)]
idx = [i for i, s in enumerate(steps) if "review-verdict/h10" in (s.get("run") or "")]
classify = steps[idx[0]]
lines = classify["run"].splitlines()
write_at = [i for i, ln in enumerate(lines) if re.match(r'\s*:\s*>\s*"\$RAN_MARKER"', ln)]
assert len(write_at) == 1, f'expected exactly one `: > "$RAN_MARKER"` in the classify body, found {len(write_at)}'
prefix = "\n".join(lines[: write_at[0] + 1])
guard = next(s for i, s in enumerate(steps) if i != idx[0] and "h10-classifier-started" in (s.get("run") or ""))
return prefix, guard["run"]
def test_the_dropped_step_guard_BEHAVIOURALLY_fails_without_the_marker_and_passes_with_it(tmp_path):
"""Executes the guard, instead of reading it — the structural test above cannot prove the exit
code, and cold review was right that a bare `exit 1` substring is satisfiable by dead code.
The marker is created by running THE CLASSIFY STEP'S OWN two prologue lines under the same
environment, never by rebuilding the path in Python. That is the point: it proves the two steps
AGREE on the path by construction. A test that computed the name itself would keep passing after
the two steps drifted apart, which is the one divergence that makes the guard fail on every run
and get deleted as broken.
`GITHUB_RUN_ID`/`GITHUB_RUN_ATTEMPT` are set to fixed values, so this also covers the run-keying
added after the live probes: if either step stopped interpolating them, the paths would differ and
case B would fail.
"""
prologue, guard = _marker_steps()
assert prologue.count("RAN_MARKER=") == 1 and ': > "$RAN_MARKER"' in prologue, (
f"could not extract the classify step's marker prologue; got {prologue!r}"
)
# The prefix must be the REAL top of the body, so a write hidden in an uncalled function is not
# executed by this test either.
assert prologue.lstrip().startswith("set -euo pipefail"), (
"the extracted prefix does not start at the top of the classify body, so it is a "
f"reconstruction rather than the production path: {prologue[:120]!r}"
)
env = {
"PATH": os.environ["PATH"],
"RUNNER_TEMP": str(tmp_path),
"GITHUB_RUN_ID": "424242",
"GITHUB_RUN_ATTEMPT": "7",
}
# A — the step was DROPPED: no marker exists. The job must fail.
a = subprocess.run(["bash", "-c", guard], env=env, capture_output=True, text=True)
assert a.returncode != 0, (
"the guard exited 0 with NO start marker present — a dropped classify step would go green "
f"again, which is the whole defect (ersatztv#751).\nstdout: {a.stdout}\nstderr: {a.stderr}"
)
assert "did not execute" in (a.stdout + a.stderr), (
f"the guard failed but without an actionable message: {a.stdout!r} {a.stderr!r}"
)
assert not list(tmp_path.glob("h10-classifier-started*")), (
"the guard itself created the marker it is supposed to be checking for"
)
# B — the classifier RAN: its own prologue created the marker. The guard must pass.
b = subprocess.run(["bash", "-c", prologue + "\n" + guard], env=env, capture_output=True, text=True)
assert b.returncode == 0, (
"the guard rejected a marker written by the classify step's OWN prologue — the two steps "
f"disagree on the path, so this guard would fail on every run.\nstdout: {b.stdout}\n"
f"stderr: {b.stderr}"
)
# C — the write must be reached by STRAIGHT-LINE code. Guarding the prefix trick itself: if the
# write were wrapped in a function or an `if`, the prefix would still contain it but production
# might not reach it. Executing the prefix with the function/conditional intact is the test; this
# assertion makes the intent explicit and fails loudly rather than subtly.
body_before = "\n".join(ln for ln in prologue.splitlines() if not ln.lstrip().startswith("#"))
# Covers all four bash spellings: `mk() {`, `mk(){`, `function mk {`, `function mk() {`. The third
# was added when review found the first regex missed it, and the FOURTH still slipped that fix —
# the union form is the natural next spelling once `function mk {` is caught. Budget three rounds
# for any string-matching predicate.
assert not re.search(r"^\s*(function\s+)?\w+\s*(\(\s*\))?\s*\{", body_before, re.M), (
"a function is defined before the marker write, so the write may be inside it and unreached "
f"in production while this test still passes:\n{body_before}"
)
written = [q.name for q in tmp_path.glob("h10-classifier-started*")]
assert written == ["h10-classifier-started-424242-7"], (
f"the marker is not keyed on the run id/attempt as intended; found {written}. A fixed name in "
"a shared RUNNER_TEMP lets a stale marker satisfy this guard on a run whose step was dropped"
)
@pytest.mark.parametrize("shape", ["null", "array"], ids=["statuses-null", "statuses-empty-array"])
def test_a_head_with_NO_statuses_YET_is_readable_and_still_gets_its_exemption(tmp_path, shape):
"""The twin of the timeline terminator, found by cold review of the fix for that one (#751).
`GET /commits/{sha}/status` returns `{"state":"pending","total_count":0,"statuses":null}` for a
head that has no statuses yet — measured on PR #739's head 5fa672e2. `read_existing_verdict`
gated on `.statuses | type == "array"`, so it read that as unreadable and took its `exit 1`
path: the job posted NOTHING. Fail-closed, but the user-visible result is exactly the outcome
this issue is about — an exempt PR with no status and, since #743, no bypass.
Both shapes are asserted because a head that HAS statuses really does return an array, so the job
must read either. Restoring the `array`-only gate turns 40+ tests red with the corrected double,
which is the measure of how thoroughly the unfaithful stub was hiding this.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_empty_shape=shape)
assert r.returncode == 0, (
f"the classifier died reading a `statuses: {shape}` body instead of treating it as "
f"'no verdict yet', so nothing was posted at all.\n{r.stdout[-1000:]}\n{r.stderr[-600:]}"
)
assert posted is not None, (
f"a docs-only PR whose head has no statuses yet (shape: {shape}) got NO status at all.\n{r.stdout[-1000:]}"
)
assert posted["state"] == "success", f"expected the docs-only exemption, got {posted}"
@pytest.mark.parametrize("path", ["CLAUDE.md", "AGENTS.md"])
def test_the_GOVERNANCE_docs_are_protected_and_get_no_docs_only_exemption(tmp_path, path):
"""`CLAUDE.md`/`AGENTS.md` are not prose about the project — they define the completion protocol,
the merge-consent convention and the H10 rule itself. `DOCS_ONLY` (`^(docs/|[^/]*\\.md$)`) matched
them, so a PR editing the document that specifies what `.claude/` enforces was auto-exemptible
while `.claude/` itself was protected: the same self-exemption the workflow header rules out, one
directory over. Found by cold review of #751 by driving this exact case through the real body.
Latent until #751, because no exemption `success` was writable at all while the classify step was
dropped — restoring the exemptions is what makes it reachable, which is why it is fixed there.
Asserted through the PROTECTED branch specifically, not merely "not exempt": `pending` is reached
by several routes and a test that accepted any of them could not tell a working guard from a dead
one.
"""
posted, r = _run_classify(tmp_path, _emitting(path))
assert posted is not None, f"the job posted nothing: {r.stderr[-1500:]}"
assert posted["state"] == "pending", (
f"{path} was granted an exemption ({posted}) — the document that DEFINES the merge gate must "
f"not be able to exempt itself from it.\n{r.stdout[-800:]}"
)
assert "protected" in r.stdout.lower(), (
f"{path} was not exempted, but not via the protected-path branch either, so that guard may be "
f"dead for it.\n{r.stdout[-800:]}"
)
def test_a_terminator_on_PAGE_ONE_does_not_certify_a_zero_retarget_count(tmp_path):
"""`null` means "exhausted" only after a real page has been read.
THE INVARIANT, not a figure that rots: a real PR's timeline always carries at least one event on
page 1, because the PR is created by a push and that is itself an event. Spot-checked non-empty
across #752/#753/#749/#739/#717; the counts are not recorded, because an earlier version of this
docstring cited five and three were stale within days. So a terminator on page 1 is anomalous, not
empty. Trusting a zero count from it would mean
certifying that no retarget happened on the strength of a response we cannot explain, which is the
one thing the fence exists to refuse. Withholding the exemption is the safe direction: the PR asks
for a human verdict instead.
This narrows rather than closes the concern — a wrong `null` on page 3 is still read as exhaustion,
and no bounded number of round-trips can rule that out.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="empty-first-page")
assert r.returncode == 0, r.stderr
assert posted is None, (
"an exemption `success` was posted from a timeline whose FIRST page was already the "
f"terminator, so no page of events was ever actually read: {posted}\n{r.stdout[-800:]}"
)
assert "trusted=no" in r.stdout, (
f"the exemption was withheld, but not because the count was untrusted:\n{r.stdout[-800:]}"
)
def test_a_SECOND_PAGE_of_statuses_refuses_to_conclude_that_no_verdict_exists(tmp_path):
"""Completeness for the "no verdict exists" conclusion — the one that licenses posting over a
verdict this job cannot see.
THE TWO OBVIOUS GUARDS ARE BOTH NO-OPS HERE, which is why this costs a round-trip:
* `.statuses | length` vs `.total_count` — `total_count` is the count for the PAGE RETURNED, not
the commit. Measured at 1.27.1 on 3aed43c6 (6 contexts): `?limit=1` -> `len=1, total_count=1`.
Equal by construction. The stub mirrors that, so this test cannot pass for that wrong reason.
* "refuse when the page came back full at the requested limit of 100" — this instance caps `limit`
at `MAX_RESPONSE_ITEMS`, measured at 50 (`/issues?limit=100` returns 50), so a response can never
hold 100 rows and the comparison was DEAD CODE. The repo already documented that cap in three
places; the guard was written against 100 anyway and a cold review caught it.
So the job asks the server, and only when the row is absent from page 1. Any rows on page 2 mean
the list is longer than one page and the verdict may be beyond it.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="twopage")
assert posted is None, (
"an exemption was posted while the status list ran to a second page, so an existing verdict "
f"beyond page 1 would have been silently overwritten: {posted}\n{r.stdout[-800:]}"
)
assert "page 2" in (r.stdout + r.stderr).lower(), (
f"nothing was posted, but not because of the page-2 completeness probe:\n{r.stdout[-800:]}"
)
def test_a_SINGLE_page_of_statuses_reads_normally(tmp_path):
"""Positive control for the probe above: without it, "refuse when page 2 has rows" could be
satisfied by refusing always, which deadlocks every PR while looking safe. 40 decoy contexts on
page 1, nothing on page 2.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="rows:40")
assert r.returncode == 0, r.stderr
assert posted is not None and posted["state"] == "success", (
f"a single page of statuses should read normally and still exempt; got {posted}\n{r.stdout[-800:]}"
)
def test_a_STRING_total_count_is_not_accepted_as_numeric_zero(tmp_path):
"""`jq -r` renders the JSON number 0 and the JSON string "0" identically, so a text compare
accepts a schema-corrupted `"total_count": "0"` as "no statuses" (cold re-review reproduced it).
The live schema uses an integer, so this is not a live failure — it is the difference between a
guard that holds because the input happens to be well-formed and one that holds because it checks.
The accept path requires the TYPE to be number.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="total-count-string")
assert posted is None, (
f"a string total_count was accepted as numeric zero, so a body that merely lost its statuses "
f"array reads as 'no verdict exists': {posted}\n{r.stdout[-800:]}"
)
@pytest.mark.parametrize(
"mode,why",
[
("page2-garbage", "a non-JSON page 2"),
("page2-error", "an HTTP error on page 2"),
],
ids=["garbage", "transport-error"],
)
def test_an_UNREADABLE_page_2_refuses_to_conclude_that_no_verdict_exists(tmp_path, mode, why):
"""The two refuse branches of the completeness probe, which cold review found untested.
Worth a test rather than trusting the shape: this file's history is two consecutive guards that
were UNREACHABLE — the `count_retargets` type gate that never saw a real terminator, and a
full-page check written against a limit of 100 on a server that caps at 50. An unexercised branch
here has a track record.
Both must fail CLOSED: the point of reading page 2 is to justify "no verdict exists", so a page 2
that cannot be read justifies nothing.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode=mode)
assert posted is None, (
f"an exemption was posted despite {why}, so 'no verdict exists' was concluded without "
f"evidence: {posted}\n{r.stdout[-800:]}"
)
assert "page 2" in (r.stdout + r.stderr).lower(), (
f"nothing was posted, but not because of the page-2 probe:\n{r.stdout[-800:]}"
)
def test_a_status_history_RUNNING_PAST_PAGE_1_is_PAGED_and_the_exemption_STANDS(tmp_path):
"""THE INVERSION (ersatztv#763). This test asserted the opposite until real paging existed, and
the change of expectation IS the fix.
Before: the post-write check read one clamped page, so "no raced row on page 1" did not establish
"no race", and #751's conservative probe treated any rows on page 2 as a race. A head with more
status rows than the cap therefore lost its exemption to a STICKY sentinel with no race anywhere
near it. That is not hypothetical — it fired on Renovate PR #761, whose head accumulated rows over
ordinary CI re-runs until the gate started reporting a human verdict as overwritten when none
existed.
After: the walk pages to a validated empty page, so rows beyond the first page are simply READ.
The stub's history is 60 ORDINARY rows — no verdict, no sentinel — and the correct answer is that
nothing raced this write, so the exemption must STAND.
This is the discriminating half of the pair: it is the assertion that goes red against the old
single-page read (which would repair here), while
`test_a_raced_verdict_on_PAGE_2_is_detected_and_repaired` goes red against a walk that pages but
stops early. Neither test alone can tell "pages correctly" from "repairs on everything".
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="second-page")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 1, (
"the exemption was repaired away even though the whole history was readable and carried no "
f"verdict — the page-2 'assume raced' probe is still in force. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success", f"expected the exemption to stand; got {seq}"
# The false `::error::` from the old probe is the thing PR #761 could not be diagnosed from, so
# pin its absence too — a run that stands green but still shouts about an overwritten verdict
# would satisfy the count assertion above while leaving the log as misleading as before.
assert "was overwritten" not in (r.stdout + r.stderr), (
f"the exemption stood, but the log still claims a verdict was overwritten:\n{r.stdout[-900:]}"
)
def test_a_raced_verdict_on_PAGE_2_is_detected_and_repaired(tmp_path):
"""The hole #763 exists to close, and the half a paging change can still get wrong.
A verdict sits BEYOND the first page — 60 ordinary rows with it inserted at index 55 — so only a
walk that continues past page 1 can find it. Under the pre-#763 code the exemption would still
have been repaired here, but for the wrong reason (rows merely EXISTED on page 2), which is why
this test is paired with the one above rather than standing alone: together they separate
"actually read page 2" from "repaired on any uncertainty".
SCOPE, stated because the double is ordering-blind: this proves WALK COMPLETENESS, not that a real
raced verdict would otherwise be missed. The live endpoint serves `created_unix DESC`, so a row
created inside the write window is among the newest and lands on page 1. The walk exists so that
the one fail-toward-SUCCESS path does not rest on that undocumented ordering.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="verdict-on-page-2")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"a human verdict sitting on page 2 of the status history was not detected, so the exemption "
f"stands over a rejection nobody read. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
assert seq[1]["description"] == "Human verdict raced this exemption write — re-post the verdict", (
f"repaired, but not to the sentinel, so the fixed point is broken: {seq[1]}"
)
# It must be reported as a REAL overwrite, not as uncertainty — the two now say different things
# in the log, and a verdict genuinely found on page 2 is the case that earns the strong wording.
assert "was overwritten" in (r.stdout + r.stderr), (
f"repaired, but the log did not report an overwritten verdict:\n{r.stdout[-900:]}"
)
def test_a_partial_mark_is_SAFE_because_the_newest_rows_are_on_page_1(tmp_path):
"""The partial-mark fallback's safety is a claim ABOUT THE ORDERING, so it is measured under the
real one — this is the only order-faithful fixture here.
The fallback exists because refusing a mark skips the post-write check entirely, which is a
fail-open. It is safe only because the server serves `created_unix DESC` and ids are monotonic
with `created_at`: page 1 therefore carries the true maximum, so a walk that fails on page 2 has
still seen it.
The history holds a PRE-EXISTING base-mismatched verdict at id 7055 — older than the newest row,
so nothing it does raced this write. Under DESC the salvaged mark is 7059 and that row is below
it: the exemption correctly STANDS. Under ASC (`sort=highestindex`, withdrawn in round 4) page 1
would be 7000..7049, the mark 7049, and that same untouched row would test as NEWER than the mark
— a sticky repair on a head nothing raced, which is exactly #761.
So re-adding the sort parameter reddens this test by BEHAVIOUR, not merely by the structural
assertion in `test_the_walk_does_not_request_a_SORT_order`.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="premark-page2-error-desc")
assert r.returncode == 0, r.stderr
log = r.stdout + r.stderr
# This pins that the walk really was INCOMPLETE — it does not prove the fallback ran, because the
# warning is emitted by a different condition than the one that salvages the mark. Deleting the
# fallback leaves this line green; the sibling test
# `test_a_PRE_WRITE_paging_failure_still_yields_a_usable_high_water_mark` is what reddens then.
assert "taking the high-water mark over the" in log, (
f"the pre-write walk completed, so this test is not exercising a partial mark at all:\n{r.stdout[-900:]}"
)
seq = _posted_sequence(tmp_path)
assert len(seq) == 1, (
"a PRE-EXISTING verdict was read as newer than the salvaged mark, so the exemption was "
f"repaired away on a head nothing raced. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success", f"expected the exemption to stand; got {seq}"
assert "was overwritten" not in log, (
f"nothing raced this write, but the log claims a verdict was overwritten:\n{r.stdout[-900:]}"
)
def test_an_EMPTY_post_write_history_is_not_evidence_that_nothing_raced(tmp_path):
"""A well-formed response that cannot be true must not be trusted, and this one is free to reach.
The walk terminates on an empty page — correct BEFORE the write, where a head nothing has posted
to genuinely has no statuses. AFTER the write it is impossible: this job has just POSTed, and
`/statuses/{sha}` returns one row per POST, so "no statuses exist" contradicts a write that
succeeded. Nothing retries it either, because the body is valid JSON of the right type.
Accepting it yields `raced=0` from a list that cannot be real, on the one path whose failure
direction is toward SUCCESS — and silently, since the walk itself reports success. Whether a
verdict actually raced this particular write is not the point and is not modelled here: the
response is not evidence either way, so concluding "nothing raced" from it is unsound regardless.
This is NOT the withdrawn currency witness. That asked "is there ANY row above the mark", which an
unrelated newer row satisfied while the rejection stayed hidden. This asks only whether the list is
EMPTY — a state no unrelated row can produce and no ordering can disguise.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="null-page1-after-post")
assert r.returncode == 0, r.stderr
log = r.stdout + r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"an empty post-write history was accepted as proof that nothing raced this write, so an "
f"exemption stands on a head whose write window was never inspected. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
assert "came back empty" in log, (
f"repaired, but the impossible empty read was not reported as the reason:\n{r.stdout[-900:]}"
)
assert "was overwritten" not in log, (
f"an unverifiable read was reported as an overwritten human verdict:\n{r.stdout[-900:]}"
)
# THE REASON IS PART OF THE FIX, so it is asserted POSITIVELY. An absence assertion alone leaves
# `raced_why` unpinned — any string at all, including a placeholder, satisfies it while the
# operator gets a sticky sentinel explained by nothing. The em-dash and full stop discriminate the
# `::error::` reason from the `::warning::` text, which continues "…, which cannot be true".
assert "— the status history came back empty after this job posted to it." in log, (
"the repair did not report its own reason; the ::error:: carries whatever `raced_why` held "
f"and nothing pins it:\n{r.stdout[-900:]}"
)
# The walk COMPLETED here, on a validated terminator — the answer was impossible, not unreadable.
# Borrowing the unreadable-history wording would send an operator staring at a sticky sentinel
# looking for an API failure that never happened.
assert "could not be read completely" not in log, (
"an empty-but-complete read was reported as an incomplete one, which points at the wrong "
f"cause:\n{r.stdout[-900:]}"
)
def test_a_NULL_page_terminates_the_walk_as_an_empty_one(tmp_path):
"""`/statuses/{sha}` returns `[]` past the end today; `/issues/{n}/timeline` returns bare `null`.
An array-only gate on the timeline was ersatztv#751: the walk never reached a validated empty
page, `rt_ok` was never `yes` for any PR, and the fence withheld EVERY exemption `success` from
the day it shipped. Re-adopting that narrowing on this endpoint would be worse, because this
walk's failure mode is the STICKY sentinel — every exempt PR would need a human verdict, per head,
clearable only by hand.
Tolerating `null` cannot misread `[]`, so it costs nothing. This asserts it rather than trusting
that the endpoint will never change its empty shape.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="null-terminator")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 1, (
"a `null` page past the end was read as unreadable rather than as exhaustion, so the walk "
f"never validated and the exemption was repaired away. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success", f"expected the exemption to stand; got {seq}"
def test_a_STRING_id_on_a_PRE_EXISTING_row_is_not_read_as_raced(tmp_path):
"""The twin of `test_a_STRING_id_cannot_inflate_the_high_water_mark`, on the COMPARISON rather
than the maximum — and the twin was live on `main`, one expression away from the fix.
jq orders strings above every number, so `.id > $since` reads a string id as newer than ANY mark.
A pre-existing row carrying one — such as a base-mismatched verdict `read_existing_verdict`
deliberately declines to honour — is therefore counted as having raced this write on EVERY run of
that PR. Each run posts the sticky sentinel and a false "was overwritten", so the exemption is
lost permanently and no re-trigger clears it.
The row here is genuinely older than the mark (`"3"` against a mark of 9999) and must be invisible.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="string-id-preexisting-row")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 1, (
"a PRE-EXISTING row with a string id was read as newer than the high-water mark and treated "
f"as a raced verdict, so this PR loses its exemption permanently. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success", f"expected the exemption to stand; got {seq}"
assert "was overwritten" not in (r.stdout + r.stderr), (
f"nothing raced this write, but the log claims a verdict was overwritten:\n{r.stdout[-900:]}"
)
def test_a_history_with_NO_numeric_ids_SKIPS_the_check_instead_of_marking_zero(tmp_path):
"""An empty history and an unreadable one are different answers, and only one of them is 0.
A head nothing has posted to has no rows and its mark is 0 — correct, every later row is newer.
A NON-empty history in which no row carries a numeric id is a schema this job cannot read.
Reporting that as 0 claims a mark was established when none was, and every pre-existing row then
tests as newer than it.
The observable difference is the log: the unusable case says so and skips the post-write check.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="no-numeric-ids")
assert r.returncode == 0, r.stderr
log = r.stdout + r.stderr
assert "was not numeric" in log, (
"a history carrying no numeric id at all was silently treated as a mark of 0, so the "
f"degradation is invisible in the log:\n{r.stdout[-900:]}"
)
seq = _posted_sequence(tmp_path)
assert len(seq) == 1 and seq[0]["state"] == "success", (
f"expected the exemption to be posted with the race check skipped; got {seq}"
)
def test_a_history_LONGER_THAN_THE_PAGE_CAP_is_not_treated_as_exhausted(tmp_path):
"""The page cap is a refusal, not a terminator.
Twenty requests of fifty rows cannot validate a history of 1050: the walk never reaches an empty
page, so it has NOT established that it saw everything. Treating the cap as exhaustion would hand
the post-write check a list it knows is partial and let it conclude "nothing raced" from it — on
the one path whose failure direction is toward SUCCESS.
Only 950 rows can ever be validated, incidentally, because the twentieth request has to be the
empty terminator.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="over-cap")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"a history longer than the page cap was treated as fully read, so a raced verdict beyond it "
f"would have been concluded absent. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
assert "could not be read completely" in (r.stdout + r.stderr), (
f"repaired, but not reported as an incomplete read:\n{r.stdout[-900:]}"
)
def test_the_walk_does_not_request_a_SORT_order(tmp_path):
"""A regression guard on a WITHDRAWAL, which otherwise nothing mechanical protects.
`sort=highestindex` returns index ASCENDING, which would put the OLDEST rows on page 1. The
partial-mark fallback then takes its maximum over the oldest rows, leaving every pre-existing row
above the mark and read as raced — a spurious STICKY repair, which is the #761 failure this change
exists to remove. It was added, measured, and withdrawn for exactly that reason.
Nothing about the code's behaviour reveals a re-added sort parameter (the tests' double is
ordering-blind by design), so the guard is structural: the request must not carry one.
"""
src = _classify_step()["run"]
# The REQUEST LINES ONLY — comments are excluded deliberately, because the withdrawal note inside
# `page_statuses` names the parameter in order to explain why it is not used. A guard that reads
# prose would fire on its own documentation.
requests = [line for line in src.splitlines() if "statuses/$SHA?" in line and not line.lstrip().startswith("#")]
assert requests, (
"no `/statuses/{sha}` request line found; the walk has been reshaped, so re-point this guard "
"rather than letting the sort-order assertion below pass vacuously"
)
offenders = [line.strip() for line in requests if "sort=" in line]
assert not offenders, (
"the status-history request carries a sort order again. ASC ordering puts the OLDEST rows on "
"page 1, which inverts the partial-mark fallback into a spurious sticky repair — the #761 "
f"failure this change removes. See the withdrawal note in `page_statuses`. Offending: {offenders}"
)
def test_a_STRING_id_cannot_inflate_the_high_water_mark(tmp_path):
"""jq sorts strings above every number, so one schema-corrupt id silently blinds the race check.
`max` over raw ids returns `"99999"` rather than the largest real id. `jq -r` then renders it as
`99999`, which sails through the `*[!0-9]*` numeric gate, and the mark is set far above anything
that exists. Every subsequent row — including a genuine human rejection racing the write — tests
as OLDER than the mark and is invisible, so the exemption stands over it.
It fails toward SUCCESS and needs no attacker: one corrupt row is enough. So the mark is taken
over numeric ids only, and a non-numeric id is excluded rather than coerced.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="string-id-inflates-mark")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"a string id inflated the high-water mark, so the raced verdict tested as older than it and "
f"the exemption stands over a human rejection. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
def test_a_TRANSIENT_page_failure_is_retried_and_the_walk_still_completes(tmp_path):
"""The retry clause, which was unproven code until this fixture existed.
Every other error mode here fails on EVERY attempt, so a one-shot walk and a retrying walk behave
identically against them — mutating `for try in 1 2` to `for try in 1` reddened nothing. That
matters because the retry is the stated reason failing closed is affordable: without it, one
momentary blip costs the head its exemption permanently, since the repair sentinel is sticky.
Here page 2 fails the first attempt of each logical read and succeeds on the retry. The history
carries no verdict, so the correct outcome is a completed walk and a STANDING exemption.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="flaky-page2")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 1, (
"a transient page failure was not retried, so the walk gave up and the exemption was repaired "
f"away on a head that nothing raced. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success", f"expected the exemption to stand; got {seq}"
def test_a_PRE_WRITE_read_that_returns_NOTHING_abandons_the_mark_and_says_so(tmp_path):
"""The boundary of the partial-list fallback, pinned so the remaining fail-open is explicit.
A partial list still yields a usable mark. A read that returns NO rows at all cannot: there is
nothing to take a maximum over, so `max_id_before` stays -1 and the post-write race check is
skipped entirely — the exemption is posted with nothing verifying it afterwards.
That is the pre-existing gap tracked as ersatztv#849, deliberately unchanged here, and it is
asserted rather than left implicit so the degradation is visible in the log and a future change
that widens or closes it has to come past this test.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="premark-page1-error")
assert r.returncode == 0, r.stderr
log = r.stdout + r.stderr
assert "Could not establish a status high-water mark" in log, (
f"the mark was abandoned silently, with no record of the degradation:\n{r.stdout[-900:]}"
)
assert "taking the high-water mark over the" not in log, (
f"a mark was salvaged from an empty read, which there is nothing to compute:\n{r.stdout[-900:]}"
)
seq = _posted_sequence(tmp_path)
assert len(seq) == 1 and seq[0]["state"] == "success", (
f"expected the exemption to be posted with the race check skipped; got {seq}"
)
def test_a_MALFORMED_creator_row_does_not_kill_the_job_after_the_green_is_posted(tmp_path):
"""A row whose `creator` is not an object used to be fatal, at the worst possible moment.
`.creator != null and .creator.login` hard-errors in jq on any non-object creator; jq exits 5 and
under `set -euo pipefail` the assignment takes the step down. That happens AFTER the exemption
`success` has been posted and BEFORE the repair is attempted, so one schema-corrupt row leaves a
green standing on a head that carries a genuine human rejection — and the job reports failure in a
way that looks like an unrelated infrastructure error.
The fix type-tests `creator` before indexing it, so the malformed row is dropped from the count
while the real verdict beside it is still counted and still repaired.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="malformed-creator-beside-verdict")
assert r.returncode == 0, f"the step died instead of skipping a malformed row: {r.stderr[-1200:]}"
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"a malformed `creator` row suppressed the repair, leaving the exemption green over the real "
f"verdict beside it. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
# THE REASON, not just the outcome — this is what makes the type guard individually provable.
# Three clauses can each rescue this fixture (the type test, the `|| raced=""` guard, and the
# fail-closed unusable-count branch), so the repair alone cannot tell them apart. Only the type
# test lets the REAL verdict beside the malformed row be counted as a genuine human race; without
# it the count comes back empty and the repair is reported as an unusable count instead.
assert "was overwritten" in (r.stdout + r.stderr), (
"the malformed row suppressed the real verdict beside it — repaired, but as an unverifiable "
f"read rather than as the human rejection it is:\n{r.stdout[-900:]}"
)
def test_a_verdict_BEYOND_A_SHORT_PAGE_is_still_found(tmp_path):
""" "Terminate only on a validated EMPTY page, never on a short one" — the rule that is invisible
against a faithful double.
On the real endpoint a short page IS the last page (measured: 50, 50, 14, `[]`), so an
implementation that stops at the first short page is indistinguishable from a correct one, and
every other test here would pass against it. The only way to observe the property is to model what
it actually guards: a TRUNCATED response. This stub serves 50 rows, then a short page of 10, then
a third page carrying the raced verdict.
A walk that treats the short page as exhaustion never reads page 3, misses the verdict, and leaves
the exemption green over a human rejection.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="verdict-after-short-page")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"a raced verdict beyond a SHORT page was not found, so the walk stopped on a short page "
f"instead of on a validated empty one. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
assert "was overwritten" in (r.stdout + r.stderr), (
f"repaired, but not reported as a found verdict:\n{r.stdout[-900:]}"
)
def test_a_PRE_WRITE_paging_failure_still_yields_a_usable_high_water_mark(tmp_path):
"""The fail-open a paging change can introduce while fixing one (ersatztv#763).
The high-water mark gates the post-write race check entirely: `max_id_before=-1` skips it, so a
human rejection landing in the write window is neither detected nor repaired. Before paging, only
a failure of the single page-1 request could reach that. Requiring a COMPLETE walk for the mark
would newly route a page-2 hiccup, an over-cap history, or one malformed id on a later page into
the same hole — a WIDER fail-open than the bug being fixed.
So a partial list still yields a mark. It can only be LOWER than the true maximum, which makes the
check more eager, never blinder.
Here page 2 fails on the PRE-write read only; the post-write read pages cleanly and carries a raced
verdict. With the mark salvaged from page 1 the verdict is above it and the exemption is repaired.
With `max_id_before=-1` the check never runs and the rejection stays green.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="premark-page2-error")
assert r.returncode == 0, r.stderr
log = r.stdout + r.stderr
assert "taking the high-water mark over the" in log, (
f"the partial-list fallback did not run, so the mark was not salvaged:\n{r.stdout[-900:]}"
)
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"the pre-write read could not be paged completely, the mark was abandoned, and the post-write "
f"race check was skipped — leaving a raced rejection green. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
@pytest.mark.parametrize(
"mode,why",
[
("second-page-garbage", "a non-JSON page 2"),
("second-page-error", "an HTTP error on page 2"),
],
ids=["garbage", "transport-error"],
)
def test_an_UNREADABLE_history_page_2_also_repairs_rather_than_leaving_green(tmp_path, mode, why):
"""Found by mutating the branch and watching nothing go red — my own coverage gap, in the same
class the review had just flagged twice.
This is the fail-toward-SUCCESS path, so uncertainty must resolve to `pending`. "I could not read
the rest of the history" is not evidence that no verdict raced this write; treating it as such is
exactly how a forged green survives over a human rejection.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode=mode)
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
f"the exemption was left standing despite {why} — a raced verdict beyond page 1 would be "
f"buried. Posts: {seq}\n{r.stdout[-900:]}"
)
assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", (
f"expected an exemption then a repair to pending; got {seq}"
)
# THE REASON IS PART OF THE FIX, so it is asserted. #761's stall was undiagnosable precisely
# because an uncertainty repair reported itself as an overwritten human verdict. Without these
# two the `raced_why` branch is unpinned — forcing it back to `human` reddens nothing.
log = r.stdout + r.stderr
assert "could not be read completely" in log, (
f"repaired, but not reported as an incomplete read:\n{r.stdout[-900:]}"
)
assert "was overwritten" not in log, (
"an uncertainty repair is reported as an overwritten human verdict — the false ::error:: that "
f"made PR #761's stall undiagnosable:\n{r.stdout[-900:]}"
)
# --- Workflow token scope (ersatztv#748) -------------------------------------------------------
#
# Both assertions guard a property whose violation is SILENT and, for the first, unrecoverable.
# `permissions:` is exhaustive: dropping a unit does not degrade, it 403s the `curl -sf` under
# `set -euo pipefail`, so `review-verdict/h10` is never posted and EVERY merge to `main` blocks —
# including the PR that would repair it, with no force-merge escape since ersatztv#743. And because
# `review-verdict.yml` is base-resolved, the PR making that edit CANNOT catch it: its own run uses
# the definition already on `main`. A test on the tree is the only place this is checkable at review
# time. Measured basis for the unit list: docs/ci-cd.md -> "Workflow token scope".
def _declared_permissions(path, job=None):
import yaml
doc = yaml.safe_load(path.read_text()) or {}
if job is None:
return doc.get("permissions")
return ((doc.get("jobs") or {}).get(job) or {}).get("permissions")
def test_the_verdict_gate_job_declares_exactly_the_units_its_api_calls_need():
"""The gate job's block is load-bearing in BOTH directions, so this pins the whole mapping.
Not just `code: write`. A missing read unit is quieter but still wrong: without `issues: read`
the retarget fence cannot establish a trusted count and the exemption is withheld, and without
`pull-requests: read` the enumeration degrades to `complete=no` and every exempt PR falls to a
generic `pending`. Those are recoverable, unlike a missing `code: write`, but they silently turn
the two exemption classes off — the ersatztv#751 failure shape, arrived at from a different
direction.
Equality, not containment: an EXTRA unit is a finding too. This job holds the only repo-write in
the repo's CI, so widening it (`contents: write`, a stray `packages:`) is exactly the drift the
scoping in ersatztv#697/#748 exists to prevent, and it would land unremarked as "just adding a
permission".
WHAT THIS DOES NOT CATCH, stated because the test's name over-promises: it TRANSCRIBES the unit
set, it does not DERIVE it from the job's API calls. Add a new `gh` call to the workflow that
needs an undeclared unit and this test stays green while the gate breaks in production — the
failure this test exists for, arriving from the other side. Deriving the mapping would mean
parsing every URL out of a 1300-line shell body and knowing Gitea's endpoint-to-unit table, which
is a different and much larger change. So: when you add an API call to this job, the unit list
here is a checklist to revisit by hand, not a net that will tell you. (Same honesty as
`test_the_verdict_workflow_has_NO_expression_delimiter_in_any_run_body` above.)
"""
wf = REPO_ROOT / ".gitea" / "workflows" / "review-verdict.yml"
assert _declared_permissions(wf) is None, (
"review-verdict.yml grew a TOP-LEVEL permissions: block. Only the job-level one on "
"set-verdict-status was probed (docs/ci-cd.md -> 'Workflow token scope'); adding a "
"top-level default changes what other jobs in this file would inherit and is unmeasured."
)
assert _declared_permissions(wf, "set-verdict-status") == {
"code": "write", # POST /statuses/{sha} — Gitea has no `statuses` scope, so this IS it
"issues": "read", # GET /issues/{n}/timeline — the ersatztv#706 retarget fence
"pull-requests": "read", # scripts/pr-changed-files.sh -> /pulls/{n}, /pulls/{n}/files
}, (
"the review-verdict gate job's permissions: block no longer matches the unit set that was "
"measured against its API calls. Dropping `code: write` makes review-verdict/h10 UNWRITABLE "
"and blocks every merge with no force-merge escape (ersatztv#743), and this workflow is "
"base-resolved so the PR changing it cannot detect that. Re-run the scratch-base probe "
"(docs/ci-cd.md -> Review-verdict gate) before changing this."
)
def test_every_tracked_workflow_declares_a_permissions_block():
"""Otherwise the convention is prose only, and a NEW workflow is the case that breaks it.
A workflow added without `permissions:` inherits the owner-level Actions default, which is
`permissive` today — i.e. a full read/write repository token, which is status-capable and can
therefore forge `review-verdict/h10` (`ci.actions-credential-scoping`). That is silent: nothing
reddens, the new file simply holds more than it needs.
Population comes from the GIT INDEX via `_workflow_files()`, not a directory listing — same
reason as the two completeness claims above (ersatztv#806/#778).
NO EXEMPTIONS, deliberately. `ci-image.yml` briefly needed one: editing that file re-pointed
`ci-image-pin`'s `expected` at the editing commit and reddened a BLOCKING job, and its own
`paths:` made the edit publish an image. ersatztv#744 took that path out of both
(`ci.toolchain-image-publish-is-a-dispatch`), so this asserts over the whole derived population
with nothing carved out — which is the form ersatztv#835 asked for. If a future file seems to
need an exemption, that is a finding about the file, not about this test.
"""
import yaml
workflows = _workflow_files()
# Anti-vacuity. `tracked_children` matches direct children of one directory, so a renamed or
# moved `.gitea/workflows/` yields an EMPTY population and every completeness claim below passes
# having examined nothing — the #778 shape this test's own docstring invokes. `_git_ls_files`'s
# assert covers the whole index, not this subset, so it does not catch it. Measured: after
# `git mv .gitea/workflows .gitea/wf-renamed` this test passed with 0 files before this guard.
assert workflows, (
"no tracked workflows found under .gitea/workflows — this test examined NOTHING and would "
"have passed vacuously. The directory was renamed/moved, or the glob no longer matches."
)
missing = []
for wf in workflows:
doc = yaml.safe_load(wf.read_text()) or {}
jobs = (doc.get("jobs") or {}).values()
# Top-level absent is fine only if EVERY job declares its own — one undeclared job still
# inherits the owner default, so `any` would pass a file that is half-covered.
declares = doc.get("permissions") is not None or (
bool(jobs) and all((job or {}).get("permissions") is not None for job in jobs)
)
if not declares:
missing.append(wf.name)
assert not missing, (
f"these workflows declare no permissions: block at either level: {missing}. Every workflow "
"in this repo declares one (ersatztv#748), with NO exemption since #744 landed, so a new "
"file cannot silently inherit the "
"owner-level default. Read-only (`permissions: {code: read}` at top level) is the "
"convention; declare write only with a stated reason at the declaration."
)