[shryssssss-maker]

What changed and why

Share one package-manager install command mapping across the fix plan, the commands executed by --fix, and the PD001/PD002 hints. The fix runner also uses the shared POSIX quoting helper for retry hints.

The regression tests assert the exact displayed commands and argument arrays for npm, pnpm, Yarn, and Bun, including workspace paths with spaces. They also verify the npm fallback for unknown package managers in phantom hints.

Verification

Ran the contributor verification sequence on Linux with Node 24, including the build, test hygiene, changed-test type check, advisory sync, and the full test suite. All 231 suites passed, with 3,547 tests passed and 3 skipped.

Closes #1627

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Improvements
  • Install and remediation instructions now use the appropriate commands for npm, pnpm, Yarn, and Bun.
  • Workspace install commands retain each package manager’s syntax, and paths containing spaces are handled correctly.
  • Unknown package managers use npm commands as a fallback.
  • Retry hints use consistent shell quoting, and unsafe package names continue to receive a plain instruction instead of a shell command.

Install-command prefixes for phantom hints, remediation plans, and --fix now use one shared mapping. The fix runner also uses shared shell quoting, while package-manager-specific workspace argument construction remains in place.

Changes: Install-command generation

Layer / File(s)	Summary
Shared mapping and displayed commands src/utils/install-command.ts, src/overrides/detectors/phantom-utils.ts, src/remediation/fix-commands.ts, tests/overrides/detectors/phantom-utils.test.ts	A typed mapping defines executable and install-subcommand pairs for npm, pnpm, Yarn, and Bun. Phantom hints and remediation plan commands use the mapping. Phantom hint tests cover those managers and the unknown-manager fallback.
Fix-runner commands src/utils/fix-runner.ts, tests/fix-runner-apply.test.ts	The fix runner uses the mapping for command labels and argument vectors, and imports the shared shell-quoting helper. Manager-specific workspace argument construction remains. Tests check development workspace commands and preserve paths with spaces as single arguments.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Possibly related PRs

• OWASP/cve-lite-cli#1427: Adds tests for phantom install-command prefixes that this change continues to cover.
• OWASP/cve-lite-cli#1387: Introduced the structured fix-command builders whose package-manager prefixes this change updates.

Merge Risk: Minimal · up to 51c0c

This refactor consolidates install-command generation without changing the printed commands or the --fix arguments. No merge-blocking risk is evident.

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 after the colon, and no trailing period. It accurately names the change, which moves the packa…
Description check		The description follows the template's "What changed and why" section and explains the shared install command mapping, the shared POSIX quoting helper for retry hints, and the regression tests for npm…
Linked Issues check		I checked #1627 against the whole-PR diff and the reviewed head. The three coding objectives are met. 1. One exported mapping for all three sites. Met. INSTALL_COMMANDS is exported from `src/utils/…
Out of Scope Changes check		Every changed file serves #1627. src/utils/install-command.ts adds the shared mapping. src/overrides/detectors/phantom-utils.ts, src/remediation/fix-commands.ts, and src/utils/fix-runner.ts sw…
Linked Issue		I checked the pull request body. It contains the line "Closes #1627". That uses the keyword "Closes" followed by an issue number, which matches the check. The linked issue is #1627, "refactor: one ins…
Closes Keyword Matches Scope		The PR body uses "Closes #1627", so I checked the linked issue against the diff. The issue asks for these items: 1. One exported install-command mapping used by all three places. Delivered. `src/u…
No New Runtime Dependency		This check passes. The PR does not change package.json. I checked the reviewed range (refs/coderabbit/pre-merge-check/base to refs/coderabbit/pre-merge-check/head). The changed-file list has six entri…

Full details: Title check

Explanation

The title follows the required "type: short description" form with the allowed type "refactor", lowercase after the colon, and no trailing period. It accurately names the change, which moves the package-manager install command prefixes into one shared mapping used by the fix plan, the --fix runner, and the PD001/PD002 hints.

Full details: Description check

Explanation

The description follows the template's "What changed and why" section and explains the shared install command mapping, the shared POSIX quoting helper for retry hints, and the regression tests for npm, pnpm, Yarn, Bun, and the unknown-manager npm fallback. It also includes a verification section and a "Closes #1627" line matching the template's closing reference. The template placeholder "Closes #" is filled in with the issue number, so the required content is present.

Full details: Linked Issues check

Explanation

I checked #1627 against the whole-PR diff and the reviewed head. The three coding objectives are met.

1. One exported mapping for all three sites. Met. INSTALL_COMMANDS is exported from src/utils/install-command.ts (npm: ["npm","install"], pnpm: ["pnpm","add"], yarn: ["yarn","add"], bun: ["bun","add"]). At the reviewed head, src/overrides/detectors/phantom-utils.ts imports it and builds the PD001/PD002 hint in phantomInstallInstruction from it. Unknown package managers fall back to the npm entry. This closes the gap in my earlier review, which had relied on the summary for this file. src/remediation/fix-commands.ts (commandPrefix) and src/utils/fix-runner.ts (commandLabelForPackageManager, buildInstallApplyArgv) also read from the mapping, according to the whole-PR summary.

2. fix-runner.ts uses the shared shellQuote. Met, per the whole-PR summary. The private helper was removed, and the shared one from src/utils/shell-quote.ts is used for retry hints. Earlier comparison showed identical quoting logic.

3. Printed commands and --fix arguments unchanged for npm, pnpm, Yarn, and Bun. Supported. The mapping values match the removed literals. Yarn and workspace-specific argument handling stays in each branch. The test changes cover all four managers, workspace paths with spaces, and the unknown-manager fallback.

The suite-passing criterion rests on the author's reported run (231 suites passed, 3,547 tests passed, 3 skipped). I did not re-run the suite. I did not check whether any other file still imports the removed installCmd export. The reported build and type check cover that risk.

Full details: Out of Scope Changes check

Explanation

Every changed file serves #1627. src/utils/install-command.ts adds the shared mapping. src/overrides/detectors/phantom-utils.ts, src/remediation/fix-commands.ts, and src/utils/fix-runner.ts switch to that mapping. fix-runner.ts also drops its duplicate shellQuote. phantom-utils.ts removes the exported installCmd helper, which the mapping replaces. The two test files cover the four package managers, the workspace-path case, and the phantom-hint fallback. I found no unrelated behavior change in the files I read.

Full details: Linked Issue

Explanation

I checked the pull request body. It contains the line "Closes #1627". That uses the keyword "Closes" followed by an issue number, which matches the check. The linked issue is #1627, "refactor: one install-command mapping for the fix plan, --fix and the phantom hints". The linked issue objectives match the PR scope. The PR body is the only evidence this check needs, so I did not run repository inspection. I read the PR body as the source of the reference. The check does not ask whether the issue is open or whether the PR fully delivers it. It only asks for the closing reference, and that reference is present.

Full details: Closes Keyword Matches Scope

Explanation

The PR body uses "Closes #1627", so I checked the linked issue against the diff. The issue asks for these items: 1. One exported install-command mapping used by all three places. Delivered. src/utils/install-command.ts (new) exports INSTALL_COMMANDS for npm, pnpm, yarn and bun. phantom-utils.ts replaces installCmd with it, and fix-commands.ts commandPrefix and fix-runner.ts commandLabelForPackageManager and buildInstallApplyArgv now read from it. The old local mappings are gone. 2. fix-runner.ts uses the shared shellQuote. Delivered. The private shellQuote is removed, and the file imports shellQuote from ./shell-quote.js. The shared helper exists in src/utils/shell-quote.ts. 3. Printed commands and --fix arguments unchanged for npm, pnpm, Yarn and Bun. The diff keeps the same literal commands and argument order. Yarn's workspace <ws> add form and the pnpm --filter placement are preserved. The phantom npm fallback for unknown maps to npm install, as before. The test files were updated to assert these displayed commands and argument arrays. 4. The suite passes. The author reports 231 suites passed, with 3,547 tests passed and 3 skipped. I could not build or run tests in this sandbox, so I did not verify this item independently. Every code item in the issue is present in the diff. The Closes keyword is appropriate because merging should close #1627. The only unverified point is the test result, which the author reports in the PR description.

Full details: No New Runtime Dependency

Explanation

This check passes. The PR does not change package.json. I checked the reviewed range (refs/coderabbit/pre-merge-check/base to refs/coderabbit/pre-merge-check/head). The changed-file list has six entries: four source files, one new file (src/utils/install-command.ts), and two test files. No package.json or lockfile appears in the list. I also compared package.json at the base and head revisions. The dependencies, optionalDependencies, and devDependencies sections are identical in both. The diff for package.json is empty. The new INSTALL_COMMANDS constant is internal code. It adds no package to any dependency section. The check therefore has no new runtime dependency to flag.

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: 51c0c50fce62.

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: a35df55
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: e65ed8dc-e280-4291-a6a6-08ef76505a99

Commits

Reviewing files that changed from the base of the PR and between 12b9b8e and 51c0c50.

Files selected for processing: 3

• src/overrides/detectors/phantom-utils.ts
• src/remediation/fix-commands.ts
• tests/overrides/detectors/phantom-utils.test.ts

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

[shryssssss-maker]

@coderabbitai review

[coderabbitai]

Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[sonukapoor]

Thanks @shryssssss-maker, this is exactly what #1627 asked for. All three places now read from one mapping, and the private quote helper in the fix runner is gone in favour of the shared one. I compared the printed commands and the --fix arguments between main and this branch across all four package managers, with and without workspaces, dev installs and awkward names, and they come out identical. The new tests catch it when any entry in the mapping or the workspace handling is broken, which is what you want from a refactor like this.

Approved. I merge these one at a time, so it will land shortly.

[sonukapoor]

Merged - thank you @shryssssss-maker!