Skip to content

feat: add prefer-package-manager builtin policy#126

Merged
NiveditJain merged 8 commits into
mainfrom
luv-126
Apr 21, 2026
Merged

feat: add prefer-package-manager builtin policy#126
NiveditJain merged 8 commits into
mainfrom
luv-126

Conversation

@NiveditJain

@NiveditJain NiveditJain commented Apr 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds new prefer-package-manager builtin policy that enforces allowed package managers via an allowlist
  • When enabled, blocks any detected package manager not in the allowed list (e.g., pip when only uv is allowed) and tells Claude to rewrite the command
  • Built-in detection for 10 managers: pip, npm, yarn, pnpm, bun, uv, poetry, pipenv, conda, cargo
  • Users can append additional blocked managers via the blocked param (e.g., pdm, pipx)
  • Two-pass detection: allowed managers checked first (handles uv pip install correctly), then blocked ones
  • Docs updated across getting-started, custom-policies, examples, and README to emphasize convention-based policies as org-wide quality standards

Configuration

{
  "enabledPolicies": ["prefer-package-manager"],
  "policyParams": {
    "prefer-package-manager": {
      "allowed": ["uv", "bun"],
      "blocked": ["pdm", "pipx"]
    }
  }
}

Test plan

  • 19 unit tests covering deny, allow, edge cases
  • 3 E2E tests via hook runner
  • All 956 unit tests pass
  • All 207 E2E tests pass
  • Build succeeds

NiveditJain and others added 2 commits April 20, 2026 23:30
Adds a new builtin policy that enforces allowed package managers by
blocking non-preferred ones (e.g., pip when uv is preferred). Users
configure an allowlist via policyParams and any detected manager not
in the list is denied with a message naming the allowed alternatives.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@NiveditJain has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 49 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 49 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bda6c3b9-ac69-4db2-baa5-387f28437473

📥 Commits

Reviewing files that changed from the base of the PR and between a7863ce and 4c59fd0.

📒 Files selected for processing (9)
  • .failproofai/policies/review-policies.mjs
  • CHANGELOG.md
  • README.md
  • __tests__/hooks/builtin-policies.test.ts
  • docs/built-in-policies.mdx
  • docs/custom-policies.mdx
  • docs/examples.mdx
  • docs/getting-started.mdx
  • src/hooks/builtin-policies.ts
📝 Walkthrough

Walkthrough

This pull request introduces a new prefer-package-manager builtin policy that restricts or allows specific package managers in Bash commands. The implementation includes policy logic for detecting and evaluating package managers, comprehensive unit and E2E test coverage, and documentation updates.

Changes

Cohort / File(s) Summary
Implementation
src/hooks/builtin-policies.ts
Implemented preferPackageManager policy with package manager detection via regex patterns. Denies commands using non-allowed managers, allows commands using allowed managers or containing no detectable manager.
Unit Tests
__tests__/hooks/builtin-policies.test.ts
Added comprehensive test suite for prefer-package-manager policy, including deny/allow assertions for multiple managers (pip, npm, uv, bun, cargo), empty allowlist handling, and formatting of denial reasons.
E2E Tests
__tests__/e2e/hooks/policy-params.e2e.test.ts
Added end-to-end test scenarios verifying policy behavior: denying disallowed managers (pip when uv allowed), allowing preferred managers (uv, bun), and treating empty allowlist as no-op.
Documentation & Changelog
docs/built-in-policies.mdx, CHANGELOG.md
Added new policy documentation under "Package managers" category with parameter definition (allowed: string[]), and updated changelog to record feature addition.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With glee, I hop through the code,
A new policy lightens the load!
"Prefer-package-manager" now guides the way,
Package managers kept in their play,
No more mixing npm with bun in dismay,
Claude rewrites commands with brilliant array! ✨🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add prefer-package-manager builtin policy' clearly and specifically summarizes the main change: the addition of a new builtin policy.
Description check ✅ Passed The PR description includes a clear summary, configuration example, and comprehensive test plan with specific numbers of tests and pass results.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch luv-126

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/built-in-policies.mdx (1)

1-7: ⚠️ Potential issue | 🟡 Minor

Update the policy counts at the top of the page.

Line 24 adds the 31st policy, but the frontmatter and intro still say “30 built-in policies”; the parameterized-policy count also needs recomputing because this PR adds another policy with allowed.

Also applies to: 24-24

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/built-in-policies.mdx` around lines 1 - 7, Update the counts in the docs
intro and frontmatter: change any occurrence of "30 built-in policies"
(frontmatter description and the opening paragraph) to "31 built-in policies",
and update the parameterized-policy count from "Nine policies accept parameters"
to "Ten policies accept parameters" because the new policy introduces an
`allowed` parameter; ensure the line mentioning "Four workflow policies" remains
correct and adjust any other numeric summaries to match the new total.
🧹 Nitpick comments (1)
__tests__/hooks/builtin-policies.test.ts (1)

1330-1474: Cover every advertised detector and mixed-command regressions.

The new suite is strong, but it does not exercise several newly supported detectors (yarn, pnpm, pnpx, bunx, pipenv, conda, cargo). Please add table-driven cases for each advertised manager/alias, plus chained-command regressions like uv --version && pip install flask.

As per coding guidelines, **/__tests__/**/*.{ts,tsx,js,jsx}: Always add unit tests for new behaviour.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@__tests__/hooks/builtin-policies.test.ts` around lines 1330 - 1474, The
"prefer-package-manager" test suite (the describe block using policy =
BUILTIN_POLICIES.find(...)) is missing cases for several advertised detectors
and mixed-command regressions; add table-driven tests within that describe:
iterate over managers/aliases
["yarn","pnpm","pnpx","bunx","pipenv","conda","cargo"] (and any existing aliases
like pip3/python -m pip) to assert deny for forbidden manager invocations and
allow when the allowed list includes the manager, and add regression tests for
chained commands such as "uv --version && pip install flask" to ensure the
policy still denies the pip segment when uv is preferred; keep tests following
the existing makeCtx pattern and assertions on policy.fn(ctx).decision and
result.reason where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Line 6: Update the CHANGELOG.md line that reads "Add `prefer-package-manager`
builtin policy to enforce allowed package managers (e.g., uv instead of pip)" by
appending the PR number "(`#126`)" to the end of that single-line entry in the "##
Unreleased" section so it follows the required format; ensure spacing and
punctuation match other entries.

In `@src/hooks/builtin-policies.ts`:
- Around line 141-153: The current manager detection (PKG_MANAGER_DETECTORS) is
applied to the entire command string, which allows bypasses; change the check to
evaluate each shell command segment independently by splitting the input on
common separators (&&, ||, ;, |, newline) and for each segment only consider the
launcher/first token as authoritative; update the detection logic that uses
PKG_MANAGER_DETECTORS so it anchors patterns to the start of the segment (or
match against the first token) and ignores mentions in arguments (so "echo pip"
or "uv --version && pip install" won't falsely authorize), and apply the same
per-segment approach to the corresponding checks referenced around the other
usages noted (lines ~884-898).

---

Outside diff comments:
In `@docs/built-in-policies.mdx`:
- Around line 1-7: Update the counts in the docs intro and frontmatter: change
any occurrence of "30 built-in policies" (frontmatter description and the
opening paragraph) to "31 built-in policies", and update the
parameterized-policy count from "Nine policies accept parameters" to "Ten
policies accept parameters" because the new policy introduces an `allowed`
parameter; ensure the line mentioning "Four workflow policies" remains correct
and adjust any other numeric summaries to match the new total.

---

Nitpick comments:
In `@__tests__/hooks/builtin-policies.test.ts`:
- Around line 1330-1474: The "prefer-package-manager" test suite (the describe
block using policy = BUILTIN_POLICIES.find(...)) is missing cases for several
advertised detectors and mixed-command regressions; add table-driven tests
within that describe: iterate over managers/aliases
["yarn","pnpm","pnpx","bunx","pipenv","conda","cargo"] (and any existing aliases
like pip3/python -m pip) to assert deny for forbidden manager invocations and
allow when the allowed list includes the manager, and add regression tests for
chained commands such as "uv --version && pip install flask" to ensure the
policy still denies the pip segment when uv is preferred; keep tests following
the existing makeCtx pattern and assertions on policy.fn(ctx).decision and
result.reason where appropriate.
🪄 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: 6b1bc649-6936-4ee7-8e5f-c4e199d8f44d

📥 Commits

Reviewing files that changed from the base of the PR and between 0beb11e and a7863ce.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • __tests__/e2e/hooks/policy-params.e2e.test.ts
  • __tests__/hooks/builtin-policies.test.ts
  • docs/built-in-policies.mdx
  • src/hooks/builtin-policies.ts

Comment thread CHANGELOG.md Outdated
Comment thread src/hooks/builtin-policies.ts
NiveditJain and others added 6 commits April 20, 2026 23:37
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Users can now append additional manager names beyond the built-in list
via the blocked param (e.g., pdm, pipx). Entries in blocked are skipped
if they also appear in allowed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update getting-started, custom-policies, examples, and README to
highlight the .failproofai/policies/ convention workflow: commit to
git, share across the team, keep improving as new failure modes are
discovered.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split compound commands on &&, ||, |, ; and check each segment
separately. Fixes bypass where an allowed manager in one segment
(e.g., uv --version) would incorrectly allow a blocked manager in
another segment (e.g., pip install flask).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds review-policies.mjs to .failproofai/policies/ that prevents
stopping if there are unresolved bot review threads (e.g. CodeRabbit)
on the PR. Runs after builtin workflow policies since convention
policies evaluate after builtins.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@NiveditJain NiveditJain merged commit 926fc21 into main Apr 21, 2026
9 checks passed
@NiveditJain NiveditJain deleted the luv-126 branch April 21, 2026 01:30
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.

1 participant