install: skip non-absolute $BUN_INSTALL when locating global dirs - #32515
Closed
robobun wants to merge 8 commits into
Closed
install: skip non-absolute $BUN_INSTALL when locating global dirs#32515robobun wants to merge 8 commits into
robobun wants to merge 8 commits into
Claude / Claude Code Review
completed
Aug 20, 2026 in 17m 12s
Code review found 2 potential issues
Found 4 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/install/PackageManager/PackageManagerOptions.rs:301-311 |
Multi-line explanatory comments flagged by comment-cop |
| 🟣 Pre-existing | src/install/PackageManager/PackageManagerOptions.rs:309-311 |
Missed sibling: bun pm diff ~/... passes unfiltered $HOME to join_abs_string |
Annotations
Check warning on line 311 in src/install/PackageManager/PackageManagerOptions.rs
claude / Claude Code Review
Multi-line explanatory comments flagged by comment-cop
The 7-line comment above `get_abs` (PackageManagerOptions.rs:301-307) and the two added comment lines at PackageManager.rs:1930-1931 both trip the repo's comment-cop check (unresolved github-actions flags at both lines) and violate REVIEW.md's "Only comment what the code cannot say. One line." rule. The rationale (join_abs_string_buf's absolute-base precondition, `~/.bun`-on-Windows example, cwd-change caveat) is already captured in the PR description and the Sentry-link comment in the test file
Check notice on line 311 in src/install/PackageManager/PackageManagerOptions.rs
claude / Claude Code Review
Missed sibling: bun pm diff ~/... passes unfiltered $HOME to join_abs_string
(pre-existing, missed sibling) `src/runtime/cli/pm_diff_command.rs:92-94` passes raw `env_var::HOME.get()` as the base to `join_abs_string::<platform::Auto>(home, &[rest])` when expanding a `~/`-prefixed `bun pm diff` argument — the same env-var-as-`join_abs_string_buf`-base pattern this PR fixes at four other sites (and the criterion the author used to include `get_home_config_path` and exclude `fetch_cache_directory_path`). Adding `.filter(|p| bun_paths::is_absolute(p))` makes a non-absolute
Loading