Skip to content

fix(npm): warn when install hooks are skipped#10280

Merged
jdx merged 4 commits into
mainfrom
codex/npm-skipped-hooks-warning
Jun 9, 2026
Merged

fix(npm): warn when install hooks are skipped#10280
jdx merged 4 commits into
mainfrom
codex/npm-skipped-hooks-warning

Conversation

@jdx

@jdx jdx commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • warn after npm installs when the installed package declares lifecycle scripts that were skipped by mise
  • honor npm_args = "--ignore-scripts=false" and other ignore-scripts overrides before deciding to warn
  • add npm backend unit coverage for effective ignore-scripts parsing and package.json lifecycle detection

Tests

  • cargo fmt --check
  • cargo test backend::npm::tests
  • git diff --check

This PR was generated by an AI coding assistant.


Note

Low Risk
Install behavior is unchanged aside from optional post-install warnings; risk is limited to noisy logs for packages with install scripts.

Overview
After npm global installs, mise now warns when the installed package defines preinstall / install / postinstall scripts that were not run because mise still passes --ignore-scripts=true.

Warning is emitted only when parsed npm_args indicate scripts are still skipped. New logic walks npm_args (including --ignore-scripts=false, --no-ignore-scripts, and =true/=false forms, with later flags winning) so opting in via npm_args = "--ignore-scripts=false" does not produce a false alarm.

The warning names the hooks, points at the installed package.json, and suggests reviewing the package before enabling scripts.

Reviewed by Cursor Bugbot for commit d318ad7. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • New Features

    • npm installation now detects and warns when lifecycle scripts are being skipped, providing guidance on how to enable them if needed.
  • Tests

    • Added test coverage for npm lifecycle script detection and configuration handling.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

npm install now parses npm_args to determine if lifecycle scripts are being ignored via --ignore-scripts flags. After install completes, if scripts were skipped and the package declares lifecycle scripts in package.json, a warning guides users to opt in with --ignore-scripts=false.

Changes

npm lifecycle scripts warning

Layer / File(s) Summary
Lifecycle scripts detection helpers
src/backend/npm.rs (lines 797–908, 1091–1139)
New functions compute the effective --ignore-scripts setting from a sequence of flags, locate package.json in the install directory, extract lifecycle script keys (preinstall, install, postinstall, prepare) from the scripts object, parse boolean string arguments, and unit tests validate detection logic and flag precedence.
npm install flow integration
src/backend/npm.rs (lines 419–420, 440–446)
During install setup, npm_args are parsed and the effective ignore-scripts state is computed. After install command execution, a warning is conditionally emitted when lifecycle scripts were skipped but the installed package declares them, including guidance to disable --ignore-scripts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit hops through npm's domain,
Detecting scripts that go unrun,
With warnings kind to guide the way—
"Set ignore-scripts to false, hooray!"
The tests all pass, the logic's sound,
A helpful guide for setup found. 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main change: emitting a warning when npm install hooks are skipped, which matches the primary objective of the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/npm-skipped-hooks-warning

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

@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a post-install warning to the npm backend when lifecycle scripts (preinstall, install, postinstall) declared in a package's package.json were skipped because mise passes --ignore-scripts=true by default. The warning is suppressed when the user has explicitly opted scripts in via npm_args.

  • effective_npm_ignore_scripts parses the user-supplied npm_args slice with last-arg-wins semantics, handling --ignore-scripts=bool, --ignore-scripts <bool>, --no-ignore-scripts, and npm's 0/1 boolean forms.
  • installed_package_lifecycle_scripts searches the installed package tree (both lib/node_modules and node_modules) for the package's package.json and returns only the install-time hooks (preinstall, install, postinstall), intentionally excluding prepare which npm skips for versioned registry installs.
  • Unit tests cover default behaviour, all override forms, last-arg-wins ordering, and lifecycle-script detection including exclusion of non-lifecycle scripts like prepare.

Confidence Score: 5/5

Safe to merge — the change is additive (warning only) and the default install behaviour is unchanged.

The logic is additive: no install behaviour changes, only a post-install warning when scripts are provably skipped. The argument parser correctly handles every npm boolean form (true/false/1/0, bare flag, --no- prefix, last-arg-wins), the path lookup covers both Linux/macOS and Windows node_modules layouts, and unit tests exercise all branches. Previous iterator and parse_bool_arg issues from earlier rounds have been addressed.

No files require special attention.

Important Files Changed

Filename Overview
src/backend/npm.rs Adds post-install lifecycle-script warning: effective_npm_ignore_scripts parses user npm_args (supporting --ignore-scripts=bool, --ignore-scripts bool, --no-ignore-scripts, and 0/1 values); warn_if_npm_package_lifecycle_scripts_skipped inspects the installed package.json and emits a warning when scripts were skipped. Unit tests cover all major parsing variants.

Reviews (4): Last reviewed commit: "fix(npm): document omitted prepare hook" | Re-trigger Greptile

Comment thread src/backend/npm.rs
Comment thread src/backend/npm.rs Outdated

jdx commented Jun 9, 2026

Copy link
Copy Markdown
Owner Author

Addressed the latest Greptile feedback in 4aa6b7b: removed prepare from the registry install lifecycle warning list, since npm does not run it for versioned registry installs. The package.json fixture now includes prepare to verify it does not trigger the warning list.

Verified with cargo fmt --check, cargo test backend::npm::tests, and git diff --check.

This comment was generated by an AI coding assistant.

jdx commented Jun 9, 2026

Copy link
Copy Markdown
Owner Author

Addressed the latest Greptile clarification in d318ad7: added a short comment explaining why prepare is intentionally excluded from the lifecycle warning list for versioned registry installs.

Verified with cargo fmt --check and git diff --check.

This comment was generated by an AI coding assistant.

@jdx jdx enabled auto-merge (squash) June 9, 2026 02:15
@jdx jdx merged commit 6542c09 into main Jun 9, 2026
34 checks passed
@jdx jdx deleted the codex/npm-skipped-hooks-warning branch June 9, 2026 02:18
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 x -- echo 19.1 ± 1.0 17.1 24.3 1.00
mise x -- echo 19.8 ± 1.4 17.8 34.0 1.03 ± 0.09

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 env 18.4 ± 0.9 16.4 22.7 1.00
mise env 19.1 ± 0.9 17.2 22.8 1.04 ± 0.07

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 hook-env 18.9 ± 0.8 17.2 22.4 1.00
mise hook-env 19.6 ± 0.9 18.0 22.8 1.04 ± 0.07

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 ls 15.4 ± 0.8 13.7 19.6 1.00
mise ls 16.0 ± 0.8 14.5 18.9 1.04 ± 0.07

xtasks/test/perf

Command mise-2026.6.1 mise Variance
install (cached) 135ms 136ms +0%
ls (cached) 58ms 58ms +0%
bin-paths (cached) 64ms 64ms +0%
task-ls (cached) 124ms 125ms +0%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants