Probe the hook that is configured, not one this check invents - #129
Merged
Conversation
doctor reported a working PreToolUse hook as broken on every binary-only install. It read .claude/settings.json to decide the hook was installed and then ran a command of its own construction, bash against scripts/commitlore-run.sh. A single-binary install has no scripts directory, so bash exited 127 and the check blamed a hook it never ran. The check now parses the command settings.json actually contains and runs that. A command it does not recognise is reported as not checked, with the reason, rather than replaced by a reconstruction — a check that verifies something other than what is configured cannot be trusted even when it passes, and that is the deeper fault here than the false failure. The suggested fix used to be to reinstall the hook, which reinstalled the same working hook and changed nothing, so the state was unrecoverable through the documented path. It now names the executable the configured hook runs, which is the thing that was actually missing. Reported from a real v0.2.1 aarch64-apple-darwin install with no node on PATH. That is a supported installation and it now passes. Verified by putting the synthesized path back: one test fails, and passes again on restoration. Ruled-out: changing what init writes into settings.json | the installed hook was correct in every reported case; the check was reading a different world from the one it was auditing Limit: the probe runs only a command it recognises, so a user who hand-edits the hook into an equivalent but different form gets not-checked rather than a verdict Blast: local Undo: easy Certainty: firm Record-Id: r-doctorprobe
CommitLore — record lintTrailers: clean — 2 commits in Active constraints for the paths this PR touchesLimits (27)
Ruled out (71)
Warnings (39)
Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
The probe spawned the configured command with the inherited PATH, so it found /Users/isaac/.local/bin/commitlore on my machine and nothing in CI. Three assertions expecting ok got fail there. The tests were reading my installation, not the code. Each test now builds the world it asserts on: a resolvable executable for the passing cases and a deliberately unresolvable one for the failure case. A probe that cannot resolve the executable is now a distinct outcome from one that ran and failed, because the remedies differ — install or fix PATH versus repair the hook — and collapsing them was the same defect this branch exists to close, arriving one level down. Verified with the directory holding my commitlore stripped from PATH: 50 tests pass where three previously failed only in CI. Ruled-out: prepending the build output to PATH inside the check | it would make CI green by giving the probe an executable no user has, which is the reverse of testing what is configured Limit: the probe still only runs a command it recognises, so a hand-edited but equivalent hook reports not-checked rather than a verdict Blast: local Undo: easy Certainty: firm Record-Id: r-probepath
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 #128. Reported from a real v0.2.1
aarch64-apple-darwininstall with nonodeon PATH.doctorreported a working PreToolUse hook as broken on every binary-only install. It read.claude/settings.jsonto decide the hook was installed, then ran a command of its own construction — bash againstscripts/commitlore-run.sh. A single-binary install has noscripts/, so bash exited 127 and the check blamed a hook it never ran.settings.jsonactually containsnot checkedwith the reason, rather than replaced by a reconstructionA check that verifies something other than what is configured cannot be trusted even when it passes. That is the deeper fault here than the false failure.
Verified by putting the synthesized path back: 1 test fails, and passes on restoration.
1398 tests pass (1396 on dev). Both typechecks clean.