Add npm ci support with opt-in strict mode#545
Open
markottaviani wants to merge 2 commits into
Open
Conversation
npm ci gives reproducible installs (exact versions from package-lock.json) where npm install can silently drift. Fall back to npm install by default for backward compatibility with apps lacking a valid lockfile; set NPM_CI_STRICT=true to hard-fail the build instead of falling back.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughChangesThe Node.js image npm CI installation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Assemble
participant npm_ci
participant npm_install
Assemble->>npm_ci: Attempt dependency installation
alt Strict mode and npm ci fails
Assemble-->>Assemble: Report error and exit 1
else Non-strict mode and npm ci fails
Assemble->>npm_install: Warn and install dependencies
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
22-minimal/s2i/bin/assemble (1)
91-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptionally check for lockfiles before running
npm cito avoid noisy errors.If a project intentionally does not use a
package-lock.jsonornpm-shrinkwrap.json,npm cipredictably fails and emits a verbosenpm ERR!stack trace into the build logs before the script gracefully falls back tonpm install. Checking for the presence of a lockfile before attemptingnpm ciprovides a cleaner build experience and reduces confusion for those users.
22-minimal/s2i/bin/assemble#L91-L102: Consider bypassing thenpm ciblock and proceeding directly tonpm installif neither lockfile exists ANDNPM_CI_STRICTis not set totrue. Apply this optimization to both the production and non-production branches.22/s2i/bin/assemble#L91-L102: Apply the lockfile optimization here as well.24-minimal/s2i/bin/assemble#L91-L102: Apply the lockfile optimization here as well.24/s2i/bin/assemble#L91-L102: Apply the lockfile optimization here as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@22-minimal/s2i/bin/assemble` around lines 91 - 102, Update the npm installation blocks in 22-minimal/s2i/bin/assemble (lines 91-102), 22/s2i/bin/assemble (lines 91-102), 24-minimal/s2i/bin/assemble (lines 91-102), and 24/s2i/bin/assemble (lines 91-102) to check for package-lock.json or npm-shrinkwrap.json before running npm ci. When neither lockfile exists and NPM_CI_STRICT is not true, bypass npm ci and proceed directly to npm install; preserve the existing strict-mode failure behavior and apply this in both production and non-production branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@22-minimal/s2i/bin/assemble`:
- Around line 107-116: Ensure the non-production dependency-installation branch
runs npm ci with NODE_ENV=development so devDependencies are installed. Update
the npm ci invocation in the assemble flow for 22-minimal/s2i/bin/assemble
(107-116), 22/s2i/bin/assemble (107-116), 24-minimal/s2i/bin/assemble (107-116),
and 24/s2i/bin/assemble (107-116); leave the existing strict-mode fallback
behavior unchanged.
---
Nitpick comments:
In `@22-minimal/s2i/bin/assemble`:
- Around line 91-102: Update the npm installation blocks in
22-minimal/s2i/bin/assemble (lines 91-102), 22/s2i/bin/assemble (lines 91-102),
24-minimal/s2i/bin/assemble (lines 91-102), and 24/s2i/bin/assemble (lines
91-102) to check for package-lock.json or npm-shrinkwrap.json before running npm
ci. When neither lockfile exists and NPM_CI_STRICT is not true, bypass npm ci
and proceed directly to npm install; preserve the existing strict-mode failure
behavior and apply this in both production and non-production branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aafbe7c9-0a42-4534-a85d-06c32b3ee129
📒 Files selected for processing (13)
18-minimal/s2i/bin/assemble18/s2i/bin/assemble20-minimal/s2i/bin/assemble20/s2i/bin/assemble22-minimal/README.md22-minimal/s2i/bin/assemble22/README.md22/s2i/bin/assemble24-minimal/README.md24-minimal/s2i/bin/assemble24/README.md24/s2i/bin/assembletest/run
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use
npm cifor dependency installs so builds are reproducible (exact versions frompackage-lock.json), falling back tonpm installwhennpm cifails, e.g. no lockfile, or a lockfile out of sync withpackage.json, so existing apps without a committed or out of date lockfile keep building as before.Adds
NPM_CI_STRICT(default:false). When set totrue, a failednpm cifails the build immediately instead of silently falling back tonpm install, for users who want CI/CD pipelines to catch a broken/missing lockfile rather than silently drift tonpm install.Applies to all supported Node versions.
This is a replacement to an old PR: #236 that never made it in.
Prior discussion in #212
Summary by CodeRabbit
npm ciovernpm install.NPM_CI_STRICT=true, build fails immediately ifnpm cifails.npm installifnpm cifails.NPM_CI_STRICTand its default (false) behavior.