perf: batch pre-commit hook inspection - #187
Conversation
johnson-ricon
left a comment
There was a problem hiding this comment.
Change request
hooks/obfuscation.template:26-31 and hooks/encryption.template:20-31 consume git diff / git check-attr through process substitutions. The parent shell sees only the while loop's status, not the producer's exit status, so set -e does not protect these boundaries. If staged-path enumeration or staged-attribute inspection fails, the corresponding array/plan can appear empty and the pre-commit hook exits successfully. That violates the fail-closed contract precisely when Git cannot establish what is staged or encrypted.
I reproduced all three paths on exact head 05c59fe2c6cde724ce86fc399746d4f0c5001ccd with command overlays that make git diff or git check-attr return nonzero; the unmodified hooks returned success. Fix-it #188 replaces the hidden process substitutions with explicitly checked temporary snapshots and adds regressions for obfuscation staged-path failure plus encryption staged-path and attribute failure.
Review coverage
I independently reviewed the complete stacked diff against exact base aade93244282686aa8f663485f557fa8a619253d. The extracted classifier is the production planner's shared classification boundary; it retains duplicate-ID, orphan hash-name, and destination-collision checks. The post-obfuscation replan correctly blocks index-only readable files that cannot be repaired from a missing worktree path. Git Crypt's zero-status not encrypted output selects per-path fallback, while nonzero batch and per-path backend outcomes otherwise remain closed. Installed hooks source the installing Notes package, not ambient notes, and the new shell uses no Bash 4-only constructs; hosted Ubuntu and macOS checks are green.
On the exact reviewed head I ran the focused hook/encryption/integration suites (72 passed), full suite (420 BATS and 9 Python passed), all eight configured lints, doctor, README generation, Bash syntax, and diff checks. The optional clone-local pre-commit hook is not installed. On fix-it #188, the focused suites pass 53 tests, the full suite passes 423 BATS and 9 Python tests, and all eight lints plus README, syntax, and diff checks pass.
Summary
git-crypt statuscall, falling back to exact per-path checks only when plaintext is reportedThis PR is stacked on #185 so the hook work could continue while that review completed. Its diff begins after exact updated #185 head
aade93244282686aa8f663485f557fa8a619253d.Validation