Compare the version the hook is pinned to, instead of printing it - #388
Merged
Conversation
`hooks install` writes commitlore.bin/commitlore.root into one repository's config. An upgrade installs the new release somewhere else and never visits the repositories that pinned the old one, so a repo can sit two releases behind on the path that actually validates its commits while `commitlore --version` reports the newest one. Reproduced against a v0.5.0 install directory alongside 0.6.0: both doctor and hooks status called it healthy, and doctor printed the stale v0.5.0 path inside its own `ok` line. It had the value in hand and did not act on it. That is not cosmetic, because the hook is the enforcement point. Everything 0.5.1 and 0.6.0 shipped that touches validate, prepare-commit-msg or post-commit is inert in such a repository -- this release's Ruled-out: narrowing, the shallow-clone gate, duplicate-id dedup, the fail-open capture hooks. It is also latent: deleting the old install directory turns every commit there into a hook failure. The comparison goes in core/hook-target.ts, the mirror both readers already share, so doctor and hooks status cannot disagree about it and the remedy they already name -- `commitlore hooks install` -- is the one that fixes it. A pin whose version cannot be established is reported too, not passed over. The pinned build still decides what runs, and "could not find out" is a different answer from "it matches"; a false green there costs a repository every fix shipped since the pin was written, which is the trade this project has made the same way everywhere else. Record-Id: r-pinskew Limit: the comparison reads the package.json above the recorded path rather than running it with --version, so a pin whose manifest and bundle disagree is reported by its manifest Ruled-out: Re-pointing the pin from doctor --fix | doctor's --fix boundary is reversible local config only, and the module comment states the commit-msg hook is reported and never written -- rewriting commitlore.bin would also change which binary validates commits, silently, from a diagnostic Ruled-out: Having the upgrade re-point every repository | install.sh cannot enumerate the repositories that ran hooks install, which is why the issue itself proposes reporting as the self-correcting half Ruled-out: Spawning the pinned bundle for --version | the recorded path is by design a value a .git/config edit can change, and hooks status spawns nothing today -- executing it on every status and every doctor is a larger door than the question is worth Ruled-out: Treating a manifest with no version as a match | that is the false green the check exists to remove Certainty: firm Blast: module Undo: easy Verified: reproduced with a v0.5.0 install pinned beside 0.6.0 -- doctor and hooks status both reported ok before, both name the skew and `commitlore hooks install` after, and running that command clears it; 6 tests added across doctor, hooks and hook-target failed first and pass now; doctor, hooks, init, cli, hooks-entry-resolution and hook-target pass at 180; npm run typecheck and npm run build clean Unverified: whether any repository other than the reporter's carries a stale pin, which nothing here can enumerate
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (85)
Ruled out (195)
Warnings (49)
Truncated: 10 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
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
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.
Closes #382
What was wrong
hooks installwritescommitlore.bin/commitlore.rootinto a singlerepository's git config. An upgrade installs the new release into a new
directory and never visits the repositories that pinned the old one, so a repo
keeps validating every commit with the build it was pinned to while
commitlore --versionreports the newest one.Reproduced with a v0.5.0 install directory sitting beside 0.6.0, pinned by a
repository whose hook was installed from it:
Both diagnostics called it healthy, and
doctorprinted the stale path insideits own
okline — it had the value and did not act on it.Why it is not cosmetic
The hook is the enforcement point. Everything 0.5.1 and 0.6.0 shipped that
touches
validate,prepare-commit-msgorpost-commitis inert in such arepository: the
Ruled-out:narrowing, the shallow-clone gate,duplicate-iddedup, the fail-open capture hooks. It is also latent breakage — deleting the
old install directory turns every commit there into a hook failure.
The change
The comparison goes in
core/hook-target.ts, the mirrordoctorandhooks statusalready share, so the two cannot disagree and the remedy theyalready name —
commitlore hooks install— is the one that fixes it.After:
Running the named remedy re-pins the config and both return to
ok, verifiedagainst the rebuilt bundle.
A pin whose version cannot be established is reported too, not passed over:
commitlore.bin does not declare a version, so it cannot be compared with this CLI (<v>). The pinned build still decides what runs, and "could not find out"is a different answer from "it matches".
Reporting, not repairing
The issue offered two expectations. This takes the second.
doctor --fix— ruled out.--fix's boundary isreversible local config only (today it writes
remote.<name>.fetchandnothing else), and the module's own comment states the commit-msg hook is
reported and never written. Rewriting
commitlore.binwould change whichbinary validates commits, silently, from a command people run to ask a
question.
install.shcannot enumerate the repositories that ran
hooks install, which is why theissue itself proposes reporting as the self-correcting half.
--version— ruled out. The recorded pathis by design a value a
.git/configedit can change, andhooks statusspawns nothing today; executing it on every
statusand everydoctoris alarger door than the question is worth. The version is read from the
package.jsonabove the recorded path — the same file--versionitselfreads.
One fixture corrected, no assertion weakened
test/hooks.test.ts's harness build wrote{"type":"module"}as its manifest,with no
version. Under the new check that harness reads as an undeterminablepin, which made
hooks status > reports the recorded bin and node for a healthy installationfail. The assertion is untouched; the manifest now declares theversion, which is the shape every real installation has (clone, tarball and
install.shall carry one).Verification
doctor,hooks statusand thehook-targetmirror,covering skew, match, and undeterminable. Confirmed failing before the fix.
npx vitest run test/doctor.test.ts test/hooks.test.ts test/init.test.ts test/cli.test.ts test/hooks-entry-resolution.test.ts test/hook-target.test.ts— 180 passed.
npm run typecheck,npm run buildclean,dist/committed with no drift.commitlore validate -c HEAD— shape ok, references ok.