feat: add PackageManagerTool for dependency management#1196
Conversation
New tool to manage project dependencies across multiple package managers: - npm, pip, go, cargo, bun, brew - Actions: install, update, remove, list, audit, outdated - Auto-detects package manager from lockfiles - Input validation per manager for security - Permission prompts for destructive actions - Timeout handling (120s)
jatmn
left a comment
There was a problem hiding this comment.
Findings
-
[P1] Register the new tool with the runtime tool list
src/tools.ts:182
The PR addsPackageManagerTool, but it is never imported intosrc/tools.tsor appended togetAllBaseTools().getTools()is built from that list, and the only references I can find are inside the new tool/test/prompt files, so users will never see or be able to call this tool despite the PR's stated user-facing impact. Please wire the tool into the same registry path as the other built-ins and add a regression test thatgetToolsForDefaultPreset()orgetTools()includesPackageManager. -
[P2] Show the exact manager/path/command in permission prompts
src/tools/PackageManagerTool/PackageManagerTool.ts:140
The permission prompt only saysRun "install" on lodash?, but this tool can run different binaries, mutate different working directories viapath, and change behavior with flags such asglobal,dev, ordryRun. For dependency management, the user needs to approve the actual command context, not just the high-level action/package list. Please build and display the resolved manager, target path, and arguments in the permission message before executing. -
[P2] Recommit the new source files as UTF-8 text
src/tools/PackageManagerTool/PackageManagerTool.ts:1
The three new TypeScript files are committed as UTF-16 LE, so GitHub andgit diff --numstatclassify them as binary (Binary files /dev/null and ... differ). That prevents normal inline review, hides future diffs/blame from common text tooling, and evengit grepskips these files. Please convertPackageManagerTool.ts,PackageManagerTool.test.ts, andprompt.tsto the repo's normal UTF-8 encoding before merge.
|
Thanks for this — a unified package-manager tool across npm/pip/cargo/etc. is a useful idea, and using Reviewed independently alongside @jatmn's review, and his three points are correct: the tool isn't registered in If you can address those and re-push, I'd be happy to take another pass — the underlying approach is sound and close. |
Summary
Impact
Testing
Notes