Skip to content

Fix release script pre-commit hook race condition #2012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 13, 2025
Merged

Fix release script pre-commit hook race condition #2012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a227ba
Fix release script pre-commit hook race condition
justin808 Nov 13, 2025
1680276
Fix lefthook glob patterns to match files in subdirectories
justin808 Nov 13, 2025
c5880a9
Make trailing-newlines hook auto-fix instead of validate
justin808 Nov 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .lefthook.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,28 @@ pre-commit:
parallel: true
commands:
autofix:
glob: "**/*.{rb,rake,ru}"
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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:

  1. Root-level files actually require formatting/linting by these hooks in your repository, or
  2. 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 -20

Length 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.

run: bin/lefthook/ruby-autofix all-changed
stage_fixed: true

rubocop:
glob: "**/*.{rb,rake,ru}"
run: bin/lefthook/ruby-lint all-changed
stage_fixed: true

eslint:
glob: "**/*.{js,jsx,ts,tsx}"
run: bin/lefthook/eslint-lint all-changed
stage_fixed: true

prettier:
glob: "**/*.{js,jsx,ts,tsx,json,md,yml,yaml}"
run: bin/lefthook/prettier-format all-changed
stage_fixed: true

trailing-newlines:
run: bin/lefthook/check-trailing-newlines all-changed
stage_fixed: true

pre-push:
commands:
Expand Down
14 changes: 2 additions & 12 deletions bin/lefthook/check-trailing-newlines
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,13 @@ else
echo "🔍 Checking trailing newlines on $CONTEXT files..."
fi

failed_files=""
for file in $files; do
if [ -f "$file" ] && [ -s "$file" ]; then
if ! tail -c 1 "$file" | grep -q '^$'; then
echo "❌ Missing trailing newline: $file"
failed_files="$failed_files $file"
echo "🔧 Adding trailing newline to: $file"
echo >> "$file"
fi
fi
done

if [ -n "$failed_files" ]; then
echo ""
echo "❌ Trailing newline check failed!"
echo "💡 Add trailing newlines to:$failed_files"
echo "🔧 Quick fix: for file in$failed_files; do echo >> \"\$file\"; done"
echo "🚫 Skip hook: git commit --no-verify"
exit 1
fi

echo "✅ All files have proper trailing newlines"
10 changes: 0 additions & 10 deletions bin/lefthook/eslint-lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ if [ -n "$root_and_packages_pro_files" ]; then
if ! yarn run eslint $root_and_packages_pro_files --fix; then
exit_code=1
fi

# Re-stage files if running on staged or all-changed context
if [ "$CONTEXT" = "staged" ] || [ "$CONTEXT" = "all-changed" ]; then
echo $root_and_packages_pro_files | xargs -r git add
fi
fi

# Lint react_on_rails_pro files (using Pro gem's ESLint config)
Expand All @@ -56,11 +51,6 @@ if [ -n "$react_on_rails_pro_files" ]; then
if ! (cd react_on_rails_pro && yarn run eslint $react_on_rails_pro_files_relative --fix); then
exit_code=1
fi

# Re-stage files if running on staged or all-changed context
if [ "$CONTEXT" = "staged" ] || [ "$CONTEXT" = "all-changed" ]; then
echo $react_on_rails_pro_files | xargs -r git add
fi
fi

if [ $exit_code -eq 0 ]; then
Expand Down
10 changes: 0 additions & 10 deletions bin/lefthook/prettier-format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ if [ -n "$root_files" ]; then
printf " %s\n" $root_files

yarn run prettier --write $root_files

# Re-stage files if running on staged or all-changed context
if [ "$CONTEXT" = "staged" ] || [ "$CONTEXT" = "all-changed" ]; then
echo $root_files | xargs -r git add
fi
fi

# Format Pro files (using Pro's Prettier config)
Expand All @@ -44,11 +39,6 @@ if [ -n "$pro_files" ]; then
pro_files_relative=$(echo "$pro_files" | sed 's|^react_on_rails_pro/||')

(cd react_on_rails_pro && yarn run prettier --write $pro_files_relative)

# Re-stage files if running on staged or all-changed context
if [ "$CONTEXT" = "staged" ] || [ "$CONTEXT" = "all-changed" ]; then
echo $pro_files | xargs -r git add
fi
fi

echo "✅ Prettier formatting complete"
6 changes: 1 addition & 5 deletions bin/lefthook/ruby-autofix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,4 @@ printf " %s\n" $files

bundle exec rake autofix

# Re-stage files if running on staged or all-changed context
if [ "$CONTEXT" = "staged" ] || [ "$CONTEXT" = "all-changed" ]; then
echo $files | xargs -r git add
echo "✅ Re-staged formatted files"
fi
echo "✅ Auto-fix complete (fixed files staged automatically)"