refactor(hooks): retire dev/scripts/git-hooks — crates is the single source#6
Merged
WomB0ComB0 merged 1 commit intomainfrom Apr 14, 2026
Merged
Conversation
…urce
Phase 4 consolidation. Canonical hook content is now owned exclusively by
resq-software/crates (crates/resq-cli/templates/git-hooks/).
scripts/install-hooks.sh:
- When `resq` is on PATH, delegate to `resq dev install-hooks` — the
binary embeds the templates via include_str! so there's no network
round-trip.
- Otherwise fall back to curl against
raw.githubusercontent.com/resq-software/crates/master/.../templates/git-hooks/
(was: resq-software/dev/main/scripts/git-hooks/ — retired).
- RESQ_CRATES_REF replaces RESQ_DEV_REF for pinning the fallback.
scripts/git-hooks/ — removed entirely (6 hook files + README.md).
tests/hooks/helpers.bash:
- Fetch templates from the crates raw URL (once per bats session,
cached under /tmp).
- RESQ_HOOK_SRC_DIR lets you point at a local template dir for
pre-push testing. 33/33 tests still pass.
.github/workflows/hooks-sync.yml — rewritten:
- No more diff-check (nothing to diff against on this side).
- `lint` job shellchecks install-hooks.sh + install-resq.sh.
- New `smoke` job: spins up a throwaway git repo, runs the installer
with a scrubbed PATH (forcing the raw-fetch fallback), and asserts
the 6 hooks are present, executable, and parse.
README.md + AGENTS.md updated to reflect the single-source story and drop
the "three copies" note.
Verified:
- shellcheck -S warning + bash -n clean
- resq-preferred path: `resq dev install-hooks` scaffolds 6 hooks
- raw-fetch fallback: curl from crates master installs 6 hooks
- bats tests/hooks/: 33/33 pass (uses cached templates from crates raw)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis change migrates canonical git hook templates from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 the git hook management system by removing local hook shims and delegating their installation to the resq CLI or a remote fetch from the resq-software/crates repository. Documentation and tests have been updated to reflect this change. Feedback includes a critical issue where the git configuration for hooksPath might be skipped during installation if the preferred path is taken, as well as several robustness concerns regarding the hook caching mechanism used in tests, such as weak cache validation and potential interference with local source overrides.
Comment on lines
+67
to
68
| git -C "$TARGET_ROOT" config core.hooksPath .git-hooks | ||
| fi |
There was a problem hiding this comment.
The git config core.hooksPath .git-hooks command was moved inside the else block. This means if the preferred path (using resq dev install-hooks) is taken, the repository's git configuration is not updated to point to the .git-hooks directory. Unless the resq binary is guaranteed to perform this configuration itself, the hooks will not be active after installation. It is safer to keep this configuration step outside the conditional block to ensure the repository is correctly configured regardless of how the hook files were placed.
Suggested change
| git -C "$TARGET_ROOT" config core.hooksPath .git-hooks | |
| fi | |
| fi | |
| git -C "$TARGET_ROOT" config core.hooksPath .git-hooks |
Comment on lines
+15
to
+21
| _ensure_hook_cache() { | ||
| [ -d "$HOOK_SRC_CACHE" ] && [ -e "$HOOK_SRC_CACHE/pre-commit" ] && return 0 | ||
| mkdir -p "$HOOK_SRC_CACHE" | ||
| for h in pre-commit commit-msg prepare-commit-msg pre-push post-checkout post-merge; do | ||
| curl -fsSL "$HOOK_RAW_BASE/$h" -o "$HOOK_SRC_CACHE/$h" | ||
| done | ||
| } |
There was a problem hiding this comment.
The _ensure_hook_cache function has a few robustness issues:
- Weak Cache Validation: The check
[ -e "$HOOK_SRC_CACHE/pre-commit" ]only verifies the first hook. If the initial download is interrupted, subsequent test runs will skip the download but fail during thecpstep ininit_repo_with_hooksbecause other hooks are missing. - Local Source Interference: If
RESQ_HOOK_SRC_DIRis set to a local directory that happens to be missingpre-commit, the function will attempt tocurlremote templates into that local directory, which is unexpected behavior for a source override. - Shared Cache Path: Using a fixed path like
/tmp/resq-canonical-hookscan lead to permission issues or race conditions on multi-user systems or shared CI runners. Consider using a user-specific path or a unique temporary directory.
WomB0ComB0
added a commit
that referenced
this pull request
Apr 14, 2026
Two stragglers missed in #6: 1. scripts/install-hooks.ps1 still raw-fetched from resq-software/dev/$Ref/scripts/git-hooks — the retired path. Windows users following the curl-pipe flow would get 404s. Rewritten to mirror install-hooks.sh's two-path strategy: prefer `resq dev install-hooks` when on PATH, fall back to the crates raw URL. RESQ_CRATES_REF replaces RESQ_DEV_REF. Adds the scaffold prompt for the local-pre-push, matching the bash mirror. 2. .github/workflows/hooks-tests.yml had `paths: scripts/git-hooks/**` in its trigger filter — that directory no longer exists, so the workflow wouldn't fire on test changes alone. Trigger now fires on tests/hooks/**, scripts/install-hooks.sh, and the workflow file. Added a weekly cron so canonical-template drift on the crates side gets caught even without a dev-side change. Verified end-to-end: install-resq.sh → fresh repo + Cargo.toml → install-hooks.sh (resq-preferred path) → `git commit` triggers `resq pre-commit` which runs through Copyright/Large Files/Debug/ Secrets/Audit/Format. Doctor reports healthy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WomB0ComB0
added a commit
that referenced
this pull request
Apr 14, 2026
Two stragglers missed in #6: 1. scripts/install-hooks.ps1 still raw-fetched from resq-software/dev/$Ref/scripts/git-hooks — the retired path. Windows users following the curl-pipe flow would get 404s. Rewritten to mirror install-hooks.sh's two-path strategy: prefer `resq dev install-hooks` when on PATH, fall back to the crates raw URL. RESQ_CRATES_REF replaces RESQ_DEV_REF. Adds the scaffold prompt for the local-pre-push, matching the bash mirror. 2. .github/workflows/hooks-tests.yml had `paths: scripts/git-hooks/**` in its trigger filter — that directory no longer exists, so the workflow wouldn't fire on test changes alone. Trigger now fires on tests/hooks/**, scripts/install-hooks.sh, and the workflow file. Added a weekly cron so canonical-template drift on the crates side gets caught even without a dev-side change. Verified end-to-end: install-resq.sh → fresh repo + Cargo.toml → install-hooks.sh (resq-preferred path) → `git commit` triggers `resq pre-commit` which runs through Copyright/Large Files/Debug/ Secrets/Audit/Format. Doctor reports healthy. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WomB0ComB0
added a commit
to resq-software/crates
that referenced
this pull request
Apr 14, 2026
- .github/workflows/hooks-sync.yml: drop `drift-check-dev` job. It fetched from resq-software/dev/main/scripts/git-hooks/ — which was retired in resq-software/dev#6 — so the job would fail every run. The remaining local drift-check between templates/ and .git-hooks/ is now the whole check surface. - AGENTS.md + CLAUDE.md: rewrite the Git hooks section to reflect the post-Phase-4 reality. This repo is the single source of truth; the old "will converge once resq dev install-hooks scaffolds from embedded templates" qualifier is obsolete (shipped in #48). The link to dev/tree/main/scripts/git-hooks no longer resolves; replaced with a forward link to the installer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 tasks
WomB0ComB0
added a commit
to resq-software/crates
that referenced
this pull request
Apr 14, 2026
…58) - .github/workflows/hooks-sync.yml: drop `drift-check-dev` job. It fetched from resq-software/dev/main/scripts/git-hooks/ — which was retired in resq-software/dev#6 — so the job would fail every run. The remaining local drift-check between templates/ and .git-hooks/ is now the whole check surface. - AGENTS.md + CLAUDE.md: rewrite the Git hooks section to reflect the post-Phase-4 reality. This repo is the single source of truth; the old "will converge once resq dev install-hooks scaffolds from embedded templates" qualifier is obsolete (shipped in #48). The link to dev/tree/main/scripts/git-hooks no longer resolves; replaced with a forward link to the installer. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
Phase 4 consolidation. The canonical hook templates now live only in
resq-software/crates/crates/resq-cli/templates/git-hooks/. This repo used to ship a byte-identical copy underscripts/git-hooks/; it's retired.scripts/install-hooks.sh— two-path installresqon PATH →resq dev install-hooks(offline, uses embedded templates)resq→curlfrom the crates master raw URLRESQ_CRATES_REFreplacesRESQ_DEV_REFfor pinning the fallback.Retired
scripts/git-hooks/{pre-commit,commit-msg,prepare-commit-msg,pre-push,post-checkout,post-merge,README.md}tests/hooks/helpers.bashFetches templates from the crates raw URL once per bats session, cached under
/tmp.RESQ_HOOK_SRC_DIRlets you point at a local template dir for pre-push testing. 33/33 bats tests still pass..github/workflows/hooks-sync.ymllint— shellcheck + bash-parse on the two installers.smoke(new) — spins up a throwaway git repo, runsinstall-hooks.shwith scrubbed PATH to force the raw-fetch path, asserts 6 hooks present + executable + parse.Docs
Test plan
shellcheck -S warning+bash -ngreen locallybats tests/hooks/→ 33/33 using the crates raw sourcehooks-sync+hooks-testsgreen on this PRBreaking change note
Any external tooling that raw-fetched from
resq-software/dev/main/scripts/git-hooks/...needs to point atresq-software/crates/master/crates/resq-cli/templates/git-hooks/.... No sibling ResQ repo does this — they all install viainstall-hooks.sh.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Documentation
Chores