Files
ersatztv/scripts/tests/test_remote_state_inventory.py
T
timothyandClaude Opus 5 4261f76dd2
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 12s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 22s
PR Gates / Docs update reminder (pull_request) Successful in 17s
PR Gates / decisions lifecycle (pull_request) Successful in 24s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 10s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Canceled after 45s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Canceled after 0s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Canceled after 0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Canceled after 0s
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
review-verdict/h10 Awaiting review verdict for 4261f76
PR Gates / Script tests (pytest) (pull_request) Canceled after 19s
Review verdict / Set review-verdict status (pull_request_target) Successful in 20s
fix(778): derive the population from git, not the disk — the guard was red on every dev checkout
Sixth cold review (a different reviewer, in-repo, worktree-isolated after the
cross-family runs wedged twice on their sandbox). One High, one Medium, two Low, two
Nit. All fixed.

HIGH, and it is the third time this population has been wrong. `rglob` is recursive,
so it also enumerated `.husky/_/` — 17 husky shims generated by `npm ci` via
web/package.json's `prepare`, gitignored and untracked. The guard therefore derived 76
files against a 59-row table and was RED on every checkout that has run `npm ci`,
while staying GREEN in CI, whose `script-tests` job checks out and pip-installs but
never runs `npm ci`. A guard that fails everywhere except where it runs is the fastest
possible route to "that test is always broken, ignore it" — on the artifact whose
entire thesis is population correctness. Reproduced, then fixed at the source rather
than with a fourth traversal patch: the population now comes from `git ls-files`. The
index is authoritative, identical for CI and every checkout, and excludes untracked
build output by construction instead of by an exclusion list someone must maintain.
That is what this PR's own record says to do; the first three attempts each derived
from whatever happened to be on disk. Three tests go red against the rglob
predecessor.

MEDIUM — twin-missed, in the fix from the previous round. Round 4 re-read the base
before the branch-protection lookup, inside the scheduled branch only, leaving the
#632 retarget DETECTION still reading the top-of-hook snapshot. The reviewer
demonstrated it with this PR's own fixture: scheduled+retarget denied while
immediate+retarget AUTO-GRANTED. The re-read is now hoisted above every base-dependent
consumer, so one read serves both paths, and the duplicate is gone. Note for the
record: the hoist is the load-bearing part — once `live_base` is fresh, #632's own
comparison catches the retarget too, so the explicit deny only bites when no verdict
records a base. The tests are scoped to exactly that case, because as first written
they passed under mutation.

LOW — a 404 from `branch_protections/<ref>` does not prove the branch is unprotected.
Gitea keys that endpoint on the RULE name, so a base covered by a glob rule 404s while
being fully protected, and an unencoded ref containing `/` (`release/26.4`) 404s
because the path is malformed. Both produced a hard deny stating a specific, false
cause — and a deny blocks outright rather than prompting. The ref is percent-encoded,
and a 404 now consults the rule list before denying; an unreadable list asks.

LOW/NIT — the scope prose attached the extension restriction to `scripts/` alone while
the guard applied it everywhere (a `.py` hook would have joined the described scope and
acquired no row); `.yaml` workflows are now in scope too. The `PINNED` definition
required re-validation, which two legitimately-pinned rows do not do because their
check and use are one step over an immutable event-payload sha. Row ordering restored.

And once more, the recurring one: adding a scope TABLE to the doc made three prose
rows parse as inventory sites — the parser reading its own documentation as data, the
same defect as the UNSAFE-KNOWN check that once parsed the paragraph defining
UNSAFE-KNOWN. Row parsing is now bounded to the inventory section explicitly.

refs #778

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-19 17:53:25 +02:00

299 lines
15 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 fnmatch
import re
import subprocess
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 = (
("scripts", ("*.sh", "*.py")),
(".claude/hooks", ("*.sh",)),
(".husky", ("*",)),
(".gitea/workflows", ("*.yml", "*.yaml")),
)
# THE POPULATION IS DERIVED FROM `git ls-files`, NOT FROM THE FILESYSTEM.
#
# This is the third time this population has been found incomplete or wrong, each time by a
# different mechanism, and the third fix is the one that stops patching the traversal:
#
# 1. a content filter on 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
# structurally invisible;
# 2. a non-recursive `Path.glob`, which missed four nested files including one that calls a live
# ErsatzTV API and acts on the reply;
# 3. `Path.rglob`, which is recursive and therefore ALSO enumerated `.husky/_/` — 17 husky shims
# generated by `npm ci` via web/package.json's `prepare` script, gitignored (`.husky/_/.gitignore`
# is `*`) and untracked. That made this guard RED on every developer checkout while staying green
# in CI, whose `script-tests` job checks out and pip-installs but never runs `npm ci`. A guard
# that fails everywhere except where it runs is worse than no guard: it trains its readers to
# ignore it, and it would have done so on the artifact whose entire thesis is population
# correctness.
#
# The lesson each time was the same one this repo already wrote down — derive the population from an
# AUTHORITATIVE source — and the filesystem is not one. It reports build output, editor droppings and
# anything else that happens to be on disk, and it varies per machine. The repo's index is
# authoritative, versioned, identical for every checkout and for CI, and it excludes untracked
# generated files by construction rather than by an exclusion list that must be maintained.
#
# `scripts/tests/` is still excluded explicitly, because those files ARE tracked. That exclusion is a
# scope decision, reviewable in one line: they run only under pytest and authorize nothing. Their
# network activity is CONFINED rather than absent — `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 — but all of it is fixture state the test creates and tears down, so there is no live remote
# to race.
EXCLUDED_DIRS = ("scripts/tests",)
# `| ` + backticked path, optionally followed by ` — <site description>`, then the class cell.
_ROW = re.compile(r"^\|\s*`([^`]+?)`[^|]*\|\s*`([^`]+)`\s*\|", re.M)
def _tracked_files() -> list[str]:
"""Every file git tracks, as repo-relative posix paths.
Fails LOUDLY rather than returning nothing: an empty population would make every completeness
assertion below pass vacuously, which is the exact failure this guard exists to prevent.
"""
proc = subprocess.run(
["git", "-C", str(REPO_ROOT), "ls-files", "-z"],
capture_output=True, check=True,
)
return [p for p in proc.stdout.decode().split("\0") if p]
def derived_population() -> set[str]:
"""Every in-scope tracked file, as a repo-relative posix path.
No content predicate of any kind: a file that reads no remote state earns an explicit `N/A` row
rather than silently staying out.
"""
found: set[str] = set()
for path in _tracked_files():
if any(path == d or path.startswith(d + "/") for d in EXCLUDED_DIRS):
continue
for directory, globs in SCOPE:
if not (path == directory or path.startswith(directory + "/")):
continue
name = path.rsplit("/", 1)[-1]
if any(fnmatch.fnmatch(name, pattern) for pattern in globs):
found.add(path)
break
return found
def _inventory_section(text: str) -> str:
"""Only the classification tables, never the surrounding prose.
This is the second time this parser has read the document's own explanation as data: the
UNSAFE-KNOWN justification check once parsed the "Columns" paragraph that DEFINES
`UNSAFE-KNOWN`, and adding a scope TABLE to the heading made three more prose rows parse as
sites. A guard that treats its own documentation as input is the failure this whole change is
about, so the boundary is explicit rather than left to a cleverer regex.
"""
start = text.index("## The inventory")
end = text.index("## Limits", start)
return text[start:end]
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(_inventory_section(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")
# Inserted INSIDE the inventory section, not appended to the file: rows are parsed only between
# "## The inventory" and "## Limits", so appending at the end would test nothing.
phantom = text.replace(
"## Limits",
"| `scripts/does-not-exist.sh` — invented | `PINNED` | n/a |\n\n## Limits", 1)
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_section(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}")
def test_the_population_never_includes_a_file_git_does_not_track(monkeypatch):
"""The regression for the third population defect, and the reason the source is the index.
`Path.rglob` enumerated `.husky/_/` — 17 husky shims generated by `npm ci`, gitignored and
untracked — so this guard was RED on every developer checkout and GREEN in CI, which never runs
`npm ci`. A guard that fails everywhere except where it runs trains its readers to ignore it.
Asserted through the mechanism rather than against the current disk, so it holds on a machine
that has never installed husky: the tracked list is narrowed, and anything outside it must
disappear from the population even though it is still sitting on disk and still matches the
scope globs.
"""
real = derived_population()
assert real, "empty population — the derivation is broken, not the repo"
victim = sorted(real)[0]
tracked = [p for p in _tracked_files() if p != victim]
monkeypatch.setattr(
"scripts.tests.test_remote_state_inventory._tracked_files",
lambda: tracked,
raising=False,
)
# Patch the module object this test is running inside, whatever name it was imported under.
import sys
mod = sys.modules[__name__]
monkeypatch.setattr(mod, "_tracked_files", lambda: tracked)
assert (REPO_ROOT / victim).is_file(), (
f"{victim} must still exist on disk for this proof to mean anything")
assert victim not in derived_population(), (
f"{victim} is on disk and matches the scope, but git no longer tracks it — it must not enter "
"the population, or untracked build output can redden this guard again")
def test_every_derived_member_is_tracked():
"""The same property stated as an invariant over the real tree, so a future refactor that goes
back to walking the filesystem fails here rather than on someone's laptop."""
tracked = set(_tracked_files())
stray = sorted(p for p in derived_population() if p not in tracked)
assert not stray, f"population contains untracked path(s): {stray}"