Skip to content

fix(install): make prek optional in prepare script (fixes #731)#746

Merged
cv merged 2 commits intoNVIDIA:mainfrom
kagura-agent:fix/prepare-prek-optional
Mar 24, 2026
Merged

fix(install): make prek optional in prepare script (fixes #731)#746
cv merged 2 commits intoNVIDIA:mainfrom
kagura-agent:fix/prepare-prek-optional

Conversation

@kagura-agent
Copy link
Contributor

@kagura-agent kagura-agent commented Mar 23, 2026

Fixes #731

Problem

The prepare script in package.json unconditionally runs prek install to set up git hooks. Since prek is a devDependency (@j178/prek), it's not installed when:

  • End users clone the repo and run npm install without --include=dev
  • The installer runs npm install --ignore-scripts then later triggers prepare
  • Users deploy from a git checkout directory

This causes npm error code 127 (sh: 1: prek: not found) and blocks installation.

Solution

Guard the prek invocation with command -v prek so the prepare hook only runs when prek is actually available:

- "prepare": "if [ -d .git ]; then prek install; fi"
+ "prepare": "if [ -d .git ] && command -v prek >/dev/null 2>&1; then prek install; fi"

Developers with devDependencies installed get hooks as before. End-user installs skip hook setup gracefully.

One-line change

Only package.json is modified.

Summary by CodeRabbit

  • Chores
    • Improved install/setup script to verify the required CLI tool is available before running setup.
    • Now emits a clear error if the project expects the tool but it’s not accessible, preventing silent failures.
    • If the tool is not present at all, the hook/setup step is skipped with an informative message.

The prepare script runs 'prek install' to set up git hooks, but prek is
a devDependency. When end users install NemoClaw from a git checkout
without --include=dev (or when the installer runs 'npm install
--ignore-scripts' then later triggers prepare), prek is not available
and npm exits with code 127.

Fix: Guard the prek invocation with 'command -v prek' so it only runs
when the binary is actually installed (i.e., in a dev environment).
End-user installs skip hook setup gracefully.
@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8a27e04a-e378-4ee6-9696-4d67de5c8a04

📥 Commits

Reviewing files that changed from the base of the PR and between 3476f41 and 3a4cf60.

📒 Files selected for processing (1)
  • package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json

📝 Walkthrough

Walkthrough

The prepare npm script in package.json was updated to gate hook setup on both the presence of a .git directory and the availability of the prek executable; it now errors if @j178/prek is installed but prek is not in PATH, and otherwise skips hook setup with a message.

Changes

Cohort / File(s) Summary
Install Script Configuration
package.json
Changed scripts.prepare to check command -v prek before running prek install. If node_modules/@j178/prek exists but prek is not in PATH the script now errors and exits 1; if neither is available it prints a skip message and does not run hooks.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 I hopped around the scripts at night,
Found a missing prek that gave a fright.
Now I check the path before I call,
No more install crash — hooray for all! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: making prek optional in the prepare script and references the fixed issue #731.
Linked Issues check ✅ Passed The pull request directly addresses all coding requirements from issue #731: adding a binary check (command -v prek) to gate hook installation and allowing graceful failure when prek is absent.
Out of Scope Changes check ✅ Passed All changes are scoped to the prepare script modification in package.json; no unrelated alterations or out-of-scope code changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@wscurran wscurran added bug Something isn't working CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. priority: medium Issue that should be addressed in upcoming releases labels Mar 24, 2026
@wscurran
Copy link
Contributor

Thanks for submitting this PR, it makes prek optional in the prepare script, which could improve the CI/CD pipeline and user experience by providing more flexibility and preventing potential issues.

Copy link
Contributor

@cv cv left a comment

Choose a reason for hiding this comment

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

Fixes the right problem, but as-is this silently skips hook setup even for developers where prek should be available (e.g. devDeps installed but binary somehow broken). Consider distinguishing "prek intentionally absent" (deploy/CI) from "prek should be here but isn't" (dev) — see inline suggestion.

@cv cv merged commit faad085 into NVIDIA:main Mar 24, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. priority: medium Issue that should be addressed in upcoming releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NemoClaw install fails in Git checkout when hook bootstrap invokes missing 'prek' binary (npm error 127)

3 participants