diff options
Diffstat (limited to 'todo.org')
| -rw-r--r-- | todo.org | 14 |
1 files changed, 10 insertions, 4 deletions
@@ -55,10 +55,15 @@ Grading: Minor severity (nothing broken now; the exposure is a future regression Fix: add =claude-templates/bin/*= to the =check_hook= loop, the same shape =languages/*/githooks/*= already uses for extensionless files. A no-op today by design — it passes immediately — which is exactly what a guard should do. -** VERIFY [#A] Parked: the secret-scan pre-commit fails open in ALL FIVE bundles (from .emacs.d) +** DONE [#A] Applied: the secret-scan pre-commit fails open in ALL FIVE bundles (from .emacs.d) :PROPERTIES: :LAST_REVIEWED: 2026-07-24 :END: +CLOSED: [2026-07-24 Fri] +Applied 2026-07-24 across all five bundles (11 sites: the secret-scan input in each, plus each bundle's staged-file list, typescript having two). Verified on all five: refuses when the diff can't be read, still blocks a real staged secret, still passes a clean commit. Adopted .emacs.d's elisp bats suite (8 tests) and extended the repo-level cross-bundle suite with two fail-closed assertions. + +Separate defect found while wiring that up: the cross-bundle suite's VARIANTS list read "elisp bash go" while python and typescript also shipped hooks, so every "in every variant" assertion had silently skipped two bundles since I added them. VARIANTS is now discovered from the tree rather than enumerated — the same failure class the tests exist to catch. + What arrived: .emacs.d found that =languages/elisp/githooks/pre-commit= builds its scan input as =added_lines="$(git diff --cached -U0 ... | grep '^+' | grep -v '^+++' || true)"=. With no pipefail and =|| true= swallowing everything, any git failure yields an empty string, so the scan searches nothing, finds nothing, reports clean, and the commit proceeds with the secret in it. The staged-file list feeding the paren check has the identical hole. *Verified independently, and it is worse than reported.* I reproduced the fail-open with a stub git that fails only the staged-diff call: exit 0 with an AWS-shaped key staged. Then I checked the other bundles, which the sender did not: *bash, go, python, and typescript all carry the same pattern and all fail open the same way.* Confirmed live on all five. Two of those (python, typescript) are hooks I wrote on 2026-07-24 by copying the bash one, so I propagated the defect while closing a different gap. @@ -70,12 +75,14 @@ The sender's fix is correct and I verified all three axes on it: refuses to proc Decision needed: the fix as sent covers elisp only. It should be applied to all five bundles, which is my scope expansion rather than the sender's proposal — hence a VERIFY rather than a silent apply. Also unresolved: where the two attached bats suites live, since the hooks are rulesets-owned and the tests currently sit in the consuming project. Prepared: [[file:working/hook-fail-open/pre-commit.diff]], plus =test-pre-commit-hook.bats= (8 tests) in the same dir. -Say "approve the hook fail-open fix" and it gets applied across all five bundles. -** VERIFY [#B] Parked: remove the validate-el auto-test cap (from .emacs.d, Craig's call) +** DONE [#B] Applied: remove the validate-el auto-test cap (from .emacs.d, Craig's call) :PROPERTIES: :LAST_REVIEWED: 2026-07-24 :END: +CLOSED: [2026-07-24 Fri] +Applied 2026-07-24. Cap and the unreachable notice helper removed; the gate is now count -ge 1. Adopted the rewritten bats suite (6 tests), with its hook path corrected from the installed .claude/hooks/ layout to the bundle's claude/hooks/ — the re-homing mismatch the sender flagged. + What arrived: a superseding handoff. The first proposed a loud notice when the test count exceeds =MAX_AUTO_TEST_FILES=20= (above the cap the block was skipped, nothing printed, exit 0 — indistinguishable from a pass). Craig chose in the .emacs.d session to remove the cap instead. The reasoning is the valuable part. Measured, a whole family runs in under two seconds (calendar-sync 63 files / 633 tests / 0.9s), so the cap bought nothing. Worse, it was *concealing* a real cross-test pollution bug: calendar-sync exits 1 when its 63 files run in one process, because one test marks a calendar as syncing and never resets it, and a sibling file's test then hits the stale guard. Invisible from both directions — =make test= runs each file in its own Emacs, and the hook skipped the family for being over the cap. @@ -85,7 +92,6 @@ Verified the superseding file drops the cap entirely (zero references, gate is n Since Craig already made this call, the remaining decision is only adoption scope: removing the cap means other consuming projects run every stem-matched test file per edit, and one may go red on first use by surfacing pollution that per-file runs hid. The sender flags that as intended. Prepared: [[file:working/hook-fail-open/validate-el.diff]], plus =test-validate-el-hook.bats= (6 tests, two of which stage a deliberately failing test so a quiet pass proves the run happened). -Say "approve the cap removal" and it gets applied. ** TODO [#B] cj-remove-block still can delete the WRONG cj block :bug: :PROPERTIES: |