[vianmangal]

What changed and why

Address the two optional suggestions in the maintainer's approval of #1485.

Map each package manager to an ordered list of lockfiles and select the first existing match, replacing the nested ternary. Reuse NPM_LOCKFILE_NAMES, introduced by #1495, so npm's shrinkwrap-first rule stays defined in one place.

When the root holds only the two npm lockfiles, explain that npm reads npm-shrinkwrap.json ahead of package-lock.json. When other managers' lockfiles are also present, explain the choice using the packageManager declaration or filename precedence instead.

Keep the existing lockfile choices, including explicit pnpm/Yarn/Bun declarations and fallback when the declared manager has no matching file. Limit manager lookup to own table keys so unsupported names such as constructor remain harmless fallbacks.

Refs #1484

Verification

• npm ci, npm run lint:tests, npm run build: passed.
• node dist/index.js advisories sync --output /private/tmp/cve-lite-1499-validation.db: passed (230,433 records; isolated validation output).
• node scripts/typecheck-changed-tests.mjs origin/main: the changed test file passes.
• npm test -- --runInBand tests/parsers/load-packages.test.ts: 21 tests passed.
• npm test -- --runInBand: 204 suites, 3,108 tests passed, using the freshly synced local advisory database.
• Both new mixed-manager regression cases fail before the note fix and pass afterward. Existing coverage preserves the npm-only explanation, package-lock fallback, and unsupported manager handling.

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Lockfile selection now respects each package manager’s preferred lockfile order. For npm, npm-shrinkwrap.json takes precedence over package-lock.json, even when the project declares npm.
  • If no lockfile for the declared package manager is available, selection falls back to the available root lockfiles.
  • Explanatory notes clarify when npm’s shrinkwrap lockfile is selected ahead of package-lock.json.

Root lockfile selection follows the declared package manager’s ordered lockfile preferences when a preferred lockfile exists. Otherwise, selection uses root lockfile precedence. The result notes explain npm shrinkwrap precedence in the covered cases.

Changes: Root lockfile selection

Layer / File(s)	Summary
Manager lockfile preferences src/parsers/index.ts	The manager map now stores ordered lockfile preferences. npm uses NPM_LOCKFILE_NAMES. Manager names are checked with Object.hasOwn.
Root selection and validation src/parsers/index.ts, tests/parsers/load-packages.test.ts	Root selection uses the first present lockfile in the declared manager’s preferences, then falls back to root precedence. Tests cover npm lockfile ordering, selection notes, competing lockfiles, and unsupported manager names.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related issues

• fix(parsers): nested lockfile selection ignores packageManager, and --debug reads the root instead of the selected lockfile #1514: This PR updates root selection rules that the issue identifies as relevant to nested discovery. The nested selection behavior described in the issue is not changed here.

Possibly related PRs

• OWASP/cve-lite-cli#1485: Both PRs address root selection when npm lockfiles are present.
• OWASP/cve-lite-cli#1506: This PR changes the root selection rules that the PR uses to select the scan’s lockfile.
• OWASP/cve-lite-cli#1515: This PR changes the root selection rules that the PR applies inside a nested folder.

Merge Risk: Minimal · up to be636

The refactor preserves lockfile choices and correctly explains npm selection when other managers’ lockfiles are present. No actionable merge-blocking issue remains.

Pre-merge checks: 7

Check name	Status	Explanation
Title check		The title follows the required "type: short description" form with the allowed type "refactor", lowercase text after the colon, and no trailing period. It matches the main change, which centralizes np…
Description check		The description contains the template's required section, "What changed and why", with a clear explanation of the change, its motivation (addressing maintainer suggestions on #1485), and related refer…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Linked Issue		The pull request body references a GitHub issue with the accepted keyword and issue number: “Refs #1484.”
Closes Keyword Matches Scope		The pull request body uses only Refs #1484 and does not use Closes, Fixes, or Resolves. The custom check therefore does not require issue-scope verification, and merging will not auto-close th…
No New Runtime Dependency		The pull request changes only src/parsers/index.ts and tests/parsers/load-packages.test.ts. It does not change package.json. The dependencies, optionalDependencies, and devDependencies sec…

Full details: Title check

Explanation

The title follows the required "type: short description" form with the allowed type "refactor", lowercase text after the colon, and no trailing period. It matches the main change, which centralizes npm lockfile selection by mapping each package manager to an ordered lockfile list and reusing NPM_LOCKFILE_NAMES.

Full details: Description check

Explanation

The description contains the template's required section, "What changed and why", with a clear explanation of the change, its motivation (addressing maintainer suggestions on #1485), and related references (#1484, #1495). It also includes verification steps. The template's "Closes #" line is missing: the description uses "Refs #1484" instead, so the issue link does not use the template's closing keyword.

Full details: Closes Keyword Matches Scope

Explanation

The pull request body uses only Refs #1484 and does not use Closes, Fixes, or Resolves. The custom check therefore does not require issue-scope verification, and merging will not auto-close the issue.

Full details: No New Runtime Dependency

Explanation

The pull request changes only src/parsers/index.ts and tests/parsers/load-packages.test.ts. It does not change package.json. The dependencies, optionalDependencies, and devDependencies sections are identical at the base and head refs, so no new runtime dependency or dev-to-runtime move was introduced.

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Automatic approval policy

Commit: be6367c69d40.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
Merge risk	Moderate	Global configuration
Security risk	Not set	Global configuration

No overrides matched. The global limits apply.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

• Source mode: Auto
• Repository: OWASP/cve-lite-cli
• Branch: main
• File: .coderabbit.yaml
• Commit: 3cb5842
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 647d2aa1-1e36-4141-8b45-7906d12ef7f9

Commits

Reviewing files that changed from the base of the PR and between 48966a7 and be6367c.

Files selected for processing: 2

• src/parsers/index.ts
• tests/parsers/load-packages.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[sonukapoor]

Thanks @vianmangal, this reads much better than the nested ternary, and each manager listing its lockfiles in order makes the precedence obvious at a glance. On the merge with current main the full suite passes (204 suites, 3,106 tests) and the build is clean.

One thing before it goes in, and one optional, both inline.

[sonukapoor]

Thanks @vianmangal, both are in. The npm entry spreads NPM_LOCKFILE_NAMES, and the shrinkwrap note now only appears when package-lock.json is the one other file, so a root that also holds pnpm-lock.yaml gets the precedence wording. On the merge with current main, 214 suites / 3,232 tests pass, and loosening either rule fails the tests. Approved. I merge these one at a time rather than in a batch, so it will land shortly.

[sonukapoor]

Merged - thank you @vianmangal!