chore(hygiene): give the commit gate teeth and guard against the blobs returning - #34
Merged
Merged
Conversation
…s returning
Two defects with the same shape: a control that is documented as enforcing
something it cannot enforce.
## The commit-msg hook could never have run
`docs/compliance/standards-alignment.md:49` claimed commit conventions were
"Enforced ✅" by `.githooks/commit-msg`. That hook was committed **mode 100644**.
Git refuses to execute a non-executable hook, so it had never run for anyone,
ever -- and it failed to *nothing*, with no error, rather than to a visible
exit 126.
It was also wired nowhere. `core.hooksPath` is local config and cannot be
committed, and the only place it appeared was a line in CONTRIBUTING.md, so it
was unset in every clone that had not read that line. Mine included.
Fixed rather than documented around:
- `git update-index --chmod=+x` on the hook, so git will run it;
- a `just hooks` recipe, made a dependency of `just bootstrap`, so enablement
follows from bootstrapping instead of from remembering;
- CONTRIBUTING.md points at `just hooks` rather than the raw git incantation.
The doc row is now split honestly: CI is *the gate* (binding on this repo,
advisory upstream), the hook is *local pre-flight* and marked ⚠ opt-in per
clone, because that is what a per-clone setting can be and no more.
## Nothing stopped the 269 MiB coming back
`.gitattributes` had no fastq or LFS rules. After the history rewrite reclaims
~96.6% of the pack, anyone could have re-added the same data the next day.
`scripts/check-blob-hygiene.sh` is the guard, with **one implementation and two
callers** -- `.githooks/pre-commit` and a new CI `Blob hygiene check` step. That
is deliberate: a hook and a CI check that re-implement one rule drift apart, and
the drift is invisible because both keep reporting success.
The primary rule is a **4 MiB size ceiling, not a path list**. A path list can
only forbid paths somebody already thought of, and that is exactly how the
Multiplex pool survived a history rewrite -- it was reachable under a second
path (`inputs/fastq/`) that the census had never enumerated, because
`git rev-list --objects` pairs each object with only one of its paths. Path
rules are kept, but as better error messages rather than as the gate.
The six `data/MiSeq_SOP/run_[AB]/*.fastq.gz` fixtures (0.6-2.6 MiB, whitelisted
at .gitignore:294-304) are allowlisted by glob. The ceiling sits above the
largest of them (2,723,348 B) with headroom.
## Verified by mutant, not by a green run
A guard that passes proves nothing until it refuses something. Five
reintroduction attempts, each refused:
uncompressed .fastq · a 5 MiB blob · a node_modules/ path ·
an inputs/ second-path copy · a logs_*.zip CI artefact
and two negative controls admitted: an allowlisted 2.6 MiB fixture, and an
ordinary source file. The positive control over the live tree examines 335
files, sees all 6 fixtures, and passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
|
Warning Review limit reachedNext included review available in 12 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two defects with the same shape: a control documented as enforcing something
it could not enforce.
1. The commit-msg hook could never have run
docs/compliance/standards-alignment.md:49claimed commit conventions were"Enforced ✅" by
.githooks/commit-msg. That hook was committed mode100644. Git refuses to execute a non-executable hook, so it had never runfor anyone, ever — and it failed to nothing, silently, rather than to a
visible
exit 126.It was also wired nowhere.
core.hooksPathis local config and cannot becommitted; the only place it appeared was one line in
CONTRIBUTING.md, so itwas unset in every clone that had not read that line.
Fixed rather than documented around:
git update-index --chmod=+xon the hook — git will now run it;just hooksrecipe, made a dependency ofjust bootstrap, soenablement follows from bootstrapping instead of from remembering;
CONTRIBUTING.mdpoints atjust hooksrather than the raw git incantation.The doc row is now split honestly — CI is the gate, the hook is local
pre-flight, marked ⚠ opt-in per clone, because that is what a per-clone
setting can be and no more.
2. Nothing stopped the 269 MiB coming back
.gitattributeshad no fastq or LFS rules. Once the history rewrite reclaims~96.6% of the pack, anyone could re-add the same data the next day.
scripts/check-blob-hygiene.shis the guard, with one implementation and twocallers —
.githooks/pre-commitand a new CIBlob hygiene checkstep. Thatis deliberate: a hook and a CI check that re-implement one rule drift apart, and
the drift is invisible because both keep reporting success.
The primary rule is a 4 MiB size ceiling, not a path list. A path list can
only forbid paths somebody already thought of — which is exactly how the
Multiplex pool survived a history rewrite: it was reachable under a second path
(
inputs/fastq/) the census never enumerated, becausegit rev-list --objectspairs each object with only one of its paths. Path rules are kept, but as
better error messages rather than as the gate.
The six
data/MiSeq_SOP/run_[AB]/*.fastq.gzfixtures (0.6–2.6 MiB, whitelistedat
.gitignore:294-304) are allowlisted by glob. The ceiling sits above thelargest (2,723,348 B) with headroom.
Verified by mutant, not by a green run
A guard that passes proves nothing until it refuses something.
.fastqnode_modules/pathinputs/second-path copylogs_*.zipCI artefactPositive control over the live tree: 335 files examined, all 6 fixtures seen,
passes.
The
commit-msghook was mutant-tested too. The subject it rejects in the testis
CI and PR fixes.— verbatim the historical commit84c7efb, which landedon
mainin violation of this very rule. The repaired hook catches it.Both hooks ran for real on this PR's own commit (
blob hygiene: okappears inthe commit output).
check-spdx.sh,check-format.shandcheck-lint.shallpass locally.
🤖 Generated with Claude Code
https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm