…be tier-invisible
test_patterns_cover_bash_guard compared scrub.py's SECRET_PATTERNS against
the DEPLOYED hook (Path.home()/'.claude'/'hooks'/'bash-guard.py') and
skipped when that file was absent. Two problems, and the second is why it
sat broken:
1. WRONG REFERENT. The deployed copy is generated from the repo file by
home-manager, so the test answered a question about the machine, not
about the commit.
2. IT COULD NOT FAIL IN CI. Keyed on $HOME it SKIPPED in the nix sandbox
and RAN only on a switched dev host. When #276 moved SECRET_PATTERNS
out of bash-guard.py into guard_core.py, the ast parser returned []
and the test failed on every dev host while the hermetic gate — the
tier that gates merges — stayed green and silent.
Now compares two files both TRACKED IN THIS REPO
(session_insight/scrub.py vs claude-hooks/guard_core.py), so it runs in
every tier and NEVER skips. Renamed to test_patterns_cover_guard_core.
The empty-parse case is now an explicit, loud assertion naming the cause
rather than the silence that hid #276.
🔴 run-tests.sh: the conditional EXPECTED_SKIPS entry that pinned the old
environment-dependent skip is REMOVED — mandatory, not cosmetic. The test
no longer skips, so leaving the pin would fail the gate with 'FEWER than
pinned'. Replaced with a comment saying why it must not come back.
REACHABILITY — both assertions broken on purpose, each failing for its OWN
reason (not a neighbour's):
* inject a pattern into guard_core.py that scrub.py lacks
-> 'scrub.py drifted ... no longer covered:
['\\bmutant-XYZ-[0-9]{9}\\b']'
* rename the literal, reproducing #276's move
-> 'could not parse SECRET_PATTERNS from guard_core.py ...'
guard_core.py restored byte-identical after each.
session_insight suite: 57 passed, 0 skipped (was 56 passed + 1 skipped in
the sandbox / 56 passed + 1 FAILED on a dev host).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second of the three regressions #276 shipped. #289 fixed the first, #290 the third.
The bug
test_patterns_cover_bash_guardasserted thatscrub.py'sSECRET_PATTERNSis a superset of the Bash hook's, byast.parseing the deployed hook:#276 moved
SECRET_PATTERNSout ofbash-guard.pyintoguard_core.py. The parser returned[]and the test began failing:Why it stayed broken — the part that matters
Keyed on
$HOME, the test skipped in the nix sandbox (synthetic HOME, no hook) and ran only on a switched dev host. So it failed on every dev host — breaking the pre-push tier — while the hermetic gate, the tier that actually gates merges, stayed green and silent.That is the property you asked me to make sure I addressed: the sandbox tier could not fail on it. It is the exact complement of #290's defect, which failed only in the sandbox. Both hid behind #289's red.
I also caught it changing under me mid-session: the same tree passed at 16:45 and failed at 17:03, because a concurrent
home-manager switchswapped the deployed hook.The fix, and the options I considered
I did not pick silently. Three ways to fix it:
A — repoint at the deployed
~/.claude/hooks/guard_core.py. Smallest diff. Rejected: keeps the tier-invisibility. It would still skip in the sandbox, so the next time this breaks, the gate again cannot see it.B — compare repo ↔ repo (chosen). Both
session_insight/scrub.pyandclaude-hooks/guard_core.pyare tracked in this repo, so the test runs in every tier and never skips. The deployed copy is generated from the repo file by home-manager, so the repo is the correct referent anyway: pointing at$HOMEanswered a question about the machine, not about the commit. It also removes a pinned skip rather than adding one.C — B plus an extra deployed-copy check that skips when absent. Strictly more coverage on dev hosts. Rejected for now: it re-introduces an environment-dependent skip and a pin to maintain, to catch a case (deployed copy diverging from its own generator) that home-manager makes structurally impossible. Happy to add it if you disagree — it is additive.
The empty-parse case is now a loud, named assertion instead of the silence that hid #276.
🔴 A required companion change, not a cosmetic one
The test no longer skips, so the conditional
EXPECTED_SKIPSentry pinning that skip must go — otherwise #284's accounting fails the gate with "FEWER than pinned: a pinned skip now RUNS". Removed, with a comment explaining why it must not come back:Reachability — both assertions broken on purpose
Each fails for its own reason, not a neighbour's:
guard_core.pythatscrub.pylacksscrub.py drifted from guard_core.py — these SECRET_PATTERNS are no longer covered: ['\bmutant-XYZ-[0-9]{9}\b']could not parse SECRET_PATTERNS from guard_core.py …guard_core.pyrestored byte-identical after each (verified against HEAD, not assumed).Counts
session_insightsuite: 57 passed, 0 skipped — was56 passed + 1 skippedin the sandbox and56 passed + 1 FAILEDon a dev host.mainmerging fix(gate): add nix to the pytests check — 10 handle tests could not run in the sandbox #290 + this PR — both touchrun-tests.shin different regions;ortauto-merged, and I checked the merged result is semantically coherent (nix-instantiatepresent inREQUIRED_TOOLS, conditional skip gone,EXPECTED_SKIPSdown to one entry) rather than trusting a clean textual merge.nix flake checkresult on that tree is in a follow-up comment.Merge order
Independent of #290 textually, but #290 should land first — on current
mainthis branch alone leaves the sandbox red for #290's reason. Neither is stacked on the other.🤖 Generated with Claude Code