Files
ersatztv/scripts/tests/test_remote_state_inventory.py
T
timothyandClaude Opus 5 25048d9ea4
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 11s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 20s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 17s
PR Gates / decisions lifecycle (pull_request) Successful in 25s
PR Gates / Docs update reminder (pull_request) Successful in 21s
review-verdict/h10 Awaiting review verdict for 25048d9
Review verdict / Set review-verdict status (pull_request_target) Successful in 19s
PR Gates / Script tests (pytest) (pull_request) Successful in 4m1s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m33s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m43s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Canceled after 22s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Canceled after 0s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Canceled after 0s
fix(778): round 3 — the fence covers BASE aliasing only, and stop citing a moving count
Third cold review: no Blockers, 1 High / 5 Medium / 1 Low / 1 Nit. All accepted.
It independently re-derived the 59-file population and matched it against `find`,
so the traversal that was wrong in rounds 1 and 2 is now verified rather than argued.

HIGH — the ABA claim was too broad. `ci.verdict-write-retarget-fence` counts
`change_target_branch` events, so it fences the BASE axis and nothing else. A
force-push H1 -> H2 -> H1 during pagination leaves the final `.head.sha` comparison
equal while the middle pages were enumerated against H2, and no counter moves. Two
rows implied the fence covered that; both now state the head residual as real and
unfenced, with what closing it would take.

Also: the record still said the scheduled-merge residual was "closed one layer down"
by the branch protection an admin may have removed — the circular sentence that was
rewritten in the inventory last round and left standing in its twin. The hook header
still called an immediate merge "sound". Both now describe the bounded window.

`docs-reminder` was over-demoted by grouping it with `decisions-guard`: it cannot
fail its job, so it authorizes nothing and is N/A, while `decisions-guard` reaches
the combined status. Split, per this file's own rule that differing classifications
get separate rows. Over-demotion is a defect too — it makes the column noise.

The scope heading and the docs/README entry now name the `scripts/tests/` exclusion
explicitly, so nobody adds a remote-reading test executable expecting a red guard
that stays green, and a wrong limit cross-reference is corrected. The exclusion's
justification was also factually false — it claimed the only network calls were to
PATH stubs, but test_hook_fire_log.py starts a real http.server on 127.0.0.1 and
drives it with real curl. The exclusion stands on confinement, not on absence, and
now says so.

COUNTS. "4 of 68 PINNED" was wrong (3), and rewriting it as "3 of 68" went stale in
the same commit when splitting a row moved the denominator to 69 — three stale
figures in three rounds, in the record warning against exactly this. The exact
denominator is gone: three rows survive as PINNED out of roughly seventy, and the
load-bearing claim is "almost nothing is pinned", not an integer. A hand-maintained
count is a second copy of the table; guard-inventory.md gives its counts an equality
check because they ARE the point, and a rationale record should not pretend to.

refs #778

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-16 13:58:40 +02:00

206 lines
11 KiB
Python

"""`docs/remote-state-inventory.md` covers exactly the executables that talk to a remote service
(ersatztv#778).
WHAT THIS CANNOT DO, said first because #778's own issue body says it: there is no lint for "this
code should have pinned a sha." `docs/defect-shapes-773.md` §4 grades detector D as a *fix pattern*
whose detector is detector A applied to an enumerated inventory. So this file does not try to grade
pinning. It guarantees that every in-scope file has been CLASSIFIED BY SOMEONE, and that no file
joined the scope without acquiring a row — which converts "remember to think about this" into "the
suite is red until you have".
The split is deliberate and mirrors `test_guard_inventory.py`:
* the POPULATION is derived from the filesystem and compared for SET EQUALITY, both directions;
* the CLASSIFICATION vocabulary is closed, so a typo cannot invent a state;
* whether a `PINNED` row is TELLING THE TRUTH is not checked here and cannot be. That stays with
review, and the inventory's prose is what review reads.
SCOPE vs POPULATION, per `testing.guard-derives-population-from-source`: the SCOPE — four
directories, two file extensions, and one excluded subdirectory — is a hand-written policy choice
and is reviewable as one. The POPULATION inside that scope is derived on every run, RECURSIVELY,
with no content predicate at all: a file that reads no remote state earns an explicit `N/A` row
rather than staying out.
That wording is the third version, and the history is the point. The first filtered the scope by an
outbound-network token list, which omitted `git fetch` — this repo's commonest remote read — so a
hook that fetches `origin/main` and derives a push decision was invisible. The second dropped the
filter but used non-recursive `glob`, so four nested files were invisible, including one that calls
a live ErsatzTV API and acts on the reply. Both were the same error at different depths: a
completeness claim resting on a traversal nobody had compared against the filesystem. Hence the
anti-vacuity floor below, and hence the scope being stated as directories rather than as a
predicate over content.
"""
from __future__ import annotations
import re
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parents[2]
INVENTORY = REPO_ROOT / "docs" / "remote-state-inventory.md"
CLASSES = {"PINNED", "CAS", "UNSAFE-KNOWN", "N/A"}
# NO CONTENT FILTER. The population is every file in the scoped directories, and a file that reads
# no remote state earns an `N/A` row rather than silently staying out.
#
# The first version filtered on a token list (`curl`, `wget`, `urllib`, ...) and called that a
# SCOPE choice rather than a population filter. Cold review rejected the distinction and was right:
# the list omitted `git fetch`, which is this repo's most common remote read, so
# `.claude/hooks/prepush-rebase-check.sh` — which fetches `origin/main` and derives a PUSH DECISION
# from it — was structurally invisible to a guard whose stated claim is "every executable that
# reads live remote state". Three more (`prepush-clean-worktree-check.sh`, `ci-detect-docs-only.sh`,
# `refresh-shared-checkout.sh`) were missing for the same reason.
#
# That is precisely the defect `testing.guard-derives-population-from-source` describes: a filter
# cannot see the member that is missing, because the absent member is not a row the predicate
# rejected, it is a row that was never produced. The defence offered for it — "over-inclusion is the
# safe direction" — was answered by the filter ALSO under-including. Enumerating the directories
# costs more rows and has no blind spot; that is the trade the rule already made.
SCOPE = (
(REPO_ROOT / "scripts", ("*.sh", "*.py")),
(REPO_ROOT / ".claude" / "hooks", ("*.sh",)),
(REPO_ROOT / ".husky", ("*",)),
(REPO_ROOT / ".gitea" / "workflows", ("*.yml",)),
)
# RECURSIVE. `Path.glob` is not, and the second cold review found the consequence: four nested files
# were outside the population, one of them `scripts/scripted-schedules/entrypoint.py`, which calls
# `get_context` against a live ErsatzTV server and then acts on the result. That is the third time
# this population has been found incomplete, each time by a different mechanism — first a content
# filter that omitted `git fetch`, then a non-recursive glob. Both were the same error: a claim of
# completeness resting on a traversal nobody had checked against the filesystem.
#
# `scripts/tests/` is the one DIRECTORY-level exclusion, and it is a scope decision rather than a
# content filter: those files run only under pytest and authorize nothing. Their network activity is
# CONFINED rather than absent — the first wording here claimed the only calls were to PATH-installed
# stubs, which cold review falsified: `test_hook_fire_log.py` starts a real `http.server` on
# 127.0.0.1 and drives it with real `curl`, and several suites create real local git remotes. All of
# it is fixture state the test itself creates and tears down, so there is no live remote to race,
# which is why the exclusion stands — but the justification had to be true to be worth anything.
# Excluding a directory is reviewable in one line; excluding files by what they contain is what kept
# failing.
EXCLUDED_DIRS = (REPO_ROOT / "scripts" / "tests",)
# `| ` + backticked path, optionally followed by ` — <site description>`, then the class cell.
_ROW = re.compile(r"^\|\s*`([^`]+?)`[^|]*\|\s*`([^`]+)`\s*\|", re.M)
def derived_population() -> set[str]:
"""Every in-scope file, as a repo-relative posix path. No content predicate of any kind."""
found: set[str] = set()
for directory, globs in SCOPE:
if not directory.is_dir():
continue
for pattern in globs:
for path in sorted(directory.rglob(pattern)):
if not path.is_file():
continue
if any(excluded in path.parents for excluded in EXCLUDED_DIRS):
continue
found.add(path.relative_to(REPO_ROOT).as_posix())
return found
def inventory_rows(text: str | None = None) -> list[tuple[str, str]]:
if text is None:
text = INVENTORY.read_text(encoding="utf-8")
return _ROW.findall(text)
def inventory_sites(text: str | None = None) -> set[str]:
return {site for site, _ in inventory_rows(text)}
def test_the_inventory_file_exists_and_is_not_empty():
assert INVENTORY.is_file(), f"{INVENTORY} is missing"
assert INVENTORY.stat().st_size > 0
def test_anti_vacuity_the_derivation_and_the_table_both_found_something():
"""The characteristic failure of a completeness check is reporting success over an empty
population. Both sides get a floor, because either one collapsing to zero would make the set
comparison below pass trivially."""
population = derived_population()
sites = inventory_sites()
assert len(population) >= 40, (
f"derived only {len(population)} in-scope files — the globs are broken, not the repo "
"(the scope held 59 files on 2026-08-16, and it only grows)")
assert len(sites) >= 40, (
f"parsed only {len(sites)} rows out of the inventory — the row regex has drifted from the "
"table format")
def test_every_in_scope_file_has_a_row_and_every_row_names_a_real_file():
"""Set equality in BOTH directions, because the two failures are different defects and a single
'sets differ' message invites fixing one and re-running.
MISSING: a script that talks to a remote service and was never classified — the defect #778
exists to prevent. PHANTOM: a row for a file that was renamed or deleted, which leaves the table
claiming coverage it has lost.
"""
population = derived_population()
sites = inventory_sites()
missing = sorted(population - sites)
phantom = sorted(sites - population)
assert not missing, (
"in scope but absent from docs/remote-state-inventory.md (classify each as "
f"PINNED / CAS / UNSAFE-KNOWN / N/A): {missing}")
assert not phantom, (
f"listed in docs/remote-state-inventory.md but no such in-scope file exists: {phantom}")
def test_MUTATION_PROOF_a_dropped_row_and_a_phantom_row_are_both_detected():
"""The proof that the set comparison above is load-bearing (`testing.guard-ships-with-mutation-
proof`). This guard IS a test, so disarming it makes it absent rather than red; the admissible
proof is therefore the contrapositive — introduce the defect into an isolated copy of the
GUARDED ARTIFACT and show the comparison reports it.
Both directions are mutated, because they are different defects: a dropped row is an
unclassified script, a phantom row is a table claiming coverage it has lost. This ran for real
on the day it was written — `dependency-scan.yml` was genuinely absent from the first draft of
the inventory and this comparison is what found it.
"""
text = INVENTORY.read_text(encoding="utf-8")
population = derived_population()
victim = sorted(population)[0]
dropped = "\n".join(
line for line in text.splitlines() if not line.startswith(f"| `{victim}`"))
assert victim not in inventory_sites(dropped), (
f"the mutation did not actually remove {victim}; the proof below would be vacuous")
assert population - inventory_sites(dropped), (
"a row was removed from the inventory and the comparison still reported complete coverage")
phantom = text + "\n| `scripts/does-not-exist.sh` — invented | `PINNED` | n/a |\n"
assert inventory_sites(phantom) - population == {"scripts/does-not-exist.sh"}, (
"a row naming a file that does not exist was not reported as phantom")
def test_every_class_cell_comes_from_the_closed_vocabulary():
bad = sorted({cls for _, cls in inventory_rows() if cls not in CLASSES})
assert not bad, (
f"unknown classification(s) {bad}; allowed: {sorted(CLASSES)}. A typo here would silently "
"create a state nobody reviews.")
def test_every_unsafe_row_states_why_the_residual_is_accepted():
"""`UNSAFE-KNOWN` means 'accepted with a reason', not 'noticed'. A row that records the window
without the argument for tolerating it is how a deferral becomes permanent by default."""
text = INVENTORY.read_text(encoding="utf-8")
thin = []
for line in text.splitlines():
# TABLE ROWS ONLY. The first version matched any line containing the token, so the prose in
# "Columns" that DEFINES `UNSAFE-KNOWN` was parsed as a row and the split blew up. A guard
# that reads its own documentation as data is the failure this whole change is about.
if not line.startswith("|") or "`UNSAFE-KNOWN`" not in line:
continue
cells = [c.strip() for c in line.strip().strip("|").split("|")]
if len(cells) < 3:
continue
site, note = cells[0], cells[-1]
if len(note) < 120:
thin.append(site[:60])
assert not thin, (
f"UNSAFE-KNOWN row(s) with no stated justification: {thin}")