fix(install): skip prek hook setup when binary not in PATH#1502
fix(install): skip prek hook setup when binary not in PATH#1502kagura-agent wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
Remove the elif branch that checked for node_modules/@j178/prek and exited with an error. After `npm install --omit=dev` the package directory may linger while the binary is not linked, causing a spurious failure on clean installs. If prek is not on PATH, simply skip hook setup regardless of node_modules state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Apologies for the extra round-trip — the original #746 should have been tested with a clean install from the start. Will be more thorough with lifecycle script changes going forward. |
Problem
PR #746 (merged) made
prekoptional in the prepare script, but introduced a regression: whennode_modules/@j178/prekdirectory exists but theprekbinary is not in PATH, the script errors withexit 1.This happens in practice when running
npm install --omit=dev(as the NemoClaw install script does) — the package directory may be present from a previous full install, but the binary won't be linked to PATH.Reported by @DanTup on a clean Ubuntu VM: #746 (comment)
Root Cause
The prepare script tried to distinguish between "prek intentionally absent" and "prek should be here but isn't" by checking for the
node_modules/@j178/prekdirectory. However, this distinction is unreliable — the directory can exist without the binary being available (e.g., after--omit=devwith a stalenode_modules).Fix
Remove the
elif [ -d node_modules/@j178/prek ]branch entirely. Ifprekis not in PATH, simply skip hook setup regardless of whether the package directory exists.Before:
After:
Test Scenarios
.gitexists +prekin PATHprek install.gitexists +preknot in PATH +node_modules/@j178/prekexistsexit 1).gitexists +preknot in PATH + nonode_modules.gitdirectoryFixes regression from #746.
Summary by CodeRabbit