Files
ersatztv/scripts/tests/test_remote_state_inventory.py
T
timothyandClaude Opus 5 a0849eec58
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 18s
PR Gates / decisions lifecycle (pull_request) Successful in 24s
PR Gates / Docs update reminder (pull_request) Successful in 9s
review-verdict/h10 Awaiting review verdict for a0849ee
Review verdict / Set review-verdict status (pull_request_target) Successful in 17s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 13s
PR Gates / Script tests (pytest) (pull_request) Successful in 3m58s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m43s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m30s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m29s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 6s
fix(778): address the cold review — drop the population filter, grade down three overclaims
Independent cross-family review (Codex, cold brief, read-only worktree) returned
BLOCKED with 9 findings. All 9 accepted; #5 partially, keeping one row PINNED with
its justification stated.

POPULATION (the finding that mattered most). The derivation filtered the scope by an
outbound-network token list and argued that was a scope choice rather than a
population filter. It omitted `git fetch` — this repo's most common remote read — so
prepush-rebase-check.sh, which fetches origin/main and derives a PUSH DECISION, was
structurally invisible to a guard claiming to cover "every executable that reads live
remote state", along with three others. The defence offered was that over-inclusion is
the safe direction; the filter also under-included. The content filter is gone: the
population is now all 55 files in the scoped directories, and a file that reads no
remote state carries an explicit N/A row.

OVERCLAIMS, graded down. Three rows asserted more than the code does:
- the scheduled-merge path was PINNED while the hook's own comment concedes the
  branch-protection read pins nothing → UNSAFE-KNOWN, "preflight, not a pin";
- pr-changed-files.sh was PINNED and claimed "any movement fails", but
  before-and-after equality is ABA-vulnerable (main → scratch → main) → UNSAFE-KNOWN,
  pointing at the caller-side event-count fence that does close it;
- the CI toolchain image was PINNED on a mutable TAG, against this file's own
  definition naming a digest → UNSAFE-KNOWN. The release smoke pull stays PINNED: it
  pulls the tag the same concurrency-serialized job just pushed.

The guard-inventory MUTATION regrade is reverted to BEHAVIOUR-ONLY (8/6/20). The
review is right on species: the test feeds the real script an input the clause
rejects, which this table explicitly defines as behaviour-only and has already
regraded three rows for. A manually-executed disarm does not change what the test is.

TWO REAL FAIL-OPENS FIXED:
- jq `index()` on a STRING is substring search, so a status_check_contexts arriving
  as "prefix-review-verdict/h10-suffix" answered yes and would auto-grant. Membership
  is now exact equality over a value first proven to be an array of strings.
- post-review-verdict.sh guarded both re-read comparisons with `[ -n "$x" ] &&`, so a
  2xx body that merely omitted .head.sha or .base.ref made the check a no-op and the
  status was posted having confirmed nothing.

That second fix carries a lesson worth the line: the obvious mutation (disarm the new
`-z` arm) stays GREEN, because the unconditional `!=` also rejects empty — the two
overlap, exactly the duplicate-guards-mask-each-other shape. The proof is taken
against the REAL predecessor with the `-n` conjunct restored, which goes red showing
returncode=0 and a status written.

Also: 404 is now separated from 403/transport (an unprotected branch is the strongest
form of the finding; `curl -sf` collapses both to an empty string), and the positive
control asserts the decision is `allow` and that the endpoint was actually reached,
rather than the absence of one phrase.

refs #778

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

176 lines
8.6 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 (which
directories, and the outbound-network token set) is a hand-written policy choice. The population
inside that scope is derived every run. The token filter deliberately OVER-includes — a file
mentioning `curl` only in a comment still needs a row — because over-inclusion costs one row and
under-inclusion costs a blind spot.
"""
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",)),
)
# `| ` + 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.glob(pattern)):
if path.is_file():
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}")