fix(hooks): add local commitlint validation hook#868
Conversation
Validates commit messages before execution, catching violations that would otherwise only fail in CI. Checks type-enum, type-case, subject-empty, subject-full-stop, header-max-length, and body/footer-max-line-length rules from config-conventional.
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Todo List:
Root Cause AnalysisProblem Being Solved: PR #857 had 90+ commits with body lines exceeding commitlint's 100-char limit, causing CI failures that could only be fixed with force-push. This hook shifts the validation left to catch violations at commit time locally. Why This Approach: Local pre-commit validation is the right solution - catches issues immediately, prevents accumulation of bad commits, and avoids CI failure cycles. The approach is justified and addresses the root cause rather than symptoms. Risk Assessment: Low risk - this is purely additive validation that improves developer experience without changing core functionality. Backlog Compliance
Critical ConcernsNone identified. This is a well-implemented defensive addition to the development workflow. Code Quality AssessmentStrengths:
Minor Issues:
Security/Performance:
Final Recommendation
Recommendation: Approve and merge. Consider adding automated tests for the hook logic in a future iteration if the project adopts hook testing infrastructure. |
Greptile SummaryAdds a Confidence Score: 5/5Safe to merge — both prior P1 issues are resolved and the one remaining finding is a minor edge case with negligible practical impact. All previous review concerns are addressed. The only remaining finding is a P2 style issue: the double-quote extractor does not skip escaped backslash-quote sequences, so an unusual message with embedded escaped quotes would be silently truncated. In practice, CLAUDE.md instructs agents to use heredoc for multiline commits (already skipped by the hook) or plain single-line -m messages, making this path essentially unreachable. No blocking issues remain. No files require special attention; commitlint-local.sh has one minor regex edge case in the double-quote extractor at line 48. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Claude Agent
participant H as commitlint-local.sh
participant V as commitlint-check.js
participant G as git commit
C->>H: PreToolUse (tool_input.command)
H->>H: Extract command from JSON
alt not git commit
H-->>C: exit 0 (allow)
else heredoc-style message
H-->>C: exit 0 (allow, cannot pre-validate)
else --amend without -m
H-->>C: exit 0 (allow, reuses existing msg)
else has -m flag
H->>H: Extract MSG via regex
alt MSG is empty
H-->>C: exit 0 (allow)
else MSG extracted
H->>V: node commitlint-check.js "$MSG"
alt violations found
V-->>H: error lines (exit 1)
H-->>C: JSON deny (permissionDecision: deny)
else clean
V-->>H: exit 0
H->>G: allow
G-->>C: commit created
end
end
end
Reviews (2): Last reviewed commit: "fix(hooks): skip heredoc commits and fix..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Extract the commit message from -m flag using node for robust parsing | ||
| MSG=$(echo "$COMMAND" | node -e " | ||
| let d=''; | ||
| process.stdin.on('data',c=>d+=c); | ||
| process.stdin.on('end',()=>{ | ||
| const cmd = d; | ||
| let msg = ''; | ||
| // Match -m \"...\" or -m '...' | ||
| const dq = cmd.match(/-m\s+\"([\\s\\S]*?)\"\s*(?:\)|$|&&|;|\s+-)/); | ||
| if (dq) { msg = dq[1]; } | ||
| else { | ||
| const sq = cmd.match(/-m\s+'([^']*)'/); | ||
| if (sq) { msg = sq[1]; } | ||
| } | ||
| // Unescape \\n to real newlines | ||
| msg = msg.replace(/\\\\n/g, '\\n'); | ||
| process.stdout.write(msg); | ||
| }); | ||
| " 2>/dev/null) || true |
There was a problem hiding this comment.
When Claude agents use the system-instructed heredoc format for multiline commits, the extraction regex captures the shell code — not the actual message — causing commitlint-check.js to see $(cat <<'EOF' as the header, which fails the header pattern and blocks a valid commit with a misleading error.
For a command like git commit -m "$(cat <<'EOF'\nfix: add feature\n\nBody.\nEOF\n)", dq[1] is $(cat <<'EOF'\nfix: add feature..., not the commit text. The system-level CLAUDE.md instructs Claude to "ALWAYS pass the commit message via a HEREDOC", so this will fire routinely.
Add a guard before the extraction block:
| # Extract the commit message from -m flag using node for robust parsing | |
| MSG=$(echo "$COMMAND" | node -e " | |
| let d=''; | |
| process.stdin.on('data',c=>d+=c); | |
| process.stdin.on('end',()=>{ | |
| const cmd = d; | |
| let msg = ''; | |
| // Match -m \"...\" or -m '...' | |
| const dq = cmd.match(/-m\s+\"([\\s\\S]*?)\"\s*(?:\)|$|&&|;|\s+-)/); | |
| if (dq) { msg = dq[1]; } | |
| else { | |
| const sq = cmd.match(/-m\s+'([^']*)'/); | |
| if (sq) { msg = sq[1]; } | |
| } | |
| // Unescape \\n to real newlines | |
| msg = msg.replace(/\\\\n/g, '\\n'); | |
| process.stdout.write(msg); | |
| }); | |
| " 2>/dev/null) || true | |
| # Skip heredoc-style messages (shell code only; can't validate pre-execution) | |
| if echo "$COMMAND" | grep -qE '\$\(cat <<'; then | |
| exit 0 | |
| fi | |
| # Extract the commit message from -m flag using node for robust parsing | |
| MSG=$(echo "$COMMAND" | node -e " | |
| let d=''; | |
| process.stdin.on('data',c=>d+=c); | |
| process.stdin.on('end',()=>{ | |
| const cmd = d; | |
| let msg = ''; | |
| // Match -m \"...\" or -m '...' | |
| const dq = cmd.match(/-m\\s+\"([\\s\\S]*?)\"\\s*(?:\\)|$|&&|;|\\s+-)/); | |
| if (dq) { msg = dq[1]; } | |
| else { | |
| const sq = cmd.match(/-m\\s+'([^']*)'/); | |
| if (sq) { msg = sq[1]; } | |
| } | |
| // Unescape \\n to real newlines | |
| msg = msg.replace(/\\\\n/g, '\\n'); | |
| process.stdout.write(msg); | |
| }); | |
| " 2>/dev/null) || true |
There was a problem hiding this comment.
Fixed in 7a5a677. Added a heredoc detection guard ($(cat <<) before the extraction block that exits 0 immediately, so heredoc-style commits pass through without false-positive rejection.
.claude/hooks/commitlint-local.sh
Outdated
| const dq = cmd.match(/-m\s+\"([\\s\\S]*?)\"\s*(?:\)|$|&&|;|\s+-)/); | ||
| if (dq) { msg = dq[1]; } |
There was a problem hiding this comment.
The double-quote terminator group (?:\)|$|&&|;|\s+-) does not match a bare filename argument that follows the message. A command like git commit -m "fix: msg" src/foo.ts leaves MSG empty (no terminator match after the closing "), silently skipping validation. Low-impact given the project convention places files before -m, but the coverage gap is invisible to the author.
There was a problem hiding this comment.
Fixed in 7a5a677. Changed the double-quote terminator regex from (?:\)|$|&&|;|\s+-) to (?:\s|$) which correctly matches a bare filename argument after the closing quote (e.g. git commit -m "fix: msg" src/foo.ts).
Add a guard to skip heredoc-style commit messages ($(cat <<'EOF'...)) that cannot be validated pre-execution. Also fix the double-quote terminator regex to match trailing whitespace or end-of-string, which correctly handles `git commit -m "msg" src/foo.ts` patterns. Addresses Greptile P1 and P2 review feedback on #868.
1 participant
Summary
@commitlint/config-conventionalrules: type-enum, type-case, subject-empty, subject-full-stop, header-max-length, body/footer-max-line-lengthcommitlint-local.sh(message extraction) andcommitlint-check.js(validation) for testabilityMotivation
PR #857 had 90+ accumulated commits with body lines exceeding the 100-char limit, causing commitlint CI failures that could only be fixed with a force-push. This hook prevents that class of issue entirely.
Test plan
update:) is blocked with clear errorFix:) is blockedgit push) are ignored