Fix release script pre-commit hook race condition #2012
Conversation
- Use lefthook's stage_fixed option instead of manual git add - Prevents index.lock errors when hooks run in parallel - Fixes rake release task failures during version bumps
|
Caution Review failedThe pull request is closed. WalkthroughRemoved manual git re-staging from lefthook hook scripts and added explicit file globs plus Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Lefthook
participant HookScript as Hook Script
participant Git
rect rgba(255,200,160,0.15)
note over Dev,Lefthook: Old flow (pre-change)
Dev->>Lefthook: Commit (pre-commit)
Lefthook->>HookScript: Run formatter/autofix/eslint --fix
HookScript->>Git: git add (manual re-stage)
Git-->>HookScript: Files staged
end
rect rgba(160,220,200,0.12)
note over Dev,Lefthook: New flow (post-change)
Dev->>Lefthook: Commit (pre-commit)
Lefthook->>HookScript: Run formatter/autofix/eslint --fix
note right of Lefthook: Lefthook `stage_fixed: true` auto-stages fixes
Lefthook->>Git: stage_fixed staging (automatic)
Git-->>Lefthook: Files staged
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.lefthook.yml(1 hunks)bin/lefthook/prettier-format(0 hunks)bin/lefthook/ruby-autofix(1 hunks)
💤 Files with no reviewable changes (1)
- bin/lefthook/prettier-format
🔇 Additional comments (4)
bin/lefthook/ruby-autofix (1)
22-22: LGTM - Manual re-staging correctly removed.The simplified completion message is appropriate. The manual
git addre-staging logic has been correctly removed since the autofix hook in.lefthook.ymlnow usesstage_fixed: trueto handle re-staging automatically, eliminating the race condition..lefthook.yml (3)
8-11: Excellent fix for the race condition.The combination of
globpattern filtering andstage_fixed: truecorrectly addresses the git index lock issue. Lefthook will now handle re-staging automatically instead of the script doing manualgit addcalls in parallel.
17-20: LGTM - eslint hook properly configured.The
stage_fixed: trueaddition ensures that any ESLint auto-fixes are automatically re-staged, preventing the race condition when running in parallel with other hooks.
22-25: LGTM - prettier hook properly configured.The
stage_fixed: trueaddition ensures that any Prettier formatting changes are automatically re-staged. This complements the removal of manualgit addfrombin/lefthook/prettier-formatmentioned in the PR summary.
- Update all glob patterns from "*.{...}" to "**/*.{...}" for recursive matching
- Ensures pre-commit hooks run on files in all subdirectories (lib/, spec/, etc.)
- Remove redundant git add from eslint-lint script (lefthook handles via stage_fixed)
- Add missing stage_fixed to rubocop command for consistency
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
- Add stage_fixed: true to trailing-newlines command in .lefthook.yml - Update check-trailing-newlines script to automatically add missing newlines instead of just reporting them - Improve ruby-autofix success message to clarify that files are auto-staged - Ensures consistency with other auto-fixing hooks (autofix, eslint, prettier) This completes the pre-commit hook race condition fix by making all auto-fixing commands use Lefthook's stage_fixed feature uniformly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.lefthook.yml(1 hunks)bin/lefthook/eslint-lint(0 hunks)
💤 Files with no reviewable changes (1)
- bin/lefthook/eslint-lint
🔇 Additional comments (2)
.lefthook.yml (2)
11-11:stage_fixed: trueconfiguration correctly addresses the race condition.The addition of
stage_fixed: trueto the autofix, rubocop, eslint, and prettier hooks is the right approach to fix the race condition. Instead of multiple hooks competing to callgit addin parallel, Lefthook now atomically re-stages modified files after each hook completes, eliminating the index.lock contention.This aligns with the PR objectives and the removal of manual re-staging logic from
bin/lefthook/ruby-autofix,bin/lefthook/eslint-lint, andbin/lefthook/prettier-formatscripts.Also applies to: 16-16, 21-21, 26-26
28-29: Trailing-newlines hook appropriately omitsstage_fixed.The trailing-newlines hook is a check-only hook that does not modify files, so
stage_fixed: trueis not needed. The current configuration is correct.
| parallel: true | ||
| commands: | ||
| autofix: | ||
| glob: "**/*.{rb,rake,ru}" |
There was a problem hiding this comment.
🧩 Analysis chain
Glob patterns may still miss root-level files—verify coverage.
The patterns have been updated to use **/*.{ext}, but according to Lefthook's glob semantics (as documented in the previous review), ** matches one-or-more directory levels and does not match files directly in the repository root. Files like babel.config.js, package.json, .eslintrc.json, or Rakefile in the root directory may not be matched.
The correct approach is to use array patterns that include both root and recursive matches:
glob: ["*.{rb,rake,ru}", "**/*.{rb,rake,ru}"]Since the PR was tested and confirmed working, verify whether:
- Root-level files actually require formatting/linting by these hooks in your repository, or
- The Lefthook version/implementation differs from the documented behavior.
Run the following script to check if root-level files exist that should be matched by these hooks:
Also applies to: 14-14, 19-19, 24-24
🏁 Script executed:
#!/bin/bash
# Check for root-level files matching the hook patterns
echo "=== Root-level Ruby files ==="
find . -maxdepth 1 -type f \( -name "*.rb" -o -name "*.rake" -o -name "*.ru" \) | head -20
echo ""
echo "=== Root-level JavaScript/TypeScript files ==="
find . -maxdepth 1 -type f \( -name "*.js" -o -name "*.jsx" -o -name "*.ts" -o -name "*.tsx" \) | head -20
echo ""
echo "=== Root-level JSON files ==="
find . -maxdepth 1 -type f -name "*.json" | head -20
echo ""
echo "=== Root-level YAML files ==="
find . -maxdepth 1 -type f \( -name "*.yml" -o -name "*.yaml" \) | head -20
echo ""
echo "=== Root-level Markdown files ==="
find . -maxdepth 1 -type f -name "*.md" | head -20Length of output: 736
Fix glob patterns to match root-level configuration files—critical issue confirmed.
The glob patterns are missing important root-level files that should be linted and formatted:
- ESLint hook (
**/*.{js,jsx,ts,tsx}): Misses./eslint.config.ts,./jest.config.base.js,./knip.ts - Prettier hook (
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}): Misses./package.json,./tsconfig.json,./.rubocop.yml,./docker-compose.yml, and root-level markdown files
The pattern **/*.{ext} matches one-or-more directory levels but does not match files directly in the repository root. Replace with array patterns:
glob: ["*.{js,jsx,ts,tsx}", "**/*.{js,jsx,ts,tsx}"] # eslint (line 19)
glob: ["*.{js,jsx,ts,tsx,json,md,yml,yaml}", "**/*.{js,jsx,ts,tsx,json,md,yml,yaml}"] # prettier (line 24)Also affects lines 9 and 14 (autofix/rubocop). While no root-level Ruby files exist now, using arrays provides forward compatibility.
Summary
rake releasetask failing with "Unable to create index.lock" errorgit addfiles simultaneously, causing a race condition for the git index lockstage_fixed: trueoption instead of manualgit addcommandsChanges
.lefthook.ymlto addstage_fixed: trueto autofix, eslint, and prettier hooksgit addcalls frombin/lefthook/prettier-formatgit addcalls frombin/lefthook/ruby-autofixTest Plan
.lefthook.yml)index.lockerrors occurImpact
This fix ensures that the release script can successfully commit version bumps without encountering race conditions when multiple formatters try to re-stage files.
This change is
Summary by CodeRabbit
Chores
Bug Fixes