Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 22s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 23s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 9s
PR Gates / decisions lifecycle (pull_request) Successful in 24s
review-verdict/h10 Awaiting review verdict for 84b4c11
Review verdict / Set review-verdict status (pull_request_target) Successful in 22s
PR Gates / Script tests (pytest) (pull_request) Successful in 4m10s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m22s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Canceled after 5m4s
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
Second cold review: no Blockers, 4 High / 1 Medium / 2 Low. All accepted. POPULATION, WRONG A SECOND TIME. Round 1 removed a content filter that had omitted `git fetch`. Round 2 found the replacement traversal used non-recursive `Path.glob`, so four nested files were still outside it — including scripts/scripted-schedules/entrypoint.py, which calls get_context() against a live ErsatzTV server and then drives define_content/reset_playout/build_playout off the result. Now rglob, with scripts/tests/ as the single stated DIRECTORY-level exclusion (a scope choice, reviewable in one line; not a predicate over content). Population 55 -> 59, rows 63 -> 68. The generalisation is in the record, because the deliverable made the same mistake twice: the scope may be hand-written, but anything narrowing the POPULATION has to be executed and its output compared against the filesystem — the members it drops are invisible by construction. That is the #774 rule turned on the artifact meant to enforce it. FIVE MORE OVERCLAIMS GRADED DOWN. Both merge-consent head/base rows (the hook returns `allow` and a separate call merges, so the window is small, not absent — "no async window" was simply false); the release smoke pull and the ci-image verify (the concurrency group is PER-REF, so a branch build and a tag build of one commit can publish the same :<short-sha>); and the workflow base-fetch rows, which are not advisory — the merge hook reads the COMBINED status, so any red context blocks the auto-grant. Also fixed a stale cross-reference where the enumeration row still said it "inherits that row's pins" from a row graded down in the same commit. Four PINNED rows survive out of 68. That ratio is the honest finding. CIRCULAR JUSTIFICATION REMOVED. The scheduled-merge row said its residual was "closed one layer down" by the very branch protection an admin may have removed. It is not closed, it is BOUNDED by a trust assumption, and the row now says so. Low: jq's `//` fires on `false` as well as null, so `status_check_contexts: false` was defaulted to [] and produced a confident deny from a shape never understood — absent and null are now defaulted explicitly, everything else is "unknown". And the title sentence claimed "every executable in this repo" while the guard covers four directories; both it and the docs/README entry now say what is actually enforced. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
201 lines
10 KiB
Python
201 lines
10 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, authorize nothing, and their only network calls
|
|
# are to stub executables the tests themselves place on PATH. 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 holds 55 files as of 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}")
|