fix(806): guard populations over FILES derive from the git index, not a filesystem walk #818
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#818
Reference in New Issue
Block a user
fixes #806
testing.guard-derives-population-from-source(#774) says a completeness guard derives its population from an authoritative source, and its worked examples are an enum and the generated OpenAPI document. It was silent on the commonest population in our own guards — files in a directory — and every one of them answered with a filesystem walk. A walk is not authoritative: it reports build output, generated shims and editor droppings, and it differs per machine.What shipped
Converted (a completeness claim over tracked files):
test_guard_inventory.py,test_hook_fire_log.py,test_ci_image_pin_population.py(which also gained*.yaml— Gitea accepts both spellings, so a.yamlworkflow adopting the toolchain image was structurally invisible), andtest_remote_state_inventory.py(folded onto the shared derivation, so the rule has one implementation).Assessed and recorded, not silently skipped:
_repo_copyintest_ci_release_path_scan_job.py(not a completeness claim, but takes its file list from the index for hermeticity);test_ci_dropped_step_guard.py(no filesystem population at all); the decisions corpus (recorded as unexamined rather than cleared). This is not "replace every glob".scripts/tests/tracked_files.pyis the single derivation.test_guard_populations_derive_from_git.pyproves it in two directions that are measured complements — exhaustive removal catches a hardcoded.exists()admit and memoisation; the call-log check catches an append-only source that yields nothing on this machine (#778's shape), which removal cannot see because it has nothing to remove.Docs: the decision record gains the file-population case;
docs/guard-inventory.mdcarries the per-guard audit table.Review
Eight rounds of independent cold review, alternating Codex GPT-5.6 and Opus, every one in an isolated worktree from a cold review-only brief. Every round confirmed the four production derivations and the shared helper sound; every blocking finding after round 1 was in the proofs or in prose claims about them, which is worth recording because it says where the risk actually was.
Findings that changed the code, each executed by the reviewer rather than argued:
derived_guard_filesstill admitted untracked files — the CALLERS were index-gated but the paths they NAME were admitted onPath.exists(). #778's third shape, inside the guard this change calls its model_git_ls_fileswas imported by name, so the patch never reached the ref-gating branchPOPULATION-EXEMPTmarker was a whole-file substring scan — a silent kill switch trippable from another file's prose. Withdrawn, not patchedtest_*.pyprobe is collected by pytest, names collide across concurrent runs,finallydoesn't survive SIGKILL. Withdrawncallsevidence on the success path —try/except Exceptionaround anrglobwas a false greenderive()'s return value, so a lazy generator ran after the patch exited_EmptyScanwas iterable but not an iterator (soos.walkstill polluted the report); the boundary was stated as "synchronous", then "while the patch is active", both falsified by measurement — it is the call log;glob.globroutes throughglob.iglob, notos.scandir; rebase-stale member countsTwo are worth calling out as process, not code:
git commit --amendhad used only the staged tree, so the manifest entry and prose corrections were never in the commit — it would have pushed red. It found this by exportinggit archive HEADand running that, not the working tree. The committed tree is now verified the same way (via a throwaway worktree, since an archive isn't a git repo — the exact failure_repo_copy's docstring warns about).Rebase
Rebased across two
mainmoves. #790 ("execute every MUTATION grade instead of asserting it") landed mid-review and touches the same two files — the "green separately, red together" case, and it fired: three tests went red on the rebase. The new guard now declares an executable clause inmutation_manifest.py, using the real defect round 1 found; #790's harness runs it and reddens exactly the named proof for the declared reason. Reviewers mechanically diffed every #790 line to confirm nothing was lost.Verification
870 passed, 2 skipped— and re-verified against the committed tree, not just the working treedecisions-validateOK; catalog up to date; doc-narrative 9 advisories, all pre-existing.husky/_absent; enumeration swallowed byexcept Exception; a lazy generator; the hardcoded.exists()gate;tracked_childrenmade recursive; the exemption removed; the AST matcher narrowedKnown residuals, stated rather than implied
The call-log check is a regression guard against the shapes that arrive by accident, not a boundary: enumeration hoisted to module import time,
from os import listdirearly binding, and anything listing in another process all pass.importlib/star-import/re-export are invisible to the AST matcher. Nested workflow directories are out of scope. Each is written down where the code is, once.test_pr_changed_files.pytoo, and correct three counts this change invalidatedCold review of the previous commit found that IT had made `docs/guard-inventory.md`'s "load-bearing for the six modules that import it" false, by adding a seventh importer — in the file it was editing, inside the sweep its own message claims to have done by subject. That is the third hand-maintained count in this change to go stale, after `mutation-claims-are-executed.md`'s three and `guard-inventory.md`'s "three of the thirteen". Correcting a number that tracks a growing population buys one commit of accuracy. So the counts that track populations are gone rather than updated: * "load-bearing for the six modules that import it" → "for every module that imports it" * "this file is not four near-copies" → "not one near-copy per derivation" * "adding a fifth derivation / a fifth index-derived guard" (twice) → "another" * "five guard modules would otherwise die" → "every module that imports this one" * "Exhaustive removal is cheap (~1s for 60 members)" → "about a second at the population sizes here" The sweep was then done MECHANICALLY — a regex over every spelled-out and numeric count in all five artifacts this change touches — rather than by eye for a fourth time. What survives is verified true rather than assumed: "three of the fourteen" and "36 guards … 14/6/16" (the latter machine-asserted by `test_the_summary_counts_match_the_table`), "unions four contributors", "five modules each derived a file population" (`DERIVATIONS` holds exactly five), and the re-measured 61/39/48/58. `guard-inventory.md`'s "three of these four rows" is #790's text about a different row set and is untouched. The figures that remain are the ones that describe a FIXED structure or carry a reproduction command; the ones that counted a set expected to grow are the ones that kept breaking, which is the distinction the sweep now encodes. Correction to the previous commit message: it called the second converted call site "the delimiter ban over every workflow". That guard is `test_every_workflow_expression_names_a_REAL_context_or_function`, an expression-context check; the delimiter ban does not use `_workflow_files()`. Both call sites are completeness claims over every workflow and both were demonstrated red against an untracked `.yaml` at the predecessor sha — only the label was wrong. refs #806 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Cold review returned MERGEABLE with two notes; both are corrections to claims this change ships, so both are taken. THE PREVIOUS COMMIT CLAIMED A REGEX OVER EVERY COUNT IN ALL FIVE ARTIFACTS and left four. They were dated and true, which is why they read as acceptable, but a single added `MUTATION` row stales all four at once — the same fuse as the counts already removed, and the claim of a complete sweep is what made keeping them dishonest rather than merely conservative: * "three of the fourteen entries" → "three entries" (it names them; the total contributes nothing) * "the remaining eleven redden behaviourally" → "every other entry reddens behaviourally" * "thirteen of the fourteen admit clause-level proof" → "all but one" * `guard-inventory.md`'s "three of the fourteen rows" → "three rows" An independent sweep with a wider pattern now returns only counts over FIXED structures — #790's four ranked rows, `derived_guard_files`'s four union terms, #685's two guards — plus the machine-asserted `36 guards … 14/6/16`, and the member figures, which are dated, carry their reproduction command, and say in place that they illustrate the gap above the floor rather than being values to assert against. That is the distinction worth keeping: a number describing a fixed shape, or one that ships with the command that regenerates it, does not rot the way a tally of a growing set does. `docs/guard-inventory.md`'s new "listed in `DERIVATIONS` there" had the wrong nearest antecedent — `DERIVATIONS` lives in `scripts/tests/test_guard_populations_derive_from_git.py`, not in `tracked_files.py`. Named explicitly. refs #806 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Review-verdict: MERGEABLE @
ded6cb1Twelve rounds of independent cold review (Codex GPT-5.6 + Opus alternating, isolated worktrees, cold briefs). Production derivations and the shared helper confirmed sound every round; final two rounds MERGEABLE on this head. CI fully green: 12 jobs, 1 skipped, no failures, no cancellations. One further instance of the shape found by whole-repo sweep and deferred to #819 with the obstacle documented.