ci + fix(git-hooks): sync workflow + reconcile legacy .git-hooks/ with templates#47
Conversation
Phase 1 of the canonical-hooks hardening roadmap. Two changes:
1. Add .github/workflows/hooks-sync.yml — three jobs:
- lint: shellcheck + bash-parse templates and installed .git-hooks/
- drift-check-local: fail if crates/resq-cli/templates/git-hooks/ and
.git-hooks/ diverge byte-for-byte
- drift-check-dev: fail if templates drift from resq-software/dev@main
scripts/git-hooks/
2. Replace the legacy .git-hooks/ content with the canonical templates.
The old hooks predated the embedded-templates work and had already
drifted (different copyright headers, different pre-push logic).
To preserve the old "build resq-cli from workspace target/" bootstrap
that was useful for contributors working on this repo specifically,
add .git-hooks/local-pre-commit that runs only when PATH + ~/.cargo/bin
lookup both fail and falls back to target/{release,debug}/resq or
`cargo build -p resq-cli`.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 36 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors and standardizes the Git hooks across the repository. Key changes include the introduction of local hook shims for repository-specific logic, improved detection of lock file changes in post-checkout and post-merge hooks, and a more robust pre-push guard for main and master branches. Feedback highlights potential script failures in the pre-push hook when handling empty reference inputs via heredocs and suggests aligning ticket ID regex patterns across different hooks for consistency.
| while read -r _local_ref local_sha _remote_ref remote_sha; do | ||
| if [ "$remote_sha" != "0000000000000000000000000000000000000000" ] && \ | ||
| [ "$local_sha" != "0000000000000000000000000000000000000000" ]; then | ||
| if ! git merge-base --is-ancestor "$remote_sha" "$local_sha" 2>/dev/null; then | ||
| echo "❌ Force push to $BRANCH is not allowed." | ||
| echo " Override with: git push --no-verify" | ||
| exit 1 | ||
| fi | ||
| fi | ||
| done <<EOF | ||
| $PUSH_REFS | ||
| EOF |
There was a problem hiding this comment.
If PUSH_REFS is empty (which can happen if the hook is triggered without ref updates or in certain CI environments), the while read loop will still execute once with empty variables because the heredoc <<EOF provides a trailing newline. This causes git merge-base to be called with empty arguments, which will fail and exit the script due to set -e. Guarding the loop with a check for non-empty input prevents this crash.
if [ -n "$PUSH_REFS" ]; then
while read -r _local_ref local_sha _remote_ref remote_sha; do
if [ "$remote_sha" != "0000000000000000000000000000000000000000" ] && \
[ "$local_sha" != "0000000000000000000000000000000000000000" ]; then
if ! git merge-base --is-ancestor "$remote_sha" "$local_sha" 2>/dev/null; then
echo "❌ Force push to $BRANCH is not allowed."
echo " Override with: git push --no-verify"
exit 1
fi
fi
done <<< "$PUSH_REFS"
fi
| # Validate only the first line (subject). | ||
| FIRST_LINE=$(head -1 "$INPUT_FILE") | ||
| SUBJECT=$(echo "$FIRST_LINE" | sed 's/^\[[A-Z][A-Z]*-[0-9]*\] //') | ||
| SUBJECT=$(sed 's/^\[[A-Z][A-Z]*-[0-9]*\][[:space:]]*//' <<<"$FIRST_LINE") |
There was a problem hiding this comment.
The regex used here to strip the ticket ID ([A-Z][A-Z]*) is slightly inconsistent with the one used in prepare-commit-msg ([A-Z]{2,}). While [A-Z]+ works, it might strip single-letter prefixes that the preparation script wouldn't have added. For consistency across hooks, consider matching the 2-character minimum.
SUBJECT=$(sed 's/^\[[A-Z][A-Z][A-Z]*-[0-9]*\][[:space:]]*//' <<<"$FIRST_LINE")
| "$LOCAL_HOOK" "$@" <<EOF | ||
| $PUSH_REFS | ||
| EOF |
There was a problem hiding this comment.
Similar to the force-push guard, re-supplying PUSH_REFS via a heredoc adds a newline even if the variable is empty. If the local hook also uses a while read loop, it might suffer from the same empty-iteration bug. Using printf to pipe the exact content is safer and more robust.
printf '%s' "$PUSH_REFS" | "$LOCAL_HOOK" "$@"
…fixes) Pulls the latest canonical fixes from resq-software/dev@fdb2a77: - pre-push empty-heredoc guard (gemini #47/high) - commit-msg WIP-before-format reorder - commit-msg ticket-prefix regex {2,} for prepare-commit-msg consistency (gemini #47/medium) - commit-msg error hint mentions the `!` breaking-change marker Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
Phase 1 of the canonical-hooks hardening roadmap. Two changes bundled because they share a workflow:
1.
.github/workflows/hooks-sync.yml— three jobsshellcheck -S warning+bash -non every template and installed hook.git-hooks/byte-differs fromcrates/resq-cli/templates/git-hooks/(this repo eats its own dogfood; both must be identical)crates/resq-cli/templates/git-hooks/drifts fromresq-software/dev@main:scripts/git-hooks/A symmetric workflow is opening in resq-software/dev#2.
2. Reconcile
.git-hooks/with the canonical templatesThe legacy hooks predated the
include_str!-embedded templates work and had already drifted (different copyright headers, different pre-push logic — seegit diffin this PR). Copying the 6 templates over brings everything into one source of truth.Behavior change note: the old
.git-hooks/pre-commithad a workspace-local fallback (buildresq-clifromtarget/when PATH lackedresq). The canonical hook only looks on PATH and~/.cargo/bin. To keep contributor ergonomics here identical, I added.git-hooks/local-pre-committhat preserves that fallback and runs only when the canonical lookup fails.Test plan
shellcheck -S warning+bash -ngreen locallydiff -qconfirms.git-hooks/<hook>==crates/resq-cli/templates/git-hooks/<hook>for all 6 hooksgit commitin this repo (with resq not on PATH) triggers the workspace-local fallback vialocal-pre-commitContext
Phase 1 of the hardening roadmap (
~/.claude/plans/binary-hugging-kite.md).🤖 Generated with Claude Code